io: light: tcs3472: Add TMD3782 proximity support by playday3008 · Pull Request #445 · msm8916-mainline/linux

playday3008 · 2026-04-29T09:17:50Z

Add TMD3782 RGBC + proximity sensor support to the tcs3472 driver and enable it on Samsung Galaxy A5/A7 device trees.

Driver changes (patches 1-6):

dt-bindings: Add amstaos,tcs3472.yaml schema
OF match table, regulator support, per-variant chip_info struct
TMD3782 register definitions, chip ID detection, CONTROL/PPULSE init
Proximity channel (IIO_PROXIMITY) with threshold events (rising/falling)
Direct-mode and buffered proximity reads with PEN/WEN coordination

Device tree changes (patches 7-8):

msm8916-samsung-a5u-eur: TMD3782 on blsp_i2c2, GPIO8 VDD, GPIO113 IRQ
msm8939-samsung-a7: TMD3782 on bit-banged i2c-sensor, pm8916_l17 VDD

Based on msm8916/6.12.1 from msm8916-mainline/linux.

wonderfulShrineMaidenOfParadise · 2026-04-29T12:08:51Z

arm64: dts: qcom: msm8916-samsung-a5u-eur: Add proximity/light sensor
...

The a5-zt variant inherits this node via its include of a5u-eur.dts.

Note that a5-zt no longer exists in newer branches and it's now a5ltezt.
Besides a5ltezt is not in mainline.

Add devicetree binding for the AMS/TAOS TCS3472 RGBC color light sensor and TMD3782 RGBC + proximity sensor. The TCS3472 has no existing binding despite having a mainline driver since 2013. The TMD3782 is a superset of TCS3472 adding an integrated proximity detector with IR LED. Additional properties for proximity configuration (pulse count, LED current, LED supply) are optional and only relevant for TMD3782.

Add of_device_id table for devicetree-based probing of TCS3472 and TMD3782 sensors. Add optional vdd/vddio regulator support with 2.4ms power-on settling delay. Update tcs3472_powerdown() to write 0x00 for full chip power-down instead of only clearing AEN+PON. This is a behavior change: the old code preserved AIEN, the new code clears everything. This is safe because resume now performs a full register restore. Update resume to rewrite all configuration registers (ATIME, WTIME, thresholds, PERS, CONFIG, CONTROL, ENABLE) and clear stale interrupt flags. This hardens against potential register state loss during PON=0. Remove hardcoded IRQF_TRIGGER_FALLING from request_threaded_irq to let the devicetree specify the interrupt trigger type.

Add tcs3472_chip_info struct to hold per-variant channel arrays, channel counts, and feature flags. This prepares for adding TMD3782 proximity support without scattering variant checks throughout the driver. No functional change — only TCS3472 is populated. The TMD3782 entry will be added when proximity support lands.

Add proximity register definitions for the TMD3782 (PILT, PIHT, PPULSE, PDATA, PIEN, PEN, WEN, PINT, PVALID) and interrupt clear commands. Detect TMD3782 chip IDs (0x60 for TMD37821, 0x69 for TMD37823) in probe. Initialize the CONTROL register with the mandatory bit 5 and LED drive current from the led-max-microamp DT property. Configure proximity pulse count from DT (default 8) and set PPERS to 3 to match downstream behavior. Fix the PERS register write to merge both APERS and PPERS nibbles, preventing the existing bug where writing APERS clobbers PPERS. Extend resume to restore proximity thresholds, pulse count, and clear stale interrupt flags from both ALS and proximity.

Add proximity channel (IIO_PROXIMITY, scan_index 4) to TMD3782 channel array. The proximity data register at 0x1C fits the existing trigger handler's CDATA+2*i arithmetic naturally: 0xB4+8 = 0xBC. Add proximity threshold events (rising/falling/enable) with a separate event spec — no IIO_EV_INFO_PERIOD since PPERS uses a linear count, not the non-linear APERS mapping. PPERS is fixed at 3 (not configurable by userspace in v1). Extend event value read/write for proximity thresholds (PILT/PIHT). Extend write_event_config to manage PEN+WEN+PIEN for proximity events, with proper coordination against buffer enable state. Guard against enabling interrupts when no IRQ is configured. Extend interrupt handler to check both AINT and PINT in STATUS and clear only the interrupts actually observed, preventing a race where a new interrupt between STATUS read and clear would be silently lost.

