Re: [PATCH 07/26] clk: vt8500: parse pmc_base from clock driver

2013-09-20 Thread Tony Prisk

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

2013-09-19 Thread Sebastian Hesselbarth

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

2013-09-19 Thread Tony Prisk

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

2013-09-19 Thread Tony Prisk

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

2013-09-19 Thread Sebastian Hesselbarth

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

2013-09-18 Thread Sebastian Hesselbarth
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/