Re: [PATCH 1/3] platform/x86: int3472: Add TPS68470 board data for Dell 7320 Detachable

Sahan Nissanka <[email protected]>
Newsgroups org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media
Message-ID <[email protected]>
Please do not apply this patch. The GPIO mapping in it is wrong, and I would
rather say so than have it land.

Charles Drolet, who has the same machine, reported against my out-of-tree
repository [1] that the front sensor probes with no pin assignment at all.
That is why a wrong mapping is invisible here: the real reset line sits
released by default, so the camera comes up either way. Two specifics.

There is no powerdown pin. This part needs no hardware to confirm - ov5675.c
requests only "reset", via devm_gpiod_get_optional(dev, "reset", ...), and no
"powerdown" con_id exists anywhere in that driver. So

	GPIO_LOOKUP("tps68470-gpio", 4, "powerdown", GPIO_ACTIVE_LOW),

is never looked up. It is dead code, and my belief that the pin was needed was
unfounded.

The reset is on tps68470-gpio 5, not 3. Charles held the line with gpioset
against a build with no pin assignment, so nothing else had claimed it, and
reloaded the module: held low the sensor fails to identify with -EIO, released
it identifies correctly. That is consistent with an active-low reset on line
5. I have not reproduced this myself yet.

I should also correct how I arrived at 3 and 4. I took them from the Dell 7212
and 5290 board data as prior art, and treated "the camera works" as
confirmation. It is not. It confirms the sensor runs; it says nothing about
whether the mapping describes this board.

The rail mapping may need revisiting too. Charles has avdd on AUX1 at 2.8 V
where this patch has it on VSIO at 1.8 V, and the part wants roughly 2.8 V
analogue. Both configurations work, which suggests these rails may serve as
enables for regulators further down the line rather than as the supplies
themselves. I would rather resolve that than guess again.

The machine this work was done on was a loan and has gone back. A second 7320
Detachable arrives in the next few days, and I will verify the mapping on it
before sending a v2 - including checking whether the board data generalises
across units or merely described the one I had.

Patches 2/3 and 3/3 are not affected by any of this. Charles independently
arrived at the same ipu-bridge entry and the same ACPI id addition.

My thanks to Charles for finding this and for testing it properly.

[1] https://github.com/githomeserver/latitude-7320-camera/issues/1

--
Sahan Nissanka
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.