[bug report] usb: typec: ucsi: extract code to read PD caps

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

Hello Dmitry Baryshkov,

Commit ad4bc68bef3e ("usb: typec: ucsi: extract code to read PD
caps") from Mar 29, 2024 (linux-next), leads to the following Smatch
static checker warning:

	drivers/usb/typec/ucsi/ucsi.c:689 ucsi_get_pd_caps()
	warn: passing zero to 'ERR_PTR'

drivers/usb/typec/ucsi/ucsi.c
    680 static struct usb_power_delivery_capabilities *ucsi_get_pd_caps(struct ucsi_connector *con,
    681                                                                 enum typec_role role,
    682                                                                 bool is_partner)
    683 {
    684         struct usb_power_delivery_capabilities_desc pd_caps;
    685         int ret;
    686 
    687         ret = ucsi_get_pdos(con, role, is_partner, pd_caps.pdo);
    688         if (ret <= 0)
--> 689                 return ERR_PTR(ret);

This is returns NULL if ucsi_get_pdos() returns zero.  It's really hard
to say if this is intentional or not...  Originally, we treated error
codes and zero the same but we didn't report errors back to the user,
the code just continued silently.  Now it's reporting errors but just
continuing along if ucsi_get_pdos() returns zero.

My instinct says that we should modify ucsi_get_pdos() to not return
zero but I don't know the code...

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 8abde8db6bcf..427b7610a074 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -650,6 +650,8 @@ static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role,
 		return ret;
 
 	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
+	if (num_pdos == 0)
+		return -EINVAL;
 	if (num_pdos < UCSI_MAX_PDOS)
 		return num_pdos;
 
Or, if returning NULL is intentional then it would be nice to add a
comment.

    690 
    691         if (ret < PDO_MAX_OBJECTS)
    692                 pd_caps.pdo[ret] = 0;
    693 
    694         pd_caps.role = role;
    695 
    696         return usb_power_delivery_register_capabilities(is_partner ? con->partner_pd : con->pd,
    697                                                         &pd_caps);
    698 }

regards,
dan carpenter