Re: [PATCH v2] net: phy: mdio-gpio: fix access that may sleep

Sergei Shtylyov
2018-11-14T09:20:29Z

Hello!

On 14.11.2018 9:37, Martin Schiller wrote:

> This commit re-enables support for slow GPIO pins. It was initially
> introduced by commit
> 2d6c9091ab7630dfcf34417c6683ce4764d7d40a
> and got lost by commit
> 7e5fbd1e0700f1bdb94508f84ec2aeb01eed7b12

This is not how you cite a commit: 12 digits is enough and they should
be followed by the commit summary enclosed in ("").

>
> Also add a warning about slow GPIO pins like it is done in i2c-gpio.
>
> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> ---
> v2:
> - fixed copy/paste bug in warning about slow GPIO pins
> ---
> drivers/net/phy/mdio-gpio.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> index 33265747bf39..6c1cca14689b 100644
> --- a/drivers/net/phy/mdio-gpio.c
> +++ b/drivers/net/phy/mdio-gpio.c
[...]
> @@ -162,6 +162,11 @@ 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))

The line continued that way blends with the branch below, the networking
code uses a different style of line continuation where the continuation line
starts immediately below the 1st gpiod_cansleep() call. Also, || should be
left on the 1st line...

> + 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) {

MBR, Sergei