Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Peter Rosin]

On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
> which can expose details collected by the TUSB320 in a far more
> precise way than extcon. Since there are existing users in the
> kernel and in DT which depend on the extcon interface, keep it
> for now.
> 
> Add TYPE-C interface and expose the supported supply current,
> direction and connector polarity via the TYPE-C interface.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Hi!

I'm very sad to report that this patch has caused a severe
regression for our devices. They can no longer be powered by simply
sticking a USB cable into their USB ports (they keep resetting
when the tusb320 driver probes), and they reset (and keep resetting)
if a USB cable is connected while the device is powered by other
means.

We have no connector node nor any typec-power-opmode property
in the device tree, so the mere touching of the new registers
happening in this patch is enough to cause the resets.

The devices were fine before this patch, and reverting on top
of a recent linus master (4bbf3422df78, from 11/11) makes the
problem go away.

Any suggestions?

Cheers,
Peter

regzbot: introduced bf7571c00dca

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support #forregzbot

[Thorsten Leemhuis]

[Note: this mail is primarily send for documentation purposes and/or for
regzbot, my Linux kernel regression tracking bot. That's why I removed
most or all folks from the list of recipients, but left any that looked
like a mailing lists. These mails usually contain '#forregzbot' in the
subject, to make them easy to spot and filter out.]

[reduced set or recipients, as this is only for regzbot]

On 13.11.22 17:59, Peter Rosin wrote:
> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>> which can expose details collected by the TUSB320 in a far more
>> precise way than extcon. Since there are existing users in the
>> kernel and in DT which depend on the extcon interface, keep it
>> for now.
>>
>> Add TYPE-C interface and expose the supported supply current,
>> direction and connector polarity via the TYPE-C interface.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
> 
> Hi!
> 
> I'm very sad to report that this patch has caused a severe
> regression for our devices. They can no longer be powered by simply
> sticking a USB cable into their USB ports (they keep resetting
> when the tusb320 driver probes), and they reset (and keep resetting)
> if a USB cable is connected while the device is powered by other
> means.
> 
> We have no connector node nor any typec-power-opmode property
> in the device tree, so the mere touching of the new registers
> happening in this patch is enough to cause the resets.
> 
> The devices were fine before this patch, and reverting on top
> of a recent linus master (4bbf3422df78, from 11/11) makes the
> problem go away.
> 
> Any suggestions?
> 
> Cheers,
> Peter
> 
> regzbot: introduced bf7571c00dca

Peter, many thx for getting regzbot in the loop, there was a small typo,
let me take care of that:

#regzbot ^introduced bf7571c00dca
#regzbot title usb: devices are not powered anymore and sometimes reset

Ciao, Thorsten

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Marek Vasut]

On 11/13/22 17:59, Peter Rosin wrote:
> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>> which can expose details collected by the TUSB320 in a far more
>> precise way than extcon. Since there are existing users in the
>> kernel and in DT which depend on the extcon interface, keep it
>> for now.
>>
>> Add TYPE-C interface and expose the supported supply current,
>> direction and connector polarity via the TYPE-C interface.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
> 
> Hi!
> 
> I'm very sad to report that this patch has caused a severe
> regression for our devices. They can no longer be powered by simply
> sticking a USB cable into their USB ports (they keep resetting
> when the tusb320 driver probes), and they reset (and keep resetting)
> if a USB cable is connected while the device is powered by other
> means.
> 
> We have no connector node nor any typec-power-opmode property
> in the device tree, so the mere touching of the new registers
> happening in this patch is enough to cause the resets.
> 
> The devices were fine before this patch, and reverting on top
> of a recent linus master (4bbf3422df78, from 11/11) makes the
> problem go away.
> 
> Any suggestions?

Can you test:

341fd15e2e18c ("extcon: usbc-tusb320: Call the Type-C IRQ handler only 
if a port is registered")

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=341fd15e2e18c24d5c738496cfc3d7a272241201

?

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Peter Rosin]

Hi!

2022-11-13 at 19:48, Marek Vasut wrote:
> On 11/13/22 17:59, Peter Rosin wrote:
>> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>>> which can expose details collected by the TUSB320 in a far more
>>> precise way than extcon. Since there are existing users in the
>>> kernel and in DT which depend on the extcon interface, keep it
>>> for now.
>>>
>>> Add TYPE-C interface and expose the supported supply current,
>>> direction and connector polarity via the TYPE-C interface.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>
>> Hi!
>>
>> I'm very sad to report that this patch has caused a severe
>> regression for our devices. They can no longer be powered by simply
>> sticking a USB cable into their USB ports (they keep resetting
>> when the tusb320 driver probes), and they reset (and keep resetting)
>> if a USB cable is connected while the device is powered by other
>> means.
>>
>> We have no connector node nor any typec-power-opmode property
>> in the device tree, so the mere touching of the new registers
>> happening in this patch is enough to cause the resets.
>>
>> The devices were fine before this patch, and reverting on top
>> of a recent linus master (4bbf3422df78, from 11/11) makes the
>> problem go away.
>>
>> Any suggestions?
> 
> Can you test:
> 
> 341fd15e2e18c ("extcon: usbc-tusb320: Call the Type-C IRQ handler only if a port is registered")
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=341fd15e2e18c24d5c738496cfc3d7a272241201
> 
> ?

Yup, that solves it. Thanks!

Cheers,
Peter

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Marek Vasut]

On 11/13/22 20:41, Peter Rosin wrote:
> Hi!

