Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220728125435.3336618-1-potin.lai.pt@gmail.com> <20220728125435.3336618-3-potin.lai.pt@gmail.com>
In-Reply-To: <20220728125435.3336618-3-potin.lai.pt@gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Thu, 28 Jul 2022 22:42:52 +0200
Message-ID: <CAHp75VebJQH_Ab3MjLhNr_=AJ8eZhUMzU7GnDXL4Gw8j_3vH7A@mail.gmail.com>
Subject: Re: [PATCH v5 2/2] iio: humidity: hdc100x: add manufacturer and
 device ID check
To:     Potin Lai <potin.lai.pt@gmail.com>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Patrick Williams <patrick@stwcx.xyz>,
        Potin Lai <potin.lai@quantatw.com>,
        linux-iio <linux-iio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jul 28, 2022 at 3:32 PM Potin Lai <potin.lai.pt@gmail.com> wrote:
>
> Add manufacturer and device ID checking during probe, and skip the
> checking if chip model not supported.
>
> Supported:
> - HDC1000
> - HDC1010
> - HDC1050
> - HDC1080
>
> Not supported:
> - HDC1008

This one is okay now, thanks!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
> ---
>  drivers/iio/humidity/hdc100x.c | 67 ++++++++++++++++++++++++++++------
>  1 file changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> index 0d514818635cb..31f18cc1cf63c 100644
> --- a/drivers/iio/humidity/hdc100x.c
> +++ b/drivers/iio/humidity/hdc100x.c
> @@ -34,6 +34,23 @@
>  #define HDC100X_REG_CONFIG_ACQ_MODE            BIT(12)
>  #define HDC100X_REG_CONFIG_HEATER_EN           BIT(13)
>
> +#define HDC100X_REG_MFR_ID     0xFE
> +#define HDC100X_REG_DEV_ID     0xFF
> +
> +#define HDC100X_MFR_ID 0x5449
> +
> +struct hdc100x_chip_data {
> +       bool support_mfr_check;
> +};
> +
> +static const struct hdc100x_chip_data hdc100x_chip_data = {
> +       .support_mfr_check      = true,
> +};
> +
> +static const struct hdc100x_chip_data hdc1008_chip_data = {
> +       .support_mfr_check      = false,
> +};
> +
>  struct hdc100x_data {
>         struct i2c_client *client;
>         struct mutex lock;
> @@ -351,8 +368,32 @@ static const struct iio_info hdc100x_info = {
>         .attrs = &hdc100x_attribute_group,
>  };
>
> +static int hdc100x_read_mfr_id(struct i2c_client *client)
> +{
> +       return i2c_smbus_read_word_swapped(client, HDC100X_REG_MFR_ID);
> +}
> +
> +static int hdc100x_read_dev_id(struct i2c_client *client)
> +{
> +       return i2c_smbus_read_word_swapped(client, HDC100X_REG_DEV_ID);
> +}
> +
> +static bool is_valid_hdc100x(struct i2c_client *client)
> +{
> +       int mfr_id, dev_id;
> +
> +       mfr_id = hdc100x_read_mfr_id(client);
> +       dev_id = hdc100x_read_dev_id(client);
> +       if (mfr_id == HDC100X_MFR_ID &&
> +          (dev_id == 0x1000 || dev_id == 0x1050))
> +               return true;
> +
> +       return false;
> +}
> +
>  static int hdc100x_probe(struct i2c_client *client)
>  {
> +       const struct hdc100x_chip_data *chip_data;
>         struct iio_dev *indio_dev;
>         struct hdc100x_data *data;
>         int ret;
> @@ -361,6 +402,10 @@ static int hdc100x_probe(struct i2c_client *client)
>                                      I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))
>                 return -EOPNOTSUPP;
>
> +       chip_data = device_get_match_data(&client->dev);
> +       if (chip_data->support_mfr_check && !is_valid_hdc100x(client))
> +               return -EINVAL;
> +
>         indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>         if (!indio_dev)
>                 return -ENOMEM;
> @@ -396,22 +441,22 @@ static int hdc100x_probe(struct i2c_client *client)
>  }
>
>  static const struct i2c_device_id hdc100x_id[] = {
> -       { "hdc100x", 0 },
> -       { "hdc1000", 0 },
> -       { "hdc1008", 0 },
> -       { "hdc1010", 0 },
> -       { "hdc1050", 0 },
> -       { "hdc1080", 0 },
> +       { "hdc100X", (kernel_ulong_t)&hdc100x_chip_data },
> +       { "hdc1000", (kernel_ulong_t)&hdc100x_chip_data },
> +       { "hdc1008", (kernel_ulong_t)&hdc1008_chip_data },
> +       { "hdc1010", (kernel_ulong_t)&hdc100x_chip_data },
> +       { "hdc1050", (kernel_ulong_t)&hdc100x_chip_data },
> +       { "hdc1080", (kernel_ulong_t)&hdc100x_chip_data },
>         { }
>  };
>  MODULE_DEVICE_TABLE(i2c, hdc100x_id);
>
>  static const struct of_device_id hdc100x_dt_ids[] = {
> -       { .compatible = "ti,hdc1000" },
> -       { .compatible = "ti,hdc1008" },
> -       { .compatible = "ti,hdc1010" },
> -       { .compatible = "ti,hdc1050" },
> -       { .compatible = "ti,hdc1080" },
> +       { .compatible = "ti,hdc1000", .data = &hdc100x_chip_data },
> +       { .compatible = "ti,hdc1008", .data = &hdc1008_chip_data },
> +       { .compatible = "ti,hdc1010", .data = &hdc100x_chip_data },
> +       { .compatible = "ti,hdc1050", .data = &hdc100x_chip_data },
> +       { .compatible = "ti,hdc1080", .data = &hdc100x_chip_data },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, hdc100x_dt_ids);
> --
> 2.31.1
>


-- 
With Best Regards,
Andy Shevchenko