| Message ID | 20211102225701.98944-1-Mr.Bossman075@gmail.com |
|---|---|
| Headers | show |
| Series | Add initial support for the i.MXRTxxxx SoC family starting from i.IMXRT1050 SoC. | expand |
Hi Jesse, On 11/2/21 11:56 PM, Jesse Taube wrote: > From: Jesse Taube <mr.bossman075@gmail.com> > > Add support for i.MXRT1050's sdhc. > > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>--- Here the 3 dashes are missing(---) Best regards
On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > static struct esdhc_soc_data usdhc_imx8qxp_data = { > .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING > @@ -357,6 +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { > { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, > { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, > { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, > + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, I thought Rob suggested to use the SoC name, so this would be: { .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, }, The same applies to the other bindings in the series. This way it would be possible to differentiate between future supported i.MX RT devices.
On 11/2/21 19:17, Fabio Estevam wrote: > On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > >> static struct esdhc_soc_data usdhc_imx8qxp_data = { >> .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING >> @@ -357,6 +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { >> { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, >> { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, >> { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, >> + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, > > I thought Rob suggested to use the SoC name, so this would be: > Uh i think that may have been for the UART. > { .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, }, > > The same applies to the other bindings in the series. > > This way it would be possible to differentiate between future > supported i.MX RT devices. > This makes sense will do in V3.
Hi Fabio, Jesse, All, On 11/3/21 12:25 AM, Jesse Taube wrote: > > > On 11/2/21 19:17, Fabio Estevam wrote: >> On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: >> >>> static struct esdhc_soc_data usdhc_imx8qxp_data = { >>> .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING >>> @@ -357,6 +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { >>> { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, >>> { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, >>> { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, >>> + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, >> >> I thought Rob suggested to use the SoC name, so this would be: >> > Uh i think that may have been for the UART. >> { .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, }, >> >> The same applies to the other bindings in the series. >> >> This way it would be possible to differentiate between future >> supported i.MX RT devices. >> > This makes sense will do in V3. > If we add every SoC we will end up having a long list for every device driver. At the moment it would be 7 parts: 1) imxrt1020 2) imxrt1024 . . . 7) imxrt1170 Is it ok anyway? Best regards
On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > +#include "clk.h" > +#define ANATOP_BASE_ADDR 0x400d8000 This is now unused. Please remove it. > + clk[IMXRT1050_CLK_USDHC1] = imx_clk_gate2("usdhc1", "usdhc1_podf", ccm_base + 0x80, 2); > + clk[IMXRT1050_CLK_USDHC2] = imx_clk_gate2("usdhc2", "usdhc2_podf", ccm_base + 0x80, 4); > + clk[IMXRT1050_CLK_LPUART1] = imx_clk_gate2("lpuart1", "lpuart_podf", ccm_base + 0x7c, 24); > + clk[IMXRT1050_CLK_LCDIF_APB] = imx_clk_gate2("lcdif", "lcdif_podf", ccm_base + 0x74, 10); > + clk[IMXRT1050_CLK_DMA] = imx_clk_gate("dma", "ipg", ccm_base + 0x7C, 6); > + clk[IMXRT1050_CLK_DMA_MUX] = imx_clk_gate("dmamux0", "ipg", ccm_base + 0x7C, 7); The imx clock drivers have been converted to the clk_hw API. For a reference, please check: f1541e15e38e ("clk: imx6sx: Switch to clk_hw based API") The same conversion could be done here. > + imx_check_clocks(clk, ARRAY_SIZE(clk)); > + clk_data.clks = clk; > + clk_data.clk_num = ARRAY_SIZE(clk); > + of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL1_ARM]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL2_SYS]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL3_USB_OTG]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL3_PFD1_664_62M]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL2_PFD2_396M]); If these clocks are essential for the SoC to work, then you could pass the CLK_IS_CRITICAL flag instead of calling clk_prepare_enable()
On 11/2/21 19:34, Fabio Estevam wrote: > On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > >> +#include "clk.h" This is necessary for the indices. >> +#define ANATOP_BASE_ADDR 0x400d8000 OOPs my bad will fix. > > This is now unused. Please remove it. > >> + clk[IMXRT1050_CLK_USDHC1] = imx_clk_gate2("usdhc1", "usdhc1_podf", ccm_base + 0x80, 2); >> + clk[IMXRT1050_CLK_USDHC2] = imx_clk_gate2("usdhc2", "usdhc2_podf", ccm_base + 0x80, 4); >> + clk[IMXRT1050_CLK_LPUART1] = imx_clk_gate2("lpuart1", "lpuart_podf", ccm_base + 0x7c, 24); >> + clk[IMXRT1050_CLK_LCDIF_APB] = imx_clk_gate2("lcdif", "lcdif_podf", ccm_base + 0x74, 10); >> + clk[IMXRT1050_CLK_DMA] = imx_clk_gate("dma", "ipg", ccm_base + 0x7C, 6); >> + clk[IMXRT1050_CLK_DMA_MUX] = imx_clk_gate("dmamux0", "ipg", ccm_base + 0x7C, 7); > > The imx clock drivers have been converted to the clk_hw API. > Oh will do, didn't know this. > For a reference, please check: > f1541e15e38e ("clk: imx6sx: Switch to clk_hw based API") > > The same conversion could be done here. > >> + imx_check_clocks(clk, ARRAY_SIZE(clk)); >> + clk_data.clks = clk; >> + clk_data.clk_num = ARRAY_SIZE(clk); >> + of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); >> + clk_prepare_enable(clk[IMXRT1050_CLK_PLL1_ARM]); >> + clk_prepare_enable(clk[IMXRT1050_CLK_PLL2_SYS]); >> + clk_prepare_enable(clk[IMXRT1050_CLK_PLL3_USB_OTG]); >> + clk_prepare_enable(clk[IMXRT1050_CLK_PLL3_PFD1_664_62M]); >> + clk_prepare_enable(clk[IMXRT1050_CLK_PLL2_PFD2_396M]); > > If these clocks are essential for the SoC to work, then you could pass > the CLK_IS_CRITICAL flag instead of calling clk_prepare_enable() > I'll look into that Thx
On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > +/ { > + model = "NXP IMXRT1050-evk board"; > + compatible = "fsl,imxrt1050-evk", "fsl,imxrt1050"; > + > + chosen { > + bootargs = "root=/dev/ram"; No need to pass bootargs here. > + stdout-path = &lpuart1; > + }; > + > + aliases { > + gpio0 = &gpio1; > + gpio1 = &gpio2; > + gpio2 = &gpio3; > + gpio3 = &gpio4; > + gpio4 = &gpio5; > + mmc0 = &usdhc1; > + serial0 = &lpuart1; > + }; > + > + memory@0 { memory@80000000 Building with W=1 should give a dtc warning due to the unit address and reg mismatch. > + device_type = "memory"; > + reg = <0x80000000 0x2000000>; > + }; > + Unneeded blank line. > + > +&iomuxc { > + pinctrl-names = "default"; > + > + imxrt1050-evk { No need for this imxrt1050-evk container. > + pinctrl_lpuart1: lpuart1grp { > + fsl,pins = < > + MXRT1050_IOMUXC_GPIO_AD_B0_12_LPUART1_TXD > + 0xf1 Put it on a single line. It helps readability. Same applies globally. > +&usdhc1 { > + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep"; > + pinctrl-0 = <&pinctrl_usdhc0>; > + pinctrl-1 = <&pinctrl_usdhc0>; > + pinctrl-2 = <&pinctrl_usdhc0>; > + pinctrl-3 = <&pinctrl_usdhc0>; > + status = "okay"; > + > + cd-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>; Make 'status' to be the last property. Remove the blank line. > + edma1: dma-controller@400e8000 { > + #dma-cells = <2>; > + compatible = "fsl,imx7ulp-edma"; > + reg = <0x400e8000 0x4000>, > + <0x400ec000 0x4000>; > + dma-channels = <32>; > + interrupts = <0>, > + <1>, > + <2>, > + <3>, > + <4>, > + <5>, > + <6>, > + <7>, > + <8>, > + <9>, > + <10>, > + <11>, > + <12>, > + <13>, > + <14>, > + <15>, > + <16>; Please group more elements into the same line. Putting one entry per line makes it extremely long. > + gpio5: gpio@400c0000 { > + compatible = "fsl,imxrt-gpio", "fsl,imx35-gpio"; > + reg = <0x400c0000 0x4000>; > + interrupts = <88>, > + <89>; Put the interrupts into a single line.
On 11/2/21 19:42, Fabio Estevam wrote: > On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > >> +/ { >> + model = "NXP IMXRT1050-evk board"; >> + compatible = "fsl,imxrt1050-evk", "fsl,imxrt1050"; >> + >> + chosen { >> + bootargs = "root=/dev/ram"; > > No need to pass bootargs here. > >> + stdout-path = &lpuart1; >> + }; >> + >> + aliases { >> + gpio0 = &gpio1; >> + gpio1 = &gpio2; >> + gpio2 = &gpio3; >> + gpio3 = &gpio4; >> + gpio4 = &gpio5; >> + mmc0 = &usdhc1; >> + serial0 = &lpuart1; >> + }; >> + >> + memory@0 { > > memory@80000000 > > Building with W=1 should give a dtc warning due to the unit address > and reg mismatch. Oh that makes sense. I guess I'm going to have to figure out how to get warnings to work as I couldn't last time I tried. > >> + device_type = "memory"; >> + reg = <0x80000000 0x2000000>; >> + }; >> + > > Unneeded blank line. >> + >> +&iomuxc { >> + pinctrl-names = "default"; >> + >> + imxrt1050-evk { > > No need for this imxrt1050-evk container. I was wondering if that was needed, u-boot has it, good to know. > >> + pinctrl_lpuart1: lpuart1grp { >> + fsl,pins = < >> + MXRT1050_IOMUXC_GPIO_AD_B0_12_LPUART1_TXD >> + 0xf1 > > Put it on a single line. It helps readability. Same applies globally. >> +&usdhc1 { >> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep"; >> + pinctrl-0 = <&pinctrl_usdhc0>; >> + pinctrl-1 = <&pinctrl_usdhc0>; >> + pinctrl-2 = <&pinctrl_usdhc0>; >> + pinctrl-3 = <&pinctrl_usdhc0>; >> + status = "okay"; >> + >> + cd-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>; > > Make 'status' to be the last property. Remove the blank line. > >> + edma1: dma-controller@400e8000 { >> + #dma-cells = <2>; >> + compatible = "fsl,imx7ulp-edma"; >> + reg = <0x400e8000 0x4000>, >> + <0x400ec000 0x4000>; >> + dma-channels = <32>; >> + interrupts = <0>, >> + <1>, >> + <2>, >> + <3>, >> + <4>, >> + <5>, >> + <6>, >> + <7>, >> + <8>, >> + <9>, >> + <10>, >> + <11>, >> + <12>, >> + <13>, >> + <14>, >> + <15>, >> + <16>; > > Please group more elements into the same line. > > Putting one entry per line makes it extremely long. > >> + gpio5: gpio@400c0000 { >> + compatible = "fsl,imxrt-gpio", "fsl,imx35-gpio"; >> + reg = <0x400c0000 0x4000>; >> + interrupts = <88>, >> + <89>; > > Put the interrupts into a single line. > Ah all these make sense, will fix, sry about that.
On Tue, 02 Nov 2021 18:56:53 -0400, Jesse Taube wrote: > From: Jesse Taube <mr.bossman075@gmail.com> > > Add DT binding documentation for i.MXRT clock driver. > > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> > --- > V1->V2: > * Replace macros with values > --- > .../bindings/clock/imxrt-clock.yaml | 70 +++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/imxrt-clock.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/clock/imxrt-clock.example.dt.yaml:0:0: /example-0/anatop@400d8000: failed to match any schema with compatible: ['fsl,imxrt-anatop'] /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/imxrt-clock.example.dt.yaml: ccm@400fc000: interrupts: [[95], [96]] is too long From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/imxrt-clock.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/imxrt-clock.example.dt.yaml: timer@401ec000: clocks: [[4294967295, 3]] is too short From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/imxrt-clock.example.dt.yaml: timer@401ec000: clock-names:0: 'ipg' was expected From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/imxrt-clock.example.dt.yaml: timer@401ec000: clock-names: ['per'] is too short From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/timer/fsl,imxgpt.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1549993 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Tue, 02 Nov 2021 18:56:50 -0400, Jesse Taube wrote: > From: Jesse Taube <mr.bossman075@gmail.com> > > Add i.MXRT1050 pinctrl binding doc > > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> > --- > V1->V2: > * Replace macros with values > * Add tab for last pinctrl value > --- > .../bindings/pinctrl/fsl,imxrt1050.yaml | 83 +++++++++++++++++++ > 1 file changed, 83 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/pinctrl/fsl,imxrt1050.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.example.dt.yaml: iomuxc@401f8000: 'fsl,mux_mask', 'imxrt1050-evk' do not match any of the regexes: 'grp$', 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.example.dt.yaml: iomuxc@401f8000: 'pinctrl-0' is a dependency of 'pinctrl-names' From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl-consumer.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1549987 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On 11/2/21 21:20, Rob Herring wrote: > On Tue, 02 Nov 2021 18:56:50 -0400, Jesse Taube wrote: >> From: Jesse Taube <mr.bossman075@gmail.com> >> >> Add i.MXRT1050 pinctrl binding doc >> >> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> >> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> >> --- >> V1->V2: >> * Replace macros with values >> * Add tab for last pinctrl value >> --- >> .../bindings/pinctrl/fsl,imxrt1050.yaml | 83 +++++++++++++++++++ >> 1 file changed, 83 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.yaml >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > Ah I thought it would stop make at error i see it now, is there a way to do one file. > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > ./Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.yaml: $id: relative path/filename doesn't match actual path or filename > expected: http://devicetree.org/schemas/pinctrl/fsl,imxrt1050.yaml# > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.example.dt.yaml: iomuxc@401f8000: 'fsl,mux_mask', 'imxrt1050-evk' do not match any of the regexes: 'grp$', 'pinctrl-[0-9]+' > From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.yaml > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pinctrl/fsl,imxrt1050.example.dt.yaml: iomuxc@401f8000: 'pinctrl-0' is a dependency of 'pinctrl-names' > From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/pinctrl/pinctrl-consumer.yaml > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/patch/1549987 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. >
On 11/2/21 19:54, Jesse Taube wrote: > > > On 11/2/21 19:42, Fabio Estevam wrote: >> On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: >> >>> +/ { >>> + model = "NXP IMXRT1050-evk board"; >>> + compatible = "fsl,imxrt1050-evk", "fsl,imxrt1050"; >>> + >>> + chosen { >>> + bootargs = "root=/dev/ram"; >> >> No need to pass bootargs here. >> >>> + stdout-path = &lpuart1; >>> + }; >>> + >>> + aliases { >>> + gpio0 = &gpio1; >>> + gpio1 = &gpio2; >>> + gpio2 = &gpio3; >>> + gpio3 = &gpio4; >>> + gpio4 = &gpio5; >>> + mmc0 = &usdhc1; >>> + serial0 = &lpuart1; >>> + }; >>> + >>> + memory@0 { >> >> memory@80000000 >> >> Building with W=1 should give a dtc warning due to the unit address >> and reg mismatch. > Oh that makes sense. > I guess I'm going to have to figure out how to get warnings to work as I > couldn't last time I tried. Oh i got it to work I did something dumb... I didn't give a warning for this i still changed it of course. >> >>> + device_type = "memory"; >>> + reg = <0x80000000 0x2000000>; >>> + }; >>> + >> >> Unneeded blank line. >>> + >>> +&iomuxc { >>> + pinctrl-names = "default"; >>> + >>> + imxrt1050-evk { >> >> No need for this imxrt1050-evk container. > I was wondering if that was needed, u-boot has it, good to know. >> >>> + pinctrl_lpuart1: lpuart1grp { >>> + fsl,pins = < >>> + MXRT1050_IOMUXC_GPIO_AD_B0_12_LPUART1_TXD >>> + 0xf1 >> >> Put it on a single line. It helps readability. Same applies globally. >>> +&usdhc1 { >>> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep"; >>> + pinctrl-0 = <&pinctrl_usdhc0>; >>> + pinctrl-1 = <&pinctrl_usdhc0>; >>> + pinctrl-2 = <&pinctrl_usdhc0>; >>> + pinctrl-3 = <&pinctrl_usdhc0>; >>> + status = "okay"; >>> + >>> + cd-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>; >> >> Make 'status' to be the last property. Remove the blank line. >> >>> + edma1: dma-controller@400e8000 { >>> + #dma-cells = <2>; >>> + compatible = "fsl,imx7ulp-edma"; >>> + reg = <0x400e8000 0x4000>, >>> + <0x400ec000 0x4000>; >>> + dma-channels = <32>; >>> + interrupts = <0>, >>> + <1>, >>> + <2>, >>> + <3>, >>> + <4>, >>> + <5>, >>> + <6>, >>> + <7>, >>> + <8>, >>> + <9>, >>> + <10>, >>> + <11>, >>> + <12>, >>> + <13>, >>> + <14>, >>> + <15>, >>> + <16>; >> >> Please group more elements into the same line. >> >> Putting one entry per line makes it extremely long. >> >>> + gpio5: gpio@400c0000 { >>> + compatible = "fsl,imxrt-gpio", "fsl,imx35-gpio"; >>> + reg = <0x400c0000 0x4000>; >>> + interrupts = <88>, >>> + <89>; >> >> Put the interrupts into a single line. >> > Ah all these make sense, will fix, sry about that. >
> -----Original Message----- > From: Jesse Taube [mailto:mr.bossman075@gmail.com] > Sent: 2021年11月3日 6:57 > To: dl-linux-imx <linux-imx@nxp.com> > Cc: mturquette@baylibre.com; sboyd@kernel.org; robh+dt@kernel.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; > festevam@gmail.com; ulf.hansson@linaro.org; Aisheng Dong > <aisheng.dong@nxp.com>; stefan@agner.ch; linus.walleij@linaro.org; > gregkh@linuxfoundation.org; arnd@arndb.de; olof@lixom.net; > soc@kernel.org; linux@armlinux.org.uk; Abel Vesa <abel.vesa@nxp.com>; > adrian.hunter@intel.com; jirislaby@kernel.org; > giulio.benetti@benettiengineering.com; nobuhiro1.iwamatsu@toshiba.co.jp; > Mr.Bossman075@gmail.com; linux-clk@vger.kernel.org; > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; > linux-gpio@vger.kernel.org; linux-serial@vger.kernel.org; Jesse Taube > <mr.bossman075@gmail.com> > Subject: [PATCH v2 11/13] mmc: sdhci-esdhc-imx: Add sdhc support for i.MXRT > series > > From: Jesse Taube <mr.bossman075@gmail.com> > > Add support for i.MXRT1050's sdhc. > > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>--- > V1->V2: > * Nothing done > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > b/drivers/mmc/host/sdhci-esdhc-imx.c > index afaf33707d46..c852a6df5611 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -305,6 +305,12 @@ static struct esdhc_soc_data usdhc_imx7ulp_data = { > | ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400 > | ESDHC_FLAG_STATE_LOST_IN_LPMODE, > }; > +static struct esdhc_soc_data usdhc_imxrt_data = { > + .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING > + | ESDHC_FLAG_HS200 | ESDHC_FLAG_ERR004536 > + | ESDHC_FLAG_BROKEN_AUTO_CMD23, > +}; > + Hi Jesse, I have few question here. Why only use manual tuning here? Does this usdhc don't support standard tuning? or meet any issue when use standard tuning? Please also double check why not use ADMA in default? Any issue found? Best Regards Haibo Chen > > static struct esdhc_soc_data usdhc_imx8qxp_data = { > .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING @@ -357,6 > +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { > { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, > { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, > { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, > + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, > { .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, }, > { /* sentinel */ } > }; > -- > 2.33.1
Hi Giulio, On Tue, Nov 2, 2021 at 8:30 PM Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > If we add every SoC we will end up having a long list for every device > driver. At the moment it would be 7 parts: > 1) imxrt1020 > 2) imxrt1024 > . > . > . > 7) imxrt1170 > > Is it ok anyway? As this patch adds the support for imxrt1050, I would go with "fsl,imxrt1050-usdhc" for now.
Hi Fabio, Jesse, Rob, All, On 11/3/21 6:51 PM, Fabio Estevam wrote: > Hi Giulio, > > On Tue, Nov 2, 2021 at 8:30 PM Giulio Benetti > <giulio.benetti@benettiengineering.com> wrote: > >> If we add every SoC we will end up having a long list for every device >> driver. At the moment it would be 7 parts: >> 1) imxrt1020 >> 2) imxrt1024 >> . >> . >> . >> 7) imxrt1170 >> >> Is it ok anyway? > > As this patch adds the support for imxrt1050, I would go with > "fsl,imxrt1050-usdhc" for now. > Ok, then it's the same as pointed by Rob for lpuart[1]; @Jesse: we will do the same for all peripherals(more or less) since it seems there are little differences in the i.MXRT family. [1]: https://lore.kernel.org/lkml/D0A3E11F-FEDE-4B2D-90AB-63DFC245A935@benettiengineering.com/T/ Thanks a lot Best regards
On Wed, Nov 03, 2021 at 12:30:17AM +0100, Giulio Benetti wrote: > Hi Fabio, Jesse, All, > > On 11/3/21 12:25 AM, Jesse Taube wrote: > > > > > > On 11/2/21 19:17, Fabio Estevam wrote: > > > On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > > > > > > > static struct esdhc_soc_data usdhc_imx8qxp_data = { > > > > .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING > > > > @@ -357,6 +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { > > > > { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, > > > > { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, > > > > { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, > > > > + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, > > > > > > I thought Rob suggested to use the SoC name, so this would be: > > > > > Uh i think that may have been for the UART. > > > { .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, }, > > > > > > The same applies to the other bindings in the series. > > > > > > This way it would be possible to differentiate between future > > > supported i.MX RT devices. > > > > > This makes sense will do in V3. > > > > If we add every SoC we will end up having a long list for every device > driver. At the moment it would be 7 parts: > 1) imxrt1020 > 2) imxrt1024 > . > . > . > 7) imxrt1170 You don't need a driver update if you use a fallback. When you add the 2nd chip, if you think it is 'the same', then you do: compatible = "fsl,imxrt1024-usdhc", "fsl,imxrt1050-usdhc"; That requires no driver update until the driver needs to handle some difference. And when there is a difference, you don't need a DT update. You could make "fsl,imxrt-usdhc" the fallback from the start if you are adverse to the first way. Rob
Hello Rob, Jesse, All, > Il giorno 4 nov 2021, alle ore 02:05, Rob Herring <robh@kernel.org> ha scritto: > > On Wed, Nov 03, 2021 at 12:30:17AM +0100, Giulio Benetti wrote: >> Hi Fabio, Jesse, All, >> >>> On 11/3/21 12:25 AM, Jesse Taube wrote: >>> >>> >>> On 11/2/21 19:17, Fabio Estevam wrote: >>>> On Tue, Nov 2, 2021 at 7:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: >>>> >>>>> static struct esdhc_soc_data usdhc_imx8qxp_data = { >>>>> .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING >>>>> @@ -357,6 +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { >>>>> { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, >>>>> { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, >>>>> { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, >>>>> + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, >>>> >>>> I thought Rob suggested to use the SoC name, so this would be: >>>> >>> Uh i think that may have been for the UART. >>>> { .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, }, >>>> >>>> The same applies to the other bindings in the series. >>>> >>>> This way it would be possible to differentiate between future >>>> supported i.MX RT devices. >>>> >>> This makes sense will do in V3. >>> >> >> If we add every SoC we will end up having a long list for every device >> driver. At the moment it would be 7 parts: >> 1) imxrt1020 >> 2) imxrt1024 >> . >> . >> . >> 7) imxrt1170 > > You don't need a driver update if you use a fallback. When you add > the 2nd chip, if you think it is 'the same', then you do: > > compatible = "fsl,imxrt1024-usdhc", "fsl,imxrt1050-usdhc"; > > That requires no driver update until the driver needs to handle some > difference. And when there is a difference, you don't need a DT update. This solution is pretty fine, we’re going with that then, for this and every driver involved. Thank you for pointing us. Best regards Giulio Benetti Benetti Engineering sas > > You could make "fsl,imxrt-usdhc" the fallback from the start if you are > adverse to the first way. > > Rob
Hi Jesse, All, this is a specific driver for imxrt1050, for example imxrt1020 is totally different while imxrt1060 is similar, so commit log needs to change to i.MXRT1050, On 11/2/21 11:56 PM, Jesse Taube wrote: > From: Jesse Taube <mr.bossman075@gmail.com> > > This patch adds initial clock driver support for the i.MXRT series. Here too ^^^ > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> > Suggested-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > --- > V1->V2: > * Kconfig: Add new line > * clk-imxrt.c: Remove unused const > * clk-imxrt.c: Remove set parents > * clk-imxrt.c: Use fsl,imxrt-anatop for anatop base address > --- > drivers/clk/imx/Kconfig | 4 + > drivers/clk/imx/Makefile | 1 + > drivers/clk/imx/clk-imxrt.c | 149 ++++++++++++++++++++++++++++++++++++ Here we need this file ^^^ to be clk-imxrt1050.c > 3 files changed, 154 insertions(+) > create mode 100644 drivers/clk/imx/clk-imxrt.c > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig > index 47d9ec3abd2f..f83ba5fe8cd3 100644 > --- a/drivers/clk/imx/Kconfig > +++ b/drivers/clk/imx/Kconfig > @@ -98,3 +98,7 @@ config CLK_IMX8QXP > select MXC_CLK_SCU > help > Build the driver for IMX8QXP SCU based clocks. > + > +config CLK_IMXRT CLK_IMXRT1050 > + def_bool SOC_IMXRT > + select MXC_CLK > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > index c24a2acbfa56..6a3fee6cd9af 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -45,3 +45,4 @@ obj-$(CONFIG_CLK_IMX6UL) += clk-imx6ul.o > obj-$(CONFIG_CLK_IMX7D) += clk-imx7d.o > obj-$(CONFIG_CLK_IMX7ULP) += clk-imx7ulp.o > obj-$(CONFIG_CLK_VF610) += clk-vf610.o > +obj-$(CONFIG_CLK_IMXRT) += clk-imxrt.o CONFIG_CLK_IMXRT1050 > diff --git a/drivers/clk/imx/clk-imxrt.c b/drivers/clk/imx/clk-imxrt.c > new file mode 100644 > index 000000000000..8e235925cdb7 > --- /dev/null > +++ b/drivers/clk/imx/clk-imxrt.c > @@ -0,0 +1,149 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) > +/* > + * Copyright (C) 2021 > + * Author(s): > + * Jesse Taube <Mr.Bossman075@gmail.com> > + * Giulio Benetti <giulio.benetti@benettiengineering.com> > + */ > +#include <linux/mm.h> > +#include <linux/delay.h> > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/clkdev.h> > +#include <linux/clk-provider.h> > +#include <linux/err.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/sizes.h> > +#include <soc/imx/revision.h> > +#include <dt-bindings/clock/imxrt1050-clock.h> Indeed here the include is imxrt1050-clock.h and every macro below starts with IMXRT1050_ prefix > + > +#include "clk.h" > +#define ANATOP_BASE_ADDR 0x400d8000 > + > +static const char * const pll_ref_sels[] = {"osc", "dummy", }; > +static const char * const per_sels[] = {"ipg_pdof", "osc", }; > +static const char * const pll1_bypass_sels[] = {"pll1_arm", "pll1_arm_ref_sel", }; > +static const char * const pll2_bypass_sels[] = {"pll2_sys", "pll2_sys_ref_sel", }; > +static const char * const pll3_bypass_sels[] = {"pll3_usb_otg", "pll3_usb_otg_ref_sel", }; > +static const char * const pll5_bypass_sels[] = {"pll5_video", "pll5_video_ref_sel", }; > +static const char *const pre_periph_sels[] = { > + "pll2_sys", "pll2_pfd2_396m", "pll2_pfd0_352m", "arm_podf", }; > +static const char *const periph_sels[] = { "pre_periph_sel", "todo", }; > +static const char *const usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", }; > +static const char *const lpuart_sels[] = { "pll3_80m", "osc", }; > +static const char *const lcdif_sels[] = { > + "pll2_sys", "pll3_pfd3_454_74m", "pll5_video", "pll2_pfd0_352m", > + "pll2_pfd1_594m", "pll3_pfd1_664_62m", }; > + > +static struct clk *clk[IMXRT1050_CLK_END]; > +static struct clk_onecell_data clk_data; > + > +static void __init imxrt_clocks_common_init(void __iomem *base) > +{ > + /* Anatop clocks */ > + clk[IMXRT1050_CLK_DUMMY] = imx_clk_fixed("dummy", 0UL); > + > + clk[IMXRT1050_CLK_PLL1_REF_SEL] = imx_clk_mux("pll1_arm_ref_sel", > + base + 0x0, 14, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)); > + clk[IMXRT1050_CLK_PLL2_REF_SEL] = imx_clk_mux("pll2_sys_ref_sel", > + base + 0x30, 14, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)); > + clk[IMXRT1050_CLK_PLL3_REF_SEL] = imx_clk_mux("pll3_usb_otg_ref_sel", > + base + 0x10, 14, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)); > + clk[IMXRT1050_CLK_PLL5_REF_SEL] = imx_clk_mux("pll5_video_ref_sel", > + base + 0xa0, 14, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels)); > + > + clk[IMXRT1050_CLK_PLL1_ARM] = imx_clk_pllv3(IMX_PLLV3_SYS, "pll1_arm", > + "pll1_arm_ref_sel", base + 0x0, 0x7f); > + clk[IMXRT1050_CLK_PLL2_SYS] = imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2_sys", > + "pll2_sys_ref_sel", base + 0x30, 0x1); > + clk[IMXRT1050_CLK_PLL3_USB_OTG] = imx_clk_pllv3(IMX_PLLV3_USB, "pll3_usb_otg", > + "pll3_usb_otg_ref_sel", base + 0x10, 0x1); > + clk[IMXRT1050_CLK_PLL5_VIDEO] = imx_clk_pllv3(IMX_PLLV3_AV, "pll5_video", > + "pll5_video_ref_sel", base + 0xa0, 0x7f); > + > + /* PLL bypass out */ > + clk[IMXRT1050_CLK_PLL1_BYPASS] = imx_clk_mux_flags("pll1_bypass", base + 0x0, 16, 1, > + pll1_bypass_sels, ARRAY_SIZE(pll1_bypass_sels), CLK_SET_RATE_PARENT); > + clk[IMXRT1050_CLK_PLL2_BYPASS] = imx_clk_mux_flags("pll2_bypass", base + 0x30, 16, 1, > + pll2_bypass_sels, ARRAY_SIZE(pll2_bypass_sels), CLK_SET_RATE_PARENT); > + clk[IMXRT1050_CLK_PLL3_BYPASS] = imx_clk_mux_flags("pll3_bypass", base + 0x10, 16, 1, > + pll3_bypass_sels, ARRAY_SIZE(pll3_bypass_sels), CLK_SET_RATE_PARENT); > + clk[IMXRT1050_CLK_PLL5_BYPASS] = imx_clk_mux_flags("pll5_bypass", base + 0xa0, 16, 1, > + pll5_bypass_sels, ARRAY_SIZE(pll5_bypass_sels), CLK_SET_RATE_PARENT); > + > + clk[IMXRT1050_CLK_VIDEO_POST_DIV_SEL] = imx_clk_divider("video_post_div_sel", > + "pll5_video", base + 0xa0, 19, 2); > + clk[IMXRT1050_CLK_VIDEO_DIV] = imx_clk_divider("video_div", > + "video_post_div_sel", base + 0x170, 30, 2); > + > + clk[IMXRT1050_CLK_PLL3_80M] = imx_clk_fixed_factor("pll3_80m", "pll3_usb_otg", 1, 6); > + > + clk[IMXRT1050_CLK_PLL2_PFD0_352M] = imx_clk_pfd("pll2_pfd0_352m", "pll2_sys", base + 0x100, 0); > + clk[IMXRT1050_CLK_PLL2_PFD1_594M] = imx_clk_pfd("pll2_pfd1_594m", "pll2_sys", base + 0x100, 1); > + clk[IMXRT1050_CLK_PLL2_PFD2_396M] = imx_clk_pfd("pll2_pfd2_396m", "pll2_sys", base + 0x100, 2); > + clk[IMXRT1050_CLK_PLL3_PFD1_664_62M] = imx_clk_pfd("pll3_pfd1_664_62m", "pll3_usb_otg", base + 0xf0, 1); > + clk[IMXRT1050_CLK_PLL3_PFD3_454_74M] = imx_clk_pfd("pll3_pfd3_454_74m", "pll3_usb_otg", base + 0xf0, 3); > +} > + > +static void __init imxrt1050_clocks_init(struct device_node *np) > +{ > + void __iomem *ccm_base; > + void __iomem *pll_base; > + struct device_node *anp; > + > + clk[IMXRT1050_CLK_OSC] = of_clk_get_by_name(np, "osc"); > + > + anp = of_find_compatible_node(NULL, NULL, "fsl,imxrt-anatop"); > + pll_base = of_iomap(anp, 0); > + WARN_ON(!pll_base); > + imxrt_clocks_common_init(pll_base); > + /* CCM clocks */ > + ccm_base = of_iomap(np, 0); > + WARN_ON(!ccm_base); > + > + clk[IMXRT1050_CLK_ARM_PODF] = imx_clk_divider("arm_podf", "pll1_arm", ccm_base + 0x10, 0, 3); > + clk[IMXRT1050_CLK_PRE_PERIPH_SEL] = imx_clk_mux("pre_periph_sel", ccm_base + 0x18, 18, 2, > + pre_periph_sels, ARRAY_SIZE(pre_periph_sels)); > + clk[IMXRT1050_CLK_PERIPH_SEL] = imx_clk_mux("periph_sel", ccm_base + 0x14, 25, 1, > + periph_sels, ARRAY_SIZE(periph_sels)); > + clk[IMXRT1050_CLK_USDHC1_SEL] = imx_clk_mux("usdhc1_sel", ccm_base + 0x1c, 16, 1, > + usdhc_sels, ARRAY_SIZE(usdhc_sels)); > + clk[IMXRT1050_CLK_USDHC2_SEL] = imx_clk_mux("usdhc2_sel", ccm_base + 0x1c, 17, 1, > + usdhc_sels, ARRAY_SIZE(usdhc_sels)); > + clk[IMXRT1050_CLK_LPUART_SEL] = imx_clk_mux("lpuart_sel", ccm_base + 0x24, 6, 1, > + lpuart_sels, ARRAY_SIZE(lpuart_sels)); > + clk[IMXRT1050_CLK_LCDIF_SEL] = imx_clk_mux("lcdif_sel", ccm_base + 0x38, 15, 3, > + lcdif_sels, ARRAY_SIZE(lcdif_sels)); > + clk[IMXRT1050_CLK_PER_CLK_SEL] = imx_clk_mux("per_sel", ccm_base + 0x1C, 6, 1, > + per_sels, ARRAY_SIZE(per_sels)); > + > + clk[IMXRT1050_CLK_AHB_PODF] = imx_clk_divider("ahb", "periph_sel", ccm_base + 0x14, 10, 3); > + clk[IMXRT1050_CLK_IPG_PDOF] = imx_clk_divider("ipg", "ahb", ccm_base + 0x14, 8, 2); > + clk[IMXRT1050_CLK_PER_PDOF] = imx_clk_divider("per", "per_sel", ccm_base + 0x1C, 0, 5); > + > + clk[IMXRT1050_CLK_USDHC1_PODF] = imx_clk_divider("usdhc1_podf", "usdhc1_sel", ccm_base + 0x24, 11, 3); > + clk[IMXRT1050_CLK_USDHC2_PODF] = imx_clk_divider("usdhc2_podf", "usdhc2_sel", ccm_base + 0x24, 16, 3); > + clk[IMXRT1050_CLK_LPUART_PODF] = imx_clk_divider("lpuart_podf", "lpuart_sel", ccm_base + 0x24, 0, 6); > + clk[IMXRT1050_CLK_LCDIF_PRED] = imx_clk_divider("lcdif_pred", "lcdif_sel", ccm_base + 0x38, 12, 3); > + clk[IMXRT1050_CLK_LCDIF_PODF] = imx_clk_divider("lcdif_podf", "lcdif_pred", ccm_base + 0x18, 23, 3); > + > + clk[IMXRT1050_CLK_USDHC1] = imx_clk_gate2("usdhc1", "usdhc1_podf", ccm_base + 0x80, 2); > + clk[IMXRT1050_CLK_USDHC2] = imx_clk_gate2("usdhc2", "usdhc2_podf", ccm_base + 0x80, 4); > + clk[IMXRT1050_CLK_LPUART1] = imx_clk_gate2("lpuart1", "lpuart_podf", ccm_base + 0x7c, 24); > + clk[IMXRT1050_CLK_LCDIF_APB] = imx_clk_gate2("lcdif", "lcdif_podf", ccm_base + 0x74, 10); > + clk[IMXRT1050_CLK_DMA] = imx_clk_gate("dma", "ipg", ccm_base + 0x7C, 6); > + clk[IMXRT1050_CLK_DMA_MUX] = imx_clk_gate("dmamux0", "ipg", ccm_base + 0x7C, 7); > + > + imx_check_clocks(clk, ARRAY_SIZE(clk)); > + clk_data.clks = clk; > + clk_data.clk_num = ARRAY_SIZE(clk); > + of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL1_ARM]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL2_SYS]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL3_USB_OTG]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL3_PFD1_664_62M]); > + clk_prepare_enable(clk[IMXRT1050_CLK_PLL2_PFD2_396M]); > +} > +CLK_OF_DECLARE(imxrt_ccm, "fsl,imxrt1050-ccm", imxrt1050_clocks_init); > Thank you and Best regards
On Tue, Nov 02, 2021 at 06:56:53PM -0400, Jesse Taube wrote: > From: Jesse Taube <mr.bossman075@gmail.com> > > Add DT binding documentation for i.MXRT clock driver. > > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> > --- > V1->V2: > * Replace macros with values > --- > .../bindings/clock/imxrt-clock.yaml | 70 +++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/imxrt-clock.yaml > > diff --git a/Documentation/devicetree/bindings/clock/imxrt-clock.yaml b/Documentation/devicetree/bindings/clock/imxrt-clock.yaml > new file mode 100644 > index 000000000000..4e92f79cf707 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/imxrt-clock.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/imxrt-clock.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Clock bindings for Freescale i.MXRT > + > +maintainers: > + - Giulio Benetti <giulio.benetti@benettiengineering.com> > + - Jesse Taube <Mr.Bossman075@gmail.com> > + > +description: | > + The clock consumer should specify the desired clock by having the clock > + ID in its "clocks" phandle cell. See include/dt-bindings/clock/imxrt*-clock.h > + for the full list of i.MXRT clock IDs. blank line > +properties: > + compatible: > + oneOf: Don't need oneOf for a single entry. > + - enum: > + - fsl,imxrt1050-ccm > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 1 > + > + clock-names: > + minItems: 1 You have to define the name. > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - '#clock-cells' > + > +additionalProperties: false > + > +examples: > + - | > + anatop: anatop@400d8000 { > + compatible = "fsl,imxrt-anatop"; > + reg = <0x400d8000 0x4000>; > + }; Not relevant to the example. > + > + ccm@400fc000 { > + compatible = "fsl,imxrt1050-ccm"; > + reg = <0x400fc000 0x4000>; > + interrupts = <95>,<96>; > + clocks = <&osc>; > + clock-names = "osc"; > + #clock-cells = <1>; > + }; > + > + gpt: timer@401ec000 { Drop unused labels. > + compatible = "fsl,imx53-gpt", "fsl,imx31-gpt"; Probably should be: "fsl,imxrt1050-gpt", "fsl,imx31-gpt" Unless there's same features/quirks as the MX53 version? > + reg = <0x401ec000 0x4000>; > + interrupts = <100>; > + clocks = <&clks 3>; > + clock-names = "per"; > + }; > -- > 2.33.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Wed, Nov 3, 2021 at 2:38 AM Jesse Taube <mr.bossman075@gmail.com> wrote: > Ah I thought it would stop make at error i see it now, is there a way to > do one file. Yes: make dt_binding_check DT_SCHEMA_FILES=Documentation/... Yours, Linus Walleij
On 11/8/21 20:07, Linus Walleij wrote: > On Wed, Nov 3, 2021 at 2:38 AM Jesse Taube <mr.bossman075@gmail.com> wrote: > >> Ah I thought it would stop make at error i see it now, is there a way to >> do one file. > > Yes: > > make dt_binding_check DT_SCHEMA_FILES=Documentation/... Thank you :). I'm sorry about the lack of activity recently but Giulio has his work and i have school, but don't worry we are making progress. Thanks so much for everyone's help with this, and dealing with my newbieness. > Yours, > Linus Walleij >
On 11/2/21 22:19, Bough Chen wrote: >> -----Original Message----- >> From: Jesse Taube [mailto:mr.bossman075@gmail.com] >> Sent: 2021年11月3日 6:57 >> To: dl-linux-imx <linux-imx@nxp.com> >> Cc: mturquette@baylibre.com; sboyd@kernel.org; robh+dt@kernel.org; >> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; >> festevam@gmail.com; ulf.hansson@linaro.org; Aisheng Dong >> <aisheng.dong@nxp.com>; stefan@agner.ch; linus.walleij@linaro.org; >> gregkh@linuxfoundation.org; arnd@arndb.de; olof@lixom.net; >> soc@kernel.org; linux@armlinux.org.uk; Abel Vesa <abel.vesa@nxp.com>; >> adrian.hunter@intel.com; jirislaby@kernel.org; >> giulio.benetti@benettiengineering.com; nobuhiro1.iwamatsu@toshiba.co.jp; >> Mr.Bossman075@gmail.com; linux-clk@vger.kernel.org; >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; >> linux-gpio@vger.kernel.org; linux-serial@vger.kernel.org; Jesse Taube >> <mr.bossman075@gmail.com> >> Subject: [PATCH v2 11/13] mmc: sdhci-esdhc-imx: Add sdhc support for > i.MXRT >> series >> >> From: Jesse Taube <mr.bossman075@gmail.com> >> >> Add support for i.MXRT1050's sdhc. >> >> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> >> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>--- >> V1->V2: >> * Nothing done >> --- >> drivers/mmc/host/sdhci-esdhc-imx.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c >> b/drivers/mmc/host/sdhci-esdhc-imx.c >> index afaf33707d46..c852a6df5611 100644 >> --- a/drivers/mmc/host/sdhci-esdhc-imx.c >> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c >> @@ -305,6 +305,12 @@ static struct esdhc_soc_data usdhc_imx7ulp_data = { >> | ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400 >> | ESDHC_FLAG_STATE_LOST_IN_LPMODE, >> }; >> +static struct esdhc_soc_data usdhc_imxrt_data = { >> + .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING >> + | ESDHC_FLAG_HS200 | ESDHC_FLAG_ERR004536 >> + | ESDHC_FLAG_BROKEN_AUTO_CMD23, >> +}; >> + > > Hi Jesse, > I have few question here. > Why only use manual tuning here? Does this usdhc don't support > standard tuning? or meet any issue when use standard tuning? No std tuning works, so does cmd23, i changed it to use them. > Please also double check why not use ADMA in default? Any issue > found? Yes this is the output with ADMA: [0.00] mmc0: Unable to allocate ADMA buffers - falling back to standard DMA NOTE: I did not look into why this occurs. > > > Best Regards > Haibo Chen >> >> static struct esdhc_soc_data usdhc_imx8qxp_data = { >> .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING @@ -357,6 >> +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { >> { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, >> { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, }, >> { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, }, >> + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, >> { .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, }, >> { /* sentinel */ } >> }; >> -- >> 2.33.1 >
> -----Original Message----- > From: Jesse Taube [mailto:mr.bossman075@gmail.com] > Sent: 2021年11月23日 11:14 > To: Bough Chen <haibo.chen@nxp.com>; dl-linux-imx <linux-imx@nxp.com> > Cc: mturquette@baylibre.com; sboyd@kernel.org; robh+dt@kernel.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; > festevam@gmail.com; ulf.hansson@linaro.org; Aisheng Dong > <aisheng.dong@nxp.com>; stefan@agner.ch; linus.walleij@linaro.org; > gregkh@linuxfoundation.org; arnd@arndb.de; olof@lixom.net; > soc@kernel.org; linux@armlinux.org.uk; Abel Vesa <abel.vesa@nxp.com>; > adrian.hunter@intel.com; jirislaby@kernel.org; > giulio.benetti@benettiengineering.com; nobuhiro1.iwamatsu@toshiba.co.jp; > linux-clk@vger.kernel.org; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > linux-mmc@vger.kernel.org; linux-gpio@vger.kernel.org; > linux-serial@vger.kernel.org > Subject: Re: [PATCH v2 11/13] mmc: sdhci-esdhc-imx: Add sdhc support for > i.MXRT series > > > > On 11/2/21 22:19, Bough Chen wrote: > >> -----Original Message----- > >> From: Jesse Taube [mailto:mr.bossman075@gmail.com] > >> Sent: 2021年11月3日 6:57 > >> To: dl-linux-imx <linux-imx@nxp.com> > >> Cc: mturquette@baylibre.com; sboyd@kernel.org; robh+dt@kernel.org; > >> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; > >> festevam@gmail.com; ulf.hansson@linaro.org; Aisheng Dong > >> <aisheng.dong@nxp.com>; stefan@agner.ch; linus.walleij@linaro.org; > >> gregkh@linuxfoundation.org; arnd@arndb.de; olof@lixom.net; > >> soc@kernel.org; linux@armlinux.org.uk; Abel Vesa <abel.vesa@nxp.com>; > >> adrian.hunter@intel.com; jirislaby@kernel.org; > >> giulio.benetti@benettiengineering.com; > >> nobuhiro1.iwamatsu@toshiba.co.jp; Mr.Bossman075@gmail.com; > >> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; > >> linux-arm-kernel@lists.infradead.org; > >> linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; > >> linux-gpio@vger.kernel.org; linux-serial@vger.kernel.org; Jesse Taube > >> <mr.bossman075@gmail.com> > >> Subject: [PATCH v2 11/13] mmc: sdhci-esdhc-imx: Add sdhc support for > > i.MXRT > >> series > >> > >> From: Jesse Taube <mr.bossman075@gmail.com> > >> > >> Add support for i.MXRT1050's sdhc. > >> > >> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > >> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>--- > >> V1->V2: > >> * Nothing done > >> --- > >> drivers/mmc/host/sdhci-esdhc-imx.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > >> b/drivers/mmc/host/sdhci-esdhc-imx.c > >> index afaf33707d46..c852a6df5611 100644 > >> --- a/drivers/mmc/host/sdhci-esdhc-imx.c > >> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > >> @@ -305,6 +305,12 @@ static struct esdhc_soc_data usdhc_imx7ulp_data > = { > >> | ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400 > >> | ESDHC_FLAG_STATE_LOST_IN_LPMODE, > >> }; > >> +static struct esdhc_soc_data usdhc_imxrt_data = { > >> + .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING > >> + | ESDHC_FLAG_HS200 | ESDHC_FLAG_ERR004536 > >> + | ESDHC_FLAG_BROKEN_AUTO_CMD23, > >> +}; > >> + > > > > Hi Jesse, > > I have few question here. > > Why only use manual tuning here? Does this usdhc don't support > > standard tuning? or meet any issue when use standard tuning? > No std tuning works, so does cmd23, i changed it to use them. Okay. > > Please also double check why not use ADMA in default? Any issue > > found? > Yes this is the output with ADMA: > [0.00] mmc0: Unable to allocate ADMA buffers - falling back to standard DMA > NOTE: I did not look into why this occurs. If you config enough space for CMA or DMA pool, I think dma_alloc_corherent() should not meet issue. ADMA descriptor do not large than one page(4KB). This is not big. Seems strange. Please double check this. If this is really a limitation on imxRT, I'm okay to use SDMA as default. Best Regards Haibo chen > > > > > > Best Regards > > Haibo Chen > >> > >> static struct esdhc_soc_data usdhc_imx8qxp_data = { > >> .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING @@ > -357,6 > >> +363,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = { > >> { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, }, > >> { .compatible = "fsl,imx8qxp-usdhc", .data = > &usdhc_imx8qxp_data, }, > >> { .compatible = "fsl,imx8mm-usdhc", .data = > &usdhc_imx8mm_data, }, > >> + { .compatible = "fsl,imxrt-usdhc", .data = &usdhc_imxrt_data, }, > >> { .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, }, > >> { /* sentinel */ } > >> }; > >> -- > >> 2.33.1 > >