On 22/03/2016 20:42, Uwe Kleine-König wrote:
> Preconditions:
> - Some of the devices a given driver handles have a reset line and
> others don't.
> - A non-empty subset (maybe all) of the devices that have a reset line
> require that this reset line is used.
>
> Then the way to handle this in the driver should be done as follows:
>
> unless reset_handling_not_necessary():
> gpio = gpiod_get_optional("reset")
> if IS_ERR(gpio):
> return PTR_ERR(gpio)
>
> Checking for -ENOSYS or GPIOLIB=n is not allowed because the device
> you're currently handling might need the GPIO, so you must not continue
> without the ability to control the line.
>
> So the options you have (as you have a phy that doesn't need the reset
> handling):
>
> - enable GPIOLIB (either in your .config or introduce a Kconfig
> dependency)
> - improve reset_handling_not_necessary() to return true for your case
>
> There is nothing else.
Here are some numbers for GPIOLIB, on an ARM build:
text data bss dec hex filename
1830 0 0 1830 726 devres.o
627 0 0 627 273 gpiolib-legacy.o
11018 40 4 11062 2b36 gpiolib.o
1598 0 0 1598 63e gpiolib-of.o
--------------------------------------------------------
15073 40 4 15117 3b0d built-in.o
So ~15 kilobytes.
By the way, since the "reset-by-GPIO" solution is used only for
the Atheros 8030, would it be possible to make the
devm_gpiod_get_optional conditional on ATH8030_PHY_ID?
I'm thinking of something along these lines, for illustration:
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 2d020a3ec0b5..576e7873e049 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -198,12 +198,16 @@ static int at803x_probe(struct phy_device *phydev)
if (!priv)
return -ENOMEM;
+ if (phydev->drv->phy_id != ATH8030_PHY_ID)
+ goto no_gpio;
+
gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(gpiod_reset))
return PTR_ERR(gpiod_reset);
priv->gpiod_reset = gpiod_reset;
+no_gpio:
phydev->priv = priv;
return 0;
Regards.
