[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZFziRIzP/sXZMgiU@corigine.com>
Date: Thu, 11 May 2023 14:40:36 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Yan Wang <rk.code@...look.com>
Cc: andrew@...n.ch, hkallweit1@...il.com, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux@...linux.org.uk
Subject: Re: [PATCH v4] net: mdiobus: Add a function to deassert reset
On Thu, May 11, 2023 at 02:59:09PM +0800, Yan Wang wrote:
> It is possible to mount multiple sub-devices on the mido bus.
> The hardware power-on does not necessarily reset these devices.
> The device may be in an uncertain state, causing the device's ID
> to not be scanned.
>
> So,before adding a reset to the scan, make sure the device is in
> normal working mode.
>
> Reported-by: kernel test robot <lkp@...el.com>
> Link: https://lore.kernel.org/oe-kbuild-all/202305101702.4xW6vT72-lkp@intel.com/
> Signed-off-by: Yan Wang <rk.code@...look.com>
...
> diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
> index 1183ef5e203e..9d7df6393059 100644
> --- a/drivers/net/mdio/fwnode_mdio.c
> +++ b/drivers/net/mdio/fwnode_mdio.c
> @@ -11,6 +11,7 @@
> #include <linux/of.h>
> #include <linux/phy.h>
> #include <linux/pse-pd/pse.h>
> +#include <linux/gpio/consumer.h>
>
> MODULE_AUTHOR("Calvin Johnson <calvin.johnson@....nxp.com>");
> MODULE_LICENSE("GPL");
> @@ -57,6 +58,35 @@ fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
> return register_mii_timestamper(arg.np, arg.args[0]);
> }
>
> +static void fwnode_mdiobus_pre_enable_phy(struct fwnode_handle *fwnode)
> +{
> + struct gpio_desc *reset;
> + unsigned int reset_assert_delay;
> + unsigned int reset_deassert_delay;
nit: Please arrange local variables for networking code in reverse xmas
tree order - longest line to shortest.
> +
> + reset = fwnode_gpiod_get_index(fwnode, "reset", 0, GPIOD_OUT_LOW, NULL);
> + if (IS_ERR(reset)) {
> + if (PTR_ERR(reset) == -EPROBE_DEFER)
> + pr_debug("%pOFn: %s: GPIOs not yet available, retry later\n",
> + to_of_node(fwnode), __func__);
> + else
> + pr_err("%pOFn: %s: Can't get reset line property\n",
> + to_of_node(fwnode), __func__);
> +
> + return;
> + }
> + fwnode_property_read_u32(fwnode, "reset-assert-us",
> + &reset_assert_delay);
> + fwnode_property_read_u32(fwnode, "reset-deassert-us",
> + &reset_deassert_delay);
Does the return value of fwnode_property_read_u32() need to be
checked for errors?
> + gpiod_set_value_cansleep(reset, 1);
> + fsleep(reset_assert_delay);
> + gpiod_set_value_cansleep(reset, 0);
> + fsleep(reset_deassert_delay);
> + /*Release phy's reset line, mdiobus_register_gpiod() need to request it*/
nit:
/* Release phy's reset line, mdiobus_register_gpiod() needs to
* request it.
*/
> + gpiod_put(reset);
> +}
> +
> int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
> struct phy_device *phy,
> struct fwnode_handle *child, u32 addr)
> @@ -119,6 +149,8 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> u32 phy_id;
> int rc;
>
> + fwnode_mdiobus_pre_enable_phy(child);
> +
> psec = fwnode_find_pse_control(child);
> if (IS_ERR(psec))
> return PTR_ERR(psec);
> --
> 2.17.1
>
>
Powered by blists - more mailing lists