Since commit a33e5e393171ae8384d3381db5cd159ba877cfcb ("HID: amd_sfh: Fix illuminance value"), the in_illuminance_raw is 0 all the time. Before that commit, the ACS/ALS has normal value.
(In reply to Kai-Heng Feng from comment #0) > Since commit a33e5e393171ae8384d3381db5cd159ba877cfcb ("HID: amd_sfh: Fix > illuminance value"), the in_illuminance_raw is 0 all the time. > > Before that commit, the ACS/ALS has normal value. On what device do you have this regression?
ALS and ACS are treated in similar codepaths, but they're actually different sensor types. "Ambient light sensor" and "Ambient color sensor". Which one has regressed? Given that commit you identified was fixing a truncation problem, it seems like it could have potentially been garbage data in the upper 16 bits rather than functionally working before this commit. Can you please share more details about how you confirmed this is a regression? Test data from before/after that matches light/color conditions in the room?
> ALS and ACS are treated in similar codepaths, but they're actually different > sensor types. "Ambient light sensor" and "Ambient color sensor". Which one > has regressed? According to the vendor, it's ACS. But kernel report it as ALS: [ 5.913345] pcie_mp2_amd 0000:c4:00.7: firmware version 0x90b2d1c [ 5.972739] pcie_mp2_amd 0000:c4:00.7: sid 0x4 (ALS) status 0x4 It was collected with the diff: --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -146,8 +146,9 @@ static const char *get_sensor_name(int idx) return "gyroscope"; case mag_idx: return "magnetometer"; - case als_idx: case ACS_IDX: /* ambient color sensor */ + return "ACS"; + case als_idx: return "ALS"; case HPD_IDX: return "HPD"; > Given that commit you identified was fixing a truncation problem, it seems > like it could have potentially been garbage data in the upper 16 bits rather > than functionally working before this commit. The value is not garbage, it goes up with brighter light and goes down if something is in front of the sensor. > Can you please share more details about how you confirmed this is a > regression? The `in_illuminance_raw` stays 0 regardless of the light condition. > Test data from before/after that matches light/color conditions in the room? Basically by moving my hand closer or away from the sensor. The value ranges from 0 ~ 3xx with normal indoor artificial light.
> According to the vendor, it's ACS. But kernel report it as ALS: [ 5.913345] pcie_mp2_amd 0000:c4:00.7: firmware version 0x90b2d1c [ 5.972739] pcie_mp2_amd 0000:c4:00.7: sid 0x4 (ALS) status 0x4 Please ask vendor to double check this. Even though Linux treats ACS and ALS the same if the wrong value is advertised in OEM firmware it might not be populating the structure exchanged between firmware and driver properly. > The value is not garbage, it goes up with brighter light and goes down if > something is in front of the sensor. > The `in_illuminance_raw` stays 0 regardless of the light condition. > > Test data from before/after that matches light/color conditions in the > room? OK. AMD's test ALS sensors (that are actually ALS) do work properly for this case.
AMD tested ACS sensors as well on reference hardware and they work too. This looks like it's a firmware bug for your platform.