2024-04-11 04:50:21

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH v2 0/5] usb: typec: ucsi: glink: rework orientation handling

Simplify the way the UCSI GLINK driver handles cable orientation. Make
the UCSI core responsible for pinging the driver to get cable status.
Use typec-port API instead of calling typec_switch_set() directly.
Also make the orientation status available via the sysfs.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
Changes in v2:
- Moved ucsi_connector forward declaration to the top of the file
(Heikki)
- Replaced UCSI_ORIENTATION_AWARE with the update_connector callback
(Heikki)
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Dmitry Baryshkov (5):
usb: typec: ucsi: add callback for connector status updates
usb: typec: ucsi: glink: move GPIO reading into connector_status callback
usb: typec: ucsi: glink: use typec_set_orientation
usb: typec: ucsi: add update_connector callback
usb: typec: ucsi: glink: set orientation aware if supported

drivers/usb/typec/ucsi/ucsi.c | 9 +++++
drivers/usb/typec/ucsi/ucsi.h | 5 +++
drivers/usb/typec/ucsi/ucsi_glink.c | 67 ++++++++++++++++++-------------------
3 files changed, 47 insertions(+), 34 deletions(-)
---
base-commit: 359b3d1a6f8190487067ec542ea7c194eee26e24
change-id: 20240408-ucsi-orient-aware-9643f2653301

Best regards,
--
Dmitry Baryshkov <[email protected]>



2024-04-11 04:50:38

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH v2 2/5] usb: typec: ucsi: glink: move GPIO reading into connector_status callback

To simplify the platform code move Type-C orientation handling into the
connector_status callback. As it is called both during connector
registration and on connector change events, duplicated code from
pmic_glink_ucsi_register() can be dropped.

Also this moves operations that can sleep into a worker thread,
removing the only sleeping operation from pmic_glink_ucsi_notify().

Tested-by: Krishna Kurapati <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/usb/typec/ucsi/ucsi_glink.c | 48 ++++++++++++++++---------------------
1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index b91d2d15d7d9..d21f8cd2fe35 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -187,10 +187,28 @@ static int pmic_glink_ucsi_sync_write(struct ucsi *__ucsi, unsigned int offset,
return ret;
}

+static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
+{
+ struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
+ int orientation;
+
+ if (con->num >= PMIC_GLINK_MAX_PORTS ||
+ !ucsi->port_orientation[con->num - 1])
+ return;
+
+ orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
+ if (orientation >= 0) {
+ typec_switch_set(ucsi->port_switch[con->num - 1],
+ orientation ? TYPEC_ORIENTATION_REVERSE
+ : TYPEC_ORIENTATION_NORMAL);
+ }
+}
+
static const struct ucsi_operations pmic_glink_ucsi_ops = {
.read = pmic_glink_ucsi_read,
.sync_write = pmic_glink_ucsi_sync_write,
- .async_write = pmic_glink_ucsi_async_write
+ .async_write = pmic_glink_ucsi_async_write,
+ .connector_status = pmic_glink_ucsi_connector_status,
};

static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len)
@@ -229,20 +247,8 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
}

con_num = UCSI_CCI_CONNECTOR(cci);
- if (con_num) {
- if (con_num <= PMIC_GLINK_MAX_PORTS &&
- ucsi->port_orientation[con_num - 1]) {
- int orientation = gpiod_get_value(ucsi->port_orientation[con_num - 1]);
-
- if (orientation >= 0) {
- typec_switch_set(ucsi->port_switch[con_num - 1],
- orientation ? TYPEC_ORIENTATION_REVERSE
- : TYPEC_ORIENTATION_NORMAL);
- }
- }
-
+ if (con_num)
ucsi_connector_change(ucsi->ucsi, con_num);
- }

if (ucsi->sync_pending &&
(cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))) {
@@ -253,20 +259,6 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
static void pmic_glink_ucsi_register(struct work_struct *work)
{
struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, register_work);
- int orientation;
- int i;
-
- for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
- if (!ucsi->port_orientation[i])
- continue;
- orientation = gpiod_get_value(ucsi->port_orientation[i]);
-
- if (orientation >= 0) {
- typec_switch_set(ucsi->port_switch[i],
- orientation ? TYPEC_ORIENTATION_REVERSE
- : TYPEC_ORIENTATION_NORMAL);
- }
- }

