* [PATCH RFC 0/2] staging: fbtft: fix 9-bit SPI support @ 2015-08-24 18:33 Stefan Wahren 2015-08-24 18:33 ` [PATCH RFC 1/2] staging: fbtft: replace master->setup() with spi_setup() Stefan Wahren 2015-08-24 18:33 ` [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection Stefan Wahren 0 siblings, 2 replies; 6+ messages in thread From: Stefan Wahren @ 2015-08-24 18:33 UTC (permalink / raw) To: Thomas Petazzoni, Noralf Trønnes Cc: Greg Kroah-Hartman, Mark Brown, Henri Chain, devel, linux-kernel, Stefan Wahren This patch series fixes the 9-bit SPI support of fbtft. Stefan Wahren (2): staging: fbtft: replace master->setup() with spi_setup() staging: fbtft: fix 9-bit SPI support detection drivers/staging/fbtft/fb_uc1611.c | 2 +- drivers/staging/fbtft/fb_watterott.c | 4 ++-- drivers/staging/fbtft/fbtft-core.c | 11 +++++++++-- drivers/staging/fbtft/flexfb.c | 11 +++++++++-- 4 files changed, 21 insertions(+), 7 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RFC 1/2] staging: fbtft: replace master->setup() with spi_setup() 2015-08-24 18:33 [PATCH RFC 0/2] staging: fbtft: fix 9-bit SPI support Stefan Wahren @ 2015-08-24 18:33 ` Stefan Wahren 2015-08-24 18:33 ` [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection Stefan Wahren 1 sibling, 0 replies; 6+ messages in thread From: Stefan Wahren @ 2015-08-24 18:33 UTC (permalink / raw) To: Thomas Petazzoni, Noralf Trønnes Cc: Greg Kroah-Hartman, Mark Brown, Henri Chain, devel, linux-kernel, Stefan Wahren Calling the setup of the SPI master directly causes a NULL pointer dereference with master drivers without a separate setup function. This problem is reproduceable on ARM MXS platform. So fix this issue by using spi_setup() instead. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- drivers/staging/fbtft/fb_uc1611.c | 2 +- drivers/staging/fbtft/fb_watterott.c | 4 ++-- drivers/staging/fbtft/fbtft-core.c | 4 ++-- drivers/staging/fbtft/flexfb.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/fbtft/fb_uc1611.c b/drivers/staging/fbtft/fb_uc1611.c index 32f3a9d..5cafa50 100644 --- a/drivers/staging/fbtft/fb_uc1611.c +++ b/drivers/staging/fbtft/fb_uc1611.c @@ -76,7 +76,7 @@ static int init_display(struct fbtft_par *par) /* Set CS active high */ par->spi->mode |= SPI_CS_HIGH; - ret = par->spi->master->setup(par->spi); + ret = spi_setup(par->spi); if (ret) { dev_err(par->info->device, "Could not set SPI_CS_HIGH\n"); return ret; diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c index 88fb2c0..8eae6ef 100644 --- a/drivers/staging/fbtft/fb_watterott.c +++ b/drivers/staging/fbtft/fb_watterott.c @@ -169,7 +169,7 @@ static int init_display(struct fbtft_par *par) /* enable SPI interface by having CS and MOSI low during reset */ save_mode = par->spi->mode; par->spi->mode |= SPI_CS_HIGH; - ret = par->spi->master->setup(par->spi); /* set CS inactive low */ + ret = spi_setup(par->spi); /* set CS inactive low */ if (ret) { dev_err(par->info->device, "Could not set SPI_CS_HIGH\n"); return ret; @@ -180,7 +180,7 @@ static int init_display(struct fbtft_par *par) par->fbtftops.reset(par); mdelay(1000); par->spi->mode = save_mode; - ret = par->spi->master->setup(par->spi); + ret = spi_setup(par->spi); if (ret) { dev_err(par->info->device, "Could not restore SPI mode\n"); return ret; diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 23392eb..3638554 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -1437,12 +1437,12 @@ int fbtft_probe_common(struct fbtft_display *display, /* 9-bit SPI setup */ if (par->spi && display->buswidth == 9) { par->spi->bits_per_word = 9; - ret = par->spi->master->setup(par->spi); + ret = spi_setup(par->spi); if (ret) { dev_warn(&par->spi->dev, "9-bit SPI not available, emulating using 8-bit.\n"); par->spi->bits_per_word = 8; - ret = par->spi->master->setup(par->spi); + ret = spi_setup(par->spi); if (ret) goto out_release; /* allocate buffer with room for dc bits */ diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c index c763efc..09fd15d 100644 --- a/drivers/staging/fbtft/flexfb.c +++ b/drivers/staging/fbtft/flexfb.c @@ -464,12 +464,12 @@ static int flexfb_probe_common(struct spi_device *sdev, par->fbtftops.write_register = fbtft_write_reg8_bus9; par->fbtftops.write_vmem = fbtft_write_vmem16_bus9; sdev->bits_per_word = 9; - ret = sdev->master->setup(sdev); + ret = spi_setup(sdev); if (ret) { dev_warn(dev, "9-bit SPI not available, emulating using 8-bit.\n"); sdev->bits_per_word = 8; - ret = sdev->master->setup(sdev); + ret = spi_setup(sdev); if (ret) goto out_release; /* allocate buffer with room for dc bits */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection 2015-08-24 18:33 [PATCH RFC 0/2] staging: fbtft: fix 9-bit SPI support Stefan Wahren 2015-08-24 18:33 ` [PATCH RFC 1/2] staging: fbtft: replace master->setup() with spi_setup() Stefan Wahren @ 2015-08-24 18:33 ` Stefan Wahren 2015-08-24 22:00 ` Noralf Trønnes 1 sibling, 1 reply; 6+ messages in thread From: Stefan Wahren @ 2015-08-24 18:33 UTC (permalink / raw) To: Thomas Petazzoni, Noralf Trønnes Cc: Greg Kroah-Hartman, Mark Brown, Henri Chain, devel, linux-kernel, Stefan Wahren Since bits_per_word isn't usually checked during SPI setup the 9-bit support must be checked manually. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- drivers/staging/fbtft/fbtft-core.c | 7 +++++++ drivers/staging/fbtft/flexfb.c | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 3638554..bd71487 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display, if (par->spi && display->buswidth == 9) { par->spi->bits_per_word = 9; ret = spi_setup(par->spi); + if (!ret) { + struct spi_master *ma = par->spi->master; + + if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9))) + ret = -EINVAL; + } + if (ret) { dev_warn(&par->spi->dev, "9-bit SPI not available, emulating using 8-bit.\n"); diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c index 09fd15d..a54a8d9 100644 --- a/drivers/staging/fbtft/flexfb.c +++ b/drivers/staging/fbtft/flexfb.c @@ -465,6 +465,13 @@ static int flexfb_probe_common(struct spi_device *sdev, par->fbtftops.write_vmem = fbtft_write_vmem16_bus9; sdev->bits_per_word = 9; ret = spi_setup(sdev); + if (!ret) { + struct spi_master *ma = par->spi->master; + + if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9))) + ret = -EINVAL; + } + if (ret) { dev_warn(dev, "9-bit SPI not available, emulating using 8-bit.\n"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection 2015-08-24 18:33 ` [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection Stefan Wahren @ 2015-08-24 22:00 ` Noralf Trønnes 2015-08-25 17:34 ` Stefan Wahren 0 siblings, 1 reply; 6+ messages in thread From: Noralf Trønnes @ 2015-08-24 22:00 UTC (permalink / raw) To: Stefan Wahren, Thomas Petazzoni Cc: Greg Kroah-Hartman, Mark Brown, Henri Chain, devel, linux-kernel Den 24.08.2015 20:33, skrev Stefan Wahren: > Since bits_per_word isn't usually checked during SPI setup the 9-bit > support must be checked manually. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > drivers/staging/fbtft/fbtft-core.c | 7 +++++++ > drivers/staging/fbtft/flexfb.c | 7 +++++++ > 2 files changed, 14 insertions(+) > > diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c > index 3638554..bd71487 100644 > --- a/drivers/staging/fbtft/fbtft-core.c > +++ b/drivers/staging/fbtft/fbtft-core.c > @@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display, > if (par->spi && display->buswidth == 9) { > par->spi->bits_per_word = 9; > ret = spi_setup(par->spi); > + if (!ret) { > + struct spi_master *ma = par->spi->master; > + > + if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9))) > + ret = -EINVAL; > + } > + > if (ret) { There's no point in calling spi_setup() when it doesn't check bits_per_word. Apparently this changed with the commit: spi: convert drivers to use bits_per_word_mask. This has not been detected earlier, because FBTFT was previously mostly used on the Raspberry Pi which had a downstream SPI driver that did this check. How about this: - par->spi->bits_per_word = 9; - ret = par->spi->master->setup(par->spi); + if (par->spi->master->bits_per_word_mask & SPI_BPW_MASK(9)) { + par->spi->bits_per_word = 9; - if (ret) { + } else { dev_warn(&par->spi->dev, "9-bit SPI not available, emulating using 8-bit.\n"); - par->spi->bits_per_word = 8; - ret = par->spi->master->setup(par->spi); - if (ret) - goto out_release; /* allocate buffer with room for dc bits */ par->extra = devm_kzalloc(par->info->device, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection 2015-08-24 22:00 ` Noralf Trønnes @ 2015-08-25 17:34 ` Stefan Wahren 2015-08-25 18:21 ` Noralf Trønnes 0 siblings, 1 reply; 6+ messages in thread From: Stefan Wahren @ 2015-08-25 17:34 UTC (permalink / raw) To: Thomas Petazzoni, Noralf Trønnes Cc: Greg Kroah-Hartman, linux-kernel, Henri Chain, devel, Mark Brown > Noralf Trønnes <noralf@tronnes.org> hat am 25. August 2015 um 00:00 > geschrieben: > > > > Den 24.08.2015 20:33, skrev Stefan Wahren: > > Since bits_per_word isn't usually checked during SPI setup the 9-bit > > support must be checked manually. > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > drivers/staging/fbtft/fbtft-core.c | 7 +++++++ > > drivers/staging/fbtft/flexfb.c | 7 +++++++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/drivers/staging/fbtft/fbtft-core.c > > b/drivers/staging/fbtft/fbtft-core.c > > index 3638554..bd71487 100644 > > --- a/drivers/staging/fbtft/fbtft-core.c > > +++ b/drivers/staging/fbtft/fbtft-core.c > > @@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display, > > if (par->spi && display->buswidth == 9) { > > par->spi->bits_per_word = 9; > > ret = spi_setup(par->spi); > > + if (!ret) { > > + struct spi_master *ma = par->spi->master; > > + > > + if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9))) > > + ret = -EINVAL; > > + } > > + > > if (ret) { > > There's no point in calling spi_setup() when it doesn't check bits_per_word. If checking of bits_per_word is the only intention of the setup call, then i agree. But i'm not sure it is safe to remove the setup call complete. Couldn't this cause regressions since there is no common spi setup call for all drivers? > Apparently this changed with the commit: > spi: convert drivers to use bits_per_word_mask. > This has not been detected earlier, because FBTFT was previously mostly > used on the Raspberry Pi which had a downstream SPI driver that did this > check. > > How about this: > > - par->spi->bits_per_word = 9; > - ret = par->spi->master->setup(par->spi); > + if (par->spi->master->bits_per_word_mask & > SPI_BPW_MASK(9)) { > + par->spi->bits_per_word = 9; > - if (ret) { > + } else { > dev_warn(&par->spi->dev, > "9-bit SPI not available, emulating > using 8-bit.\n"); > - par->spi->bits_per_word = 8; I think this assignment should stay. > - ret = par->spi->master->setup(par->spi); > - if (ret) > - goto out_release; > /* allocate buffer with room for dc bits */ > par->extra = devm_kzalloc(par->info->device, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection 2015-08-25 17:34 ` Stefan Wahren @ 2015-08-25 18:21 ` Noralf Trønnes 0 siblings, 0 replies; 6+ messages in thread From: Noralf Trønnes @ 2015-08-25 18:21 UTC (permalink / raw) To: Stefan Wahren, Thomas Petazzoni Cc: Greg Kroah-Hartman, linux-kernel, Henri Chain, devel, Mark Brown Den 25.08.2015 19:34, skrev Stefan Wahren: >> Noralf Trønnes <noralf@tronnes.org> hat am 25. August 2015 um 00:00 >> geschrieben: >> >> >> >> Den 24.08.2015 20:33, skrev Stefan Wahren: >>> Since bits_per_word isn't usually checked during SPI setup the 9-bit >>> support must be checked manually. >>> >>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >>> --- >>> drivers/staging/fbtft/fbtft-core.c | 7 +++++++ >>> drivers/staging/fbtft/flexfb.c | 7 +++++++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/drivers/staging/fbtft/fbtft-core.c >>> b/drivers/staging/fbtft/fbtft-core.c >>> index 3638554..bd71487 100644 >>> --- a/drivers/staging/fbtft/fbtft-core.c >>> +++ b/drivers/staging/fbtft/fbtft-core.c >>> @@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display, >>> if (par->spi && display->buswidth == 9) { >>> par->spi->bits_per_word = 9; >>> ret = spi_setup(par->spi); >>> + if (!ret) { >>> + struct spi_master *ma = par->spi->master; >>> + >>> + if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9))) >>> + ret = -EINVAL; >>> + } >>> + >>> if (ret) { >> There's no point in calling spi_setup() when it doesn't check bits_per_word. > If checking of bits_per_word is the only intention of the setup call, then i > agree. > But i'm not sure it is safe to remove the setup call complete. > > Couldn't this cause regressions since there is no common spi setup call for all > drivers? spi_add_device() calls spi_setup() for all spi devices so it's already done. If spi_setup() did something with bits_per_word, then I agree that we should call it in the 9-bit case. But this is not the case, and it's possible to change bits_per_word per transfer later, so I can't see that this is needed. >> Apparently this changed with the commit: >> spi: convert drivers to use bits_per_word_mask. >> This has not been detected earlier, because FBTFT was previously mostly >> used on the Raspberry Pi which had a downstream SPI driver that did this >> check. >> >> How about this: >> >> - par->spi->bits_per_word = 9; >> - ret = par->spi->master->setup(par->spi); >> + if (par->spi->master->bits_per_word_mask & >> SPI_BPW_MASK(9)) { >> + par->spi->bits_per_word = 9; >> - if (ret) { >> + } else { >> dev_warn(&par->spi->dev, >> "9-bit SPI not available, emulating >> using 8-bit.\n"); >> - par->spi->bits_per_word = 8; > I think this assignment should stay. Not needed since we haven't changed the default set by spi_setup(): if (!spi->bits_per_word) spi->bits_per_word = 8; Callchain for DT registered spi devices: spi_register_master->of_register_spi_devices->of_register_spi_device-> spi_add_device->spi_setup ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-25 18:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-24 18:33 [PATCH RFC 0/2] staging: fbtft: fix 9-bit SPI support Stefan Wahren 2015-08-24 18:33 ` [PATCH RFC 1/2] staging: fbtft: replace master->setup() with spi_setup() Stefan Wahren 2015-08-24 18:33 ` [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection Stefan Wahren 2015-08-24 22:00 ` Noralf Trønnes 2015-08-25 17:34 ` Stefan Wahren 2015-08-25 18:21 ` Noralf Trønnes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox