Re: [PATCH v5 0/4] platform/x86: int3472: Add support for GPIO type 0x02 (strobe)

[Marco Nenciarini <mnencia@kcore.it>]

Hi Hans,

Thank you for the detailed feedback.

> My main remark on the current patch set is that IMHO
> the LED name really should be: OVTIxxxx:00::ir_flood_led
> since that is what it actually does, strobe is typically
> related to flash LEDs which despite the naming in Intel's
> side this is not.

The series actually started with "ir_flood" in v1-v2. It was renamed to
"strobe" in v3 to match the GPIO type name used in Intel's ACPI _DSM
tables. But I agree with you that the userspace-visible LED name should
describe what the hardware actually does, not mirror an internal ACPI
label. I am happy to go back to "ir_flood" for the LED name.

We could keep INT3472_GPIO_TYPE_STROBE for the define (matching the ACPI
type value) but pass "ir_flood" as the con_id to the LED registration,
so userspace sees OVTIxxxx:00::ir_flood_led. Would that work for
everyone?

> We really need to get some input from the V4L2 maintainers
> here on if this is a good idea, before merging this series.

Agreed.

> I can even imagine
> a default simple mode where the v4l2-core just turns
> on the LED when streaming starts and off again when
> streaming stops (very much like the privacy LED) and
> then in the future maybe we can extend this, e.g.
> add a control on the sensor device to make this
> configurable ?

That makes a lot of sense. The current series intentionally keeps things
minimal (just exposing the LED under /sys/class/leds with no V4L2
integration), but this future direction sounds right.

I will hold off on sending v6 until we have agreement on the naming and
Sakari has had a chance to weigh in.

Regards,
Marco