ucsi_register(ucsi->ucsi);
}

--
2.39.2


2024-04-11 04:50:53

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH v2 3/5] usb: typec: ucsi: glink: use typec_set_orientation

Use typec_set_orientation() instead of calling typec_switch_set()
manually. This way the rest of the typec framework and the userspace are
notified about the orientation change.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/usb/typec/ucsi/ucsi_glink.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index d21f8cd2fe35..d279e2cf9bba 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -58,7 +58,6 @@ struct pmic_glink_ucsi {
struct device *dev;

struct gpio_desc *port_orientation[PMIC_GLINK_MAX_PORTS];
- struct typec_switch *port_switch[PMIC_GLINK_MAX_PORTS];

struct pmic_glink_client *client;

@@ -198,9 +197,10 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)

orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
if (orientation >= 0) {
- typec_switch_set(ucsi->port_switch[con->num - 1],
- orientation ? TYPEC_ORIENTATION_REVERSE
- : TYPEC_ORIENTATION_NORMAL);
+ typec_set_orientation(con->port,
+ orientation ?
+ TYPEC_ORIENTATION_REVERSE :
+ TYPEC_ORIENTATION_NORMAL);
}
}

@@ -378,11 +378,6 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
return dev_err_probe(dev, PTR_ERR(desc),
"unable to acquire orientation gpio\n");
ucsi->port_orientation[port] = desc;
-
- ucsi->port_switch[port] = fwnode_typec_switch_get(fwnode);
- if (IS_ERR(ucsi->port_switch[port]))
- return dev_err_probe(dev, PTR_ERR(ucsi->port_switch[port]),
- "failed to acquire orientation-switch\n");
}

ucsi->client = devm_pmic_glink_register_client(dev,

--
2.39.2


2024-04-11 04:51:14

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH v2 4/5] usb: typec: ucsi: add update_connector callback

Add a callback to allow glue drivers to update the connector before
registering corresponding power supply and Type-C port. In particular
this is useful if glue drivers want to touch the connector's Type-C
capabilities structure.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/usb/typec/ucsi/ucsi.c | 3 +++
drivers/usb/typec/ucsi/ucsi.h | 2 ++
2 files changed, 5 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 7ad544c968e4..57e73b823a4c 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1559,6 +1559,9 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
cap->driver_data = con;
cap->ops = &ucsi_ops;

+ if (ucsi->ops->update_connector)
+ ucsi->ops->update_connector(con);
+
ret = ucsi_register_port_psy(con);
if (ret)
goto out;
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 3e1241e38f3c..c4d103db9d0f 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -60,6 +60,7 @@ struct dentry;
* @sync_write: Blocking write operation
* @async_write: Non-blocking write operation
* @update_altmodes: Squashes duplicate DP altmodes
+ * @update_connector: Update connector capabilities before registering
* @connector_status: Updates connector status, called holding connector lock
*
* Read and write routines for UCSI interface. @sync_write must wait for the
@@ -75,6 +76,7 @@ struct ucsi_operations {
const void *val, size_t val_len);
bool (*update_altmodes)(struct ucsi *ucsi, struct ucsi_altmode *orig,
struct ucsi_altmode *updated);
+ void (*update_connector)(struct ucsi_connector *con);
void (*connector_status)(struct ucsi_connector *con);
};


--
2.39.2


2024-04-11 04:55:44

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH v2 5/5] usb: typec: ucsi: glink: set orientation aware if supported

If the PMIC-GLINK device has orientation GPIOs declared, then it will
report connection orientation. In this case set the flag to mark
registered ports as orientation-aware.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/usb/typec/ucsi/ucsi_glink.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index d279e2cf9bba..f7546bb488c3 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -186,6 +186,17 @@ static int pmic_glink_ucsi_sync_write(struct ucsi *__ucsi, unsigned int offset,
return ret;
}

+static void pmic_glink_ucsi_update_connector(struct ucsi_connector *con)
+{
+ struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
+ int i;
+
+ for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
+ if (ucsi->port_orientation[i])
+ con->typec_cap.orientation_aware = true;
+ }
+}
+
static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
{
struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
@@ -208,6 +219,7 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = {
.read = pmic_glink_ucsi_read,
.sync_write = pmic_glink_ucsi_sync_write,
.async_write = pmic_glink_ucsi_async_write,
+ .update_connector = pmic_glink_ucsi_update_connector,
.connector_status = pmic_glink_ucsi_connector_status,
};


--
2.39.2


2024-04-15 09:26:28

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] usb: typec: ucsi: glink: move GPIO reading into connector_status callback

On Thu, Apr 11, 2024 at 07:49:54AM +0300, Dmitry Baryshkov wrote:
> To simplify the platform code move Type-C orientation handling into the
> connector_status callback. As it is called both during connector
> registration and on connector change events, duplicated code from
> pmic_glink_ucsi_register() can be dropped.
>
> Also this moves operations that can sleep into a worker thread,
> removing the only sleeping operation from pmic_glink_ucsi_notify().
>
> Tested-by: Krishna Kurapati <[email protected]>
> Signed-off-by: Dmitry Baryshkov <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 48 ++++++++++++++++---------------------
> 1 file changed, 20 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index b91d2d15d7d9..d21f8cd2fe35 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -187,10 +187,28 @@ static int pmic_glink_ucsi_sync_write(struct ucsi *__ucsi, unsigned int offset,
> return ret;
> }
>
> +static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> +{
> + struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> + int orientation;
> +
> + if (con->num >= PMIC_GLINK_MAX_PORTS ||
> + !ucsi->port_orientation[con->num - 1])
> + return;
> +
> + orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
> + if (orientation >= 0) {
> + typec_switch_set(ucsi->port_switch[con->num - 1],
> + orientation ? TYPEC_ORIENTATION_REVERSE
> + : TYPEC_ORIENTATION_NORMAL);
> + }
> +}
> +
> static const struct ucsi_operations pmic_glink_ucsi_ops = {
> .read = pmic_glink_ucsi_read,
> .sync_write = pmic_glink_ucsi_sync_write,
> - .async_write = pmic_glink_ucsi_async_write
> + .async_write = pmic_glink_ucsi_async_write,
> + .connector_status = pmic_glink_ucsi_connector_status,
> };
>
> static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len)
> @@ -229,20 +247,8 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
> }
>
> con_num = UCSI_CCI_CONNECTOR(cci);
> - if (con_num) {
> - if (con_num <= PMIC_GLINK_MAX_PORTS &&
> - ucsi->port_orientation[con_num - 1]) {
> - int orientation = gpiod_get_value(ucsi->port_orientation[con_num - 1]);
> -
> - if (orientation >= 0) {
> - typec_switch_set(ucsi->port_switch[con_num - 1],
> - orientation ? TYPEC_ORIENTATION_REVERSE
> - : TYPEC_ORIENTATION_NORMAL);
> - }
> - }
> -
> + if (con_num)
> ucsi_connector_change(ucsi->ucsi, con_num);
> - }
>
> if (ucsi->sync_pending &&
> (cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))) {
> @@ -253,20 +259,6 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
> static void pmic_glink_ucsi_register(struct work_struct *work)
> {
> struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, register_work);
> - int orientation;
> - int i;
> -
> - for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
> - if (!ucsi->port_orientation[i])
> - continue;
> - orientation = gpiod_get_value(ucsi->port_orientation[i]);
> -
> - if (orientation >= 0) {
> - typec_switch_set(ucsi->port_switch[i],
> - orientation ? TYPEC_ORIENTATION_REVERSE
> - : TYPEC_ORIENTATION_NORMAL);
> - }
> - }
>
> ucsi_register(ucsi->ucsi);
> }
>
> --
> 2.39.2

--
heikki

2024-04-15 09:36:33

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] usb: typec: ucsi: glink: set orientation aware if supported

