> From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > 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 devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
