Paul Jimenez wrote:
> Werner Almesberger wrote:
>
>> Andy Green wrote:
>>
>>
>>> +static int usb_get_property(struct power_supply *psy,
>>> + enum power_supply_property psp,
>>> + union power_supply_propval *val)
>>> +{
>>> + int ret = 0;
>>> + struct bq27000_device_info *di = container_of(psy, struct
>>> bq27000_device_info, usb);
>>> +
>>> + if (!(di->pdata->hdq_initialized)())
>>> + return -EINVAL;
>>> +
>>> + switch (psp) {
>>> + case POWER_SUPPLY_PROP_ONLINE:
>>> + if (di->pdata->get_charger_online_status)
>>> + val->intval = (di->pdata->get_charger_online_status)();
>>> + else
>>> + return -EINVAL;
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> + return ret;
>>> +}
>>>
>>>
>> This looks a bit chatty. Maybe a future style improvement could look like
>> this ?
>>
>> {
>> struct bq27000_device_info *di = container_of(psy, struct
>> bq27000_device_info, usb);
>>
>> if (!di->pdata->hdq_initialized())
>> return -EINVAL;
>>
>> switch (psp) {
>> case POWER_SUPPLY_PROP_ONLINE:
>> if (!di->pdata->get_charger_online_status)
>> return -EINVAL;
>> val->intval = di->pdata->get_charger_online_status();
>> break;
>> default:
>> return -EINVAL;
>> }
>> return 0;
>> }
>>
>> - Werner
>>
>>
>>
> I don't grok the use of switch() for a two case... case. Try:
>
>
> {
> struct bq27000_device_info *di = container_of(psy, struct
> bq27000_device_info, usb);
>
> if (!di->pdata->hdq_initialized())
> return -EINVAL;
>
> if (psp != POWER_SUPPLY_PROP_ONLINE ||
> !di->pdata->get_charger_online_status)
> return -EINVAL;
>
> val->intval = di->pdata->get_charger_online_status();
>
> return 0;
> }
>
> I don't know whether current style says to split up the || or join it with
> the test above, but those are other possibilities.
There is something to say about consistency with the other class
handlers, plus the easy addition of more property types. I doubt the
code is that much less efficient as a switch, so I'd prefer to leave
as-is for that reason. But of course I'd also prefer smaller and more
readable code. Please feel free to submit cleanup patches.
Cheers,
Sean
