TPS25750 USB type-C PD controller has the same register offsets as
tps6598x. The following is a summary of incorporating TPS25750 into
TPS6598x driver:
- Only Check VID register (0x00) for TPS6598x and cd321x, as TPS25750 doesn't
have VID register.
- TypeC port registration will be registered differently for each PD
controller. TPS6598x uses system configuration register (0x28) to get
pr/dr capabilities. On the other hand, TPS25750 will use data role property
and PD status register (0x40) to get pr/dr capabilities as TPS25750 doesn't
have register 0x28 supported.
- TPS25750 requires writing a binary configuration to switch PD
controller from PTCH mode to APP mode which needs the following changes:
- Add PTCH mode to the modes list.
- Add an argument to tps6598x_check_mode to return the current mode.
- Currently, tps6598x_exec_cmd has cmd timeout hardcoded to 1 second,
and doesn't wait before checking DATA_OUT response. In TPS25750, patch 4CCs
take longer than 1 second to execute and some requires a delay before
checking DATA_OUT. To accommodate that, cmd_timeout and response_delay will
be added as arguments to tps6598x_exec_cmd.
- Implement applying patch sequence for TPS25750.
- In pm suspend callback, patch mode needs to be checked and the binary
configuration should be applied if needed.
- For interrupt, TPS25750 has only one event register (0x14) and one mask
register (0x16) of 11 bytes each, where TPS6598x has two event
and two mask registers of 8 bytes each. Both TPS25750 and TPS65986x
shares the same bit field offsets for events/masks/clear but many of
there fields are reserved in TPS25750, the following needs to be done in
tps6598x_interrupt:
- Read EVENT1 register as a block of 11 bytes when tps25750 is present
- Write CLEAR1 register as a block of 11 bytes when tps25750 is present
- Add trace_tps25750_irq
- During testing, I noticed that when a cable is plugged into the PD
controller and before PD controller switches to APP mode, there is a
lag between dr/pr updates and PlugInsertOrRemoval Event, so a check
for dr/pr change needs to be added along TPS_REG_INT_PLUG_EVENT check
- Add TPS25750 traces for status and power status registers. Trace for
data register won't be added as it doesn't exist in the device.
- Configure sleep mode for TPS25750.
v9:
- PATCH 1:
- Add Reviewed-by
- PATCH 2..7:
- No changes
- PATCH 8:
- Move of_device_id to its original place
- Move device data structs to the top of of_device_id
- Use device_get_match_data to get device data
- PATCH 9:
- No changes
- PATCH 10:
- Move device data structs to the top of of_device_id
- PATCH 11,12:
- No changes
- PATCH 13,14:
- Move device data structs to the top of of_device_id
v8:
- PATCH 1:
- Define reg-names at top-level
- PATCH 2:
- Add Reviewed-by
- PATCH 3:
- Revert mode check return
- PATCH 4:
- Return mode when mode is checked
- Use device_is_compatible instead of of_device_is_compatible
- PATCH 5,6:
- No changes
- PATCH 7:
- Use device_is_compatible instead of of_device_is_compatible
- PATCH 8,9:
- No changes
- PATCH 10:
- Change tps->cb to tps->data
- PATCH 11,12:
- No changes
- PATCH 13,14:
- Change tps->cb to tps->data
v7:
- PATCH 1:
- Define reg at top-level
- Remove description from reg-names
- PATCH 2..7: Add tps6598x to the subject
- PATCH 8:
- Add tps6598x to the subject
- Create tps25750 interrupt handler
- PATCH 9..11: Add tps6598x to the subject
- PATCH 12:
- Add driver name to commit subject
- Call trace_tps25750_irq directly from tps25750 interrupt
handler
- PATCH 13-14: Add tps6598x to the subject
v6:
- PATCH 1: Use reg property for patch address
- PATCH 2: Use tps6598x_exec_cmd as a wrapper
- PATCH 3: Return current mode and check it directly
- PATCH 4:
- Don't check VID for tps25750 as the VID register doesn't exist
- Remove is_tps25750 flag from tps6598x struct
- Get patch address from reg property
- PATCH 5: Update eeprom macro to use TPS instead
- PATCH 6: No changes
- PATCH 7: Check tps25750 using is_compatiable device node
- PATCH 8: Create tipd callbacks factory
- PATCH 9: No changes
- PATCH 10: Add port registration to tipd data factory
- PATCH 11: Use tps25750_init instead of tps25750_apply_patch in resume
as it initializes sleep mode
- PATCH 12: Add trace irq to tipd callbacks factory
- PATCH 13: Add trace power status to tipd data factory
- PATCH 14: Add trace status to tipd data factory
v5:
- PATCH 1: Add tps25750 bindings to tps6598x
- PATCH 2: Remove tps25750 driver and incorperate tps25750
into tps6598x driver
- PATCH [3..15]: Incorporating tps25750 into tps6598x driver
v4:
- PATCH 1: No change
- PATCH 2: Fix comments style and drop of_match_ptr
v3:
- PATCH 1: Fix node name
- PATCH 2: Upload tps25750 driver patch
v2:
- PATCH 1: General properties clean up
Abdel Alkuor (14):
dt-bindings: usb: tps6598x: Add tps25750
USB: typec: tsp6598x: Add cmd timeout and response delay
USB: typec: tps6598x: Add patch mode to tps6598x
USB: typec: tps6598x: Load TPS25750 patch bundle
USB: typec: tps6598x: Check for EEPROM present
USB: typec: tps6598x: Clear dead battery flag
USB: typec: tps6598x: Apply patch again after power resume
USB: typec: tps6598x: Add interrupt support for TPS25750
USB: typec: tps6598x: Refactor tps6598x port registration
USB: typec: tps6598x: Add port registration for tps25750
USB: typec: tps6598x: Enable sleep mode for tps25750
USB: typec: tps6598x: Add trace for tps25750 irq
USB: typec: tps6598x: Add power status trace for tps25750
USB: typec: tps6598x: Add status trace for tps25750
.../devicetree/bindings/usb/ti,tps6598x.yaml | 81 ++-
drivers/usb/typec/tipd/core.c | 624 ++++++++++++++++--
drivers/usb/typec/tipd/tps6598x.h | 36 +
drivers/usb/typec/tipd/trace.h | 92 +++
4 files changed, 762 insertions(+), 71 deletions(-)
--
2.34.1
From: Abdel Alkuor <[email protected]>
tps25750 status register is a subset of tps6598x status register, hence
a trace for tps25750 status register is added.
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- Move device data structs to the top of of_device_id
Changes in v8:
- Change tps->cb to tps->data
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Add trace status to tipd data factory
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 12 +++++++----
drivers/usb/typec/tipd/trace.h | 37 ++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index a114e786b1a7..4f2bfa19d2f5 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -110,6 +110,7 @@ struct tipd_data {
irq_handler_t irq_handler;
int (*register_port)(struct tps6598x *tps, struct fwnode_handle *node);
void (*trace_power_status)(u16 status);
+ void (*trace_status)(u32 status);
};
struct tps6598x {
@@ -469,7 +470,9 @@ static bool tps6598x_read_status(struct tps6598x *tps, u32 *status)
dev_err(tps->dev, "%s: failed to read status\n", __func__);
return false;
}
- trace_tps6598x_status(*status);
+
+ if (tps->data->trace_status)
+ tps->data->trace_status(*status);
return true;
}
@@ -1243,10 +1246,8 @@ static int tps6598x_probe(struct i2c_client *client)
if (ret)
goto err_reset_controller;
- ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
- if (ret < 0)
+ if (!tps6598x_read_status(tps, &status))
goto err_clear_mask;
- trace_tps6598x_status(status);
/*
* This fwnode has a "compatible" property, but is never populated as a
@@ -1396,18 +1397,21 @@ static const struct tipd_data cd321x_data = {
.irq_handler = cd321x_interrupt,
.register_port = tps6598x_register_port,
.trace_power_status = trace_tps6598x_power_status,
+ .trace_status = trace_tps6598x_status,
};
static const struct tipd_data tps6598x_data = {
.irq_handler = tps6598x_interrupt,
.register_port = tps6598x_register_port,
.trace_power_status = trace_tps6598x_power_status,
+ .trace_status = trace_tps6598x_status,
};
static const struct tipd_data tps25750_data = {
.irq_handler = tps25750_interrupt,
.register_port = tps25750_register_port,
.trace_power_status = trace_tps25750_power_status,
+ .trace_status = trace_tps25750_status,
};
static const struct of_device_id tps6598x_of_match[] = {
diff --git a/drivers/usb/typec/tipd/trace.h b/drivers/usb/typec/tipd/trace.h
index 739b0a2a867d..0669cca12ea1 100644
--- a/drivers/usb/typec/tipd/trace.h
+++ b/drivers/usb/typec/tipd/trace.h
@@ -91,6 +91,14 @@
TPS_STATUS_USB_HOST_PRESENT_MASK | \
TPS_STATUS_LEGACY_MASK))
+#define TPS25750_STATUS_FLAGS_MASK (GENMASK(31, 0) ^ (TPS_STATUS_CONN_STATE_MASK | \
+ GENMASK(19, 7) | \
+ TPS_STATUS_VBUS_STATUS_MASK | \
+ TPS_STATUS_USB_HOST_PRESENT_MASK | \
+ TPS_STATUS_LEGACY_MASK | \
+ BIT(26) | \
+ GENMASK(31, 28)))
+
#define show_status_conn_state(status) \
__print_symbolic(TPS_STATUS_CONN_STATE((status)), \
{ TPS_STATUS_CONN_STATE_CONN_WITH_R_A, "conn-Ra" }, \
@@ -148,6 +156,14 @@
{ TPS_STATUS_HIGH_VOLAGE_WARNING, "HIGH_VOLAGE_WARNING" }, \
{ TPS_STATUS_HIGH_LOW_VOLTAGE_WARNING, "HIGH_LOW_VOLTAGE_WARNING" })
+#define show_tps25750_status_flags(flags) \
+ __print_flags((flags & TPS25750_STATUS_FLAGS_MASK), "|", \
+ { TPS_STATUS_PLUG_PRESENT, "PLUG_PRESENT" }, \
+ { TPS_STATUS_PLUG_UPSIDE_DOWN, "UPSIDE_DOWN" }, \
+ { TPS_STATUS_PORTROLE, "PORTROLE" }, \
+ { TPS_STATUS_DATAROLE, "DATAROLE" }, \
+ { TPS_STATUS_BIST, "BIST" })
+
#define show_power_status_source_sink(power_status) \
__print_symbolic(TPS_POWER_STATUS_SOURCESINK(power_status), \
{ 1, "sink" }, \
@@ -292,6 +308,27 @@ TRACE_EVENT(tps6598x_status,
)
);
+TRACE_EVENT(tps25750_status,
+ TP_PROTO(u32 status),
+ TP_ARGS(status),
+
+ TP_STRUCT__entry(
+ __field(u32, status)
+ ),
+
+ TP_fast_assign(
+ __entry->status = status;
+ ),
+
+ TP_printk("conn: %s, vbus: %s, usb-host: %s, legacy: %s, flags: %s",
+ show_status_conn_state(__entry->status),
+ show_status_vbus_status(__entry->status),
+ show_status_usb_host_present(__entry->status),
+ show_status_legacy(__entry->status),
+ show_tps25750_status_flags(__entry->status)
+ )
+);
+
TRACE_EVENT(tps6598x_power_status,
TP_PROTO(u16 power_status),
TP_ARGS(power_status),
--
2.34.1
From: Abdel Alkuor <[email protected]>
TPS25750 PD controller might be powered off externally at power suspend,
after resuming PD controller power back, apply the patch again.
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- Use device_is_compatible instead of of_device_is_compatible
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Check tps25750 using is_compatiable device node
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 2598433a69cf..32e42798688f 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -1203,6 +1203,17 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct tps6598x *tps = i2c_get_clientdata(client);
+ int ret;
+
+ ret = tps6598x_check_mode(tps);
+ if (ret < 0)
+ return ret;
+
+ if (device_is_compatible(tps->dev, "ti,tps25750") && ret == TPS_MODE_PTCH) {
+ ret = tps25750_apply_patch(tps);
+ if (ret)
+ return ret;
+ }
if (tps->wakeup) {
disable_irq_wake(client->irq);
--
2.34.1
From: Abdel Alkuor <[email protected]>
Dead battery flag must be cleared after switching tps25750 to APP mode
so the PD controller becomes fully functional.
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- No changes
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- No changes
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 16 ++++++++++++++++
drivers/usb/typec/tipd/tps6598x.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 21b0ea2c9627..2598433a69cf 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -946,6 +946,22 @@ static int tps25750_apply_patch(struct tps6598x *tps)
} while (ret != TPS_MODE_APP);
+ /*
+ * The dead battery flag may be triggered when the controller
+ * port is connected to a device that can source power and
+ * attempts to power up both the controller and the board it is on.
+ * To restore controller functionality, it is necessary to clear
+ * this flag
+ */
+ if (status & TPS_BOOT_STATUS_DEAD_BATTERY_FLAG) {
+ ret = tps6598x_exec_cmd(tps, "DBfg", 0, NULL, 0, NULL);
+ if (ret) {
+ dev_err(tps->dev,
+ "failed to clear dead battery %d\n", ret);
+ return ret;
+ }
+ }
+
dev_info(tps->dev, "controller switched to \"APP\" mode\n");
return 0;
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index a80d0929f3ee..c000170f4547 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -200,6 +200,7 @@
#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
/* BOOT STATUS REG*/
+#define TPS_BOOT_STATUS_DEAD_BATTERY_FLAG BIT(2)
#define TPS_BOOT_STATUS_I2C_EEPROM_PRESENT BIT(3)
#endif /* __TPS6598X_H__ */
--
2.34.1
From: Abdel Alkuor <[email protected]>
TPS25750 controller requires a binary to be loaded with a configuration
binary by an EEPROM or a host.
Appling a patch bundling using a host is implemented based on the flow
diagram pg.62 in TPS25750 host interface manual.
https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
The flow diagram can be summarized as following:
- Start the patch loading sequence with patch bundle information by
executing PBMs
- Write the whole patch at once
- When writing the patch fails, execute PBMe which instructs the PD controller
to end the patching process
- After writing the patch successfully, execute PBMc which verifies the patch
integrity and applies the patch internally
- Wait for the device to switch into APP mode (normal operation)
The execuation flow diagram polls the events register and then polls the
corresponding register related to the event as well before advancing to the next
state. Polling the events register is a redundant step, in this implementation
only the corresponding register related to the event is polled.
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- Return mode when mode is checked
- Use device_is_compatible instead of of_device_is_compatible
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Don't check VID for tps25750 as the VID register doesn't exist
- Remove is_tps25750 flag from tps6598x struct
- Get patch address from reg property
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 263 ++++++++++++++++++++++++++++++++--
1 file changed, 255 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index c5bbf03cb74a..2e7b9eafaf04 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -17,6 +17,7 @@
#include <linux/usb/typec_altmode.h>
#include <linux/usb/role.h>
#include <linux/workqueue.h>
+#include <linux/firmware.h>
#include "tps6598x.h"
#include "trace.h"
@@ -43,6 +44,23 @@
/* TPS_REG_SYSTEM_CONF bits */
#define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
+/*
+ * BPMs task timeout, recommended 5 seconds
+ * pg.48 TPS2575 Host Interface Technical Reference
+ * Manual (Rev. A)
+ * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
+ */
+#define TPS_BUNDLE_TIMEOUT 0x32
+
+/* BPMs return code */
+#define TPS_TASK_BPMS_INVALID_BUNDLE_SIZE 0x4
+#define TPS_TASK_BPMS_INVALID_SLAVE_ADDR 0x5
+#define TPS_TASK_BPMS_INVALID_TIMEOUT 0x6
+
+/* PBMc data out */
+#define TPS_PBMC_RC 0 /* Return code */
+#define TPS_PBMC_DPCS 2 /* device patch complete status */
+
enum {
TPS_PORTINFO_SINK,
TPS_PORTINFO_SINK_ACCESSORY,
@@ -595,13 +613,15 @@ static int tps6598x_check_mode(struct tps6598x *tps)
if (ret)
return ret;
- switch (match_string(modes, ARRAY_SIZE(modes), mode)) {
+ ret = match_string(modes, ARRAY_SIZE(modes), mode);
+
+ switch (ret) {
case TPS_MODE_APP:
case TPS_MODE_PTCH:
- return 0;
+ return ret;
case TPS_MODE_BOOT:
dev_warn(tps->dev, "dead-battery condition\n");
- return 0;
+ return ret;
case TPS_MODE_BIST:
case TPS_MODE_DISC:
default:
@@ -711,6 +731,213 @@ static int devm_tps6598_psy_register(struct tps6598x *tps)
return PTR_ERR_OR_ZERO(tps->psy);
}
+static int
+tps25750_write_firmware(struct tps6598x *tps,
+ u8 bpms_addr, const u8 *data, size_t len)
+{
+ struct i2c_client *client = to_i2c_client(tps->dev);
+ int ret;
+ u8 slave_addr;
+ int timeout;
+
+ slave_addr = client->addr;
+ timeout = client->adapter->timeout;
+
+ /*
+ * binary configuration size is around ~16Kbytes
+ * which might take some time to finish writing it
+ */
+ client->adapter->timeout = msecs_to_jiffies(5000);
+ client->addr = bpms_addr;
+
+ ret = regmap_raw_write(tps->regmap, data[0], &data[1], len - 1);
+
+ client->addr = slave_addr;
+ client->adapter->timeout = timeout;
+
+ return ret;
+}
+
+static int
+tps25750_exec_pbms(struct tps6598x *tps, u8 *in_data, size_t in_len)
+{
+ int ret;
+ u8 rc;
+
+ ret = tps6598x_exec_cmd_tmo(tps, "PBMs", in_len, in_data,
+ sizeof(rc), &rc, 4000, 0);
+ if (ret)
+ return ret;
+
+ switch (rc) {
+ case TPS_TASK_BPMS_INVALID_BUNDLE_SIZE:
+ dev_err(tps->dev, "%s: invalid fw size\n", __func__);
+ return -EINVAL;
+ case TPS_TASK_BPMS_INVALID_SLAVE_ADDR:
+ dev_err(tps->dev, "%s: invalid slave address\n", __func__);
+ return -EINVAL;
+ case TPS_TASK_BPMS_INVALID_TIMEOUT:
+ dev_err(tps->dev, "%s: timed out\n", __func__);
+ return -ETIMEDOUT;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int tps25750_abort_patch_process(struct tps6598x *tps)
+{
+ int ret;
+
+ ret = tps6598x_exec_cmd(tps, "PBMe", 0, NULL, 0, NULL);
+ if (ret)
+ return ret;
+
+ ret = tps6598x_check_mode(tps);
+ if (ret != TPS_MODE_PTCH)
+ dev_err(tps->dev, "failed to switch to \"PTCH\" mode\n");
+
+ return ret;
+}
+
+static int tps25750_start_patch_burst_mode(struct tps6598x *tps)
+{
+ int ret;
+ const struct firmware *fw;
+ const char *firmware_name;
+ struct {
+ u32 fw_size;
+ u8 addr;
+ u8 timeout;
+ } __packed bpms_data;
+ u32 addr;
+ struct device_node *np = tps->dev->of_node;
+
+ ret = device_property_read_string(tps->dev, "firmware-name",
+ &firmware_name);
+ if (ret)
+ return ret;
+
+ ret = request_firmware(&fw, firmware_name, tps->dev);
+ if (ret) {
+ dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name);
+ return ret;
+ }
+
+ if (fw->size == 0) {
+ ret = -EINVAL;
+ goto release_fw;
+ }
+
+ ret = of_property_match_string(np, "reg-names", "patch-address");
+ if (ret < 0) {
+ dev_err(tps->dev, "failed to get patch-address %d\n", ret);
+ return ret;
+ }
+
+ ret = of_property_read_u32_index(np, "reg", ret, &addr);
+ if (ret)
+ return ret;
+
+ if (addr == 0 || (addr >= 0x20 && addr <= 0x23)) {
+ dev_err(tps->dev, "wrong patch address %u\n", addr);
+ return -EINVAL;
+ }
+
+ bpms_data.addr = (u8)addr;
+ bpms_data.fw_size = fw->size;
+ bpms_data.timeout = TPS_BUNDLE_TIMEOUT;
+
+ ret = tps25750_exec_pbms(tps, (u8 *)&bpms_data, sizeof(bpms_data));
+ if (ret)
+ goto release_fw;
+
+ ret = tps25750_write_firmware(tps, bpms_data.addr, fw->data, fw->size);
+ if (ret) {
+ dev_err(tps->dev, "Failed to write patch %s of %zu bytes\n",
+ firmware_name, fw->size);
+ goto release_fw;
+ }
+
+ /*
+ * A delay of 500us is required after the firmware is written
+ * based on pg.62 in tps6598x Host Interface Technical
+ * Reference Manual
+ * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
+ */
+ udelay(500);
+
+release_fw:
+ release_firmware(fw);
+
+ return ret;
+}
+
+static int tps25750_complete_patch_process(struct tps6598x *tps)
+{
+ int ret;
+ u8 out_data[40];
+ u8 dummy[2] = { };
+
+ /*
+ * Without writing something to DATA_IN, this command would
+ * return an error
+ */
+ ret = tps6598x_exec_cmd_tmo(tps, "PBMc", sizeof(dummy), dummy,
+ sizeof(out_data), out_data, 2000, 20);
+ if (ret)
+ return ret;
+
+ if (out_data[TPS_PBMC_RC]) {
+ dev_err(tps->dev,
+ "%s: pbmc failed: %u\n", __func__,
+ out_data[TPS_PBMC_RC]);
+ return -EIO;
+ }
+
+ if (out_data[TPS_PBMC_DPCS]) {
+ dev_err(tps->dev,
+ "%s: failed device patch complete status: %u\n",
+ __func__, out_data[TPS_PBMC_DPCS]);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int tps25750_apply_patch(struct tps6598x *tps)
+{
+ int ret;
+ unsigned long timeout;
+
+ ret = tps25750_start_patch_burst_mode(tps);
+ if (ret) {
+ tps25750_abort_patch_process(tps);
+ return ret;
+ }
+
+ ret = tps25750_complete_patch_process(tps);
+ if (ret)
+ return ret;
+
+ timeout = jiffies + msecs_to_jiffies(1000);
+
+ do {
+ ret = tps6598x_check_mode(tps);
+ if (ret < 0)
+ return ret;
+
+ if (time_is_before_jiffies(timeout))
+ return -ETIMEDOUT;
+
+ } while (ret != TPS_MODE_APP);
+
+ dev_info(tps->dev, "controller switched to \"APP\" mode\n");
+
+ return 0;
+};
+
static int tps6598x_probe(struct i2c_client *client)
{
irq_handler_t irq_handler = tps6598x_interrupt;
@@ -723,6 +950,7 @@ static int tps6598x_probe(struct i2c_client *client)
u32 vid;
int ret;
u64 mask1;
+ bool is_tps25750;
tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
if (!tps)
@@ -735,9 +963,12 @@ static int tps6598x_probe(struct i2c_client *client)
if (IS_ERR(tps->regmap))
return PTR_ERR(tps->regmap);
- ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
- if (ret < 0 || !vid)
- return -ENODEV;
+ is_tps25750 = device_is_compatible(tps->dev, "ti,tps25750");
+ if (!is_tps25750) {
+ ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
+ if (ret < 0 || !vid)
+ return -ENODEV;
+ }
/*
* Checking can the adapter handle SMBus protocol. If it can not, the
@@ -768,12 +999,18 @@ static int tps6598x_probe(struct i2c_client *client)
tps->irq_handler = irq_handler;
/* Make sure the controller has application firmware running */
ret = tps6598x_check_mode(tps);
- if (ret)
+ if (ret < 0)
return ret;
+ if (is_tps25750 && ret == TPS_MODE_PTCH) {
+ ret = tps25750_apply_patch(tps);
+ if (ret)
+ return ret;
+ }
+
ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, mask1);
if (ret)
- return ret;
+ goto err_reset_controller;
ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
if (ret < 0)
@@ -893,6 +1130,10 @@ static int tps6598x_probe(struct i2c_client *client)
fwnode_handle_put(fwnode);
err_clear_mask:
tps6598x_write64(tps, TPS_REG_INT_MASK1, 0);
+err_reset_controller:
+ /* Reset PD controller to remove any applied patch */
+ if (is_tps25750)
+ tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
return ret;
}
@@ -903,9 +1144,14 @@ static void tps6598x_remove(struct i2c_client *client)
if (!client->irq)
cancel_delayed_work_sync(&tps->wq_poll);
+ devm_free_irq(tps->dev, client->irq, tps);
tps6598x_disconnect(tps, 0);
typec_unregister_port(tps->port);
usb_role_switch_put(tps->role_sw);
+
+ /* Reset PD controller to remove any applied patch */
+ if (device_is_compatible(tps->dev, "ti,tps25750"))
+ tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
}
static int __maybe_unused tps6598x_suspend(struct device *dev)
@@ -948,6 +1194,7 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
static const struct of_device_id tps6598x_of_match[] = {
{ .compatible = "ti,tps6598x", },
{ .compatible = "apple,cd321x", },
+ { .compatible = "ti,tps25750", },
{}
};
MODULE_DEVICE_TABLE(of, tps6598x_of_match);
--
2.34.1
From: Abdel Alkuor <[email protected]>
Some commands in tps25750 take longer than 1 second
to complete, and some responses need some delay before
the result becomes available.
Signed-off-by: Abdel Alkuor <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- Add Reviewed-by
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Use tps6598x_exec_cmd as a wrapper
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 37b56ce75f39..32420c61660d 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -282,9 +282,10 @@ static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
power_supply_changed(tps->psy);
}
-static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
+static int tps6598x_exec_cmd_tmo(struct tps6598x *tps, const char *cmd,
size_t in_len, u8 *in_data,
- size_t out_len, u8 *out_data)
+ size_t out_len, u8 *out_data,
+ u32 cmd_timeout_ms, u32 res_delay_ms)
{
unsigned long timeout;
u32 val;
@@ -307,8 +308,7 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
if (ret < 0)
return ret;
- /* XXX: Using 1s for now, but it may not be enough for every command. */
- timeout = jiffies + msecs_to_jiffies(1000);
+ timeout = jiffies + msecs_to_jiffies(cmd_timeout_ms);
do {
ret = tps6598x_read32(tps, TPS_REG_CMD1, &val);
@@ -321,6 +321,9 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
return -ETIMEDOUT;
} while (val);
+ /* some commands require delay for the result to be available */
+ mdelay(res_delay_ms);
+
if (out_len) {
ret = tps6598x_block_read(tps, TPS_REG_DATA1,
out_data, out_len);
@@ -345,6 +348,14 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
return 0;
}
+static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
+ size_t in_len, u8 *in_data,
+ size_t out_len, u8 *out_data)
+{
+ return tps6598x_exec_cmd_tmo(tps, cmd, in_len, in_data,
+ out_len, out_data, 1000, 0);
+}
+
static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
{
const char *cmd = (role == TYPEC_DEVICE) ? "SWUF" : "SWDF";
--
2.34.1
From: Abdel Alkuor <[email protected]>
tps25750 event registers structure is different than tps6598x's,
tps25750 has 11 bytes of events which are read at once where
tps6598x has two event registers of 8 bytes each which are read
separately. Likewise MASK event registers. Also, not all events
are supported in both devices.
- Create a new handler to accommodate tps25750 interrupt
- Add device data to of_device_id
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- Move of_device_id to its original place
- Move device data structs to the top of of_device_id
- Use device_get_match_data to get device data
Changes in v8:
- Populate of_device_id with device data
- Change tps->cb to tps->data
- Assign matched data to tps data
Changes in v7:
- Add driver name to commit subject
- Create tps25750 interrupt handler
Changes in v6:
- Create tipd callbacks factory
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 96 +++++++++++++++++++++++++++++++----
1 file changed, 87 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 32e42798688f..52dc1cc16bed 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -18,6 +18,7 @@
#include <linux/usb/role.h>
#include <linux/workqueue.h>
#include <linux/firmware.h>
+#include <linux/of_device.h>
#include "tps6598x.h"
#include "trace.h"
@@ -101,6 +102,10 @@ static const char *const modes[] = {
/* Unrecognized commands will be replaced with "!CMD" */
#define INVALID_CMD(_cmd_) (_cmd_ == 0x444d4321)
+struct tipd_data {
+ irq_handler_t irq_handler;
+};
+
struct tps6598x {
struct device *dev;
struct regmap *regmap;
@@ -118,9 +123,11 @@ struct tps6598x {
enum power_supply_usb_type usb_type;
int wakeup;
+ u32 status; /* status reg */
u16 pwr_status;
struct delayed_work wq_poll;
- irq_handler_t irq_handler;
+
+ const struct tipd_data *data;
};
static enum power_supply_property tps6598x_psy_props[] = {
@@ -545,6 +552,64 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
return IRQ_NONE;
}
+static bool tps6598x_has_role_changed(struct tps6598x *tps, u32 status)
+{
+ status ^= tps->status;
+
+ return status & (TPS_STATUS_PORTROLE | TPS_STATUS_DATAROLE);
+}
+
+static irqreturn_t tps25750_interrupt(int irq, void *data)
+{
+ struct tps6598x *tps = data;
+ u64 event[2] = { };
+ u32 status;
+ int ret;
+
+ mutex_lock(&tps->lock);
+
+ ret = tps6598x_block_read(tps, TPS_REG_INT_EVENT1, event, 11);
+ if (ret) {
+ dev_err(tps->dev, "%s: failed to read events\n", __func__);
+ goto err_unlock;
+ }
+
+ if (!(event[0] | event[1]))
+ goto err_unlock;
+
+ if (!tps6598x_read_status(tps, &status))
+ goto err_clear_ints;
+
+ if ((event[0] | event[1]) & TPS_REG_INT_POWER_STATUS_UPDATE)
+ if (!tps6598x_read_power_status(tps))
+ goto err_clear_ints;
+
+ if ((event[0] | event[1]) & TPS_REG_INT_DATA_STATUS_UPDATE)
+ if (!tps6598x_read_data_status(tps))
+ goto err_clear_ints;
+
+ /*
+ * data/port roles could be updated independently after
+ * a plug event. Therefore, we need to check
+ * for pr/dr status change to set TypeC dr/pr accordingly.
+ */
+ if ((event[0] | event[1]) & TPS_REG_INT_PLUG_EVENT ||
+ tps6598x_has_role_changed(tps, status))
+ tps6598x_handle_plug_event(tps, status);
+
+ tps->status = status;
+
+err_clear_ints:
+ tps6598x_block_write(tps, TPS_REG_INT_CLEAR1, event, 11);
+
+err_unlock:
+ mutex_unlock(&tps->lock);
+
+ if (event[0] | event[1])
+ return IRQ_HANDLED;
+ return IRQ_NONE;
+}
+
static irqreturn_t tps6598x_interrupt(int irq, void *data)
{
struct tps6598x *tps = data;
@@ -600,7 +665,7 @@ static void tps6598x_poll_work(struct work_struct *work)
struct tps6598x *tps = container_of(to_delayed_work(work),
struct tps6598x, wq_poll);
- tps->irq_handler(0, tps);
+ tps->data->irq_handler(0, tps);
queue_delayed_work(system_power_efficient_wq,
&tps->wq_poll, msecs_to_jiffies(POLL_INTERVAL));
}
@@ -969,7 +1034,6 @@ static int tps25750_apply_patch(struct tps6598x *tps)
static int tps6598x_probe(struct i2c_client *client)
{
- irq_handler_t irq_handler = tps6598x_interrupt;
struct device_node *np = client->dev.of_node;
struct typec_capability typec_cap = { };
struct tps6598x *tps;
@@ -1017,7 +1081,6 @@ static int tps6598x_probe(struct i2c_client *client)
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 */
mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
@@ -1025,7 +1088,10 @@ static int tps6598x_probe(struct i2c_client *client)
TPS_REG_INT_PLUG_EVENT;
}
- tps->irq_handler = irq_handler;
+ tps->data = device_get_match_data(tps->dev);
+ if (!tps->data)
+ return -EINVAL;
+
/* Make sure the controller has application firmware running */
ret = tps6598x_check_mode(tps);
if (ret < 0)
@@ -1125,7 +1191,7 @@ static int tps6598x_probe(struct i2c_client *client)
if (client->irq) {
ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
- irq_handler,
+ tps->data->irq_handler,
IRQF_SHARED | IRQF_ONESHOT,
dev_name(&client->dev), tps);
} else {
@@ -1231,10 +1297,22 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(tps6598x_suspend, tps6598x_resume)
};
+static const struct tipd_data cd321x_data = {
+ .irq_handler = cd321x_interrupt,
+};
+
+static const struct tipd_data tps6598x_data = {
+ .irq_handler = tps6598x_interrupt,
+};
+
+static const struct tipd_data tps25750_data = {
+ .irq_handler = tps25750_interrupt,
+};
+
static const struct of_device_id tps6598x_of_match[] = {
- { .compatible = "ti,tps6598x", },
- { .compatible = "apple,cd321x", },
- { .compatible = "ti,tps25750", },
+ { .compatible = "ti,tps6598x", &tps6598x_data},
+ { .compatible = "apple,cd321x", &cd321x_data},
+ { .compatible = "ti,tps25750", &tps25750_data},
{}
};
MODULE_DEVICE_TABLE(of, tps6598x_of_match);
--
2.34.1
From: Abdel Alkuor <[email protected]>
tps25750 event1 register doesn't have all bits in tps6598x
event registers, only show the events that are masked
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- No changes
Changes in v7:
- Add driver name to commit subject
- Call trace_tps25750_irq directly from tps25750 interrupt
handler
Changes in v6:
- Add trace irq to tipd callbacks factory
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 1 +
drivers/usb/typec/tipd/trace.h | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index ddb5db7d5855..f775e479b92b 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -583,6 +583,7 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
dev_err(tps->dev, "%s: failed to read events\n", __func__);
goto err_unlock;
}
+ trace_tps25750_irq(event[0]);
if (!(event[0] | event[1]))
goto err_unlock;
diff --git a/drivers/usb/typec/tipd/trace.h b/drivers/usb/typec/tipd/trace.h
index 12cad1bde7cc..28725234a2d8 100644
--- a/drivers/usb/typec/tipd/trace.h
+++ b/drivers/usb/typec/tipd/trace.h
@@ -74,6 +74,13 @@
{ APPLE_CD_REG_INT_DATA_STATUS_UPDATE, "DATA_STATUS_UPDATE" }, \
{ APPLE_CD_REG_INT_STATUS_UPDATE, "STATUS_UPDATE" })
+#define show_tps25750_irq_flags(flags) \
+ __print_flags_u64(flags, "|", \
+ { TPS_REG_INT_PLUG_EVENT, "PLUG_EVENT" }, \
+ { TPS_REG_INT_POWER_STATUS_UPDATE, "POWER_STATUS_UPDATE" }, \
+ { TPS_REG_INT_STATUS_UPDATE, "STATUS_UPDATE" }, \
+ { TPS_REG_INT_PD_STATUS_UPDATE, "PD_STATUS_UPDATE" })
+
#define TPS6598X_STATUS_FLAGS_MASK (GENMASK(31, 0) ^ (TPS_STATUS_CONN_STATE_MASK | \
TPS_STATUS_PP_5V0_SWITCH_MASK | \
TPS_STATUS_PP_HV_SWITCH_MASK | \
@@ -230,6 +237,21 @@ TRACE_EVENT(cd321x_irq,
show_cd321x_irq_flags(__entry->event))
);
+TRACE_EVENT(tps25750_irq,
+ TP_PROTO(u64 event),
+ TP_ARGS(event),
+
+ TP_STRUCT__entry(
+ __field(u64, event)
+ ),
+
+ TP_fast_assign(
+ __entry->event = event;
+ ),
+
+ TP_printk("event=%s", show_tps25750_irq_flags(__entry->event))
+);
+
TRACE_EVENT(tps6598x_status,
TP_PROTO(u32 status),
TP_ARGS(status),
--
2.34.1
From: Abdel Alkuor <[email protected]>
When an EEPROM is present, tps25750 loads the binary configuration from
EEPROM. Hence, all we need to do is wait for the device to switch to APP
mode
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- No changes
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Update eeprom macro to use TPS instead
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 13 +++++++++++++
drivers/usb/typec/tipd/tps6598x.h | 3 +++
2 files changed, 16 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 2e7b9eafaf04..21b0ea2c9627 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -37,6 +37,7 @@
#define TPS_REG_STATUS 0x1a
#define TPS_REG_SYSTEM_CONF 0x28
#define TPS_REG_CTRL_CONF 0x29
+#define TPS_REG_BOOT_STATUS 0x2D
#define TPS_REG_POWER_STATUS 0x3f
#define TPS_REG_RX_IDENTITY_SOP 0x48
#define TPS_REG_DATA_STATUS 0x5f
@@ -910,6 +911,17 @@ static int tps25750_apply_patch(struct tps6598x *tps)
{
int ret;
unsigned long timeout;
+ u64 status = 0;
+
+ ret = tps6598x_block_read(tps, TPS_REG_BOOT_STATUS, &status, 5);
+ if (ret)
+ return ret;
+ /*
+ * Nothing to be done if the configuration
+ * is being loaded from EERPOM
+ */
+ if (status & TPS_BOOT_STATUS_I2C_EEPROM_PRESENT)
+ goto wait_for_app;
ret = tps25750_start_patch_burst_mode(tps);
if (ret) {
@@ -921,6 +933,7 @@ static int tps25750_apply_patch(struct tps6598x *tps)
if (ret)
return ret;
+wait_for_app:
timeout = jiffies + msecs_to_jiffies(1000);
do {
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index 527857549d69..a80d0929f3ee 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -199,4 +199,7 @@
#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_A BIT(2)
#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
+/* BOOT STATUS REG*/
+#define TPS_BOOT_STATUS_I2C_EEPROM_PRESENT BIT(3)
+
#endif /* __TPS6598X_H__ */
--
2.34.1
From: Abdel Alkuor <[email protected]>
TPS25750 doesn't have system configuration register to get dr/pr of the
current applied binary configuration.
Get data role from the device node and power role from PD status register.
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- Move device data structs to the top of of_device_id
Changes in v8:
- Change tps->cb to tps->data
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Add port registration to tipd data factory
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 68 ++++++++++++++++++++++++++++++-
drivers/usb/typec/tipd/tps6598x.h | 10 +++++
2 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 0195eabd96bf..ae08c5306707 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -40,6 +40,7 @@
#define TPS_REG_CTRL_CONF 0x29
#define TPS_REG_BOOT_STATUS 0x2D
#define TPS_REG_POWER_STATUS 0x3f
+#define TPS_REG_PD_STATUS 0x40
#define TPS_REG_RX_IDENTITY_SOP 0x48
#define TPS_REG_DATA_STATUS 0x5f
@@ -102,8 +103,11 @@ static const char *const modes[] = {
/* Unrecognized commands will be replaced with "!CMD" */
#define INVALID_CMD(_cmd_) (_cmd_ == 0x444d4321)
+struct tps6598x;
+
struct tipd_data {
irq_handler_t irq_handler;
+ int (*register_port)(struct tps6598x *tps, struct fwnode_handle *node);
};
struct tps6598x {
@@ -208,6 +212,11 @@ static inline int tps6598x_read64(struct tps6598x *tps, u8 reg, u64 *val)
return tps6598x_block_read(tps, reg, val, sizeof(u64));
}
+static inline int tps6598x_write8(struct tps6598x *tps, u8 reg, u8 val)
+{
+ return tps6598x_block_write(tps, reg, &val, sizeof(u8));
+}
+
static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
{
return tps6598x_block_write(tps, reg, &val, sizeof(u64));
@@ -1084,6 +1093,60 @@ tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
return 0;
}
+static int
+tps25750_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
+{
+ struct typec_capability typec_cap = { };
+ const char *data_role;
+ u8 pd_status;
+ int ret;
+
+ ret = tps6598x_read8(tps, TPS_REG_PD_STATUS, &pd_status);
+ if (ret)
+ return ret;
+
+ ret = fwnode_property_read_string(fwnode, "data-role", &data_role);
+ if (ret) {
+ dev_err(tps->dev, "data-role not found: %d\n", ret);
+ return ret;
+ }
+
+ ret = typec_find_port_data_role(data_role);
+ if (ret < 0) {
+ dev_err(tps->dev, "unknown data-role: %s\n", data_role);
+ return ret;
+ }
+
+ typec_cap.data = ret;
+ typec_cap.revision = USB_TYPEC_REV_1_3;
+ typec_cap.pd_revision = 0x300;
+ typec_cap.driver_data = tps;
+ typec_cap.ops = &tps6598x_ops;
+ typec_cap.fwnode = fwnode;
+ typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+
+ switch (TPS_PD_STATUS_PORT_TYPE(pd_status)) {
+ case TPS_PD_STATUS_PORT_TYPE_SINK_SOURCE:
+ case TPS_PD_STATUS_PORT_TYPE_SOURCE_SINK:
+ typec_cap.type = TYPEC_PORT_DRP;
+ break;
+ case TPS_PD_STATUS_PORT_TYPE_SINK:
+ typec_cap.type = TYPEC_PORT_SNK;
+ break;
+ case TPS_PD_STATUS_PORT_TYPE_SOURCE:
+ typec_cap.type = TYPEC_PORT_SRC;
+ break;
+ default:
+ return -ENODEV;
+ }
+
+ tps->port = typec_register_port(tps->dev, &typec_cap);
+ if (IS_ERR(tps->port))
+ return PTR_ERR(tps->port);
+
+ return 0;
+}
+
static int tps6598x_probe(struct i2c_client *client)
{
struct device_node *np = client->dev.of_node;
@@ -1183,7 +1246,7 @@ static int tps6598x_probe(struct i2c_client *client)
if (ret)
goto err_role_put;
- ret = tps6598x_register_port(tps, fwnode);
+ ret = tps->data->register_port(tps, fwnode);
if (ret)
goto err_role_put;
@@ -1308,14 +1371,17 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
static const struct tipd_data cd321x_data = {
.irq_handler = cd321x_interrupt,
+ .register_port = tps6598x_register_port,
};
static const struct tipd_data tps6598x_data = {
.irq_handler = tps6598x_interrupt,
+ .register_port = tps6598x_register_port,
};
static const struct tipd_data tps25750_data = {
.irq_handler = tps25750_interrupt,
+ .register_port = tps25750_register_port,
};
static const struct of_device_id tps6598x_of_match[] = {
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index c000170f4547..3a9a43394134 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -203,4 +203,14 @@
#define TPS_BOOT_STATUS_DEAD_BATTERY_FLAG BIT(2)
#define TPS_BOOT_STATUS_I2C_EEPROM_PRESENT BIT(3)
+/* PD STATUS REG */
+#define TPS_REG_PD_STATUS_PORT_TYPE_MASK GENMASK(5, 4)
+#define TPS_PD_STATUS_PORT_TYPE(x) \
+ TPS_FIELD_GET(TPS_REG_PD_STATUS_PORT_TYPE_MASK, x)
+
+#define TPS_PD_STATUS_PORT_TYPE_SINK_SOURCE 0
+#define TPS_PD_STATUS_PORT_TYPE_SINK 1
+#define TPS_PD_STATUS_PORT_TYPE_SOURCE 2
+#define TPS_PD_STATUS_PORT_TYPE_SOURCE_SINK 3
+
#endif /* __TPS6598X_H__ */
--
2.34.1
From: Abdel Alkuor <[email protected]>
tps6598x and cd321x use TPS_REG_SYSTEM_CONF to get dr/pr roles
where other similar devices don't have this register such as tps25750.
Move tps6598x port registration to its own function
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- No changes
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- No changes
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 99 +++++++++++++++++++----------------
1 file changed, 54 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 52dc1cc16bed..0195eabd96bf 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -1032,14 +1032,64 @@ static int tps25750_apply_patch(struct tps6598x *tps)
return 0;
};
+static int
+tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
+{
+ int ret;
+ u32 conf;
+ struct typec_capability typec_cap = { };
+
+ ret = tps6598x_read32(tps, TPS_REG_SYSTEM_CONF, &conf);
+ if (ret)
+ return ret;
+
+ typec_cap.revision = USB_TYPEC_REV_1_2;
+ typec_cap.pd_revision = 0x200;
+ typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+ typec_cap.driver_data = tps;
+ typec_cap.ops = &tps6598x_ops;
+ typec_cap.fwnode = fwnode;
+
+ switch (TPS_SYSCONF_PORTINFO(conf)) {
+ case TPS_PORTINFO_SINK_ACCESSORY:
+ case TPS_PORTINFO_SINK:
+ typec_cap.type = TYPEC_PORT_SNK;
+ typec_cap.data = TYPEC_PORT_UFP;
+ break;
+ case TPS_PORTINFO_DRP_UFP_DRD:
+ case TPS_PORTINFO_DRP_DFP_DRD:
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_DRD;
+ break;
+ case TPS_PORTINFO_DRP_UFP:
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_UFP;
+ break;
+ case TPS_PORTINFO_DRP_DFP:
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_DFP;
+ break;
+ case TPS_PORTINFO_SOURCE:
+ typec_cap.type = TYPEC_PORT_SRC;
+ typec_cap.data = TYPEC_PORT_DFP;
+ break;
+ default:
+ return -ENODEV;
+ }
+
+ tps->port = typec_register_port(tps->dev, &typec_cap);
+ if (IS_ERR(tps->port))
+ return PTR_ERR(tps->port);
+
+ return 0;
+}
+
static int tps6598x_probe(struct i2c_client *client)
{
struct device_node *np = client->dev.of_node;
- struct typec_capability typec_cap = { };
struct tps6598x *tps;
struct fwnode_handle *fwnode;
u32 status;
- u32 conf;
u32 vid;
int ret;
u64 mask1;
@@ -1112,10 +1162,6 @@ static int tps6598x_probe(struct i2c_client *client)
goto err_clear_mask;
trace_tps6598x_status(status);
- ret = tps6598x_read32(tps, TPS_REG_SYSTEM_CONF, &conf);
- if (ret < 0)
- goto err_clear_mask;
-
/*
* This fwnode has a "compatible" property, but is never populated as a
* struct device. Instead we simply parse it to read the properties.
@@ -1133,50 +1179,13 @@ static int tps6598x_probe(struct i2c_client *client)
goto err_fwnode_put;
}
- typec_cap.revision = USB_TYPEC_REV_1_2;
- typec_cap.pd_revision = 0x200;
- typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
- typec_cap.driver_data = tps;
- typec_cap.ops = &tps6598x_ops;
- typec_cap.fwnode = fwnode;
-
- switch (TPS_SYSCONF_PORTINFO(conf)) {
- case TPS_PORTINFO_SINK_ACCESSORY:
- case TPS_PORTINFO_SINK:
- typec_cap.type = TYPEC_PORT_SNK;
- typec_cap.data = TYPEC_PORT_UFP;
- break;
- case TPS_PORTINFO_DRP_UFP_DRD:
- case TPS_PORTINFO_DRP_DFP_DRD:
- typec_cap.type = TYPEC_PORT_DRP;
- typec_cap.data = TYPEC_PORT_DRD;
- break;
- case TPS_PORTINFO_DRP_UFP:
- typec_cap.type = TYPEC_PORT_DRP;
- typec_cap.data = TYPEC_PORT_UFP;
- break;
- case TPS_PORTINFO_DRP_DFP:
- typec_cap.type = TYPEC_PORT_DRP;
- typec_cap.data = TYPEC_PORT_DFP;
- break;
- case TPS_PORTINFO_SOURCE:
- typec_cap.type = TYPEC_PORT_SRC;
- typec_cap.data = TYPEC_PORT_DFP;
- break;
- default:
- ret = -ENODEV;
- goto err_role_put;
- }
-
ret = devm_tps6598_psy_register(tps);
if (ret)
goto err_role_put;
- tps->port = typec_register_port(&client->dev, &typec_cap);
- if (IS_ERR(tps->port)) {
- ret = PTR_ERR(tps->port);
+ ret = tps6598x_register_port(tps, fwnode);
+ if (ret)
goto err_role_put;
- }
if (status & TPS_STATUS_PLUG_PRESENT) {
ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &tps->pwr_status);
--
2.34.1
From: Abdel Alkuor <[email protected]>
Allow controller to enter sleep mode after the device
is idle for sleep time.
Signed-off-by: Abdel Alkuor <[email protected]>
---
Changes in v9:
- No changes
Changes in v8:
- No changes
Changes in v7:
- Add driver name to commit subject
Changes in v6:
- Use tps25750_init instead of tps25750_apply_patch in resume
as it initializes sleep mode
Changes in v5:
- Incorporating tps25750 into tps6598x driver
drivers/usb/typec/tipd/core.c | 23 +++++++++++++++++++++--
drivers/usb/typec/tipd/tps6598x.h | 3 +++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index ae08c5306707..ddb5db7d5855 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -43,6 +43,7 @@
#define TPS_REG_PD_STATUS 0x40
#define TPS_REG_RX_IDENTITY_SOP 0x48
#define TPS_REG_DATA_STATUS 0x5f
+#define TPS_REG_SLEEP_CONF 0x70
/* TPS_REG_SYSTEM_CONF bits */
#define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
@@ -1041,6 +1042,24 @@ static int tps25750_apply_patch(struct tps6598x *tps)
return 0;
};
+static int tps25750_init(struct tps6598x *tps)
+{
+ int ret;
+
+ ret = tps25750_apply_patch(tps);
+ if (ret)
+ return ret;
+
+ ret = tps6598x_write8(tps, TPS_REG_SLEEP_CONF,
+ TPS_SLEEP_CONF_SLEEP_MODE_ALLOWED);
+ if (ret)
+ dev_warn(tps->dev,
+ "%s: failed to enable sleep mode: %d\n",
+ __func__, ret);
+
+ return 0;
+}
+
static int
tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
{
@@ -1211,7 +1230,7 @@ static int tps6598x_probe(struct i2c_client *client)
return ret;
if (is_tps25750 && ret == TPS_MODE_PTCH) {
- ret = tps25750_apply_patch(tps);
+ ret = tps25750_init(tps);
if (ret)
return ret;
}
@@ -1348,7 +1367,7 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
return ret;
if (device_is_compatible(tps->dev, "ti,tps25750") && ret == TPS_MODE_PTCH) {
- ret = tps25750_apply_patch(tps);
+ ret = tps25750_init(tps);
if (ret)
return ret;
}
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index 3a9a43394134..f86b5e96efba 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -213,4 +213,7 @@
#define TPS_PD_STATUS_PORT_TYPE_SOURCE 2
#define TPS_PD_STATUS_PORT_TYPE_SOURCE_SINK 3
+/* SLEEP CONF REG */
+#define TPS_SLEEP_CONF_SLEEP_MODE_ALLOWED BIT(0)
+
#endif /* __TPS6598X_H__ */
--
2.34.1
From: Abdel Alkuor <[email protected]>
TPS25750 is USB TypeC PD controller which is a subset of TPS6598x.
Signed-off-by: Abdel Alkuor <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
Changes in v9:
- Add Reviewed-by
Changes in v8:
- Define reg-names at top-level
Changes in v7:
- Define reg at top-level
- Remove description from reg-names
Changes in v6:
- Use reg property for patch address
Changes in v5:
- Add tps25750 bindings
.../devicetree/bindings/usb/ti,tps6598x.yaml | 81 ++++++++++++++++++-
.../devicetree/bindings/usb/ti,tps6598x.yaml | 81 ++++++++++++++++++-
1 file changed, 80 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
index 5497a60cddbc..72ac534e6ed2 100644
--- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
@@ -20,8 +20,23 @@ properties:
enum:
- ti,tps6598x
- apple,cd321x
+ - ti,tps25750
+
reg:
- maxItems: 1
+ minItems: 1
+ items:
+ - description: main PD controller address
+ - description: |
+ I2C slave address field in PBMs input data
+ which is used as the device address when writing the
+ patch for TPS25750.
+ The patch address can be any value except 0x00, 0x20,
+ 0x21, 0x22, and 0x23
+
+ reg-names:
+ items:
+ - const: main
+ - const: patch-address
wakeup-source: true
@@ -32,10 +47,42 @@ properties:
items:
- const: irq
+ firmware-name:
+ description: |
+ Should contain the name of the default patch binary
+ file located on the firmware search path which is
+ used to switch the controller into APP mode.
+ This is used when tps25750 doesn't have an EEPROM
+ connected to it.
+ maxItems: 1
+
required:
- compatible
- reg
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,tps25750
+ then:
+ properties:
+ reg:
+ maxItems: 2
+
+ connector:
+ required:
+ - data-role
+
+ required:
+ - connector
+ - reg-names
+ else:
+ properties:
+ reg:
+ maxItems: 1
+
additionalProperties: true
examples:
@@ -68,4 +115,36 @@ examples:
};
};
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ typec@21 {
+ compatible = "ti,tps25750";
+ reg = <0x21>, <0x0f>;
+ reg-names = "main", "patch-address";
+
+ interrupt-parent = <&msmgpio>;
+ interrupts = <100 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "irq";
+ firmware-name = "tps25750.bin";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&typec_pins>;
+
+ typec_con0: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ data-role = "dual";
+ port {
+ typec_ep0: endpoint {
+ remote-endpoint = <&otg_ep>;
+ };
+ };
+ };
+ };
+ };
...
--
2.34.1
On Sun, Oct 01, 2023 at 04:11:24AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> TPS25750 controller requires a binary to be loaded with a configuration
> binary by an EEPROM or a host.
>
> Appling a patch bundling using a host is implemented based on the flow
> diagram pg.62 in TPS25750 host interface manual.
> https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
>
> The flow diagram can be summarized as following:
> - Start the patch loading sequence with patch bundle information by
> executing PBMs
> - Write the whole patch at once
> - When writing the patch fails, execute PBMe which instructs the PD controller
> to end the patching process
> - After writing the patch successfully, execute PBMc which verifies the patch
> integrity and applies the patch internally
> - Wait for the device to switch into APP mode (normal operation)
>
> The execuation flow diagram polls the events register and then polls the
> corresponding register related to the event as well before advancing to the next
> state. Polling the events register is a redundant step, in this implementation
> only the corresponding register related to the event is polled.
>
> Signed-off-by: Abdel Alkuor <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> Changes in v9:
> - No changes
> Changes in v8:
> - Return mode when mode is checked
> - Use device_is_compatible instead of of_device_is_compatible
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - Don't check VID for tps25750 as the VID register doesn't exist
> - Remove is_tps25750 flag from tps6598x struct
> - Get patch address from reg property
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
> drivers/usb/typec/tipd/core.c | 263 ++++++++++++++++++++++++++++++++--
> 1 file changed, 255 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index c5bbf03cb74a..2e7b9eafaf04 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -17,6 +17,7 @@
> #include <linux/usb/typec_altmode.h>
> #include <linux/usb/role.h>
> #include <linux/workqueue.h>
> +#include <linux/firmware.h>
>
> #include "tps6598x.h"
> #include "trace.h"
> @@ -43,6 +44,23 @@
> /* TPS_REG_SYSTEM_CONF bits */
> #define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
>
> +/*
> + * BPMs task timeout, recommended 5 seconds
> + * pg.48 TPS2575 Host Interface Technical Reference
> + * Manual (Rev. A)
> + * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
> + */
> +#define TPS_BUNDLE_TIMEOUT 0x32
> +
> +/* BPMs return code */
> +#define TPS_TASK_BPMS_INVALID_BUNDLE_SIZE 0x4
> +#define TPS_TASK_BPMS_INVALID_SLAVE_ADDR 0x5
> +#define TPS_TASK_BPMS_INVALID_TIMEOUT 0x6
> +
> +/* PBMc data out */
> +#define TPS_PBMC_RC 0 /* Return code */
> +#define TPS_PBMC_DPCS 2 /* device patch complete status */
> +
> enum {
> TPS_PORTINFO_SINK,
> TPS_PORTINFO_SINK_ACCESSORY,
> @@ -595,13 +613,15 @@ static int tps6598x_check_mode(struct tps6598x *tps)
> if (ret)
> return ret;
>
> - switch (match_string(modes, ARRAY_SIZE(modes), mode)) {
> + ret = match_string(modes, ARRAY_SIZE(modes), mode);
> +
> + switch (ret) {
> case TPS_MODE_APP:
> case TPS_MODE_PTCH:
> - return 0;
> + return ret;
> case TPS_MODE_BOOT:
> dev_warn(tps->dev, "dead-battery condition\n");
> - return 0;
> + return ret;
> case TPS_MODE_BIST:
> case TPS_MODE_DISC:
> default:
> @@ -711,6 +731,213 @@ static int devm_tps6598_psy_register(struct tps6598x *tps)
> return PTR_ERR_OR_ZERO(tps->psy);
> }
>
> +static int
> +tps25750_write_firmware(struct tps6598x *tps,
> + u8 bpms_addr, const u8 *data, size_t len)
> +{
> + struct i2c_client *client = to_i2c_client(tps->dev);
> + int ret;
> + u8 slave_addr;
> + int timeout;
> +
> + slave_addr = client->addr;
> + timeout = client->adapter->timeout;
> +
> + /*
> + * binary configuration size is around ~16Kbytes
> + * which might take some time to finish writing it
> + */
> + client->adapter->timeout = msecs_to_jiffies(5000);
> + client->addr = bpms_addr;
> +
> + ret = regmap_raw_write(tps->regmap, data[0], &data[1], len - 1);
> +
> + client->addr = slave_addr;
> + client->adapter->timeout = timeout;
> +
> + return ret;
> +}
> +
> +static int
> +tps25750_exec_pbms(struct tps6598x *tps, u8 *in_data, size_t in_len)
> +{
> + int ret;
> + u8 rc;
> +
> + ret = tps6598x_exec_cmd_tmo(tps, "PBMs", in_len, in_data,
> + sizeof(rc), &rc, 4000, 0);
> + if (ret)
> + return ret;
> +
> + switch (rc) {
> + case TPS_TASK_BPMS_INVALID_BUNDLE_SIZE:
> + dev_err(tps->dev, "%s: invalid fw size\n", __func__);
> + return -EINVAL;
> + case TPS_TASK_BPMS_INVALID_SLAVE_ADDR:
> + dev_err(tps->dev, "%s: invalid slave address\n", __func__);
> + return -EINVAL;
> + case TPS_TASK_BPMS_INVALID_TIMEOUT:
> + dev_err(tps->dev, "%s: timed out\n", __func__);
> + return -ETIMEDOUT;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int tps25750_abort_patch_process(struct tps6598x *tps)
> +{
> + int ret;
> +
> + ret = tps6598x_exec_cmd(tps, "PBMe", 0, NULL, 0, NULL);
> + if (ret)
> + return ret;
> +
> + ret = tps6598x_check_mode(tps);
> + if (ret != TPS_MODE_PTCH)
> + dev_err(tps->dev, "failed to switch to \"PTCH\" mode\n");
> +
> + return ret;
> +}
> +
> +static int tps25750_start_patch_burst_mode(struct tps6598x *tps)
> +{
> + int ret;
> + const struct firmware *fw;
> + const char *firmware_name;
> + struct {
> + u32 fw_size;
> + u8 addr;
> + u8 timeout;
> + } __packed bpms_data;
> + u32 addr;
> + struct device_node *np = tps->dev->of_node;
> +
> + ret = device_property_read_string(tps->dev, "firmware-name",
> + &firmware_name);
> + if (ret)
> + return ret;
> +
> + ret = request_firmware(&fw, firmware_name, tps->dev);
> + if (ret) {
> + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name);
> + return ret;
> + }
> +
> + if (fw->size == 0) {
> + ret = -EINVAL;
> + goto release_fw;
> + }
> +
> + ret = of_property_match_string(np, "reg-names", "patch-address");
> + if (ret < 0) {
> + dev_err(tps->dev, "failed to get patch-address %d\n", ret);
> + return ret;
> + }
> +
> + ret = of_property_read_u32_index(np, "reg", ret, &addr);
> + if (ret)
> + return ret;
> +
> + if (addr == 0 || (addr >= 0x20 && addr <= 0x23)) {
> + dev_err(tps->dev, "wrong patch address %u\n", addr);
> + return -EINVAL;
> + }
> +
> + bpms_data.addr = (u8)addr;
> + bpms_data.fw_size = fw->size;
> + bpms_data.timeout = TPS_BUNDLE_TIMEOUT;
> +
> + ret = tps25750_exec_pbms(tps, (u8 *)&bpms_data, sizeof(bpms_data));
> + if (ret)
> + goto release_fw;
> +
> + ret = tps25750_write_firmware(tps, bpms_data.addr, fw->data, fw->size);
> + if (ret) {
> + dev_err(tps->dev, "Failed to write patch %s of %zu bytes\n",
> + firmware_name, fw->size);
> + goto release_fw;
> + }
> +
> + /*
> + * A delay of 500us is required after the firmware is written
> + * based on pg.62 in tps6598x Host Interface Technical
> + * Reference Manual
> + * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
> + */
> + udelay(500);
> +
> +release_fw:
> + release_firmware(fw);
> +
> + return ret;
> +}
> +
> +static int tps25750_complete_patch_process(struct tps6598x *tps)
> +{
> + int ret;
> + u8 out_data[40];
> + u8 dummy[2] = { };
> +
> + /*
> + * Without writing something to DATA_IN, this command would
> + * return an error
> + */
> + ret = tps6598x_exec_cmd_tmo(tps, "PBMc", sizeof(dummy), dummy,
> + sizeof(out_data), out_data, 2000, 20);
> + if (ret)
> + return ret;
> +
> + if (out_data[TPS_PBMC_RC]) {
> + dev_err(tps->dev,
> + "%s: pbmc failed: %u\n", __func__,
> + out_data[TPS_PBMC_RC]);
> + return -EIO;
> + }
> +
> + if (out_data[TPS_PBMC_DPCS]) {
> + dev_err(tps->dev,
> + "%s: failed device patch complete status: %u\n",
> + __func__, out_data[TPS_PBMC_DPCS]);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int tps25750_apply_patch(struct tps6598x *tps)
> +{
> + int ret;
> + unsigned long timeout;
> +
> + ret = tps25750_start_patch_burst_mode(tps);
> + if (ret) {
> + tps25750_abort_patch_process(tps);
> + return ret;
> + }
> +
> + ret = tps25750_complete_patch_process(tps);
> + if (ret)
> + return ret;
> +
> + timeout = jiffies + msecs_to_jiffies(1000);
> +
> + do {
> + ret = tps6598x_check_mode(tps);
> + if (ret < 0)
> + return ret;
> +
> + if (time_is_before_jiffies(timeout))
> + return -ETIMEDOUT;
> +
> + } while (ret != TPS_MODE_APP);
> +
> + dev_info(tps->dev, "controller switched to \"APP\" mode\n");
> +
> + return 0;
> +};
> +
> static int tps6598x_probe(struct i2c_client *client)
> {
> irq_handler_t irq_handler = tps6598x_interrupt;
> @@ -723,6 +950,7 @@ static int tps6598x_probe(struct i2c_client *client)
> u32 vid;
> int ret;
> u64 mask1;
> + bool is_tps25750;
>
> tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
> if (!tps)
> @@ -735,9 +963,12 @@ static int tps6598x_probe(struct i2c_client *client)
> if (IS_ERR(tps->regmap))
> return PTR_ERR(tps->regmap);
>
> - ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
> - if (ret < 0 || !vid)
> - return -ENODEV;
> + is_tps25750 = device_is_compatible(tps->dev, "ti,tps25750");
> + if (!is_tps25750) {
> + ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
> + if (ret < 0 || !vid)
> + return -ENODEV;
> + }
>
> /*
> * Checking can the adapter handle SMBus protocol. If it can not, the
> @@ -768,12 +999,18 @@ static int tps6598x_probe(struct i2c_client *client)
> tps->irq_handler = irq_handler;
> /* Make sure the controller has application firmware running */
> ret = tps6598x_check_mode(tps);
> - if (ret)
> + if (ret < 0)
> return ret;
>
> + if (is_tps25750 && ret == TPS_MODE_PTCH) {
> + ret = tps25750_apply_patch(tps);
> + if (ret)
> + return ret;
> + }
> +
> ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, mask1);
> if (ret)
> - return ret;
> + goto err_reset_controller;
>
> ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
> if (ret < 0)
> @@ -893,6 +1130,10 @@ static int tps6598x_probe(struct i2c_client *client)
> fwnode_handle_put(fwnode);
> err_clear_mask:
> tps6598x_write64(tps, TPS_REG_INT_MASK1, 0);
> +err_reset_controller:
> + /* Reset PD controller to remove any applied patch */
> + if (is_tps25750)
> + tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> return ret;
> }
>
> @@ -903,9 +1144,14 @@ static void tps6598x_remove(struct i2c_client *client)
> if (!client->irq)
> cancel_delayed_work_sync(&tps->wq_poll);
>
> + devm_free_irq(tps->dev, client->irq, tps);
> tps6598x_disconnect(tps, 0);
> typec_unregister_port(tps->port);
> usb_role_switch_put(tps->role_sw);
> +
> + /* Reset PD controller to remove any applied patch */
> + if (device_is_compatible(tps->dev, "ti,tps25750"))
> + tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> }
>
> static int __maybe_unused tps6598x_suspend(struct device *dev)
> @@ -948,6 +1194,7 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
> static const struct of_device_id tps6598x_of_match[] = {
> { .compatible = "ti,tps6598x", },
> { .compatible = "apple,cd321x", },
> + { .compatible = "ti,tps25750", },
> {}
> };
> MODULE_DEVICE_TABLE(of, tps6598x_of_match);
> --
> 2.34.1
--
heikki
On Sun, Oct 01, 2023 at 04:11:25AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> When an EEPROM is present, tps25750 loads the binary configuration from
> EEPROM. Hence, all we need to do is wait for the device to switch to APP
> mode
>
> Signed-off-by: Abdel Alkuor <[email protected]>
I'm not sure I understand why this needs separate patch, but in any
case:
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> Changes in v9:
> - No changes
> Changes in v8:
> - No changes
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - Update eeprom macro to use TPS instead
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
>
> drivers/usb/typec/tipd/core.c | 13 +++++++++++++
> drivers/usb/typec/tipd/tps6598x.h | 3 +++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 2e7b9eafaf04..21b0ea2c9627 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -37,6 +37,7 @@
> #define TPS_REG_STATUS 0x1a
> #define TPS_REG_SYSTEM_CONF 0x28
> #define TPS_REG_CTRL_CONF 0x29
> +#define TPS_REG_BOOT_STATUS 0x2D
> #define TPS_REG_POWER_STATUS 0x3f
> #define TPS_REG_RX_IDENTITY_SOP 0x48
> #define TPS_REG_DATA_STATUS 0x5f
> @@ -910,6 +911,17 @@ static int tps25750_apply_patch(struct tps6598x *tps)
> {
> int ret;
> unsigned long timeout;
> + u64 status = 0;
> +
> + ret = tps6598x_block_read(tps, TPS_REG_BOOT_STATUS, &status, 5);
> + if (ret)
> + return ret;
> + /*
> + * Nothing to be done if the configuration
> + * is being loaded from EERPOM
> + */
> + if (status & TPS_BOOT_STATUS_I2C_EEPROM_PRESENT)
> + goto wait_for_app;
>
> ret = tps25750_start_patch_burst_mode(tps);
> if (ret) {
> @@ -921,6 +933,7 @@ static int tps25750_apply_patch(struct tps6598x *tps)
> if (ret)
> return ret;
>
> +wait_for_app:
> timeout = jiffies + msecs_to_jiffies(1000);
>
> do {
> diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
> index 527857549d69..a80d0929f3ee 100644
> --- a/drivers/usb/typec/tipd/tps6598x.h
> +++ b/drivers/usb/typec/tipd/tps6598x.h
> @@ -199,4 +199,7 @@
> #define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_A BIT(2)
> #define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
>
> +/* BOOT STATUS REG*/
> +#define TPS_BOOT_STATUS_I2C_EEPROM_PRESENT BIT(3)
> +
> #endif /* __TPS6598X_H__ */
> --
> 2.34.1
--
heikki
On Sun, Oct 01, 2023 at 04:11:26AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> Dead battery flag must be cleared after switching tps25750 to APP mode
> so the PD controller becomes fully functional.
>
> Signed-off-by: Abdel Alkuor <[email protected]>
I'm sorry I noticed these so late, but this one really feels like it
should be part of the patch 4/14. Is there some reason why you do this
separately?
> ---
> Changes in v9:
> - No changes
> Changes in v8:
> - No changes
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - No changes
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
>
> drivers/usb/typec/tipd/core.c | 16 ++++++++++++++++
> drivers/usb/typec/tipd/tps6598x.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 21b0ea2c9627..2598433a69cf 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -946,6 +946,22 @@ static int tps25750_apply_patch(struct tps6598x *tps)
>
> } while (ret != TPS_MODE_APP);
>
> + /*
> + * The dead battery flag may be triggered when the controller
> + * port is connected to a device that can source power and
> + * attempts to power up both the controller and the board it is on.
> + * To restore controller functionality, it is necessary to clear
> + * this flag
> + */
> + if (status & TPS_BOOT_STATUS_DEAD_BATTERY_FLAG) {
> + ret = tps6598x_exec_cmd(tps, "DBfg", 0, NULL, 0, NULL);
> + if (ret) {
> + dev_err(tps->dev,
> + "failed to clear dead battery %d\n", ret);
One line is enough.
> + return ret;
> + }
> + }
> +
> dev_info(tps->dev, "controller switched to \"APP\" mode\n");
>
> return 0;
> diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
> index a80d0929f3ee..c000170f4547 100644
> --- a/drivers/usb/typec/tipd/tps6598x.h
> +++ b/drivers/usb/typec/tipd/tps6598x.h
> @@ -200,6 +200,7 @@
> #define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
>
> /* BOOT STATUS REG*/
> +#define TPS_BOOT_STATUS_DEAD_BATTERY_FLAG BIT(2)
> #define TPS_BOOT_STATUS_I2C_EEPROM_PRESENT BIT(3)
>
> #endif /* __TPS6598X_H__ */
> --
> 2.34.1
Br,
--
heikki
On Sun, Oct 01, 2023 at 04:11:27AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> TPS25750 PD controller might be powered off externally at power suspend,
> after resuming PD controller power back, apply the patch again.
>
> Signed-off-by: Abdel Alkuor <[email protected]>
This one looks also like something that should be part of the patch 4.
My concern is that with these separated features you are creating points
into the kernel git tree where TPS25750 is enabled, but it's not fully
functional, or even broken in scenarious like this (suspend/resume).
You can't do that unless you have some really good reason.
Since all of these add only a bit of code each, I think it would be
better to just merge these into the initial patch that enabled
TPS25750 - so I belive patch 4/14.
> ---
> Changes in v9:
> - No changes
> Changes in v8:
> - Use device_is_compatible instead of of_device_is_compatible
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - Check tps25750 using is_compatiable device node
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
> drivers/usb/typec/tipd/core.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 2598433a69cf..32e42798688f 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1203,6 +1203,17 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> struct tps6598x *tps = i2c_get_clientdata(client);
> + int ret;
> +
> + ret = tps6598x_check_mode(tps);
> + if (ret < 0)
> + return ret;
> +
> + if (device_is_compatible(tps->dev, "ti,tps25750") && ret == TPS_MODE_PTCH) {
> + ret = tps25750_apply_patch(tps);
> + if (ret)
> + return ret;
> + }
>
> if (tps->wakeup) {
> disable_irq_wake(client->irq);
> --
> 2.34.1
--
heikki
On Sun, Oct 01, 2023 at 04:11:28AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> tps25750 event registers structure is different than tps6598x's,
> tps25750 has 11 bytes of events which are read at once where
> tps6598x has two event registers of 8 bytes each which are read
> separately. Likewise MASK event registers. Also, not all events
> are supported in both devices.
>
> - Create a new handler to accommodate tps25750 interrupt
> - Add device data to of_device_id
>
> Signed-off-by: Abdel Alkuor <[email protected]>
I'm sorry, but I just realised that this one also has to be in place
by the time TPS25750 is enabled in patch 4/14. Otherwise the series is
not bisectable.
I think you need to refactor the patch order a bit:
First come the patches that prepare everything that needs preparing -
introduction of struct tipd_data (without anything TPS25750 specific),
and so on.
Then you have patches for things that are specific to TPS25750 (small
stuff just merge together) if needed.
In the very last patches you finally enable TPS25750.
thanks,
> ---
> Changes in v9:
> - Move of_device_id to its original place
> - Move device data structs to the top of of_device_id
> - Use device_get_match_data to get device data
> Changes in v8:
> - Populate of_device_id with device data
> - Change tps->cb to tps->data
> - Assign matched data to tps data
> Changes in v7:
> - Add driver name to commit subject
> - Create tps25750 interrupt handler
> Changes in v6:
> - Create tipd callbacks factory
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
>
> drivers/usb/typec/tipd/core.c | 96 +++++++++++++++++++++++++++++++----
> 1 file changed, 87 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 32e42798688f..52dc1cc16bed 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -18,6 +18,7 @@
> #include <linux/usb/role.h>
> #include <linux/workqueue.h>
> #include <linux/firmware.h>
> +#include <linux/of_device.h>
>
> #include "tps6598x.h"
> #include "trace.h"
> @@ -101,6 +102,10 @@ static const char *const modes[] = {
> /* Unrecognized commands will be replaced with "!CMD" */
> #define INVALID_CMD(_cmd_) (_cmd_ == 0x444d4321)
>
> +struct tipd_data {
> + irq_handler_t irq_handler;
> +};
> +
> struct tps6598x {
> struct device *dev;
> struct regmap *regmap;
> @@ -118,9 +123,11 @@ struct tps6598x {
> enum power_supply_usb_type usb_type;
>
> int wakeup;
> + u32 status; /* status reg */
> u16 pwr_status;
> struct delayed_work wq_poll;
> - irq_handler_t irq_handler;
> +
> + const struct tipd_data *data;
> };
>
> static enum power_supply_property tps6598x_psy_props[] = {
> @@ -545,6 +552,64 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
> return IRQ_NONE;
> }
>
> +static bool tps6598x_has_role_changed(struct tps6598x *tps, u32 status)
> +{
> + status ^= tps->status;
> +
> + return status & (TPS_STATUS_PORTROLE | TPS_STATUS_DATAROLE);
> +}
> +
> +static irqreturn_t tps25750_interrupt(int irq, void *data)
> +{
> + struct tps6598x *tps = data;
> + u64 event[2] = { };
> + u32 status;
> + int ret;
> +
> + mutex_lock(&tps->lock);
> +
> + ret = tps6598x_block_read(tps, TPS_REG_INT_EVENT1, event, 11);
> + if (ret) {
> + dev_err(tps->dev, "%s: failed to read events\n", __func__);
> + goto err_unlock;
> + }
> +
> + if (!(event[0] | event[1]))
> + goto err_unlock;
> +
> + if (!tps6598x_read_status(tps, &status))
> + goto err_clear_ints;
> +
> + if ((event[0] | event[1]) & TPS_REG_INT_POWER_STATUS_UPDATE)
> + if (!tps6598x_read_power_status(tps))
> + goto err_clear_ints;
> +
> + if ((event[0] | event[1]) & TPS_REG_INT_DATA_STATUS_UPDATE)
> + if (!tps6598x_read_data_status(tps))
> + goto err_clear_ints;
> +
> + /*
> + * data/port roles could be updated independently after
> + * a plug event. Therefore, we need to check
> + * for pr/dr status change to set TypeC dr/pr accordingly.
> + */
> + if ((event[0] | event[1]) & TPS_REG_INT_PLUG_EVENT ||
> + tps6598x_has_role_changed(tps, status))
> + tps6598x_handle_plug_event(tps, status);
> +
> + tps->status = status;
> +
> +err_clear_ints:
> + tps6598x_block_write(tps, TPS_REG_INT_CLEAR1, event, 11);
> +
> +err_unlock:
> + mutex_unlock(&tps->lock);
> +
> + if (event[0] | event[1])
> + return IRQ_HANDLED;
> + return IRQ_NONE;
> +}
> +
> static irqreturn_t tps6598x_interrupt(int irq, void *data)
> {
> struct tps6598x *tps = data;
> @@ -600,7 +665,7 @@ static void tps6598x_poll_work(struct work_struct *work)
> struct tps6598x *tps = container_of(to_delayed_work(work),
> struct tps6598x, wq_poll);
>
> - tps->irq_handler(0, tps);
> + tps->data->irq_handler(0, tps);
> queue_delayed_work(system_power_efficient_wq,
> &tps->wq_poll, msecs_to_jiffies(POLL_INTERVAL));
> }
> @@ -969,7 +1034,6 @@ static int tps25750_apply_patch(struct tps6598x *tps)
>
> static int tps6598x_probe(struct i2c_client *client)
> {
> - irq_handler_t irq_handler = tps6598x_interrupt;
> struct device_node *np = client->dev.of_node;
> struct typec_capability typec_cap = { };
> struct tps6598x *tps;
> @@ -1017,7 +1081,6 @@ static int tps6598x_probe(struct i2c_client *client)
> 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 */
> mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
> @@ -1025,7 +1088,10 @@ static int tps6598x_probe(struct i2c_client *client)
> TPS_REG_INT_PLUG_EVENT;
> }
>
> - tps->irq_handler = irq_handler;
> + tps->data = device_get_match_data(tps->dev);
> + if (!tps->data)
> + return -EINVAL;
> +
> /* Make sure the controller has application firmware running */
> ret = tps6598x_check_mode(tps);
> if (ret < 0)
> @@ -1125,7 +1191,7 @@ static int tps6598x_probe(struct i2c_client *client)
>
> if (client->irq) {
> ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> - irq_handler,
> + tps->data->irq_handler,
> IRQF_SHARED | IRQF_ONESHOT,
> dev_name(&client->dev), tps);
> } else {
> @@ -1231,10 +1297,22 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(tps6598x_suspend, tps6598x_resume)
> };
>
> +static const struct tipd_data cd321x_data = {
> + .irq_handler = cd321x_interrupt,
> +};
> +
> +static const struct tipd_data tps6598x_data = {
> + .irq_handler = tps6598x_interrupt,
> +};
> +
> +static const struct tipd_data tps25750_data = {
> + .irq_handler = tps25750_interrupt,
> +};
> +
> static const struct of_device_id tps6598x_of_match[] = {
> - { .compatible = "ti,tps6598x", },
> - { .compatible = "apple,cd321x", },
> - { .compatible = "ti,tps25750", },
> + { .compatible = "ti,tps6598x", &tps6598x_data},
> + { .compatible = "apple,cd321x", &cd321x_data},
> + { .compatible = "ti,tps25750", &tps25750_data},
> {}
> };
> MODULE_DEVICE_TABLE(of, tps6598x_of_match);
> --
> 2.34.1
--
heikki
On Sun, Oct 01, 2023 at 04:11:29AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> tps6598x and cd321x use TPS_REG_SYSTEM_CONF to get dr/pr roles
> where other similar devices don't have this register such as tps25750.
>
> Move tps6598x port registration to its own function
>
> Signed-off-by: Abdel Alkuor <[email protected]>
This one can be moved to the beginning of the series.
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> Changes in v9:
> - No changes
> Changes in v8:
> - No changes
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - No changes
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
>
> drivers/usb/typec/tipd/core.c | 99 +++++++++++++++++++----------------
> 1 file changed, 54 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 52dc1cc16bed..0195eabd96bf 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1032,14 +1032,64 @@ static int tps25750_apply_patch(struct tps6598x *tps)
> return 0;
> };
>
> +static int
> +tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
> +{
> + int ret;
> + u32 conf;
> + struct typec_capability typec_cap = { };
> +
> + ret = tps6598x_read32(tps, TPS_REG_SYSTEM_CONF, &conf);
> + if (ret)
> + return ret;
> +
> + typec_cap.revision = USB_TYPEC_REV_1_2;
> + typec_cap.pd_revision = 0x200;
> + typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> + typec_cap.driver_data = tps;
> + typec_cap.ops = &tps6598x_ops;
> + typec_cap.fwnode = fwnode;
> +
> + switch (TPS_SYSCONF_PORTINFO(conf)) {
> + case TPS_PORTINFO_SINK_ACCESSORY:
> + case TPS_PORTINFO_SINK:
> + typec_cap.type = TYPEC_PORT_SNK;
> + typec_cap.data = TYPEC_PORT_UFP;
> + break;
> + case TPS_PORTINFO_DRP_UFP_DRD:
> + case TPS_PORTINFO_DRP_DFP_DRD:
> + typec_cap.type = TYPEC_PORT_DRP;
> + typec_cap.data = TYPEC_PORT_DRD;
> + break;
> + case TPS_PORTINFO_DRP_UFP:
> + typec_cap.type = TYPEC_PORT_DRP;
> + typec_cap.data = TYPEC_PORT_UFP;
> + break;
> + case TPS_PORTINFO_DRP_DFP:
> + typec_cap.type = TYPEC_PORT_DRP;
> + typec_cap.data = TYPEC_PORT_DFP;
> + break;
> + case TPS_PORTINFO_SOURCE:
> + typec_cap.type = TYPEC_PORT_SRC;
> + typec_cap.data = TYPEC_PORT_DFP;
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + tps->port = typec_register_port(tps->dev, &typec_cap);
> + if (IS_ERR(tps->port))
> + return PTR_ERR(tps->port);
> +
> + return 0;
> +}
> +
> static int tps6598x_probe(struct i2c_client *client)
> {
> struct device_node *np = client->dev.of_node;
> - struct typec_capability typec_cap = { };
> struct tps6598x *tps;
> struct fwnode_handle *fwnode;
> u32 status;
> - u32 conf;
> u32 vid;
> int ret;
> u64 mask1;
> @@ -1112,10 +1162,6 @@ static int tps6598x_probe(struct i2c_client *client)
> goto err_clear_mask;
> trace_tps6598x_status(status);
>
> - ret = tps6598x_read32(tps, TPS_REG_SYSTEM_CONF, &conf);
> - if (ret < 0)
> - goto err_clear_mask;
> -
> /*
> * This fwnode has a "compatible" property, but is never populated as a
> * struct device. Instead we simply parse it to read the properties.
> @@ -1133,50 +1179,13 @@ static int tps6598x_probe(struct i2c_client *client)
> goto err_fwnode_put;
> }
>
> - typec_cap.revision = USB_TYPEC_REV_1_2;
> - typec_cap.pd_revision = 0x200;
> - typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> - typec_cap.driver_data = tps;
> - typec_cap.ops = &tps6598x_ops;
> - typec_cap.fwnode = fwnode;
> -
> - switch (TPS_SYSCONF_PORTINFO(conf)) {
> - case TPS_PORTINFO_SINK_ACCESSORY:
> - case TPS_PORTINFO_SINK:
> - typec_cap.type = TYPEC_PORT_SNK;
> - typec_cap.data = TYPEC_PORT_UFP;
> - break;
> - case TPS_PORTINFO_DRP_UFP_DRD:
> - case TPS_PORTINFO_DRP_DFP_DRD:
> - typec_cap.type = TYPEC_PORT_DRP;
> - typec_cap.data = TYPEC_PORT_DRD;
> - break;
> - case TPS_PORTINFO_DRP_UFP:
> - typec_cap.type = TYPEC_PORT_DRP;
> - typec_cap.data = TYPEC_PORT_UFP;
> - break;
> - case TPS_PORTINFO_DRP_DFP:
> - typec_cap.type = TYPEC_PORT_DRP;
> - typec_cap.data = TYPEC_PORT_DFP;
> - break;
> - case TPS_PORTINFO_SOURCE:
> - typec_cap.type = TYPEC_PORT_SRC;
> - typec_cap.data = TYPEC_PORT_DFP;
> - break;
> - default:
> - ret = -ENODEV;
> - goto err_role_put;
> - }
> -
> ret = devm_tps6598_psy_register(tps);
> if (ret)
> goto err_role_put;
>
> - tps->port = typec_register_port(&client->dev, &typec_cap);
> - if (IS_ERR(tps->port)) {
> - ret = PTR_ERR(tps->port);
> + ret = tps6598x_register_port(tps, fwnode);
> + if (ret)
> goto err_role_put;
> - }
>
> if (status & TPS_STATUS_PLUG_PRESENT) {
> ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &tps->pwr_status);
> --
> 2.34.1
--
heikki
On Sun, Oct 01, 2023 at 04:11:30AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> TPS25750 doesn't have system configuration register to get dr/pr of the
> current applied binary configuration.
>
> Get data role from the device node and power role from PD status register.
>
> Signed-off-by: Abdel Alkuor <[email protected]>
This also has to be done at the same time you enable TPS25750.
> ---
> Changes in v9:
> - Move device data structs to the top of of_device_id
> Changes in v8:
> - Change tps->cb to tps->data
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - Add port registration to tipd data factory
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
>
> drivers/usb/typec/tipd/core.c | 68 ++++++++++++++++++++++++++++++-
> drivers/usb/typec/tipd/tps6598x.h | 10 +++++
> 2 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 0195eabd96bf..ae08c5306707 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -40,6 +40,7 @@
> #define TPS_REG_CTRL_CONF 0x29
> #define TPS_REG_BOOT_STATUS 0x2D
> #define TPS_REG_POWER_STATUS 0x3f
> +#define TPS_REG_PD_STATUS 0x40
> #define TPS_REG_RX_IDENTITY_SOP 0x48
> #define TPS_REG_DATA_STATUS 0x5f
>
> @@ -102,8 +103,11 @@ static const char *const modes[] = {
> /* Unrecognized commands will be replaced with "!CMD" */
> #define INVALID_CMD(_cmd_) (_cmd_ == 0x444d4321)
>
> +struct tps6598x;
> +
> struct tipd_data {
> irq_handler_t irq_handler;
> + int (*register_port)(struct tps6598x *tps, struct fwnode_handle *node);
> };
>
> struct tps6598x {
> @@ -208,6 +212,11 @@ static inline int tps6598x_read64(struct tps6598x *tps, u8 reg, u64 *val)
> return tps6598x_block_read(tps, reg, val, sizeof(u64));
> }
>
> +static inline int tps6598x_write8(struct tps6598x *tps, u8 reg, u8 val)
> +{
> + return tps6598x_block_write(tps, reg, &val, sizeof(u8));
> +}
> +
> static inline int tps6598x_write64(struct tps6598x *tps, u8 reg, u64 val)
> {
> return tps6598x_block_write(tps, reg, &val, sizeof(u64));
> @@ -1084,6 +1093,60 @@ tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
> return 0;
> }
>
> +static int
> +tps25750_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
> +{
> + struct typec_capability typec_cap = { };
> + const char *data_role;
> + u8 pd_status;
> + int ret;
> +
> + ret = tps6598x_read8(tps, TPS_REG_PD_STATUS, &pd_status);
> + if (ret)
> + return ret;
> +
> + ret = fwnode_property_read_string(fwnode, "data-role", &data_role);
> + if (ret) {
> + dev_err(tps->dev, "data-role not found: %d\n", ret);
> + return ret;
> + }
> +
> + ret = typec_find_port_data_role(data_role);
> + if (ret < 0) {
> + dev_err(tps->dev, "unknown data-role: %s\n", data_role);
> + return ret;
> + }
> +
> + typec_cap.data = ret;
> + typec_cap.revision = USB_TYPEC_REV_1_3;
> + typec_cap.pd_revision = 0x300;
> + typec_cap.driver_data = tps;
> + typec_cap.ops = &tps6598x_ops;
> + typec_cap.fwnode = fwnode;
> + typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> +
> + switch (TPS_PD_STATUS_PORT_TYPE(pd_status)) {
> + case TPS_PD_STATUS_PORT_TYPE_SINK_SOURCE:
> + case TPS_PD_STATUS_PORT_TYPE_SOURCE_SINK:
> + typec_cap.type = TYPEC_PORT_DRP;
> + break;
> + case TPS_PD_STATUS_PORT_TYPE_SINK:
> + typec_cap.type = TYPEC_PORT_SNK;
> + break;
> + case TPS_PD_STATUS_PORT_TYPE_SOURCE:
> + typec_cap.type = TYPEC_PORT_SRC;
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + tps->port = typec_register_port(tps->dev, &typec_cap);
> + if (IS_ERR(tps->port))
> + return PTR_ERR(tps->port);
> +
> + return 0;
> +}
> +
> static int tps6598x_probe(struct i2c_client *client)
> {
> struct device_node *np = client->dev.of_node;
> @@ -1183,7 +1246,7 @@ static int tps6598x_probe(struct i2c_client *client)
> if (ret)
> goto err_role_put;
>
> - ret = tps6598x_register_port(tps, fwnode);
> + ret = tps->data->register_port(tps, fwnode);
> if (ret)
> goto err_role_put;
>
> @@ -1308,14 +1371,17 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
>
> static const struct tipd_data cd321x_data = {
> .irq_handler = cd321x_interrupt,
> + .register_port = tps6598x_register_port,
> };
>
> static const struct tipd_data tps6598x_data = {
> .irq_handler = tps6598x_interrupt,
> + .register_port = tps6598x_register_port,
> };
>
> static const struct tipd_data tps25750_data = {
> .irq_handler = tps25750_interrupt,
> + .register_port = tps25750_register_port,
> };
>
> static const struct of_device_id tps6598x_of_match[] = {
> diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
> index c000170f4547..3a9a43394134 100644
> --- a/drivers/usb/typec/tipd/tps6598x.h
> +++ b/drivers/usb/typec/tipd/tps6598x.h
> @@ -203,4 +203,14 @@
> #define TPS_BOOT_STATUS_DEAD_BATTERY_FLAG BIT(2)
> #define TPS_BOOT_STATUS_I2C_EEPROM_PRESENT BIT(3)
>
> +/* PD STATUS REG */
> +#define TPS_REG_PD_STATUS_PORT_TYPE_MASK GENMASK(5, 4)
> +#define TPS_PD_STATUS_PORT_TYPE(x) \
> + TPS_FIELD_GET(TPS_REG_PD_STATUS_PORT_TYPE_MASK, x)
> +
> +#define TPS_PD_STATUS_PORT_TYPE_SINK_SOURCE 0
> +#define TPS_PD_STATUS_PORT_TYPE_SINK 1
> +#define TPS_PD_STATUS_PORT_TYPE_SOURCE 2
> +#define TPS_PD_STATUS_PORT_TYPE_SOURCE_SINK 3
> +
> #endif /* __TPS6598X_H__ */
> --
> 2.34.1
--
heikki
Hi,
One more comment below.
On Sun, Oct 01, 2023 at 04:11:24AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> TPS25750 controller requires a binary to be loaded with a configuration
> binary by an EEPROM or a host.
>
> Appling a patch bundling using a host is implemented based on the flow
> diagram pg.62 in TPS25750 host interface manual.
> https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
>
> The flow diagram can be summarized as following:
> - Start the patch loading sequence with patch bundle information by
> executing PBMs
> - Write the whole patch at once
> - When writing the patch fails, execute PBMe which instructs the PD controller
> to end the patching process
> - After writing the patch successfully, execute PBMc which verifies the patch
> integrity and applies the patch internally
> - Wait for the device to switch into APP mode (normal operation)
>
> The execuation flow diagram polls the events register and then polls the
> corresponding register related to the event as well before advancing to the next
> state. Polling the events register is a redundant step, in this implementation
> only the corresponding register related to the event is polled.
>
> Signed-off-by: Abdel Alkuor <[email protected]>
> ---
> Changes in v9:
> - No changes
> Changes in v8:
> - Return mode when mode is checked
> - Use device_is_compatible instead of of_device_is_compatible
> Changes in v7:
> - Add driver name to commit subject
> Changes in v6:
> - Don't check VID for tps25750 as the VID register doesn't exist
> - Remove is_tps25750 flag from tps6598x struct
> - Get patch address from reg property
> Changes in v5:
> - Incorporating tps25750 into tps6598x driver
> drivers/usb/typec/tipd/core.c | 263 ++++++++++++++++++++++++++++++++--
> 1 file changed, 255 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index c5bbf03cb74a..2e7b9eafaf04 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -17,6 +17,7 @@
> #include <linux/usb/typec_altmode.h>
> #include <linux/usb/role.h>
> #include <linux/workqueue.h>
> +#include <linux/firmware.h>
>
> #include "tps6598x.h"
> #include "trace.h"
> @@ -43,6 +44,23 @@
> /* TPS_REG_SYSTEM_CONF bits */
> #define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
>
> +/*
> + * BPMs task timeout, recommended 5 seconds
> + * pg.48 TPS2575 Host Interface Technical Reference
> + * Manual (Rev. A)
> + * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
> + */
> +#define TPS_BUNDLE_TIMEOUT 0x32
> +
> +/* BPMs return code */
> +#define TPS_TASK_BPMS_INVALID_BUNDLE_SIZE 0x4
> +#define TPS_TASK_BPMS_INVALID_SLAVE_ADDR 0x5
> +#define TPS_TASK_BPMS_INVALID_TIMEOUT 0x6
> +
> +/* PBMc data out */
> +#define TPS_PBMC_RC 0 /* Return code */
> +#define TPS_PBMC_DPCS 2 /* device patch complete status */
> +
> enum {
> TPS_PORTINFO_SINK,
> TPS_PORTINFO_SINK_ACCESSORY,
> @@ -595,13 +613,15 @@ static int tps6598x_check_mode(struct tps6598x *tps)
> if (ret)
> return ret;
>
> - switch (match_string(modes, ARRAY_SIZE(modes), mode)) {
> + ret = match_string(modes, ARRAY_SIZE(modes), mode);
> +
> + switch (ret) {
> case TPS_MODE_APP:
> case TPS_MODE_PTCH:
> - return 0;
> + return ret;
> case TPS_MODE_BOOT:
> dev_warn(tps->dev, "dead-battery condition\n");
> - return 0;
> + return ret;
> case TPS_MODE_BIST:
> case TPS_MODE_DISC:
> default:
> @@ -711,6 +731,213 @@ static int devm_tps6598_psy_register(struct tps6598x *tps)
> return PTR_ERR_OR_ZERO(tps->psy);
> }
>
> +static int
> +tps25750_write_firmware(struct tps6598x *tps,
> + u8 bpms_addr, const u8 *data, size_t len)
> +{
> + struct i2c_client *client = to_i2c_client(tps->dev);
> + int ret;
> + u8 slave_addr;
> + int timeout;
> +
> + slave_addr = client->addr;
> + timeout = client->adapter->timeout;
> +
> + /*
> + * binary configuration size is around ~16Kbytes
> + * which might take some time to finish writing it
> + */
> + client->adapter->timeout = msecs_to_jiffies(5000);
> + client->addr = bpms_addr;
> +
> + ret = regmap_raw_write(tps->regmap, data[0], &data[1], len - 1);
> +
> + client->addr = slave_addr;
> + client->adapter->timeout = timeout;
> +
> + return ret;
> +}
> +
> +static int
> +tps25750_exec_pbms(struct tps6598x *tps, u8 *in_data, size_t in_len)
> +{
> + int ret;
> + u8 rc;
> +
> + ret = tps6598x_exec_cmd_tmo(tps, "PBMs", in_len, in_data,
> + sizeof(rc), &rc, 4000, 0);
> + if (ret)
> + return ret;
> +
> + switch (rc) {
> + case TPS_TASK_BPMS_INVALID_BUNDLE_SIZE:
> + dev_err(tps->dev, "%s: invalid fw size\n", __func__);
> + return -EINVAL;
> + case TPS_TASK_BPMS_INVALID_SLAVE_ADDR:
> + dev_err(tps->dev, "%s: invalid slave address\n", __func__);
> + return -EINVAL;
> + case TPS_TASK_BPMS_INVALID_TIMEOUT:
> + dev_err(tps->dev, "%s: timed out\n", __func__);
> + return -ETIMEDOUT;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int tps25750_abort_patch_process(struct tps6598x *tps)
> +{
> + int ret;
> +
> + ret = tps6598x_exec_cmd(tps, "PBMe", 0, NULL, 0, NULL);
> + if (ret)
> + return ret;
> +
> + ret = tps6598x_check_mode(tps);
> + if (ret != TPS_MODE_PTCH)
> + dev_err(tps->dev, "failed to switch to \"PTCH\" mode\n");
> +
> + return ret;
> +}
> +
> +static int tps25750_start_patch_burst_mode(struct tps6598x *tps)
> +{
> + int ret;
> + const struct firmware *fw;
> + const char *firmware_name;
> + struct {
> + u32 fw_size;
> + u8 addr;
> + u8 timeout;
> + } __packed bpms_data;
> + u32 addr;
> + struct device_node *np = tps->dev->of_node;
> +
> + ret = device_property_read_string(tps->dev, "firmware-name",
> + &firmware_name);
> + if (ret)
> + return ret;
> +
> + ret = request_firmware(&fw, firmware_name, tps->dev);
> + if (ret) {
> + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name);
> + return ret;
> + }
> +
> + if (fw->size == 0) {
> + ret = -EINVAL;
> + goto release_fw;
> + }
> +
> + ret = of_property_match_string(np, "reg-names", "patch-address");
> + if (ret < 0) {
> + dev_err(tps->dev, "failed to get patch-address %d\n", ret);
> + return ret;
> + }
> +
> + ret = of_property_read_u32_index(np, "reg", ret, &addr);
> + if (ret)
> + return ret;
> +
> + if (addr == 0 || (addr >= 0x20 && addr <= 0x23)) {
> + dev_err(tps->dev, "wrong patch address %u\n", addr);
> + return -EINVAL;
> + }
> +
> + bpms_data.addr = (u8)addr;
> + bpms_data.fw_size = fw->size;
> + bpms_data.timeout = TPS_BUNDLE_TIMEOUT;
> +
> + ret = tps25750_exec_pbms(tps, (u8 *)&bpms_data, sizeof(bpms_data));
> + if (ret)
> + goto release_fw;
> +
> + ret = tps25750_write_firmware(tps, bpms_data.addr, fw->data, fw->size);
> + if (ret) {
> + dev_err(tps->dev, "Failed to write patch %s of %zu bytes\n",
> + firmware_name, fw->size);
> + goto release_fw;
> + }
> +
> + /*
> + * A delay of 500us is required after the firmware is written
> + * based on pg.62 in tps6598x Host Interface Technical
> + * Reference Manual
> + * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
> + */
> + udelay(500);
> +
> +release_fw:
> + release_firmware(fw);
> +
> + return ret;
> +}
> +
> +static int tps25750_complete_patch_process(struct tps6598x *tps)
> +{
> + int ret;
> + u8 out_data[40];
> + u8 dummy[2] = { };
> +
> + /*
> + * Without writing something to DATA_IN, this command would
> + * return an error
> + */
> + ret = tps6598x_exec_cmd_tmo(tps, "PBMc", sizeof(dummy), dummy,
> + sizeof(out_data), out_data, 2000, 20);
> + if (ret)
> + return ret;
> +
> + if (out_data[TPS_PBMC_RC]) {
> + dev_err(tps->dev,
> + "%s: pbmc failed: %u\n", __func__,
> + out_data[TPS_PBMC_RC]);
> + return -EIO;
> + }
> +
> + if (out_data[TPS_PBMC_DPCS]) {
> + dev_err(tps->dev,
> + "%s: failed device patch complete status: %u\n",
> + __func__, out_data[TPS_PBMC_DPCS]);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int tps25750_apply_patch(struct tps6598x *tps)
> +{
> + int ret;
> + unsigned long timeout;
> +
> + ret = tps25750_start_patch_burst_mode(tps);
> + if (ret) {
> + tps25750_abort_patch_process(tps);
> + return ret;
> + }
> +
> + ret = tps25750_complete_patch_process(tps);
> + if (ret)
> + return ret;
> +
> + timeout = jiffies + msecs_to_jiffies(1000);
> +
> + do {
> + ret = tps6598x_check_mode(tps);
> + if (ret < 0)
> + return ret;
> +
> + if (time_is_before_jiffies(timeout))
> + return -ETIMEDOUT;
> +
> + } while (ret != TPS_MODE_APP);
> +
> + dev_info(tps->dev, "controller switched to \"APP\" mode\n");
> +
> + return 0;
> +};
> +
> static int tps6598x_probe(struct i2c_client *client)
> {
> irq_handler_t irq_handler = tps6598x_interrupt;
> @@ -723,6 +950,7 @@ static int tps6598x_probe(struct i2c_client *client)
> u32 vid;
> int ret;
> u64 mask1;
> + bool is_tps25750;
>
> tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
> if (!tps)
> @@ -735,9 +963,12 @@ static int tps6598x_probe(struct i2c_client *client)
> if (IS_ERR(tps->regmap))
> return PTR_ERR(tps->regmap);
>
> - ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
> - if (ret < 0 || !vid)
> - return -ENODEV;
> + is_tps25750 = device_is_compatible(tps->dev, "ti,tps25750");
> + if (!is_tps25750) {
> + ret = tps6598x_read32(tps, TPS_REG_VID, &vid);
> + if (ret < 0 || !vid)
> + return -ENODEV;
> + }
>
> /*
> * Checking can the adapter handle SMBus protocol. If it can not, the
> @@ -768,12 +999,18 @@ static int tps6598x_probe(struct i2c_client *client)
> tps->irq_handler = irq_handler;
> /* Make sure the controller has application firmware running */
> ret = tps6598x_check_mode(tps);
> - if (ret)
> + if (ret < 0)
> return ret;
>
> + if (is_tps25750 && ret == TPS_MODE_PTCH) {
> + ret = tps25750_apply_patch(tps);
> + if (ret)
> + return ret;
> + }
Make apply_patch a callback in struct tipd_data.
thanks,
> ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, mask1);
> if (ret)
> - return ret;
> + goto err_reset_controller;
>
> ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
> if (ret < 0)
> @@ -893,6 +1130,10 @@ static int tps6598x_probe(struct i2c_client *client)
> fwnode_handle_put(fwnode);
> err_clear_mask:
> tps6598x_write64(tps, TPS_REG_INT_MASK1, 0);
> +err_reset_controller:
> + /* Reset PD controller to remove any applied patch */
> + if (is_tps25750)
> + tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> return ret;
> }
>
> @@ -903,9 +1144,14 @@ static void tps6598x_remove(struct i2c_client *client)
> if (!client->irq)
> cancel_delayed_work_sync(&tps->wq_poll);
>
> + devm_free_irq(tps->dev, client->irq, tps);
> tps6598x_disconnect(tps, 0);
> typec_unregister_port(tps->port);
> usb_role_switch_put(tps->role_sw);
> +
> + /* Reset PD controller to remove any applied patch */
> + if (device_is_compatible(tps->dev, "ti,tps25750"))
> + tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> }
>
> static int __maybe_unused tps6598x_suspend(struct device *dev)
> @@ -948,6 +1194,7 @@ static const struct dev_pm_ops tps6598x_pm_ops = {
> static const struct of_device_id tps6598x_of_match[] = {
> { .compatible = "ti,tps6598x", },
> { .compatible = "apple,cd321x", },
> + { .compatible = "ti,tps25750", },
> {}
> };
> MODULE_DEVICE_TABLE(of, tps6598x_of_match);
> --
> 2.34.1
--
heikki
On Tue, Oct 03, 2023 at 08:34:11AM +0300, Heikki Krogerus wrote:
> On Sun, Oct 01, 2023 at 04:11:25AM -0400, Abdel Alkuor wrote:
> > From: Abdel Alkuor <[email protected]>
> >
> > When an EEPROM is present, tps25750 loads the binary configuration from
> > EEPROM. Hence, all we need to do is wait for the device to switch to APP
> > mode
> >
> > Signed-off-by: Abdel Alkuor <[email protected]>
>
> I'm not sure I understand why this needs separate patch, but in any
> case:
>
> Reviewed-by: Heikki Krogerus <[email protected]>
>
I will move it to be part of the patch that loads the config patch and enables tps25750
Thanks,
Abdel
> --
> heikki
On Tue, Oct 03, 2023 at 09:03:42AM +0300, Heikki Krogerus wrote:
> On Sun, Oct 01, 2023 at 04:11:26AM -0400, Abdel Alkuor wrote:
> > From: Abdel Alkuor <[email protected]>
> >
> > Dead battery flag must be cleared after switching tps25750 to APP mode
> > so the PD controller becomes fully functional.
> >
> > Signed-off-by: Abdel Alkuor <[email protected]>
>
> I'm sorry I noticed these so late, but this one really feels like it
> should be part of the patch 4/14. Is there some reason why you do this
> separately?
>
That's a good. There is no actual reason, it was just simply part of the
implementation flow. I will move it to be part the patch that supports
tps25750 implementation
Thanks,
Abdel
> Br,
>
> --
> heikki
On Tue, Oct 03, 2023 at 09:21:08AM +0300, Heikki Krogerus wrote:
> On Sun, Oct 01, 2023 at 04:11:27AM -0400, Abdel Alkuor wrote:
> > From: Abdel Alkuor <[email protected]>
> >
> > TPS25750 PD controller might be powered off externally at power suspend,
> > after resuming PD controller power back, apply the patch again.
> >
> > Signed-off-by: Abdel Alkuor <[email protected]>
>
> This one looks also like something that should be part of the patch 4.
>
> My concern is that with these separated features you are creating points
> into the kernel git tree where TPS25750 is enabled, but it's not fully
> functional, or even broken in scenarious like this (suspend/resume).
> You can't do that unless you have some really good reason.
>
> Since all of these add only a bit of code each, I think it would be
> better to just merge these into the initial patch that enabled
> TPS25750 - so I belive patch 4/14.
>
Makes a lot of sense. I will add part of full tps25750 support patch
> --
> heikki
Thanks,
Abdel
On Tue, Oct 03, 2023 at 09:39:59AM +0300, Heikki Krogerus wrote:
> On Sun, Oct 01, 2023 at 04:11:28AM -0400, Abdel Alkuor wrote:
> > From: Abdel Alkuor <[email protected]>
> >
> > tps25750 event registers structure is different than tps6598x's,
> > tps25750 has 11 bytes of events which are read at once where
> > tps6598x has two event registers of 8 bytes each which are read
> > separately. Likewise MASK event registers. Also, not all events
> > are supported in both devices.
> >
> > - Create a new handler to accommodate tps25750 interrupt
> > - Add device data to of_device_id
> >
> > Signed-off-by: Abdel Alkuor <[email protected]>
>
> I'm sorry, but I just realised that this one also has to be in place
> by the time TPS25750 is enabled in patch 4/14. Otherwise the series is
> not bisectable.
>
> I think you need to refactor the patch order a bit:
>
> First come the patches that prepare everything that needs preparing -
> introduction of struct tipd_data (without anything TPS25750 specific),
> and so on.
>
> Then you have patches for things that are specific to TPS25750 (small
> stuff just merge together) if needed.
>
> In the very last patches you finally enable TPS25750.
>
No worries. I will aggregate all the patches that are related to
tps25750 into one patch that supports tps25750 implementation and enable
it. The remainig patches after enabling tps25750 are to add trace
support.
Thanks,
Abdel