[RFC] Add clause 45 ioctl

[RFC] Add clause 45 ioctl

[Eilon Greenstein]

Hi,

This patch adds clause 45 definitions to the mii.h. This will allow
enhancing the networking device drivers ioctl to handle clause 45
related operations just as it can handle clause 22 commands today. Since
clause 45 requires additional 16 bits for the bank address, the clause
22 structure cannot be used. The motivation is to allow user space
applications to access PHY registers in clause 45.

I'm sending this as RFC - if anyone has alternative suggestions on how
user space application can access the PHY, I would appreciate it.

Thanks,
Eilon

---
 include/linux/mii.h     |   17 +++++++++++++++++
 include/linux/sockios.h |    3 +++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/include/linux/mii.h b/include/linux/mii.h
index ad74858..328ad14 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -139,6 +139,13 @@
 #define FLOW_CTRL_TX		0x01
 #define FLOW_CTRL_RX		0x02
 
+/* Generic CL45 device addresses. */
+#define MII_CL45_DEVAD_PMA	0x01	/* The PMA device address */
+#define	MII_CL45_DEVAD_WIS	0x02	/* The WIS device address */
+#define MII_CL45_DEVAD_PCS	0x03	/* The PCS device address */
+#define MII_CL45_DEVAD_XS	0x04	/* The XS device address  */
+#define	MII_CL45_DEVAD_AN	0x07	/* Autoneg device address */
+
 /* This structure is used in all SIOCxMIIxxx ioctl calls */
 struct mii_ioctl_data {
 	__u16		phy_id;
@@ -147,6 +154,16 @@ struct mii_ioctl_data {
 	__u16		val_out;
 };
 
+/* This structure is used in all SIOCxCL45xxx ioctl calls */
+struct mii_cl45_ioctl_data {
+	__u16		phy_id;
+	__u16		devad;
+	__u16		reg_num;
+	__u16		val_in;
+	__u16		val_out;
+	__u16		reserved;
+};
+
 #ifdef __KERNEL__ 
 
 #include <linux/if.h>
diff --git a/include/linux/sockios.h b/include/linux/sockios.h
index 241f179..001e46b 100644
--- a/include/linux/sockios.h
+++ b/include/linux/sockios.h
@@ -83,6 +83,9 @@
 
 #define SIOCWANDEV	0x894A		/* get/set netdev parameters	*/
 
+#define SIOCGCL45PHY	0x894B		/* Get address PHY for CL45 use */
+#define SIOCGCL45REG	0x894C		/* Read PHY register using CL45 */
+#define SIOCSCL45REG	0x894D		/* Write PHY register using CL45*/
 /* ARP cache control calls. */
 		    /*  0x8950 - 0x8952  * obsolete calls, don't re-use */
 #define SIOCDARP	0x8953		/* delete ARP table entry	*/
-- 
1.5.4.3

Re: [RFC] Add clause 45 ioctl

[Ben Hutchings]

On Tue, 2009-04-28 at 13:08 +0300, Eilon Greenstein wrote:
> Hi,
> 
> This patch adds clause 45 definitions to the mii.h. This will allow
> enhancing the networking device drivers ioctl to handle clause 45
> related operations just as it can handle clause 22 commands today. Since
> clause 45 requires additional 16 bits for the bank address,

No, DEVAD is only 5 bits.

> the clause
> 22 structure cannot be used. The motivation is to allow user space
> applications to access PHY registers in clause 45.
> 
> I'm sending this as RFC - if anyone has alternative suggestions on how
> user space application can access the PHY, I would appreciate it.
[...]

I was working on an alternate interface that would use the existing
structure and ioctls.  There are at least two drivers (cxgb3 and sfc)
that already do this, though they currently pack PRTAD and DEVAD
differently in the phy_id field.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC] Add clause 45 ioctl

[Eilon Greenstein]

On Tue, 2009-04-28 at 06:22 -0700, Ben Hutchings wrote:
> > I'm sending this as RFC - if anyone has alternative suggestions on how
> > user space application can access the PHY, I would appreciate it.
> [...]
> 
> I was working on an alternate interface that would use the existing
> structure and ioctls.  There are at least two drivers (cxgb3 and sfc)
> that already do this, though they currently pack PRTAD and DEVAD
> differently in the phy_id field.

