| Message ID | 6687de05226dd055ee362933d4841a12b038792d.1601655904.git.npcomplete13@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | ARM: dts: BCM5301X: Linksys EA9500 device tree changes | expand |
On Wed, Oct 07, 2020 at 03:01:50PM -0400, Vivek Unune wrote:
> Forgo the use of mmioreg mdio mux infavor of the pinctrl
Hi Vivek
Could you add some more details please. I don't know this
hardware. I'm assuming there are two MDIO busses, external as talked
about in the comments, and an internal one? And for this hardware you
only need one of them? But i don't see what pinmux has to do with
this?
Thanks
Andrew
On Wed, Oct 07, 2020 at 11:01:34PM +0200, Andrew Lunn wrote: > On Wed, Oct 07, 2020 at 03:01:50PM -0400, Vivek Unune wrote: > > Forgo the use of mmioreg mdio mux infavor of the pinctrl > > Hi Vivek > > Could you add some more details please. I don't know this > hardware. I'm assuming there are two MDIO busses, external as talked > about in the comments, and an internal one? And for this hardware you > only need one of them? But i don't see what pinmux has to do with > this? Hi Andrew, There are indeed two mdio busses. To access the external bus, 9th bit of the mdio register has to be set. And to enable mii function, one has to set the registers 6 & 7 which is part of the pin controller. Earlier the pin controller was not defined and I resorted to use a combination of memory mapped io mux to change desired bits. Now that we have a pin controller - which is resposnsible for other functionality such as pwm, i2c, uart2, it makes sense to have a consistent device tree Hope this helps, Vivek > > Thanks > Andrew >
On Wed, Oct 07, 2020 at 05:46:33PM -0400, Vivek Unune wrote: > On Wed, Oct 07, 2020 at 11:01:34PM +0200, Andrew Lunn wrote: > > On Wed, Oct 07, 2020 at 03:01:50PM -0400, Vivek Unune wrote: > > > Forgo the use of mmioreg mdio mux infavor of the pinctrl > > > > Hi Vivek > > > > Could you add some more details please. I don't know this > > hardware. I'm assuming there are two MDIO busses, external as talked > > about in the comments, and an internal one? And for this hardware you > > only need one of them? But i don't see what pinmux has to do with > > this? > Hi Andrew, > > There are indeed two mdio busses. To access the external bus, 9th bit > of the mdio register has to be set. And to enable mii function, > one has to set the registers 6 & 7 which is part of the pin controller. > Earlier the pin controller was not defined and I resorted to use a > combination of memory mapped io mux to change desired bits. > > Now that we have a pin controller - which is resposnsible for other > functionality such as pwm, i2c, uart2, it makes sense to have a consistent > device tree What makes it confusing is that you make multiple changes at once. It would be easier to follow if you added the pinmux and removed the mmioreg mux, and move the switch into the mdio-bus-mux node. Then in a second patch rearrange the mdio-bus-mux. Small simple steps, with good commit messages are much easier to follow and say, Yes, this is correct. Andrew
On Thu, Oct 08, 2020 at 02:26:21AM +0200, Andrew Lunn wrote: > On Wed, Oct 07, 2020 at 05:46:33PM -0400, Vivek Unune wrote: > > On Wed, Oct 07, 2020 at 11:01:34PM +0200, Andrew Lunn wrote: > > > On Wed, Oct 07, 2020 at 03:01:50PM -0400, Vivek Unune wrote: > > > > Forgo the use of mmioreg mdio mux infavor of the pinctrl > > > > > > Hi Vivek > > > > > > Could you add some more details please. I don't know this > > > hardware. I'm assuming there are two MDIO busses, external as talked > > > about in the comments, and an internal one? And for this hardware you > > > only need one of them? But i don't see what pinmux has to do with > > > this? > > Hi Andrew, > > > > There are indeed two mdio busses. To access the external bus, 9th bit > > of the mdio register has to be set. And to enable mii function, > > one has to set the registers 6 & 7 which is part of the pin controller. > > Earlier the pin controller was not defined and I resorted to use a > > combination of memory mapped io mux to change desired bits. > > > > Now that we have a pin controller - which is resposnsible for other > > functionality such as pwm, i2c, uart2, it makes sense to have a consistent > > device tree > > What makes it confusing is that you make multiple changes at once. It > would be easier to follow if you added the pinmux and removed the > mmioreg mux, and move the switch into the mdio-bus-mux node. Then in a > second patch rearrange the mdio-bus-mux. Small simple steps, with good > commit messages are much easier to follow and say, Yes, this is > correct. > Sure, le me declutter this. Thanks, Vivek
On 11/4/2020 12:29 PM, Vivek Unune wrote: > Now that we have a pin controller, use that instead of manuplating the > mdio/mdc pins directly. i.e. we no longer require the mdio-mii-mux I am a bit confused here as I thought the mux was intended to dynamically switch the pins in order to support both internal and external MDIO devices but given the register ranges that were used, these were actually the pinmux configuration for the MDC and MDIO pins. This does not break USB and/or PCIe PHY communication does it?
On Wed, Nov 04, 2020 at 12:37:45PM -0800, Florian Fainelli wrote: > > > On 11/4/2020 12:29 PM, Vivek Unune wrote: > > Now that we have a pin controller, use that instead of manuplating the > > mdio/mdc pins directly. i.e. we no longer require the mdio-mii-mux > > I am a bit confused here as I thought the mux was intended to > dynamically switch the pins in order to support both internal and > external MDIO devices but given the register ranges that were used, > these were actually the pinmux configuration for the MDC and MDIO pins. > > This does not break USB and/or PCIe PHY communication does it? Hi Florian, The external and internal MDIO logic is controlled by mdio-bus-mux. Which controls the BIT(9) of the mdio register. This stays. The removal of mdio-mii-mux and it's replacement with usage of pinctrl doesn't affect USB3 or PCIe. See below USB3 detection. [ 4295.450118] usb 1-1: new high-speed USB device number 2 using ehci-platform [ 4295.690183] usb 4-1: new SuperSpeed Gen 1 USB device number 2 using xhci-hcd [ 4295.721888] usb-storage 4-1:1.0: USB Mass Storage device detected [ 4295.728349] scsi host0: usb-storage 4-1:1.0 [ 4296.811047] scsi 0:0:0:0: Direct-Access SanDisk Ultra Fit 1.00 PQ: 0 ANSI: 6 [ 4296.821159] sd 0:0:0:0: [sda] 60063744 512-byte logical blocks: (30.8 GB/28.6 GiB) [ 4296.829667] sd 0:0:0:0: [sda] Write Protect is off [ 4296.834502] sd 0:0:0:0: [sda] Mode Sense: 43 00 00 00 [ 4296.834864] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA [ 4296.852604] GPT:Primary header thinks Alt. header is not at the end of the disk. [ 4296.860079] GPT:1540387 != 60063743 [ 4296.863586] GPT:Alternate GPT header not at the end of the disk. [ 4296.869600] GPT:1540387 != 60063743 [ 4296.873090] GPT: Use GNU Parted to correct GPT errors. [ 4296.878266] sda: sda1 sda2 [ 4296.884416] sd 0:0:0:0: [sda] Attached SCSI removable disk Thanks, Vivek
On Wed, Nov 4, 2020 at 3:58 PM Vivek Unune <npcomplete13@gmail.com> wrote: > > On Wed, Nov 04, 2020 at 12:37:45PM -0800, Florian Fainelli wrote: > > > > > > On 11/4/2020 12:29 PM, Vivek Unune wrote: > > > Now that we have a pin controller, use that instead of manuplating the > > > mdio/mdc pins directly. i.e. we no longer require the mdio-mii-mux > > > > I am a bit confused here as I thought the mux was intended to > > dynamically switch the pins in order to support both internal and > > external MDIO devices but given the register ranges that were used, > > these were actually the pinmux configuration for the MDC and MDIO pins. > > > > This does not break USB and/or PCIe PHY communication does it? > > Hi Florian, > > The external and internal MDIO logic is controlled by mdio-bus-mux. > Which controls the BIT(9) of the mdio register. This stays. > > The removal of mdio-mii-mux and it's replacement with usage of > pinctrl doesn't affect USB3 or PCIe. See below USB3 detection. > > [ 4295.450118] usb 1-1: new high-speed USB device number 2 using ehci-platform > [ 4295.690183] usb 4-1: new SuperSpeed Gen 1 USB device number 2 using xhci-hcd > [ 4295.721888] usb-storage 4-1:1.0: USB Mass Storage device detected > [ 4295.728349] scsi host0: usb-storage 4-1:1.0 > [ 4296.811047] scsi 0:0:0:0: Direct-Access SanDisk Ultra Fit 1.00 PQ: 0 ANSI: 6 > [ 4296.821159] sd 0:0:0:0: [sda] 60063744 512-byte logical blocks: (30.8 GB/28.6 GiB) > [ 4296.829667] sd 0:0:0:0: [sda] Write Protect is off > [ 4296.834502] sd 0:0:0:0: [sda] Mode Sense: 43 00 00 00 > [ 4296.834864] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA > [ 4296.852604] GPT:Primary header thinks Alt. header is not at the end of the disk. > [ 4296.860079] GPT:1540387 != 60063743 > [ 4296.863586] GPT:Alternate GPT header not at the end of the disk. > [ 4296.869600] GPT:1540387 != 60063743 > [ 4296.873090] GPT: Use GNU Parted to correct GPT errors. > [ 4296.878266] sda: sda1 sda2 > [ 4296.884416] sd 0:0:0:0: [sda] Attached SCSI removable disk > Hi Florian, Does this clarify your confusion? Thanks, Vivek
On 11/9/2020 5:24 AM, Vivek Unune wrote: > On Wed, Nov 4, 2020 at 3:58 PM Vivek Unune <npcomplete13@gmail.com> wrote: >> >> On Wed, Nov 04, 2020 at 12:37:45PM -0800, Florian Fainelli wrote: >>> >>> >>> On 11/4/2020 12:29 PM, Vivek Unune wrote: >>>> Now that we have a pin controller, use that instead of manuplating the >>>> mdio/mdc pins directly. i.e. we no longer require the mdio-mii-mux >>> >>> I am a bit confused here as I thought the mux was intended to >>> dynamically switch the pins in order to support both internal and >>> external MDIO devices but given the register ranges that were used, >>> these were actually the pinmux configuration for the MDC and MDIO pins. >>> >>> This does not break USB and/or PCIe PHY communication does it? >> >> Hi Florian, >> >> The external and internal MDIO logic is controlled by mdio-bus-mux. >> Which controls the BIT(9) of the mdio register. This stays. >> >> The removal of mdio-mii-mux and it's replacement with usage of >> pinctrl doesn't affect USB3 or PCIe. See below USB3 detection. >> >> [ 4295.450118] usb 1-1: new high-speed USB device number 2 using ehci-platform >> [ 4295.690183] usb 4-1: new SuperSpeed Gen 1 USB device number 2 using xhci-hcd >> [ 4295.721888] usb-storage 4-1:1.0: USB Mass Storage device detected >> [ 4295.728349] scsi host0: usb-storage 4-1:1.0 >> [ 4296.811047] scsi 0:0:0:0: Direct-Access SanDisk Ultra Fit 1.00 PQ: 0 ANSI: 6 >> [ 4296.821159] sd 0:0:0:0: [sda] 60063744 512-byte logical blocks: (30.8 GB/28.6 GiB) >> [ 4296.829667] sd 0:0:0:0: [sda] Write Protect is off >> [ 4296.834502] sd 0:0:0:0: [sda] Mode Sense: 43 00 00 00 >> [ 4296.834864] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA >> [ 4296.852604] GPT:Primary header thinks Alt. header is not at the end of the disk. >> [ 4296.860079] GPT:1540387 != 60063743 >> [ 4296.863586] GPT:Alternate GPT header not at the end of the disk. >> [ 4296.869600] GPT:1540387 != 60063743 >> [ 4296.873090] GPT: Use GNU Parted to correct GPT errors. >> [ 4296.878266] sda: sda1 sda2 >> [ 4296.884416] sd 0:0:0:0: [sda] Attached SCSI removable disk >> > > Hi Florian, > > Does this clarify your confusion? It does, thank you for bearing with me, I will apply this later today.
diff --git a/arch/arm/boot/dts/bcm47094-linksys-panamera.dts b/arch/arm/boot/dts/bcm47094-linksys-panamera.dts index 0faae8950375..f8443d9f86b7 100644 --- a/arch/arm/boot/dts/bcm47094-linksys-panamera.dts +++ b/arch/arm/boot/dts/bcm47094-linksys-panamera.dts @@ -122,87 +122,6 @@ bluebar8 { gpios = <&chipcommon 8 GPIO_ACTIVE_HIGH>; }; }; - - mdio-bus-mux { - #address-cells = <1>; - #size-cells = <0>; - - /* BIT(9) = 1 => external mdio */ - mdio_ext: mdio@200 { - reg = <0x200>; - #address-cells = <1>; - #size-cells = <0>; - }; - }; - - mdio-mii-mux { - compatible = "mdio-mux-mmioreg"; - mdio-parent-bus = <&mdio_ext>; - #address-cells = <1>; - #size-cells = <0>; - reg = <0x1800c1c0 0x4>; - - /* BIT(6) = mdc, BIT(7) = mdio */ - mux-mask = <0xc0>; - - mdio-mii@0 { - /* Enable MII function */ - reg = <0x0>; - #address-cells = <1>; - #size-cells = <0>; - - switch@0 { - compatible = "brcm,bcm53125"; - #address-cells = <1>; - #size-cells = <0>; - reset-gpios = <&chipcommon 10 GPIO_ACTIVE_LOW>; - reset-names = "robo_reset"; - reg = <0>; - dsa,member = <1 0>; - - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; - label = "lan1"; - }; - - port@1 { - reg = <1>; - label = "lan5"; - }; - - port@2 { - reg = <2>; - label = "lan2"; - }; - - port@3 { - reg = <3>; - label = "lan6"; - }; - - port@4 { - reg = <4>; - label = "lan3"; - }; - - sw1_p8: port@8 { - reg = <8>; - ethernet = <&sw0_p0>; - label = "cpu"; - - fixed-link { - speed = <1000>; - full-duplex; - }; - }; - }; - }; - }; - }; }; &usb2 { @@ -265,6 +184,78 @@ fixed-link { }; }; +&pinctrl { + compatible = "brcm,bcm4709-pinmux"; + + pinmux_mdio: mdio { + groups = "mdio_grp"; + function = "mdio"; + }; +}; + +&mdio_bus_mux { + + /* BIT(9) = 1 => external mdio */ + mdio@200 { + reg = <0x200>; + #address-cells = <1>; + #size-cells = <0>; + + switch@0 { + compatible = "brcm,bcm53125"; + #address-cells = <1>; + #size-cells = <0>; + reset-gpios = <&chipcommon 10 GPIO_ACTIVE_LOW>; + reset-names = "robo_reset"; + reg = <0>; + dsa,member = <1 0>; + pinctrl-names = "default"; + pinctrl-0 = <&pinmux_mdio>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + label = "lan1"; + }; + + port@1 { + reg = <1>; + label = "lan5"; + }; + + port@2 { + reg = <2>; + label = "lan2"; + }; + + port@3 { + reg = <3>; + label = "lan6"; + }; + + port@4 { + reg = <4>; + label = "lan3"; + }; + + sw1_p8: port@8 { + reg = <8>; + ethernet = <&sw0_p0>; + label = "cpu"; + + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + }; + }; + }; +}; + &usb3_phy { status = "okay"; }; diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi index 9d9e8fe3f6ae..1476375f88bb 100644 --- a/arch/arm/boot/dts/bcm5301x.dtsi +++ b/arch/arm/boot/dts/bcm5301x.dtsi @@ -369,7 +369,7 @@ mdio: mdio@18003000 { #address-cells = <1>; }; - mdio-bus-mux@18003000 { + mdio_bus_mux: mdio-bus-mux@18003000 { compatible = "mdio-mux-mmioreg"; mdio-parent-bus = <&mdio>; #address-cells = <1>; @@ -428,7 +428,7 @@ cru@100 { #address-cells = <1>; #size-cells = <1>; - pin-controller@1c0 { + pinctrl: pin-controller@1c0 { compatible = "brcm,bcm4708-pinmux"; reg = <0x1c0 0x24>; reg-names = "cru_gpio_control";
Forgo the use of mmioreg mdio mux infavor of the pinctrl Signed-off-by: Vivek Unune <npcomplete13@gmail.com> --- .../boot/dts/bcm47094-linksys-panamera.dts | 153 +++++++++--------- arch/arm/boot/dts/bcm5301x.dtsi | 4 +- 2 files changed, 74 insertions(+), 83 deletions(-)