| Message ID | 20220313002536.13068-2-michael@walle.cc |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | net: mscc-miim: add integrated PHY reset support | expand |
| Context | Check | Description |
|---|---|---|
| robh/patch-applied | success | |
| robh/checkpatch | success | |
| robh/dtbs-check | success | |
| robh/dt-meta-schema | success |
On 13/03/2022 01:25, Michael Walle wrote: > The MDIO controller has support to release the internal PHYs from reset > by specifying a second memory resource. This is different between the > currently supported SparX-5 and the LAN966x. Add a new compatible to > distiguish between these two. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt b/Documentation/devicetree/bindings/net/mscc-miim.txt > index 7104679cf59d..a9efff252ca6 100644 > --- a/Documentation/devicetree/bindings/net/mscc-miim.txt > +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt > @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO > ================================================= > > Properties: > -- compatible: must be "mscc,ocelot-miim" > +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" No wildcards, use one, specific compatible. Best regards, Krzysztof
Hi Krzysztof, Am 2022-03-13 10:47, schrieb Krzysztof Kozlowski: > On 13/03/2022 01:25, Michael Walle wrote: >> The MDIO controller has support to release the internal PHYs from >> reset >> by specifying a second memory resource. This is different between the >> currently supported SparX-5 and the LAN966x. Add a new compatible to >> distiguish between these two. >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt >> b/Documentation/devicetree/bindings/net/mscc-miim.txt >> index 7104679cf59d..a9efff252ca6 100644 >> --- a/Documentation/devicetree/bindings/net/mscc-miim.txt >> +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt >> @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO >> ================================================= >> >> Properties: >> -- compatible: must be "mscc,ocelot-miim" >> +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" > > No wildcards, use one, specific compatible. I'm in a kind of dilemma here, have a look yourself: grep -r "lan966[28x]-" Documentation Should I deviate from the common "name" now? To make things worse, there was a similar request by Arnd [1]. But the solution feels like cheating ("lan966x" -> "lan966") ;) On a side note, I understand that there should be no wildcards, because the compatible should target one specific implementation, right? But then the codename "ocelot" represents a whole series of chips. Therefore, names for whole families shouldn't be used neither, right? -michael [1] https://lore.kernel.org/lkml/CAK8P3a2kRhCOoXnvcMyqS-zK2WDZjtUq4aqOzE5VV=VMg=pVOA@mail.gmail.com/
On 13/03/2022 11:47, Michael Walle wrote: > Hi Krzysztof, > > Am 2022-03-13 10:47, schrieb Krzysztof Kozlowski: >> On 13/03/2022 01:25, Michael Walle wrote: >>> The MDIO controller has support to release the internal PHYs from >>> reset >>> by specifying a second memory resource. This is different between the >>> currently supported SparX-5 and the LAN966x. Add a new compatible to >>> distiguish between these two. Typo here, BTW. >>> >>> Signed-off-by: Michael Walle <michael@walle.cc> >>> --- >>> Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt >>> b/Documentation/devicetree/bindings/net/mscc-miim.txt >>> index 7104679cf59d..a9efff252ca6 100644 >>> --- a/Documentation/devicetree/bindings/net/mscc-miim.txt >>> +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt >>> @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO >>> ================================================= >>> >>> Properties: >>> -- compatible: must be "mscc,ocelot-miim" >>> +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" >> >> No wildcards, use one, specific compatible. > > I'm in a kind of dilemma here, have a look yourself: > grep -r "lan966[28x]-" Documentation > > Should I deviate from the common "name" now? To make things > worse, there was a similar request by Arnd [1]. But the > solution feels like cheating ("lan966x" -> "lan966") ;) The previous 966x cases were added by one person from Microchip, so he actually might know something. But do you know whether lan966x will cover all current and future designs from Microchip? E.g. lan9669 (if ever made) will be the same? Avoiding wildcard is the easiest, just choose one implementation, e.g. "lan9662". Different topic is that all current lan966[28] are from Microchip and you still add Microsemi, even though it was acquired by Microchip. That's an inconsistency which should be rather fixed. > > On a side note, I understand that there should be no wildcards, > because the compatible should target one specific implementation, > right? But then the codename "ocelot" represents a whole series of > chips. Therefore, names for whole families shouldn't be used neither, > right? You're not adding "ocelot" now, so it is separate topic. However a compatible like "mscc,ocelot" feels wrong, unless it is used as a fallback (see: git grep 'apple,'). Best regards, Krzysztof
[adding Horatiu and Kavyasree from Microchip] Am 2022-03-13 17:10, schrieb Krzysztof Kozlowski: > On 13/03/2022 11:47, Michael Walle wrote: >> Am 2022-03-13 10:47, schrieb Krzysztof Kozlowski: >>> On 13/03/2022 01:25, Michael Walle wrote: >>>> The MDIO controller has support to release the internal PHYs from >>>> reset >>>> by specifying a second memory resource. This is different between >>>> the >>>> currently supported SparX-5 and the LAN966x. Add a new compatible to >>>> distiguish between these two. > > Typo here, BTW. > >>>> >>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>> --- >>>> Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt >>>> b/Documentation/devicetree/bindings/net/mscc-miim.txt >>>> index 7104679cf59d..a9efff252ca6 100644 >>>> --- a/Documentation/devicetree/bindings/net/mscc-miim.txt >>>> +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt >>>> @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO >>>> ================================================= >>>> >>>> Properties: >>>> -- compatible: must be "mscc,ocelot-miim" >>>> +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" >>> >>> No wildcards, use one, specific compatible. >> >> I'm in a kind of dilemma here, have a look yourself: >> grep -r "lan966[28x]-" Documentation >> >> Should I deviate from the common "name" now? To make things >> worse, there was a similar request by Arnd [1]. But the >> solution feels like cheating ("lan966x" -> "lan966") ;) > > The previous 966x cases were added by one person from Microchip, so he > actually might know something. But do you know whether lan966x will > cover all current and future designs from Microchip? E.g. lan9669 (if > ever made) will be the same? Avoiding wildcard is the easiest, just > choose one implementation, e.g. "lan9662". So if Microchip would review/ack this it would be ok? I don't really have a strong opinion, I just want to avoid any inconsistencies. If no one from Microchip will answer, I'll use microchip,lan9668-miim. > Different topic is that all current lan966[28] are from Microchip and > you still add Microsemi, even though it was acquired by Microchip. > That's an inconsistency which should be rather fixed. Agreed, that was an oversight by me. >> On a side note, I understand that there should be no wildcards, >> because the compatible should target one specific implementation, >> right? But then the codename "ocelot" represents a whole series of >> chips. Therefore, names for whole families shouldn't be used neither, >> right? > > You're not adding "ocelot" now, so it is separate topic. However a > compatible like "mscc,ocelot" feels wrong, unless it is used as a > fallback (see: git grep 'apple,'). Sure, it was just a question for my understanding, not to make a point for a discussion. -michael
On 13/03/2022 17:30, Michael Walle wrote: > [adding Horatiu and Kavyasree from Microchip] > > Am 2022-03-13 17:10, schrieb Krzysztof Kozlowski: >> On 13/03/2022 11:47, Michael Walle wrote: >>> Am 2022-03-13 10:47, schrieb Krzysztof Kozlowski: >>>> On 13/03/2022 01:25, Michael Walle wrote: >>>>> The MDIO controller has support to release the internal PHYs from >>>>> reset >>>>> by specifying a second memory resource. This is different between >>>>> the >>>>> currently supported SparX-5 and the LAN966x. Add a new compatible to >>>>> distiguish between these two. >> >> Typo here, BTW. >> >>>>> >>>>> Signed-off-by: Michael Walle <michael@walle.cc> >>>>> --- >>>>> Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt >>>>> b/Documentation/devicetree/bindings/net/mscc-miim.txt >>>>> index 7104679cf59d..a9efff252ca6 100644 >>>>> --- a/Documentation/devicetree/bindings/net/mscc-miim.txt >>>>> +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt >>>>> @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO >>>>> ================================================= >>>>> >>>>> Properties: >>>>> -- compatible: must be "mscc,ocelot-miim" >>>>> +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" >>>> >>>> No wildcards, use one, specific compatible. >>> >>> I'm in a kind of dilemma here, have a look yourself: >>> grep -r "lan966[28x]-" Documentation >>> >>> Should I deviate from the common "name" now? To make things >>> worse, there was a similar request by Arnd [1]. But the >>> solution feels like cheating ("lan966x" -> "lan966") ;) >> >> The previous 966x cases were added by one person from Microchip, so he >> actually might know something. But do you know whether lan966x will >> cover all current and future designs from Microchip? E.g. lan9669 (if >> ever made) will be the same? Avoiding wildcard is the easiest, just >> choose one implementation, e.g. "lan9662". > > So if Microchip would review/ack this it would be ok? I don't really > have a strong opinion, I just want to avoid any inconsistencies. If no > one from Microchip will answer, I'll use microchip,lan9668-miim. Sure. Best regards, Krzysztof
The 03/13/2022 17:30, Michael Walle wrote: Hi Michael, > > [adding Horatiu and Kavyasree from Microchip] > > Am 2022-03-13 17:10, schrieb Krzysztof Kozlowski: > > On 13/03/2022 11:47, Michael Walle wrote: > > > Am 2022-03-13 10:47, schrieb Krzysztof Kozlowski: > > > > On 13/03/2022 01:25, Michael Walle wrote: > > > > > The MDIO controller has support to release the internal PHYs from > > > > > reset > > > > > by specifying a second memory resource. This is different between > > > > > the > > > > > currently supported SparX-5 and the LAN966x. Add a new compatible to > > > > > distiguish between these two. > > > > Typo here, BTW. > > > > > > > > > > > > Signed-off-by: Michael Walle <michael@walle.cc> > > > > > --- > > > > > Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt > > > > > b/Documentation/devicetree/bindings/net/mscc-miim.txt > > > > > index 7104679cf59d..a9efff252ca6 100644 > > > > > --- a/Documentation/devicetree/bindings/net/mscc-miim.txt > > > > > +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt > > > > > @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO > > > > > ================================================= > > > > > > > > > > Properties: > > > > > -- compatible: must be "mscc,ocelot-miim" > > > > > +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" > > > > > > > > No wildcards, use one, specific compatible. > > > > > > I'm in a kind of dilemma here, have a look yourself: > > > grep -r "lan966[28x]-" Documentation > > > > > > Should I deviate from the common "name" now? To make things > > > worse, there was a similar request by Arnd [1]. But the > > > solution feels like cheating ("lan966x" -> "lan966") ;) > > > > The previous 966x cases were added by one person from Microchip, so he > > actually might know something. But do you know whether lan966x will > > cover all current and future designs from Microchip? E.g. lan9669 (if > > ever made) will be the same? Avoiding wildcard is the easiest, just > > choose one implementation, e.g. "lan9662". > > So if Microchip would review/ack this it would be ok? I don't really > have a strong opinion, I just want to avoid any inconsistencies. If no > one from Microchip will answer, I'll use microchip,lan9668-miim. I think it is OK to use microchip,lan966x. I am not aware of any plans to create future lan966x designed(lan9664 or lan9669). But we can also be on the safe side and use microchip,lan9668. I don't have any strong opinion on this. Acked-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > > Different topic is that all current lan966[28] are from Microchip and > > you still add Microsemi, even though it was acquired by Microchip. > > That's an inconsistency which should be rather fixed. > > Agreed, that was an oversight by me. > > > > On a side note, I understand that there should be no wildcards, > > > because the compatible should target one specific implementation, > > > right? But then the codename "ocelot" represents a whole series of > > > chips. Therefore, names for whole families shouldn't be used neither, > > > right? > > > > You're not adding "ocelot" now, so it is separate topic. However a > > compatible like "mscc,ocelot" feels wrong, unless it is used as a > > fallback (see: git grep 'apple,'). > > Sure, it was just a question for my understanding, not to make a > point for a discussion. > > -michael
diff --git a/Documentation/devicetree/bindings/net/mscc-miim.txt b/Documentation/devicetree/bindings/net/mscc-miim.txt index 7104679cf59d..a9efff252ca6 100644 --- a/Documentation/devicetree/bindings/net/mscc-miim.txt +++ b/Documentation/devicetree/bindings/net/mscc-miim.txt @@ -2,7 +2,7 @@ Microsemi MII Management Controller (MIIM) / MDIO ================================================= Properties: -- compatible: must be "mscc,ocelot-miim" +- compatible: must be "mscc,ocelot-miim" or "mscc,lan966x-miim" - reg: The base address of the MDIO bus controller register bank. Optionally, a second register bank can be defined if there is an associated reset register for internal PHYs
The MDIO controller has support to release the internal PHYs from reset by specifying a second memory resource. This is different between the currently supported SparX-5 and the LAN966x. Add a new compatible to distiguish between these two. Signed-off-by: Michael Walle <michael@walle.cc> --- Documentation/devicetree/bindings/net/mscc-miim.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)