[PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Sakari Ailus]

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(-)

-- 
2.39.2

[PATCH 1/3] acpi: property: Let args be NULL in __acpi_node_get_property_reference

[Sakari Ailus]

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.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index d60ee0510311..fa473fc2617b 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -879,7 +879,8 @@ static struct fwnode_handle *acpi_parse_string_ref(const struct fwnode_handle *f
  * @propname: Name of the property
  * @index: Index of the reference to return
  * @num_args: Maximum number of arguments after each reference
- * @args: Location to store the returned reference with optional arguments
+ * @args: Location to store the returned reference with optional arguments (may
+ *	  be NULL)
  *
  * Find property with @name, verifify that it is a package containing at least
  * one object reference and if so, store the ACPI device object pointer to the
@@ -937,8 +938,10 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 		if (!device)
 			return -EINVAL;
 
-		args->fwnode = acpi_fwnode_handle(device);
-		args->nargs = 0;
+		if (args) {
+			args->fwnode = acpi_fwnode_handle(device);
+			args->nargs = 0;
+		}
 
 		return 0;
 	case ACPI_TYPE_STRING:
@@ -949,8 +952,10 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 		if (!ref_fwnode)
 			return -EINVAL;
 
-		args->fwnode = ref_fwnode;
-		args->nargs = 0;
+		if (args) {
+			args->fwnode = ref_fwnode;
+			args->nargs = 0;
+		}
 
 		return 0;
 	case ACPI_TYPE_PACKAGE:
-- 
2.39.2

[PATCH 2/3] software node: Let args be NULL in software_node_get_reference_args

[Sakari Ailus]

fwnode_get_property_reference() may not be called with args argument NULL
and while OF already supports this. Add the missing NULL check.

The purpose is to be able to count the references.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/swnode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 1886995a0b3a..079bd14bdedc 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -541,6 +541,9 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
 	if (nargs > NR_FWNODE_REFERENCE_ARGS)
 		return -EINVAL;
 
+	if (!args)
+		return 0;
+
 	args->fwnode = software_node_get(refnode);
 	args->nargs = nargs;
 
-- 
2.39.2

[PATCH 3/3] device property: fwnode_property_get_reference allows NULL args now

[Sakari Ailus]

All three fwnode_property_get_reference() now allow args argument to be
NULL. Document this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 89a6e0833356..b39d3ae04877 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -508,6 +508,7 @@ EXPORT_SYMBOL_GPL(fwnode_property_match_string);
  * @nargs:	Number of arguments. Ignored if @nargs_prop is non-NULL.
  * @index:	Index of the reference, from zero onwards.
  * @args:	Result structure with reference and integer arguments.
+ *		May be NULL.
  *
  * Obtain a reference based on a named property in an fwnode, with
  * integer arguments.
-- 
2.39.2

Re: [PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] acpi: property: Let args be NULL in __acpi_node_get_property_reference

[Andy Shevchenko]

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)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Sakari Ailus]

Hi Andy,

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.

I can add a Fixes: tag if you like.

-- 
Sakari Ailus

Re: [PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Sakari Ailus]

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.

-- 
Sakari Ailus

Re: [PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Rafael J. Wysocki]

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?

Re: [PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Sakari Ailus]

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.

-- 
Regards,

Sakari Ailus

Re: [PATCH 0/3] Make fwnode_property_get_reference_args accept NULL args

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko