2022-06-24 15:55:41

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH 2/4] usb: typec: ucsi: stm32g0: add support for stm32g0 i2c controller

STM32G0 provides an integrated USB Type-C and power delivery interface.
It can be programmed with a firmware to handle UCSI protocol over I2C
interface. A GPIO is used as an interrupt line.

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/usb/typec/ucsi/Kconfig | 10 ++
drivers/usb/typec/ucsi/Makefile | 1 +
drivers/usb/typec/ucsi/ucsi_stm32g0.c | 218 ++++++++++++++++++++++++++
3 files changed, 229 insertions(+)
create mode 100644 drivers/usb/typec/ucsi/ucsi_stm32g0.c

diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index 5e9b37b3f25e1..8f9c4b9f31f79 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -48,4 +48,14 @@ config UCSI_ACPI
To compile the driver as a module, choose M here: the module will be
called ucsi_acpi

+config UCSI_STM32G0
+ tristate "UCSI Interface Driver for STM32G0"
+ depends on I2C
+ help
+ This driver enables UCSI support on platforms that expose a STM32G0
+ Type-C controller over I2C interface.
+
+ To compile the driver as a module, choose M here: the module will be
+ called ucsi_stm32g0.
+
endif
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index 8a8eb5cb8e0f0..480d533d762fe 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -17,3 +17,4 @@ endif

obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o
obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
+obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o
diff --git a/drivers/usb/typec/ucsi/ucsi_stm32g0.c b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
new file mode 100644
index 0000000000000..d1f22cee8c6c9
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * UCSI driver for STMicroelectronics STM32G0 Type-C controller
+ *
+ * Copyright (C) 2022, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <[email protected]>.
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "ucsi.h"
+
+struct ucsi_stm32g0 {
+ struct i2c_client *client;
+ struct completion complete;
+ struct device *dev;
+ unsigned long flags;
+ struct ucsi *ucsi;
+};
+
+static int ucsi_stm32g0_read(struct ucsi *ucsi, unsigned int offset, void *val, size_t len)
+{
+ struct ucsi_stm32g0 *g0 = ucsi_get_drvdata(ucsi);
+ struct i2c_client *client = g0->client;
+ u8 reg = offset;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = &reg,
+ },
+ {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = len,
+ .buf = val,
+ },
+ };
+ int ret;
+
+ ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ if (ret != ARRAY_SIZE(msg)) {
+ dev_err(g0->dev, "i2c read %02x, %02x error: %d\n", client->addr, reg, ret);
+
+ return ret < 0 ? ret : -EIO;
+ }
+
+ return 0;
+}
+
+static int ucsi_stm32g0_async_write(struct ucsi *ucsi, unsigned int offset, const void *val,
+ size_t len)
+{
+ struct ucsi_stm32g0 *g0 = ucsi_get_drvdata(ucsi);
+ struct i2c_client *client = g0->client;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ }
+ };
+ unsigned char *buf;
+ int ret;
+
+ buf = kzalloc(len + 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = offset;
+ memcpy(&buf[1], val, len);
+ msg[0].len = len + 1;
+ msg[0].buf = buf;
+
+ ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ kfree(buf);
+ if (ret != ARRAY_SIZE(msg)) {
+ dev_err(g0->dev, "i2c write %02x, %02x error: %d\n", client->addr, buf[0], ret);
+
+ return ret < 0 ? ret : -EIO;
+ }
+
+ return 0;
+}
+
+static int ucsi_stm32g0_sync_write(struct ucsi *ucsi, unsigned int offset, const void *val,
+ size_t len)
+{
+ struct ucsi_stm32g0 *g0 = ucsi_get_drvdata(ucsi);
+ int ret;
+
+ set_bit(COMMAND_PENDING, &g0->flags);
+
+ ret = ucsi_stm32g0_async_write(ucsi, offset, val, len);
+ if (ret)
+ goto out_clear_bit;
+
+ if (!wait_for_completion_timeout(&g0->complete, msecs_to_jiffies(5000)))
+ ret = -ETIMEDOUT;
+
+out_clear_bit:
+ clear_bit(COMMAND_PENDING, &g0->flags);
+
+ return ret;
+}
+
+static irqreturn_t ucsi_stm32g0_irq_handler(int irq, void *data)
+{
+ struct ucsi_stm32g0 *g0 = data;
+ u32 cci;
+ int ret;
+
+ ret = ucsi_stm32g0_read(g0->ucsi, UCSI_CCI, &cci, sizeof(cci));
+ if (ret)
+ return IRQ_NONE;
+
+ if (UCSI_CCI_CONNECTOR(cci))
+ ucsi_connector_change(g0->ucsi, UCSI_CCI_CONNECTOR(cci));
+
+ if (test_bit(COMMAND_PENDING, &g0->flags) &&
+ cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))
+ complete(&g0->complete);
+
+ return IRQ_HANDLED;
+}
+
+static const struct ucsi_operations ucsi_stm32g0_ops = {
+ .read = ucsi_stm32g0_read,
+ .sync_write = ucsi_stm32g0_sync_write,
+ .async_write = ucsi_stm32g0_async_write,
+};
+
+static int ucsi_stm32g0_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct ucsi_stm32g0 *g0;
+ int ret;
+
+ g0 = devm_kzalloc(dev, sizeof(*g0), GFP_KERNEL);
+ if (!g0)
+ return -ENOMEM;
+
+ g0->dev = dev;
+ g0->client = client;
+ init_completion(&g0->complete);
+ i2c_set_clientdata(client, g0);
+
+ g0->ucsi = ucsi_create(dev, &ucsi_stm32g0_ops);
+ if (IS_ERR(g0->ucsi))
+ return PTR_ERR(g0->ucsi);
+
+ ucsi_set_drvdata(g0->ucsi, g0);
+
+ /* Request alert interrupt */
+ ret = request_threaded_irq(client->irq, NULL, ucsi_stm32g0_irq_handler, IRQF_ONESHOT,
+ dev_name(&client->dev), g0);
+ if (ret) {
+ dev_err_probe(dev, ret, "request IRQ failed\n");
+ goto destroy;
+ }
+
+ ret = ucsi_register(g0->ucsi);
+ if (ret) {
+ dev_err_probe(dev, ret, "ucsi_register failed\n");
+ goto freeirq;
+ }
+
+ return 0;
+
+freeirq:
+ free_irq(client->irq, g0);
+destroy:
+ ucsi_destroy(g0->ucsi);
+
+ return ret;
+}
+
+static int ucsi_stm32g0_remove(struct i2c_client *client)
+{
+ struct ucsi_stm32g0 *g0 = i2c_get_clientdata(client);
+
+ ucsi_unregister(g0->ucsi);
+ free_irq(client->irq, g0);
+ ucsi_destroy(g0->ucsi);
+
+ return 0;
+}
+
+static const struct of_device_id __maybe_unused ucsi_stm32g0_typec_of_match[] = {
+ { .compatible = "st,stm32g0-typec" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ucsi_stm32g0_typec_of_match);
+
+static const struct i2c_device_id ucsi_stm32g0_typec_i2c_devid[] = {
+ {"stm32g0-typec", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, ucsi_stm32g0_typec_i2c_devid);
+
+static struct i2c_driver ucsi_stm32g0_i2c_driver = {
+ .driver = {
+ .name = "ucsi-stm32g0-i2c",
+ .of_match_table = of_match_ptr(ucsi_stm32g0_typec_of_match),
+ },
+ .probe = ucsi_stm32g0_probe,
+ .remove = ucsi_stm32g0_remove,
+ .id_table = ucsi_stm32g0_typec_i2c_devid
+};
+module_i2c_driver(ucsi_stm32g0_i2c_driver);
+
+MODULE_AUTHOR("Fabrice Gasnier <[email protected]>");
+MODULE_DESCRIPTION("STMicroelectronics STM32G0 Type-C controller");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_ALIAS("platform:ucsi-stm32g0");
--
2.25.1


2022-06-25 06:52:37

by Christophe JAILLET

[permalink] [raw]
Subject: Re: [PATCH 2/4] usb: typec: ucsi: stm32g0: add support for stm32g0 i2c controller

Le 24/06/2022 à 17:54, Fabrice Gasnier a écrit :
> STM32G0 provides an integrated USB Type-C and power delivery interface.
> It can be programmed with a firmware to handle UCSI protocol over I2C
> interface. A GPIO is used as an interrupt line.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-rj0Iel/[email protected]>
> ---
> drivers/usb/typec/ucsi/Kconfig | 10 ++
> drivers/usb/typec/ucsi/Makefile | 1 +
> drivers/usb/typec/ucsi/ucsi_stm32g0.c | 218 ++++++++++++++++++++++++++
> 3 files changed, 229 insertions(+)
> create mode 100644 drivers/usb/typec/ucsi/ucsi_stm32g0.c
>

[...]

> +static int ucsi_stm32g0_async_write(struct ucsi *ucsi, unsigned int offset, const void *val,
> + size_t len)
> +{
> + struct ucsi_stm32g0 *g0 = ucsi_get_drvdata(ucsi);
> + struct i2c_client *client = g0->client;
> + struct i2c_msg msg[] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> + }
> + };
> + unsigned char *buf;
> + int ret;
> +
> + buf = kzalloc(len + 1, GFP_KERNEL);

Hi,

Nit: kmalloc() would be enough here, the whole buffer is written just a
few lines after.

> + if (!buf)
> + return -ENOMEM;
> +
> + buf[0] = offset;
> + memcpy(&buf[1], val, len);
> + msg[0].len = len + 1;
> + msg[0].buf = buf;
> +
> + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> + kfree(buf);
> + if (ret != ARRAY_SIZE(msg)) {
> + dev_err(g0->dev, "i2c write %02x, %02x error: %d\n", client->addr, buf[0], ret);

Use-after-free of buf.

> +
> + return ret < 0 ? ret : -EIO;
> + }
> +
> + return 0;
> +}
> +

