| Message ID | 20220711160519.741990-3-sean.anderson@seco.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | net: pcs: Add support for devices probed in the "usual" manner | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 11/07/2022 18:05, Sean Anderson wrote: > This allows multiple phandles to be specified for pcs-handle, such as > when multiple PCSs are present for a single MAC. To differentiate > between them, also add a pcs-names property. > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > .../devicetree/bindings/net/ethernet-controller.yaml | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > index 4f15463611f8..c033e536f869 100644 > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > @@ -107,11 +107,16 @@ properties: > $ref: "#/properties/phy-connection-type" > > pcs-handle: > - $ref: /schemas/types.yaml#/definitions/phandle > + $ref: /schemas/types.yaml#/definitions/phandle-array > description: > Specifies a reference to a node representing a PCS PHY device on a MDIO > bus to link with an external PHY (phy-handle) if exists. You need to update all existing bindings and add maxItems:1. > > + pcs-names: To be consistent with other properties this should be "pcs-handle-names" and the other "pcs-handles"... and then actually drop the "handle". > + $ref: /schemas/types.yaml#/definitions/string-array > + description: > + The name of each PCS in pcs-handle. You also need: dependencies: pcs-names: [ pcs-handle ] > + > phy-handle: > $ref: /schemas/types.yaml#/definitions/phandle > description: Best regards, Krzysztof
Hi Krzysztof, On 7/12/22 4:51 AM, Krzysztof Kozlowski wrote: > On 11/07/2022 18:05, Sean Anderson wrote: >> This allows multiple phandles to be specified for pcs-handle, such as >> when multiple PCSs are present for a single MAC. To differentiate >> between them, also add a pcs-names property. >> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >> --- >> >> .../devicetree/bindings/net/ethernet-controller.yaml | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml >> index 4f15463611f8..c033e536f869 100644 >> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml >> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml >> @@ -107,11 +107,16 @@ properties: >> $ref: "#/properties/phy-connection-type" >> >> pcs-handle: >> - $ref: /schemas/types.yaml#/definitions/phandle >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> description: >> Specifies a reference to a node representing a PCS PHY device on a MDIO >> bus to link with an external PHY (phy-handle) if exists. > > You need to update all existing bindings and add maxItems:1. > >> >> + pcs-names: > > To be consistent with other properties this should be "pcs-handle-names" > and the other "pcs-handles"... and then actually drop the "handle". Sorry, I'm not sure what you're recommending in the second half here. >> + $ref: /schemas/types.yaml#/definitions/string-array >> + description: >> + The name of each PCS in pcs-handle. > > You also need: > dependencies: > pcs-names: [ pcs-handle ] > OK --Sean
On 12/07/2022 17:06, Sean Anderson wrote: > Hi Krzysztof, > > On 7/12/22 4:51 AM, Krzysztof Kozlowski wrote: >> On 11/07/2022 18:05, Sean Anderson wrote: >>> This allows multiple phandles to be specified for pcs-handle, such as >>> when multiple PCSs are present for a single MAC. To differentiate >>> between them, also add a pcs-names property. >>> >>> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >>> --- >>> >>> .../devicetree/bindings/net/ethernet-controller.yaml | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml >>> index 4f15463611f8..c033e536f869 100644 >>> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml >>> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml >>> @@ -107,11 +107,16 @@ properties: >>> $ref: "#/properties/phy-connection-type" >>> >>> pcs-handle: >>> - $ref: /schemas/types.yaml#/definitions/phandle >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>> description: >>> Specifies a reference to a node representing a PCS PHY device on a MDIO >>> bus to link with an external PHY (phy-handle) if exists. >> >> You need to update all existing bindings and add maxItems:1. >> >>> >>> + pcs-names: >> >> To be consistent with other properties this should be "pcs-handle-names" >> and the other "pcs-handles"... and then actually drop the "handle". > > Sorry, I'm not sure what you're recommending in the second half here. I would be happy to see consistent naming with other xxxs/xxx-names properties, therefore I recommend to: 1. deprecate pcs-handle because anyway the naming is encoding DT spec into the name ("handle"), 2. add new property 'pcs' or 'pcss' (the 's' at the end like clocks but maybe that's too much) with pcs-names. However before implementing this, please wait for more feedback. Maybe Rob or net folks will have different opinions. Best regards, Krzysztof
On 7/12/22 11:18 AM, Krzysztof Kozlowski wrote: > On 12/07/2022 17:06, Sean Anderson wrote: >> Hi Krzysztof, >> >> On 7/12/22 4:51 AM, Krzysztof Kozlowski wrote: >>> On 11/07/2022 18:05, Sean Anderson wrote: >>>> This allows multiple phandles to be specified for pcs-handle, such as >>>> when multiple PCSs are present for a single MAC. To differentiate >>>> between them, also add a pcs-names property. >>>> >>>> Signed-off-by: Sean Anderson <sean.anderson@seco.com> >>>> --- >>>> >>>> .../devicetree/bindings/net/ethernet-controller.yaml | 7 ++++++- >>>> 1 file changed, 6 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml >>>> index 4f15463611f8..c033e536f869 100644 >>>> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml >>>> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml >>>> @@ -107,11 +107,16 @@ properties: >>>> $ref: "#/properties/phy-connection-type" >>>> >>>> pcs-handle: >>>> - $ref: /schemas/types.yaml#/definitions/phandle >>>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>>> description: >>>> Specifies a reference to a node representing a PCS PHY device on a MDIO >>>> bus to link with an external PHY (phy-handle) if exists. >>> >>> You need to update all existing bindings and add maxItems:1. >>> >>>> >>>> + pcs-names: >>> >>> To be consistent with other properties this should be "pcs-handle-names" >>> and the other "pcs-handles"... and then actually drop the "handle". >> >> Sorry, I'm not sure what you're recommending in the second half here. > > I would be happy to see consistent naming with other xxxs/xxx-names > properties, therefore I recommend to: > 1. deprecate pcs-handle because anyway the naming is encoding DT spec > into the name ("handle"), I agree with you here. > 2. add new property 'pcs' or 'pcss' (the 's' at the end like clocks but > maybe that's too much) with pcs-names. > > However before implementing this, please wait for more feedback. Maybe > Rob or net folks will have different opinions. For some context: https://lore.kernel.org/netdev/20211004191527.1610759-2-sean.anderson@seco.com/ https://lore.kernel.org/netdev/20220321152515.287119-3-andy.chiu@sifive.com/ --Sean
On Tue, Jul 12, 2022 at 05:18:05PM +0200, Krzysztof Kozlowski wrote: > On 12/07/2022 17:06, Sean Anderson wrote: > > Hi Krzysztof, > > > > On 7/12/22 4:51 AM, Krzysztof Kozlowski wrote: > >> On 11/07/2022 18:05, Sean Anderson wrote: > >>> This allows multiple phandles to be specified for pcs-handle, such as > >>> when multiple PCSs are present for a single MAC. To differentiate > >>> between them, also add a pcs-names property. > >>> > >>> Signed-off-by: Sean Anderson <sean.anderson@seco.com> > >>> --- > >>> > >>> .../devicetree/bindings/net/ethernet-controller.yaml | 7 ++++++- > >>> 1 file changed, 6 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > >>> index 4f15463611f8..c033e536f869 100644 > >>> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > >>> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > >>> @@ -107,11 +107,16 @@ properties: > >>> $ref: "#/properties/phy-connection-type" > >>> > >>> pcs-handle: > >>> - $ref: /schemas/types.yaml#/definitions/phandle > >>> + $ref: /schemas/types.yaml#/definitions/phandle-array > >>> description: > >>> Specifies a reference to a node representing a PCS PHY device on a MDIO > >>> bus to link with an external PHY (phy-handle) if exists. > >> > >> You need to update all existing bindings and add maxItems:1. > >> > >>> > >>> + pcs-names: > >> > >> To be consistent with other properties this should be "pcs-handle-names" > >> and the other "pcs-handles"... and then actually drop the "handle". > > > > Sorry, I'm not sure what you're recommending in the second half here. > > I would be happy to see consistent naming with other xxxs/xxx-names > properties, therefore I recommend to: > 1. deprecate pcs-handle because anyway the naming is encoding DT spec > into the name ("handle"), > 2. add new property 'pcs' or 'pcss' (the 's' at the end like clocks but > maybe that's too much) with pcs-names. > > However before implementing this, please wait for more feedback. Maybe > Rob or net folks will have different opinions. We decided on "pcs-handle" for PCS for several drivers, to be consistent with the situation for network PHYs (which are "phy-handle", settled on after we also had "phy" and "phy-device" and decided to deprecate these two. Surely we should have consistency within the net code - so either "phy" and "pcs" or "phy-handle" and "pcs-handle" but not a mixture of both?
On 12/07/2022 17:59, Russell King (Oracle) wrote: >> However before implementing this, please wait for more feedback. Maybe >> Rob or net folks will have different opinions. > > We decided on "pcs-handle" for PCS for several drivers, to be consistent > with the situation for network PHYs (which are "phy-handle", settled on > after we also had "phy" and "phy-device" and decided to deprecate these > two. > > Surely we should have consistency within the net code - so either "phy" > and "pcs" or "phy-handle" and "pcs-handle" but not a mixture of both? True. Then the new property should be "pcs-handle-names"? Best regards, Krzysztof
On Thu, Jul 14, 2022 at 12:45:54PM +0200, Krzysztof Kozlowski wrote: > On 12/07/2022 17:59, Russell King (Oracle) wrote: > >> However before implementing this, please wait for more feedback. Maybe > >> Rob or net folks will have different opinions. > > > > We decided on "pcs-handle" for PCS for several drivers, to be consistent > > with the situation for network PHYs (which are "phy-handle", settled on > > after we also had "phy" and "phy-device" and decided to deprecate these > > two. > > > > Surely we should have consistency within the net code - so either "phy" > > and "pcs" or "phy-handle" and "pcs-handle" but not a mixture of both? > > True. Then the new property should be "pcs-handle-names"? IMO, just keep "pcs-handle" and then "pcs-handle-names". We never seem to get free of the deprecated versions (-gpio). While just add/remove 's' would be nice, we have to deal with things like 'mboxes' and I think some other inconsistencies. Rob
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index 4f15463611f8..c033e536f869 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -107,11 +107,16 @@ properties: $ref: "#/properties/phy-connection-type" pcs-handle: - $ref: /schemas/types.yaml#/definitions/phandle + $ref: /schemas/types.yaml#/definitions/phandle-array description: Specifies a reference to a node representing a PCS PHY device on a MDIO bus to link with an external PHY (phy-handle) if exists. + pcs-names: + $ref: /schemas/types.yaml#/definitions/string-array + description: + The name of each PCS in pcs-handle. + phy-handle: $ref: /schemas/types.yaml#/definitions/phandle description:
This allows multiple phandles to be specified for pcs-handle, such as when multiple PCSs are present for a single MAC. To differentiate between them, also add a pcs-names property. Signed-off-by: Sean Anderson <sean.anderson@seco.com> --- .../devicetree/bindings/net/ethernet-controller.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)