| Message ID | 20240125145007.748295-24-tudor.ambarus@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/28] spi: s3c64xx: explicitly include <linux/io.h> | expand |
On Thu, Jan 25, 2024 at 11:33 AM Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jan 25, 2024 at 02:50:01PM +0000, Tudor Ambarus wrote: > > > Allow SoCs that have multiple instances of the SPI IP with different > > FIFO sizes to specify their FIFO size via the "samsung,spi-fifosize" > > device tree property. With this we can break the dependency between the > > SPI alias, the fifo_lvl_mask and the FIFO size. > > OK, so we do actually have SoCs with multiple instances of the IP with > different FIFO depths (and who knows what else other differences)? I think that's why we can see .fifo_lvl_mask[] with different values for different IP instances. For example, ExynosAutoV9 has this (in upstream driver, yes): .fifo_lvl_mask = { 0x1ff, 0x1ff, 0x7f, 0x7f, 0x7f, 0x7f, 0x1ff, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f}, And I'm pretty sure the comment (in struct s3c64xx_spi_port_config) for .fifo_lvl_mask field is not correct anymore: * @fifo_lvl_mask: Bit-mask for {TX|RX}_FIFO_LVL bits in SPI_STATUS register. Maybe it used to indicate the bit number in SPI_STATUS register for {TX|RX}_FIFO_LVL fields, but not anymore. Nowadays it looks like .fifo_lvl_mask just specifies FIFO depth for each IP instance.
On Fri, Jan 26, 2024 at 09:16:53PM +0100, Arnd Bergmann wrote: > That sounds like the same bug as in the serial port driver, > by assuming that the alias values in the devicetree have > a particular meaning in identifying instances. This immediately > breaks when there is a dtb file that does not use the same > alias values, e.g. because it only needs some of the SPI > ports. It'll be the result of a conversion from board files where that was a normal way of doing things.
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 7a99f6b02319..3e7797d915c5 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -1114,7 +1114,7 @@ static int s3c64xx_spi_get_fifosize(const struct platform_device *pdev, const struct s3c64xx_spi_port_config *port = sdd->port_conf; const int *fifo_lvl_mask = port->fifo_lvl_mask; struct device_node *np = pdev->dev.of_node; - int id; + int id, ret; if (!np) { if (pdev->id < 0) @@ -1130,6 +1130,10 @@ static int s3c64xx_spi_get_fifosize(const struct platform_device *pdev, return 0; } + ret = of_property_read_u32(np, "samsung,spi-fifosize", &sdd->fifosize); + if (ret == 0) + return 0; + id = of_alias_get_id(np, "spi"); if (id < 0) return dev_err_probe(&pdev->dev, id,
Allow SoCs that have multiple instances of the SPI IP with different FIFO sizes to specify their FIFO size via the "samsung,spi-fifosize" device tree property. With this we can break the dependency between the SPI alias, the fifo_lvl_mask and the FIFO size. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- drivers/spi/spi-s3c64xx.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)