Re: [PATCH] platform/x86: int3472: Add handshake GPIO function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hans,

Sorry for the late response.

The power control logic settings in ACPI table are the same between Windows OS and Linux, so I directly add another "handshake" pin handling to Linux power control driver as what Windows power control driver did.

On 2023/10/12 20:22, Hans de Goede wrote:
Hi,

On 10/7/23 04:12, Hao Yao wrote:
Handshake pin is used for Lattice MIPI aggregator to enable the
camera sensor. After pulled up, recommend to wail ~250ms to get
everything ready.
> If this is a pin on the "Lattice MIPI aggregator" and
not on the sensor itself then this really should be
modeled as such and should not be registered as a GPIO
consumed by the sensor since the actual sensor does not
have a handshake pin at all.


Yes. This pin is actually connects to Lattice, not sensor.

Also we really don't want to need to patch all involved
sensor drivers to toggle a handshake pin, especially since
the sensor itself does not physically have this pin.

I agree. Adding GPIO pin controlling code in all sensor drivers is not a good idea.

Can you explain a bit more:

1. What the "Lattice MIPI aggregator" is

It actually manages all MIPI lanes, both RGB and IR cameras. It is something like the iVSC + LJCA combination which are in kernel v6.6 mainline.

2. What its functions are, does this control reset + pwdn
    GPIOs for the sensor? Voltages to the sensor? Clk
    to the sensor ?

It starts outputing MIPI packages when we pull handshake pins high. It also has USB-IO function which can supply virtual I2C and GPIO for host to control the camera (not physically as actually Lattice handles them). I didn't get the schematics, but I believe the GPIOs/Voltages/clocks are all controlled by Lattice.

3. How the aggregator is connected to both the main
    CPU/SoC as well as how it is connected to the sensor ?
    Some example diagram would be really helpful here.

In this case Lattice stands between host SoC and camera sensors. All MIPI lanes from sensor and all reset/power pins are managed by Lattice. Once one of the "handshake" pins on Lattice is pulled high, Lattice start to passthrough MIPI packages from corresponding sensor to host SoC.

Then with this info in hand we can try to come up
with a way how to model this.

Assuming this controls the entire power-up sequence
for the sensor then I think it could be modelled
as a GPIO regulator. This also allows making the
regulator core take care of the necessary delay
between setting the GPIO and trying to talk to
the sensor.

Regards,

Hans




Signed-off-by: Hao Yao <hao.yao@xxxxxxxxx>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---
  drivers/platform/x86/intel/int3472/common.h   | 1 +
  drivers/platform/x86/intel/int3472/discrete.c | 5 +++++
  2 files changed, 6 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/common.h b/drivers/platform/x86/intel/int3472/common.h
index 655ae3ec0593..3ad4c72afb45 100644
--- a/drivers/platform/x86/intel/int3472/common.h
+++ b/drivers/platform/x86/intel/int3472/common.h
@@ -23,6 +23,7 @@
  #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
  #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
  #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
+#define INT3472_GPIO_TYPE_HANDSHAKE				0x12
#define INT3472_PDEV_MAX_NAME_LEN 23
  #define INT3472_MAX_SENSOR_GPIOS				3
diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index b644ce65c990..4753161b4080 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -111,6 +111,10 @@ static void int3472_get_func_and_polarity(u8 type, const char **func, u32 *polar
  		*func = "power-enable";
  		*polarity = GPIO_ACTIVE_HIGH;
  		break;
+	case INT3472_GPIO_TYPE_HANDSHAKE:
+		*func = "handshake";
+		*polarity = GPIO_ACTIVE_HIGH;
+		break;
  	default:
  		*func = "unknown";
  		*polarity = GPIO_ACTIVE_HIGH;
@@ -201,6 +205,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
  	switch (type) {
  	case INT3472_GPIO_TYPE_RESET:
  	case INT3472_GPIO_TYPE_POWERDOWN:
+	case INT3472_GPIO_TYPE_HANDSHAKE:
  		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, func, polarity);
  		if (ret)
  			err_msg = "Failed to map GPIO pin to sensor\n";


Best Regards,
Hao Yao




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux