[U-Boot] [PATCH v2 0/4] kirkwood spi_claim/release_bus support

[U-Boot] [PATCH v2 0/4] kirkwood spi_claim/release_bus support

[Valentin Longchamp]

This series adds generic support for the spi_claim/release_bus functions for
the kirkwood processors.

The implementation was already discussed in another thread following my first
board specific submission of the patch.

The series adds two functions to the kirkwood mpp code to be able to temporarily
save and then restore the mpp configuration.

Changes for v2:
	- save MPP configuration with mpp_read function as dicussed on ML
	- moved CS pin MPP config to spi_setup_slave only
	- add backup fo CS pin in spi_setup_slave and reset in spi_free_slave

Valentin Longchamp (4):
  kirkwood: add kirkwood_mpp_read function
  kw_spi: backup and reset the MPP of the chosen CS pin
  kw_spi: support spi_claim/release_bus functions
  kw_spi: add weak functions board_spi_claim/release_bus

 arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   41 +++++++++++++++++++
 arch/arm/include/asm/arch-kirkwood/mpp.h |    1 +
 arch/arm/include/asm/arch-kirkwood/spi.h |   11 +++++
 drivers/spi/kirkwood_spi.c               |   65 +++++++++++++++++++++++++----
 4 files changed, 109 insertions(+), 9 deletions(-)

[U-Boot] [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function

[Valentin Longchamp]

This function can be used to save current mpp state of all mpp pins
given in the mpp_list argument by reading the mpp registers, in the
second mpp_saved argument.

A later call to kirkwood_mpp_conf function with this saved list will
reset the mpp configuration as it was when kirkwood_mpp_read was called.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
---
 arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   41 ++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-kirkwood/mpp.h |    1 +
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
index 3da6c98..9fb9aea 100644
--- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
+++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
@@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list)
 	debug("\n");
 
 }
+
+void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved)
+{
+	u32 mpp_ctrl[MPP_NR_REGS];
+	unsigned int variant_mask;
+	int i;
+
+	variant_mask = kirkwood_variant();
+	if (!variant_mask)
+		return;
+
+	for (i = 0; i < MPP_NR_REGS; i++) {
+		mpp_ctrl[i] = readl(MPP_CTRL(i));
+		debug(" %08x", mpp_ctrl[i]);
+	}
+
+	while (*mpp_list) {
+		unsigned int num = MPP_NUM(*mpp_list);
+		unsigned int sel;
+		int shift;
+
+		if (num > MPP_MAX) {
+			debug("kirkwood_mpp_conf: invalid MPP "
+					"number (%u)\n", num);
+			continue;
+		}
+		if (!(*mpp_list & variant_mask)) {
+			debug("kirkwood_mpp_conf: requested MPP%u config "
+				"unavailable on this hardware\n", num);
+			continue;
+		}
+
+		shift = (num & 7) << 2;
+		sel = (mpp_ctrl[num / 8] >> shift) & 0xf;
+		*mpp_saved = num | (sel << 8) | variant_mask;
+
+		mpp_list++;
+		mpp_saved++;
+	}
+}
+
diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h b/arch/arm/include/asm/arch-kirkwood/mpp.h
index b3c090e..22a64b8 100644
--- a/arch/arm/include/asm/arch-kirkwood/mpp.h
+++ b/arch/arm/include/asm/arch-kirkwood/mpp.h
@@ -313,5 +313,6 @@
 #define MPP_MAX			49
 
 void kirkwood_mpp_conf(unsigned int *mpp_list);
+void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved);
 
 #endif
-- 
1.7.1

[U-Boot] [PATCH v2 2/4] kw_spi: backup and reset the MPP of the chosen CS pin

[Valentin Longchamp]

This was not done before, and in the case of a shared pin (for MPP0
between NF_IO[2] and CSn) this could lead to problems.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
---
 drivers/spi/kirkwood_spi.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index db8ba8b..2fd89d2 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -34,16 +34,14 @@
 
 static struct kwspi_registers *spireg = (struct kwspi_registers *)KW_SPI_BASE;
 
+u32 cs_spi_mpp_back[2];
+
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 				unsigned int max_hz, unsigned int mode)
 {
 	struct spi_slave *slave;
 	u32 data;
-	u32 kwspi_mpp_config[] = {
-		MPP0_GPIO,
-		MPP7_SPI_SCn,
-		0
-	};
+	u32 kwspi_mpp_config[] = { 0, 0 };
 
 	if (!spi_cs_is_valid(bus, cs))
 		return NULL;
@@ -70,12 +68,11 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 
 	/* program mpp registers to select  SPI_CSn */
 	if (cs) {
-		kwspi_mpp_config[0] = MPP0_GPIO;
-		kwspi_mpp_config[1] = MPP7_SPI_SCn;
+		kwspi_mpp_config[0] = MPP7_SPI_SCn;
 	} else {
 		kwspi_mpp_config[0] = MPP0_SPI_SCn;
-		kwspi_mpp_config[1] = MPP7_GPO;
 	}
+	kirkwood_mpp_read(kwspi_mpp_config, cs_spi_mpp_back);
 	kirkwood_mpp_conf(kwspi_mpp_config);
 
 	return slave;
@@ -83,6 +80,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 
 void spi_free_slave(struct spi_slave *slave)
 {
+	kirkwood_mpp_conf(cs_spi_mpp_back);
 	free(slave);
 }
 
-- 
1.7.1

[U-Boot] [PATCH v2 3/4] kw_spi: support spi_claim/release_bus functions

[Valentin Longchamp]

These two function nows ensure that the MPP is configured correctly for
the SPI controller before any SPI access, and restore the initial
configuration when the access is over.

Since the used pins for the SPI controller can differ (2 possibilities
for each signal), the used pins are configured with CONFIG_SYS_KW_SPI_MPP.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
---
 arch/arm/include/asm/arch-kirkwood/spi.h |   11 ++++++++
 drivers/spi/kirkwood_spi.c               |   38 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h b/arch/arm/include/asm/arch-kirkwood/spi.h
index 1d5043f..c79bed7 100644
--- a/arch/arm/include/asm/arch-kirkwood/spi.h
+++ b/arch/arm/include/asm/arch-kirkwood/spi.h
@@ -37,6 +37,17 @@ struct kwspi_registers {
 	u32 irq_mask;	/* 0x10614 */
 };
 
+/* They are used to define CONFIG_SYS_KW_SPI_MPP
+ * each of the below #defines selects which mpp is
+ * configured for each SPI signal in spi_claim_bus
+ * bit 0: selects pin for MOSI (MPP1 if 0, MPP6 if 1)
+ * bit 1: selects pin for SCK (MPP2 if 0, MPP10 if 1)
+ * bit 2: selects pin for MISO (MPP3 if 0, MPP11 if 1)
+ */
+#define MOSI_MPP6	(1 << 0)
+#define SCK_MPP10	(1 << 1)
+#define MISO_MPP11	(1 << 2)
+
 #define KWSPI_CLKPRESCL_MASK	0x1f
 #define KWSPI_CSN_ACT		1 /* Activates serial memory interface */
 #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index 2fd89d2..a3e6b6e 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -84,13 +84,51 @@ void spi_free_slave(struct spi_slave *slave)
 	free(slave);
 }
 
+#if defined(CONFIG_SYS_KW_SPI_MPP)
+u32 spi_mpp_backup[4];
+#endif
+
 int spi_claim_bus(struct spi_slave *slave)
 {
+#if defined(CONFIG_SYS_KW_SPI_MPP)
+	u32 config;
+	u32 spi_mpp_config[4];
+
+	config = CONFIG_SYS_KW_SPI_MPP;
+
+	if (config & MOSI_MPP6)
+		spi_mpp_config[0] = MPP6_SPI_MOSI;
+	else
+		spi_mpp_config[0] = MPP1_SPI_MOSI;
+
+	if (config & SCK_MPP10)
+		spi_mpp_config[1] = MPP10_SPI_SCK;
+	else
+		spi_mpp_config[1] = MPP2_SPI_SCK;
+
+	if (config & MISO_MPP11)
+		spi_mpp_config[2] = MPP11_SPI_MISO;
+	else
+		spi_mpp_config[2] = MPP3_SPI_MISO;
+
+	spi_mpp_config[3] = 0;
+	spi_mpp_backup[3] = 0;
+
+	/* save current mpp configuration */
+	kirkwood_mpp_read(spi_mpp_config, spi_mpp_backup);
+
+	/* finally set chosen mpp spi configuration */
+	kirkwood_mpp_conf(spi_mpp_config);
+#endif
+
 	return 0;
 }
 
 void spi_release_bus(struct spi_slave *slave)
 {
+#if defined(CONFIG_SYS_KW_SPI_MPP)
+	kirkwood_mpp_conf(spi_mpp_backup);
+#endif
 }
 
 #ifndef CONFIG_SPI_CS_IS_VALID
-- 
1.7.1

[U-Boot] [PATCH v2 4/4] kw_spi: add weak functions board_spi_claim/release_bus

[Valentin Longchamp]

This allows a final, board specific, step in the claim/relase_bus
function for the SPI controller, which may be needed for some hardware
designs.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
---
 drivers/spi/kirkwood_spi.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index a3e6b6e..a0da527 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -88,6 +88,11 @@ void spi_free_slave(struct spi_slave *slave)
 u32 spi_mpp_backup[4];
 #endif
 
+__attribute__((weak)) int board_spi_claim_bus(struct spi_slave *slave)
+{
+	return 0;
+}
+
 int spi_claim_bus(struct spi_slave *slave)
 {
 #if defined(CONFIG_SYS_KW_SPI_MPP)
@@ -121,7 +126,11 @@ int spi_claim_bus(struct spi_slave *slave)
 	kirkwood_mpp_conf(spi_mpp_config);
 #endif
 
-	return 0;
+	return board_spi_claim_bus(slave);
+}
+
+__attribute__((weak)) void board_spi_release_bus(struct spi_slave *slave)
+{
 }
 
 void spi_release_bus(struct spi_slave *slave)
@@ -129,6 +138,8 @@ void spi_release_bus(struct spi_slave *slave)
 #if defined(CONFIG_SYS_KW_SPI_MPP)
 	kirkwood_mpp_conf(spi_mpp_backup);
 #endif
+
+	board_spi_release_bus(slave);
 }
 
 #ifndef CONFIG_SPI_CS_IS_VALID
-- 
1.7.1

[U-Boot] [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 30 May 2012 21:12
> To: Prafulla Wadaskar
> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
> boot at lists.denx.de; Prafulla Wadaskar
> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function
> 
> This function can be used to save current mpp state of all mpp pins
> given in the mpp_list argument by reading the mpp registers, in the
> second mpp_saved argument.
> 
> A later call to kirkwood_mpp_conf function with this saved list will
> reset the mpp configuration as it was when kirkwood_mpp_read was
> called.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   41
> ++++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-kirkwood/mpp.h |    1 +
>  2 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> index 3da6c98..9fb9aea 100644
> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list)
>  	debug("\n");
> 
>  }
> +
> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved)
> +{
> +	u32 mpp_ctrl[MPP_NR_REGS];
> +	unsigned int variant_mask;
> +	int i;
> +
> +	variant_mask = kirkwood_variant();
> +	if (!variant_mask)
> +		return;
> +
> +	for (i = 0; i < MPP_NR_REGS; i++) {
> +		mpp_ctrl[i] = readl(MPP_CTRL(i));
> +		debug(" %08x", mpp_ctrl[i]);
> +	}
> +
> +	while (*mpp_list) {
> +		unsigned int num = MPP_NUM(*mpp_list);
> +		unsigned int sel;
> +		int shift;
> +
> +		if (num > MPP_MAX) {
> +			debug("kirkwood_mpp_conf: invalid MPP "
> +					"number (%u)\n", num);
> +			continue;
> +		}
> +		if (!(*mpp_list & variant_mask)) {
> +			debug("kirkwood_mpp_conf: requested MPP%u config "
> +				"unavailable on this hardware\n", num);
> +			continue;
> +		}
> +
> +		shift = (num & 7) << 2;
> +		sel = (mpp_ctrl[num / 8] >> shift) & 0xf;
> +		*mpp_saved = num | (sel << 8) | variant_mask;
> +
> +		mpp_list++;
> +		mpp_saved++;
> +	}
> +}
> +

Hi Valentin
There is code duplication, similar code it already there in function  kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read function within kirkwood_mpp_conf

Regards..
Prafulla . . .

> diff --git a/arch/arm/include/asm/arch-kirkwood/mpp.h
> b/arch/arm/include/asm/arch-kirkwood/mpp.h
> index b3c090e..22a64b8 100644
> --- a/arch/arm/include/asm/arch-kirkwood/mpp.h
> +++ b/arch/arm/include/asm/arch-kirkwood/mpp.h
> @@ -313,5 +313,6 @@
>  #define MPP_MAX			49
> 
>  void kirkwood_mpp_conf(unsigned int *mpp_list);
> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved);
> 
>  #endif
> --
> 1.7.1

[U-Boot] [PATCH v2 2/4] kw_spi: backup and reset the MPP of the chosen CS pin

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 30 May 2012 21:12
> To: Prafulla Wadaskar
> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
> boot at lists.denx.de; Prafulla Wadaskar
> Subject: [PATCH v2 2/4] kw_spi: backup and reset the MPP of the chosen
> CS pin
> 
> This was not done before, and in the case of a shared pin (for MPP0
> between NF_IO[2] and CSn) this could lead to problems.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  drivers/spi/kirkwood_spi.c |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index db8ba8b..2fd89d2 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -34,16 +34,14 @@
> 
>  static struct kwspi_registers *spireg = (struct kwspi_registers
> *)KW_SPI_BASE;
> 
> +u32 cs_spi_mpp_back[2];
> +
>  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>  				unsigned int max_hz, unsigned int mode)
>  {
>  	struct spi_slave *slave;
>  	u32 data;
> -	u32 kwspi_mpp_config[] = {
> -		MPP0_GPIO,
> -		MPP7_SPI_SCn,
> -		0
> -	};
> +	u32 kwspi_mpp_config[] = { 0, 0 };
> 
>  	if (!spi_cs_is_valid(bus, cs))
>  		return NULL;
> @@ -70,12 +68,11 @@ struct spi_slave *spi_setup_slave(unsigned int
> bus, unsigned int cs,
> 
>  	/* program mpp registers to select  SPI_CSn */
>  	if (cs) {
> -		kwspi_mpp_config[0] = MPP0_GPIO;
> -		kwspi_mpp_config[1] = MPP7_SPI_SCn;
> +		kwspi_mpp_config[0] = MPP7_SPI_SCn;
>  	} else {
>  		kwspi_mpp_config[0] = MPP0_SPI_SCn;
> -		kwspi_mpp_config[1] = MPP7_GPO;
>  	}
> +	kirkwood_mpp_read(kwspi_mpp_config, cs_spi_mpp_back);
>  	kirkwood_mpp_conf(kwspi_mpp_config);
> 
>  	return slave;
> @@ -83,6 +80,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
> unsigned int cs,
> 
>  void spi_free_slave(struct spi_slave *slave)
>  {
> +	kirkwood_mpp_conf(cs_spi_mpp_back);
>  	free(slave);
>  }
> 
> --

Acked-By: Prafulla Wadaskar <prafulla@maravell.com>
But as you said functionality need to be verified, I hope you have verified it on any of Kirkwood variant.

Regards..
Prafulla . . .

> 1.7.1

[U-Boot] [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function

[Valentin Longchamp]

Hi Prafulla,

On 05/31/2012 10:30 AM, Prafulla Wadaskar wrote:
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 30 May 2012 21:12
>> To: Prafulla Wadaskar
>> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
>> boot at lists.denx.de; Prafulla Wadaskar
>> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function
>>
>> This function can be used to save current mpp state of all mpp pins
>> given in the mpp_list argument by reading the mpp registers, in the
>> second mpp_saved argument.
>>
>> A later call to kirkwood_mpp_conf function with this saved list will
>> reset the mpp configuration as it was when kirkwood_mpp_read was
>> called.
>>
>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>> cc: Holger Brunck <holger.brunck@keymile.com>
>> cc: Prafulla Wadaskar <prafulla@marvell.com>
>> ---
>>  arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   41
>> ++++++++++++++++++++++++++++++
>>  arch/arm/include/asm/arch-kirkwood/mpp.h |    1 +
>>  2 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> index 3da6c98..9fb9aea 100644
>> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list)
>>  	debug("\n");
>>
>>  }
>> +
>> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved)
>> +{
>> +	u32 mpp_ctrl[MPP_NR_REGS];
>> +	unsigned int variant_mask;
>> +	int i;
>> +
>> +	variant_mask = kirkwood_variant();
>> +	if (!variant_mask)
>> +		return;
>> +
>> +	for (i = 0; i < MPP_NR_REGS; i++) {
>> +		mpp_ctrl[i] = readl(MPP_CTRL(i));
>> +		debug(" %08x", mpp_ctrl[i]);
>> +	}
>> +
>> +	while (*mpp_list) {
>> +		unsigned int num = MPP_NUM(*mpp_list);
>> +		unsigned int sel;
>> +		int shift;
>> +
>> +		if (num > MPP_MAX) {
>> +			debug("kirkwood_mpp_conf: invalid MPP "
>> +					"number (%u)\n", num);
>> +			continue;
>> +		}
>> +		if (!(*mpp_list & variant_mask)) {
>> +			debug("kirkwood_mpp_conf: requested MPP%u config "
>> +				"unavailable on this hardware\n", num);
>> +			continue;
>> +		}
>> +
>> +		shift = (num & 7) << 2;
>> +		sel = (mpp_ctrl[num / 8] >> shift) & 0xf;
>> +		*mpp_saved = num | (sel << 8) | variant_mask;
>> +
>> +		mpp_list++;
>> +		mpp_saved++;
>> +	}
>> +}
>> +
> 
> Hi Valentin
> There is code duplication, similar code it already there in function  kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read function within kirkwood_mpp_conf
> 

Not sure I understand what you mean. You want me to implement the
kirkwood_mpp_read functionnality directly into kirkwood_mpp_conf ?

If this is so, it would mean that I would have to change kirkwood_mpp_conf "API"
to add the second argument (mpp_saved) and then I would have to fix all the
calls to this function. Is that what you mean ?

Val

[U-Boot] [PATCH v2 3/4] kw_spi: support spi_claim/release_bus functions

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 30 May 2012 21:12
> To: Prafulla Wadaskar
> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
> boot at lists.denx.de; Prafulla Wadaskar
> Subject: [PATCH v2 3/4] kw_spi: support spi_claim/release_bus
> functions
> 
> These two function nows ensure that the MPP is configured correctly
> for
> the SPI controller before any SPI access, and restore the initial
> configuration when the access is over.
> 
> Since the used pins for the SPI controller can differ (2 possibilities
> for each signal), the used pins are configured with
> CONFIG_SYS_KW_SPI_MPP.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  arch/arm/include/asm/arch-kirkwood/spi.h |   11 ++++++++
>  drivers/spi/kirkwood_spi.c               |   38
> ++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h
> b/arch/arm/include/asm/arch-kirkwood/spi.h
> index 1d5043f..c79bed7 100644
> --- a/arch/arm/include/asm/arch-kirkwood/spi.h
> +++ b/arch/arm/include/asm/arch-kirkwood/spi.h
> @@ -37,6 +37,17 @@ struct kwspi_registers {
>  	u32 irq_mask;	/* 0x10614 */
>  };
> 
> +/* They are used to define CONFIG_SYS_KW_SPI_MPP
> + * each of the below #defines selects which mpp is
> + * configured for each SPI signal in spi_claim_bus
> + * bit 0: selects pin for MOSI (MPP1 if 0, MPP6 if 1)
> + * bit 1: selects pin for SCK (MPP2 if 0, MPP10 if 1)
> + * bit 2: selects pin for MISO (MPP3 if 0, MPP11 if 1)
> + */
> +#define MOSI_MPP6	(1 << 0)
> +#define SCK_MPP10	(1 << 1)
> +#define MISO_MPP11	(1 << 2)
> +
>  #define KWSPI_CLKPRESCL_MASK	0x1f
>  #define KWSPI_CSN_ACT		1 /* Activates serial memory interface
> */
>  #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index 2fd89d2..a3e6b6e 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -84,13 +84,51 @@ void spi_free_slave(struct spi_slave *slave)
>  	free(slave);
>  }
> 
> +#if defined(CONFIG_SYS_KW_SPI_MPP)
> +u32 spi_mpp_backup[4];
> +#endif
> +
>  int spi_claim_bus(struct spi_slave *slave)
>  {
> +#if defined(CONFIG_SYS_KW_SPI_MPP)
> +	u32 config;
> +	u32 spi_mpp_config[4];
> +
> +	config = CONFIG_SYS_KW_SPI_MPP;
> +
> +	if (config & MOSI_MPP6)
> +		spi_mpp_config[0] = MPP6_SPI_MOSI;
> +	else
> +		spi_mpp_config[0] = MPP1_SPI_MOSI;
> +
> +	if (config & SCK_MPP10)
> +		spi_mpp_config[1] = MPP10_SPI_SCK;
> +	else
> +		spi_mpp_config[1] = MPP2_SPI_SCK;
> +
> +	if (config & MISO_MPP11)
> +		spi_mpp_config[2] = MPP11_SPI_MISO;
> +	else
> +		spi_mpp_config[2] = MPP3_SPI_MISO;
> +
> +	spi_mpp_config[3] = 0;
> +	spi_mpp_backup[3] = 0;
> +
> +	/* save current mpp configuration */
> +	kirkwood_mpp_read(spi_mpp_config, spi_mpp_backup);
> +
> +	/* finally set chosen mpp spi configuration */
> +	kirkwood_mpp_conf(spi_mpp_config);
> +#endif
> +
>  	return 0;
>  }
> 
>  void spi_release_bus(struct spi_slave *slave)
>  {
> +#if defined(CONFIG_SYS_KW_SPI_MPP)
> +	kirkwood_mpp_conf(spi_mpp_backup);
> +#endif
>  }
> 

Acked-By: Prafulla Wadaskar <Prafulla@marvell.com>
One more comment: do you think we should safeguard spi_claim_bus for repeatative calls, for ex. once bus is claimed, it should be reclaimed until we release it?

Regards...
Prafulla . . .

>  #ifndef CONFIG_SPI_CS_IS_VALID
> --
> 1.7.1

[U-Boot] [PATCH v2 2/4] kw_spi: backup and reset the MPP of the chosen CS pin

[Valentin Longchamp]

On 05/31/2012 10:36 AM, Prafulla Wadaskar wrote:
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 30 May 2012 21:12
>> To: Prafulla Wadaskar
>> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
>> boot at lists.denx.de; Prafulla Wadaskar
>> Subject: [PATCH v2 2/4] kw_spi: backup and reset the MPP of the chosen
>> CS pin
>>
>> This was not done before, and in the case of a shared pin (for MPP0
>> between NF_IO[2] and CSn) this could lead to problems.
>>
>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>> cc: Holger Brunck <holger.brunck@keymile.com>
>> cc: Prafulla Wadaskar <prafulla@marvell.com>
>> ---
>>  drivers/spi/kirkwood_spi.c |   14 ++++++--------
>>  1 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
>> index db8ba8b..2fd89d2 100644
>> --- a/drivers/spi/kirkwood_spi.c
>> +++ b/drivers/spi/kirkwood_spi.c
>> @@ -34,16 +34,14 @@
>>
>>  static struct kwspi_registers *spireg = (struct kwspi_registers
>> *)KW_SPI_BASE;
>>
>> +u32 cs_spi_mpp_back[2];
>> +
>>  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>>  				unsigned int max_hz, unsigned int mode)
>>  {
>>  	struct spi_slave *slave;
>>  	u32 data;
>> -	u32 kwspi_mpp_config[] = {
>> -		MPP0_GPIO,
>> -		MPP7_SPI_SCn,
>> -		0
>> -	};
>> +	u32 kwspi_mpp_config[] = { 0, 0 };
>>
>>  	if (!spi_cs_is_valid(bus, cs))
>>  		return NULL;
>> @@ -70,12 +68,11 @@ struct spi_slave *spi_setup_slave(unsigned int
>> bus, unsigned int cs,
>>
>>  	/* program mpp registers to select  SPI_CSn */
>>  	if (cs) {
>> -		kwspi_mpp_config[0] = MPP0_GPIO;
>> -		kwspi_mpp_config[1] = MPP7_SPI_SCn;
>> +		kwspi_mpp_config[0] = MPP7_SPI_SCn;
>>  	} else {
>>  		kwspi_mpp_config[0] = MPP0_SPI_SCn;
>> -		kwspi_mpp_config[1] = MPP7_GPO;
>>  	}
>> +	kirkwood_mpp_read(kwspi_mpp_config, cs_spi_mpp_back);
>>  	kirkwood_mpp_conf(kwspi_mpp_config);
>>
>>  	return slave;
>> @@ -83,6 +80,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>> unsigned int cs,
>>
>>  void spi_free_slave(struct spi_slave *slave)
>>  {
>> +	kirkwood_mpp_conf(cs_spi_mpp_back);
>>  	free(slave);
>>  }
>>
>> --
> 
> Acked-By: Prafulla Wadaskar <prafulla@maravell.com>
> But as you said functionality need to be verified, I hope you have verified it on any of Kirkwood variant.
> 

Sure, it's always been tested successfully on our kirkwood boards.

[U-Boot] [PATCH v2 4/4] kw_spi: add weak functions board_spi_claim/release_bus

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 30 May 2012 21:12
> To: Prafulla Wadaskar
> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
> boot at lists.denx.de; Prafulla Wadaskar
> Subject: [PATCH v2 4/4] kw_spi: add weak functions
> board_spi_claim/release_bus
> 
> This allows a final, board specific, step in the claim/relase_bus
> function for the SPI controller, which may be needed for some hardware
> designs.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  drivers/spi/kirkwood_spi.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index a3e6b6e..a0da527 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -88,6 +88,11 @@ void spi_free_slave(struct spi_slave *slave)
>  u32 spi_mpp_backup[4];
>  #endif
> 
> +__attribute__((weak)) int board_spi_claim_bus(struct spi_slave
> *slave)
> +{
> +	return 0;
> +}
> +
>  int spi_claim_bus(struct spi_slave *slave)
>  {
>  #if defined(CONFIG_SYS_KW_SPI_MPP)
> @@ -121,7 +126,11 @@ int spi_claim_bus(struct spi_slave *slave)
>  	kirkwood_mpp_conf(spi_mpp_config);
>  #endif
> 
> -	return 0;
> +	return board_spi_claim_bus(slave);
> +}
> +
> +__attribute__((weak)) void board_spi_release_bus(struct spi_slave
> *slave)
> +{
>  }
> 
>  void spi_release_bus(struct spi_slave *slave)
> @@ -129,6 +138,8 @@ void spi_release_bus(struct spi_slave *slave)
>  #if defined(CONFIG_SYS_KW_SPI_MPP)
>  	kirkwood_mpp_conf(spi_mpp_backup);
>  #endif
> +
> +	board_spi_release_bus(slave);
>  }
> 
>  #ifndef CONFIG_SPI_CS_IS_VALID
> --
> 1.7.1

Acked-By: Prafulla Wadaskar <Prafulla@marvell.com>

Regards..
Prafulla . . .

[U-Boot] [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 31 May 2012 14:14
> To: Prafulla Wadaskar
> Cc: holger.brunck at keymile.com; u-boot at lists.denx.de
> Subject: Re: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function
> 
> Hi Prafulla,
> 
> On 05/31/2012 10:30 AM, Prafulla Wadaskar wrote:
> >> -----Original Message-----
> >> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> >> Sent: 30 May 2012 21:12
> >> To: Prafulla Wadaskar
> >> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
> >> boot at lists.denx.de; Prafulla Wadaskar
> >> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function
> >>
> >> This function can be used to save current mpp state of all mpp pins
> >> given in the mpp_list argument by reading the mpp registers, in the
> >> second mpp_saved argument.
> >>
> >> A later call to kirkwood_mpp_conf function with this saved list
> will
> >> reset the mpp configuration as it was when kirkwood_mpp_read was
> >> called.
> >>
> >> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> >> cc: Holger Brunck <holger.brunck@keymile.com>
> >> cc: Prafulla Wadaskar <prafulla@marvell.com>
> >> ---
> >>  arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   41
> >> ++++++++++++++++++++++++++++++
> >>  arch/arm/include/asm/arch-kirkwood/mpp.h |    1 +
> >>  2 files changed, 42 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> index 3da6c98..9fb9aea 100644
> >> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
> >> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list)
> >>  	debug("\n");
> >>
> >>  }
> >> +
> >> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved)
> >> +{
> >> +	u32 mpp_ctrl[MPP_NR_REGS];
> >> +	unsigned int variant_mask;
> >> +	int i;
> >> +
> >> +	variant_mask = kirkwood_variant();
> >> +	if (!variant_mask)
> >> +		return;
> >> +
> >> +	for (i = 0; i < MPP_NR_REGS; i++) {
> >> +		mpp_ctrl[i] = readl(MPP_CTRL(i));
> >> +		debug(" %08x", mpp_ctrl[i]);
> >> +	}
> >> +
> >> +	while (*mpp_list) {
> >> +		unsigned int num = MPP_NUM(*mpp_list);
> >> +		unsigned int sel;
> >> +		int shift;
> >> +
> >> +		if (num > MPP_MAX) {
> >> +			debug("kirkwood_mpp_conf: invalid MPP "
> >> +					"number (%u)\n", num);
>> +			continue;
> >> +		}
> >> +		if (!(*mpp_list & variant_mask)) {
> >> +			debug("kirkwood_mpp_conf: requested MPP%u config "
> >> +				"unavailable on this hardware\n", num);
> >> +			continue;
> >> +		}
> >> +
> >> +		shift = (num & 7) << 2;
> >> +		sel = (mpp_ctrl[num / 8] >> shift) & 0xf;
> >> +		*mpp_saved = num | (sel << 8) | variant_mask;
> >> +
> >> +		mpp_list++;
> >> +		mpp_saved++;
> >> +	}
> >> +}
> >> +
> >
> > Hi Valentin
> > There is code duplication, similar code it already there in function
> kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read
> function within kirkwood_mpp_conf
> >
> 
> Not sure I understand what you mean. You want me to implement the
> kirkwood_mpp_read functionnality directly into kirkwood_mpp_conf ?

Yes,

> 
> If this is so, it would mean that I would have to change
> kirkwood_mpp_conf "API"
> to add the second argument (mpp_saved) and then I would have to fix
> all the
> calls to this function. Is that what you mean ?

Yes, my objective here is - how good we can optimise the code.

I will not stretch it further, it's up to you.

Regards..
Prafulla . . .

[U-Boot] [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function

[Valentin Longchamp]

On 05/31/2012 10:49 AM, Prafulla Wadaskar wrote:
> 
> 
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 31 May 2012 14:14
>> To: Prafulla Wadaskar
>> Cc: holger.brunck at keymile.com; u-boot at lists.denx.de
>> Subject: Re: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function
>>
>> Hi Prafulla,
>>
>> On 05/31/2012 10:30 AM, Prafulla Wadaskar wrote:
>>>> -----Original Message-----
>>>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>>>> Sent: 30 May 2012 21:12
>>>> To: Prafulla Wadaskar
>>>> Cc: Valentin Longchamp; holger.brunck at keymile.com; u-
>>>> boot at lists.denx.de; Prafulla Wadaskar
>>>> Subject: [PATCH v2 1/4] kirkwood: add kirkwood_mpp_read function
>>>>
>>>> This function can be used to save current mpp state of all mpp pins
>>>> given in the mpp_list argument by reading the mpp registers, in the
>>>> second mpp_saved argument.
>>>>
>>>> A later call to kirkwood_mpp_conf function with this saved list
>> will
>>>> reset the mpp configuration as it was when kirkwood_mpp_read was
>>>> called.
>>>>
>>>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>>>> cc: Holger Brunck <holger.brunck@keymile.com>
>>>> cc: Prafulla Wadaskar <prafulla@marvell.com>
>>>> ---
>>>>  arch/arm/cpu/arm926ejs/kirkwood/mpp.c    |   41
>>>> ++++++++++++++++++++++++++++++
>>>>  arch/arm/include/asm/arch-kirkwood/mpp.h |    1 +
>>>>  2 files changed, 42 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>>>> b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>>>> index 3da6c98..9fb9aea 100644
>>>> --- a/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>>>> +++ b/arch/arm/cpu/arm926ejs/kirkwood/mpp.c
>>>> @@ -80,3 +80,44 @@ void kirkwood_mpp_conf(u32 *mpp_list)
>>>>  	debug("\n");
>>>>
>>>>  }
>>>> +
>>>> +void kirkwood_mpp_read(u32 *mpp_list, u32 *mpp_saved)
>>>> +{
>>>> +	u32 mpp_ctrl[MPP_NR_REGS];
>>>> +	unsigned int variant_mask;
>>>> +	int i;
>>>> +
>>>> +	variant_mask = kirkwood_variant();
>>>> +	if (!variant_mask)
>>>> +		return;
>>>> +
>>>> +	for (i = 0; i < MPP_NR_REGS; i++) {
>>>> +		mpp_ctrl[i] = readl(MPP_CTRL(i));
>>>> +		debug(" %08x", mpp_ctrl[i]);
>>>> +	}
>>>> +
>>>> +	while (*mpp_list) {
>>>> +		unsigned int num = MPP_NUM(*mpp_list);
>>>> +		unsigned int sel;
>>>> +		int shift;
>>>> +
>>>> +		if (num > MPP_MAX) {
>>>> +			debug("kirkwood_mpp_conf: invalid MPP "
>>>> +					"number (%u)\n", num);
>>> +			continue;
>>>> +		}
>>>> +		if (!(*mpp_list & variant_mask)) {
>>>> +			debug("kirkwood_mpp_conf: requested MPP%u config "
>>>> +				"unavailable on this hardware\n", num);
>>>> +			continue;
>>>> +		}
>>>> +
>>>> +		shift = (num & 7) << 2;
>>>> +		sel = (mpp_ctrl[num / 8] >> shift) & 0xf;
>>>> +		*mpp_saved = num | (sel << 8) | variant_mask;
>>>> +
>>>> +		mpp_list++;
>>>> +		mpp_saved++;
>>>> +	}
>>>> +}
>>>> +
>>>
>>> Hi Valentin
>>> There is code duplication, similar code it already there in function
>> kirkwood_mpp_conf(), to make it short you should use kirkwood_mpp_read
>> function within kirkwood_mpp_conf
>>>
>>
>> Not sure I understand what you mean. You want me to implement the
>> kirkwood_mpp_read functionnality directly into kirkwood_mpp_conf ?
> 
> Yes,
> 
>>
>> If this is so, it would mean that I would have to change
>> kirkwood_mpp_conf "API"
>> to add the second argument (mpp_saved) and then I would have to fix
>> all the
>> calls to this function. Is that what you mean ?
> 
> Yes, my objective here is - how good we can optimise the code.
> 
> I will not stretch it further, it's up to you.
> 

OK, I have done it. Sending v3 of the series in a few minutes.