diff mbox series

spi: spi-fsl-spi: support use of the SPISEL_BOOT signal on MPC8309

Message ID 20190306103156.19897-1-rasmus.villemoes@prevas.dk (mailing list archive)
State Accepted
Commit 69b921acae8a5b7feef03921d9b42c3634f3b9d1
Headers show
Series spi: spi-fsl-spi: support use of the SPISEL_BOOT signal on MPC8309 | expand

Commit Message

Rasmus Villemoes March 6, 2019, 10:32 a.m. UTC
The MPC8309 has a dedicated signal, SPISEL_BOOT, usually used as chip
select for the flash device from which the bootloader is loaded. It is
not an ordinary gpio, but is simply controlled via the SPI_CS register
in the system configuration.

To allow accessing such a spi slave, we need to teach
fsl_spi_cs_control() how to control the SPISEL_BOOT signal. To
distinguish the gpio-controlled slaves, continue to have those use
chip_select values of 0..ngpios-1, and use chip_select == ngpios for
the boot flash.

I'm not too happy with all the ifdeffery, but it seems to be necessary
for guarding the sysdev/fsl_soc.h and use of
get_immrbase() (spi-fsl-lib.c already contains similar ifdeffery).

Googling suggests that the MPC8306 is similar, with the SPI_CS
register at the same offset.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 .../devicetree/bindings/spi/fsl-spi.txt       |  4 ++
 drivers/spi/spi-fsl-lib.h                     |  2 +
 drivers/spi/spi-fsl-spi.c                     | 40 ++++++++++++++++---
 3 files changed, 41 insertions(+), 5 deletions(-)

Comments

Rasmus Villemoes March 18, 2019, 7:43 a.m. UTC | #1
ping

