All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] spi: Switch call order of spi master setup and spi_set_cs
@ 2015-10-12 21:01 ` Franklin S Cooper Jr
  0 siblings, 0 replies; 14+ messages in thread
From: Franklin S Cooper Jr @ 2015-10-12 21:01 UTC (permalink / raw)
  To: linux-kernel, linux-spi, broonie, nsekhar, ssantosh, iivanov,
	m-karicheri2
  Cc: Franklin S Cooper Jr

Keystone 2 devices currently fail to boot in linux-next after the
below commit was applied:

spi: bitbang: switch to the generic implementation of transfer_one_message
commit: 0037686596832572bbca05ab168d9884d7d704c1

This patch allows Keystone 2 devices to boot again in linux-next.

Tested this patch on K2E evm and am437 starterkit which both have SPI
devices to insure regressions aren't seen.


Franklin S Cooper Jr (1):
  spi: Setup the master controller driver before setting the chipselect

 drivers/spi/spi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.6.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC] spi: Switch call order of spi master setup and spi_set_cs
@ 2015-10-12 21:01 ` Franklin S Cooper Jr
  0 siblings, 0 replies; 14+ messages in thread
From: Franklin S Cooper Jr @ 2015-10-12 21:01 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	nsekhar-l0cyMroinI0, ssantosh-DgEjT+Ai2ygdnm+yROfE0A,
	iivanov-NEYub+7Iv8PQT0dZR+AlfA, m-karicheri2-l0cyMroinI0
  Cc: Franklin S Cooper Jr

Keystone 2 devices currently fail to boot in linux-next after the
below commit was applied:

spi: bitbang: switch to the generic implementation of transfer_one_message
commit: 0037686596832572bbca05ab168d9884d7d704c1

This patch allows Keystone 2 devices to boot again in linux-next.

Tested this patch on K2E evm and am437 starterkit which both have SPI
devices to insure regressions aren't seen.


Franklin S Cooper Jr (1):
  spi: Setup the master controller driver before setting the chipselect

 drivers/spi/spi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.6.1

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
  2015-10-12 21:01 ` Franklin S Cooper Jr
  (?)
@ 2015-10-12 21:01 ` Franklin S Cooper Jr
  2015-10-14  9:47     ` Ivan T. Ivanov
       [not found]   ` <1444683671-15570-2-git-send-email-fcooper-l0cyMroinI0@public.gmane.org>
  -1 siblings, 2 replies; 14+ messages in thread
From: Franklin S Cooper Jr @ 2015-10-12 21:01 UTC (permalink / raw)
  To: linux-kernel, linux-spi, broonie, nsekhar, ssantosh, iivanov,
	m-karicheri2
  Cc: Franklin S Cooper Jr

Some devices depend on the master controller driver setup function being
called before calling any chipselect functions.

Insure that this is done otherwise uninitialized structures may be
accessed causing a kernel panic.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 drivers/spi/spi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 38006cc..9374d82 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
 	if (!spi->max_speed_hz)
 		spi->max_speed_hz = spi->master->max_speed_hz;
 
-	spi_set_cs(spi, false);
-
 	if (spi->master->setup)
 		status = spi->master->setup(spi);
 
+	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)),
 			(spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
-- 
2.6.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC] spi: Switch call order of spi master setup and spi_set_cs
@ 2015-10-13 12:10   ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-10-13 12:10 UTC (permalink / raw)
  To: Franklin S Cooper Jr
  Cc: linux-kernel, linux-spi, nsekhar, ssantosh, iivanov, m-karicheri2

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Mon, Oct 12, 2015 at 04:01:10PM -0500, Franklin S Cooper Jr wrote:
> Keystone 2 devices currently fail to boot in linux-next after the
> below commit was applied:

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] spi: Switch call order of spi master setup and spi_set_cs
@ 2015-10-13 12:10   ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-10-13 12:10 UTC (permalink / raw)
  To: Franklin S Cooper Jr
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
	ssantosh-DgEjT+Ai2ygdnm+yROfE0A, iivanov-NEYub+7Iv8PQT0dZR+AlfA,
	m-karicheri2-l0cyMroinI0

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Mon, Oct 12, 2015 at 04:01:10PM -0500, Franklin S Cooper Jr wrote:
> Keystone 2 devices currently fail to boot in linux-next after the
> below commit was applied:

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC] spi: Switch call order of spi master setup and spi_set_cs
  2015-10-13 12:10   ` Mark Brown
  (?)
@ 2015-10-13 14:03   ` Franklin S Cooper Jr.
  -1 siblings, 0 replies; 14+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-13 14:03 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, linux-spi, nsekhar, ssantosh, iivanov, m-karicheri2



On 10/13/2015 07:10 AM, Mark Brown wrote:
> On Mon, Oct 12, 2015 at 04:01:10PM -0500, Franklin S Cooper Jr wrote:
>> Keystone 2 devices currently fail to boot in linux-next after the
>> below commit was applied:
> Please don't send cover letters for single patches, if there is anything
> that needs saying put it in the changelog of the patch or after the ---
> if it's administrative stuff.  This reduces mail volume and ensures that
> any important information is recorded in the changelog rather than being
> lost.
Sorry about that. Will do next time.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
       [not found]   ` <1444683671-15570-2-git-send-email-fcooper-l0cyMroinI0@public.gmane.org>
@ 2015-10-14  9:47     ` Ivan T. Ivanov
  0 siblings, 0 replies; 14+ messages in thread
From: Ivan T. Ivanov @ 2015-10-14  9:47 UTC (permalink / raw)
  To: Franklin S Cooper Jr, Andy Shevchenko
  Cc: linux-kernel, linux-spi, broonie, nsekhar, ssantosh,
	Ivan T. Ivanov, m-karicheri2

Adding Andy.


> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper@ti.com> wrote:
> 
> Some devices depend on the master controller driver setup function being
> called before calling any chipselect functions.
> 
> Insure that this is done otherwise uninitialized structures may be
> accessed causing a kernel panic.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
> drivers/spi/spi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 38006cc..9374d82 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
> 	if (!spi->max_speed_hz)
> 		spi->max_speed_hz = spi->master->max_speed_hz;
> 
> -	spi_set_cs(spi, false);
> -
> 	if (spi->master->setup)
> 		status = spi->master->setup(spi);
> 
> +	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)),
> 			(spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
> -- 
> 2.6.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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-14  9:47     ` Ivan T. Ivanov
  0 siblings, 0 replies; 14+ messages in thread
From: Ivan T. Ivanov @ 2015-10-14  9:47 UTC (permalink / raw)
  To: Franklin S Cooper Jr, Andy Shevchenko
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	nsekhar-l0cyMroinI0, ssantosh-DgEjT+Ai2ygdnm+yROfE0A,
	Ivan T. Ivanov, m-karicheri2-l0cyMroinI0

Adding Andy.


> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org> wrote:
> 
> Some devices depend on the master controller driver setup function being
> called before calling any chipselect functions.
> 
> Insure that this is done otherwise uninitialized structures may be
> accessed causing a kernel panic.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
> ---
> drivers/spi/spi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 38006cc..9374d82 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
> 	if (!spi->max_speed_hz)
> 		spi->max_speed_hz = spi->master->max_speed_hz;
> 
> -	spi_set_cs(spi, false);
> -
> 	if (spi->master->setup)
> 		status = spi->master->setup(spi);
> 
> +	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)),
> 			(spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
> -- 
> 2.6.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-14 11:08       ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2015-10-14 11:08 UTC (permalink / raw)
  To: Ivan T. Ivanov, Jarkko Nikula
  Cc: Franklin S Cooper Jr, linux-kernel, linux-spi, Mark Brown,
	Sekhar Nori, ssantosh, Ivan T. Ivanov, m-karicheri2

+Cc: Jarkko to see from spi-pxa2xx prospective

On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz@gmail.com> wrote:
> Adding Andy.
>
>
>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper@ti.com> wrote:
>>
>> Some devices depend on the master controller driver setup function being
>> called before calling any chipselect functions.
>>
>> Insure that this is done otherwise uninitialized structures may be
>> accessed causing a kernel panic.

As far as I understand my concern should be about spi-dw driver.

So, I have just tested yesterday's linux-next with and without
proposed patch. Works for me:
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>> drivers/spi/spi.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>> index 38006cc..9374d82 100644
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
>>       if (!spi->max_speed_hz)
>>               spi->max_speed_hz = spi->master->max_speed_hz;
>>
>> -     spi_set_cs(spi, false);
>> -
>>       if (spi->master->setup)
>>               status = spi->master->setup(spi);
>>
>> +     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)),
>>                       (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
>> --
>> 2.6.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
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-14 11:08       ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2015-10-14 11:08 UTC (permalink / raw)
  To: Ivan T. Ivanov, Jarkko Nikula
  Cc: Franklin S Cooper Jr, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Sekhar Nori,
	ssantosh-DgEjT+Ai2ygdnm+yROfE0A, Ivan T. Ivanov,
	m-karicheri2-l0cyMroinI0

+Cc: Jarkko to see from spi-pxa2xx prospective

On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Adding Andy.
>
>
>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org> wrote:
>>
>> Some devices depend on the master controller driver setup function being
>> called before calling any chipselect functions.
>>
>> Insure that this is done otherwise uninitialized structures may be
>> accessed causing a kernel panic.

As far as I understand my concern should be about spi-dw driver.

So, I have just tested yesterday's linux-next with and without
proposed patch. Works for me:
Tested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>> ---
>> drivers/spi/spi.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>> index 38006cc..9374d82 100644
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
>>       if (!spi->max_speed_hz)
>>               spi->max_speed_hz = spi->master->max_speed_hz;
>>
>> -     spi_set_cs(spi, false);
>> -
>>       if (spi->master->setup)
>>               status = spi->master->setup(spi);
>>
>> +     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)),
>>                       (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
>> --
>> 2.6.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
  2015-10-14 11:08       ` Andy Shevchenko
  (?)
@ 2015-10-14 11:45       ` Heiner Kallweit
  2015-10-15 20:50           ` Franklin S Cooper Jr.
  -1 siblings, 1 reply; 14+ messages in thread
From: Heiner Kallweit @ 2015-10-14 11:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Ivan T. Ivanov, Jarkko Nikula, Franklin S Cooper Jr,
	linux-kernel, linux-spi, Mark Brown, Sekhar Nori, ssantosh,
	Ivan T. Ivanov, m-karicheri2

On Wed, Oct 14, 2015 at 1:08 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> +Cc: Jarkko to see from spi-pxa2xx prospective
>
> On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz@gmail.com> wrote:
>> Adding Andy.
>>
>>
>>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper@ti.com> wrote:
>>>
>>> Some devices depend on the master controller driver setup function being
>>> called before calling any chipselect functions.
>>>
>>> Insure that this is done otherwise uninitialized structures may be
>>> accessed causing a kernel panic.
>
> As far as I understand my concern should be about spi-dw driver.
>
> So, I have just tested yesterday's linux-next with and without
> proposed patch. Works for me:
> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
>>>
>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>> ---
>>> drivers/spi/spi.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>> index 38006cc..9374d82 100644
>>> --- a/drivers/spi/spi.c
>>> +++ b/drivers/spi/spi.c
>>> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
>>>       if (!spi->max_speed_hz)
>>>               spi->max_speed_hz = spi->master->max_speed_hz;
>>>
>>> -     spi_set_cs(spi, false);
>>> -
>>>       if (spi->master->setup)
>>>               status = spi->master->setup(spi);
>>>
>>> +     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)),
>>>                       (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
>>> --
>>> 2.6.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
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
> --
> 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
The recent change to the bitbang driver leads to the the set_cs hook
of spi_master being set
now for all drivers using the bitbang layer. This hook is called also
from spi_setup and therefore
one possible side effect is issues with bitbang drivers implementing
the chipselect hook of
spi_bitbang with a dependency on the master being set up before.
The proposed patch looks good to me.
There should be no impact on drivers not using bitbang.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-15 20:50           ` Franklin S Cooper Jr.
  0 siblings, 0 replies; 14+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-15 20:50 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Ivan T. Ivanov, Jarkko Nikula, linux-kernel, linux-spi,
	Mark Brown, Sekhar Nori, ssantosh, Ivan T. Ivanov, m-karicheri2,
	Andy Shevchenko




On 10/14/2015 06:45 AM, Heiner Kallweit wrote:
> On Wed, Oct 14, 2015 at 1:08 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> +Cc: Jarkko to see from spi-pxa2xx prospective
>>
>> On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz@gmail.com> wrote:
>>> Adding Andy.
>>>
>>>
>>>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper@ti.com> wrote:
>>>>
>>>> Some devices depend on the master controller driver setup function being
>>>> called before calling any chipselect functions.
>>>>
>>>> Insure that this is done otherwise uninitialized structures may be
>>>> accessed causing a kernel panic.
>> As far as I understand my concern should be about spi-dw driver.
>>
>> So, I have just tested yesterday's linux-next with and without
>> proposed patch. Works for me:
>> Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>> ---
>>>> drivers/spi/spi.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>>> index 38006cc..9374d82 100644
>>>> --- a/drivers/spi/spi.c
>>>> +++ b/drivers/spi/spi.c
>>>> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
>>>>       if (!spi->max_speed_hz)
>>>>               spi->max_speed_hz = spi->master->max_speed_hz;
>>>>
>>>> -     spi_set_cs(spi, false);
>>>> -
>>>>       if (spi->master->setup)
>>>>               status = spi->master->setup(spi);
>>>>
>>>> +     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)),
>>>>                       (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
>>>> --
>>>> 2.6.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
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>> --
>> 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
> The recent change to the bitbang driver leads to the the set_cs hook
> of spi_master being set
> now for all drivers using the bitbang layer. This hook is called also
> from spi_setup and therefore
> one possible side effect is issues with bitbang drivers implementing
> the chipselect hook of
> spi_bitbang with a dependency on the master being set up before.
> The proposed patch looks good to me.
> There should be no impact on drivers not using bitbang.
Thank all. Since nothing obvious seems wrong with this patch I will resend this patch without the RFC.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-15 20:50           ` Franklin S Cooper Jr.
  0 siblings, 0 replies; 14+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-15 20:50 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Ivan T. Ivanov, Jarkko Nikula,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Sekhar Nori,
	ssantosh-DgEjT+Ai2ygdnm+yROfE0A, Ivan T. Ivanov,
	m-karicheri2-l0cyMroinI0, Andy Shevchenko




On 10/14/2015 06:45 AM, Heiner Kallweit wrote:
> On Wed, Oct 14, 2015 at 1:08 PM, Andy Shevchenko
> <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> +Cc: Jarkko to see from spi-pxa2xx prospective
>>
>> On Wed, Oct 14, 2015 at 12:47 PM, Ivan T. Ivanov <iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Adding Andy.
>>>
>>>
>>>> On Oct 13, 2015, at 12:01 AM, Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org> wrote:
>>>>
>>>> Some devices depend on the master controller driver setup function being
>>>> called before calling any chipselect functions.
>>>>
>>>> Insure that this is done otherwise uninitialized structures may be
>>>> accessed causing a kernel panic.
>> As far as I understand my concern should be about spi-dw driver.
>>
>> So, I have just tested yesterday's linux-next with and without
>> proposed patch. Works for me:
>> Tested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>>>> ---
>>>> drivers/spi/spi.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>>> index 38006cc..9374d82 100644
>>>> --- a/drivers/spi/spi.c
>>>> +++ b/drivers/spi/spi.c
>>>> @@ -2053,11 +2053,11 @@ int spi_setup(struct spi_device *spi)
>>>>       if (!spi->max_speed_hz)
>>>>               spi->max_speed_hz = spi->master->max_speed_hz;
>>>>
>>>> -     spi_set_cs(spi, false);
>>>> -
>>>>       if (spi->master->setup)
>>>>               status = spi->master->setup(spi);
>>>>
>>>> +     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)),
>>>>                       (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
>>>> --
>>>> 2.6.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> The recent change to the bitbang driver leads to the the set_cs hook
> of spi_master being set
> now for all drivers using the bitbang layer. This hook is called also
> from spi_setup and therefore
> one possible side effect is issues with bitbang drivers implementing
> the chipselect hook of
> spi_bitbang with a dependency on the master being set up before.
> The proposed patch looks good to me.
> There should be no impact on drivers not using bitbang.
Thank all. Since nothing obvious seems wrong with this patch I will resend this patch without the RFC.


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Applied "spi: Setup the master controller driver before setting the chipselect" to the spi tree
       [not found]   ` <1444683671-15570-2-git-send-email-fcooper-l0cyMroinI0@public.gmane.org>
@ 2015-10-28  0:52     ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-10-28  0:52 UTC (permalink / raw)
  To: Franklin S Cooper Jr, Grygorii Strashko, Ivan Khoronzhuk, Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

The patch

   spi: Setup the master controller driver before setting the chipselect

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From abeedb0159eec42c52a28fc44457164f71aa12a9 Mon Sep 17 00:00:00 2001
From: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
Date: Fri, 16 Oct 2015 10:29:03 -0500
Subject: [PATCH] spi: Setup the master controller driver before setting the
 chipselect

SPI controllers may need to be properly setup before chip selects
can be used. Therefore, wait until the spi controller has a chance
to perform their setup procedure before trying to use the chip
select.

This also insures that the chip selects pins are in a good
state before asseting them which otherwise may cause confusion.

Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
Tested-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
Tested-by: Ivan Khoronzhuk <ivan.khoronzhuk-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 3abb390..7b528b0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1993,11 +1993,11 @@ int spi_setup(struct spi_device *spi)
 	if (!spi->max_speed_hz)
 		spi->max_speed_hz = spi->master->max_speed_hz;
 
-	spi_set_cs(spi, false);
-
 	if (spi->master->setup)
 		status = spi->master->setup(spi);
 
+	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)),
 			(spi->mode & SPI_CS_HIGH) ? "cs_high, " : "",
-- 
2.6.1

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-10-28  0:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12 21:01 [RFC] spi: Switch call order of spi master setup and spi_set_cs Franklin S Cooper Jr
2015-10-12 21:01 ` Franklin S Cooper Jr
2015-10-12 21:01 ` [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect Franklin S Cooper Jr
2015-10-14  9:47   ` Ivan T. Ivanov
2015-10-14  9:47     ` Ivan T. Ivanov
2015-10-14 11:08     ` Andy Shevchenko
2015-10-14 11:08       ` Andy Shevchenko
2015-10-14 11:45       ` Heiner Kallweit
2015-10-15 20:50         ` Franklin S Cooper Jr.
2015-10-15 20:50           ` Franklin S Cooper Jr.
     [not found]   ` <1444683671-15570-2-git-send-email-fcooper-l0cyMroinI0@public.gmane.org>
2015-10-28  0:52     ` Applied "spi: Setup the master controller driver before setting the chipselect" to the spi tree Mark Brown
2015-10-13 12:10 ` [RFC] spi: Switch call order of spi master setup and spi_set_cs Mark Brown
2015-10-13 12:10   ` Mark Brown
2015-10-13 14:03   ` Franklin S Cooper Jr.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.