[PATCH 0/2] Add a interface for passing supported PD rev from TCPC

[PATCH 0/2] Add a interface for passing supported PD rev from TCPC

[Potin Lai]

This series add an interface for TCPC passing supported PD revision.
supported_pd_rev is optional, TCPM will get PD rev for negotiation if 
lower level driver has implementaion, otherwise use PD_MAX_REV for 
negotiation

Potin Lai (2):
  usb: typec: tcpm: add interface for passing supported_pd_rev from
    tcpc_dev
  usb: typec: fusb302: add support of supported_pd_rev

 drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
 drivers/usb/typec/tcpm/tcpm.c    | 14 ++++++++++++--
 include/linux/usb/tcpm.h         |  4 ++++
 3 files changed, 36 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev

[Potin Lai]

Current TCPM allways assume using PD_MAX_REV for negotiation,
but for some USB controller only support PD 2.0, adding an interface
for passing supported_pd_rev from tcpc_dev.

Signed-off-by: Potin Lai <potin.lai@quantatw.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++--
 include/linux/usb/tcpm.h      |  4 ++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 59d4fa2443f2..31770fa8643d 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port)
 				    port->cc2 == TYPEC_CC_OPEN)));
 }
 
+static u32 tcpm_pd_supported_rev(struct tcpm_port *port)
+{
+	u32 rev = PD_MAX_REV;
+
+	if (port->tcpc->supported_pd_rev)
+		rev = port->tcpc->supported_pd_rev(port->tcpc);
+
+	return (rev > PD_MAX_REV) ? PD_MAX_REV : rev;
+}
+
 /*
  * Logging
  */
@@ -3932,7 +3942,7 @@ static void run_state_machine(struct tcpm_port *port)
 		typec_set_pwr_opmode(port->typec_port, opmode);
 		port->pwr_opmode = TYPEC_PWR_MODE_USB;
 		port->caps_count = 0;
-		port->negotiated_rev = PD_MAX_REV;
+		port->negotiated_rev = tcpm_pd_supported_rev(port);
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		port->explicit_contract = false;
@@ -4167,7 +4177,7 @@ static void run_state_machine(struct tcpm_port *port)
 					      port->cc2 : port->cc1);
 		typec_set_pwr_opmode(port->typec_port, opmode);
 		port->pwr_opmode = TYPEC_PWR_MODE_USB;
-		port->negotiated_rev = PD_MAX_REV;
+		port->negotiated_rev = tcpm_pd_supported_rev(port);
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		port->explicit_contract = false;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index bffc8d3e14ad..36282b2a9d9c 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -114,6 +114,9 @@ enum tcpm_transmit_type {
  *              Optional; The USB Communications Capable bit indicates if port
  *              partner is capable of communication over the USB data lines
  *              (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit.
+ * @supported_pd_rev:
+ *              Optional; TCPM call this function to get supported PD revesion
+ *              from lower level driver.
  */
 struct tcpc_dev {
 	struct fwnode_handle *fwnode;
@@ -148,6 +151,7 @@ struct tcpc_dev {
 						 bool pps_active, u32 requested_vbus_voltage);
 	bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
 	void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable);
+	u32 (*supported_pd_rev)(struct tcpc_dev *dev);
 };
 
 struct tcpm_port;
-- 
2.17.1

[PATCH 2/2] usb: typec: fusb302: add support of supported_pd_rev

[Potin Lai]

Add support for passing supported PD rev to TCPM.
If "supported-pd-rev" property exist, then return supported_pd_rev as
defined value in DTS, otherwise return PD_MAX_REV

Example of DTS:

fusb302: typec-portc@22 {
    compatible = "fcs,fusb302";
    reg = <0x22>;
    ...
    supported-pd-rev=<1>; // PD_REV20
    ...
};

Signed-off-by: Potin Lai <potin.lai@quantatw.com>
---
 drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 72f9001b0792..8cff92d58b96 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -109,6 +109,9 @@ struct fusb302_chip {
 	enum typec_cc_status cc2;
 	u32 snk_pdo[PDO_MAX_OBJECTS];
 
+	/* supported pd rev */
+	u32 supported_pd_rev;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	/* lock for log buffer access */
@@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type,
 	return ret;
 }
 
+static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
+{
+	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
+						 tcpc_dev);
+	return chip->supported_pd_rev;
+}
+
 static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
 {
 	if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
@@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
 	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
 	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
+	fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
 }
 
 static const char * const cc_polarity_name[] = {
@@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client,
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter = client->adapter;
 	struct device *dev = &client->dev;
+	struct device_node *np = dev->of_node;
 	const char *name;
 	int ret = 0;
 
@@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client,
 		dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
 		goto tcpm_unregister_port;
 	}
