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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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";
> >>> +};
> >>>
> >> .
> >>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux