| Message ID | 20250122-socfpga_sip_svc_misc-v1-3-cbdcd034ae34@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | stratix10: miscellaneous changes and fix for child driver probing | expand |
On 22/01/2025 06:58, Mahesh Rao wrote: > Add of_platform_default_populate() to stratix10-svc > driver as the firmware/svc node was moved out of soc. > This fixes the failed probing of child drivers of > svc node. > > Fixes: 23c3ebed382a ("arm64: dts: socfpga: agilex: move firmware out of soc node") > There is never a blank line between tags. Use: `git log` > Signed-off-by: Mahesh Rao <mahesh.rao@intel.com> > --- > drivers/firmware/stratix10-svc.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c > index c5c78b869561b0c1e9602823ad1f501e98e3ce51..15a7207f7753dcd4e94da4aa9a6162fedb577fe9 100644 > --- a/drivers/firmware/stratix10-svc.c > +++ b/drivers/firmware/stratix10-svc.c > @@ -1227,13 +1227,19 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > if (!svc->intel_svc_fcs) { > dev_err(dev, "failed to allocate %s device\n", INTEL_FCS); > ret = -ENOMEM; > - goto err_unregister_dev; > + goto err_unregister_rsu_dev; > } > > ret = platform_device_add(svc->intel_svc_fcs); > if (ret) { > platform_device_put(svc->intel_svc_fcs); > - goto err_unregister_dev; > + goto err_unregister_rsu_dev; > + } > + > + ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); > + if (ret < 0) { > + of_platform_depopulate(dev); > + goto err_unregister_fcs_dev; > } > > dev_set_drvdata(dev, svc); > @@ -1242,7 +1248,9 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > > return 0; > > -err_unregister_dev: > +err_unregister_fcs_dev: > + platform_device_unregister(svc->intel_svc_fcs); > +err_unregister_rsu_dev: > platform_device_unregister(svc->stratix10_svc_rsu); > err_free_kfifo: > kfifo_free(&controller->svc_fifo); You need to update remove() callback. Best regards, Krzysztof
On Wed, Jan 22, 2025 at 01:58:45PM +0800, Mahesh Rao wrote: > Add of_platform_default_populate() to stratix10-svc > driver as the firmware/svc node was moved out of soc. > This fixes the failed probing of child drivers of > svc node. > > Fixes: 23c3ebed382a ("arm64: dts: socfpga: agilex: move firmware out of soc node") > > Signed-off-by: Mahesh Rao <mahesh.rao@intel.com> > --- > drivers/firmware/stratix10-svc.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c > index c5c78b869561b0c1e9602823ad1f501e98e3ce51..15a7207f7753dcd4e94da4aa9a6162fedb577fe9 100644 > --- a/drivers/firmware/stratix10-svc.c > +++ b/drivers/firmware/stratix10-svc.c > @@ -1227,13 +1227,19 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > if (!svc->intel_svc_fcs) { > dev_err(dev, "failed to allocate %s device\n", INTEL_FCS); > ret = -ENOMEM; > - goto err_unregister_dev; > + goto err_unregister_rsu_dev; > } > > ret = platform_device_add(svc->intel_svc_fcs); > if (ret) { > platform_device_put(svc->intel_svc_fcs); > - goto err_unregister_dev; > + goto err_unregister_rsu_dev; > + } > + > + ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); > + if (ret < 0) { if (ret) is just fine. > + of_platform_depopulate(dev); > + goto err_unregister_fcs_dev; You wanna destroy everything even if some child drivers work? And do we need to do depopulation on driver remove? I'm actually a little confused how to handle populate() fail and depopulate(). Thanks, Yilun > } > > dev_set_drvdata(dev, svc); > @@ -1242,7 +1248,9 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > > return 0; > > -err_unregister_dev: > +err_unregister_fcs_dev: > + platform_device_unregister(svc->intel_svc_fcs); > +err_unregister_rsu_dev: > platform_device_unregister(svc->stratix10_svc_rsu); > err_free_kfifo: > kfifo_free(&controller->svc_fifo); > > -- > 2.35.3 > >
Hi Krzysztof, Thanks for Reviewing the patch. On Thu, 23 Jan 2025 08:26:08 +0100, Krzysztof Kozlowski wrote: > > Add of_platform_default_populate() to stratix10-svc driver as the > > firmware/svc node was moved out of soc. > > This fixes the failed probing of child drivers of svc node. > > > > Fixes: 23c3ebed382a ("arm64: dts: socfpga: agilex: move firmware out > > of soc node") > > > > > There is never a blank line between tags. Use: `git log` > > > Signed-off-by: Mahesh Rao <mahesh.rao@intel.com> Understood, I will revise this in the next revision. > > > --- > > drivers/firmware/stratix10-svc.c | 14 +++++++++++--- > > 1 file changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/firmware/stratix10-svc.c > > b/drivers/firmware/stratix10-svc.c > > index > > platform_device_unregister(svc->stratix10_svc_rsu); > > err_free_kfifo: > > kfifo_free(&controller->svc_fifo); > > > You need to update remove() callback. > Sure, I will add the changes in the next revision. Best regards, Mahesh Rao
Hi Yilun, Thanks for reviewing the patch. On Sun, 26 Jan 2025 16:28:55 +0800, Xu Yilun wrote: > > Add of_platform_default_populate() to stratix10-svc driver as the > > firmware/svc node was moved out of soc. > > This fixes the failed probing of child drivers of svc node. > > > > Fixes: 23c3ebed382a ("arm64: dts: socfpga: agilex: move firmware out > > of soc node") > > + ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); > > + if (ret < 0) { > > if (ret) is just fine. ok ,I will make the change. > > > + of_platform_depopulate(dev); > > + goto err_unregister_fcs_dev; > > You wanna destroy everything even if some child drivers work? Currently, there is no requirement to retain the driver if a child component fails. we will handle it if it is needed in the future. > And do we need to do depopulation on driver remove? I think yes , I have missed this. I will add depopulate in the remove callback(). > I'm actually a little confused how to handle populate() fail and depopulate(). I think this was a mistake on my side. I will make the change in next revision. Best Regards, Mahesh Rao
diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c index c5c78b869561b0c1e9602823ad1f501e98e3ce51..15a7207f7753dcd4e94da4aa9a6162fedb577fe9 100644 --- a/drivers/firmware/stratix10-svc.c +++ b/drivers/firmware/stratix10-svc.c @@ -1227,13 +1227,19 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) if (!svc->intel_svc_fcs) { dev_err(dev, "failed to allocate %s device\n", INTEL_FCS); ret = -ENOMEM; - goto err_unregister_dev; + goto err_unregister_rsu_dev; } ret = platform_device_add(svc->intel_svc_fcs); if (ret) { platform_device_put(svc->intel_svc_fcs); - goto err_unregister_dev; + goto err_unregister_rsu_dev; + } + + ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); + if (ret < 0) { + of_platform_depopulate(dev); + goto err_unregister_fcs_dev; } dev_set_drvdata(dev, svc); @@ -1242,7 +1248,9 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) return 0; -err_unregister_dev: +err_unregister_fcs_dev: + platform_device_unregister(svc->intel_svc_fcs); +err_unregister_rsu_dev: platform_device_unregister(svc->stratix10_svc_rsu); err_free_kfifo: kfifo_free(&controller->svc_fifo);
Add of_platform_default_populate() to stratix10-svc driver as the firmware/svc node was moved out of soc. This fixes the failed probing of child drivers of svc node. Fixes: 23c3ebed382a ("arm64: dts: socfpga: agilex: move firmware out of soc node") Signed-off-by: Mahesh Rao <mahesh.rao@intel.com> --- drivers/firmware/stratix10-svc.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)