| Message ID | 20181115052428.8133-1-ms@dev.tdt.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v5] net: phy: mdio-gpio: Fix working over slow can_sleep GPIOs | expand |
On Thu, Nov 15, 2018 at 06:24:28AM +0100, Martin Schiller wrote: > Up until commit 7e5fbd1e0700 ("net: mdio-gpio: Convert to use gpiod > functions where possible"), the _cansleep variants of the gpio_ API was > used. After that commit and the change to gpiod_ API, the _cansleep() > was dropped. This then results in WARN_ON() when used with GPIO > devices which do sleep. Add back the _cansleep() to avoid this. > > Fixes: 7e5fbd1e0700 ("net: mdio-gpio: Convert to use gpiod functions where possible") > Signed-off-by: Martin Schiller <ms@dev.tdt.de> > --- > v5: > - reworked commit message > - added "Fixes:" tag > - based on DaveM net tree instead of mainline Hi Martin Thanks for these changes. We are much closer now. > @@ -162,6 +162,10 @@ static int mdio_gpio_probe(struct platform_device *pdev) > if (ret) > return ret; > > + if (gpiod_cansleep(bitbang->mdc) || gpiod_cansleep(bitbang->mdio) || > + gpiod_cansleep(bitbang->mdo)) > + dev_warn(&pdev->dev, "Slow GPIO pins might wreak havoc into MDIO bus timing"); > + I talked with Florian about this. We would like this hunk of the patch dropped 1) For a patch which is going to stable, it does not fit. It does not actually fix anything. 2) I'm not sure it has any value. The hardware has been designed like that. There is nothing which can be done about it. Printing a message is not going to help users. Andrew
From: Martin Schiller <ms@dev.tdt.de> Date: Thu, 15 Nov 2018 06:24:28 +0100 > Up until commit 7e5fbd1e0700 ("net: mdio-gpio: Convert to use gpiod > functions where possible"), the _cansleep variants of the gpio_ API was > used. After that commit and the change to gpiod_ API, the _cansleep() > was dropped. This then results in WARN_ON() when used with GPIO > devices which do sleep. Add back the _cansleep() to avoid this. > > Fixes: 7e5fbd1e0700 ("net: mdio-gpio: Convert to use gpiod functions where possible") > Signed-off-by: Martin Schiller <ms@dev.tdt.de> Ok it seems there was still discussion going on here and I accidently applied v4 of this patch. I'll revert it and wait for the review to conclude.
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c index 33265747bf39..3a5a24daf384 100644 --- a/drivers/net/phy/mdio-gpio.c +++ b/drivers/net/phy/mdio-gpio.c @@ -63,7 +63,7 @@ static void mdio_dir(struct mdiobb_ctrl *ctrl, int dir) * assume the pin serves as pull-up. If direction is * output, the default value is high. */ - gpiod_set_value(bitbang->mdo, 1); + gpiod_set_value_cansleep(bitbang->mdo, 1); return; } @@ -78,7 +78,7 @@ static int mdio_get(struct mdiobb_ctrl *ctrl) struct mdio_gpio_info *bitbang = container_of(ctrl, struct mdio_gpio_info, ctrl); - return gpiod_get_value(bitbang->mdio); + return gpiod_get_value_cansleep(bitbang->mdio); } static void mdio_set(struct mdiobb_ctrl *ctrl, int what) @@ -87,9 +87,9 @@ static void mdio_set(struct mdiobb_ctrl *ctrl, int what) container_of(ctrl, struct mdio_gpio_info, ctrl); if (bitbang->mdo) - gpiod_set_value(bitbang->mdo, what); + gpiod_set_value_cansleep(bitbang->mdo, what); else - gpiod_set_value(bitbang->mdio, what); + gpiod_set_value_cansleep(bitbang->mdio, what); } static void mdc_set(struct mdiobb_ctrl *ctrl, int what) @@ -97,7 +97,7 @@ static void mdc_set(struct mdiobb_ctrl *ctrl, int what) struct mdio_gpio_info *bitbang = container_of(ctrl, struct mdio_gpio_info, ctrl); - gpiod_set_value(bitbang->mdc, what); + gpiod_set_value_cansleep(bitbang->mdc, what); } static const struct mdiobb_ops mdio_gpio_ops = { @@ -162,6 +162,10 @@ static int mdio_gpio_probe(struct platform_device *pdev) if (ret) return ret; + if (gpiod_cansleep(bitbang->mdc) || gpiod_cansleep(bitbang->mdio) || + gpiod_cansleep(bitbang->mdo)) + dev_warn(&pdev->dev, "Slow GPIO pins might wreak havoc into MDIO bus timing"); + if (pdev->dev.of_node) { bus_id = of_alias_get_id(pdev->dev.of_node, "mdio-gpio"); if (bus_id < 0) {
Up until commit 7e5fbd1e0700 ("net: mdio-gpio: Convert to use gpiod functions where possible"), the _cansleep variants of the gpio_ API was used. After that commit and the change to gpiod_ API, the _cansleep() was dropped. This then results in WARN_ON() when used with GPIO devices which do sleep. Add back the _cansleep() to avoid this. Fixes: 7e5fbd1e0700 ("net: mdio-gpio: Convert to use gpiod functions where possible") Signed-off-by: Martin Schiller <ms@dev.tdt.de> --- v5: - reworked commit message - added "Fixes:" tag - based on DaveM net tree instead of mainline v4: - remove linewrap of kernel message v3: - modified commit summary - fixed commit cites in commit message - fixed line continuation v2: - fixed copy/paste bug in warning about slow GPIO pins --- drivers/net/phy/mdio-gpio.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)