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? >> + */ >> + int (*set_enabled)(struct udevice *dev, bool val); >> }; >> >> #endif /* _MISC_H_ */ >> -- >> 2.16.1 >> > > Regards, > Simon > Best regards, Mario _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot