| Message ID | 1399993327-12543-1-git-send-email-balbi@ti.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 13 May 2014, Felipe Balbi wrote: > drivers/reset/core.c does not provide an empty > stub for cases when CONFIG_RESET_CONTROLLER isn't > enabled, which might break build of the MSM PHY > driver if that driver is enabled but > CONFIG_RESET_CONTROLLER isn't. > > We make the driver depend on CONFIG_RESET_CONTROLLER > so we can never have such a case This may be a dumb comment... Would it be better to add the appropriate stub routines instead? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 13, 2014 at 11:13:58AM -0400, Alan Stern wrote: > On Tue, 13 May 2014, Felipe Balbi wrote: > > > drivers/reset/core.c does not provide an empty > > stub for cases when CONFIG_RESET_CONTROLLER isn't > > enabled, which might break build of the MSM PHY > > driver if that driver is enabled but > > CONFIG_RESET_CONTROLLER isn't. > > > > We make the driver depend on CONFIG_RESET_CONTROLLER > > so we can never have such a case > > This may be a dumb comment... Would it be better to add the > appropriate stub routines instead? maybe...
On Tuesday, May 13, 2014 8:30 AM, Felipe Balbi wrote: > > On Tue, May 13, 2014 at 11:13:58AM -0400, Alan Stern wrote: > > On Tue, 13 May 2014, Felipe Balbi wrote: > > > > > drivers/reset/core.c does not provide an empty > > > stub for cases when CONFIG_RESET_CONTROLLER isn't > > > enabled, which might break build of the MSM PHY > > > driver if that driver is enabled but > > > CONFIG_RESET_CONTROLLER isn't. > > > > > > We make the driver depend on CONFIG_RESET_CONTROLLER > > > so we can never have such a case > > > > This may be a dumb comment... Would it be better to add the > > appropriate stub routines instead? > > maybe... I don't think so. I haven't tested, but I believe the driver now relies on operational (not stub) resets. I think this comment is a bit off. I think it would be better to just say that the driver now depends on the reset sub-system, and thus needs the config dependency. Maybe there's a case where the driver works fine even without the resets, but if so it's not obvious to me. Out of curiosity - is this something that many sub-systems do: provide stubs for compilation, and support operating with or without real functionality. This seems kind of odd. I admit to being a bit sensitive about this, since I got bit by a situation where the driver relied on the firmware to get the hardware into a proper state. :-) -- Tim -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 13, 2014 at 05:49:06PM +0200, Bird, Tim wrote: > On Tuesday, May 13, 2014 8:30 AM, Felipe Balbi wrote: > > > > On Tue, May 13, 2014 at 11:13:58AM -0400, Alan Stern wrote: > > > On Tue, 13 May 2014, Felipe Balbi wrote: > > > > > > > drivers/reset/core.c does not provide an empty > > > > stub for cases when CONFIG_RESET_CONTROLLER isn't > > > > enabled, which might break build of the MSM PHY > > > > driver if that driver is enabled but > > > > CONFIG_RESET_CONTROLLER isn't. > > > > > > > > We make the driver depend on CONFIG_RESET_CONTROLLER > > > > so we can never have such a case > > > > > > This may be a dumb comment... Would it be better to add the > > > appropriate stub routines instead? > > > > maybe... > > I don't think so. I haven't tested, but I believe the driver now relies > on operational (not stub) resets. I think this comment is a bit off. > I think it would be better to just say that the driver now depends on the > reset sub-system, and thus needs the config dependency. > > Maybe there's a case where the driver works fine even without > the resets, but if so it's not obvious to me. > > Out of curiosity - is this something that many sub-systems do: > provide stubs for compilation, and support operating with or without > real functionality. This seems kind of odd. heh, stubs are only stubs... they're there so the driver can still build otherwise every module's dependency list would be gigantic. IOW, stubs are only for randconfig's sake.
On Tue, 13 May 2014, Bird, Tim wrote: > I don't think so. I haven't tested, but I believe the driver now relies > on operational (not stub) resets. I think this comment is a bit off. > I think it would be better to just say that the driver now depends on the > reset sub-system, and thus needs the config dependency. > > Maybe there's a case where the driver works fine even without > the resets, but if so it's not obvious to me. > > Out of curiosity - is this something that many sub-systems do: > provide stubs for compilation, and support operating with or without > real functionality. This seems kind of odd. Some subsystems do it when the functionality they provide is optional. Runtime PM is a good example. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2014-05-13 at 12:07 -0400, Alan Stern wrote: > On Tue, 13 May 2014, Bird, Tim wrote: > > > I don't think so. I haven't tested, but I believe the driver now relies > > on operational (not stub) resets. I think this comment is a bit off. > > I think it would be better to just say that the driver now depends on the > > reset sub-system, and thus needs the config dependency. > > > > Maybe there's a case where the driver works fine even without > > the resets, but if so it's not obvious to me. > > > > Out of curiosity - is this something that many sub-systems do: > > provide stubs for compilation, and support operating with or without > > real functionality. This seems kind of odd. > > Some subsystems do it when the functionality they provide is optional. > Runtime PM is a good example. If I could add. clk and regulator frameworks also provide stub API's, why reset framework should be different? Probably none of the drivers, which use clk or regulator, did not have dependency to these frameworks. I am pretty sure that these drivers will not work properly without proper back end implementation. Regards, Ivan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index fbbced8..ec531a4 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -173,6 +173,7 @@ config USB_ISP1301 config USB_MSM_OTG tristate "Qualcomm on-chip USB OTG controller support" depends on (USB || USB_GADGET) && (ARCH_MSM || ARCH_QCOM || COMPILE_TEST) + depends on RESET_CONTROLLER select USB_PHY help Enable this to support the USB OTG transceiver on Qualcomm chips. It
drivers/reset/core.c does not provide an empty stub for cases when CONFIG_RESET_CONTROLLER isn't enabled, which might break build of the MSM PHY driver if that driver is enabled but CONFIG_RESET_CONTROLLER isn't. We make the driver depend on CONFIG_RESET_CONTROLLER so we can never have such a case Cc: Ivan T. Ivanov <iivanov@mm-sol.com> Cc: Tim Bird <tim.bird@sonymobile.com> Reported-by: Jim Davis <jim.epost@gmail.com> Reported-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/usb/phy/Kconfig | 1 + 1 file changed, 1 insertion(+)