[PATCH 1/3] spi/bcm63xx-hsspi: allow providing clock rate through a second clock

[PATCH 1/3] spi/bcm63xx-hsspi: allow providing clock rate through a second clock

[Jonas Gorski]

Instead of requiring the hsspi clock to have a rate, allow using a second
clock for providing the Hz rate, which is probably more correct anyway.

Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bcm63xx-hsspi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 55789f7cda92..79096d17ebde 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -351,8 +351,16 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
 		return PTR_ERR(clk);
 
 	rate = clk_get_rate(clk);
-	if (!rate)
-		return -EINVAL;
+	if (!rate) {
+		struct clk *pll_clk = devm_clk_get(dev, "pll");
+
+		if (IS_ERR(pll_clk))
+			return PTR_ERR(pll_clk);
+
+		rate = clk_get_rate(pll_clk);
+		if (!rate)
+			return -EINVAL;
+	}
 
 	ret = clk_prepare_enable(clk);
 	if (ret)
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] dt-bindings: spi: document bcm63xx HS SPI devicetree bindings

[Jonas Gorski]

Add documentation for the bindings of the high speed SPI controller found
on newer bcm63xx SoCs.

Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/spi/spi-bcm63xx-hsspi.txt  | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
new file mode 100644
index 000000000000..3b0a2220b896
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
@@ -0,0 +1,35 @@
+Binding for Broadcom BCM6328 SPI controller
+
+Required properties:
+- compatible: must contain of "brcm,bcm6328-hsspi".
+- reg: Base address and size of the controllers memory area.
+- interrupts: Interrupt for the SPI block.
+- clocks: phandle of the SPI clock.
+- clock-names: must be "hsspi".
+- #address-cells: <1>, as required by generic SPI binding.
+- #size-cells: <0>, also as required by generic SPI binding.
+
+Optional properties:
+- num-cs: some controllers have less than 8 cs signals. Defaults to 8
+  if absent.
+- clocks: a second handle for the PLL clock.
+- clock-names: must be named "pll", if present.
+
+Child nodes as per the generic SPI binding.
+
+Example:
+
+	spi@10001000 {
+		compatible = "brcm,bcm6328-hsspi";
+		reg = <0x10001000 0x600>;
+
+		interrupts = <29>;
+
+		clocks = <&clkctl 9>, <&hsspi_pll>;
+		clock-names = "hsspi", "pll";
+
+		num-cs = <2>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] dt-bindings: spi: document bcm63xx HS SPI devicetree bindings

[Rob Herring]

On Wed, Feb 22, 2017 at 02:19:39PM +0100, Jonas Gorski wrote:
> Add documentation for the bindings of the high speed SPI controller found
> on newer bcm63xx SoCs.
> 
> Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/spi/spi-bcm63xx-hsspi.txt  | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] dt-bindings: spi: document bcm63xx HS SPI devicetree bindings

[Florian Fainelli]

On 02/22/2017 05:19 AM, Jonas Gorski wrote:
> Add documentation for the bindings of the high speed SPI controller found
> on newer bcm63xx SoCs.
> 
> Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/spi/spi-bcm63xx-hsspi.txt  | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
> new file mode 100644
> index 000000000000..3b0a2220b896
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
> @@ -0,0 +1,35 @@
> +Binding for Broadcom BCM6328 SPI controller
> +
> +Required properties:
> +- compatible: must contain of "brcm,bcm6328-hsspi".
> +- reg: Base address and size of the controllers memory area.
> +- interrupts: Interrupt for the SPI block.
> +- clocks: phandle of the SPI clock.
> +- clock-names: must be "hsspi".
> +- #address-cells: <1>, as required by generic SPI binding.
> +- #size-cells: <0>, also as required by generic SPI binding.
> +
> +Optional properties:
> +- num-cs: some controllers have less than 8 cs signals. Defaults to 8
> +  if absent.
> +- clocks: a second handle for the PLL clock.
> +- clock-names: must be named "pll", if present.

I have not found chips where the PLL may be optional, but there may be
ways to have the same PLL and UBUS clocks feeding into the HSSPI block.

Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] dt-bindings: spi: document bcm63xx HS SPI devicetree bindings

[Jonas Gorski]

Hi,

On 27 February 2017 at 23:49, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 02/22/2017 05:19 AM, Jonas Gorski wrote:
>> Add documentation for the bindings of the high speed SPI controller found
>> on newer bcm63xx SoCs.
>>
>> Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/spi/spi-bcm63xx-hsspi.txt  | 35 ++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
>> new file mode 100644
>> index 000000000000..3b0a2220b896
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
>> @@ -0,0 +1,35 @@
>> +Binding for Broadcom BCM6328 SPI controller

