Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1245136 > unrolled thread

[RFC] spi: Switch call order of spi master setup and spi_set_cs

Back to article view | Back to linux.kernel

Contents

  [RFC] spi: Switch call order of spi master setup and spi_set_cs Franklin S Cooper Jr <fcooper@ti.com> - 2015-10-12 23:10 +0200
    [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect Franklin S Cooper Jr <fcooper@ti.com> - 2015-10-12 23:10 +0200
      Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect "Ivan T. Ivanov" <iivanov.xz@gmail.com> - 2015-10-14 11:50 +0200
        Re: [RFC][PATCH] spi: Setup the master controller driver before  setting the chipselect Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-10-14 13:10 +0200
          Re: [RFC][PATCH] spi: Setup the master controller driver before  setting the chipselect Heiner Kallweit <hkallweit1@gmail.com> - 2015-10-14 13:50 +0200
            Re: [RFC][PATCH] spi: Setup the master controller driver before  setting the chipselect "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-15 23:00 +0200
    Re: [RFC] spi: Switch call order of spi master setup and spi_set_cs Mark Brown <broonie@kernel.org> - 2015-10-13 14:20 +0200
      Re: [RFC] spi: Switch call order of spi master setup and spi_set_cs "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-13 16:10 +0200

#1245136 — [RFC] spi: Switch call order of spi master setup and spi_set_cs

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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1245141 — [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect

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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246473 — Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect

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

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

[toc] | [prev] | [next] | [standalone]

#1246521 — Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect

+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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246577 — Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248176 — Re: [RFC][PATCH] spi: Setup the master controller driver before setting the chipselect

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.


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

[toc] | [prev] | [next] | [standalone]

#1245666

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

#1245763

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web