[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Karl Beldan]

This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 25f0f8e..bc10e7e 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -41,6 +41,42 @@
 			pinctrl-single,function-mask = <0xf>;
 			status = "disabled";
 
+			serial0_rtscts_pins: pinmux_serial0_rtscts_pins {
+				pinctrl-single,bits = <
+					/* UART0_RTS UART0_CTS */
+					0x0c 0x22000000 0xff000000
+				>;
+			};
+			serial0_rxtx_pins: pinmux_serial0_rxtx_pins {
+				pinctrl-single,bits = <
+					/* UART0_TXD UART0_RXD */
+					0x0c 0x00220000 0x00ff0000
+				>;
+			};
+			serial1_rtscts_pins: pinmux_serial1_rtscts_pins {
+				pinctrl-single,bits = <
+					/* UART1_CTS UART1_RTS */
+					0x00 0x00440000 0x00ff0000
+				>;
+			};
+			serial1_rxtx_pins: pinmux_serial1_rxtx_pins {
+				pinctrl-single,bits = <
+					/* UART1_TXD UART1_RXD */
+					0x10 0x22000000 0xff000000
+				>;
+			};
+			serial2_rtscts_pins: pinmux_serial2_rtscts_pins {
+				pinctrl-single,bits = <
+					/* UART2_CTS UART2_RTS */
+					0x00 0x44000000 0xff000000
+				>;
+			};
+			serial2_rxtx_pins: pinmux_serial2_rxtx_pins {
+				pinctrl-single,bits = <
+					/* UART2_TXD UART2_RXD */
+					0x10 0x00220000 0x00ff0000
+				>;
+			};
 			nand_cs3_pins: pinmux_nand_pins {
 				pinctrl-single,bits = <
 					/* EMA_OE, EMA_WE */
-- 
2.9.0.rc1

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Kevin Hilman]

+Sekhar

Karl Beldan <kbeldan@baylibre.com> writes:

> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 25f0f8e..bc10e7e 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -41,6 +41,42 @@
>  			pinctrl-single,function-mask = <0xf>;
>  			status = "disabled";
>  
> +			serial0_rtscts_pins: pinmux_serial0_rtscts_pins {
> +				pinctrl-single,bits = <
> +					/* UART0_RTS UART0_CTS */
> +					0x0c 0x22000000 0xff000000
> +				>;
> +			};
> +			serial0_rxtx_pins: pinmux_serial0_rxtx_pins {
> +				pinctrl-single,bits = <
> +					/* UART0_TXD UART0_RXD */
> +					0x0c 0x00220000 0x00ff0000
> +				>;
> +			};
> +			serial1_rtscts_pins: pinmux_serial1_rtscts_pins {
> +				pinctrl-single,bits = <
> +					/* UART1_CTS UART1_RTS */
> +					0x00 0x00440000 0x00ff0000
> +				>;
> +			};
> +			serial1_rxtx_pins: pinmux_serial1_rxtx_pins {
> +				pinctrl-single,bits = <
> +					/* UART1_TXD UART1_RXD */
> +					0x10 0x22000000 0xff000000
> +				>;
> +			};
> +			serial2_rtscts_pins: pinmux_serial2_rtscts_pins {
> +				pinctrl-single,bits = <
> +					/* UART2_CTS UART2_RTS */
> +					0x00 0x44000000 0xff000000
> +				>;
> +			};
> +			serial2_rxtx_pins: pinmux_serial2_rxtx_pins {
> +				pinctrl-single,bits = <
> +					/* UART2_TXD UART2_RXD */
> +					0x10 0x00220000 0x00ff0000
> +				>;
> +			};
>  			nand_cs3_pins: pinmux_nand_pins {
>  				pinctrl-single,bits = <
>  					/* EMA_OE, EMA_WE */

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Kevin Hilman]

Karl Beldan <kbeldan@baylibre.com> writes:

> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>

Should da850-evm be updated to use the serial2_rxtx_pins also?

Tested on LCDK (which uses the serial2 pins).

Tested-by: Kevin Hilman <khilman@baylibre.com>

Kevin

> ---
>  arch/arm/boot/dts/da850.dtsi | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 25f0f8e..bc10e7e 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -41,6 +41,42 @@
>  			pinctrl-single,function-mask = <0xf>;
>  			status = "disabled";
>  
> +			serial0_rtscts_pins: pinmux_serial0_rtscts_pins {
> +				pinctrl-single,bits = <
> +					/* UART0_RTS UART0_CTS */
> +					0x0c 0x22000000 0xff000000
> +				>;
> +			};
> +			serial0_rxtx_pins: pinmux_serial0_rxtx_pins {
> +				pinctrl-single,bits = <
> +					/* UART0_TXD UART0_RXD */
> +					0x0c 0x00220000 0x00ff0000
> +				>;
> +			};
> +			serial1_rtscts_pins: pinmux_serial1_rtscts_pins {
> +				pinctrl-single,bits = <
> +					/* UART1_CTS UART1_RTS */
> +					0x00 0x00440000 0x00ff0000
> +				>;
> +			};
> +			serial1_rxtx_pins: pinmux_serial1_rxtx_pins {
> +				pinctrl-single,bits = <
> +					/* UART1_TXD UART1_RXD */
> +					0x10 0x22000000 0xff000000
> +				>;
> +			};
> +			serial2_rtscts_pins: pinmux_serial2_rtscts_pins {
> +				pinctrl-single,bits = <
> +					/* UART2_CTS UART2_RTS */
> +					0x00 0x44000000 0xff000000
> +				>;
> +			};
> +			serial2_rxtx_pins: pinmux_serial2_rxtx_pins {
> +				pinctrl-single,bits = <
> +					/* UART2_TXD UART2_RXD */
> +					0x10 0x00220000 0x00ff0000
> +				>;
> +			};
>  			nand_cs3_pins: pinmux_nand_pins {
>  				pinctrl-single,bits = <
>  					/* EMA_OE, EMA_WE */

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Karl Beldan]

On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
> Karl Beldan <kbeldan@baylibre.com> writes:
> 
> > This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
> >
> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> 
> Should da850-evm be updated to use the serial2_rxtx_pins also?
> 
I could not find the EVM schematics on the net and I only have an LCDK,
but according to the code it should, however I can't tell whether flow
control pins are used.
 
Karl

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Kevin Hilman]

Karl Beldan <kbeldan@baylibre.com> writes:

> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
>> Karl Beldan <kbeldan@baylibre.com> writes:
>> 
>> > This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>> >
>> > Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>> 
>> Should da850-evm be updated to use the serial2_rxtx_pins also?
>> 
> I could not find the EVM schematics on the net and I only have an LCDK,
> but according to the code it should, however I can't tell whether flow
> control pins are used.

Ok, let's just leave it for now, since it's working fine.  Sekhar can
fix that up if he can dig up the schematics.

Kevin

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
> Karl Beldan <kbeldan@baylibre.com> writes:
> 
>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
>>> Karl Beldan <kbeldan@baylibre.com> writes:
>>>
>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>>>>
>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>
>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
>>>
>> I could not find the EVM schematics on the net and I only have an LCDK,
>> but according to the code it should, however I can't tell whether flow
>> control pins are used.
> 
> Ok, let's just leave it for now, since it's working fine.  Sekhar can
> fix that up if he can dig up the schematics.

Looks like the flow control pins are being used for McASP also on the
EVM. So lets leave the EVM as-is.

Regards,
Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
> On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
>> Karl Beldan <kbeldan@baylibre.com> writes:
>>
>>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
>>>> Karl Beldan <kbeldan@baylibre.com> writes:
>>>>
>>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>>>>>
>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>
>>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
>>>>
>>> I could not find the EVM schematics on the net and I only have an LCDK,
>>> but according to the code it should, however I can't tell whether flow
>>> control pins are used.
>>
>> Ok, let's just leave it for now, since it's working fine.  Sekhar can
>> fix that up if he can dig up the schematics.
> 
> Looks like the flow control pins are being used for McASP also on the
> EVM. So lets leave the EVM as-is.

Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
the LCDK. Right now it seems to be relying on bootloader to serial2
setup pimux correctly. I can make a patch to fix that. Or if you can do
it, that will be great too.

Regards,
Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Karl Beldan]

