diff mbox series

[3/3] spi: mxic: add maximum speed of spi host

Message ID 1611908018-29937-4-git-send-email-zhengxunli@mxic.com.tw
State Not Applicable
Headers show
Series Add octal DTR support for Macronix flash | expand

Commit Message

Zhengxun Li Jan. 29, 2021, 8:13 a.m. UTC
Add the maximum speed of the host to avoid over-speed
operation of the device.

Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
---
 Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 ++++
 drivers/spi/spi-mxic.c                             | 7 +++++++
 2 files changed, 11 insertions(+)

Comments

Miquel Raynal Feb. 1, 2021, 3:02 p.m. UTC | #1
Hi,

zhengxunli <zhengxunli@mxic.com.tw> wrote on Fri, 29 Jan 2021 16:13:38
+0800:

> Add the maximum speed of the host to avoid over-speed
> operation of the device.
> 
> Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw>
> ---
>  Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 ++++
>  drivers/spi/spi-mxic.c                             | 7 +++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt b/Documentation/devicetree/bindings/spi/spi-mxic.txt
> index 529f2da..ff6937c 100644
> --- a/Documentation/devicetree/bindings/spi/spi-mxic.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-mxic.txt

Binding and driver changes should be in two separate commits.

> @@ -13,6 +13,9 @@ Required properties:
>  - clocks: should contain 3 entries for the "ps_clk", "send_clk" and
>  	  "send_dly_clk" clocks
>  
> +Recommended properties:
> +- spi-max-frequency: Maximum SPI clocking speed of the device in Hz.

I don't think this property should be part of the controller binding.
It is a per device property and is already parsed by the core (see
of_spi_parse_dt() in spi.c).

> +
>  Example:
>  
>  	spi@43c30000 {
> @@ -21,6 +24,7 @@ Example:
>  		reg-names = "regs", "dirmap";
>  		clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>;
>  		clock-names = "send_clk", "send_dly_clk", "ps_clk";
> +		spi-max-frequency = <200000000>;
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
> index 821328a..7786d33 100644
> --- a/drivers/spi/spi-mxic.c
> +++ b/drivers/spi/spi-mxic.c
> @@ -12,6 +12,7 @@
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/spi/spi.h>
> @@ -536,7 +537,9 @@ static int mxic_spi_probe(struct platform_device *pdev)
>  {
>  	struct spi_master *master;
>  	struct resource *res;
> +	struct device_node *np = pdev->dev.of_node;
>  	struct mxic_spi *mxic;
> +	u32 max_freq;
>  	int ret;
>  
>  	master = devm_spi_alloc_master(&pdev->dev, sizeof(struct mxic_spi));
> @@ -580,6 +583,10 @@ static int mxic_spi_probe(struct platform_device *pdev)
>  			SPI_RX_QUAD | SPI_TX_QUAD |
>  			SPI_RX_OCTAL | SPI_TX_OCTAL;
>  
> +	if (of_property_read_u32(np, "spi-max-frequency",
> +				 &master->max_speed_hz))
> +		master->max_speed_hz = 25000000; /* 25MHz */

As said before, there are two limitations to take into account:
1- your controller maximum speed
2- your flash maximum speed

(1) I don't think this needs to be exported through the DT, the driver
    knows his own limitations and enforces them.

(2) Should be either advertised by the DT through the spi-max-frequency
    property or advertised somehow by the vendor driver (macronix.c).

> +
>  	mxic_spi_hw_init(mxic);
>  
>  	ret = spi_register_master(master);

Thanks,
Miquèl
Miquel Raynal Feb. 2, 2021, 8:04 a.m. UTC | #2
Hello,

> > > @@ -536,7 +537,9 @@ static int mxic_spi_probe(struct platform_device   
> *pdev)
> > >  {
> > >     struct spi_master *master;
> > >     struct resource *res;
> > > +   struct device_node *np = pdev->dev.of_node;
> > >     struct mxic_spi *mxic;
> > > +   u32 max_freq;
> > >     int ret;
> > > 
> > >     master = devm_spi_alloc_master(&pdev->dev, sizeof(struct   
> mxic_spi));
> > > @@ -580,6 +583,10 @@ static int mxic_spi_probe(struct platform_device   
> *pdev)
> > >           SPI_RX_QUAD | SPI_TX_QUAD |
> > >           SPI_RX_OCTAL | SPI_TX_OCTAL;
> > > 
> > > +   if (of_property_read_u32(np, "spi-max-frequency",
> > > +             &master->max_speed_hz))
> > > +      master->max_speed_hz = 25000000; /* 25MHz */  
> > 
> > As said before, there are two limitations to take into account:
> > 1- your controller maximum speed
> > 2- your flash maximum speed
> > 
> > (1) I don't think this needs to be exported through the DT, the driver
> >     knows his own limitations and enforces them.  
> 
> In fact, if master->max_speed_hz is not set, spi->max_speed_hz will be 0 
> and the
> host frequency cannot be set correctly.

You should set master->max_speed_hz in the probe. This is the maximum
speed your host can run at and has *nothing to do* with whatever device
is attached on the bus.

In the DT, use the spi-max-frequency property to inform the controller
about the maximum speed the device supports.

Then the controller should receive requests at the frequency chosen
with: min(master_max_freq, device_max_freq).

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt b/Documentation/devicetree/bindings/spi/spi-mxic.txt
index 529f2da..ff6937c 100644
--- a/Documentation/devicetree/bindings/spi/spi-mxic.txt
+++ b/Documentation/devicetree/bindings/spi/spi-mxic.txt
@@ -13,6 +13,9 @@  Required properties:
 - clocks: should contain 3 entries for the "ps_clk", "send_clk" and
 	  "send_dly_clk" clocks
 
+Recommended properties:
+- spi-max-frequency: Maximum SPI clocking speed of the device in Hz.
+
 Example:
 
 	spi@43c30000 {
@@ -21,6 +24,7 @@  Example:
 		reg-names = "regs", "dirmap";
 		clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>;
 		clock-names = "send_clk", "send_dly_clk", "ps_clk";
+		spi-max-frequency = <200000000>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 821328a..7786d33 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -12,6 +12,7 @@ 
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/spi/spi.h>
@@ -536,7 +537,9 @@  static int mxic_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
 	struct resource *res;
+	struct device_node *np = pdev->dev.of_node;
 	struct mxic_spi *mxic;
+	u32 max_freq;
 	int ret;
 
 	master = devm_spi_alloc_master(&pdev->dev, sizeof(struct mxic_spi));
@@ -580,6 +583,10 @@  static int mxic_spi_probe(struct platform_device *pdev)
 			SPI_RX_QUAD | SPI_TX_QUAD |
 			SPI_RX_OCTAL | SPI_TX_OCTAL;
 
+	if (of_property_read_u32(np, "spi-max-frequency",
+				 &master->max_speed_hz))
+		master->max_speed_hz = 25000000; /* 25MHz */
+
 	mxic_spi_hw_init(mxic);
 
 	ret = spi_register_master(master);