[U-Boot] [PATCH] i2c: muxes: pca954x: Add support for GPIO reset line

[Moritz Fischer]

Hi Simon,

On Fri, Sep 8, 2017 at 9:55 PM, Simon Glass <sjg at chromium.org> wrote:
> On 5 September 2017 at 12:24, Moritz Fischer <moritz.fischer at ettus.com> wrote:
>> This commit adds support for GPIO reset lines matching the
>> common linux "reset-gpios" devicetree binding.
>>
>> Signed-off-by: Moritz Fischer <moritz.fischer at ettus.com>
>> Cc: Heiko Schocher <hs at denx.de>
>> Cc: Stefan Roese <sr at denx.de>
>> Cc: Marek Behún <marek.behun at nic.cz>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Michal Simek <monstr at monstr.eu>
>>
>> ---
>>  drivers/i2c/muxes/pca954x.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/muxes/pca954x.c b/drivers/i2c/muxes/pca954x.c
>> index 383f72f552..dd28ff057b 100644
>> --- a/drivers/i2c/muxes/pca954x.c
>> +++ b/drivers/i2c/muxes/pca954x.c
>> @@ -1,5 +1,6 @@
>>  /*
>>   * Copyright (C) 2015 - 2016 Xilinx, Inc.
>> + * Copyright (C) 2017 National Instruments Corp
>>   * Written by Michal Simek
>>   *
>>   * SPDX-License-Identifier:    GPL-2.0+
>> @@ -9,7 +10,10 @@
>>  #include <dm.h>
>>  #include <errno.h>
>>  #include <i2c.h>
>> -#include <asm/gpio.h>
>> +
>> +#if CONFIG_DM_GPIO
>> +# include <asm-generic/gpio.h>
>> +#endif /* CONFIG_DM_GPIO */
>
> Can we drop the #ifdef?

Yeah, will do.
>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -30,6 +34,9 @@ struct chip_desc {
>>  struct pca954x_priv {
>>         u32 addr; /* I2C mux address */
>>         u32 width; /* I2C mux width - number of busses */
>> +#ifdef CONFIG_DM_GPIO
>> +       struct gpio_desc gpio_mux_reset;
>> +#endif /* CONFIG_DM_GPIO */
>>  };
>>
>>  static const struct chip_desc chips[] = {
>> @@ -105,10 +112,45 @@ static int pca954x_ofdata_to_platdata(struct udevice *dev)
>>         return 0;
>>  }
>>
>> +static int pca954x_probe(struct udevice *dev)
>> +{
>> +#ifdef CONFIG_DM_GPIO
>
> Can we use if (IS_ENABLED(CONFIG_DM_GPIO)) ?

I suppose. I was wondering. Is this in general preferable?
>
>> +       struct pca954x_priv *priv = dev_get_priv(dev);
>> +       int err;
>> +
>> +       err = gpio_request_by_name(dev, "reset-gpios", 0,
>> +                       &priv->gpio_mux_reset, GPIOD_IS_OUT);
>> +
>> +       /* it's optional so only bail if we get a real error */
>> +       if (err && (err != -ENOENT))
>> +               return err;
>> +
>> +       /* dm will take care of polarity */
>> +       if (dm_gpio_is_valid(&priv->gpio_mux_reset))
>> +               dm_gpio_set_value(&priv->gpio_mux_reset, 0);
>> +
>> +#endif /* CONFIG_DM_GPIO */
>> +       return 0;
>> +}
>> +
>> +static int pca954x_remove(struct udevice *dev)
>> +{
>> +#ifdef CONFIG_DM_GPIO
>> +       struct pca954x_priv *priv = dev_get_priv(dev);
>> +
>> +       if (dm_gpio_is_valid(&priv->gpio_mux_reset))
>> +               dm_gpio_free(dev, &priv->gpio_mux_reset);
>> +
>> +#endif /* CONFIG_DM_GPIO */
>> +       return 0;
>> +}
>> +
>>  U_BOOT_DRIVER(pca954x) = {
>>         .name = "pca954x",
>>         .id = UCLASS_I2C_MUX,
>>         .of_match = pca954x_ids,
>> +       .probe = pca954x_probe,
>> +       .remove = pca954x_remove,
>>         .ops = &pca954x_ops,
>>         .ofdata_to_platdata = pca954x_ofdata_to_platdata,
>>         .priv_auto_alloc_size = sizeof(struct pca954x_priv),
>> --
>> 2.14.1
>>

Thanks for the review,

Moritz