Add support for settle-time-us property. If this is defined in device
tree then add this delay to mux APIs.
v2->v3:
- move the binding changes into i2c-mux.yaml
- read at a later point the DT property 'settle-time-us'
v1->v2:
- add the changes to i2c-mux-gpmux instead of i2c-mux-gpio to be able
to use mux_control_select_delay
Horatiu Vultur (2):
dt-bindings: i2c-mux: Add property for settle time
i2c-mux-gpmux: Support settle-time-us property
Documentation/devicetree/bindings/i2c/i2c-mux.yaml | 6 ++++++
drivers/i2c/muxes/i2c-mux-gpmux.c | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
--
2.33.0
Add support for settle-time-us property. If this is defined in device
tree then add this delay to mux APIs.
Signed-off-by: Lars Povlsen <[email protected]>
Signed-off-by: Horatiu Vultur <[email protected]>
---
drivers/i2c/muxes/i2c-mux-gpmux.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c
index d3acd8d66c32..f64a4b6034df 100644
--- a/drivers/i2c/muxes/i2c-mux-gpmux.c
+++ b/drivers/i2c/muxes/i2c-mux-gpmux.c
@@ -16,6 +16,7 @@
struct mux {
struct mux_control *control;
+ u32 delay_us;
bool do_not_deselect;
};
@@ -25,7 +26,7 @@ static int i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
struct mux *mux = i2c_mux_priv(muxc);
int ret;
- ret = mux_control_select(mux->control, chan);
+ ret = mux_control_select_delay(mux->control, chan, mux->delay_us);
mux->do_not_deselect = ret < 0;
return ret;
@@ -106,6 +107,7 @@ static int i2c_mux_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, muxc);
+ of_property_read_u32(np, "settle-time-us", &mux->delay_us);
muxc->mux_locked = of_property_read_bool(np, "mux-locked");
for_each_child_of_node(np, child) {
--
2.33.0
Some HW requires some time for the signals to settle after the muxing is
changed. Allow this time to be specified in device tree.
Signed-off-by: Horatiu Vultur <[email protected]>
---
Documentation/devicetree/bindings/i2c/i2c-mux.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
index 24cac36037f5..4628ff6340c1 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
@@ -29,6 +29,12 @@ properties:
'#size-cells':
const: 0
+ settle-time-us:
+ default: 0
+ description:
+ The time required for the signals to settle. Currently only the
+ i2c-mux-gpmux driver supports this optional binding.
+
patternProperties:
'^i2c@[0-9a-f]+$':
$ref: /schemas/i2c/i2c-controller.yaml
--
2.33.0
On 2021-11-01 13:25, Horatiu Vultur wrote:
> Some HW requires some time for the signals to settle after the muxing is
> changed. Allow this time to be specified in device tree.
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mux.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> index 24cac36037f5..4628ff6340c1 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> @@ -29,6 +29,12 @@ properties:
> '#size-cells':
> const: 0
>
> + settle-time-us:
> + default: 0
> + description:
> + The time required for the signals to settle. Currently only the
> + i2c-mux-gpmux driver supports this optional binding.
The information about how i2c-mux-gpmux is special is bound to go stale,
and I don't think we should mention such specific details in the binding.
What I meant was a generic warnings about optional bindings perhaps not
being supported by all drivers, along the lines of this from i2c.txt:
"These properties may not be supported by all drivers. However, if a driver
wants to support one of the below features, it should adapt these bindings."
However, I now notice that this sentence makes no sense. It looks like it
should be s/adapt/adopt/.
And, in the i2c-mux.yaml case it can simply say "Optional properties"
instead of "These properites" (which refers to a subset of properties
immediately below the text) since with a yaml binding it is always
clear which properties are optional and which are required. Lastly, I
guess this warning belongs in the description.
> +
> patternProperties:
> '^i2c@[0-9a-f]+$':
> $ref: /schemas/i2c/i2c-controller.yaml
>
Since this is the first optional property, you now need to specify what
properties are required, which is everything but settle-time-us. If you
don't, all properties are required. Which is not what we want...
Something like this should do it, I think:
required:
- compatible
- '#address-cells'
- '#size-cells'
Cheers,
Peter
The 11/01/2021 15:32, Peter Rosin wrote:
Hi Peter,
>
> On 2021-11-01 13:25, Horatiu Vultur wrote:
> > Some HW requires some time for the signals to settle after the muxing is
> > changed. Allow this time to be specified in device tree.
> >
> > Signed-off-by: Horatiu Vultur <[email protected]>
> > ---
> > Documentation/devicetree/bindings/i2c/i2c-mux.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > index 24cac36037f5..4628ff6340c1 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > @@ -29,6 +29,12 @@ properties:
> > '#size-cells':
> > const: 0
> >
> > + settle-time-us:
> > + default: 0
> > + description:
> > + The time required for the signals to settle. Currently only the
> > + i2c-mux-gpmux driver supports this optional binding.
>
> The information about how i2c-mux-gpmux is special is bound to go stale,
> and I don't think we should mention such specific details in the binding.
> What I meant was a generic warnings about optional bindings perhaps not
> being supported by all drivers, along the lines of this from i2c.txt:
>
> "These properties may not be supported by all drivers. However, if a driver
> wants to support one of the below features, it should adapt these bindings."
>
> However, I now notice that this sentence makes no sense. It looks like it
> should be s/adapt/adopt/.
>
> And, in the i2c-mux.yaml case it can simply say "Optional properties"
> instead of "These properites" (which refers to a subset of properties
> immediately below the text) since with a yaml binding it is always
> clear which properties are optional and which are required. Lastly, I
> guess this warning belongs in the description.
>
> > +
> > patternProperties:
> > '^i2c@[0-9a-f]+$':
> > $ref: /schemas/i2c/i2c-controller.yaml
> >
>
> Since this is the first optional property, you now need to specify what
> properties are required, which is everything but settle-time-us. If you
> don't, all properties are required. Which is not what we want...
>
> Something like this should do it, I think:
>
> required:
> - compatible
> - '#address-cells'
> - '#size-cells'
Thanks for a detail explanation but I am still struggling with these
bindings. Were you thinking to have something like this?
---
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
index 24cac36037f5..c9fde1bb0fea 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
@@ -19,6 +19,9 @@ description: |+
populating the i2c child busses. If an 'i2c-mux' subnode is present, only
subnodes of this will be considered as i2c child busses.
+ Optional properties may not be supported by all drivers. However, if a driver
+ wants to support one of the below features, it should adopt these bindings.
+
properties:
$nodename:
pattern: '^(i2c-?)?mux'
@@ -29,6 +32,11 @@ properties:
'#size-cells':
const: 0
+ settle-time-us:
+ default: 0
+ description:
+ The time required for the signals to settle.
+
patternProperties:
'^i2c@[0-9a-f]+$':
$ref: /schemas/i2c/i2c-controller.yaml
@@ -41,6 +49,11 @@ patternProperties:
additionalProperties: true
+required:
+ - compatible
+ - '#address-cells'
+ - '#size-cells'
+
examples:
- |
/*
---
If I have this then my problem is with the required properties because then I
start to get new warnings once I run:
make ARCH=arm CROSS_COMPILE=arm-linux- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/i2c-mux.yaml
For example, one of new the warnings is this:
/home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: 'compatible' is a required property
From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
/home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#address-cells' is a required property
From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
/home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#size-cells' is a required property
From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
If I don't have the required properties then I don't see these new warnings.
Does it mean that actually the properties are optional by default?
>
> Cheers,
> Peter
--
/Horatiu
On Mon, Nov 01, 2021 at 10:32:01PM +0100, Horatiu Vultur wrote:
> The 11/01/2021 15:32, Peter Rosin wrote:
>
> Hi Peter,
>
> >
> > On 2021-11-01 13:25, Horatiu Vultur wrote:
> > > Some HW requires some time for the signals to settle after the muxing is
> > > changed. Allow this time to be specified in device tree.
> > >
> > > Signed-off-by: Horatiu Vultur <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/i2c/i2c-mux.yaml | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > > index 24cac36037f5..4628ff6340c1 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > > @@ -29,6 +29,12 @@ properties:
> > > '#size-cells':
> > > const: 0
> > >
> > > + settle-time-us:
> > > + default: 0
> > > + description:
> > > + The time required for the signals to settle. Currently only the
> > > + i2c-mux-gpmux driver supports this optional binding.
> >
> > The information about how i2c-mux-gpmux is special is bound to go stale,
> > and I don't think we should mention such specific details in the binding.
> > What I meant was a generic warnings about optional bindings perhaps not
> > being supported by all drivers, along the lines of this from i2c.txt:
> >
> > "These properties may not be supported by all drivers. However, if a driver
> > wants to support one of the below features, it should adapt these bindings."
> >
> > However, I now notice that this sentence makes no sense. It looks like it
> > should be s/adapt/adopt/.
> >
> > And, in the i2c-mux.yaml case it can simply say "Optional properties"
> > instead of "These properites" (which refers to a subset of properties
> > immediately below the text) since with a yaml binding it is always
> > clear which properties are optional and which are required. Lastly, I
> > guess this warning belongs in the description.
> >
> > > +
> > > patternProperties:
> > > '^i2c@[0-9a-f]+$':
> > > $ref: /schemas/i2c/i2c-controller.yaml
> > >
> >
> > Since this is the first optional property, you now need to specify what
> > properties are required, which is everything but settle-time-us. If you
> > don't, all properties are required. Which is not what we want...
> >
> > Something like this should do it, I think:
> >
> > required:
> > - compatible
> > - '#address-cells'
> > - '#size-cells'
>
> Thanks for a detail explanation but I am still struggling with these
> bindings. Were you thinking to have something like this?
>
> ---
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> index 24cac36037f5..c9fde1bb0fea 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> @@ -19,6 +19,9 @@ description: |+
> populating the i2c child busses. If an 'i2c-mux' subnode is present, only
> subnodes of this will be considered as i2c child busses.
>
> + Optional properties may not be supported by all drivers. However, if a driver
> + wants to support one of the below features, it should adopt these bindings.
> +
> properties:
> $nodename:
> pattern: '^(i2c-?)?mux'
> @@ -29,6 +32,11 @@ properties:
> '#size-cells':
> const: 0
>
> + settle-time-us:
> + default: 0
> + description:
> + The time required for the signals to settle.
> +
> patternProperties:
> '^i2c@[0-9a-f]+$':
> $ref: /schemas/i2c/i2c-controller.yaml
> @@ -41,6 +49,11 @@ patternProperties:
>
> additionalProperties: true
>
> +required:
> + - compatible
compatible should not be required here.
> + - '#address-cells'
> + - '#size-cells'
> +
> examples:
> - |
> /*
> ---
>
> If I have this then my problem is with the required properties because then I
> start to get new warnings once I run:
>
> make ARCH=arm CROSS_COMPILE=arm-linux- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/i2c-mux.yaml
>
> For example, one of new the warnings is this:
>
> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: 'compatible' is a required property
> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#address-cells' is a required property
> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#size-cells' is a required property
> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
This is because of the $nodename pattern being pretty lax and matches
on mux-mii-hog by mistake. We have 2 options. Change the nodename
pattern to '^(i2c-?)?mux(@.*)?$' or add 'select: false'. The former
would still match on 'mux' or 'mux@.*' which might still have problems.
For the latter, we just need to make sure all the i2c-mux schemas have a
$ref to this schema. Also, with that change we'd stop checking 'i2c-mux'
nodes that don't yet have a specific schema. That said, I do lean toward
the latter option.
Rob
The 11/02/2021 13:37, Rob Herring wrote:
Hi Rob,
>
> On Mon, Nov 01, 2021 at 10:32:01PM +0100, Horatiu Vultur wrote:
> > The 11/01/2021 15:32, Peter Rosin wrote:
> >
> > Hi Peter,
> >
> > >
> > > On 2021-11-01 13:25, Horatiu Vultur wrote:
> > > > Some HW requires some time for the signals to settle after the muxing is
> > > > changed. Allow this time to be specified in device tree.
> > > >
> > > > Signed-off-by: Horatiu Vultur <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/i2c/i2c-mux.yaml | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > > > index 24cac36037f5..4628ff6340c1 100644
> > > > --- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > > > @@ -29,6 +29,12 @@ properties:
> > > > '#size-cells':
> > > > const: 0
> > > >
> > > > + settle-time-us:
> > > > + default: 0
> > > > + description:
> > > > + The time required for the signals to settle. Currently only the
> > > > + i2c-mux-gpmux driver supports this optional binding.
> > >
> > > The information about how i2c-mux-gpmux is special is bound to go stale,
> > > and I don't think we should mention such specific details in the binding.
> > > What I meant was a generic warnings about optional bindings perhaps not
> > > being supported by all drivers, along the lines of this from i2c.txt:
> > >
> > > "These properties may not be supported by all drivers. However, if a driver
> > > wants to support one of the below features, it should adapt these bindings."
> > >
> > > However, I now notice that this sentence makes no sense. It looks like it
> > > should be s/adapt/adopt/.
> > >
> > > And, in the i2c-mux.yaml case it can simply say "Optional properties"
> > > instead of "These properites" (which refers to a subset of properties
> > > immediately below the text) since with a yaml binding it is always
> > > clear which properties are optional and which are required. Lastly, I
> > > guess this warning belongs in the description.
> > >
> > > > +
> > > > patternProperties:
> > > > '^i2c@[0-9a-f]+$':
> > > > $ref: /schemas/i2c/i2c-controller.yaml
> > > >
> > >
> > > Since this is the first optional property, you now need to specify what
> > > properties are required, which is everything but settle-time-us. If you
> > > don't, all properties are required. Which is not what we want...
> > >
> > > Something like this should do it, I think:
> > >
> > > required:
> > > - compatible
> > > - '#address-cells'
> > > - '#size-cells'
> >
> > Thanks for a detail explanation but I am still struggling with these
> > bindings. Were you thinking to have something like this?
> >
> > ---
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > index 24cac36037f5..c9fde1bb0fea 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > @@ -19,6 +19,9 @@ description: |+
> > populating the i2c child busses. If an 'i2c-mux' subnode is present, only
> > subnodes of this will be considered as i2c child busses.
> >
> > + Optional properties may not be supported by all drivers. However, if a driver
> > + wants to support one of the below features, it should adopt these bindings.
> > +
> > properties:
> > $nodename:
> > pattern: '^(i2c-?)?mux'
> > @@ -29,6 +32,11 @@ properties:
> > '#size-cells':
> > const: 0
> >
> > + settle-time-us:
> > + default: 0
> > + description:
> > + The time required for the signals to settle.
> > +
> > patternProperties:
> > '^i2c@[0-9a-f]+$':
> > $ref: /schemas/i2c/i2c-controller.yaml
> > @@ -41,6 +49,11 @@ patternProperties:
> >
> > additionalProperties: true
> >
> > +required:
> > + - compatible
>
> compatible should not be required here.
>
> > + - '#address-cells'
> > + - '#size-cells'
> > +
> > examples:
> > - |
> > /*
> > ---
> >
> > If I have this then my problem is with the required properties because then I
> > start to get new warnings once I run:
> >
> > make ARCH=arm CROSS_COMPILE=arm-linux- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> >
> > For example, one of new the warnings is this:
> >
> > /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: 'compatible' is a required property
> > From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#address-cells' is a required property
> > From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> > /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#size-cells' is a required property
> > From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
>
> This is because of the $nodename pattern being pretty lax and matches
> on mux-mii-hog by mistake. We have 2 options. Change the nodename
> pattern to '^(i2c-?)?mux(@.*)?$' or add 'select: false'. The former
> would still match on 'mux' or 'mux@.*' which might still have problems.
> For the latter, we just need to make sure all the i2c-mux schemas have a
> $ref to this schema. Also, with that change we'd stop checking 'i2c-mux'
> nodes that don't yet have a specific schema. That said, I do lean toward
> the latter option.
From what I can see there are only two i2c-mux schemas and both of them
have a $ref to this schema [1][2]
[1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.yaml#L33
[2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml#L16
>
> Rob
--
/Horatiu
On 2021-11-02 23:27, Horatiu Vultur wrote:
> The 11/02/2021 13:37, Rob Herring wrote:
>> On Mon, Nov 01, 2021 at 10:32:01PM +0100, Horatiu Vultur wrote:
>>> The 11/01/2021 15:32, Peter Rosin wrote:
*snip*
>>>
>>> +required:
>>> + - compatible
>>
>> compatible should not be required here.
>>
>>> + - '#address-cells'
>>> + - '#size-cells'
>>> +
>>> examples:
>>> - |
>>> /*
>>> ---
>>>
>>> If I have this then my problem is with the required properties because then I
>>> start to get new warnings once I run:
>>>
>>> make ARCH=arm CROSS_COMPILE=arm-linux- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/i2c-mux.yaml
>>>
>>> For example, one of new the warnings is this:
>>>
>>> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: 'compatible' is a required property
>>> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
>>> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#address-cells' is a required property
>>> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
>>> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#size-cells' is a required property
>>> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
>>
>> This is because of the $nodename pattern being pretty lax and matches
>> on mux-mii-hog by mistake. We have 2 options. Change the nodename
>> pattern to '^(i2c-?)?mux(@.*)?$' or add 'select: false'. The former
>> would still match on 'mux' or 'mux@.*' which might still have problems.
>> For the latter, we just need to make sure all the i2c-mux schemas have a
>> $ref to this schema. Also, with that change we'd stop checking 'i2c-mux'
>> nodes that don't yet have a specific schema. That said, I do lean toward
>> the latter option.
>
> From what I can see there are only two i2c-mux schemas and both of them
> have a $ref to this schema [1][2]
>
> [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.yaml#L33
> [2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml#L16
I'm a relative yaml bindings newbie, but I assume adding "select: false" will
have the side effect of not enforcing this i2c-mux schema on i2c-muxes that
have not yet been converted to yaml? E.g. i2c-mux-gpio.txt, i2c-mux-pinctrl.txt
etc etc. But there are not too many of those. Is it a prerequisite to update
those bindings to yaml before doing "select: false"? Looking further I think
there's a total of about 15-20 drivers doing i2c-muxing (or arbing/gating),
and some of those exist outside the "i2c umbrella".
I wonder if e.g. this one [1] should really reference i2c-controller.yaml as
it is currently doing, or if i2c-mux.yaml is correct?
[1] Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.yaml
Maybe i2c-mux.yaml didn't work in that case because the node names were
"wrong" and did not match the pattern and then someone stuck
i2c-controller.yaml in there simply because that was close enough, and
also happened to work?
Cheers,
Peter
On Wed, Nov 3, 2021 at 6:45 AM Peter Rosin <[email protected]> wrote:
>
> On 2021-11-02 23:27, Horatiu Vultur wrote:
> > The 11/02/2021 13:37, Rob Herring wrote:
> >> On Mon, Nov 01, 2021 at 10:32:01PM +0100, Horatiu Vultur wrote:
> >>> The 11/01/2021 15:32, Peter Rosin wrote:
>
> *snip*
>
> >>>
> >>> +required:
> >>> + - compatible
> >>
> >> compatible should not be required here.
> >>
> >>> + - '#address-cells'
> >>> + - '#size-cells'
> >>> +
> >>> examples:
> >>> - |
> >>> /*
> >>> ---
> >>>
> >>> If I have this then my problem is with the required properties because then I
> >>> start to get new warnings once I run:
> >>>
> >>> make ARCH=arm CROSS_COMPILE=arm-linux- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> >>>
> >>> For example, one of new the warnings is this:
> >>>
> >>> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: 'compatible' is a required property
> >>> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> >>> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#address-cells' is a required property
> >>> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
> >>> /home/hvultur/linux/arch/arm/boot/dts/am335x-icev2.dt.yaml: mux-mii-hog: '#size-cells' is a required property
> >>> From schema: /home/hvultur/linux/Documentation/devicetree/bindings/i2c/i2c-mux.yaml
There's actually a ton of 'mux' nodes that should be causing warnings too.
> >> This is because of the $nodename pattern being pretty lax and matches
> >> on mux-mii-hog by mistake. We have 2 options. Change the nodename
> >> pattern to '^(i2c-?)?mux(@.*)?$' or add 'select: false'. The former
> >> would still match on 'mux' or 'mux@.*' which might still have problems.
> >> For the latter, we just need to make sure all the i2c-mux schemas have a
> >> $ref to this schema. Also, with that change we'd stop checking 'i2c-mux'
> >> nodes that don't yet have a specific schema. That said, I do lean toward
> >> the latter option.
> >
> > From what I can see there are only two i2c-mux schemas and both of them
> > have a $ref to this schema [1][2]
> >
> > [1] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.yaml#L33
> > [2] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml#L16
>
> I'm a relative yaml bindings newbie, but I assume adding "select: false" will
> have the side effect of not enforcing this i2c-mux schema on i2c-muxes that
> have not yet been converted to yaml? E.g. i2c-mux-gpio.txt, i2c-mux-pinctrl.txt
> etc etc. But there are not too many of those. Is it a prerequisite to update
> those bindings to yaml before doing "select: false"?
No. We may be losing some checks temporarily, but we've got plenty of
other warnings to keep busy. And most cases in tree seem to be pca954x
anyways.
> Looking further I think
> there's a total of about 15-20 drivers doing i2c-muxing (or arbing/gating),
> and some of those exist outside the "i2c umbrella".
>
> I wonder if e.g. this one [1] should really reference i2c-controller.yaml as
> it is currently doing, or if i2c-mux.yaml is correct?
>
> [1] Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.yaml
>
> Maybe i2c-mux.yaml didn't work in that case because the node names were
> "wrong" and did not match the pattern and then someone stuck
> i2c-controller.yaml in there simply because that was close enough, and
> also happened to work?
While the device does have muxing capability, it does much more and
the use is rather specific. So I think it is fine.
Rob