| Message ID | 20230927175348.18041-1-alkuor@gmail.com |
|---|---|
| Headers | show |
| Series | Add TPS25750 USB type-C PD controller support | expand |
On Wed, Sep 27, 2023 at 01:53:36PM -0400, Abdel Alkuor wrote: > From: Abdel Alkuor <abdelalkuor@geotab.com> > > 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 <abdelalkuor@geotab.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > 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
On Wed, Sep 27, 2023 at 01:53:37PM -0400, Abdel Alkuor wrote: > From: Abdel Alkuor <abdelalkuor@geotab.com> > > TPS25750 has a patch mode indicating the device requires > a configuration to get the device into operational mode > > Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com> > --- > Changes in v7: > - Add driver name to commit subject > Changes in v6: > - Return current mode and check it directly > Changes in v5: > - Incorporating tps25750 into tps6598x driver > > drivers/usb/typec/tipd/core.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index 32420c61660d..58679b1c0cfe 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -68,6 +68,7 @@ enum { > TPS_MODE_BOOT, > TPS_MODE_BIST, > TPS_MODE_DISC, > + TPS_MODE_PTCH, > }; > > static const char *const modes[] = { > @@ -75,6 +76,7 @@ static const char *const modes[] = { > [TPS_MODE_BOOT] = "BOOT", > [TPS_MODE_BIST] = "BIST", > [TPS_MODE_DISC] = "DISC", > + [TPS_MODE_PTCH] = "PTCH", > }; > > /* Unrecognized commands will be replaced with "!CMD" */ > @@ -593,12 +595,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: > - return 0; > + case TPS_MODE_PTCH: > + 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: > @@ -765,7 +770,7 @@ 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; You are doing two things in this patch - you are adding PTCH mode, and changing the meaning of the tps6598x_check_mode() return value. Please make a note also about the latter in the commit message. Or, just return the mode in the patch were you start using it in tps6598x_probe(). Br,
On Wed, Sep 27, 2023 at 01:53:38PM -0400, Abdel Alkuor wrote: > From: Abdel Alkuor <abdelalkuor@geotab.com> > > 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 <abdelalkuor@geotab.com> This looks okay, but let's see how the final DT bindings look like. Few nitpicks below: > --- > 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 | 254 +++++++++++++++++++++++++++++++++- > 1 file changed, 250 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index 58679b1c0cfe..f96a9ff07fba 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, > @@ -713,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; > @@ -725,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) > @@ -737,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 = of_device_is_compatible(np, "ti,tps25750"); Please use the unified device property interface everywhere possible so the support is ready for ACPI too. So: 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 > @@ -773,9 +1002,15 @@ static int tps6598x_probe(struct i2c_client *client) > 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) > @@ -895,19 +1130,29 @@ 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; > } > > static void tps6598x_remove(struct i2c_client *client) > { > struct tps6598x *tps = i2c_get_clientdata(client); > + struct device_node *np = client->dev.of_node; > > 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 (of_device_is_compatible(np, "ti,tps25750")) Ditto. > + tps6598x_exec_cmd_tmo(tps, "GAID", 0, NULL, 0, NULL, 2000, 0); > } > > static int __maybe_unused tps6598x_suspend(struct device *dev) > @@ -950,6 +1195,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); thanks,
On Wed, Sep 27, 2023 at 01:53:42PM -0400, Abdel Alkuor wrote: > From: Abdel Alkuor <abdelalkuor@geotab.com> > > 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 > > Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com> > --- > 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 | 90 ++++++++++++++++++++++++++++++++--- > 1 file changed, 84 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index 56ffffe225f2..40a76826a44e 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -101,6 +101,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 +122,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; > + > + struct tipd_data cb; > }; > > static enum power_supply_property tps6598x_psy_props[] = { > @@ -545,6 +551,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 +664,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->cb.irq_handler(0, tps); > queue_delayed_work(system_power_efficient_wq, > &tps->wq_poll, msecs_to_jiffies(POLL_INTERVAL)); > } > @@ -967,9 +1031,20 @@ static int tps25750_apply_patch(struct tps6598x *tps) > return 0; > }; > > +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 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,15 +1092,18 @@ 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; > + tps->cb = cd321x_data; > } else { > + if (is_tps25750) > + tps->cb = tps25750_data; > + else > + tps->cb = tps6598x_data; No. You assing those to the "data" member of your of_device_id: static const struct of_device_id tps6598x_of_match[] = { { .compatible = "apple,cd321x", .data = &cd321_data }, { .compatible = "ti,tps25750", .data = &tps25750_data }, { .compatible = "ti,tps6598x", .data = &tps6598x_data }, { } }; thanks,
From: Abdel Alkuor <abdelalkuor@geotab.com> 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. 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 | 622 ++++++++++++++++-- drivers/usb/typec/tipd/tps6598x.h | 36 + drivers/usb/typec/tipd/trace.h | 92 +++ 4 files changed, 762 insertions(+), 69 deletions(-)