On Thu, Apr 11, 2024 at 07:49:57AM +0300, Dmitry Baryshkov wrote:
> If the PMIC-GLINK device has orientation GPIOs declared, then it will
> report connection orientation. In this case set the flag to mark
> registered ports as orientation-aware.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index d279e2cf9bba..f7546bb488c3 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -186,6 +186,17 @@ static int pmic_glink_ucsi_sync_write(struct ucsi *__ucsi, unsigned int offset,
> return ret;
> }
>
> +static void pmic_glink_ucsi_update_connector(struct ucsi_connector *con)
> +{
> + struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> + int i;
> +
> + for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
> + if (ucsi->port_orientation[i])
> + con->typec_cap.orientation_aware = true;
> + }
> +}
> +
> static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> {
> struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> @@ -208,6 +219,7 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = {
> .read = pmic_glink_ucsi_read,
> .sync_write = pmic_glink_ucsi_sync_write,
> .async_write = pmic_glink_ucsi_async_write,
> + .update_connector = pmic_glink_ucsi_update_connector,
> .connector_status = pmic_glink_ucsi_connector_status,
> };
>
>
> --
> 2.39.2

--
heikki

2024-04-15 10:42:13

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] usb: typec: ucsi: add update_connector callback

On Thu, Apr 11, 2024 at 07:49:56AM +0300, Dmitry Baryshkov wrote:
> Add a callback to allow glue drivers to update the connector before
> registering corresponding power supply and Type-C port. In particular
> this is useful if glue drivers want to touch the connector's Type-C
> capabilities structure.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
> drivers/usb/typec/ucsi/ucsi.c | 3 +++
> drivers/usb/typec/ucsi/ucsi.h | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 7ad544c968e4..57e73b823a4c 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1559,6 +1559,9 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
> cap->driver_data = con;
> cap->ops = &ucsi_ops;
>
> + if (ucsi->ops->update_connector)
> + ucsi->ops->update_connector(con);
> +
> ret = ucsi_register_port_psy(con);
> if (ret)
> goto out;
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index 3e1241e38f3c..c4d103db9d0f 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -60,6 +60,7 @@ struct dentry;
> * @sync_write: Blocking write operation
> * @async_write: Non-blocking write operation
> * @update_altmodes: Squashes duplicate DP altmodes
> + * @update_connector: Update connector capabilities before registering
> * @connector_status: Updates connector status, called holding connector lock
> *
> * Read and write routines for UCSI interface. @sync_write must wait for the
> @@ -75,6 +76,7 @@ struct ucsi_operations {
> const void *val, size_t val_len);
> bool (*update_altmodes)(struct ucsi *ucsi, struct ucsi_altmode *orig,
> struct ucsi_altmode *updated);
> + void (*update_connector)(struct ucsi_connector *con);
> void (*connector_status)(struct ucsi_connector *con);
> };
>
>
> --
> 2.39.2

--
heikki

2024-04-15 11:47:27

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] usb: typec: ucsi: glink: use typec_set_orientation

On Thu, Apr 11, 2024 at 07:49:55AM +0300, Dmitry Baryshkov wrote:
> Use typec_set_orientation() instead of calling typec_switch_set()
> manually. This way the rest of the typec framework and the userspace are
> notified about the orientation change.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index d21f8cd2fe35..d279e2cf9bba 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -58,7 +58,6 @@ struct pmic_glink_ucsi {
> struct device *dev;
>
> struct gpio_desc *port_orientation[PMIC_GLINK_MAX_PORTS];
> - struct typec_switch *port_switch[PMIC_GLINK_MAX_PORTS];
>
> struct pmic_glink_client *client;
>
> @@ -198,9 +197,10 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
>
> orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
> if (orientation >= 0) {
> - typec_switch_set(ucsi->port_switch[con->num - 1],
> - orientation ? TYPEC_ORIENTATION_REVERSE
> - : TYPEC_ORIENTATION_NORMAL);
> + typec_set_orientation(con->port,
> + orientation ?
> + TYPEC_ORIENTATION_REVERSE :
> + TYPEC_ORIENTATION_NORMAL);
> }
> }
>
> @@ -378,11 +378,6 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
> return dev_err_probe(dev, PTR_ERR(desc),
> "unable to acquire orientation gpio\n");
> ucsi->port_orientation[port] = desc;
> -
> - ucsi->port_switch[port] = fwnode_typec_switch_get(fwnode);
> - if (IS_ERR(ucsi->port_switch[port]))
> - return dev_err_probe(dev, PTR_ERR(ucsi->port_switch[port]),
> - "failed to acquire orientation-switch\n");
> }
>
> ucsi->client = devm_pmic_glink_register_client(dev,
>
> --
> 2.39.2

--
heikki