[16/18] i2c: i2c-core-base: Use the new i2c_acpi_dev_name() in i2c_set_dev_name()

Message ID 20201130133129.1024662-17-djrscally@gmail.com
State New
Headers show
Series
  • Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows
Related show

Commit Message

Daniel Scally Nov. 30, 2020, 1:31 p.m.
From: Dan Scally <djrscally@gmail.com>

To make sure the new i2c_acpi_dev_name() always reflects the name of i2c
devices sourced from ACPI, use it in i2c_set_dev_name().

Signed-off-by: Dan Scally <djrscally@gmail.com>
---
Changes since RFC v3:

	- Patch introduced

 drivers/i2c/i2c-core-base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Scally Dec. 1, 2020, 11:50 p.m. | #1
On 30/11/2020 17:12, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Mon, Nov 30, 2020 at 01:31:27PM +0000, Daniel Scally wrote:
>> From: Dan Scally <djrscally@gmail.com>
>>
>> To make sure the new i2c_acpi_dev_name() always reflects the name of i2c
>> devices sourced from ACPI, use it in i2c_set_dev_name().
>>
>> Signed-off-by: Dan Scally <djrscally@gmail.com>
> 
> I'd squash this with 15/18, which would make it clear there's a memory
> leak :-)

Ah - that was sloppy...switched from devm_ and forgot to go fix that.
I'll add the kfree into i2c_unregister_device() and squash to 15/18

>> ---
>> Changes since RFC v3:
>>
>> 	- Patch introduced
>>
>>  drivers/i2c/i2c-core-base.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
>> index 573b5da145d1..a6d4ceb01077 100644
>> --- a/drivers/i2c/i2c-core-base.c
>> +++ b/drivers/i2c/i2c-core-base.c
>> @@ -814,7 +814,7 @@ static void i2c_dev_set_name(struct i2c_adapter *adap,
>>  	}
>>  
>>  	if (adev) {
>> -		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
>> +		dev_set_name(&client->dev, i2c_acpi_dev_name(adev));
>>  		return;
>>  	}
>>  
>
Andy Shevchenko Dec. 2, 2020, 9:35 a.m. | #2
Dan, does this mail among other my replies reach you?
It seems you answered to Laurent's mails and leaving mine ignored. Just
wondering if our servers have an issue again...

On Mon, Nov 30, 2020 at 09:18:56PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 30, 2020 at 07:12:41PM +0200, Laurent Pinchart wrote:
> > On Mon, Nov 30, 2020 at 01:31:27PM +0000, Daniel Scally wrote:
> > > From: Dan Scally <djrscally@gmail.com>
> > > 
> > > To make sure the new i2c_acpi_dev_name() always reflects the name of i2c
> > > devices sourced from ACPI, use it in i2c_set_dev_name().
> > > 
> > > Signed-off-by: Dan Scally <djrscally@gmail.com>
> > 
> > I'd squash this with 15/18, which would make it clear there's a memory
> > leak :-)
> 
> ...
> 
> > >  	if (adev) {
> > > -		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
> > > +		dev_set_name(&client->dev, i2c_acpi_dev_name(adev));
> > >  		return;
> 
> But you split pattern used in i2c_dev_set_name().
> What you need is to provide something like this
> 
> #define I2C_DEV_NAME_FORMAT	"i2c-%s"
> 
> const char *i2c_acpi_get_dev_name(...)
> {
> 	return kasprintf(..., I2C_DEV_NAME_FORMAT, ...);
> }
> 
> (Possible in the future if anybody needs
>   const char *i2c_dev_get_name_by_bus_and_addr(int bus, unsigned short addr)
> )
> 
> And here
> 
> -		dev_set_name(&client->dev, "i2c-%s", info->dev_name);
> +		dev_set_name(&client->dev, I2C_DEV_NAME_FORMAT, info->dev_name);
> 
> -		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
> +		dev_set_name(&client->dev, I2C_DEV_NAME_FORMAT, acpi_dev_name(adev));
> 
> -- 
> With Best Regards,
> Andy Shevchenko;
> 
>
Daniel Scally Dec. 2, 2020, 9:49 a.m. | #3
On 02/12/2020 09:35, Andy Shevchenko wrote:
> Dan, does this mail among other my replies reach you?
> It seems you answered to Laurent's mails and leaving mine ignored. Just
> wondering if our servers have an issue again...
Morning - I got it, sorry. I just read Laurent's first and then called
it a night
> On Mon, Nov 30, 2020 at 09:18:56PM +0200, Andy Shevchenko wrote:
>> On Mon, Nov 30, 2020 at 07:12:41PM +0200, Laurent Pinchart wrote:
>>> On Mon, Nov 30, 2020 at 01:31:27PM +0000, Daniel Scally wrote:
>>>> From: Dan Scally <djrscally@gmail.com>
>>>>
>>>> To make sure the new i2c_acpi_dev_name() always reflects the name of i2c
>>>> devices sourced from ACPI, use it in i2c_set_dev_name().
>>>>
>>>> Signed-off-by: Dan Scally <djrscally@gmail.com>
>>> I'd squash this with 15/18, which would make it clear there's a memory
>>> leak :-)
>> ...
>>
>>>>  	if (adev) {
>>>> -		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
>>>> +		dev_set_name(&client->dev, i2c_acpi_dev_name(adev));
>>>>  		return;
>> But you split pattern used in i2c_dev_set_name().
>> What you need is to provide something like this
>>
>> #define I2C_DEV_NAME_FORMAT	"i2c-%s"
>>
>> const char *i2c_acpi_get_dev_name(...)
>> {
>> 	return kasprintf(..., I2C_DEV_NAME_FORMAT, ...);
>> }
>>
>> (Possible in the future if anybody needs
>>   const char *i2c_dev_get_name_by_bus_and_addr(int bus, unsigned short addr)
>> )
>>
>> And here
>>
>> -		dev_set_name(&client->dev, "i2c-%s", info->dev_name);
>> +		dev_set_name(&client->dev, I2C_DEV_NAME_FORMAT, info->dev_name);
>>
>> -		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
>> +		dev_set_name(&client->dev, I2C_DEV_NAME_FORMAT, acpi_dev_name(adev));
>>
Yeah ok, I like this approach much better, I'll switch to that.

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 573b5da145d1..a6d4ceb01077 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -814,7 +814,7 @@  static void i2c_dev_set_name(struct i2c_adapter *adap,
 	}
 
 	if (adev) {
-		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
+		dev_set_name(&client->dev, i2c_acpi_dev_name(adev));
 		return;
 	}