2023-09-17 23:10:16

by Abdel Alkuor

[permalink] [raw]
Subject: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle

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]>
---
drivers/usb/typec/tipd/core.c | 237 +++++++++++++++++++++++++++++++++-
1 file changed, 236 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 6d2151325fbb..fea139c72d6d 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,
@@ -88,6 +106,8 @@ struct tps6598x {
struct mutex lock; /* device lock */
u8 i2c_protocol:1;

+ u8 is_tps25750:1;
+
struct typec_port *port;
struct typec_partner *partner;
struct usb_pd_identity partner_identity;
@@ -708,6 +728,203 @@ 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(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;
+ u8 mode;
+
+ ret = tps6598x_exec_cmd(tps, "PBMe", 0, NULL, 0, NULL, 1000, 0);
+ if (ret)
+ return ret;
+
+ ret = tps6598x_check_mode(tps, &mode);
+ if (mode != 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;
+
+ 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 = device_property_read_u8(tps->dev, "ti,patch-address", &bpms_data.addr);
+ if (ret) {
+ dev_err(tps->dev, "failed to get patch address\n");
+ return ret;
+ }
+
+ 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 %lu 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(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;
+ u8 mode;
+
+ 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, &mode);
+ if (ret)
+ return ret;
+
+ if (time_is_before_jiffies(timeout))
+ return -ETIMEDOUT;
+
+ } while (mode != 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;
@@ -757,6 +974,8 @@ static int tps6598x_probe(struct i2c_client *client)

irq_handler = cd321x_interrupt;
} else {
+
+ tps->is_tps25750 = of_device_is_compatible(np, "ti,tps25750");
/* Enable power status, data status and plug event interrupts */
mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
TPS_REG_INT_DATA_STATUS_UPDATE |
@@ -769,9 +988,15 @@ static int tps6598x_probe(struct i2c_client *client)
if (ret)
return ret;

+ if (tps->is_tps25750 && mode == 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)
@@ -891,6 +1116,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 (tps->is_tps25750)
+ tps6598x_exec_cmd(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
return ret;
}

@@ -901,9 +1130,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 (tps->is_tps25750)
+ tps6598x_exec_cmd(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
}

static int __maybe_unused tps6598x_suspend(struct device *dev)
@@ -946,6 +1180,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


2023-09-18 00:37:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle

Hi Abdel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on linus/master v6.6-rc1 next-20230915]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Abdel-Alkuor/dt-bindings-usb-tps6598x-Add-tps25750/20230917-233037
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230917152639.21443-5-alkuor%40gmail.com
patch subject: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230918/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230918/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from include/linux/device.h:15,
from include/linux/acpi.h:14,
from include/linux/i2c.h:13,
from drivers/usb/typec/tipd/core.c:9:
drivers/usb/typec/tipd/core.c: In function 'tps25750_start_patch_burst_mode':
>> drivers/usb/typec/tipd/core.c:844:35: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
844 | dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/usb/typec/tipd/core.c:844:17: note: in expansion of macro 'dev_err'
844 | dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
| ^~~~~~~
drivers/usb/typec/tipd/core.c:844:66: note: format string is defined here
844 | dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
| ~~^
| |
| long unsigned int
| %u


vim +844 drivers/usb/typec/tipd/core.c

801
802 static int tps25750_start_patch_burst_mode(struct tps6598x *tps)
803 {
804 int ret;
805 const struct firmware *fw;
806 const char *firmware_name;
807 struct {
808 u32 fw_size;
809 u8 addr;
810 u8 timeout;
811 } __packed bpms_data;
812
813 ret = device_property_read_string(tps->dev, "firmware-name",
814 &firmware_name);
815 if (ret)
816 return ret;
817
818 ret = request_firmware(&fw, firmware_name, tps->dev);
819 if (ret) {
820 dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name);
821 return ret;
822 }
823
824 if (fw->size == 0) {
825 ret = -EINVAL;
826 goto release_fw;
827 }
828
829 ret = device_property_read_u8(tps->dev, "ti,patch-address", &bpms_data.addr);
830 if (ret) {
831 dev_err(tps->dev, "failed to get patch address\n");
832 return ret;
833 }
834
835 bpms_data.fw_size = fw->size;
836 bpms_data.timeout = TPS_BUNDLE_TIMEOUT;
837
838 ret = tps25750_exec_pbms(tps, (u8 *)&bpms_data, sizeof(bpms_data));
839 if (ret)
840 goto release_fw;
841
842 ret = tps25750_write_firmware(tps, bpms_data.addr, fw->data, fw->size);
843 if (ret) {
> 844 dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
845 firmware_name, fw->size);
846 goto release_fw;
847 }
848
849 /*
850 * A delay of 500us is required after the firmware is written
851 * based on pg.62 in tps6598x Host Interface Technical
852 * Reference Manual
853 * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
854 */
855 udelay(500);
856
857 release_fw:
858 release_firmware(fw);
859
860 return ret;
861 }
862

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-09-18 02:15:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle

Hi Abdel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.6-rc1 next-20230915]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Abdel-Alkuor/dt-bindings-usb-tps6598x-Add-tps25750/20230917-233037
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230917152639.21443-5-alkuor%40gmail.com
patch subject: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle
config: i386-buildonly-randconfig-006-20230917 (https://download.01.org/0day-ci/archive/20230918/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230918/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from include/linux/device.h:15,
from include/linux/acpi.h:14,
from include/linux/i2c.h:13,
from drivers/usb/typec/tipd/core.c:9:
drivers/usb/typec/tipd/core.c: In function 'tps25750_start_patch_burst_mode':
>> drivers/usb/typec/tipd/core.c:844:21: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'const unsigned int'} [-Wformat=]
844 | dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:16: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:49: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/usb/typec/tipd/core.c:844:3: note: in expansion of macro 'dev_err'
844 | dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
| ^~~~~~~
drivers/usb/typec/tipd/core.c:844:52: note: format string is defined here
844 | dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
| ~~^
| |
| long unsigned int
| %u


vim +844 drivers/usb/typec/tipd/core.c

801
802 static int tps25750_start_patch_burst_mode(struct tps6598x *tps)
803 {
804 int ret;
805 const struct firmware *fw;
806 const char *firmware_name;
807 struct {
808 u32 fw_size;
809 u8 addr;
810 u8 timeout;
811 } __packed bpms_data;
812
813 ret = device_property_read_string(tps->dev, "firmware-name",
814 &firmware_name);
815 if (ret)
816 return ret;
817
818 ret = request_firmware(&fw, firmware_name, tps->dev);
819 if (ret) {
820 dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name);
821 return ret;
822 }
823
824 if (fw->size == 0) {
825 ret = -EINVAL;
826 goto release_fw;
827 }
828
829 ret = device_property_read_u8(tps->dev, "ti,patch-address", &bpms_data.addr);
830 if (ret) {
831 dev_err(tps->dev, "failed to get patch address\n");
832 return ret;
833 }
834
835 bpms_data.fw_size = fw->size;
836 bpms_data.timeout = TPS_BUNDLE_TIMEOUT;
837
838 ret = tps25750_exec_pbms(tps, (u8 *)&bpms_data, sizeof(bpms_data));
839 if (ret)
840 goto release_fw;
841
842 ret = tps25750_write_firmware(tps, bpms_data.addr, fw->data, fw->size);
843 if (ret) {
> 844 dev_err(tps->dev, "Failed to write patch %s of %lu bytes\n",
845 firmware_name, fw->size);
846 goto release_fw;
847 }
848
849 /*
850 * A delay of 500us is required after the firmware is written
851 * based on pg.62 in tps6598x Host Interface Technical
852 * Reference Manual
853 * https://www.ti.com/lit/ug/slvuc05a/slvuc05a.pdf
854 */
855 udelay(500);
856
857 release_fw:
858 release_firmware(fw);
859
860 return ret;
861 }
862

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-09-18 14:45:50

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle

On Sun, Sep 17, 2023 at 11:26:28AM -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]>
> ---
> drivers/usb/typec/tipd/core.c | 237 +++++++++++++++++++++++++++++++++-
> 1 file changed, 236 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 6d2151325fbb..fea139c72d6d 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,
> @@ -88,6 +106,8 @@ struct tps6598x {
> struct mutex lock; /* device lock */
> u8 i2c_protocol:1;
>
> + u8 is_tps25750:1;
> +
> struct typec_port *port;
> struct typec_partner *partner;
> struct usb_pd_identity partner_identity;
> @@ -708,6 +728,203 @@ 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(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;
> + u8 mode;
> +
> + ret = tps6598x_exec_cmd(tps, "PBMe", 0, NULL, 0, NULL, 1000, 0);
> + if (ret)
> + return ret;
> +
> + ret = tps6598x_check_mode(tps, &mode);
> + if (mode != 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;
> +
> + 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 = device_property_read_u8(tps->dev, "ti,patch-address", &bpms_data.addr);
> + if (ret) {
> + dev_err(tps->dev, "failed to get patch address\n");
> + return ret;
> + }
> +
> + 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 %lu 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(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;
> + u8 mode;
> +
> + 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, &mode);
> + if (ret)
> + return ret;
> +
> + if (time_is_before_jiffies(timeout))
> + return -ETIMEDOUT;
> +
> + } while (mode != 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;
> @@ -757,6 +974,8 @@ static int tps6598x_probe(struct i2c_client *client)
>
> irq_handler = cd321x_interrupt;
> } else {
> +
> + tps->is_tps25750 = of_device_is_compatible(np, "ti,tps25750");
> /* Enable power status, data status and plug event interrupts */
> mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
> TPS_REG_INT_DATA_STATUS_UPDATE |
> @@ -769,9 +988,15 @@ static int tps6598x_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> + if (tps->is_tps25750 && mode == 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)
> @@ -891,6 +1116,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 (tps->is_tps25750)
> + tps6598x_exec_cmd(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> return ret;
> }
>
> @@ -901,9 +1130,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 (tps->is_tps25750)
> + tps6598x_exec_cmd(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> }
>
> static int __maybe_unused tps6598x_suspend(struct device *dev)
> @@ -946,6 +1180,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", },

This is probable OK for now, we can mix this stuff into core.c now, but
later I want this driver (core.c) to be converted into a library that
contains only the common functionality. TPS5750 will at that point have
its own probe/glue driver.

> {}
> };
> MODULE_DEVICE_TABLE(of, tps6598x_of_match);
> --
> 2.34.1

thanks,

--
heikki

2023-09-20 16:14:00

by Abdel Alkuor

[permalink] [raw]
Subject: Re: [PATCH v5 04/15] USB: typec: Load TPS25750 patch bundle

On Mon, Sep 18, 2023 at 02:31:54PM +0300, Heikki Krogerus wrote:
> On Sun, Sep 17, 2023 at 11:26:28AM -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]>
> > ---
> > drivers/usb/typec/tipd/core.c | 237 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 236 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> > index 6d2151325fbb..fea139c72d6d 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,
> > @@ -88,6 +106,8 @@ struct tps6598x {
> > struct mutex lock; /* device lock */
> > u8 i2c_protocol:1;
> >
> > + u8 is_tps25750:1;
> > +
> > struct typec_port *port;
> > struct typec_partner *partner;
> > struct usb_pd_identity partner_identity;
> > @@ -708,6 +728,203 @@ 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(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;
> > + u8 mode;
> > +
> > + ret = tps6598x_exec_cmd(tps, "PBMe", 0, NULL, 0, NULL, 1000, 0);
> > + if (ret)
> > + return ret;
> > +
> > + ret = tps6598x_check_mode(tps, &mode);
> > + if (mode != 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;
> > +
> > + 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 = device_property_read_u8(tps->dev, "ti,patch-address", &bpms_data.addr);
> > + if (ret) {
> > + dev_err(tps->dev, "failed to get patch address\n");
> > + return ret;
> > + }
> > +
> > + 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 %lu 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(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;
> > + u8 mode;
> > +
> > + 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, &mode);
> > + if (ret)
> > + return ret;
> > +
> > + if (time_is_before_jiffies(timeout))
> > + return -ETIMEDOUT;
> > +
> > + } while (mode != 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;
> > @@ -757,6 +974,8 @@ static int tps6598x_probe(struct i2c_client *client)
> >
> > irq_handler = cd321x_interrupt;
> > } else {
> > +
> > + tps->is_tps25750 = of_device_is_compatible(np, "ti,tps25750");
> > /* Enable power status, data status and plug event interrupts */
> > mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
> > TPS_REG_INT_DATA_STATUS_UPDATE |
> > @@ -769,9 +988,15 @@ static int tps6598x_probe(struct i2c_client *client)
> > if (ret)
> > return ret;
> >
> > + if (tps->is_tps25750 && mode == 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)
> > @@ -891,6 +1116,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 (tps->is_tps25750)
> > + tps6598x_exec_cmd(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> > return ret;
> > }
> >
> > @@ -901,9 +1130,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 (tps->is_tps25750)
> > + tps6598x_exec_cmd(tps, "GAID", 0, NULL, 0, NULL, 2000, 0);
> > }
> >
> > static int __maybe_unused tps6598x_suspend(struct device *dev)
> > @@ -946,6 +1180,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", },
>
> This is probable OK for now, we can mix this stuff into core.c now, but
> later I want this driver (core.c) to be converted into a library that
> contains only the common functionality. TPS5750 will at that point have
> its own probe/glue driver.
>
Duly noted.
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, tps6598x_of_match);
> > --
> > 2.34.1
>
> thanks,
>
> --
> heikki

Abdel