Hi Andy, all

On 30/11/2020 20:07, Andy Shevchenko wrote:

On Mon, Nov 30, 2020 at 01:31:29PM +0000, Daniel Scally wrote:

quoted

On platforms where ACPI is designed for use with Windows, resources
that are intended to be consumed by sensor devices are sometimes in
the _CRS of a dummy INT3472 device upon which the sensor depends. This
driver binds to the dummy acpi device (which does not represent a

acpi device -> acpi_device

quoted

physical PMIC) and maps them into GPIO lines and regulators for use by
the sensor device instead.

...

quoted

This patch contains the bits of this process that we're least sure about.
The sensors in scope for this work are called out as dependent (in their
DSDT entry's _DEP) on a device with _HID INT3472. These come in at least
2 kinds; those with an I2cSerialBusV2 entry (which we presume therefore
are legitimate tps68470 PMICs that need handling by those drivers - work
on that in the future). And those without an I2C device. For those without
an I2C device they instead have an array of GPIO pins defined in _CRS. So
for example, my Lenovo Miix 510's OVTI2680 sensor is dependent on one of
the _latter_ kind of INT3472 devices, with this _CRS:

Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
{
    Name (SBUF, ResourceTemplate ()
    {
        GpioIo (Exclusive, PullDefault, 0x0000, 0x0000,
	    IoRestrictionOutputOnly, "\\_SB.PCI0.GPI0",
	    0x00, ResourceConsumer, ,
            )
            {   // Pin list
                0x0079
            }
        GpioIo (Exclusive, PullDefault, 0x0000, 0x0000,
	    IoRestrictionOutputOnly, "\\_SB.PCI0.GPI0",
	    0x00, ResourceConsumer, ,
            )
            {   // Pin list
                0x007A
            }
        GpioIo (Exclusive, PullDefault, 0x0000, 0x0000,
	    IoRestrictionOutputOnly, "\\_SB.PCI0.GPI0",
	    0x00, ResourceConsumer, ,
            )
            {   // Pin list
                0x008F
            }
    })
    Return (SBUF) /* \_SB_.PCI0.PMI1._CRS.SBUF */
}

and the same device has a _DSM Method, which returns 32-bit ints where
the second lowest byte we noticed to match the pin numbers of the GPIO
lines:

Method (_DSM, 4, NotSerialized)  // _DSM: Device-Specific Method
{
    If ((Arg0 == ToUUID ("79234640-9e10-4fea-a5c1-b5aa8b19756f")))
    {
        If ((Arg2 == One))
        {
            Return (0x03)
        }

        If ((Arg2 == 0x02))
        {
            Return (0x01007900)
        }

        If ((Arg2 == 0x03))
        {
            Return (0x01007A0C)
        }

        If ((Arg2 == 0x04))
        {
            Return (0x01008F01)
        }
    }

    Return (Zero)
}

We know that at least some of those pins have to be toggled active for the
sensor devices to be available in i2c, so the conclusion we came to was
that those GPIO entries assigned to the INT3472 device actually represent
GPIOs and regulators to be consumed by the sensors themselves. Tsuchiya
noticed that the lowest byte in the return values of the _DSM method
seemed to represent the type or function of the GPIO line, and we
confirmed that by testing on each surface device that GPIO lines where the
low byte in the _DSM entry for that pin was 0x0d controlled the privacy
LED of the cameras.

We're guessing as to the exact meaning of the function byte, but I
conclude they're something like this:

0x00 - probably a reset GPIO
0x01 - regulator for the sensor
0x0c - regulator for the sensor
0x0b - regulator again, but for a VCM or EEPROM
0x0d - privacy led (only one we're totally confident of since we can see
       it happen!)

It's solely Windows driver design...
Luckily I found some information and can clarify above table:

0x00 Reset
0x01 Power down
0x0b Power enable
0x0c Clock enable
0x0d LED (active high)

The above text perhaps should go somewhere under Documentation.

Coming back to this; there's a bit of an anomaly with the 0x01 Power
Down pin for at least one platform.  As listed above, the OV2680 on one
of my platforms has 3 GPIOs defined, and the table above gives them as
type Reset, Power down and Clock enable. I'd assumed from this table
that "power down" meant a powerdown GPIO (I.E. the one usually called
PWDNB in Omnivision datasheets and "powerdown" in drivers), but the
datasheet for the OV2680 doesn't list a separate reset and powerdown
pin, but rather a single pin that performs both functions.


Am I wrong to treat that as something that ought to be mapped as a
powerdown GPIO to the sensors? Or do you know of any other way to
reconcile that discrepancy?


Failing that; the only way I can think to handle this is to register
proxy GPIO pins assigned to the sensors as you suggested previously, and
have them toggle the GPIO's assigned to the INT3472 based on platform
specific mapping data (I.E. we register a pin called "reset", which on
most platforms just toggles the 0x00 pin, but on this specific platform
would drive both 0x00 and 0x01 together. We're already heading that way
for the regulator consumer supplies so it's sort of nothing new, but I'd
still rather not if it can be avoided.