diff mbox

net: phy: Add support for SMSC/Microchip LAN9303 3-port switch

Message ID 1393492072-15364-1-git-send-email-sr@denx.de
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Stefan Roese Feb. 27, 2014, 9:07 a.m. UTC
This driver exposes a sysfs interface to access the LAN9303 registers to
userspace. These sysfs files can be used to configure the switch.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Roger Meier <r.meier@siemens.com>
Cc: Lukas Stockmann <lukas.stockmann@siemens.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/Kconfig   |   7 ++
 drivers/net/phy/Makefile  |   1 +
 drivers/net/phy/lan9303.c | 241 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+)
 create mode 100644 drivers/net/phy/lan9303.c

Comments

David Miller Feb. 27, 2014, 10:02 p.m. UTC | #1
From: Stefan Roese <sr@denx.de>
Date: Thu, 27 Feb 2014 10:07:52 +0100

> This driver exposes a sysfs interface to access the LAN9303 registers to
> userspace. These sysfs files can be used to configure the switch.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>

We have an abstraction for programming switch devices called DSA.

Even if DSA doesn't fullfill your needs, doing adjustments using sysfs
files in userland is going to be the worst user experience possible.

Every driver will export different things to tweak, each device will
have different semantics and limitations for these settings, etc.

Better is to come up with a real, types, interface for programming
such devices and providing an implementation of that.

I'm not applying this, sorry.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli Feb. 27, 2014, 10:21 p.m. UTC | #2
2014-02-27 14:02 GMT-08:00 David Miller <davem@davemloft.net>:
> From: Stefan Roese <sr@denx.de>
> Date: Thu, 27 Feb 2014 10:07:52 +0100
>
>> This driver exposes a sysfs interface to access the LAN9303 registers to
>> userspace. These sysfs files can be used to configure the switch.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>
> We have an abstraction for programming switch devices called DSA.
>
> Even if DSA doesn't fullfill your needs, doing adjustments using sysfs
> files in userland is going to be the worst user experience possible.

If you remember the discussion that started around the 'swconfig'
patches, DSA is not suitable for all types of switches, especially not
those which have no proprietary tagging support (which are about 80%
of the switches used in embedded systems). That discussion has kind of
stalled now, so we should probably bring it back...

>
> Every driver will export different things to tweak, each device will
> have different semantics and limitations for these settings, etc.
>
> Better is to come up with a real, types, interface for programming
> such devices and providing an implementation of that.

These are definitively the goals of the 'swconfig' patches that were
posted a while ago, to provide an unified programming API.
Stefan Roese Feb. 28, 2014, 9:32 a.m. UTC | #3
On 27.02.2014 23:02, David Miller wrote:
> From: Stefan Roese <sr@denx.de>
> Date: Thu, 27 Feb 2014 10:07:52 +0100
>
>> This driver exposes a sysfs interface to access the LAN9303 registers to
>> userspace. These sysfs files can be used to configure the switch.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>
> We have an abstraction for programming switch devices called DSA.
>
> Even if DSA doesn't fullfill your needs, doing adjustments using sysfs
> files in userland is going to be the worst user experience possible.

DSA would have been optimal for us, as we really want to expose the 
external switch ports to the host cpu as (virtual) ethernet ports. But 
as Florian already mentioned, this switch does not support DSA. Only 
VLAN tagging with some special configuration options to make this setup 
possible for us.

I'm not sure but perhaps its possible to use the kernel DSA 
infrastructure for this switch after all? Are some non-DSA compatible 
switches supported right now? I could not find any reference for such 
non-DSA devices. Perhaps I missed something here.

> Every driver will export different things to tweak, each device will
> have different semantics and limitations for these settings, etc.
>
> Better is to come up with a real, types, interface for programming
> such devices and providing an implementation of that.

Right. Such a thing would be better. But as Florian already pointed out, 
nothing like this is available in kernel.org right now. Thats the reason 
why I chose to implement this "simple" driver, btw as done in 
drivers/net/phy/spi_ks8995.c.

