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]

Message-ID: <1362754401.3248.8.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Fri, 8 Mar 2013 14:53:21 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Mugunthan V N <mugunthanvnm@...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 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.

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).

Ben.

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

--
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