On 2018-06-23 12:17, Boris Brezillon wrote: > Hi Peter, > > On Fri, 22 Jun 2018 23:35:34 +0200 > Peter Rosin <peda@xxxxxxxxxx> wrote: > >> On 2018-06-22 12:49, Boris Brezillon wrote: >>> Add core infrastructure to support I3C in Linux and document it. >>> >>> This infrastructure is not complete yet and will be extended over >>> time. >>> >>> There are a few design choices that are worth mentioning because they >>> impact the way I3C device drivers can interact with their devices: >>> >>> - all functions used to send I3C/I2C frames must be called in >>> non-atomic context. Mainly done this way to ease implementation, but >>> this is still open to discussion. Please let me know if you think >>> it's worth considering an asynchronous model here >>> - the bus element is a separate object and is not implicitly described >>> by the master (as done in I2C). The reason is that I want to be able >>> to handle multiple master connected to the same bus and visible to >>> Linux. >>> In this situation, we should only have one instance of the device and >>> not one per master, and sharing the bus object would be part of the >>> solution to gracefully handle this case. >>> I'm not sure we will ever need to deal with multiple masters >>> controlling the same bus and exposed under Linux, but separating the >>> bus and master concept is pretty easy, hence the decision to do it >>> like that. >>> The other benefit of separating the bus and master concepts is that >>> master devices appear under the bus directory in sysfs. >> >> Are bus multiplexers relevant to I3C? > > Not yet, but who knows. > >> The locking needed for handling >> muxes for I2C is, well, convoluted... > > Do you remember what was the problem? > > Anyway, I'd really like to have basic support upstreamed before we > start considering advanced use cases that do not exist yet. Don't get > me wrong, I'm not against having the multiplexer/locking discussion, > but it should not block inclusion of the I3C subsystem. I'm trying to avoid the unfortunate situation in I2C where there are two slightly incompatible locking schemes for muxes. There's probably nothing to worry about until the first I3C mux is added though. But since I2C devices are supposedly compatible with I3C that might be the case from day one? --- If I read your code right, I3C client drivers will typically call i3c_device_do_priv_xfer (instead of i2c_transfer/i2c_smbus_xfer) and i3c_device_do_prov_xfer will grab the i3c_bus_normaluse_lock during the transfer. This seems equivalent to normal use in I2C with i2c_transfer/i2c_smbus_xfer. When muxes are thrown into the mix, you find yourself needing to do more than the "real" transfer with some lock held. In I2C there is an unlocked __i2c_transfer, and locking/unlocking is exposed. Muxes typically grab the lock, set the mux in the appropriate position, do an unlocked __i2c_transfer, optionally sets the mux in some default position and then lets go of the lock. See e.g. i2c/muxes/i2c-mux-pca954x.c However, that is the simple case. There are also muxes that are controlled with GPIO pins, and that gets hairy if the GPIO pins are controlled from the same I2C bus that is muxed. The GPIO driver would have to know that some GPIO pins need to use unlocked I2C transfers for that to work with the above locking scheme. And no GPIO driver does not want to know about that at all. I.e. you have two fundamentally different requirement depending on if the GPIO pins controlling the mux are controlled using the muxed bus or if the pins are controlled in some completely unrelated way. The latter case is probably the normal case, with the GPIO mux controlled directly from some SoC pins. In the latter case you also want to prevent any transfer on the bus while the GPIO pins for the mux are changing, i.e. the total opposite of the former case. It gets really really hairy if you have multiple levels of muxes... There are a some old drivers (e.g. i2c/busses/i2c-amd756-s4882.c) that handles this by simply bypassing the GPIO subsystem, but that is not really an option if some pins are used to mux the I2C bus while some pins are used for other things. I don't know if this affects I3C before muxes are added, but I suspect muxes will happen sooner rather than later, since the spec mentions that the bus only support 11 devices maximum. 11 don't seem like a lot, and it seems likely that there will be a need to have more devices somehow... But just maybe, in order to not run into the above situation, it needs to be handled right from the start with preparatory and cleanup stages of each transfers, or something? Cheers, Peter