Extend tcs3472_req_data() to accept a status mask parameter for polling AVALID, PVALID, or both. For direct-mode proximity reads (in_proximity_raw), temporarily enable PEN+WEN when no events or buffer are active. Uses a re-check-under-lock pattern on teardown to avoid clobbering concurrent event enables. Add buffer preenable/postdisable callbacks that manage PEN+WEN when the proximity channel (scan_index 4) is in the active scan mask. This prevents the trigger handler from hanging on PVALID when PEN is off. Coordinates with event enable state so neither path clobbers the other.

Add TMD3782 RGBC + proximity sensor on blsp_i2c2 at address 0x39. Uses GPIO113 as level-triggered interrupt input and GPIO8 as VDD enable for the sensor (modeled as a fixed regulator). The sensor shares the I2C bus with the existing BMC150 accelerometer and magnetometer. VDD (GPIO8) and VDDIO (pm8916_l5) need explicit control. The a5-zt variant inherits this node via its include of a5u-eur.dts.

Add TMD3782 RGBC + proximity sensor on the bit-banged i2c-sensor bus at address 0x39. Uses GPIO113 as level-triggered interrupt input. Unlike the A5 which uses a GPIO-controlled fixed regulator for sensor VDD, the A7 powers the sensor directly from pm8916_l17.

playday3008 · 2026-04-29T14:34:28Z

Note that a5-zt no longer exists in newer branches and it's now a5ltezt. Besides a5ltezt is not in mainline.

Well, I was working against same kernel version that was in pmOS edge for Galaxy A5, couldn't get latest kernel from source running, so, was writing patches against same version that was on device.

erikas9987 · 2026-05-04T14:30:14Z

You should rebase this on the latest kernel branch available. At the time of writing this comment, it's wip/msm8916/7.0.
And ofc, your signoff is missing, hence why checkpatch is complaining.

erikas9987 · 2026-05-09T09:07:44Z

 	struct i2c_client *client;
+	const struct tcs3472_chip_info *chip_info;
 	struct mutex lock;
+	bool prox_event_enabled;


That's quite a lot of variables here, have you considered switching to regmap and then enabling regcache? It'd replace your current caching mechanism in a cleaner manner, for all variables.
See https://github.com/msm8916-mainline/linux/blob/wip/msm8916/7.0/drivers/iio/light/ltr501.c on how it's done. Keep in mind that your regmap settings will be different, for example, at the very least, you will need REGMAP_LITTLE_ENDIAN

linux/drivers/iio/light/ltr501.c

Lines 150 to 165 in ea3a5c5

struct ltr501_data {

struct i2c_client *client;

struct mutex lock_als, lock_ps;

const struct ltr501_chip_info *chip_info;

u8 als_contr, ps_contr;

int als_period, ps_period; /* period in micro seconds */

struct regmap *regmap;

struct regmap_field *reg_it;

struct regmap_field *reg_als_intr;

struct regmap_field *reg_ps_intr;

struct regmap_field *reg_als_rate;

struct regmap_field *reg_ps_rate;

struct regmap_field *reg_als_prst;

struct regmap_field *reg_ps_prst;

uint32_t near_level;

};

linux/drivers/iio/light/ltr501.c

Lines 197 to 231 in ea3a5c5

static int ltr501_als_read_samp_freq(const struct ltr501_data *data,

int *val, int *val2)

{

int ret, i;

ret = regmap_field_read(data->reg_als_rate, &i);

if (ret < 0)

return ret;

if (i < 0 || i >= ARRAY_SIZE(ltr501_als_samp_table))

return -EINVAL;

*val = ltr501_als_samp_table[i].freq_val / 1000000;

*val2 = ltr501_als_samp_table[i].freq_val % 1000000;

return IIO_VAL_INT_PLUS_MICRO;

}

static int ltr501_ps_read_samp_freq(const struct ltr501_data *data,

int *val, int *val2)

{

int ret, i;

ret = regmap_field_read(data->reg_ps_rate, &i);

if (ret < 0)

return ret;

if (i < 0 || i >= ARRAY_SIZE(ltr501_ps_samp_table))

return -EINVAL;

*val = ltr501_ps_samp_table[i].freq_val / 1000000;

*val2 = ltr501_ps_samp_table[i].freq_val % 1000000;

return IIO_VAL_INT_PLUS_MICRO;

}

linux/drivers/iio/light/ltr501.c

Lines 1400 to 1407 in ea3a5c5

