Re: [PATCH v2 2/3] clk: meson: meson8b: register the built-in reset controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Neil,

On Tue, Jul 25, 2017 at 9:58 AM, Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote:
>
>
> Le 22/07/2017 20:58, Martin Blumenstingl a écrit :
>> The clock controller also includes some reset lines. This patch
>> implements a reset controller to assert and de-assert these resets.
>> The reset controller itself is registered early (through
>> CLK_OF_DECLARE_DRIVER) because it is needed very early in the boot
>> process (to start the secondary CPU cores).
>>
>> According to the public S805 datasheet there are two more reset bits
>> in the HHI_SYS_CPU_CLK_CNTL0 register, which are not implemented by
>> this patch (as these seem to be unused in Amlogic's vendor Linux kernel
>> sources and their u-boot tree):
>> - bit 15: GEN_DIV_SOFT_RESET
>> - bit 14: SOFT_RESET
>>
>> All information was taken from the public S805 Datasheet and Amlogic's
>> vendor GPL kernel sources. This patch is based on an earlier version
>> submitted by Carlo Caione.
>>
>> Suggested-by: Carlo Caione <carlo@xxxxxxxxxxxx>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
>> ---
>>  drivers/clk/meson/Kconfig   |   1 +
>>  drivers/clk/meson/meson8b.c | 159 ++++++++++++++++++++++++++++++++++++++++----
>>  drivers/clk/meson/meson8b.h |  25 ++++++-
>>  3 files changed, 172 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index 5588f75a8414..d2d0174a6eca 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -6,6 +6,7 @@ config COMMON_CLK_AMLOGIC
>>  config COMMON_CLK_MESON8B
>>       bool
>>       depends on COMMON_CLK_AMLOGIC
>> +     select RESET_CONTROLLER
>>       help
>>         Support for the clock controller on AmLogic S802 (Meson8),
>>         S805 (Meson8b) and S812 (Meson8m2) devices. Say Y if you
>> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
>> index bb3f1de876b1..0f853057885c 100644
>> --- a/drivers/clk/meson/meson8b.c
>> +++ b/drivers/clk/meson/meson8b.c
>> @@ -25,6 +25,8 @@
>>  #include <linux/clk-provider.h>
>>  #include <linux/of_address.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/slab.h>
>>  #include <linux/init.h>
>>
>>  #include "clkc.h"
>> @@ -32,6 +34,13 @@
>>
>>  static DEFINE_SPINLOCK(clk_lock);
>>
>> +static void __iomem *clk_base;
>> +
>> +struct meson8b_clk_reset {
>> +     struct reset_controller_dev reset;
>> +     void __iomem *base;
>> +};
>> +
>>  static const struct pll_rate_table sys_pll_rate_table[] = {
>>       PLL_RATE(312000000, 52, 1, 2),
>>       PLL_RATE(336000000, 56, 1, 2),
>> @@ -690,20 +699,114 @@ static struct clk_divider *const meson8b_clk_dividers[] = {
>>       &meson8b_mpeg_clk_div,
>>  };
>>
>> +static const struct meson8b_clk_reset_line {
>> +     u32 reg;
>> +     u8 bit_idx;
>> +} meson8b_clk_reset_bits[] = {
>> +     [RESETID_L2_CACHE_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 30
>> +     },
>> +     [RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 29
>> +     },
>> +     [RESETID_SCU_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 28
>> +     },
>> +     [RESETID_CPU3_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 27
>> +     },
>> +     [RESETID_CPU2_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 26
>> +     },
>> +     [RESETID_CPU1_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 25
>> +     },
>> +     [RESETID_CPU0_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 24
>> +     },
>> +     [RESETID_A5_GLOBAL_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 18
>> +     },
>> +     [RESETID_A5_AXI_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 17
>> +     },
>> +     [RESETID_A5_ABP_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL0, .bit_idx = 16
>> +     },
>> +     [RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET] = {
>> +             .reg = HHI_SYS_CPU_CLK_CNTL1, .bit_idx = 30
>> +     },
>> +     [RESETID_VID_CLK_CNTL_SOFT_RESET] = {
>> +             .reg = HHI_VID_CLK_CNTL, .bit_idx = 15
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 7
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 3
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_RESET_N_POST] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 1
>> +     },
>> +     [RESETID_VID_DIVIDER_CNTL_RESET_N_PRE] = {
>> +             .reg = HHI_VID_DIVIDER_CNTL, .bit_idx = 0
>> +     },
>> +};
>> +
>> +static int meson8b_clk_reset_update(struct reset_controller_dev *rcdev,
>> +                                 unsigned long id, bool assert)
>> +{
>> +     struct meson8b_clk_reset *meson8b_clk_reset =
>> +             container_of(rcdev, struct meson8b_clk_reset, reset);
>> +     unsigned long flags;
>> +     const struct meson8b_clk_reset_line *reset;
>> +     u32 val;
>> +
>> +     if (id >= ARRAY_SIZE(meson8b_clk_reset_bits))
>> +             return -EINVAL;
>> +
>> +     reset = &meson8b_clk_reset_bits[id];
>> +
>> +     spin_lock_irqsave(&clk_lock, flags);
>> +
>> +     val = readl(meson8b_clk_reset->base + reset->reg);
>> +     if (assert)
>> +             val |= BIT(reset->bit_idx);
>> +     else
>> +             val &= ~BIT(reset->bit_idx);
>> +     writel(val, meson8b_clk_reset->base + reset->reg);
>> +
>> +     spin_unlock_irqrestore(&clk_lock, flags);
>> +
>> +     return 0;
>> +}
>> +
>> +static int meson8b_clk_reset_assert(struct reset_controller_dev *rcdev,
>> +                                  unsigned long id)
>> +{
>> +     return meson8b_clk_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int meson8b_clk_reset_deassert(struct reset_controller_dev *rcdev,
>> +                                    unsigned long id)
>> +{
>> +     return meson8b_clk_reset_update(rcdev, id, false);
>> +}
>> +
>> +static const struct reset_control_ops meson8b_clk_reset_ops = {
>> +     .assert = meson8b_clk_reset_assert,
>> +     .deassert = meson8b_clk_reset_deassert,
>> +};
>> +
>>  static int meson8b_clkc_probe(struct platform_device *pdev)
>>  {
>> -     void __iomem *clk_base;
>>       int ret, clkid, i;
>>       struct clk_hw *parent_hw;
>>       struct clk *parent_clk;
>>       struct device *dev = &pdev->dev;
>>
>> -     /*  Generic clocks and PLLs */
>> -     clk_base = of_iomap(dev->of_node, 1);
>> -     if (!clk_base) {
>> -             pr_err("%s: Unable to map clk base\n", __func__);
>> +     if (!clk_base)
>>               return -ENXIO;
>> -     }
>>
>>       /* Populate base address for PLLs */
>>       for (i = 0; i < ARRAY_SIZE(meson8b_clk_plls); i++)
>> @@ -743,7 +846,7 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
>>               /* FIXME convert to devm_clk_register */
>>               ret = devm_clk_hw_register(dev, meson8b_hw_onecell_data.hws[clkid]);
>>               if (ret)
>> -                     goto iounmap;
>> +                     return ret;
>>       }
>>
>>       /*
>> @@ -766,15 +869,11 @@ static int meson8b_clkc_probe(struct platform_device *pdev)
>>       if (ret) {
>>               pr_err("%s: failed to register clock notifier for cpu_clk\n",
>>                               __func__);
>> -             goto iounmap;
>> +             return ret;
>>       }
>>
>>       return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
>>                       &meson8b_hw_onecell_data);
>> -
>> -iounmap:
>> -     iounmap(clk_base);
>> -     return ret;
>>  }
>>
>>  static const struct of_device_id meson8b_clkc_match_table[] = {
>> @@ -793,3 +892,39 @@ static struct platform_driver meson8b_driver = {
>>  };
>>
>>  builtin_platform_driver(meson8b_driver);
>> +
>> +static void __init meson8b_clkc_reset_init(struct device_node *np)
>> +{
>> +     struct meson8b_clk_reset *rstc;
>> +     int ret;
>> +
>> +     /* Generic clocks, PLLs and some of the reset-bits */
>> +     clk_base = of_iomap(np, 1);
>> +     if (!clk_base) {
>> +             pr_err("%s: Unable to map clk base\n", __func__);
>> +             return;
>> +     }
>> +
>> +     rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
>> +     if (!rstc)
>> +             return;
>> +
>> +     /* Reset Controller */
>> +     rstc->base = clk_base;
>> +     rstc->reset.ops = &meson8b_clk_reset_ops;
>> +     rstc->reset.nr_resets = ARRAY_SIZE(meson8b_clk_reset_bits);
>> +     rstc->reset.of_node = np;
>> +     ret = reset_controller_register(&rstc->reset);
>> +     if (ret) {
>> +             pr_err("%s: Failed to register clkc reset controller: %d\n",
>> +                    __func__, ret);
>> +             return;
>> +     }
>> +}
>> +
>> +CLK_OF_DECLARE_DRIVER(meson8_clkc, "amlogic,meson8-clkc",
>> +                   meson8b_clkc_reset_init);
>> +CLK_OF_DECLARE_DRIVER(meson8b_clkc, "amlogic,meson8b-clkc",
>> +                   meson8b_clkc_reset_init);
>> +CLK_OF_DECLARE_DRIVER(meson8m2_clkc, "amlogic,meson8m2-clkc",
>> +                   meson8b_clkc_reset_init);
>> diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h
>> index a687e02547dc..5f4d8e49dd4d 100644
>> --- a/drivers/clk/meson/meson8b.h
>> +++ b/drivers/clk/meson/meson8b.h
>> @@ -37,6 +37,9 @@
>>  #define HHI_GCLK_AO                  0x154 /* 0x55 offset in data sheet */
>>  #define HHI_SYS_CPU_CLK_CNTL1                0x15c /* 0x57 offset in data sheet */
>>  #define HHI_MPEG_CLK_CNTL            0x174 /* 0x5d offset in data sheet */
>> +#define HHI_VID_CLK_CNTL             0x17c /* 0x5f offset in data sheet */
>> +#define HHI_VID_DIVIDER_CNTL         0x198 /* 0x66 offset in data sheet */
>> +#define HHI_SYS_CPU_CLK_CNTL0                0x19c /* 0x67 offset in data sheet */
>>  #define HHI_MPLL_CNTL                        0x280 /* 0xa0 offset in data sheet */
>>  #define HHI_SYS_PLL_CNTL             0x300 /* 0xc0 offset in data sheet */
>>  #define HHI_VID_PLL_CNTL             0x320 /* 0xc8 offset in data sheet */
>> @@ -163,7 +166,27 @@
>>
>>  #define CLK_NR_CLKS          96
>>
>> -/* include the CLKIDs that have been made part of the stable DT binding */
>> +#define RESETID_L2_CACHE_SOFT_RESET                          0
>> +#define RESETID_AXI_64_TO_128_BRIDGE_A5_SOFT_RESET           1
>> +#define RESETID_SCU_SOFT_RESET                                       2
>> +#define RESETID_CPU0_SOFT_RESET                                      3
>> +#define RESETID_CPU1_SOFT_RESET                                      4
>> +#define RESETID_CPU2_SOFT_RESET                                      5
>> +#define RESETID_CPU3_SOFT_RESET                                      6
>> +#define RESETID_A5_GLOBAL_RESET                                      7
>> +#define RESETID_A5_AXI_SOFT_RESET                            8
>> +#define RESETID_A5_ABP_SOFT_RESET                            9
>> +#define RESETID_AXI_64_TO_128_BRIDGE_MMC_SOFT_RESET          10
>> +#define RESETID_VID_CLK_CNTL_SOFT_RESET                              11
>> +#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_POST             12
>> +#define RESETID_VID_DIVIDER_CNTL_SOFT_RESET_PRE                      13
>> +#define RESETID_VID_DIVIDER_CNTL_RESET_N_POST                        14
>> +#define RESETID_VID_DIVIDER_CNTL_RESET_N_PRE                 15
>
> I think you should directly expose them in a dt-bindings/reset/meson8b-clkc-reset.h file and include it here.
just to confirm: do you want to move these to separate (new) header or
would meson8b-clkc.h be fine as well?
in either case: I suspect that changes to these will be very rare (as
I've re-checked that Amlogic's GPL kernel or u-boot definitely uses
these), so exposing them directly probably makes sense

>> +
>> +/*
>> + * include the CLKID and RESETID that have
>> + * been made part of the stable DT binding
>> + */
>>  #include <dt-bindings/clock/meson8b-clkc.h>
>>
>>  #endif /* __MESON8B_H */
>>
>
> Apart from that :
> Reviewed-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
thank you for reviewing my patches from this and the SMP/hotplug series!


Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux