[PATCH 0/3] iio: light: stk3310: per-chip match data and STK36C61 support

[PATCH 0/3] iio: light: stk3310: per-chip match data and STK36C61 support

[Jorijn van der Graaf]

The Sensortek STK36C61 is a 3-in-1 ambient light / proximity / RGB
colour sensor whose register interface is compatible with the stk3310
across the ALS and proximity data, threshold, gain and
integration-time registers this driver uses. The part name comes from
the Fairphone 6 manual, which lists the component as "3IN1 ALPS/RGB
sensor/STK36C61". There is no public datasheet; the layout was probed
on the device: chip ID 0x95, the stk3310 STATE/FLAG bit layout, data
and threshold registers, gain steps that scale the ALS reading
accordingly, and an RGBC data block directly after the ALS data.

Tested on a Fairphone 6 running a 7.1.2-based tree carrying these
patches, adapted only to that base's older style for the bulk-read
size argument and the i2c device-ID table initialisers. The stk3310.c
delta between that base and this one (chiefly the PS-interrupt
suspend/resume rework) does not alter behaviour on the paths this
series touches. A board DTS node using the new compatible follows
separately via linux-arm-msm.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>

Jorijn van der Graaf (3):
  dt-bindings: iio: light: stk33xx: document the Sensortek STK36C61
  iio: light: stk3310: add per-chip match data
  iio: light: stk3310: support the Sensortek STK36C61

 .../bindings/iio/light/stk33xx.yaml           |   9 +-
 drivers/iio/light/stk3310.c                   | 151 +++++++++++++-----
 2 files changed, 115 insertions(+), 45 deletions(-)


base-commit: 350d1fb9204b13c5f95e511e98b8bcb47574d425
-- 
2.55.0

[PATCH 1/3] dt-bindings: iio: light: stk33xx: document the Sensortek STK36C61

[Jorijn van der Graaf]

The STK36C61 is a 3-in-1 ambient light / proximity / RGB colour sensor
found in the Fairphone 6. Its ambient light and proximity register
interface is compatible with the stk3310's, while the dedicated
compatible identifies the part, whose colour channels the fallback
does not describe.

Add sensortek,stk36c61 with a sensortek,stk3310 fallback, mirroring
the stk3013 entry, and widen the title and description to cover the
part.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 Documentation/devicetree/bindings/iio/light/stk33xx.yaml | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
index e4341fdced98..0165a348918b 100644
--- a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
+++ b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
@@ -4,14 +4,14 @@
 $id: http://devicetree.org/schemas/iio/light/stk33xx.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: |
-  Sensortek STK33xx I2C Ambient Light and Proximity sensor
+title: Sensortek STK33xx/STK36C61 I2C Ambient Light and Proximity sensor
 
 maintainers:
   - Jonathan Cameron <jic23@kernel.org>
 
-description: |
-  Ambient light and proximity sensor over an i2c interface.
+description:
+  Ambient light and proximity sensor over an i2c interface. The STK36C61
+  additionally provides RGBC colour channels.
 
 allOf:
   - $ref: ../common.yaml#
@@ -26,6 +26,7 @@ properties:
       - items:
           - enum:
               - sensortek,stk3013
+              - sensortek,stk36c61
           - const: sensortek,stk3310
 
   reg:
-- 
2.55.0

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

[Jorijn van der Graaf]

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.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 drivers/iio/light/stk3310.c | 107 +++++++++++++++++++++++-------------
 1 file changed, 69 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index 7c8a1d2b2ed0..249396b9bf80 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -181,24 +181,48 @@ static const struct iio_chan_spec_ext_info stk3310_ext_info[] = {
 	{ }
 };
 
+#define STK3310_LIGHT_CHANNEL {				\
+	.type = IIO_LIGHT,				\
+	.address = STK3310_REG_ALS_DATA_MSB,		\
+	.info_mask_separate =				\
+		BIT(IIO_CHAN_INFO_RAW) |		\
+		BIT(IIO_CHAN_INFO_SCALE) |		\
+		BIT(IIO_CHAN_INFO_INT_TIME),		\
+}
+
+#define STK3310_PROXIMITY_CHANNEL {			\
+	.type = IIO_PROXIMITY,				\
+	.address = STK3310_REG_PS_DATA_MSB,		\
+	.info_mask_separate =				\
+		BIT(IIO_CHAN_INFO_RAW) |		\
+		BIT(IIO_CHAN_INFO_SCALE) |		\
+		BIT(IIO_CHAN_INFO_INT_TIME),		\
+	.event_spec = stk3310_events,			\
+	.num_event_specs = ARRAY_SIZE(stk3310_events),	\
+	.ext_info = stk3310_ext_info,			\
+}
+
 static const struct iio_chan_spec stk3310_channels[] = {
-	{
-		.type = IIO_LIGHT,
-		.info_mask_separate =
-			BIT(IIO_CHAN_INFO_RAW) |
-			BIT(IIO_CHAN_INFO_SCALE) |
-			BIT(IIO_CHAN_INFO_INT_TIME),
-	},
-	{
-		.type = IIO_PROXIMITY,
-		.info_mask_separate =
-			BIT(IIO_CHAN_INFO_RAW) |
-			BIT(IIO_CHAN_INFO_SCALE) |
-			BIT(IIO_CHAN_INFO_INT_TIME),
-		.event_spec = stk3310_events,
-		.num_event_specs = ARRAY_SIZE(stk3310_events),
-		.ext_info = stk3310_ext_info,
-	}
+	STK3310_LIGHT_CHANNEL,
+	STK3310_PROXIMITY_CHANNEL,
+};
+
+/**
+ * struct stk3310_chip_info - chip-specific data
+ * @name: device name reported to the IIO core
+ * @channels: channel specification
+ * @num_channels: number of channels
+ */
+struct stk3310_chip_info {
+	const char			*name;
+	const struct iio_chan_spec	*channels __counted_by_ptr(num_channels);
+	unsigned int			num_channels;
+};
+
+static const struct stk3310_chip_info stk3310_chip_info = {
+	.name = STK3310_DRIVER_NAME,
+	.channels = stk3310_channels,
+	.num_channels = ARRAY_SIZE(stk3310_channels),
 };
 
 static IIO_CONST_ATTR(in_illuminance_scale_available, STK3310_SCALE_AVAILABLE);
@@ -370,7 +394,6 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
 {
-	u8 reg;
 	__be16 buf;
 	int ret;
 	unsigned int index;
@@ -382,13 +405,9 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (chan->type == IIO_LIGHT)
-			reg = STK3310_REG_ALS_DATA_MSB;
-		else
-			reg = STK3310_REG_PS_DATA_MSB;
-
 		mutex_lock(&data->lock);
-		ret = regmap_bulk_read(data->regmap, reg, &buf, sizeof(buf));
+		ret = regmap_bulk_read(data->regmap, chan->address, &buf,
+				       sizeof(buf));
 		if (ret < 0) {
 			dev_err(&client->dev, "register read failed\n");
 			mutex_unlock(&data->lock);
@@ -635,10 +654,22 @@ static irqreturn_t stk3310_irq_event_handler(int irq, void *private)
 
 static int stk3310_probe(struct i2c_client *client)
 {
+	const struct stk3310_chip_info *chip_info;
 	int ret;
 	struct iio_dev *indio_dev;
 	struct stk3310_data *data;
 
+	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;
+	}
+
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -657,10 +688,10 @@ static int stk3310_probe(struct i2c_client *client)
 		return ret;
 
 	indio_dev->info = &stk3310_info;
-	indio_dev->name = STK3310_DRIVER_NAME;
+	indio_dev->name = chip_info->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = stk3310_channels;
-	indio_dev->num_channels = ARRAY_SIZE(stk3310_channels);
+	indio_dev->channels = chip_info->channels;
+	indio_dev->num_channels = chip_info->num_channels;
 
 	ret = stk3310_init(indio_dev);
 	if (ret < 0)
@@ -766,28 +797,28 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stk3310_pm_ops, stk3310_suspend,
 				stk3310_resume);
 
 static const struct i2c_device_id stk3310_i2c_id[] = {
-	{ .name = "STK3013" },
-	{ .name = "STK3310" },
-	{ .name = "STK3311" },
-	{ .name = "STK3335" },
+	{ .name = "STK3013", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
+	{ .name = "STK3310", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
+	{ .name = "STK3311", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
+	{ .name = "STK3335", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, stk3310_i2c_id);
 
 static const struct acpi_device_id stk3310_acpi_id[] = {
-	{"STK3013", 0},
-	{"STK3310", 0},
-	{"STK3311", 0},
+	{"STK3013", (kernel_ulong_t)&stk3310_chip_info},
+	{"STK3310", (kernel_ulong_t)&stk3310_chip_info},
+	{"STK3311", (kernel_ulong_t)&stk3310_chip_info},
 	{ }
 };
 
 MODULE_DEVICE_TABLE(acpi, stk3310_acpi_id);
 
 static const struct of_device_id stk3310_of_match[] = {
-	{ .compatible = "sensortek,stk3013", },
-	{ .compatible = "sensortek,stk3310", },
-	{ .compatible = "sensortek,stk3311", },
-	{ .compatible = "sensortek,stk3335", },
+	{ .compatible = "sensortek,stk3013", .data = &stk3310_chip_info },
+	{ .compatible = "sensortek,stk3310", .data = &stk3310_chip_info },
+	{ .compatible = "sensortek,stk3311", .data = &stk3310_chip_info },
+	{ .compatible = "sensortek,stk3335", .data = &stk3310_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, stk3310_of_match);
-- 
2.55.0

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

[Andy Shevchenko]

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

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

[Jorijn van der Graaf]

On Mon, Aug 10, 2026 at 09:34:42PM +0300, Andy Shevchenko wrote:
> > +	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?

No conversion - the client name is whatever is echoed into the sysfs
new_device file, and the lowercase spelling is the one that binds via
the OF table: for a client with no firmware node,
i2c_of_match_device() falls through to i2c_of_match_device_sysfs(),
which matches the client name against each compatible and its part
after the vendor prefix, so "stk3310" binds through
"sensortek,stk3310" - the same lowercase name a DT client gets from
of_alias_from_compatible(). The id table lookup stays case-sensitive,
which is why i2c_get_match_data() comes back NULL for such a client:
device_get_match_data() needs the firmware node it does not have, and
i2c_match_id() only knows the historic uppercase names.

> - What's wrong with simply failing the probe?

It would break a path that works before this patch: probe consumed no
match data, so "echo stk3310 0x48 > new_device" bound and probed fine
(we used exactly that during bring-up, before the DT node existed).
Failing it would be a userspace-visible regression in a patch that
claims no functional change.

> 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?

The path preexists; only this patch makes probe care about match
data. Before it the driver had no per-chip data at all - every client
got the same two channels and the hardcoded "stk3310" name however it
matched. The fallback selects stk3310_chip_info, which is exactly
that, so a client that probed before this patch behaves identically
after it. What the default cannot give is the new part's extra
channels: an STK36C61 instantiated under the lowercase name gets the
reduced legacy profile, and the id table spelling "STK36C61" selects
the full one.

Thanks,
Jorijn

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

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 11:51:57PM +0200, Jorijn van der Graaf wrote:
> On Mon, Aug 10, 2026 at 09:34:42PM +0300, Andy Shevchenko wrote:
> > > +	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?
> 
> No conversion - the client name is whatever is echoed into the sysfs
> new_device file, and the lowercase spelling is the one that binds via
> the OF table: for a client with no firmware node,
> i2c_of_match_device() falls through to i2c_of_match_device_sysfs(),
> which matches the client name against each compatible and its part
> after the vendor prefix, so "stk3310" binds through
> "sensortek,stk3310" - the same lowercase name a DT client gets from
> of_alias_from_compatible(). The id table lookup stays case-sensitive,
> which is why i2c_get_match_data() comes back NULL for such a client:
> device_get_match_data() needs the firmware node it does not have, and
> i2c_match_id() only knows the historic uppercase names.
> 
> > - What's wrong with simply failing the probe?
> 
> It would break a path that works before this patch: probe consumed no
> match data, so "echo stk3310 0x48 > new_device" bound and probed fine
> (we used exactly that during bring-up, before the DT node existed).
> Failing it would be a userspace-visible regression in a patch that
> claims no functional change.
> 
> > 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?
> 
> The path preexists; only this patch makes probe care about match
> data. Before it the driver had no per-chip data at all - every client
> got the same two channels and the hardcoded "stk3310" name however it
> matched. The fallback selects stk3310_chip_info, which is exactly
> that, so a client that probed before this patch behaves identically
> after it. What the default cannot give is the new part's extra
> channels: an STK36C61 instantiated under the lowercase name gets the
> reduced legacy profile, and the id table spelling "STK36C61" selects
> the full one.

The problem preexists. I just looked into the Git history of this rather broken
table IDs and what I have to say and state is the following:

- the driver initially was written for ACPI and the author blindly (cargo cult?)
  used the same IDs as for ACPI _HID, while they have different rules

- there is no existing in kernel driver that supports enumeration based
on I2C ID table

With that being said, I think we may just lower the case for the I²C ID
table and fail enumeration when there is no chip_info provided.
Yes, it will break very weird use case with capital letters via sysfs,
but TBH it's a niche and people should know that most of the devices
are enumerated as part number of OF compatible in that case.

Also note that the DT users should also not use sysfs, and instead call
DT overlay.

I can take all the blame on very unlikely event of a breakage of somebody's
setup in the future. TL;DR: just add a first patch to lower case the IDs
with Suggested-by my name and I will give a Rb tag with full conscience and
responsibility.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/3] iio: light: stk3310: support the Sensortek STK36C61

[Jorijn van der Graaf]

The Sensortek STK36C61 is a 3-in-1 ambient light / proximity / RGB
colour sensor (chip ID 0x95) found in the Fairphone 6. Its register
interface is compatible with the feature set this driver uses: the
STATE/FLAG bit layout, the data and threshold registers and the gain
and integration-time fields, verified on that device (the ALS and
proximity readings scale with their gain and integration-time fields,
thresholds written through the event interface read back from the
chip, and the FLAG near/far bit crosses with them). Add its chip ID to
the known-ID list and the device table entries.

Whenever the ALS engine runs, the chip also measures four colour
channels, laid out directly after the ALS data as 16-bit big-endian
values in R (0x15), G (0x17), B (0x19), C (0x1B) order; the R, G and B
assignments were each confirmed by the matching channel dominating
under red, green and blue illumination, and clear by its broadband
response. The ALS data register tracks the green channel exactly.
There is no public datasheet and therefore no counts-to-physical
mapping for the colour channels, so they are exposed as raw-only
intensity channels; the gain field the scale attribute controls does
not govern clear either.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
 drivers/iio/light/stk3310.c | 44 ++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
index 249396b9bf80..288b544a8abb 100644
--- a/drivers/iio/light/stk3310.c
+++ b/drivers/iio/light/stk3310.c
@@ -40,6 +40,11 @@
 #define STK3310_REG_PS_DATA_LSB			0x12
 #define STK3310_REG_ALS_DATA_MSB		0x13
 #define STK3310_REG_ALS_DATA_LSB		0x14
+#define STK36C61_REG_RED_DATA_MSB		0x15
+#define STK36C61_REG_GREEN_DATA_MSB		0x17
+#define STK36C61_REG_BLUE_DATA_MSB		0x19
+#define STK36C61_REG_CLEAR_DATA_MSB		0x1B
+#define STK36C61_REG_CLEAR_DATA_LSB		0x1C
 #define STK3310_REG_ID				0x3E
 #define STK3310_MAX_REG				0x80
 
@@ -54,6 +59,7 @@
 #define STK3311S34_CHIP_ID_VAL			0x1E
 #define STK3311X_CHIP_ID_VAL			0x12
 #define STK3335_CHIP_ID_VAL			0x51
+#define STK36C61_CHIP_ID_VAL			0x95
 #define STK3310_PSINT_EN			0x01
 #define STK3310_PS_MAX_VAL			0xFFFF
 
@@ -102,6 +108,7 @@ static const u8 stk3310_chip_ids[] = {
 	STK3311X_CHIP_ID_VAL,
 	STK3311_CHIP_ID_VAL,
 	STK3335_CHIP_ID_VAL,
+	STK36C61_CHIP_ID_VAL,
 };
 
 /* Estimate maximum proximity values with regard to measurement scale. */
@@ -202,11 +209,28 @@ static const struct iio_chan_spec_ext_info stk3310_ext_info[] = {
 	.ext_info = stk3310_ext_info,			\
 }
 
+#define STK36C61_INTENSITY_CHANNEL(_mod, _reg) {	\
+	.type = IIO_INTENSITY,				\
+	.modified = 1,					\
+	.channel2 = IIO_MOD_LIGHT_##_mod,		\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
+	.address = _reg,				\
+}
+
 static const struct iio_chan_spec stk3310_channels[] = {
 	STK3310_LIGHT_CHANNEL,
 	STK3310_PROXIMITY_CHANNEL,
 };
 
+static const struct iio_chan_spec stk36c61_channels[] = {
+	STK3310_LIGHT_CHANNEL,
+	STK3310_PROXIMITY_CHANNEL,
+	STK36C61_INTENSITY_CHANNEL(RED, STK36C61_REG_RED_DATA_MSB),
+	STK36C61_INTENSITY_CHANNEL(GREEN, STK36C61_REG_GREEN_DATA_MSB),
+	STK36C61_INTENSITY_CHANNEL(BLUE, STK36C61_REG_BLUE_DATA_MSB),
+	STK36C61_INTENSITY_CHANNEL(CLEAR, STK36C61_REG_CLEAR_DATA_MSB),
+};
+
 /**
  * struct stk3310_chip_info - chip-specific data
  * @name: device name reported to the IIO core
@@ -225,6 +249,12 @@ static const struct stk3310_chip_info stk3310_chip_info = {
 	.num_channels = ARRAY_SIZE(stk3310_channels),
 };
 
+static const struct stk3310_chip_info stk36c61_chip_info = {
+	.name = "stk36c61",
+	.channels = stk36c61_channels,
+	.num_channels = ARRAY_SIZE(stk36c61_channels),
+};
+
 static IIO_CONST_ATTR(in_illuminance_scale_available, STK3310_SCALE_AVAILABLE);
 
 static IIO_CONST_ATTR(in_proximity_scale_available, STK3310_SCALE_AVAILABLE);
@@ -400,7 +430,8 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
 	struct stk3310_data *data = iio_priv(indio_dev);
 	struct i2c_client *client = data->client;
 
-	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY)
+	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY &&
+	    chan->type != IIO_INTENSITY)
 		return -EINVAL;
 
 	switch (mask) {
@@ -419,8 +450,10 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_INT_TIME:
 		if (chan->type == IIO_LIGHT)
 			ret = regmap_field_read(data->reg_als_it, &index);
-		else
+		else if (chan->type == IIO_PROXIMITY)
 			ret = regmap_field_read(data->reg_ps_it, &index);
+		else
+			return -EINVAL;
 		if (ret < 0)
 			return ret;
 
@@ -430,8 +463,10 @@ static int stk3310_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		if (chan->type == IIO_LIGHT)
 			ret = regmap_field_read(data->reg_als_gain, &index);
-		else
+		else if (chan->type == IIO_PROXIMITY)
 			ret = regmap_field_read(data->reg_ps_gain, &index);
+		else
+			return -EINVAL;
 		if (ret < 0)
 			return ret;
 
@@ -567,6 +602,7 @@ static bool stk3310_is_volatile_reg(struct device *dev, unsigned int reg)
 	switch (reg) {
 	case STK3310_REG_ALS_DATA_MSB:
 	case STK3310_REG_ALS_DATA_LSB:
+	case STK36C61_REG_RED_DATA_MSB ... STK36C61_REG_CLEAR_DATA_LSB:
 	case STK3310_REG_PS_DATA_LSB:
 	case STK3310_REG_PS_DATA_MSB:
 	case STK3310_REG_FLAG:
@@ -801,6 +837,7 @@ static const struct i2c_device_id stk3310_i2c_id[] = {
 	{ .name = "STK3310", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
 	{ .name = "STK3311", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
 	{ .name = "STK3335", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
+	{ .name = "STK36C61", .driver_data = (kernel_ulong_t)&stk36c61_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, stk3310_i2c_id);
@@ -819,6 +856,7 @@ static const struct of_device_id stk3310_of_match[] = {
 	{ .compatible = "sensortek,stk3310", .data = &stk3310_chip_info },
 	{ .compatible = "sensortek,stk3311", .data = &stk3310_chip_info },
 	{ .compatible = "sensortek,stk3335", .data = &stk3310_chip_info },
+	{ .compatible = "sensortek,stk36c61", .data = &stk36c61_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, stk3310_of_match);
-- 
2.55.0

Re: [PATCH 3/3] iio: light: stk3310: support the Sensortek STK36C61

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The IIO device is unregistered before the threaded IRQ handler is freed, creating a Use-After-Free race condition during device removal.
--

--- Patch [3]: [PATCH 3/3] iio: light: stk3310: support the Sensortek STK36C61 ---
commit c621ace8607632002893d3b296ea6bb5b3e8c5f1
Author: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>

iio: light: stk3310: support the Sensortek STK36C61

This commit adds support for the Sensortek STK36C61 ambient light,
proximity, and RGB color sensor to the stk3310 driver. It introduces the
new chip ID, registers, and intensity channels for color data.

> diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
> index 17b1ff42bf6e1..289900aeafa78 100644
> --- a/drivers/iio/light/stk3310.c
> +++ b/drivers/iio/light/stk3310.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
use-after-free race during device removal in this driver.

In stk3310_remove(), the IIO device is unregistered before the devm-managed
threaded IRQ handler is freed:

drivers/iio/light/stk3310.c:stk3310_remove() {
	struct iio_dev *indio_dev = i2c_get_clientdata(client);

	iio_device_unregister(indio_dev);
	stk3310_set_state(iio_priv(indio_dev), STK3310_STATE_STANDBY);
}

Since the interrupt is requested with devm_request_threaded_irq() in
stk3310_probe(), it remains active until after stk3310_remove() returns.

If an interrupt fires during or immediately after iio_device_unregister(),
the IRQ handler can call iio_push_event().

Because iio_device_unregister() frees the event interface, could this result
in a use-after-free when the active IRQ handler writes to the freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810110423.41697-1-jorijnvdgraaf@catcrafts.net?part=3

Re: [PATCH 3/3] iio: light: stk3310: support the Sensortek STK36C61

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 01:04:23PM +0200, Jorijn van der Graaf wrote:
> The Sensortek STK36C61 is a 3-in-1 ambient light / proximity / RGB
> colour sensor (chip ID 0x95) found in the Fairphone 6. Its register
> interface is compatible with the feature set this driver uses: the
> STATE/FLAG bit layout, the data and threshold registers and the gain
> and integration-time fields, verified on that device (the ALS and
> proximity readings scale with their gain and integration-time fields,
> thresholds written through the event interface read back from the
> chip, and the FLAG near/far bit crosses with them). Add its chip ID to
> the known-ID list and the device table entries.
> 
> Whenever the ALS engine runs, the chip also measures four colour
> channels, laid out directly after the ALS data as 16-bit big-endian
> values in R (0x15), G (0x17), B (0x19), C (0x1B) order; the R, G and B
> assignments were each confirmed by the matching channel dominating
> under red, green and blue illumination, and clear by its broadband
> response. The ALS data register tracks the green channel exactly.
> There is no public datasheet and therefore no counts-to-physical
> mapping for the colour channels, so they are exposed as raw-only
> intensity channels; the gain field the scale attribute controls does
> not govern clear either.

...

> -	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY)
> +	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY &&
> +	    chan->type != IIO_INTENSITY)
>  		return -EINVAL;

Is this now a duplicating a bit with the below choices?

...

>  	switch (mask) {

>  	case IIO_CHAN_INFO_INT_TIME:
>  		if (chan->type == IIO_LIGHT)
>  			ret = regmap_field_read(data->reg_als_it, &index);
> -		else
> +		else if (chan->type == IIO_PROXIMITY)
>  			ret = regmap_field_read(data->reg_ps_it, &index);
> +		else
> +			return -EINVAL;

If you want to keep a single point of return, use

			ret = -EINVAL;

here and below in _read_raw().

>  		if (ret < 0)
>  			return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: light: stk3310: support the Sensortek STK36C61

[Jorijn van der Graaf]

On Mon, Aug 10, 2026 at 09:37:21PM +0300, Andy Shevchenko wrote:
> > -	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY)
> > +	if (chan->type != IIO_LIGHT && chan->type != IIO_PROXIMITY &&
> > +	    chan->type != IIO_INTENSITY)
> >  		return -EINVAL;
>
> Is this now a duplicating a bit with the below choices?

A bit, in the int_time and scale arms. The check at the top is
pre-existing (this patch only adds IIO_INTENSITY to it) and is the
only type filter for the RAW case, which serves all three types
through chan->address with no per-type branching of its own.

> >  	case IIO_CHAN_INFO_INT_TIME:
> >  		if (chan->type == IIO_LIGHT)
> >  			ret = regmap_field_read(data->reg_als_it, &index);
> > -		else
> > +		else if (chan->type == IIO_PROXIMITY)
> >  			ret = regmap_field_read(data->reg_ps_it, &index);
> > +		else
> > +			return -EINVAL;
>
> If you want to keep a single point of return, use
>
> 			ret = -EINVAL;
>
> here and below in _read_raw().

Neither of the new else branches is reachable today: the colour
channels declare RAW only, so no int_time or scale attribute exists
for them, and in-kernel readers are gated by iio_channel_has_info().
I would rather keep them than have a bare else read as proximity for
a colour channel. v2 writes them with ret = -EINVAL as you suggest,
falling into the existing if (ret < 0) return ret.

Thanks,
Jorijn