Hi,

> 2022-11-13 at 19:48, Marek Vasut wrote:
>> On 11/13/22 17:59, Peter Rosin wrote:
>>> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>>>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>>>> which can expose details collected by the TUSB320 in a far more
>>>> precise way than extcon. Since there are existing users in the
>>>> kernel and in DT which depend on the extcon interface, keep it
>>>> for now.
>>>>
>>>> Add TYPE-C interface and expose the supported supply current,
>>>> direction and connector polarity via the TYPE-C interface.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>
>>> Hi!
>>>
>>> I'm very sad to report that this patch has caused a severe
>>> regression for our devices. They can no longer be powered by simply
>>> sticking a USB cable into their USB ports (they keep resetting
>>> when the tusb320 driver probes), and they reset (and keep resetting)
>>> if a USB cable is connected while the device is powered by other
>>> means.
>>>
>>> We have no connector node nor any typec-power-opmode property
>>> in the device tree, so the mere touching of the new registers
>>> happening in this patch is enough to cause the resets.
>>>
>>> The devices were fine before this patch, and reverting on top
>>> of a recent linus master (4bbf3422df78, from 11/11) makes the
>>> problem go away.
>>>
>>> Any suggestions?
>>
>> Can you test:
>>
>> 341fd15e2e18c ("extcon: usbc-tusb320: Call the Type-C IRQ handler only if a port is registered")
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=341fd15e2e18c24d5c738496cfc3d7a272241201
>>
>> ?
> 
> Yup, that solves it. Thanks!

You're welcome.

Since it has a Fixes: tag, it will land in stable releases shortly too I 
think.

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support #forregzbot

[Thorsten Leemhuis]

On 13.11.22 18:38, Thorsten Leemhuis wrote:
> [Note: this mail is primarily send for documentation purposes and/or for
> regzbot, my Linux kernel regression tracking bot. That's why I removed
> most or all folks from the list of recipients, but left any that looked
> like a mailing lists. These mails usually contain '#forregzbot' in the
> subject, to make them easy to spot and filter out.]

>> I'm very sad to report that this patch has caused a severe
>> regression for our devices. They can no longer be powered by simply
>> sticking a USB cable into their USB ports (they keep resetting
>> when the tusb320 driver probes), and they reset (and keep resetting)
>> if a USB cable is connected while the device is powered by other
>> means.
>>
>> We have no connector node nor any typec-power-opmode property
>> in the device tree, so the mere touching of the new registers
>> happening in this patch is enough to cause the resets.
>>
>> The devices were fine before this patch, and reverting on top
>> of a recent linus master (4bbf3422df78, from 11/11) makes the
>> problem go away.
>
>> regzbot: introduced bf7571c00dca
> 
> Peter, many thx for getting regzbot in the loop, there was a small typo,
> let me take care of that:
> 
> #regzbot ^introduced bf7571c00dca
> #regzbot title usb: devices are not powered anymore and sometimes reset

#regzbot fixed-by 341fd15e2e18c24d5c738496cfc3d7a272241201

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Chanwoo Choi]

