Messages in this thread | | | Date | Wed, 24 Jan 2024 00:07:33 +0100 | From | Andrew Lunn <> | Subject | Re: [PATCH net] net: dsa: qca8k: fix illegal usage of GPIO |
| |
On Mon, Jan 22, 2024 at 01:04:09PM +0100, Michal Vokáč wrote: > When working with GPIO, its direction must be set either when the GPIO is > requested by gpiod_get*() or later on by one of the gpiod_direction_*() > functions. Neither of this is done here which result in undefined behavior > on some systems. > > As the reset GPIO is used right after it is requested here, it makes sense > to configure it as GPIOD_OUT_HIGH right away. > Fixes: a653f2f538f9 ("net: dsa: qca8k: introduce reset via gpio feature") > Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com> > --- > drivers/net/dsa/qca/qca8k-8xxx.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c > index 4ce68e655a63..83b19c2d7b97 100644 > --- a/drivers/net/dsa/qca/qca8k-8xxx.c > +++ b/drivers/net/dsa/qca/qca8k-8xxx.c > @@ -2037,8 +2037,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev) > priv->dev = &mdiodev->dev; > priv->info = of_device_get_match_data(priv->dev); > > - priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset", > - GPIOD_ASIS); > + priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(priv->reset_gpio)) > return PTR_ERR(priv->reset_gpio);
Hi Michal
So the current code is:
priv->reset_gpio = devm_gpiod_get_optional(priv->dev, "reset", GPIOD_ASIS); if (IS_ERR(priv->reset_gpio)) return PTR_ERR(priv->reset_gpio);
if (priv->reset_gpio) { gpiod_set_value_cansleep(priv->reset_gpio, 1); /* The active low duration must be greater than 10 ms * and checkpatch.pl wants 20 ms. */ msleep(20); gpiod_set_value_cansleep(priv->reset_gpio, 0); }
Doesn't your change make the gpiod_set_value_cansleep() pointless?
Please extend your patch to remove it, maybe extending the comment a little.
Please also make sure what v2 Is Cc: to the qca8k Maintainers.
Andrew
--- pw-bot: cr
| |