On 06/03/2019 11.32, Rasmus Villemoes wrote:
> The MPC8309 has a dedicated signal, SPISEL_BOOT, usually used as chip
> select for the flash device from which the bootloader is loaded. It is
> not an ordinary gpio, but is simply controlled via the SPI_CS register
> in the system configuration.
> 
> To allow accessing such a spi slave, we need to teach
> fsl_spi_cs_control() how to control the SPISEL_BOOT signal. To
> distinguish the gpio-controlled slaves, continue to have those use
> chip_select values of 0..ngpios-1, and use chip_select == ngpios for
> the boot flash.
> 
> I'm not too happy with all the ifdeffery, but it seems to be necessary
> for guarding the sysdev/fsl_soc.h and use of
> get_immrbase() (spi-fsl-lib.c already contains similar ifdeffery).
> 
> Googling suggests that the MPC8306 is similar, with the SPI_CS
> register at the same offset.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  .../devicetree/bindings/spi/fsl-spi.txt       |  4 ++
>  drivers/spi/spi-fsl-lib.h                     |  2 +
>  drivers/spi/spi-fsl-spi.c                     | 40 ++++++++++++++++---
>  3 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/fsl-spi.txt b/Documentation/devicetree/bindings/spi/fsl-spi.txt
> index 8854004a1d3a..411375eac54d 100644
> --- a/Documentation/devicetree/bindings/spi/fsl-spi.txt
> +++ b/Documentation/devicetree/bindings/spi/fsl-spi.txt
> @@ -18,6 +18,10 @@ Optional properties:
>  - gpios : specifies the gpio pins to be used for chipselects.
>    The gpios will be referred to as reg = <index> in the SPI child nodes.
>    If unspecified, a single SPI device without a chip select can be used.
> +- fsl,spisel_boot : for the MPC8306 and MPC8309, specifies that the
> +  SPISEL_BOOT signal is used as chip select for a slave device. Use
> +  reg = <number of gpios> in the corresponding child node, i.e. 0 if
> +  the gpios property is not present.
>  
>  Example:
>  	spi@4c0 {
> diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h
> index f303f306b38e..483734bc1b1e 100644
> --- a/drivers/spi/spi-fsl-lib.h
> +++ b/drivers/spi/spi-fsl-lib.h
> @@ -95,8 +95,10 @@ static inline u32 mpc8xxx_spi_read_reg(__be32 __iomem *reg)
>  
>  struct mpc8xxx_spi_probe_info {
>  	struct fsl_spi_platform_data pdata;
> +	int ngpios;
>  	int *gpios;
>  	bool *alow_flags;
> +	__be32 __iomem *immr_spi_cs;
>  };
>  
>  extern u32 mpc8xxx_spi_tx_buf_u8(struct mpc8xxx_spi *mpc8xxx_spi);
> diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
> index 8f2e97857e8b..3d7b50c65f36 100644
> --- a/drivers/spi/spi-fsl-spi.c
> +++ b/drivers/spi/spi-fsl-spi.c
> @@ -39,6 +39,14 @@
>  #include <linux/spi/spi_bitbang.h>
>  #include <linux/types.h>
>  
> +#ifdef CONFIG_FSL_SOC
> +#include <sysdev/fsl_soc.h>
> +#endif
> +
> +/* Specific to the MPC8306/MPC8309 */
> +#define IMMR_SPI_CS_OFFSET 0x14c
> +#define SPI_BOOT_SEL_BIT   0x80000000
> +
>  #include "spi-fsl-lib.h"
>  #include "spi-fsl-cpm.h"
>  #include "spi-fsl-spi.h"
> @@ -701,10 +709,17 @@ static void fsl_spi_cs_control(struct spi_device *spi, bool on)
>  	struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
>  	struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
>  	u16 cs = spi->chip_select;
> -	int gpio = pinfo->gpios[cs];
> -	bool alow = pinfo->alow_flags[cs];
>  
> -	gpio_set_value(gpio, on ^ alow);
> +	if (cs < pinfo->ngpios) {
> +		int gpio = pinfo->gpios[cs];
> +		bool alow = pinfo->alow_flags[cs];
> +
> +		gpio_set_value(gpio, on ^ alow);
> +	} else {
> +		if (WARN_ON_ONCE(cs > pinfo->ngpios || !pinfo->immr_spi_cs))
> +			return;
> +		iowrite32be(on ? SPI_BOOT_SEL_BIT : 0, pinfo->immr_spi_cs);
> +	}
>  }
>  
>  static int of_fsl_spi_get_chipselects(struct device *dev)
> @@ -712,12 +727,15 @@ static int of_fsl_spi_get_chipselects(struct device *dev)
>  	struct device_node *np = dev->of_node;
>  	struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
>  	struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
> +	bool spisel_boot = IS_ENABLED(CONFIG_FSL_SOC) &&
> +		of_property_read_bool(np, "fsl,spisel_boot");
>  	int ngpios;
>  	int i = 0;
>  	int ret;
>  
>  	ngpios = of_gpio_count(np);
> -	if (ngpios <= 0) {
> +	ngpios = max(ngpios, 0);
> +	if (ngpios == 0 && !spisel_boot) {
>  		/*
>  		 * SPI w/o chip-select line. One SPI device is still permitted
>  		 * though.
> @@ -726,6 +744,7 @@ static int of_fsl_spi_get_chipselects(struct device *dev)
>  		return 0;
>  	}
>  
> +	pinfo->ngpios = ngpios;
>  	pinfo->gpios = kmalloc_array(ngpios, sizeof(*pinfo->gpios),
>  				     GFP_KERNEL);
>  	if (!pinfo->gpios)
> @@ -769,7 +788,18 @@ static int of_fsl_spi_get_chipselects(struct device *dev)
>  		}
>  	}
>  
> -	pdata->max_chipselect = ngpios;
> +#if IS_ENABLED(CONFIG_FSL_SOC)
> +	if (spisel_boot) {
> +		pinfo->immr_spi_cs = ioremap(get_immrbase() + IMMR_SPI_CS_OFFSET, 4);
> +		if (!pinfo->immr_spi_cs) {
> +			ret = -ENOMEM;
> +			i = ngpios - 1;
> +			goto err_loop;
> +		}
> +	}
> +#endif
> +
> +	pdata->max_chipselect = ngpios + spisel_boot;
>  	pdata->cs_control = fsl_spi_cs_control;
>  
>  	return 0;
>
Mark Brown March 18, 2019, 10:22 a.m. UTC | #2
On Mon, Mar 18, 2019 at 07:43:47AM +0000, Rasmus Villemoes wrote:
> ping
> 
> On 06/03/2019 11.32, Rasmus Villemoes wrote:

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/fsl-spi.txt b/Documentation/devicetree/bindings/spi/fsl-spi.txt
index 8854004a1d3a..411375eac54d 100644
--- a/Documentation/devicetree/bindings/spi/fsl-spi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-spi.txt
@@ -18,6 +18,10 @@  Optional properties:
 - gpios : specifies the gpio pins to be used for chipselects.
   The gpios will be referred to as reg = <index> in the SPI child nodes.
   If unspecified, a single SPI device without a chip select can be used.
+- fsl,spisel_boot : for the MPC8306 and MPC8309, specifies that the
+  SPISEL_BOOT signal is used as chip select for a slave device. Use
+  reg = <number of gpios> in the corresponding child node, i.e. 0 if
+  the gpios property is not present.
 
 Example:
 	spi@4c0 {
diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h
index f303f306b38e..483734bc1b1e 100644
--- a/drivers/spi/spi-fsl-lib.h
+++ b/drivers/spi/spi-fsl-lib.h
@@ -95,8 +95,10 @@  static inline u32 mpc8xxx_spi_read_reg(__be32 __iomem *reg)
 
 struct mpc8xxx_spi_probe_info {
 	struct fsl_spi_platform_data pdata;
+	int ngpios;
 	int *gpios;
 	bool *alow_flags;
+	__be32 __iomem *immr_spi_cs;
 };
 
 extern u32 mpc8xxx_spi_tx_buf_u8(struct mpc8xxx_spi *mpc8xxx_spi);
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 8f2e97857e8b..3d7b50c65f36 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -39,6 +39,14 @@ 
 #include <linux/spi/spi_bitbang.h>
 #include <linux/types.h>
 
+#ifdef CONFIG_FSL_SOC
+#include <sysdev/fsl_soc.h>
+#endif
+
+/* Specific to the MPC8306/MPC8309 */
+#define IMMR_SPI_CS_OFFSET 0x14c
+#define SPI_BOOT_SEL_BIT   0x80000000
+
 #include "spi-fsl-lib.h"
 #include "spi-fsl-cpm.h"
 #include "spi-fsl-spi.h"
@@ -701,10 +709,17 @@  static void fsl_spi_cs_control(struct spi_device *spi, bool on)
 	struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
 	struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
 	u16 cs = spi->chip_select;
-	int gpio = pinfo->gpios[cs];
-	bool alow = pinfo->alow_flags[cs];
 
-	gpio_set_value(gpio, on ^ alow);
+	if (cs < pinfo->ngpios) {
+		int gpio = pinfo->gpios[cs];
+		bool alow = pinfo->alow_flags[cs];
+
+		gpio_set_value(gpio, on ^ alow);
+	} else {
+		if (WARN_ON_ONCE(cs > pinfo->ngpios || !pinfo->immr_spi_cs))
+			return;
+		iowrite32be(on ? SPI_BOOT_SEL_BIT : 0, pinfo->immr_spi_cs);
+	}
 }
 
 static int of_fsl_spi_get_chipselects(struct device *dev)
@@ -712,12 +727,15 @@  static int of_fsl_spi_get_chipselects(struct device *dev)
 	struct device_node *np = dev->of_node;
 	struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
 	struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
+	bool spisel_boot = IS_ENABLED(CONFIG_FSL_SOC) &&
+		of_property_read_bool(np, "fsl,spisel_boot");
 	int ngpios;
 	int i = 0;
 	int ret;
 
 	ngpios = of_gpio_count(np);
-	if (ngpios <= 0) {
+	ngpios = max(ngpios, 0);
+	if (ngpios == 0 && !spisel_boot) {
 		/*
 		 * SPI w/o chip-select line. One SPI device is still permitted
 		 * though.
@@ -726,6 +744,7 @@  static int of_fsl_spi_get_chipselects(struct device *dev)
 		return 0;
 	}
 
+	pinfo->ngpios = ngpios;
 	pinfo->gpios = kmalloc_array(ngpios, sizeof(*pinfo->gpios),
 				     GFP_KERNEL);
 	if (!pinfo->gpios)
@@ -769,7 +788,18 @@  static int of_fsl_spi_get_chipselects(struct device *dev)
 		}
 	}
 
-	pdata->max_chipselect = ngpios;
+#if IS_ENABLED(CONFIG_FSL_SOC)
+	if (spisel_boot) {
+		pinfo->immr_spi_cs = ioremap(get_immrbase() + IMMR_SPI_CS_OFFSET, 4);
+		if (!pinfo->immr_spi_cs) {
+			ret = -ENOMEM;
+			i = ngpios - 1;
+			goto err_loop;
+		}
+	}
+#endif
+
+	pdata->max_chipselect = ngpios + spisel_boot;
 	pdata->cs_control = fsl_spi_cs_control;
 
 	return 0;