Messages in this thread | | | Subject | Re: [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection | From | Noralf Trønnes <> | Date | Tue, 25 Aug 2015 20:21:29 +0200 |
| |
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
| |