Re: [PATCH 07/26] clk: vt8500: parse pmc_base from clock driver
On 20/09/13 18:23, Sebastian Hesselbarth wrote: On 09/20/2013 06:51 AM, Tony Prisk wrote: On 20/09/13 07:12, Sebastian Hesselbarth wrote: On 09/19/2013 09:02 PM, Tony Prisk wrote: On 19/09/13 05:53, Sebastian Hesselbarth wrote: Currently, clock providers for vt8500 depend on machine_init providing pmc_base address before calling of_clk_init. With upcoming arch-wide .time_init calling of_clk_init, we should make clock providers independent of mach code. This adds a pmc_base parsing helper to current clock provider that gets called if there is no pmc_base set, yet. Signed-off-by: Sebastian Hesselbarth --- Cc: Olof Johansson Cc: Arnd Bergmann Cc: Tony Prisk Cc: Mike Turquette Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/clk/clk-vt8500.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c index 82306f5..a5ee01c 100644 --- a/drivers/clk/clk-vt8500.c +++ b/drivers/clk/clk-vt8500.c @@ -15,11 +15,14 @@ #include #include +#include #include #include #include #include +#define LEGACY_PMC_BASE0xD813 + /* All clocks share the same lock as none can be changed concurrently */ static DEFINE_SPINLOCK(_lock); @@ -626,6 +629,21 @@ const struct clk_ops vtwm_pll_ops = { .recalc_rate = vtwm_pll_recalc_rate, }; +static __init void vtwm_set_pmc_base(void) +{ +struct device_node *np = +of_find_compatible_node(NULL, NULL, "via,vt8500-pmc"); + +if (np) +pmc_base = of_iomap(np, 0); +else +pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000); +of_node_put(np); + +if (!pmc_base) +pr_err("%s:of_iomap(pmc) failed\n", __func__); +} + static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) { u32 reg; @@ -636,6 +654,9 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) struct clk_init_data init; int rc; +if (!pmc_base) +vtwm_set_pmc_base(); + rc = of_property_read_u32(node, "reg", ®); if (WARN_ON(rc)) return; What happens if the first clock registered is a 'device clock' rather than a 'pll'? static __init void vtwm_device_clk_init(struct device_node *node) { u32 en_reg, div_reg; struct clk *clk; struct clk_device *dev_clk; const char *clk_name = node->name; const char *parent_name; struct clk_init_data init; int rc; int clk_init_flags = 0; dev_clk = kzalloc(sizeof(*dev_clk), GFP_KERNEL); if (WARN_ON(!dev_clk)) return; dev_clk->lock = &_lock; rc = of_property_read_u32(node, "enable-reg", &en_reg); if (!rc) { dev_clk->en_reg = pmc_base + en_reg; ... } CLK_OF_DECLARE(vt8500_device, "via,vt8500-device-clock", vtwm_device_clk_init); If a device clock is initialized first, pmc_base will be null and dev_clk->en_reg (+ other register offsets) will be incorrect. Tony, looks like I just missed to add the same check for !pmc_base to vtwm_device_clk_init. If you are ok with the general approach, I send v2 for this patch shortly. Optionally, you can also choose to take care of clk-vt8500 yourself, as mach-vt8500 has its own .init_time callback and will be unaffected by the arch-wide default callback. If so, I will drop vt8500 to not stall this series too much now. Sebastian I have no issue with the concept - just pointing out the missing bit. If you can fix that small issue for v2 then you can also add my: Acked-by: Tony Prisk Just to make sure, does that also count for the other vt8500 patches? Sebastian Sorry, I should have been more specific. For the whole series (vt8500-related): Acked-by: Tony Prisk Regards Tony P -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/26] clk: vt8500: parse pmc_base from clock driver
On 09/20/2013 06:51 AM, Tony Prisk wrote: On 20/09/13 07:12, Sebastian Hesselbarth wrote: On 09/19/2013 09:02 PM, Tony Prisk wrote: On 19/09/13 05:53, Sebastian Hesselbarth wrote: Currently, clock providers for vt8500 depend on machine_init providing pmc_base address before calling of_clk_init. With upcoming arch-wide .time_init calling of_clk_init, we should make clock providers independent of mach code. This adds a pmc_base parsing helper to current clock provider that gets called if there is no pmc_base set, yet. Signed-off-by: Sebastian Hesselbarth --- Cc: Olof Johansson Cc: Arnd Bergmann Cc: Tony Prisk Cc: Mike Turquette Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/clk/clk-vt8500.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c index 82306f5..a5ee01c 100644 --- a/drivers/clk/clk-vt8500.c +++ b/drivers/clk/clk-vt8500.c @@ -15,11 +15,14 @@ #include #include +#include #include #include #include #include +#define LEGACY_PMC_BASE0xD813 + /* All clocks share the same lock as none can be changed concurrently */ static DEFINE_SPINLOCK(_lock); @@ -626,6 +629,21 @@ const struct clk_ops vtwm_pll_ops = { .recalc_rate = vtwm_pll_recalc_rate, }; +static __init void vtwm_set_pmc_base(void) +{ +struct device_node *np = +of_find_compatible_node(NULL, NULL, "via,vt8500-pmc"); + +if (np) +pmc_base = of_iomap(np, 0); +else +pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000); +of_node_put(np); + +if (!pmc_base) +pr_err("%s:of_iomap(pmc) failed\n", __func__); +} + static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) { u32 reg; @@ -636,6 +654,9 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) struct clk_init_data init; int rc; +if (!pmc_base) +vtwm_set_pmc_base(); + rc = of_property_read_u32(node, "reg", ®); if (WARN_ON(rc)) return; What happens if the first clock registered is a 'device clock' rather than a 'pll'? static __init void vtwm_device_clk_init(struct device_node *node) { u32 en_reg, div_reg; struct clk *clk; struct clk_device *dev_clk; const char *clk_name = node->name; const char *parent_name; struct clk_init_data init; int rc; int clk_init_flags = 0; dev_clk = kzalloc(sizeof(*dev_clk), GFP_KERNEL); if (WARN_ON(!dev_clk)) return; dev_clk->lock = &_lock; rc = of_property_read_u32(node, "enable-reg", &en_reg); if (!rc) { dev_clk->en_reg = pmc_base + en_reg; ... } CLK_OF_DECLARE(vt8500_device, "via,vt8500-device-clock", vtwm_device_clk_init); If a device clock is initialized first, pmc_base will be null and dev_clk->en_reg (+ other register offsets) will be incorrect. Tony, looks like I just missed to add the same check for !pmc_base to vtwm_device_clk_init. If you are ok with the general approach, I send v2 for this patch shortly. Optionally, you can also choose to take care of clk-vt8500 yourself, as mach-vt8500 has its own .init_time callback and will be unaffected by the arch-wide default callback. If so, I will drop vt8500 to not stall this series too much now. Sebastian I have no issue with the concept - just pointing out the missing bit. If you can fix that small issue for v2 then you can also add my: Acked-by: Tony Prisk Just to make sure, does that also count for the other vt8500 patches? Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/26] clk: vt8500: parse pmc_base from clock driver
On 20/09/13 07:12, Sebastian Hesselbarth wrote: On 09/19/2013 09:02 PM, Tony Prisk wrote: On 19/09/13 05:53, Sebastian Hesselbarth wrote: Currently, clock providers for vt8500 depend on machine_init providing pmc_base address before calling of_clk_init. With upcoming arch-wide .time_init calling of_clk_init, we should make clock providers independent of mach code. This adds a pmc_base parsing helper to current clock provider that gets called if there is no pmc_base set, yet. Signed-off-by: Sebastian Hesselbarth --- Cc: Olof Johansson Cc: Arnd Bergmann Cc: Tony Prisk Cc: Mike Turquette Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/clk/clk-vt8500.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c index 82306f5..a5ee01c 100644 --- a/drivers/clk/clk-vt8500.c +++ b/drivers/clk/clk-vt8500.c @@ -15,11 +15,14 @@ #include #include +#include #include #include #include #include +#define LEGACY_PMC_BASE0xD813 + /* All clocks share the same lock as none can be changed concurrently */ static DEFINE_SPINLOCK(_lock); @@ -626,6 +629,21 @@ const struct clk_ops vtwm_pll_ops = { .recalc_rate = vtwm_pll_recalc_rate, }; +static __init void vtwm_set_pmc_base(void) +{ +struct device_node *np = +of_find_compatible_node(NULL, NULL, "via,vt8500-pmc"); + +if (np) +pmc_base = of_iomap(np, 0); +else +pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000); +of_node_put(np); + +if (!pmc_base) +pr_err("%s:of_iomap(pmc) failed\n", __func__); +} + static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) { u32 reg; @@ -636,6 +654,9 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) struct clk_init_data init; int rc; +if (!pmc_base) +vtwm_set_pmc_base(); + rc = of_property_read_u32(node, "reg", ®); if (WARN_ON(rc)) return; What happens if the first clock registered is a 'device clock' rather than a 'pll'? static __init void vtwm_device_clk_init(struct device_node *node) { u32 en_reg, div_reg; struct clk *clk; struct clk_device *dev_clk; const char *clk_name = node->name; const char *parent_name; struct clk_init_data init; int rc; int clk_init_flags = 0; dev_clk = kzalloc(sizeof(*dev_clk), GFP_KERNEL); if (WARN_ON(!dev_clk)) return; dev_clk->lock = &_lock; rc = of_property_read_u32(node, "enable-reg", &en_reg); if (!rc) { dev_clk->en_reg = pmc_base + en_reg; ... } CLK_OF_DECLARE(vt8500_device, "via,vt8500-device-clock", vtwm_device_clk_init); If a device clock is initialized first, pmc_base will be null and dev_clk->en_reg (+ other register offsets) will be incorrect. Tony, looks like I just missed to add the same check for !pmc_base to vtwm_device_clk_init. If you are ok with the general approach, I send v2 for this patch shortly. Optionally, you can also choose to take care of clk-vt8500 yourself, as mach-vt8500 has its own .init_time callback and will be unaffected by the arch-wide default callback. If so, I will drop vt8500 to not stall this series too much now. Sebastian I have no issue with the concept - just pointing out the missing bit. If you can fix that small issue for v2 then you can also add my: Acked-by: Tony Prisk Regards Tony P -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/26] clk: vt8500: parse pmc_base from clock driver
On 19/09/13 05:53, Sebastian Hesselbarth wrote: Currently, clock providers for vt8500 depend on machine_init providing pmc_base address before calling of_clk_init. With upcoming arch-wide .time_init calling of_clk_init, we should make clock providers independent of mach code. This adds a pmc_base parsing helper to current clock provider that gets called if there is no pmc_base set, yet. Signed-off-by: Sebastian Hesselbarth --- Cc: Olof Johansson Cc: Arnd Bergmann Cc: Tony Prisk Cc: Mike Turquette Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/clk/clk-vt8500.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c index 82306f5..a5ee01c 100644 --- a/drivers/clk/clk-vt8500.c +++ b/drivers/clk/clk-vt8500.c @@ -15,11 +15,14 @@ #include #include +#include #include #include #include #include +#define LEGACY_PMC_BASE 0xD813 + /* All clocks share the same lock as none can be changed concurrently */ static DEFINE_SPINLOCK(_lock); @@ -626,6 +629,21 @@ const struct clk_ops vtwm_pll_ops = { .recalc_rate = vtwm_pll_recalc_rate, }; +static __init void vtwm_set_pmc_base(void) +{ + struct device_node *np = + of_find_compatible_node(NULL, NULL, "via,vt8500-pmc"); + + if (np) + pmc_base = of_iomap(np, 0); + else + pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000); + of_node_put(np); + + if (!pmc_base) + pr_err("%s:of_iomap(pmc) failed\n", __func__); +} + static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) { u32 reg; @@ -636,6 +654,9 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) struct clk_init_data init; int rc; + if (!pmc_base) + vtwm_set_pmc_base(); + rc = of_property_read_u32(node, "reg", ®); if (WARN_ON(rc)) return; What happens if the first clock registered is a 'device clock' rather than a 'pll'? static __init void vtwm_device_clk_init(struct device_node *node) { u32 en_reg, div_reg; struct clk *clk; struct clk_device *dev_clk; const char *clk_name = node->name; const char *parent_name; struct clk_init_data init; int rc; int clk_init_flags = 0; dev_clk = kzalloc(sizeof(*dev_clk), GFP_KERNEL); if (WARN_ON(!dev_clk)) return; dev_clk->lock = &_lock; rc = of_property_read_u32(node, "enable-reg", &en_reg); if (!rc) { dev_clk->en_reg = pmc_base + en_reg; ... } CLK_OF_DECLARE(vt8500_device, "via,vt8500-device-clock", vtwm_device_clk_init); If a device clock is initialized first, pmc_base will be null and dev_clk->en_reg (+ other register offsets) will be incorrect. Regards Tony Prisk -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/26] clk: vt8500: parse pmc_base from clock driver
On 09/19/2013 09:02 PM, Tony Prisk wrote: On 19/09/13 05:53, Sebastian Hesselbarth wrote: Currently, clock providers for vt8500 depend on machine_init providing pmc_base address before calling of_clk_init. With upcoming arch-wide .time_init calling of_clk_init, we should make clock providers independent of mach code. This adds a pmc_base parsing helper to current clock provider that gets called if there is no pmc_base set, yet. Signed-off-by: Sebastian Hesselbarth --- Cc: Olof Johansson Cc: Arnd Bergmann Cc: Tony Prisk Cc: Mike Turquette Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/clk/clk-vt8500.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c index 82306f5..a5ee01c 100644 --- a/drivers/clk/clk-vt8500.c +++ b/drivers/clk/clk-vt8500.c @@ -15,11 +15,14 @@ #include #include +#include #include #include #include #include +#define LEGACY_PMC_BASE0xD813 + /* All clocks share the same lock as none can be changed concurrently */ static DEFINE_SPINLOCK(_lock); @@ -626,6 +629,21 @@ const struct clk_ops vtwm_pll_ops = { .recalc_rate = vtwm_pll_recalc_rate, }; +static __init void vtwm_set_pmc_base(void) +{ +struct device_node *np = +of_find_compatible_node(NULL, NULL, "via,vt8500-pmc"); + +if (np) +pmc_base = of_iomap(np, 0); +else +pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000); +of_node_put(np); + +if (!pmc_base) +pr_err("%s:of_iomap(pmc) failed\n", __func__); +} + static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) { u32 reg; @@ -636,6 +654,9 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) struct clk_init_data init; int rc; +if (!pmc_base) +vtwm_set_pmc_base(); + rc = of_property_read_u32(node, "reg", ®); if (WARN_ON(rc)) return; What happens if the first clock registered is a 'device clock' rather than a 'pll'? static __init void vtwm_device_clk_init(struct device_node *node) { u32 en_reg, div_reg; struct clk *clk; struct clk_device *dev_clk; const char *clk_name = node->name; const char *parent_name; struct clk_init_data init; int rc; int clk_init_flags = 0; dev_clk = kzalloc(sizeof(*dev_clk), GFP_KERNEL); if (WARN_ON(!dev_clk)) return; dev_clk->lock = &_lock; rc = of_property_read_u32(node, "enable-reg", &en_reg); if (!rc) { dev_clk->en_reg = pmc_base + en_reg; ... } CLK_OF_DECLARE(vt8500_device, "via,vt8500-device-clock", vtwm_device_clk_init); If a device clock is initialized first, pmc_base will be null and dev_clk->en_reg (+ other register offsets) will be incorrect. Tony, looks like I just missed to add the same check for !pmc_base to vtwm_device_clk_init. If you are ok with the general approach, I send v2 for this patch shortly. Optionally, you can also choose to take care of clk-vt8500 yourself, as mach-vt8500 has its own .init_time callback and will be unaffected by the arch-wide default callback. If so, I will drop vt8500 to not stall this series too much now. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 07/26] clk: vt8500: parse pmc_base from clock driver
Currently, clock providers for vt8500 depend on machine_init providing pmc_base address before calling of_clk_init. With upcoming arch-wide .time_init calling of_clk_init, we should make clock providers independent of mach code. This adds a pmc_base parsing helper to current clock provider that gets called if there is no pmc_base set, yet. Signed-off-by: Sebastian Hesselbarth --- Cc: Olof Johansson Cc: Arnd Bergmann Cc: Tony Prisk Cc: Mike Turquette Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/clk/clk-vt8500.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c index 82306f5..a5ee01c 100644 --- a/drivers/clk/clk-vt8500.c +++ b/drivers/clk/clk-vt8500.c @@ -15,11 +15,14 @@ #include #include +#include #include #include #include #include +#define LEGACY_PMC_BASE0xD813 + /* All clocks share the same lock as none can be changed concurrently */ static DEFINE_SPINLOCK(_lock); @@ -626,6 +629,21 @@ const struct clk_ops vtwm_pll_ops = { .recalc_rate = vtwm_pll_recalc_rate, }; +static __init void vtwm_set_pmc_base(void) +{ + struct device_node *np = + of_find_compatible_node(NULL, NULL, "via,vt8500-pmc"); + + if (np) + pmc_base = of_iomap(np, 0); + else + pmc_base = ioremap(LEGACY_PMC_BASE, 0x1000); + of_node_put(np); + + if (!pmc_base) + pr_err("%s:of_iomap(pmc) failed\n", __func__); +} + static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) { u32 reg; @@ -636,6 +654,9 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type) struct clk_init_data init; int rc; + if (!pmc_base) + vtwm_set_pmc_base(); + rc = of_property_read_u32(node, "reg", ®); if (WARN_ON(rc)) return; -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/