[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0hUELtKc9CK=z47XQvSAAx=wTWvoVwP-PaMqugaXaCgZQ@mail.gmail.com>
Date: Tue, 19 Jan 2021 14:19:16 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Daniel Scally <djrscally@...il.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
linux-gpio@...r.kernel.org, linux-i2c <linux-i2c@...r.kernel.org>,
Platform Driver <platform-driver-x86@...r.kernel.org>,
"open list:ACPI COMPONENT ARCHITECTURE (ACPICA)" <devel@...ica.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <lenb@...nel.org>, andy@...nel.org,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
Wolfram Sang <wsa@...nel.org>,
Lee Jones <lee.jones@...aro.org>,
Hans de Goede <hdegoede@...hat.com>,
Mark Gross <mgross@...ux.intel.com>,
Robert Moore <robert.moore@...el.com>,
Erik Kaneda <erik.kaneda@...el.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Kieran Bingham <kieran.bingham@...asonboard.com>
Subject: Re: [PATCH v2 4/7] i2c: i2c-core-acpi: Add i2c_acpi_dev_name()
On Mon, Jan 18, 2021 at 9:55 PM Laurent Pinchart
<laurent.pinchart@...asonboard.com> wrote:
>
> Hi Daniel,
>
> Thank you for the patch.
>
> On Mon, Jan 18, 2021 at 12:34:25AM +0000, Daniel Scally wrote:
> > We want to refer to an i2c device by name before it has been
>
> s/i2c device/acpi i2c device/ ?
>
> > created by the kernel; add a function that constructs the name
> > from the acpi device instead.
> >
> > Signed-off-by: Daniel Scally <djrscally@...il.com>
> > ---
> > Changes in v2:
> >
> > - Stopped using devm_kasprintf()
> >
> > drivers/i2c/i2c-core-acpi.c | 16 ++++++++++++++++
> > include/linux/i2c.h | 5 +++++
> > 2 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> > index 37c510d9347a..98c3ba9a2350 100644
> > --- a/drivers/i2c/i2c-core-acpi.c
> > +++ b/drivers/i2c/i2c-core-acpi.c
> > @@ -497,6 +497,22 @@ struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
> > }
> > EXPORT_SYMBOL_GPL(i2c_acpi_new_device);
> >
> > +/**
> > + * i2c_acpi_dev_name - Construct i2c device name for devs sourced from ACPI
> > + * @adev: ACPI device to construct the name for
> > + *
> > + * Constructs the name of an i2c device matching the format used by
> > + * i2c_dev_set_name() to allow users to refer to an i2c device by name even
> > + * before they have been instantiated.
> > + *
> > + * The caller is responsible for freeing the returned pointer.
> > + */
> > +char *i2c_acpi_dev_name(struct acpi_device *adev)
> > +{
> > + return kasprintf(GFP_KERNEL, I2C_DEV_NAME_FORMAT, acpi_dev_name(adev));
>
> There's a real danger of a memory leak, as the function name sounds very
> similar to dev_name() or acpi_dev_name() and those don't allocate
> memory. I'm not sure what a better name would be, but given that this
> function is only used in patch 6/7 and not in the I2C subsystem itself,
> I wonder if we should inline this kasprintf() call in the caller and
> drop this patch.
IMO if this is a one-off usage, it's better to open-code it.
Powered by blists - more mailing lists