| 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
| ||
|
Message-ID: <b2e011b9d565bf73975a746d02429b9d@akkea.ca>
Date: Tue, 15 Sep 2020 06:32:51 -0700
From: Angus Ainslie <angus@...ea.ca>
To: Chanwoo Choi <cw00.choi@...sung.com>
Cc: kernel@...i.sm, 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 2020-09-14 18:19, Chanwoo Choi wrote:
> 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'.
>
Sure I can change that for v2
Angus
>> + 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,
>>
Powered by blists - more mailing lists