Just my 2c,
CJ

[...]

2022-06-27 13:41:52

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 2/4] usb: typec: ucsi: stm32g0: add support for stm32g0 i2c controller

Hi,

On Fri, Jun 24, 2022 at 05:54:11PM +0200, Fabrice Gasnier wrote:
> +static int ucsi_stm32g0_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct ucsi_stm32g0 *g0;
> + int ret;
> +
> + g0 = devm_kzalloc(dev, sizeof(*g0), GFP_KERNEL);
> + if (!g0)
> + return -ENOMEM;
> +
> + g0->dev = dev;
> + g0->client = client;
> + init_completion(&g0->complete);
> + i2c_set_clientdata(client, g0);
> +
> + g0->ucsi = ucsi_create(dev, &ucsi_stm32g0_ops);
> + if (IS_ERR(g0->ucsi))
> + return PTR_ERR(g0->ucsi);
> +
> + ucsi_set_drvdata(g0->ucsi, g0);
> +
> + /* Request alert interrupt */
> + ret = request_threaded_irq(client->irq, NULL, ucsi_stm32g0_irq_handler, IRQF_ONESHOT,
> + dev_name(&client->dev), g0);
> + if (ret) {
> + dev_err_probe(dev, ret, "request IRQ failed\n");
> + goto destroy;
> + }
> +
> + ret = ucsi_register(g0->ucsi);
> + if (ret) {
> + dev_err_probe(dev, ret, "ucsi_register failed\n");
> + goto freeirq;
> + }

If there isn't UCSI firmware, then ucsi_register() will always safely
fail here, right?


> + return 0;
> +
> +freeirq:
> + free_irq(client->irq, g0);
> +destroy:
> + ucsi_destroy(g0->ucsi);
> +
> + return ret;
> +}


thanks,

--
heikki

2022-06-28 07:23:41

by Fabrice Gasnier

[permalink] [raw]
Subject: Re: [PATCH 2/4] usb: typec: ucsi: stm32g0: add support for stm32g0 i2c controller

On 6/27/22 15:17, Heikki Krogerus wrote:
> Hi,
>
> On Fri, Jun 24, 2022 at 05:54:11PM +0200, Fabrice Gasnier wrote:
>> +static int ucsi_stm32g0_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> +{
>> + struct device *dev = &client->dev;
>> + struct ucsi_stm32g0 *g0;
>> + int ret;
>> +
>> + g0 = devm_kzalloc(dev, sizeof(*g0), GFP_KERNEL);
>> + if (!g0)
>> + return -ENOMEM;
>> +
>> + g0->dev = dev;
>> + g0->client = client;
>> + init_completion(&g0->complete);
>> + i2c_set_clientdata(client, g0);
>> +
>> + g0->ucsi = ucsi_create(dev, &ucsi_stm32g0_ops);
>> + if (IS_ERR(g0->ucsi))
>> + return PTR_ERR(g0->ucsi);
>> +
>> + ucsi_set_drvdata(g0->ucsi, g0);
>> +
>> + /* Request alert interrupt */
>> + ret = request_threaded_irq(client->irq, NULL, ucsi_stm32g0_irq_handler, IRQF_ONESHOT,
>> + dev_name(&client->dev), g0);
>> + if (ret) {
>> + dev_err_probe(dev, ret, "request IRQ failed\n");
>> + goto destroy;
>> + }
>> +
>> + ret = ucsi_register(g0->ucsi);
>> + if (ret) {
>> + dev_err_probe(dev, ret, "ucsi_register failed\n");
>> + goto freeirq;
>> + }
>
> If there isn't UCSI firmware, then ucsi_register() will always safely
> fail here, right?

Hi Heikki,

Yes, in such a case, the first i2c read (UCSI_VERSION) in
ucsi_register() will return an error and safely fail here.

Thanks for reviewing,
Best Regards,
Fabrice

>
>
>> + return 0;
>> +
>> +freeirq:
>> + free_irq(client->irq, g0);
>> +destroy:
>> + ucsi_destroy(g0->ucsi);
>> +
>> + return ret;
>> +}
>
>
> thanks,
>

2022-06-28 10:01:10

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 2/4] usb: typec: ucsi: stm32g0: add support for stm32g0 i2c controller

On Tue, Jun 28, 2022 at 09:21:12AM +0200, Fabrice Gasnier wrote:
> On 6/27/22 15:17, Heikki Krogerus wrote:
> > Hi,
> >
> > On Fri, Jun 24, 2022 at 05:54:11PM +0200, Fabrice Gasnier wrote:
> >> +static int ucsi_stm32g0_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >> +{
> >> + struct device *dev = &client->dev;
> >> + struct ucsi_stm32g0 *g0;
> >> + int ret;
> >> +
> >> + g0 = devm_kzalloc(dev, sizeof(*g0), GFP_KERNEL);
> >> + if (!g0)
> >> + return -ENOMEM;
> >> +
> >> + g0->dev = dev;
> >> + g0->client = client;
> >> + init_completion(&g0->complete);
> >> + i2c_set_clientdata(client, g0);
> >> +
> >> + g0->ucsi = ucsi_create(dev, &ucsi_stm32g0_ops);
> >> + if (IS_ERR(g0->ucsi))
> >> + return PTR_ERR(g0->ucsi);
> >> +
> >> + ucsi_set_drvdata(g0->ucsi, g0);
> >> +
> >> + /* Request alert interrupt */
> >> + ret = request_threaded_irq(client->irq, NULL, ucsi_stm32g0_irq_handler, IRQF_ONESHOT,
> >> + dev_name(&client->dev), g0);
> >> + if (ret) {
> >> + dev_err_probe(dev, ret, "request IRQ failed\n");
> >> + goto destroy;
> >> + }
> >> +
> >> + ret = ucsi_register(g0->ucsi);
> >> + if (ret) {
> >> + dev_err_probe(dev, ret, "ucsi_register failed\n");
> >> + goto freeirq;
> >> + }
> >
> > If there isn't UCSI firmware, then ucsi_register() will always safely
> > fail here, right?
>
> Hi Heikki,
>
> Yes, in such a case, the first i2c read (UCSI_VERSION) in
> ucsi_register() will return an error and safely fail here.

Okay, thanks.

--
heikki