All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Mack <zonque@gmail.com>
To: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>,
	netdev@vger.kernel.org, davem@davemloft.net, ujhelyi.m@gmail.com,
	vaibhav.bedia@ti.com, d-gerlach@ti.com,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 3/4] net: ethernet: cpsw: add support for hardware interface mode config
Date: Fri, 23 Aug 2013 10:15:02 +0200	[thread overview]
Message-ID: <52171A06.9040400@gmail.com> (raw)
In-Reply-To: <5216FDBC.3010600@ti.com>

On 23.08.2013 08:14, Mugunthan V N wrote:
> On Friday 23 August 2013 11:00 AM, Sekhar Nori wrote:
>>> @@ -728,6 +736,44 @@ static void _cpsw_adjust_link(struct cpsw_slave *slave,
>>>>  	u32			mac_control = 0;
>>>>  	u32			slave_port;
>>>>  
>>>> +	if (priv->gmii_sel_reg && of_machine_is_compatible("ti,am33xx")) {
>> This sounds like the DT version of cpu_is_am335x()! Looks like you need
>> to introduce a new compatible "ti,am3352-cpsw" to indicate the AM3352
>> specific features of the CPSW IP (yeah, control module is not really
>> part of the IP, but by introducing it in the driver, we are treating it
>> such anyway. And you can see this register as extension of IP since its
>> not shared for any other purpose).
>>
> 
> I also agree but it should be ti,am3352-cpsw or ti.am33xx-cpsw?

I think the rule is to be as specific as possible, so I'll take
ti,am3352-cpsw.

Thank you very much for the review, everyone. I'll respin a new series
with all issues addressed.


Daniel

WARNING: multiple messages have this Message-ID (diff)
From: zonque@gmail.com (Daniel Mack)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] net: ethernet: cpsw: add support for hardware interface mode config
Date: Fri, 23 Aug 2013 10:15:02 +0200	[thread overview]
Message-ID: <52171A06.9040400@gmail.com> (raw)
In-Reply-To: <5216FDBC.3010600@ti.com>

On 23.08.2013 08:14, Mugunthan V N wrote:
> On Friday 23 August 2013 11:00 AM, Sekhar Nori wrote:
>>> @@ -728,6 +736,44 @@ static void _cpsw_adjust_link(struct cpsw_slave *slave,
>>>>  	u32			mac_control = 0;
>>>>  	u32			slave_port;
>>>>  
>>>> +	if (priv->gmii_sel_reg && of_machine_is_compatible("ti,am33xx")) {
>> This sounds like the DT version of cpu_is_am335x()! Looks like you need
>> to introduce a new compatible "ti,am3352-cpsw" to indicate the AM3352
>> specific features of the CPSW IP (yeah, control module is not really
>> part of the IP, but by introducing it in the driver, we are treating it
>> such anyway. And you can see this register as extension of IP since its
>> not shared for any other purpose).
>>
> 
> I also agree but it should be ti,am3352-cpsw or ti.am33xx-cpsw?

I think the rule is to be as specific as possible, so I'll take
ti,am3352-cpsw.

Thank you very much for the review, everyone. I'll respin a new series
with all issues addressed.


Daniel

  reply	other threads:[~2013-08-23  8:15 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-22 11:37 [PATCH 0/4] cpsw: support for control module register Daniel Mack
2013-08-22 11:37 ` Daniel Mack
2013-08-22 11:37 ` [PATCH 1/4] net: ethernet: cpsw: switch to devres allocations Daniel Mack
2013-08-22 11:37   ` Daniel Mack
2013-08-22 11:37 ` [PATCH 2/4] net: ethernet: cpsw: add optional third memory region for CONTROL module Daniel Mack
2013-08-22 11:37   ` Daniel Mack
2013-08-22 18:12   ` Sergei Shtylyov
2013-08-22 18:12     ` Sergei Shtylyov
2013-08-22 18:37     ` Daniel Mack
2013-08-22 18:37       ` Daniel Mack
2013-08-22 11:37 ` [PATCH 3/4] net: ethernet: cpsw: add support for hardware interface mode config Daniel Mack
2013-08-22 11:37   ` Daniel Mack
2013-08-23  5:30   ` Sekhar Nori
2013-08-23  5:30     ` Sekhar Nori
2013-08-23  5:30     ` Sekhar Nori
2013-08-23  6:14     ` Mugunthan V N
2013-08-23  6:14       ` Mugunthan V N
2013-08-23  6:14       ` Mugunthan V N
2013-08-23  8:15       ` Daniel Mack [this message]
2013-08-23  8:15         ` Daniel Mack
2013-08-23  6:11   ` Mugunthan V N
2013-08-23  6:11     ` Mugunthan V N
2013-08-23  6:11     ` Mugunthan V N
2013-08-22 11:37 ` [PATCH 4/4] ARM: dts: am33xx: add third memory region to cpsw block Daniel Mack
2013-08-22 11:37   ` Daniel Mack
2013-08-23  3:02 ` [PATCH 0/4] cpsw: support for control module register David Miller
2013-08-23  3:02   ` 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=52171A06.9040400@gmail.com \
    --to=zonque@gmail.com \
    --cc=d-gerlach@ti.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mugunthanvnm@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=nsekhar@ti.com \
    --cc=ujhelyi.m@gmail.com \
    --cc=vaibhav.bedia@ti.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.