[PATCH v3 0/3] extcon: usbc-tusb320: Initial TUSB320L support

[PATCH v3 0/3] extcon: usbc-tusb320: Initial TUSB320L support

[Yassine Oudjana]

The TUSB320L is a newer chip with additional features. It is close enough to TUSB320 so it works
to some extent out-of-the-box, but for some reason it can start in UFP mode which is not desirable.

This series adds support for reset and mode setting for both chips, and does that while probing
to ensure the chip starts with the default mode of following the PORT pin.

Changes since v2:
 - Read state before setting default mode, then update it again after resetting.
 - Remove mode tracing from irq handler
 - Add a delay after reset to handle tSOFT_RESET
 - Use a separate mode setting function for each of TUSB320 and TUSB320L.
Changes since v1:
 - Split first patch into two patches, one adding support for mode setting and reset on TUSB320,
   and the other adding support for TUSB320L.
 - Fix dt_binding_check warning:
   ../Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml:15:6: [warning] wrong indentation: expected 6 but found 5 (indentation)

Yassine Oudjana (3):
  extcon: usbc-tusb320: Add support for mode setting and reset
  extcon: usbc-tusb320: Add support for TUSB320L
  dt-bindings: extcon: usbc-tusb320: Add TUSB320L compatible string

 .../bindings/extcon/extcon-usbc-tusb320.yaml  |   4 +-
 drivers/extcon/extcon-usbc-tusb320.c          | 164 +++++++++++++++++-
 2 files changed, 162 insertions(+), 6 deletions(-)

-- 
2.32.0

[PATCH v3 1/3] extcon: usbc-tusb320: Add support for mode setting and reset

[Yassine Oudjana]

Reset the chip and set its mode to default (maintain mode set by PORT pin)
during probe to make sure it comes up in the default state.

Changes since v2:
 - Read state before setting default mode, then update it again after resetting.
 - Remove mode tracing from irq handler
 - Add a delay after reset to handle tSOFT_RESET

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/extcon/extcon-usbc-tusb320.c | 92 ++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 805af73b4152..867fb2bf2bdc 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -19,15 +19,32 @@
 #define TUSB320_REG9_ATTACHED_STATE_MASK	0x3
 #define TUSB320_REG9_CABLE_DIRECTION		BIT(5)
 #define TUSB320_REG9_INTERRUPT_STATUS		BIT(4)
