On Mon, Nov 25, 2013 at 01:35:44PM +0100, martin sperl wrote: > We just filling in the "template" value for the register used the next > time the control-register needs to get configured for a transfer.
> If you look exactly you will find that > bcm2835_spi_start_transfer makes use of: > bs->cs_device_flags[spi->chip_select] > and bcm2835_spi_transfer_one uses the: > bs->cs_device_flags_idle > to fill in the "flags" and then writes to the control-register. > So if there would be a race between spi_setup and the functions above, > then the worsted case situation is that: > bs->cs_device_flags[spi->chip_select] and/or bs->cs_device_flags_idle > still contains the "old" flags. It doesn't look like that, it looks like the update first resets things to a default and then writes in the new value. It also appears to affect the entire array rather than just one value. The other thing is that as with all your other code this is full of coding style issues, these will need to be fixed. Please, as previously requested, read and follow CodingStyle. > But if this is a race, then it exists regardless of if there is a > spinlock protecting access of those variables or not, because then > it is a race between spi_transfer and spi_setup which is totally outside > of the driver. Yes, this is the point - the two aren't protected against each other, setup() is supposed to only take effect on the next transfer. > Maybe that a device may change spi->mode while the driver is working? Yes. > If so then such an action would require a full bus-lock to avoid running > into a race with other device-drivers. Then the driver could reconfigure > the polarity settings on the device itself and then it can change > spi->mode and can call spi_setup and then finally release the bus lock. This doesn't require a bus lock, it only affects the individual device.
signature.asc
Description: Digital signature