+
+	if (of_property_read_u32(np, "supported-pd-rev",
+				&chip->supported_pd_rev) < 0) {
+		chip->supported_pd_rev = PD_MAX_REV;
+	} else if (chip->supported_pd_rev > PD_MAX_REV) {
+		chip->supported_pd_rev = PD_MAX_REV;
+	}
+
 	enable_irq_wake(chip->gpio_int_n_irq);
 	i2c_set_clientdata(client, chip);
 
-- 
2.17.1

Re: [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev

[Greg Kroah-Hartman]

On Tue, Feb 08, 2022 at 04:20:25PM +0800, Potin Lai wrote:
> Current TCPM allways assume using PD_MAX_REV for negotiation,
> but for some USB controller only support PD 2.0, adding an interface
> for passing supported_pd_rev from tcpc_dev.
> 
> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++--
>  include/linux/usb/tcpm.h      |  4 ++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 59d4fa2443f2..31770fa8643d 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port)
>  				    port->cc2 == TYPEC_CC_OPEN)));
>  }
>  
> +static u32 tcpm_pd_supported_rev(struct tcpm_port *port)
> +{
> +	u32 rev = PD_MAX_REV;
> +
> +	if (port->tcpc->supported_pd_rev)
> +		rev = port->tcpc->supported_pd_rev(port->tcpc);
> +
> +	return (rev > PD_MAX_REV) ? PD_MAX_REV : rev;

Please spell this out in a real if statement to make it obvious:
	if (rev > PD_MAX_REV)
		return PD_MAX_REV
	return rev

Or better yet:
	return min(PD_MAX_REV, rev);

right?

thanks,

greg k-h

[PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC

[Potin Lai]

This series add an interface for TCPC passing supported PD revision.
supported_pd_rev is optional, TCPM will get PD rev for negotiation if
lower level driver has implementaion, otherwise use PD_MAX_REV for
negotiation

changes v1 -> v2:
  - tcpm.c
    * use min() to simplify the statement

Potin Lai (2):
  usb: typec: tcpm: add interface for passing supported_pd_rev from
    tcpc_dev
  usb: typec: fusb302: add support of supported_pd_rev

 drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
 drivers/usb/typec/tcpm/tcpm.c    | 14 ++++++++++++--
 include/linux/usb/tcpm.h         |  4 ++++
 3 files changed, 36 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev

[Potin Lai]

Current TCPM allways assume using PD_MAX_REV for negotiation,
but for some USB controller only support PD 2.0, adding an interface
for passing supported_pd_rev from tcpc_dev.

Signed-off-by: Potin Lai <potin.lai@quantatw.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++--
 include/linux/usb/tcpm.h      |  4 ++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 59d4fa2443f2..22e7d226826e 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port)
 				    port->cc2 == TYPEC_CC_OPEN)));
 }
 
+static u32 tcpm_pd_supported_rev(struct tcpm_port *port)
+{
+	u32 rev = PD_MAX_REV;
+
+	if (port->tcpc->supported_pd_rev)
+		rev = port->tcpc->supported_pd_rev(port->tcpc);
+
+	return min(rev, PD_MAX_REV);
+}
+
 /*
  * Logging
  */
@@ -3932,7 +3942,7 @@ static void run_state_machine(struct tcpm_port *port)
 		typec_set_pwr_opmode(port->typec_port, opmode);
 		port->pwr_opmode = TYPEC_PWR_MODE_USB;
 		port->caps_count = 0;
