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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Fri, 15 Oct 2010 21:39:49 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	David Daney <ddaney@...iumnetworks.com>
Cc:	devicetree-discuss@...ts.ozlabs.org,
	Netdev <netdev@...r.kernel.org>
Subject: Re: RFD: OF device tree vs. PHY flags.

Hi David,

On Fri, Oct 15, 2010 at 11:18:00AM -0700, David Daney wrote:
> I am in the process of planning a conversion of Octeon SOC platform
> code to use the OF device tree in the Linux kernel.
> 
> One issue that I have encountered, is that for some boards, we need
> to pass a non-zero flags argument to the phy_attach_direct() method.
> The value of the flags is board dependent, so it would make some
> sense to encode its value in the device tree itself.  The flags I am
> interested in control the configuration of clocking modes and status
> LED connections.
> 
> I would suggest the following:
> 
> o Add a new property to "ethernet-phy" dts bindings called
> "linux,flags".  It would contain a comma separated string of flag
> names.  Something like "led-mode1,clock-mode2".  The semantics of
> the flag names would be interpreted by the PHY driver...

It does make sense to encode board-specific properties into the device
tree.  However, it is a very bad idea to do it in this form because it
encodes Linux-specific implementation details into the hardware
description.  Instead, figure out what needs to be described about the
*hardware* and write a binding that encodes that information.  Let the
driver take care of translating the hardware description into the set
of flags that it needs to use.

Otherwise you'll end up in the situation where the phylib
implementation changes and the data in the device tree will no longer
make sense.  That is the path of pain.

> o Add a new function pointer to struct phy_driver: u32
> (*of_parse_flags)(struct phy_device *phydev).  This would parse and
> return the flags value for the "linux,flags" property from the
> device_node associated with the particular PHY device in question.

I'm not clear why a new hook is needed.  What prevents the phy driver
from parsing the device tree data at .probe() time?  The caller of
phy_attach_direct() would then never need to be exposed to the parsing
of the tree data.

> o Modify of_phy_connect() to do something like the following:
> 
> .
> .
> .
> 	if (phy->driver && phy->driver->of_parse_flags)
> 		flags |= phy->driver->of_parse_flags(phy);
> .
> .
> .
> 
> o Perhaps add some helper functions to of_mdio.c to assist in
> parsing the "linux,flags" properties string.
> 
> o Any extra code in the PHY drivers and struct phy_driver would be
> protected by #ifdef CONFIG_OF

Yes.

--
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

Powered by blists - more mailing lists