diff mbox

[4/7] ARM: dts: sun7i: Add pinmux settings for LCD0 RGB888 output.

Message ID 1521662593-25468-4-git-send-email-giulio.benetti@micronovasrl.com (mailing list archive)
State Rejected, archived
Headers show

Commit Message

Giulio Benetti March 21, 2018, 8:03 p.m. UTC
The A20 supports RGB888 with H/V sync from LCD0. Add a pinmux setting
for the needed pins.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Paul Kocialkowski March 25, 2018, 2:09 p.m. UTC | #1
Hi,

Le mercredi 21 mars 2018 à 21:03 +0100, Giulio Benetti a écrit :
> The A20 supports RGB888 with H/V sync from LCD0. Add a pinmux setting
> for the needed pins.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> b/arch/arm/boot/dts/sun7i-a20.dtsi
> index efb5607..bfe6728 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -922,6 +922,14 @@
>  				pins = "PI20", "PI21";
>  				function = "uart7";
>  			};
> +
> +			lcd0_rgb888_pins: lcd0-rgb888-pins {

It would be more consistent with other pins definitions to have
underscores in both names and to indicate the index, such as:
lcd0_rgb888_pins: lcd0_rgb888_pins@0 {

This way, other set of pins for LCD (PH0-PH27) can be declared as @1
when they are needed in the future.

> +				pins = "PD0", "PD1", "PD2", "PD3",
> "PD4", "PD5", "PD6", "PD7",
> +				       "PD8", "PD9", "PD10", "PD11",
> "PD12", "PD13", "PD14", "PD15",
> +				       "PD16", "PD17", "PD18",
> "PD19", "PD20", "PD21", "PD22", "PD23",
> +				       "PD24", "PD25", "PD26",
> "PD27";
> +				function = "lcd0";
> +			};
>  		};
>  
>  		timer@1c20c00 {
Maxime Ripard March 26, 2018, 10:01 a.m. UTC | #2
Hi,

On Sun, Mar 25, 2018 at 04:09:13PM +0200, Paul Kocialkowski wrote:
> Le mercredi 21 mars 2018 à 21:03 +0100, Giulio Benetti a écrit :
> > The A20 supports RGB888 with H/V sync from LCD0. Add a pinmux setting
> > for the needed pins.
> > 
> > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > ---
> >  arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> > b/arch/arm/boot/dts/sun7i-a20.dtsi
> > index efb5607..bfe6728 100644
> > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > @@ -922,6 +922,14 @@
> >  				pins = "PI20", "PI21";
> >  				function = "uart7";
> >  			};
> > +
> > +			lcd0_rgb888_pins: lcd0-rgb888-pins {
> 
> It would be more consistent with other pins definitions to have
> underscores in both names and to indicate the index, such as:
> lcd0_rgb888_pins: lcd0_rgb888_pins@0 {

Both your suggestions will generate DTC warnings, and we'd like to get
rid of them eventually :)

> This way, other set of pins for LCD (PH0-PH27) can be declared as @1
> when they are needed in the future.

A better idea would be to call it lcd0-rgb888-pd-pins, and introduce
the ph variant when it's done.

Maxime
Giulio Benetti March 26, 2018, 11:27 a.m. UTC | #3
Hi,

Il 26/03/2018 12:01, Maxime Ripard ha scritto:
> Hi,
> 
> On Sun, Mar 25, 2018 at 04:09:13PM +0200, Paul Kocialkowski wrote:
>> Le mercredi 21 mars 2018 à 21:03 +0100, Giulio Benetti a écrit :
>>> The A20 supports RGB888 with H/V sync from LCD0. Add a pinmux setting
>>> for the needed pins.
>>>
>>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>> ---
>>>   arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
>>> b/arch/arm/boot/dts/sun7i-a20.dtsi
>>> index efb5607..bfe6728 100644
>>> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
>>> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
>>> @@ -922,6 +922,14 @@
>>>   				pins = "PI20", "PI21";
>>>   				function = "uart7";
>>>   			};
>>> +
>>> +			lcd0_rgb888_pins: lcd0-rgb888-pins {
>>
>> It would be more consistent with other pins definitions to have
>> underscores in both names and to indicate the index, such as:
>> lcd0_rgb888_pins: lcd0_rgb888_pins@0 {
> 
> Both your suggestions will generate DTC warnings, and we'd like to get
> rid of them eventually :)
> 
>> This way, other set of pins for LCD (PH0-PH27) can be declared as @1
>> when they are needed in the future.
> 
> A better idea would be to call it lcd0-rgb888-pd-pins, and introduce
> the ph variant when it's done.

As I know, only PD is muxed with LCD0.
And PH is for LCD1 only.

And LCD0 seems to come out only from PD port according to datasheet,
this is why I didn't put @0 after lcd0-rgb888-pins.

So I don't think it makes sense to handle pins in the way Paul suggests.

What do you all think?

Giulio

> 
> Maxime
> 
> 
>
Maxime Ripard March 26, 2018, 11:46 a.m. UTC | #4
On Mon, Mar 26, 2018 at 01:27:05PM +0200, Giulio Benetti wrote:
> Hi,
> 
> Il 26/03/2018 12:01, Maxime Ripard ha scritto:
> > Hi,
> > 
> > On Sun, Mar 25, 2018 at 04:09:13PM +0200, Paul Kocialkowski wrote:
> > > Le mercredi 21 mars 2018 à 21:03 +0100, Giulio Benetti a écrit :
> > > > The A20 supports RGB888 with H/V sync from LCD0. Add a pinmux setting
> > > > for the needed pins.
> > > > 
> > > > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> > > > ---
> > > >   arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
> > > >   1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> > > > b/arch/arm/boot/dts/sun7i-a20.dtsi
> > > > index efb5607..bfe6728 100644
> > > > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > > > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > > > @@ -922,6 +922,14 @@
> > > >   				pins = "PI20", "PI21";
> > > >   				function = "uart7";
> > > >   			};
> > > > +
> > > > +			lcd0_rgb888_pins: lcd0-rgb888-pins {
> > > 
> > > It would be more consistent with other pins definitions to have
> > > underscores in both names and to indicate the index, such as:
> > > lcd0_rgb888_pins: lcd0_rgb888_pins@0 {
> > 
> > Both your suggestions will generate DTC warnings, and we'd like to get
> > rid of them eventually :)
> > 
> > > This way, other set of pins for LCD (PH0-PH27) can be declared as @1
> > > when they are needed in the future.
> > 
> > A better idea would be to call it lcd0-rgb888-pd-pins, and introduce
> > the ph variant when it's done.
> 
> As I know, only PD is muxed with LCD0.
> And PH is for LCD1 only.
> 
> And LCD0 seems to come out only from PD port according to datasheet,
> this is why I didn't put @0 after lcd0-rgb888-pins.
> 
> So I don't think it makes sense to handle pins in the way Paul suggests.
> 
> What do you all think?
y
That seems sensible in this case, yes.

Maxime
diff mbox

Patch

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index efb5607..bfe6728 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -922,6 +922,14 @@ 
 				pins = "PI20", "PI21";
 				function = "uart7";
 			};
+
+			lcd0_rgb888_pins: lcd0-rgb888-pins {
+				pins = "PD0", "PD1", "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
+				       "PD8", "PD9", "PD10", "PD11", "PD12", "PD13", "PD14", "PD15",
+				       "PD16", "PD17", "PD18", "PD19", "PD20", "PD21", "PD22", "PD23",
+				       "PD24", "PD25", "PD26", "PD27";
+				function = "lcd0";
+			};
 		};
 
 		timer@1c20c00 {