Subject: Re: [PATCH RFC net-next] dt-bindings: ethernet-controller: document signal multiplexer

From: Rob Herring <[email-masked]>
Date: Thu, 1 Jul 2021 12:04:22 -0600

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:
>   &eth2 {
>     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 <[email-masked]>
> ---
> 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
> 
>