From mboxrd@z Thu Jan 1 00:00:00 1970 From: Parthiban Nallathambi Date: Wed, 10 Apr 2019 15:23:41 +0200 Subject: [U-Boot] [PATCH] imx: Extend PCL063 support for phyCORE-i.MX6ULL SOM In-Reply-To: <20afb39a-fd23-2651-44d6-744f2f57399b@phytec.de> References: <20190407175654.2924091-1-parthitce@gmail.com> <7e6b830a02c34d9df57f49a61210db8012e220ef.camel@collabora.com> <6405a060-6214-47fc-a520-7fac73c781de@gmail.com> <6280cf7718d07ec7d85d764f409a0082c48739d4.camel@collabora.com> <9fb551d5-ccdd-5bfd-2988-8ffe930e3812@gmail.com> <20afb39a-fd23-2651-44d6-744f2f57399b@phytec.de> Message-ID: <34178a52-44e3-6a45-60a4-a4c619a94ee2@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Wadim, Thanks for sharing the details. On 4/10/19 10:35 AM, Wadim Egorov wrote: > Martyn, > > On 09.04.19 12:46, Martyn Welch wrote: >> On Tue, 2019-04-09 at 11:30 +0200, Parthiban Nallathambi wrote: >>> Hello Martyn, >>> >>> On 4/9/19 10:49 AM, Martyn Welch wrote: >>>> On Mon, 2019-04-08 at 20:04 +0200, Parthiban wrote: >>>>> Hello Martyn, >>>>> >>>>> On 4/8/19 7:45 PM, Martyn Welch wrote: >>>>>> On Sun, 2019-04-07 at 19:56 +0200, Parthiban Nallathambi wrote: >>>>>>> diff --git a/board/phytec/pcl063/spl.c >>>>>>> b/board/phytec/pcl063/spl.c >>>>>>> index b93cd493f2..73a774645d 100644 >>>>>>> --- a/board/phytec/pcl063/spl.c >>>>>>> +++ b/board/phytec/pcl063/spl.c >>>>>>> @@ -13,6 +13,7 @@ >>>>>>> #include >>>>>>> #include >>>>>>> #include >>>>>>> +#include >>>>>>> #include >>>>>>> >>>>>>> /* Configuration for Micron MT41K256M16TW-107 IT:P, 32M x >>>>>>> 16 x 8 >>>>>>> -> >>>>>>> 256MiB */ >>>>>>> @@ -117,11 +118,32 @@ static iomux_v3_cfg_t const >>>>>>> usdhc1_pads[] = >>>>>>> { >>>>>>> MX6_PAD_UART1_RTS_B__USDHC1_CD_B | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> }; >>>>>>> >>>>>>> +#ifndef CONFIG_NAND_MXS >>>>>>> +static iomux_v3_cfg_t const usdhc2_pads[] = { >>>>>>> + MX6_PAD_NAND_RE_B__USDHC2_CLK | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_WE_B__USDHC2_CMD | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA00__USDHC2_DATA0 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA01__USDHC2_DATA1 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA02__USDHC2_DATA2 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA03__USDHC2_DATA3 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA04__USDHC2_DATA4 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA05__USDHC2_DATA5 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA06__USDHC2_DATA6 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> + MX6_PAD_NAND_DATA07__USDHC2_DATA7 | >>>>>>> MUX_PAD_CTRL(USDHC_PAD_CTRL), >>>>>>> +}; >>>>>>> +#endif >>>>>>> + >>>>>> Umm, these pins are already used a few lines up for the NAND, >>>>>> via >>>>>> gpmi: >>>>> I understand. But pcl063 can't co-exit with NAND and eMMC >>>>> together. I >>>>> comes >>>>> either with eMMC or NAND. >>>> Opps, sorry, just realised that I added this comment in the wrong >>>> place. This is in relation to the following being added to >>>> pcl063-common.dtsi: >>>> >>>> + >>>> + pinctrl_usdhc2: usdhc2grp { >>>> + fsl,pins = < >>>> + MX6UL_PAD_NAND_WE_B__USDHC2_CMD 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_RE_B__USDHC2_CLK 0x1 >>>> 00f9 >>>> + MX6UL_PAD_NAND_DATA00__USDHC2_DATA0 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA01__USDHC2_DATA1 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA02__USDHC2_DATA2 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA03__USDHC2_DATA3 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA04__USDHC2_DATA4 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA05__USDHC2_DATA5 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA06__USDHC2_DATA6 0x1 >>>> 70f9 >>>> + MX6UL_PAD_NAND_DATA07__USDHC2_DATA7 0x1 >>>> 70f9 >>>> + >; >>>> + }; >>>> >>>> If there exists pcl063 modules that have eMMC and others that have >>>> NAND >>>> using the same pins, then this configuration is not common and >>>> therefore shouldn't be in pcl063-common.dtsi. Is it dependent on >>>> the >>>> flavour of i.MX used? If so I'd suggest the gpmi config needs to be >>>> pulled out into imx6ul-phycore-segin.dts and the usdhc2 config >>>> needs to >>>> be in imx6ull-phycore-segin.dts. >>> From phytec I understand that pcl063 SoM is a common platform for >>> imx6UL >>> and imx6ULL. This can either be shipped with eMMC or NAND, but not >>> both. > > This is correct. There are PCL-063 SOMs with eMMC or NAND. And each > PCL-063 can be a 6UL or 6ULL. > > >>> >> Looking a bit deeper, this seems a little odd as the product >> description suggests that NAND is provided onboard and 2 SD/SDIO/MMC >> connections are provided to the edge connector of the pcl063 for >> expansion. >> >> The schematic suggests the only way they could achieve eMMC onboard >> would be with an eMMC that is pin compatible with the NAND they use. > > eMMC is connected via usdhc2. The usdhc2 pins conflict with the gpmi pins. > > >> >> Additionally, looking at the DTBs for this board in Phytec's own kernel >> tree, the only use of usdhc2 that I can see is for their WLAN expansion >> board[1] and I would have expected their tree to have supported such an >> option if it was available (they seem to have gone to some length to >> support a lot of configurations there). >> >> Are you sure that the eMMC is provided on the pcl063 and not off board? > > eMMC is on the PCL-063 and not on a carrier board. > > >> >> (CCing Wadim who might be able to shed some light on this) >> >>> So there exist a possibility that phytec can provide imx6UL with eMMC >>> as >>> well. IMO, both pinmux detailing for NAND and eMMC should still >>> reside >>> in common.dtsi. >>> >> Assuming Phytec do in fact sell a pcl0063 with eMMC on board, the DTB >> describes the hardware. You've said Phytec provide the board >> either with eMMC *or* NAND. The device tree, as used on a specific >> board, should show either the existence of NAND or eMMC. > > AFAIK, the idea was to put the muxing for both flash devices in the > pcl063-som.dtsi and keep them disabled. eMMC or NAND will be then > enabled in a higher level carrier board dts file. The handling of the > SOM variant is designed by the carrier board dts file name, e.g. > imx6ul-phytec-segin-ff-rdk-nand.dts and in theory a > imx6ul-phytec-segin-ff-rdk-emmc.dts (Right now there is no 6UL eMMC SOM > variant, so you can not find it in our kernel repository). I agree to place both gpmi and eMMC pinmux details in common.dtsi with default "disabled" and enable them only in board dts. @Martyn, please advise. Thanks, Parthiban N > > I agree, that this does not describe the SOM hardware. This is just the > way we handle the SOM variants. > > It seems the upstream situation for kernel and u-boot differs to what we > did quite a lot at the moment. I am just curious how you want to handle > this now. Unfortunately, we did no had the time to bring our 6UL boards > upstream and clarify the situation. But we can talk about it now and > find out a way to handle the variants properly. > > CC'ed Stefan who is maintaining the imx6 boards at Phytec. > > I hope this helps :) > > Regards, > Wadim > >> >> I suspect having both options in the common file is going to lead to >> issues with the pinmuxing for one or the other option. The pins can't >> be muxed for both simultaneously, hence these need to be described in >> separate files. >> >>> Creating multiple common.dtsi based on these variants is not >>> friendly. >>> So I suggest to keep these changes in common.dtsi as such and decide >>> in >>> board dts whether to enable or disable usdhc1, usdhc2 explicitly. >> This isn't about the selection of usdhc1 or usdhc2, it's whether gpmi >> or usdhc2 is using the pins of the SOC. >> >> Martyn >> >> [1] >> https://git.phytec.de/linux-mainline/tree/arch/arm/boot/dts/imx6ul-phytec-peb-wlbt-01.dtsi?h=v4.14.39-phy#n51 >> >> >>>>>> pinctrl_gpmi_nand: gpminandgrp { >>>>>> fsl,pins = < >>>>>> MX6UL_PAD_NAND_CLE__RAWNAND_CLE >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_ALE__RAWNAND_ALE >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_WP_B__RAWNAND_WP_B >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_READY_B__RAWNAND_READY_ >>>>>> B >>>>>> 0x0b000 >>>>>> MX6UL_PAD_NAND_CE0_B__RAWNAND_CE0_B >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_RE_B__RAWNAND_RE_B >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_WE_B__RAWNAND_WE_B >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA00__RAWNAND_DATA00 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA01__RAWNAND_DATA01 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA02__RAWNAND_DATA02 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA03__RAWNAND_DATA03 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA04__RAWNAND_DATA04 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA05__RAWNAND_DATA05 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA06__RAWNAND_DATA06 >>>>>> 0x0 >>>>>> b0b1 >>>>>> MX6UL_PAD_NAND_DATA07__RAWNAND_DATA07 >>>>>> 0x0 >>>>>> b0b1 >>>>>> >; >>>>>> }; >>>>>>