RE: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support

From: Vikas MANOCHA
Date: Wed Apr 12 2017 - 20:04:24 EST

Next message: Len Brown: "[GIT PULL PATCH 0/7] turbostat fixes"
Previous message: Florian Fainelli: "Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers"
In reply to: Alexandre Torgue: "Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support"
Next in thread: Vikas Manocha: "[PATCH] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Alex,

> -----Original Message-----
> From: Alexandre TORGUE
> Sent: Tuesday, April 11, 2017 12:51 AM
> To: Vikas MANOCHA <vikas.manocha@xxxxxx>; Patrice CHOTARD <patrice.chotard@xxxxxx>
> Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@xxxxxxxxxxxxxxx>; moderated list:ARM PORT
> <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>; Maxime
> Coquelin <mcoquelin.stm32@xxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Russell King <linux@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
>
> Hi Vikas
>
> On 04/10/2017 08:40 PM, Vikas Manocha wrote:
> > Thanks Alex,
> >
> > On 04/10/2017 12:23 AM, Alexandre Torgue wrote:
> >> Hi
> >>
> >> On 04/08/2017 03:12 AM, Vikas Manocha wrote:
> >>> Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the
> >>> major spces of the two boards:
> >>>
> >>> stm32f769I discovery board:
> >>> - Cortex-M7 core @216MHz
> >>> - 2MB mcu internal flash
> >>> - 512KB internal sram
> >>> - 16MB sdram memory
> >>> - 64MB qspi flash memory
> >>> - 4 inch wvga LCD-TFT Display
> >>>
> >>> stm32f746 discovery board:
> >>> - Cortex-M7 core @216MHz
> >>> - 1MB mcu internal flash
> >>> - 320KB internal sram
> >>> - 8MB sdram memory
> >>> - 16MB qspi flash memory
> >>> - 4.3 inch 480x272 LCD-TFT display
> >>>
> >>> Signed-off-by: Vikas Manocha <vikas.manocha@xxxxxx>
> >>> ---
> >>> arch/arm/boot/dts/Makefile | 2 +
> >>> arch/arm/boot/dts/stm32f746-disco.dts | 101 ++++++++++++++++++++++++++++++++++
> >>> arch/arm/boot/dts/stm32f746.dtsi | 2 +-
> >>> arch/arm/boot/dts/stm32f769-disco.dts | 101
> >>> ++++++++++++++++++++++++++++++++++
> >>> 4 files changed, 205 insertions(+), 1 deletion(-) create mode
> >>> 100644 arch/arm/boot/dts/stm32f746-disco.dts
> >>> create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts
> >>>
> >>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >>> index 0118084..a119f74 100644
> >>> --- a/arch/arm/boot/dts/Makefile
> >>> +++ b/arch/arm/boot/dts/Makefile
> >>> @@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
> >>> dtb-$(CONFIG_ARCH_STM32)+= \
> >>> stm32f429-disco.dtb \
> >>> stm32f469-disco.dtb \
> >>> + stm32f746-disco.dtb \
> >>> + stm32f769-disco.dtb \
> >>> stm32429i-eval.dtb \
> >>> stm32746g-eval.dtb
> >>> dtb-$(CONFIG_MACH_SUN4I) += \
> >>> diff --git a/arch/arm/boot/dts/stm32f746-disco.dts
> >>> b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> new file mode 100644
> >>> index 0000000..c0e313f
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha@xxxxxx>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + * a) This file is free software; you can redistribute it and/or
> >>> + * modify it under the terms of the GNU General Public License as
> >>> + * published by the Free Software Foundation; either version 2 of the
> >>> + * License, or (at your option) any later version.
> >>> + *
> >>> + * This file is distributed in the hope that it will be useful,
> >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>> + * GNU General Public License for more details.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + * b) Permission is hereby granted, free of charge, to any person
> >>> + * obtaining a copy of this software and associated documentation
> >>> + * files (the "Software"), to deal in the Software without
> >>> + * restriction, including without limitation the rights to use,
> >>> + * copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + * sell copies of the Software, and to permit persons to whom the
> >>> + * Software is furnished to do so, subject to the following
> >>> + * conditions:
> >>> + *
> >>> + * The above copyright notice and this permission notice shall be
> >>> + * included in all copies or substantial portions of the Software.
> >>> + *
> >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + * OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> + model = "STMicroelectronics STM32F746-DISCO board";
> >>> + compatible = "st,stm32f746-disco", "st,stm32f746";
> >>> +
> >>> + chosen {
> >>> + bootargs = "root=/dev/ram";
> >>> + stdout-path = "serial0:115200n8";
> >>> + };
> >>> +
> >>> + memory {
> >>> + reg = <0xC0000000 0x800000>;
> >>> + };
> >>> +
> >>> + aliases {
> >>> + serial0 = &usart1;
> >>> + };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> + clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >>
> >> Pin muxing is not defined in board file. Please move it into SOC dtsi file.

Ok, I will move pin muxing in the stm32f746.dtsi file as per current implementation & send V2.

> >
> > Pin muxing used is different for different boards. e.g. usart1_rx pad is PA10 for stm32f769-disco board while it is PB7 for stm32f746-
> disco board.
> > The other possibilities for same pad (usart1_rx) is PB15. To make situation bit more complex, it is only available in f769 device.
> >
> > Putting in SOC dtsi file means having lot of combinations for different pins in separate groups.
> > e.g. only for one instance of one ip (usart1), following groups might be required at one point of time:
> >
> > usart1_pa10_pa9 {..}
> > usart1_pa10_pb14 {..}
> > usart1_pa10_pb6 {..}
> >
> > usart1_pb7_pa9 {..}
> > usart1_pb7_pb14 {..}
> > usart1_pb7_pb6 {..}
> >
> > usart1_pb15_pa9 {..}
> > usart1_pb15_pb14 {..}
> > usart1_pb15_pb6 {..}
> >
> > In case of boards based on stm32f746 device, all the above mentioned groups with pb14 & pb15 will not be available.
> > One solution (to avoid using not available groups) could be to have separate dtsi (or separate pinmux.dtsi) for different devices of
> same family like one for stm32f746 & other for stm32f769. Still it does not resolve the need to have lot of groups combinations for
> each instance of every peripheral in dtsi as mentioned above.
>
> Yes, it is what I want to have. I did the job on for STM32F4 (F429 / F469). You could have a look on ARM Linux patchwork:
> https://patchwork.kernel.org/patch/9669433/
>
> To sum-up the implementation:
>
> -Pinmuxing is defined in separate files which will be included in board dts file.
>
> -We have a common pinmuxing file + a dedicated pinmuxing file per SOC.
>
> Example for STMF469-disco:
> stm32f4-pinctrl.dtsi --> stm32f469-pinctrl.dtsi --> stm32f469-disco.dts
>
> stm32f4-pinctrl.dtsi contains common pinmuxing bindings between STM32F4 stm32f469-pinctrl.dtsi contains dedicated pinmuxing
> bindings for
> STM32F469 (ex: QSPI pins, gpio-ranges ...)
>
> This implementation is under review.

Sounds good.

Cheers,
Vikas

>
>
> > It seems cleaner solution would be pin muxing in board dts file. Please let me know if there is some drawback of this approach. One
> point which i can think of is : duplication of pinmux groups in different board dts files.
>
> Pinmuxing in board file is currently not my choice. In the board file we only select the group to use.
>
> Regards
> Alex
>
>
> >
> > Cheers,
> > Vikas
> >
> >>> + usart1_pins: usart1@0 {
> >>> + pins1 {
> >>> + pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> + bias-disable;
> >>> + drive-push-pull;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + pins2 {
> >>> + pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
> >>> + bias-disable;
> >>> + };
> >>> + };
> >>> +
> >>> + qspi_pins: qspi@0 {
> >>> + pins {
> >>> + pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> + <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> + <STM32F746_PD11_FUNC_QUADSPI_BK1_IO0>,
> >>> + <STM32F746_PD12_FUNC_QUADSPI_BK1_IO1>,
> >>> + <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> + <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> + pinctrl-0 = <&usart1_pins>;
> >>> + pinctrl-names = "default";
> >>> + status = "okay";
> >>> +};
> >>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi
> >>> b/arch/arm/boot/dts/stm32f746.dtsi
> >>> index f321ffe..826700f 100644
> >>> --- a/arch/arm/boot/dts/stm32f746.dtsi
> >>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
> >>> @@ -178,7 +178,7 @@
> >>> interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
> >>> };
> >>>
> >>> - pin-controller {
> >>> + pinctrl: pin-controller {
> >>> #address-cells = <1>;
> >>> #size-cells = <1>;
> >>> compatible = "st,stm32f746-pinctrl"; diff --git
> >>> a/arch/arm/boot/dts/stm32f769-disco.dts
> >>> b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> new file mode 100644
> >>> index 0000000..5f8558e
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha@xxxxxx>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + * a) This file is free software; you can redistribute it and/or
> >>> + * modify it under the terms of the GNU General Public License as
> >>> + * published by the Free Software Foundation; either version 2 of the
> >>> + * License, or (at your option) any later version.
> >>> + *
> >>> + * This file is distributed in the hope that it will be useful,
> >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>> + * GNU General Public License for more details.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + * b) Permission is hereby granted, free of charge, to any person
> >>> + * obtaining a copy of this software and associated documentation
> >>> + * files (the "Software"), to deal in the Software without
> >>> + * restriction, including without limitation the rights to use,
> >>> + * copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + * sell copies of the Software, and to permit persons to whom the
> >>> + * Software is furnished to do so, subject to the following
> >>> + * conditions:
> >>> + *
> >>> + * The above copyright notice and this permission notice shall be
> >>> + * included in all copies or substantial portions of the Software.
> >>> + *
> >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + * OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> + model = "STMicroelectronics STM32F769-DISCO board";
> >>> + compatible = "st,stm32f769-disco", "st,stm32f7";
> >>> +
> >>> + chosen {
> >>> + bootargs = "root=/dev/ram";
> >>> + stdout-path = "serial0:115200n8";
> >>> + };
> >>> +
> >>> + memory {
> >>> + reg = <0xC0000000 0x1000000>;
> >>> + };
> >>> +
> >>> + aliases {
> >>> + serial0 = &usart1;
> >>> + };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> + clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >> same.
> >>
> >>> + usart1_pins: usart1@0 {
> >>> + pins1 {
> >>> + pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> + bias-disable;
> >>> + drive-push-pull;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + pins2 {
> >>> + pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
> >>> + bias-disable;
> >>> + };
> >>> + };
> >>> +
> >>> + qspi_pins: qspi@0 {
> >>> + pins {
> >>> + pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> + <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> + <STM32F746_PC9_FUNC_QUADSPI_BK1_IO0>,
> >>> + <STM32F746_PC10_FUNC_QUADSPI_BK1_IO1>,
> >>> + <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> + <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> + pinctrl-0 = <&usart1_pins>;
> >>> + pinctrl-names = "default";
> >>> + status = "okay";
> >>> +};
> >>>
> >> .
> >>

Next message: Len Brown: "[GIT PULL PATCH 0/7] turbostat fixes"
Previous message: Florian Fainelli: "Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers"
In reply to: Alexandre Torgue: "Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support"
Next in thread: Vikas Manocha: "[PATCH] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]