-		port->negotiated_rev = PD_MAX_REV;
+		port->negotiated_rev = tcpm_pd_supported_rev(port);
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		port->explicit_contract = false;
@@ -4167,7 +4177,7 @@ static void run_state_machine(struct tcpm_port *port)
 					      port->cc2 : port->cc1);
 		typec_set_pwr_opmode(port->typec_port, opmode);
 		port->pwr_opmode = TYPEC_PWR_MODE_USB;
-		port->negotiated_rev = PD_MAX_REV;
+		port->negotiated_rev = tcpm_pd_supported_rev(port);
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		port->explicit_contract = false;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index bffc8d3e14ad..36282b2a9d9c 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -114,6 +114,9 @@ enum tcpm_transmit_type {
  *              Optional; The USB Communications Capable bit indicates if port
  *              partner is capable of communication over the USB data lines
  *              (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit.
+ * @supported_pd_rev:
+ *              Optional; TCPM call this function to get supported PD revesion
+ *              from lower level driver.
  */
 struct tcpc_dev {
 	struct fwnode_handle *fwnode;
@@ -148,6 +151,7 @@ struct tcpc_dev {
 						 bool pps_active, u32 requested_vbus_voltage);
 	bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
 	void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable);
+	u32 (*supported_pd_rev)(struct tcpc_dev *dev);
 };
 
 struct tcpm_port;
-- 
2.17.1

[PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev

[Potin Lai]

Add support for passing supported PD rev to TCPM.
If "supported-pd-rev" property exist, then return supported_pd_rev as
defined value in DTS, otherwise return PD_MAX_REV

Example of DTS:

fusb302: typec-portc@22 {
    compatible = "fcs,fusb302";
    reg = <0x22>;
    ...
    supported-pd-rev=<1>; // PD_REV20
    ...
};

Signed-off-by: Potin Lai <potin.lai@quantatw.com>
---
 drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 72f9001b0792..8cff92d58b96 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -109,6 +109,9 @@ struct fusb302_chip {
 	enum typec_cc_status cc2;
 	u32 snk_pdo[PDO_MAX_OBJECTS];
 
+	/* supported pd rev */
+	u32 supported_pd_rev;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	/* lock for log buffer access */
@@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type,
 	return ret;
 }
 
+static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
+{
+	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
+						 tcpc_dev);
+	return chip->supported_pd_rev;
+}
+
 static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
 {
 	if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
@@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
 	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
 	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
+	fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
 }
 
 static const char * const cc_polarity_name[] = {
@@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client,
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter = client->adapter;
 	struct device *dev = &client->dev;
+	struct device_node *np = dev->of_node;
 	const char *name;
 	int ret = 0;
 
@@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client,
 		dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
 		goto tcpm_unregister_port;
 	}
+
+	if (of_property_read_u32(np, "supported-pd-rev",
+				&chip->supported_pd_rev) < 0) {
+		chip->supported_pd_rev = PD_MAX_REV;
+	} else if (chip->supported_pd_rev > PD_MAX_REV) {
+		chip->supported_pd_rev = PD_MAX_REV;
+	}
+
 	enable_irq_wake(chip->gpio_int_n_irq);
 	i2c_set_clientdata(client, chip);
 
-- 
2.17.1

Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev

[Guenter Roeck]

On 2/8/22 03:22, Potin Lai wrote:
> Add support for passing supported PD rev to TCPM.
> If "supported-pd-rev" property exist, then return supported_pd_rev as
> defined value in DTS, otherwise return PD_MAX_REV
> 
> Example of DTS:
> 
> fusb302: typec-portc@22 {
>      compatible = "fcs,fusb302";
>      reg = <0x22>;
>      ...
>      supported-pd-rev=<1>; // PD_REV20
>      ...
> };
> 

The new property needs to be documented. However, I am not entirely sure
I understand why it is needed. Wouldn't the supported PD revision
be a chip (fusb302) specific constant ? If so, why does it need a
devicetree property ? I think that needs some additional explanation.

Thanks,
Guenter

> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 72f9001b0792..8cff92d58b96 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -109,6 +109,9 @@ struct fusb302_chip {
>   	enum typec_cc_status cc2;
>   	u32 snk_pdo[PDO_MAX_OBJECTS];
>   
> +	/* supported pd rev */
> +	u32 supported_pd_rev;
> +
>   #ifdef CONFIG_DEBUG_FS
>   	struct dentry *dentry;
>   	/* lock for log buffer access */
> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>   	return ret;
>   }
>   
> +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
> +{
> +	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
> +						 tcpc_dev);
> +	return chip->supported_pd_rev;
> +}
> +
>   static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
>   {
>   	if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>   	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
>   	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>   	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
> +	fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
>   }
>   
>   static const char * const cc_polarity_name[] = {
> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client,
>   	struct fusb302_chip *chip;
>   	struct i2c_adapter *adapter = client->adapter;
>   	struct device *dev = &client->dev;
> +	struct device_node *np = dev->of_node;
>   	const char *name;
>   	int ret = 0;
>   
> @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client,
>   		dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
>   		goto tcpm_unregister_port;
>   	}
> +
> +	if (of_property_read_u32(np, "supported-pd-rev",
> +				&chip->supported_pd_rev) < 0) {
> +		chip->supported_pd_rev = PD_MAX_REV;
> +	} else if (chip->supported_pd_rev > PD_MAX_REV) {
> +		chip->supported_pd_rev = PD_MAX_REV;
> +	}

