Re: [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider

From: Chanwoo Choi
Date: Mon Sep 14 2020 - 21:07:36 EST


Hi,

On 9/15/20 1:46 AM, Angus Ainslie wrote:
> The tps6598x type C chip can negotiate the VBUS sink/source status as
> well as the VBUS voltage and current. Notify the extcon consumers of
> these changes.
>
> Signed-off-by: Angus Ainslie <angus@xxxxxxxx>
> ---
> drivers/usb/typec/tps6598x.c | 138 +++++++++++++++++++++++++++++++++++
> 1 file changed, 138 insertions(+)
>
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index 3db33bb622c3..3b06d43c810d 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -8,6 +8,8 @@
>
> #include <linux/i2c.h>
> #include <linux/acpi.h>
> +#include <linux/extcon.h>
> +#include <linux/extcon-provider.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
> #include <linux/interrupt.h>
> @@ -95,7 +97,12 @@ struct tps6598x {
> struct typec_port *port;
> struct typec_partner *partner;
> struct usb_pd_identity partner_identity;
> +
> struct usb_role_switch *role_sw;
> +
> +#ifdef CONFIG_EXTCON

Is it necessary of 'ifdef CONFIG_EXTCON'?
If you just add 'select EXTCON' for this driver,
you don't need to check CONFIG_EXTCON.

If any device driver need some framework,
we can add the 'select EXTCON'.

> + struct extcon_dev *edev;
> +#endif
> };
>
> /*
> @@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
> typec_set_data_role(tps->port, role);
> }
>
> +#ifdef CONFIG_EXTCON
> +static void tps6589x_set_extcon_state(struct tps6598x *tps,
> + u32 status, u16 pwr_status, bool state)
> +{
> + union extcon_property_value val;
> + int max_current;
> +
> + if (TPS_STATUS_DATAROLE(status)) {
> + extcon_set_state(tps->edev, EXTCON_USB, false);
> + extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
> + } else {
> + extcon_set_state(tps->edev, EXTCON_USB, state);
> + extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
> + }
> +
> + val.intval = TPS_STATUS_PORTROLE(status);
> + extcon_set_property(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_SRC,
> + val);
> +
> + extcon_set_property(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_SRC,
> + val);
> +
> + switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
> + case TYPEC_PWR_MODE_USB:
> + max_current = 500;
> + extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
> + extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
> + break;
> + case TYPEC_PWR_MODE_1_5A:
> + max_current = 1500;
> + break;
> + case TYPEC_PWR_MODE_3_0A:
> + max_current = 3000;
> + break;
> + case TYPEC_PWR_MODE_PD:
> + max_current = 500;
> + break;
> + }
> +
> + val.intval = max_current;
> + extcon_set_property(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_CURRENT,
> + val);
> + extcon_set_property(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_CURRENT,
> + val);
> +
> + extcon_sync(tps->edev, EXTCON_USB);
> + extcon_sync(tps->edev, EXTCON_USB_HOST);
> + extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
> + extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
> +}
> +#endif
> +
> static int tps6598x_connect(struct tps6598x *tps, u32 status)
> {
> struct typec_partner_desc desc;
> @@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
> if (desc.identity)
> typec_partner_set_identity(tps->partner);
>
> +#ifdef CONFIG_EXTCON
> + tps6598x_set_extcon_state(tps, status, pwr_status, true);
> +#endif
> +
> return 0;
> }
>
> static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
> {
> + enum typec_pwr_opmode mode;
> + u16 pwr_status;
> + int ret;
> +
> if (!IS_ERR(tps->partner))
> typec_unregister_partner(tps->partner);
> tps->partner = NULL;
> typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
> typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
> typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
> + typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
> tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
> +
> + ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
> + if (ret < 0)
> + return;
> +
> + mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
> +
> + dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role %d\n",
> + __func__, mode, TPS_STATUS_PORTROLE(status),
> + TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
> +
> +#ifdef CONFIG_EXTCON
> + tps6598x_set_extcon_state(tps, status, 0, false);
> +#endif
> }
>
> static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
> @@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> goto err_unlock;
> }
>
> + dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
> + (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF));
> +
> ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
> if (ret) {
> dev_err(tps->dev, "%s: failed to read status\n", __func__);
> @@ -467,6 +556,18 @@ static const struct regmap_config tps6598x_regmap_config = {
> .max_register = 0x7F,
> };
>
> +#ifdef CONFIG_EXTCON
> +/* List of detectable cables */
> +static const unsigned int tps6598x_extcon_cable[] = {
> + EXTCON_USB,
> + EXTCON_USB_HOST,
> + EXTCON_CHG_USB_SDP,
> + EXTCON_CHG_USB_SLOW,
> + EXTCON_CHG_USB_FAST,
> + EXTCON_NONE,
> +};
> +#endif
> +
> static int tps6598x_probe(struct i2c_client *client)
> {
> struct typec_capability typec_cap = { };
> @@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client *client)
> }
> fwnode_handle_put(fwnode);
>
> +#ifdef CONFIG_EXTCON
> + /* Allocate extcon device */
> + tps->edev = devm_extcon_dev_allocate(tps->dev, tps6598x_extcon_cable);
> + if (IS_ERR(tps->edev)) {
> + dev_err(tps->dev, "failed to allocate memory for extcon\n");
> + typec_unregister_port(tps->port);
> + return -ENOMEM;
> + }
> +
> + /* Register extcon device */
> + ret = devm_extcon_dev_register(tps->dev, tps->edev);
> + if (ret) {
> + dev_err(tps->dev, "failed to register extcon device\n");
> + typec_unregister_port(tps->port);
> + return ret;
> + }
> +
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS);
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_SRC);
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_VOLTAGE);
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_CURRENT);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_SRC);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_VOLTAGE);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_CURRENT);
> +#endif
> +
> if (status & TPS_STATUS_PLUG_PRESENT) {
> ret = tps6598x_connect(tps, status);
> if (ret)
> dev_err(&client->dev, "failed to register partner\n");
> + } else {
> + tps6598x_disconnect(tps, status);
> }
>
> ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics