* [PATCH 1/3] watchdog: Select CONFIG_PARAMETER @ 2020-01-21 11:44 Christian Eggers 2020-01-21 11:44 ` [PATCH 2/3] usb: otg: " Christian Eggers ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Christian Eggers @ 2020-01-21 11:44 UTC (permalink / raw) To: barebox; +Cc: Christian Eggers, ceggers Without CONFIG_PARAMETER, watchdog_register() will always fail. Signed-off-by: Christian Eggers <ceggers@arri.de> --- drivers/watchdog/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 45dd41a2a..34b7fea39 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -4,6 +4,7 @@ config WATCHDOG_IMX_RESET_SOURCE menuconfig WATCHDOG bool "Watchdog support" + select PARAMETER help Many platforms support a watchdog to keep track of a working machine. This framework provides routines to handle these watchdogs. -- Christian Eggers Embedded software developer Arnold & Richter Cine Technik GmbH & Co. Betriebs KG Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918 Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477 Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] usb: otg: Select CONFIG_PARAMETER 2020-01-21 11:44 [PATCH 1/3] watchdog: Select CONFIG_PARAMETER Christian Eggers @ 2020-01-21 11:44 ` Christian Eggers 2020-01-21 11:44 ` [PATCH 3/3] globalvar: " Christian Eggers 2020-01-22 8:21 ` [PATCH 1/3] watchdog: " Sascha Hauer 2 siblings, 0 replies; 7+ messages in thread From: Christian Eggers @ 2020-01-21 11:44 UTC (permalink / raw) To: barebox; +Cc: Christian Eggers, ceggers Without CONFIG_PARAMETER, usb_register_otg_device() will always fail. Signed-off-by: Christian Eggers <ceggers@arri.de> --- drivers/usb/otg/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig index 2c094452b..20b4d7f8c 100644 --- a/drivers/usb/otg/Kconfig +++ b/drivers/usb/otg/Kconfig @@ -9,4 +9,5 @@ config USB_TWL4030 config USB_OTGDEV bool + select PARAMETER -- Christian Eggers Embedded software developer Arnold & Richter Cine Technik GmbH & Co. Betriebs KG Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918 Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477 Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] globalvar: Select CONFIG_PARAMETER 2020-01-21 11:44 [PATCH 1/3] watchdog: Select CONFIG_PARAMETER Christian Eggers 2020-01-21 11:44 ` [PATCH 2/3] usb: otg: " Christian Eggers @ 2020-01-21 11:44 ` Christian Eggers 2020-01-22 8:21 ` [PATCH 1/3] watchdog: " Sascha Hauer 2 siblings, 0 replies; 7+ messages in thread From: Christian Eggers @ 2020-01-21 11:44 UTC (permalink / raw) To: barebox; +Cc: Christian Eggers, ceggers Without CONFIG_PARAMETER, __nvar_add() will always fail. Signed-off-by: Christian Eggers <ceggers@arri.de> --- common/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/common/Kconfig b/common/Kconfig index 60237d305..c2ec995f1 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -166,6 +166,7 @@ config GLOBALVAR bool "global environment variables support" default y if !SHELL_NONE select FNMATCH + select PARAMETER help Global environment variables begin with "global.". Unlike normal shell variables they have the same values in all contexts. Global -- Christian Eggers Embedded software developer Arnold & Richter Cine Technik GmbH & Co. Betriebs KG Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918 Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477 Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] watchdog: Select CONFIG_PARAMETER 2020-01-21 11:44 [PATCH 1/3] watchdog: Select CONFIG_PARAMETER Christian Eggers 2020-01-21 11:44 ` [PATCH 2/3] usb: otg: " Christian Eggers 2020-01-21 11:44 ` [PATCH 3/3] globalvar: " Christian Eggers @ 2020-01-22 8:21 ` Sascha Hauer 2020-01-22 9:39 ` Christian Eggers 2 siblings, 1 reply; 7+ messages in thread From: Sascha Hauer @ 2020-01-22 8:21 UTC (permalink / raw) To: Christian Eggers; +Cc: barebox, ceggers Hi Christian, On Tue, Jan 21, 2020 at 12:44:19PM +0100, Christian Eggers wrote: > Without CONFIG_PARAMETER, watchdog_register() will always fail. > > Signed-off-by: Christian Eggers <ceggers@arri.de> > --- > drivers/watchdog/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 45dd41a2a..34b7fea39 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -4,6 +4,7 @@ config WATCHDOG_IMX_RESET_SOURCE > > menuconfig WATCHDOG > bool "Watchdog support" > + select PARAMETER I think this goes into the wrong direction. With CONFIG_PARAMETER enabled we get support for adjusting device parameters from the shell. In environments without shell support parameter support is not needed. For example the watchdog C API doesn't need parameter support and is still usable. The static inline wrappers for dev_add_param_* should return NULL instead of returning ERR_PTR(-ENOSYS). Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] watchdog: Select CONFIG_PARAMETER 2020-01-22 8:21 ` [PATCH 1/3] watchdog: " Sascha Hauer @ 2020-01-22 9:39 ` Christian Eggers 2020-01-22 19:54 ` Sascha Hauer 0 siblings, 1 reply; 7+ messages in thread From: Christian Eggers @ 2020-01-22 9:39 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox, ceggers Hi Sascha, Am Mittwoch, 22. Januar 2020, 09:21:15 CET schrieb Sascha Hauer: > Hi Christian, > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > index 45dd41a2a..34b7fea39 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -4,6 +4,7 @@ config WATCHDOG_IMX_RESET_SOURCE > > > > menuconfig WATCHDOG > > > > bool "Watchdog support" > > > > + select PARAMETER > > I think this goes into the wrong direction. With CONFIG_PARAMETER > enabled we get support for adjusting device parameters from the shell. > In environments without shell support parameter support is not needed. > For example the watchdog C API doesn't need parameter support and is > still usable. > > The static inline wrappers for dev_add_param_* should return NULL > instead of returning ERR_PTR(-ENOSYS). initially I came to the same result. But previous commits to param.h went in the opposite direction: > 03b59bdb64 ("paramter: The dev_add_param_*() return ERR_PTR(), change > no-ops") to return ERR_PTR(-ENOSYS) instead of NULL > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> and > c5d95eb4c7 ("param: make parameter functions more consistent") > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Most of the callers of dev_add_param*() don't care about the returned param pointer at all. Some are checking against PTR_ERR() which would not be hit if returning NULL (this is what we want). A few callers have to changed if a NULL pointer can be returned: - __nvvar_add() - state_string_create() stores the result in state_string::param, but seems to be used nowhere - mci_register() dito for mci::param_probe - state_uint8_create() dito for state_uint32::param - state_uint32_create() dito For me it looks reasonable to return a NULL pointer if CONFIG_PARAMETER is not set (as you suggested). Only __nvvar_add() needs slight changes and I would remove needless storage of param in structs state_string, mci and state_uint32. Shall I start? regards Christian _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] watchdog: Select CONFIG_PARAMETER 2020-01-22 9:39 ` Christian Eggers @ 2020-01-22 19:54 ` Sascha Hauer 2020-01-23 11:18 ` Marc Kleine-Budde 0 siblings, 1 reply; 7+ messages in thread From: Sascha Hauer @ 2020-01-22 19:54 UTC (permalink / raw) To: Christian Eggers; +Cc: barebox, ceggers On Wed, Jan 22, 2020 at 10:39:07AM +0100, Christian Eggers wrote: > Hi Sascha, > > Am Mittwoch, 22. Januar 2020, 09:21:15 CET schrieb Sascha Hauer: > > Hi Christian, > > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > > index 45dd41a2a..34b7fea39 100644 > > > --- a/drivers/watchdog/Kconfig > > > +++ b/drivers/watchdog/Kconfig > > > @@ -4,6 +4,7 @@ config WATCHDOG_IMX_RESET_SOURCE > > > > > > menuconfig WATCHDOG > > > > > > bool "Watchdog support" > > > > > > + select PARAMETER > > > > I think this goes into the wrong direction. With CONFIG_PARAMETER > > enabled we get support for adjusting device parameters from the shell. > > In environments without shell support parameter support is not needed. > > For example the watchdog C API doesn't need parameter support and is > > still usable. > > > > The static inline wrappers for dev_add_param_* should return NULL > > instead of returning ERR_PTR(-ENOSYS). > > initially I came to the same result. But previous commits to param.h went in > the opposite direction: > > > 03b59bdb64 ("paramter: The dev_add_param_*() return ERR_PTR(), change > > no-ops") to return ERR_PTR(-ENOSYS) instead of NULL Shouldn't have merged this one as it lacks an explanation why this has been done. Marc, do you have an idea what the motivation for this patch was? > > > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > and > > > c5d95eb4c7 ("param: make parameter functions more consistent") > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > Most of the callers of dev_add_param*() don't care about the returned param > pointer at all. Some are checking against PTR_ERR() which would not be hit if > returning NULL (this is what we want). > > A few callers have to changed if a NULL pointer can be returned: > - __nvvar_add() nvvar doesn't really make sense without CONFIG_PARAMETER enabled. There currently is no dependency between these options, but probably there should be. > - state_string_create() stores the result in state_string::param, but seems > to be used nowhere > - mci_register() dito for mci::param_probe > - state_uint8_create() dito for state_uint32::param > - state_uint32_create() dito > > For me it looks reasonable to return a NULL pointer if CONFIG_PARAMETER is not > set (as you suggested). Only __nvvar_add() needs slight changes and I would > remove needless storage of param in structs state_string, mci and > state_uint32. > > Shall I start? Let's wait for Marc if he has an idea why we introduced 03b59bdb64, but otherwise yes, this sounds like a good plan. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] watchdog: Select CONFIG_PARAMETER 2020-01-22 19:54 ` Sascha Hauer @ 2020-01-23 11:18 ` Marc Kleine-Budde 0 siblings, 0 replies; 7+ messages in thread From: Marc Kleine-Budde @ 2020-01-23 11:18 UTC (permalink / raw) To: Sascha Hauer, Christian Eggers; +Cc: barebox, ceggers [-- Attachment #1.1.1: Type: text/plain, Size: 1731 bytes --] On 1/22/20 8:54 PM, Sascha Hauer wrote: > On Wed, Jan 22, 2020 at 10:39:07AM +0100, Christian Eggers wrote: >> Hi Sascha, >> >> Am Mittwoch, 22. Januar 2020, 09:21:15 CET schrieb Sascha Hauer: >>> Hi Christian, >>> >>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>>> index 45dd41a2a..34b7fea39 100644 >>>> --- a/drivers/watchdog/Kconfig >>>> +++ b/drivers/watchdog/Kconfig >>>> @@ -4,6 +4,7 @@ config WATCHDOG_IMX_RESET_SOURCE >>>> >>>> menuconfig WATCHDOG >>>> >>>> bool "Watchdog support" >>>> >>>> + select PARAMETER >>> >>> I think this goes into the wrong direction. With CONFIG_PARAMETER >>> enabled we get support for adjusting device parameters from the shell. >>> In environments without shell support parameter support is not needed. >>> For example the watchdog C API doesn't need parameter support and is >>> still usable. >>> >>> The static inline wrappers for dev_add_param_* should return NULL >>> instead of returning ERR_PTR(-ENOSYS). >> >> initially I came to the same result. But previous commits to param.h went in >> the opposite direction: >> >>> 03b59bdb64 ("paramter: The dev_add_param_*() return ERR_PTR(), change >>> no-ops") to return ERR_PTR(-ENOSYS) instead of NULL > > Shouldn't have merged this one as it lacks an explanation why this has > been done. Marc, do you have an idea what the motivation for this patch > was? Sorry, I don't remember.... Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 149 bytes --] _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-23 11:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-21 11:44 [PATCH 1/3] watchdog: Select CONFIG_PARAMETER Christian Eggers 2020-01-21 11:44 ` [PATCH 2/3] usb: otg: " Christian Eggers 2020-01-21 11:44 ` [PATCH 3/3] globalvar: " Christian Eggers 2020-01-22 8:21 ` [PATCH 1/3] watchdog: " Sascha Hauer 2020-01-22 9:39 ` Christian Eggers 2020-01-22 19:54 ` Sascha Hauer 2020-01-23 11:18 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox