[v4,6/8] wlcore: sdio: add wilink clock providers

Commit Message

Luciano Coelho July 30, 2013, 1:04 p.m. UTC

Add refclock and tcxoclock as clock providers in WiLink.  These clocks
are not accesible outside the WiLink module, but they are registered
in the clock framework anyway.  Only the WiLink chip consumes these
clocks.

In theory, the WiLink chip could be connected to external clocks
instead of using these internal clocks, so make the clock consumer
code generic enough.  If external clocks are used, then the internal
clock device tree nodes are not necessary, but the external ones must
be specified.

Signed-off-by: Luciano Coelho <[e-mail]>
Reviewed-by: Felipe Balbi <[e-mail]>
---
 drivers/net/wireless/ti/wlcore/sdio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Mike Turquette July 30, 2013, 10:35 p.m. UTC | #1

Quoting Luciano Coelho (2013-07-30 06:04:34)
> +static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
> +       { .compatible = "ti,wilink-clock" },
> +};
> +
>  static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device *dev)
>  {
>         struct wl12xx_platform_data *pdata;
>         struct device_node *np = dev->of_node;
> +       struct device_node *clock_node;
>  
>         if (!np) {
>                 np = of_find_matching_node(NULL, dev->driver->of_match_table);
> @@ -241,6 +247,9 @@ static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device *dev)
>                 goto out_free;
>         }
>  
> +       for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
> +               of_fixed_clk_setup(clock_node);

Hi Luciano,

Any reason for establishing your own compatible string if you just plan
to use the fixed rate clock? You could just use "fixed-clock" compatible
in your DTS.

I will be posting patches this week which makes the fixed-rate clock a
proper driver and matches that compatible string to instantiate those
clocks. That means that your driver could probably remove the clock
setup code completely.

Regards,
Mike

> +
>         goto out;
>  
>  out_free:
> -- 
> 1.8.3.2

Luciano Coelho July 30, 2013, 11:04 p.m. UTC | #2

On Tue, 2013-07-30 at 15:35 -0700, Mike Turquette wrote:
> Quoting Luciano Coelho (2013-07-30 06:04:34)
> > +static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
> > +       { .compatible = "ti,wilink-clock" },
> > +};
> > +
> >  static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device *dev)
> >  {
> >         struct wl12xx_platform_data *pdata;
> >         struct device_node *np = dev->of_node;
> > +       struct device_node *clock_node;
> >  
> >         if (!np) {
> >                 np = of_find_matching_node(NULL, dev->driver->of_match_table);
> > @@ -241,6 +247,9 @@ static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device *dev)
> >                 goto out_free;
> >         }
> >  
> > +       for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
> > +               of_fixed_clk_setup(clock_node);
> 
> Hi Luciano,

Hi Mike,


> Any reason for establishing your own compatible string if you just plan
> to use the fixed rate clock? You could just use "fixed-clock" compatible
> in your DTS.

The reason is that I can't call of_clk_init(), because this function is
not exported and my module can't use it.  I would have to link with the
clk code to be able to call it.

Also, I reckoned that, since these clock cannot be used by anyone else
than the WiLink module itself, it would make sense to have a different
compatible string.


> I will be posting patches this week which makes the fixed-rate clock a
> proper driver and matches that compatible string to instantiate those
> clocks. That means that your driver could probably remove the clock
> setup code completely.

Okay, if this is done, then I could probably use "fixed-clock" directly,
since the driver itself will take care of going through the DT and
initializing all the fixed-clocks.

--
Luca.

Patch

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
index 9370d7e..980bf3d 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -34,6 +34,7 @@ 
 #include <linux/wl12xx.h>
 #include <linux/pm_runtime.h>
 #include <linux/printk.h>
+#include <linux/clk-provider.h>
 
 #include "wlcore.h"
 #include "wl12xx_80211.h"
@@ -214,10 +215,15 @@  static struct wl1271_if_operations sdio_ops = {
 	.set_block_size = wl1271_sdio_set_block_size,
 };
 
+static const struct of_device_id wlcore_sdio_of_clk_match_table[] = {
+	{ .compatible = "ti,wilink-clock" },
+};
+
 static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device *dev)
 {
 	struct wl12xx_platform_data *pdata;
 	struct device_node *np = dev->of_node;
+	struct device_node *clock_node;
 
 	if (!np) {
 		np = of_find_matching_node(NULL, dev->driver->of_match_table);
@@ -241,6 +247,9 @@  static struct wl12xx_platform_data *wlcore_get_pdata_from_of(struct device *dev)
 		goto out_free;
 	}
 
+	for_each_matching_node(clock_node, wlcore_sdio_of_clk_match_table)
+		of_fixed_clk_setup(clock_node);
+
 	goto out;
 
 out_free: