netdev - Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20141112070127.GF30369@pengutronix.de>
Date:	Wed, 12 Nov 2014 08:01:27 +0100
From:	Sascha Hauer <s.hauer@...gutronix.de>
To:	Johan Hovold <johan@...nel.org>
Cc:	Mark Rutland <mark.rutland@....com>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to
 eth-phy binding

On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > documentation.
> > > 
> > > Cc: devicetree@...r.kernel.org
> > > Signed-off-by: Johan Hovold <johan@...nel.org>
> > > ---
> > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > index a1bab5eaae02..9b08dd6551dd 100644
> > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > @@ -19,6 +19,11 @@ Optional properties:
> > >  
> > >                See the respective PHY datasheet for the mode values.
> > >  
> > > + - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
> > > +
> > > +		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
> > > +		when the rmii_ref_clk_sel bit is set.
> > 
> > s/_/-/ in property names please.
> 
> Ouch, copied from variable name, sorry.
> 
> > That said, I don't follow the meaning. Does this cause the kernel to do
> > something different, or is is simply that a 25MHz ref clock is wired up?
> 
> Yes, the driver currently sets this configuration bit based on a common
> clock binding.
> 
> However, it turns out the meaning of the bit is reversed on some PHY
> variants. On most PHYs 50 MHz mode is selected by setting this bit,
> whereas on the PHYs that need this new property, setting it selects 25
> MHz mode instead.

Maybe rename the property to something like rmii-ref-clk-25mhz-active-high
then? Also you should probably make it more explicit that this is a
hardware property and not for adjusting the clock.

It's not very nice from Micrel to create Phys with exactly the same
device id but the meaning of this bit reversed.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
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