I think all these sysctls should be removed. CS polarity is an attribute of the slave device, not the controller.

Don't do these reads until after waiting for BCM_SPI_BUSY below.

this call needs to happen after modifying the clock speed, phase, and polarity. Apparently the interrupt bits are what start the transaction, and if the clock phase and polarity and speed are modified AFTER that, it might be causing a glitch that I observed when doing a mode 3 transfer.

if all 3 CS pins have the same polarity this will work. however, there's only a single polarity bit for all 3 CS lines. Given that fact, it may be better to NOT set the SPI_CS_CSPOL bit, but have a sysctl var for it (or device parameter) instead? For now it may simply need to be documented that having different polarities for the 3 CS pins is going to cause problems if you're using multiple SPI devices on the same bus.

once the SPI_CS_TA bit is set, the CS lines flip to whatever state is described. This also suggests that the SPI_CS_CSPOL bit should be 'sticky' across transactions, or you could have a real _mess_, depending on the state of the clock line following the transaction. It should also be verified as being properly set when the 'spibus' devices are first created from FDT info, though setting it correctly on the very first transaction will probably work in most cases.

This is all because clock polarities, depending on the mode, may attempt to shift in/out a bit if the CS line has the wrong polarity while the clock mode is being changed. Further, if the default clock polarity is opposite, it's possible that the mode switch itself might trigger something if CS polarity is wrong at the moment this all happens. It really complicates things and suggests that CS polarity should probably happen first, then the mode and clock speed, and finally the SPI_CS_TA bit [with int flags on as well, like before].

If all of this can't be accomplished 'safely', the SPI_CS_CSPOL bit should probably be left as 0, with 'TODO' comments in the code to implement this when it can be done.

I'm inclined to just comment out this section, and not support the SPIBUS_CS_HIGH bit, mostly because it's rarely used, and because its implementation may require additional support from spibus that affects every single SPI driver.

I have to wonder whether it would be better to leave the clock phase, polarity, and speed the way it is, rather than restoring it.

The interpretation of clock is that the device wants the bus to go as fast as possible, but no faster than clock. The clock should be reset for each transfer using that logic. That is, if clock is 1mhz for cs 0 and 50mhz for cs 1, it's not okay to do 1mhz transfers on cs 1 just because that's the value already in the register and it's lower than the max.

This concept of "0 means default" is insane. What does that even mean? I can't remember a single spi controller I've ever seen that has the idea of a default speed. About the closest we might come to that is to capture whatever is in the clock divider register at attach() time and call that the default (i.e., default means "whatever uboot set up"). But if that value is zero or an uninitialized invalid value, who knows what happens.

I don't think there's any need to restore anything. Each new transfer should set up mode and clock before starting the transfer. If we're going to have the idea of a default clock, we should capture its value once at attach time.