| Message ID | 1466150245-2648-2-git-send-email-andi.shyti@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 17, 2016 at 04:57:21PM +0900, Andi Shyti wrote: > Some SPI connected devices do not have any CS line connected as > some devices are alway enabled. Indeed, until now, a common > workaround was to assign to num_chipselect a -1 value or 255 > (num_chipselect is unsigned). > > In this case do not fail and defer to the SPI device drivers the > responsibility to check whether the num-cs is '0'. A SPI controller always has one chip seelct, it may not be controllable but it's at least logically present.
Hi Mark, > > Some SPI connected devices do not have any CS line connected as > > some devices are alway enabled. Indeed, until now, a common > > workaround was to assign to num_chipselect a -1 value or 255 > > (num_chipselect is unsigned). > > > > In this case do not fail and defer to the SPI device drivers the > > responsibility to check whether the num-cs is '0'. > > A SPI controller always has one chip seelct, it may not be controllable > but it's at least logically present. This is true, but there are cases where the CS is not connected and this case needs to be treated separately to allow the device to work. This is the case of: ./drivers/spi/spi-mpc52xx-psc.c:391: master->num_chipselect = 255; ./drivers/spi/spi-oc-tiny.c:256: master->num_chipselect = 255; and ./drivers/spi/spi-pxa2xx-pci.c:64: .num_chipselect = -1, ./drivers/spi/spi-bcm2835aux.c:436: master->num_chipselect = -1; that in my opinion make even less sense. The latter is completely broken, as num_chipselect is an u16 variable. Because of this I had to do the same in a driver that I will send next (and perhaps it will be rejected). So, IMHO, it's better to not be that strong on num_chipselect being non zero, as there are cases where the CS _line_ is not connected. Andi
On Fri, Jun 17, 2016 at 08:36:22PM +0900, Andi Shyti wrote: > > > In this case do not fail and defer to the SPI device drivers the > > > responsibility to check whether the num-cs is '0'. > > A SPI controller always has one chip seelct, it may not be controllable > > but it's at least logically present. > This is true, but there are cases where the CS is not connected > and this case needs to be treated separately to allow the device > to work. In what way? It is just as easy for a device with no physical chip select to have a logical chip select of 0 that it does nothing with as it is for that device to handle any other number. > This is the case of: > ./drivers/spi/spi-mpc52xx-psc.c:391: master->num_chipselect = 255; > ./drivers/spi/spi-oc-tiny.c:256: master->num_chipselect = 255; > ./drivers/spi/spi-pxa2xx-pci.c:64: .num_chipselect = -1, > ./drivers/spi/spi-bcm2835aux.c:436: master->num_chipselect = -1; These need fixing.
Hi Mark, > > > > In this case do not fail and defer to the SPI device drivers the > > > > responsibility to check whether the num-cs is '0'. > > > > A SPI controller always has one chip seelct, it may not be controllable > > > but it's at least logically present. > > > This is true, but there are cases where the CS is not connected > > and this case needs to be treated separately to allow the device > > to work. > > In what way? It is just as easy for a device with no physical chip > select to have a logical chip select of 0 that it does nothing with as > it is for that device to handle any other number. That is indeed my case: the s3c64xx doesn't send anything, unless I manually enable CS (from the next patches I need to write '0' in the CS register). But I need smoething to tell to the device that the CS line is not connected, for example a flag in the DTS. It comes natural to me to set "num-cs = <0>" instead of defining a new property. In this case I will consider that there is no CS line, even though there is a CS controller. Thanks, Andi
On Sun, Jun 19, 2016 at 03:09:02PM +0900, Andi Shyti wrote: > > > This is true, but there are cases where the CS is not connected > > > and this case needs to be treated separately to allow the device > > > to work. > > In what way? It is just as easy for a device with no physical chip > > select to have a logical chip select of 0 that it does nothing with as > > it is for that device to handle any other number. > That is indeed my case: the s3c64xx doesn't send anything, unless > I manually enable CS (from the next patches I need to write '0' > in the CS register). But I need smoething to tell to the > device that the CS line is not connected, for example a flag in > the DTS. I'm sorry but I just don't understand what you're saying here. You are saying that there is no chip select but you need to enable the chip select?
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 77e6e45..f22dc27 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -504,8 +504,13 @@ int spi_add_device(struct spi_device *spi) struct device *dev = master->dev.parent; int status; - /* Chipselects are numbered 0..max; validate. */ - if (spi->chip_select >= master->num_chipselect) { + /* + * Chipselects are numbered 0..max; validate. + * If there is no chip select (i.e. num_chipselect == 0), + * the comparison doesn't make sense. + */ + if (master->num_chipselect && + spi->chip_select >= master->num_chipselect) { dev_err(dev, "cs%d >= max %d\n", spi->chip_select, master->num_chipselect); @@ -1851,12 +1856,6 @@ int spi_register_master(struct spi_master *master) if (status) return status; - /* even if it's just one always-selected device, there must - * be at least one chipselect - */ - if (master->num_chipselect == 0) - return -EINVAL; - if ((master->bus_num < 0) && master->dev.of_node) master->bus_num = of_alias_get_id(master->dev.of_node, "spi");
Some SPI connected devices do not have any CS line connected as some devices are alway enabled. Indeed, until now, a common workaround was to assign to num_chipselect a -1 value or 255 (num_chipselect is unsigned). In this case do not fail and defer to the SPI device drivers the responsibility to check whether the num-cs is '0'. Signed-off-by: Andi Shyti <andi.shyti@samsung.com> --- drivers/spi/spi.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)