* [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x
@ 2020-09-14 16:46 Angus Ainslie
2020-09-14 16:46 ` [PATCH 1/4] extcon: Add USB VBUS properties Angus Ainslie
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Angus Ainslie @ 2020-09-14 16:46 UTC (permalink / raw)
To: kernel
Cc: MyungJoo Ham, Chanwoo Choi, Heikki Krogerus, Greg Kroah-Hartman,
linux-kernel, linux-usb, bryan.odonoghue, Angus Ainslie
We have a complex set of hardware components to manage our USB C data and
power. For these to work together we decided to use extcon to communicate
the system changes as various cables and devices are plugged in/out. We did
look at usb_roleswitch and the charging framework but thought it would be
preferable to keep all of the information together in one system.
The components we have in the system are:
1) TPS65982 type USB type C controller
2) dwc3 IP in the imx8mq
3) BQ25895 battery charger
I'll break this into 2 parts the data role and the power role.
For the data role the TPS65982 senses connect and disconnect as well as data
source/sink. It is also controlling the USB 3 data lanes. The display port and
USB 3 muxing is handled by a different chip and we'll submit patches for that
later on. The dwc3 controls the USB 2 data lanes.
On the power side there are even more moving pieces. The TPS65982 negotiates
the power delivery contract, the dwc3 senses the BC1.2 charging current and the
BQ25895 sets whether we are sinking or sourcing current and what the current
limit is of the sink and source.
For both the data and power roles no single chip has all of the required
information. Is using extcon the correct way of doing this and if not what
are the alternatives ?
The extcon extensions allow us to communicate the the power roles amongst
the various chips.
This patch series has been tested with the 5.9-rc4 kernel on the Purism
Librem5 HW. Assuming this is the correct way to use extcon there will be
follow on patches to the BQ25895 and dwc3 drivers.
Strictly speaking only the first 3 patches are needed for extcon support, the
forth patch decodes the power delivery contracts which makes use of the extcon
system.
Angus Ainslie (4):
extcon: Add USB VBUS properties
usb: typec: tps6589x: register as an extcon provider
usb: typec: tps6598x: Add the extcon USB chargers
usb: typec: tps6598x: Add the power delivery irq
drivers/usb/typec/tps6598x.c | 488 ++++++++++++++++++++++++++++++++++-
include/linux/extcon.h | 17 +-
2 files changed, 503 insertions(+), 2 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/4] extcon: Add USB VBUS properties 2020-09-14 16:46 [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Angus Ainslie @ 2020-09-14 16:46 ` Angus Ainslie 2020-09-15 1:40 ` Chanwoo Choi 2020-09-14 16:46 ` [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider Angus Ainslie ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Angus Ainslie @ 2020-09-14 16:46 UTC (permalink / raw) To: kernel Cc: MyungJoo Ham, Chanwoo Choi, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue, Angus Ainslie USB type C, USB BC1.2 and USB power delivery allow different voltages and currents for VBUS so we need these additional properties. Also USB type C allows separate device and power roles so add a VBUS SRC property. Signed-off-by: Angus Ainslie <angus@akkea.ca> --- include/linux/extcon.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/include/linux/extcon.h b/include/linux/extcon.h index fd183fb9c20f..c4d48f4f74c4 100644 --- a/include/linux/extcon.h +++ b/include/linux/extcon.h @@ -117,14 +117,29 @@ * @type: integer (intval) * @value: 0 (USB/USB2) or 1 (USB3) * @default: 0 (USB/USB2) + * - EXTCON_PROP_USB_VBUS_SRC + * @type: integer (intval) + * @value: 0 (sink) or 1 (source) + * @default: 0 (sink) + * - EXTCON_PROP_USB_VBUS_VOLTAGE + * @type: integer (intval) + * @value: negotiated vbus voltage in mV + * @default: 5000 + * - EXTCON_PROP_USB_VBUS_CURRENT + * @type: integer (intval) + * @value: negotiated vbus current in mA + * @default: 100 * */ #define EXTCON_PROP_USB_VBUS 0 #define EXTCON_PROP_USB_TYPEC_POLARITY 1 #define EXTCON_PROP_USB_SS 2 +#define EXTCON_PROP_USB_VBUS_SRC 3 +#define EXTCON_PROP_USB_VBUS_VOLTAGE 4 +#define EXTCON_PROP_USB_VBUS_CURRENT 5 #define EXTCON_PROP_USB_MIN 0 -#define EXTCON_PROP_USB_MAX 2 +#define EXTCON_PROP_USB_MAX 5 #define EXTCON_PROP_USB_CNT (EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1) /* Properties of EXTCON_TYPE_CHG. */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] extcon: Add USB VBUS properties 2020-09-14 16:46 ` [PATCH 1/4] extcon: Add USB VBUS properties Angus Ainslie @ 2020-09-15 1:40 ` Chanwoo Choi 2020-09-15 13:31 ` Angus Ainslie 0 siblings, 1 reply; 12+ messages in thread From: Chanwoo Choi @ 2020-09-15 1:40 UTC (permalink / raw) To: Angus Ainslie, kernel Cc: MyungJoo Ham, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue Hi, On 9/15/20 1:46 AM, Angus Ainslie wrote: > USB type C, USB BC1.2 and USB power delivery allow different voltages > and currents for VBUS so we need these additional properties. > > Also USB type C allows separate device and power roles so add a VBUS SRC > property. > > Signed-off-by: Angus Ainslie <angus@akkea.ca> > --- > include/linux/extcon.h | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/include/linux/extcon.h b/include/linux/extcon.h > index fd183fb9c20f..c4d48f4f74c4 100644 > --- a/include/linux/extcon.h > +++ b/include/linux/extcon.h > @@ -117,14 +117,29 @@ > * @type: integer (intval) > * @value: 0 (USB/USB2) or 1 (USB3) > * @default: 0 (USB/USB2) > + * - EXTCON_PROP_USB_VBUS_SRC Could you explain more correct meaning of both sink and source? > + * @type: integer (intval) > + * @value: 0 (sink) or 1 (source) > + * @default: 0 (sink) > + * - EXTCON_PROP_USB_VBUS_VOLTAGE > + * @type: integer (intval) > + * @value: negotiated vbus voltage in mV > + * @default: 5000 Could you suggest the data why do you set default value as 5000? > + * - EXTCON_PROP_USB_VBUS_CURRENT > + * @type: integer (intval) > + * @value: negotiated vbus current in mA > + * @default: 100 ditto. Why default value is 100? > * > */ > #define EXTCON_PROP_USB_VBUS 0 > #define EXTCON_PROP_USB_TYPEC_POLARITY 1 > #define EXTCON_PROP_USB_SS 2 > +#define EXTCON_PROP_USB_VBUS_SRC 3 > +#define EXTCON_PROP_USB_VBUS_VOLTAGE 4 > +#define EXTCON_PROP_USB_VBUS_CURRENT 5 > > #define EXTCON_PROP_USB_MIN 0 > -#define EXTCON_PROP_USB_MAX 2 > +#define EXTCON_PROP_USB_MAX 5 > #define EXTCON_PROP_USB_CNT (EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1) > > /* Properties of EXTCON_TYPE_CHG. */ > -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] extcon: Add USB VBUS properties 2020-09-15 1:40 ` Chanwoo Choi @ 2020-09-15 13:31 ` Angus Ainslie 0 siblings, 0 replies; 12+ messages in thread From: Angus Ainslie @ 2020-09-15 13:31 UTC (permalink / raw) To: Chanwoo Choi Cc: kernel, MyungJoo Ham, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue Hi, On 2020-09-14 18:40, Chanwoo Choi wrote: > Hi, > > On 9/15/20 1:46 AM, Angus Ainslie wrote: >> USB type C, USB BC1.2 and USB power delivery allow different voltages >> and currents for VBUS so we need these additional properties. >> >> Also USB type C allows separate device and power roles so add a VBUS >> SRC >> property. >> >> Signed-off-by: Angus Ainslie <angus@akkea.ca> >> --- >> include/linux/extcon.h | 17 ++++++++++++++++- >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/extcon.h b/include/linux/extcon.h >> index fd183fb9c20f..c4d48f4f74c4 100644 >> --- a/include/linux/extcon.h >> +++ b/include/linux/extcon.h >> @@ -117,14 +117,29 @@ >> * @type: integer (intval) >> * @value: 0 (USB/USB2) or 1 (USB3) >> * @default: 0 (USB/USB2) >> + * - EXTCON_PROP_USB_VBUS_SRC > > Could you explain more correct meaning of both sink and source? > Sure I can add some comments. >> + * @type: integer (intval) >> + * @value: 0 (sink) or 1 (source) >> + * @default: 0 (sink) >> + * - EXTCON_PROP_USB_VBUS_VOLTAGE >> + * @type: integer (intval) >> + * @value: negotiated vbus voltage in mV >> + * @default: 5000 > > Could you suggest the data why do you set default value as 5000? > The lowest USB VBUS is 5V so I can add that to the comments. >> + * - EXTCON_PROP_USB_VBUS_CURRENT >> + * @type: integer (intval) >> + * @value: negotiated vbus current in mA >> + * @default: 100 > > ditto. Why default value is 100? > USB spec says that until the current is negotiated the max that anything can draw is 100mA. I can add a comment to that effect. Thanks Angus >> * >> */ >> #define EXTCON_PROP_USB_VBUS 0 >> #define EXTCON_PROP_USB_TYPEC_POLARITY 1 >> #define EXTCON_PROP_USB_SS 2 >> +#define EXTCON_PROP_USB_VBUS_SRC 3 >> +#define EXTCON_PROP_USB_VBUS_VOLTAGE 4 >> +#define EXTCON_PROP_USB_VBUS_CURRENT 5 >> >> #define EXTCON_PROP_USB_MIN 0 >> -#define EXTCON_PROP_USB_MAX 2 >> +#define EXTCON_PROP_USB_MAX 5 >> #define EXTCON_PROP_USB_CNT (EXTCON_PROP_USB_MAX - >> EXTCON_PROP_USB_MIN + 1) >> >> /* Properties of EXTCON_TYPE_CHG. */ >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider 2020-09-14 16:46 [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Angus Ainslie 2020-09-14 16:46 ` [PATCH 1/4] extcon: Add USB VBUS properties Angus Ainslie @ 2020-09-14 16:46 ` Angus Ainslie 2020-09-15 1:19 ` Chanwoo Choi 2020-09-14 16:46 ` [PATCH 3/4] usb: typec: tps6598x: Add the extcon USB chargers Angus Ainslie ` (2 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Angus Ainslie @ 2020-09-14 16:46 UTC (permalink / raw) To: kernel Cc: MyungJoo Ham, Chanwoo Choi, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue, Angus Ainslie 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@akkea.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 + 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, -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider 2020-09-14 16:46 ` [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider Angus Ainslie @ 2020-09-15 1:19 ` Chanwoo Choi 2020-09-15 13:32 ` Angus Ainslie 0 siblings, 1 reply; 12+ messages in thread From: Chanwoo Choi @ 2020-09-15 1:19 UTC (permalink / raw) To: Angus Ainslie, kernel Cc: MyungJoo Ham, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue 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@akkea.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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider 2020-09-15 1:19 ` Chanwoo Choi @ 2020-09-15 13:32 ` Angus Ainslie 0 siblings, 0 replies; 12+ messages in thread From: Angus Ainslie @ 2020-09-15 13:32 UTC (permalink / raw) To: Chanwoo Choi Cc: kernel, MyungJoo Ham, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue 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@akkea.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, >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] usb: typec: tps6598x: Add the extcon USB chargers 2020-09-14 16:46 [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Angus Ainslie 2020-09-14 16:46 ` [PATCH 1/4] extcon: Add USB VBUS properties Angus Ainslie 2020-09-14 16:46 ` [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider Angus Ainslie @ 2020-09-14 16:46 ` Angus Ainslie 2020-09-14 16:46 ` [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq Angus Ainslie 2020-09-21 14:37 ` [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Heikki Krogerus 4 siblings, 0 replies; 12+ messages in thread From: Angus Ainslie @ 2020-09-14 16:46 UTC (permalink / raw) To: kernel Cc: MyungJoo Ham, Chanwoo Choi, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue, Angus Ainslie Notify extcon devcies that the charge current has changed Signed-off-by: Angus Ainslie <angus@akkea.ca> --- drivers/usb/typec/tps6598x.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c index 3b06d43c810d..d5aa58c9da0a 100644 --- a/drivers/usb/typec/tps6598x.c +++ b/drivers/usb/typec/tps6598x.c @@ -240,17 +240,25 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps, EXTCON_PROP_USB_VBUS_SRC, val); + extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0); + switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) { case TYPEC_PWR_MODE_USB: - max_current = 500; + max_current = val.intval = 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; + max_current = val.intval = 1500; + extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state); break; case TYPEC_PWR_MODE_3_0A: - max_current = 3000; + max_current = val.intval = 3000; + extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state); break; case TYPEC_PWR_MODE_PD: max_current = 500; @@ -265,10 +273,14 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps, 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_CDP); + extcon_sync(tps->edev, EXTCON_CHG_USB_DCP); extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW); + extcon_sync(tps->edev, EXTCON_CHG_USB_FAST); + /* do these last for extcon notifiers */ + extcon_sync(tps->edev, EXTCON_USB); + extcon_sync(tps->edev, EXTCON_USB_HOST); } #endif @@ -562,6 +574,8 @@ static const unsigned int tps6598x_extcon_cable[] = { EXTCON_USB, EXTCON_USB_HOST, EXTCON_CHG_USB_SDP, + EXTCON_CHG_USB_CDP, + EXTCON_CHG_USB_DCP, EXTCON_CHG_USB_SLOW, EXTCON_CHG_USB_FAST, EXTCON_NONE, -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq 2020-09-14 16:46 [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Angus Ainslie ` (2 preceding siblings ...) 2020-09-14 16:46 ` [PATCH 3/4] usb: typec: tps6598x: Add the extcon USB chargers Angus Ainslie @ 2020-09-14 16:46 ` Angus Ainslie 2020-09-24 14:42 ` kernel test robot 2020-09-21 14:37 ` [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Heikki Krogerus 4 siblings, 1 reply; 12+ messages in thread From: Angus Ainslie @ 2020-09-14 16:46 UTC (permalink / raw) To: kernel Cc: MyungJoo Ham, Chanwoo Choi, Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue, Angus Ainslie Enable the PD irq and decode the contract. Signed-off-by: Angus Ainslie <angus@akkea.ca> --- drivers/usb/typec/tps6598x.c | 386 ++++++++++++++++++++++++++++++++--- 1 file changed, 360 insertions(+), 26 deletions(-) diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c index d5aa58c9da0a..2eb883ff822c 100644 --- a/drivers/usb/typec/tps6598x.c +++ b/drivers/usb/typec/tps6598x.c @@ -30,11 +30,19 @@ #define TPS_REG_STATUS 0x1a #define TPS_REG_SYSTEM_CONF 0x28 #define TPS_REG_CTRL_CONF 0x29 +#define TPS_REG_ACTIVE_CONTRACT_PDO 0x34 +#define TPS_REG_ACTIVE_CONTRACT_RDO 0x35 #define TPS_REG_POWER_STATUS 0x3f #define TPS_REG_RX_IDENTITY_SOP 0x48 +#define TPS_REG_DATA_STATUS 0x5f /* TPS_REG_INT_* bits */ #define TPS_REG_INT_PLUG_EVENT BIT(3) +#define TPS_REG_INT_NEW_CONTRACT_SNK BIT(12) +#define TPS_REG_INT_PWR_STATUS_UPDATE BIT(24) +#define TPS_REG_INT_DATA_STATUS_UPDATE BIT(25) +#define TPS_REG_INT_STATUS_UPDATE BIT(26) +#define TPS_REG_INT_PD_STATUS_UPDATE BIT(27) /* TPS_REG_STATUS bits */ #define TPS_STATUS_PLUG_PRESENT BIT(0) @@ -57,8 +65,16 @@ enum { }; /* TPS_REG_POWER_STATUS bits */ +#define TPS_POWER_STATUS_CONNECTION BIT(0) #define TPS_POWER_STATUS_SOURCESINK BIT(1) +#define TPS_POWER_STATUS_BC12_CON BIT(4) +#define TPS_POWER_STATUS_BC_SDP 0 +#define TPS_POWER_STATUS_BC_CDP 2 +#define TPS_POWER_STATUS_BC_DCP 3 + +#define TPS_POWER_STATUS_BCOPMODE(p) (((p) & GENMASK(6, 5)) >> 5) #define TPS_POWER_STATUS_PWROPMODE(p) (((p) & GENMASK(3, 2)) >> 2) +#define TPS_POWER_STATUS_SRC(p) (!(((p) & BIT(1)) >> 1)) /* TPS_REG_RX_IDENTITY_SOP */ struct tps6598x_rx_identity_reg { @@ -217,21 +233,237 @@ static void tps6598x_set_data_role(struct tps6598x *tps, } #ifdef CONFIG_EXTCON -static void tps6589x_set_extcon_state(struct tps6598x *tps, - u32 status, u16 pwr_status, bool state) + +/* + * the PDO decoding comes from here + * https://www.ti.com/lit/an/slva842/slva842.pdf + */ + +struct contract_terms { + u8 type; + bool dr_power; + bool dr_data; + bool higher_cap; + bool ext_powered; + bool usb_comms; + int max_voltage; + int max_current; + int max_power; +}; + +#define PDO_CONTRACT_TYPE(x) (x >> 30 & 0x3) +#define PDO_CONTRACT_FIXED 0 +#define PDO_CONTRACT_BATTERY 1 +#define PDO_CONTRACT_VARIABLE 2 + +#define PDO_CONTRACT_DR_POWER BIT(29) +#define PDO_CONTRACT_HIGHER_CAP BIT(28) +#define PDO_CONTRACT_EXTERNAL_PWR BIT(27) +#define PDO_CONTRACT_USB_COMMS BIT(26) +#define PDO_CONTRACT_DR_DATA BIT(25) + +#define PDO_CONTRACT_VOLTAGE(x) ((x >> 10 & 0x3ff) * 50) +#define PDO_CONTRACT_MIN_VOLTAGE(x) PDO_CONTRACT_VOLTAGE(x) +#define PDO_CONTRACT_MAX_VOLTAGE(x) ((x >> 20 & 0x3ff) * 50) +#define PDO_CONTRACT_CURRENT(x) ((x & 0x3ff) * 10) +#define PDO_CONTRACT_POWER(x) ((x & 0x3ff) * 250) + +static int tps6598x_decode_pdo_contract(struct tps6598x *tps, u32 contract, + struct contract_terms *terms) +{ + int min_voltage = 5000; // mV + + memset(terms, 0, sizeof(struct contract_terms)); + + dev_dbg(tps->dev, "%s 0x%x\n", __func__, contract); + + switch (PDO_CONTRACT_TYPE(contract)) { + case PDO_CONTRACT_FIXED: + terms->type = PDO_CONTRACT_FIXED; + if (contract & PDO_CONTRACT_DR_POWER) { + dev_dbg(tps->dev, "Dual role power\n"); + terms->dr_power = true; + } + if (contract & PDO_CONTRACT_DR_DATA) { + dev_dbg(tps->dev, "Dual role data\n"); + terms->dr_data = true; + } + if (contract & PDO_CONTRACT_HIGHER_CAP) { + dev_dbg(tps->dev, "Higher capbility\n"); + terms->higher_cap = true; + } + if (contract & PDO_CONTRACT_EXTERNAL_PWR) { + dev_dbg(tps->dev, "Externally powered\n"); + terms->ext_powered = true; + } + if (contract & PDO_CONTRACT_USB_COMMS) { + dev_dbg(tps->dev, "USB communications capable\n"); + terms->usb_comms = true; + } + terms->max_voltage = PDO_CONTRACT_VOLTAGE(contract); + terms->max_current = PDO_CONTRACT_CURRENT(contract); + terms->max_power = (terms->max_voltage * terms->max_current) / 1000; + dev_dbg(tps->dev, "Fixed contract uVMIN:uVMAX:uA:uW %d:%d:%d:%d\n", + min_voltage, terms->max_voltage, terms->max_current, + terms->max_power); + break; + case PDO_CONTRACT_BATTERY: + min_voltage = PDO_CONTRACT_MIN_VOLTAGE(contract); + terms->max_voltage = PDO_CONTRACT_MAX_VOLTAGE(contract); + terms->max_power = PDO_CONTRACT_POWER(contract); + terms->max_current = terms->max_power / (terms->max_voltage / 1000); + dev_dbg(tps->dev, "Battery contract uVMIN:uVMAX:uA:uW %d:%d:%d:%d\n", + min_voltage, terms->max_voltage, terms->max_current, + terms->max_power); + break; + case PDO_CONTRACT_VARIABLE: + min_voltage = PDO_CONTRACT_MIN_VOLTAGE(contract); + terms->max_voltage = PDO_CONTRACT_MAX_VOLTAGE(contract); + terms->max_current = PDO_CONTRACT_CURRENT(contract); + terms->max_power = (terms->max_voltage * terms->max_current) / 1000; + dev_dbg(tps->dev, "Variable contract uVMIN:uVMAX:uA:uW %d:%d:%d:%d\n", + min_voltage, terms->max_voltage, terms->max_current, + terms->max_power); + break; + default: + dev_warn(tps->dev, "Unknown supply\n"); + return -EINVAL; + } + + return 0; +} + +static int tps6598x_pdo_contract(struct tps6598x *tps, u32 contract) { union extcon_property_value val; - int max_current; + struct contract_terms terms; - if (TPS_STATUS_DATAROLE(status)) { - extcon_set_state(tps->edev, EXTCON_USB, false); - extcon_set_state(tps->edev, EXTCON_USB_HOST, state); + tps6598x_decode_pdo_contract(tps, contract, &terms); + + val.intval = terms.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); + + val.intval = terms.max_voltage; + extcon_set_property(tps->edev, EXTCON_USB, + EXTCON_PROP_USB_VBUS_VOLTAGE, + val); + extcon_set_property(tps->edev, EXTCON_USB_HOST, + EXTCON_PROP_USB_VBUS_VOLTAGE, + val); + + return terms.max_current; +} + +#define RDO_CONTRACT_OBJ_POSITION BIT(29) +#define RDO_CONTRACT_CAPABILITY_MISMATCH BIT(27) +#define RDO_CONTRACT_USB_COMMS BIT(26) +#define RDO_CONTRACT_NO_SUSPEND BIT(25) + +#define RDO_CONTRACT_CURRENT(x) ((x >> 10 & 0x3ff) * 10) +#define RDO_CONTRACT_POWER(x) ((x >> 10 & 0x3ff) * 250) +#define RDO_CONTRACT_MAX_CURRENT(x) ((x & 0x3ff) * 10) +#define RDO_CONTRACT_MAX_POWER(x) ((x & 0x3ff) * 250) + +static int tps6598x_decode_rdo_contract(struct tps6598x *tps, u32 contract, bool battery, + struct contract_terms *terms) +{ + memset(terms, 0, sizeof(struct contract_terms)); + + if (contract & RDO_CONTRACT_OBJ_POSITION) + dev_dbg(tps->dev, "Object position\n"); + if (contract & RDO_CONTRACT_CAPABILITY_MISMATCH) + dev_dbg(tps->dev, "Capbility mismatch\n"); + if (contract & RDO_CONTRACT_USB_COMMS) + dev_dbg(tps->dev, "USB communications capable\n"); + if (contract & RDO_CONTRACT_NO_SUSPEND) + dev_dbg(tps->dev, "No USB suspend\n"); + if (battery) { + terms->max_power = RDO_CONTRACT_MAX_POWER(contract); + dev_dbg(tps->dev, "Power %d max power %d\n", RDO_CONTRACT_POWER(contract), + terms->max_power); } else { - extcon_set_state(tps->edev, EXTCON_USB, state); - extcon_set_state(tps->edev, EXTCON_USB_HOST, false); + terms->max_current = RDO_CONTRACT_MAX_CURRENT(contract); /* mA */ + dev_dbg(tps->dev, "Current %d max current %d\n", RDO_CONTRACT_CURRENT(contract), + terms->max_current); } - val.intval = TPS_STATUS_PORTROLE(status); + return 0; +} + +static int tps6598x_rdo_contract(struct tps6598x *tps, u32 contract, bool battery) +{ + union extcon_property_value val; + struct contract_terms terms; + + if (battery) + dev_dbg(tps->dev, "Battery supply\n"); + + tps6598x_decode_rdo_contract(tps, contract, battery, &terms); + + if (terms.max_current > 0) { + val.intval = terms.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); + + } + + return terms.max_current; +} + +static int tps6598x_set_extcon_pd_state(struct tps6598x *tps, bool state) +{ + u32 pdo_contract, rdo_contract; + int ret, max_current; + + ret = tps6598x_read32(tps, + TPS_REG_ACTIVE_CONTRACT_RDO, + &rdo_contract); + if (ret) { + dev_dbg(tps->dev, + "failed to read RDO contract - try PDO contract\n"); + rdo_contract = 0; + } + + if (rdo_contract != 0) { + max_current = tps6598x_rdo_contract(tps, rdo_contract, false); + } else { + ret = tps6598x_read32(tps, + TPS_REG_ACTIVE_CONTRACT_PDO, + &pdo_contract); + max_current = tps6598x_pdo_contract(tps, pdo_contract); + if (ret || pdo_contract == 0) + dev_err(tps->dev, "failed to read PDO contract\n"); + } + + extcon_set_state(tps->edev, EXTCON_CHG_USB_PD, state); + extcon_sync(tps->edev, EXTCON_CHG_USB_PD); + + return max_current; +} + +static void tps6598x_set_extcon_power_state(struct tps6598x *tps, u32 status, + u16 pwr_status) +{ + int max_current = 100; + union extcon_property_value val; + u8 connected; + + /* USB C power source */ + val.intval = TPS_POWER_STATUS_SRC(pwr_status) && + TPS_STATUS_PORTROLE(status); + + dev_dbg(tps->dev, "Power status 0x%x Source %d\n", pwr_status, + val.intval); + extcon_set_property(tps->edev, EXTCON_USB_HOST, EXTCON_PROP_USB_VBUS_SRC, val); @@ -240,28 +472,55 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps, EXTCON_PROP_USB_VBUS_SRC, val); - extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0); - extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0); - extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0); - extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0); - extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0); + connected = pwr_status & TPS_POWER_STATUS_CONNECTION; + + if (pwr_status & TPS_POWER_STATUS_BC12_CON) { + switch (TPS_POWER_STATUS_BCOPMODE(pwr_status)) { + case TPS_POWER_STATUS_BC_SDP: + /* TODO: detect USB3 900mA */ + extcon_set_state(tps->edev, + EXTCON_CHG_USB_SLOW, connected); + dev_dbg(tps->dev, "BC1.2 SDP detected\n"); + extcon_set_state(tps->edev, + EXTCON_CHG_USB_SDP, connected); + break; + case TPS_POWER_STATUS_BC_CDP: + dev_dbg(tps->dev, "BC1.2 CDP detected\n"); + extcon_set_state(tps->edev, + EXTCON_CHG_USB_CDP, connected); + break; + case TPS_POWER_STATUS_BC_DCP: + dev_dbg(tps->dev, "BC1.2 DCP detected\n"); + extcon_set_state(tps->edev, + EXTCON_CHG_USB_DCP, connected); + break; + default: + dev_dbg(tps->dev, "BC1.2 unknown - reserved\n"); + break; + + } + } switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) { case TYPEC_PWR_MODE_USB: - max_current = val.intval = 500; - extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state); - extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state); + if (max_current < 500) + max_current = 500; + /* TODO: detect UBS3 900mA */ + extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, connected); + extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, connected); break; case TYPEC_PWR_MODE_1_5A: - max_current = val.intval = 1500; - extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state); + if (max_current < 1500) + max_current = 1500; + extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, connected); break; case TYPEC_PWR_MODE_3_0A: - max_current = val.intval = 3000; - extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state); + if (max_current < 3000) + max_current = 3000; break; case TYPEC_PWR_MODE_PD: - max_current = 500; + extcon_set_state(tps->edev, EXTCON_CHG_USB_PD, connected); + max_current = tps6598x_set_extcon_pd_state(tps, connected); break; } @@ -272,6 +531,38 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps, extcon_set_property(tps->edev, EXTCON_USB_HOST, EXTCON_PROP_USB_VBUS_CURRENT, val); +} + +static void tps6598x_set_extcon_state(struct tps6598x *tps, + u32 status, u16 pwr_status, bool state) +{ + union extcon_property_value val; + + 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, + val); + + extcon_set_property(tps->edev, EXTCON_USB, + EXTCON_PROP_USB_VBUS, + val); + + extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0); + extcon_set_state(tps->edev, EXTCON_CHG_USB_PD, 0); + + tps6598x_set_extcon_power_state(tps, status, pwr_status); extcon_sync(tps->edev, EXTCON_CHG_USB_SDP); extcon_sync(tps->edev, EXTCON_CHG_USB_CDP); @@ -323,9 +614,9 @@ 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 + 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)); return 0; } @@ -494,7 +785,12 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data) u64 event1; u64 event2; u32 status; + u32 data_status; + u16 power_status; int ret; +#ifdef CONFIG_EXTCON + bool connected; +#endif mutex_lock(&tps->lock); @@ -514,8 +810,20 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data) goto err_clear_ints; } + connected = status & TPS_STATUS_PLUG_PRESENT; + ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &power_status); + if (ret < 0) + goto err_clear_ints; + + ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &data_status); + if (ret < 0) + goto err_clear_ints; + + dev_dbg(tps->dev, "Status: 0x%x data: 0x%x power: 0x%x", status, + data_status, power_status); + /* Handle plug insert or removal */ - if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT) { + if ((event1 | event2) & (TPS_REG_INT_PLUG_EVENT)) { if (status & TPS_STATUS_PLUG_PRESENT) { ret = tps6598x_connect(tps, status); if (ret) @@ -526,6 +834,19 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data) } } +#ifdef CONFIG_EXTCON + /* updated power or data status register */ + /* power bits sometimes change without triggering an interrupt */ + if ((event1 | event2) & (TPS_REG_INT_STATUS_UPDATE | + TPS_REG_INT_PWR_STATUS_UPDATE | + TPS_REG_INT_DATA_STATUS_UPDATE | + TPS_REG_INT_NEW_CONTRACT_SNK | + TPS_REG_INT_PD_STATUS_UPDATE)) { + dev_dbg(tps->dev, "%s: update extcon\n", __func__); + tps6598x_set_extcon_state(tps, status, power_status, connected); + } +#endif + err_clear_ints: tps6598x_write64(tps, TPS_REG_INT_CLEAR1, event1); tps6598x_write64(tps, TPS_REG_INT_CLEAR2, event2); @@ -591,6 +912,7 @@ static int tps6598x_probe(struct i2c_client *client) u32 conf; u32 vid; int ret; + u64 mask; tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL); if (!tps) @@ -735,6 +1057,18 @@ static int tps6598x_probe(struct i2c_client *client) goto err_role_put; } + mask = TPS_REG_INT_NEW_CONTRACT_SNK + | TPS_REG_INT_PLUG_EVENT + | TPS_REG_INT_PWR_STATUS_UPDATE + | TPS_REG_INT_DATA_STATUS_UPDATE + | TPS_REG_INT_STATUS_UPDATE + | TPS_REG_INT_PD_STATUS_UPDATE; + + ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, mask); + ret |= tps6598x_write64(tps, TPS_REG_INT_MASK2, mask); + if (ret < 0) + dev_err(&client->dev, "failed to set irq mask %d\n", ret); + i2c_set_clientdata(client, tps); return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq 2020-09-14 16:46 ` [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq Angus Ainslie @ 2020-09-24 14:42 ` kernel test robot 0 siblings, 0 replies; 12+ messages in thread From: kernel test robot @ 2020-09-24 14:42 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 3562 bytes --] Hi Angus, Thank you for the patch! Yet something to improve: [auto build test ERROR on usb/usb-testing] [also build test ERROR on chanwoo-extcon/extcon-next v5.9-rc6 next-20200924] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Angus-Ainslie/RFC-USB-C-extcon-patchset-for-the-tps6598x/20200915-010019 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: riscv-randconfig-r034-20200923 (attached as .config) compiler: riscv32-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/usb/typec/tps6598x.c: In function 'tps6598x_interrupt': >> drivers/usb/typec/tps6598x.c:813:2: error: 'connected' undeclared (first use in this function) 813 | connected = status & TPS_STATUS_PLUG_PRESENT; | ^~~~~~~~~ drivers/usb/typec/tps6598x.c:813:2: note: each undeclared identifier is reported only once for each function it appears in # https://github.com/0day-ci/linux/commit/2ddad1e8ed61dd0c0c003e334ae0085519a7d44b git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Angus-Ainslie/RFC-USB-C-extcon-patchset-for-the-tps6598x/20200915-010019 git checkout 2ddad1e8ed61dd0c0c003e334ae0085519a7d44b vim +/connected +813 drivers/usb/typec/tps6598x.c 794 795 mutex_lock(&tps->lock); 796 797 ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1); 798 ret |= tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2); 799 if (ret) { 800 dev_err(tps->dev, "%s: failed to read events\n", __func__); 801 goto err_unlock; 802 } 803 804 dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1, 805 (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF)); 806 807 ret = tps6598x_read32(tps, TPS_REG_STATUS, &status); 808 if (ret) { 809 dev_err(tps->dev, "%s: failed to read status\n", __func__); 810 goto err_clear_ints; 811 } 812 > 813 connected = status & TPS_STATUS_PLUG_PRESENT; 814 ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &power_status); 815 if (ret < 0) 816 goto err_clear_ints; 817 818 ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &data_status); 819 if (ret < 0) 820 goto err_clear_ints; 821 822 dev_dbg(tps->dev, "Status: 0x%x data: 0x%x power: 0x%x", status, 823 data_status, power_status); 824 825 /* Handle plug insert or removal */ 826 if ((event1 | event2) & (TPS_REG_INT_PLUG_EVENT)) { 827 if (status & TPS_STATUS_PLUG_PRESENT) { 828 ret = tps6598x_connect(tps, status); 829 if (ret) 830 dev_err(tps->dev, 831 "failed to register partner\n"); 832 } else { 833 tps6598x_disconnect(tps, status); 834 } 835 } 836 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 26586 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x 2020-09-14 16:46 [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Angus Ainslie ` (3 preceding siblings ...) 2020-09-14 16:46 ` [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq Angus Ainslie @ 2020-09-21 14:37 ` Heikki Krogerus 2020-09-22 21:15 ` Angus Ainslie 4 siblings, 1 reply; 12+ messages in thread From: Heikki Krogerus @ 2020-09-21 14:37 UTC (permalink / raw) To: Angus Ainslie Cc: kernel, MyungJoo Ham, Chanwoo Choi, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote: > We have a complex set of hardware components to manage our USB C data and > power. For these to work together we decided to use extcon to communicate > the system changes as various cables and devices are plugged in/out. We did > look at usb_roleswitch and the charging framework but thought it would be > preferable to keep all of the information together in one system. > > The components we have in the system are: > > 1) TPS65982 type USB type C controller > 2) dwc3 IP in the imx8mq > 3) BQ25895 battery charger > > I'll break this into 2 parts the data role and the power role. > > For the data role the TPS65982 senses connect and disconnect as well as data > source/sink. It is also controlling the USB 3 data lanes. The display port and > USB 3 muxing is handled by a different chip and we'll submit patches for that > later on. The dwc3 controls the USB 2 data lanes. > > On the power side there are even more moving pieces. The TPS65982 negotiates > the power delivery contract, the dwc3 senses the BC1.2 charging current and the > BQ25895 sets whether we are sinking or sourcing current and what the current > limit is of the sink and source. > > For both the data and power roles no single chip has all of the required > information. Is using extcon the correct way of doing this and if not what > are the alternatives ? Do not use extcon with the Type-C drivers unless you have some really good reason for not using the dedicated frameworks for each thing. The reason why we even have some of the dedicated frameworks in the first place, for example the USB role switch class, is because extcon simply could not be made to work on every type of hardware architecture. So you will need to register a power supply in tps6598x.c just like the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982 does not communicated directly with the BQ25895. That can be one of "supplied_from" (and also "supplied_to" if needed for sourcing) for the bq25890_changer. You probable only need to implement the external_power_changed() hook for it if it's missing in order to make it work. You can also register a power supply in dwc3 and use it as a second supply for bq25890 if you still really need to handle BC1.2. The data role should already be handled for you. dwc3 already registers an USB role switch, and tps6598x.c already configures one. For data role you should not need any additional code. Please note that there is also framework for the alt mode muxes. > The extcon extensions allow us to communicate the the power roles amongst > the various chips. > > This patch series has been tested with the 5.9-rc4 kernel on the Purism > Librem5 HW. Assuming this is the correct way to use extcon there will be > follow on patches to the BQ25895 and dwc3 drivers. > > Strictly speaking only the first 3 patches are needed for extcon support, the > forth patch decodes the power delivery contracts which makes use of the extcon > system. > > > Angus Ainslie (4): > extcon: Add USB VBUS properties > usb: typec: tps6589x: register as an extcon provider > usb: typec: tps6598x: Add the extcon USB chargers > usb: typec: tps6598x: Add the power delivery irq > > drivers/usb/typec/tps6598x.c | 488 ++++++++++++++++++++++++++++++++++- > include/linux/extcon.h | 17 +- > 2 files changed, 503 insertions(+), 2 deletions(-) > > -- > 2.25.1 thanks, -- heikki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x 2020-09-21 14:37 ` [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Heikki Krogerus @ 2020-09-22 21:15 ` Angus Ainslie 0 siblings, 0 replies; 12+ messages in thread From: Angus Ainslie @ 2020-09-22 21:15 UTC (permalink / raw) To: Heikki Krogerus Cc: kernel, MyungJoo Ham, Chanwoo Choi, Greg Kroah-Hartman, linux-kernel, linux-usb, bryan.odonoghue Hi Heikki, On 2020-09-21 07:37, Heikki Krogerus wrote: > On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote: >> We have a complex set of hardware components to manage our USB C data >> and >> power. For these to work together we decided to use extcon to >> communicate >> the system changes as various cables and devices are plugged in/out. >> We did >> look at usb_roleswitch and the charging framework but thought it would >> be >> preferable to keep all of the information together in one system. >> >> The components we have in the system are: >> >> 1) TPS65982 type USB type C controller >> 2) dwc3 IP in the imx8mq >> 3) BQ25895 battery charger >> >> I'll break this into 2 parts the data role and the power role. >> >> For the data role the TPS65982 senses connect and disconnect as well >> as data >> source/sink. It is also controlling the USB 3 data lanes. The display >> port and >> USB 3 muxing is handled by a different chip and we'll submit patches >> for that >> later on. The dwc3 controls the USB 2 data lanes. >> >> On the power side there are even more moving pieces. The TPS65982 >> negotiates >> the power delivery contract, the dwc3 senses the BC1.2 charging >> current and the >> BQ25895 sets whether we are sinking or sourcing current and what the >> current >> limit is of the sink and source. >> >> For both the data and power roles no single chip has all of the >> required >> information. Is using extcon the correct way of doing this and if not >> what >> are the alternatives ? > > Do not use extcon with the Type-C drivers unless you have some really > good reason for not using the dedicated frameworks for each thing. The > reason why we even have some of the dedicated frameworks in the first > place, for example the USB role switch class, is because extcon simply > could not be made to work on every type of hardware architecture. > > So you will need to register a power supply in tps6598x.c just like > the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982 > does not communicated directly with the BQ25895. That can be one > of "supplied_from" (and also "supplied_to" if needed for sourcing) for > the bq25890_changer. You probable only need to implement the > external_power_changed() hook for it if it's missing in order to make > it work. You can also register a power supply in dwc3 and use it as a > second supply for bq25890 if you still really need to handle BC1.2. > > The data role should already be handled for you. dwc3 already > registers an USB role switch, and tps6598x.c already configures one. > For data role you should not need any additional code. > > Please note that there is also framework for the alt mode muxes. > Thanks for looking this over. I'll investigate the power supply framework. Angus > >> The extcon extensions allow us to communicate the the power roles >> amongst >> the various chips. >> >> This patch series has been tested with the 5.9-rc4 kernel on the >> Purism >> Librem5 HW. Assuming this is the correct way to use extcon there will >> be >> follow on patches to the BQ25895 and dwc3 drivers. >> >> Strictly speaking only the first 3 patches are needed for extcon >> support, the >> forth patch decodes the power delivery contracts which makes use of >> the extcon >> system. >> >> >> Angus Ainslie (4): >> extcon: Add USB VBUS properties >> usb: typec: tps6589x: register as an extcon provider >> usb: typec: tps6598x: Add the extcon USB chargers >> usb: typec: tps6598x: Add the power delivery irq >> >> drivers/usb/typec/tps6598x.c | 488 >> ++++++++++++++++++++++++++++++++++- >> include/linux/extcon.h | 17 +- >> 2 files changed, 503 insertions(+), 2 deletions(-) >> >> -- >> 2.25.1 > > thanks, ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-09-24 14:42 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-14 16:46 [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Angus Ainslie 2020-09-14 16:46 ` [PATCH 1/4] extcon: Add USB VBUS properties Angus Ainslie 2020-09-15 1:40 ` Chanwoo Choi 2020-09-15 13:31 ` Angus Ainslie 2020-09-14 16:46 ` [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider Angus Ainslie 2020-09-15 1:19 ` Chanwoo Choi 2020-09-15 13:32 ` Angus Ainslie 2020-09-14 16:46 ` [PATCH 3/4] usb: typec: tps6598x: Add the extcon USB chargers Angus Ainslie 2020-09-14 16:46 ` [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq Angus Ainslie 2020-09-24 14:42 ` kernel test robot 2020-09-21 14:37 ` [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x Heikki Krogerus 2020-09-22 21:15 ` Angus Ainslie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.