Thanks,
Stefan

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Feb. 28, 2014, 6 p.m. UTC | #4
From: Stefan Roese <sr@denx.de>
Date: Fri, 28 Feb 2014 10:32:20 +0100

> On 27.02.2014 23:02, David Miller wrote:
>> From: Stefan Roese <sr@denx.de>
>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>
>> Better is to come up with a real, types, interface for programming
>> such devices and providing an implementation of that.
> 
> Right. Such a thing would be better. But as Florian already pointed
> out, nothing like this is available in kernel.org right now. Thats the
> reason why I chose to implement this "simple" driver, btw as done in
> drivers/net/phy/spi_ks8995.c.

When a suitable user facing interface is lacking, you create one
and submit it for review here.  You don't just submit adhoc driver
local stuff.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yegor Yefremov March 11, 2014, 3:49 p.m. UTC | #5
Hi Florian,

On Thu, Feb 27, 2014 at 11:21 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 2014-02-27 14:02 GMT-08:00 David Miller <davem@davemloft.net>:
>> From: Stefan Roese <sr@denx.de>
>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>
>>> This driver exposes a sysfs interface to access the LAN9303 registers to
>>> userspace. These sysfs files can be used to configure the switch.
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>
>> We have an abstraction for programming switch devices called DSA.
>>
>> Even if DSA doesn't fullfill your needs, doing adjustments using sysfs
>> files in userland is going to be the worst user experience possible.
>
> If you remember the discussion that started around the 'swconfig'
> patches, DSA is not suitable for all types of switches, especially not
> those which have no proprietary tagging support (which are about 80%
> of the switches used in embedded systems). That discussion has kind of
> stalled now, so we should probably bring it back...
>
>>
>> Every driver will export different things to tweak, each device will
>> have different semantics and limitations for these settings, etc.
>>
>> Better is to come up with a real, types, interface for programming
>> such devices and providing an implementation of that.
>
> These are definitively the goals of the 'swconfig' patches that were
> posted a while ago, to provide an unified programming API.
> --
> Florian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Have you tried to apply your swconfig patches to 3.14? There some
compiler errors meanwhile. genl_register_ops() is not available, so
one has to switch to genl_register_family_with_ops() or similar
macros. It would be great, if you could post the updates series and if
possible with ICplus drive included.

Yegor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yegor Yefremov March 11, 2014, 8:11 p.m. UTC | #6
On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
> From: Stefan Roese <sr@denx.de>
> Date: Fri, 28 Feb 2014 10:32:20 +0100
>
>> On 27.02.2014 23:02, David Miller wrote:
>>> From: Stefan Roese <sr@denx.de>
>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>>
>>> Better is to come up with a real, types, interface for programming
>>> such devices and providing an implementation of that.
>>
>> Right. Such a thing would be better. But as Florian already pointed
>> out, nothing like this is available in kernel.org right now. Thats the
>> reason why I chose to implement this "simple" driver, btw as done in
>> drivers/net/phy/spi_ks8995.c.
>
> When a suitable user facing interface is lacking, you create one
> and submit it for review here.  You don't just submit adhoc driver
> local stuff.

David, what about enabling ethtool to read/write from/to the PHY
registers (via passing PHY ID and register number)? I'm trying to get
ICplus IP175D running and it has lots of registers, that I would like
to look at dynamically.

Yegor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli March 11, 2014, 8:14 p.m. UTC | #7
2014-03-11 13:11 GMT-07:00 Yegor Yefremov <yegorslists@googlemail.com>:
> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
>> From: Stefan Roese <sr@denx.de>
>> Date: Fri, 28 Feb 2014 10:32:20 +0100
>>
>>> On 27.02.2014 23:02, David Miller wrote:
>>>> From: Stefan Roese <sr@denx.de>
>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>>>
>>>> Better is to come up with a real, types, interface for programming
>>>> such devices and providing an implementation of that.
>>>
>>> Right. Such a thing would be better. But as Florian already pointed
>>> out, nothing like this is available in kernel.org right now. Thats the
>>> reason why I chose to implement this "simple" driver, btw as done in
>>> drivers/net/phy/spi_ks8995.c.
>>
>> When a suitable user facing interface is lacking, you create one
>> and submit it for review here.  You don't just submit adhoc driver
>> local stuff.
>
> David, what about enabling ethtool to read/write from/to the PHY
> registers (via passing PHY ID and register number)? I'm trying to get
> ICplus IP175D running and it has lots of registers, that I would like
> to look at dynamically.

