| Message ID | 20160627105716.GA7240@samsunx.samsung (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 27, 2016 at 07:57:16PM +0900, Andi Shyti wrote: > What I meant is that if we do not like num-cs = <0>, the > unlinked CS line can be handled only this way (case of the > s3c64xx driver): > +- broken-cs: the CS line is disconnected, therefore the device should not wait > + for the CS protocol to be established So what you're saying here is that you just need a property for the inability to read back the chip select status? That seems like a totally reasonable thing to have which fits in idiomatically with the rest of the subsystem. I might call it no-cs-readback or something. > Which is not something I like, because it means adding a new > flag in the dts. > What I want to suggest, instead, is to slightly change the logic > behind the num-cs property: i.e. if "num-cs = <0>", doesn't > necessarily mean that we don't have a CS controller, but, while > we can have as many as we wish, non of them is connected. I disagree, I think from a system integration point of view this is just a chip select which can't be changed and it's less likely that we will run into nasty surprises later on with things assuming that chip selects exist. AFAICT you only need this property in your case because this controller has some features that rely on readback of the chip select status, that's not very common - normally it'd be write only. I'd expect most controllers would just say they have one chip select and that'd be that.
Hi Mark, > > What I meant is that if we do not like num-cs = <0>, the > > unlinked CS line can be handled only this way (case of the > > s3c64xx driver): > > > +- broken-cs: the CS line is disconnected, therefore the device should not wait > > + for the CS protocol to be established > > So what you're saying here is that you just need a property for the > inability to read back the chip select status? That seems like a > totally reasonable thing to have which fits in idiomatically with the > rest of the subsystem. I might call it no-cs-readback or something. > > > Which is not something I like, because it means adding a new > > flag in the dts. > > > What I want to suggest, instead, is to slightly change the logic > > behind the num-cs property: i.e. if "num-cs = <0>", doesn't > > necessarily mean that we don't have a CS controller, but, while > > we can have as many as we wish, non of them is connected. > > I disagree, I think from a system integration point of view this is just > a chip select which can't be changed and it's less likely that we will > run into nasty surprises later on with things assuming that chip selects > exist. AFAICT you only need this property in your case because this > controller has some features that rely on readback of the chip select > status, that's not very common - normally it'd be write only. I'd > expect most controllers would just say they have one chip select and > that'd be that. thanks for your feedback, I will then do as you say, I will use the no-cs-readback flag. Thanks again, Andi -- 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/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt index 6dbdeb3..62f19b6 100644 --- a/Documentation/devicetree/bindings/spi/spi-samsung.txt +++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt @@ -40,6 +40,9 @@ Optional Board Specific Properties: - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt) +- broken-cs: the CS line is disconnected, therefore the device should not wait + for the CS protocol to be established + SPI Controller specific data in SPI slave nodes: - The spi slave nodes should provide the following information which is required diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 972367d..5e6ad59 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable) struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(spi->master); + if (sdd->cntrlr_info->broken_cs) + return; + if (enable) { if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) { writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL); [ ... ] @@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev) sci->num_cs = temp; } + sci->broken_cs = of_property_read_bool(dev->of_node, "broken-cs"); + return sci; } #else diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h index fb5625b..75793b9 100644 --- a/include/linux/platform_data/spi-s3c64xx.h +++ b/include/linux/platform_data/spi-s3c64xx.h @@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo { struct s3c64xx_spi_info { int src_clk_nr; int num_cs; + bool broken_cs; int (*cfg_gpio)(void); dma_filter_fn filter; void *dma_tx;