On 22. 7. 31. 03:05, Marek Vasut wrote:
> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
> which can expose details collected by the TUSB320 in a far more
> precise way than extcon. Since there are existing users in the
> kernel and in DT which depend on the extcon interface, keep it
> for now.
> 
> Add TYPE-C interface and expose the supported supply current,
> direction and connector polarity via the TYPE-C interface.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Yassine Oudjana <y.oudjana@protonmail.com>
> To: linux-usb@vger.kernel.org
> ---
>  drivers/extcon/Kconfig               |   2 +-
>  drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index c69d40ae5619a..7684b3afa6304 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -180,7 +180,7 @@ config EXTCON_USBC_CROS_EC
>  
>  config EXTCON_USBC_TUSB320
>  	tristate "TI TUSB320 USB-C extcon support"
> -	depends on I2C
> +	depends on I2C && TYPEC
>  	select REGMAP_I2C
>  	help
>  	  Say Y here to enable support for USB Type C cable detection extcon
> diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
> index aced4bbb455dc..edb8c3f997c91 100644
> --- a/drivers/extcon/extcon-usbc-tusb320.c
> +++ b/drivers/extcon/extcon-usbc-tusb320.c
> @@ -6,6 +6,7 @@
>   * Author: Michael Auchter <michael.auchter@ni.com>
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/extcon-provider.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
> @@ -13,6 +14,24 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> +#include <linux/usb/typec.h>
> +
> +#define TUSB320_REG8				0x8
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB	0x0
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A	0x1
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A	0x2
> +#define TUSB320_REG8_CURRENT_MODE_DETECT	GENMASK(5, 4)
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_DEF	0x0
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
> +#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
> +#define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
>  
>  #define TUSB320_REG9				0x9
>  #define TUSB320_REG9_ATTACHED_STATE_SHIFT	6
> @@ -55,6 +74,10 @@ struct tusb320_priv {
>  	struct extcon_dev *edev;
>  	struct tusb320_ops *ops;
>  	enum tusb320_attached_state state;
> +	struct typec_port *port;
> +	struct typec_capability	cap;
> +	enum typec_port_type port_type;
> +	enum typec_pwr_opmode pwr_opmode;
>  };
>  
>  static const char * const tusb_attached_states[] = {
> @@ -184,6 +207,44 @@ static struct tusb320_ops tusb320l_ops = {
>  	.get_revision = tusb320l_get_revision,
>  };
>  
> +static int tusb320_set_adv_pwr_mode(struct tusb320_priv *priv)
> +{
> +	u8 mode;
> +
> +	if (priv->pwr_opmode == TYPEC_PWR_MODE_USB)
> +		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB;
> +	else if (priv->pwr_opmode == TYPEC_PWR_MODE_1_5A)
> +		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A;
> +	else if (priv->pwr_opmode == TYPEC_PWR_MODE_3_0A)
> +		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A;
> +	else	/* No other mode is supported. */
> +		return -EINVAL;
> +
> +	return regmap_write_bits(priv->regmap, TUSB320_REG8,
> +				 TUSB320_REG8_CURRENT_MODE_ADVERTISE,
> +				 FIELD_PREP(TUSB320_REG8_CURRENT_MODE_ADVERTISE,
> +					    mode));
> +}
> +
> +static int tusb320_port_type_set(struct typec_port *port,
> +				 enum typec_port_type type)
> +{
> +	struct tusb320_priv *priv = typec_get_drvdata(port);
> +
> +	if (type == TYPEC_PORT_SRC)
> +		return priv->ops->set_mode(priv, TUSB320_MODE_DFP);
> +	else if (type == TYPEC_PORT_SNK)
> +		return priv->ops->set_mode(priv, TUSB320_MODE_UFP);
> +	else if (type == TYPEC_PORT_DRP)
> +		return priv->ops->set_mode(priv, TUSB320_MODE_DRP);
> +	else
> +		return priv->ops->set_mode(priv, TUSB320_MODE_PORT);
> +}
> +
> +static const struct typec_operations tusb320_typec_ops = {
> +	.port_type_set	= tusb320_port_type_set,
> +};
> +
>  static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
>  {
>  	int state, polarity;
> @@ -211,6 +272,47 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
>  	priv->state = state;
>  }
>  
> +static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> +{
> +	struct typec_port *port = priv->port;
> +	struct device *dev = priv->dev;
> +	u8 mode, role, state;
> +	int ret, reg8;
> +	bool ori;
> +
> +	ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
> +	typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
> +					  TYPEC_ORIENTATION_NORMAL);
> +
> +	state = (reg9 >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
> +		TUSB320_REG9_ATTACHED_STATE_MASK;
> +	if (state == TUSB320_ATTACHED_STATE_DFP)
> +		role = TYPEC_SOURCE;
> +	else
> +		role = TYPEC_SINK;
> +
> +	typec_set_vconn_role(port, role);
> +	typec_set_pwr_role(port, role);
> +	typec_set_data_role(port, role == TYPEC_SOURCE ?
> +				  TYPEC_HOST : TYPEC_DEVICE);
> +
> +	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
> +	if (ret) {
> +		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
> +		return;
> +	}
> +
> +	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
> +	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
> +	else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_MED)
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_1_5A);
> +	else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_HI)
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_3_0A);
> +	else	/* Charge through accessory */
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
> +}
> +
>  static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
>  {
>  	struct tusb320_priv *priv = dev_id;
> @@ -225,6 +327,7 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
>  		return IRQ_NONE;
>  
>  	tusb320_extcon_irq_handler(priv, reg);
> +	tusb320_typec_irq_handler(priv, reg);
>  
>  	regmap_write(priv->regmap, TUSB320_REG9, reg);
>  
> @@ -260,6 +363,58 @@ static int tusb320_extcon_probe(struct tusb320_priv *priv)
>  	return 0;
>  }
>  
> +static int tusb320_typec_probe(struct i2c_client *client,
> +			       struct tusb320_priv *priv)
> +{
> +	struct fwnode_handle *connector;
> +	const char *cap_str;
> +	int ret;
> +
> +	/* The Type-C connector is optional, for backward compatibility. */
> +	connector = device_get_named_child_node(&client->dev, "connector");
> +	if (!connector)
> +		return 0;
> +
> +	/* Type-C connector found. */
> +	ret = typec_get_fw_cap(&priv->cap, connector);
> +	if (ret)
> +		return ret;
> +
> +	priv->port_type = priv->cap.type;
> +
> +	/* This goes into register 0x8 field CURRENT_MODE_ADVERTISE */
> +	ret = fwnode_property_read_string(connector, "typec-power-opmode", &cap_str);
> +	if (ret)
> +		return ret;
> +
> +	ret = typec_find_pwr_opmode(cap_str);
> +	if (ret < 0)
> +		return ret;
> +	if (ret == TYPEC_PWR_MODE_PD)
> +		return -EINVAL;
> +
> +	priv->pwr_opmode = ret;
> +
> +	/* Initialize the hardware with the devicetree settings. */
> +	ret = tusb320_set_adv_pwr_mode(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->cap.revision		= USB_TYPEC_REV_1_1;
> +	priv->cap.accessory[0]		= TYPEC_ACCESSORY_AUDIO;
> +	priv->cap.accessory[1]		= TYPEC_ACCESSORY_DEBUG;
> +	priv->cap.orientation_aware	= true;
> +	priv->cap.driver_data		= priv;
> +	priv->cap.ops			= &tusb320_typec_ops;
> +	priv->cap.fwnode		= connector;
> +
> +	priv->port = typec_register_port(&client->dev, &priv->cap);
> +	if (IS_ERR(priv->port))
> +		return PTR_ERR(priv->port);
> +
> +	return 0;
> +}
> +
>  static int tusb320_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -300,6 +455,10 @@ static int tusb320_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> +	ret = tusb320_typec_probe(client, priv);
> +	if (ret)
> +		return ret;
> +
>  	/* update initial state */
>  	tusb320_irq_handler(client->irq, priv);
>  

Applied it. Thanks.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Marek Vasut]