ethtool does not do that already for Ethernet MACs attached to a
single Ethernet PHY, so I do not think this would be the right tool
for this job. Something custom which uses SIOC{G,S}MIIREG might be
more suited for this as you would have control over the target PHY
address.

Maybe swconfig needs to be resubmitted again...
David Miller March 11, 2014, 8:39 p.m. UTC | #8
From: Yegor Yefremov <yegorslists@googlemail.com>
Date: Tue, 11 Mar 2014 21:11:16 +0100

> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
>> From: Stefan Roese <sr@denx.de>
>> Date: Fri, 28 Feb 2014 10:32:20 +0100
>>
>>> On 27.02.2014 23:02, David Miller wrote:
>>>> From: Stefan Roese <sr@denx.de>
>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>>>
>>>> Better is to come up with a real, types, interface for programming
>>>> such devices and providing an implementation of that.
>>>
>>> Right. Such a thing would be better. But as Florian already pointed
>>> out, nothing like this is available in kernel.org right now. Thats the
>>> reason why I chose to implement this "simple" driver, btw as done in
>>> drivers/net/phy/spi_ks8995.c.
>>
>> When a suitable user facing interface is lacking, you create one
>> and submit it for review here.  You don't just submit adhoc driver
>> local stuff.
> 
> David, what about enabling ethtool to read/write from/to the PHY
> registers (via passing PHY ID and register number)? I'm trying to get
> ICplus IP175D running and it has lots of registers, that I would like
> to look at dynamically.

For debugging, register dumps are already supported by ethtool.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yegor Yefremov March 12, 2014, 8:43 a.m. UTC | #9
On Tue, Mar 11, 2014 at 9:39 PM, David Miller <davem@davemloft.net> wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> Date: Tue, 11 Mar 2014 21:11:16 +0100
>
>> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Stefan Roese <sr@denx.de>
>>> Date: Fri, 28 Feb 2014 10:32:20 +0100
>>>
>>>> On 27.02.2014 23:02, David Miller wrote:
>>>>> From: Stefan Roese <sr@denx.de>
>>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>>>>
>>>>> Better is to come up with a real, types, interface for programming
>>>>> such devices and providing an implementation of that.
>>>>
>>>> Right. Such a thing would be better. But as Florian already pointed
>>>> out, nothing like this is available in kernel.org right now. Thats the
>>>> reason why I chose to implement this "simple" driver, btw as done in
>>>> drivers/net/phy/spi_ks8995.c.
>>>
>>> When a suitable user facing interface is lacking, you create one
>>> and submit it for review here.  You don't just submit adhoc driver
>>> local stuff.
>>
>> David, what about enabling ethtool to read/write from/to the PHY
>> registers (via passing PHY ID and register number)? I'm trying to get
>> ICplus IP175D running and it has lots of registers, that I would like
>> to look at dynamically.
>
> For debugging, register dumps are already supported by ethtool.

Do you mean 'ethtool -e' option? AFAIK it dumps Ethernet controller
regs, if it implements ethtool_ops.get_regs(). What I need are PHY
registers, that I can get via MII interface. ethtool should provide
access to PHY registers like mdiobus_read() and mdiobus_write()
routines. Another option were to add get_regs() to 'struct
phy_driver', but in this case one won't be able to write to the
registers.

