| Message ID | 20211001000417.15334-3-leoyang.li@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup of LS1021a device trees | expand |
On Thu, 30 Sep 2021 19:04:03 -0500, Li Yang wrote: > When the binding was converted from txt to yaml, it actually added more > constrains than the original txt binding which was already used in many > in-tree DTSes. Some of the newly added constrains are either not valid > or not neccessary. > > Not all SoCs use ipg as the clock name for i2c. There is no point in > having SoC integration information defined in i2c binding. Remove the > clock name requirement in the schema. > > The original txt binding didn't require the order of tx and rx for > dmas/dma-names. Many in tree DTSes are already using the other order. > Both orders should just work fine. Update the schema to allow both. > > Signed-off-by: Li Yang <leoyang.li@nxp.com> > --- > v2: > Updated the patch description > > Documentation/devicetree/bindings/i2c/i2c-imx.yaml | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings. Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future. Full log is available here: https://patchwork.ozlabs.org/patch/1535099 i2c@21a4000: clock-frequency:0:0: 50000 is not one of [100000, 400000] arch/arm/boot/dts/imx6dl-alti6p.dt.yaml i2c@21f8000: clock-frequency:0:0: 50000 is not one of [100000, 400000] arch/arm/boot/dts/imx6dl-alti6p.dt.yaml i2c@30a20000: clock-frequency:0:0: 387000 is not one of [100000, 400000] arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r4.dt.yaml i2c@30a30000: clock-frequency:0:0: 387000 is not one of [100000, 400000] arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r4.dt.yaml i2c@30a40000: clock-frequency:0:0: 387000 is not one of [100000, 400000] arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r4.dt.yaml i2c@30a50000: clock-frequency:0:0: 387000 is not one of [100000, 400000] arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dt.yaml arch/arm64/boot/dts/freescale/imx8mq-librem5-r4.dt.yaml
On Thu, Sep 30, 2021 at 7:04 PM Li Yang <leoyang.li@nxp.com> wrote: > > When the binding was converted from txt to yaml, it actually added more > constrains than the original txt binding which was already used in many > in-tree DTSes. Some of the newly added constrains are either not valid > or not neccessary. IMO, both of these should be fixed in the dts files. > Not all SoCs use ipg as the clock name for i2c. There is no point in > having SoC integration information defined in i2c binding. Remove the > clock name requirement in the schema. Any name you want is not fine. Your choices are remove clock-names, add all the names used, or change everyone to use 'ipg'. > The original txt binding didn't require the order of tx and rx for > dmas/dma-names. Many in tree DTSes are already using the other order. > Both orders should just work fine. Update the schema to allow both. Doesn't sound like a case where defining the order is challenging. Rob
On Fri, Oct 1, 2021 at 8:24 AM Rob Herring <robh+dt@kernel.org> wrote: > > On Thu, Sep 30, 2021 at 7:04 PM Li Yang <leoyang.li@nxp.com> wrote: > > > > When the binding was converted from txt to yaml, it actually added more > > constrains than the original txt binding which was already used in many > > in-tree DTSes. Some of the newly added constrains are either not valid > > or not neccessary. > > IMO, both of these should be fixed in the dts files. > > > Not all SoCs use ipg as the clock name for i2c. There is no point in > > having SoC integration information defined in i2c binding. Remove the > > clock name requirement in the schema. > > Any name you want is not fine. Your choices are remove clock-names, > add all the names used, or change everyone to use 'ipg'. I understand that the name should be important as clocks are referenced by name. But since the i2c controller only has one clock , the name is never referenced in the driver. If we really want to define the name, IMO, it should be from the perspective of the i2c controller like "clkin" or "i2c" instead of the "ipg" from the perspective of SoC integration which could be changing with a different integration. I can list both "i2c" and "ipg" for now as a workaround though. > > > The original txt binding didn't require the order of tx and rx for > > dmas/dma-names. Many in tree DTSes are already using the other order. > > Both orders should just work fine. Update the schema to allow both. > > Doesn't sound like a case where defining the order is challenging. No, it is not challenging. But as dma channel is only referenced by name instead of index. I don't see too much benefit in enforcing the order other than easier to create the schema. > > Rob
On Fri, Oct 01, 2021 at 12:37:54PM -0500, Li Yang wrote: > On Fri, Oct 1, 2021 at 8:24 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Thu, Sep 30, 2021 at 7:04 PM Li Yang <leoyang.li@nxp.com> wrote: > > > > > > When the binding was converted from txt to yaml, it actually added more > > > constrains than the original txt binding which was already used in many > > > in-tree DTSes. Some of the newly added constrains are either not valid > > > or not neccessary. > > > > IMO, both of these should be fixed in the dts files. > > > > > Not all SoCs use ipg as the clock name for i2c. There is no point in > > > having SoC integration information defined in i2c binding. Remove the > > > clock name requirement in the schema. > > > > Any name you want is not fine. Your choices are remove clock-names, > > add all the names used, or change everyone to use 'ipg'. > > I understand that the name should be important as clocks are > referenced by name. But since the i2c controller only has one clock , > the name is never referenced in the driver. Then just remove 'clock-names' from the dts file. > If we really want to define the name, IMO, it should be from the > perspective of the i2c controller like "clkin" or "i2c" instead of the > "ipg" from the perspective of SoC integration which could be changing > with a different integration. I can list both "i2c" and "ipg" for now > as a workaround though. $modulename for $foo-names always looks made up and pointless to me. > > > > > > The original txt binding didn't require the order of tx and rx for > > > dmas/dma-names. Many in tree DTSes are already using the other order. > > > Both orders should just work fine. Update the schema to allow both. > > > > Doesn't sound like a case where defining the order is challenging. > > No, it is not challenging. But as dma channel is only referenced by > name instead of index. I don't see too much benefit in enforcing the > order other than easier to create the schema. Easier is nice, and that's the 'DT way' is the other reason. Rob
On Fri, Oct 8, 2021 at 5:20 PM Rob Herring <robh@kernel.org> wrote: > > On Fri, Oct 01, 2021 at 12:37:54PM -0500, Li Yang wrote: > > On Fri, Oct 1, 2021 at 8:24 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > On Thu, Sep 30, 2021 at 7:04 PM Li Yang <leoyang.li@nxp.com> wrote: > > > > > > > > When the binding was converted from txt to yaml, it actually added more > > > > constrains than the original txt binding which was already used in many > > > > in-tree DTSes. Some of the newly added constrains are either not valid > > > > or not neccessary. > > > > > > IMO, both of these should be fixed in the dts files. > > > > > > > Not all SoCs use ipg as the clock name for i2c. There is no point in > > > > having SoC integration information defined in i2c binding. Remove the > > > > clock name requirement in the schema. > > > > > > Any name you want is not fine. Your choices are remove clock-names, > > > add all the names used, or change everyone to use 'ipg'. > > > > I understand that the name should be important as clocks are > > referenced by name. But since the i2c controller only has one clock , > > the name is never referenced in the driver. > > Then just remove 'clock-names' from the dts file. Had thought the clock-names are mandatory, but it turns out not. Removing it should be great. > > > If we really want to define the name, IMO, it should be from the > > perspective of the i2c controller like "clkin" or "i2c" instead of the > > "ipg" from the perspective of SoC integration which could be changing > > with a different integration. I can list both "i2c" and "ipg" for now > > as a workaround though. > > $modulename for $foo-names always looks made up and pointless to me. > > > > > > > > > > The original txt binding didn't require the order of tx and rx for > > > > dmas/dma-names. Many in tree DTSes are already using the other order. > > > > Both orders should just work fine. Update the schema to allow both. > > > > > > Doesn't sound like a case where defining the order is challenging. > > > > No, it is not challenging. But as dma channel is only referenced by > > name instead of index. I don't see too much benefit in enforcing the > > order other than easier to create the schema. > > Easier is nice, and that's the 'DT way' is the other reason. Ok. Regards, Leo
diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.yaml b/Documentation/devicetree/bindings/i2c/i2c-imx.yaml index 3592d49235e0..da55d37a09a4 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-imx.yaml +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.yaml @@ -54,20 +54,20 @@ properties: maxItems: 1 clock-names: - const: ipg + maxItems: 1 clock-frequency: enum: [ 100000, 400000 ] dmas: - items: - - description: DMA controller phandle and request line for RX - - description: DMA controller phandle and request line for TX + minItems: 2 + maxItems: 2 dma-names: + minItems: 2 + maxItems: 2 items: - - const: rx - - const: tx + enum: [ "rx", "tx" ] sda-gpios: maxItems: 1
When the binding was converted from txt to yaml, it actually added more constrains than the original txt binding which was already used in many in-tree DTSes. Some of the newly added constrains are either not valid or not neccessary. Not all SoCs use ipg as the clock name for i2c. There is no point in having SoC integration information defined in i2c binding. Remove the clock name requirement in the schema. The original txt binding didn't require the order of tx and rx for dmas/dma-names. Many in tree DTSes are already using the other order. Both orders should just work fine. Update the schema to allow both. Signed-off-by: Li Yang <leoyang.li@nxp.com> --- v2: Updated the patch description Documentation/devicetree/bindings/i2c/i2c-imx.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)