I missed a "High Speed" here.

>> +
>> +Required properties:
>> +- compatible: must contain of "brcm,bcm6328-hsspi".
>> +- reg: Base address and size of the controllers memory area.
>> +- interrupts: Interrupt for the SPI block.
>> +- clocks: phandle of the SPI clock.
>> +- clock-names: must be "hsspi".
>> +- #address-cells: <1>, as required by generic SPI binding.
>> +- #size-cells: <0>, also as required by generic SPI binding.
>> +
>> +Optional properties:
>> +- num-cs: some controllers have less than 8 cs signals. Defaults to 8
>> +  if absent.
>> +- clocks: a second handle for the PLL clock.
>> +- clock-names: must be named "pll", if present.
>
> I have not found chips where the PLL may be optional, but there may be
> ways to have the same PLL and UBUS clocks feeding into the HSSPI block.

Indeed, I guess I make it just non-optional. Having it as an optional
clock was (not intentionally) more describing the driver, not the
hardware. If there is ever only one clock, one could just pass it
twice. I'll update and send a V2.


Jonas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] spi/bcm63xx-hsspi: allow for probing through devicetree

[Jonas Gorski]

Add required binding support to probe through device tree.

Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-bcm63xx-hsspi.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 79096d17ebde..77c249a22c9b 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/spi/spi.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 
 #define HSSPI_GLOBAL_CTRL_REG			0x0
 #define GLOBAL_CTRL_CS_POLARITY_SHIFT		0
@@ -91,6 +92,7 @@
 
 #define HSSPI_MAX_SYNC_CLOCK			30000000
 
