Hi Daniel,
On 30.07.20 07:49, Stefan Roese wrote:
Hi Daniel,
On 24.07.20 15:56, Daniel Schwierzeck wrote:
Am Donnerstag, den 23.07.2020, 12:17 +0200 schrieb Stefan Roese:
From: Suneel Garapati sgarapati@marvell.com
Adds support for SPI controllers found on Octeon II/III and Octeon TX TX2 SoC platforms.
Signed-off-by: Aaron Williams awilliams@marvell.com Signed-off-by: Suneel Garapati sgarapati@marvell.com Signed-off-by: Stefan Roese sr@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Aaron Williams awilliams@marvell.com Cc: Chandrakala Chavva cchavva@marvell.com Cc: Jagan Teki jagan@amarulasolutions.com
<snip>
+static const struct dm_spi_ops octeon_spi_ops = { + .claim_bus = octeon_spi_claim_bus, + .release_bus = octeon_spi_release_bus, + .set_speed = octeon_spi_set_speed, + .set_mode = octeon_spi_set_mode, + .xfer = octeon_spi_xfer, +};
+static const struct dm_spi_ops octeontx2_spi_ops = { + .claim_bus = octeon_spi_claim_bus, + .release_bus = octeon_spi_release_bus, + .set_speed = octeon_spi_set_speed, + .set_mode = octeon_spi_set_mode, + .xfer = octeontx2_spi_xfer, + .mem_ops = &octeontx2_spi_mem_ops, +};
+static int octeon_spi_probe(struct udevice *dev) +{ + struct octeon_spi *priv = dev_get_priv(dev); + const struct octeon_spi_data *data; + int ret;
+ data = (const struct octeon_spi_data *)dev_get_driver_data(dev); + if (data->probe == PROBE_PCI) { + pci_dev_t bdf = dm_pci_get_bdf(dev);
+ debug("SPI PCI device: %x\n", bdf); + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + PCI_REGION_MEM); + } else { + priv->base = dev_remap_addr(dev); + }
+ priv->base += data->reg_offs;
+ /* Octeon TX2 needs a different ops struct */ + if (device_is_compatible(dev, "cavium,thunderx-spi")) { + /* + * "ops" is const and can't be written directly. So we need + * to write the Octeon TX2 ops value using this trick + */ + writeq((u64)&octeontx2_spi_ops, (void *)&dev->driver->ops); + }
can't you simply add a xfer() function pointer to "struct octeon_spi_data" and assign the according xfer function to it? Then in octeon_spi_xfer() you can simply call that function pointer. With this you only need one instance of "struct dm_spi_ops" and don't need this ugly hack ;)
Unfortuantely its not that easy, as the Octeon TX2 ops struct also has a " mem_ops" member, which the driver does not support for the other Octeon models. I could clear this "mem_ops" member in the non Octeon TX2 case, which is a bit better than the 2nd ops struct. But its still not really elegent.
Or do you have some other idea on how to implement this in a "better way"?
BTW: If this SPI driver is the only patch in this series, that you feel is not ready, then I suggest to drop this one from this patch series and push the remaining ones to mainline (if they have no issues of course).
Thanks, Stefan