[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 8 Aug 2018 13:58:38 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Tony Lindgren <tony@...mide.com>
Cc: David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
linux-omap@...r.kernel.org, devicetree@...r.kernel.org,
Grygorii Strashko <grygorii.strashko@...com>,
Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>,
Mark Rutland <mark.rutland@....com>,
Murali Karicheri <m-karicheri2@...com>,
Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH] net: ethernet: cpsw-phy-sel: prefer phandle for phy sel
and update binding
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -19,6 +19,9 @@ Required properties:
> - slaves : Specifies number for slaves
> - active_slave : Specifies the slave to use for time stamping,
> ethtool and SIOCGMIIPHY
> +- cpsw-phy-sel : Specifies the phandle to the CPSW phy mode selection
> + device. Note that in legacy cases cpsw-phy-sel may be
> + a child device instead of a phandle.
Hi Tony
It would be good to reference cpsw-phy-sel.txt.
> Optional properties:
> - ti,hwmods : Must be "cpgmac0"
> @@ -75,6 +78,7 @@ Examples:
> cpts_clock_mult = <0x80000000>;
> cpts_clock_shift = <29>;
> syscon = <&cm>;
> + cpsw-phy-sel = <&phy_sel>;
> cpsw_emac0: slave@0 {
> phy_id = <&davinci_mdio>, <0>;
> phy-mode = "rgmii-txid";
> @@ -103,6 +107,7 @@ Examples:
> cpts_clock_mult = <0x80000000>;
> cpts_clock_shift = <29>;
> syscon = <&cm>;
> + cpsw-phy-sel = <&phy_sel>;
> cpsw_emac0: slave@0 {
> phy_id = <&davinci_mdio>, <0>;
> phy-mode = "rgmii-txid";
> diff --git a/drivers/net/ethernet/ti/cpsw-phy-sel.c b/drivers/net/ethernet/ti/cpsw-phy-sel.c
> --- a/drivers/net/ethernet/ti/cpsw-phy-sel.c
> +++ b/drivers/net/ethernet/ti/cpsw-phy-sel.c
> @@ -170,10 +170,13 @@ void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave)
> struct device_node *node;
> struct cpsw_phy_sel_priv *priv;
>
> - node = of_get_child_by_name(dev->of_node, "cpsw-phy-sel");
> + node = of_parse_phandle(dev->of_node, "cpsw-phy-sel", 0);
> if (!node) {
Do you need to handle EPROBE_DEFER here? The phandle points to a
device which has not yet been loaded? I'm not sure exactly where it
will be returned, maybe it is bus_find_device(), but i expect to see
some handling of it somewhere in this function.
> - dev_err(dev, "Phy mode driver DT not found\n");
> - return;
> + node = of_get_child_by_name(dev->of_node, "cpsw-phy-sel");
> + if (!node) {
> + dev_err(dev, "Phy mode driver DT not found\n");
> + return;
> + }
> }
Andrew
Powered by blists - more mailing lists