diff mbox

[RFC] spi: Ensure that CS line is in non-active state after setup

Message ID 1425656716-25363-1-git-send-email-iivanov@mm-sol.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ivan T. Ivanov March 6, 2015, 3:45 p.m. UTC
When drivers didn't provide setup() method, SPI core should ensure
that CS line is driven in non-active state after spi_setup().

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
---
 drivers/spi/spi.c | 2 ++
 1 file changed, 2 insertions(+)

--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mark Brown March 8, 2015, 8:01 p.m. UTC | #1
On Fri, Mar 06, 2015 at 05:45:15PM +0200, Ivan T. Ivanov wrote:

>  	if (spi->master->setup)
>  		status = spi->master->setup(spi);
> +	else
> +		spi_set_cs(spi, false);

Why would this be conditional on there not being a setup operation?
It's a good idea, I just don't see why the backstop isn't always useful.
Ivan T. Ivanov March 9, 2015, 7:11 a.m. UTC | #2
On Sun, 2015-03-08 at 20:01 +0000, Mark Brown wrote:
> On Fri, Mar 06, 2015 at 05:45:15PM +0200, Ivan T. Ivanov wrote:
> 
> >         if (spi->master->setup)
> >                 status = spi->master->setup(spi);
> > +       else
> > +               spi_set_cs(spi, false);
> 
> Why would this be conditional on there not being a setup operation?
> It's a good idea, I just don't see why the backstop isn't always useful.

I was thinking that this is expected to be handled inside
setup operation, nothing more.

Ivan 
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko March 9, 2015, 12:11 p.m. UTC | #3
On Mon, Mar 9, 2015 at 9:11 AM, Ivan T. Ivanov <iivanov@mm-sol.com> wrote:
>
> On Sun, 2015-03-08 at 20:01 +0000, Mark Brown wrote:
>> On Fri, Mar 06, 2015 at 05:45:15PM +0200, Ivan T. Ivanov wrote:
>>
>> >         if (spi->master->setup)
>> >                 status = spi->master->setup(spi);
>> > +       else
>> > +               spi_set_cs(spi, false);
>>
>> Why would this be conditional on there not being a setup operation?
>> It's a good idea, I just don't see why the backstop isn't always useful.
>
> I was thinking that this is expected to be handled inside
> setup operation, nothing more.

Agree with Mark. It's perhaps a good idea to call unconditionally
before ->setup().
diff mbox

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c64a3e5..bfba2d9 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1895,6 +1895,8 @@  int spi_setup(struct spi_device *spi)

 	if (spi->master->setup)
 		status = spi->master->setup(spi);
+	else
+		spi_set_cs(spi, false);

 	dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%u bits/w, %u Hz max --> %d\n",
 			(int) (spi->mode & (SPI_CPOL | SPI_CPHA)),