| Message ID | 20230614-topic-camss_grpah-v1-1-5f4b516310fa@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] media: camss: Intepret OF graph connections more sensibly | expand |
On 14/06/2023 14:22, Konrad Dybcio wrote: > Not all endpoints of camss have to be populated. In fact, most of the > time they shouldn't be as n-th auxilliary cameras are usually ewaste. > > Don't fail probing the entire camss even even one endpoint is not > linked and throw an error when none is found. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/media/platform/qcom/camss/camss.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index 1ef26aea3eae..3aa03fbc94e2 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -1084,9 +1084,8 @@ static int camss_of_parse_ports(struct camss *camss) > > remote = of_graph_get_remote_port_parent(node); > if (!remote) { > - dev_err(dev, "Cannot get remote parent\n"); > - ret = -EINVAL; > - goto err_cleanup; > + of_node_put(node); > + continue; > } > > csd = v4l2_async_nf_add_fwnode(&camss->notifier, > @@ -1105,7 +1104,7 @@ static int camss_of_parse_ports(struct camss *camss) > num_subdevs++; > } > > - return num_subdevs; > + return num_subdevs ? num_subdevs : -EINVAL; > > err_cleanup: > of_node_put(node); > > --- > base-commit: b16049b21162bb649cdd8519642a35972b7910fe > change-id: 20230614-topic-camss_grpah-39f9a4f7420c > > Best regards, Can you give an example of the DT that is causing this ? --- bod
On 14.06.2023 15:48, Bryan O'Donoghue wrote: > On 14/06/2023 14:22, Konrad Dybcio wrote: >> Not all endpoints of camss have to be populated. In fact, most of the >> time they shouldn't be as n-th auxilliary cameras are usually ewaste. >> >> Don't fail probing the entire camss even even one endpoint is not >> linked and throw an error when none is found. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/media/platform/qcom/camss/camss.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c >> index 1ef26aea3eae..3aa03fbc94e2 100644 >> --- a/drivers/media/platform/qcom/camss/camss.c >> +++ b/drivers/media/platform/qcom/camss/camss.c >> @@ -1084,9 +1084,8 @@ static int camss_of_parse_ports(struct camss *camss) >> remote = of_graph_get_remote_port_parent(node); >> if (!remote) { >> - dev_err(dev, "Cannot get remote parent\n"); >> - ret = -EINVAL; >> - goto err_cleanup; >> + of_node_put(node); >> + continue; >> } >> csd = v4l2_async_nf_add_fwnode(&camss->notifier, >> @@ -1105,7 +1104,7 @@ static int camss_of_parse_ports(struct camss *camss) >> num_subdevs++; >> } >> - return num_subdevs; >> + return num_subdevs ? num_subdevs : -EINVAL; >> err_cleanup: >> of_node_put(node); >> >> --- >> base-commit: b16049b21162bb649cdd8519642a35972b7910fe >> change-id: 20230614-topic-camss_grpah-39f9a4f7420c >> >> Best regards, > > Can you give an example of the DT that is causing this ? None upstream (8916 has a single port and 845c's sec camera has never been enabled), but it's easy to spot if you leave csiphy0_ep unoccupied and add something to csiphyN_ep for N>=1 Konrad > > --- > bod
On 14/06/2023 14:50, Konrad Dybcio wrote: > On 14.06.2023 15:48, Bryan O'Donoghue wrote: >> On 14/06/2023 14:22, Konrad Dybcio wrote: >>> Not all endpoints of camss have to be populated. In fact, most of the >>> time they shouldn't be as n-th auxilliary cameras are usually ewaste. >>> >>> Don't fail probing the entire camss even even one endpoint is not >>> linked and throw an error when none is found. >>> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> --- >>> drivers/media/platform/qcom/camss/camss.c | 7 +++---- >>> 1 file changed, 3 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c >>> index 1ef26aea3eae..3aa03fbc94e2 100644 >>> --- a/drivers/media/platform/qcom/camss/camss.c >>> +++ b/drivers/media/platform/qcom/camss/camss.c >>> @@ -1084,9 +1084,8 @@ static int camss_of_parse_ports(struct camss *camss) >>> remote = of_graph_get_remote_port_parent(node); >>> if (!remote) { >>> - dev_err(dev, "Cannot get remote parent\n"); >>> - ret = -EINVAL; >>> - goto err_cleanup; >>> + of_node_put(node); >>> + continue; >>> } >>> csd = v4l2_async_nf_add_fwnode(&camss->notifier, >>> @@ -1105,7 +1104,7 @@ static int camss_of_parse_ports(struct camss *camss) >>> num_subdevs++; >>> } >>> - return num_subdevs; >>> + return num_subdevs ? num_subdevs : -EINVAL; Can you if/else this instead of the ? Then add my Acked-by: --- bod
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c index 1ef26aea3eae..3aa03fbc94e2 100644 --- a/drivers/media/platform/qcom/camss/camss.c +++ b/drivers/media/platform/qcom/camss/camss.c @@ -1084,9 +1084,8 @@ static int camss_of_parse_ports(struct camss *camss) remote = of_graph_get_remote_port_parent(node); if (!remote) { - dev_err(dev, "Cannot get remote parent\n"); - ret = -EINVAL; - goto err_cleanup; + of_node_put(node); + continue; } csd = v4l2_async_nf_add_fwnode(&camss->notifier, @@ -1105,7 +1104,7 @@ static int camss_of_parse_ports(struct camss *camss) num_subdevs++; } - return num_subdevs; + return num_subdevs ? num_subdevs : -EINVAL; err_cleanup: of_node_put(node);
Not all endpoints of camss have to be populated. In fact, most of the time they shouldn't be as n-th auxilliary cameras are usually ewaste. Don't fail probing the entire camss even even one endpoint is not linked and throw an error when none is found. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/media/platform/qcom/camss/camss.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) --- base-commit: b16049b21162bb649cdd8519642a35972b7910fe change-id: 20230614-topic-camss_grpah-39f9a4f7420c Best regards,