On 8/23/22 14:57, Alvin Šipraga wrote:
> On Tue, Aug 23, 2022 at 12:39:28PM +0200, Marek Vasut wrote:
>> On 8/23/22 11:49, Alvin Šipraga wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>>>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>>>> which can expose details collected by the TUSB320 in a far more
>>>> precise way than extcon. Since there are existing users in the
>>>> kernel and in DT which depend on the extcon interface, keep it
>>>> for now.
>>>>
>>>> Add TYPE-C interface and expose the supported supply current,
>>>> direction and connector polarity via the TYPE-C interface.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Cc: Chanwoo Choi <cw00.choi@samsung.com>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>>> Cc: Yassine Oudjana <y.oudjana@protonmail.com>
>>>> To: linux-usb@vger.kernel.org
>>>> ---
>>>>    drivers/extcon/Kconfig               |   2 +-
>>>>    drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
>>>>    2 files changed, 160 insertions(+), 1 deletion(-)
>>>
>>> Happy to see I'm not the only one that observed this. I wonder if you
>>> saw also my previous stab at this:
>>>
>>> https://lore.kernel.org/linux-usb/20220301132010.115258-1-alvin@pqrs.dk/
>>
>> I have not.
>>
>>> I had some issues with the dt-bindings which I could not reconcile, but
>>> the basic problem was how to describe a typec accessory mode mux
>>> connected to the TUSB320. Perhaps you have a better intuition for how
>>> this should look?
>>>
>>> One thing that is missing from your implementation that we are using on
>>> our end is the USB role switch. I set this from the typec driver via
>>> usb_role_switch_set_role().
>>
>> I only use this chip to detect charger type (and cable polarity), the device
>> where this is integrated is always peripheral and cannot charge other
>> devices or become host.
>>
>> But I think those aforementioned requirements could be extended on top of
>> this patch, can they not ? I recall I looked at least at the direction
>> detection and that could be added easily. I have no way of testing any of
>> that functionality, so I didn't add them as part of the patch.
> 
> Sure - if your patch gets merged then I'll just extend it. Fair enough
> that you cannot test on your board.
> 
> To that end, you can add:
> 
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Thanks.

If you plan to submit anything on top, I might be able to test at least 
the charger detect and plug orientation still works ... but that's 
probably something you can also test on your own, that's the easy part 
of the USB-C.

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Alvin Šipraga]

On Tue, Aug 23, 2022 at 12:39:28PM +0200, Marek Vasut wrote:
> On 8/23/22 11:49, Alvin Šipraga wrote:
> > Hi Marek,
> 
> Hi,
> 
> > On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
> > > The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
> > > which can expose details collected by the TUSB320 in a far more
> > > precise way than extcon. Since there are existing users in the
> > > kernel and in DT which depend on the extcon interface, keep it
> > > for now.
> > > 
> > > Add TYPE-C interface and expose the supported supply current,
> > > direction and connector polarity via the TYPE-C interface.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > Cc: Yassine Oudjana <y.oudjana@protonmail.com>
> > > To: linux-usb@vger.kernel.org
> > > ---
> > >   drivers/extcon/Kconfig               |   2 +-
> > >   drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
> > >   2 files changed, 160 insertions(+), 1 deletion(-)
> > 
> > Happy to see I'm not the only one that observed this. I wonder if you
> > saw also my previous stab at this:
> > 
> > https://lore.kernel.org/linux-usb/20220301132010.115258-1-alvin@pqrs.dk/
> 
> I have not.
> 
> > I had some issues with the dt-bindings which I could not reconcile, but
> > the basic problem was how to describe a typec accessory mode mux
> > connected to the TUSB320. Perhaps you have a better intuition for how
> > this should look?
> > 
> > One thing that is missing from your implementation that we are using on
> > our end is the USB role switch. I set this from the typec driver via
> > usb_role_switch_set_role().
> 
> I only use this chip to detect charger type (and cable polarity), the device
> where this is integrated is always peripheral and cannot charge other
> devices or become host.
> 
> But I think those aforementioned requirements could be extended on top of
> this patch, can they not ? I recall I looked at least at the direction
> detection and that could be added easily. I have no way of testing any of
> that functionality, so I didn't add them as part of the patch.

Sure - if your patch gets merged then I'll just extend it. Fair enough
that you cannot test on your board.

To that end, you can add:

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Thanks!

Kind regards,
Alvin

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Marek Vasut]

On 8/2/22 09:40, Heikki Krogerus wrote:
> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>> which can expose details collected by the TUSB320 in a far more
>> precise way than extcon. Since there are existing users in the
>> kernel and in DT which depend on the extcon interface, keep it
>> for now.
>>
>> Add TYPE-C interface and expose the supported supply current,
>> direction and connector polarity via the TYPE-C interface.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
> 
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

I wonder, can this now be applied ?

The LKP robot is picking up on macro name, i.e. the report seems bogus ?

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Marek Vasut]

On 8/23/22 11:49, Alvin Šipraga wrote:
> Hi Marek,

Hi,

