Hi Mario, On 4 May 2018 at 03:01, Mario Six <mario....@gdsys.cc> wrote: > Hi Simon, > > On Thu, May 3, 2018 at 9:02 PM, Simon Glass <s...@chromium.org> wrote: >> Hi Mario, >> >> On 27 April 2018 at 06:52, Mario Six <mario....@gdsys.cc> wrote: >>> Add generic enable/disable function to the misc uclass. >>> >>> Signed-off-by: Mario Six <mario....@gdsys.cc> >>> --- >>> >>> v1 -> v2: >>> * Merged the two functions into one function >>> * Explained the semantics of enabling/disabling more throughly >>> >>> --- >>> drivers/misc/misc-uclass.c | 10 ++++++++++ >>> include/misc.h | 25 +++++++++++++++++++++++++ >>> 2 files changed, 35 insertions(+) >>> >>> diff --git a/drivers/misc/misc-uclass.c b/drivers/misc/misc-uclass.c >>> index d9eea3dac5..0e3e0e8bf7 100644 >>> --- a/drivers/misc/misc-uclass.c >>> +++ b/drivers/misc/misc-uclass.c >>> @@ -56,6 +56,16 @@ 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_set_enabled(struct udevice *dev, bool val) >>> +{ >>> + const struct misc_ops *ops = device_get_ops(dev); >>> + >>> + if (!ops->set_enabled) >>> + return -ENOSYS; >>> + >>> + return ops->set_enabled(dev, val); >>> +} >>> + >>> UCLASS_DRIVER(misc) = { >>> .id = UCLASS_MISC, >>> .name = "misc", >>> diff --git a/include/misc.h b/include/misc.h >>> index 03ef55cdc8..04a4b65155 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 or disable a device. >>> + * >>> + * The semantics of "disable" and "enable" should be understood here as >>> + * activating or deactivating the device's primary function, hence a "disabled" >>> + * device should be dormant, but still answer to commands and queries. >>> + * >>> + * A probed device may start in a disabled or enabled state, depending on the >>> + * driver and hardware. >> >> This makes be wonder whether we should have this as a concept >> understood by the DM core. But perhaps not until we have more use >> cases. So far I know of only display that needs this. >> > > The concept seems useful in general, true. > >>> + * >>> + * @dev: the device to enable or disable. >>> + * @val: the flag that tells the driver to either enable or disable the device. >>> + * @return: 0 if OK, -ve on error >>> + */ >>> +int misc_set_enabled(struct udevice *dev, bool val); >>> + >>> /* >>> * struct misc_ops - Driver model Misc operations >>> * >>> @@ -109,6 +125,15 @@ struct misc_ops { >>> */ >>> int (*call)(struct udevice *dev, int msgid, void *tx_msg, int tx_size, >>> void *rx_msg, int rx_size); >>> + /* >>> + * Enable or disable a device, optional. >>> + * >>> + * @dev: the device to enable. >>> + * @val: the flag that tells the driver to either enable or disable the >>> + * device. >>> + * @return: 0 if OK, -ve on error >> >> How about returning the old state (0 or 1)? >> > > Good idea. Wouldn't it be good to have a distinct output parameter (e.g. bool > *old_state) though, so that we can still check whether the execution failed for > some reason?
I think just returning an int would work: < 0 : error 0 success, old value was false 1 success, old value was try Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot