Re: [EXT] Re: [v2,2/2] dt-bindings: i2c-mux-pca954x: Add optional property i2c-mux-never-disable

[Peter Rosin]

On 2019-10-14 06:16, Biwen Li wrote:
>>
>>>
>>> On Mon, Sep 30, 2019 at 11:25:03AM +0800, Biwen Li wrote:
>>>> The patch adds an optional property i2c-mux-never-disable
>>>>
>>>> Signed-off-by: Biwen Li <biwen.li@xxxxxxx>
>>>> ---
>>>> Change in v2:
>>>>       - update documentation
>>>>
>>>>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> index 30ac6a60f041..71b73d0fdb62 100644
>>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> @@ -34,6 +34,7 @@ Optional Properties:
>>>>      - first cell is the pin number
>>>>      - second cell is used to specify flags.
>>>>      See also
>>>> Documentation/devicetree/bindings/interrupt-controller/interrupts.tx
>>>> t
>>>> +  - i2c-mux-never-disable: always forces mux to be enabled.
>>>
>>> Either needs to have a vendor prefix or be documented as a common
>>> property.
> I choose to be documented as a common property.

Can we please just drop the never-disable approach and focus on idle-state
instead?

>>>
>>> IIRC, we already have a property for mux default state which seems
>>> like that would cover this unless you need to leave it in different states.
>> Okay, you are right, thank you so much. I will try it in v3.
> Do you mean that the property is i2c-mux-idle-disconnect in Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt?
> If so, the property i2c-mux-idle-disconnect is not good for me.
> Because condition of the property i2c-mux-idle-disconnect is in idle state(sometimes).
> But I need always enable i2c multiplexer in whatever state(anytime), so I add a common property i2c-mux-never-disable.

No, I do not think any new property is needed. AFAICT, idle-state fits
perfectly, and I will not consider this i2c-mux-never-disable
approach until some compelling reason is presented why idle-state
is not appropriate. You promised to take a stab at it, and until
I hear back on that, this series is on hold. As indicated here [1].

You need to patch the driver to look at the idle-state property
instead of inventing a new (and less flexible) property. If you
implement idle-state for this driver and set the idle-state to
some channel in the dts, the mux will never disconnect. Problem solved.
Perhaps not your first solution, but it does solve your problem and
may actually be useful for other purposes than your broken hardware.
And it is consistent across other i2c-muxes. I see no downside.

Cheers,
Peter

[1] https://lore.kernel.org/lkml/07d85748-0721-39d4-d2be-13eb16b0f1de@xxxxxxxxxx/