Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On Mon, Jan 13, 2014 at 06:45:36AM +, Ben Dooks wrote: > - If pm-runtime is not enabled then we need something to manage the > clocks for the driver. If we put that code in the driver then there > is not a lot of point in having the pm-runtime clock code here as > the driver really only needs a helper to turn them on and off at > the right place. Last time I looked at the code the runtime PM stuff was also being used to manage actual power domains in at least some of those SoCs (which is the more common use for power domains). This was a while ago while I was doing power domain support for s3c64xx though. > When discussing this on freenode's #armkernel channel, several people > including Mark Brown wanted to keep this as it made driver's handling > of clocks much easier (there was no longer any need to deal with the > clk code when writing a simple driver). My view is it is a pain as we > now have a mix of drivers which expect to do their own clock work and > some that do not. (It is possible there are even some shmobile drivers > that still do their own clock management). I don't massively care one way or another but it is a totally reasonable decision for a platform to do this especially if the clocks are tied to the power domains in some way, for example a single functional clock shared over the domain. The arguments people have for doing this have been more about removing knowledge of the SoC integration from the driver - having the functional clocks for the IP visible in the kernel can make IPs harder to share with platforms that lack meaningful clock management - and factoring out boilerplate code that just acquires, enables and disables clocks. > Personally I do not like hiding the implementation of this, as it ends > up confusing people when they first come to it. It wouldn't do that if we did it all the time of course; there is an argument for consistency. signature.asc Description: Digital signature
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On Mon, Jan 13, 2014 at 06:45:36AM +, Ben Dooks wrote: - If pm-runtime is not enabled then we need something to manage the clocks for the driver. If we put that code in the driver then there is not a lot of point in having the pm-runtime clock code here as the driver really only needs a helper to turn them on and off at the right place. Last time I looked at the code the runtime PM stuff was also being used to manage actual power domains in at least some of those SoCs (which is the more common use for power domains). This was a while ago while I was doing power domain support for s3c64xx though. When discussing this on freenode's #armkernel channel, several people including Mark Brown wanted to keep this as it made driver's handling of clocks much easier (there was no longer any need to deal with the clk code when writing a simple driver). My view is it is a pain as we now have a mix of drivers which expect to do their own clock work and some that do not. (It is possible there are even some shmobile drivers that still do their own clock management). I don't massively care one way or another but it is a totally reasonable decision for a platform to do this especially if the clocks are tied to the power domains in some way, for example a single functional clock shared over the domain. The arguments people have for doing this have been more about removing knowledge of the SoC integration from the driver - having the functional clocks for the IP visible in the kernel can make IPs harder to share with platforms that lack meaningful clock management - and factoring out boilerplate code that just acquires, enables and disables clocks. Personally I do not like hiding the implementation of this, as it ends up confusing people when they first come to it. It wouldn't do that if we did it all the time of course; there is an argument for consistency. signature.asc Description: Digital signature
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben, On Monday 13 January 2014 06:45:36 Ben Dooks wrote: > On 12/01/14 22:01, Laurent Pinchart wrote: > > On Sunday 12 January 2014 22:54:15 Laurent Pinchart wrote: > >> Hi Ben, > >> > >> Thank you for the patch. > >> > >> On Saturday 11 January 2014 13:06:29 Ben Dooks wrote: > >>> If the kernel is built to support multi-arm configurmation with shmobile > >>> support built in, then the drivers/sh is not built. This contains > >>> drivers that are essential to devices support by that configuration, > >>> including the PM runtime code in drivers/sh/pm_runtime.c (which > >>> implicitly enables the bus clocks for all devices). > > > > Thinking a bit more about this, I think the approach taken in > > drivers/sh/pm_runtime.c isn't good. The code enables device clocks when > > devices are bound to a driver, increasing power consumption when devices > > are idle. Instead of enabling it for ARCH_SHMOBILE_MULTI I'd like to > > either add explicit clock support to drivers, or to integrate clocks with > > runtime PM only. > > If pm-runtime is enabled, then I believe that the device clocks are > kept in sync with the active state of the device, which means that > they should be shut down when the device is not needed. There have > been recent discussions about this with respect to the PCI bridges > used by the USB host system. > > Given the above, I'm not sure exactly what you mean by the statement > "I think the approach taken in drivers/sh/pm_runtime.c isn't good." I might have read the code too fast. I was under the impression that the code enabled the device clock as soon as the device was bound to a driver. Upon closer inspection this doesn't seem to be the case. > as if we're going to abstract the clock management we have the following > issues. > > - If pm-runtime is not enabled then we need something to manage the clocks > for the driver. If we put that code in the driver then there is not a lot > of point in having the pm-runtime clock code here as the driver really > only needs a helper to turn them on and off at the right place. > > - If just standard power management is enabled, then do we really care > about the power consumption of leaving peripherals running when their > devices are bound? Managing the device clock optimally is hardly a concern > if device drivers are not going to be idled when they are not being used. Many devices are not bound to a power domain handled by runtime PM. Adding a runtime PM dependency to clock handling for those devices doesn't make me really happy. Does automatic clock handling even work at all with runtime PM disabled ? The clk_enable() call seems to come from pm_clk_resume() only, which is used as the runtime_resume handler. with runtime PM disabled that code looks like it won't be called at all. > When discussing this on freenode's #armkernel channel, several people > including Mark Brown wanted to keep this as it made driver's handling > of clocks much easier (there was no longer any need to deal with the > clk code when writing a simple driver). My view is it is a pain as we > now have a mix of drivers which expect to do their own clock work and > some that do not. (It is possible there are even some shmobile drivers > that still do their own clock management). We could remove manual clock handling from some drivers, but the drivers that need to handle several clocks will still need to do so manually. As long as the core code allows this (which I think it does, but I'm not too familiar with the code) I won't complain (too much at least :-)). > Personally I do not like hiding the implementation of this, as it ends > up confusing people when they first come to it. I like explicit implementations as well, but I have to admit that devices that require a single clock and have clock requirements that are in sync with the PM runtime requirements would probably benefit from automatic clock handling, at least from a driver code complexity point of view, to the expense of a less explicit implementation. -- Regards, Laurent Pinchart -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben,
On Mon, Jan 13, 2014 at 10:35 AM, Ben Dooks wrote:
> On 13/01/14 09:28, Geert Uytterhoeven wrote:
>> On Fri, Jan 10, 2014 at 4:18 PM, Ben Dooks
>> wrote:
>>>
>>> --- a/drivers/sh/Makefile
>>> +++ b/drivers/sh/Makefile
>>> @@ -3,7 +3,10 @@
>>> #
>>> obj-y := intc/
>>>
>>> +ifeq ($(CONFIG_COMMON_CLK),n)
>>> obj-$(CONFIG_HAVE_CLK) += clk/
>>> +endif
>>> +
>>
>>
>> This part breaks my Koelsch legacy (non-reference) build:
>>
>> arch/arm/kernel/built-in.o: In function `twd_local_timer_common_register':
>> io.c:(.init.text+0x1804): undefined reference to `clk_enable'
>> io.c:(.init.text+0x1828): undefined reference to `clk_get_rate'
>
> ...
>
>>
>> Both of my Koelsch legacy and reference configs have CONFIG_HAVE_CLK=y,
>> but only the reference once has CONFIG_COMMON_CLK=y.
>
> Hmm, thought undefined symbols got set to 'n'.
No, they're empty.
Doh, should have realized that myself...
> I can either fix this by
>
> ifneq ($(CONFIG_COMMON_CLK),y)
> endif
That's the typical pattern for this.
Sometimes people use
obj-$(CONFIG_COMMON_CLK)$(CONFIG_HAVE_CLK) += clk/
but that's less readable.
> or adding an extra Kconfig for SH specific legacy clock code.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On 13/01/14 09:28, Geert Uytterhoeven wrote: On Fri, Jan 10, 2014 at 4:18 PM, Ben Dooks wrote: --- a/drivers/sh/Makefile +++ b/drivers/sh/Makefile @@ -3,7 +3,10 @@ # obj-y := intc/ +ifeq ($(CONFIG_COMMON_CLK),n) obj-$(CONFIG_HAVE_CLK) += clk/ +endif + This part breaks my Koelsch legacy (non-reference) build: arch/arm/kernel/built-in.o: In function `twd_local_timer_common_register': io.c:(.init.text+0x1804): undefined reference to `clk_enable' io.c:(.init.text+0x1828): undefined reference to `clk_get_rate' ... Both of my Koelsch legacy and reference configs have CONFIG_HAVE_CLK=y, but only the reference once has CONFIG_COMMON_CLK=y. Hmm, thought undefined symbols got set to 'n'. I can either fix this by ifneq ($(CONFIG_COMMON_CLK),y) endif or adding an extra Kconfig for SH specific legacy clock code. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On Fri, Jan 10, 2014 at 4:18 PM, Ben Dooks wrote:
> --- a/drivers/sh/Makefile
> +++ b/drivers/sh/Makefile
> @@ -3,7 +3,10 @@
> #
> obj-y := intc/
>
> +ifeq ($(CONFIG_COMMON_CLK),n)
> obj-$(CONFIG_HAVE_CLK) += clk/
> +endif
> +
This part breaks my Koelsch legacy (non-reference) build:
arch/arm/kernel/built-in.o: In function `twd_local_timer_common_register':
io.c:(.init.text+0x1804): undefined reference to `clk_enable'
io.c:(.init.text+0x1828): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `sci_port_disable':
clkdev.c:(.text+0x2ae30): undefined reference to `clk_disable'
clkdev.c:(.text+0x2ae38): undefined reference to `clk_disable'
drivers/built-in.o: In function `sci_port_enable':
clkdev.c:(.text+0x2ae74): undefined reference to `clk_enable'
clkdev.c:(.text+0x2ae7c): undefined reference to `clk_get_rate'
clkdev.c:(.text+0x2ae88): undefined reference to `clk_enable'
drivers/built-in.o: In function `sci_notifier':
clkdev.c:(.text+0x2c85c): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `__pm_clk_remove':
clkdev.c:(.text+0x40844): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_suspend':
clkdev.c:(.text+0x40c6c): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_resume':
clkdev.c:(.text+0x40ce0): undefined reference to `clk_enable'
drivers/built-in.o: In function `qspi_set_config_register':
clkdev.c:(.text+0x515a4): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `rspi_set_config_register':
clkdev.c:(.text+0x516dc): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `rspi_remove':
clkdev.c:(.text+0x518ac): undefined reference to `clk_disable'
drivers/built-in.o: In function `rspi_probe':
clkdev.c:(.text+0x51a20): undefined reference to `clk_enable'
clkdev.c:(.text+0x51d60): undefined reference to `clk_disable'
drivers/built-in.o: In function `hspi_transfer_one_message':
clkdev.c:(.text+0x5312c): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `sh_msiof_spi_txrx':
clkdev.c:(.text+0x53d90): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `sh_msiof_spi_chipselect':
clkdev.c:(.text+0x54200): undefined reference to `clk_enable'
clkdev.c:(.text+0x542b8): undefined reference to `clk_disable'
drivers/built-in.o: In function `sh_cmt_start':
clkdev.c:(.text+0x6f900): undefined reference to `clk_enable'
clkdev.c:(.text+0x6f93c): undefined reference to `clk_get_rate'
clkdev.c:(.text+0x6f95c): undefined reference to `clk_get_rate'
clkdev.c:(.text+0x6fa08): undefined reference to `clk_disable'
drivers/built-in.o: In function `sh_cmt_stop':
clkdev.c:(.text+0x6fc14): undefined reference to `clk_disable'
drivers/built-in.o: In function `__sh_tmu_disable':
clkdev.c:(.text+0x70590): undefined reference to `clk_disable'
drivers/built-in.o: In function `__sh_tmu_enable':
clkdev.c:(.text+0x70980): undefined reference to `clk_enable'
clkdev.c:(.text+0x709f0): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `em_sti_start':
clkdev.c:(.text+0x70f44): undefined reference to `clk_enable'
clkdev.c:(.text+0x70f68): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `em_sti_stop':
clkdev.c:(.text+0x710d0): undefined reference to `clk_disable'
arch/arm/mach-shmobile/built-in.o: In function `shmobile_clk_init':
platsmp-apmu.c:(.init.text+0x7ac): undefined reference to
`recalculate_root_clocks'
platsmp-apmu.c:(.init.text+0x7b0): undefined reference to
`clk_enable_init_clocks'
arch/arm/mach-shmobile/built-in.o: In function `r8a7791_clock_init':
platsmp-apmu.c:(.init.text+0x8fc): undefined reference to `clk_register'
platsmp-apmu.c:(.init.text+0x920): undefined reference to `sh_clk_mstp_register'
arch/arm/mach-shmobile/built-in.o:(.data+0x16f0): undefined reference
to `followparent_recalc'
Both of my Koelsch legacy and reference configs have CONFIG_HAVE_CLK=y,
but only the reference once has CONFIG_COMMON_CLK=y.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On Fri, Jan 10, 2014 at 4:18 PM, Ben Dooks ben.do...@codethink.co.uk wrote:
--- a/drivers/sh/Makefile
+++ b/drivers/sh/Makefile
@@ -3,7 +3,10 @@
#
obj-y := intc/
+ifeq ($(CONFIG_COMMON_CLK),n)
obj-$(CONFIG_HAVE_CLK) += clk/
+endif
+
This part breaks my Koelsch legacy (non-reference) build:
arch/arm/kernel/built-in.o: In function `twd_local_timer_common_register':
io.c:(.init.text+0x1804): undefined reference to `clk_enable'
io.c:(.init.text+0x1828): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `sci_port_disable':
clkdev.c:(.text+0x2ae30): undefined reference to `clk_disable'
clkdev.c:(.text+0x2ae38): undefined reference to `clk_disable'
drivers/built-in.o: In function `sci_port_enable':
clkdev.c:(.text+0x2ae74): undefined reference to `clk_enable'
clkdev.c:(.text+0x2ae7c): undefined reference to `clk_get_rate'
clkdev.c:(.text+0x2ae88): undefined reference to `clk_enable'
drivers/built-in.o: In function `sci_notifier':
clkdev.c:(.text+0x2c85c): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `__pm_clk_remove':
clkdev.c:(.text+0x40844): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_suspend':
clkdev.c:(.text+0x40c6c): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_resume':
clkdev.c:(.text+0x40ce0): undefined reference to `clk_enable'
drivers/built-in.o: In function `qspi_set_config_register':
clkdev.c:(.text+0x515a4): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `rspi_set_config_register':
clkdev.c:(.text+0x516dc): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `rspi_remove':
clkdev.c:(.text+0x518ac): undefined reference to `clk_disable'
drivers/built-in.o: In function `rspi_probe':
clkdev.c:(.text+0x51a20): undefined reference to `clk_enable'
clkdev.c:(.text+0x51d60): undefined reference to `clk_disable'
drivers/built-in.o: In function `hspi_transfer_one_message':
clkdev.c:(.text+0x5312c): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `sh_msiof_spi_txrx':
clkdev.c:(.text+0x53d90): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `sh_msiof_spi_chipselect':
clkdev.c:(.text+0x54200): undefined reference to `clk_enable'
clkdev.c:(.text+0x542b8): undefined reference to `clk_disable'
drivers/built-in.o: In function `sh_cmt_start':
clkdev.c:(.text+0x6f900): undefined reference to `clk_enable'
clkdev.c:(.text+0x6f93c): undefined reference to `clk_get_rate'
clkdev.c:(.text+0x6f95c): undefined reference to `clk_get_rate'
clkdev.c:(.text+0x6fa08): undefined reference to `clk_disable'
drivers/built-in.o: In function `sh_cmt_stop':
clkdev.c:(.text+0x6fc14): undefined reference to `clk_disable'
drivers/built-in.o: In function `__sh_tmu_disable':
clkdev.c:(.text+0x70590): undefined reference to `clk_disable'
drivers/built-in.o: In function `__sh_tmu_enable':
clkdev.c:(.text+0x70980): undefined reference to `clk_enable'
clkdev.c:(.text+0x709f0): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `em_sti_start':
clkdev.c:(.text+0x70f44): undefined reference to `clk_enable'
clkdev.c:(.text+0x70f68): undefined reference to `clk_get_rate'
drivers/built-in.o: In function `em_sti_stop':
clkdev.c:(.text+0x710d0): undefined reference to `clk_disable'
arch/arm/mach-shmobile/built-in.o: In function `shmobile_clk_init':
platsmp-apmu.c:(.init.text+0x7ac): undefined reference to
`recalculate_root_clocks'
platsmp-apmu.c:(.init.text+0x7b0): undefined reference to
`clk_enable_init_clocks'
arch/arm/mach-shmobile/built-in.o: In function `r8a7791_clock_init':
platsmp-apmu.c:(.init.text+0x8fc): undefined reference to `clk_register'
platsmp-apmu.c:(.init.text+0x920): undefined reference to `sh_clk_mstp_register'
arch/arm/mach-shmobile/built-in.o:(.data+0x16f0): undefined reference
to `followparent_recalc'
Both of my Koelsch legacy and reference configs have CONFIG_HAVE_CLK=y,
but only the reference once has CONFIG_COMMON_CLK=y.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On 13/01/14 09:28, Geert Uytterhoeven wrote: On Fri, Jan 10, 2014 at 4:18 PM, Ben Dooks ben.do...@codethink.co.uk wrote: --- a/drivers/sh/Makefile +++ b/drivers/sh/Makefile @@ -3,7 +3,10 @@ # obj-y := intc/ +ifeq ($(CONFIG_COMMON_CLK),n) obj-$(CONFIG_HAVE_CLK) += clk/ +endif + This part breaks my Koelsch legacy (non-reference) build: arch/arm/kernel/built-in.o: In function `twd_local_timer_common_register': io.c:(.init.text+0x1804): undefined reference to `clk_enable' io.c:(.init.text+0x1828): undefined reference to `clk_get_rate' ... Both of my Koelsch legacy and reference configs have CONFIG_HAVE_CLK=y, but only the reference once has CONFIG_COMMON_CLK=y. Hmm, thought undefined symbols got set to 'n'. I can either fix this by ifneq ($(CONFIG_COMMON_CLK),y) endif or adding an extra Kconfig for SH specific legacy clock code. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben,
On Mon, Jan 13, 2014 at 10:35 AM, Ben Dooks ben.do...@codethink.co.uk wrote:
On 13/01/14 09:28, Geert Uytterhoeven wrote:
On Fri, Jan 10, 2014 at 4:18 PM, Ben Dooks ben.do...@codethink.co.uk
wrote:
--- a/drivers/sh/Makefile
+++ b/drivers/sh/Makefile
@@ -3,7 +3,10 @@
#
obj-y := intc/
+ifeq ($(CONFIG_COMMON_CLK),n)
obj-$(CONFIG_HAVE_CLK) += clk/
+endif
+
This part breaks my Koelsch legacy (non-reference) build:
arch/arm/kernel/built-in.o: In function `twd_local_timer_common_register':
io.c:(.init.text+0x1804): undefined reference to `clk_enable'
io.c:(.init.text+0x1828): undefined reference to `clk_get_rate'
...
Both of my Koelsch legacy and reference configs have CONFIG_HAVE_CLK=y,
but only the reference once has CONFIG_COMMON_CLK=y.
Hmm, thought undefined symbols got set to 'n'.
No, they're empty.
Doh, should have realized that myself...
I can either fix this by
ifneq ($(CONFIG_COMMON_CLK),y)
endif
That's the typical pattern for this.
Sometimes people use
obj-$(CONFIG_COMMON_CLK)$(CONFIG_HAVE_CLK) += clk/
but that's less readable.
or adding an extra Kconfig for SH specific legacy clock code.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben, On Monday 13 January 2014 06:45:36 Ben Dooks wrote: On 12/01/14 22:01, Laurent Pinchart wrote: On Sunday 12 January 2014 22:54:15 Laurent Pinchart wrote: Hi Ben, Thank you for the patch. On Saturday 11 January 2014 13:06:29 Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). Thinking a bit more about this, I think the approach taken in drivers/sh/pm_runtime.c isn't good. The code enables device clocks when devices are bound to a driver, increasing power consumption when devices are idle. Instead of enabling it for ARCH_SHMOBILE_MULTI I'd like to either add explicit clock support to drivers, or to integrate clocks with runtime PM only. If pm-runtime is enabled, then I believe that the device clocks are kept in sync with the active state of the device, which means that they should be shut down when the device is not needed. There have been recent discussions about this with respect to the PCI bridges used by the USB host system. Given the above, I'm not sure exactly what you mean by the statement I think the approach taken in drivers/sh/pm_runtime.c isn't good. I might have read the code too fast. I was under the impression that the code enabled the device clock as soon as the device was bound to a driver. Upon closer inspection this doesn't seem to be the case. as if we're going to abstract the clock management we have the following issues. - If pm-runtime is not enabled then we need something to manage the clocks for the driver. If we put that code in the driver then there is not a lot of point in having the pm-runtime clock code here as the driver really only needs a helper to turn them on and off at the right place. - If just standard power management is enabled, then do we really care about the power consumption of leaving peripherals running when their devices are bound? Managing the device clock optimally is hardly a concern if device drivers are not going to be idled when they are not being used. Many devices are not bound to a power domain handled by runtime PM. Adding a runtime PM dependency to clock handling for those devices doesn't make me really happy. Does automatic clock handling even work at all with runtime PM disabled ? The clk_enable() call seems to come from pm_clk_resume() only, which is used as the runtime_resume handler. with runtime PM disabled that code looks like it won't be called at all. When discussing this on freenode's #armkernel channel, several people including Mark Brown wanted to keep this as it made driver's handling of clocks much easier (there was no longer any need to deal with the clk code when writing a simple driver). My view is it is a pain as we now have a mix of drivers which expect to do their own clock work and some that do not. (It is possible there are even some shmobile drivers that still do their own clock management). We could remove manual clock handling from some drivers, but the drivers that need to handle several clocks will still need to do so manually. As long as the core code allows this (which I think it does, but I'm not too familiar with the code) I won't complain (too much at least :-)). Personally I do not like hiding the implementation of this, as it ends up confusing people when they first come to it. I like explicit implementations as well, but I have to admit that devices that require a single clock and have clock requirements that are in sync with the PM runtime requirements would probably benefit from automatic clock handling, at least from a driver code complexity point of view, to the expense of a less explicit implementation. -- Regards, Laurent Pinchart -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On 12/01/14 22:01, Laurent Pinchart wrote: Hi Ben, On Sunday 12 January 2014 22:54:15 Laurent Pinchart wrote: Hi Ben, Thank you for the patch. On Saturday 11 January 2014 13:06:29 Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). Thinking a bit more about this, I think the approach taken in drivers/sh/pm_runtime.c isn't good. The code enables device clocks when devices are bound to a driver, increasing power consumption when devices are idle. Instead of enabling it for ARCH_SHMOBILE_MULTI I'd like to either add explicit clock support to drivers, or to integrate clocks with runtime PM only. If pm-runtime is enabled, then I believe that the device clocks are kept in sync with the active state of the device, which means that they should be shut down when the device is not needed. There have been recent discussions about this with respect to the PCI bridges used by the USB host system. Given the above, I'm not sure exactly what you mean by the statement "I think the approach taken in drivers/sh/pm_runtime.c isn't good." as if we're going to abstract the clock management we have the following issues. - If pm-runtime is not enabled then we need something to manage the clocks for the driver. If we put that code in the driver then there is not a lot of point in having the pm-runtime clock code here as the driver really only needs a helper to turn them on and off at the right place. - If just standard power management is enabled, then do we really care about the power consumption of leaving peripherals running when their devices are bound? Managing the device clock optimally is hardly a concern if device drivers are not going to be idled when they are not being used. When discussing this on freenode's #armkernel channel, several people including Mark Brown wanted to keep this as it made driver's handling of clocks much easier (there was no longer any need to deal with the clk code when writing a simple driver). My view is it is a pain as we now have a mix of drivers which expect to do their own clock work and some that do not. (It is possible there are even some shmobile drivers that still do their own clock management). Personally I do not like hiding the implementation of this, as it ends up confusing people when they first come to it. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On 13/01/14 00:30, Simon Horman wrote:
On Fri, Jan 10, 2014 at 03:18:15PM +, Ben Dooks wrote:
If the kernel is built to support multi-arm configurmation with shmobile
support built in, then the drivers/sh is not built. This contains drivers
that are essential to devices support by that configuration, including the
PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the
bus clocks for all devices).
If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory,
but ensure that bits that may conflict (drivers/sh/clk if the common
clock framework is not enabled) are built.
The ARCH_SHMOBILE_MULTI was added by efacfce5f8a ("ARM: shmobile: Introduce
ARCH_SHMOBILE_MULTI") but this has only just recently been found due to
building device-tree only kernels.
Cc: Linux Kernel list
Cc: Linus SH list
Cc: Simon Horman
Cc: Magnus Damm
Cc: Greg Kroah-Hartman
Signed-off-by: Ben Dooks
---
drivers/Makefile| 1 +
drivers/sh/Makefile | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/Makefile b/drivers/Makefile
index 8e3b8b0..abc4744 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN)+= sn/
obj-y += firmware/
obj-$(CONFIG_CRYPTO) += crypto/
obj-$(CONFIG_SUPERH) += sh/
+obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += sh/
obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)+= sh/
Can't we just do the following?
I think this is probably a better fix. I am wondering what the original
logic of not doing this was.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On Fri, Jan 10, 2014 at 03:18:15PM +, Ben Dooks wrote:
> If the kernel is built to support multi-arm configurmation with shmobile
> support built in, then the drivers/sh is not built. This contains drivers
> that are essential to devices support by that configuration, including the
> PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the
> bus clocks for all devices).
>
> If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory,
> but ensure that bits that may conflict (drivers/sh/clk if the common
> clock framework is not enabled) are built.
>
> The ARCH_SHMOBILE_MULTI was added by efacfce5f8a ("ARM: shmobile: Introduce
> ARCH_SHMOBILE_MULTI") but this has only just recently been found due to
> building device-tree only kernels.
>
> Cc: Linux Kernel list
> Cc: Linus SH list
> Cc: Simon Horman
> Cc: Magnus Damm
> Cc: Greg Kroah-Hartman
> Signed-off-by: Ben Dooks
> ---
> drivers/Makefile| 1 +
> drivers/sh/Makefile | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 8e3b8b0..abc4744 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN) += sn/
> obj-y+= firmware/
> obj-$(CONFIG_CRYPTO) += crypto/
> obj-$(CONFIG_SUPERH) += sh/
> +obj-$(CONFIG_ARCH_SHMOBILE_MULTI)+= sh/
> obj-$(CONFIG_ARCH_SHMOBILE_LEGACY) += sh/
Can't we just do the following?
obj-$(CONFIG_ARCH_SHMOBILE) += sh/
> ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
> obj-y+= clocksource/
> diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile
> index fc67f56..86604a5 100644
> --- a/drivers/sh/Makefile
> +++ b/drivers/sh/Makefile
> @@ -3,7 +3,10 @@
> #
> obj-y:= intc/
>
> +ifeq ($(CONFIG_COMMON_CLK),n)
> obj-$(CONFIG_HAVE_CLK) += clk/
> +endif
> +
> obj-$(CONFIG_MAPLE) += maple/
> obj-$(CONFIG_SUPERHYWAY) += superhyway/
>
> --
> 1.8.5.2
>
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben,
On Sunday 12 January 2014 22:54:15 Laurent Pinchart wrote:
> Hi Ben,
>
> Thank you for the patch.
>
> On Saturday 11 January 2014 13:06:29 Ben Dooks wrote:
> > If the kernel is built to support multi-arm configurmation with shmobile
> > support built in, then the drivers/sh is not built. This contains drivers
> > that are essential to devices support by that configuration, including the
> > PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the
> > bus clocks for all devices).
Thinking a bit more about this, I think the approach taken in
drivers/sh/pm_runtime.c isn't good. The code enables device clocks when
devices are bound to a driver, increasing power consumption when devices are
idle. Instead of enabling it for ARCH_SHMOBILE_MULTI I'd like to either add
explicit clock support to drivers, or to integrate clocks with runtime PM
only.
> > If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory,
> > but ensure that bits that may conflict (drivers/sh/clk if the common
> > clock framework is not enabled) are built.
> >
> > The ARCH_SHMOBILE_MULTI was added by efacfce5f8a ("ARM: shmobile:
> > Introduce ARCH_SHMOBILE_MULTI") but this has only just recently been found
> > due to building device-tree only kernels.
> >
> > Cc: Linux Kernel list
> > Cc: Linus SH list
> > Cc: Simon Horman
> > Cc: Magnus Damm
> > Cc: Greg Kroah-Hartman
> > Signed-off-by: Ben Dooks
> > ---
> >
> > drivers/Makefile| 1 +
> > drivers/sh/Makefile | 3 +++
> > 2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 8e3b8b0..abc4744 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN)+= sn/
> >
> > obj-y += firmware/
> > obj-$(CONFIG_CRYPTO) += crypto/
> > obj-$(CONFIG_SUPERH) += sh/
> >
> > +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += sh/
> >
> > obj-$(CONFIG_ARCH_SHMOBILE_LEGACY) += sh/
> > ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
> > obj-y += clocksource/
> >
> > diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile
> > index fc67f56..86604a5 100644
> > --- a/drivers/sh/Makefile
> > +++ b/drivers/sh/Makefile
> > @@ -3,7 +3,10 @@
> >
> > #
> > obj-y := intc/
>
> Is intc needed as well ?
>
> > +ifeq ($(CONFIG_COMMON_CLK),n)
> >
> > obj-$(CONFIG_HAVE_CLK) += clk/
> >
> > +endif
> > +
> >
> > obj-$(CONFIG_MAPLE)+= maple/
> > obj-$(CONFIG_SUPERHYWAY) += superhyway/
--
Regards,
Laurent Pinchart
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben,
Thank you for the patch.
On Saturday 11 January 2014 13:06:29 Ben Dooks wrote:
> If the kernel is built to support multi-arm configurmation with shmobile
> support built in, then the drivers/sh is not built. This contains drivers
> that are essential to devices support by that configuration, including the
> PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the
> bus clocks for all devices).
>
> If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory,
> but ensure that bits that may conflict (drivers/sh/clk if the common
> clock framework is not enabled) are built.
>
> The ARCH_SHMOBILE_MULTI was added by efacfce5f8a ("ARM: shmobile: Introduce
> ARCH_SHMOBILE_MULTI") but this has only just recently been found due to
> building device-tree only kernels.
>
> Cc: Linux Kernel list
> Cc: Linus SH list
> Cc: Simon Horman
> Cc: Magnus Damm
> Cc: Greg Kroah-Hartman
> Signed-off-by: Ben Dooks
> ---
> drivers/Makefile| 1 +
> drivers/sh/Makefile | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 8e3b8b0..abc4744 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN) += sn/
> obj-y+= firmware/
> obj-$(CONFIG_CRYPTO) += crypto/
> obj-$(CONFIG_SUPERH) += sh/
> +obj-$(CONFIG_ARCH_SHMOBILE_MULTI)+= sh/
> obj-$(CONFIG_ARCH_SHMOBILE_LEGACY) += sh/
> ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
> obj-y+= clocksource/
> diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile
> index fc67f56..86604a5 100644
> --- a/drivers/sh/Makefile
> +++ b/drivers/sh/Makefile
> @@ -3,7 +3,10 @@
> #
> obj-y:= intc/
Is intc needed as well ?
>
> +ifeq ($(CONFIG_COMMON_CLK),n)
> obj-$(CONFIG_HAVE_CLK) += clk/
> +endif
> +
> obj-$(CONFIG_MAPLE) += maple/
> obj-$(CONFIG_SUPERHYWAY) += superhyway/
--
Regards,
Laurent Pinchart
--
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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben, Thank you for the patch. On Saturday 11 January 2014 13:06:29 Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that bits that may conflict (drivers/sh/clk if the common clock framework is not enabled) are built. The ARCH_SHMOBILE_MULTI was added by efacfce5f8a (ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI) but this has only just recently been found due to building device-tree only kernels. Cc: Linux Kernel list linux-kernel@vger.kernel.org Cc: Linus SH list linux...@vger.kernel.org Cc: Simon Horman ho...@verge.net.au Cc: Magnus Damm magnus.d...@gmail.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Ben Dooks ben.do...@codethink.co.uk --- drivers/Makefile| 1 + drivers/sh/Makefile | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/Makefile b/drivers/Makefile index 8e3b8b0..abc4744 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN) += sn/ obj-y+= firmware/ obj-$(CONFIG_CRYPTO) += crypto/ obj-$(CONFIG_SUPERH) += sh/ +obj-$(CONFIG_ARCH_SHMOBILE_MULTI)+= sh/ obj-$(CONFIG_ARCH_SHMOBILE_LEGACY) += sh/ ifndef CONFIG_ARCH_USES_GETTIMEOFFSET obj-y+= clocksource/ diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile index fc67f56..86604a5 100644 --- a/drivers/sh/Makefile +++ b/drivers/sh/Makefile @@ -3,7 +3,10 @@ # obj-y:= intc/ Is intc needed as well ? +ifeq ($(CONFIG_COMMON_CLK),n) obj-$(CONFIG_HAVE_CLK) += clk/ +endif + obj-$(CONFIG_MAPLE) += maple/ obj-$(CONFIG_SUPERHYWAY) += superhyway/ -- Regards, Laurent Pinchart -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
Hi Ben, On Sunday 12 January 2014 22:54:15 Laurent Pinchart wrote: Hi Ben, Thank you for the patch. On Saturday 11 January 2014 13:06:29 Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). Thinking a bit more about this, I think the approach taken in drivers/sh/pm_runtime.c isn't good. The code enables device clocks when devices are bound to a driver, increasing power consumption when devices are idle. Instead of enabling it for ARCH_SHMOBILE_MULTI I'd like to either add explicit clock support to drivers, or to integrate clocks with runtime PM only. If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that bits that may conflict (drivers/sh/clk if the common clock framework is not enabled) are built. The ARCH_SHMOBILE_MULTI was added by efacfce5f8a (ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI) but this has only just recently been found due to building device-tree only kernels. Cc: Linux Kernel list linux-kernel@vger.kernel.org Cc: Linus SH list linux...@vger.kernel.org Cc: Simon Horman ho...@verge.net.au Cc: Magnus Damm magnus.d...@gmail.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Ben Dooks ben.do...@codethink.co.uk --- drivers/Makefile| 1 + drivers/sh/Makefile | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/Makefile b/drivers/Makefile index 8e3b8b0..abc4744 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN)+= sn/ obj-y += firmware/ obj-$(CONFIG_CRYPTO) += crypto/ obj-$(CONFIG_SUPERH) += sh/ +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += sh/ obj-$(CONFIG_ARCH_SHMOBILE_LEGACY) += sh/ ifndef CONFIG_ARCH_USES_GETTIMEOFFSET obj-y += clocksource/ diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile index fc67f56..86604a5 100644 --- a/drivers/sh/Makefile +++ b/drivers/sh/Makefile @@ -3,7 +3,10 @@ # obj-y := intc/ Is intc needed as well ? +ifeq ($(CONFIG_COMMON_CLK),n) obj-$(CONFIG_HAVE_CLK) += clk/ +endif + obj-$(CONFIG_MAPLE)+= maple/ obj-$(CONFIG_SUPERHYWAY) += superhyway/ -- Regards, Laurent Pinchart -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On Fri, Jan 10, 2014 at 03:18:15PM +, Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that bits that may conflict (drivers/sh/clk if the common clock framework is not enabled) are built. The ARCH_SHMOBILE_MULTI was added by efacfce5f8a (ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI) but this has only just recently been found due to building device-tree only kernels. Cc: Linux Kernel list linux-kernel@vger.kernel.org Cc: Linus SH list linux...@vger.kernel.org Cc: Simon Horman ho...@verge.net.au Cc: Magnus Damm magnus.d...@gmail.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Ben Dooks ben.do...@codethink.co.uk --- drivers/Makefile| 1 + drivers/sh/Makefile | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/Makefile b/drivers/Makefile index 8e3b8b0..abc4744 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN) += sn/ obj-y+= firmware/ obj-$(CONFIG_CRYPTO) += crypto/ obj-$(CONFIG_SUPERH) += sh/ +obj-$(CONFIG_ARCH_SHMOBILE_MULTI)+= sh/ obj-$(CONFIG_ARCH_SHMOBILE_LEGACY) += sh/ Can't we just do the following? obj-$(CONFIG_ARCH_SHMOBILE) += sh/ ifndef CONFIG_ARCH_USES_GETTIMEOFFSET obj-y+= clocksource/ diff --git a/drivers/sh/Makefile b/drivers/sh/Makefile index fc67f56..86604a5 100644 --- a/drivers/sh/Makefile +++ b/drivers/sh/Makefile @@ -3,7 +3,10 @@ # obj-y:= intc/ +ifeq ($(CONFIG_COMMON_CLK),n) obj-$(CONFIG_HAVE_CLK) += clk/ +endif + obj-$(CONFIG_MAPLE) += maple/ obj-$(CONFIG_SUPERHYWAY) += superhyway/ -- 1.8.5.2 -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On 13/01/14 00:30, Simon Horman wrote: On Fri, Jan 10, 2014 at 03:18:15PM +, Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). If CONFIG_ARCH_SHMOBILE_MULTI then build the drivers/sh directory, but ensure that bits that may conflict (drivers/sh/clk if the common clock framework is not enabled) are built. The ARCH_SHMOBILE_MULTI was added by efacfce5f8a (ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI) but this has only just recently been found due to building device-tree only kernels. Cc: Linux Kernel list linux-kernel@vger.kernel.org Cc: Linus SH list linux...@vger.kernel.org Cc: Simon Horman ho...@verge.net.au Cc: Magnus Damm magnus.d...@gmail.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Ben Dooks ben.do...@codethink.co.uk --- drivers/Makefile| 1 + drivers/sh/Makefile | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/Makefile b/drivers/Makefile index 8e3b8b0..abc4744 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -118,6 +118,7 @@ obj-$(CONFIG_SGI_SN)+= sn/ obj-y += firmware/ obj-$(CONFIG_CRYPTO) += crypto/ obj-$(CONFIG_SUPERH) += sh/ +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += sh/ obj-$(CONFIG_ARCH_SHMOBILE_LEGACY)+= sh/ Can't we just do the following? I think this is probably a better fix. I am wondering what the original logic of not doing this was. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- 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/
Re: [PATCH] ARM: shmobile: compile drivers/sh for CONFIG_ARCH_SHMOBILE_MULTI
On 12/01/14 22:01, Laurent Pinchart wrote: Hi Ben, On Sunday 12 January 2014 22:54:15 Laurent Pinchart wrote: Hi Ben, Thank you for the patch. On Saturday 11 January 2014 13:06:29 Ben Dooks wrote: If the kernel is built to support multi-arm configurmation with shmobile support built in, then the drivers/sh is not built. This contains drivers that are essential to devices support by that configuration, including the PM runtime code in drivers/sh/pm_runtime.c (which implicitly enables the bus clocks for all devices). Thinking a bit more about this, I think the approach taken in drivers/sh/pm_runtime.c isn't good. The code enables device clocks when devices are bound to a driver, increasing power consumption when devices are idle. Instead of enabling it for ARCH_SHMOBILE_MULTI I'd like to either add explicit clock support to drivers, or to integrate clocks with runtime PM only. If pm-runtime is enabled, then I believe that the device clocks are kept in sync with the active state of the device, which means that they should be shut down when the device is not needed. There have been recent discussions about this with respect to the PCI bridges used by the USB host system. Given the above, I'm not sure exactly what you mean by the statement I think the approach taken in drivers/sh/pm_runtime.c isn't good. as if we're going to abstract the clock management we have the following issues. - If pm-runtime is not enabled then we need something to manage the clocks for the driver. If we put that code in the driver then there is not a lot of point in having the pm-runtime clock code here as the driver really only needs a helper to turn them on and off at the right place. - If just standard power management is enabled, then do we really care about the power consumption of leaving peripherals running when their devices are bound? Managing the device clock optimally is hardly a concern if device drivers are not going to be idled when they are not being used. When discussing this on freenode's #armkernel channel, several people including Mark Brown wanted to keep this as it made driver's handling of clocks much easier (there was no longer any need to deal with the clk code when writing a simple driver). My view is it is a pain as we now have a mix of drivers which expect to do their own clock work and some that do not. (It is possible there are even some shmobile drivers that still do their own clock management). Personally I do not like hiding the implementation of this, as it ends up confusing people when they first come to it. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- 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/
