On 2020-05-15 09:04, Adamski, Krzysztof (Nokia - PL/Wrocław) wrote: > Hi Wolfram, Peter, > > Thank you for the quick response. > > W dniu 14.05.2020 o 16:50, Wolfram Sang pisze: >> Hi Krzysztof, >> >> On Thu, May 14, 2020 at 02:41:17PM +0200, Adamski, Krzysztof (Nokia - PL/Wrocław) wrote: >> >> Adding Peter as the mux maintainer to CC. >> >>> I have a problem that I think cannot be currently easily addressed by I2C framework in the kernel and I'm seeking for an >>> advice on how to approach this. I have an I2C device that can be accessed from two I2C masters connected to I2C bus >>> master selector channels. Both masters must do such a sequence before performing long operation: >> >> I need a diagram of that setup. What is the BMS? A chip? Some software? >> Can you draw a graph and give names of chips etc...? >> >> And, of course, why on earth do you need to access the same chip from >> two masters within one Linux? :) (That's how I understood it) >> *snip* useful layout. > I was thinking that maybe a lock like this could be expressed by i2c_lock_bus with some special flag that would make > sure no deselect is called in i2c_mux_master_xfer() (and would be ignored if our parent is not an arbiter)? We already > have the I2C_MUX_ARBITRATOR flag and the i2c_mux_core does have an arbitrator bool so it is just a matter of allowing to > do this kind of deeper lock on the bus. Yes, that definitely makes it clearer. So, what you need is something more complex than an I2C xfer between select/deselect. Your proposal to add a flag to not do the deselect should probably have a matching flag to not do the select on the following xfer. Otherwise that second xfer is likely to do useless work. This should probably also be two independent flags, so that you can add intermediate xfers that neither select nor deselect. But you also need an explicit deselect mechanism for use if there is a problem half way through... But, I think all that exposes the too much to the users, and while it is certainly possible (most things are), I not a huge fan of it. Maintainers of other subsystems will not know the rules, and drivers are, over time, bound to start using these facilities in half-baked ways. I would rather have some generic I2C mechanism to request more than a single xfer as a unit, such that the muxes/arbs/gates can then lock the mux/arb/gate, do the select, feed the unit to the parent adapter and finally deselect and unlock the mux/arb/gate. With the locking and selecting centralized instead of spread out to all users. This helps avoid bugs where things are not cleaned up on error paths etc. How that "I2C unit" should be constructed is another matter. The cool thing of the day seems to be eBPF programs, but I don't know if that fits here? Note that there are other users needing more complex "units" than xfers. Muxes themselves could use it, and there are a number of drivers that lock the adapter manually, do a couple of unlocked xfers, and then unlock the adapter. All that could probably be cleaned up a bit with a generic "I2C unit" facility. Cheers, Peter
