Hi,
On 3/22/23 17:09, Daniel Scally wrote:
> Add LED lookup data to tps68470_board_data.c for the Microsoft
> Surface Go line of devices.
>
> Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> ---
> .../x86/intel/int3472/tps68470_board_data.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> index 0d46a238b630..e2c53319e112 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> @@ -157,10 +157,27 @@ static const struct tps68470_led_platform_data surface_go_tps68470_led_pdata = {
> .wledctl_disled2 = false,
> };
>
> +static struct tps68470_led_lookups surface_go_tps68470_led_lookups = {
> + .n_lookups = 2,
> + .lookup_table = {
> + {
> + .provider = "tps68470-iled_a::indicator",
> + .dev_id = "i2c-INT347A:00",
> + .con_id = "privacy-led",
> + },
> + {
> + .provider = "tps68470-wled::indicator",
> + .dev_id = "i2c-INT347E:00",
> + .con_id = "privacy-led",
> + },
So this feels wrong to me in 2 ways:
1. It is abusing .con_id = "privacy-led" to enable the WLED
2. You are not activating the front privacy LED for the IR projector. I have noticed on IPU6 devices that the _DSM listing GPIOs for the discrete INT3472 device lists a privacy-LED GPIO for the IR sensor too, which I so far have been guessing activates the actual privacy-LED. As I'm typing this I'm wondering if instead this is doing the same hack as you are doing here ?
Regardless I think it would be best to turn on the front privacy LED when the IR camera is used too, right ?
IMHO this should look like this (with either the media-core or the driver consuming "ir-led"):
static struct tps68470_led_lookups surface_go_tps68470_led_lookups = {
.n_lookups = 3,
.lookup_table = {
{
.provider = "tps68470-iled_a::indicator",
.dev_id = "i2c-INT347A:00",
.con_id = "privacy-led",
},
{
/* Use regular front-sensor privacy LED for ir-sensor too */
.provider = "INT33BE_00::privacy_led",
.dev_id = "i2c-INT347E:00",
.con_id = "privacy-led",
},
{
.provider = "tps68470-wled::indicator",
.dev_id = "i2c-INT347E:00",
.con_id = "ir-led",
},
}
Regards,
Hans