The else part is also checked in the tcpm code and thus seems
to be redundant.

> +
>   	enable_irq_wake(chip->gpio_int_n_irq);
>   	i2c_set_clientdata(client, chip);
>   

Re: [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev

[Guenter Roeck]

On 2/8/22 03:22, Potin Lai wrote:
> Current TCPM allways assume using PD_MAX_REV for negotiation,
> but for some USB controller only support PD 2.0, adding an interface
> for passing supported_pd_rev from tcpc_dev.
> 

The PD revision supported by the usb controller is a constant.
I don't see why this would need a callback function. Other
capabilitied are passed to tcpm using the fwnode pointer.
I don't see why this capability would have to be handled
differently.

Guenter

> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> ---
>   drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++--
>   include/linux/usb/tcpm.h      |  4 ++++
>   2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 59d4fa2443f2..22e7d226826e 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port)
>   				    port->cc2 == TYPEC_CC_OPEN)));
>   }
>   
> +static u32 tcpm_pd_supported_rev(struct tcpm_port *port)
> +{
> +	u32 rev = PD_MAX_REV;
> +
> +	if (port->tcpc->supported_pd_rev)
> +		rev = port->tcpc->supported_pd_rev(port->tcpc);
> +
> +	return min(rev, PD_MAX_REV);
> +}
> +
>   /*
>    * Logging
>    */
> @@ -3932,7 +3942,7 @@ static void run_state_machine(struct tcpm_port *port)
>   		typec_set_pwr_opmode(port->typec_port, opmode);
>   		port->pwr_opmode = TYPEC_PWR_MODE_USB;
>   		port->caps_count = 0;
> -		port->negotiated_rev = PD_MAX_REV;
> +		port->negotiated_rev = tcpm_pd_supported_rev(port);
>   		port->message_id = 0;
>   		port->rx_msgid = -1;
>   		port->explicit_contract = false;
> @@ -4167,7 +4177,7 @@ static void run_state_machine(struct tcpm_port *port)
>   					      port->cc2 : port->cc1);
>   		typec_set_pwr_opmode(port->typec_port, opmode);
>   		port->pwr_opmode = TYPEC_PWR_MODE_USB;
> -		port->negotiated_rev = PD_MAX_REV;
> +		port->negotiated_rev = tcpm_pd_supported_rev(port);
>   		port->message_id = 0;
>   		port->rx_msgid = -1;
>   		port->explicit_contract = false;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index bffc8d3e14ad..36282b2a9d9c 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -114,6 +114,9 @@ enum tcpm_transmit_type {
>    *              Optional; The USB Communications Capable bit indicates if port
>    *              partner is capable of communication over the USB data lines
>    *              (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit.
> + * @supported_pd_rev:
> + *              Optional; TCPM call this function to get supported PD revesion
> + *              from lower level driver.
>    */
>   struct tcpc_dev {
>   	struct fwnode_handle *fwnode;
> @@ -148,6 +151,7 @@ struct tcpc_dev {
>   						 bool pps_active, u32 requested_vbus_voltage);
>   	bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
>   	void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable);
> +	u32 (*supported_pd_rev)(struct tcpc_dev *dev);
>   };
>   
>   struct tcpm_port;

Re: [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev

[kernel test robot]

Hi Potin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.17-rc3 next-20220208]
[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/Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: hexagon-randconfig-r034-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090006.YLbevIuT-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project e8bff9ae54a55b4dbfeb6ba55f723abbd81bf494)
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/0day-ci/linux/commit/322696594704fa918e63d1c80fa6d346a02e9a28
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246
        git checkout 322696594704fa918e63d1c80fa6d346a02e9a28
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/usb/typec/tcpm/

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 >>):

