mbox series

[0/3] Make fwnode_property_get_reference_args accept NULL args

Message ID 20231101090737.1148303-1-sakari.ailus@linux.intel.com
Headers show
Series Make fwnode_property_get_reference_args accept NULL args | expand

Message

Sakari Ailus Nov. 1, 2023, 9:07 a.m. UTC
Hi all,

The of_parse_phandle_with_args() accepts NULL args but
fwnode_property_get_reference_args() does not currently, in its ACPI or
software node implementations. Fix this.

Sakari Ailus (3):
  acpi: property: Let args be NULL in __acpi_node_get_property_reference
  software node: Let args be NULL in software_node_get_reference_args
  device property: fwnode_property_get_reference allows NULL args now

 drivers/acpi/property.c | 15 ++++++++++-----
 drivers/base/property.c |  1 +
 drivers/base/swnode.c   |  3 +++
 3 files changed, 14 insertions(+), 5 deletions(-)

Comments

Andy Shevchenko Nov. 1, 2023, 9:52 a.m. UTC | #1
On Wed, Nov 01, 2023 at 11:07:35AM +0200, Sakari Ailus wrote:
> fwnode_get_property_reference() may not be called with args argument NULL
> on ACPI, OF already supports this. Add the missing NULL checks and
> document this.
> 
> The purpose is to be able to count the references.

...

> + * @args: Location to store the returned reference with optional arguments (may
> + *	  be NULL)

I would wrap it as

 * @args: Location to store the returned reference with optional arguments
 *	  (may be NULL)
Rafael J. Wysocki Nov. 1, 2023, 6:01 p.m. UTC | #2
On Wed, Nov 1, 2023 at 11:11 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> On Wed, Nov 01, 2023 at 11:51:35AM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 01, 2023 at 11:07:34AM +0200, Sakari Ailus wrote:
> > > Hi all,
> > >
> > > The of_parse_phandle_with_args() accepts NULL args but
> > > fwnode_property_get_reference_args() does not currently, in its ACPI or
> > > software node implementations. Fix this.
> >
> > The last sentence assumes Fixes tag(s) which I can't see.
>
> Oops. I realised this is actually changing code added in DisCo for Imaging
> patches. It'd be better to address this before those.

So how exactly does this affect those patches?
Sakari Ailus Nov. 1, 2023, 8:28 p.m. UTC | #3
Hi Rafael,

On Wed, Nov 01, 2023 at 07:01:24PM +0100, Rafael J. Wysocki wrote:
> On Wed, Nov 1, 2023 at 11:11 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > On Wed, Nov 01, 2023 at 11:51:35AM +0200, Andy Shevchenko wrote:
> > > On Wed, Nov 01, 2023 at 11:07:34AM +0200, Sakari Ailus wrote:
> > > > Hi all,
> > > >
> > > > The of_parse_phandle_with_args() accepts NULL args but
> > > > fwnode_property_get_reference_args() does not currently, in its ACPI or
> > > > software node implementations. Fix this.
> > >
> > > The last sentence assumes Fixes tag(s) which I can't see.
> >
> > Oops. I realised this is actually changing code added in DisCo for Imaging
> > patches. It'd be better to address this before those.
> 
> So how exactly does this affect those patches?

Not much --- the patch adding string reference support will move the
affected lines, hence the conflict. It's fairly trivial to resolve though.
I'll send v2.
Andy Shevchenko Nov. 2, 2023, 12:59 p.m. UTC | #4
On Wed, Nov 01, 2023 at 10:05:02AM +0000, Sakari Ailus wrote:
> On Wed, Nov 01, 2023 at 11:51:35AM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 01, 2023 at 11:07:34AM +0200, Sakari Ailus wrote:
> > > Hi all,
> > > 
> > > The of_parse_phandle_with_args() accepts NULL args but
> > > fwnode_property_get_reference_args() does not currently, in its ACPI or
> > > software node implementations. Fix this.
> > 
> > The last sentence assumes Fixes tag(s) which I can't see.
> 
> This obviously hasn't been an issue for the existing users so backporting
> it has little value. From API consistency PoV this does matter though.

Fixes is not always about backporting. But if some code starts using that
and needs to be backported with that in mind...

> I can add a Fixes: tag if you like.

I think some clarity needs to be done, either Fixes tag(s) or changing language
to explain that this is an improvement and not an immediate fix.