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

[Sergei Shtylyov]

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@xxxxxxxxxx>
> ---
> 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