-#define TUSB320_ATTACHED_STATE_NONE		0x0
-#define TUSB320_ATTACHED_STATE_DFP		0x1
-#define TUSB320_ATTACHED_STATE_UFP		0x2
-#define TUSB320_ATTACHED_STATE_ACC		0x3
+
+#define TUSB320_REGA				0xa
+#define TUSB320_REGA_I2C_SOFT_RESET		BIT(3)
+#define TUSB320_REGA_MODE_SELECT_SHIFT		4
+#define TUSB320_REGA_MODE_SELECT_MASK		0x3
+
+enum tusb320_attached_state {
+	TUSB320_ATTACHED_STATE_NONE,
+	TUSB320_ATTACHED_STATE_DFP,
+	TUSB320_ATTACHED_STATE_UFP,
+	TUSB320_ATTACHED_STATE_ACC,
+};
+
+enum tusb320_mode {
+	TUSB320_MODE_PORT,
+	TUSB320_MODE_UFP,
+	TUSB320_MODE_DFP,
+	TUSB320_MODE_DRP,
+};
 
 struct tusb320_priv {
 	struct device *dev;
 	struct regmap *regmap;
 	struct extcon_dev *edev;
+
+	enum tusb320_attached_state state;
 };
 
 static const char * const tusb_attached_states[] = {
@@ -37,6 +54,13 @@ static const char * const tusb_attached_states[] = {
 	[TUSB320_ATTACHED_STATE_ACC]  = "accessory",
 };
 
+static const char * const tusb_modes[] = {
+	[TUSB320_MODE_PORT] = "maintain mode set by PORT pin",
+	[TUSB320_MODE_UFP]  = "upstream facing port",
+	[TUSB320_MODE_DFP]  = "downstream facing port",
+	[TUSB320_MODE_DRP]  = "dual role port",
+};
+
 static const unsigned int tusb320_extcon_cable[] = {
 	EXTCON_USB,
 	EXTCON_USB_HOST,
@@ -62,6 +86,53 @@ static int tusb320_check_signature(struct tusb320_priv *priv)
 	return 0;
 }
 
+static int tusb320_set_mode(struct tusb320_priv *priv, enum tusb320_mode mode)
+{
+	int ret;
+
+	/* Mode cannot be changed while cable is attached */
+	if (priv->state != TUSB320_ATTACHED_STATE_NONE)
+		return -EBUSY;
+
+	/* Write mode */
+	ret = regmap_write_bits(priv->regmap, TUSB320_REGA,
+		TUSB320_REGA_MODE_SELECT_MASK << TUSB320_REGA_MODE_SELECT_SHIFT,
+		mode << TUSB320_REGA_MODE_SELECT_SHIFT);
+	if (ret) {
+		dev_err(priv->dev, "failed to write mode: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tusb320_reset(struct tusb320_priv *priv)
+{
+	int ret;
+
+	/* Set mode to default (follow PORT pin) */
+	ret = tusb320_set_mode(priv, TUSB320_MODE_PORT);
+	if (ret && ret != -EBUSY) {
+		dev_err(priv->dev,
+			"failed to set mode to PORT: %d\n", ret);
+		return ret;
+	}
+
+	/* Perform soft reset */
+	ret = regmap_write_bits(priv->regmap, TUSB320_REGA,
+			TUSB320_REGA_I2C_SOFT_RESET, 1);
+	if (ret) {
+		dev_err(priv->dev,
+			"failed to write soft reset bit: %d\n", ret);
+		return ret;
+	}
+
+	/* Wait for chip to go through reset */
+	msleep(95);
+
+	return 0;
+}
+
 static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
 {
 	struct tusb320_priv *priv = dev_id;
@@ -96,6 +167,8 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
 	extcon_sync(priv->edev, EXTCON_USB);
 	extcon_sync(priv->edev, EXTCON_USB_HOST);
 
+	priv->state = state;
+
 	regmap_write(priv->regmap, TUSB320_REG9, reg);
 
 	return IRQ_HANDLED;
@@ -145,6 +218,17 @@ static int tusb320_extcon_probe(struct i2c_client *client,
 	/* update initial state */
 	tusb320_irq_handler(client->irq, priv);
 
+	/* Reset chip to its default state */
+	ret = tusb320_reset(priv);
+	if (ret)
+		dev_warn(priv->dev, "failed to reset chip: %d\n", ret);
+	else
+		/*
+		 * State and polarity might change after a reset, so update
+		 * them again and make sure the interrupt status bit is cleared.
+		 */
+		tusb320_irq_handler(client->irq, priv);
+
 	ret = devm_request_threaded_irq(priv->dev, client->irq, NULL,
 					tusb320_irq_handler,
 					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-- 
2.32.0

[PATCH v3 2/3] extcon: usbc-tusb320: Add support for TUSB320L

[Yassine Oudjana]

TUSB320L is a newer chip with additional features, and it has additional steps
in its mode changing sequence:
 - Disable CC state machine,
 - Write to mode register,
 - Wait for 5 ms,
 - Re-enable CC state machine.
It also has an additional register that a revision number can be read from.

Add support for the mode changing sequence, and read the revision number during
probe and print it as info.

Changes since v2:
 - Use a separate mode setting function for each of TUSB320 and TUSB320L.  

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/extcon/extcon-usbc-tusb320.c | 74 +++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 867fb2bf2bdc..5a80ca7a15eb 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -21,10 +21,13 @@
 #define TUSB320_REG9_INTERRUPT_STATUS		BIT(4)
 
 #define TUSB320_REGA				0xa
+#define TUSB320L_REGA_DISABLE_TERM		BIT(0)
 #define TUSB320_REGA_I2C_SOFT_RESET		BIT(3)
 #define TUSB320_REGA_MODE_SELECT_SHIFT		4
 #define TUSB320_REGA_MODE_SELECT_MASK		0x3
 
+#define TUSB320L_REGA0_REVISION			0xa0
+
 enum tusb320_attached_state {
 	TUSB320_ATTACHED_STATE_NONE,
 	TUSB320_ATTACHED_STATE_DFP,
@@ -39,9 +42,16 @@ enum tusb320_mode {
 	TUSB320_MODE_DRP,
 };
 
+enum tusb320_type {
+	TYPE_TUSB320,
+	TYPE_TUSB320L,
+};
+
+
 struct tusb320_priv {
 	struct device *dev;
 	struct regmap *regmap;
+	enum tusb320_type type;
 	struct extcon_dev *edev;
 
 	enum tusb320_attached_state state;
@@ -106,12 +116,53 @@ static int tusb320_set_mode(struct tusb320_priv *priv, enum tusb320_mode mode)
 	return 0;
 }
 
+static int tusb320l_set_mode(struct tusb320_priv *priv, enum tusb320_mode mode)
+{
+	int ret;
+
+	/* Disable CC state machine */
+	ret = regmap_write_bits(priv->regmap, TUSB320_REGA,
+		TUSB320L_REGA_DISABLE_TERM, 1);
+	if (ret) {
+		dev_err(priv->dev,
+			"failed to disable CC state machine: %d\n", ret);
+		return ret;
+	}
+
+	/* Write mode */
+	ret = regmap_write_bits(priv->regmap, TUSB320_REGA,
+		TUSB320_REGA_MODE_SELECT_MASK << TUSB320_REGA_MODE_SELECT_SHIFT,
+		mode << TUSB320_REGA_MODE_SELECT_SHIFT);
+	if (ret) {
+		dev_err(priv->dev, "failed to write mode: %d\n", ret);
+		goto err;
+	}
+
+	msleep(5);
+err:
+	/* Re-enable CC state machine */
+	ret = regmap_write_bits(priv->regmap, TUSB320_REGA,
+		TUSB320L_REGA_DISABLE_TERM, 0);
+	if (ret)
+		dev_err(priv->dev,
+			"failed to re-enable CC state machine: %d\n", ret);
+
+	return ret;
+}
+
 static int tusb320_reset(struct tusb320_priv *priv)
 {
 	int ret;
 
 	/* Set mode to default (follow PORT pin) */
-	ret = tusb320_set_mode(priv, TUSB320_MODE_PORT);
+	switch (priv->type) {
+	case TYPE_TUSB320:
+		ret = tusb320_set_mode(priv, TUSB320_MODE_PORT);
+		break;
+	case TYPE_TUSB320L:
+		ret = tusb320l_set_mode(priv, TUSB320_MODE_PORT);
+		break;
+	}
 	if (ret && ret != -EBUSY) {
 		dev_err(priv->dev,
 			"failed to set mode to PORT: %d\n", ret);
@@ -183,6 +234,8 @@ static int tusb320_extcon_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
 	struct tusb320_priv *priv;
+	const void *match_data;
+	unsigned int revision;
 	int ret;
 
 	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
@@ -198,12 +251,28 @@ static int tusb320_extcon_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
+	match_data = device_get_match_data(&client->dev);
+	if (!match_data)
+		return -EINVAL;
+
+	priv->type = (enum tusb320_type)match_data;
+
 	priv->edev = devm_extcon_dev_allocate(priv->dev, tusb320_extcon_cable);
 	if (IS_ERR(priv->edev)) {
 		dev_err(priv->dev, "failed to allocate extcon device\n");
 		return PTR_ERR(priv->edev);
 	}
 
+	if (priv->type == TYPE_TUSB320L) {
+		ret = regmap_read(priv->regmap, TUSB320L_REGA0_REVISION, &revision);
+
+		if (ret)
+			dev_warn(priv->dev,
+				"failed to read revision register: %d\n", ret);
+		else
+			dev_info(priv->dev, "chip revision %d\n", revision);
+	}
+
 	ret = devm_extcon_dev_register(priv->dev, priv->edev);
 	if (ret < 0) {
 		dev_err(priv->dev, "failed to register extcon device\n");
@@ -238,7 +307,8 @@ static int tusb320_extcon_probe(struct i2c_client *client,
 }
 
 static const struct of_device_id tusb320_extcon_dt_match[] = {
-	{ .compatible = "ti,tusb320", },
+	{ .compatible = "ti,tusb320", .data = (void *)TYPE_TUSB320, },
+	{ .compatible = "ti,tusb320l", .data = (void *)TYPE_TUSB320L, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tusb320_extcon_dt_match);
-- 
2.32.0

[PATCH v3 3/3] dt-bindings: extcon: usbc-tusb320: Add TUSB320L compatible string

[Yassine Oudjana]

Add a compatible string for TUSB320L.

Changes since v1:
 - Fix dt_binding_check warning:
   ../Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml:15:6: [warning] wrong indentation: expected 6 but found 5 (indentation)

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 .../devicetree/bindings/extcon/extcon-usbc-tusb320.yaml       | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml b/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml
index 9875b4d5c356..71a9f2e5d0dc 100644
--- a/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml
+++ b/Documentation/devicetree/bindings/extcon/extcon-usbc-tusb320.yaml
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: ti,tusb320
+    enum:
+      - ti,tusb320
+      - ti,tusb320l
 
   reg:
     maxItems: 1
-- 
2.32.0

Re: [PATCH v3 1/3] extcon: usbc-tusb320: Add support for mode setting and reset

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8042 bytes --]

Hi Yassine,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on chanwoo-extcon/extcon-next]
[also build test WARNING on robh/for-next v5.14-rc3 next-20210730]
[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/Yassine-Oudjana/extcon-usbc-tusb320-Initial-TUSB320L-support/20210801-180933
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
config: s390-randconfig-r014-20210801 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
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
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/35cd3dbcb16777ecb2e3d96327b1e2d89eafc874
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yassine-Oudjana/extcon-usbc-tusb320-Initial-TUSB320L-support/20210801-180933
        git checkout 35cd3dbcb16777ecb2e3d96327b1e2d89eafc874
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

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

All warnings (new ones prefixed by >>):

   In file included from drivers/extcon/extcon-usbc-tusb320.c:10:
   In file included from include/linux/i2c.h:18:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:25:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from drivers/extcon/extcon-usbc-tusb320.c:10:
   In file included from include/linux/i2c.h:18:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:25:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from drivers/extcon/extcon-usbc-tusb320.c:10:
   In file included from include/linux/i2c.h:18:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:25:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/extcon/extcon-usbc-tusb320.c:57:27: warning: unused variable 'tusb_modes' [-Wunused-const-variable]
   static const char * const tusb_modes[] = {
                             ^
   13 warnings generated.


vim +/tusb_modes +57 drivers/extcon/extcon-usbc-tusb320.c

    56	
  > 57	static const char * const tusb_modes[] = {
    58		[TUSB320_MODE_PORT] = "maintain mode set by PORT pin",
    59		[TUSB320_MODE_UFP]  = "upstream facing port",
    60		[TUSB320_MODE_DFP]  = "downstream facing port",
    61		[TUSB320_MODE_DRP]  = "dual role port",
    62	};
    63	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41175 bytes --]