> From: Dan Carpenter [mailto:[email protected]]
> Sent: Monday, September 30, 2013 6:09 PM
>
> Yeah. I guess it's fine... I was going to suggest adding the + 1 in a
> different place but actually it doesn't matter.
>
> The key to understanding dwc2_set_param_host_channels() is that the
> "val" parameter is always -1. That means it always returns -EINVAL and
> the caller jumbles the error code in with some other error codes and
> then ignores any errors. :/
The intent of this was that the value can be overridden by the platform
code if required, in which case "val" would not be -1. However, to my
knowledge, no in-tree platforms do that, so I guess it would be fine to
redo this as you suggest below. We can always add it back if needed.
--
Paul
> 2182 int dwc2_set_param_host_channels(struct dwc2_hsotg *hsotg, int val)
> 2183 {
> 2184 int valid = 1;
> 2185 int retval = 0;
> 2186
> 2187 if (val < 1 || val > hsotg->hw_params.host_channels)
> 2188 valid = 0;
> 2189
> 2190 if (!valid) {
> 2191 if (val >= 0)
> 2192 dev_err(hsotg->dev,
> 2193 "%d invalid for host_channels. Check
> HW configuration.\n",
> 2194 val);
> 2195 val = hsotg->hw_params.host_channels;
> 2196 dev_dbg(hsotg->dev, "Setting host_channels to %d\n",
> val);
> 2197 retval = -EINVAL;
> 2198 }
> 2199
> 2200 hsotg->core_params->host_channels = val;
> 2201 return retval;
> 2202 }
>
> It would be better to re-write this with all the dead cruft removed:
>
> int dwc2_set_param_host_channels(struct dwc2_hsotg *hsotg)
> {
> hsotg->core_params->host_channels = hsotg->hw_params.host_channels + 1;
> return 0;
> }
>
> regards,
> dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
