lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8e7247d6-9a4f-dc22-971f-9a253c159a37@samsung.com>
Date:   Tue, 15 Sep 2020 10:19:29 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     Angus Ainslie <angus@...ea.ca>, kernel@...i.sm
Cc:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        bryan.odonoghue@...aro.org
Subject: Re: [PATCH 2/4] usb: typec: tps6589x: register as an extcon
 provider

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@...ea.ca>
> ---
>  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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