Re: [PATCH v6 2/2] clk: clk-pic32: Add PIC32 clock driver
Hi Joshua,
Quoting Joshua Henderson (2016-02-08 13:28:17)
> +static const struct of_device_id pic32_clk_match[] __initconst = {
> + {
> + .compatible = "microchip,pic32mzda-refoclk",
> + .data = of_refo_clk_setup,
> + },
> + {
> + .compatible = "microchip,pic32mzda-pbclk",
> + .data = of_periph_clk_setup,
> + },
> + {
> + .compatible = "microchip,pic32mzda-syspll",
> + .data = of_sys_pll_setup,
> + },
> + {
> + .compatible = "microchip,pic32mzda-sosc",
> + .data = of_sosc_clk_setup,
> + },
> + {
> + .compatible = "microchip,pic32mzda-frcdivclk",
> + .data = of_frcdiv_setup,
> + },
> + {
> + .compatible = "microchip,pic32mzda-sysclk-v2",
> + .data = of_sys_mux_setup,
> + },
> + {}
> +};
As I mentioned in my review of the DT binding, instead of having a bunch
of compatible strings for stuff that is inside of your SoC, you should
have clock generator nodes in DT, and put the actual clock data here in
your driver. You might only need one node, perhaps a second for the
USBPLL.
In your case this would result in static inits of struct pic32_spll,
struct pic32_sclk, etc.
I'm guessing you have a lot of derivative chips with slightly different
clock trees? If so you should create a drivers/clk/pic32 directory. The
code in this patch could be common code re-used by your derivatives and
then you could store your clock data as a platform_driver in the same
directory that inits the static data and uses these common clk_ops
functions.
Each chip derivative would have a new compatible string, instead of each
clock node having a compatible string, which makes no sense.
> +static void __init of_pic32_soc_clock_init(struct device_node *np)
> +{
> + void (*clk_setup)(struct device_node *);
> + const struct of_device_id *clk_id;
> + struct device_node *childnp;
> +
> + pic32_clk_regbase = of_io_request_and_map(np, 0,
> of_node_full_name(np));
> + if (IS_ERR(pic32_clk_regbase))
> + panic("pic32-clk: failed to map registers\n");
> +
> + for_each_child_of_node(np, childnp) {
> + clk_id = of_match_node(pic32_clk_match, childnp);
> + if (!clk_id)
> + continue;
> + clk_setup = clk_id->data;
> + clk_setup(childnp);
> + }
> +
> + /* register failsafe-clock-monitor NMI */
> + register_nmi_notifier(&failsafe_clk_notifier);
> +}
> +
> +CLK_OF_DECLARE(pic32_soc_clk, "microchip,pic32mzda-clk",
> + of_pic32_soc_clock_init);
I hate CLK_OF_DECLARE. Sometimes we absolutely must live with
it, but most of the time you can create a proper platform_driver that
gets probed and registers its clocks from within the Linux Driver Model.
Please try to make this into a platform_driver. You requested an example
in V5, so please see:
drivers/clk/qcom/gcc-apq8084.c
Best regards,
Mike
[PATCH v6 2/2] clk: clk-pic32: Add PIC32 clock driver
From: Purna Chandra Mandal This clock driver implements PIC32 specific clock-tree. clock-tree entities can only be configured through device-tree file (OF). Signed-off-by: Purna Chandra Mandal Signed-off-by: Joshua Henderson Cc: Ralf Baechle Cc: Michael Turquette Cc: Stephen Boyd --- Changes since v5: - sort linux includes and asm includes. - use BIT() wherever applicable - drop 'microchip,ignore-unused' usage, handling in favor of critical clock - drop 'fixed divider' handling for periph clock - drop use of 'debug_init()' clk operation callback for register dump - drop clk_lock(), clk_unlock() spinlock wrapper - drop unimplemented pic32_devcon_syslock() macro - use readl()/writel() instead of clk_readl()/clk_writel() - drop redundant spinlock, unlock calls in sosc_clk_enable()/disable() - use CLK_SET_RATE_GATE, _SET_PARENT_GATE for refo-clocks - use kcalloc() instead of kmalloc() wherever applicable - use of_io_request_and_map() in soc_clock_init() - drop use of pbclk(/roclk)_endisable() inline function - use readl_poll_timeout_atomic() for wait_for_bit() type loop - drop cpu_relax() after clk gating - promote u8, u16 to u32 wherever applicable - fix sosc clock status polling - drop memory alloc from pic32_of_clk_get_parent_indices() instead callers will supply buffer to hold output parent indices - reword comments about spll_clk_set_rate() pre-conditions - drop use of CLK_BASIC wherever applicable - reword comments in sclk_set_parent() Changes since v4: None Changes since v3: None Changes since v2: - Replace __clk_debug with pr_debug - Add of_clk_parent_fill usage in PIC32 clock driver Changes since v1: - Remove unused PIC32 MPLL support. - Remove support for initializing default parent/rate for REFOSC clocks. --- drivers/clk/Kconfig |3 + drivers/clk/Makefile|1 + drivers/clk/clk-pic32.c | 1470 +++ 3 files changed, 1474 insertions(+) create mode 100644 drivers/clk/clk-pic32.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index eca8e01..41e9c14 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -200,6 +200,9 @@ config COMMON_CLK_CDCE706 ---help--- This driver supports TI CDCE706 programmable 3-PLL clock synthesizer. +config COMMON_CLK_PIC32 + def_bool COMMON_CLK && MACH_PIC32 + source "drivers/clk/bcm/Kconfig" source "drivers/clk/hisilicon/Kconfig" source "drivers/clk/qcom/Kconfig" diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index b038e36..88a5ce6 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_ARCH_MOXART) += clk-moxart.o obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o obj-$(CONFIG_COMMON_CLK_PALMAS)+= clk-palmas.o +obj-$(CONFIG_COMMON_CLK_PIC32) += clk-pic32.o obj-$(CONFIG_CLK_QORIQ)+= clk-qoriq.o obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o diff --git a/drivers/clk/clk-pic32.c b/drivers/clk/clk-pic32.c new file mode 100644 index 000..43ae30f --- /dev/null +++ b/drivers/clk/clk-pic32.c @@ -0,0 +1,1470 @@ +/* + * Purna Chandra Mandal, + * Copyright (C) 2015 Microchip Technology Inc. All rights reserved. + * + * This program is free software; you can distribute it and/or modify it + * under the terms of the GNU General Public License (Version 2) as + * published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* OSCCON Reg fields */ +#define OSC_CUR_MASK 0x07 +#define OSC_CUR_SHIFT 12 +#define OSC_NEW_MASK 0x07 +#define OSC_NEW_SHIFT 8 +#define OSC_SWEN BIT(0) +#define OSC_CLK_FAILED BIT(2) + +/* SPLLCON Reg fields */ +#define PLL_RANGE_MASK 0x07 +#define PLL_RANGE_SHIFT0 +#define PLL_ICLK_MASK 0x01 +#define PLL_ICLK_SHIFT 7 +#define PLL_IDIV_MASK 0x07 +#define PLL_IDIV_SHIFT 8 +#define PLL_ODIV_MASK 0x07 +#define PLL_ODIV_SHIFT 24 +#define PLL_MULT_MASK 0x7F +#define PLL_MULT_SHIFT 16 +#define PLL_MULT_MAX 128 +#define PLL_ODIV_MIN 1 +#define PLL_ODIV_MAX 5 + +/* Peripheral Bus Clock Reg Fields */ +#define PB_DIV_MASK0x7f +#define PB_DIV_SHIFT
