diff mbox series

[U-Boot,v5,2/2] DW SPI: Get clock value from Device Tree

Message ID 20171114153345.8655-3-Eugeniy.Paltsev@synopsys.com
State Superseded
Delegated to: Marek Vasut
Headers show
Series DW SPI: Get clock value from Device Tree | expand

Commit Message

Eugeniy Paltsev Nov. 14, 2017, 3:33 p.m. UTC
Add option to set spi controller clock frequency via device tree
using standard clock bindings.

Define dw_spi_get_clk function as 'weak' as some targets
(like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) don't use standard clock API
and implement dw_spi_get_clk their own way in their clock manager.

Get rid of clock_manager.h include as we don't use
cm_get_spi_controller_clk_hz function anymore. (we use redefined
dw_spi_get_clk in SOCFPGA clock managers instead)

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 drivers/spi/designware_spi.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

Comments

Marek Vasut Nov. 15, 2017, 9:24 a.m. UTC | #1
On 11/14/2017 04:33 PM, Eugeniy Paltsev wrote:
> Add option to set spi controller clock frequency via device tree
> using standard clock bindings.
> 
> Define dw_spi_get_clk function as 'weak' as some targets
> (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) don't use standard clock API
> and implement dw_spi_get_clk their own way in their clock manager.
> 
> Get rid of clock_manager.h include as we don't use
> cm_get_spi_controller_clk_hz function anymore. (we use redefined
> dw_spi_get_clk in SOCFPGA clock managers instead)
> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  drivers/spi/designware_spi.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
> index 6cc4f51..470a3a7 100644
> --- a/drivers/spi/designware_spi.c
> +++ b/drivers/spi/designware_spi.c
> @@ -12,6 +12,7 @@
>  
>  #include <asm-generic/gpio.h>
>  #include <common.h>
> +#include <clk.h>
>  #include <dm.h>
>  #include <errno.h>
>  #include <malloc.h>
> @@ -19,7 +20,6 @@
>  #include <fdtdec.h>
>  #include <linux/compat.h>
>  #include <asm/io.h>
> -#include <asm/arch/clock_manager.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -95,6 +95,7 @@ struct dw_spi_priv {
>  	void __iomem *regs;
>  	unsigned int freq;		/* Default frequency */
>  	unsigned int mode;
> +	unsigned long bus_clk_rate;
>  
>  	struct gpio_desc cs_gpio;	/* External chip-select gpio */
>  
> @@ -195,14 +196,51 @@ static void spi_hw_init(struct dw_spi_priv *priv)
>  	debug("%s: fifo_len=%d\n", __func__, priv->fifo_len);
>  }
>  
> +/*
> + * We define dw_spi_get_clk function as 'weak' as some targets
> + * (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API
> + * and implement dw_spi_get_clk their own way in their clock manager.
> + */
> +__weak int dw_spi_get_clk(struct udevice *bus, ulong *rate)
> +{
> +	struct clk clk;
> +	int ret;
> +
> +	ret = clk_get_by_index(bus, 0, &clk);
> +	if (ret)
> +		return -EINVAL;
> +
> +	ret = clk_enable(&clk);
> +	if (ret && ret != -ENOSYS)
> +		return ret;
> +
> +	*rate = clk_get_rate(&clk);
> +	if (!*rate) {
> +		clk_disable(&clk);
> +		return -EINVAL;
> +	}
> +
> +	debug("%s: get spi controller clk via device tree: %lu Hz\n",
> +	      __func__, *rate);
> +
> +	clk_free(&clk);

You probably want to retain the handle to these clock in the private
data, since otherwise you won't be able to turn the clock off in
.remove() callback of the driver (if/once it's implemented)

Otherwise the patch look good, thanks !

> +	return 0;
> +}
> +
>  static int dw_spi_probe(struct udevice *bus)
>  {
>  	struct dw_spi_platdata *plat = dev_get_platdata(bus);
>  	struct dw_spi_priv *priv = dev_get_priv(bus);
> +	int ret;
>  
>  	priv->regs = plat->regs;
>  	priv->freq = plat->frequency;
>  
> +	ret = dw_spi_get_clk(bus, &priv->bus_clk_rate);
> +	if (ret)
> +		return ret;
> +
>  	/* Currently only bits_per_word == 8 supported */
>  	priv->bits_per_word = 8;
>  
> @@ -411,7 +449,7 @@ static int dw_spi_set_speed(struct udevice *bus, uint speed)
>  	spi_enable_chip(priv, 0);
>  
>  	/* clk_div doesn't support odd number */
> -	clk_div = cm_get_spi_controller_clk_hz() / speed;
> +	clk_div = priv->bus_clk_rate / speed;
>  	clk_div = (clk_div + 1) & 0xfffe;
>  	dw_write(priv, DW_SPI_BAUDR, clk_div);
>  
>
Eugeniy Paltsev Dec. 9, 2017, 3:23 p.m. UTC | #2
On Wed, 2017-11-15 at 10:24 +0100, Marek Vasut wrote:
> On 11/14/2017 04:33 PM, Eugeniy Paltsev wrote:

> > Add option to set spi controller clock frequency via device tree

> > using standard clock bindings.

> > 

> > Define dw_spi_get_clk function as 'weak' as some targets

> > (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) don't use standard clock API

> > and implement dw_spi_get_clk their own way in their clock manager.

> > 

> > Get rid of clock_manager.h include as we don't use

> > cm_get_spi_controller_clk_hz function anymore. (we use redefined

> > dw_spi_get_clk in SOCFPGA clock managers instead)

> > 

> > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>

> > ---

> >  drivers/spi/designware_spi.c | 42 ++++++++++++++++++++++++++++++++++++++++--

> >  1 file changed, 40 insertions(+), 2 deletions(-)

> > 

> > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c

> > index 6cc4f51..470a3a7 100644

> > --- a/drivers/spi/designware_spi.c

> > +++ b/drivers/spi/designware_spi.c

> > @@ -12,6 +12,7 @@

> >  

> > +/*

> > + * We define dw_spi_get_clk function as 'weak' as some targets

> > + * (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API

> > + * and implement dw_spi_get_clk their own way in their clock manager.

> > + */

> > +__weak int dw_spi_get_clk(struct udevice *bus, ulong *rate)

> > +{

> > +	struct clk clk;

> > +	int ret;

> > +

> > +	ret = clk_get_by_index(bus, 0, &clk);

> > +	if (ret)

> > +		return -EINVAL;

> > +

> > +	ret = clk_enable(&clk);

> > +	if (ret && ret != -ENOSYS)

> > +		return ret;

> > +

> > +	*rate = clk_get_rate(&clk);

> > +	if (!*rate) {

> > +		clk_disable(&clk);

> > +		return -EINVAL;

> > +	}

> > +

> > +	debug("%s: get spi controller clk via device tree: %lu Hz\n",

> > +	      __func__, *rate);

> > +

> > +	clk_free(&clk);

> 

> You probably want to retain the handle to these clock in the private

> data, since otherwise you won't be able to turn the clock off in

> .remove() callback of the driver (if/once it's implemented)


No, .remove() callback isn't implemented in this driver, so it isn't necessary.

> Otherwise the patch look good, thanks !

> 

> 

-- 
 Eugeniy Paltsev
Marek Vasut Dec. 9, 2017, 3:37 p.m. UTC | #3
On 12/09/2017 04:23 PM, Eugeniy Paltsev wrote:
> On Wed, 2017-11-15 at 10:24 +0100, Marek Vasut wrote:
>> On 11/14/2017 04:33 PM, Eugeniy Paltsev wrote:
>>> Add option to set spi controller clock frequency via device tree
>>> using standard clock bindings.
>>>
>>> Define dw_spi_get_clk function as 'weak' as some targets
>>> (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) don't use standard clock API
>>> and implement dw_spi_get_clk their own way in their clock manager.
>>>
>>> Get rid of clock_manager.h include as we don't use
>>> cm_get_spi_controller_clk_hz function anymore. (we use redefined
>>> dw_spi_get_clk in SOCFPGA clock managers instead)
>>>
>>> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>>> ---
>>>  drivers/spi/designware_spi.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 40 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
>>> index 6cc4f51..470a3a7 100644
>>> --- a/drivers/spi/designware_spi.c
>>> +++ b/drivers/spi/designware_spi.c
>>> @@ -12,6 +12,7 @@
>>>  
>>> +/*
>>> + * We define dw_spi_get_clk function as 'weak' as some targets
>>> + * (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API
>>> + * and implement dw_spi_get_clk their own way in their clock manager.
>>> + */
>>> +__weak int dw_spi_get_clk(struct udevice *bus, ulong *rate)
>>> +{
>>> +	struct clk clk;
>>> +	int ret;
>>> +
>>> +	ret = clk_get_by_index(bus, 0, &clk);
>>> +	if (ret)
>>> +		return -EINVAL;
>>> +
>>> +	ret = clk_enable(&clk);
>>> +	if (ret && ret != -ENOSYS)
>>> +		return ret;
>>> +
>>> +	*rate = clk_get_rate(&clk);
>>> +	if (!*rate) {
>>> +		clk_disable(&clk);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	debug("%s: get spi controller clk via device tree: %lu Hz\n",
>>> +	      __func__, *rate);
>>> +
>>> +	clk_free(&clk);
>>
>> You probably want to retain the handle to these clock in the private
>> data, since otherwise you won't be able to turn the clock off in
>> .remove() callback of the driver (if/once it's implemented)
> 
> No, .remove() callback isn't implemented in this driver, so it isn't necessary.

Either implement it, or if you're lazy, put the clock handle into the
private data, since that's a good practice.

>> Otherwise the patch look good, thanks !
>>
>>
diff mbox series

Patch

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 6cc4f51..470a3a7 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -12,6 +12,7 @@ 
 
 #include <asm-generic/gpio.h>
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
 #include <errno.h>
 #include <malloc.h>
@@ -19,7 +20,6 @@ 
 #include <fdtdec.h>
 #include <linux/compat.h>
 #include <asm/io.h>
-#include <asm/arch/clock_manager.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -95,6 +95,7 @@  struct dw_spi_priv {
 	void __iomem *regs;
 	unsigned int freq;		/* Default frequency */
 	unsigned int mode;
+	unsigned long bus_clk_rate;
 
 	struct gpio_desc cs_gpio;	/* External chip-select gpio */
 
@@ -195,14 +196,51 @@  static void spi_hw_init(struct dw_spi_priv *priv)
 	debug("%s: fifo_len=%d\n", __func__, priv->fifo_len);
 }
 
+/*
+ * We define dw_spi_get_clk function as 'weak' as some targets
+ * (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API
+ * and implement dw_spi_get_clk their own way in their clock manager.
+ */
+__weak int dw_spi_get_clk(struct udevice *bus, ulong *rate)
+{
+	struct clk clk;
+	int ret;
+
+	ret = clk_get_by_index(bus, 0, &clk);
+	if (ret)
+		return -EINVAL;
+
+	ret = clk_enable(&clk);
+	if (ret && ret != -ENOSYS)
+		return ret;
+
+	*rate = clk_get_rate(&clk);
+	if (!*rate) {
+		clk_disable(&clk);
+		return -EINVAL;
+	}
+
+	debug("%s: get spi controller clk via device tree: %lu Hz\n",
+	      __func__, *rate);
+
+	clk_free(&clk);
+
+	return 0;
+}
+
 static int dw_spi_probe(struct udevice *bus)
 {
 	struct dw_spi_platdata *plat = dev_get_platdata(bus);
 	struct dw_spi_priv *priv = dev_get_priv(bus);
+	int ret;
 
 	priv->regs = plat->regs;
 	priv->freq = plat->frequency;
 
+	ret = dw_spi_get_clk(bus, &priv->bus_clk_rate);
+	if (ret)
+		return ret;
+
 	/* Currently only bits_per_word == 8 supported */
 	priv->bits_per_word = 8;
 
@@ -411,7 +449,7 @@  static int dw_spi_set_speed(struct udevice *bus, uint speed)
 	spi_enable_chip(priv, 0);
 
 	/* clk_div doesn't support odd number */
-	clk_div = cm_get_spi_controller_clk_hz() / speed;
+	clk_div = priv->bus_clk_rate / speed;
 	clk_div = (clk_div + 1) & 0xfffe;
 	dw_write(priv, DW_SPI_BAUDR, clk_div);