On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
> On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
> > On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
> >> Karl Beldan <kbeldan@baylibre.com> writes:
> >>
> >>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
> >>>> Karl Beldan <kbeldan@baylibre.com> writes:
> >>>>
> >>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
> >>>>>
> >>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>
> >>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
> >>>>
> >>> I could not find the EVM schematics on the net and I only have an LCDK,
> >>> but according to the code it should, however I can't tell whether flow
> >>> control pins are used.
> >>
> >> Ok, let's just leave it for now, since it's working fine.  Sekhar can
> >> fix that up if he can dig up the schematics.
> > 
> > Looks like the flow control pins are being used for McASP also on the
> > EVM. So lets leave the EVM as-is.
> 
> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
> the LCDK. Right now it seems to be relying on bootloader to serial2
> setup pimux correctly. I can make a patch to fix that. Or if you can do
> it, that will be great too.
> 

Indeed ATM the EVM relies on the bootloader to setup the pin muxing.

I just checked the uart pins routing of the EVM, the dts:
- should reclaim serial2_rxtx_pins and serial2_rtscts_pins
- can reclaim serial1_rxtx_pins (out on Audio connector but very
  unlikely used for audio)
- should leave serial1_rtscts_pins (out on Audio connector but used by
  McASP so used for audio)

Also I think it would be better for the serial nodes to reclaim the rxtx
pins in the dtsi, and override the reclaimed pins in the .dts only for
the nodes reclaiming flow control (some other pins could also be
directly reclaimed in the dtsi).

Some other cleanups would also be in order for the da850*:
- use labels for non dtsi (like I did for the LCDK)
- add chosen,memory nodes (I guess currently only the LCDK can
  dispense with ATAGS)
- use a null range translation in the da850 dtsi for the soc node
  (computing the offsets is error prone and is there a point)

Sure I could fix that, along with some of the above suggestions if you
are ok with it.

Rgds, 
Karl

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
> On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
>> On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
>>> On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
>>>> Karl Beldan <kbeldan@baylibre.com> writes:
>>>>
>>>>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
>>>>>> Karl Beldan <kbeldan@baylibre.com> writes:
>>>>>>
>>>>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>>>>>>>
>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>
>>>>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
>>>>>>
>>>>> I could not find the EVM schematics on the net and I only have an LCDK,
>>>>> but according to the code it should, however I can't tell whether flow
>>>>> control pins are used.
>>>>
>>>> Ok, let's just leave it for now, since it's working fine.  Sekhar can
>>>> fix that up if he can dig up the schematics.
>>>
>>> Looks like the flow control pins are being used for McASP also on the
>>> EVM. So lets leave the EVM as-is.
>>
>> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
>> the LCDK. Right now it seems to be relying on bootloader to serial2
>> setup pimux correctly. I can make a patch to fix that. Or if you can do
>> it, that will be great too.
>>
> 
> Indeed ATM the EVM relies on the bootloader to setup the pin muxing.
> 
> I just checked the uart pins routing of the EVM, the dts:
> - should reclaim serial2_rxtx_pins and serial2_rtscts_pins

Can you please clarify what you mean by dts should "reclaim". You mean
move the pins from da850.dtsi to da850-evm.dts or something else?

Thanks,
Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Karl Beldan]

On Thu, Aug 25, 2016 at 07:12:16PM +0530, Sekhar Nori wrote:
> On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
> > On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
> >> On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
> >>> On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
> >>>> Karl Beldan <kbeldan@baylibre.com> writes:
> >>>>
> >>>>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
> >>>>>> Karl Beldan <kbeldan@baylibre.com> writes:
> >>>>>>
> >>>>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
> >>>>>>>
> >>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>
> >>>>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
> >>>>>>
> >>>>> I could not find the EVM schematics on the net and I only have an LCDK,
> >>>>> but according to the code it should, however I can't tell whether flow
> >>>>> control pins are used.
> >>>>
> >>>> Ok, let's just leave it for now, since it's working fine.  Sekhar can
> >>>> fix that up if he can dig up the schematics.
> >>>
> >>> Looks like the flow control pins are being used for McASP also on the
> >>> EVM. So lets leave the EVM as-is.
> >>
> >> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
> >> the LCDK. Right now it seems to be relying on bootloader to serial2
> >> setup pimux correctly. I can make a patch to fix that. Or if you can do
> >> it, that will be great too.
> >>
> > 
> > Indeed ATM the EVM relies on the bootloader to setup the pin muxing.
> > 
> > I just checked the uart pins routing of the EVM, the dts:
> > - should reclaim serial2_rxtx_pins and serial2_rtscts_pins
> 
> Can you please clarify what you mean by dts should "reclaim". You mean
> move the pins from da850.dtsi to da850-evm.dts or something else?
>

No, in the part you are replying to I am not referring to moving the
pins but reclaiming them. The pins are declared in the dtsi, as they
should. Eg. the LCDK dts reclaims serial2_rxtx_pins but doesn't reclaim
serial2_rtscts_pins while their declarations remain in the dtsi.

The purpose of this part was to clarify what I saw on the schematics
because your emails include the following 2 statements:

> >>> Looks like the flow control pins are being used for McASP also on the
> >>> EVM. So lets leave the EVM as-is.

> >> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
> >> the LCDK. Right now it seems to be relying on bootloader to serial2

My email was to inform that it concerns the flow control pins of the
UART1 only and that the UART2 flow control pins are not concerned and, 
since they are dedicated to the UART2, should be reclaimed unlike
in the LCDK.

 
Karl

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Thursday 25 August 2016 07:45 PM, Karl Beldan wrote:

>>> Indeed ATM the EVM relies on the bootloader to setup the pin muxing.
>>>
>>> I just checked the uart pins routing of the EVM, the dts:
>>> - should reclaim serial2_rxtx_pins and serial2_rtscts_pins
>>
>> Can you please clarify what you mean by dts should "reclaim". You mean
>> move the pins from da850.dtsi to da850-evm.dts or something else?
>>
> 
> No, in the part you are replying to I am not referring to moving the
> pins but reclaiming them. The pins are declared in the dtsi, as they
> should. Eg. the LCDK dts reclaims serial2_rxtx_pins but doesn't reclaim
> serial2_rtscts_pins while their declarations remain in the dtsi.

Alright. I am not sure if "reclaim" is the right term for this, but it
doesn't matter ;) I understand the intent now.

> The purpose of this part was to clarify what I saw on the schematics
> because your emails include the following 2 statements:
> 
>>>>> Looks like the flow control pins are being used for McASP also on the
>>>>> EVM. So lets leave the EVM as-is.
> 
>>>> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
>>>> the LCDK. Right now it seems to be relying on bootloader to serial2
> 
> My email was to inform that it concerns the flow control pins of the
> UART1 only and that the UART2 flow control pins are not concerned and, 
> since they are dedicated to the UART2, should be reclaimed unlike
> in the LCDK.

I see now.

I think I was thrown off by looking at the fact that mcasp0_pins in
da850-evm.dts muxes UART2_RTS as AMUTE. But, its the mcasp0_pins that
needs fixing. AMUTE is not used by audio on the board. As is AFSR. Plus,
that pinmux entry goes and writes to PINMIX0[31:28] the value of 0x1
which is reserved. Sigh.

I will review your original e-mail and reply back.

Thanks,
Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
> On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
>> On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
>>> On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
>>>> Karl Beldan <kbeldan@baylibre.com> writes:
>>>>
>>>>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
>>>>>> Karl Beldan <kbeldan@baylibre.com> writes:
>>>>>>
>>>>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
>>>>>>>
>>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
>>>>>>
>>>>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
>>>>>>
>>>>> I could not find the EVM schematics on the net and I only have an LCDK,
>>>>> but according to the code it should, however I can't tell whether flow
>>>>> control pins are used.
>>>>
>>>> Ok, let's just leave it for now, since it's working fine.  Sekhar can
>>>> fix that up if he can dig up the schematics.
>>>
>>> Looks like the flow control pins are being used for McASP also on the
>>> EVM. So lets leave the EVM as-is.
>>
>> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
>> the LCDK. Right now it seems to be relying on bootloader to serial2
>> setup pimux correctly. I can make a patch to fix that. Or if you can do
>> it, that will be great too.
>>
> 
> Indeed ATM the EVM relies on the bootloader to setup the pin muxing.
> 
> I just checked the uart pins routing of the EVM, the dts:
> - should reclaim serial2_rxtx_pins and serial2_rtscts_pins

Agree.

> - can reclaim serial1_rxtx_pins (out on Audio connector but very
>   unlikely used for audio)

I would leave alone pins which are unused on the board. Most likely the
SPI pins are being send to audio connector for codec control over SPI.

> - should leave serial1_rtscts_pins (out on Audio connector but used by
>   McASP so used for audio)

Agree.

> Also I think it would be better for the serial nodes to reclaim the rxtx
> pins in the dtsi, and override the reclaimed pins in the .dts only for
> the nodes reclaiming flow control (some other pins could also be
> directly reclaimed in the dtsi).

You lost me here. I guess I will benefit from a code snippet to
illustrate the intention.

> 
> Some other cleanups would also be in order for the da850*:
> - use labels for non dtsi (like I did for the LCDK)

Agree. That would be better and more modern.

> - add chosen,memory nodes (I guess currently only the LCDK can
>   dispense with ATAGS)

ok.

> - use a null range translation in the da850 dtsi for the soc node
>   (computing the offsets is error prone and is there a point)

I do agree its easier to read if offsets directly matches addresses
specified in the technical reference manual than doing the math with
offsets and making sure they are correct.

That said, I am not sure null range translation is really the preferred
approach. I would go with what DT maintainers say here.

> 
> Sure I could fix that, along with some of the above suggestions if you
> are ok with it.

It would be nice if you could fixup mcasp0_pins in da850-evm.dts. Surely
it is doing more than what is needed.

Regards,
Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Karl Beldan]

On Fri, Aug 26, 2016 at 05:55:23PM +0530, Sekhar Nori wrote:
> On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
> > On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
> >> On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
> >>> On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
> >>>> Karl Beldan <kbeldan@baylibre.com> writes:
> >>>>
> >>>>> On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
> >>>>>> Karl Beldan <kbeldan@baylibre.com> writes:
> >>>>>>
> >>>>>>> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
> >>>>>>>
> >>>>>>> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>
> >>>>>>
> >>>>>> Should da850-evm be updated to use the serial2_rxtx_pins also?
> >>>>>>
> >>>>> I could not find the EVM schematics on the net and I only have an LCDK,
> >>>>> but according to the code it should, however I can't tell whether flow
> >>>>> control pins are used.
> >>>>
> >>>> Ok, let's just leave it for now, since it's working fine.  Sekhar can
> >>>> fix that up if he can dig up the schematics.
> >>>
> >>> Looks like the flow control pins are being used for McASP also on the
> >>> EVM. So lets leave the EVM as-is.
> >>
> >> Rather, the EVM dts file should be updated to use serial2_rxtx_pins like
> >> the LCDK. Right now it seems to be relying on bootloader to serial2
> >> setup pimux correctly. I can make a patch to fix that. Or if you can do
> >> it, that will be great too.
> >>
> > 
> > Indeed ATM the EVM relies on the bootloader to setup the pin muxing.
> > 
> > I just checked the uart pins routing of the EVM, the dts:
> > - should reclaim serial2_rxtx_pins and serial2_rtscts_pins
> 
> Agree.
> 
> > - can reclaim serial1_rxtx_pins (out on Audio connector but very
> >   unlikely used for audio)
> 
> I would leave alone pins which are unused on the board. Most likely the
> SPI pins are being send to audio connector for codec control over SPI.
> 
> > - should leave serial1_rtscts_pins (out on Audio connector but used by
> >   McASP so used for audio)
> 
> Agree.
> 
> > Also I think it would be better for the serial nodes to reclaim the rxtx
> > pins in the dtsi, and override the reclaimed pins in the .dts only for
> > the nodes reclaiming flow control (some other pins could also be
> > directly reclaimed in the dtsi).
> 
> You lost me here. I guess I will benefit from a code snippet to
> illustrate the intention.
> 