static const struct regmap_config ltr501_regmap_config = {

.name = "ltr501_regmap",

.reg_bits = 8,

.val_bits = 8,

.max_register = LTR501_MAX_REG,

.cache_type = REGCACHE_MAPLE,

.volatile_reg = ltr501_is_volatile_reg,

};

linux/drivers/iio/light/ltr501.c

Lines 1432 to 1436 in ea3a5c5

regmap = devm_regmap_init_i2c(client, &ltr501_regmap_config);

if (IS_ERR(regmap)) {

dev_err(&client->dev, "Regmap initialization failed.\n");

return PTR_ERR(regmap);

}

linux/drivers/iio/light/ltr501.c

Lines 1452 to 1457 in ea3a5c5

data->reg_it = devm_regmap_field_alloc(&client->dev, regmap,

reg_field_it);

if (IS_ERR(data->reg_it)) {

dev_err(&client->dev, "Integ time reg field init failed.\n");

return PTR_ERR(data->reg_it);

}

playday3008 · 2026-05-11T09:37:08Z

Glad I made PR here first, before trying to upstream, I imagine it would take much longer to iterate on issues and improvements using mailing lists. I'll look into all comments and reviews, for now I'll make code changes as new commits, but after that I'll squash them into respective current commits to keep history clean. If everything will look good by y'all then I'll close that PR and will try to make upstream contribution. Thanks

The PDRIVE field exists only in TMD3782, not TCS3472. Rename TCS3472_CONTROL_PDRIVE_MASK to TMD3782_CONTROL_PDRIVE_MASK to make this explicit.

… table Replace the enum-indexed tcs3472_chip_info_tbl[] array with standalone tcs3472_chip_info and tmd3782_chip_info structs, following the pattern recommended for modern IIO drivers (cf. veml6030). Add driver_data to the i2c_device_id table so i2c_get_match_data() works for non-OF paths.

Check AINT and PINT status bits independently and clear each interrupt separately, rather than using ALL_INTR_CLEAR when both fire simultaneously. Both interrupt sources can be pending at the same time and should each be cleared after being handled.

Add a reference to the TMD3782 v2 datasheet hosted on DigiKey.

…ement Replace all manual mutex_lock/mutex_unlock pairs with guard(mutex), which auto-releases on scope exit. This eliminates goto-based error cleanup in write_event and resume, and prevents potential missed unlocks on early-return paths.

…bute Expose the TMD3782 proximity LED drive current (PDRIVE) as an IIO_CHAN_INFO_CALIBBIAS attribute on the proximity channel, allowing runtime control from userspace. Remove the led-max-microamp DT property since the hardware default (100mA) applies and userspace can override via calibbias. Accepted values: 12500, 25000, 50000, 100000 (microamps).

…tribute Expose the TMD3782 proximity pulse count (PPULSE) as an IIO_CHAN_INFO_OVERSAMPLING_RATIO attribute on the proximity channel, allowing runtime tuning from userspace. Remove the amstaos,proximity-pulse-count DT property since the driver defaults to 6 pulses and userspace can override. Accepted values: 1-255.

erikas9987

Right now if you run monitor-sensor on your device, you will notice that it reports no proximity sensor, even though if there is one. That is because iio-sensor-proxy, the userspace daemon reading measurements from your sensors, doesn't understand how to interpret data from your proximity sensor.
You can add proximity-near-level device tree property to your driver, like it's done so in vcnl4000. It's needed so iio-sensor-proxy can properly pick up far/near measurements and expose them to userspace.

linux/drivers/iio/light/vcnl4000.c

Line 2025 in ea3a5c5

device_property_read_u32(dev, "proximity-near-level", &data->near_level);

linux/drivers/iio/light/vcnl4000.c

Lines 1592 to 1600 in ea3a5c5

    
           static ssize_t vcnl4000_read_near_level(struct iio_dev *indio_dev, 
        
           					uintptr_t priv, 
        
           					const struct iio_chan_spec *chan, 
        
           					char *buf) 
        
           { 
        
           	struct vcnl4000_data *data = iio_priv(indio_dev); 
        
           	return sprintf(buf, "%u\n", data->near_level); 
        
           }

Though in the function above, you should use sysfs_emit instead of sprintf.

linux/drivers/iio/light/vcnl4000.c

Lines 1733 to 1740 in ea3a5c5

    
           static const struct iio_chan_spec_ext_info vcnl4000_ext_info[] = { 
        
           	{ 
        
           		.name = "nearlevel", 
        
           		.shared = IIO_SEPARATE, 
        
           		.read = vcnl4000_read_near_level, 
        
           	}, 
        
           	{ } 
        
           };

