Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting

On 3/8/2013 8:23 PM, Ben Hutchings wrote:

On Fri, 2013-03-08 at 12:53 +0530, Mugunthan V N wrote:

quoted

On 3/8/2013 1:29 AM, Ben Hutchings wrote:

quoted

On Thu, 2013-03-07 at 14:24 +0100, Peter Korsgaard wrote:

quoted

quoted

quoted

quoted

quoted

quoted

"M" == Mugunthan V N [off-list ref] writes:

   M> This patch implements get/set of the phy settings via ethtool apis
   M> Signed-off-by: Mugunthan V N [off-list ref]
   M> ---
   M>  Documentation/devicetree/bindings/net/cpsw.txt |    3 +++
   M>  drivers/net/ethernet/ti/cpsw.c                 |   32 ++++++++++++++++++++++++
   M>  include/linux/platform_data/cpsw.h             |    1 +
   M>  3 files changed, 36 insertions(+)

   M> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
   M> index ecfdf75..8d61300 100644
   M> --- a/Documentation/devicetree/bindings/net/cpsw.txt
   M> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
   M> @@ -20,6 +20,7 @@ Required properties:
   M>  - cpts_clock_shift	: Denominator to convert input clock ticks into nanoseconds
   M>  - phy_id		: Specifies slave phy id
   M>  - mac-address		: Specifies slave MAC address
   M> +- ethtool-active-slave	: Specifies the slave to use for ethtool command

That again sounds like something Linux specific rather than a hardware
property.

Yes, indeed.  Isn't it redundant with the phy_id?

Ben.

phy_id is part of slave data and will be present for both the slaves.
so phy_id cannot be used for get/set phy setting until phy framework
allows to change settings without going through eth interface.

Now I've looked at the examples in this file, I think I see what you're
getting at.  What confused me is that you're adding to a single list of
properties without a proper distinction of which devices they are
applied to.  It really ought to be properly divided between switch and
'slave' properties.

Will fix this in next patch series.

The 'active slave' property would also be needed for the SIOCGMIIPHY
ioctl and not just ethtool.  But it's really quite arbitrary.  Perhaps
each of them should have their own net device (as with DSA).

But if we have net device for each of the slaves then it is dual EMAC 
which will kill
hardware switching functionality. To achieve switching bridge has to be 
done and
there will be a performance drop as well.

As Peter Korsgaard mentioned we need to have infrastructure to handle CPSW
kind of hardware.

Regards
Mugunthan V N