Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
> "M" == Mugunthan V N writes: M> This patch implements get/set of the phy settings via ethtool apis M> Signed-off-by: Mugunthan V N 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. It would be good if all these special things (dual emac mode, vlan handling, switching) could be handled using the existing kernel (bridging/vlan) infrastructure, and the driver always just exposing 2 network interfaces instead of these configuration properties. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
On 3/7/2013 6:54 PM, Peter Korsgaard wrote: "M" == Mugunthan V N writes: M> This patch implements get/set of the phy settings via ethtool apis M> Signed-off-by: Mugunthan V N 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. It would be good if all these special things (dual emac mode, vlan handling, switching) could be handled using the existing kernel (bridging/vlan) infrastructure, and the driver always just exposing 2 network interfaces instead of these configuration properties. Switch and Dual Emac modes of operation of CPSW are two different features of the hardware and packet routing between the slaves in the hardware are different in both the modes. If by default it is brought up as Dual EMAC then hardware switching is blocked and use-cases like IP phone etc cannot be achieved. Since CPSW as a hardware Switch, it cannot not be handled in existing kernel feature. Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
> "Mugunthan" == Mugunthan V N writes: Hi, M> +- ethtool-active-slave : Specifies the slave to use for ethtool command >> >> That again sounds like something Linux specific rather than a hardware >> property. >> >> It would be good if all these special things (dual emac mode, vlan >> handling, switching) could be handled using the existing kernel >> (bridging/vlan) infrastructure, and the driver always just exposing 2 >> network interfaces instead of these configuration properties. Mugunthan> Switch and Dual Emac modes of operation of CPSW are two Mugunthan> different features of the hardware and packet routing Mugunthan> between the slaves in the hardware are different in both the Mugunthan> modes. Mugunthan> If by default it is brought up as Dual EMAC then hardware Mugunthan> switching is blocked and use-cases like IP phone etc cannot Mugunthan> be achieved. Well, you could use the (sw) bridge functionality of the kernel network stack, but performance naturally wouldn't be as good. Mugunthan> Since CPSW as a hardware Switch, it cannot not be handled in Mugunthan> existing kernel feature. Well, we do have net/dsa, which is conceptually quite similar (even though it has never been extended to hook into the bridging stuff). I agree that we don't have infrastructure to handle hw like cpsw in a really good way today, but it would be very nice to move towards it. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
On Thu, 2013-03-07 at 14:24 +0100, Peter Korsgaard wrote: > > "M" == Mugunthan V N writes: > > M> This patch implements get/set of the phy settings via ethtool apis > M> Signed-off-by: Mugunthan V N > 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. > It would be good if all these special things (dual emac mode, vlan > handling, switching) could be handled using the existing kernel > (bridging/vlan) infrastructure, and the driver always just exposing 2 > network interfaces instead of these configuration properties. > -- 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 linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
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 writes: M> This patch implements get/set of the phy settings via ethtool apis M> Signed-off-by: Mugunthan V N 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. Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 writes: > >> M> This patch implements get/set of the phy settings via ethtool apis > >> M> Signed-off-by: Mugunthan V N > >> 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 linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
> "Ben" == Ben Hutchings writes: Hi, Ben> The 'active slave' property would also be needed for the SIOCGMIIPHY Ben> ioctl and not just ethtool. But it's really quite arbitrary. Perhaps Ben> each of them should have their own net device (as with DSA). Indeed, I think that would simplify all of this quite a bit. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 writes: M> This patch implements get/set of the phy settings via ethtool apis M> Signed-off-by: Mugunthan V N 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 linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
On 3/8/2013 8:34 PM, Peter Korsgaard wrote: "Ben" == Ben Hutchings writes: Hi, Ben> The 'active slave' property would also be needed for the SIOCGMIIPHY Ben> ioctl and not just ethtool. But it's really quite arbitrary. Perhaps Ben> each of them should have their own net device (as with DSA). Indeed, I think that would simplify all of this quite a bit. I will update this in the next patch series Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
On Fri, Mar 08, 2013 at 08:37:00PM +0530, Mugunthan V N wrote: > > As Peter Korsgaard mentioned we need to have infrastructure to handle CPSW > kind of hardware. This will be a big job, I think, but I agree that it is worth doing. I can think of one other switch-with-host-port device. Maybe we should start off by making a list of actual products and their capabilities, in order to get an overall idea of what we would need to develop. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] driver: net: ethernet: cpsw: implement ethtool get/set phy setting
> "Richard" == Richard Cochran writes: Hi, >> As Peter Korsgaard mentioned we need to have infrastructure to >> handle CPSW kind of hardware. Richard> This will be a big job, I think, but I agree that it is worth doing. Richard> I can think of one other switch-with-host-port device. Maybe Richard> we should start off by making a list of actual products and Richard> their capabilities, in order to get an overall idea of what we Richard> would need to develop. Next to the dsa stuff with external switches, I can think of 3 other devices off the top of my head: Freescale imx287: http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=i.MX287 Freescale T1022: http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=T1020 Ralink RT3502: http://www.netcheif.com/Reviews/VigorFly200/PDF/RT3052-product%20brief.pdf -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
