[U-Boot] [RFC 17/35] clk: sunxi: Add initial CLK driver for H3_H5
Jagan Teki
jagan at amarulasolutions.com
Tue Jul 17 16:50:42 UTC 2018
On Tue, Jul 17, 2018 at 6:06 PM, Maxime Ripard
<maxime.ripard at bootlin.com> wrote:
> On Mon, Jul 16, 2018 at 05:55:25PM +0100, Andre Przywara wrote:
>> Hi,
>>
>> On 16/07/18 13:59, Maxime Ripard wrote:
>> > On Mon, Jul 16, 2018 at 04:58:32PM +0530, Jagan Teki wrote:
>> >> Add initial clock driver Allwinner for H3_H5.
>> >>
>> >> Implemented clock enable and disable functions for
>> >> USB OHCI, EHCI, OTG and PHY gate and clock registers.
>> >>
>> >> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>> >> ---
>> >> drivers/clk/sunxi/Kconfig | 7 ++
>> >> drivers/clk/sunxi/Makefile | 2 +
>> >> drivers/clk/sunxi/clk_h3.c | 131 +++++++++++++++++++++++++++++++++++++
>> >> 3 files changed, 140 insertions(+)
>> >> create mode 100644 drivers/clk/sunxi/clk_h3.c
>> >>
>> >> diff --git a/drivers/clk/sunxi/Kconfig b/drivers/clk/sunxi/Kconfig
>> >> index 3a86c91e75..065cadf2fe 100644
>> >> --- a/drivers/clk/sunxi/Kconfig
>> >> +++ b/drivers/clk/sunxi/Kconfig
>> >> @@ -8,6 +8,13 @@ config CLK_SUNXI
>> >>
>> >> if CLK_SUNXI
>> >>
>> >> +config CLK_SUN8I_H3
>> >> + bool "Clock driver for Allwinner H3/H5"
>> >> + default MACH_SUNXI_H3_H5
>> >> + help
>> >> + This enables common clock driver support for platforms based
>> >> + on Allwinner H3/H5 SoC.
>> >> +
>> >> config CLK_SUN50I_A64
>> >> bool "Clock driver for Allwinner A64"
>> >> default MACH_SUN50I
>> >> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> >> index 860bb6dfea..37e6bcb147 100644
>> >> --- a/drivers/clk/sunxi/Makefile
>> >> +++ b/drivers/clk/sunxi/Makefile
>> >> @@ -5,4 +5,6 @@
>> >> #
>> >>
>> >> obj-$(CONFIG_CLK_SUNXI) += clk_sunxi.o
>> >> +
>> >> +obj-$(CONFIG_CLK_SUN8I_H3) += clk_h3.o
>> >> obj-$(CONFIG_CLK_SUN50I_A64) += clk_a64.o
>> >> diff --git a/drivers/clk/sunxi/clk_h3.c b/drivers/clk/sunxi/clk_h3.c
>> >> new file mode 100644
>> >> index 0000000000..e924017717
>> >> --- /dev/null
>> >> +++ b/drivers/clk/sunxi/clk_h3.c
>> >> @@ -0,0 +1,131 @@
>> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> >> +/*
>> >> + * Copyright (C) 2018 Amarula Solutions B.V.
>> >> + * Author: Jagan Teki <jagan at amarulasolutions.com>
>> >> + */
>> >> +
>> >> +#include <common.h>
>> >> +#include <clk-uclass.h>
>> >> +#include <dm.h>
>> >> +#include <errno.h>
>> >> +#include <asm/io.h>
>> >> +#include <asm/arch/clock.h>
>> >> +#include <dt-bindings/clock/sun8i-h3-ccu.h>
>> >> +
>> >> +struct h3_clk_priv {
>> >> + void *base;
>> >> +};
>> >> +
>> >> +static int h3_clk_enable(struct clk *clk)
>> >> +{
>> >> + struct h3_clk_priv *priv = dev_get_priv(clk->dev);
>> >> +
>> >> + debug("%s(#%ld)\n", __func__, clk->id);
>> >> +
>> >> + switch (clk->id) {
>> >> + case CLK_BUS_OTG:
>> >> + case CLK_BUS_EHCI0:
>> >> + case CLK_BUS_EHCI1:
>> >> + case CLK_BUS_EHCI2:
>> >> + case CLK_BUS_EHCI3:
>> >> + case CLK_BUS_OHCI0:
>> >> + case CLK_BUS_OHCI1:
>> >> + case CLK_BUS_OHCI2:
>> >> + case CLK_BUS_OHCI3:
>> >> + setbits_le32(priv->base + 0x60,
>> >> + BIT(23 + (clk->id - CLK_BUS_OTG)));
>> >> + return 0;
>> >> + case CLK_USB_PHY0:
>> >> + case CLK_USB_PHY1:
>> >> + case CLK_USB_PHY2:
>> >> + case CLK_USB_PHY3:
>> >> + setbits_le32(priv->base + 0xcc,
>> >> + BIT(8 + (clk->id - CLK_USB_PHY0)));
>> >> + return 0;
>> >> + case CLK_USB_OHCI0:
>> >> + case CLK_USB_OHCI1:
>> >> + case CLK_USB_OHCI2:
>> >> + case CLK_USB_OHCI3:
>> >> + setbits_le32(priv->base + 0xcc,
>> >> + BIT(16 + (clk->id - CLK_USB_OHCI0)));
>> >> + return 0;
>> >> + default:
>> >> + debug("%s (CLK#%ld) unhandled\n", __func__, clk->id);
>> >> + return -ENODEV;
>> >> + }
>> >> +}
>> >> +
>> >> +static int h3_clk_disable(struct clk *clk)
>> >> +{
>> >> + struct h3_clk_priv *priv = dev_get_priv(clk->dev);
>> >> +
>> >> + debug("%s(#%ld)\n", __func__, clk->id);
>> >> +
>> >> + switch (clk->id) {
>> >> + case CLK_BUS_OTG:
>> >> + case CLK_BUS_EHCI0:
>> >> + case CLK_BUS_EHCI1:
>> >> + case CLK_BUS_EHCI2:
>> >> + case CLK_BUS_EHCI3:
>> >> + case CLK_BUS_OHCI0:
>> >> + case CLK_BUS_OHCI1:
>> >> + case CLK_BUS_OHCI2:
>> >> + case CLK_BUS_OHCI3:
>> >> + clrbits_le32(priv->base + 0x60,
>> >> + BIT(23 + (clk->id - CLK_BUS_OTG)));
>> >> + return 0;
>> >> + case CLK_USB_PHY0:
>> >> + case CLK_USB_PHY1:
>> >> + case CLK_USB_PHY2:
>> >> + case CLK_USB_PHY3:
>> >> + clrbits_le32(priv->base + 0xcc,
>> >> + BIT(8 + (clk->id - CLK_USB_PHY0)));
>> >> + return 0;
>> >> + case CLK_USB_OHCI0:
>> >> + case CLK_USB_OHCI1:
>> >> + case CLK_USB_OHCI2:
>> >> + case CLK_USB_OHCI3:
>> >> + clrbits_le32(priv->base + 0xcc,
>> >> + BIT(16 + (clk->id - CLK_USB_OHCI0)));
>> >> + return 0;
>> >> + default:
>> >> + debug("%s (CLK#%ld) unhandled\n", __func__, clk->id);
>> >> + return -ENODEV;
>> >> + }
>> >> +}
>> >> +
>> >> +static struct clk_ops h3_clk_ops = {
>> >> + .enable = h3_clk_enable,
>> >> + .disable = h3_clk_disable,
>> >> +};
>> >> +
>> >> +static int h3_clk_probe(struct udevice *dev)
>> >> +{
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int h3_clk_ofdata_to_platdata(struct udevice *dev)
>> >> +{
>> >> + struct h3_clk_priv *priv = dev_get_priv(dev);
>> >> +
>> >> + priv->base = dev_read_addr_ptr(dev);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static const struct udevice_id h3_clk_ids[] = {
>> >> + { .compatible = "allwinner,sun8i-h3-ccu" },
>> >> + { .compatible = "allwinner,sun50i-h5-ccu" },
>> >> + { }
>> >> +};
>> >> +
>> >> +U_BOOT_DRIVER(clk_sun8i_h3) = {
>> >> + .name = "sun8i_h3_ccu",
>> >> + .id = UCLASS_CLK,
>> >> + .of_match = h3_clk_ids,
>> >> + .priv_auto_alloc_size = sizeof(struct h3_clk_priv),
>> >> + .ofdata_to_platdata = h3_clk_ofdata_to_platdata,
>> >> + .ops = &h3_clk_ops,
>> >> + .probe = h3_clk_probe,
>> >> + .bind = sunxi_clk_bind,
>> >> +};
>> >
>> > Speaking from experience, you do not want to have separate
>> > implementations for each and every SoCs. This might be enough for
>> > enabling / disabling the clocks, but as soon as you'll throw the
>> > set_rate / get_rate callbacks into the mix it's going to turn into a
>> > nightmare.
>>
>> I agree, but I guess it won't be too pretty anyway:
>> The CLK_BUS_[EO]HCIx definitions are different for each SoC, but share
>> the same symbol. So we can't use a nicely readable switch/case anymore.
>> Unless we translate the values to a common namespace?
>>
>> But I support that we should share as much code as possible, maybe by
>> using macros to instantiate the driver boilerplates and by using a
>> shared file with the gist of the clock programming code and then just
>> have shim layers to connect the bits?
>>
>> In case it's just bit and register offsets differing, we could define a
>> structure holding register and bit offsets, filling this for the various
>> SoCs, then tie those together with the compatible strings:
>> struct sunxi_clk_defs {
>> uint16_t clk_bus_usb_offset;
>> uint16_t clk_bus_usb_bit;
>> ...
>> } sun8i_h3_h5_clk_defs = {
>> .clk_bus_usb_offset = 0x60;
>> .clk_bus_usb_bit = 23;
>> };
>> ... case CLK_BUS_OHCI3:
>> clrbits_le32(priv->base + priv->clk_bus_usb_offset,
>> BIT(priv->clk_bus_usb_bit + (clk->id - CLK_BUS_OTG)));
>> ....
>> static const struct udevice_id sunxi_clk_ids[] = {
>> { .compatible = "allwinner,sun8i-h3-ccu",
>> .data = sun8i_h3_h5_clk_defs },
>> };
>>
>> Just an example, not sure we are actually much different in those bits
>> there.
>>
>> Or we put the DT clock numbers into that struct and look those up:
>> int sunxi_clk_bus_usb_idx (struct sunxi_clk_defs *priv, int clkid)
>> {
>> if (clkid >= priv->first_bus_usb &&
>> clkid <= priv->last_bus_usb)
>> return clkid - priv->first_bus_usb;
>> return -1;
>> }
>> static int h3_clk_enable(struct clk *clk)
>> {
>> ...
>> idx = sunxi_clk_bus_usb_idx(priv, clk->id));
>> if (idx >= 0)
>> setbits_le32(priv->base + 0x60, BIT(23 + idx));
>> idx = sunxi_clk_usb_phy_idx(priv, clk->id));
>> if (idx >= 0)
>> setbits_le32(priv->base + 0xcc, BIT(8 + idx));
>
> I guess we could also give to a common code a key / value (register -
> offset) pair that would be SoC specific, a simpler version of what
> we're doing in Linux.
is it something like ccu_reset_map struct with reg and bit members?
[RST_USB_PHY0] = { 0x0cc, BIT(0) },
More information about the U-Boot
mailing list