| Message ID | 588ea0a3175fcf5d409ca32249f24760f2f6f839.1475990489.git.dalias@libc.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rich, On Sun, Oct 09, 2016 at 05:34:22AM +0000, Rich Felker wrote: > At the hardware level, the J-Core PIT is integrated with the interrupt > controller, but it is represented as its own device and has an > independent programming interface. It provides a 12-bit countdown > timer, which is not presently used, and a periodic timer. The interval > length for the latter is programmable via a 32-bit throttle register > whose units are determined by a bus-period register. The periodic > timer is used to implement both periodic and oneshot clock event > modes; in oneshot mode the interrupt handler simply disables the timer > as soon as it fires. > > Despite its device tree node representing an interrupt for the PIT, > the actual irq generated is programmable, not hard-wired. The driver > is responsible for programming the PIT to generate the hardware irq > number that the DT assigns to it. > > On SMP configurations, J-Core provides cpu-local instances of the PIT; > no broadcast timer is needed. This driver supports the creation of the > necessary per-cpu clock_event_device instances. For my personnal information, why no broadcast timer is needed ? Are the CPUs on always-on power down ? > A nanosecond-resolution clocksource is provided using the J-Core "RTC" > registers, which give a 64-bit seconds count and 32-bit nanoseconds > that wrap every second. The driver converts these to a full-range > 32-bit nanoseconds count. > > Signed-off-by: Rich Felker <dalias@libc.org> > --- > drivers/clocksource/Kconfig | 10 ++ > drivers/clocksource/Makefile | 1 + > drivers/clocksource/jcore-pit.c | 231 ++++++++++++++++++++++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 4 files changed, 243 insertions(+) > create mode 100644 drivers/clocksource/jcore-pit.c > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 5677886..95dd78b 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -407,6 +407,16 @@ config SYS_SUPPORTS_SH_TMU > config SYS_SUPPORTS_EM_STI > bool > > +config CLKSRC_JCORE_PIT > + bool "J-Core PIT timer driver" > + depends on OF && (SUPERH || COMPILE_TEST) Actually the idea is to have the SUPERH to select this timer, not create a dependency on SUPERH from here. We don't want to prompt in the configuration menu the drivers because it would be impossible to anyone to know which timer comes with which hardware, so we let the platform to select the timer it needs. The exception is to enable in order to compile on non-native platform to compile-test a bunch of drivers (eg. compile most of the clocksource / clockevents drivers on a x86 big server). That's why we should have: config CLKSRC_JCORE_PIT bool "J-Core PIT timer driver" if COMPILE_TEST So the jcore pit driver option appears only if compile test is enabled otherwise it is a silent option and the user doesn't have to deal with it. Having consistent dependency like the other drivers will help future changes to consolidate the Kconfig. [ ... ] > +#define REG_PITEN 0x00 > +#define REG_THROT 0x10 > +#define REG_COUNT 0x14 > +#define REG_BUSPD 0x18 > +#define REG_SECHI 0x20 > +#define REG_SECLO 0x24 > +#define REG_NSEC 0x28 > + > +struct jcore_pit { > + struct clock_event_device ced; > + __iomem void *base; It is not '__iomem void *' but 'void __iomem *'. This appears multiple times in this code. > + unsigned long periodic_delta; > + unsigned cpu; This field is pointless, 'cpu' is only used for a trace in the init function which has already the cpu has parameter. > + u32 enable_val; > +}; > + > +static __iomem void *jcore_pit_base; static void __iomem *jcore_pit_base; > +struct jcore_pit __percpu *jcore_pit_percpu; static. [ ... ] > +static int __init jcore_pit_init(struct device_node *node) > +{ [ ... ] > + /* > + * The J-Core PIT is not hard-wired to a particular IRQ, but > + * integrated with the interrupt controller such that the IRQ it > + * generates is programmable. The programming interface has a > + * legacy field which was an interrupt priority for AIC1, but > + * which is OR'd onto bits 2-5 of the generated IRQ number when > + * used with J-Core AIC2, so set it to match these bits. > + */ > + hwirq = irq_get_irq_data(pit_irq)->hwirq; > + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; > + enable_val = (1U << PIT_ENABLE_SHIFT) > + | (hwirq << PIT_IRQ_SHIFT) > + | (irqprio << PIT_PRIO_SHIFT); > + Why mention AIC1 if there is not test to check if AIC1 || AIC2 ? Will be the same information available if the irqchip is AIC1 ? I have to admin I find strange this driver has to invoke irq_get_irq_data(), it is the only one and it sounds even strange it has to be stored per cpu below. > + for_each_present_cpu(cpu) { [ ... ] > + pit->enable_val = enable_val; > + } > + > + cpuhp_setup_state(CPUHP_AP_JCORE_TIMER_STARTING, > + "AP_JCORE_TIMER_STARTING", > + jcore_pit_local_init, NULL); > + > + return 0; > +} > + Thanks ! -- Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 11, 2016 at 08:18:12PM +0200, Daniel Lezcano wrote: > > Hi Rich, > > On Sun, Oct 09, 2016 at 05:34:22AM +0000, Rich Felker wrote: > > At the hardware level, the J-Core PIT is integrated with the interrupt > > controller, but it is represented as its own device and has an > > independent programming interface. It provides a 12-bit countdown > > timer, which is not presently used, and a periodic timer. The interval > > length for the latter is programmable via a 32-bit throttle register > > whose units are determined by a bus-period register. The periodic > > timer is used to implement both periodic and oneshot clock event > > modes; in oneshot mode the interrupt handler simply disables the timer > > as soon as it fires. > > > > Despite its device tree node representing an interrupt for the PIT, > > the actual irq generated is programmable, not hard-wired. The driver > > is responsible for programming the PIT to generate the hardware irq > > number that the DT assigns to it. > > > > On SMP configurations, J-Core provides cpu-local instances of the PIT; > > no broadcast timer is needed. This driver supports the creation of the > > necessary per-cpu clock_event_device instances. > > For my personnal information, why no broadcast timer is needed ? Broadcast timer is only needed if you don't have percpu local timers. Early on in SMP development I actually tested with an ipi broadcast timer and performance was noticably worse. > Are the CPUs on always-on power down ? For now they are always on and don't even have the sleep instruction (i.e. stop cpu clock until interrupt) implemented. Adding sleep will be the first power-saving step, and perhaps the only one for now, since there doesn't seem to be any indication (according to the ppl working on the hardware) that a deeper sleep would provide significant additional savings. > > A nanosecond-resolution clocksource is provided using the J-Core "RTC" > > registers, which give a 64-bit seconds count and 32-bit nanoseconds > > that wrap every second. The driver converts these to a full-range > > 32-bit nanoseconds count. > > > > Signed-off-by: Rich Felker <dalias@libc.org> > > --- > > drivers/clocksource/Kconfig | 10 ++ > > drivers/clocksource/Makefile | 1 + > > drivers/clocksource/jcore-pit.c | 231 ++++++++++++++++++++++++++++++++++++++++ > > include/linux/cpuhotplug.h | 1 + > > 4 files changed, 243 insertions(+) > > create mode 100644 drivers/clocksource/jcore-pit.c > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > index 5677886..95dd78b 100644 > > --- a/drivers/clocksource/Kconfig > > +++ b/drivers/clocksource/Kconfig > > @@ -407,6 +407,16 @@ config SYS_SUPPORTS_SH_TMU > > config SYS_SUPPORTS_EM_STI > > bool > > > > +config CLKSRC_JCORE_PIT > > + bool "J-Core PIT timer driver" > > + depends on OF && (SUPERH || COMPILE_TEST) > > Actually the idea is to have the SUPERH to select this timer, not create > a dependency on SUPERH from here. > > We don't want to prompt in the configuration menu the drivers because it > would be impossible to anyone to know which timer comes with which > hardware, so we let the platform to select the timer it needs. I thought we discussed this before. For users building a kernel for legacy SH systems, especially in the current state where they're only supported with hard-coded board files rather than device tree, it makes no sense to build drivers for J-core hardware. It would make sense to be on by default for CONFIG_SH_DEVICE_TREE with a compatible CPU selection, but at least at this time, not for SUPERH in general. Anyway I'd really like to do this non-invasively as long as we have a mix of legacy and new stuff and the legacy stuff is not readily testable. Once all of arch/sh is moved over to device tree, could we revisit this and make all the drivers follow a common policy (on by default if they're associated with boards/SoCs using a matching or compatible CPU model, or something like that, but still able to be disabled manually, since the user might be trying to get a tiny-ish embedded kernel)? > The exception is to enable in order to compile on non-native platform to > compile-test a bunch of drivers (eg. compile most of the clocksource / > clockevents drivers on a x86 big server). > > That's why we should have: > > config CLKSRC_JCORE_PIT > bool "J-Core PIT timer driver" if COMPILE_TEST > > So the jcore pit driver option appears only if compile test is enabled > otherwise it is a silent option and the user doesn't have to deal with > it. Having consistent dependency like the other drivers will help future > changes to consolidate the Kconfig. I don't think this matches the user expectation for the arch yet. For now we have a j2_defconfig that enables the drivers and other kernel settings expected to be useful for J-core socs. I want to modernize this all but that's a separate project. > [ ... ] > > > +#define REG_PITEN 0x00 > > +#define REG_THROT 0x10 > > +#define REG_COUNT 0x14 > > +#define REG_BUSPD 0x18 > > +#define REG_SECHI 0x20 > > +#define REG_SECLO 0x24 > > +#define REG_NSEC 0x28 > > + > > +struct jcore_pit { > > + struct clock_event_device ced; > > + __iomem void *base; > > It is not '__iomem void *' but 'void __iomem *'. This appears multiple > times in this code. OK, I'll change that. > > + unsigned long periodic_delta; > > + unsigned cpu; > > This field is pointless, 'cpu' is only used for a trace in the init > function which has already the cpu has parameter. OK, will remove. It was needed for the old notify framework but not for cpu hotplug framework, I think. > > + u32 enable_val; > > +}; > > + > > +static __iomem void *jcore_pit_base; > > static void __iomem *jcore_pit_base; OK. > > +struct jcore_pit __percpu *jcore_pit_percpu; > > static. OK. > [ ... ] > > > +static int __init jcore_pit_init(struct device_node *node) > > +{ > > [ ... ] Was there something analogous you wanted me to change here, or just leftover quoting? > > + /* > > + * The J-Core PIT is not hard-wired to a particular IRQ, but > > + * integrated with the interrupt controller such that the IRQ it > > + * generates is programmable. The programming interface has a > > + * legacy field which was an interrupt priority for AIC1, but > > + * which is OR'd onto bits 2-5 of the generated IRQ number when > > + * used with J-Core AIC2, so set it to match these bits. > > + */ > > + hwirq = irq_get_irq_data(pit_irq)->hwirq; > > + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; > > + enable_val = (1U << PIT_ENABLE_SHIFT) > > + | (hwirq << PIT_IRQ_SHIFT) > > + | (irqprio << PIT_PRIO_SHIFT); > > + > > Why mention AIC1 if there is not test to check if AIC1 || AIC2 ? > > Will be the same information available if the irqchip is AIC1 ? The bit layout of the PIT enable register is: .....e..ppppiiiiiiii............ where the .'s indicate unrelated/unused bits, e is enable, p is priority, and i is hard irq number. For the PIT included in AIC1 (obsolete but still in use), any hard irq (trap number) can be programmed via the 8 iiiiiiii bits, and a priority (0-15) is programmable separately in the pppp bits. For the PIT included in AIC2 (current), the programming interface is equivalent modulo interrupt mapping. This is why a different compatible tag was not used. However only traps 64-127 (the ones actually intended to be used for interrupts, rather than syscalls/exceptions/etc.) can be programmed (the high 2 bits of i are ignored) and the priority pppp is <<2'd and or'd onto the irq number. This was a poor decision made on the hardware engineering side based on a wrong assumption that preserving old priority mapping of outdated software was important, whereas priorities weren't/aren't even being used. When we do the next round of interrupt controller improvements (AIC3) the PIT programming interface should remain compatible with the driver; likely the priority bits will just be ignored. If we do want to change the programming interface beyond this at some point (that maay be a good idea, since we have identified several things that are less than ideal for Linux, like the sechi/seclo/ns clocksource), a new compatible tag will be added instead. > I have to admin I find strange this driver has to invoke irq_get_irq_data(), > it is the only one and it sounds even strange it has to be stored per cpu below. The timer will not actually generate the irq it's assigned to (or any interrupt at all) unless/until it's programmed for the (hard) irq number. The need to use irq_get_irq_data comes from the fact that the hardware needs the hard irq number, not a virq number, which could in principle be different. There's probably some argument that could have been made that the irqchip and clocksource/timer driver should have been unified since they're unified in the hardware and have this awkward programming relationship, but that didn't fit the Linux model very well and I think having them factored like this encourages future versions of the hardware to adapt to the model the software wants. Rich -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 11, 2016 at 04:28:50PM -0400, Rich Felker wrote: > On Tue, Oct 11, 2016 at 08:18:12PM +0200, Daniel Lezcano wrote: > > > > Hi Rich, > > > > On Sun, Oct 09, 2016 at 05:34:22AM +0000, Rich Felker wrote: > > > At the hardware level, the J-Core PIT is integrated with the interrupt > > > controller, but it is represented as its own device and has an > > > independent programming interface. It provides a 12-bit countdown > > > timer, which is not presently used, and a periodic timer. The interval > > > length for the latter is programmable via a 32-bit throttle register > > > whose units are determined by a bus-period register. The periodic > > > timer is used to implement both periodic and oneshot clock event > > > modes; in oneshot mode the interrupt handler simply disables the timer > > > as soon as it fires. > > > > > > Despite its device tree node representing an interrupt for the PIT, > > > the actual irq generated is programmable, not hard-wired. The driver > > > is responsible for programming the PIT to generate the hardware irq > > > number that the DT assigns to it. > > > > > > On SMP configurations, J-Core provides cpu-local instances of the PIT; > > > no broadcast timer is needed. This driver supports the creation of the > > > necessary per-cpu clock_event_device instances. > > > > For my personnal information, why no broadcast timer is needed ? > > Broadcast timer is only needed if you don't have percpu local timers. > Early on in SMP development I actually tested with an ipi broadcast > timer and performance was noticably worse. Obviously. I thought there were another reason related to power management. > > Are the CPUs on always-on power down ? > > For now they are always on and don't even have the sleep instruction > (i.e. stop cpu clock until interrupt) implemented. Adding sleep will > be the first power-saving step, and perhaps the only one for now, > since there doesn't seem to be any indication (according to the ppl > working on the hardware) that a deeper sleep would provide significant > additional savings. Ok. However, the 'sleep' state is not, in the power management terminology, the idle state described above. It is called "clock gated" / "Wait for Interrupt". The 'sleep' state lose the CPU context. > > > A nanosecond-resolution clocksource is provided using the J-Core "RTC" > > > registers, which give a 64-bit seconds count and 32-bit nanoseconds > > > that wrap every second. The driver converts these to a full-range > > > 32-bit nanoseconds count. > > > > > > Signed-off-by: Rich Felker <dalias@libc.org> > > > --- > > > drivers/clocksource/Kconfig | 10 ++ > > > drivers/clocksource/Makefile | 1 + > > > drivers/clocksource/jcore-pit.c | 231 ++++++++++++++++++++++++++++++++++++++++ > > > include/linux/cpuhotplug.h | 1 + > > > 4 files changed, 243 insertions(+) > > > create mode 100644 drivers/clocksource/jcore-pit.c > > > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > > index 5677886..95dd78b 100644 > > > --- a/drivers/clocksource/Kconfig > > > +++ b/drivers/clocksource/Kconfig > > > @@ -407,6 +407,16 @@ config SYS_SUPPORTS_SH_TMU > > > config SYS_SUPPORTS_EM_STI > > > bool > > > > > > +config CLKSRC_JCORE_PIT > > > + bool "J-Core PIT timer driver" > > > + depends on OF && (SUPERH || COMPILE_TEST) > > > > Actually the idea is to have the SUPERH to select this timer, not create > > a dependency on SUPERH from here. > > > > We don't want to prompt in the configuration menu the drivers because it > > would be impossible to anyone to know which timer comes with which > > hardware, so we let the platform to select the timer it needs. > > I thought we discussed this before. For users building a kernel for > legacy SH systems, especially in the current state where they're only > supported with hard-coded board files rather than device tree, it > makes no sense to build drivers for J-core hardware. It would make > sense to be on by default for CONFIG_SH_DEVICE_TREE with a compatible > CPU selection, but at least at this time, not for SUPERH in general. Probably I am missing the point but why the user would have to unselect this driver manually ? The user wants a config file nothing more or a very trivial option. Can you imagine someone can know every single IP block for each boards of the same arch and be able to disable/enable the right ones ? > Anyway I'd really like to do this non-invasively as long as we have a > mix of legacy and new stuff and the legacy stuff is not readily > testable. Once all of arch/sh is moved over to device tree, could we > revisit this and make all the drivers follow a common policy (on by > default if they're associated with boards/SoCs using a matching or > compatible CPU model, or something like that, but still able to be > disabled manually, since the user might be trying to get a tiny-ish > embedded kernel)? I understand the goal is to have one single configuration and everything DT based and it sounds great but what is missing here is just a subarch, not an option to enable/disable the timer. Give a try with: make ARCH=arm multi_v7_defconfig menuconfig --> System Type That is what you are looking for, a SUPERH config option selecting all the common options and then a JCORE config option adding the different missing bits, namely the CLKSRC_JCORE_PIT. > > The exception is to enable in order to compile on non-native platform to > > compile-test a bunch of drivers (eg. compile most of the clocksource / > > clockevents drivers on a x86 big server). > > > > That's why we should have: > > > > config CLKSRC_JCORE_PIT > > bool "J-Core PIT timer driver" if COMPILE_TEST > > > > So the jcore pit driver option appears only if compile test is enabled > > otherwise it is a silent option and the user doesn't have to deal with > > it. Having consistent dependency like the other drivers will help future > > changes to consolidate the Kconfig. > > I don't think this matches the user expectation for the arch yet. For > now we have a j2_defconfig that enables the drivers and other kernel > settings expected to be useful for J-core socs. I want to modernize > this all but that's a separate project. > > > [ ... ] > > > > > +#define REG_PITEN 0x00 > > > +#define REG_THROT 0x10 > > > +#define REG_COUNT 0x14 > > > +#define REG_BUSPD 0x18 > > > +#define REG_SECHI 0x20 > > > +#define REG_SECLO 0x24 > > > +#define REG_NSEC 0x28 > > > + > > > +struct jcore_pit { > > > + struct clock_event_device ced; > > > + __iomem void *base; > > > > It is not '__iomem void *' but 'void __iomem *'. This appears multiple > > times in this code. > > OK, I'll change that. > > > > + unsigned long periodic_delta; > > > + unsigned cpu; > > > > This field is pointless, 'cpu' is only used for a trace in the init > > function which has already the cpu has parameter. > > OK, will remove. It was needed for the old notify framework but not > for cpu hotplug framework, I think. > > > > + u32 enable_val; > > > +}; > > > + > > > +static __iomem void *jcore_pit_base; > > > > static void __iomem *jcore_pit_base; > > OK. > > > > +struct jcore_pit __percpu *jcore_pit_percpu; > > > > static. > > OK. > > > [ ... ] > > > > > +static int __init jcore_pit_init(struct device_node *node) > > > +{ > > > > [ ... ] > > Was there something analogous you wanted me to change here, or just > leftover quoting? No, I just cut until the function name in order to give the context for the next comment and then cut inside the function. > > > + /* > > > + * The J-Core PIT is not hard-wired to a particular IRQ, but > > > + * integrated with the interrupt controller such that the IRQ it > > > + * generates is programmable. The programming interface has a > > > + * legacy field which was an interrupt priority for AIC1, but > > > + * which is OR'd onto bits 2-5 of the generated IRQ number when > > > + * used with J-Core AIC2, so set it to match these bits. > > > + */ > > > + hwirq = irq_get_irq_data(pit_irq)->hwirq; > > > + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; > > > + enable_val = (1U << PIT_ENABLE_SHIFT) > > > + | (hwirq << PIT_IRQ_SHIFT) > > > + | (irqprio << PIT_PRIO_SHIFT); > > > + > > > > Why mention AIC1 if there is not test to check if AIC1 || AIC2 ? > > > > Will be the same information available if the irqchip is AIC1 ? > > The bit layout of the PIT enable register is: > > .....e..ppppiiiiiiii............ > > where the .'s indicate unrelated/unused bits, e is enable, p is > priority, and i is hard irq number. > > For the PIT included in AIC1 (obsolete but still in use), any hard irq > (trap number) can be programmed via the 8 iiiiiiii bits, and a > priority (0-15) is programmable separately in the pppp bits. > > For the PIT included in AIC2 (current), the programming interface is > equivalent modulo interrupt mapping. This is why a different > compatible tag was not used. However only traps 64-127 (the ones > actually intended to be used for interrupts, rather than > syscalls/exceptions/etc.) can be programmed (the high 2 bits of i are > ignored) and the priority pppp is <<2'd and or'd onto the irq number. > This was a poor decision made on the hardware engineering side based > on a wrong assumption that preserving old priority mapping of outdated > software was important, whereas priorities weren't/aren't even being > used. > > When we do the next round of interrupt controller improvements (AIC3) > the PIT programming interface should remain compatible with the > driver; likely the priority bits will just be ignored. > > If we do want to change the programming interface beyond this at some > point (that maay be a good idea, since we have identified several > things that are less than ideal for Linux, like the sechi/seclo/ns > clocksource), a new compatible tag will be added instead. Ok, thanks for the clarification. Can you add your answer as a comment for the bits dance above ? > > I have to admin I find strange this driver has to invoke irq_get_irq_data(), > > it is the only one and it sounds even strange it has to be stored per cpu below. > > The timer will not actually generate the irq it's assigned to (or any > interrupt at all) unless/until it's programmed for the (hard) irq > number. The need to use irq_get_irq_data comes from the fact that the > hardware needs the hard irq number, not a virq number, which could in > principle be different. > > There's probably some argument that could have been made that the > irqchip and clocksource/timer driver should have been unified since > they're unified in the hardware and have this awkward programming > relationship, but that didn't fit the Linux model very well and I > think having them factored like this encourages future versions of the > hardware to adapt to the model the software wants. Ok, thanks. -- Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 12, 2016 at 11:27:11AM +0200, Daniel Lezcano wrote: > > > Are the CPUs on always-on power down ? > > > > For now they are always on and don't even have the sleep instruction > > (i.e. stop cpu clock until interrupt) implemented. Adding sleep will > > be the first power-saving step, and perhaps the only one for now, > > since there doesn't seem to be any indication (according to the ppl > > working on the hardware) that a deeper sleep would provide significant > > additional savings. > > Ok. > > However, the 'sleep' state is not, in the power management terminology, > the idle state described above. It is called "clock gated" / "Wait for > Interrupt". > > The 'sleep' state lose the CPU context. I use the term "sleep" because that's the name of the SH instruction mnemonic for the opcode for entering the wait-for-interrupt state. Sorry it's confusing like the "RTC" all over again. :-) > > > > A nanosecond-resolution clocksource is provided using the J-Core "RTC" > > > > registers, which give a 64-bit seconds count and 32-bit nanoseconds > > > > that wrap every second. The driver converts these to a full-range > > > > 32-bit nanoseconds count. > > > > > > > > Signed-off-by: Rich Felker <dalias@libc.org> > > > > --- > > > > drivers/clocksource/Kconfig | 10 ++ > > > > drivers/clocksource/Makefile | 1 + > > > > drivers/clocksource/jcore-pit.c | 231 ++++++++++++++++++++++++++++++++++++++++ > > > > include/linux/cpuhotplug.h | 1 + > > > > 4 files changed, 243 insertions(+) > > > > create mode 100644 drivers/clocksource/jcore-pit.c > > > > > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > > > index 5677886..95dd78b 100644 > > > > --- a/drivers/clocksource/Kconfig > > > > +++ b/drivers/clocksource/Kconfig > > > > @@ -407,6 +407,16 @@ config SYS_SUPPORTS_SH_TMU > > > > config SYS_SUPPORTS_EM_STI > > > > bool > > > > > > > > +config CLKSRC_JCORE_PIT > > > > + bool "J-Core PIT timer driver" > > > > + depends on OF && (SUPERH || COMPILE_TEST) > > > > > > Actually the idea is to have the SUPERH to select this timer, not create > > > a dependency on SUPERH from here. > > > > > > We don't want to prompt in the configuration menu the drivers because it > > > would be impossible to anyone to know which timer comes with which > > > hardware, so we let the platform to select the timer it needs. > > > > I thought we discussed this before. For users building a kernel for > > legacy SH systems, especially in the current state where they're only > > supported with hard-coded board files rather than device tree, it > > makes no sense to build drivers for J-core hardware. It would make > > sense to be on by default for CONFIG_SH_DEVICE_TREE with a compatible > > CPU selection, but at least at this time, not for SUPERH in general. > > Probably I am missing the point but why the user would have to unselect > this driver manually ? The user wants a config file nothing more or a very > trivial option. Can you imagine someone can know every single IP block for > each boards of the same arch and be able to disable/enable the right ones ? The common case I imagine is just accepting defaults that include more than you need, but for space-constrained (or more likely, fast boot, when the kernel is being loaded from a slow spi-based SD card interface) setups the user might be trying to minimize kernel size and turn off drivers they know they don't want. I'm not so worried about this driver, specifically, because it's small, but I am concerned about the general policy -- when we get rid of all the legacy board files and everything is move to device tree, will the user be stuck including a bunch of Renesas SH drivers when building a kernel the intend to use only on J-core? > > Anyway I'd really like to do this non-invasively as long as we have a > > mix of legacy and new stuff and the legacy stuff is not readily > > testable. Once all of arch/sh is moved over to device tree, could we > > revisit this and make all the drivers follow a common policy (on by > > default if they're associated with boards/SoCs using a matching or > > compatible CPU model, or something like that, but still able to be > > disabled manually, since the user might be trying to get a tiny-ish > > embedded kernel)? > > I understand the goal is to have one single configuration and everything > DT based and it sounds great but what is missing here is just a subarch, > not an option to enable/disable the timer. > > Give a try with: > > make ARCH=arm multi_v7_defconfig menuconfig > > --> System Type > > That is what you are looking for, a SUPERH config option selecting all the > common options and then a JCORE config option adding the different missing > bits, namely the CLKSRC_JCORE_PIT. We do have something like "system type" in arch/sh, and it's what I'm trying to deprecate since it's the switch to select between all the hard-coded board files, _or_ device tree. Since part of the goal of my DT overhaul is to be able (but not forced) to produce kernels that run on a wide range of hardware, rather than having a "system type (select one)" option, what about individual boolean options like: config JCORE_SOC bool "Support for J-Core SoCs" select CLKSRC_JCORE_PIT select JCORE_AIC ... Note that there are other drivers that should probably be optional even if you have JCORE_SOC enabled, like the SPI controller, DMA controller (not implemented yet), Ethernet (not submitted upstream yet), etc. Maybe they could depend on JCORE_SOC and be default-yes but configurable if available? In any case, the SoC support is supposedly there in the current kernel release (4.8) but not working because of missing essential drivers, so I'd really like to fix that without making the fix dependent on restructuring the arch/sh system type handling, which is an ongoing, independent project for which I'm waiting for help converting and testing the conversions of legacy board support. My preference would be to keep the Kconfig stuff the way I submitted it for now -- j2_defconfig already handles enabling thse right drivers -- and do something more user-friendly as part of the bigger arch overhaul project. > > > > + /* > > > > + * The J-Core PIT is not hard-wired to a particular IRQ, but > > > > + * integrated with the interrupt controller such that the IRQ it > > > > + * generates is programmable. The programming interface has a > > > > + * legacy field which was an interrupt priority for AIC1, but > > > > + * which is OR'd onto bits 2-5 of the generated IRQ number when > > > > + * used with J-Core AIC2, so set it to match these bits. > > > > + */ > > > > + hwirq = irq_get_irq_data(pit_irq)->hwirq; > > > > + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; > > > > + enable_val = (1U << PIT_ENABLE_SHIFT) > > > > + | (hwirq << PIT_IRQ_SHIFT) > > > > + | (irqprio << PIT_PRIO_SHIFT); > > > > + > > > > > > Why mention AIC1 if there is not test to check if AIC1 || AIC2 ? > > > > > > Will be the same information available if the irqchip is AIC1 ? > > > > The bit layout of the PIT enable register is: > > > > .....e..ppppiiiiiiii............ > > > > where the .'s indicate unrelated/unused bits, e is enable, p is > > priority, and i is hard irq number. > > > > For the PIT included in AIC1 (obsolete but still in use), any hard irq > > (trap number) can be programmed via the 8 iiiiiiii bits, and a > > priority (0-15) is programmable separately in the pppp bits. > > > > For the PIT included in AIC2 (current), the programming interface is > > equivalent modulo interrupt mapping. This is why a different > > compatible tag was not used. However only traps 64-127 (the ones > > actually intended to be used for interrupts, rather than > > syscalls/exceptions/etc.) can be programmed (the high 2 bits of i are > > ignored) and the priority pppp is <<2'd and or'd onto the irq number. > > This was a poor decision made on the hardware engineering side based > > on a wrong assumption that preserving old priority mapping of outdated > > software was important, whereas priorities weren't/aren't even being > > used. > > > > When we do the next round of interrupt controller improvements (AIC3) > > the PIT programming interface should remain compatible with the > > driver; likely the priority bits will just be ignored. > > > > If we do want to change the programming interface beyond this at some > > point (that maay be a good idea, since we have identified several > > things that are less than ideal for Linux, like the sechi/seclo/ns > > clocksource), a new compatible tag will be added instead. > > Ok, thanks for the clarification. Can you add your answer as a comment for > the bits dance above ? Are you happy with the whole quoted text above as a comment? If so I'm happy to include it verbatim. I would lean towards condensing or omitting the last 2 paragraphs (starting with "When we do...") if that's okay with you since they are not documenting the hw but future plans/policy. Rich -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 12, 2016 at 01:02:36PM -0400, Rich Felker wrote: > On Wed, Oct 12, 2016 at 11:27:11AM +0200, Daniel Lezcano wrote: > > > > Are the CPUs on always-on power down ? > > > > > > For now they are always on and don't even have the sleep instruction > > > (i.e. stop cpu clock until interrupt) implemented. Adding sleep will > > > be the first power-saving step, and perhaps the only one for now, > > > since there doesn't seem to be any indication (according to the ppl > > > working on the hardware) that a deeper sleep would provide significant > > > additional savings. > > > > Ok. > > > > However, the 'sleep' state is not, in the power management terminology, > > the idle state described above. It is called "clock gated" / "Wait for > > Interrupt". > > > > The 'sleep' state lose the CPU context. > > I use the term "sleep" because that's the name of the SH instruction > mnemonic for the opcode for entering the wait-for-interrupt state. > Sorry it's confusing like the "RTC" all over again. :-) > > > > > > A nanosecond-resolution clocksource is provided using the J-Core "RTC" > > > > > registers, which give a 64-bit seconds count and 32-bit nanoseconds > > > > > that wrap every second. The driver converts these to a full-range > > > > > 32-bit nanoseconds count. > > > > > > > > > > Signed-off-by: Rich Felker <dalias@libc.org> > > > > > --- > > > > > drivers/clocksource/Kconfig | 10 ++ > > > > > drivers/clocksource/Makefile | 1 + > > > > > drivers/clocksource/jcore-pit.c | 231 ++++++++++++++++++++++++++++++++++++++++ > > > > > include/linux/cpuhotplug.h | 1 + > > > > > 4 files changed, 243 insertions(+) > > > > > create mode 100644 drivers/clocksource/jcore-pit.c > > > > > > > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > > > > index 5677886..95dd78b 100644 > > > > > --- a/drivers/clocksource/Kconfig > > > > > +++ b/drivers/clocksource/Kconfig > > > > > @@ -407,6 +407,16 @@ config SYS_SUPPORTS_SH_TMU > > > > > config SYS_SUPPORTS_EM_STI > > > > > bool > > > > > > > > > > +config CLKSRC_JCORE_PIT > > > > > + bool "J-Core PIT timer driver" > > > > > + depends on OF && (SUPERH || COMPILE_TEST) > > > > > > > > Actually the idea is to have the SUPERH to select this timer, not create > > > > a dependency on SUPERH from here. > > > > > > > > We don't want to prompt in the configuration menu the drivers because it > > > > would be impossible to anyone to know which timer comes with which > > > > hardware, so we let the platform to select the timer it needs. > > > > > > I thought we discussed this before. For users building a kernel for > > > legacy SH systems, especially in the current state where they're only > > > supported with hard-coded board files rather than device tree, it > > > makes no sense to build drivers for J-core hardware. It would make > > > sense to be on by default for CONFIG_SH_DEVICE_TREE with a compatible > > > CPU selection, but at least at this time, not for SUPERH in general. > > > > Probably I am missing the point but why the user would have to unselect > > this driver manually ? The user wants a config file nothing more or a very > > trivial option. Can you imagine someone can know every single IP block for > > each boards of the same arch and be able to disable/enable the right ones ? > > The common case I imagine is just accepting defaults that include more > than you need, but for space-constrained (or more likely, fast boot, > when the kernel is being loaded from a slow spi-based SD card > interface) setups the user might be trying to minimize kernel size and > turn off drivers they know they don't want. I'm not so worried about > this driver, specifically, because it's small, but I am concerned > about the general policy -- when we get rid of all the legacy board > files and everything is move to device tree, will the user be stuck > including a bunch of Renesas SH drivers when building a kernel the > intend to use only on J-core? > > > > Anyway I'd really like to do this non-invasively as long as we have a > > > mix of legacy and new stuff and the legacy stuff is not readily > > > testable. Once all of arch/sh is moved over to device tree, could we > > > revisit this and make all the drivers follow a common policy (on by > > > default if they're associated with boards/SoCs using a matching or > > > compatible CPU model, or something like that, but still able to be > > > disabled manually, since the user might be trying to get a tiny-ish > > > embedded kernel)? > > > > I understand the goal is to have one single configuration and everything > > DT based and it sounds great but what is missing here is just a subarch, > > not an option to enable/disable the timer. > > > > Give a try with: > > > > make ARCH=arm multi_v7_defconfig menuconfig > > > > --> System Type > > > > That is what you are looking for, a SUPERH config option selecting all the > > common options and then a JCORE config option adding the different missing > > bits, namely the CLKSRC_JCORE_PIT. > > We do have something like "system type" in arch/sh, and it's what I'm > trying to deprecate since it's the switch to select between all the > hard-coded board files, _or_ device tree. > > Since part of the goal of my DT overhaul is to be able (but not > forced) to produce kernels that run on a wide range of hardware, > rather than having a "system type (select one)" option, what about > individual boolean options like: > > config JCORE_SOC > bool "Support for J-Core SoCs" > select CLKSRC_JCORE_PIT > select JCORE_AIC > ... I'm perfectly fine with this. > Note that there are other drivers that should probably be optional > even if you have JCORE_SOC enabled, like the SPI controller, DMA > controller (not implemented yet), Ethernet (not submitted upstream > yet), etc. Maybe they could depend on JCORE_SOC and be default-yes but > configurable if available? That sounds fine also. > In any case, the SoC support is supposedly there in the current kernel > release (4.8) but not working because of missing essential drivers, so > I'd really like to fix that without making the fix dependent on > restructuring the arch/sh system type handling, which is an ongoing, > independent project for which I'm waiting for help converting and > testing the conversions of legacy board support. My preference would > be to keep the Kconfig stuff the way I submitted it for now -- > j2_defconfig already handles enabling thse right drivers -- and do > something more user-friendly as part of the bigger arch overhaul > project. I prefer the move the option to config JCORE_SOC. That is not a big deal to add this bool in the sh's Kconfig and select the timer from there. > > > > > + /* > > > > > + * The J-Core PIT is not hard-wired to a particular IRQ, but > > > > > + * integrated with the interrupt controller such that the IRQ it > > > > > + * generates is programmable. The programming interface has a > > > > > + * legacy field which was an interrupt priority for AIC1, but > > > > > + * which is OR'd onto bits 2-5 of the generated IRQ number when > > > > > + * used with J-Core AIC2, so set it to match these bits. > > > > > + */ > > > > > + hwirq = irq_get_irq_data(pit_irq)->hwirq; > > > > > + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; > > > > > + enable_val = (1U << PIT_ENABLE_SHIFT) > > > > > + | (hwirq << PIT_IRQ_SHIFT) > > > > > + | (irqprio << PIT_PRIO_SHIFT); > > > > > + > > > > > > > > Why mention AIC1 if there is not test to check if AIC1 || AIC2 ? > > > > > > > > Will be the same information available if the irqchip is AIC1 ? > > > > > > The bit layout of the PIT enable register is: > > > > > > .....e..ppppiiiiiiii............ > > > > > > where the .'s indicate unrelated/unused bits, e is enable, p is > > > priority, and i is hard irq number. > > > > > > For the PIT included in AIC1 (obsolete but still in use), any hard irq > > > (trap number) can be programmed via the 8 iiiiiiii bits, and a > > > priority (0-15) is programmable separately in the pppp bits. > > > > > > For the PIT included in AIC2 (current), the programming interface is > > > equivalent modulo interrupt mapping. This is why a different > > > compatible tag was not used. However only traps 64-127 (the ones > > > actually intended to be used for interrupts, rather than > > > syscalls/exceptions/etc.) can be programmed (the high 2 bits of i are > > > ignored) and the priority pppp is <<2'd and or'd onto the irq number. > > > This was a poor decision made on the hardware engineering side based > > > on a wrong assumption that preserving old priority mapping of outdated > > > software was important, whereas priorities weren't/aren't even being > > > used. > > > > > > When we do the next round of interrupt controller improvements (AIC3) > > > the PIT programming interface should remain compatible with the > > > driver; likely the priority bits will just be ignored. > > > > > > If we do want to change the programming interface beyond this at some > > > point (that maay be a good idea, since we have identified several > > > things that are less than ideal for Linux, like the sechi/seclo/ns > > > clocksource), a new compatible tag will be added instead. > > > > Ok, thanks for the clarification. Can you add your answer as a comment for > > the bits dance above ? > > Are you happy with the whole quoted text above as a comment? If so I'm > happy to include it verbatim. I would lean towards condensing or > omitting the last 2 paragraphs (starting with "When we do...") if > that's okay with you since they are not documenting the hw but future > plans/policy. Makes sense. Agree for the verbatim minus the last 2 paragraphs. -- Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 12, 2016 at 11:31:26PM +0200, Daniel Lezcano wrote: > > > I understand the goal is to have one single configuration and everything > > > DT based and it sounds great but what is missing here is just a subarch, > > > not an option to enable/disable the timer. > > > > > > Give a try with: > > > > > > make ARCH=arm multi_v7_defconfig menuconfig > > > > > > --> System Type > > > > > > That is what you are looking for, a SUPERH config option selecting all the > > > common options and then a JCORE config option adding the different missing > > > bits, namely the CLKSRC_JCORE_PIT. > > > > We do have something like "system type" in arch/sh, and it's what I'm > > trying to deprecate since it's the switch to select between all the > > hard-coded board files, _or_ device tree. > > > > Since part of the goal of my DT overhaul is to be able (but not > > forced) to produce kernels that run on a wide range of hardware, > > rather than having a "system type (select one)" option, what about > > individual boolean options like: > > > > config JCORE_SOC > > bool "Support for J-Core SoCs" > > select CLKSRC_JCORE_PIT > > select JCORE_AIC > > ... > > I'm perfectly fine with this. > > > Note that there are other drivers that should probably be optional > > even if you have JCORE_SOC enabled, like the SPI controller, DMA > > controller (not implemented yet), Ethernet (not submitted upstream > > yet), etc. Maybe they could depend on JCORE_SOC and be default-yes but > > configurable if available? > > That sounds fine also. > > > In any case, the SoC support is supposedly there in the current kernel > > release (4.8) but not working because of missing essential drivers, so > > I'd really like to fix that without making the fix dependent on > > restructuring the arch/sh system type handling, which is an ongoing, > > independent project for which I'm waiting for help converting and > > testing the conversions of legacy board support. My preference would > > be to keep the Kconfig stuff the way I submitted it for now -- > > j2_defconfig already handles enabling thse right drivers -- and do > > something more user-friendly as part of the bigger arch overhaul > > project. > > I prefer the move the option to config JCORE_SOC. That is not a big deal > to add this bool in the sh's Kconfig and select the timer from there. OK, I'll do this and add the patch to my planned pull request, and send a corresponding Kconfig patch for the interrupt controller. > > > > > > + /* > > > > > > + * The J-Core PIT is not hard-wired to a particular IRQ, but > > > > > > + * integrated with the interrupt controller such that the IRQ it > > > > > > + * generates is programmable. The programming interface has a > > > > > > + * legacy field which was an interrupt priority for AIC1, but > > > > > > + * which is OR'd onto bits 2-5 of the generated IRQ number when > > > > > > + * used with J-Core AIC2, so set it to match these bits. > > > > > > + */ > > > > > > + hwirq = irq_get_irq_data(pit_irq)->hwirq; > > > > > > + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; > > > > > > + enable_val = (1U << PIT_ENABLE_SHIFT) > > > > > > + | (hwirq << PIT_IRQ_SHIFT) > > > > > > + | (irqprio << PIT_PRIO_SHIFT); > > > > > > + > > > > > > > > > > Why mention AIC1 if there is not test to check if AIC1 || AIC2 ? > > > > > > > > > > Will be the same information available if the irqchip is AIC1 ? > > > > > > > > The bit layout of the PIT enable register is: > > > > > > > > .....e..ppppiiiiiiii............ > > > > > > > > where the .'s indicate unrelated/unused bits, e is enable, p is > > > > priority, and i is hard irq number. > > > > > > > > For the PIT included in AIC1 (obsolete but still in use), any hard irq > > > > (trap number) can be programmed via the 8 iiiiiiii bits, and a > > > > priority (0-15) is programmable separately in the pppp bits. > > > > > > > > For the PIT included in AIC2 (current), the programming interface is > > > > equivalent modulo interrupt mapping. This is why a different > > > > compatible tag was not used. However only traps 64-127 (the ones > > > > actually intended to be used for interrupts, rather than > > > > syscalls/exceptions/etc.) can be programmed (the high 2 bits of i are > > > > ignored) and the priority pppp is <<2'd and or'd onto the irq number. > > > > This was a poor decision made on the hardware engineering side based > > > > on a wrong assumption that preserving old priority mapping of outdated > > > > software was important, whereas priorities weren't/aren't even being > > > > used. > > > > > > > > When we do the next round of interrupt controller improvements (AIC3) > > > > the PIT programming interface should remain compatible with the > > > > driver; likely the priority bits will just be ignored. > > > > > > > > If we do want to change the programming interface beyond this at some > > > > point (that maay be a good idea, since we have identified several > > > > things that are less than ideal for Linux, like the sechi/seclo/ns > > > > clocksource), a new compatible tag will be added instead. > > > > > > Ok, thanks for the clarification. Can you add your answer as a comment for > > > the bits dance above ? > > > > Are you happy with the whole quoted text above as a comment? If so I'm > > happy to include it verbatim. I would lean towards condensing or > > omitting the last 2 paragraphs (starting with "When we do...") if > > that's okay with you since they are not documenting the hw but future > > plans/policy. > > Makes sense. > > Agree for the verbatim minus the last 2 paragraphs. OK. I'll prepare a new patch with this and the previously-discussed changes. Rich -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 12, 2016 at 11:31:26PM +0200, Daniel Lezcano wrote: > > > --> System Type > > > > > > That is what you are looking for, a SUPERH config option selecting all the > > > common options and then a JCORE config option adding the different missing > > > bits, namely the CLKSRC_JCORE_PIT. > > > > We do have something like "system type" in arch/sh, and it's what I'm > > trying to deprecate since it's the switch to select between all the > > hard-coded board files, _or_ device tree. > > > > Since part of the goal of my DT overhaul is to be able (but not > > forced) to produce kernels that run on a wide range of hardware, > > rather than having a "system type (select one)" option, what about > > individual boolean options like: > > > > config JCORE_SOC > > bool "Support for J-Core SoCs" > > select CLKSRC_JCORE_PIT > > select JCORE_AIC > > ... > > I'm perfectly fine with this. Does this adequately ensure that dependencies for the clocksource and irq driver are met? If not, what else do I need to do? Rich -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 13, 2016 at 03:25:42PM -0400, Rich Felker wrote: > On Wed, Oct 12, 2016 at 11:31:26PM +0200, Daniel Lezcano wrote: > > > > --> System Type > > > > > > > > That is what you are looking for, a SUPERH config option selecting all the > > > > common options and then a JCORE config option adding the different missing > > > > bits, namely the CLKSRC_JCORE_PIT. > > > > > > We do have something like "system type" in arch/sh, and it's what I'm > > > trying to deprecate since it's the switch to select between all the > > > hard-coded board files, _or_ device tree. > > > > > > Since part of the goal of my DT overhaul is to be able (but not > > > forced) to produce kernels that run on a wide range of hardware, > > > rather than having a "system type (select one)" option, what about > > > individual boolean options like: > > > > > > config JCORE_SOC > > > bool "Support for J-Core SoCs" > > > select CLKSRC_JCORE_PIT > > > select JCORE_AIC > > > ... > > > > I'm perfectly fine with this. > > Does this adequately ensure that dependencies for the clocksource and > irq driver are met? If not, what else do I need to do? I've checked and it seems the dependencies are always met for arch/sh, at least with device tree enabled (on which JCORE_SOC will depend), so it shouldn't matter. Rich -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 5677886..95dd78b 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -407,6 +407,16 @@ config SYS_SUPPORTS_SH_TMU config SYS_SUPPORTS_EM_STI bool +config CLKSRC_JCORE_PIT + bool "J-Core PIT timer driver" + depends on OF && (SUPERH || COMPILE_TEST) + depends on GENERIC_CLOCKEVENTS + depends on HAS_IOMEM + select CLKSRC_MMIO + help + This enables build of clocksource and clockevent driver for + the integrated PIT in the J-Core synthesizable, open source SoC. + config SH_TIMER_CMT bool "Renesas CMT timer driver" if COMPILE_TEST depends on GENERIC_CLOCKEVENTS diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index fd9d6df..cf87f40 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_ATMEL_TCB_CLKSRC) += tcb_clksrc.o obj-$(CONFIG_X86_PM_TIMER) += acpi_pm.o obj-$(CONFIG_SCx200HR_TIMER) += scx200_hrt.o obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC) += cs5535-clockevt.o +obj-$(CONFIG_CLKSRC_JCORE_PIT) += jcore-pit.o obj-$(CONFIG_SH_TIMER_CMT) += sh_cmt.o obj-$(CONFIG_SH_TIMER_MTU2) += sh_mtu2.o obj-$(CONFIG_SH_TIMER_TMU) += sh_tmu.o diff --git a/drivers/clocksource/jcore-pit.c b/drivers/clocksource/jcore-pit.c new file mode 100644 index 0000000..4f3ef69 --- /dev/null +++ b/drivers/clocksource/jcore-pit.c @@ -0,0 +1,231 @@ +/* + * J-Core SoC PIT/clocksource driver + * + * Copyright (C) 2015-2016 Smart Energy Instruments, Inc. + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + */ + +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/interrupt.h> +#include <linux/clockchips.h> +#include <linux/clocksource.h> +#include <linux/sched_clock.h> +#include <linux/cpu.h> +#include <linux/cpuhotplug.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> + +#define PIT_IRQ_SHIFT 12 +#define PIT_PRIO_SHIFT 20 +#define PIT_ENABLE_SHIFT 26 +#define PIT_IRQ_MASK 0x3f +#define PIT_PRIO_MASK 0xf + +#define REG_PITEN 0x00 +#define REG_THROT 0x10 +#define REG_COUNT 0x14 +#define REG_BUSPD 0x18 +#define REG_SECHI 0x20 +#define REG_SECLO 0x24 +#define REG_NSEC 0x28 + +struct jcore_pit { + struct clock_event_device ced; + __iomem void *base; + unsigned long periodic_delta; + unsigned cpu; + u32 enable_val; +}; + +static __iomem void *jcore_pit_base; +struct jcore_pit __percpu *jcore_pit_percpu; + +static notrace u64 jcore_sched_clock_read(void) +{ + u32 seclo, nsec, seclo0; + __iomem void *base = jcore_pit_base; + + seclo = readl(base + REG_SECLO); + do { + seclo0 = seclo; + nsec = readl(base + REG_NSEC); + seclo = readl(base + REG_SECLO); + } while (seclo0 != seclo); + + return seclo * NSEC_PER_SEC + nsec; +} + +static cycle_t jcore_clocksource_read(struct clocksource *cs) +{ + return jcore_sched_clock_read(); +} + +static int jcore_pit_disable(struct jcore_pit *pit) +{ + writel(0, pit->base + REG_PITEN); + return 0; +} + +static int jcore_pit_set(unsigned long delta, struct jcore_pit *pit) +{ + jcore_pit_disable(pit); + writel(delta, pit->base + REG_THROT); + writel(pit->enable_val, pit->base + REG_PITEN); + return 0; +} + +static int jcore_pit_set_state_shutdown(struct clock_event_device *ced) +{ + struct jcore_pit *pit = container_of(ced, struct jcore_pit, ced); + + return jcore_pit_disable(pit); +} + +static int jcore_pit_set_state_oneshot(struct clock_event_device *ced) +{ + struct jcore_pit *pit = container_of(ced, struct jcore_pit, ced); + + return jcore_pit_disable(pit); +} + +static int jcore_pit_set_state_periodic(struct clock_event_device *ced) +{ + struct jcore_pit *pit = container_of(ced, struct jcore_pit, ced); + + return jcore_pit_set(pit->periodic_delta, pit); +} + +static int jcore_pit_set_next_event(unsigned long delta, + struct clock_event_device *ced) +{ + struct jcore_pit *pit = container_of(ced, struct jcore_pit, ced); + + return jcore_pit_set(delta, pit); +} + +static int jcore_pit_local_init(unsigned cpu) +{ + struct jcore_pit *pit = this_cpu_ptr(jcore_pit_percpu); + unsigned buspd, freq; + + pr_info("Local J-Core PIT init on cpu %u\n", pit->cpu); + + buspd = readl(pit->base + REG_BUSPD); + freq = DIV_ROUND_CLOSEST(NSEC_PER_SEC, buspd); + pit->periodic_delta = DIV_ROUND_CLOSEST(NSEC_PER_SEC, HZ * buspd); + + clockevents_config_and_register(&pit->ced, freq, 1, ULONG_MAX); + + return 0; +} + +static irqreturn_t jcore_timer_interrupt(int irq, void *dev_id) +{ + struct jcore_pit *pit = this_cpu_ptr(dev_id); + + if (clockevent_state_oneshot(&pit->ced)) + jcore_pit_disable(pit); + + pit->ced.event_handler(&pit->ced); + + return IRQ_HANDLED; +} + +static int __init jcore_pit_init(struct device_node *node) +{ + int err; + unsigned pit_irq, cpu; + unsigned long hwirq; + u32 irqprio, enable_val; + + jcore_pit_base = of_iomap(node, 0); + if (!jcore_pit_base) { + pr_err("Error: Cannot map base address for J-Core PIT\n"); + return -ENXIO; + } + + pit_irq = irq_of_parse_and_map(node, 0); + if (!pit_irq) { + pr_err("Error: J-Core PIT has no IRQ\n"); + return -ENXIO; + } + + pr_info("Initializing J-Core PIT at %p IRQ %d\n", + jcore_pit_base, pit_irq); + + err = clocksource_mmio_init(jcore_pit_base, "jcore_pit_cs", + NSEC_PER_SEC, 400, 32, + jcore_clocksource_read); + if (err) { + pr_err("Error registering clocksource device: %d\n", err); + return err; + } + + sched_clock_register(jcore_sched_clock_read, 32, NSEC_PER_SEC); + + jcore_pit_percpu = alloc_percpu(struct jcore_pit); + if (!jcore_pit_percpu) { + pr_err("Failed to allocate memory for clock event device\n"); + return -ENOMEM; + } + + err = request_irq(pit_irq, jcore_timer_interrupt, + IRQF_TIMER | IRQF_PERCPU, + "jcore_pit", jcore_pit_percpu); + if (err) { + pr_err("pit irq request failed: %d\n", err); + free_percpu(jcore_pit_percpu); + return err; + } + + /* + * The J-Core PIT is not hard-wired to a particular IRQ, but + * integrated with the interrupt controller such that the IRQ it + * generates is programmable. The programming interface has a + * legacy field which was an interrupt priority for AIC1, but + * which is OR'd onto bits 2-5 of the generated IRQ number when + * used with J-Core AIC2, so set it to match these bits. + */ + hwirq = irq_get_irq_data(pit_irq)->hwirq; + irqprio = (hwirq >> 2) & PIT_PRIO_MASK; + enable_val = (1U << PIT_ENABLE_SHIFT) + | (hwirq << PIT_IRQ_SHIFT) + | (irqprio << PIT_PRIO_SHIFT); + + for_each_present_cpu(cpu) { + struct jcore_pit *pit = per_cpu_ptr(jcore_pit_percpu, cpu); + + pit->base = of_iomap(node, cpu); + if (!pit->base) { + pr_err("Unable to map PIT for cpu %u\n", cpu); + continue; + } + + pit->ced.name = "jcore_pit"; + pit->ced.features = CLOCK_EVT_FEAT_PERIODIC + | CLOCK_EVT_FEAT_ONESHOT + | CLOCK_EVT_FEAT_PERCPU; + pit->ced.cpumask = cpumask_of(cpu); + pit->ced.rating = 400; + pit->ced.irq = pit_irq; + pit->ced.set_state_shutdown = jcore_pit_set_state_shutdown; + pit->ced.set_state_periodic = jcore_pit_set_state_periodic; + pit->ced.set_state_oneshot = jcore_pit_set_state_oneshot; + pit->ced.set_next_event = jcore_pit_set_next_event; + + pit->cpu = cpu; + pit->enable_val = enable_val; + } + + cpuhp_setup_state(CPUHP_AP_JCORE_TIMER_STARTING, + "AP_JCORE_TIMER_STARTING", + jcore_pit_local_init, NULL); + + return 0; +} + +CLOCKSOURCE_OF_DECLARE(jcore_pit, "jcore,pit", jcore_pit_init); diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h index 34bd805..e2e6c8c 100644 --- a/include/linux/cpuhotplug.h +++ b/include/linux/cpuhotplug.h @@ -52,6 +52,7 @@ enum cpuhp_state { CPUHP_AP_ARM_ARCH_TIMER_STARTING, CPUHP_AP_ARM_GLOBAL_TIMER_STARTING, CPUHP_AP_DUMMY_TIMER_STARTING, + CPUHP_AP_JCORE_TIMER_STARTING, CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, CPUHP_AP_ARM_TWD_STARTING, CPUHP_AP_METAG_TIMER_STARTING,
At the hardware level, the J-Core PIT is integrated with the interrupt controller, but it is represented as its own device and has an independent programming interface. It provides a 12-bit countdown timer, which is not presently used, and a periodic timer. The interval length for the latter is programmable via a 32-bit throttle register whose units are determined by a bus-period register. The periodic timer is used to implement both periodic and oneshot clock event modes; in oneshot mode the interrupt handler simply disables the timer as soon as it fires. Despite its device tree node representing an interrupt for the PIT, the actual irq generated is programmable, not hard-wired. The driver is responsible for programming the PIT to generate the hardware irq number that the DT assigns to it. On SMP configurations, J-Core provides cpu-local instances of the PIT; no broadcast timer is needed. This driver supports the creation of the necessary per-cpu clock_event_device instances. A nanosecond-resolution clocksource is provided using the J-Core "RTC" registers, which give a 64-bit seconds count and 32-bit nanoseconds that wrap every second. The driver converts these to a full-range 32-bit nanoseconds count. Signed-off-by: Rich Felker <dalias@libc.org> --- drivers/clocksource/Kconfig | 10 ++ drivers/clocksource/Makefile | 1 + drivers/clocksource/jcore-pit.c | 231 ++++++++++++++++++++++++++++++++++++++++ include/linux/cpuhotplug.h | 1 + 4 files changed, 243 insertions(+) create mode 100644 drivers/clocksource/jcore-pit.c