Skip to contentSkip to conversation

pinctrl: tegra-xusb: Correct lane mux options

PATCH In review Jon Hunter 3 participants 10 messages Oct 16, 2015 – Oct 27, 2015

Subsystems (3)
lore
Share & tools
lore

Jon Hunter

author maintainer
#0 lore raw

PATCH 1/1 pinctrl: tegra-xusb: Correct lane mux options

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124 documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

Function: Lanes:
pcie1 x2: pcie3, pcie4
pcie1 x4: pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1): pcie0
pcie2 x1 (option2): pcie2
usb3 port 0: pcie0
usb3 port 1 (option 1): pcie1
usb3 port 1 (option 2): sata0
sata: sata0

---
 drivers/pinctrl/pinctrl-tegra-xusb.c | 9 ---------
 1 file changed, 9 deletions(-)
drivers/pinctrl/pinctrl-tegra-xusb.c +0 -9
index 2651d04bd1be..84a43e612952 100644
@@ -760,24 +760,15 @@ static const char * const tegra124_pcie_groups[] = {
 	"pcie-2",
 	"pcie-3",
 	"pcie-4",
-	"sata-0",
 };
 
 static const char * const tegra124_usb3_groups[] = {
 	"pcie-0",
 	"pcie-1",
-	"pcie-2",
-	"pcie-3",
-	"pcie-4",
 	"sata-0",
 };
 
 static const char * const tegra124_sata_groups[] = {
-	"pcie-0",
-	"pcie-1",
-	"pcie-2",
-	"pcie-3",
-	"pcie-4",
 	"sata-0",
 };
 
Signature · 1 line
2.1.4
#1 lore raw
Quoted 6 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124 documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

FWIW, there's better documentation of this in the Tegra210 TRM, although the options have been expanded on that chip, so the docs don't entirely apply to Tegra124.

Quoted 9 lines

Function: Lanes:
pcie1 x2: pcie3, pcie4
pcie1 x4: pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1): pcie0
pcie2 x1 (option2): pcie2
usb3 port 0: pcie0
usb3 port 1 (option 1): pcie1
usb3 port 1 (option 2): sata0
sata: sata0

I think this change needs a DT binding change to go along with it. Can you take a look at:

http://www.spinics.net/lists/arm-kernel/msg449647.html
[PATCH 1/2] dt: update Tegra XUSB padctl binding for Tegra210

(Sorry, I didn't realize anyone other than Thierry and Andrew were working on XUSB/padctl so didn't explicitly CC you on that.)

... to see what would need to be changed there? Or from a binding perspective should we simply assume that people will refer to the HW docs (or other information sources) for the exact list of available options?

author maintainer
#2 lore raw
Quoted 10 lines · Stephen Warren
Quoted 8 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124
documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more
limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

Quoted 3 lines

FWIW, there's better documentation of this in the Tegra210 TRM, although the options have been expanded on that chip, so the docs don't entirely apply to Tegra124.

Yes I have been trying to get this better documented.

Quoted 16 lines
Quoted 9 lines
Function:        Lanes:
pcie1 x2:        pcie3, pcie4
pcie1 x4:        pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):    pcie0
pcie2 x1 (option2):    pcie2
usb3 port 0:        pcie0
usb3 port 1 (option 1):    pcie1
usb3 port 1 (option 2):    sata0
sata:            sata0

I think this change needs a DT binding change to go along with it. Can you take a look at:

http://www.spinics.net/lists/arm-kernel/msg449647.html
[PATCH 1/2] dt: update Tegra XUSB padctl binding for Tegra210

Quoted 7 lines

(Sorry, I didn't realize anyone other than Thierry and Andrew were working on XUSB/padctl so didn't explicitly CC you on that.)

... to see what would need to be changed there? Or from a binding perspective should we simply assume that people will refer to the HW docs (or other information sources) for the exact list of available options?

Yes may be the above needs to go in the binding doc. For t124, I don't think that the TRM will get updated now.

Jon

author maintainer
#3 lore raw
In reply to Stephen Warren
Quoted 24 lines · Stephen Warren
Quoted 8 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124
documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more
limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

FWIW, there's better documentation of this in the Tegra210 TRM, although the options have been expanded on that chip, so the docs don't entirely apply to Tegra124.

Quoted 9 lines
Function:        Lanes:
pcie1 x2:        pcie3, pcie4
pcie1 x4:        pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):    pcie0
pcie2 x1 (option2):    pcie2
usb3 port 0:        pcie0
usb3 port 1 (option 1):    pcie1
usb3 port 1 (option 2):    sata0
sata:            sata0
Quoted 5 lines

I think this change needs a DT binding change to go along with it. Can you take a look at:

http://www.spinics.net/lists/arm-kernel/msg449647.html
[PATCH 1/2] dt: update Tegra XUSB padctl binding for Tegra210

I took a look at the above and it looks fine to me. Do you want me to put the above info into the DT binding doc? I am not sure that we need to update the binding itself.

Jon

#4 lore raw
Quoted 29 lines · Jon Hunter
Quoted 27 lines · Stephen Warren
Quoted 8 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124
documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more
limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

FWIW, there's better documentation of this in the Tegra210 TRM, although the options have been expanded on that chip, so the docs don't entirely apply to Tegra124.

Quoted 9 lines
Function:        Lanes:
pcie1 x2:        pcie3, pcie4
pcie1 x4:        pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):    pcie0
pcie2 x1 (option2):    pcie2
usb3 port 0:        pcie0
usb3 port 1 (option 1):    pcie1
usb3 port 1 (option 2):    sata0
sata:            sata0

I think this change needs a DT binding change to go along with it. Can you take a look at:

Quoted 6 lines
Quoted 2 lines

http://www.spinics.net/lists/arm-kernel/msg449647.html
[PATCH 1/2] dt: update Tegra XUSB padctl binding for Tegra210

I took a look at the above and it looks fine to me. Do you want me to put the above info into the DT binding doc? I am not sure that we need to update the binding itself.

Hmm. I guess there /should/ be no need for the DT bindings to list out all the valid combinations; it should just say "go read the HW docs". Of course as you mentioned our HW docs aren't quite as complete as they should be in this area, but still solving that in the DT binding doc may not be the best approach. But then again, the DT binding doc already lists which functions are valid for which groups of pins, but perhaps that's more about understanding the structure of the binding than the HW.

I guess I'll leave it up to you which way to go. Perhaps let's not pursue adding this to the binding doc until we get the PHY-per-lane changes in place or rejected or the two changes will conflict badly?

author maintainer
#5 lore raw
Quoted 39 lines · Stephen Warren
Quoted 31 lines · Jon Hunter
Quoted 29 lines · Stephen Warren
Quoted 10 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124
documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has
been confirmed that this is not the case and the mux'ing options much more
limited. Unfortunately, the public documentation has not been updated to
reflect this and so detail the actual mux'ing options here by function:

FWIW, there's better documentation of this in the Tegra210 TRM, although the options have been expanded on that chip, so the docs don't entirely apply to Tegra124.

Quoted 9 lines
Function:        Lanes:
pcie1 x2:        pcie3, pcie4
pcie1 x4:        pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):    pcie0
pcie2 x1 (option2):    pcie2
usb3 port 0:        pcie0
usb3 port 1 (option 1):    pcie1
usb3 port 1 (option 2):    sata0
sata:            sata0

I think this change needs a DT binding change to go along with it. Can you take a look at:

Quoted 6 lines
Quoted 2 lines

http://www.spinics.net/lists/arm-kernel/msg449647.html
[PATCH 1/2] dt: update Tegra XUSB padctl binding for Tegra210

I took a look at the above and it looks fine to me. Do you want me to put the above info into the DT binding doc? I am not sure that we need to update the binding itself.

Quoted 7 lines

Hmm. I guess there /should/ be no need for the DT bindings to list out all the valid combinations; it should just say "go read the HW docs". Of course as you mentioned our HW docs aren't quite as complete as they should be in this area, but still solving that in the DT binding doc may not be the best approach. But then again, the DT binding doc already lists which functions are valid for which groups of pins, but perhaps that's more about understanding the structure of the binding than the HW.

I had thought about trying to put the options in the tegra124.dtsi, but I am not sure if there is an easy way to do that without having ...

	padctl@0,7009f000 {
		...

		padctl_option1: pinmux {
			usb3 {...};		
			pcie {...};
			sata {...};
		};
		padctl_option2: pinmux {
			usb3 {...};
			pcie {...};
			sata {...};
		};
		...
		padctl_optionN: pinmux {
			usb3 {...};
			pcie {...};
			sata {...};
		};
	};

... that would be a long-ish list. Unless there is a better way to do it?

Quoted 3 lines

I guess I'll leave it up to you which way to go. Perhaps let's not pursue adding this to the binding doc until we get the PHY-per-lane changes in place or rejected or the two changes will conflict badly?

That's fine with me. Are you ok with this patch as-is going upstream for now?

Jon

#6 lore raw
Quoted 73 lines · Jon Hunter
Quoted 39 lines · Stephen Warren
Quoted 31 lines · Jon Hunter
Quoted 29 lines · Stephen Warren
Quoted 10 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124
documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has
been confirmed that this is not the case and the mux'ing options much more
limited. Unfortunately, the public documentation has not been updated to
reflect this and so detail the actual mux'ing options here by function:

FWIW, there's better documentation of this in the Tegra210 TRM, although the options have been expanded on that chip, so the docs don't entirely apply to Tegra124.

Quoted 9 lines
Function:        Lanes:
pcie1 x2:        pcie3, pcie4
pcie1 x4:        pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):    pcie0
pcie2 x1 (option2):    pcie2
usb3 port 0:        pcie0
usb3 port 1 (option 1):    pcie1
usb3 port 1 (option 2):    sata0
sata:            sata0

I think this change needs a DT binding change to go along with it. Can you take a look at:

Quoted 6 lines
Quoted 2 lines

http://www.spinics.net/lists/arm-kernel/msg449647.html
[PATCH 1/2] dt: update Tegra XUSB padctl binding for Tegra210

I took a look at the above and it looks fine to me. Do you want me to put the above info into the DT binding doc? I am not sure that we need to update the binding itself.

Quoted 7 lines

Hmm. I guess there /should/ be no need for the DT bindings to list out all the valid combinations; it should just say "go read the HW docs". Of course as you mentioned our HW docs aren't quite as complete as they should be in this area, but still solving that in the DT binding doc may not be the best approach. But then again, the DT binding doc already lists which functions are valid for which groups of pins, but perhaps that's more about understanding the structure of the binding than the HW.

I had thought about trying to put the options in the tegra124.dtsi, but I am not sure if there is an easy way to do that without having ...

	padctl@0,7009f000 {
		...

		padctl_option1: pinmux {
			usb3 {...};		
			pcie {...};
			sata {...};
		};
		padctl_option2: pinmux {
			usb3 {...};
			pcie {...};
			sata {...};
		};
		...
		padctl_optionN: pinmux {
			usb3 {...};
			pcie {...};
			sata {...};
		};
	};
Quoted 1 line

... that would be a long-ish list. Unless there is a better way to do it?

Well, you could break it down to N options per controller type or port rather than all N global options which might simplify things a bit (fewer combinatorics to worry about), but I expect it'd still be more trouble than its worth. People designing HW are who actually care about the set of valid options, and they're more likely to get that information via talking to NV systems engineers rather than looking at the DT binding.

Quoted 6 lines
Quoted 3 lines

I guess I'll leave it up to you which way to go. Perhaps let's not pursue adding this to the binding doc until we get the PHY-per-lane changes in place or rejected or the two changes will conflict badly?

That's fine with me. Are you ok with this patch as-is going upstream for now?

Yes, the code change is fine as is.

author maintainer
#7 lore raw
Quoted 34 lines · Stephen Warren
Quoted 33 lines · Jon Hunter
Quoted 31 lines · Stephen Warren
Quoted 30 lines · Jon Hunter
Quoted 28 lines · Stephen Warren
Quoted 11 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124
documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has
been confirmed that this is not the case and the mux'ing options much more
limited. Unfortunately, the public documentation has not been updated to
reflect this and so detail the actual mux'ing options here by function:

FWIW, there's better documentation of this in the Tegra210 TRM, although
the options have been expanded on that chip, so the docs don't entirely
apply to Tegra124.

Quoted 9 lines
Function:        Lanes:
pcie1 x2:        pcie3, pcie4
pcie1 x4:        pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):    pcie0
pcie2 x1 (option2):    pcie2
usb3 port 0:        pcie0
usb3 port 1 (option 1):    pcie1
usb3 port 1 (option 2):    sata0
sata:            sata0

[snip]

Quoted 4 lines
Quoted 2 lines

That's fine with me. Are you ok with this patch as-is going upstream for now?

Yes, the code change is fine as is.

Care to ACK this so Linus can pick it up?

Cheers
Jon

Acked-by on v1 1/1
#8 lore raw
In reply to Jon Hunter
Quoted 7 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124 documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

Quoted 9 lines

Function: Lanes:
pcie1 x2: pcie3, pcie4
pcie1 x4: pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1): pcie0
pcie2 x1 (option2): pcie2
usb3 port 0: pcie0
usb3 port 1 (option 1): pcie1
usb3 port 1 (option 2): sata0
sata: sata0

I didn't check the actual lists of values, but it sounds about right from memory.

#9 lore raw
In reply to Jon Hunter
Quoted 17 lines · Jon Hunter

The description of the XUSB_PADCTL_USB3_PAD_MUX_0 register in the Tegra124 documentation implies that all functions (pcie, usb3 and sata) can be muxed onto to all lanes (pcie lanes 0-4 and sata lane 0). However, it has been confirmed that this is not the case and the mux'ing options much more limited. Unfortunately, the public documentation has not been updated to reflect this and so detail the actual mux'ing options here by function:

Function:               Lanes:
pcie1 x2:               pcie3, pcie4
pcie1 x4:               pcie1, pcie2, pcie3, pcie4
pcie2 x1 (option1):     pcie0
pcie2 x1 (option2):     pcie2
usb3 port 0:            pcie0
usb3 port 1 (option 1): pcie1
usb3 port 1 (option 2): sata0
sata:                   sata0
Quoted 1 line

Patch applied with Stephen's ACK.

Yours,
Linus Walleij