Yegor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yegor Yefremov March 12, 2014, 8:47 a.m. UTC | #10
On Tue, Mar 11, 2014 at 9:14 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 2014-03-11 13:11 GMT-07:00 Yegor Yefremov <yegorslists@googlemail.com>:
>> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Stefan Roese <sr@denx.de>
>>> Date: Fri, 28 Feb 2014 10:32:20 +0100
>>>
>>>> On 27.02.2014 23:02, David Miller wrote:
>>>>> From: Stefan Roese <sr@denx.de>
>>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>>>>>
>>>>> Better is to come up with a real, types, interface for programming
>>>>> such devices and providing an implementation of that.
>>>>
>>>> Right. Such a thing would be better. But as Florian already pointed
>>>> out, nothing like this is available in kernel.org right now. Thats the
>>>> reason why I chose to implement this "simple" driver, btw as done in
>>>> drivers/net/phy/spi_ks8995.c.
>>>
>>> When a suitable user facing interface is lacking, you create one
>>> and submit it for review here.  You don't just submit adhoc driver
>>> local stuff.
>>
>> David, what about enabling ethtool to read/write from/to the PHY
>> registers (via passing PHY ID and register number)? I'm trying to get
>> ICplus IP175D running and it has lots of registers, that I would like
>> to look at dynamically.
>
> ethtool does not do that already for Ethernet MACs attached to a
> single Ethernet PHY, so I do not think this would be the right tool
> for this job. Something custom which uses SIOC{G,S}MIIREG might be
> more suited for this as you would have control over the target PHY
> address.
>
> Maybe swconfig needs to be resubmitted again...

It definitely should. Another option were to create a repo (kernel
fork) at GitHub and create a per release branch for swconfig. This way
people can contribute swconfig compatible drivers for different switch
chips. We would have all source at one central place. And when the
decision about final/official switch interface will be made, we would
have 'only' to rewrite the drivers for the final swconfig version.

Yegor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Meier, Roger March 12, 2014, 8:55 a.m. UTC | #11
> -----Original Message-----

> From: Yegor Yefremov [mailto:yegorslists@googlemail.com]

> On Tue, Mar 11, 2014 at 9:14 PM, Florian Fainelli <f.fainelli@gmail.com>

> wrote:

> > 2014-03-11 13:11 GMT-07:00 Yegor Yefremov <yegorslists@googlemail.com>:

> >> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:

> >>> From: Stefan Roese <sr@denx.de>

> >>> Date: Fri, 28 Feb 2014 10:32:20 +0100

> >>>

> >>>> On 27.02.2014 23:02, David Miller wrote:

> >>>>> From: Stefan Roese <sr@denx.de>

> >>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100

> >>>>>

> >>>>> Better is to come up with a real, types, interface for programming

> >>>>> such devices and providing an implementation of that.

> >>>>

> >>>> Right. Such a thing would be better. But as Florian already pointed

> >>>> out, nothing like this is available in kernel.org right now. Thats the

> >>>> reason why I chose to implement this "simple" driver, btw as done in

> >>>> drivers/net/phy/spi_ks8995.c.

> >>>

> >>> When a suitable user facing interface is lacking, you create one

> >>> and submit it for review here.  You don't just submit adhoc driver

> >>> local stuff.

> >>

> >> David, what about enabling ethtool to read/write from/to the PHY

> >> registers (via passing PHY ID and register number)? I'm trying to get

> >> ICplus IP175D running and it has lots of registers, that I would like

> >> to look at dynamically.

> >

> > ethtool does not do that already for Ethernet MACs attached to a

> > single Ethernet PHY, so I do not think this would be the right tool

> > for this job. Something custom which uses SIOC{G,S}MIIREG might be

> > more suited for this as you would have control over the target PHY

> > address.

> >

> > Maybe swconfig needs to be resubmitted again...

> 

> It definitely should. Another option were to create a repo (kernel

> fork) at GitHub and create a per release branch for swconfig. This way

> people can contribute swconfig compatible drivers for different switch

> chips. We would have all source at one central place. And when the

> decision about final/official switch interface will be made, we would

> have 'only' to rewrite the drivers for the final swconfig version.

