Message ID | 20210701005347.8280-1-kabel@kernel.org |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | [RFC,net-next] dt-bindings: ethernet-controller: document signal multiplexer | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/dt-meta-schema | fail | build log |
On Thu, 01 Jul 2021 02:53:47 +0200, Marek Behún wrote: > There are devices where the MAC signals from the ethernet controller are > not directly connected to an ethernet PHY or a SFP cage, but to a > multiplexer, so that the device can switch between the endpoints. > > For example on Turris Omnia the WAN controller is connected to a SerDes > switch, which multiplexes the SerDes lanes between SFP cage and ethernet > PHY, depending on whether a SFP module is present (MOD_DEF0 GPIO from > the SFP cage). > > Document how to describe such a situation for an ethernet controller in > the device tree bindings. > > Example usage could then look like: > ð2 { > status = "okay"; > phys = <&comphy5 2>; > buffer-manager = <&bm>; > bm,pool-long = <2>; > bm,pool-short = <3>; > > signal-multiplexer { > compatible = "gpio-signal-multiplexer"; > gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; > > endpoint@0 { > phy-mode = "sgmii"; > phy-handle = <&phy1>; > }; > > endpoint@1 { > sfp = <&sfp>; > phy-mode = "sgmii"; > managed = "in-band-status"; > }; > }; > }; > > Signed-off-by: Marek Behún <kabel@kernel.org> > --- > I wonder if this is the proper way to do this. > > We already have framework for multiplexers in Linux, in drivers/mux. > But as I understand it, that framework is meant to be used when the > multiplexer state is to be set by kernel, while here it is possible > that the multiplexer state can be (and on Turris Omnia is) set by > the user plugging a SFP module into the SFP cage. > > We theoretically could add a method for getting mux state into the mux > framework and state notification support. But using the mux framework > to solve this case in phylink would be rather complicated, especially > since mux framework is abstract, and if the multiplexer state is > determined by the MOD_DEF0 GPIO, which is also used by SFP code, the > implementation would get rather complicate in phylink... > > I wonder whether driver implementation complexity should play a role > when proposing device tree bindings :-) > > Some thoughts? > --- > .../bindings/net/ethernet-controller.yaml | 60 +++++++++++++++++++ > 1 file changed, 60 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:258:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:261:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:264:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:267:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:270:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:273:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:276:18: [error] empty value in block mapping (empty-values) ./Documentation/devicetree/bindings/net/ethernet-controller.yaml:279:18: [error] empty value in block mapping (empty-values) dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:phy-connection-type:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:phy-mode:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:fixed-link:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:phy:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:phy-device:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:sfp:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:managed:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: properties:signal-multiplexer:patternProperties:^endpoint@[01]$:properties:phy-handle:$ref: None is not of type 'string' from schema $id: http://json-schema.org/draft-07/schema# schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-connection-type: $ref ./Documentation/devicetree/bindings/net/ethernet-controller.yaml: Unresolvable JSON pointer: 'properties/phy-connection-type' ./Documentation/devicetree/bindings/net/snps,dwmac.yaml: Unresolvable JSON pointer: 'properties/phy-connection-type' /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: sfp: $ref warning: no schema found in file: ./Documentation/devicetree/bindings/net/ethernet-controller.yaml schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: sfp: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-handle: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-handle: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: sfp: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-handle: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: fixed-link: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-connection-type: $ref /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'compatible' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'reg-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'ranges' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'clocks' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'clock-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'power-domains' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'dmas' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'dma-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: '#address-cells' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: '#size-cells' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dt.yaml: ethernet@46000000: ethernet-ports:port@1: 'label', 'phy-handle', 'phy-mode', 'phys', 'ti,mac-only', 'ti,syscon-efuse' do not match any of the regexes: '^cpts@[0-9a-f]+', '^mdio@[0-9a-f]+$', 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-device: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: fixed-link: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-device: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy-device: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: fixed-link: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: fixed-link: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: sfp: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: managed: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy: $ref dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: fixed-link: $ref dt-validate: recursion error: Check for prior errors in a referenced schema dt-validate: recursion error: Check for prior errors in a referenced schema schemas/net/ethernet-controller.yaml: ignoring, error in schema: properties: signal-multiplexer: patternProperties: ^endpoint@[01]$: properties: phy: $ref /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'compatible' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'ranges' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'clocks' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'clock-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'interrupt-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: '#address-cells' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: '#size-cells' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'label', 'mac-address', 'phy-handle', 'phy-mode', 'phys', 'ti,dual-emac-pvid' do not match any of the regexes: '^mdio@' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@1: 'oneOf' conditional failed, one must be fixed: 'interrupts' is a required property 'interrupts-extended' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'compatible' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'ranges' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'clocks' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'clock-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'interrupt-names' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: '#address-cells' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: '#size-cells' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'label', 'mac-address', 'phy-handle', 'phy-mode', 'phys', 'ti,dual-emac-pvid' do not match any of the regexes: '^mdio@' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: switch@0: ethernet-ports:port@2: 'oneOf' conditional failed, one must be fixed: 'interrupts' is a required property 'interrupts-extended' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml \ndoc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1499168 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Thu, Jul 01, 2021 at 02:53:47AM +0200, Marek Behún wrote: > There are devices where the MAC signals from the ethernet controller are > not directly connected to an ethernet PHY or a SFP cage, but to a > multiplexer, so that the device can switch between the endpoints. > > For example on Turris Omnia the WAN controller is connected to a SerDes > switch, which multiplexes the SerDes lanes between SFP cage and ethernet > PHY, depending on whether a SFP module is present (MOD_DEF0 GPIO from > the SFP cage). And s/w can read the MOD_DEF0 state to determine if SFP is present? > > Document how to describe such a situation for an ethernet controller in > the device tree bindings. > > Example usage could then look like: > ð2 { > status = "okay"; > phys = <&comphy5 2>; > buffer-manager = <&bm>; > bm,pool-long = <2>; > bm,pool-short = <3>; > > signal-multiplexer { > compatible = "gpio-signal-multiplexer"; > gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; > > endpoint@0 { > phy-mode = "sgmii"; > phy-handle = <&phy1>; > }; > > endpoint@1 { > sfp = <&sfp>; > phy-mode = "sgmii"; > managed = "in-band-status"; > }; > }; > }; > > Signed-off-by: Marek Behún <kabel@kernel.org> > --- > I wonder if this is the proper way to do this. > > We already have framework for multiplexers in Linux, in drivers/mux. > But as I understand it, that framework is meant to be used when the > multiplexer state is to be set by kernel, while here it is possible > that the multiplexer state can be (and on Turris Omnia is) set by > the user plugging a SFP module into the SFP cage. Right, seems like not a good fit ATM. > > We theoretically could add a method for getting mux state into the mux > framework and state notification support. But using the mux framework > to solve this case in phylink would be rather complicated, especially > since mux framework is abstract, and if the multiplexer state is > determined by the MOD_DEF0 GPIO, which is also used by SFP code, the > implementation would get rather complicate in phylink... This doesn't seem like it would be very common, so I think I'd stick with the simple solution unless there's a strong desire to make the mux control work for this use case. Generically it would be a read-only or externally controlled mux. > I wonder whether driver implementation complexity should play a role > when proposing device tree bindings :-) Yes, at least in the sense of complicating any driver implementation. Keep in mind that using a binding doesn't require using a subsystem. You could use the mux binding, but not the mux framework. (And the latter could evolve with the OS.) > > Some thoughts? > --- > .../bindings/net/ethernet-controller.yaml | 60 +++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > index b0933a8c295a..a7770edaec2b 100644 > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > @@ -226,6 +226,66 @@ properties: > required: > - speed > > + signal-multiplexer: > + type: object > + description: > + Specifies that the signal pins (for example SerDes lanes) are connected > + to a multiplexer from which they can be multiplexed to several different > + endpoints, depending on the multiplexer configuration. (For example SerDes > + lanes can be switched between an ethernet PHY and a SFP cage.) > + > + properties: > + compatible: > + const: gpio-signal-multiplexer > + > + gpios: > + maxItems: 1 > + description: > + GPIO to determine which endpoint the multiplexer is switched to. > + > + patternProperties: > + "^endpoint@[01]$": 'endpoint' as a node name is already taken by the OF graph binding, so pick something else. > + type: object > + description: > + Specifies a multiplexer endpoint settings. Each endpoint can have > + different settings. (For example in the case when multiplexing between > + an ethernet PHY and a SFP cage, the SFP cage endpoint should specify > + SFP phandle, while the PHY endpoint should specify PHY handle.) > + > + properties: > + reg: > + enum: [ 0, 1 ] > + > + phy-connection-type: > + $ref: #/properties/phy-connection-type > + > + phy-mode: > + $ref: #/properties/phy-mode > + > + phy-handle: > + $ref: #/properties/phy-handle > + > + phy: > + $ref: #/properties/phy > + > + phy-device: > + $ref: #/properties/phy-device > + > + sfp: > + $ref: #/properties/sfp > + > + managed: > + $ref: #/properties/managed > + > + fixed-link: > + $ref: #/properties/fixed-link > + > + required: > + - reg > + > + required: > + - gpios > + > additionalProperties: true > > ... > -- > 2.31.1 > >
On Thu, Jul 01, 2021 at 02:53:47AM +0200, Marek Behún wrote: > There are devices where the MAC signals from the ethernet controller are > not directly connected to an ethernet PHY or a SFP cage, but to a > multiplexer, so that the device can switch between the endpoints. > > For example on Turris Omnia the WAN controller is connected to a SerDes > switch, which multiplexes the SerDes lanes between SFP cage and ethernet > PHY, depending on whether a SFP module is present (MOD_DEF0 GPIO from > the SFP cage). At the moment, i don't think phylink supports this. It does not have a way to dynamically switch PHY. If the SFP disappears, you probably want to configure the PHY, so that it is up, autoneg started, etc. When the SFP reappears, the PHY needs to be configured down, the SFP probably needs its TX GPIO line set active, etc. None of this currently exists. The Marvell switches have something similar but different. Which ever gets link first, SFP or PHY gets the data path. In this case, you probably want phylink to configure both the SFP and the PHY, and then wait and see what happens. The hardware will then set the mux when one of them gets link. phylink should then configure the other down. Again, non of this exists at the moment. I would imaging a similar binding could be used for these two conditions. But until we get the needed code, it is hard for me to say. So i think i would prefer to wait until we do have code. I also wonder how wise it is to put this into the generic ethernet controller binding. Muxing based on MOD_DEF0 i expect to be very rare. Muxing based on first port having link seems more likely. But both i expect are pretty unusual. So i would be tempted to make it a standalone binding, which can be imported into an MAC binding which actually needs it. Or it actually becomes part of the phylink binding, since this all appears to be PHY related, not MAC. Andrew
On Fri, 2 Jul 2021 02:55:54 +0200 Andrew Lunn <andrew@lunn.ch> wrote: > On Thu, Jul 01, 2021 at 02:53:47AM +0200, Marek Behún wrote: > > There are devices where the MAC signals from the ethernet controller are > > not directly connected to an ethernet PHY or a SFP cage, but to a > > multiplexer, so that the device can switch between the endpoints. > > > > For example on Turris Omnia the WAN controller is connected to a SerDes > > switch, which multiplexes the SerDes lanes between SFP cage and ethernet > > PHY, depending on whether a SFP module is present (MOD_DEF0 GPIO from > > the SFP cage). > > At the moment, i don't think phylink supports this. It does not have a > way to dynamically switch PHY. If the SFP disappears, you probably > want to configure the PHY, so that it is up, autoneg started, > etc. When the SFP reappears, the PHY needs to be configured down, the > SFP probably needs its TX GPIO line set active, etc. None of this > currently exists. Of course this is not supported by phylink: it can't be, since we don't even have a binding description :) I am figuring out how to do correct binding while working on implementing this into phylink. > The Marvell switches have something similar but different. Which ever > gets link first, SFP or PHY gets the data path. In this case, you > probably want phylink to configure both the SFP and the PHY, and then > wait and see what happens. The hardware will then set the mux when one > of them gets link. phylink should then configure the other > down. Again, non of this exists at the moment. > > I would imaging a similar binding could be used for these two > conditions. But until we get the needed code, it is hard for me to > say. So i think i would prefer to wait until we do have code. > I now have an idea that might be sane for bindings, so next time I will send the code as well. > I also wonder how wise it is to put this into the generic ethernet > controller binding. Muxing based on MOD_DEF0 i expect to be very > rare. Muxing based on first port having link seems more likely. But > both i expect are pretty unusual. So i would be tempted to make it a > standalone binding, which can be imported into an MAC binding which > actually needs it. Or it actually becomes part of the phylink > binding, since this all appears to be PHY related, not MAC. > > Andrew We'll see. Stay tuned for my patch series. :) Marek
On Wed, Jul 07, 2021 at 01:22:24AM +0200, Marek Behún wrote: > On Fri, 2 Jul 2021 02:55:54 +0200 > Andrew Lunn <andrew@lunn.ch> wrote: > > On Thu, Jul 01, 2021 at 02:53:47AM +0200, Marek Behún wrote: > > > There are devices where the MAC signals from the ethernet controller are > > > not directly connected to an ethernet PHY or a SFP cage, but to a > > > multiplexer, so that the device can switch between the endpoints. > > > > > > For example on Turris Omnia the WAN controller is connected to a SerDes > > > switch, which multiplexes the SerDes lanes between SFP cage and ethernet > > > PHY, depending on whether a SFP module is present (MOD_DEF0 GPIO from > > > the SFP cage). > > > > At the moment, i don't think phylink supports this. It does not have a > > way to dynamically switch PHY. If the SFP disappears, you probably > > want to configure the PHY, so that it is up, autoneg started, > > etc. When the SFP reappears, the PHY needs to be configured down, the > > SFP probably needs its TX GPIO line set active, etc. None of this > > currently exists. > > Of course this is not supported by phylink: it can't be, since we don't > even have a binding description :) I am figuring out how to do correct > binding while working on implementing this into phylink. I have been thinking that we need phylink to separate the PHY pointer that was probed by the network adapter and the PHY pointer for the SFP. The reason being that currently, a network adapter can remove the SFP PHY when it didn't create it - which is obviously not a good idea as it doesn't own it. The other reason is to do with this situation where we have separate PHY and SFP paths. I'm not intending at this point to add support for this, only to separate the two PHYs and have something like: static struct phy_device *phylink_phy(struct phylink *pl) { if (pl->sfp_phy) return pl->sfp_phydev; return pl->phydev; } and use that everywhere we want to get at pl->phydev in the independent parts of the code. Those which want to get at a specific PHY will continue using their appropriate pointers directly.
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index b0933a8c295a..a7770edaec2b 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -226,6 +226,66 @@ properties: required: - speed + signal-multiplexer: + type: object + description: + Specifies that the signal pins (for example SerDes lanes) are connected + to a multiplexer from which they can be multiplexed to several different + endpoints, depending on the multiplexer configuration. (For example SerDes + lanes can be switched between an ethernet PHY and a SFP cage.) + + properties: + compatible: + const: gpio-signal-multiplexer + + gpios: + maxItems: 1 + description: + GPIO to determine which endpoint the multiplexer is switched to. + + patternProperties: + "^endpoint@[01]$": + type: object + description: + Specifies a multiplexer endpoint settings. Each endpoint can have + different settings. (For example in the case when multiplexing between + an ethernet PHY and a SFP cage, the SFP cage endpoint should specify + SFP phandle, while the PHY endpoint should specify PHY handle.) + + properties: + reg: + enum: [ 0, 1 ] + + phy-connection-type: + $ref: #/properties/phy-connection-type + + phy-mode: + $ref: #/properties/phy-mode + + phy-handle: + $ref: #/properties/phy-handle + + phy: + $ref: #/properties/phy + + phy-device: + $ref: #/properties/phy-device + + sfp: + $ref: #/properties/sfp + + managed: + $ref: #/properties/managed + + fixed-link: + $ref: #/properties/fixed-link + + required: + - reg + + required: + - gpios + additionalProperties: true ...
There are devices where the MAC signals from the ethernet controller are not directly connected to an ethernet PHY or a SFP cage, but to a multiplexer, so that the device can switch between the endpoints. For example on Turris Omnia the WAN controller is connected to a SerDes switch, which multiplexes the SerDes lanes between SFP cage and ethernet PHY, depending on whether a SFP module is present (MOD_DEF0 GPIO from the SFP cage). Document how to describe such a situation for an ethernet controller in the device tree bindings. Example usage could then look like: ð2 { status = "okay"; phys = <&comphy5 2>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>; signal-multiplexer { compatible = "gpio-signal-multiplexer"; gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; endpoint@0 { phy-mode = "sgmii"; phy-handle = <&phy1>; }; endpoint@1 { sfp = <&sfp>; phy-mode = "sgmii"; managed = "in-band-status"; }; }; }; Signed-off-by: Marek Behún <kabel@kernel.org> --- I wonder if this is the proper way to do this. We already have framework for multiplexers in Linux, in drivers/mux. But as I understand it, that framework is meant to be used when the multiplexer state is to be set by kernel, while here it is possible that the multiplexer state can be (and on Turris Omnia is) set by the user plugging a SFP module into the SFP cage. We theoretically could add a method for getting mux state into the mux framework and state notification support. But using the mux framework to solve this case in phylink would be rather complicated, especially since mux framework is abstract, and if the multiplexer state is determined by the MOD_DEF0 GPIO, which is also used by SFP code, the implementation would get rather complicate in phylink... I wonder whether driver implementation complexity should play a role when proposing device tree bindings :-) Some thoughts? --- .../bindings/net/ethernet-controller.yaml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+)