[U-Boot] [EXT] [PATCH v2 2/4] doc: bindings: add mdio.txt describing generic MDIO properties

[Ken Ma]

Please see my inline rely, thanks a lot!

On 7/25/2019 1:08 PM, Ken Ma wrote:
> I added the optional property name in order that the legacy
> miiphy_get_dev_by_name() could be used with the mdio name.
That works with these patches, miiphy_get_dev_by_name is used, among others, by mdio command and mdio command shows the device with the name from DT, if the name property exists in DT.
There are two differences.  First is property name which is now device-name instead of mdio-name, I used that hoping that it caches on and other classes would use the same.
The other difference is that the device-name binding is now part of mdio, rather than marvell-mdio, and the code is in the uclass rather than MDIO driver so all can use it.
###[Ken] Thank you very much for your detailed explanation, I have not read uboot codes for almost 1 year, 😊


> And I think that my mdio searching way was much better than linux at 
> least at last year. > I do not know now how in linux net interface 
> finds its phy's mdio bus now. But at last year, linux stored all phys' 
> FDT node addesses under all mdio buses, and when searching network 
> interface' phy, linux driver compared its phy fdt node addresses to 
> each phy fdt node address it stored one by one.

OK, you're probably looking at of_phy_find_device.  It does search PHYs by DT node, which is useful for instance when using a phy-handle property in the ethernet node.  I suppose your comment is that the search function could be simpler and I could agree with that.  You could alternatively use mdiobus_get_phy which takes in a PHY address and doesn't care about DT nodes, but this only works if the PHY was probed already, I think.  But that's all Linux implementation, it doesn't mean U-Boot has to do the exact same thing.
###[Ken] I think Linux cycle searching has too low efficiency, 😊

> So I added 
> mdio_device_get_from_phy/mdio_mii_bus_get_from_phy/mdio_device_get_fro
> m_eth
> to simply the mdio/phy searching. >
> Yours,
> Ken

For instance you would have a phy-handle property, eth driver would call mdio_device_get_from_phy on the phy node to get the MDIO bus and then call dm_mdio_phy_connect with the PHY address, correct?

I am planning to add a helper that actually does all that in one go, not just return a reference to MDIO bus based on PHY node or eth interface.

Wwe could have something similar to of_phy_get_and_connect in Linux.  It would take in the ethernet udevice, read phy-handle from associated DT node, get PHY address and MDIO bus from DT and create the new PHY device.  Right now I added that code to our eth driver (see [1]), but given that bindings are generic it would be nice to move all that code out into generic code as a helper function.  The API would be something like this:

struct phy_device *dm_eth_phy_connect(struct udevice *eth); returning a phy device or null if something is missing.

It would be nice to also support fixed link bindings in there, but that is off-topic.
###[Ken] I had though this solution in my old design, but later I denied it because mdio_device_get_from_phy() should be called in eth device probe step while dm_mdio_phy_connect() should be called in eth device start step, eth probe and start are 2 different steps, 😊

Does that look reasonable to you?
Thanks!
Alex

[1] enetc_start_phy in https://patchwork.ozlabs.org/patch/1126769/

> 
> -----Original Message-----
> From: Alex Marginean <alexandru.marginean at nxp.com>
> Sent: Thursday, July 25, 2019 11:33 AM
> To: u-boot at lists.denx.de
> Cc: Joe Hershberger <joe.hershberger at ni.com>; Ken Ma 
> <make at marvell.com>; Nevo Hed <nhed+uboot at starry.com>; Alex Marginean 
> <alexandru.marginean at nxp.com>; Alex Marginean 
> <alexm.osslist at gmail.com>
> Subject: [EXT] [PATCH v2 2/4] doc: bindings: add mdio.txt describing 
> generic MDIO properties
> 
> External Email
> 
> ----------------------------------------------------------------------
> Adds a binding document for mdio.  A notable deviation from corresponding Linux binding is the introduction of device-name optional property, which can be used to name MDIO buses.  Two reset optional properties described by Linux binding are also not present as they don't seem to be used in U-Boot at this time.
> 
> Signed-off-by: Alex Marginean <alexm.osslist at gmail.com>
> Acked-by: Joe Hershberger <joe.hershberger at ni.com>
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>   doc/device-tree-bindings/net/mdio.txt | 36 +++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>   create mode 100644 doc/device-tree-bindings/net/mdio.txt
> 
> diff --git a/doc/device-tree-bindings/net/mdio.txt 
> b/doc/device-tree-bindings/net/mdio.txt
> new file mode 100644
> index 0000000000..1595325050
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/mdio.txt
> @@ -0,0 +1,36 @@
> +Common MDIO bus properties.
> +
> +These are generic properties that can apply to any MDIO bus.
> +
> +Optional properties:
> +	- device-name - If present it is used to name the device and MDIO bus.
> +			The name must be unique and must not contain spaces.
> +
> +A list of child nodes, one per device on the bus is expected.  These 
> +could be PHYs, switches or similar devices and child nodes should 
> +follow the specific binding for the device type.
> +
> +Example :
> +This example shows the structure used for the external MDIO bus on 
> +NXP LS1028A RDB board.  Note that this MDIO device is an integrated 
> +PCI function and requires no compatible property for probing.
> +
> +/* definition in SoC dtsi file */
> +	pcie at 1f0000000 {
> +
> +		mdio0: pci at 0,3 {
> +			#address-cells=<0>;
> +			#size-cells=<1>;
> +			reg = <0x000300 0 0 0 0>;
> +			status = "disabled";
> +			device-name = "emdio";
> +		};
> +	};
> +/* definition of PHYs in RDB dts file */
> +&mdio0 {
> +	status = "okay";
> +	rdb_phy0: phy at 2 {
> +		reg = <2>;
> +	};
> +};
> +
> --
> 2.17.1
>