I like that idea.
We really need such a common switch configuration api to simplify usage
and reduce efforts for the growing community that uses ethernet switches.

-roger
Ben Hutchings March 12, 2014, 8:21 p.m. UTC | #12
On Wed, 2014-03-12 at 09:43 +0100, Yegor Yefremov wrote:
> On Tue, Mar 11, 2014 at 9:39 PM, David Miller <davem@davemloft.net> wrote:
> > From: Yegor Yefremov <yegorslists@googlemail.com>
> > Date: Tue, 11 Mar 2014 21:11:16 +0100
> >
> >> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
> >>> From: Stefan Roese <sr@denx.de>
> >>> Date: Fri, 28 Feb 2014 10:32:20 +0100
> >>>
> >>>> On 27.02.2014 23:02, David Miller wrote:
> >>>>> From: Stefan Roese <sr@denx.de>
> >>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
> >>>>>
> >>>>> Better is to come up with a real, types, interface for programming
> >>>>> such devices and providing an implementation of that.
> >>>>
> >>>> Right. Such a thing would be better. But as Florian already pointed
> >>>> out, nothing like this is available in kernel.org right now. Thats the
> >>>> reason why I chose to implement this "simple" driver, btw as done in
> >>>> drivers/net/phy/spi_ks8995.c.
> >>>
> >>> When a suitable user facing interface is lacking, you create one
> >>> and submit it for review here.  You don't just submit adhoc driver
> >>> local stuff.
> >>
> >> David, what about enabling ethtool to read/write from/to the PHY
> >> registers (via passing PHY ID and register number)? I'm trying to get
> >> ICplus IP175D running and it has lots of registers, that I would like
> >> to look at dynamically.
> >
> > For debugging, register dumps are already supported by ethtool.
> 
> Do you mean 'ethtool -e' option? AFAIK it dumps Ethernet controller
> regs, if it implements ethtool_ops.get_regs(). What I need are PHY
> registers, that I can get via MII interface. ethtool should provide
> access to PHY registers like mdiobus_read() and mdiobus_write()
> routines.
[...]

Perhaps, but only as a test/debug interface.  It is not acceptable as a
production configuration interface - that needs some abstraction so the
same command will work on any switch chip that can support the
operation.

Ben.
Yegor Yefremov March 13, 2014, 10:42 p.m. UTC | #13
On Wed, Mar 12, 2014 at 9:21 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Wed, 2014-03-12 at 09:43 +0100, Yegor Yefremov wrote:
>> On Tue, Mar 11, 2014 at 9:39 PM, David Miller <davem@davemloft.net> wrote:
>> > From: Yegor Yefremov <yegorslists@googlemail.com>
>> > Date: Tue, 11 Mar 2014 21:11:16 +0100
>> >
>> >> On Fri, Feb 28, 2014 at 7:00 PM, David Miller <davem@davemloft.net> wrote:
>> >>> From: Stefan Roese <sr@denx.de>
>> >>> Date: Fri, 28 Feb 2014 10:32:20 +0100
>> >>>
>> >>>> On 27.02.2014 23:02, David Miller wrote:
>> >>>>> From: Stefan Roese <sr@denx.de>
>> >>>>> Date: Thu, 27 Feb 2014 10:07:52 +0100
>> >>>>>
>> >>>>> Better is to come up with a real, types, interface for programming
>> >>>>> such devices and providing an implementation of that.
>> >>>>
>> >>>> Right. Such a thing would be better. But as Florian already pointed
>> >>>> out, nothing like this is available in kernel.org right now. Thats the
>> >>>> reason why I chose to implement this "simple" driver, btw as done in
>> >>>> drivers/net/phy/spi_ks8995.c.
>> >>>
>> >>> When a suitable user facing interface is lacking, you create one
>> >>> and submit it for review here.  You don't just submit adhoc driver
>> >>> local stuff.
>> >>
>> >> David, what about enabling ethtool to read/write from/to the PHY
>> >> registers (via passing PHY ID and register number)? I'm trying to get
>> >> ICplus IP175D running and it has lots of registers, that I would like
>> >> to look at dynamically.
>> >
>> > For debugging, register dumps are already supported by ethtool.
>>
>> Do you mean 'ethtool -e' option? AFAIK it dumps Ethernet controller
>> regs, if it implements ethtool_ops.get_regs(). What I need are PHY
>> registers, that I can get via MII interface. ethtool should provide
>> access to PHY registers like mdiobus_read() and mdiobus_write()
>> routines.
> [...]
>
> Perhaps, but only as a test/debug interface.  It is not acceptable as a
> production configuration interface - that needs some abstraction so the
> same command will work on any switch chip that can support the
> operation.

