| Message ID | 20250513142353.2572563-6-vladimir.zapolskiy@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | media: qcom: camss: a number of cleanups and fixes | expand |
On 13/05/2025 15:23, Vladimir Zapolskiy wrote: > For sake of simplicity it makes sense to register async notifier > for all type of subdevices, both CAMSS components and sensors. > > The case of sensors not connected to CAMSS is extraordinary and > degenerate, it does not deserve any specific optimization. Degenerate is an odd word to use. > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > --- > drivers/media/platform/qcom/camss/camss.c | 30 ++++++----------------- > 1 file changed, 8 insertions(+), 22 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index 976b70cc6d6a..4e91e4b6ef52 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -3556,7 +3556,6 @@ static int camss_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct camss *camss; > - int num_subdevs; > int ret; > > camss = devm_kzalloc(dev, sizeof(*camss), GFP_KERNEL); > @@ -3627,11 +3626,9 @@ static int camss_probe(struct platform_device *pdev) > > pm_runtime_enable(dev); > > - num_subdevs = camss_of_parse_ports(camss); > - if (num_subdevs < 0) { > - ret = num_subdevs; > + ret = camss_of_parse_ports(camss); > + if (ret < 0) > goto err_v4l2_device_unregister; > - } > > ret = camss_register_entities(camss); > if (ret < 0) > @@ -3647,23 +3644,12 @@ static int camss_probe(struct platform_device *pdev) > goto err_register_subdevs; > } > > - if (num_subdevs) { > - camss->notifier.ops = &camss_subdev_notifier_ops; > - > - ret = v4l2_async_nf_register(&camss->notifier); > - if (ret) { > - dev_err(dev, > - "Failed to register async subdev nodes: %d\n", > - ret); > - goto err_media_device_unregister; > - } > - } else { > - ret = v4l2_device_register_subdev_nodes(&camss->v4l2_dev); > - if (ret < 0) { > - dev_err(dev, "Failed to register subdev nodes: %d\n", > - ret); > - goto err_media_device_unregister; > - } > + camss->notifier.ops = &camss_subdev_notifier_ops; > + ret = v4l2_async_nf_register(&camss->notifier); > + if (ret) { > + dev_err(dev, > + "Failed to register async subdev nodes: %d\n", ret); > + goto err_media_device_unregister; > } I'm a little concerned about changing the current flow. Have you tested this out without sensors attached, the TPG on rb5 for example ? --- bod
Hi Bryan. On 6/13/25 12:11, Bryan O'Donoghue wrote: > On 13/05/2025 15:23, Vladimir Zapolskiy wrote: >> For sake of simplicity it makes sense to register async notifier >> for all type of subdevices, both CAMSS components and sensors. >> >> The case of sensors not connected to CAMSS is extraordinary and >> degenerate, it does not deserve any specific optimization. > > Degenerate is an odd word to use. Well, here the wording "degenerate case" is a direct borrowing from a mathematical term "degenerate case" with no intended change of meaning: https://en.wikipedia.org/wiki/Degeneracy_(mathematics) >> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >> --- >> drivers/media/platform/qcom/camss/camss.c | 30 ++++++----------------- >> 1 file changed, 8 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c >> index 976b70cc6d6a..4e91e4b6ef52 100644 >> --- a/drivers/media/platform/qcom/camss/camss.c >> +++ b/drivers/media/platform/qcom/camss/camss.c >> @@ -3556,7 +3556,6 @@ static int camss_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct camss *camss; >> - int num_subdevs; >> int ret; >> >> camss = devm_kzalloc(dev, sizeof(*camss), GFP_KERNEL); >> @@ -3627,11 +3626,9 @@ static int camss_probe(struct platform_device *pdev) >> >> pm_runtime_enable(dev); >> >> - num_subdevs = camss_of_parse_ports(camss); >> - if (num_subdevs < 0) { >> - ret = num_subdevs; >> + ret = camss_of_parse_ports(camss); >> + if (ret < 0) >> goto err_v4l2_device_unregister; >> - } >> >> ret = camss_register_entities(camss); >> if (ret < 0) >> @@ -3647,23 +3644,12 @@ static int camss_probe(struct platform_device *pdev) >> goto err_register_subdevs; >> } >> >> - if (num_subdevs) { >> - camss->notifier.ops = &camss_subdev_notifier_ops; >> - >> - ret = v4l2_async_nf_register(&camss->notifier); >> - if (ret) { >> - dev_err(dev, >> - "Failed to register async subdev nodes: %d\n", >> - ret); >> - goto err_media_device_unregister; >> - } >> - } else { >> - ret = v4l2_device_register_subdev_nodes(&camss->v4l2_dev); >> - if (ret < 0) { >> - dev_err(dev, "Failed to register subdev nodes: %d\n", >> - ret); >> - goto err_media_device_unregister; >> - } >> + camss->notifier.ops = &camss_subdev_notifier_ops; >> + ret = v4l2_async_nf_register(&camss->notifier); >> + if (ret) { >> + dev_err(dev, >> + "Failed to register async subdev nodes: %d\n", ret); >> + goto err_media_device_unregister; >> } > I'm a little concerned about changing the current flow. Have you tested > this out without sensors attached, the TPG on rb5 for example ? Yes, I have tested this out with the TPG on RB5 by removing a sensor description from the RB5 vision mezzanine, I don't find any regression. Please let me know, if you find any issues with the change. -- Best wishes, Vladimir
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c index 976b70cc6d6a..4e91e4b6ef52 100644 --- a/drivers/media/platform/qcom/camss/camss.c +++ b/drivers/media/platform/qcom/camss/camss.c @@ -3556,7 +3556,6 @@ static int camss_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct camss *camss; - int num_subdevs; int ret; camss = devm_kzalloc(dev, sizeof(*camss), GFP_KERNEL); @@ -3627,11 +3626,9 @@ static int camss_probe(struct platform_device *pdev) pm_runtime_enable(dev); - num_subdevs = camss_of_parse_ports(camss); - if (num_subdevs < 0) { - ret = num_subdevs; + ret = camss_of_parse_ports(camss); + if (ret < 0) goto err_v4l2_device_unregister; - } ret = camss_register_entities(camss); if (ret < 0) @@ -3647,23 +3644,12 @@ static int camss_probe(struct platform_device *pdev) goto err_register_subdevs; } - if (num_subdevs) { - camss->notifier.ops = &camss_subdev_notifier_ops; - - ret = v4l2_async_nf_register(&camss->notifier); - if (ret) { - dev_err(dev, - "Failed to register async subdev nodes: %d\n", - ret); - goto err_media_device_unregister; - } - } else { - ret = v4l2_device_register_subdev_nodes(&camss->v4l2_dev); - if (ret < 0) { - dev_err(dev, "Failed to register subdev nodes: %d\n", - ret); - goto err_media_device_unregister; - } + camss->notifier.ops = &camss_subdev_notifier_ops; + ret = v4l2_async_nf_register(&camss->notifier); + if (ret) { + dev_err(dev, + "Failed to register async subdev nodes: %d\n", ret); + goto err_media_device_unregister; } return 0;
For sake of simplicity it makes sense to register async notifier for all type of subdevices, both CAMSS components and sensors. The case of sensors not connected to CAMSS is extraordinary and degenerate, it does not deserve any specific optimization. Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> --- drivers/media/platform/qcom/camss/camss.c | 30 ++++++----------------- 1 file changed, 8 insertions(+), 22 deletions(-)