> On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
>> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
>> which can expose details collected by the TUSB320 in a far more
>> precise way than extcon. Since there are existing users in the
>> kernel and in DT which depend on the extcon interface, keep it
>> for now.
>>
>> Add TYPE-C interface and expose the supported supply current,
>> direction and connector polarity via the TYPE-C interface.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Chanwoo Choi <cw00.choi@samsung.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Cc: Yassine Oudjana <y.oudjana@protonmail.com>
>> To: linux-usb@vger.kernel.org
>> ---
>>   drivers/extcon/Kconfig               |   2 +-
>>   drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
>>   2 files changed, 160 insertions(+), 1 deletion(-)
> 
> Happy to see I'm not the only one that observed this. I wonder if you
> saw also my previous stab at this:
> 
> https://lore.kernel.org/linux-usb/20220301132010.115258-1-alvin@pqrs.dk/

I have not.

> I had some issues with the dt-bindings which I could not reconcile, but
> the basic problem was how to describe a typec accessory mode mux
> connected to the TUSB320. Perhaps you have a better intuition for how
> this should look?
> 
> One thing that is missing from your implementation that we are using on
> our end is the USB role switch. I set this from the typec driver via
> usb_role_switch_set_role().

I only use this chip to detect charger type (and cable polarity), the 
device where this is integrated is always peripheral and cannot charge 
other devices or become host.

But I think those aforementioned requirements could be extended on top 
of this patch, can they not ? I recall I looked at least at the 
direction detection and that could be added easily. I have no way of 
testing any of that functionality, so I didn't add them as part of the 
patch.

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Alvin Šipraga]

Hi Marek,

On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
> which can expose details collected by the TUSB320 in a far more
> precise way than extcon. Since there are existing users in the
> kernel and in DT which depend on the extcon interface, keep it
> for now.
> 
> Add TYPE-C interface and expose the supported supply current,
> direction and connector polarity via the TYPE-C interface.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Yassine Oudjana <y.oudjana@protonmail.com>
> To: linux-usb@vger.kernel.org
> ---
>  drivers/extcon/Kconfig               |   2 +-
>  drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+), 1 deletion(-)

Happy to see I'm not the only one that observed this. I wonder if you
saw also my previous stab at this:

https://lore.kernel.org/linux-usb/20220301132010.115258-1-alvin@pqrs.dk/

I had some issues with the dt-bindings which I could not reconcile, but
the basic problem was how to describe a typec accessory mode mux
connected to the TUSB320. Perhaps you have a better intuition for how
this should look?

One thing that is missing from your implementation that we are using on
our end is the USB role switch. I set this from the typec driver via
usb_role_switch_set_role().

Kind regards,
Alvin

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Heikki Krogerus]

