| Message ID | 20220801150106.1.I5e77bf65d8eb58cb0e4691334381b4bc1ec857b2@changeid |
|---|---|
| State | Rejected |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | timer: bcmbca: Add Broadcom BCMBCA timer support | expand |
On 2.08.2022 00:03, William Zhang wrote: > This driver supports the peripheral block timer found on the Broadcom > BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used > as the system clock source such as on BCM63138. > > Signed-off-by: William Zhang <william.zhang@broadcom.com> > > (...) > > +static const struct udevice_id bcmbca_timer_ids[] = { > + { .compatible = "brcm,bcm-timers" }, > + { } > +}; > + > +U_BOOT_DRIVER(bcmbca_timer) = { > + .name = "bcmbca_timer", > + .id = UCLASS_TIMER, > + .of_match = bcmbca_timer_ids, > + .priv_auto = sizeof(struct bcmbca_timer_priv), > + .probe = bcmbca_timer_probe, > + .ops = &bcmbca_timer_ops, > + .flags = DM_FLAG_PRE_RELOC, > +}; That "brcm,bcm-timers" seems like a really wide bidding. Is that exact timer block guaranteed to be present on all Broadcom devices? Does it exist e.g. on Northstar SoCs? Or old MIPS SoCs like BCM4706? It seems that even across BCMBCA devices this block may differ and may need different bindings. Most SoCs have 4 CTL and 4 CNT registers but some have only 3 + 3 (BCM6838 BCM60333 BCM63268). Finally could we have that binding actually documented?
Hi Rafal, On 08/01/2022 10:26 PM, Rafał Miłecki wrote: > On 2.08.2022 00:03, William Zhang wrote: >> This driver supports the peripheral block timer found on the Broadcom >> BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used >> as the system clock source such as on BCM63138. >> Signed-off-by: William Zhang <william.zhang@broadcom.com> >> >> (...) >> >> +static const struct udevice_id bcmbca_timer_ids[] = { >> + { .compatible = "brcm,bcm-timers" }, >> + { } >> +}; >> + >> +U_BOOT_DRIVER(bcmbca_timer) = { >> + .name = "bcmbca_timer", >> + .id = UCLASS_TIMER, >> + .of_match = bcmbca_timer_ids, >> + .priv_auto = sizeof(struct bcmbca_timer_priv), >> + .probe = bcmbca_timer_probe, >> + .ops = &bcmbca_timer_ops, >> + .flags = DM_FLAG_PRE_RELOC, >> +}; > > That "brcm,bcm-timers" seems like a really wide bidding. Is that exact > timer block guaranteed to be present on all Broadcom devices? Does it > exist e.g. on Northstar SoCs? Or old MIPS SoCs like BCM4706? > Agree I will make change to use brcm,bcmbca-timers. > It seems that even across BCMBCA devices this block may differ and may > need different bindings. Most SoCs have 4 CTL and 4 CNT registers but > some have only 3 + 3 (BCM6838 BCM60333 BCM63268). This timer driver is intended for the clock source for u-boot only so only need the first channel. Although it could work for the old mips based dsl chip, BCMBCA chip family support is only for the new ARM based chips which is stated in the BCMBA introduction patch. > > Finally could we have that binding actually documented? I don't see u-boot has binding document. I believe it generally use linux binding document. I have no plan to upstream this driver to linux. But I can put the binding info in the driver if that is the right way to do in u-boot.
On 8/2/2022 5:07 PM, William Zhang wrote: > Hi Rafal, > > On 08/01/2022 10:26 PM, Rafał Miłecki wrote: >> On 2.08.2022 00:03, William Zhang wrote: >>> This driver supports the peripheral block timer found on the Broadcom >>> BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used >>> as the system clock source such as on BCM63138. >>> Signed-off-by: William Zhang <william.zhang@broadcom.com> >>> >>> (...) >>> >>> +static const struct udevice_id bcmbca_timer_ids[] = { >>> + { .compatible = "brcm,bcm-timers" }, >>> + { } >>> +}; >>> + >>> +U_BOOT_DRIVER(bcmbca_timer) = { >>> + .name = "bcmbca_timer", >>> + .id = UCLASS_TIMER, >>> + .of_match = bcmbca_timer_ids, >>> + .priv_auto = sizeof(struct bcmbca_timer_priv), >>> + .probe = bcmbca_timer_probe, >>> + .ops = &bcmbca_timer_ops, >>> + .flags = DM_FLAG_PRE_RELOC, >>> +}; >> >> That "brcm,bcm-timers" seems like a really wide bidding. Is that exact >> timer block guaranteed to be present on all Broadcom devices? Does it >> exist e.g. on Northstar SoCs? Or old MIPS SoCs like BCM4706? >> > Agree I will make change to use brcm,bcmbca-timers. Why not change to use a compatible string that uses the first chip in which this timer block was introduced, was that 6345 or later? > >> It seems that even across BCMBCA devices this block may differ and may >> need different bindings. Most SoCs have 4 CTL and 4 CNT registers but >> some have only 3 + 3 (BCM6838 BCM60333 BCM63268). > This timer driver is intended for the clock source for u-boot only so > only need the first channel. Although it could work for the old mips > based dsl chip, BCMBCA chip family support is only for the new ARM based > chips which is stated in the BCMBA introduction patch. > >> >> Finally could we have that binding actually documented? > I don't see u-boot has binding document. I believe it generally use > linux binding document. I have no plan to upstream this driver to > linux. But I can put the binding info in the driver if that is the right > way to do in u-boot. Out of curiosity, why does u-boot require this timer as opposed to using the Cortex-A9 architected timers? Is it necessary to boot strap the A9 timers?
Hi Florian, On 08/03/2022 08:21 PM, Florian Fainelli wrote: > > > On 8/2/2022 5:07 PM, William Zhang wrote: >> Hi Rafal, >> >> On 08/01/2022 10:26 PM, Rafał Miłecki wrote: >>> On 2.08.2022 00:03, William Zhang wrote: >>>> This driver supports the peripheral block timer found on the Broadcom >>>> BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used >>>> as the system clock source such as on BCM63138. >>>> Signed-off-by: William Zhang <william.zhang@broadcom.com> >>>> >>>> (...) >>>> >>>> +static const struct udevice_id bcmbca_timer_ids[] = { >>>> + { .compatible = "brcm,bcm-timers" }, >>>> + { } >>>> +}; >>>> + >>>> +U_BOOT_DRIVER(bcmbca_timer) = { >>>> + .name = "bcmbca_timer", >>>> + .id = UCLASS_TIMER, >>>> + .of_match = bcmbca_timer_ids, >>>> + .priv_auto = sizeof(struct bcmbca_timer_priv), >>>> + .probe = bcmbca_timer_probe, >>>> + .ops = &bcmbca_timer_ops, >>>> + .flags = DM_FLAG_PRE_RELOC, >>>> +}; >>> >>> That "brcm,bcm-timers" seems like a really wide bidding. Is that exact >>> timer block guaranteed to be present on all Broadcom devices? Does it >>> exist e.g. on Northstar SoCs? Or old MIPS SoCs like BCM4706? >>> >> Agree I will make change to use brcm,bcmbca-timers. > > Why not change to use a compatible string that uses the first chip in > which this timer block was introduced, was that 6345 or later? > I don't mind to change to 6345. That chip should support the same timer block. Looks u-boot has a lot of Broadcom MIPS based chips that uses 6345 in compatible string name for various blocks. >> >>> It seems that even across BCMBCA devices this block may differ and may >>> need different bindings. Most SoCs have 4 CTL and 4 CNT registers but >>> some have only 3 + 3 (BCM6838 BCM60333 BCM63268). >> This timer driver is intended for the clock source for u-boot only so >> only need the first channel. Although it could work for the old mips >> based dsl chip, BCMBCA chip family support is only for the new ARM >> based chips which is stated in the BCMBA introduction patch. >> >>> >>> Finally could we have that binding actually documented? >> I don't see u-boot has binding document. I believe it generally use >> linux binding document. I have no plan to upstream this driver to >> linux. But I can put the binding info in the driver if that is the >> right way to do in u-boot. > > Out of curiosity, why does u-boot require this timer as opposed to using > the Cortex-A9 architected timers? Is it necessary to boot strap the A9 > timers? Cortex-A9 has an arch timer called global timer. But I don't see a generic global timer driver in the u-boot. There is one from STMicroelectronics sti-timer.c which is a a9 global timer but it is implemented under STI_TIMER and default enabled for ARCH_STI. I guess we can use that too when we have the clk driver(iproc clock driver) ported to u-boot. Just curious, does anyone in this list know why sti timer is not put under global timer config and file name? I don't see anything in that driver is sti specific. CC'ed driver maintainer Patrice as well.
On 08/04/2022 01:25 PM, William Zhang wrote: > Hi Florian, > > On 08/03/2022 08:21 PM, Florian Fainelli wrote: >> >> >> On 8/2/2022 5:07 PM, William Zhang wrote: >>> Hi Rafal, >>> >>> On 08/01/2022 10:26 PM, Rafał Miłecki wrote: >>>> On 2.08.2022 00:03, William Zhang wrote: >>>>> This driver supports the peripheral block timer found on the Broadcom >>>>> BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used >>>>> as the system clock source such as on BCM63138. >>>>> Signed-off-by: William Zhang <william.zhang@broadcom.com> >>>>> >>>>> (...) >>>>> >>>>> +static const struct udevice_id bcmbca_timer_ids[] = { >>>>> + { .compatible = "brcm,bcm-timers" }, >>>>> + { } >>>>> +}; >>>>> + >>>>> +U_BOOT_DRIVER(bcmbca_timer) = { >>>>> + .name = "bcmbca_timer", >>>>> + .id = UCLASS_TIMER, >>>>> + .of_match = bcmbca_timer_ids, >>>>> + .priv_auto = sizeof(struct bcmbca_timer_priv), >>>>> + .probe = bcmbca_timer_probe, >>>>> + .ops = &bcmbca_timer_ops, >>>>> + .flags = DM_FLAG_PRE_RELOC, >>>>> +}; >>>> >>>> That "brcm,bcm-timers" seems like a really wide bidding. Is that exact >>>> timer block guaranteed to be present on all Broadcom devices? Does it >>>> exist e.g. on Northstar SoCs? Or old MIPS SoCs like BCM4706? >>>> >>> Agree I will make change to use brcm,bcmbca-timers. >> >> Why not change to use a compatible string that uses the first chip in >> which this timer block was introduced, was that 6345 or later? >> > I don't mind to change to 6345. That chip should support the same timer > block. Looks u-boot has a lot of Broadcom MIPS based chips that uses > 6345 in compatible string name for various blocks. > >>> >>>> It seems that even across BCMBCA devices this block may differ and may >>>> need different bindings. Most SoCs have 4 CTL and 4 CNT registers but >>>> some have only 3 + 3 (BCM6838 BCM60333 BCM63268). >>> This timer driver is intended for the clock source for u-boot only so >>> only need the first channel. Although it could work for the old mips >>> based dsl chip, BCMBCA chip family support is only for the new ARM >>> based chips which is stated in the BCMBA introduction patch. >>> >>>> >>>> Finally could we have that binding actually documented? >>> I don't see u-boot has binding document. I believe it generally use >>> linux binding document. I have no plan to upstream this driver to >>> linux. But I can put the binding info in the driver if that is the >>> right way to do in u-boot. >> >> Out of curiosity, why does u-boot require this timer as opposed to >> using the Cortex-A9 architected timers? Is it necessary to boot strap >> the A9 timers? > Cortex-A9 has an arch timer called global timer. But I don't see a > generic global timer driver in the u-boot. There is one from > STMicroelectronics sti-timer.c which is a a9 global timer but it is > implemented under STI_TIMER and default enabled for ARCH_STI. I guess we > can use that too when we have the clk driver(iproc clock driver) ported > to u-boot. Just curious, does anyone in this list know why sti timer is > not put under global timer config and file name? I don't see anything > in that driver is sti specific. CC'ed driver maintainer Patrice as well. Since there is already global timer support in u-boot, I decided to use that for BCM63138. So I withdrew this patch and please discard. I will also send a patch to move the sti a9 global timer to generic arm a9 global timer driver if Patrice agrees or I don't hear from him back next week.
diff --git a/MAINTAINERS b/MAINTAINERS index d1d13c733db4..8c3a7d77f17d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -216,6 +216,7 @@ M: Joel Peshkin <joel.peshkin@broadcom.com> S: Maintained F: arch/arm/mach-bcmbca/ F: board/broadcom/bcmbca/ +F: drivers/timer/bcmbca-timer.c N: bcmbca N: bcm[9]?47622 N: bcm[9]?63148 diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 20b5af7e260f..2e5af97b3ad5 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -125,6 +125,14 @@ config SPL_ATMEL_TCB_TIMER Select this to enable the use of the timer counter as a monotonic counter in SPL. + +config BCMBCA_TIMER + bool "Broadcom BCMBCA SoC timer support" + depends on TIMER + help + Select this to enable peripheral timer support for Broadcom + BCMBCA SoC devices + config CADENCE_TTC_TIMER bool "Cadence TTC (Triple Timer Counter)" depends on TIMER diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index d9822a537009..d581db7563c5 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -10,6 +10,7 @@ obj-$(CONFIG_AST_TIMER) += ast_timer.o obj-$(CONFIG_ATCPIT100_TIMER) += atcpit100_timer.o obj-$(CONFIG_$(SPL_)ATMEL_PIT_TIMER) += atmel_pit_timer.o obj-$(CONFIG_$(SPL_)ATMEL_TCB_TIMER) += atmel_tcb_timer.o +obj-$(CONFIG_BCMBCA_TIMER) += bcmbca_timer.o obj-$(CONFIG_CADENCE_TTC_TIMER) += cadence-ttc.o obj-$(CONFIG_DESIGNWARE_APB_TIMER) += dw-apb-timer.o obj-$(CONFIG_GXP_TIMER) += gxp-timer.o diff --git a/drivers/timer/bcmbca_timer.c b/drivers/timer/bcmbca_timer.c new file mode 100644 index 000000000000..78a8a67f22a7 --- /dev/null +++ b/drivers/timer/bcmbca_timer.c @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Broadcom BCMBCA Broadband SoC timer driver + * Copyright 2022 Broadcom Ltd. + */ + +#include <common.h> +#include <dm.h> +#include <timer.h> +#include <clk.h> +#include <asm/io.h> +#include <linux/ioport.h> +#include <linux/io.h> + +#define TIMER_CTL 0 +#define TIMER_CNT 0x10 + +#define BCMBCA_TIMER_ENABLE BIT(31) +#define BCMBCA_TIMER_CNT_MASK 0x3fffffff +#define BCMBCA_TIMER_CNT_HIGH_SHIFT 30 + +struct bcmbca_timer_priv { + void __iomem *reg_base; + u32 cnt_high; + u32 cnt_low; +}; + +static u64 bcmbca_timer_get_count(struct udevice *dev) +{ + u64 cnt_high_64; + struct bcmbca_timer_priv *priv = dev_get_priv(dev); + u32 val = readl(priv->reg_base + TIMER_CNT) & BCMBCA_TIMER_CNT_MASK; + + /* handle hardware counter overflow case*/ + if (val < priv->cnt_low) + priv->cnt_high++; + priv->cnt_low = val; + cnt_high_64 = (u64)priv->cnt_high; + + return (cnt_high_64 << BCMBCA_TIMER_CNT_HIGH_SHIFT | priv->cnt_low); +} + +static void bcmbca_timer_start(struct udevice *dev) +{ + struct bcmbca_timer_priv *priv = dev_get_priv(dev); + + writel(BCMBCA_TIMER_ENABLE | BCMBCA_TIMER_CNT_MASK, + priv->reg_base + TIMER_CTL); +} + +static int bcmbca_timer_probe(struct udevice *dev) +{ + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + struct bcmbca_timer_priv *priv = dev_get_priv(dev); + struct clk clk; + int ret; + + priv->reg_base = dev_remap_addr(dev); + + if (!priv->reg_base) + return -ENOENT; + + ret = clk_get_by_index(dev, 0, &clk); + if (ret) + return ret; + + uc_priv->clock_rate = clk_get_rate(&clk); + if (!uc_priv->clock_rate) + return -EINVAL; + + bcmbca_timer_start(dev); + + return 0; +} + +static const struct timer_ops bcmbca_timer_ops = { + .get_count = bcmbca_timer_get_count, +}; + +static const struct udevice_id bcmbca_timer_ids[] = { + { .compatible = "brcm,bcm-timers" }, + { } +}; + +U_BOOT_DRIVER(bcmbca_timer) = { + .name = "bcmbca_timer", + .id = UCLASS_TIMER, + .of_match = bcmbca_timer_ids, + .priv_auto = sizeof(struct bcmbca_timer_priv), + .probe = bcmbca_timer_probe, + .ops = &bcmbca_timer_ops, + .flags = DM_FLAG_PRE_RELOC, +};
This driver supports the peripheral block timer found on the Broadcom BCA SoCs. It is 30-bit up-count timer running at 50MHz and can be used as the system clock source such as on BCM63138. Signed-off-by: William Zhang <william.zhang@broadcom.com> --- MAINTAINERS | 1 + drivers/timer/Kconfig | 8 ++++ drivers/timer/Makefile | 1 + drivers/timer/bcmbca_timer.c | 93 ++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+) create mode 100644 drivers/timer/bcmbca_timer.c