| Message ID | 1367528578-518090-10-git-send-email-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 2, 2013 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote: > The use of 'select' in the ux500 platform is a huge mess, we should > not be selecting most of the drivers that are currently selected > there, but rather enable them from the defconfig. > > As a fixup for 3.10, let's at least select the dependencies for > the other drivers we already select, to make it consistent. > > warning: (UX500_SOC_COMMON) selects AB8500_CORE which has unmet direct dependencies (HAS_IOMEM && GENERIC_HARDIRQS && ABX500_CORE && MFD_DB8500_PRCMU) > > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I do not necessarily agree with this commit message. The only mess I see is that this very patch is missing... If we look at this: config UX500_SOC_DB8500 bool select CPU_FREQ_TABLE if CPU_FREQ select MFD_DB8500_PRCMU select PINCTRL_DB8500 select PINCTRL_DB8540 select PINCTRL_AB8500 select PINCTRL_AB8505 select PINCTRL_AB9540 select PINCTRL_AB8540 select REGULATOR select REGULATOR_DB8500_PRCMU The PRCMU, the PINCTRL drivers, and the PRCMU regulators are necessary to boot the system correctly, by which I mean necessary so as not to cause potential damage to the hardware. The PRCMU is a system controller without which the system cannot really be brought up, and the ABx500's are the PMICs which are developed in pair with the SoC and they are in practice Siamese twins, there is no chance you can build a Ux500 system without both of them. It just won't even start. There are deep dependencies between the PRCMU and the ABx500, as the PRCMU firmware will talk directly to the ABx500 without the kernel intervening. I.e. the whole set of hardware has to be there. While I understand that from a compile-time point of view it does not seem nice, and all things would be separate units that you plug in, from a run-time and system architecture point of view it makes a lot of sense to select these. Worlds collide... Apart from that: Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Friday 03 May 2013 10:14:28 Linus Walleij wrote: > > I do not necessarily agree with this commit message. > > The only mess I see is that this very patch is missing... > > If we look at this: > > config UX500_SOC_DB8500 > bool > select CPU_FREQ_TABLE if CPU_FREQ > select MFD_DB8500_PRCMU > select PINCTRL_DB8500 > select PINCTRL_DB8540 > select PINCTRL_AB8500 > select PINCTRL_AB8505 > select PINCTRL_AB9540 > select PINCTRL_AB8540 > select REGULATOR > select REGULATOR_DB8500_PRCMU > > The PRCMU, the PINCTRL drivers, and the PRCMU regulators > are necessary to boot the system correctly, by which I mean > necessary so as not to cause potential damage to the hardware. > > The PRCMU is a system controller without which the system > cannot really be brought up, and the ABx500's are the PMICs > which are developed in pair with the SoC and they are in practice > Siamese twins, there is no chance you can build a Ux500 system > without both of them. It just won't even start. There are deep > dependencies between the PRCMU and the ABx500, as the > PRCMU firmware will talk directly to the ABx500 without the > kernel intervening. I.e. the whole set of hardware has to be > there. > > While I understand that from a compile-time point of view it does > not seem nice, and all things would be separate units that you > plug in, from a run-time and system architecture point of view it > makes a lot of sense to select these. Worlds collide... > > Apart from that: > Acked-by: Linus Walleij <linus.walleij@linaro.org> If they are completely essential, we should not have an entry like config AB8500_CORE bool "ST-Ericsson AB8500 Mixed Signal Power Management chip" depends on GENERIC_HARDIRQS && ABX500_CORE && MFD_DB8500_PRCMU select POWER_SUPPLY select MFD_CORE select IRQ_DOMAIN help Select this option to enable access to AB8500 power management chip. This connects to U8500 either on the SSP/SPI bus (deprecated since hardware version v1.0) or the I2C bus via PRCMU. It also adds the irq_chip parts for handling the Mixed Signal chip events. This chip embeds various other multimedia funtionalities as well. (and more of the same) in drivers/mfd/Kconfig. The rule is that you either make a Kconfig option user-selectable or make it always selected by another option. My preference would be to make it user-selectable and enable it only in the defconfig, but if that can damage the hardware, we should not actually provide a named option. Arnd
On Fri, May 3, 2013 at 1:28 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 03 May 2013 10:14:28 Linus Walleij wrote: >> While I understand that from a compile-time point of view it does >> not seem nice, and all things would be separate units that you >> plug in, from a run-time and system architecture point of view it >> makes a lot of sense to select these. Worlds collide... (...) > My preference would be to make it user-selectable and enable it only in > the defconfig, but if that can damage the hardware, we should not actually > provide a named option. Hm I'd lean towards the latter. However the AB8500 has subdrivers and *some* of these are optional, such as the RTC. And it'd be strange to have that depend on some totally unrelated config symbol like UX500_SOC_COMMON. I think that in that case we should make AB8500_CORE a hidden (non-user selectable) bool so it is more logical. However any if these approaches will lead to unaestetic menuconfig with AB8500 suboptions appearing right in the MFD menu instead of being ordered under a AB8500_CORE node which is better for anyone actually trying to use menuconfig interactively. The latter aestetic is one of the reasons why it looks as it does IIRC, but maybe that is HTML thinking :-) Yours, Linus Walleij
On Friday 03 May 2013 14:13:20 Linus Walleij wrote: > On Fri, May 3, 2013 at 1:28 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Friday 03 May 2013 10:14:28 Linus Walleij wrote: > > >> While I understand that from a compile-time point of view it does > >> not seem nice, and all things would be separate units that you > >> plug in, from a run-time and system architecture point of view it > >> makes a lot of sense to select these. Worlds collide... > (...) > > My preference would be to make it user-selectable and enable it only in > > the defconfig, but if that can damage the hardware, we should not actually > > provide a named option. > > Hm I'd lean towards the latter. However the AB8500 has subdrivers > and *some* of these are optional, such as the RTC. And it'd be > strange to have that depend on some totally unrelated config symbol > like UX500_SOC_COMMON. > > I think that in that case we should make AB8500_CORE a hidden > (non-user selectable) bool so it is more logical. Yes, makes sense. > However any if these approaches will lead to unaestetic menuconfig > with AB8500 suboptions appearing right in the MFD menu instead of > being ordered under a AB8500_CORE node which is better for > anyone actually trying to use menuconfig interactively. Yes, I can see that. Right now we have: -*- ST-Ericsson ABX500 Mixed Signal Circuit register functions [*] ST-Ericsson AB3100 Mixed Signal Circuit core functions [*] ST-Ericsson AB3100 OTP functions -*- ST-Ericsson AB8500 Mixed Signal Power Management chip [*] ST-Ericsson AB8500 GPADC driver [*] Enable debug info via debugfs -*- ST-Ericsson DB8500 Power Reset Control Management Unit I think we can certainly eliminate the ABX500 option, this one can just get selected by AB3100 and AB8500. The PRCMU can also be silent because as you say it is always needed. One possible reason to allow them to be selected is to enable build-time coverage on other platforms, but at the moment, they all depend on ARCH_U300|ARCH_U8500, presumably because until very recently they would not actually build anywhere else. The GPADC driver is interesting: it does not have its own user interface (besides the debugging driver) and could just get selected by the other drivers using its exported interfaces. It would also be better to convert it to the standard IIO ADC interface, but I don't know if that's realistic. This leavs the debugging interface as the only sub-option. Do you know if that is actually being used by developers, or is this one of those interfaces that were created during bringup and then never touched again after everything worked? Regarding AB3100, I assume the relationship to U300 is similar to that between DB8500 and AB8500. Are you able to boot a U300 system without touching AB3100? Arnd
On Fri, May 3, 2013 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote: > I think we can certainly eliminate the ABX500 option, this one can > just get selected by AB3100 and AB8500. The PRCMU can also > be silent because as you say it is always needed. OK sounds like a plan. > The GPADC driver is interesting: it does not have its own user > interface (besides the debugging driver) and could just get selected > by the other drivers using its exported interfaces. It would > also be better to convert it to the standard IIO ADC interface, > but I don't know if that's realistic. This should be moved over to IIO ADC, that subsystem however did not exist when this driver was submitted, that is why it ended up in MFD. As usual someone needs to go hack on it... > This leavs the debugging > interface as the only sub-option. Do you know if that is > actually being used by developers, or is this one of those interfaces > that were created during bringup and then never touched again > after everything worked? Something like that, but as long as new platforms using that ASIC is coming out it remains quite usable for this. It is also used for hardware validation, where you hook up the system with a lot of electric probes and want to set specific bits here and there to validate functionality. There just a minor change to the silicon or packaging triggers such re-validation. The usecase is similar to the debug interface we invented for pin control recently, however there we managed to make things centralized. > Regarding AB3100, I assume the relationship to U300 is similar to > that between DB8500 and AB8500. Are you able to boot a U300 system > without touching AB3100? I would say no. The system will be in some bootstrap state unless you bring up the AB3100, and especially the regulators. These are default y but should arguably be selected y instead. The U300 SoC will bootstrap power in a strange way until the regulators are up, and then hand over to software-controlled regulators for powering the SoC and release that bootstrap. This is not harmful but a very strange and it's doubtful whether I would call that "fully booted". It's something like booting into "safe mode" http://en.wikipedia.org/wiki/Safe_mode Not enabling regulators on the U300 will have other strange effects ... like MMC/SD not working. Yet we can not have depends on REGULATORS in the Kconfig for the MMCI driver since this driver is also used on systems without the regulator framework (hardwired power). All these run-time "makes sense" is in a bit of flux as compared to config-and-compile-time "makes sense" unfortunately. I lean toward selecting AB3100 and REGULATOR_AB3100 for the U300 actually. Yours, Linus Walleij
On Friday 03 May 2013 15:33:17 Linus Walleij wrote: > On Fri, May 3, 2013 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > This leavs the debugging > > interface as the only sub-option. Do you know if that is > > actually being used by developers, or is this one of those interfaces > > that were created during bringup and then never touched again > > after everything worked? > > Something like that, but as long as new platforms using that > ASIC is coming out it remains quite usable for this. > > It is also used for hardware validation, where you hook up the > system with a lot of electric probes and want to set specific > bits here and there to validate functionality. There just a minor > change to the silicon or packaging triggers such re-validation. > > The usecase is similar to the debug interface we invented for > pin control recently, however there we managed to make things > centralized. Ok, makes sense. > > Regarding AB3100, I assume the relationship to U300 is similar to > > that between DB8500 and AB8500. Are you able to boot a U300 system > > without touching AB3100? > > I would say no. The system will be in some bootstrap state unless > you bring up the AB3100, and especially the regulators. > > These are default y but should arguably be selected y > instead. > > The U300 SoC will bootstrap power in a strange way until the > regulators are up, and then hand over to software-controlled > regulators for powering the SoC and release that bootstrap. > > This is not harmful but a very strange and it's doubtful whether > I would call that "fully booted". It's something like booting > into "safe mode" > http://en.wikipedia.org/wiki/Safe_mode I see. > Not enabling regulators on the U300 will have other strange > effects ... like MMC/SD not working. Yet we can not have > depends on REGULATORS in the Kconfig for the MMCI driver > since this driver is also used on systems without the regulator > framework (hardwired power). > > All these run-time "makes sense" is in a bit of flux as compared > to config-and-compile-time "makes sense" unfortunately. > > I lean toward selecting AB3100 and REGULATOR_AB3100 > for the U300 actually. Should we still allow building them on other platforms then? Right now, these depend on the platforms that actually use the hardware, but in other places, we just allow everything to be built that compiles without errors. Is it theoretically possible to use the AB in combination with another (non-ST-Ericsson) digital baseband? Arnd
On Fri, May 3, 2013 at 3:56 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 03 May 2013 15:33:17 Linus Walleij wrote: >> I lean toward selecting AB3100 and REGULATOR_AB3100 >> for the U300 actually. > > Should we still allow building them on other platforms then? > Right now, these depend on the platforms that actually use > the hardware, but in other places, we just allow everything to > be built that compiles without errors. Not in my opinion but IIRC in 2008 or so some other subsystem maintainer beat us up for not allowing it to build on every other platform, and the rationale given was that allowin this to build on e.g. x86_64 gives some nice compile coverage. I don't know which argument wins :-) > Is it theoretically possible to use the AB in combination with > another (non-ST-Ericsson) digital baseband? Theoretically (it's just a bunch of regulators etc when all comes around), but it won't happen since these components are not sold separately. Yours, Linus Walleij
On Friday 03 May 2013 16:06:57 Linus Walleij wrote: > On Fri, May 3, 2013 at 3:56 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Friday 03 May 2013 15:33:17 Linus Walleij wrote: > > >> I lean toward selecting AB3100 and REGULATOR_AB3100 > >> for the U300 actually. > > > > Should we still allow building them on other platforms then? > > Right now, these depend on the platforms that actually use > > the hardware, but in other places, we just allow everything to > > be built that compiles without errors. > > Not in my opinion but IIRC in 2008 or so some other > subsystem maintainer beat us up for not allowing it > to build on every other platform, and the rationale given > was that allowin this to build on e.g. x86_64 gives some > nice compile coverage. > > I don't know which argument wins I know that a certain other Linus frequently complains when ARM specific options show up on his x86 machine during "make oldconfig" ;-) One idea I had is to create a global CONFIG_SOC option that would hide all on-chip peripherals when building for a PC-like system. On ARM, we would always select CONFIG_SOC, except for a few special cases like ARCH_RPC, ARCH_FOOTBRIDGE and ARCH_SHARK. > > Is it theoretically possible to use the AB in combination with > > another (non-ST-Ericsson) digital baseband? > > Theoretically (it's just a bunch of regulators etc when all comes > around), but it won't happen since these components > are not sold separately. Right, I was specifically asking about the technical possibilities, not what people are likely to do. Arnd
On Fri, May 3, 2013 at 4:20 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 03 May 2013 16:06:57 Linus Walleij wrote: >> >> Not in my opinion but IIRC in 2008 or so some other >> subsystem maintainer beat us up for not allowing it >> to build on every other platform, and the rationale given >> was that allowin this to build on e.g. x86_64 gives some >> nice compile coverage. >> >> I don't know which argument wins > > I know that a certain other Linus frequently complains when ARM specific > options show up on his x86 machine during "make oldconfig" ;-) OK let's make it SoC-specific. > One idea I had is to create a global CONFIG_SOC option that would > hide all on-chip peripherals when building for a PC-like system. > On ARM, we would always select CONFIG_SOC, except for a few special > cases like ARCH_RPC, ARCH_FOOTBRIDGE and ARCH_SHARK. Sounds like a deal. The AB chips aren't on-chip though they sit on I2C or SPI actually. But they are still strongly coupled with the SoC. I just took a stroll around the premises and found this Aardvark I2C/SPI "embedded systems interface". This is a USB thing that makes it possible to connect any I2C or SPI peripheral to the desktop ... I don't know how such things should be used with Linux really, it seems like it's something that normally gets used from userspace by libusb but in theory we could have a kernel driver for this and plug in whatever SoC I2C/SPI device into a desktop. Whatever use it would be ... but could be useful for developing drivers for a chip outside of the target system I believe. Yours, Linus Walleij
On Friday 03 May 2013 20:43:42 Linus Walleij wrote: > > Sounds like a deal. The AB chips aren't on-chip though > they sit on I2C or SPI actually. But they are still > strongly coupled with the SoC. > > I just took a stroll around the premises and found this > Aardvark I2C/SPI "embedded systems interface". > > This is a USB thing that makes it possible to connect > any I2C or SPI peripheral to the desktop ... I don't know how > such things should be used with Linux really, it seems like > it's something that normally gets used from userspace by > libusb but in theory we could have a kernel driver for this > and plug in whatever SoC I2C/SPI device into a desktop. > > Whatever use it would be ... but could be useful for > developing drivers for a chip outside of the target > system I believe. I would argue that the AB8500 is close enough to a 'system-on-a-chip' to make it depend on CONFIG_SOC, even though it is technically one half of a 'system-on-two-chips'. I would also allow x86 and all other architectures to enable CONFIG_SOC, since there are lots of SoCs these days that combine an x86 core with the same kind of peripherals we have on ARM systems. It's just not what you you find in a regualar PC or Laptop, and anyone building their own kernel can ignore those. Arnd
diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig index f66d7de..6a4387e 100644 --- a/arch/arm/mach-ux500/Kconfig +++ b/arch/arm/mach-ux500/Kconfig @@ -19,6 +19,8 @@ if ARCH_U8500 config UX500_SOC_COMMON bool default y + select ABX500_CORE + select AB8500_CORE select ARM_ERRATA_754322 select ARM_ERRATA_764369 if SMP select ARM_GIC
The use of 'select' in the ux500 platform is a huge mess, we should not be selecting most of the drivers that are currently selected there, but rather enable them from the defconfig. As a fixup for 3.10, let's at least select the dependencies for the other drivers we already select, to make it consistent. warning: (UX500_SOC_COMMON) selects AB8500_CORE which has unmet direct dependencies (HAS_IOMEM && GENERIC_HARDIRQS && ABX500_CORE && MFD_DB8500_PRCMU) Cc: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/arm/mach-ux500/Kconfig | 2 ++ 1 file changed, 2 insertions(+)