+#define HSSPI_SPI_MAX_CS			8
 #define HSSPI_BUS_NUM				1 /* 0 is legacy SPI */
 
 struct bcm63xx_hsspi {
@@ -332,7 +334,7 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct clk *clk;
 	int irq, ret;
-	u32 reg, rate;
+	u32 reg, rate, num_cs = HSSPI_SPI_MAX_CS;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
@@ -382,8 +384,20 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
 	mutex_init(&bs->bus_mutex);
 	init_completion(&bs->done);
 
-	master->bus_num = HSSPI_BUS_NUM;
-	master->num_chipselect = 8;
+	if (dev->of_node) {
+		master->dev.of_node = dev->of_node;
+
+		of_property_read_u32(dev->of_node, "num-cs", &num_cs);
+		if (num_cs > 8) {
+			dev_warn(dev, "unsupported number of cs (%i), reducing to 8\n",
+				 num_cs);
+			num_cs = HSSPI_SPI_MAX_CS;
+		}
+	} else {
+		master->bus_num = HSSPI_BUS_NUM;
+	}
+
+	master->num_chipselect = num_cs;
 	master->setup = bcm63xx_hsspi_setup;
 	master->transfer_one_message = bcm63xx_hsspi_transfer_one;
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
@@ -469,10 +483,16 @@ static int bcm63xx_hsspi_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(bcm63xx_hsspi_pm_ops, bcm63xx_hsspi_suspend,
 			 bcm63xx_hsspi_resume);
 
+static const struct of_device_id bcm63xx_hsspi_of_match[] = {
+	{ .compatible = "brcm,bcm6328-hsspi", },
+	{ },
+};
+
 static struct platform_driver bcm63xx_hsspi_driver = {
 	.driver = {
 		.name	= "bcm63xx-hsspi",
 		.pm	= &bcm63xx_hsspi_pm_ops,
+		.of_match_table = bcm63xx_hsspi_of_match,
 	},
 	.probe		= bcm63xx_hsspi_probe,
 	.remove		= bcm63xx_hsspi_remove,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] spi/bcm63xx-hsspi: allow for probing through devicetree

[Florian Fainelli]

On 02/22/2017 05:19 AM, Jonas Gorski wrote:
> Add required binding support to probe through device tree.
> 
> Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/spi/spi-bcm63xx-hsspi.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> index 79096d17ebde..77c249a22c9b 100644
> --- a/drivers/spi/spi-bcm63xx-hsspi.c
> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/spi/spi.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  
>  #define HSSPI_GLOBAL_CTRL_REG			0x0
>  #define GLOBAL_CTRL_CS_POLARITY_SHIFT		0
> @@ -91,6 +92,7 @@
>  
>  #define HSSPI_MAX_SYNC_CLOCK			30000000
>  
> +#define HSSPI_SPI_MAX_CS			8
>  #define HSSPI_BUS_NUM				1 /* 0 is legacy SPI */
>  
>  struct bcm63xx_hsspi {
> @@ -332,7 +334,7 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct clk *clk;
>  	int irq, ret;
> -	u32 reg, rate;
> +	u32 reg, rate, num_cs = HSSPI_SPI_MAX_CS;
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -382,8 +384,20 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
>  	mutex_init(&bs->bus_mutex);
>  	init_completion(&bs->done);
>  
> -	master->bus_num = HSSPI_BUS_NUM;
> -	master->num_chipselect = 8;
> +	if (dev->of_node) {
> +		master->dev.of_node = dev->of_node;

You could move this out of the if () statement here.

> +
> +		of_property_read_u32(dev->of_node, "num-cs", &num_cs);
> +		if (num_cs > 8) {
> +			dev_warn(dev, "unsupported number of cs (%i), reducing to 8\n",
> +				 num_cs);
> +			num_cs = HSSPI_SPI_MAX_CS;
> +		}
> +	} else {
> +		master->bus_num = HSSPI_BUS_NUM;

And this one too, since the core will take care of assigning it based on
aliases and such when master->dev.of_node is correctly set.

With that changed:

Acked-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] spi/bcm63xx-hsspi: allow for probing through devicetree

[Jonas Gorski]

On 27 February 2017 at 23:54, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 02/22/2017 05:19 AM, Jonas Gorski wrote:
>> Add required binding support to probe through device tree.
>>
>> Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/spi/spi-bcm63xx-hsspi.c | 26 +++++++++++++++++++++++---
>>  1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
>> index 79096d17ebde..77c249a22c9b 100644
>> --- a/drivers/spi/spi-bcm63xx-hsspi.c
>> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/mutex.h>
>> +#include <linux/of.h>
>>
>>  #define HSSPI_GLOBAL_CTRL_REG                        0x0
>>  #define GLOBAL_CTRL_CS_POLARITY_SHIFT                0
>> @@ -91,6 +92,7 @@
>>
>>  #define HSSPI_MAX_SYNC_CLOCK                 30000000
>>
>> +#define HSSPI_SPI_MAX_CS                     8
>>  #define HSSPI_BUS_NUM                                1 /* 0 is legacy SPI */
>>
>>  struct bcm63xx_hsspi {
>> @@ -332,7 +334,7 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
>>       struct device *dev = &pdev->dev;
>>       struct clk *clk;
>>       int irq, ret;
>> -     u32 reg, rate;
>> +     u32 reg, rate, num_cs = HSSPI_SPI_MAX_CS;
>>
>>       irq = platform_get_irq(pdev, 0);
>>       if (irq < 0) {
>> @@ -382,8 +384,20 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
>>       mutex_init(&bs->bus_mutex);
>>       init_completion(&bs->done);
>>
>> -     master->bus_num = HSSPI_BUS_NUM;
>> -     master->num_chipselect = 8;
>> +     if (dev->of_node) {
>> +             master->dev.of_node = dev->of_node;
>
> You could move this out of the if () statement here.

Indeed, of_property_read_u32 is safe to call with a NULL np, so I
don't need to guard it extra.

>> +
>> +             of_property_read_u32(dev->of_node, "num-cs", &num_cs);
>> +             if (num_cs > 8) {
>> +                     dev_warn(dev, "unsupported number of cs (%i), reducing to 8\n",
>> +                              num_cs);
>> +                     num_cs = HSSPI_SPI_MAX_CS;
>> +             }
>> +     } else {
>> +             master->bus_num = HSSPI_BUS_NUM;
>
> And this one too, since the core will take care of assigning it based on
> aliases and such when master->dev.of_node is correctly set.

It will only do if bus_num is < 0, so I will still need to guard it
for !of_node, else the alias-based assignent won't work.


>
> With that changed:
>
> Acked-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks for the review!


Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] spi/bcm63xx-hsspi: allow providing clock rate through a second clock

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Wed, Feb 22, 2017 at 02:19:38PM +0100, Jonas Gorski wrote:
> Instead of requiring the hsspi clock to have a rate, allow using a second
> clock for providing the Hz rate, which is probably more correct anyway.

I'm sorry but I just can't follow the logic here at all - why would we
use "a second clock" to get the rate of a clock and why would that be
more correct?  There's a few steps in the reasoning missing here I
think!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] spi/bcm63xx-hsspi: allow providing clock rate through a second clock

[Jonas Gorski]

On 22 February 2017 at 19:26, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Feb 22, 2017 at 02:19:38PM +0100, Jonas Gorski wrote:
>> Instead of requiring the hsspi clock to have a rate, allow using a second
>> clock for providing the Hz rate, which is probably more correct anyway.
>
> I'm sorry but I just can't follow the logic here at all - why would we
> use "a second clock" to get the rate of a clock and why would that be
> more correct?  There's a few steps in the reasoning missing here I
> think!

The PLL rate is hardcoded in the original broadcom code (with
#if-deffery for different SoCs); when I added support for the HSSPI
controller in upstream linux, I thought "hey, let's put it as the rate
of the (gateable) HSSPI clock, then I don't need to pass it as
platform data". Part of the issue is also that bcm63xx has its own clk
code, so couldn't use clk lookups even if I wanted.

The register for the gated clock control is calld blkEnables (so might
be just power gating?), and the #define for the rate is called
HS_SPI_PLL_FREQ, which at least gives me the impression these could be
two different things/clocks. Whether it's a separate PLL from the
gated clock or not someone with actual hardware knowledge would have
to tell.

To (eventually) get rid of bcm63xx's own clk code and replace it with
a proper clock driver with lookups and stuff, it would make it much
easier to just move the PLL_FREQ into a separate fixed-rate clock, as
it is the only clock in the bcm63xx code that has a rate attached and
is gated. So I wouldn't need to create composite clocks to and could
just directly make it a simple gated-clock driver.

Hope that explanation helps.


Regards
Jonas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] spi/bcm63xx-hsspi: allow providing clock rate through a second clock

[Florian Fainelli]

On 02/22/2017 12:00 PM, Jonas Gorski wrote:
> On 22 February 2017 at 19:26, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Feb 22, 2017 at 02:19:38PM +0100, Jonas Gorski wrote:
>>> Instead of requiring the hsspi clock to have a rate, allow using a second
>>> clock for providing the Hz rate, which is probably more correct anyway.
>>
>> I'm sorry but I just can't follow the logic here at all - why would we
>> use "a second clock" to get the rate of a clock and why would that be
>> more correct?  There's a few steps in the reasoning missing here I
>> think!
> 
> The PLL rate is hardcoded in the original broadcom code (with
> #if-deffery for different SoCs); when I added support for the HSSPI
> controller in upstream linux, I thought "hey, let's put it as the rate
> of the (gateable) HSSPI clock, then I don't need to pass it as
> platform data". Part of the issue is also that bcm63xx has its own clk
> code, so couldn't use clk lookups even if I wanted.
> 
> The register for the gated clock control is calld blkEnables (so might
> be just power gating?), and the #define for the rate is called
> HS_SPI_PLL_FREQ, which at least gives me the impression these could be
> two different things/clocks. Whether it's a separate PLL from the
> gated clock or not someone with actual hardware knowledge would have
> to tell.

I looked at the HSSPI datasheet again and there are indeed two separate
input clock signals to the HSSPI block, one which is a PLL clock and
that is used to control the external SPI output/input clock. The other
clock is the UBUS register interface clock and also clock gates the
block when unused.

So with that:

Acked-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> 
> To (eventually) get rid of bcm63xx's own clk code and replace it with
> a proper clock driver with lookups and stuff, it would make it much
> easier to just move the PLL_FREQ into a separate fixed-rate clock, as
> it is the only clock in the bcm63xx code that has a rate attached and
> is gated. So I wouldn't need to create composite clocks to and could
> just directly make it a simple gated-clock driver.
> 
> Hope that explanation helps.
> 
> 
> Regards
> Jonas
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Applied "spi/bcm63xx-hsspi: allow providing clock rate through a second clock" to the spi tree

[Mark Brown]

The patch

   spi/bcm63xx-hsspi: allow providing clock rate through a second clock

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From ff18e1ef04e2073889569b07a5ddd54a6527917f Mon Sep 17 00:00:00 2001
From: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Wed, 1 Mar 2017 10:08:12 +0100
Subject: [PATCH] spi/bcm63xx-hsspi: allow providing clock rate through a
 second clock

The HSSPI block actually has two clock inputs, one for gating the block,
and one for the PLL rate. To allow these to be represented as two clocks,
add support for retrieving the rate from a separate "pll" clock, if the
"hsspi" clock does not provide one.

Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-bcm63xx-hsspi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 55789f7cda92..79096d17ebde 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -351,8 +351,16 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
 		return PTR_ERR(clk);
 
 	rate = clk_get_rate(clk);
-	if (!rate)
-		return -EINVAL;
+	if (!rate) {
+		struct clk *pll_clk = devm_clk_get(dev, "pll");
+
+		if (IS_ERR(pll_clk))
+			return PTR_ERR(pll_clk);
+
+		rate = clk_get_rate(pll_clk);
+		if (!rate)
+			return -EINVAL;
+	}
 
 	ret = clk_prepare_enable(clk);
 	if (ret)
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html