linux/drivers/iio/light/vcnl4000.c

Lines 1790 to 1800 in ea3a5c5

    
           static const struct iio_chan_spec vcnl4000_channels[] = { 
        
           	{ 
        
           		.type = IIO_LIGHT, 
        
           		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | 
        
           			BIT(IIO_CHAN_INFO_SCALE), 
        
           	}, { 
        
           		.type = IIO_PROXIMITY, 
        
           		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), 
        
           		.ext_info = vcnl4000_ext_info, 
        
           	} 
        
           };

linux/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml

Line 50 in ea3a5c5

proximity-near-level: true

erikas9987 · 2026-05-11T20:46:56Z


+	data->enable_saved = data->enable;
+
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,


This function has a managed (devm_*) counterpart. Use it so the resources are cleaned up after the driver's lifetime.

erikas9987 · 2026-05-11T20:47:05Z

 		return ret;

 	if (client->irq) {
 		ret = request_threaded_irq(client->irq, NULL,


erikas9987 · 2026-05-11T20:52:48Z

 	struct mutex lock;
+	bool prox_event_enabled;
+	bool prox_buf_enabled;
 	u16 low_thresh;


If you are making a distinction between color variables and proximity variables, maybe name them accordingly? For example, you can name this variable cs_low_thresh

erikas9987 · 2026-05-11T20:56:09Z

+		if (chan->type == IIO_PROXIMITY) {
+			bool cold;
+
+			{


Are you sure you need this scope here? If not, drop it. If you do, scoped_guard is a better choice.

erikas9987 · 2026-05-11T20:58:20Z

 		*val2 = (256 - data->atime) * 2400;
 		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		*val = tmd3782_pdrive_uamp[FIELD_GET(TMD3782_CONTROL_PDRIVE_MASK,


You should split this into two declarations for easier readability, like so:

i = FIELD_GET(TMD3782_CONTROL_PDRIVE_MASK, data->control); *val = tmd3782_pdrive_uamp[i]

erikas9987 · 2026-05-11T21:03:14Z

 			goto buffer_cleanup;
 	}

 	ret = iio_device_register(indio_dev);


erikas9987 · 2026-05-11T21:04:15Z


 free_irq:
 	if (client->irq)
 		free_irq(client->irq, indio_dev);


Once you convert the resource allocating calls to their managed counterparts, don't forget to remove the cleanup calls as well.

erikas9987 · 2026-05-11T21:04:47Z

 	return ret;
 }

 static void tcs3472_remove(struct i2c_client *client)


This function can be dropped as well afterwards.

erikas9987 · 2026-05-11T21:09:38Z

 		if (ret < 0)
 			return ret;
-		if (ret & TCS3472_STATUS_AVALID)
+		if ((ret & status_mask) == status_mask)


This can be simplified:

if (ret & status_mask)

erikas9987 · 2026-05-11T21:13:09Z

@@ -455,29 +770,56 @@
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 	mutex_init(&data->lock);


This function has a managed (devm_*) counterpart. Use it so the mutex is freed after driver's lifetime.

erikas9987 · 2026-05-11T21:29:12Z

Oh, and any cleanups of the driver should come before addition of new functionality and/or hardware.

playday3008 · 2026-05-11T23:41:56Z

Damn dude, thanks, you seem far more knowledgeable about contributing to Linux. I'll look again maybe in 1-2 days, will try to build it again and thoroughly test it on real hardware.

wonderfulShrineMaidenOfParadise reviewed Apr 29, 2026

View reviewed changes

Comment thread arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts Outdated

PlayDay added 8 commits April 29, 2026 16:25

playday3008 force-pushed the wip/msm8916/6.12.1/tmd3782 branch from 803ff70 to 8798788 Compare April 29, 2026 14:27

erikas9987 reviewed May 4, 2026

View reviewed changes

Comment thread drivers/iio/light/tcs3472.c Outdated

Comment thread drivers/iio/light/tcs3472.c Outdated

Comment thread drivers/iio/light/tcs3472.c Outdated

Comment thread drivers/iio/light/tcs3472.c Outdated

stephan-gh added the needs-upstreaming label May 7, 2026

erikas9987 reviewed May 9, 2026

View reviewed changes

PlayDay added 7 commits May 11, 2026 16:47

iio: light: tcs3472: Rename PDRIVE mask with TMD3782 prefix

3a0037b

The PDRIVE field exists only in TMD3782, not TCS3472. Rename TCS3472_CONTROL_PDRIVE_MASK to TMD3782_CONTROL_PDRIVE_MASK to make this explicit.

dt-bindings: iio: light: amstaos,tcs3472: Add TMD3782 datasheet link

64e92da

Add a reference to the TMD3782 v2 datasheet hosted on DigiKey.

playday3008 requested review from erikas9987 and wonderfulShrineMaidenOfParadise May 11, 2026 16:57

erikas9987 reviewed May 11, 2026

View reviewed changes

	struct ltr501_data {
	struct i2c_client *client;
	struct mutex lock_als, lock_ps;
	const struct ltr501_chip_info *chip_info;
	u8 als_contr, ps_contr;
	int als_period, ps_period; /* period in micro seconds */
	struct regmap *regmap;
	struct regmap_field *reg_it;
	struct regmap_field *reg_als_intr;
	struct regmap_field *reg_ps_intr;
	struct regmap_field *reg_als_rate;
	struct regmap_field *reg_ps_rate;
	struct regmap_field *reg_als_prst;
	struct regmap_field *reg_ps_prst;
	uint32_t near_level;
	};

	static int ltr501_als_read_samp_freq(const struct ltr501_data *data,
	int val, int val2)
	{
	int ret, i;

	ret = regmap_field_read(data->reg_als_rate, &i);
	if (ret < 0)
	return ret;

	if (i < 0 \|\| i >= ARRAY_SIZE(ltr501_als_samp_table))
	return -EINVAL;

	*val = ltr501_als_samp_table[i].freq_val / 1000000;
	*val2 = ltr501_als_samp_table[i].freq_val % 1000000;

	return IIO_VAL_INT_PLUS_MICRO;
	}

	static int ltr501_ps_read_samp_freq(const struct ltr501_data *data,
	int val, int val2)
	{
	int ret, i;

	ret = regmap_field_read(data->reg_ps_rate, &i);
	if (ret < 0)
	return ret;

	if (i < 0 \|\| i >= ARRAY_SIZE(ltr501_ps_samp_table))
	return -EINVAL;

	*val = ltr501_ps_samp_table[i].freq_val / 1000000;
	*val2 = ltr501_ps_samp_table[i].freq_val % 1000000;

	return IIO_VAL_INT_PLUS_MICRO;
	}

	static const struct regmap_config ltr501_regmap_config = {
	.name = "ltr501_regmap",
	.reg_bits = 8,
	.val_bits = 8,
	.max_register = LTR501_MAX_REG,
	.cache_type = REGCACHE_MAPLE,
	.volatile_reg = ltr501_is_volatile_reg,
	};

	regmap = devm_regmap_init_i2c(client, &ltr501_regmap_config);
	if (IS_ERR(regmap)) {
	dev_err(&client->dev, "Regmap initialization failed.\n");
	return PTR_ERR(regmap);
	}

	data->reg_it = devm_regmap_field_alloc(&client->dev, regmap,
	reg_field_it);
	if (IS_ERR(data->reg_it)) {
	dev_err(&client->dev, "Integ time reg field init failed.\n");
	return PTR_ERR(data->reg_it);
	}

	static ssize_t vcnl4000_read_near_level(struct iio_dev *indio_dev,
	uintptr_t priv,
	const struct iio_chan_spec *chan,
	char *buf)
	{
	struct vcnl4000_data *data = iio_priv(indio_dev);

	return sprintf(buf, "%u\n", data->near_level);
	}

	static const struct iio_chan_spec_ext_info vcnl4000_ext_info[] = {
	{
	.name = "nearlevel",
	.shared = IIO_SEPARATE,
	.read = vcnl4000_read_near_level,
	},
	{ }
	};

	static const struct iio_chan_spec vcnl4000_channels[] = {
	{
	.type = IIO_LIGHT,
	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) \|
	BIT(IIO_CHAN_INFO_SCALE),
	}, {
	.type = IIO_PROXIMITY,
	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
	.ext_info = vcnl4000_ext_info,
	}
	};


		data->enable_saved = data->enable;

		ret = iio_triggered_buffer_setup(indio_dev, NULL,

Conversation

playday3008 commented Apr 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wonderfulShrineMaidenOfParadise commented Apr 29, 2026

Uh oh!

Uh oh!

playday3008 commented Apr 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erikas9987 commented May 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

playday3008 commented May 11, 2026

Uh oh!

erikas9987 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erikas9987 commented May 11, 2026

Uh oh!

playday3008 commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants