| Message ID | 20230310144725.1545315-1-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: xilinx: Use of_property_present() for testing DT property presence | expand |
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
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
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
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 --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;
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(-)