Re: [PATCH 1/2] dt-bindings: iio: light: Add ROHM BH1730FVC binding

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 11/05/2026 18:14, Jonathan Cameron wrote:
> On Mon, 11 May 2026 13:43:56 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> On 11/05/2026 11:22, Matti Vaittinen wrote:
>>> Thanks for patches Alexandre!
>>>
>>> It's nice to see these upstreamed :)
>>>
>>> On 10/05/2026 21:09, Alexandre Hamamdjian wrote:
>>>> From: CTCaer <ctcaer@gmail.com>
>>>>
>>>> Add a YAML binding for the ROHM BH1730FVC ambient light sensor.
>>>> Documents the required compatible string, the als-vdd/als-vid
>>>> regulators, and the rohm,integration-cycle, rohm,lux-multiplier,
>>>> rohm,opt-win-coeff and rohm,gain-coeff calibration properties
>>>> consumed by the driver.
>>
>> // snip
>>
>>>> +  rohm,opt-win-coeff:
>>>> +    description:
>>>> +      Optical-window calibration coefficients. Specified as a flat
>>>> list of
>>>> +      triplets <rc cv ci>, one triplet per window region, where rc is
>>>> the
>>>> +      visible/IR ratio cutoff and cv/ci are the visible and IR weighting
>>>> +      factors used in that region.
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
>>>> +    items:
>>>> +      minItems: 3
>>>> +      maxItems: 3
>>>
>>> I am not sure if I read the driver patch (2/2) correctly, but if I did,
>>> then these coefficients are used to compute Luxes out of the raw sensor
>>> data. I believe it would help anyone integrating (or investigating) this
>>> sensor, if you added the actual formula here as a comment. If I read
>>> this right, the formula is _somehting_ like:
>>>
>>>
>>> Lx = (cv[win] * ch0_data - ci[win] * ch1_data) / gain / int_time
>>>
>>> Here the cv[win] and ci[win] are selected from the opt-win-coeff -table,
>>> depending on the measured ch1_data/ch0_data ratio, right?
>>
>> One thing came to my mind. This 'window' -approach for lux calculation
>> is not too unique. For example the rohm-bu27034.c uses similar approach.
>>
>> The thing is that some of the sensors have more than 2 channels. (For
>> example, the first version of BU27034 did. [That was BU27034NUC, which
>> got cancelled when BU27034_A_NUC emerged]). These ICs may still may use
>> similar approach of having light regions, determined by ratio of (2)
>> channels. BUT, they may then have more than 2 coefficients / window.
>>
>> So, maybe this could be made generic enough so it could be re-used for
>> such devices if needed? I am not sure if other manufacturers but ROHM
>> does this in Lux computations - if yes, then it might be worth making
>> this more generic and not just a ROHM property? Maybe Jonathan has some
>> insight on other Lux computations.
> 
> It used to be very common to have multiple sensor / window setups for
> ambient light sensors - though perhaps less so on more modern devices
> (we have one on list today where they just say use the green channel
>   of an RGB sensor - so there are more windows but not relevant to
>   illuminance measurement).
> 
> Sometimes the window bit isn't well enough described in the datasheet
> so we only dealt with the parts on the actual sensor package and those
> were handled in driver rather than being in dt.
> 
> What I'm not sure on here is how much of what is being described
> is part of the 'chip' packaging - i.e. the bit that is constant for
> all instances of this device and how much is part of the wider
> device - i.e. the laptop / phone etc window infront of the sensor.
> 
> The chip bit we shouldn't have dt, the other part we should and it
> would indeed be interesting to work on a generalizing that description.

Hm. I kind of agree. The constant, 'common for all sensors' 
-coefficients should probably be hard-coded as sensor defaults. The 
BH1730 data-sheet seems to be describing a set of coefficients. I 
suppose that's what comes from the packaging(?)

And just to contradict myself...

I can imagine two reasons to alter these coefficients:

1.(st) being what Jonathan described. Eg. having something like a lens 
or a glass or whatever, on top of the sensor. That can alter the light 
entering the sensor, and require change to the coefficients. And indeed, 
it would make sense to only describe this in the DT, because the sensor 
is already described. That'd mean the DT should only contain the 
'coefficient delta' caused by <add the cause here>.

2. In theory, there could also be another reason. I bet the sensors are 
all 'individuals' to some extent. So, for something requiring very high 
accuracy, there could be some kind of calibration process at device 
manufacturing. This might produce more accurate, device specific 
coefficients. Considering this use-case, I am not 100% convinced it's 
"wrong" to be able to give the device-specific coefficients from the DT. 
(Because, here the coefficients really are a property of the device 
itself and aren't added by some external part).

And, if we consider allowing describing (the more accurate) device 
coefficients from the DT (for case 2), then it would just be simpler to 
always provide the "full coefficients" from the DT, no matter if they 
are caused by the device packaging or added lens/glass/XXX. And, as the 
infamous, and not even existing rohm,dh2228fv device shows, people do 
use the simplest solutions even when it isn't really right ;)

So, I am tempted to suggest we just go with the flow, and allow 
describing the coefficients from the DT, without separating the source, 
(lens/glass/device-packaging) which is what this patch does. (If I read 
it right.) However, I would like to see a property which can be re-used 
by other devices as this seems to be pretty common. Yeah, it's probably 
not _right_, but it feels practical.

Well, this is just my 0.5 cents, and even I may change my opinion on 
this though :)

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~