linux-kernel - Re: [PATCH v2 3/5] iio: mcp9600: Add compatibility for mcp9601

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHp75Vc6DwpCps9kuXjaCCPrYycbFf3NV2Ye+aEM2_9LWJqMBA@mail.gmail.com>
Date: Wed, 13 Aug 2025 18:40:25 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Ben Collins <bcollins@...ter.com>
Cc: Jonathan Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, 
	Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/5] iio: mcp9600: Add compatibility for mcp9601

On Wed, Aug 13, 2025 at 5:17 PM Ben Collins <bcollins@...ter.com> wrote:
>
> MCP9601 is a super set of MCP9600. The drivers works without changes

superset

> on this chipset.

...

>  config MCP9600
> -       tristate "MCP9600 thermocouple EMF converter"
> +       tristate "MCP9600 and similar thermocouple EMF converters"
>         depends on I2C
>         help
> -         If you say yes here you get support for MCP9600
> -         thermocouple EMF converter connected via I2C.

> +         If you say yes here you get support for MCP9600, MCP9601, and
> +         similar thermocouple EMF converters connected via I2C.

To avoid a potential churn in the further changes to support a new HW,
I would suggest to convert this to the list of supported chips:

  ...get support for:
  - MCP9600
  - MCP9601
  and similar...

>           This driver can also be built as a module. If so, the module
>           will be called mcp9600.

...

> +       switch (dev_id) {
> +       case MCP9600_DEVICE_ID_MCP9600:
> +       case MCP9600_DEVICE_ID_MCP9601:
> +               if (dev_id != id->driver_data)

I prefer to see this to be converted to use chip_info before getting
to a new HW support.

> +                       dev_warn(&client->dev,
> +                                "Expected id %x but detected %x. Ensure dt is correct\n",

dt --> firmware description
(the world is not rotating around DT only)

> +                                (u8)id->driver_data, (u8)dev_id);

Use proper specifiers and drop castings.

> +               break;
>

> +       default:
> +               dev_warn(&client->dev, "Unknown id %x, using %x\n", (u8)dev_id,
> +                       (u8)id->driver_data);

Ditto.

> +       }

...

> +       { "mcp9600", MCP9600_DEVICE_ID_MCP9600 },
> +       { "mcp9601", MCP9600_DEVICE_ID_MCP9601 },

Nope, use chip_info from day 1, please.

...

>  static const struct of_device_id mcp9600_of_match[] = {
>         { .compatible = "microchip,mcp9600" },
> +       { .compatible = "microchip,mcp9601" },

Missed driver data.

>         { }
>  };

-- 
With Best Regards,
Andy Shevchenko