Messages in this thread | | | Subject | Re: [PATCH v5 4/5] typec: tcpm: Represent source supply through power_supply | From | Guenter Roeck <> | Date | Thu, 22 Mar 2018 06:20:09 -0700 |
| |
On 03/22/2018 03:40 AM, Adam Thomson wrote: > On 22 March 2018 04:09, Guenter Roeck wrote: > >>> +static int tcpm_psy_set_prop(struct power_supply *psy, >>> + enum power_supply_property psp, >>> + const union power_supply_propval *val) >>> +{ >>> + struct tcpm_port *port = power_supply_get_drvdata(psy); >>> + int ret = 0; >>> + >>> + switch (psp) { >>> + case POWER_SUPPLY_PROP_ONLINE: >>> + ret = tcpm_psy_set_online(port, val); >>> + break; >>> + case POWER_SUPPLY_PROP_VOLTAGE_NOW: >>> + if ((val->intval < (port->pps_data.min_volt * 1000)) || >>> + (val->intval > (port->pps_data.max_volt * 1000))) >>> + ret = -EINVAL; >>> + else >>> + ret = tcpm_pps_set_out_volt(port, (val->intval / 1000)); >>> + break; >>> + case POWER_SUPPLY_PROP_CURRENT_NOW: >>> + if (val->intval > (port->pps_data.max_curr * 1000)) >>> + ret = -EINVAL; >>> + else >>> + ret = tcpm_pps_set_op_curr(port, (val->intval / 1000)); >> >> I am really not a friend of excessive ( ). > > Yes, I got that. :) I am of the opinion that they should be used to enforce > precedence. This to me is good coding practice and makes it unambiguous for the > reader. That's why I use them as above. Do you think the above uses make it > harder to understand or more difficult to maintain? > It confuses me and makes me think I am missing something, and causes me to miss the _real_ problems. If the compiler is not able to enforce precedence, even more so in situations like the above, I think it is about time to dump it.
Either case, your call to make. I wont give patches with excessive ( ) a Reviewed-by:, but then others can review the code.
Thanks, Guenter
| |