Re: [PATCH v2 02/12] usb: typec: Start using ERR_PTR
From: Guenter Roeck
Date: Sun Feb 25 2018 - 16:38:25 EST
On 02/25/2018 07:25 AM, Hans de Goede wrote:
From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
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 <heikki.krogerus@xxxxxxxxxxxxxxx>
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
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, &port->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 = &tps->partner_identity;
}
- tps->partner = typec_register_partner(tps->port, &desc);
- 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, &desc);
+ 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(&client->dev, &tps->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
+++ b/drivers/usb/typec/typec.c
@@ -386,7 +386,7 @@ typec_register_altmode(struct device *parent,
alt = kzalloc(sizeof(*alt), GFP_KERNEL);
if (!alt)
- return NULL;
+ return ERR_PTR(-ENOMEM);
alt->svid = desc->svid;
alt->n_modes = desc->n_modes;
@@ -402,7 +402,7 @@ typec_register_altmode(struct device *parent,
dev_err(parent, "failed to register alternate mode (%d)\n",
ret);
put_device(&alt->dev);
- return NULL;
+ return ERR_PTR(ret);
}
return alt;
@@ -417,7 +417,7 @@ typec_register_altmode(struct device *parent,
*/
void typec_unregister_altmode(struct typec_altmode *alt)
{
- if (alt)
+ if (!IS_ERR_OR_NULL(alt))
device_unregister(&alt->dev);
}
EXPORT_SYMBOL_GPL(typec_unregister_altmode);
@@ -509,7 +509,7 @@ EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
*
* Registers a device for USB Type-C Partner described in @desc.
*
- * Returns handle to the partner on success or NULL on failure.
+ * Returns handle to the partner on success or ERR_PTR on failure.
*/
struct typec_partner *typec_register_partner(struct typec_port *port,
struct typec_partner_desc *desc)
@@ -519,7 +519,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
partner = kzalloc(sizeof(*partner), GFP_KERNEL);
if (!partner)
- return NULL;
+ return ERR_PTR(-ENOMEM);
partner->usb_pd = desc->usb_pd;
partner->accessory = desc->accessory;
@@ -542,7 +542,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
if (ret) {
dev_err(&port->dev, "failed to register partner (%d)\n", ret);
put_device(&partner->dev);
- return NULL;
+ return ERR_PTR(ret);
}
return partner;
@@ -557,7 +557,7 @@ EXPORT_SYMBOL_GPL(typec_register_partner);
*/
void typec_unregister_partner(struct typec_partner *partner)
{
- if (partner)
+ if (!IS_ERR_OR_NULL(partner))
device_unregister(&partner->dev);
}
EXPORT_SYMBOL_GPL(typec_unregister_partner);
@@ -587,7 +587,7 @@ static const struct device_type typec_plug_dev_type = {
* the plug lists in response to Discover Modes command need to be listed in an
* array in @desc.
*
- * Returns handle to the alternate mode on success or NULL on failure.
+ * Returns handle to the alternate mode on success or ERR_PTR on failure.
*/
struct typec_altmode *
typec_plug_register_altmode(struct typec_plug *plug,
@@ -606,7 +606,7 @@ EXPORT_SYMBOL_GPL(typec_plug_register_altmode);
* Cable Plug represents a plug with electronics in it that can response to USB
* Power Delivery SOP Prime or SOP Double Prime packages.
*
- * Returns handle to the cable plug on success or NULL on failure.
+ * Returns handle to the cable plug on success or ERR_PTR on failure.
*/
struct typec_plug *typec_register_plug(struct typec_cable *cable,
struct typec_plug_desc *desc)
@@ -617,7 +617,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
plug = kzalloc(sizeof(*plug), GFP_KERNEL);
if (!plug)
- return NULL;
+ return ERR_PTR(-ENOMEM);
sprintf(name, "plug%d", desc->index);
@@ -631,7 +631,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
if (ret) {
dev_err(&cable->dev, "failed to register plug (%d)\n", ret);
put_device(&plug->dev);
- return NULL;
+ return ERR_PTR(ret);
}
return plug;
@@ -646,7 +646,7 @@ EXPORT_SYMBOL_GPL(typec_register_plug);
*/
void typec_unregister_plug(struct typec_plug *plug)
{
- if (plug)
+ if (!IS_ERR_OR_NULL(plug))
device_unregister(&plug->dev);
}
EXPORT_SYMBOL_GPL(typec_unregister_plug);
@@ -724,7 +724,7 @@ EXPORT_SYMBOL_GPL(typec_cable_set_identity);
* Registers a device for USB Type-C Cable described in @desc. The cable will be
* parent for the optional cable plug devises.
*
- * Returns handle to the cable on success or NULL on failure.
+ * Returns handle to the cable on success or ERR_PTR on failure.
*/
struct typec_cable *typec_register_cable(struct typec_port *port,
struct typec_cable_desc *desc)
@@ -734,7 +734,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
cable = kzalloc(sizeof(*cable), GFP_KERNEL);
if (!cable)
- return NULL;
+ return ERR_PTR(-ENOMEM);
cable->type = desc->type;
cable->active = desc->active;
@@ -757,7 +757,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
if (ret) {
dev_err(&port->dev, "failed to register cable (%d)\n", ret);
put_device(&cable->dev);
- return NULL;
+ return ERR_PTR(ret);
}
return cable;
@@ -772,7 +772,7 @@ EXPORT_SYMBOL_GPL(typec_register_cable);
*/
void typec_unregister_cable(struct typec_cable *cable)
{
- if (cable)
+ if (!IS_ERR_OR_NULL(cable))
device_unregister(&cable->dev);
}
EXPORT_SYMBOL_GPL(typec_unregister_cable);
@@ -1256,7 +1256,7 @@ EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
* This routine is used to register an alternate mode that @port is capable of
* supporting.
*
- * Returns handle to the alternate mode on success or NULL on failure.
+ * Returns handle to the alternate mode on success or ERR_PTR on failure.
*/
struct typec_altmode *
typec_port_register_altmode(struct typec_port *port,
@@ -1273,7 +1273,7 @@ EXPORT_SYMBOL_GPL(typec_port_register_altmode);
*
* Registers a device for USB Type-C Port described in @cap.
*
- * Returns handle to the port on success or NULL on failure.
+ * Returns handle to the port on success or ERR_PTR on failure.
*/
struct typec_port *typec_register_port(struct device *parent,
const struct typec_capability *cap)
@@ -1285,12 +1285,12 @@ struct typec_port *typec_register_port(struct device *parent,
port = kzalloc(sizeof(*port), GFP_KERNEL);
if (!port)
- return NULL;
+ return ERR_PTR(-ENOMEM);
id = ida_simple_get(&typec_index_ida, 0, 0, GFP_KERNEL);
if (id < 0) {
kfree(port);
- return NULL;
+ return ERR_PTR(id);
}
if (cap->type == TYPEC_PORT_DFP)
@@ -1326,7 +1326,7 @@ struct typec_port *typec_register_port(struct device *parent,
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(&port->dev);
- return NULL;
+ return ERR_PTR(ret);
}
return port;
@@ -1341,7 +1341,7 @@ EXPORT_SYMBOL_GPL(typec_register_port);
*/
void typec_unregister_port(struct typec_port *port)
{
- if (port)
+ if (!IS_ERR_OR_NULL(port))
device_unregister(&port->dev);
}
EXPORT_SYMBOL_GPL(typec_unregister_port);
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 79046fe66426..69d544cfcd45 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -260,38 +260,45 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con)
static int ucsi_register_partner(struct ucsi_connector *con)
{
- struct typec_partner_desc partner;
+ struct typec_partner_desc desc;
+ struct typec_partner *partner;
if (con->partner)
return 0;
- memset(&partner, 0, sizeof(partner));
+ memset(&desc, 0, sizeof(desc));
switch (con->status.partner_type) {
case UCSI_CONSTAT_PARTNER_TYPE_DEBUG:
- partner.accessory = TYPEC_ACCESSORY_DEBUG;
+ desc.accessory = TYPEC_ACCESSORY_DEBUG;
break;
case UCSI_CONSTAT_PARTNER_TYPE_AUDIO:
- partner.accessory = TYPEC_ACCESSORY_AUDIO;
+ desc.accessory = TYPEC_ACCESSORY_AUDIO;
break;
default:
break;
}
- partner.usb_pd = con->status.pwr_op_mode == UCSI_CONSTAT_PWR_OPMODE_PD;
+ desc.usb_pd = con->status.pwr_op_mode == UCSI_CONSTAT_PWR_OPMODE_PD;
- con->partner = typec_register_partner(con->port, &partner);
- if (!con->partner) {
- dev_err(con->ucsi->dev, "con%d: failed to register partner\n",
- con->num);
- return -ENODEV;
+ partner = typec_register_partner(con->port, &desc);
+ if (IS_ERR(partner)) {
+ dev_err(con->ucsi->dev,
+ "con%d: failed to register partner (%ld)\n", con->num,
+ PTR_ERR(partner));
+ return PTR_ERR(partner);
}
+ con->partner = partner;
+
return 0;
}
static void ucsi_unregister_partner(struct ucsi_connector *con)
{
+ if (!con->partner)
+ return;
+
typec_unregister_partner(con->partner);
con->partner = NULL;
}
@@ -606,8 +613,8 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
/* Register the connector */
con->port = typec_register_port(ucsi->dev, cap);
- if (!con->port)
- return -ENODEV;
+ if (IS_ERR(con->port))
+ return PTR_ERR(con->port);
/* Get the status */
UCSI_CMD_GET_CONNECTOR_STATUS(ctrl, con->num);