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: <HE1PR0402MB33233F0E876E2091DFD9C9D0F1F80@HE1PR0402MB3323.eurprd04.prod.outlook.com>
Date:   Thu, 18 Oct 2018 04:30:04 +0000
From:   Pankaj Bansal <pankaj.bansal@....com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer driven
 by a regmap device

Hi Florian

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@...il.com]
> Sent: Sunday, October 7, 2018 11:32 PM
> 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.
> 

Actually the platform driver mdio-mux-regmap.c, is generalization of mdio-mux-mmioreg.c
As such, it doesn't describe any new hardware, so no new properties are described by it.
The only new property is compatible field.
I don't know how to describe this driver otherwise.  Can you please help?

> > +
> > +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.

We are reading only single cell of this property using "of_propert_read_u32".
That is why I put the size in this.

> 
> > +
> > +- 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.

We are reading only single cell of this property using "of_propert_read_u32".
That is why I put the size in this.

> 
> 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).
> 

I will mention #address-cells/#size-cells values

> > +
> > +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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