| Message ID | 1403761178-5371-1-git-send-email-sachin.kamat@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/26/2014 11:09 AM, Sachin Kamat wrote: > EHCI and OHCI drivers on Exynos platforms do not work without their > corresponding SoC specific phy drivers. Hence it makes no sense to > keep these phy drivers as user selectable. Instead select them from > the respective USB configs to make things easier for the end user. > While at it enable 5250 phy for Exynos 5420 SoC too. > > Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> > Cc: Kishon Vijay Abraham I <kishon@ti.com> > --- Reviewed-by: Tushar Behera <tushar.b@samsung.com> > drivers/phy/Kconfig | 37 +++++++------------------------------ > drivers/usb/host/Kconfig | 10 ++++++---- > 2 files changed, 13 insertions(+), 34 deletions(-) > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 16a2f067c242..7fe7ef5f1322 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -121,44 +121,21 @@ config PHY_SUN4I_USB > parts, as well as the 2 regular USB 2 host PHYs. > > config PHY_SAMSUNG_USB2 > - tristate "Samsung USB 2.0 PHY driver" > + tristate > select GENERIC_PHY > select MFD_SYSCON > - help > - Enable this to support the Samsung USB 2.0 PHY driver for Samsung > - SoCs. This driver provides the interface for USB 2.0 PHY. Support for > - particular SoCs has to be enabled in addition to this driver. Number > - and type of supported phys depends on the SoC. > + select PHY_EXYNOS4210_USB2 if CPU_EXYNOS4210 > + select PHY_EXYNOS4X12_USB2 if (SOC_EXYNOS4212 || SOC_EXYNOS4412) > + select PHY_EXYNOS5250_USB2 if (SOC_EXYNOS5250 || SOC_EXYNOS5420) > > config PHY_EXYNOS4210_USB2 > - bool "Support for Exynos 4210" > - depends on PHY_SAMSUNG_USB2 > - depends on CPU_EXYNOS4210 > - help > - Enable USB PHY support for Exynos 4210. This option requires that > - Samsung USB 2.0 PHY driver is enabled and means that support for this > - particular SoC is compiled in the driver. In case of Exynos 4210 four > - phys are available - device, host, HSIC0 and HSIC1. > + bool > > config PHY_EXYNOS4X12_USB2 > - bool "Support for Exynos 4x12" > - depends on PHY_SAMSUNG_USB2 > - depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) > - help > - Enable USB PHY support for Exynos 4x12. This option requires that > - Samsung USB 2.0 PHY driver is enabled and means that support for this > - particular SoC is compiled in the driver. In case of Exynos 4x12 four > - phys are available - device, host, HSIC0 and HSIC1. > + bool > > config PHY_EXYNOS5250_USB2 > - bool "Support for Exynos 5250" > - depends on PHY_SAMSUNG_USB2 > - depends on SOC_EXYNOS5250 > - help > - Enable USB PHY support for Exynos 5250. This option requires that > - Samsung USB 2.0 PHY driver is enabled and means that support for this > - particular SoC is compiled in the driver. In case of Exynos 5250 four > - phys are available - device, host, HSIC0 and HSIC. > + bool > > config PHY_EXYNOS5_USBDRD > tristate "Exynos5 SoC series USB DRD PHY driver" > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 61b7817bd66b..2938807331de 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -211,10 +211,11 @@ config USB_EHCI_SH > If you use the PCI EHCI controller, this option is not necessary. > > config USB_EHCI_EXYNOS > - tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" > - depends on PLAT_S5P || ARCH_EXYNOS > - help > - Enable support for the Samsung Exynos SOC's on-chip EHCI controller. > + tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" > + depends on PLAT_S5P || ARCH_EXYNOS > + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS > + help > + Enable support for the Samsung Exynos SOC's on-chip EHCI controller. > > config USB_EHCI_MV > bool "EHCI support for Marvell PXA/MMP USB controller" > @@ -520,6 +521,7 @@ config USB_OHCI_SH > config USB_OHCI_EXYNOS > tristate "OHCI support for Samsung S5P/EXYNOS SoC Series" > depends on PLAT_S5P || ARCH_EXYNOS > + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS > help > Enable support for the Samsung Exynos SOC's on-chip OHCI controller. > >
Hi Sachin, On Thu, Jun 26, 2014 at 11:09 AM, Sachin Kamat <sachin.kamat@samsung.com> wrote: > EHCI and OHCI drivers on Exynos platforms do not work without their > corresponding SoC specific phy drivers. Hence it makes no sense to > keep these phy drivers as user selectable. Instead select them from > the respective USB configs to make things easier for the end user. > While at it enable 5250 phy for Exynos 5420 SoC too. > > Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> > Cc: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/phy/Kconfig | 37 +++++++------------------------------ > drivers/usb/host/Kconfig | 10 ++++++---- > 2 files changed, 13 insertions(+), 34 deletions(-) > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 16a2f067c242..7fe7ef5f1322 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -121,44 +121,21 @@ config PHY_SUN4I_USB > parts, as well as the 2 regular USB 2 host PHYs. > > config PHY_SAMSUNG_USB2 > - tristate "Samsung USB 2.0 PHY driver" > + tristate > select GENERIC_PHY > select MFD_SYSCON > - help > - Enable this to support the Samsung USB 2.0 PHY driver for Samsung > - SoCs. This driver provides the interface for USB 2.0 PHY. Support for > - particular SoCs has to be enabled in addition to this driver. Number > - and type of supported phys depends on the SoC. > + select PHY_EXYNOS4210_USB2 if CPU_EXYNOS4210 > + select PHY_EXYNOS4X12_USB2 if (SOC_EXYNOS4212 || SOC_EXYNOS4412) > + select PHY_EXYNOS5250_USB2 if (SOC_EXYNOS5250 || SOC_EXYNOS5420) also || SOC_EXYNOS5800 same controller present on Exynos5800 too. > > config PHY_EXYNOS4210_USB2 > - bool "Support for Exynos 4210" > - depends on PHY_SAMSUNG_USB2 > - depends on CPU_EXYNOS4210 > - help > - Enable USB PHY support for Exynos 4210. This option requires that > - Samsung USB 2.0 PHY driver is enabled and means that support for this > - particular SoC is compiled in the driver. In case of Exynos 4210 four > - phys are available - device, host, HSIC0 and HSIC1. > + bool > > config PHY_EXYNOS4X12_USB2 > - bool "Support for Exynos 4x12" > - depends on PHY_SAMSUNG_USB2 > - depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) > - help > - Enable USB PHY support for Exynos 4x12. This option requires that > - Samsung USB 2.0 PHY driver is enabled and means that support for this > - particular SoC is compiled in the driver. In case of Exynos 4x12 four > - phys are available - device, host, HSIC0 and HSIC1. > + bool > > config PHY_EXYNOS5250_USB2 > - bool "Support for Exynos 5250" > - depends on PHY_SAMSUNG_USB2 > - depends on SOC_EXYNOS5250 > - help > - Enable USB PHY support for Exynos 5250. This option requires that > - Samsung USB 2.0 PHY driver is enabled and means that support for this > - particular SoC is compiled in the driver. In case of Exynos 5250 four > - phys are available - device, host, HSIC0 and HSIC. > + bool > > config PHY_EXYNOS5_USBDRD > tristate "Exynos5 SoC series USB DRD PHY driver" > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 61b7817bd66b..2938807331de 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -211,10 +211,11 @@ config USB_EHCI_SH > If you use the PCI EHCI controller, this option is not necessary. > > config USB_EHCI_EXYNOS > - tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" > - depends on PLAT_S5P || ARCH_EXYNOS > - help > - Enable support for the Samsung Exynos SOC's on-chip EHCI controller. > + tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" > + depends on PLAT_S5P || ARCH_EXYNOS > + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS > + help > + Enable support for the Samsung Exynos SOC's on-chip EHCI controller. > > config USB_EHCI_MV > bool "EHCI support for Marvell PXA/MMP USB controller" > @@ -520,6 +521,7 @@ config USB_OHCI_SH > config USB_OHCI_EXYNOS > tristate "OHCI support for Samsung S5P/EXYNOS SoC Series" > depends on PLAT_S5P || ARCH_EXYNOS > + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS > help > Enable support for the Samsung Exynos SOC's on-chip OHCI controller. > rest looks fine.
Hi Vivek, On Thu, Jun 26, 2014 at 1:39 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote: > Hi Sachin, > > > On Thu, Jun 26, 2014 at 11:09 AM, Sachin Kamat <sachin.kamat@samsung.com> wrote: >> EHCI and OHCI drivers on Exynos platforms do not work without their >> corresponding SoC specific phy drivers. Hence it makes no sense to >> keep these phy drivers as user selectable. Instead select them from >> the respective USB configs to make things easier for the end user. >> While at it enable 5250 phy for Exynos 5420 SoC too. <snip> >> + select PHY_EXYNOS4210_USB2 if CPU_EXYNOS4210 >> + select PHY_EXYNOS4X12_USB2 if (SOC_EXYNOS4212 || SOC_EXYNOS4412) >> + select PHY_EXYNOS5250_USB2 if (SOC_EXYNOS5250 || SOC_EXYNOS5420) > > also || SOC_EXYNOS5800 > same controller present on Exynos5800 too. Thanks. Will add. <snip> >> @@ -520,6 +521,7 @@ config USB_OHCI_SH >> config USB_OHCI_EXYNOS >> tristate "OHCI support for Samsung S5P/EXYNOS SoC Series" >> depends on PLAT_S5P || ARCH_EXYNOS >> + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS >> help >> Enable support for the Samsung Exynos SOC's on-chip OHCI controller. >> > > rest looks fine. Thanks. Sachin. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sachin, On Thursday 26 June 2014 11:09 AM, Sachin Kamat wrote: > EHCI and OHCI drivers on Exynos platforms do not work without their > corresponding SoC specific phy drivers. Hence it makes no sense to > keep these phy drivers as user selectable. Instead select them from > the respective USB configs to make things easier for the end user. > While at it enable 5250 phy for Exynos 5420 SoC too. > > Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> > Cc: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/phy/Kconfig | 37 +++++++------------------------------ > drivers/usb/host/Kconfig | 10 ++++++---- > 2 files changed, 13 insertions(+), 34 deletions(-) > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 16a2f067c242..7fe7ef5f1322 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig <snip> > > config USB_EHCI_EXYNOS > - tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" > - depends on PLAT_S5P || ARCH_EXYNOS > - help > - Enable support for the Samsung Exynos SOC's on-chip EHCI controller. > + tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" > + depends on PLAT_S5P || ARCH_EXYNOS > + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS I am skeptical to add select after the problems it created during previous releases. Maybe use 'default y if ARCH_EXYNOS' in PHY_SAMSUNG_USB2 Kconfig? Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vivek, On Thu, Jun 26, 2014 at 1:55 PM, Sachin Kamat <sachin.kamat@samsung.com> wrote: > Hi Vivek, > > On Thu, Jun 26, 2014 at 1:39 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote: >> Hi Sachin, >> >> >> On Thu, Jun 26, 2014 at 11:09 AM, Sachin Kamat <sachin.kamat@samsung.com> wrote: >>> EHCI and OHCI drivers on Exynos platforms do not work without their >>> corresponding SoC specific phy drivers. Hence it makes no sense to >>> keep these phy drivers as user selectable. Instead select them from >>> the respective USB configs to make things easier for the end user. >>> While at it enable 5250 phy for Exynos 5420 SoC too. > > <snip> > >>> + select PHY_EXYNOS4210_USB2 if CPU_EXYNOS4210 >>> + select PHY_EXYNOS4X12_USB2 if (SOC_EXYNOS4212 || SOC_EXYNOS4412) >>> + select PHY_EXYNOS5250_USB2 if (SOC_EXYNOS5250 || SOC_EXYNOS5420) >> >> also || SOC_EXYNOS5800 >> same controller present on Exynos5800 too. Just checked that SOC_EXYNOS5800 is dependent on SOC_EXYNOS5420. Hence explicit option for SOC_EXYNOS5800 is redundant here. Will keep it as is. Regards, Sachin. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kishon, On Thu, Jun 26, 2014 at 2:19 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > Hi Sachin, > > On Thursday 26 June 2014 11:09 AM, Sachin Kamat wrote: >> EHCI and OHCI drivers on Exynos platforms do not work without their >> corresponding SoC specific phy drivers. Hence it makes no sense to >> keep these phy drivers as user selectable. Instead select them from >> the respective USB configs to make things easier for the end user. >> While at it enable 5250 phy for Exynos 5420 SoC too. >> >> Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> >> Cc: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> drivers/phy/Kconfig | 37 +++++++------------------------------ >> drivers/usb/host/Kconfig | 10 ++++++---- >> 2 files changed, 13 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index 16a2f067c242..7fe7ef5f1322 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig > > <snip> > >> >> config USB_EHCI_EXYNOS >> - tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" >> - depends on PLAT_S5P || ARCH_EXYNOS >> - help >> - Enable support for the Samsung Exynos SOC's on-chip EHCI controller. >> + tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" >> + depends on PLAT_S5P || ARCH_EXYNOS >> + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS > > I am skeptical to add select after the problems it created during previous > releases. If you could point me to some link/discussion about this, I could check if such problem is applicable here. IMHO, there shouldn't be any in this case. > Maybe use 'default y if ARCH_EXYNOS' in PHY_SAMSUNG_USB2 Kconfig? I would prefer to use this option only if there is a strong objection to doing it the current way as I do not see any benefits of exposing the PHY entries that are only prerequisites for the USB functionality (in this case) to users. Regards, Sachin. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sachin, On Thu, Jun 26, 2014 at 2:23 PM, Sachin Kamat <sachin.kamat@samsung.com> wrote: > Hi Vivek, > > On Thu, Jun 26, 2014 at 1:55 PM, Sachin Kamat <sachin.kamat@samsung.com> wrote: >> Hi Vivek, >> >> On Thu, Jun 26, 2014 at 1:39 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote: >>> Hi Sachin, >>> >>> >>> On Thu, Jun 26, 2014 at 11:09 AM, Sachin Kamat <sachin.kamat@samsung.com> wrote: >>>> EHCI and OHCI drivers on Exynos platforms do not work without their >>>> corresponding SoC specific phy drivers. Hence it makes no sense to >>>> keep these phy drivers as user selectable. Instead select them from >>>> the respective USB configs to make things easier for the end user. >>>> While at it enable 5250 phy for Exynos 5420 SoC too. >> >> <snip> >> >>>> + select PHY_EXYNOS4210_USB2 if CPU_EXYNOS4210 >>>> + select PHY_EXYNOS4X12_USB2 if (SOC_EXYNOS4212 || SOC_EXYNOS4412) >>>> + select PHY_EXYNOS5250_USB2 if (SOC_EXYNOS5250 || SOC_EXYNOS5420) >>> >>> also || SOC_EXYNOS5800 >>> same controller present on Exynos5800 too. > > Just checked that SOC_EXYNOS5800 is dependent on SOC_EXYNOS5420. Hence > explicit option for SOC_EXYNOS5800 is redundant here. Will keep it as is. Aah ! right, it's fine then. Sorry for the noise.
On Thu, Jun 26, 2014 at 11:09:37AM +0530, Sachin Kamat wrote: > EHCI and OHCI drivers on Exynos platforms do not work without their > corresponding SoC specific phy drivers. Hence it makes no sense to > keep these phy drivers as user selectable. Instead select them from > the respective USB configs to make things easier for the end user. > While at it enable 5250 phy for Exynos 5420 SoC too. > > Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> > Cc: Kishon Vijay Abraham I <kishon@ti.com> no more selects, please. We've already had way too many issues because of misused selects.
Felipe, On Fri, Jun 27, 2014 at 8:46 AM, Felipe Balbi <balbi@ti.com> wrote: > On Thu, Jun 26, 2014 at 11:09:37AM +0530, Sachin Kamat wrote: >> EHCI and OHCI drivers on Exynos platforms do not work without their >> corresponding SoC specific phy drivers. Hence it makes no sense to >> keep these phy drivers as user selectable. Instead select them from >> the respective USB configs to make things easier for the end user. >> While at it enable 5250 phy for Exynos 5420 SoC too. >> >> Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> >> Cc: Kishon Vijay Abraham I <kishon@ti.com> > > no more selects, please. We've already had way too many issues because > of misused selects. I'll admit to not having been involved with the previous discussions, but this seems strange to me. Are we throwing in the towel and deciding that it's too hard to get the Kconfigs right and that we'll just rely on individual users to figure out the right answer for themselves? Certainly the Exynos USB driver is not useful without the Exynos USB Phy driver, so it seems awfully strange to allow the user to select one without getting the other... ...and if including an extra USB Phy driver will break something then it seems like we have bigger problems (aren't we supposed to have one kernel that works across a wide variety of boards?) -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 27, 2014 at 08:55:31AM -0700, Doug Anderson wrote: > Felipe, > > On Fri, Jun 27, 2014 at 8:46 AM, Felipe Balbi <balbi@ti.com> wrote: > > On Thu, Jun 26, 2014 at 11:09:37AM +0530, Sachin Kamat wrote: > >> EHCI and OHCI drivers on Exynos platforms do not work without their > >> corresponding SoC specific phy drivers. Hence it makes no sense to > >> keep these phy drivers as user selectable. Instead select them from > >> the respective USB configs to make things easier for the end user. > >> While at it enable 5250 phy for Exynos 5420 SoC too. > >> > >> Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> > >> Cc: Kishon Vijay Abraham I <kishon@ti.com> > > > > no more selects, please. We've already had way too many issues because > > of misused selects. > > I'll admit to not having been involved with the previous discussions, > but this seems strange to me. Are we throwing in the towel and > deciding that it's too hard to get the Kconfigs right and that we'll > just rely on individual users to figure out the right answer for > themselves? no. select prevents a driver from be built as a dynamically linked module and distro-kernels might want to enable everything as modules. > Certainly the Exynos USB driver is not useful without the Exynos USB > Phy driver, so it seems awfully strange to allow the user to select > one without getting the other... ...and if including an extra USB Phy > driver will break something then it seems like we have bigger problems > (aren't we supposed to have one kernel that works across a wide > variety of boards?) yeah, but for the kernel to "work" it doesn't depend on the PHY driver, does it ? The USB parts of the SoC depend on the PHY, but even PHY drivers should be allowed to be built as modules. Find another way
Felipe, On Fri, Jun 27, 2014 at 8:59 AM, Felipe Balbi <balbi@ti.com> wrote: >> I'll admit to not having been involved with the previous discussions, >> but this seems strange to me. Are we throwing in the towel and >> deciding that it's too hard to get the Kconfigs right and that we'll >> just rely on individual users to figure out the right answer for >> themselves? > > no. select prevents a driver from be built as a dynamically linked > module and distro-kernels might want to enable everything as modules. Ah, that's what the problem was! I wasn't aware of this issue with SELECT. Sorry for the noob-ness. Really we want the PHY to be "=y" if the USB driver is "=y" or "=m" if the USB driver is "=m", I think. You could argue that one might want to build the main USB driver into the kernel but have the phy drivers as modules, so you could possibly also try to support that... If there's not a good way to specify that, I guess we'll just have to use "default" and rely on the user not to purposely choose the wrong thing. Like the following (untested): config PHY_SAMSUNG_USB2 depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS default y if ARCH_EXYNOS=y && (USB_OHCI_EXYNOS=y || USB_EHCI_EXYNOS=y) default m if ARCH_EXYNOS=m && (USB_OHCI_EXYNOS=m || USB_EHCI_EXYNOS=m) ... I see some syntax like that elsewhere in Kconfig so I assume it's reasonable... With the above the user could purposely enable the OHCI or EHCI driver and disable the PHY driver which is not really sensible. ...but it wouldn't cause a compile failure or crash--USB just won't work. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 27, 2014 at 9:32 AM, Doug Anderson <dianders@chromium.org> wrote: > Felipe, > > On Fri, Jun 27, 2014 at 8:59 AM, Felipe Balbi <balbi@ti.com> wrote: >>> I'll admit to not having been involved with the previous discussions, >>> but this seems strange to me. Are we throwing in the towel and >>> deciding that it's too hard to get the Kconfigs right and that we'll >>> just rely on individual users to figure out the right answer for >>> themselves? >> >> no. select prevents a driver from be built as a dynamically linked >> module and distro-kernels might want to enable everything as modules. > > Ah, that's what the problem was! I wasn't aware of this issue with > SELECT. Sorry for the noob-ness. Select is also fragile, for example if a main subsystem isn't enabled, the specific option will still be enabled (or there'll be a warning/error about it). Overall it tends to cause headaches so many maintainers are starting to push back against it. > Really we want the PHY to be "=y" if the USB driver is "=y" or "=m" if > the USB driver is "=m", I think. You could argue that one might want > to build the main USB driver into the kernel but have the phy drivers > as modules, so you could possibly also try to support that... > > If there's not a good way to specify that, I guess we'll just have to > use "default" and rely on the user not to purposely choose the wrong > thing. Like the following (untested): > > config PHY_SAMSUNG_USB2 > depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS > default y if ARCH_EXYNOS=y && (USB_OHCI_EXYNOS=y || USB_EHCI_EXYNOS=y) > default m if ARCH_EXYNOS=m && (USB_OHCI_EXYNOS=m || USB_EHCI_EXYNOS=m) > ... > > I see some syntax like that elsewhere in Kconfig so I assume it's reasonable... I think you can take out the test for ARCH_EXYNOS here (first of all, it can never be modular). I'm not sure it makes sense to work so hard to set the default right either, as long as the dependencies are correct. I.e. it'll still mess up randconfig if the combination doesn't build, and distros can still downgrade to 'm' if they want to. That'll just leave: config PHY_SAMSUNG_USB2 tristate "foo" depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS default y (no need to add an if since it's taken care of by the depends) > With the above the user could purposely enable the OHCI or EHCI driver > and disable the PHY driver which is not really sensible. ...but it > wouldn't cause a compile failure or crash--USB just won't work. Just make the sub-drivers silent options with defaults. I.e: config PHY_EXYNOS5250_USB2 bool SOC_EXYNOS5250 depends on PHY_SAMSUNG_USB2 and so on. Note that it doesn't actually scale to make it depend on a specific SoC though, so it should probably just be cut down the line of EXYNOS4/5 and err on the side of including a bit too much code instead. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 27, 2014 at 10:23 PM, Olof Johansson <olof@lixom.net> wrote: > On Fri, Jun 27, 2014 at 9:32 AM, Doug Anderson <dianders@chromium.org> wrote: >> Felipe, >> >> On Fri, Jun 27, 2014 at 8:59 AM, Felipe Balbi <balbi@ti.com> wrote: >>>> I'll admit to not having been involved with the previous discussions, >>>> but this seems strange to me. Are we throwing in the towel and >>>> deciding that it's too hard to get the Kconfigs right and that we'll >>>> just rely on individual users to figure out the right answer for >>>> themselves? >>> >>> no. select prevents a driver from be built as a dynamically linked >>> module and distro-kernels might want to enable everything as modules. >> >> Ah, that's what the problem was! I wasn't aware of this issue with >> SELECT. Sorry for the noob-ness. > > Select is also fragile, for example if a main subsystem isn't enabled, > the specific option will still be enabled (or there'll be a > warning/error about it). Overall it tends to cause headaches so many > maintainers are starting to push back against it. > >> Really we want the PHY to be "=y" if the USB driver is "=y" or "=m" if >> the USB driver is "=m", I think. You could argue that one might want >> to build the main USB driver into the kernel but have the phy drivers >> as modules, so you could possibly also try to support that... >> >> If there's not a good way to specify that, I guess we'll just have to >> use "default" and rely on the user not to purposely choose the wrong >> thing. Like the following (untested): >> >> config PHY_SAMSUNG_USB2 >> depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS >> default y if ARCH_EXYNOS=y && (USB_OHCI_EXYNOS=y || USB_EHCI_EXYNOS=y) >> default m if ARCH_EXYNOS=m && (USB_OHCI_EXYNOS=m || USB_EHCI_EXYNOS=m) >> ... >> >> I see some syntax like that elsewhere in Kconfig so I assume it's reasonable... > > I think you can take out the test for ARCH_EXYNOS here (first of all, > it can never be modular). > > I'm not sure it makes sense to work so hard to set the default right > either, as long as the dependencies are correct. I.e. it'll still mess > up randconfig if the combination doesn't build, and distros can still > downgrade to 'm' if they want to. That'll just leave: > > config PHY_SAMSUNG_USB2 > tristate "foo" > depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS > default y (no need to add an if since it's taken care of by the depends) > >> With the above the user could purposely enable the OHCI or EHCI driver >> and disable the PHY driver which is not really sensible. ...but it >> wouldn't cause a compile failure or crash--USB just won't work. > > Just make the sub-drivers silent options with defaults. I.e: > > config PHY_EXYNOS5250_USB2 > bool SOC_EXYNOS5250 > depends on PHY_SAMSUNG_USB2 > > and so on. Note that it doesn't actually scale to make it depend on a > specific SoC though, so it should probably just be cut down the line > of EXYNOS4/5 and err on the side of including a bit too much code > instead. Yes, that seems the right thing to do. However, for now I will retain the SoC based structure. Considering the fragility involved in using 'select', I will re-do this by playing around with the default option. Thanks everyone for your inputs.
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 16a2f067c242..7fe7ef5f1322 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -121,44 +121,21 @@ config PHY_SUN4I_USB parts, as well as the 2 regular USB 2 host PHYs. config PHY_SAMSUNG_USB2 - tristate "Samsung USB 2.0 PHY driver" + tristate select GENERIC_PHY select MFD_SYSCON - help - Enable this to support the Samsung USB 2.0 PHY driver for Samsung - SoCs. This driver provides the interface for USB 2.0 PHY. Support for - particular SoCs has to be enabled in addition to this driver. Number - and type of supported phys depends on the SoC. + select PHY_EXYNOS4210_USB2 if CPU_EXYNOS4210 + select PHY_EXYNOS4X12_USB2 if (SOC_EXYNOS4212 || SOC_EXYNOS4412) + select PHY_EXYNOS5250_USB2 if (SOC_EXYNOS5250 || SOC_EXYNOS5420) config PHY_EXYNOS4210_USB2 - bool "Support for Exynos 4210" - depends on PHY_SAMSUNG_USB2 - depends on CPU_EXYNOS4210 - help - Enable USB PHY support for Exynos 4210. This option requires that - Samsung USB 2.0 PHY driver is enabled and means that support for this - particular SoC is compiled in the driver. In case of Exynos 4210 four - phys are available - device, host, HSIC0 and HSIC1. + bool config PHY_EXYNOS4X12_USB2 - bool "Support for Exynos 4x12" - depends on PHY_SAMSUNG_USB2 - depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) - help - Enable USB PHY support for Exynos 4x12. This option requires that - Samsung USB 2.0 PHY driver is enabled and means that support for this - particular SoC is compiled in the driver. In case of Exynos 4x12 four - phys are available - device, host, HSIC0 and HSIC1. + bool config PHY_EXYNOS5250_USB2 - bool "Support for Exynos 5250" - depends on PHY_SAMSUNG_USB2 - depends on SOC_EXYNOS5250 - help - Enable USB PHY support for Exynos 5250. This option requires that - Samsung USB 2.0 PHY driver is enabled and means that support for this - particular SoC is compiled in the driver. In case of Exynos 5250 four - phys are available - device, host, HSIC0 and HSIC. + bool config PHY_EXYNOS5_USBDRD tristate "Exynos5 SoC series USB DRD PHY driver" diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 61b7817bd66b..2938807331de 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -211,10 +211,11 @@ config USB_EHCI_SH If you use the PCI EHCI controller, this option is not necessary. config USB_EHCI_EXYNOS - tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" - depends on PLAT_S5P || ARCH_EXYNOS - help - Enable support for the Samsung Exynos SOC's on-chip EHCI controller. + tristate "EHCI support for Samsung S5P/EXYNOS SoC Series" + depends on PLAT_S5P || ARCH_EXYNOS + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS + help + Enable support for the Samsung Exynos SOC's on-chip EHCI controller. config USB_EHCI_MV bool "EHCI support for Marvell PXA/MMP USB controller" @@ -520,6 +521,7 @@ config USB_OHCI_SH config USB_OHCI_EXYNOS tristate "OHCI support for Samsung S5P/EXYNOS SoC Series" depends on PLAT_S5P || ARCH_EXYNOS + select PHY_SAMSUNG_USB2 if ARCH_EXYNOS help Enable support for the Samsung Exynos SOC's on-chip OHCI controller.
EHCI and OHCI drivers on Exynos platforms do not work without their corresponding SoC specific phy drivers. Hence it makes no sense to keep these phy drivers as user selectable. Instead select them from the respective USB configs to make things easier for the end user. While at it enable 5250 phy for Exynos 5420 SoC too. Signed-off-by: Sachin Kamat <sachin.kamat@samsung.com> Cc: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/phy/Kconfig | 37 +++++++------------------------------ drivers/usb/host/Kconfig | 10 ++++++---- 2 files changed, 13 insertions(+), 34 deletions(-)