On Fri, Feb 24, 2017 at 04:40:42AM +0100, Andreas Färber wrote:
> Implement clocksource and clockevents for Actions Semi S500.
>
> Based on LeMaker linux-actions tree.
>
> Signed-off-by: Andreas Färber <afaer...@suse.de>
As this is a new driver, please give some technical information about the
driver itself and a pointer to documentation if it is publicly available.
[ ... ]
> +#define OWL_T0_CMP 0x0c
> +#define OWL_T0_VAL 0x10
> +#define OWL_T1_CTL 0x14
> +#define OWL_T1_CMP 0x18
> +#define OWL_T1_VAL 0x1c
> +
> +#define OWL_Tx_CTL_INTEN (1 << 1)
> +#define OWL_Tx_CTL_EN (1 << 2)
s/(1 << 1)/BIT(1)/
s/(1 << 2)/BIT(2)/
> +
> +static void __iomem *owl_timer_base;
> +
> +static u64 notrace owl_timer_sched_read(void)
> +{
> + return (u64)readl(owl_timer_base + OWL_T0_VAL);
> +}
> +
> +static int owl_timer_set_state_shutdown(struct clock_event_device *evt)
> +{
> + writel(0, owl_timer_base + OWL_T1_CTL);
> +
> + return 0;
> +}
> +
> +static int owl_timer_set_state_oneshot(struct clock_event_device *evt)
> +{
> + writel(0, owl_timer_base + OWL_T1_CTL);
> + writel(0, owl_timer_base + OWL_T1_VAL);
> + writel(0, owl_timer_base + OWL_T1_CMP);
> +
> + return 0;
> +}
> +
> +static int owl_timer_tick_resume(struct clock_event_device *evt)
> +{
> + return 0;
> +}
> +
> +static int owl_timer_set_next_event(unsigned long evt,
> + struct clock_event_device *ev)
> +{
> + writel(0, owl_timer_base + OWL_T1_CTL);
> +
> + writel(0, owl_timer_base + OWL_T1_VAL);
> + writel(evt, owl_timer_base + OWL_T1_CMP);
> +
> + writel(OWL_Tx_CTL_EN | OWL_Tx_CTL_INTEN, owl_timer_base + OWL_T1_CTL);
> +
> + return 0;
> +}
> +
> +static struct clock_event_device owl_clockevent = {
> + .name = "owl_tick",
> + .rating = 200,
> + .features = CLOCK_EVT_FEAT_ONESHOT,
Did you consider adding CLOCK_EVT_FEAT_DYNIRQ ?
> + .set_state_shutdown = owl_timer_set_state_shutdown,
> + .set_state_oneshot = owl_timer_set_state_oneshot,
> + .tick_resume = owl_timer_tick_resume,
> + .set_next_event = owl_timer_set_next_event,
> +};
> +
> +static irqreturn_t owl_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *evt = (struct clock_event_device *)dev_id;
> +
> + evt->event_handler(evt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static struct irqaction owl_timer_irq = {
> + .name = "owl-timer",
> + .flags = IRQF_TIMER,
> + .handler = owl_timer_interrupt,
> + .dev_id = &owl_clockevent,
> +};
> +
> +static int __init owl_timer_init(struct device_node *node)
> +{
> + const unsigned long rate = 24000000;
Use DT, either use clock-frequency or a clock ref.
> + int irq1, ret;
> +
> + owl_timer_base = of_io_request_and_map(node, 0, "owl-timer");
> + if (IS_ERR(owl_timer_base)) {
> + pr_err("Can't map timer registers");
> + return -ENXIO;
Why not PTR_ERR(owl_timer_base) ?
> + }
> +
> + irq1 = irq_of_parse_and_map(node, 1);
> + if (irq1 <= 0) {
> + pr_err("Can't parse timer1 IRQ");
> + return -EINVAL;
> + }
> +
> + writel(0, owl_timer_base + OWL_T0_CTL);
> + writel(0, owl_timer_base + OWL_T0_VAL);
> + writel(0, owl_timer_base + OWL_T0_CMP);
> + writel(OWL_Tx_CTL_EN, owl_timer_base + OWL_T0_CTL);
Please factor out these calls into a function.
> +
> + sched_clock_register(owl_timer_sched_read, 32, rate);
> + clocksource_mmio_init(owl_timer_base + OWL_T0_VAL, node->name,
> + rate, 200, 32, clocksource_mmio_readl_up);
> +
> + writel(0, owl_timer_base + OWL_T1_CTL);
> + writel(0, owl_timer_base + OWL_T1_VAL);
> + writel(0, owl_timer_base + OWL_T1_CMP);
> +
> + ret = setup_irq(irq1, &owl_timer_irq);
> + if (ret) {
> + pr_warn("failed to setup irq %d\n", irq1);
> + return ret;
> + }
s/setup_irq/request_irq/
> +
> + owl_clockevent.cpumask = cpumask_of(0);
> + owl_clockevent.irq = irq1;
> +
> + clockevents_config_and_register(&owl_clockevent, rate,
> + 0xf, 0xffffffff);
> +
> + return 0;
> +}
> +CLOCKSOURCE_OF_DECLARE(owl, "actions,owl-timer", owl_timer_init);
Thanks !
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
