| Message ID | 20230720-ti-mdio-pinmux-v1-4-0bd3bd1cf759@kernel.org |
|---|---|
| State | Superseded |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl | expand |
On 20/07/2023 12:55, Maxime Ripard wrote: > Dropping ranges entirely doesn't work since the register offset on the > MDIO device node will now be completely off, so we need to adjust it to > the right value without the translation. > > We also need to have an empty ranges property for the reg address to be > properly evaluated. > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > --- > arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi > index fe1c7408a89d..2ec3fff99f32 100644 > --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi > +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi > @@ -122,8 +122,8 @@ > reg = <0x0 0x8000000 0x0 0x200000>, > <0x0 0x43000200 0x0 0x8>; > reg-names = "cpsw_nuss", "mac_efuse"; > - /delete-property/ ranges; > bootph-pre-ram; > + ranges; > > cpsw-phy-sel@04044 { > compatible = "ti,am64-phy-gmii-sel"; > @@ -132,6 +132,10 @@ > }; > }; > > +&cpsw3g_mdio { > + reg = <0x0 0x8000f00 0x0 0x100>; > +}; > + Why this change? Linux DT has cpsw3g_mdio: mdio@f00 { compatible = "ti,cpsw-mdio","ti,davinci_mdio"; reg = <0x00 0xf00 0x00 0x100>; And it is a child of cpsw3g node. > &cpsw_port1 { > bootph-pre-ram; > }; >
On Thu, Jul 20, 2023 at 06:27:56PM +0300, Roger Quadros wrote: > > > On 20/07/2023 12:55, Maxime Ripard wrote: > > Dropping ranges entirely doesn't work since the register offset on the > > MDIO device node will now be completely off, so we need to adjust it to > > the right value without the translation. > > > > We also need to have an empty ranges property for the reg address to be > > properly evaluated. > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > > --- > > arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi > > index fe1c7408a89d..2ec3fff99f32 100644 > > --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi > > +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi > > @@ -122,8 +122,8 @@ > > reg = <0x0 0x8000000 0x0 0x200000>, > > <0x0 0x43000200 0x0 0x8>; > > reg-names = "cpsw_nuss", "mac_efuse"; > > - /delete-property/ ranges; > > bootph-pre-ram; > > + ranges; > > > > cpsw-phy-sel@04044 { > > compatible = "ti,am64-phy-gmii-sel"; > > @@ -132,6 +132,10 @@ > > }; > > }; > > > > +&cpsw3g_mdio { > > + reg = <0x0 0x8000f00 0x0 0x100>; > > +}; > > + > > Why this change? > > Linux DT has > cpsw3g_mdio: mdio@f00 { > compatible = "ti,cpsw-mdio","ti,davinci_mdio"; > reg = <0x00 0xf00 0x00 0x100>; > > And it is a child of cpsw3g node. Right, but Linux also has on the cpsw3g node: ranges = <0x00 0x00 0x00 0x08000000 0x00 0x200000>; Which means that child node don't get a 1:1 mapping but we get a 0x08000000 offset. Nishanth's patch was removing the ranges property because the translation is troublesome for the new cpsw-phy-sel@04044 node (I assume), so we need to add the offset to the mdio node so that it still expresses the same base address. Maxime
On 21/07/2023 10:46, Maxime Ripard wrote: > On Thu, Jul 20, 2023 at 06:27:56PM +0300, Roger Quadros wrote: >> >> >> On 20/07/2023 12:55, Maxime Ripard wrote: >>> Dropping ranges entirely doesn't work since the register offset on the >>> MDIO device node will now be completely off, so we need to adjust it to >>> the right value without the translation. >>> >>> We also need to have an empty ranges property for the reg address to be >>> properly evaluated. >>> >>> Signed-off-by: Maxime Ripard <mripard@kernel.org> >>> --- >>> arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi >>> index fe1c7408a89d..2ec3fff99f32 100644 >>> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi >>> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi >>> @@ -122,8 +122,8 @@ >>> reg = <0x0 0x8000000 0x0 0x200000>, >>> <0x0 0x43000200 0x0 0x8>; >>> reg-names = "cpsw_nuss", "mac_efuse"; >>> - /delete-property/ ranges; >>> bootph-pre-ram; >>> + ranges; >>> >>> cpsw-phy-sel@04044 { >>> compatible = "ti,am64-phy-gmii-sel"; >>> @@ -132,6 +132,10 @@ >>> }; >>> }; >>> >>> +&cpsw3g_mdio { >>> + reg = <0x0 0x8000f00 0x0 0x100>; >>> +}; >>> + >> >> Why this change? >> >> Linux DT has >> cpsw3g_mdio: mdio@f00 { >> compatible = "ti,cpsw-mdio","ti,davinci_mdio"; >> reg = <0x00 0xf00 0x00 0x100>; >> >> And it is a child of cpsw3g node. > > Right, but Linux also has on the cpsw3g node: > ranges = <0x00 0x00 0x00 0x08000000 0x00 0x200000>; > > Which means that child node don't get a 1:1 mapping but we get a > 0x08000000 offset. The child nodes should get 0x08000000 offset because they sit inside CPSW address range. The addition of cpsw-phy-sel@04044 node in u-boot.dtsi is wrong as it sits in the control module and not in CPSW address range. I'll send out the patch to teach am65-cpsw u-boot driver to correctly fetch the cpsw-phy-sel via syscon handle phy. The /delete-property/ ranges can then be removed. > > Nishanth's patch was removing the ranges property because the > translation is troublesome for the new cpsw-phy-sel@04044 node (I > assume), so we need to add the offset to the mdio node so that it still > expresses the same base address. No, ranges property should be there else address translation will fail for MDIO node. And that's why you don't need to change the cpsw3g_mdio address in this patch.
On Fri, Jul 21, 2023 at 12:14:35PM +0300, Roger Quadros wrote: > > > On 21/07/2023 10:46, Maxime Ripard wrote: > > On Thu, Jul 20, 2023 at 06:27:56PM +0300, Roger Quadros wrote: > >> > >> > >> On 20/07/2023 12:55, Maxime Ripard wrote: > >>> Dropping ranges entirely doesn't work since the register offset on the > >>> MDIO device node will now be completely off, so we need to adjust it to > >>> the right value without the translation. > >>> > >>> We also need to have an empty ranges property for the reg address to be > >>> properly evaluated. > >>> > >>> Signed-off-by: Maxime Ripard <mripard@kernel.org> > >>> --- > >>> arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +++++- > >>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi > >>> index fe1c7408a89d..2ec3fff99f32 100644 > >>> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi > >>> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi > >>> @@ -122,8 +122,8 @@ > >>> reg = <0x0 0x8000000 0x0 0x200000>, > >>> <0x0 0x43000200 0x0 0x8>; > >>> reg-names = "cpsw_nuss", "mac_efuse"; > >>> - /delete-property/ ranges; > >>> bootph-pre-ram; > >>> + ranges; > >>> > >>> cpsw-phy-sel@04044 { > >>> compatible = "ti,am64-phy-gmii-sel"; > >>> @@ -132,6 +132,10 @@ > >>> }; > >>> }; > >>> > >>> +&cpsw3g_mdio { > >>> + reg = <0x0 0x8000f00 0x0 0x100>; > >>> +}; > >>> + > >> > >> Why this change? > >> > >> Linux DT has > >> cpsw3g_mdio: mdio@f00 { > >> compatible = "ti,cpsw-mdio","ti,davinci_mdio"; > >> reg = <0x00 0xf00 0x00 0x100>; > >> > >> And it is a child of cpsw3g node. > > > > Right, but Linux also has on the cpsw3g node: > > ranges = <0x00 0x00 0x00 0x08000000 0x00 0x200000>; > > > > Which means that child node don't get a 1:1 mapping but we get a > > 0x08000000 offset. > > The child nodes should get 0x08000000 offset because they sit inside > CPSW address range. Right. > The addition of cpsw-phy-sel@04044 node in u-boot.dtsi is wrong > as it sits in the control module and not in CPSW address range. > > I'll send out the patch to teach am65-cpsw u-boot driver to correctly > fetch the cpsw-phy-sel via syscon handle phy. > > The /delete-property/ ranges can then be removed. Great. > > > > Nishanth's patch was removing the ranges property because the > > translation is troublesome for the new cpsw-phy-sel@04044 node (I > > assume), so we need to add the offset to the mdio node so that it still > > expresses the same base address. > > No, ranges property should be there else address translation will fail > for MDIO node. And that's why you don't need to change the cpsw3g_mdio > address in this patch. Sure. My point was that, with the /delete-property/ ranges above, that translation doesn't occur anymore and the MDIO base address is wrong. This patch is an attempt at fixing that, removing the delete-property is another. Maxime
diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi index fe1c7408a89d..2ec3fff99f32 100644 --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi @@ -122,8 +122,8 @@ reg = <0x0 0x8000000 0x0 0x200000>, <0x0 0x43000200 0x0 0x8>; reg-names = "cpsw_nuss", "mac_efuse"; - /delete-property/ ranges; bootph-pre-ram; + ranges; cpsw-phy-sel@04044 { compatible = "ti,am64-phy-gmii-sel"; @@ -132,6 +132,10 @@ }; }; +&cpsw3g_mdio { + reg = <0x0 0x8000f00 0x0 0x100>; +}; + &cpsw_port1 { bootph-pre-ram; };
Dropping ranges entirely doesn't work since the register offset on the MDIO device node will now be completely off, so we need to adjust it to the right value without the translation. We also need to have an empty ranges property for the reg address to be properly evaluated. Signed-off-by: Maxime Ripard <mripard@kernel.org> --- arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)