netdev - Re: [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer driven by a regmap device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cfe986e2-53bc-f514-311a-d35355744687@gmail.com>
Date:   Sun, 7 Oct 2018 11:01:59 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Pankaj Bansal <pankaj.bansal@....com>, Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer driven
 by a regmap device



On 10/07/18 11:24, Pankaj Bansal wrote:
> Add support for an MDIO bus multiplexer controlled by a regmap
> device, like an FPGA.
> 
> Tested on a NXP LX2160AQDS board which uses the "QIXIS" FPGA
> attached to the i2c bus.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@....com>
> ---
> 
> Notes:
>     V2:
>      - Fixed formatting error caused by using space instead of tab
>      - Add newline between property list and subnode
>      - Add newline between two subnodes
> 
>  .../bindings/net/mdio-mux-regmap.txt         | 95 ++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt b/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
> new file mode 100644
> index 000000000000..88ebac26c1c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
> @@ -0,0 +1,95 @@
> +Properties for an MDIO bus multiplexer controlled by a regmap
> +
> +This is a special case of a MDIO bus multiplexer.  A regmap device,
> +like an FPGA, is used to control which child bus is connected.  The mdio-mux
> +node must be a child of the device that is controlled by a regmap.
> +The driver currently only supports devices with upto 32-bit registers.

I would omit any sort of details about Linux constructs designed to
support specific needs (e.g: regmap) as well as putting driver
limitations into a binding document because it really ought to be
restricted to describing hardware.

> +
> +Required properties in addition to the generic multiplexer properties:
> +
> +- compatible : string, must contain "mdio-mux-regmap"
> +
> +- reg : integer, contains the offset of the register that controls the bus
> +	multiplexer. it can be 32 bit number.

Technically it could be any "reg" property size, the only requirement is
that it must be of the appropriate size with respect to what the parent
node of this "mdio-mux-regmap" node has, determined by
#address-cells/#size-cells width.

> +
> +- mux-mask : integer, contains an 32 bit mask that specifies which
> +	bits in the register control the actual bus multiplexer.  The
> +	'reg' property of each child mdio-mux node must be constrained by
> +	this mask.

Same thing here.

Since this is a MDIO mux, I would invite you to specify what the correct
#address-cells/#size-cells values should be (1, and 0 respectively as
your example correctly shows).

> +
> +Example:
> +
> +The FPGA node defines a i2c connected FPGA with a register space of 0x30 bytes.
> +For the "EMI2" MDIO bus, register 0x54 (BRDCFG4) controls the mux on that bus.
> +A bitmask of 0x07 means that bits 0, 1 and 2 (bit 0 is lsb) are the bits on
> +BRDCFG4 that control the actual mux.
> +
> +i2c@...0000 {
> +	compatible = "fsl,vf610-i2c";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reg = <0x0 0x2000000 0x0 0x10000>;
> +	interrupts = <0 34 0x4>; // Level high type
> +	clock-names = "i2c";
> +	clocks = <&clockgen 4 7>;
> +	fsl-scl-gpio = <&gpio2 15 0>;
> +	status = "okay";
> +
> +	/* The FPGA node */
> +	fpga@66 {
> +		compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> +		reg = <0x66>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		mdio1_mux@54 {
> +			compatible = "mdio-mux-regmap", "mdio-mux";
> +			mdio-parent-bus = <&emdio2>; /* MDIO bus */
> +			reg = <0x54>;		 /* BRDCFG4 */
> +			mux-mask = <0x07>;      /* EMI2_MDIO */
> +			#address-cells=<1>;
> +			#size-cells = <0>;
> +
> +			mdio1_ioslot1@0 {   // Slot 1
> +				reg = <0x00>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				phy1@1 {
> +					reg = <1>;
> +					compatible = "ethernet-phy-id0210.7441";
> +				};
> +
> +				phy1@0 {
> +					reg = <0>;
> +					compatible = "ethernet-phy-id0210.7441";
> +				};
> +			};
> +
> +			mdio1_ioslot2@1 {   // Slot 2
> +				reg = <0x01>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +			};
> +
> +			mdio1_ioslot3@2 {   // Slot 3
> +				reg = <0x02>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +			};
> +		};
> +	};
> +};
> +
> +	/* The parent MDIO bus. */
> +	emdio2: mdio@...B97000 {
> +		compatible = "fsl,fman-memac-mdio";
> +		reg = <0x0 0x8B97000 0x0 0x1000>;
> +		device_type = "mdio";
> +		little-endian;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> 

-- 
Florian