Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1350560 > unrolled thread

[PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if there's no endpoint

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if there's no endpoint Javier Martinez Canillas <javier@osg.samsung.com> - 2016-03-04 21:30 +0100
    Re: [PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if  there's no endpoint Sylwester Nawrocki <s.nawrocki@samsung.com> - 2016-03-11 14:10 +0100
      Re: [PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if  there's no endpoint Javier Martinez Canillas <javier@dowhile0.org> - 2016-03-11 16:00 +0100

#1350560 — [PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if there's no endpoint

The fimc_md_parse_port_node() function return 0 if an endpoint node is
not found but according to Documentation/devicetree/bindings/graph.txt,
a port must always have at least one enpoint.

So return an -EINVAL errno code to the caller instead, so it knows that
the port node parse failed due an invalid Device Tree description.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/media/platform/exynos4-is/media-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index feb521f28e14..06f3d75c9a0e 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -394,7 +394,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
 	/* Assume here a port node can have only one endpoint node. */
 	ep = of_get_next_child(port, NULL);
 	if (!ep)
-		return 0;
+		return -EINVAL;
 
 	ret = v4l2_of_parse_endpoint(ep, &endpoint);
 	if (ret) {
-- 
2.5.0

[toc] | [next] | [standalone]

#1355894 — Re: [PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if there's no endpoint

On 03/04/2016 09:20 PM, Javier Martinez Canillas wrote:
> The fimc_md_parse_port_node() function return 0 if an endpoint node is
> not found but according to Documentation/devicetree/bindings/graph.txt,
> a port must always have at least one enpoint.
> 
> So return an -EINVAL errno code to the caller instead, so it knows that
> the port node parse failed due an invalid Device Tree description.

I don't think it is forbidden to have a port node in device tree
containing no endpoint nodes. Empty port node means only that,
for example, a subsystem has a port/bus for connecting external
devices but nothing is actually connected to it.

In case of Exynos CSIS it might not be so useful to have an empty
port node specified in some top level *.dtsi file and only
the endpoints specified in a board specific dts file. Nevertheless,
I wouldn't be saying in general a port node must always have some
endpoint node defined.

I could apply this patch as it doesn't do any harm considering
existing dts files in the kernel tree (arch/arm/boot/dts/
exynos4412-trats2.dts), but the commit description would need to
be changed.

-- 
Thanks,
Sylwester

[toc] | [prev] | [next] | [standalone]

#1355971 — Re: [PATCH 2/2] [media] exynos4-is: FIMC port parse should fail if there's no endpoint

Hello Sylwester,

On Fri, Mar 11, 2016 at 10:03 AM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> On 03/04/2016 09:20 PM, Javier Martinez Canillas wrote:
>> The fimc_md_parse_port_node() function return 0 if an endpoint node is
>> not found but according to Documentation/devicetree/bindings/graph.txt,
>> a port must always have at least one enpoint.
>>
>> So return an -EINVAL errno code to the caller instead, so it knows that
>> the port node parse failed due an invalid Device Tree description.
>
> I don't think it is forbidden to have a port node in device tree
> containing no endpoint nodes. Empty port node means only that,
> for example, a subsystem has a port/bus for connecting external
> devices but nothing is actually connected to it.
>

That's not what I understood by reading both
Documentation/devicetree/bindings/media/video-interfaces.txt and
Documentation/devicetree/bindings/graph.txt but maybe these are not
that clear about it or I just failed to parse the english.

> In case of Exynos CSIS it might not be so useful to have an empty
> port node specified in some top level *.dtsi file and only
> the endpoints specified in a board specific dts file. Nevertheless,
> I wouldn't be saying in general a port node must always have some
> endpoint node defined.
>

Ok, but if that is valid then I believe that at the very least
Documentation/devicetree/bindings/media/samsung-fimc.txt should
explicitly mention which (sub)nodes are optional and which are
required so the DT parsing logic could follow what's documented there.

> I could apply this patch as it doesn't do any harm considering
> existing dts files in the kernel tree (arch/arm/boot/dts/
> exynos4412-trats2.dts), but the commit description would need to
> be changed.
>

I don't mind if you want to change the commit message but if those
nodes are really optional then a follow-up should be to update the DT
binding docs to make that clear IMHO.

> --
> Thanks,
> Sylwester
> --

Best regards,
Javier

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web