Hi

On Fri, Dec 7, 2018 at 12:12 PM Hans Verkuil <hverk...@xs4all.nl> wrote:
>
> Subject: [PATCH 4/5] si470x-i2c: Add optional reset-gpio support
> Date: Wed,  5 Dec 2018 16:47:49 +0100
> From: Paweł Chmiel <pawel.mikolaj.chm...@gmail.com>
> To: mche...@kernel.org, robh...@kernel.org, mark.rutl...@arm.com
> CC: hverk...@xs4all.nl, fischerdougl...@gmail.com, keesc...@chromium.org, 
> linux-media@vger.kernel.org, linux-ker...@vger.kernel.org,
> devicet...@vger.kernel.org, Paweł Chmiel <pawel.mikolaj.chm...@gmail.com>
>
> If reset-gpio is defined, use it to bring device out of reset.
> Without this, it's not possible to access si470x registers.
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chm...@gmail.com>
> ---
> For some reason this patch was not picked up by patchwork. Resending to see if
> it is picked up now.
> ---
>  drivers/media/radio/si470x/radio-si470x-i2c.c | 15 +++++++++++++++
>  drivers/media/radio/si470x/radio-si470x.h     |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c 
> b/drivers/media/radio/si470x/radio-si470x-i2c.c
> index a7ac09c55188..15eea2b2c90f 100644
> --- a/drivers/media/radio/si470x/radio-si470x-i2c.c
> +++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
> @@ -28,6 +28,7 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/interrupt.h>
>   #include "radio-si470x.h"
> @@ -392,6 +393,17 @@ static int si470x_i2c_probe(struct i2c_client *client,
>         radio->videodev.release = video_device_release_empty;
>         video_set_drvdata(&radio->videodev, radio);
>  +      radio->gpio_reset = devm_gpiod_get_optional(&client->dev, "reset",
> +                                                   GPIOD_OUT_LOW);
> +       if (IS_ERR(radio->gpio_reset)) {
> +               retval = PTR_ERR(radio->gpio_reset);
> +               dev_err(&client->dev, "Failed to request gpio: %d\n", retval);
> +               goto err_all;
> +       }
> +
> +       if (radio->gpio_reset)
> +               gpiod_set_value(radio->gpio_reset, 1);
> +
>         /* power up : need 110ms */
>         radio->registers[POWERCFG] = POWERCFG_ENABLE;
>         if (si470x_set_register(radio, POWERCFG) < 0) {
> @@ -478,6 +490,9 @@ static int si470x_i2c_remove(struct i2c_client *client)
>         video_unregister_device(&radio->videodev);
>  +      if (radio->gpio_reset)
> +               gpiod_set_value(radio->gpio_reset, 0);

I have a question for you. If the gpio is the last of the bank
acquired for this cpu, when you put to 0, then the gpio will
be free on remove and the clock of the logic will be deactivated so I
think that you don't have any
garantee that the state will be 0

Michael

> +
>         return 0;
>  }
>  diff --git a/drivers/media/radio/si470x/radio-si470x.h 
> b/drivers/media/radio/si470x/radio-si470x.h
> index 35fa0f3bbdd2..6fd6a399cb77 100644
> --- a/drivers/media/radio/si470x/radio-si470x.h
> +++ b/drivers/media/radio/si470x/radio-si470x.h
> @@ -189,6 +189,7 @@ struct si470x_device {
>   #if IS_ENABLED(CONFIG_I2C_SI470X)
>         struct i2c_client *client;
> +       struct gpio_desc *gpio_reset;
>  #endif
>  };
>  -- 2.17.1
>


-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

Reply via email to