On 2018-05-14 14:26, Bastian Stender wrote: > Hi Peter, > > On 05/07/2018 11:07 AM, Peter Rosin wrote: >> On 2018-05-04 15:10, Bastian Stender wrote: >>> On 05/04/2018 03:04 PM, Bastian Stender wrote: >>>> Signed-off-by: Bastian Stender <bst@xxxxxxxxxxxxxx> >>>> --- >>>> .../devicetree/bindings/i2c/i2c-mux-pca954x.txt | 16 ++++++++++++++++ >>>> 1 file changed, 16 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt >>>> index 34d91501342e..864ac91f8c1c 100644 >>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt >>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt >>>> @@ -36,6 +36,22 @@ Optional Properties: >>>> - first cell is the pin number >>>> - second cell is used to specify flags. >>>> See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt >>>> + - mux-locked: If present, explicitly allow unrelated I2C transactions on the >>>> + parent I2C adapter at these times: >>>> + + during setup of the multiplexer >>>> + + between setup of the multiplexer and the child bus I2C transaction >>>> + + between the child bus I2C transaction and releasing of the multiplexer >>>> + + during releasing of the multiplexer >>>> + >>>> + However, I2C transactions to devices behind all I2C multiplexers connected >>>> + to the same parent adapter that this multiplexer is connected to are blocked >>>> + for the full duration of the complete multiplexed I2C transaction (i.e. >>>> + including the times covered by the above list). >>>> + If mux-locked is not present, the multiplexer is assumed to be parent-locked. >>>> + This means that no unrelated I2C transactions are allowed on the parent I2C >>>> + adapter for the complete multiplexed I2C transaction. >>>> + The properties of mux-locked and parent-locked multiplexers are discussed >>>> + in more detail in Documentation/i2c/i2c-topology. >>> >>> I am not sure about this. I think it will act like the gpmux driver here >>> so I copied it from >>> Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt. Is this correct? >> >> I don't think it's wrong, but it might be a little bit too generic. The gpmux >> binding cannot assume too much about the actual mux, but in this case we >> know exactly how the mux operates. I.e. the initial 4-point list can drop >> the points "during setup/releasing of the multiplexer" because those actions >> happen as i2c transactions that are locking the parent adapter meaning that >> nothing can disturb them. >> >> However, it would be very good to know what the actual deadlock is that this >> mux-locked is fixing. And a description of that deadlock would fit nicely in >> the commit message. I understood it as if you could trigger it quite easily? >> Any chance that you could make another attempt to pinpoint it with some >> printk-debugging or something, given that lockdep wasn't helpful? > > The root cause was now identified by a colleague of mine. The following > patch set was used along with the proposed patches: > > https://www.spinics.net/lists/linux-i2c/msg32324.html > > As a result __i2c_transfer() ends up without enabled clocks. The problem > is solved in our case by switching to the mainline solution: > > https://www.spinics.net/lists/linux-i2c/msg33890.html Nice, thanks for keeping me in the loop! > The question remains whether the proposed patch (along with the > suggested modifications) should be applied nonetheless? Maybe? I will not block it because there certainly are cases where it is needed, but someone<tm> needs to make the adjustments. Cheers, Peter
