Use devm_of_platform_populate() to populate subs in the tree.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
drivers/media/platform/qcom/camss/camss.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 00b87fd9afbd8..66ea057291f6d 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -16,6 +16,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_graph.h>
+#include <linux/of_platform.h>
#include <linux/pm_runtime.h>
#include <linux/pm_domain.h>
#include <linux/slab.h>
@@ -4964,6 +4965,8 @@ static int camss_probe(struct platform_device *pdev)
if (!camss)
return -ENOMEM;
+ devm_of_platform_populate(dev);
+
camss->res = of_device_get_match_data(dev);
atomic_set(&camss->ref_count, 0);
--
2.52.0On Thu, Mar 26, 2026 at 2:28 AM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > > Use devm_of_platform_populate() to populate subs in the tree. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Other drivers typically call the populate function at the end of the probe function. In this case, however, it is invoked before the main resources are enabled. I assume this is because the CSIPHY device needs to be available early. Aside from that, it looks good to me. Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com> > > --- > drivers/media/platform/qcom/camss/camss.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index 00b87fd9afbd8..66ea057291f6d 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -16,6 +16,7 @@ > #include <linux/of.h> > #include <linux/of_device.h> > #include <linux/of_graph.h> > +#include <linux/of_platform.h> > #include <linux/pm_runtime.h> > #include <linux/pm_domain.h> > #include <linux/slab.h> > @@ -4964,6 +4965,8 @@ static int camss_probe(struct platform_device *pdev) > if (!camss) > return -ENOMEM; > > + devm_of_platform_populate(dev); > + > camss->res = of_device_get_match_data(dev); > > atomic_set(&camss->ref_count, 0); > > -- > 2.52.0 > >
On Fri, Mar 27, 2026 at 10:22:04PM +0100, Loic Poulain wrote: > On Thu, Mar 26, 2026 at 2:28 AM Bryan O'Donoghue > <bryan.odonoghue@linaro.org> wrote: > > > > Use devm_of_platform_populate() to populate subs in the tree. > > > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > Other drivers typically call the populate function at the end of the > probe function. In this case, however, it is invoked before the main > resources are enabled. I assume this is because the CSIPHY device > needs to be available early. Aside from that, it looks good to me. This becomes fragile. The CSI PHY might be built as a module, which might be loaded later. > > Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com> > > > @@ -4964,6 +4965,8 @@ static int camss_probe(struct platform_device *pdev) > > if (!camss) > > return -ENOMEM; > > > > + devm_of_platform_populate(dev); > > + > > camss->res = of_device_get_match_data(dev); > > > > atomic_set(&camss->ref_count, 0); And this looks suspicious. What if drivers for submodules are already there and start probing once populated? Do they have a chance to access this ref_count? -- With best wishes Dmitry
On 27/03/2026 21:33, Dmitry Baryshkov wrote: >> Other drivers typically call the populate function at the end of the >> probe function. In this case, however, it is invoked before the main >> resources are enabled. I assume this is because the CSIPHY device >> needs to be available early. Aside from that, it looks good to me. > This becomes fragile. The CSI PHY might be built as a module, which > might be loaded later. Is it any more or less fragile than "simple-mfd" in a DT though ? Krzysztof isn't very much in favour of simple-mfd so this method of population is the alternative to hand. The CSIPHY driver uses devm_of_phy_get which handles deferred probe. If the PHY module isn't loaded yet when CAMSS tries to get it, CAMSS gets -EPROBE_DEFER and retries. > >> Reviewed-by: Loic Poulain<loic.poulain@oss.qualcomm.com> >> >>> @@ -4964,6 +4965,8 @@ static int camss_probe(struct platform_device *pdev) >>> if (!camss) >>> return -ENOMEM; >>> >>> + devm_of_platform_populate(dev); >>> + >>> camss->res = of_device_get_match_data(dev); >>> >>> atomic_set(&camss->ref_count, 0); > And this looks suspicious. What if drivers for submodules are already > there and start probing once populated? Do they have a chance to access > this ref_count? Nope, we don't share the camss pointer or any of the data-structures in the existing upstream driver with sub-modules. --- bod
On Fri, Mar 27, 2026 at 11:37 PM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > > On 27/03/2026 21:33, Dmitry Baryshkov wrote: > >> Other drivers typically call the populate function at the end of the > >> probe function. In this case, however, it is invoked before the main > >> resources are enabled. I assume this is because the CSIPHY device > >> needs to be available early. Aside from that, it looks good to me. > > This becomes fragile. The CSI PHY might be built as a module, which > > might be loaded later. > > Is it any more or less fragile than "simple-mfd" in a DT though ? > Krzysztof isn't very much in favour of simple-mfd so this method of > population is the alternative to hand. > > The CSIPHY driver uses devm_of_phy_get which handles deferred probe. If > the PHY module isn't loaded yet when CAMSS tries to get it, CAMSS gets > -EPROBE_DEFER and retries. What about relying on v4l2_async_nf_register() in the same way as for the sensors? That would allow both the sensors and the CSIPHY to be bound asynchronously when they are ready, assuming the CSIPHY driver registers a V4L2 subdevice... > > > > >> Reviewed-by: Loic Poulain<loic.poulain@oss.qualcomm.com> > >> > >>> @@ -4964,6 +4965,8 @@ static int camss_probe(struct platform_device *pdev) > >>> if (!camss) > >>> return -ENOMEM; > >>> > >>> + devm_of_platform_populate(dev); > >>> + > >>> camss->res = of_device_get_match_data(dev); > >>> > >>> atomic_set(&camss->ref_count, 0); > > And this looks suspicious. What if drivers for submodules are already > > there and start probing once populated? Do they have a chance to access > > this ref_count? > > Nope, we don't share the camss pointer or any of the data-structures in > the existing upstream driver with sub-modules. > > --- > bod
On 27/03/2026 22:48, Loic Poulain wrote: > On Fri, Mar 27, 2026 at 11:37 PM Bryan O'Donoghue > <bryan.odonoghue@linaro.org> wrote: >> On 27/03/2026 21:33, Dmitry Baryshkov wrote: >>>> Other drivers typically call the populate function at the end of the >>>> probe function. In this case, however, it is invoked before the main >>>> resources are enabled. I assume this is because the CSIPHY device >>>> needs to be available early. Aside from that, it looks good to me. >>> This becomes fragile. The CSI PHY might be built as a module, which >>> might be loaded later. >> Is it any more or less fragile than "simple-mfd" in a DT though ? >> Krzysztof isn't very much in favour of simple-mfd so this method of >> population is the alternative to hand. >> >> The CSIPHY driver uses devm_of_phy_get which handles deferred probe. If >> the PHY module isn't loaded yet when CAMSS tries to get it, CAMSS gets >> -EPROBE_DEFER and retries. > What about relying on v4l2_async_nf_register() in the same way as for > the sensors? That would allow both the sensors and the CSIPHY to be > bound asynchronously when they are ready, assuming the CSIPHY driver > registers a V4L2 subdevice... The point of + devm_of_platform_populate(dev); or simple-mfd is to allow all sub-devices to asynchronously probe wrt the existing camss node. OPE, IPE, BPS, ICP, CSIPHY - for the case of CSIPHY CAMSS cares about a phandle but for the others it does not. There's no bug here to solve that devm_of_phy_get() doesn't solve. --- bod
© 2016 - 2026 Red Hat, Inc.