>> drivers/usb/typec/tcpm/tcpm.c:581:9: warning: comparison of distinct pointer types ('typeof (rev) *' (aka 'unsigned int *') and 'typeof (2) *' (aka 'int *')) [-Wcompare-distinct-pointer-types]
           return min(rev, PD_MAX_REV);
                  ^~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   1 warning generated.


vim +581 drivers/usb/typec/tcpm/tcpm.c

   573	
   574	static u32 tcpm_pd_supported_rev(struct tcpm_port *port)
   575	{
   576		u32 rev = PD_MAX_REV;
   577	
   578		if (port->tcpc->supported_pd_rev)
   579			rev = port->tcpc->supported_pd_rev(port->tcpc);
   580	
 > 581		return min(rev, PD_MAX_REV);
   582	}
   583	

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

Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev

[Potin Lai]

Guenter Roeck 於 2022/2/8 下午 11:22 寫道:
> On 2/8/22 03:22, Potin Lai wrote:
>> Add support for passing supported PD rev to TCPM.
>> If "supported-pd-rev" property exist, then return supported_pd_rev as
>> defined value in DTS, otherwise return PD_MAX_REV
>>
>> Example of DTS:
>>
>> fusb302: typec-portc@22 {
>>      compatible = "fcs,fusb302";
>>      reg = <0x22>;
>>      ...
>>      supported-pd-rev=<1>; // PD_REV20
>>      ...
>> };
>>
>
> The new property needs to be documented. However, I am not entirely sure
> I understand why it is needed. Wouldn't the supported PD revision
> be a chip (fusb302) specific constant ? If so, why does it need a
> devicetree property ? I think that needs some additional explanation.
>
> Thanks,
> Guenter
>

My initially intend was adding flexibility for developer to decided 
which PD revision
they want to use for negotiation.

I saw your reply in another patch,  I agree with you, it will be simple 
and consistent if
move "supported-pd-rev" to tcpm fwnode as other capabilities.

Just want to double confirm, is "usb-connector.yaml" right place to put 
documentation
if adding "supported-pd-rev" in fwnode?

Thanks,
Potin

>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>> ---
>>   drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/usb/typec/tcpm/fusb302.c 
>> b/drivers/usb/typec/tcpm/fusb302.c
>> index 72f9001b0792..8cff92d58b96 100644
>> --- a/drivers/usb/typec/tcpm/fusb302.c
>> +++ b/drivers/usb/typec/tcpm/fusb302.c
>> @@ -109,6 +109,9 @@ struct fusb302_chip {
>>       enum typec_cc_status cc2;
>>       u32 snk_pdo[PDO_MAX_OBJECTS];
>>   +    /* supported pd rev */
>> +    u32 supported_pd_rev;
>> +
>>   #ifdef CONFIG_DEBUG_FS
>>       struct dentry *dentry;
>>       /* lock for log buffer access */
>> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev 
>> *dev, enum tcpm_transmit_type type,
>>       return ret;
>>   }
>>   +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
>> +{
>> +    struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>> +                         tcpc_dev);
>> +    return chip->supported_pd_rev;
>> +}
>> +
>>   static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
>>   {
>>       if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
>> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev 
>> *fusb302_tcpc_dev)
>>       fusb302_tcpc_dev->set_roles = tcpm_set_roles;
>>       fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>>       fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>> +    fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
>>   }
>>     static const char * const cc_polarity_name[] = {
>> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client 
>> *client,
>>       struct fusb302_chip *chip;
>>       struct i2c_adapter *adapter = client->adapter;
>>       struct device *dev = &client->dev;
>> +    struct device_node *np = dev->of_node;
>>       const char *name;
>>       int ret = 0;
>>   @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client 
>> *client,
>>           dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", 
>> ret);
>>           goto tcpm_unregister_port;
>>       }
>> +
>> +    if (of_property_read_u32(np, "supported-pd-rev",
>> +                &chip->supported_pd_rev) < 0) {
>> +        chip->supported_pd_rev = PD_MAX_REV;
>> +    } else if (chip->supported_pd_rev > PD_MAX_REV) {
>> +        chip->supported_pd_rev = PD_MAX_REV;
>> +    }
>
> The else part is also checked in the tcpm code and thus seems
> to be redundant.
>
>> +
>>       enable_irq_wake(chip->gpio_int_n_irq);
>>       i2c_set_clientdata(client, chip);
>