I can use the same approach and overload the CL22 definitions, but don't
you think it is cleaner to add the CL45 definition? I think that the
fact that two drivers are already overloading the CL22 for CL45 usage is
showing that CL45 is needed, and the fact that they are doing that
differently shows that there is a need for a clean definition. I'm just
thinking about someone trying to write an application for all CL45
supporting drivers - it is easier if there is a clean interface.

What do you say?

Thanks,
Eilon

Re: [RFC] Add clause 45 ioctl

[Ben Hutchings]

On Thu, 2009-04-30 at 18:05 +0300, Eilon Greenstein wrote:
> On Tue, 2009-04-28 at 06:22 -0700, Ben Hutchings wrote:
> > > I'm sending this as RFC - if anyone has alternative suggestions on how
> > > user space application can access the PHY, I would appreciate it.
> > [...]
> > 
> > I was working on an alternate interface that would use the existing
> > structure and ioctls.  There are at least two drivers (cxgb3 and sfc)
> > that already do this, though they currently pack PRTAD and DEVAD
> > differently in the phy_id field.
> 
> I can use the same approach and overload the CL22 definitions, but don't
> you think it is cleaner to add the CL45 definition?

I don't know that it's cleaner, but it's certainly more compatible.
This interface can also be implemented in out-of-tree drivers for older
kernels whereas new generic ioctls cannot.

> I think that the
> fact that two drivers are already overloading the CL22 for CL45 usage is
> showing that CL45 is needed, and the fact that they are doing that
> differently shows that there is a need for a clean definition. I'm just
> thinking about someone trying to write an application for all CL45
> supporting drivers - it is easier if there is a clean interface.
> 
> What do you say?

I defined a generic format:

clause 45 generic: 100000pppppddddd

which is distinguishable from the existing:

clause 22:         00000000000ppppp
clause 45 sfc:     000001pppppddddd
clause 45 cxgb3:   000ppppp000ddddd (prtad != 0)

The drivers that already had their own formats will convert them to the
generic format before calling the generic handler.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC] Add clause 45 ioctl

[Eilon Greenstein]

On Thu, 2009-04-30 at 08:49 -0700, Ben Hutchings wrote:
> On Thu, 2009-04-30 at 18:05 +0300, Eilon Greenstein wrote:
> > On Tue, 2009-04-28 at 06:22 -0700, Ben Hutchings wrote:
> > > > I'm sending this as RFC - if anyone has alternative suggestions on how
> > > > user space application can access the PHY, I would appreciate it.
> > > [...]
> > > 
> > > I was working on an alternate interface that would use the existing
> > > structure and ioctls.  There are at least two drivers (cxgb3 and sfc)
> > > that already do this, though they currently pack PRTAD and DEVAD
> > > differently in the phy_id field.
> > 
> > I can use the same approach and overload the CL22 definitions, but don't
> > you think it is cleaner to add the CL45 definition?
> 
> I don't know that it's cleaner, but it's certainly more compatible.
> This interface can also be implemented in out-of-tree drivers for older
> kernels whereas new generic ioctls cannot.
> 
> > I think that the
> > fact that two drivers are already overloading the CL22 for CL45 usage is
> > showing that CL45 is needed, and the fact that they are doing that
> > differently shows that there is a need for a clean definition. I'm just
> > thinking about someone trying to write an application for all CL45
> > supporting drivers - it is easier if there is a clean interface.
> > 
> > What do you say?
> 
> I defined a generic format:
> 
> clause 45 generic: 100000pppppddddd
> 
> which is distinguishable from the existing:
> 
> clause 22:         00000000000ppppp
> clause 45 sfc:     000001pppppddddd
> clause 45 cxgb3:   000ppppp000ddddd (prtad != 0)
> 
> The drivers that already had their own formats will convert them to the
> generic format before calling the generic handler.

OK - I will implement it as according to your new definition:
clause 45 generic: 100000pppppddddd

Thanks!
Eilon