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

7 messages, 3 authors, 2016-08-30 · open the first message on its own page

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

From: Sekhar Nori <hidden>
Date: 2016-08-23 11:09:08

On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
Karl Beldan [off-list ref] writes:
quoted
On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <redacted>
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

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

From: Sekhar Nori <hidden>
Date: 2016-08-23 11:16:03

On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <redacted>
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

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

From: Karl Beldan <hidden>
Date: 2016-08-24 08:38:15

On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
quoted
On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <redacted>
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

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

From: Sekhar Nori <hidden>
Date: 2016-08-25 13:42:16

On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
quoted
On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
quoted
On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <redacted>
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

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

From: Sekhar Nori <hidden>
Date: 2016-08-26 12:25:23

On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
quoted
On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
quoted
On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <redacted>
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

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

From: Karl Beldan <hidden>
Date: 2016-08-26 17:17:18

On Fri, Aug 26, 2016 at 05:55:23PM +0530, Sekhar Nori wrote:
On Wednesday 24 August 2016 02:08 PM, Karl Beldan wrote:
quoted
On Tue, Aug 23, 2016 at 04:46:03PM +0530, Sekhar Nori wrote:
quoted
On Tuesday 23 August 2016 04:39 PM, Sekhar Nori wrote:
quoted
On Friday 05 August 2016 04:30 AM, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
On Thu, Aug 04, 2016 at 12:20:27PM -0700, Kevin Hilman wrote:
quoted
Karl Beldan [off-list ref] writes:
quoted
This adds 2 pinctrl groups (rtscts, rxtx) for each of the 3 UARTs.

Signed-off-by: Karl Beldan <redacted>
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.
quoted
- 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.
quoted
- should leave serial1_rtscts_pins (out on Audio connector but used by
  McASP so used for audio)
Agree.
quoted
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";
};

}
quoted
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.
quoted
- add chosen,memory nodes (I guess currently only the LCDK can
  dispense with ATAGS)
ok.
quoted
- 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.
quoted
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

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

From: Sekhar Nori <hidden>
Date: 2016-08-30 08:54:53

On Friday 26 August 2016 10:47 PM, Karl Beldan wrote:
quoted
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help