Re: [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev

[kernel test robot]

Hi Potin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.17-rc3 next-20220208]
[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/Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm64-randconfig-s031-20220208 (https://download.01.org/0day-ci/archive/20220209/202202091453.L5BVOjcx-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/322696594704fa918e63d1c80fa6d346a02e9a28
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246
        git checkout 322696594704fa918e63d1c80fa6d346a02e9a28
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/usb/typec/tcpm/

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


sparse warnings: (new ones prefixed by >>)
>> drivers/usb/typec/tcpm/tcpm.c:581:16: sparse: sparse: incompatible types in comparison expression (different signedness):
>> drivers/usb/typec/tcpm/tcpm.c:581:16: sparse:    unsigned int *
>> drivers/usb/typec/tcpm/tcpm.c:581:16: sparse:    int *

vim +581 drivers/usb/typec/tcpm/tcpm.c

   573	
   574	static u32 tcpm_pd_supported_rev(struct tcpm_port *port)
   575	{
   576		u32 rev = PD_MAX_REV;
   577	
   578		if (port->tcpc->supported_pd_rev)
   579			rev = port->tcpc->supported_pd_rev(port->tcpc);
   580	
 > 581		return min(rev, PD_MAX_REV);
   582	}
   583	

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

Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev

[Guenter Roeck]

On 2/8/22 19:34, Potin Lai wrote:
> 
> Guenter Roeck 於 2022/2/8 下午 11:22 寫道:
>> On 2/8/22 03:22, Potin Lai wrote:
>>> Add support for passing supported PD rev to TCPM.
>>> If "supported-pd-rev" property exist, then return supported_pd_rev as
>>> defined value in DTS, otherwise return PD_MAX_REV
>>>
>>> Example of DTS:
>>>
>>> fusb302: typec-portc@22 {
>>>      compatible = "fcs,fusb302";
>>>      reg = <0x22>;
>>>      ...
>>>      supported-pd-rev=<1>; // PD_REV20
>>>      ...
>>> };
>>>
>>
>> The new property needs to be documented. However, I am not entirely sure
>> I understand why it is needed. Wouldn't the supported PD revision
>> be a chip (fusb302) specific constant ? If so, why does it need a
>> devicetree property ? I think that needs some additional explanation.
>>
>> Thanks,
>> Guenter
>>
> 
> My initially intend was adding flexibility for developer to decided which PD revision
> they want to use for negotiation.
> 

I may be missing something, but I don't think that "flexibility for developer
to decide which PD revision they want to use for negotiation" is entirely appropriate.
This should be a chip property, not something a developer should decide. As written,
the code does accept PC version 3 for FUSB302 by default, which seems odd and unusual.
If the chip supports PD version 3, why artificially limit it to earlier PD revisions ?

I think this requires a detailed description of the use case. Is fusb302 indeed not able
to support a specific version of the power delivery specification ? If yes,
what is the limitation, and why would it need to be configurable with a devicetree
property ?

Thanks,
Guenter

> I saw your reply in another patch,  I agree with you, it will be simple and consistent if
> move "supported-pd-rev" to tcpm fwnode as other capabilities.
> 
> Just want to double confirm, is "usb-connector.yaml" right place to put documentation
> if adding "supported-pd-rev" in fwnode?
> 
> Thanks,
> Potin
> 
>>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>>> ---
>>>   drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
>>>   1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
>>> index 72f9001b0792..8cff92d58b96 100644
>>> --- a/drivers/usb/typec/tcpm/fusb302.c
>>> +++ b/drivers/usb/typec/tcpm/fusb302.c
>>> @@ -109,6 +109,9 @@ struct fusb302_chip {
>>>       enum typec_cc_status cc2;
>>>       u32 snk_pdo[PDO_MAX_OBJECTS];
>>>   +    /* supported pd rev */
>>> +    u32 supported_pd_rev;
>>> +
>>>   #ifdef CONFIG_DEBUG_FS
>>>       struct dentry *dentry;
>>>       /* lock for log buffer access */
>>> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>>>       return ret;
>>>   }
>>>   +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
>>> +{
>>> +    struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>>> +                         tcpc_dev);
>>> +    return chip->supported_pd_rev;
>>> +}
>>> +
>>>   static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
>>>   {
>>>       if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
>>> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>>>       fusb302_tcpc_dev->set_roles = tcpm_set_roles;
>>>       fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>>>       fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>>> +    fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
>>>   }
>>>     static const char * const cc_polarity_name[] = {
>>> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client,
>>>       struct fusb302_chip *chip;
>>>       struct i2c_adapter *adapter = client->adapter;
>>>       struct device *dev = &client->dev;
>>> +    struct device_node *np = dev->of_node;
>>>       const char *name;
>>>       int ret = 0;
>>>   @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client,
>>>           dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
>>>           goto tcpm_unregister_port;
>>>       }
>>> +
>>> +    if (of_property_read_u32(np, "supported-pd-rev",
>>> +                &chip->supported_pd_rev) < 0) {
>>> +        chip->supported_pd_rev = PD_MAX_REV;
>>> +    } else if (chip->supported_pd_rev > PD_MAX_REV) {
>>> +        chip->supported_pd_rev = PD_MAX_REV;
>>> +    }
>>
>> The else part is also checked in the tcpm code and thus seems
>> to be redundant.
>>
>>> +
>>>       enable_irq_wake(chip->gpio_int_n_irq);
>>>       i2c_set_clientdata(client, chip);
>>

Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev

[Potin Lai (賴柏廷)]

Guenter Roeck 於 2022/2/10 上午 01:50 寫道:
> On 2/8/22 19:34, Potin Lai wrote:
>>
>> Guenter Roeck 於 2022/2/8 下午 11:22 寫道:
>>> On 2/8/22 03:22, Potin Lai wrote:
>>>> Add support for passing supported PD rev to TCPM.
>>>> If "supported-pd-rev" property exist, then return supported_pd_rev as
>>>> defined value in DTS, otherwise return PD_MAX_REV
>>>>
>>>> Example of DTS:
>>>>
>>>> fusb302: typec-portc@22 {
>>>>      compatible = "fcs,fusb302";
>>>>      reg = <0x22>;
>>>>      ...
>>>>      supported-pd-rev=<1>; // PD_REV20
>>>>      ...
>>>> };
>>>>
>>>
>>> The new property needs to be documented. However, I am not entirely sure
>>> I understand why it is needed. Wouldn't the supported PD revision
>>> be a chip (fusb302) specific constant ? If so, why does it need a
>>> devicetree property ? I think that needs some additional explanation.
>>>
>>> Thanks,
>>> Guenter
>>>
>>
>> My initially intend was adding flexibility for developer to decided which PD revision
>> they want to use for negotiation.
>>
>
> I may be missing something, but I don't think that "flexibility for developer
> to decide which PD revision they want to use for negotiation" is entirely appropriate.
> This should be a chip property, not something a developer should decide. As written,
> the code does accept PC version 3 for FUSB302 by default, which seems odd and unusual.
> If the chip supports PD version 3, why artificially limit it to earlier PD revisions ?
>
> I think this requires a detailed description of the use case. Is fusb302 indeed not able
> to support a specific version of the power delivery specification ? If yes,
> what is the limitation, and why would it need to be configurable with a devicetree
> property ?
>
> Thanks,
> Guenter
>

[  414.375215] AMS DISCOVER_IDENTITY start
[  414.375228] cc:=4
[  414.380834] PD RX, header: 0x291 [1]
[  414.380846] AMS DISCOVER_IDENTITY finished
[  414.380850] cc:=5
[  414.388203] PD RX, header: 0x291 [1]
[  414.388211] PD RX, header: 0x291 [1]
[  414.388220] PD TX, header: 0x7b0
[  414.499594] Received hard reset
[  414.499615] state change SRC_READY -> HARD_RESET_START [rev3 HARD_RESET]


In my case, if I keep it as PD_MAX_REV (3.0), I always receive hard reset after "PD RX, header: 0x291" (Get_Source_Cap_Extended), then entire state machine will start over again and again.

If I force PD rev at 2.0, then I won't receive Get_Source_Cap_Extended message, and state machine become stable.

Not quite sure the reason of receiving hard reset, my guessing, fus302 not recognize PD 3.0 message, so it doesn't reply GoodCRC automatically, and then it causing timeout or events to trigger hard reset from the other side.

Thanks,
Potin
>> I saw your reply in another patch,  I agree with you, it will be simple and consistent if
>> move "supported-pd-rev" to tcpm fwnode as other capabilities.
>>
>> Just want to double confirm, is "usb-connector.yaml" right place to put documentation
>> if adding "supported-pd-rev" in fwnode?
>>
>> Thanks,
>> Potin
>>
>>>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>>>> ---
>>>>   drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++
>>>>   1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
>>>> index 72f9001b0792..8cff92d58b96 100644
>>>> --- a/drivers/usb/typec/tcpm/fusb302.c
>>>> +++ b/drivers/usb/typec/tcpm/fusb302.c
>>>> @@ -109,6 +109,9 @@ struct fusb302_chip {
>>>>       enum typec_cc_status cc2;
>>>>       u32 snk_pdo[PDO_MAX_OBJECTS];
>>>>   +    /* supported pd rev */
>>>> +    u32 supported_pd_rev;
>>>> +
>>>>   #ifdef CONFIG_DEBUG_FS
>>>>       struct dentry *dentry;
>>>>       /* lock for log buffer access */
>>>> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>>>>       return ret;
>>>>   }
>>>>   +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev)
>>>> +{
>>>> +    struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>>>> +                         tcpc_dev);
>>>> +    return chip->supported_pd_rev;
>>>> +}
>>>> +
>>>>   static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl)
>>>>   {
>>>>       if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX)
>>>> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>>>>       fusb302_tcpc_dev->set_roles = tcpm_set_roles;
>>>>       fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>>>>       fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>>>> +    fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev;
>>>>   }
>>>>     static const char * const cc_polarity_name[] = {
>>>> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client,
>>>>       struct fusb302_chip *chip;
>>>>       struct i2c_adapter *adapter = client->adapter;
>>>>       struct device *dev = &client->dev;
>>>> +    struct device_node *np = dev->of_node;
>>>>       const char *name;
>>>>       int ret = 0;
>>>>   @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client,
>>>>           dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
>>>>           goto tcpm_unregister_port;
>>>>       }
>>>> +
>>>> +    if (of_property_read_u32(np, "supported-pd-rev",
>>>> +                &chip->supported_pd_rev) < 0) {
>>>> +        chip->supported_pd_rev = PD_MAX_REV;
>>>> +    } else if (chip->supported_pd_rev > PD_MAX_REV) {
>>>> +        chip->supported_pd_rev = PD_MAX_REV;
>>>> +    }
>>>
>>> The else part is also checked in the tcpm code and thus seems
>>> to be redundant.
>>>
>>>> +
>>>>       enable_irq_wake(chip->gpio_int_n_irq);
>>>>       i2c_set_clientdata(client, chip);
>>>
>