| Message ID | 1385921962-19843-3-git-send-email-takasi-y@ops.dti.ne.jp (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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
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 --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)) {