netdev - Re: [PATCH v2 1/2] Documentation: add Device tree bindings for Hisilicon hix5hd2 ethernet

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140528125354.GA2092@leverpostej>
Date:	Wed, 28 May 2014 13:53:54 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	zhangfei <zhangfei.gao@...aro.org>
Cc:	"davem@...emloft.net" <davem@...emloft.net>,
	"arnd@...db.de" <arnd@...db.de>,
	"f.fainelli@...il.com" <f.fainelli@...il.com>,
	"sergei.shtylyov@...entembedded.com" 
	<sergei.shtylyov@...entembedded.com>,
	"David.Laight@...LAB.COM" <David.Laight@...LAB.COM>,
	"eric.dumazet@...il.com" <eric.dumazet@...il.com>,
	"haifeng.yan@...aro.org" <haifeng.yan@...aro.org>,
	"jchxue@...il.com" <jchxue@...il.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] Documentation: add Device tree bindings for
 Hisilicon hix5hd2 ethernet

On Wed, May 28, 2014 at 06:41:40AM +0100, zhangfei wrote:
> Dear Mark
> 
> On 05/27/2014 09:34 PM, Mark Rutland wrote:
> > On Tue, May 27, 2014 at 01:44:26PM +0100, Zhangfei Gao wrote:
> >> Signed-off-by: Zhangfei Gao <zhangfei.gao@...aro.org>
> >> ---
> >>   .../bindings/net/hisilicon-hix5hd2-gmac.txt        |   36 ++++++++++++++++++++
> >>   1 file changed, 36 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> >> new file mode 100644
> >> index 0000000..5fe3835
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> >> @@ -0,0 +1,36 @@
> >> +Hisilicon hix5hd2 gmac controller
> >
> > Just to clarify, is the SoC name "hix5hd2", or is the 'x' a wildcard?
> "hix5hd2" is Soc name, which contains a series of similar chips.

How similar are they?

It's preferable to have a precise name, even when used as a fallback for
other similar devices.

> >
> >> +
> >> +Required properties:
> >> +- compatible: should be "hisilicon,hix5hd2-gmac".
> >> +- reg: specifies base physical address(s) and size of the device registers.
> >> +  The first region is the MAC register base and size.
> >> +  The second region is external interface control register.
> >
> > Single registers? Are these not part of a larger block?
> It is a single register, outside of the memory region.
> gmac0: reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
> gmac1: reg = <0xf9841000 0x1000>,<0xf9843010 0x4>;
> 
> The register is controlling interface mode, duplex etc.
> In fact it is rather a bug fix to the silicon, when it is added with 
> intension of not touching the original ip design.
> 
> It may be moved to the memory region in the future silicon design.
> However, currently without such register, it can not work.

I see. Is it possible that a future revision might have this fixed, such
that the second reg entry would become optional?

> >
> >> +- interrupts: should contain the MAC interrupts
> >
> > How many, in which order?
> There is only one interrrupt.
> How about:
> - interrupts: interrupt for the device

The original wording is OK, all you need to do is change the original
wording to get rid of the trailing 's':

- interrupts: should contain the MAC interrupt

> 
> >
> >> +- #address-cells: must be <1>.
> >> +- #size-cells: must be <0>.
> >> +- phy-mode: see ethernet.txt [1].
> >> +- phy-handle: see ethernet.txt [1].
> >> +- mac-address: see ethernet.txt [1].
> >> +- clocks: clock phandle and specifier pair.
> >
> > Is this the only clock input to the gmac block?
> >
> > Is the clock input named in any documentation?
> 
> We have abstract only ONE mac clock input, which is in other patch for 
> drivers/clk/hisilicon/clk-hix5hd2.c
> The clock input name is in include/dt-bindings/clock/hix5hd2-clock.h, 
> described in "Documentation/devicetree/bindings/clock/hix5hd2-clock.txt"

If you have only one physical clock input in the GMAC block, that's
fine. Is there a name for that input line from the POV of the GMAC
block?

> How about:
> - clocks: a pointer to the reference clock for this device.

I think the original wording is OK, I'd just like to be clear on what
the HW looks like.

> 
> Or should I mention hix5hd2-clock.h in this binding?

No, that's part of the description of the clock provider. It shouldn't
be mentioned here.

Cheers,
Mark.
--
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