On Sat, Jul 30, 2022 at 08:05:00PM +0200, Marek Vasut wrote:
> The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
> which can expose details collected by the TUSB320 in a far more
> precise way than extcon. Since there are existing users in the
> kernel and in DT which depend on the extcon interface, keep it
> for now.
> 
> Add TYPE-C interface and expose the supported supply current,
> direction and connector polarity via the TYPE-C interface.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Yassine Oudjana <y.oudjana@protonmail.com>
> To: linux-usb@vger.kernel.org
> ---
>  drivers/extcon/Kconfig               |   2 +-
>  drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index c69d40ae5619a..7684b3afa6304 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -180,7 +180,7 @@ config EXTCON_USBC_CROS_EC
>  
>  config EXTCON_USBC_TUSB320
>  	tristate "TI TUSB320 USB-C extcon support"
> -	depends on I2C
> +	depends on I2C && TYPEC
>  	select REGMAP_I2C
>  	help
>  	  Say Y here to enable support for USB Type C cable detection extcon
> diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
> index aced4bbb455dc..edb8c3f997c91 100644
> --- a/drivers/extcon/extcon-usbc-tusb320.c
> +++ b/drivers/extcon/extcon-usbc-tusb320.c
> @@ -6,6 +6,7 @@
>   * Author: Michael Auchter <michael.auchter@ni.com>
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/extcon-provider.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
> @@ -13,6 +14,24 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> +#include <linux/usb/typec.h>
> +
> +#define TUSB320_REG8				0x8
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB	0x0
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A	0x1
> +#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A	0x2
> +#define TUSB320_REG8_CURRENT_MODE_DETECT	GENMASK(5, 4)
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_DEF	0x0
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
> +#define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
> +#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
> +#define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
>  
>  #define TUSB320_REG9				0x9
>  #define TUSB320_REG9_ATTACHED_STATE_SHIFT	6
> @@ -55,6 +74,10 @@ struct tusb320_priv {
>  	struct extcon_dev *edev;
>  	struct tusb320_ops *ops;
>  	enum tusb320_attached_state state;
> +	struct typec_port *port;
> +	struct typec_capability	cap;
> +	enum typec_port_type port_type;
> +	enum typec_pwr_opmode pwr_opmode;
>  };
>  
>  static const char * const tusb_attached_states[] = {
> @@ -184,6 +207,44 @@ static struct tusb320_ops tusb320l_ops = {
>  	.get_revision = tusb320l_get_revision,
>  };
>  
> +static int tusb320_set_adv_pwr_mode(struct tusb320_priv *priv)
> +{
> +	u8 mode;
> +
> +	if (priv->pwr_opmode == TYPEC_PWR_MODE_USB)
> +		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB;
> +	else if (priv->pwr_opmode == TYPEC_PWR_MODE_1_5A)
> +		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A;
> +	else if (priv->pwr_opmode == TYPEC_PWR_MODE_3_0A)
> +		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A;
> +	else	/* No other mode is supported. */
> +		return -EINVAL;
> +
> +	return regmap_write_bits(priv->regmap, TUSB320_REG8,
> +				 TUSB320_REG8_CURRENT_MODE_ADVERTISE,
> +				 FIELD_PREP(TUSB320_REG8_CURRENT_MODE_ADVERTISE,
> +					    mode));
> +}
> +
> +static int tusb320_port_type_set(struct typec_port *port,
> +				 enum typec_port_type type)
> +{
> +	struct tusb320_priv *priv = typec_get_drvdata(port);
> +
> +	if (type == TYPEC_PORT_SRC)
> +		return priv->ops->set_mode(priv, TUSB320_MODE_DFP);
> +	else if (type == TYPEC_PORT_SNK)
> +		return priv->ops->set_mode(priv, TUSB320_MODE_UFP);
> +	else if (type == TYPEC_PORT_DRP)
> +		return priv->ops->set_mode(priv, TUSB320_MODE_DRP);
> +	else
> +		return priv->ops->set_mode(priv, TUSB320_MODE_PORT);
> +}
> +
> +static const struct typec_operations tusb320_typec_ops = {
> +	.port_type_set	= tusb320_port_type_set,
> +};
> +
>  static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
>  {
>  	int state, polarity;
> @@ -211,6 +272,47 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
>  	priv->state = state;
>  }
>  
> +static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> +{
> +	struct typec_port *port = priv->port;
> +	struct device *dev = priv->dev;
> +	u8 mode, role, state;
> +	int ret, reg8;
> +	bool ori;
> +
> +	ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
> +	typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
> +					  TYPEC_ORIENTATION_NORMAL);
> +
> +	state = (reg9 >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
> +		TUSB320_REG9_ATTACHED_STATE_MASK;
> +	if (state == TUSB320_ATTACHED_STATE_DFP)
> +		role = TYPEC_SOURCE;
> +	else
> +		role = TYPEC_SINK;
> +
> +	typec_set_vconn_role(port, role);
> +	typec_set_pwr_role(port, role);
> +	typec_set_data_role(port, role == TYPEC_SOURCE ?
> +				  TYPEC_HOST : TYPEC_DEVICE);
> +
> +	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
> +	if (ret) {
> +		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
> +		return;
> +	}
> +
> +	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
> +	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
> +	else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_MED)
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_1_5A);
> +	else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_HI)
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_3_0A);
> +	else	/* Charge through accessory */
> +		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
> +}
> +
>  static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
>  {
>  	struct tusb320_priv *priv = dev_id;
> @@ -225,6 +327,7 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
>  		return IRQ_NONE;
>  
>  	tusb320_extcon_irq_handler(priv, reg);
> +	tusb320_typec_irq_handler(priv, reg);
>  
>  	regmap_write(priv->regmap, TUSB320_REG9, reg);
>  
> @@ -260,6 +363,58 @@ static int tusb320_extcon_probe(struct tusb320_priv *priv)
>  	return 0;
>  }
>  
> +static int tusb320_typec_probe(struct i2c_client *client,
> +			       struct tusb320_priv *priv)
> +{
> +	struct fwnode_handle *connector;
> +	const char *cap_str;
> +	int ret;
> +
> +	/* The Type-C connector is optional, for backward compatibility. */
> +	connector = device_get_named_child_node(&client->dev, "connector");
> +	if (!connector)
> +		return 0;
> +
> +	/* Type-C connector found. */
> +	ret = typec_get_fw_cap(&priv->cap, connector);
> +	if (ret)
> +		return ret;
> +
> +	priv->port_type = priv->cap.type;
> +
> +	/* This goes into register 0x8 field CURRENT_MODE_ADVERTISE */
> +	ret = fwnode_property_read_string(connector, "typec-power-opmode", &cap_str);
> +	if (ret)
> +		return ret;
> +
> +	ret = typec_find_pwr_opmode(cap_str);
> +	if (ret < 0)
> +		return ret;
> +	if (ret == TYPEC_PWR_MODE_PD)
> +		return -EINVAL;
> +
> +	priv->pwr_opmode = ret;
> +
> +	/* Initialize the hardware with the devicetree settings. */
> +	ret = tusb320_set_adv_pwr_mode(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->cap.revision		= USB_TYPEC_REV_1_1;
> +	priv->cap.accessory[0]		= TYPEC_ACCESSORY_AUDIO;
> +	priv->cap.accessory[1]		= TYPEC_ACCESSORY_DEBUG;
> +	priv->cap.orientation_aware	= true;
> +	priv->cap.driver_data		= priv;
> +	priv->cap.ops			= &tusb320_typec_ops;
> +	priv->cap.fwnode		= connector;
> +
> +	priv->port = typec_register_port(&client->dev, &priv->cap);
> +	if (IS_ERR(priv->port))
> +		return PTR_ERR(priv->port);
> +
> +	return 0;
> +}
> +
>  static int tusb320_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> @@ -300,6 +455,10 @@ static int tusb320_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> +	ret = tusb320_typec_probe(client, priv);
> +	if (ret)
> +		return ret;
> +
>  	/* update initial state */
>  	tusb320_irq_handler(client->irq, priv);
>  
> -- 
> 2.35.1

-- 
heikki

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[kernel test robot]

Hi Marek,

I love your patch! Perhaps something to improve:

[auto build test WARNING on chanwoo-extcon/extcon-next]
[also build test WARNING on linus/master v5.19-rc8 next-20220728]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Marek-Vasut/extcon-usbc-tusb320-Factor-out-extcon-into-dedicated-functions/20220731-020545
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
config: sparc-randconfig-r031-20220731 (https://download.01.org/0day-ci/archive/20220731/202207310423.4ybeL2WB-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.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
        # https://github.com/intel-lab-lkp/linux/commit/7bce9c0377f5e41fa29f57af40f436a48e2260b1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marek-Vasut/extcon-usbc-tusb320-Factor-out-extcon-into-dedicated-functions/20220731-020545
        git checkout 7bce9c0377f5e41fa29f57af40f436a48e2260b1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/extcon/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/extcon/extcon-usbc-tusb320.c:19: warning: expecting prototype for drivers/extcon/extcon-tusb320.c(). Prototype was for TUSB320_REG8() instead


vim +19 drivers/extcon/extcon-usbc-tusb320.c

    18	
  > 19	#define TUSB320_REG8				0x8
    20	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
    21	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB	0x0
    22	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A	0x1
    23	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A	0x2
    24	#define TUSB320_REG8_CURRENT_MODE_DETECT	GENMASK(5, 4)
    25	#define TUSB320_REG8_CURRENT_MODE_DETECT_DEF	0x0
    26	#define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
    27	#define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
    28	#define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
    29	#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
    30	#define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
    31	#define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
    32	#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
    33	#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
    34	#define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
    35	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[kernel test robot]

Hi Marek,

I love your patch! Perhaps something to improve:

[auto build test WARNING on chanwoo-extcon/extcon-next]
[also build test WARNING on linus/master v5.19-rc8 next-20220728]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Marek-Vasut/extcon-usbc-tusb320-Factor-out-extcon-into-dedicated-functions/20220731-020545
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220731/202207310431.Rtpq2bqx-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
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
        # https://github.com/intel-lab-lkp/linux/commit/7bce9c0377f5e41fa29f57af40f436a48e2260b1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marek-Vasut/extcon-usbc-tusb320-Factor-out-extcon-into-dedicated-functions/20220731-020545
        git checkout 7bce9c0377f5e41fa29f57af40f436a48e2260b1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/extcon/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/extcon/extcon-usbc-tusb320.c:19: warning: expecting prototype for drivers/extcon/extcon-tusb320.c(). Prototype was for TUSB320_REG8() instead


vim +19 drivers/extcon/extcon-usbc-tusb320.c

    18	
  > 19	#define TUSB320_REG8				0x8
    20	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
    21	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB	0x0
    22	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A	0x1
    23	#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A	0x2
    24	#define TUSB320_REG8_CURRENT_MODE_DETECT	GENMASK(5, 4)
    25	#define TUSB320_REG8_CURRENT_MODE_DETECT_DEF	0x0
    26	#define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
    27	#define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
    28	#define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
    29	#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
    30	#define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
    31	#define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
    32	#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
    33	#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
    34	#define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
    35	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[PATCH 2/2] extcon: usbc-tusb320: Add USB TYPE-C support

[Marek Vasut]

The TI TUSB320 seems like a better fit for USB TYPE-C subsystem,
which can expose details collected by the TUSB320 in a far more
precise way than extcon. Since there are existing users in the
kernel and in DT which depend on the extcon interface, keep it
for now.

Add TYPE-C interface and expose the supported supply current,
direction and connector polarity via the TYPE-C interface.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Yassine Oudjana <y.oudjana@protonmail.com>
To: linux-usb@vger.kernel.org
---
 drivers/extcon/Kconfig               |   2 +-
 drivers/extcon/extcon-usbc-tusb320.c | 159 +++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index c69d40ae5619a..7684b3afa6304 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -180,7 +180,7 @@ config EXTCON_USBC_CROS_EC
 
 config EXTCON_USBC_TUSB320
 	tristate "TI TUSB320 USB-C extcon support"
-	depends on I2C
+	depends on I2C && TYPEC
 	select REGMAP_I2C
 	help
 	  Say Y here to enable support for USB Type C cable detection extcon
diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index aced4bbb455dc..edb8c3f997c91 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -6,6 +6,7 @@
  * Author: Michael Auchter <michael.auchter@ni.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/extcon-provider.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -13,6 +14,24 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/usb/typec.h>
+
+#define TUSB320_REG8				0x8
+#define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
+#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB	0x0
+#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A	0x1
+#define TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A	0x2
+#define TUSB320_REG8_CURRENT_MODE_DETECT	GENMASK(5, 4)
+#define TUSB320_REG8_CURRENT_MODE_DETECT_DEF	0x0
+#define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
+#define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
+#define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
+#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
+#define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
+#define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
+#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
+#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
+#define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
 
 #define TUSB320_REG9				0x9
 #define TUSB320_REG9_ATTACHED_STATE_SHIFT	6
@@ -55,6 +74,10 @@ struct tusb320_priv {
 	struct extcon_dev *edev;
 	struct tusb320_ops *ops;
 	enum tusb320_attached_state state;
+	struct typec_port *port;
+	struct typec_capability	cap;
+	enum typec_port_type port_type;
+	enum typec_pwr_opmode pwr_opmode;
 };
 
 static const char * const tusb_attached_states[] = {
@@ -184,6 +207,44 @@ static struct tusb320_ops tusb320l_ops = {
 	.get_revision = tusb320l_get_revision,
 };
 
+static int tusb320_set_adv_pwr_mode(struct tusb320_priv *priv)
+{
+	u8 mode;
+
+	if (priv->pwr_opmode == TYPEC_PWR_MODE_USB)
+		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_USB;
+	else if (priv->pwr_opmode == TYPEC_PWR_MODE_1_5A)
+		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_15A;
+	else if (priv->pwr_opmode == TYPEC_PWR_MODE_3_0A)
+		mode = TUSB320_REG8_CURRENT_MODE_ADVERTISE_30A;
+	else	/* No other mode is supported. */
+		return -EINVAL;
+
+	return regmap_write_bits(priv->regmap, TUSB320_REG8,
+				 TUSB320_REG8_CURRENT_MODE_ADVERTISE,
+				 FIELD_PREP(TUSB320_REG8_CURRENT_MODE_ADVERTISE,
+					    mode));
+}
+
+static int tusb320_port_type_set(struct typec_port *port,
+				 enum typec_port_type type)
+{
+	struct tusb320_priv *priv = typec_get_drvdata(port);
+
+	if (type == TYPEC_PORT_SRC)
+		return priv->ops->set_mode(priv, TUSB320_MODE_DFP);
+	else if (type == TYPEC_PORT_SNK)
+		return priv->ops->set_mode(priv, TUSB320_MODE_UFP);
+	else if (type == TYPEC_PORT_DRP)
+		return priv->ops->set_mode(priv, TUSB320_MODE_DRP);
+	else
+		return priv->ops->set_mode(priv, TUSB320_MODE_PORT);
+}
+
+static const struct typec_operations tusb320_typec_ops = {
+	.port_type_set	= tusb320_port_type_set,
+};
+
 static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
 {
 	int state, polarity;
@@ -211,6 +272,47 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
 	priv->state = state;
 }
 
+static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
+{
+	struct typec_port *port = priv->port;
+	struct device *dev = priv->dev;
+	u8 mode, role, state;
+	int ret, reg8;
+	bool ori;
+
+	ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
+	typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
+					  TYPEC_ORIENTATION_NORMAL);
+
+	state = (reg9 >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
+		TUSB320_REG9_ATTACHED_STATE_MASK;
+	if (state == TUSB320_ATTACHED_STATE_DFP)
+		role = TYPEC_SOURCE;
+	else
+		role = TYPEC_SINK;
+
+	typec_set_vconn_role(port, role);
+	typec_set_pwr_role(port, role);
+	typec_set_data_role(port, role == TYPEC_SOURCE ?
+				  TYPEC_HOST : TYPEC_DEVICE);
+
+	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
+	if (ret) {
+		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
+		return;
+	}
+
+	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
+	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
+		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
+	else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_MED)
+		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_1_5A);
+	else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_HI)
+		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_3_0A);
+	else	/* Charge through accessory */
+		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
+}
+
 static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
 {
 	struct tusb320_priv *priv = dev_id;
@@ -225,6 +327,7 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	tusb320_extcon_irq_handler(priv, reg);
+	tusb320_typec_irq_handler(priv, reg);
 
 	regmap_write(priv->regmap, TUSB320_REG9, reg);
 
@@ -260,6 +363,58 @@ static int tusb320_extcon_probe(struct tusb320_priv *priv)
 	return 0;
 }
 
