| Message ID | 1444683671-15570-2-git-send-email-fcooper@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | abeedb0159eec42c52a28fc44457164f71aa12a9 |
| Headers | show |
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-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+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 >
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-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, " : "",
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(-)