| 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 | expand |
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 :-) > --- > 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; > } >
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));
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; >> } >> >
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.
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; }