+static int tusb320_typec_probe(struct i2c_client *client,
+			       struct tusb320_priv *priv)
+{
+	struct fwnode_handle *connector;
+	const char *cap_str;
+	int ret;
+
+	/* The Type-C connector is optional, for backward compatibility. */
+	connector = device_get_named_child_node(&client->dev, "connector");
+	if (!connector)
+		return 0;
+
+	/* Type-C connector found. */
+	ret = typec_get_fw_cap(&priv->cap, connector);
+	if (ret)
+		return ret;
+
+	priv->port_type = priv->cap.type;
+
+	/* This goes into register 0x8 field CURRENT_MODE_ADVERTISE */
+	ret = fwnode_property_read_string(connector, "typec-power-opmode", &cap_str);
+	if (ret)
+		return ret;
+
+	ret = typec_find_pwr_opmode(cap_str);
+	if (ret < 0)
+		return ret;
+	if (ret == TYPEC_PWR_MODE_PD)
+		return -EINVAL;
+
+	priv->pwr_opmode = ret;
+
+	/* Initialize the hardware with the devicetree settings. */
+	ret = tusb320_set_adv_pwr_mode(priv);
+	if (ret)
+		return ret;
+
+	priv->cap.revision		= USB_TYPEC_REV_1_1;
+	priv->cap.accessory[0]		= TYPEC_ACCESSORY_AUDIO;
+	priv->cap.accessory[1]		= TYPEC_ACCESSORY_DEBUG;
+	priv->cap.orientation_aware	= true;
+	priv->cap.driver_data		= priv;
+	priv->cap.ops			= &tusb320_typec_ops;
+	priv->cap.fwnode		= connector;
+
+	priv->port = typec_register_port(&client->dev, &priv->cap);
+	if (IS_ERR(priv->port))
+		return PTR_ERR(priv->port);
+
+	return 0;
+}
+
 static int tusb320_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -300,6 +455,10 @@ static int tusb320_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
+	ret = tusb320_typec_probe(client, priv);
+	if (ret)
+		return ret;
+
 	/* update initial state */
 	tusb320_irq_handler(client->irq, priv);
 
-- 
2.35.1