* [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-15 20:51 ` Franklin S Cooper Jr
0 siblings, 0 replies; 10+ messages in thread
From: Franklin S Cooper Jr @ 2015-10-15 20:51 UTC (permalink / raw
To: linux-kernel, linux-spi, broonie, nsekhar, ssantosh, iivanov,
m-karicheri2, iivanov.xz, andy.shevchenko, jarkko.nikula,
hkallweit1
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.
Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
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.
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 4c638f3..9d5525a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2059,11 +2059,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] 10+ messages in thread
* [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-15 20:51 ` Franklin S Cooper Jr
0 siblings, 0 replies; 10+ messages in thread
From: Franklin S Cooper Jr @ 2015-10-15 20:51 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,
iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
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.
Tested-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
---
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.
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 4c638f3..9d5525a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2059,11 +2059,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] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 10:28 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-10-16 10:28 UTC (permalink / raw
To: Franklin S Cooper Jr
Cc: linux-kernel, linux-spi, nsekhar, ssantosh, iivanov, m-karicheri2,
iivanov.xz, andy.shevchenko, jarkko.nikula, hkallweit1
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On Thu, Oct 15, 2015 at 03:51:45PM -0500, Franklin S Cooper Jr 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.
I think this is a sensible change but can we please have a better
changelog - why is this the best thing to do rather than fixing the
driver to not crash? There are a bunch of good reasons to do the master
setup before asserting chip select but a driver crashing isn't one of
them, it's not clear that this isn't a bug in the driver.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 10:28 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-10-16 10:28 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, iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On Thu, Oct 15, 2015 at 03:51:45PM -0500, Franklin S Cooper Jr 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.
I think this is a sensible change but can we please have a better
changelog - why is this the best thing to do rather than fixing the
driver to not crash? There are a bunch of good reasons to do the master
setup before asserting chip select but a driver crashing isn't one of
them, it's not clear that this isn't a bug in the driver.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 14:27 ` Franklin S Cooper Jr.
0 siblings, 0 replies; 10+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-16 14:27 UTC (permalink / raw
To: Mark Brown
Cc: linux-kernel, linux-spi, nsekhar, ssantosh, iivanov, m-karicheri2,
iivanov.xz, andy.shevchenko, jarkko.nikula, hkallweit1
On 10/16/2015 05:28 AM, Mark Brown wrote:
> On Thu, Oct 15, 2015 at 03:51:45PM -0500, Franklin S Cooper Jr 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.
> I think this is a sensible change but can we please have a better
> changelog - why is this the best thing to do rather than fixing the
> driver to not crash? There are a bunch of good reasons to do the master
> setup before asserting chip select but a driver crashing isn't one of
> them, it's not clear that this isn't a bug in the driver.
Mark,
Makes sense. How about something like this?
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 14:27 ` Franklin S Cooper Jr.
0 siblings, 0 replies; 10+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-16 14:27 UTC (permalink / raw
To: Mark Brown
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
ssantosh-DgEjT+Ai2ygdnm+yROfE0A, iivanov-NEYub+7Iv8PQT0dZR+AlfA,
m-karicheri2-l0cyMroinI0, iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
On 10/16/2015 05:28 AM, Mark Brown wrote:
> On Thu, Oct 15, 2015 at 03:51:45PM -0500, Franklin S Cooper Jr 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.
> I think this is a sensible change but can we please have a better
> changelog - why is this the best thing to do rather than fixing the
> driver to not crash? There are a bunch of good reasons to do the master
> setup before asserting chip select but a driver crashing isn't one of
> them, it's not clear that this isn't a bug in the driver.
Mark,
Makes sense. How about something like this?
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.
--
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] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 14:43 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-10-16 14:43 UTC (permalink / raw
To: Franklin S Cooper Jr.
Cc: linux-kernel, linux-spi, nsekhar, ssantosh, iivanov, m-karicheri2,
iivanov.xz, andy.shevchenko, jarkko.nikula, hkallweit1
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
On Fri, Oct 16, 2015 at 09:27:50AM -0500, Franklin S Cooper Jr. wrote:
> Makes sense. How about something like this?
> 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.
Yes, that's a lot better - there's also the fact that if we assert chip
select before we've got the pins in a good state we may cause the device
to see signals that confuse it as we change.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 14:43 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-10-16 14:43 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, iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
On Fri, Oct 16, 2015 at 09:27:50AM -0500, Franklin S Cooper Jr. wrote:
> Makes sense. How about something like this?
> 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.
Yes, that's a lot better - there's also the fact that if we assert chip
select before we've got the pins in a good state we may cause the device
to see signals that confuse it as we change.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 14:45 ` Franklin S Cooper Jr.
0 siblings, 0 replies; 10+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-16 14:45 UTC (permalink / raw
To: Mark Brown
Cc: linux-kernel, linux-spi, nsekhar, ssantosh, iivanov, m-karicheri2,
iivanov.xz, andy.shevchenko, jarkko.nikula, hkallweit1
On 10/16/2015 09:43 AM, Mark Brown wrote:
> On Fri, Oct 16, 2015 at 09:27:50AM -0500, Franklin S Cooper Jr. wrote:
>
>> Makes sense. How about something like this?
>> 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.
> Yes, that's a lot better - there's also the fact that if we assert chip
> select before we've got the pins in a good state we may cause the device
> to see signals that confuse it as we change.
Ok. I'll submit a rev2 with my updated commit message and incorporate
your suggestion also.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: Setup the master controller driver before setting the chipselect
@ 2015-10-16 14:45 ` Franklin S Cooper Jr.
0 siblings, 0 replies; 10+ messages in thread
From: Franklin S Cooper Jr. @ 2015-10-16 14:45 UTC (permalink / raw
To: Mark Brown
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
ssantosh-DgEjT+Ai2ygdnm+yROfE0A, iivanov-NEYub+7Iv8PQT0dZR+AlfA,
m-karicheri2-l0cyMroinI0, iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
On 10/16/2015 09:43 AM, Mark Brown wrote:
> On Fri, Oct 16, 2015 at 09:27:50AM -0500, Franklin S Cooper Jr. wrote:
>
>> Makes sense. How about something like this?
>> 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.
> Yes, that's a lot better - there's also the fact that if we assert chip
> select before we've got the pins in a good state we may cause the device
> to see signals that confuse it as we change.
Ok. I'll submit a rev2 with my updated commit message and incorporate
your suggestion also.
--
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] 10+ messages in thread
end of thread, other threads:[~2015-10-16 14:46 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-15 20:51 [PATCH] spi: Setup the master controller driver before setting the chipselect Franklin S Cooper Jr
2015-10-15 20:51 ` Franklin S Cooper Jr
2015-10-16 10:28 ` Mark Brown
2015-10-16 10:28 ` Mark Brown
2015-10-16 14:27 ` Franklin S Cooper Jr.
2015-10-16 14:27 ` Franklin S Cooper Jr.
2015-10-16 14:43 ` Mark Brown
2015-10-16 14:43 ` Mark Brown
2015-10-16 14:45 ` Franklin S Cooper Jr.
2015-10-16 14:45 ` 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.