netdev - RE: [PATCH] net: phy: adds backplane driver for Freescale's PCS PHY

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]

Date:	Mon, 21 Dec 2015 12:17:03 +0000
From:	Shaohui Xie <shaohui.xie@....com>
To:	Andrew Lunn <andrew@...n.ch>,
	"shh.xie@...il.com" <shh.xie@...il.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"f.fainelli@...il.com" <f.fainelli@...il.com>,
	"Xie Shaohui-B21989" <Shaohui.Xie@...escale.com>,
	Shaohui Xie <shaohui.xie@....com>
Subject: RE: [PATCH] net: phy: adds backplane driver for Freescale's PCS PHY

> On Fri, Dec 18, 2015 at 05:30:54PM +0800, shh.xie@...il.com wrote:
> > +static int fsl_backplane_probe(struct phy_device *phydev) {
> > +	struct fsl_xgkr_inst *xgkr_inst;
> > +	struct device_node *child, *parent, *lane_node;
> > +	const char *lane_name;
> > +	int len;
> > +	int ret;
> > +	u32 mode;
> > +	u32 lane[2];
> > +
> > +	child = phydev->dev.of_node;
> > +	parent = of_get_parent(child);
> > +	if (!parent) {
> > +		dev_err(&phydev->dev, "could not get parent node");
> > +		return 0;
> > +	}
> > +
> > +	lane_name = of_get_property(parent, "lane-instance", &len);
> > +	if (!lane_name)
> > +		return 0;
> > +
> > +	if (strstr(lane_name, "1000BASE-KX"))
> > +		mode = BACKPLANE_1G_KX;
> > +	else
> > +		mode = BACKPLANE_10G_KR;
> > +
> > +	lane_node = of_parse_phandle(child, "lane-handle", 0);
> 
> 
> Hi Shaohui
> 
> You are missing the device tree binding Documentation.
> 
> Parent will be the mdio bus device and you want 'lane-instance' and
> 'lane-handle' properties to be in the mdio bus node?
[S.H] Hi Andrew,

Thanks for reviewing the patch!

I did missed the device tree binding documentation.
This driver expected a property "lane-instance" in mdio bus node, and
"lane-handle" and "lane-range" properties in phy node.

The "lane-instance" indicates what the phy should be probed as,
1000BASE-KX or 10GBASE-KR, seems phy node is a better place than mdio bus node
to hold this property, maybe a better name "phy-mode" should be used?

The "lane-handle" pointed to a serdes node which looks like below:
E.g. in arch/powerpc/boot/dts/fsl/t4240si-post.dtsi:

        serdes: serdes@...00 {
                compatible = "fsl,t4240-serdes";
                reg        = <0xea000 0x4000>;
        };

The "lane-handle" property would be: lane-handle = <&serdes>;

The serdes node has "reg" property for the whole registers space,
for a PCS phy, only some serdes registers (lane control registers)
are related and needed to be configured at runtime, so I used the 
"lane-range" property to hold such information, it would be e.g.:
lane-range = <0x1800 0x40>;
'0x1800' is offset of the lane control registers in serdes, '0x40' is 
size of the space.
Different PCS phy has different offset, but the size is same.

"phy-mode" (if it can be used here) has been documented in: 
Documentation/devicetree/bindings/net/ethernet.txt,
I'm not sure where the serdes lane related properties should go, 
 'fsl-tsec-phy.txt' or a new binding like 'fsl-pcs-phy.txt'?
Please comment.

Thanks!
Shaohui
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html