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

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:	Fri, 8 Mar 2013 20:37:00 +0530
From:	Mugunthan V N <mugunthanvnm@...com>
To:	Ben Hutchings <bhutchings@...arflare.com>
CC:	Peter Korsgaard <jacmet@...site.dk>, <netdev@...r.kernel.org>,
	<davem@...emloft.net>, <devicetree-discuss@...ts.ozlabs.org>,
	<linux-omap@...r.kernel.org>, <b-cousson@...com>, <paul@...an.com>
Subject: 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:
>> On 3/8/2013 1:29 AM, Ben Hutchings wrote:
>>> On Thu, 2013-03-07 at 14:24 +0100, Peter Korsgaard wrote:
>>>>>>>>> "M" == Mugunthan V N <mugunthanvnm@...com> writes:
>>>>    M> This patch implements get/set of the phy settings via ethtool apis
>>>>    M> Signed-off-by: Mugunthan V N <mugunthanvnm@...com>
>>>>    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
--
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