| Message ID | 20210208002437.13500-5-mr.bossman075@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | timer: imx-gpt: Add timer support for i.MX SoCs family | expand |
On 2/7/21 7:24 PM, Jesse Taube wrote: > Signed-off-by: Jesse Taube <mr.bossman075@gmail.com> > Cc: Stefano Babic <tsbabic@denx.de> > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Cc: Jesse Taube <mr.bossman075@gmail.com> > > This timer driver is using GPT Timer (General Purpose Timer) available > on almost all i.MX SoCs family. Add code to enable timer. Add code get a defualt prescaler. > Add defines for register masks. Please squash this with your first patch. > --- > drivers/timer/imx-gpt-timer.c | 50 +++++++++++++++++++++++++---------- > 1 file changed, 36 insertions(+), 14 deletions(-) > > diff --git a/drivers/timer/imx-gpt-timer.c b/drivers/timer/imx-gpt-timer.c > index c4e8c12a42..ed6487af97 100644 > --- a/drivers/timer/imx-gpt-timer.c > +++ b/drivers/timer/imx-gpt-timer.c > @@ -13,8 +13,29 @@ > > #include <asm/io.h> > > -#define GPT_CR_SWR 0x00008000 > +#define GPT_CR_SWR 0x00008000 > +#define GPT_CR_CLKSRC 0x000001C0 > +#define GPT_CR_EN_24M 0x00004000 > +#define GPT_CR_EN 0x00000001 > +#define GPT_PR_PRESCALER 0x00000FFF > +#define GPT_PR_PRESCALER24M 0x0000F000 > > + > + > +#define NO_CLOCK (0) > +#define IPG_CLK (1<<6) > +#define IPG_CLK_HF (2<<6) > +#define IPG_EXT (3<<6) > +#define IPG_CLK_32K (4<<6) > +#define IPG_CLK_24M (5<<6) > + > + > +/* > +ipg_clk ipg_clk_root Peripheral clock > +ipg_clk_32k ckil_sync_clk_root Low-frequency reference clock (32 kHz) > +ipg_clk_highfreq perclk_clk_root High-frequency reference clock > +ipg_clk_s ipg_clk_root Peripheral access clock > +*/ > struct imx_gpt_timer_regs { > u32 cr; > u32 pr; > @@ -32,14 +53,12 @@ struct imx_gpt_timer_priv { > struct imx_gpt_timer_regs *base; > }; > > -static int imx_gpt_timer_get_count(struct udevice *dev, u64 *count) > +static u64 imx_gpt_timer_get_count(struct udevice *dev) > { > struct imx_gpt_timer_priv *priv = dev_get_priv(dev); > struct imx_gpt_timer_regs *regs = priv->base; > > - *count = readl(®s->cnt); > - > - return 0; > + return readl(®s->cnt); > } > > static int imx_gpt_timer_probe(struct udevice *dev) > @@ -50,7 +69,7 @@ static int imx_gpt_timer_probe(struct udevice *dev) > struct clk clk; > fdt_addr_t addr; > u32 prescaler; > - u32 rate, pr; > + u32 rate; > int ret; > > addr = dev_read_addr(dev); > @@ -84,16 +103,20 @@ static int imx_gpt_timer_probe(struct udevice *dev) > > /* Get timer clock rate */ > rate = clk_get_rate(&clk); > + if((int)rate <= 0){ > + debug("Could not get clock rate, setting to default value...\n"); > + rate = 1056000000UL; > + } > > /* we set timer prescaler to obtain a 1MHz timer counter frequency */ > -// pr = (rate / CONFIG_SYS_HZ_CLOCK) - 1; > - writel(pr, ®s->pr); > - > + prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1; > + writel(GPT_PR_PRESCALER&prescaler, ®s->pr); > /* Set timer frequency to 1MHz */ > - uc_priv->clock_rate = rate / prescaler; > - > + uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK; > + clrbits_le32(®s->cr,GPT_CR_CLKSRC); > + setbits_le32(®s->cr, IPG_CLK); > /* start timer */ > -// setbits_le32(®s->cr1, CR1_CEN); > + setbits_le32(®s->cr, GPT_CR_EN); > > return 0; > } > @@ -111,8 +134,7 @@ U_BOOT_DRIVER(imx_gpt_timer) = { > .name = "imx_gpt_timer", > .id = UCLASS_TIMER, > .of_match = imx_gpt_timer_ids, > - .priv_auto_alloc_size = sizeof(struct imx_gpt_timer_priv), > + .priv_auto = sizeof(struct imx_gpt_timer_priv), > .probe = imx_gpt_timer_probe, > .ops = &imx_gpt_timer_ops, > }; > - >
Hi Jesse, this patch should have been squashed with patch 1/4 as already suggested. On 2/8/21 1:24 AM, Jesse Taube wrote: > Signed-off-by: Jesse Taube <mr.bossman075@gmail.com> > Cc: Stefano Babic <tsbabic@denx.de> > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Cc: Jesse Taube <mr.bossman075@gmail.com> > > This timer driver is using GPT Timer (General Purpose Timer) available > on almost all i.MX SoCs family. Add code to enable timer. Add code get a defualt prescaler. > Add defines for register masks. > --- > drivers/timer/imx-gpt-timer.c | 50 +++++++++++++++++++++++++---------- > 1 file changed, 36 insertions(+), 14 deletions(-) > > diff --git a/drivers/timer/imx-gpt-timer.c b/drivers/timer/imx-gpt-timer.c > index c4e8c12a42..ed6487af97 100644 > --- a/drivers/timer/imx-gpt-timer.c > +++ b/drivers/timer/imx-gpt-timer.c > @@ -13,8 +13,29 @@ > > #include <asm/io.h> > > -#define GPT_CR_SWR 0x00008000 > +#define GPT_CR_SWR 0x00008000 > +#define GPT_CR_CLKSRC 0x000001C0 > +#define GPT_CR_EN_24M 0x00004000 > +#define GPT_CR_EN 0x00000001 > +#define GPT_PR_PRESCALER 0x00000FFF > +#define GPT_PR_PRESCALER24M 0x0000F000 > > + > + Please avoid one whiteline > +#define NO_CLOCK (0) > +#define IPG_CLK (1<<6) > +#define IPG_CLK_HF (2<<6) > +#define IPG_EXT (3<<6) > +#define IPG_CLK_32K (4<<6) > +#define IPG_CLK_24M (5<<6) > + > + ditto > +/* > +ipg_clk ipg_clk_root Peripheral clock > +ipg_clk_32k ckil_sync_clk_root Low-frequency reference clock (32 kHz) > +ipg_clk_highfreq perclk_clk_root High-frequency reference clock > +ipg_clk_s ipg_clk_root Peripheral access clock > +*/ > struct imx_gpt_timer_regs { > u32 cr; > u32 pr; > @@ -32,14 +53,12 @@ struct imx_gpt_timer_priv { > struct imx_gpt_timer_regs *base; > }; > > -static int imx_gpt_timer_get_count(struct udevice *dev, u64 *count) > +static u64 imx_gpt_timer_get_count(struct udevice *dev) > { > struct imx_gpt_timer_priv *priv = dev_get_priv(dev); > struct imx_gpt_timer_regs *regs = priv->base; > > - *count = readl(®s->cnt); > - > - return 0; > + return readl(®s->cnt); > } > > static int imx_gpt_timer_probe(struct udevice *dev) > @@ -50,7 +69,7 @@ static int imx_gpt_timer_probe(struct udevice *dev) > struct clk clk; > fdt_addr_t addr; > u32 prescaler; > - u32 rate, pr; > + u32 rate; > int ret; > > addr = dev_read_addr(dev); > @@ -84,16 +103,20 @@ static int imx_gpt_timer_probe(struct udevice *dev) > > /* Get timer clock rate */ > rate = clk_get_rate(&clk); > + if((int)rate <= 0){ > + debug("Could not get clock rate, setting to default value...\n"); > + rate = 1056000000UL; > + } Here I don't agree, having a timer ticking at 1ms(1Khz) doesn't justify having a parent clock of 1Ghz. What I've thought here was using 24M crystal directly. But obviously this is determined by clk_get_rate(). So IMHO I would keep 24M only as source and only if needed I would make it different. Using only that reference you make this driver accepting only a 24M clock, but to begin it's enough for me. Most of i.MX* SoC setup exactly timer using 24M source clock. Also using ipg clock implies that clock drivers supports it, but it doesn't so this driver shouldn't work as is. Best regards
diff --git a/drivers/timer/imx-gpt-timer.c b/drivers/timer/imx-gpt-timer.c index c4e8c12a42..ed6487af97 100644 --- a/drivers/timer/imx-gpt-timer.c +++ b/drivers/timer/imx-gpt-timer.c @@ -13,8 +13,29 @@ #include <asm/io.h> -#define GPT_CR_SWR 0x00008000 +#define GPT_CR_SWR 0x00008000 +#define GPT_CR_CLKSRC 0x000001C0 +#define GPT_CR_EN_24M 0x00004000 +#define GPT_CR_EN 0x00000001 +#define GPT_PR_PRESCALER 0x00000FFF +#define GPT_PR_PRESCALER24M 0x0000F000 + + +#define NO_CLOCK (0) +#define IPG_CLK (1<<6) +#define IPG_CLK_HF (2<<6) +#define IPG_EXT (3<<6) +#define IPG_CLK_32K (4<<6) +#define IPG_CLK_24M (5<<6) + + +/* +ipg_clk ipg_clk_root Peripheral clock +ipg_clk_32k ckil_sync_clk_root Low-frequency reference clock (32 kHz) +ipg_clk_highfreq perclk_clk_root High-frequency reference clock +ipg_clk_s ipg_clk_root Peripheral access clock +*/ struct imx_gpt_timer_regs { u32 cr; u32 pr; @@ -32,14 +53,12 @@ struct imx_gpt_timer_priv { struct imx_gpt_timer_regs *base; }; -static int imx_gpt_timer_get_count(struct udevice *dev, u64 *count) +static u64 imx_gpt_timer_get_count(struct udevice *dev) { struct imx_gpt_timer_priv *priv = dev_get_priv(dev); struct imx_gpt_timer_regs *regs = priv->base; - *count = readl(®s->cnt); - - return 0; + return readl(®s->cnt); } static int imx_gpt_timer_probe(struct udevice *dev) @@ -50,7 +69,7 @@ static int imx_gpt_timer_probe(struct udevice *dev) struct clk clk; fdt_addr_t addr; u32 prescaler; - u32 rate, pr; + u32 rate; int ret; addr = dev_read_addr(dev); @@ -84,16 +103,20 @@ static int imx_gpt_timer_probe(struct udevice *dev) /* Get timer clock rate */ rate = clk_get_rate(&clk); + if((int)rate <= 0){ + debug("Could not get clock rate, setting to default value...\n"); + rate = 1056000000UL; + } /* we set timer prescaler to obtain a 1MHz timer counter frequency */ -// pr = (rate / CONFIG_SYS_HZ_CLOCK) - 1; - writel(pr, ®s->pr); - + prescaler = (rate / CONFIG_SYS_HZ_CLOCK) - 1; + writel(GPT_PR_PRESCALER&prescaler, ®s->pr); /* Set timer frequency to 1MHz */ - uc_priv->clock_rate = rate / prescaler; - + uc_priv->clock_rate = CONFIG_SYS_HZ_CLOCK; + clrbits_le32(®s->cr,GPT_CR_CLKSRC); + setbits_le32(®s->cr, IPG_CLK); /* start timer */ -// setbits_le32(®s->cr1, CR1_CEN); + setbits_le32(®s->cr, GPT_CR_EN); return 0; } @@ -111,8 +134,7 @@ U_BOOT_DRIVER(imx_gpt_timer) = { .name = "imx_gpt_timer", .id = UCLASS_TIMER, .of_match = imx_gpt_timer_ids, - .priv_auto_alloc_size = sizeof(struct imx_gpt_timer_priv), + .priv_auto = sizeof(struct imx_gpt_timer_priv), .probe = imx_gpt_timer_probe, .ops = &imx_gpt_timer_ops, }; -
Signed-off-by: Jesse Taube <mr.bossman075@gmail.com> Cc: Stefano Babic <tsbabic@denx.de> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> Cc: Jesse Taube <mr.bossman075@gmail.com> This timer driver is using GPT Timer (General Purpose Timer) available on almost all i.MX SoCs family. Add code to enable timer. Add code get a defualt prescaler. Add defines for register masks. --- drivers/timer/imx-gpt-timer.c | 50 +++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 14 deletions(-)