[U-Boot] [PATCH 1/2] misc: uclass: Add enable/disable functions

[Mario Six]

Hi Simon,

On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mario,
>
> On 28 March 2018 at 20:38, Mario Six <mario.six at gdsys.cc> wrote:
>> Add generic enable/disable functions to the misc uclass.
>>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>> ---
>>  drivers/misc/misc-uclass.c | 20 ++++++++++++++++++++
>>  include/misc.h             | 30 ++++++++++++++++++++++++++++++
>>  2 files changed, 50 insertions(+)
>>
>> diff --git a/drivers/misc/misc-uclass.c b/drivers/misc/misc-uclass.c
>> index d9eea3dac5..942fa8da94 100644
>> --- a/drivers/misc/misc-uclass.c
>> +++ b/drivers/misc/misc-uclass.c
>> @@ -56,6 +56,26 @@ int misc_call(struct udevice *dev, int msgid, void *tx_msg, int tx_size,
>>         return ops->call(dev, msgid, tx_msg, tx_size, rx_msg, rx_size);
>>  }
>>
>> +int misc_enable(struct udevice *dev)
>> +{
>> +       const struct misc_ops *ops = device_get_ops(dev);
>> +
>> +       if (!ops->enable)
>> +               return -ENOSYS;
>> +
>> +       return ops->enable(dev);
>> +}
>> +
>> +int misc_disable(struct udevice *dev)
>> +{
>> +       const struct misc_ops *ops = device_get_ops(dev);
>> +
>> +       if (!ops->enable)
>> +               return -ENOSYS;
>> +
>> +       return ops->disable(dev);
>> +}
>> +
>>  UCLASS_DRIVER(misc) = {
>>         .id             = UCLASS_MISC,
>>         .name           = "misc",
>> diff --git a/include/misc.h b/include/misc.h
>> index 03ef55cdc8..6f86396443 100644
>> --- a/include/misc.h
>> +++ b/include/misc.h
>> @@ -58,6 +58,22 @@ int misc_ioctl(struct udevice *dev, unsigned long request, void *buf);
>>  int misc_call(struct udevice *dev, int msgid, void *tx_msg, int tx_size,
>>               void *rx_msg, int rx_size);
>>
>> +/*
>> + * Enable a device.
>> + *
>> + * @dev: the device to enable.
>> + * @return: 0 if OK, -ve on error
>> + */
>> +int misc_enable(struct udevice *dev);
>
> How about a single function like:
>
> int misc_set_enable(struct udevice *dev, bool enable);
>
> Also please state in the comment what exactly it means to
> enable/disable a device.
>

OK, v2 will have one function and more comments.

>> +
>> +/*
>> + * Disable a device.
>> + *
>> + * @dev: the device to disable.
>> + * @return: 0 if OK, -ve on error
>> + */
>> +int misc_disable(struct udevice *dev);
>> +
>>  /*
>>   * struct misc_ops - Driver model Misc operations
>>   *
>> @@ -109,6 +125,20 @@ struct misc_ops {
>>          */
>>         int (*call)(struct udevice *dev, int msgid, void *tx_msg, int tx_size,
>>                     void *rx_msg, int rx_size);
>> +       /*
>> +        * Enable a device, optional.
>> +        *
>> +        * @dev: the device to enable.
>> +        * @return: 0 if OK, -ve on error
>> +        */
>> +       int (*enable)(struct udevice *dev);
>> +       /*
>> +        * Disable a device, optional.
>> +        *
>> +        * @dev: the device to disable.
>> +        * @return: 0 if OK, -ve on error
>> +        */
>> +       int (*disable)(struct udevice *dev);
>>  };
>>
>>  #endif /* _MISC_H_ */
>> --
>> 2.16.1
>>
>
> Regards,
> Simon
>

Best regards,

Mario