| Message ID | 20240426125707.585269-3-christophe.roullier@foss.st.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Series to deliver Ethernets for STM32MP13 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 4/26/24 2:56 PM, Christophe Roullier wrote: > Phandle to a regulator that provides power to the PHY. This > regulator will be managed during the PHY power on/off sequence. > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> Maybe this entire regulator business should be separate series from the MP13 DWMAC ethernet series ?
On Fri, Apr 26, 2024 at 02:56:58PM +0200, Christophe Roullier wrote: > Phandle to a regulator that provides power to the PHY. This > regulator will be managed during the PHY power on/off sequence. > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> > --- > Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml > index b901a432dfa9..7c3aa181abcb 100644 > --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml > @@ -84,6 +84,9 @@ properties: > - description: offset of the control register > - description: field to set mask in register > > + phy-supply: > + description: PHY regulator This is for which PHY? The serdes phy or ethernet phy? This only makes sense here if the phy is part of the MAC. Otherwise, it belongs in the phy node. Rob
Hi, On 4/26/24 16:47, Marek Vasut wrote: > On 4/26/24 2:56 PM, Christophe Roullier wrote: >> Phandle to a regulator that provides power to the PHY. This >> regulator will be managed during the PHY power on/off sequence. >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> > > Maybe this entire regulator business should be separate series from > the MP13 DWMAC ethernet series ? I prefer push it with MP13 Ethernet series if possible.
Hi On 4/26/24 17:30, Rob Herring wrote: > On Fri, Apr 26, 2024 at 02:56:58PM +0200, Christophe Roullier wrote: >> Phandle to a regulator that provides power to the PHY. This >> regulator will be managed during the PHY power on/off sequence. >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >> --- >> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >> index b901a432dfa9..7c3aa181abcb 100644 >> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >> @@ -84,6 +84,9 @@ properties: >> - description: offset of the control register >> - description: field to set mask in register >> >> + phy-supply: >> + description: PHY regulator > This is for which PHY? The serdes phy or ethernet phy? This only makes > sense here if the phy is part of the MAC. Otherwise, it belongs in the > phy node. > > Rob You are right, normally it should be managed in Ethernet PHY (Realtek, Microchip etc...) Lots of glue manage this like this. Does it forbidden now ? if yes need to update PHY driver to manage this property.
On 5/13/24 1:45 PM, Christophe ROULLIER wrote: > Hi, Hi, > On 4/26/24 16:47, Marek Vasut wrote: >> On 4/26/24 2:56 PM, Christophe Roullier wrote: >>> Phandle to a regulator that provides power to the PHY. This >>> regulator will be managed during the PHY power on/off sequence. >>> >>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >> >> Maybe this entire regulator business should be separate series from >> the MP13 DWMAC ethernet series ? > I prefer push it with MP13 Ethernet series if possible. This is separate functionality, independent of the MP13 support and not required for the MP13 support, correct ? If yes, move it into separate patch(set) to make both series easier to review.
On 5/13/24 4:06 PM, Christophe ROULLIER wrote: > Hi > > On 4/26/24 17:30, Rob Herring wrote: >> On Fri, Apr 26, 2024 at 02:56:58PM +0200, Christophe Roullier wrote: >>> Phandle to a regulator that provides power to the PHY. This >>> regulator will be managed during the PHY power on/off sequence. >>> >>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >>> --- >>> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >>> b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >>> index b901a432dfa9..7c3aa181abcb 100644 >>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >>> @@ -84,6 +84,9 @@ properties: >>> - description: offset of the control register >>> - description: field to set mask in register >>> + phy-supply: >>> + description: PHY regulator >> This is for which PHY? The serdes phy or ethernet phy? This only makes >> sense here if the phy is part of the MAC. Otherwise, it belongs in the >> phy node. >> >> Rob > > You are right, normally it should be managed in Ethernet PHY (Realtek, > Microchip etc...) > > Lots of glue manage this like this. Does it forbidden now ? if yes need > to update PHY driver to manage this property. If the regulator is connected to the PHY, then the supply should be described in the PHY node and you wouldn't even need these PHY patches (also see my comment that you should split the PHY regulator part of this patchset into separate series).
diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml index b901a432dfa9..7c3aa181abcb 100644 --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml @@ -84,6 +84,9 @@ properties: - description: offset of the control register - description: field to set mask in register + phy-supply: + description: PHY regulator + st,eth-clk-sel: description: set this property in RGMII PHY when you want to select RCC clock instead of ETH_CLK125.