Re: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

On 10/23/20 1:24 PM, Dan Carpenter wrote:
These variables are enums but in this situation GCC will treat them as
unsigned so the conditions are never true.

Fixes: da0cb6310094 ("usb: typec: add support for STUSB160x Type-C controller family")
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
  drivers/usb/typec/stusb160x.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/stusb160x.c b/drivers/usb/typec/stusb160x.c
index f7369e371dd4..da7f1957bcb3 100644
--- a/drivers/usb/typec/stusb160x.c
+++ b/drivers/usb/typec/stusb160x.c
@@ -545,7 +545,7 @@ static int stusb160x_get_fw_caps(struct stusb160x *chip,
  	ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
  	if (!ret) {
  		chip->port_type = typec_find_port_power_role(cap_str);
-		if (chip->port_type < 0) {
+		if ((int)chip->port_type < 0) {
  			ret = chip->port_type;
  			return ret;
  		}

I was preparing a patch for this one, and it uses the ret instead of the cast:
	ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
	if (!ret) {
		ret = typec_find_port_power_role(cap_str);
		if (ret < 0)
			return ret;
		chip->port_type = ret;

	}

@@ -567,9 +567,10 @@ static int stusb160x_get_fw_caps(struct stusb160x *chip,
  	if (!ret) {
  		chip->pwr_opmode = typec_find_pwr_opmode(cap_str);
  		/* Power delivery not yet supported */
-		if (chip->pwr_opmode < 0 ||
+		if ((int)chip->pwr_opmode < 0 ||
  		    chip->pwr_opmode == TYPEC_PWR_MODE_PD) {
-			ret = chip->pwr_opmode < 0 ? chip->pwr_opmode : -EINVAL;
+			ret = (int)chip->pwr_opmode < 0 ? chip->pwr_opmode :
+							  -EINVAL;
  			dev_err(chip->dev, "bad power operation mode: %d\n",
  				chip->pwr_opmode);
  			return ret;


	if (!ret) {
		ret = typec_find_pwr_opmode(cap_str);
		/* Power delivery not yet supported */
		if (ret < 0 || ret == TYPEC_PWR_MODE_PD) {
			dev_err(chip->dev, "bad power operation mode: %d\n", ret);
			return -EINVAL;
		}
		chip->pwr_opmode = ret;
	}


So, which fix sounds better ? IMHO using ret make the code more readable.

Regards,
Amelie



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux