diff mbox series

[v3,5/5] dt-bindings: display: ti, tfp410.yaml: make the ports node optional

Message ID 20200611102356.31563-6-ricardo.canuelo@collabora.com (mailing list archive)
State New, archived
Headers show
Series dt-bindings: display: ti,tfp410.txt: convert to yaml | expand

Commit Message

Ricardo Cañuelo June 11, 2020, 10:23 a.m. UTC
Make the ports node optional, since there are some DTs that don't define
any ports for ti,tfp410.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 -
 1 file changed, 1 deletion(-)

Comments

Laurent Pinchart June 11, 2020, 4:08 p.m. UTC | #1
Hi Ricardo,

Thank you for the patch.

On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
> Make the ports node optional, since there are some DTs that don't define
> any ports for ti,tfp410.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>

Shouldn't we fix those DTs instead ? What's the point of a TFP410
without ports in DT ?

> ---
>  Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> index 605831c1e836..1c9421eb80fa 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> @@ -83,7 +83,6 @@ properties:
>  
>  required:
>    - compatible
> -  - ports
>  
>  if:
>    required:
Ricardo Cañuelo June 15, 2020, 9:38 a.m. UTC | #2
Hi Laurent,

Thanks for reviewing the patch

On Thu, 2020-06-11 at 19:08 +0300, Laurent Pinchart wrote:
> Hi Ricardo,
> 
> Thank you for the patch.
> 
> On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
> > Make the ports node optional, since there are some DTs that don't define
> > any ports for ti,tfp410.
> > 
> > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> 
> Shouldn't we fix those DTs instead ? What's the point of a TFP410
> without ports in DT ?

This comes from the discussion in the previous version of this series.

In the DTs that don't define any ports (it's dove-sbc-a510.dts only, actually)
it's not clear how to define the ports (I'm not familiar with this board).
Initially I defined a set of empty ports just to comply with the binding
requirements, but Rob suggested that we might as well declare them as optional,
since having an empty port definition with no remote endpoints is no better than
having no ports at all.

I understand both opinions but I just don't know which is the best option at
this point.

Cheers,
Ricardo
Laurent Pinchart June 16, 2020, 1:51 a.m. UTC | #3
Hi Ricardo,

On Mon, Jun 15, 2020 at 11:38:07AM +0200, Ricardo Cañuelo wrote:
> On Thu, 2020-06-11 at 19:08 +0300, Laurent Pinchart wrote:
> > On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
> > > Make the ports node optional, since there are some DTs that don't define
> > > any ports for ti,tfp410.
> > > 
> > > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> > 
> > Shouldn't we fix those DTs instead ? What's the point of a TFP410
> > without ports in DT ?
> 
> This comes from the discussion in the previous version of this series.
> 
> In the DTs that don't define any ports (it's dove-sbc-a510.dts only, actually)
> it's not clear how to define the ports (I'm not familiar with this board).
> Initially I defined a set of empty ports just to comply with the binding
> requirements, but Rob suggested that we might as well declare them as optional,
> since having an empty port definition with no remote endpoints is no better than
> having no ports at all.
> 
> I understand both opinions but I just don't know which is the best option at
> this point.

How about keeping the ports mandatory, and leaving dove-sbc-a510.dts to
be fixed later ?
Ricardo Cañuelo June 16, 2020, 6:42 a.m. UTC | #4
Hi Laurent,

On Tue, 2020-06-16 at 04:51 +0300, Laurent Pinchart wrote:
> How about keeping the ports mandatory, and leaving dove-sbc-a510.dts to
> be fixed later ?

That's fine by me, I'll prepare a new version of the series.
Thanks for your input!

Cheers,
Ricardo
Rob Herring (Arm) June 17, 2020, 10:34 p.m. UTC | #5
On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
> Make the ports node optional, since there are some DTs that don't define
> any ports for ti,tfp410.

Only arch/arm/boot/dts/dove-sbc-a510.dts AFAICT... It should be updated 
IMO.

> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> index 605831c1e836..1c9421eb80fa 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> @@ -83,7 +83,6 @@ properties:
>  
>  required:
>    - compatible
> -  - ports
>  
>  if:
>    required:
> -- 
> 2.18.0
>
Rob Herring (Arm) June 17, 2020, 10:37 p.m. UTC | #6
On Mon, Jun 15, 2020 at 11:38:07AM +0200, Ricardo Cañuelo wrote:
> Hi Laurent,
> 
> Thanks for reviewing the patch
> 
> On Thu, 2020-06-11 at 19:08 +0300, Laurent Pinchart wrote:
> > Hi Ricardo,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
> > > Make the ports node optional, since there are some DTs that don't define
> > > any ports for ti,tfp410.
> > > 
> > > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> > 
> > Shouldn't we fix those DTs instead ? What's the point of a TFP410
> > without ports in DT ?
> 
> This comes from the discussion in the previous version of this series.
> 
> In the DTs that don't define any ports (it's dove-sbc-a510.dts only, actually)
> it's not clear how to define the ports (I'm not familiar with this board).
> Initially I defined a set of empty ports just to comply with the binding
> requirements, but Rob suggested that we might as well declare them as optional,
> since having an empty port definition with no remote endpoints is no better than
> having no ports at all.

I did? Must have missed some context.

> I understand both opinions but I just don't know which is the best option at
> this point.

Just leave the warning to be fixed.

Rob
Laurent Pinchart June 17, 2020, 10:43 p.m. UTC | #7
Hi Rob,

On Wed, Jun 17, 2020 at 04:34:55PM -0600, Rob Herring wrote:
> On Thu, Jun 11, 2020 at 12:23:56PM +0200, Ricardo Cañuelo wrote:
> > Make the ports node optional, since there are some DTs that don't define
> > any ports for ti,tfp410.
> 
> Only arch/arm/boot/dts/dove-sbc-a510.dts AFAICT... It should be updated 
> IMO.

Agreed, but Ricardo wasn't sure how to update it. It would be nice if
someone with knowledge of the hardware could have a look.

By the way, this patch is dropped from v4 of the series.

> > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> > ---
> >  Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> > index 605831c1e836..1c9421eb80fa 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> > @@ -83,7 +83,6 @@ properties:
> >  
> >  required:
> >    - compatible
> > -  - ports
> >  
> >  if:
> >    required:
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
index 605831c1e836..1c9421eb80fa 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
@@ -83,7 +83,6 @@  properties:
 
 required:
   - compatible
-  - ports
 
 if:
   required: