[U-Boot] [PATCH] imx: Extend PCL063 support for phyCORE-i.MX6ULL SOM

[Parthiban Nallathambi <parthitce@gmail.com>]

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 <asm/arch/mx6-ddr.h>
>>>>>>>    #include <asm/arch/mx6-pins.h>
>>>>>>>    #include <asm/arch/crm_regs.h>
>>>>>>> +#include <asm/arch/sys_proto.h>
>>>>>>>    #include <fsl_esdhc.h>
>>>>>>>    
>>>>>>>    /* 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
>>>>>>                   >;
>>>>>>           };
>>>>>>