2021-11-20 03:07:39

by Hector Martin

[permalink] [raw]
Subject: [PATCH RESEND 0/2] usb: typec: tipd: Fixes for Apple M1 (CD321X) support

Hi folks,

These two fixes make tipd work properly on Apple M1 devices, in
particular in the case where the bootloader hasn't initialized
the controllers yet.

We normally do it in m1n1 (so the machine can charge and so bootloaders
get working USB without needing this driver), but that was causing this
codepath to never get properly exercised, so we never caught it. I
noticed on the new machines with 3+1 ports, since m1n1 was only
initializing 2 and the other 2 were failing to initialize.

No changes since the previous version, just updating the commit tags.

Hector Martin (2):
usb: typec: tipd: Fix typo in cd321x_switch_power_state
usb: typec: tipd: Fix initialization sequence for cd321x

drivers/usb/typec/tipd/core.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)

--
2.33.0



2021-11-20 03:07:46

by Hector Martin

[permalink] [raw]
Subject: [PATCH RESEND 1/2] usb: typec: tipd: Fix typo in cd321x_switch_power_state

SPSS should've been SSPS.

Fixes: c9c14be664cf ("usb: typec: tipd: Switch CD321X power state to S0")
Reviewed-by: Heikki Krogerus <[email protected]>
Reviewed-by: Sven Peter <[email protected]>
Signed-off-by: Hector Martin <[email protected]>
---
drivers/usb/typec/tipd/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index fb8ef12bbe9c..4da5a0b2aed2 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -653,7 +653,7 @@ static int cd321x_switch_power_state(struct tps6598x *tps, u8 target_state)
if (state == target_state)
return 0;

- ret = tps6598x_exec_cmd(tps, "SPSS", sizeof(u8), &target_state, 0, NULL);
+ ret = tps6598x_exec_cmd(tps, "SSPS", sizeof(u8), &target_state, 0, NULL);
if (ret)
return ret;

--
2.33.0


2021-11-20 03:07:46

by Hector Martin

[permalink] [raw]
Subject: [PATCH RESEND 2/2] usb: typec: tipd: Fix initialization sequence for cd321x

The power state switch needs to happen first, as that
kickstarts the firmware into normal mode.

Fixes: c9c14be664cf ("usb: typec: tipd: Switch CD321X power state to S0")
Reviewed-by: Heikki Krogerus <[email protected]>
Reviewed-by: Sven Peter <[email protected]>
Signed-off-by: Hector Martin <[email protected]>
---
drivers/usb/typec/tipd/core.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 4da5a0b2aed2..6d27a5b5e3ca 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -707,6 +707,7 @@ static int tps6598x_probe(struct i2c_client *client)
u32 conf;
u32 vid;
int ret;
+ u64 mask1;

tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
if (!tps)
@@ -730,11 +731,6 @@ static int tps6598x_probe(struct i2c_client *client)
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
tps->i2c_protocol = true;

- /* Make sure the controller has application firmware running */
- ret = tps6598x_check_mode(tps);
- if (ret)
- return ret;
-
if (np && of_device_is_compatible(np, "apple,cd321x")) {
/* Switch CD321X chips to the correct system power state */
ret = cd321x_switch_power_state(tps, TPS_SYSTEM_POWER_STATE_S0);
@@ -742,24 +738,27 @@ static int tps6598x_probe(struct i2c_client *client)
return ret;

/* CD321X chips have all interrupts masked initially */
- ret = tps6598x_write64(tps, TPS_REG_INT_MASK1,
- APPLE_CD_REG_INT_POWER_STATUS_UPDATE |
- APPLE_CD_REG_INT_DATA_STATUS_UPDATE |
- APPLE_CD_REG_INT_PLUG_EVENT);
- if (ret)
- return ret;
+ mask1 = APPLE_CD_REG_INT_POWER_STATUS_UPDATE |
+ APPLE_CD_REG_INT_DATA_STATUS_UPDATE |
+ APPLE_CD_REG_INT_PLUG_EVENT;

irq_handler = cd321x_interrupt;
} else {
/* Enable power status, data status and plug event interrupts */
- ret = tps6598x_write64(tps, TPS_REG_INT_MASK1,
- TPS_REG_INT_POWER_STATUS_UPDATE |
- TPS_REG_INT_DATA_STATUS_UPDATE |
- TPS_REG_INT_PLUG_EVENT);
- if (ret)
- return ret;
+ mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
+ TPS_REG_INT_DATA_STATUS_UPDATE |
+ TPS_REG_INT_PLUG_EVENT;
}

+ /* Make sure the controller has application firmware running */
+ ret = tps6598x_check_mode(tps);
+ if (ret)
+ return ret;
+
+ ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, mask1);
+ if (ret)
+ return ret;
+
ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
if (ret < 0)
return ret;
--
2.33.0