Re: [PATCH v2 02/12] usb: typec: Start using ERR_PTR

[Guenter Roeck]

On 02/25/2018 07:25 AM, Hans de Goede wrote:

From: Heikki Krogerus 

In order to allow the USB Type-C Class driver take care of
things like muxes and other possible dependencies for the
port drivers, returning ERR_PTR instead of NULL from the
registration functions in case of failure.

The reason for taking over control of the muxes for example
is because handling them in the port drivers would be just
boilerplate.

Signed-off-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede 


Reviewed-by: Guenter Roeck 


---
Changes in v2:
-Add IS_ERR_OR_NULL() checks to the unregister functions
---
  drivers/usb/typec/tcpm.c  | 16 +---
  drivers/usb/typec/tps6598x.c  | 15 ---
  drivers/usb/typec/typec.c | 44 +--
  drivers/usb/typec/ucsi/ucsi.c | 31 ++
  4 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index f4d563ee7690..7cd28b700a7f 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1044,7 +1044,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
break;
case CMDT_RSP_ACK:
/* silently drop message if we are not connected */
-   if (!port->partner)
+   if (IS_ERR_OR_NULL(port->partner))
break;
  
  		switch (cmd) {

@@ -3743,8 +3743,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
port->port_type = tcpc->config->type;
  
  	port->typec_port = typec_register_port(port->dev, >typec_caps);

-   if (!port->typec_port) {
-   err = -ENOMEM;
+   if (IS_ERR(port->typec_port)) {
+   err = PTR_ERR(port->typec_port);
goto out_destroy_wq;
}
  
@@ -3753,15 +3753,17 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
  
  		i = 0;

while (paltmode->svid && i < ARRAY_SIZE(port->port_altmode)) {
-   port->port_altmode[i] =
- typec_port_register_altmode(port->typec_port,
- paltmode);
-   if (!port->port_altmode[i]) {
+   struct typec_altmode *alt;
+
+   alt = typec_port_register_altmode(port->typec_port,
+ paltmode);
+   if (IS_ERR(alt)) {
tcpm_log(port,
 "%s: failed to register port alternate mode 
0x%x",
 dev_name(dev), paltmode->svid);
break;
}
+   port->port_altmode[i] = alt;
i++;
paltmode++;
}
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 2719f5d382f7..37a15c14a6c6 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -158,15 +158,15 @@ static int tps6598x_connect(struct tps6598x *tps, u32 
status)
desc.identity = >partner_identity;
}
  
-	tps->partner = typec_register_partner(tps->port, );

-   if (!tps->partner)
-   return -ENODEV;
-
typec_set_pwr_opmode(tps->port, mode);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
  
+	tps->partner = typec_register_partner(tps->port, );

+   if (IS_ERR(tps->partner))
+   return PTR_ERR(tps->partner);
+
if (desc.identity)
typec_partner_set_identity(tps->partner);
  
@@ -175,7 +175,8 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
  
  static void tps6598x_disconnect(struct tps6598x *tps, u32 status)

  {
-   typec_unregister_partner(tps->partner);
+   if (!IS_ERR(tps->partner))
+   typec_unregister_partner(tps->partner);
tps->partner = NULL;
typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
@@ -418,8 +419,8 @@ static int tps6598x_probe(struct i2c_client *client)
tps->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
  
  	tps->port = typec_register_port(>dev, >typec_cap);

-   if (!tps->port)
-   return -ENODEV;
+   if (IS_ERR(tps->port))
+   return PTR_ERR(tps->port);
  
  	if (status & TPS_STATUS_PLUG_PRESENT) {

ret = tps6598x_connect(tps, status);
diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 

[PATCH v2 02/12] usb: typec: Start using ERR_PTR

[Hans de Goede]

From: Heikki Krogerus 

In order to allow the USB Type-C Class driver take care of
things like muxes and other possible dependencies for the
port drivers, returning ERR_PTR instead of NULL from the
registration functions in case of failure.

The reason for taking over control of the muxes for example
is because handling them in the port drivers would be just
boilerplate.

Signed-off-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Signed-off-by: Hans de Goede 
---
Changes in v2:
-Add IS_ERR_OR_NULL() checks to the unregister functions
---
 drivers/usb/typec/tcpm.c  | 16 +---
 drivers/usb/typec/tps6598x.c  | 15 ---
 drivers/usb/typec/typec.c | 44 +--
 drivers/usb/typec/ucsi/ucsi.c | 31 ++
 4 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index f4d563ee7690..7cd28b700a7f 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1044,7 +1044,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
break;
case CMDT_RSP_ACK:
/* silently drop message if we are not connected */
-   if (!port->partner)
+   if (IS_ERR_OR_NULL(port->partner))
break;
 
switch (cmd) {
@@ -3743,8 +3743,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
port->port_type = tcpc->config->type;
 
port->typec_port = typec_register_port(port->dev, >typec_caps);
-   if (!port->typec_port) {
-   err = -ENOMEM;
+   if (IS_ERR(port->typec_port)) {
+   err = PTR_ERR(port->typec_port);
goto out_destroy_wq;
}
 
@@ -3753,15 +3753,17 @@ struct tcpm_port *tcpm_register_port(struct device 
*dev, struct tcpc_dev *tcpc)
 
i = 0;
while (paltmode->svid && i < ARRAY_SIZE(port->port_altmode)) {
-   port->port_altmode[i] =
- typec_port_register_altmode(port->typec_port,
- paltmode);
-   if (!port->port_altmode[i]) {
+   struct typec_altmode *alt;
+
+   alt = typec_port_register_altmode(port->typec_port,
+ paltmode);
+   if (IS_ERR(alt)) {
tcpm_log(port,
 "%s: failed to register port alternate 
mode 0x%x",
 dev_name(dev), paltmode->svid);
break;
}
+   port->port_altmode[i] = alt;
i++;
paltmode++;
}
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 2719f5d382f7..37a15c14a6c6 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -158,15 +158,15 @@ static int tps6598x_connect(struct tps6598x *tps, u32 
status)
desc.identity = >partner_identity;
}
 
-   tps->partner = typec_register_partner(tps->port, );
-   if (!tps->partner)
-   return -ENODEV;
-
typec_set_pwr_opmode(tps->port, mode);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
 
+   tps->partner = typec_register_partner(tps->port, );
+   if (IS_ERR(tps->partner))
+   return PTR_ERR(tps->partner);
+
if (desc.identity)
typec_partner_set_identity(tps->partner);
 
@@ -175,7 +175,8 @@ static int tps6598x_connect(struct tps6598x *tps, u32 
status)
 
 static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
 {
-   typec_unregister_partner(tps->partner);
+   if (!IS_ERR(tps->partner))
+   typec_unregister_partner(tps->partner);
tps->partner = NULL;
typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
@@ -418,8 +419,8 @@ static int tps6598x_probe(struct i2c_client *client)
tps->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
 
tps->port = typec_register_port(>dev, >typec_cap);
-   if (!tps->port)
-   return -ENODEV;
+   if (IS_ERR(tps->port))
+   return PTR_ERR(tps->port);
 
if (status & TPS_STATUS_PLUG_PRESENT) {
ret = tps6598x_connect(tps, status);
diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 735726ced602..dc28ad868d93 100644
--- a/drivers/usb/typec/typec.c
+++