Sure, instead of having: {

### board.dts:
&serialN {
	pinctrl-names = "default";
	pinctrl-0 = <&serialN_rxtx_pins>;
	status = "okay";
};
&serialN+1 {
	pinctrl-names = "default";
	pinctrl-0 = <&serialN+1_rxtx_pins>, <&serialN+1_rtscts_pins>;
	status = "okay";
};

} use ------------------ {

### plat.dtsi:
serialN: serial at xxxxx {
	compatible = "ns16550a";
	pinctrl-0 = <&serialN_rxtx_pins>;
	status = "disabled";
	[...]
};
serialN+1: serial at xxxxx {
	compatible = "ns16550a";
	pinctrl-0 = <&serialN+1_rxtx_pins>;
	status = "disabled";
	[...]
};

### board.dts:
&serialN {
	status = "okay";
};
&serialN+1 {
	pinctrl-0 = <&serialN+1_rxtx_pins>, <&serialN+1_rtscts_pins>;
	status = "okay";
};

}

> > 
> > Some other cleanups would also be in order for the da850*:
> > - use labels for non dtsi (like I did for the LCDK)
> 
> Agree. That would be better and more modern.
> 
> > - add chosen,memory nodes (I guess currently only the LCDK can
> >   dispense with ATAGS)
> 
> ok.
> 
> > - use a null range translation in the da850 dtsi for the soc node
> >   (computing the offsets is error prone and is there a point)
> 
> I do agree its easier to read if offsets directly matches addresses
> specified in the technical reference manual than doing the math with
> offsets and making sure they are correct.
> 
> That said, I am not sure null range translation is really the preferred
> approach. I would go with what DT maintainers say here.
> 
> > 
> > Sure I could fix that, along with some of the above suggestions if you
> > are ok with it.
> 
> It would be nice if you could fixup mcasp0_pins in da850-evm.dts. Surely
> it is doing more than what is needed.
> 

According to the schematics it is indeed, I could squeeze it in the series.

Rgds, 
Karl

> Regards,
> Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Friday 26 August 2016 10:47 PM, Karl Beldan wrote:

>>> Also I think it would be better for the serial nodes to reclaim the rxtx
>>> pins in the dtsi, and override the reclaimed pins in the .dts only for
>>> the nodes reclaiming flow control (some other pins could also be
>>> directly reclaimed in the dtsi).
>>
>> You lost me here. I guess I will benefit from a code snippet to
>> illustrate the intention.
>>
> 
> Sure, instead of having: {
> 
> ### board.dts:
> &serialN {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&serialN_rxtx_pins>;
> 	status = "okay";
> };
> &serialN+1 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&serialN+1_rxtx_pins>, <&serialN+1_rtscts_pins>;
> 	status = "okay";
> };
> 
> } use ------------------ {
> 
> ### plat.dtsi:
> serialN: serial at xxxxx {
> 	compatible = "ns16550a";
> 	pinctrl-0 = <&serialN_rxtx_pins>;
> 	status = "disabled";
> 	[...]
> };
> serialN+1: serial at xxxxx {
> 	compatible = "ns16550a";
> 	pinctrl-0 = <&serialN+1_rxtx_pins>;
> 	status = "disabled";
> 	[...]
> };
> 
> ### board.dts:
> &serialN {
> 	status = "okay";
> };
> &serialN+1 {
> 	pinctrl-0 = <&serialN+1_rxtx_pins>, <&serialN+1_rtscts_pins>;
> 	status = "okay";
> };

I dont think it is a good idea to specify a pinctrl setting in
da850.dtsi and override it in board.dts. Fundamentally I am against
having properties in <soc.dtsi> overridden from <board.dts>.  Sure, it
saves some code in some cases, but it is confusing to read/debug and you
are never sure that the property you seen set in one place is the one
finally taking effect.

The only notable exception is 'status' property. If you see other
exceptions in DaVinci, its only because of my oversight.

>> It would be nice if you could fixup mcasp0_pins in da850-evm.dts. Surely
>> it is doing more than what is needed.
>>
> 
> According to the schematics it is indeed, I could squeeze it in the series.

Thanks!

Regards,
Sekhar

[PATCH] ARM: dts: da850: Add missing pin muxing for the UARTs

[Sekhar Nori]

On Thursday 04 August 2016 04:36 PM, Karl Beldan wrote:
> This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.
> 
> Signed-off-by: Karl Beldan <kbeldan@baylibre.com>

Applying.

Thanks,
Sekhar