Re: [PATCH 1/2] drivers/clocksource/mediatek: Split mediatek drivers into 2 files

From: Evan Benn <evanbenn@chromium.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Yingjoe Chen <yingjoe.chen@mediatek.com>,
	Julia Lawall <Julia.Lawall@lip6.fr>,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Fabien Parent <fparent@baylibre.com>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	Alexey Klimov <alexey.klimov@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Russell King <linux@armlinux.org.uk>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH 1/2] drivers/clocksource/mediatek: Split mediatek drivers into 2 files
Date: Wed, 24 Mar 2021 11:53:43 +1100	[thread overview]
Message-ID: <CAKz_xw1wPxEpPGs1A4jzuoKqbL-okczb9GCdLFqcCbaBs2F-0A@mail.gmail.com> (raw)
In-Reply-To: <5a0272dc-bb00-ac4b-4e44-5a2980d7baaa@linaro.org>

On Mon, Mar 22, 2021 at 10:19 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
>
> Hi Evan,
>
> On 18/03/2021 06:04, Evan Benn wrote:
> > mtk_gpt and mtk_syst drivers for mt6577 and mt6765 devices were not
> > sharing any code. So split them into separate files.
>
> For the sake of consistency, keeping all in one is better.

Thanks Daniel,

I think you are right.
The other patch does apply cleanly without this one, so please just
ignore this patch.

>
> Thanks
>   -- Daniel
>
> > Signed-off-by: Evan Benn <evanbenn@chromium.org>
> > ---
> >
> >  arch/arm/mach-mediatek/Kconfig                |   3 +-
> >  arch/arm64/Kconfig.platforms                  |   3 +-
> >  drivers/clocksource/Kconfig                   |  13 +-
> >  drivers/clocksource/Makefile                  |   3 +-
> >  ...mer-mediatek.c => timer-mediatek-mt6577.c} | 100 -------------
> >  drivers/clocksource/timer-mediatek-mt6765.c   | 135 ++++++++++++++++++
> >  6 files changed, 151 insertions(+), 106 deletions(-)
> >  rename drivers/clocksource/{timer-mediatek.c => timer-mediatek-mt6577.c} (69%)
> >  create mode 100644 drivers/clocksource/timer-mediatek-mt6765.c
> >
> > diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
> > index 9e0f592d87d8..8686f992c4b6 100644
> > --- a/arch/arm/mach-mediatek/Kconfig
> > +++ b/arch/arm/mach-mediatek/Kconfig
> > @@ -4,7 +4,8 @@ menuconfig ARCH_MEDIATEK
> >       depends on ARCH_MULTI_V7
> >       select ARM_GIC
> >       select PINCTRL
> > -     select MTK_TIMER
> > +     select TIMER_MEDIATEK_MT6577
> > +     select TIMER_MEDIATEK_MT6765
> >       select MFD_SYSCON
> >       help
> >         Support for Mediatek MT65xx & MT81xx SoCs
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index cdfd5fed457f..d4752375ab0b 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -161,7 +161,8 @@ config ARCH_MEDIATEK
> >       bool "MediaTek SoC Family"
> >       select ARM_GIC
> >       select PINCTRL
> > -     select MTK_TIMER
> > +     select TIMER_MEDIATEK_MT6577
> > +     select TIMER_MEDIATEK_MT6765
> >       help
> >         This enables support for MediaTek MT27xx, MT65xx, MT76xx
> >         & MT81xx ARMv8 SoCs
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 39aa21d01e05..d697c799284e 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -438,13 +438,20 @@ config OXNAS_RPS_TIMER
> >  config SYS_SUPPORTS_SH_CMT
> >       bool
> >
> > -config MTK_TIMER
> > -     bool "Mediatek timer driver" if COMPILE_TEST
> > +config TIMER_MEDIATEK_MT6577
> > +     bool "Mediatek mt6577 timer driver" if COMPILE_TEST
> >       depends on HAS_IOMEM
> >       select TIMER_OF
> >       select CLKSRC_MMIO
> >       help
> > -       Support for Mediatek timer driver.
> > +       Enables clocksource and clockevent driver for Mediatek mt6577 timer.
> > +
> > +config TIMER_MEDIATEK_MT6765
> > +     bool "Mediatek mt6765 timer driver" if COMPILE_TEST
> > +     depends on HAS_IOMEM
> > +     select TIMER_OF
> > +     help
> > +       Enables clockevent driver for Mediatek mt6765 timer.
> >
> >  config SPRD_TIMER
> >       bool "Spreadtrum timer driver" if EXPERT
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index c17ee32a7151..b1f06ce114f9 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -49,7 +49,8 @@ obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)    += samsung_pwm_timer.o
> >  obj-$(CONFIG_FSL_FTM_TIMER)  += timer-fsl-ftm.o
> >  obj-$(CONFIG_VF_PIT_TIMER)   += timer-vf-pit.o
> >  obj-$(CONFIG_CLKSRC_QCOM)    += timer-qcom.o
> > -obj-$(CONFIG_MTK_TIMER)              += timer-mediatek.o
> > +obj-$(CONFIG_TIMER_MEDIATEK_MT6577)          += timer-mediatek-mt6577.o
> > +obj-$(CONFIG_TIMER_MEDIATEK_MT6765)          += timer-mediatek-mt6765.o
> >  obj-$(CONFIG_CLKSRC_PISTACHIO)       += timer-pistachio.o
> >  obj-$(CONFIG_CLKSRC_TI_32K)  += timer-ti-32k.o
> >  obj-$(CONFIG_OXNAS_RPS_TIMER)        += timer-oxnas-rps.o
> > diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek-mt6577.c
> > similarity index 69%
> > rename from drivers/clocksource/timer-mediatek.c
> > rename to drivers/clocksource/timer-mediatek-mt6577.c
> > index 9318edcd8963..9e5241d1876d 100644
> > --- a/drivers/clocksource/timer-mediatek.c
> > +++ b/drivers/clocksource/timer-mediatek-mt6577.c
> > @@ -47,86 +47,8 @@
> >  #define GPT_CNT_REG(val)        (0x08 + (0x10 * (val)))
> >  #define GPT_CMP_REG(val)        (0x0C + (0x10 * (val)))
> >
> > -/* system timer */
> > -#define SYST_BASE               (0x40)
> > -
> > -#define SYST_CON                (SYST_BASE + 0x0)
> > -#define SYST_VAL                (SYST_BASE + 0x4)
> > -
> > -#define SYST_CON_REG(to)        (timer_of_base(to) + SYST_CON)
> > -#define SYST_VAL_REG(to)        (timer_of_base(to) + SYST_VAL)
> > -
> > -/*
> > - * SYST_CON_EN: Clock enable. Shall be set to
> > - *   - Start timer countdown.
> > - *   - Allow timeout ticks being updated.
> > - *   - Allow changing interrupt functions.
> > - *
> > - * SYST_CON_IRQ_EN: Set to allow interrupt.
> > - *
> > - * SYST_CON_IRQ_CLR: Set to clear interrupt.
> > - */
> > -#define SYST_CON_EN              BIT(0)
> > -#define SYST_CON_IRQ_EN          BIT(1)
> > -#define SYST_CON_IRQ_CLR         BIT(4)
> > -
> >  static void __iomem *gpt_sched_reg __read_mostly;
> >
> > -static void mtk_syst_ack_irq(struct timer_of *to)
> > -{
> > -     /* Clear and disable interrupt */
> > -     writel(SYST_CON_IRQ_CLR | SYST_CON_EN, SYST_CON_REG(to));
> > -}
> > -
> > -static irqreturn_t mtk_syst_handler(int irq, void *dev_id)
> > -{
> > -     struct clock_event_device *clkevt = dev_id;
> > -     struct timer_of *to = to_timer_of(clkevt);
> > -
> > -     mtk_syst_ack_irq(to);
> > -     clkevt->event_handler(clkevt);
> > -
> > -     return IRQ_HANDLED;
> > -}
> > -
> > -static int mtk_syst_clkevt_next_event(unsigned long ticks,
> > -                                   struct clock_event_device *clkevt)
> > -{
> > -     struct timer_of *to = to_timer_of(clkevt);
> > -
> > -     /* Enable clock to allow timeout tick update later */
> > -     writel(SYST_CON_EN, SYST_CON_REG(to));
> > -
> > -     /*
> > -      * Write new timeout ticks. Timer shall start countdown
> > -      * after timeout ticks are updated.
> > -      */
> > -     writel(ticks, SYST_VAL_REG(to));
> > -
> > -     /* Enable interrupt */
> > -     writel(SYST_CON_EN | SYST_CON_IRQ_EN, SYST_CON_REG(to));
> > -
> > -     return 0;
> > -}
> > -
> > -static int mtk_syst_clkevt_shutdown(struct clock_event_device *clkevt)
> > -{
> > -     /* Disable timer */
> > -     writel(0, SYST_CON_REG(to_timer_of(clkevt)));
> > -
> > -     return 0;
> > -}
> > -
> > -static int mtk_syst_clkevt_resume(struct clock_event_device *clkevt)
> > -{
> > -     return mtk_syst_clkevt_shutdown(clkevt);
> > -}
> > -
> > -static int mtk_syst_clkevt_oneshot(struct clock_event_device *clkevt)
> > -{
> > -     return 0;
> > -}
> > -
> >  static u64 notrace mtk_gpt_read_sched_clock(void)
> >  {
> >       return readl_relaxed(gpt_sched_reg);
> > @@ -255,27 +177,6 @@ static struct timer_of to = {
> >       },
> >  };
> >
> > -static int __init mtk_syst_init(struct device_node *node)
> > -{
> > -     int ret;
> > -
> > -     to.clkevt.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_ONESHOT;
> > -     to.clkevt.set_state_shutdown = mtk_syst_clkevt_shutdown;
> > -     to.clkevt.set_state_oneshot = mtk_syst_clkevt_oneshot;
> > -     to.clkevt.tick_resume = mtk_syst_clkevt_resume;
> > -     to.clkevt.set_next_event = mtk_syst_clkevt_next_event;
> > -     to.of_irq.handler = mtk_syst_handler;
> > -
> > -     ret = timer_of_init(node, &to);
> > -     if (ret)
> > -             return ret;
> > -
> > -     clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
> > -                                     TIMER_SYNC_TICKS, 0xffffffff);
> > -
> > -     return 0;
> > -}
> > -
> >  static int __init mtk_gpt_init(struct device_node *node)
> >  {
> >       int ret;
> > @@ -310,4 +211,3 @@ static int __init mtk_gpt_init(struct device_node *node)
> >       return 0;
> >  }
> >  TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
> > -TIMER_OF_DECLARE(mtk_mt6765, "mediatek,mt6765-timer", mtk_syst_init);
> > diff --git a/drivers/clocksource/timer-mediatek-mt6765.c b/drivers/clocksource/timer-mediatek-mt6765.c
> > new file mode 100644
> > index 000000000000..b4f22f226feb
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-mediatek-mt6765.c
> > @@ -0,0 +1,135 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Mediatek SoCs General-Purpose Timer handling.
> > + *
> > + * Copyright (C) 2014 Matthias Brugger
> > + *
> > + * Matthias Brugger <matthias.bgg@gmail.com>
> > + */
> > +
> > +#define pr_fmt(fmt)  KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/clockchips.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irqreturn.h>
> > +#include <linux/sched_clock.h>
> > +#include <linux/slab.h>
> > +#include "timer-of.h"
> > +
> > +#define TIMER_SYNC_TICKS        (3)
> > +
> > +/* system timer */
> > +#define SYST_BASE               (0x40)
> > +
> > +#define SYST_CON                (SYST_BASE + 0x0)
> > +#define SYST_VAL                (SYST_BASE + 0x4)
> > +
> > +#define SYST_CON_REG(to)        (timer_of_base(to) + SYST_CON)
> > +#define SYST_VAL_REG(to)        (timer_of_base(to) + SYST_VAL)
> > +
> > +/*
> > + * SYST_CON_EN: Clock enable. Shall be set to
> > + *   - Start timer countdown.
> > + *   - Allow timeout ticks being updated.
> > + *   - Allow changing interrupt functions.
> > + *
> > + * SYST_CON_IRQ_EN: Set to allow interrupt.
> > + *
> > + * SYST_CON_IRQ_CLR: Set to clear interrupt.
> > + */
> > +#define SYST_CON_EN              BIT(0)
> > +#define SYST_CON_IRQ_EN          BIT(1)
> > +#define SYST_CON_IRQ_CLR         BIT(4)
> > +
> > +static void mtk_syst_ack_irq(struct timer_of *to)
> > +{
> > +     /* Clear and disable interrupt */
> > +     writel(SYST_CON_IRQ_CLR | SYST_CON_EN, SYST_CON_REG(to));
> > +}
> > +
> > +static irqreturn_t mtk_syst_handler(int irq, void *dev_id)
> > +{
> > +     struct clock_event_device *clkevt = dev_id;
> > +     struct timer_of *to = to_timer_of(clkevt);
> > +
> > +     mtk_syst_ack_irq(to);
> > +     clkevt->event_handler(clkevt);
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_syst_clkevt_next_event(unsigned long ticks,
> > +                                   struct clock_event_device *clkevt)
> > +{
> > +     struct timer_of *to = to_timer_of(clkevt);
> > +
> > +     /* Enable clock to allow timeout tick update later */
> > +     writel(SYST_CON_EN, SYST_CON_REG(to));
> > +
> > +     /*
> > +      * Write new timeout ticks. Timer shall start countdown
> > +      * after timeout ticks are updated.
> > +      */
> > +     writel(ticks, SYST_VAL_REG(to));
> > +
> > +     /* Enable interrupt */
> > +     writel(SYST_CON_EN | SYST_CON_IRQ_EN, SYST_CON_REG(to));
> > +
> > +     return 0;
> > +}
> > +
> > +static int mtk_syst_clkevt_shutdown(struct clock_event_device *clkevt)
> > +{
> > +     /* Disable timer */
> > +     writel(0, SYST_CON_REG(to_timer_of(clkevt)));
> > +
> > +     return 0;
> > +}
> > +
> > +static int mtk_syst_clkevt_resume(struct clock_event_device *clkevt)
> > +{
> > +     return mtk_syst_clkevt_shutdown(clkevt);
> > +}
> > +
> > +static int mtk_syst_clkevt_oneshot(struct clock_event_device *clkevt)
> > +{
> > +     return 0;
> > +}
> > +
> > +static struct timer_of to = {
> > +     .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> > +
> > +     .clkevt = {
> > +             .name = "mtk-clkevt",
> > +             .rating = 300,
> > +             .cpumask = cpu_possible_mask,
> > +     },
> > +
> > +     .of_irq = {
> > +             .flags = IRQF_TIMER | IRQF_IRQPOLL,
> > +     },
> > +};
> > +
> > +static int __init mtk_syst_init(struct device_node *node)
> > +{
> > +     int ret;
> > +
> > +     to.clkevt.features = CLOCK_EVT_FEAT_DYNIRQ | CLOCK_EVT_FEAT_ONESHOT;
> > +     to.clkevt.set_state_shutdown = mtk_syst_clkevt_shutdown;
> > +     to.clkevt.set_state_oneshot = mtk_syst_clkevt_oneshot;
> > +     to.clkevt.tick_resume = mtk_syst_clkevt_resume;
> > +     to.clkevt.set_next_event = mtk_syst_clkevt_next_event;
> > +     to.of_irq.handler = mtk_syst_handler;
> > +
> > +     ret = timer_of_init(node, &to);
> > +     if (ret)
> > +             return ret;
> > +
> > +     clockevents_config_and_register(&to.clkevt, timer_of_rate(&to),
> > +                                     TIMER_SYNC_TICKS, 0xffffffff);
> > +
> > +     return 0;
> > +}
> > +
> > +TIMER_OF_DECLARE(mtk_mt6765, "mediatek,mt6765-timer", mtk_syst_init);
> >
>
>
> --
> <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