Hi Sakari, On Thu, Sep 03, 2020 at 11:15:44AM +0300, Sakari Ailus wrote: > > Hi all, > > These patches enable calling (and finishing) a driver's probe function > without powering on the respective device on busses where the practice is > to power on the device for probe. While it generally is a driver's job to > check the that the device is there, there are cases where it might be > undesirable. (In this case it stems from a combination of hardware design > and user expectations; see below.) The downside with this change is that > if there is something wrong with the device, it will only be found at the > time the device is used. In this case (the camera sensors + EEPROM in a > sensor) I don't see any tangible harm from that though. > > An indication both from the driver and the firmware is required to allow > the device's power state to remain off during probe (see the first patch). > > > The use case is such that there is a privacy LED next to an integrated > user-facing laptop camera, and this LED is there to signal the user that > the camera is recording a video or capturing images. That LED also happens > to be wired to one of the power supplies of the camera, so whenever you > power on the camera, the LED will be lit, whether images are captured from > the camera --- or not. There's no way to implement this differently > without additional software control (allowing of which is itself a > hardware design decision) on most CSI-2-connected camera sensors as they > simply have no pin to signal the camera streaming state. > > This is also what happens during driver probe: the camera will be powered > on by the I²C subsystem calling dev_pm_domain_attach() and the device is > already powered on when the driver's own probe function is called. To the > user this visible during the boot process as a blink of the privacy LED, > suggesting that the camera is recording without the user having used an > application to do that. From the end user's point of view the behaviour is > not expected and for someone unfamiliar with internal workings of a > computer surely seems quite suspicious --- even if images are not being > actually captured. > > I've tested these on linux-next master. They also apply to Wolfram's > i2c/for-next branch, there's a patch that affects the I²C core changes > here (see below). The patches apart from that apply to Bartosz's > at24/for-next as well as Mauro's linux-media master branch. Besides the suggestion to make the defintions added less specific to i2c (but still keeping the implementation so for now), feel free to add: Reviewed-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> Best regards, Tomasz
