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 <[email protected]>

Adds support for SPI controllers found on Octeon II/III and Octeon TX
TX2 SoC platforms.

Signed-off-by: Aaron Williams <[email protected]>
Signed-off-by: Suneel Garapati <[email protected]>
Signed-off-by: Stefan Roese <[email protected]>
Cc: Daniel Schwierzeck <[email protected]>
Cc: Aaron Williams <[email protected]>
Cc: Chandrakala Chavva <[email protected]>
Cc: Jagan Teki <[email protected]>

<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"?

Thanks,
Stefan

Reply via email to