From: Peng Fan <[email protected]>
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/[email protected]/
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
From: Peng Fan <[email protected]>
Add pinctrl bindings for fsl-imx-esdhc yaml
Signed-off-by: Peng Fan <[email protected]>
---
Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index 802c9df23752..a7fbd8cc1e38 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -103,6 +103,15 @@ properties:
Only eMMC HS400 mode need to take care of this property.
default: 0
+ pinctrl-names:
+ minItems: 1
+ maxItems: 4
+ items:
+ - const: default
+ - const: state_100mhz
+ - const: state_200mhz
+ - const: sleep
+
required:
- compatible
- reg
--
2.30.0
From: Peng Fan <[email protected]>
Add clock bindings for fsl-imx-esdhc yaml
Signed-off-by: Peng Fan <[email protected]>
---
.../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
+
pinctrl-names:
minItems: 1
maxItems: 4
--
2.30.0
From: Peng Fan <[email protected]>
When imx_data->pinctrl is not a valid pointer, pinctrl_lookup_state
will trigger kernel panic.
When we boot Dual OS on Jailhouse hypervisor, we let the 1st Linux to
configure pinmux ready for the 2nd OS, so the 2nd OS not have pinctrl
settings.
Similar to this commit b62eee9f804e ("mmc: sdhci-esdhc-imx: no fail when no pinctrl available").
Reviewed-by: Bough Chen <[email protected]>
Reviewed-by: Alice Guo <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/mmc/host/sdhci-esdhc-imx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a20459744d21..94327988da91 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1488,7 +1488,7 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
mmc_of_parse_voltage(np, &host->ocr_mask);
- if (esdhc_is_usdhc(imx_data)) {
+ if (esdhc_is_usdhc(imx_data) && !IS_ERR(imx_data->pinctrl)) {
imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
ESDHC_PINCTRL_STATE_100MHZ);
imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
--
2.30.0
From: Peng Fan <[email protected]>
Align with all other i.MX using the mmc controller, align
the clock-names.
Signed-off-by: Peng Fan <[email protected]>
---
Documentation/devicetree/bindings/clock/imx8qxp-lpcg.yaml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.yaml b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.yaml
index 940486ef1051..0f6fe365ebf3 100644
--- a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.yaml
+++ b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.yaml
@@ -107,8 +107,8 @@ examples:
interrupts = <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b010000 0x10000>;
clocks = <&sdhc0_lpcg IMX_LPCG_CLK_4>,
- <&sdhc0_lpcg IMX_LPCG_CLK_0>,
- <&sdhc0_lpcg IMX_LPCG_CLK_5>;
- clock-names = "ipg", "per", "ahb";
+ <&sdhc0_lpcg IMX_LPCG_CLK_5>,
+ <&sdhc0_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_0>;
};
--
2.30.0
From: Peng Fan <[email protected]>
Per dt-bindings, the clock-names sequence should be ipg ahb per to pass
dtbs_check.
Signed-off-by: Peng Fan <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index e46faac1fe71..1d522de7b017 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -366,9 +366,9 @@ usdhc1: mmc@5b010000 {
interrupts = <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b010000 0x10000>;
clocks = <&conn_lpcg IMX_CONN_LPCG_SDHC0_IPG_CLK>,
- <&conn_lpcg IMX_CONN_LPCG_SDHC0_PER_CLK>,
- <&conn_lpcg IMX_CONN_LPCG_SDHC0_HCLK>;
- clock-names = "ipg", "per", "ahb";
+ <&conn_lpcg IMX_CONN_LPCG_SDHC0_HCLK>,
+ <&conn_lpcg IMX_CONN_LPCG_SDHC0_PER_CLK>;
+ clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_0>;
status = "disabled";
};
@@ -378,9 +378,9 @@ usdhc2: mmc@5b020000 {
interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b020000 0x10000>;
clocks = <&conn_lpcg IMX_CONN_LPCG_SDHC1_IPG_CLK>,
- <&conn_lpcg IMX_CONN_LPCG_SDHC1_PER_CLK>,
- <&conn_lpcg IMX_CONN_LPCG_SDHC1_HCLK>;
- clock-names = "ipg", "per", "ahb";
+ <&conn_lpcg IMX_CONN_LPCG_SDHC1_HCLK>,
+ <&conn_lpcg IMX_CONN_LPCG_SDHC1_PER_CLK>;
+ clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_1>;
fsl,tuning-start-tap = <20>;
fsl,tuning-step= <2>;
@@ -392,9 +392,9 @@ usdhc3: mmc@5b030000 {
interrupts = <GIC_SPI 234 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b030000 0x10000>;
clocks = <&conn_lpcg IMX_CONN_LPCG_SDHC2_IPG_CLK>,
- <&conn_lpcg IMX_CONN_LPCG_SDHC2_PER_CLK>,
- <&conn_lpcg IMX_CONN_LPCG_SDHC2_HCLK>;
- clock-names = "ipg", "per", "ahb";
+ <&conn_lpcg IMX_CONN_LPCG_SDHC2_HCLK>,
+ <&conn_lpcg IMX_CONN_LPCG_SDHC2_PER_CLK>;
+ clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_2>;
status = "disabled";
};
--
2.30.0
On Thu, 25 Feb 2021 at 04:22, <[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> 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/[email protected]/
>
> 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, <[email protected]> wrote:
> >
> > From: Peng Fan <[email protected]>
> >
> > 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 <[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> Add clock bindings for fsl-imx-esdhc yaml
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> .../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 <[email protected]> wrote:
> >
> > From: Peng Fan <[email protected]>
> >
> > Add clock bindings for fsl-imx-esdhc yaml
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > .../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) <[email protected]>
> Sent: Thursday, February 25, 2021 11:10 AM
>
> From: Peng Fan <[email protected]>
>
> Add clock bindings for fsl-imx-esdhc yaml
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> .../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 <[email protected]> wrote:
>
> Hi Rob,
>
> > From: Peng Fan (OSS) <[email protected]>
> > Sent: Thursday, February 25, 2021 11:10 AM
> >
> > From: Peng Fan <[email protected]>
> >
> > Add clock bindings for fsl-imx-esdhc yaml
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > .../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 <[email protected]>
> Sent: Friday, March 5, 2021 10:14 PM
>
> On Fri, Mar 5, 2021 at 8:09 AM Aisheng Dong <[email protected]>
> wrote:
> >
> > Hi Rob,
> >
> > > From: Peng Fan (OSS) <[email protected]>
> > > Sent: Thursday, February 25, 2021 11:10 AM
> > >
> > > From: Peng Fan <[email protected]>
> > >
> > > Add clock bindings for fsl-imx-esdhc yaml
> > >
> > > Signed-off-by: Peng Fan <[email protected]>
> > > ---
> > > .../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 Thu, 25 Feb 2021 11:10:01 +0800, [email protected] wrote:
> From: Peng Fan <[email protected]>
>
> Align with all other i.MX using the mmc controller, align
> the clock-names.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/imx8qxp-lpcg.yaml | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Acked-by: Rob Herring <[email protected]>
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, <[email protected]> wrote:
> > >
> > > From: Peng Fan <[email protected]>
> > >
> > > 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.