| Message ID | 20190930032503.44425-2-biwen.li@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] i2c: pca954x: Add property to skip disabling PCA954x MUX device | expand |
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@nxp.com> > --- > 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.txt > + - i2c-mux-never-disable: always forces mux to be enabled. Either needs to have a vendor prefix or be documented as a common property. 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. Rob
> > 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@nxp.com> > > --- > > 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.txt > > + - i2c-mux-never-disable: always forces mux to be enabled. > > Either needs to have a vendor prefix or be documented as a common > property. > > 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. > > Rob
> > > > > 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@nxp.com> > > > --- > > > 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. > > > > 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. > > > > Rob
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@nxp.com> >>>> --- >>>> 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@axentia.se/
> > 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@nxp.com> > >>>> --- > >>>> 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.t > >>>> x > >>>> 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? I will focus on idle-state property, thanks. > > >>> > >>> 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. Got it, thanks, I will try it. > > Cheers, > Peter > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k > ernel.org%2Flkml%2F07d85748-0721-39d4-d2be-13eb16b0f1de%40axenti > a.se%2F&data=02%7C01%7Cbiwen.li%40nxp.com%7C4fdea02b48b94 > ed1031f08d7507509ac%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% > 7C0%7C637066335914038253&sdata=aZfxDhLPX%2FSMFGuW8ryM > %2BcxQetFUDpdxxLa%2BuUQs7I4%3D&reserved=0
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.txt + - i2c-mux-never-disable: always forces mux to be enabled. Example:
The patch adds an optional property i2c-mux-never-disable Signed-off-by: Biwen Li <biwen.li@nxp.com> --- Change in v2: - update documentation Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 1 + 1 file changed, 1 insertion(+)