Sure, I'd like to have this functionality for test/debug purpose.

Yegor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings March 14, 2014, 2:29 p.m. UTC | #14
On Thu, 2014-03-13 at 23:42 +0100, Yegor Yefremov wrote:
> On Wed, Mar 12, 2014 at 9:21 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> > On Wed, 2014-03-12 at 09:43 +0100, Yegor Yefremov wrote:
> >> On Tue, Mar 11, 2014 at 9:39 PM, David Miller <davem@davemloft.net> wrote:
> >> > From: Yegor Yefremov <yegorslists@googlemail.com>
[...]
> >> >> David, what about enabling ethtool to read/write from/to the PHY
> >> >> registers (via passing PHY ID and register number)? I'm trying to get
> >> >> ICplus IP175D running and it has lots of registers, that I would like
> >> >> to look at dynamically.
> >> >
> >> > For debugging, register dumps are already supported by ethtool.
> >>
> >> Do you mean 'ethtool -e' option? AFAIK it dumps Ethernet controller
> >> regs, if it implements ethtool_ops.get_regs(). What I need are PHY
> >> registers, that I can get via MII interface. ethtool should provide
> >> access to PHY registers like mdiobus_read() and mdiobus_write()
> >> routines.
> > [...]
> >
> > Perhaps, but only as a test/debug interface.  It is not acceptable as a
> > production configuration interface - that needs some abstraction so the
> > same command will work on any switch chip that can support the
> > operation.
> 
> Sure, I'd like to have this functionality for test/debug purpose.

OK.  First check that mii-tool doesn't already provide what you need.

If it doesn't, please submit patches for ethtool to support MDIO
register get/set.  These should cover both clause 22 and clause 45 PHYs.
Register dump may also be useful, but requires some thought about how to
handle the clause 45 address range (65536 registers per MMD, potentially
2 million per PRTAD).

Ben.
diff mbox

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 9b5d46c..451cfd3d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -193,6 +193,13 @@  config MDIO_BUS_MUX_MMIOREG
 
 	  Currently, only 8-bit registers are supported.
 
+config SMSC_LAN9303
+	tristate "SMSC LAN9303 3-ports 10/100 ethernet switch"
+	depends on SYSFS
+	help
+	  This module provides a driver for SMSC LAN9303 3 port ethernet
+	  switch. Configuration can be done via the sysfs entries.
+
 endif # PHYLIB
 
 config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 9013dfa..00244eb 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -32,3 +32,4 @@  obj-$(CONFIG_MDIO_BUS_MUX_GPIO)	+= mdio-mux-gpio.o
 obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o
 obj-$(CONFIG_MDIO_SUN4I)	+= mdio-sun4i.o
 obj-$(CONFIG_MDIO_MOXART)	+= mdio-moxart.o
+obj-$(CONFIG_SMSC_LAN9303)	+= lan9303.o
diff --git a/drivers/net/phy/lan9303.c b/drivers/net/phy/lan9303.c
new file mode 100644
index 0000000..50959eb
--- /dev/null
+++ b/drivers/net/phy/lan9303.c
@@ -0,0 +1,241 @@ 
+/*
+ * SMSC/Microchip LAN9303 switch driver
+ *
+ * Copyright (c) 2014 Stefan Roese <sr@denx.de>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+
+/* Generate phy-addr and -reg from the input address */
+#define PHY_ADDR(x)		((((x) >> 6) + 0x10) & 0x1f)
+#define PHY_REG(x)		(((x) >> 1) & 0x1f)
+
+#define LAN9303_ID		0x0050
+#define LAN9303_ID_VAL		0x9303
+
+#define BYTE_TEST		0x0064
+
+#define SWITCH_CSR_DATA		0x01ac
+#define SWITCH_CSR_CMD		0x01b0
+
+#define SWITCH_CSR_CMD_BUSY	0x80000000
+#define SWITCH_CSR_CMD_READ	0x40000000
+#define SWITCH_CSR_CMD_BE_ALL	0x000f0000
+
+#define TIMEOUT			100	/* msecs */
+
+static u32 reg_addr;
+static DEFINE_SPINLOCK(sysfs_lock);
+
+static u16 lan9303_read(struct phy_device *phydev, int reg)
+{
+	return phydev->bus->read(phydev->bus, PHY_ADDR(reg), PHY_REG(reg));
+}
+
+static void lan9303_write(struct phy_device *phydev, int reg, u16 val)
+{
+	phydev->bus->write(phydev->bus, PHY_ADDR(reg), PHY_REG(reg), val);
+}
+
+static u32 lan9303_read32(struct phy_device *phydev, int reg)
+{
+	u32 val;
+
+	mutex_lock(&phydev->bus->mdio_lock);
+	val = lan9303_read(phydev, reg);
+	val |= (lan9303_read(phydev, reg + 2) << 16) & 0xffff0000;
+	mutex_unlock(&phydev->bus->mdio_lock);
+
+	return val;
+}
+
+static void lan9303_write32(struct phy_device *phydev, int reg, u32 val)
+{
+	mutex_lock(&phydev->bus->mdio_lock);
+	lan9303_write(phydev, reg, val & 0xffff);
+	lan9303_write(phydev, reg + 2, (val >> 16) & 0xffff);
+	mutex_unlock(&phydev->bus->mdio_lock);
+}
+
+static int lan9303_wait_idle(struct phy_device *dev)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(TIMEOUT);
+
+	while (time_after(timeout, jiffies)) {
+		if (!(lan9303_read32(dev, SWITCH_CSR_CMD) & SWITCH_CSR_CMD_BUSY))
+			return 0;
+	}
+
+	pr_err("Timed out waiting for idle (reg=0x%08x)!\n",
+	       lan9303_read32(dev, SWITCH_CSR_CMD));
+
+	return -ETIMEDOUT;
+}
+
+static u32 lan9303_read_indirect(struct phy_device *dev, int reg)
+{
+	u32 tmp;
+
+	lan9303_wait_idle(dev);
+	lan9303_write32(dev, SWITCH_CSR_CMD,
+			SWITCH_CSR_CMD_BUSY | SWITCH_CSR_CMD_READ |
+			SWITCH_CSR_CMD_BE_ALL | reg);
+
+	/* Flush the previous write by reading the BYTE_TEST register */
+	tmp = lan9303_read32(dev, BYTE_TEST);
+	lan9303_wait_idle(dev);
+
+	return lan9303_read32(dev, SWITCH_CSR_DATA);
+}
+
+static void lan9303_write_indirect(struct phy_device *dev, int reg, u32 val)
+{
+	u32 tmp;
+
+	lan9303_wait_idle(dev);
+	lan9303_write32(dev, SWITCH_CSR_DATA, val);
+	lan9303_write32(dev, SWITCH_CSR_CMD,
+			SWITCH_CSR_CMD_BUSY | SWITCH_CSR_CMD_BE_ALL | reg);
+
+	/* Flush the previous write by reading the BYTE_TEST register */
+	tmp = lan9303_read32(dev, BYTE_TEST);
+	lan9303_wait_idle(dev);
+}
+
+static ssize_t lan9303_reg_addr_show(struct device *d,
+				     struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%04x\n", reg_addr);
+}
+
+static ssize_t lan9303_reg_addr_store(struct device *d,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	sscanf(buf, "%x", &reg_addr);
+
+	return strnlen(buf, count);
+}
+
+static DEVICE_ATTR(lan9303_reg_addr, S_IRUGO | S_IWUSR,
+		   lan9303_reg_addr_show, lan9303_reg_addr_store);
+
+static ssize_t lan9303_reg_val_show(struct device *d,
+				    struct device_attribute *attr, char *buf)
+{
+	struct phy_device *phydev = dev_get_drvdata(d);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&sysfs_lock, flags);
+	val = lan9303_read_indirect(phydev, reg_addr);
+	spin_unlock_irqrestore(&sysfs_lock, flags);
+
+	return sprintf(buf, "%08x\n", val);
+}
+
+static ssize_t lan9303_reg_val_store(struct device *d,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct phy_device *phydev = dev_get_drvdata(d);
+	unsigned long flags;
+	u32 val;
+
+	sscanf(buf, "%x", &val);
+
+	spin_lock_irqsave(&sysfs_lock, flags);
+	lan9303_write_indirect(phydev, reg_addr, val);
+	spin_unlock_irqrestore(&sysfs_lock, flags);
+
+	return strnlen(buf, count);
+}
+
+static DEVICE_ATTR(lan9303_reg_val, S_IRUGO | S_IWUSR,
+		   lan9303_reg_val_show, lan9303_reg_val_store);
+
+static struct attribute *lan9303_sysfs_entries[] = {
+	&dev_attr_lan9303_reg_addr.attr,
+	&dev_attr_lan9303_reg_val.attr,
+	NULL
+};
+
+static struct attribute_group lan9303_attribute_group = {
+	.name = NULL,		/* put in device directory */
+	.attrs = lan9303_sysfs_entries,
+};
+
+static int lan9303_match_phy_device(struct phy_device *phydev)
+{
+	/*
+	 * One dummy read access needed to reliably detect the switch
+	 */
+	lan9303_read32(phydev, BYTE_TEST);
+
+	if (lan9303_read(phydev, LAN9303_ID + 2) == LAN9303_ID_VAL) {
+		phydev->phy_id = lan9303_read32(phydev, LAN9303_ID);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int lan9303_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	dev_set_drvdata(&phydev->dev, phydev);
+	ret = sysfs_create_group(&phydev->dev.kobj, &lan9303_attribute_group);
+	if (ret) {
+		dev_err(&phydev->dev, "unable to create sysfs file, err=%d\n",
+			ret);
+		return ret;
+	}
+
+	dev_info(&phydev->dev, "SMSC LAN9303 switch found, Chip ID:%04x, Revision:%04x\n",
+		 lan9303_read(phydev, LAN9303_ID + 2),
+		 lan9303_read(phydev, LAN9303_ID));
+
+	return 0;
+}
+
+static struct phy_driver lan9303_pdriver = {
+	.phy_id		= LAN9303_ID_VAL,
+	.phy_id_mask	= 0xffffffff,
+	.name		= "SMSC LAN9303",
+	.features	= PHY_BASIC_FEATURES,
+	.flags		= 0,
+	.config_init	= lan9303_config_init,
+	.config_aneg	= genphy_config_aneg,
+	.read_status	= genphy_read_status,
+	.match_phy_device = lan9303_match_phy_device,
+	.driver		= { .owner = THIS_MODULE, },
+};
+
+static int __init lan9303_init(void)
+{
+	return phy_driver_register(&lan9303_pdriver);
+}
+module_init(lan9303_init);
+
+static void __exit lan9303_exit(void)
+{
+	phy_driver_unregister(&lan9303_pdriver);
+}
+module_exit(lan9303_exit);
+
+MODULE_DESCRIPTION("SMSC LAN9303 switch driver");
+MODULE_AUTHOR("Stefan Roese <sr@denx.de>");
+MODULE_LICENSE("GPL-2.0+");