* [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
@ 2020-10-23 11:24 Dan Carpenter
2020-10-28 12:26 ` Amelie DELAUNAY
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2020-10-23 11:24 UTC (permalink / raw)
To: Heikki Krogerus, Amelie Delaunay
Cc: Greg Kroah-Hartman, linux-usb, kernel-janitors
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@oracle.com>
---
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;
}
@@ -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;
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
2020-10-23 11:24 [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs Dan Carpenter
@ 2020-10-28 12:26 ` Amelie DELAUNAY
2020-10-28 13:23 ` Dan Carpenter
2020-10-29 8:21 ` AW: " Walter Harms
0 siblings, 2 replies; 6+ messages in thread
From: Amelie DELAUNAY @ 2020-10-28 12:26 UTC (permalink / raw)
To: Dan Carpenter, Heikki Krogerus
Cc: Greg Kroah-Hartman, linux-usb, kernel-janitors
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@oracle.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
2020-10-28 12:26 ` Amelie DELAUNAY
@ 2020-10-28 13:23 ` Dan Carpenter
2020-10-28 14:38 ` Amelie DELAUNAY
2020-10-29 8:21 ` AW: " Walter Harms
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2020-10-28 13:23 UTC (permalink / raw)
To: Amelie DELAUNAY
Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, kernel-janitors
On Wed, Oct 28, 2020 at 01:26:16PM +0100, Amelie DELAUNAY wrote:
> > @@ -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.
Yeah. Your patch is nicer, but Greg *just* merged mine so it might
be too late...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
2020-10-28 13:23 ` Dan Carpenter
@ 2020-10-28 14:38 ` Amelie DELAUNAY
2020-10-28 15:40 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Amelie DELAUNAY @ 2020-10-28 14:38 UTC (permalink / raw)
To: Dan Carpenter, Greg Kroah-Hartman
Cc: Heikki Krogerus, linux-usb, kernel-janitors
Hi Greg,
I know I'm a bit late for the review, but is it still possible to fixup
this patch in your usb-linus branch?
Regards,
Amelie
On 10/28/20 2:23 PM, Dan Carpenter wrote:
> On Wed, Oct 28, 2020 at 01:26:16PM +0100, Amelie DELAUNAY wrote:
>>> @@ -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.
>
> Yeah. Your patch is nicer, but Greg *just* merged mine so it might
> be too late...
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
2020-10-28 14:38 ` Amelie DELAUNAY
@ 2020-10-28 15:40 ` Greg Kroah-Hartman
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-28 15:40 UTC (permalink / raw)
To: Amelie DELAUNAY
Cc: Dan Carpenter, Heikki Krogerus, linux-usb, kernel-janitors
On Wed, Oct 28, 2020 at 03:38:54PM +0100, Amelie DELAUNAY wrote:
> Hi Greg,
>
> I know I'm a bit late for the review, but is it still possible to fixup this
> patch in your usb-linus branch?
Send me a patch on top of the existing one and I will be glad to review
and take it if needed.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* AW: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
2020-10-28 12:26 ` Amelie DELAUNAY
2020-10-28 13:23 ` Dan Carpenter
@ 2020-10-29 8:21 ` Walter Harms
1 sibling, 0 replies; 6+ messages in thread
From: Walter Harms @ 2020-10-29 8:21 UTC (permalink / raw)
To: Amelie DELAUNAY, Dan Carpenter, Heikki Krogerus
Cc: Greg Kroah-Hartman, linux-usb, kernel-janitors
this is much better to read
________________________________________
Von: Amelie DELAUNAY [amelie.delaunay@st.com]
Gesendet: Mittwoch, 28. Oktober 2020 13:26
An: Dan Carpenter; Heikki Krogerus
Cc: Greg Kroah-Hartman; linux-usb@vger.kernel.org; kernel-janitors@vger.kernel.org
Betreff: Re: [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs
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@oracle.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-29 8:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 11:24 [PATCH 2/2] usb: typec: stusb160x: fix some signedness bugs Dan Carpenter
2020-10-28 12:26 ` Amelie DELAUNAY
2020-10-28 13:23 ` Dan Carpenter
2020-10-28 14:38 ` Amelie DELAUNAY
2020-10-28 15:40 ` Greg Kroah-Hartman
2020-10-29 8:21 ` AW: " Walter Harms
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).