[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqLHrHWtiikSt7F39LoExoL2aep5Pqax5cGWWBw-yhz79g@mail.gmail.com>
Date: Wed, 27 Apr 2016 10:25:56 -0500
From: Rob Herring <robh@...nel.org>
To: Yisen Zhuang <Yisen.zhuang@...wei.com>
Cc: David Miller <davem@...emloft.net>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
netdev <netdev@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Will Deacon <will.deacon@....com>,
Catalin Marinas <catalin.marinas@....com>,
yankejian <yankejian@...wei.com>,
huangdaode <huangdaode@...ilicon.com>, salil.mehta@...wei.com,
lipeng321@...wei.com, liguozhu@...wei.com, xieqianqian@...wei.com,
Wei Xu <xuwei5@...ilicon.com>, Linuxarm <linuxarm@...wei.com>
Subject: Re: [PATCH v2 net-next 11/13] Documentation: Bindings: Update DT
binding for separating dsaf dev support
On Tue, Apr 26, 2016 at 10:33 PM, Yisen Zhuang <Yisen.zhuang@...wei.com> wrote:
> Hi Rob and David,
>
> Please see my comments inline.
>
> David have merged this series to net-next, but we need to modify some codes according
> to Rob's comments. I am not sure if i need to send V3 for this series, or separate
> patches of documentation to independent series and generate a new patch for hns base
> on current net-next?
That's David's call. I'm guessing he wants follow-up patches on top of these.
> 在 2016/4/26 20:48, Rob Herring 写道:
>> On Sat, Apr 23, 2016 at 05:05:15PM +0800, Yisen Zhuang wrote:
>>> Because debug dsaf port was separated from service dsaf port, this patch
>>> updates the related information of DT binding.
>>
>> Separated when? New version of the h/w? If so, where's the new
>> compatible string? This is quite a big binding change.
>
> There isn't any change of h/w. I separated debug dsaf port from sevice dsaf
> port to make the code more simple and readability.
Okay.
[...]
>>> + serdes-syscon rather than this address.
>>> The third region is the PPE register base and size.
>>> - The fourth region is dsa fabric base register and size.
>>> - The fifth region is cpld base register and size, it is not required if do not use cpld.
>>> -- phy-handle: phy handle of physicl port, 0 if not any phy device. see ethernet.txt [1].
>>> + The fourth region is dsa fabric base register and size. It is not required for
>>> + single-port mode.
>>> +- reg-names: may be ppe-base and(or) dsaf-base. It is used to find the
>>> + corresponding reg's index.
>>
>> But you have up to 5 regions.
>>
>> The variable nature of what regions you have tells me you need more
>> specific compatible strings for each chip.
>
> we didn't add support of new h/w. We added these regions to make code simple and readability.
> If we need to add support of next h/w version next time, we don't need to add many branches
> for these attributes. So we didn't add a new compatible here.
Not sure what you mean by branches. It's fine to put properties for
things that vary among h/w versions, but new compatible strings will
be needed for any new versions.
>>> +- port: subnodes of dsaf. A dsaf node may contain several port nodes(Depending
>>> + on mode of dsaf). Port node contain some attributes listed below:
>>> +- port-id: is physical port index in one dsaf.
>>
>> Indexes should generally be avoided. What does the number correspond
>> to in h/w (if anything)?
>
> port-id is index for a port in dsaf, it is correspond to index of PHY showed below.
Okay, you should use reg property here instead.
>
> CPU
> |
> -----------------------------------
> | | |
> ---------------------------------------------- --------- ---------
> | | | | | | | |
> | PPE | | PPE | | PPE |
> | | | | | | | | |
> | | | | | | | | |
> | crossbar | | | | | | |
> | | | | | | | | |
> | ---------------------------------- | | | | | | |
> | | | | | | | | | | | | | |
> | | | | | | | | | | | | | |
> | MAC MAC MAC MAC MAC MAC | | MAC | | MAC |
> | | | | | | | | | | | | | |
> ---------------------------------------------- --------- ---------
> | | | | | | \ / | / |
> PHY PHY PHY PHY PHY PHY \ / PHY / PHY
> \ / /
> \ / /
> DSAF(three platform device)
>
>>
>>> +- phy-handle: phy handle of physicl port. It is not required if there isn't
Another typo here.
Rob
Powered by blists - more mailing lists