Hi, On Fri, Jan 10, 2014 at 9:30 PM, Philipp Zabel <p.za...@pengutronix.de> wrote: > Hi, > > [Added Ivan, Stephen and Barry to Cc:] > > Am Freitag, den 10.01.2014, 15:00 +0800 schrieb Chen-Yu Tsai: >> Some drivers are shared between platforms that may or may not >> have RESET_CONTROLLER selected for them. > > I expected that drivers compiled for platforms without reset controllers > but use the reset API should select or depend on RESET_CONTROLLER. > Stubbing out device_reset and reset_control_get will turn a compile time > error into a runtime error for everyone forgetting to do this when > writing a new driver that needs the reset.
Since this was the intended behavior, I'll drop this patch and select RESET_CONTROLLER for the stmmac driver for now. Thanks ChenYu > >> Add dummy functions >> when RESET_CONTROLLER is not selected, thereby eliminating the >> need for drivers to enclose reset_control_*() calls within >> #ifdef CONFIG_RESET_CONTROLLER, #endif >> >> Signed-off-by: Chen-Yu Tsai <w...@csie.org> > > This was already proposed by Ivan and Barry earlier, and so far we > didn't get to a proper conclusion: > > https://lkml.org/lkml/2013/10/10/179 > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/174758.html > > If included, the stubs should at least return an error to indicate a > reset couldn't be performed, but then I lose the compile time error for > drivers which should select RESET_CONTROLLER but don't. > > Also this alone won't help you if you build multi-arch kernels where one > platform enables RESET_CONTROLLER and the other expects it to be > disabled. > > regards > Philipp > >> --- >> include/linux/reset.h | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/include/linux/reset.h b/include/linux/reset.h >> index 6082247..38aa616 100644 >> --- a/include/linux/reset.h >> +++ b/include/linux/reset.h >> @@ -4,6 +4,8 @@ >> struct device; >> struct reset_control; >> >> +#ifdef CONFIG_RESET_CONTROLLER >> + >> int reset_control_reset(struct reset_control *rstc); >> int reset_control_assert(struct reset_control *rstc); >> int reset_control_deassert(struct reset_control *rstc); >> @@ -14,4 +16,41 @@ struct reset_control *devm_reset_control_get(struct >> device *dev, const char *id) >> >> int device_reset(struct device *dev); >> >> +#else /* !CONFIG_RESET_CONTROLLER */ >> + >> +static inline int reset_control_reset(struct reset_control *rstc) >> +{ >> + return 0; >> +} >> + >> +static inline int reset_control_assert(struct reset_control *rstc) >> +{ >> + return 0; >> +} >> + >> +static inline int reset_control_deassert(struct reset_control *rstc) >> +{ >> + return 0; >> +} > > Those should probably have a WARN_ON(1) like the GPIO API stubs. > >> + >> +static inline struct reset_control *reset_control_get(struct device *dev, >> + const char *id) >> +{ >> + return NULL; >> +} > [...] >> +static inline struct reset_control *devm_reset_control_get(struct device >> *dev, >> + const char *id) >> +{ >> + return NULL; >> +} > > These should return ERR_PTR(-ENOSYS). > >> + >> +static inline int device_reset(struct device *dev) >> +{ >> + return 0; >> +} > > And this should return -ENOSYS. > > Drivers that also need to run on platforms with CONFIG_RESET_CONTROLLER > disabled (and that don't need the reset) should ignore -ENOSYS and > -ENOENT return values from device_reset/(devm_)reset_control_get. > > I wonder if it might be a good idea to add a RESET_CONTROLLER_OPTIONAL > that drivers need to select to enable the API stubs? That way we could > keep the compile time error for new drivers that need resets but forget > to select RESET_CONTROLLER. > Or add a > #warning If this driver can work without reset, please select > CONFIG_RESET_CONTROLLER_OPTIONAL > > Another possibility would be to add device_reset_optional and > (devm_)reset_control_get_optional variants and only provide stubs for > those, but not for device_reset/(devm_)reset_control_get. Then drivers > that need to work on platforms without the reset controller API enabled > could explicitly use the _optional variants, and all other drivers would > still be checked at compile time. > > regards > Philipp > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
