diff mbox series

soc: xilinx: Use of_property_present() for testing DT property presence

Message ID 20230310144725.1545315-1-robh@kernel.org
State New
Headers
Series soc: xilinx: Use of_property_present() for testing DT property presence |

Commit Message

Rob Herring March 10, 2023, 2:47 p.m. UTC 
  It is preferred to use typed property access functions (i.e.
of_property_read_<type> functions) rather than low-level
of_get_property/of_find_property functions for reading properties. As
part of this, convert of_get_property/of_find_property calls to the
recently added of_property_present() helper when we just want to test
for presence of a property and nothing more.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/soc/xilinx/zynqmp_power.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

kernel test robot March 11, 2023, 4:15 p.m. UTC | #1 
Hi Rob,

I love your patch! Yet something to improve:

[auto build test ERROR on xilinx-xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rob-Herring/soc-xilinx-Use-of_property_present-for-testing-DT-property-presence/20230310-225437
base:   https://github.com/Xilinx/linux-xlnx master
patch link:    https://lore.kernel.org/r/20230310144725.1545315-1-robh%40kernel.org
patch subject: [PATCH] soc: xilinx: Use of_property_present() for testing DT property presence
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20230312/202303120017.BIw01Y21-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7d21b118deba11d338baf0365e962a8889f3ac68
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Rob-Herring/soc-xilinx-Use-of_property_present-for-testing-DT-property-presence/20230310-225437
        git checkout 7d21b118deba11d338baf0365e962a8889f3ac68
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/soc/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303120017.BIw01Y21-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/soc/xilinx/zynqmp_power.c: In function 'zynqmp_pm_probe':
>> drivers/soc/xilinx/zynqmp_power.c:221:20: error: implicit declaration of function 'of_property_present'; did you mean 'fwnode_property_present'? [-Werror=implicit-function-declaration]
     221 |         } else if (of_property_present(pdev->dev.of_node, "mboxes")) {
         |                    ^~~~~~~~~~~~~~~~~~~
         |                    fwnode_property_present
   cc1: some warnings being treated as errors


vim +221 drivers/soc/xilinx/zynqmp_power.c

   183	
   184	static int zynqmp_pm_probe(struct platform_device *pdev)
   185	{
   186		int ret, irq;
   187		u32 pm_api_version;
   188		struct mbox_client *client;
   189	
   190		zynqmp_pm_get_api_version(&pm_api_version);
   191	
   192		/* Check PM API version number */
   193		if (pm_api_version < ZYNQMP_PM_VERSION)
   194			return -ENODEV;
   195	
   196		/*
   197		 * First try to use Xilinx Event Manager by registering suspend_event_callback
   198		 * for suspend/shutdown event.
   199		 * If xlnx_register_event() returns -EACCES (Xilinx Event Manager
   200		 * is not available to use) or -ENODEV(Xilinx Event Manager not compiled),
   201		 * then use ipi-mailbox or interrupt method.
   202		 */
   203		ret = xlnx_register_event(PM_INIT_SUSPEND_CB, 0, 0, false,
   204					  suspend_event_callback, NULL);
   205		if (!ret) {
   206			zynqmp_pm_init_suspend_work = devm_kzalloc(&pdev->dev,
   207								   sizeof(struct zynqmp_pm_work_struct),
   208								   GFP_KERNEL);
   209			if (!zynqmp_pm_init_suspend_work) {
   210				xlnx_unregister_event(PM_INIT_SUSPEND_CB, 0, 0,
   211						      suspend_event_callback, NULL);
   212				return -ENOMEM;
   213			}
   214			event_registered = true;
   215	
   216			INIT_WORK(&zynqmp_pm_init_suspend_work->callback_work,
   217				  zynqmp_pm_init_suspend_work_fn);
   218		} else if (ret != -EACCES && ret != -ENODEV) {
   219			dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
   220			return ret;
 > 221		} else if (of_property_present(pdev->dev.of_node, "mboxes")) {
   222			zynqmp_pm_init_suspend_work =
   223				devm_kzalloc(&pdev->dev,
   224					     sizeof(struct zynqmp_pm_work_struct),
   225					     GFP_KERNEL);
   226			if (!zynqmp_pm_init_suspend_work)
   227				return -ENOMEM;
   228	
   229			INIT_WORK(&zynqmp_pm_init_suspend_work->callback_work,
   230				  zynqmp_pm_init_suspend_work_fn);
   231			client = devm_kzalloc(&pdev->dev, sizeof(*client), GFP_KERNEL);
   232			if (!client)
   233				return -ENOMEM;
   234	
   235			client->dev = &pdev->dev;
   236			client->rx_callback = ipi_receive_callback;
   237	
   238			rx_chan = mbox_request_channel_byname(client, "rx");
   239			if (IS_ERR(rx_chan)) {
   240				dev_err(&pdev->dev, "Failed to request rx channel\n");
   241				return PTR_ERR(rx_chan);
   242			}
   243		} else if (of_property_present(pdev->dev.of_node, "interrupts")) {
   244			irq = platform_get_irq(pdev, 0);
   245			if (irq <= 0)
   246				return -ENXIO;
   247	
   248			ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
   249							zynqmp_pm_isr,
   250							IRQF_NO_SUSPEND | IRQF_ONESHOT,
   251							dev_name(&pdev->dev),
   252							&pdev->dev);
   253			if (ret) {
   254				dev_err(&pdev->dev, "devm_request_threaded_irq '%d' failed with %d\n",
   255					irq, ret);
   256				return ret;
   257			}
   258		} else {
   259			dev_err(&pdev->dev, "Required property not found in DT node\n");
   260			return -ENOENT;
   261		}
   262	
   263		ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_suspend_mode.attr);
   264		if (ret) {
   265			if (event_registered) {
   266				xlnx_unregister_event(PM_INIT_SUSPEND_CB, 0, 0, suspend_event_callback,
   267						      NULL);
   268				event_registered = false;
   269			}
   270			dev_err(&pdev->dev, "unable to create sysfs interface\n");
   271			return ret;
   272		}
   273	
   274		return 0;
   275	}
   276
Michal Simek March 13, 2023, 10:18 a.m. UTC | #2 
On 3/10/23 15:47, Rob Herring wrote:
> It is preferred to use typed property access functions (i.e.
> of_property_read_<type> functions) rather than low-level
> of_get_property/of_find_property functions for reading properties. As
> part of this, convert of_get_property/of_find_property calls to the
> recently added of_property_present() helper when we just want to test
> for presence of a property and nothing more.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>   drivers/soc/xilinx/zynqmp_power.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
> index 78a8a7545d1e..641dcc958911 100644
> --- a/drivers/soc/xilinx/zynqmp_power.c
> +++ b/drivers/soc/xilinx/zynqmp_power.c
> @@ -218,7 +218,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>          } else if (ret != -EACCES && ret != -ENODEV) {
>                  dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
>                  return ret;
> -       } else if (of_find_property(pdev->dev.of_node, "mboxes", NULL)) {
> +       } else if (of_property_present(pdev->dev.of_node, "mboxes")) {
>                  zynqmp_pm_init_suspend_work =
>                          devm_kzalloc(&pdev->dev,
>                                       sizeof(struct zynqmp_pm_work_struct),
> @@ -240,7 +240,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>                          dev_err(&pdev->dev, "Failed to request rx channel\n");
>                          return PTR_ERR(rx_chan);
>                  }
> -       } else if (of_find_property(pdev->dev.of_node, "interrupts", NULL)) {
> +       } else if (of_property_present(pdev->dev.of_node, "interrupts")) {
>                  irq = platform_get_irq(pdev, 0);
>                  if (irq <= 0)
>                          return -ENXIO;
> --
> 2.39.2
> 

Waiting for v2 because of missing of.h header reported by lkp.
https://lore.kernel.org/all/202303120017.BIw01Y21-lkp@intel.com/

M
Rob Herring March 14, 2023, 1:23 p.m. UTC | #3 
On Mon, Mar 13, 2023 at 5:19 AM Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 3/10/23 15:47, Rob Herring wrote:
> > It is preferred to use typed property access functions (i.e.
> > of_property_read_<type> functions) rather than low-level
> > of_get_property/of_find_property functions for reading properties. As
> > part of this, convert of_get_property/of_find_property calls to the
> > recently added of_property_present() helper when we just want to test
> > for presence of a property and nothing more.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >   drivers/soc/xilinx/zynqmp_power.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
> > index 78a8a7545d1e..641dcc958911 100644
> > --- a/drivers/soc/xilinx/zynqmp_power.c
> > +++ b/drivers/soc/xilinx/zynqmp_power.c
> > @@ -218,7 +218,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
> >          } else if (ret != -EACCES && ret != -ENODEV) {
> >                  dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
> >                  return ret;
> > -       } else if (of_find_property(pdev->dev.of_node, "mboxes", NULL)) {
> > +       } else if (of_property_present(pdev->dev.of_node, "mboxes")) {
> >                  zynqmp_pm_init_suspend_work =
> >                          devm_kzalloc(&pdev->dev,
> >                                       sizeof(struct zynqmp_pm_work_struct),
> > @@ -240,7 +240,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
> >                          dev_err(&pdev->dev, "Failed to request rx channel\n");
> >                          return PTR_ERR(rx_chan);
> >                  }
> > -       } else if (of_find_property(pdev->dev.of_node, "interrupts", NULL)) {
> > +       } else if (of_property_present(pdev->dev.of_node, "interrupts")) {
> >                  irq = platform_get_irq(pdev, 0);
> >                  if (irq <= 0)
> >                          return -ENXIO;
> > --
> > 2.39.2
> >
>
> Waiting for v2 because of missing of.h header reported by lkp.
> https://lore.kernel.org/all/202303120017.BIw01Y21-lkp@intel.com/

It's a false positive. The header change is in v6.3-rc1, but 0-day is
applying these to branches not yet updated to rc1.

Rob
Michal Simek March 28, 2023, 7:37 a.m. UTC | #4 
On 3/14/23 14:23, Rob Herring wrote:
> On Mon, Mar 13, 2023 at 5:19 AM Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 3/10/23 15:47, Rob Herring wrote:
>>> It is preferred to use typed property access functions (i.e.
>>> of_property_read_<type> functions) rather than low-level
>>> of_get_property/of_find_property functions for reading properties. As
>>> part of this, convert of_get_property/of_find_property calls to the
>>> recently added of_property_present() helper when we just want to test
>>> for presence of a property and nothing more.
>>>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> ---
>>>    drivers/soc/xilinx/zynqmp_power.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
>>> index 78a8a7545d1e..641dcc958911 100644
>>> --- a/drivers/soc/xilinx/zynqmp_power.c
>>> +++ b/drivers/soc/xilinx/zynqmp_power.c
>>> @@ -218,7 +218,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>>>           } else if (ret != -EACCES && ret != -ENODEV) {
>>>                   dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
>>>                   return ret;
>>> -       } else if (of_find_property(pdev->dev.of_node, "mboxes", NULL)) {
>>> +       } else if (of_property_present(pdev->dev.of_node, "mboxes")) {
>>>                   zynqmp_pm_init_suspend_work =
>>>                           devm_kzalloc(&pdev->dev,
>>>                                        sizeof(struct zynqmp_pm_work_struct),
>>> @@ -240,7 +240,7 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>>>                           dev_err(&pdev->dev, "Failed to request rx channel\n");
>>>                           return PTR_ERR(rx_chan);
>>>                   }
>>> -       } else if (of_find_property(pdev->dev.of_node, "interrupts", NULL)) {
>>> +       } else if (of_property_present(pdev->dev.of_node, "interrupts")) {
>>>                   irq = platform_get_irq(pdev, 0);
>>>                   if (irq <= 0)
>>>                           return -ENXIO;
>>> --
>>> 2.39.2
>>>
>>
>> Waiting for v2 because of missing of.h header reported by lkp.
>> https://lore.kernel.org/all/202303120017.BIw01Y21-lkp@intel.com/
> 
> It's a false positive. The header change is in v6.3-rc1, but 0-day is
> applying these to branches not yet updated to rc1.

ok. applied.

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
index 78a8a7545d1e..641dcc958911 100644
--- a/drivers/soc/xilinx/zynqmp_power.c
+++ b/drivers/soc/xilinx/zynqmp_power.c
@@ -218,7 +218,7 @@  static int zynqmp_pm_probe(struct platform_device *pdev)
 	} else if (ret != -EACCES && ret != -ENODEV) {
 		dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
 		return ret;
-	} else if (of_find_property(pdev->dev.of_node, "mboxes", NULL)) {
+	} else if (of_property_present(pdev->dev.of_node, "mboxes")) {
 		zynqmp_pm_init_suspend_work =
 			devm_kzalloc(&pdev->dev,
 				     sizeof(struct zynqmp_pm_work_struct),
@@ -240,7 +240,7 @@  static int zynqmp_pm_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Failed to request rx channel\n");
 			return PTR_ERR(rx_chan);
 		}
-	} else if (of_find_property(pdev->dev.of_node, "interrupts", NULL)) {
+	} else if (of_property_present(pdev->dev.of_node, "interrupts")) {
 		irq = platform_get_irq(pdev, 0);
 		if (irq <= 0)
 			return -ENXIO;