| Message ID | 1614222604-27066-1-git-send-email-peng.fan@oss.nxp.com |
|---|---|
| Headers | show |
| Series | imx esdhc dt/driver update | expand |
On Thu, 25 Feb 2021 at 04:22, <peng.fan@oss.nxp.com> wrote: > > From: Peng Fan <peng.fan@nxp.com> > > V3: > Patch 1, drop unneeded pinctrl-0/1/2 > Patch 2 is new to avoid break dt bindings check > V2: > patch 1, 2, 3 is new > patch 4 is not changed > https://patchwork.kernel.org/project/linux-arm-kernel/cover/1611198593-16287-1-git-send-email-peng.fan@nxp.com/ > > Peng Fan (5): > dt-bindings: mmc: fsl-imx-esdhc: add pinctrl bindings > dt-bindings: clock: imx8qxp-lpcg: correct the example clock-names > arm64: dts: imx8qxp: correct usdhc clock-names sequence > dt-bindings: mmc: fsl-imx-esdhc: add clock bindings > mmc: sdhci-esdhc-imx: validate pinctrl before use it > > .../bindings/clock/imx8qxp-lpcg.yaml | 6 +++--- > .../bindings/mmc/fsl-imx-esdhc.yaml | 20 +++++++++++++++++++ > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 18 ++++++++--------- > drivers/mmc/host/sdhci-esdhc-imx.c | 2 +- > 4 files changed, 33 insertions(+), 13 deletions(-) > > -- > 2.30.0 > Applied patch 1, 4 and 5, thanks! Kind regards Uffe
Hi Shawn, > Subject: Re: [PATCH V3 0/5] imx esdhc dt/driver update > > On Thu, 25 Feb 2021 at 04:22, <peng.fan@oss.nxp.com> wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > V3: > > Patch 1, drop unneeded pinctrl-0/1/2 > > Patch 2 is new to avoid break dt bindings check > > V2: > > patch 1, 2, 3 is new > > patch 4 is not changed > > > > > > Peng Fan (5): > > dt-bindings: mmc: fsl-imx-esdhc: add pinctrl bindings > > dt-bindings: clock: imx8qxp-lpcg: correct the example clock-names > > arm64: dts: imx8qxp: correct usdhc clock-names sequence > > dt-bindings: mmc: fsl-imx-esdhc: add clock bindings > > mmc: sdhci-esdhc-imx: validate pinctrl before use it > > > > .../bindings/clock/imx8qxp-lpcg.yaml | 6 +++--- > > .../bindings/mmc/fsl-imx-esdhc.yaml | 20 > +++++++++++++++++++ > > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 18 ++++++++--------- > > drivers/mmc/host/sdhci-esdhc-imx.c | 2 +- > > 4 files changed, 33 insertions(+), 13 deletions(-) > > > > -- > > 2.30.0 > > > > Applied patch 1, 4 and 5, thanks! Would you pick patch 2,3? Thanks, Peng. > > Kind regards > Uffe
On Wed, Feb 24, 2021 at 9:23 PM <peng.fan@oss.nxp.com> wrote: > > From: Peng Fan <peng.fan@nxp.com> > > Add clock bindings for fsl-imx-esdhc yaml > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) Looks like this landed in linux-next and introduces warnings: /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.example.dt.yaml: mmc@5b010000: clock-names:1: 'ahb' was expected From schema: /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.example.dt.yaml: mmc@5b010000: clock-names:2: 'per' was expected From schema: /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
Hi Rob, > Subject: Re: [PATCH V3 4/5] dt-bindings: mmc: fsl-imx-esdhc: add clock > bindings > > On Wed, Feb 24, 2021 at 9:23 PM <peng.fan@oss.nxp.com> wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > Add clock bindings for fsl-imx-esdhc yaml > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 11 > +++++++++++ > > 1 file changed, 11 insertions(+) > > Looks like this landed in linux-next and introduces warnings: Patch 2,3 has not been picked by Shawn. After patch 2, 3 picked up, there will be no warnings. Thanks, Peng. > > /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/clo > ck/imx8qxp-lpcg.example.dt.yaml: > mmc@5b010000: clock-names:1: 'ahb' was expected From schema: > /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/m > mc/fsl-imx-esdhc.yaml > /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/clo > ck/imx8qxp-lpcg.example.dt.yaml: > mmc@5b010000: clock-names:2: 'per' was expected From schema: > /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/m > mc/fsl-imx-esdhc.yaml
Hi Rob, > From: Peng Fan (OSS) <peng.fan@oss.nxp.com> > Sent: Thursday, February 25, 2021 11:10 AM > > From: Peng Fan <peng.fan@nxp.com> > > Add clock bindings for fsl-imx-esdhc yaml > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > index a7fbd8cc1e38..369471814496 100644 > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > @@ -103,6 +103,17 @@ properties: > Only eMMC HS400 mode need to take care of this property. > default: 0 > > + clocks: > + maxItems: 3 > + description: > + Handle clocks for the sdhc controller. > + > + clock-names: > + items: > + - const: ipg > + - const: ahb > + - const: per One question: The side effect of this patch is that it imposes a forced order of clk names In DT which actually was not needed. Do we really have to do that? Or any other better approach to allow a random order to match the DT usage better? Regards Aisheng > + > pinctrl-names: > minItems: 1 > maxItems: 4 > -- > 2.30.0
On Fri, Mar 5, 2021 at 8:09 AM Aisheng Dong <aisheng.dong@nxp.com> wrote: > > Hi Rob, > > > From: Peng Fan (OSS) <peng.fan@oss.nxp.com> > > Sent: Thursday, February 25, 2021 11:10 AM > > > > From: Peng Fan <peng.fan@nxp.com> > > > > Add clock bindings for fsl-imx-esdhc yaml > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > index a7fbd8cc1e38..369471814496 100644 > > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > @@ -103,6 +103,17 @@ properties: > > Only eMMC HS400 mode need to take care of this property. > > default: 0 > > > > + clocks: > > + maxItems: 3 > > + description: > > + Handle clocks for the sdhc controller. > > + > > + clock-names: > > + items: > > + - const: ipg > > + - const: ahb > > + - const: per > > One question: > The side effect of this patch is that it imposes a forced order of clk names > In DT which actually was not needed. > > Do we really have to do that? Yes. > Or any other better approach to allow a random order to match the DT > usage better? Why do you need random order? We can not enforce the order, but we only do that when there's multiple optional entries. Rob
> From: Rob Herring <robh+dt@kernel.org> > Sent: Friday, March 5, 2021 10:14 PM > > On Fri, Mar 5, 2021 at 8:09 AM Aisheng Dong <aisheng.dong@nxp.com> > wrote: > > > > Hi Rob, > > > > > From: Peng Fan (OSS) <peng.fan@oss.nxp.com> > > > Sent: Thursday, February 25, 2021 11:10 AM > > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > Add clock bindings for fsl-imx-esdhc yaml > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 11 > +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > > b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > > index a7fbd8cc1e38..369471814496 100644 > > > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml > > > @@ -103,6 +103,17 @@ properties: > > > Only eMMC HS400 mode need to take care of this property. > > > default: 0 > > > > > > + clocks: > > > + maxItems: 3 > > > + description: > > > + Handle clocks for the sdhc controller. > > > + > > > + clock-names: > > > + items: > > > + - const: ipg > > > + - const: ahb > > > + - const: per > > > > One question: > > The side effect of this patch is that it imposes a forced order of clk > > names In DT which actually was not needed. > > > > Do we really have to do that? > > Yes. > > > Or any other better approach to allow a random order to match the DT > > usage better? > > Why do you need random order? > Thanks for the feedback. I thought the DT itself supports the random order of strings/names in a property and the OF API in kernel can also handle the random name order properly. That means DT binding don't enforce the order of names when people writing DTS. e.g. Order1: clock-names = "ipg", "per", "ahb" can function the same as: Order2: clock-names = "ipg", "ahb", "per"; However, the schema in this patch enforced the name order which caused dt binding check fail. https://lore.kernel.org/linux-arm-kernel/CAL_JsqKAOUKnVLvu-VNeDVg0ShXPy56wxhCQv38+rO2k961v+g@mail.gmail.com/#t And this seems like a common issue in kernel DT because DT supports random name order before. If we have to fix it, should we need fix them all in kernel? And finally, If all the names property are fixed by dt schema definition, the driver may also work without it by using index. > We can not enforce the order, but we only do that when there's multiple > optional entries. Understood, probably this is the simplest way to do a accurate DT schema checking. Regards Aisheng > > Rob
On Wed, Mar 03, 2021 at 03:00:57AM +0000, Peng Fan (OSS) wrote: > Hi Shawn, > > > Subject: Re: [PATCH V3 0/5] imx esdhc dt/driver update > > > > On Thu, 25 Feb 2021 at 04:22, <peng.fan@oss.nxp.com> wrote: > > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > V3: > > > Patch 1, drop unneeded pinctrl-0/1/2 > > > Patch 2 is new to avoid break dt bindings check > > > V2: > > > patch 1, 2, 3 is new > > > patch 4 is not changed > > > > > > > > > Peng Fan (5): > > > dt-bindings: mmc: fsl-imx-esdhc: add pinctrl bindings > > > dt-bindings: clock: imx8qxp-lpcg: correct the example clock-names > > > arm64: dts: imx8qxp: correct usdhc clock-names sequence > > > dt-bindings: mmc: fsl-imx-esdhc: add clock bindings > > > mmc: sdhci-esdhc-imx: validate pinctrl before use it > > > > > > .../bindings/clock/imx8qxp-lpcg.yaml | 6 +++--- > > > .../bindings/mmc/fsl-imx-esdhc.yaml | 20 > > +++++++++++++++++++ > > > arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 18 ++++++++--------- > > > drivers/mmc/host/sdhci-esdhc-imx.c | 2 +- > > > 4 files changed, 33 insertions(+), 13 deletions(-) > > > > > > -- > > > 2.30.0 > > > > > > > Applied patch 1, 4 and 5, thanks! > > > Would you pick patch 2,3? Done.
From: Peng Fan <peng.fan@nxp.com> V3: Patch 1, drop unneeded pinctrl-0/1/2 Patch 2 is new to avoid break dt bindings check V2: patch 1, 2, 3 is new patch 4 is not changed https://patchwork.kernel.org/project/linux-arm-kernel/cover/1611198593-16287-1-git-send-email-peng.fan@nxp.com/ Peng Fan (5): dt-bindings: mmc: fsl-imx-esdhc: add pinctrl bindings dt-bindings: clock: imx8qxp-lpcg: correct the example clock-names arm64: dts: imx8qxp: correct usdhc clock-names sequence dt-bindings: mmc: fsl-imx-esdhc: add clock bindings mmc: sdhci-esdhc-imx: validate pinctrl before use it .../bindings/clock/imx8qxp-lpcg.yaml | 6 +++--- .../bindings/mmc/fsl-imx-esdhc.yaml | 20 +++++++++++++++++++ arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 18 ++++++++--------- drivers/mmc/host/sdhci-esdhc-imx.c | 2 +- 4 files changed, 33 insertions(+), 13 deletions(-)