SPI driver probe problem during boot

Hi,

I've just finished debugging a SPI comms problem for a TPM device on a
TI Sitara AM4372 SoC.  The SPI bus has 3 devices

1) CAN FD Controller 0 (Using native CS)
2) CAN FD Controller 1 (Using GPIO for CS)
3) TPM Device (Using GPIO for CS)

All CS are active low.

During boot the TPM would fail it's probe but if I unbind and then
rebind the driver after boot the TPM would work fine so I got the
logic analyser out to probe the SPI bus and could see that the voltage
on the MISO line was half the expected voltage whilst the TPM was
being probed and this was due to CS0 being driven low for this period
of time.  After the TPM was probed the CS0 went high.  After debugging
this I found that the OMAP SPI controller defaults internal CS lines
to active high so when the SPI master controller is initialised this
is the state of CS0 so it's driven low for inactive.  During the probe
of the SPI master controller it calls devm_spi_register_master ->
spi_register_master which calls of_register_spi_devices which will add
the devices to the SPI master controller via spi_add_device.  This
function would call device_add. Due to the way the device tree is
parsed the SPI devices above are enumerated from the highest CS to the
lowest so the TPM device is setup first.  When device_add is called it
triggers it's probe function but the SPI bus hasn't been setup yet and
hence the TPM driver tries to communicate with the TPM device whilst
CS0 is being driven low causing the CAN FD controller to also respond
reducing the voltage on the MISO line.  In spi_device_add it calls

status = spi_setup(spi);

which would setup the CS lines so I modified the
of_register_spi_devices function to make it a 2 phase operation so the
CS lines are all setup and then it iterates of the SPI devices a
second time to add them to the driver model via device_add and the TPM
now probes fine.

Here's the change I made (it won't apply I'm afraid as gmail doesn't
like pasting tabs)
  continue;
@@ -1678,6 +1680,30 @@ static void of_register_spi_devices(struct
spi_master *master)
  nc->full_name);
  of_node_clear_flag(nc, OF_POPULATED);
  }
+ spi_dev_elem = kmalloc(sizeof(*spi_dev_elem), GFP_KERNEL);
+ if (spi_dev_elem == NULL) {
+ printk(KERN_ERR "of_register_spi_devices : failed to allocate spi
dev elem\n");
+ continue;
+ }
+ spi_dev_elem->spi_dev = spi;
+ list_add_tail(&spi_dev_elem->list, &spi_devices);
+ }
+
+ list_for_each_safe(elem, tmp, &spi_devices) {
+ int status;
+ struct spi_device *spi_dev;
+
+ spi_dev_elem = list_entry(elem, struct spi_device_list_elem, list);
+ spi_dev = spi_dev_elem->spi_dev;
+ status = device_add(&spi_dev->dev);
+ if (status < 0)
+ dev_err(&master->dev, "can't add %s, status %d\n",
+ dev_name(&spi_dev->dev), status);
+ else
+ dev_dbg(&master->dev, "registered child %s\n", dev_name(&spi_dev->dev));
+
+ list_del(elem);
+ kfree(elem);
  }
 }
 #else
@@ -1772,12 +1798,18 @@ static acpi_status
acpi_register_spi_device(struct spi_master *master,
  acpi_device_set_enumerated(adev);

  adev->power.flags.ignore_parent = true;
- if (spi_add_device(spi)) {
- adev->power.flags.ignore_parent = false;
- dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
- dev_name(&adev->dev));
- spi_dev_put(spi);
- }
+ if (spi_add_device(spi))
+ goto err_put_dev;
+ if (device_add(&proxy->dev))
+ goto err_put_dev;
+
+ return AE_OK;
+
+err_put_dev:
+ adev->power.flags.ignore_parent = false;
+ dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
+ dev_name(&adev->dev));
+ spi_dev_put(spi);

  return AE_OK;
 }

Now this is on a oldish kernel (4.12) and moving the kernel forward
isn't trivial.  I was just wondering if this has been fixed already so
I could backport it, I couldn't see anything in the latest kernel but
maybe it has been solved a different way.  If not is there a better
way of fixing this? Or is the OMAP SPI controller driver the problem,
should it parse the child nodes first and set itself up accordingly?

I'm more than happy to cleanup the hack I have put in place and submit
it if you think it's an acceptable solution.

Regards,
Martin.