Message ID | 87lfhlrtwp.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
---|---|
State | Under Review |
Delegated to: | Kieran Bingham |
Headers | show |
Series | arm64: renesas: enable HDMI Display/Sound on R-Car M3-W+ Salvator-XS | expand |
Hi Morimoto-san, Thank you for the patch. On Tue, Sep 08, 2020 at 09:34:11AM +0900, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > required is "renesas,r8a7795-hdmi", instead of "renesas,r8a7795-dw-hdmi" > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > .../devicetree/bindings/display/bridge/renesas,dw-hdmi.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > index 819f3e31013c..e6526ab485d0 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > @@ -42,7 +42,7 @@ Optional properties: > Example: > > hdmi0: hdmi@fead0000 { > - compatible = "renesas,r8a7795-dw-hdmi"; > + compatible = "renesas,r8a7795-hdmi"; > reg = <0 0xfead0000 0 0x10000>; > interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>;
Hi Morimoto-san, On 08/09/2020 01:34, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > required is "renesas,r8a7795-hdmi", instead of "renesas,r8a7795-dw-hdmi" Hrm, technically the driver will currently only match on : "renesas,rcar-gen3-hdmi" But I see how the '-dw-' has probably snuck in from other devices, and is inappropriate. Perhaps this should be more clear that it matches on the generic compatible: renesas,rcar-gen3-hdmi (or a combination of both?) > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> But if the generic isn't required, then this patch alone does fix what I would call an error, so ... Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> We could always make this more clear when converting to YAML. > --- > .../devicetree/bindings/display/bridge/renesas,dw-hdmi.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > index 819f3e31013c..e6526ab485d0 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > @@ -42,7 +42,7 @@ Optional properties: > Example: > > hdmi0: hdmi@fead0000 { > - compatible = "renesas,r8a7795-dw-hdmi"; > + compatible = "renesas,r8a7795-hdmi"; > reg = <0 0xfead0000 0 0x10000>; > interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>; >
Hi Kieran, On Tue, Sep 08, 2020 at 03:18:20PM +0100, Kieran Bingham wrote: > On 08/09/2020 01:34, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > required is "renesas,r8a7795-hdmi", instead of "renesas,r8a7795-dw-hdmi" > > Hrm, technically the driver will currently only match on : > "renesas,rcar-gen3-hdmi" > > But I see how the '-dw-' has probably snuck in from other devices, and > is inappropriate. > > Perhaps this should be more clear that it matches on the generic compatible: > renesas,rcar-gen3-hdmi > > (or a combination of both?) > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > But if the generic isn't required, then this patch alone does fix what I > would call an error, so ... You're right, the generic compatible string should be required. I'll update this patch accordingly, and will address the bindings as part of the conversion to YAML. > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > We could always make this more clear when converting to YAML. > > > --- > > .../devicetree/bindings/display/bridge/renesas,dw-hdmi.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > > index 819f3e31013c..e6526ab485d0 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > > @@ -42,7 +42,7 @@ Optional properties: > > Example: > > > > hdmi0: hdmi@fead0000 { > > - compatible = "renesas,r8a7795-dw-hdmi"; > > + compatible = "renesas,r8a7795-hdmi"; > > reg = <0 0xfead0000 0 0x10000>; > > interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; > > clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>; > >
On Tue, Sep 08, 2020 at 05:43:25PM +0300, Laurent Pinchart wrote: > Hi Kieran, > > On Tue, Sep 08, 2020 at 03:18:20PM +0100, Kieran Bingham wrote: > > On 08/09/2020 01:34, Kuninori Morimoto wrote: > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > > > required is "renesas,r8a7795-hdmi", instead of "renesas,r8a7795-dw-hdmi" > > > > Hrm, technically the driver will currently only match on : > > "renesas,rcar-gen3-hdmi" > > > > But I see how the '-dw-' has probably snuck in from other devices, and > > is inappropriate. > > > > Perhaps this should be more clear that it matches on the generic compatible: > > renesas,rcar-gen3-hdmi > > > > (or a combination of both?) > > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > But if the generic isn't required, then this patch alone does fix what I > > would call an error, so ... > > You're right, the generic compatible string should be required. I'll > update this patch accordingly, and will address the bindings as part of > the conversion to YAML. Proposed new commit: commit cc487cbb06d841ca76efade63201a41bd04f6015 Author: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Date: Tue Sep 8 09:34:11 2020 +0900 dt-bindings: display: renesas: dw-hdmi: Tidyup example compatible The DT example erronously uses the "renesas,r8a7795-dw-hdmi", when the correct value is "renesas,r8a7795-hdmi". It is furthermore missing the generic "renesas,rcar-gen3-hdmi" compatible string. Fix it. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> [Add "renesas,rcar-gen3-hdmi" and rework commit message] Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt index f275997ab947..9c56c5169a88 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt @@ -43,7 +43,7 @@ Optional properties: Example: hdmi0: hdmi@fead0000 { - compatible = "renesas,r8a7795-dw-hdmi"; + compatible = "renesas,r8a7795-hdmi", "renesas,rcar-gen3-hdmi"; reg = <0 0xfead0000 0 0x10000>; interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>; > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > We could always make this more clear when converting to YAML. > > > > > --- > > > .../devicetree/bindings/display/bridge/renesas,dw-hdmi.txt | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > > > index 819f3e31013c..e6526ab485d0 100644 > > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > > > @@ -42,7 +42,7 @@ Optional properties: > > > Example: > > > > > > hdmi0: hdmi@fead0000 { > > > - compatible = "renesas,r8a7795-dw-hdmi"; > > > + compatible = "renesas,r8a7795-hdmi"; > > > reg = <0 0xfead0000 0 0x10000>; > > > interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; > > > clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>;
Hi Laurent, On 08/09/2020 15:46, Laurent Pinchart wrote: > On Tue, Sep 08, 2020 at 05:43:25PM +0300, Laurent Pinchart wrote: >> Hi Kieran, >> >> On Tue, Sep 08, 2020 at 03:18:20PM +0100, Kieran Bingham wrote: >>> On 08/09/2020 01:34, Kuninori Morimoto wrote: >>>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >>>> >>>> required is "renesas,r8a7795-hdmi", instead of "renesas,r8a7795-dw-hdmi" >>> >>> Hrm, technically the driver will currently only match on : >>> "renesas,rcar-gen3-hdmi" >>> >>> But I see how the '-dw-' has probably snuck in from other devices, and >>> is inappropriate. >>> >>> Perhaps this should be more clear that it matches on the generic compatible: >>> renesas,rcar-gen3-hdmi >>> >>> (or a combination of both?) >>> >>>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >>> >>> But if the generic isn't required, then this patch alone does fix what I >>> would call an error, so ... >> >> You're right, the generic compatible string should be required. I'll >> update this patch accordingly, and will address the bindings as part of >> the conversion to YAML. > > Proposed new commit: > > commit cc487cbb06d841ca76efade63201a41bd04f6015 > Author: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Date: Tue Sep 8 09:34:11 2020 +0900 > > dt-bindings: display: renesas: dw-hdmi: Tidyup example compatible > > The DT example erronously uses the "renesas,r8a7795-dw-hdmi", when the > correct value is "renesas,r8a7795-hdmi". It is furthermore missing the > generic "renesas,rcar-gen3-hdmi" compatible string. Fix it. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > [Add "renesas,rcar-gen3-hdmi" and rework commit message] > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > index f275997ab947..9c56c5169a88 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt > @@ -43,7 +43,7 @@ Optional properties: > Example: > > hdmi0: hdmi@fead0000 { > - compatible = "renesas,r8a7795-dw-hdmi"; > + compatible = "renesas,r8a7795-hdmi", "renesas,rcar-gen3-hdmi"; > reg = <0 0xfead0000 0 0x10000>; > interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>; > Perfect, looks good to me. -- Kieran >>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >>> >>> We could always make this more clear when converting to YAML. >>> >>>> --- >>>> .../devicetree/bindings/display/bridge/renesas,dw-hdmi.txt | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt >>>> index 819f3e31013c..e6526ab485d0 100644 >>>> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt >>>> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt >>>> @@ -42,7 +42,7 @@ Optional properties: >>>> Example: >>>> >>>> hdmi0: hdmi@fead0000 { >>>> - compatible = "renesas,r8a7795-dw-hdmi"; >>>> + compatible = "renesas,r8a7795-hdmi"; >>>> reg = <0 0xfead0000 0 0x10000>; >>>> interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; >>>> clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>; >
Hi Laurent, > > Author: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Date: Tue Sep 8 09:34:11 2020 +0900 > > > > dt-bindings: display: renesas: dw-hdmi: Tidyup example compatible > > > > The DT example erronously uses the "renesas,r8a7795-dw-hdmi", when the > > correct value is "renesas,r8a7795-hdmi". It is furthermore missing the > > generic "renesas,rcar-gen3-hdmi" compatible string. Fix it. > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > [Add "renesas,rcar-gen3-hdmi" and rework commit message] > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> I will send Acked-by: to my (?) patch :) Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt index 819f3e31013c..e6526ab485d0 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt @@ -42,7 +42,7 @@ Optional properties: Example: hdmi0: hdmi@fead0000 { - compatible = "renesas,r8a7795-dw-hdmi"; + compatible = "renesas,r8a7795-hdmi"; reg = <0 0xfead0000 0 0x10000>; interrupts = <0 389 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cpg CPG_CORE R8A7795_CLK_S0D4>, <&cpg CPG_MOD 729>;