All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: David Miller <davem@davemloft.net>, <netdev@vger.kernel.org>,
	<linux-omap@vger.kernel.org>, <devicetree@vger.kernel.org>,
	Andrew Lunn <andrew@lunn.ch>,
	Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Murali Karicheri <m-karicheri2@ti.com>,
	Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: net: cpsw: Document cpsw-phy-sel usage but prefer phandle
Date: Thu, 30 Aug 2018 12:04:49 -0500	[thread overview]
Message-ID: <69161960-259c-6911-67b5-546ccb433165@ti.com> (raw)
In-Reply-To: <20180830004745.GU7523@atomide.com>



On 08/29/2018 07:47 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [180830 00:12]:
>> Hi Tony,
>>
>> On 08/29/2018 10:00 AM, Tony Lindgren wrote:
>>> The current cpsw usage for cpsw-phy-sel is undocumented but is used for
>>> all the boards using cpsw. And cpsw-phy-sel is not really a child of
>>> the cpsw device, it lives in the system control module instead.
>>>
>>> Let's document the existing usage, and improve it a bit where we prefer
>>> to use a phandle instead of a child device for it. That way we can
>>> properly describe the hardware in dts files for things like genpd.
>>
>> I'm ok with this series, but I really don't like cpsw-phy-sel in general.
> 
> Yeah this binding predates any standards. This series
> only fixes the nasty issue of cpsw claiming a module as a
> child that's outside it's IO range.
> 
>> It was introduced long time back and now I'm thinking about possibility to replace it with
>> one of current generic interfaces - for example mux-controller.
>> Each port will control up to 3 muxes (port mode, idmode and rmii_ext_clk) and
>> transform phy-mode => mux states.
>> What do you think?
> 
> Sure a mux-controller here makes sense.
> 
>> Another option is to use phy, but it'd be complicated.
> 
> For the port muxes, how about a phy driver just using
> a pinctrl driver?
> 
> In general, it seems cpsw is just an interconnect instance
> (L4_FAST) with a control module (CPSW_WR) and a pile of
> independent other modules. That's described nicely in
> am437x TRM chapter "2.1.4 L4 Fast Peripheral Memory Map".
> So from that point of view the binding reg entries right
> now are all wrong :)

TRM not consistent - for am5 it's one MMIO region.

> 
> In the long run cpsw should be really treated as an
> interconnect instance with it's control module providing
> standard Linux framework services such as clock /
> regulator / phy / pinctrl / iio whatever for the other
> modules.
> 
> Just my 2c based on looking at the interconnect, I'm
> not too familiar with cpsw otherwise.

It's not separate modules. this is composite module which have only one 
fck/ick and most of blocks can't even function without each other.
Above might be the case for Keystone 2, but not omap CPSW.
Keystone 2 - has packet processor, security accelerator, queue manager 
in addition to its basic switch block.

-- 
regards,
-grygorii

WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org, linux-omap@vger.kernel.org,
	devicetree@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Murali Karicheri <m-karicheri2@ti.com>,
	Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: net: cpsw: Document cpsw-phy-sel usage but prefer phandle
Date: Thu, 30 Aug 2018 12:04:49 -0500	[thread overview]
Message-ID: <69161960-259c-6911-67b5-546ccb433165@ti.com> (raw)
In-Reply-To: <20180830004745.GU7523@atomide.com>



On 08/29/2018 07:47 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [180830 00:12]:
>> Hi Tony,
>>
>> On 08/29/2018 10:00 AM, Tony Lindgren wrote:
>>> The current cpsw usage for cpsw-phy-sel is undocumented but is used for
>>> all the boards using cpsw. And cpsw-phy-sel is not really a child of
>>> the cpsw device, it lives in the system control module instead.
>>>
>>> Let's document the existing usage, and improve it a bit where we prefer
>>> to use a phandle instead of a child device for it. That way we can
>>> properly describe the hardware in dts files for things like genpd.
>>
>> I'm ok with this series, but I really don't like cpsw-phy-sel in general.
> 
> Yeah this binding predates any standards. This series
> only fixes the nasty issue of cpsw claiming a module as a
> child that's outside it's IO range.
> 
>> It was introduced long time back and now I'm thinking about possibility to replace it with
>> one of current generic interfaces - for example mux-controller.
>> Each port will control up to 3 muxes (port mode, idmode and rmii_ext_clk) and
>> transform phy-mode => mux states.
>> What do you think?
> 
> Sure a mux-controller here makes sense.
> 
>> Another option is to use phy, but it'd be complicated.
> 
> For the port muxes, how about a phy driver just using
> a pinctrl driver?
> 
> In general, it seems cpsw is just an interconnect instance
> (L4_FAST) with a control module (CPSW_WR) and a pile of
> independent other modules. That's described nicely in
> am437x TRM chapter "2.1.4 L4 Fast Peripheral Memory Map".
> So from that point of view the binding reg entries right
> now are all wrong :)

TRM not consistent - for am5 it's one MMIO region.

> 
> In the long run cpsw should be really treated as an
> interconnect instance with it's control module providing
> standard Linux framework services such as clock /
> regulator / phy / pinctrl / iio whatever for the other
> modules.
> 
> Just my 2c based on looking at the interconnect, I'm
> not too familiar with cpsw otherwise.

It's not separate modules. this is composite module which have only one 
fck/ick and most of blocks can't even function without each other.
Above might be the case for Keystone 2, but not omap CPSW.
Keystone 2 - has packet processor, security accelerator, queue manager 
in addition to its basic switch block.

-- 
regards,
-grygorii

  parent reply	other threads:[~2018-08-30 21:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-29 15:00 [PATCH 1/2] dt-bindings: net: cpsw: Document cpsw-phy-sel usage but prefer phandle Tony Lindgren
2018-08-29 15:00 ` [PATCH 2/2] net: ethernet: cpsw-phy-sel: prefer phandle for phy sel Tony Lindgren
2018-09-02 20:52   ` David Miller
2018-08-30  0:08 ` [PATCH 1/2] dt-bindings: net: cpsw: Document cpsw-phy-sel usage but prefer phandle Grygorii Strashko
2018-08-30  0:08   ` Grygorii Strashko
2018-08-30  0:47   ` Tony Lindgren
2018-08-30  1:18     ` Andrew Lunn
2018-08-30 17:04     ` Grygorii Strashko [this message]
2018-08-30 17:04       ` Grygorii Strashko
2018-08-30 21:55       ` Tony Lindgren
2018-09-02 20:52 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=69161960-259c-6911-67b5-546ccb433165@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=ivan.khoronzhuk@linaro.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=m-karicheri2@ti.com \
    --cc=mark.rutland@arm.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.