diff mbox

[03/11] spi: spi-sh-msiof: let it run even without GPIO

Message ID 1385921962-19843-3-git-send-email-takasi-y@ops.dti.ne.jp
State Changes Requested
Headers show

Commit Message

takasi-y@ops.dti.ne.jp Dec. 1, 2013, 6:19 p.m. UTC
From: Takashi Yoshii <takasi-y@ops.dti.ne.jp>

In current implementation, CS is controlled by GPIO, which is passed
through spi->controller_data. But because sh-msiof HW module has a
function to output CS by itself, which is already enabled and actual
switch will be done by pinmux, we can silently ignore if GPIO is null.

Signed-off-by: Takashi Yoshii <takasi-y@ops.dti.ne.jp>
---
 drivers/spi/spi-sh-msiof.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Sergei Shtylyov Dec. 2, 2013, 10:59 a.m. UTC | #1
Hello.

On 01-12-2013 22:19, takasi-y@ops.dti.ne.jp wrote:

> From: Takashi Yoshii <takasi-y@ops.dti.ne.jp>

> In current implementation, CS is controlled by GPIO, which is passed
> through spi->controller_data. But because sh-msiof HW module has a
> function to output CS by itself, which is already enabled and actual
> switch will be done by pinmux, we can silently ignore if GPIO is null.

> Signed-off-by: Takashi Yoshii <takasi-y@ops.dti.ne.jp>
> ---
>   drivers/spi/spi-sh-msiof.c |    6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)

> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index ac8795f..2a2b0dd 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -420,8 +420,10 @@ static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
>   					  !!(spi->mode & SPI_LSB_FIRST));
>   	}
>
> -	/* use spi->controller data for CS (same strategy as spi_gpio) */
> -	gpio_set_value((unsigned)spi->controller_data, value);
> +	/* use spi->controller data for CS (same strategy as spi_gpio),
> +	 * if any. otherwise let HW controll CS */

    The preferred style of multi-line comments is this:

/*
  * bla
  * bla
  */

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Dec. 2, 2013, 12:53 p.m. UTC | #2
On Mon, Dec 02, 2013 at 03:19:14AM +0900, takasi-y@ops.dti.ne.jp wrote:

> -	/* use spi->controller data for CS (same strategy as spi_gpio) */
> -	gpio_set_value((unsigned)spi->controller_data, value);
> +	/* use spi->controller data for CS (same strategy as spi_gpio),
> +	 * if any. otherwise let HW controll CS */
> +	if (spi->controller_data)
> +		gpio_set_value((unsigned)spi->controller_data, value);

This makes sense in terms of what it tries to do however the standard
thing is to set the GPIO value to an error value if it isn't there
(especially since 0 is in theory a valid GPIO) so the test should be
different.

I'd also suggest changing to use the core cs_gpio field but that's a
separate thing.
diff mbox

Patch

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index ac8795f..2a2b0dd 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -420,8 +420,10 @@  static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
 					  !!(spi->mode & SPI_LSB_FIRST));
 	}
 
-	/* use spi->controller data for CS (same strategy as spi_gpio) */
-	gpio_set_value((unsigned)spi->controller_data, value);
+	/* use spi->controller data for CS (same strategy as spi_gpio),
+	 * if any. otherwise let HW controll CS */
+	if (spi->controller_data)
+		gpio_set_value((unsigned)spi->controller_data, value);
 
 	if (is_on == BITBANG_CS_INACTIVE) {
 		if (test_and_clear_bit(0, &p->flags)) {