Re: [PATCH 2/3] iio: light: stk3310: add per-chip match data

From: Andy Shevchenko

Date: Mon Aug 10 2026 - 14:35:04 EST


On Mon, Aug 10, 2026 at 01:04:22PM +0200, Jorijn van der Graaf wrote:
> Introduce a chip_info structure carrying the device name and channel
> specification, attach it to every i2c, OF and ACPI table entry, and let
> probe take it from the match data. Clients instantiated through the
> sysfs new_device interface under the lowercase compatible-derived name
> have no firmware node and do not match the uppercase id table entries,
> so absent match data falls back to the original chip data. The channel
> data registers move into .address and the shared channel definitions
> into macros.
>
> This is a preparatory change for a variant that provides more channels
> than the existing parts. No functional change.

...

> + ret = regmap_bulk_read(data->regmap, chan->address, &buf,
> + sizeof(buf));

I would add

struct regmap *map = data->regmap;

and make this on a single line.

> if (ret < 0) {
> dev_err(&client->dev, "register read failed\n");

...

> + chip_info = i2c_get_match_data(client);
> + if (!chip_info) {
> + /*
> + * Clients instantiated through the sysfs new_device
> + * interface under the lowercase compatible-derived name
> + * have no firmware node and do not match the uppercase
> + * id table entries.
> + */
> + chip_info = &stk3310_chip_info;

This is an interesting comment and approach.

> + }

- Where does this lowercase come from? Is it Linux forced conversion?
- What's wrong with simply failing the probe?

As far as I understand the problem is preexisted. Or was there any default
taken? How do we know that the chosen default is a good one?

--
With Best Regards,
Andy Shevchenko