From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0781C4321E for ; Wed, 30 Nov 2022 16:35:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230280AbiK3Qf4 (ORCPT ); Wed, 30 Nov 2022 11:35:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230114AbiK3Qfx (ORCPT ); Wed, 30 Nov 2022 11:35:53 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7A1064842D for ; Wed, 30 Nov 2022 08:35:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669826099; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ykv46HoHJsKjboHcMm93CTGhjBNnlsrLv9WEv8+JXlk=; b=AlAzIOXaR9ElqL2dvx7nEwZ3/UjyPFf2xlBYvzs5oHkOurget8N6R7VrpdYKkIrOCRBRLx Zm9J1GYUY9VP5pkCxfBpqG53m0WQ0lQ5LDelp1rDQTL0Fyh/Bu6vLe+HkgT+BtErbghk1J PMDuZHgm8GwQ8qIGhEBPPAelklHmfAs= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-363-BuDXJUesMliRm05qDdGB9w-1; Wed, 30 Nov 2022 11:34:58 -0500 X-MC-Unique: BuDXJUesMliRm05qDdGB9w-1 Received: by mail-ej1-f70.google.com with SMTP id qk16-20020a1709077f9000b007c080a6b4ddso4680952ejc.18 for ; Wed, 30 Nov 2022 08:34:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Ykv46HoHJsKjboHcMm93CTGhjBNnlsrLv9WEv8+JXlk=; b=wpM1ayaB9n3iNdSj16gCbMv2ghrHL9hkOEqDS1DHhfibRAU7tE509f2m4G2e3jcKph jzdeuf3H0WtTp2s042auSl7N6G1b7HapEs20gxBBljLi3lTphfLE6OnFm2OjVkKPvlfY Lp4oF/oF4Y8Vep9NqZq9fryVOur1FycaLxL+mASmIncbIBKB+YAS/pOONeOqp27Fmouu fWIOmmkEcD2xImQijmFUfvLPn+AatM/Br6GmTrx9mIkzjvl2e1nWrFR6DUmNzPf6EQ1W 2K72flOFozhleE7ixY1gEKoSFlbXL/l8eKxJL0EL6scFVfgRJpEbyrJax3i1Nfw2Wybh l2ZA== X-Gm-Message-State: ANoB5pmsg7b1mTEe6qBrMHP8E27D3wZtHDT77mu3vVJ5ZVg/mnOLwxIs +NntQFwtC3idA4S6zOmuYZHQgcMi/Bxuxg4plYtNMyiSXbFQy5IFxerWJaSMlJHykynpUpDVzFD ENQ9aFxYOwXBBd/ckPCOySVGXV4Qpg8uPhg== X-Received: by 2002:a17:906:490:b0:7c0:7efe:7ba4 with SMTP id f16-20020a170906049000b007c07efe7ba4mr10171762eja.521.1669826097188; Wed, 30 Nov 2022 08:34:57 -0800 (PST) X-Google-Smtp-Source: AA0mqf4avfUWAc3t4pAmJnNoo0hVhwRymTwXpXlMqoOhlkyWQeX/a1Z14ayB1E9utbEdwlsupIUeOA== X-Received: by 2002:a17:906:490:b0:7c0:7efe:7ba4 with SMTP id f16-20020a170906049000b007c07efe7ba4mr10171751eja.521.1669826096968; Wed, 30 Nov 2022 08:34:56 -0800 (PST) Received: from ?IPV6:2001:1c00:c1e:bf00:d69d:5353:dba5:ee81? (2001-1c00-0c1e-bf00-d69d-5353-dba5-ee81.cable.dynamic.v6.ziggo.nl. [2001:1c00:c1e:bf00:d69d:5353:dba5:ee81]) by smtp.gmail.com with ESMTPSA id x10-20020a1709060a4a00b007c073be0127sm797258ejf.202.2022.11.30.08.34.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 30 Nov 2022 08:34:56 -0800 (PST) Message-ID: Date: Wed, 30 Nov 2022 17:34:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH 1/6] media: ov5693: Add support for a privacy-led GPIO Content-Language: en-US, nl To: Sakari Ailus Cc: Mark Gross , Andy Shevchenko , Daniel Scally , Laurent Pinchart , platform-driver-x86@vger.kernel.org, Kate Hsuan , Mark Pearson , linux-media@vger.kernel.org References: <20221129231149.697154-1-hdegoede@redhat.com> <20221129231149.697154-2-hdegoede@redhat.com> From: Hans de Goede In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org Hi, On 11/30/22 15:52, Sakari Ailus wrote: > Hi Hans, > > On Wed, Nov 30, 2022 at 02:56:46PM +0100, Hans de Goede wrote: >> Hi Sakari, >> >> On 11/30/22 14:41, Sakari Ailus wrote: >>> Hi Hans, >>> >>> On Wed, Nov 30, 2022 at 12:11:44AM +0100, Hans de Goede wrote: >>>> Add support for a privacy-led GPIO. >>>> >>>> Making the privacy LED to controlable from userspace, as using the LED >>>> class subsystem would do, would make it too easy for spy-ware to disable >>>> the LED. >>>> >>>> To avoid this have the sensor driver directly control the LED. >>>> >>>> Signed-off-by: Hans de Goede >>>> --- >>>> Note an additional advantage of directly controlling the GPIO is that >>>> GPIOs are tied directly to consumer devices. Where as with a LED class >>>> device, there would need to be some mechanism to tie the right LED >>>> (e.g front or back) to the right sensor. >>> >>> Thanks for the patch. >>> >>> This approach has the drawback that support needs to be added for each >>> sensor separately. Any idea how many sensor drivers might need this? >> >> Quite a few probably. But as discussed here I plan to write a generic >> sensor_power helper library since many sensor drivers have a lot of >> boilerplate code to get clks + regulators + enable/reset gpios. The plan >> is to add support for a "privacy-led" to this library so that all sensors >> which use this get support for free. > > I'm not sure how well this could be generalised. While most sensors do > something similar there are subtle differences. If those can be taken into > account I guess it should be doable. But would it simplify things or reduce > the number of lines of code as a whole? > > The privacy LED is separate from sensor, including its power on/off > sequences which suggests it could be at least as well be handled > separately. > >> >> Laurent pointed out that some sensors may have more complex power-up >> sequence demands, which is true. But looking at existing drivers >> then many follow a std simple pattern which can be supported in >> a helper-library. >> >>> Most implementations have privacy LED hard-wired to the sensor's power >>> rails so it'll be lit whenever the sensor is powered on. >>> >>> If there would be more than just a couple of these I'd instead create a LED >>> class device and hook it up to the sensor in V4L2. >> >> >> A LED cladd device will allow userspace to override the privacy-led >> value which is considered bad from a privacy point of view. This >> was actually already discussed here: >> >> https://lore.kernel.org/platform-driver-x86/e5d8913c-13ba-3b11-94bc-5d1ee1d736b0@ideasonboard.com/ >> >> See the part of the thread on the cover-letter with Dan, Laurent >> and me participating. >> >> And a LED class device also will be a challenge to bind to the right >> sensor on devices with more then one sensor, where as mentioned >> above using GPIO-mappings give us the binding to the right sensor >> for free. > > Whether the privacy LED is controlled via the LED framework or GPIO doesn't > really matter from this PoV, it could be controlled via the V4L2 framework > in both cases. It might not be very pretty but I think I'd prefer that than > putting this in either drivers or some sensor power sequence helper > library. In sensors described in ACPI, esp. the straight forward described sensors on atomisp2 devices, the GPIO resources inluding the LED one are listed as resources of the i2c_client for the sensor. And in a sense the same applies to later IPU3 / IPU6 devices where there is a separate INT3472 device describing all the GPIOS which is also tied to a specific sensor and we currently map all the GPIOs from the INT3472 device to the sensor. So it looks like that at least for x86/ACPI windows devices if the LED has its own GPIO the hardware description clearly counts that as part of the sensor's GPIOs. So the sensor driver has direct access to this, where as any v4l2 framework driver would needed to start poking inside the fwnode of the sensor which really isn't pretty. Where as if you look at this patch set adding the privacy-LED GPIO from the INT3472 (IPU3 / IPU6) to the sensor fwnode is a 1 line change. This really by far is the most KISS solution and we have so much other things which need work that I believe that over-engineering this is not doing ourselves any favours. Regards, Hans