| Message ID | 77304b3f-08e7-5afe-708a-ca5efd65137d@denx.de |
|---|---|
| State | Not Applicable |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Stefano, On Mon, Oct 24, 2016 at 5:05 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi Jagan, > > On 24/10/2016 13:03, Jagan Teki wrote: >> Hi Stefano, >> >> On Mon, Oct 24, 2016 at 4:18 PM, Jagan Teki <jagan@openedev.com> wrote: >>> On Mon, Oct 24, 2016 at 3:53 PM, Jagan Teki <jagan@openedev.com> wrote: >>>> + Peng >>>> >>>> On Mon, Oct 24, 2016 at 3:46 PM, Stefano Babic <sbabic@denx.de> wrote: >>>>> Hi Jagan, Peng, >>>>> >>>>> On 24/10/2016 11:05, Stefano Babic wrote: >>>>> >>>>>>> Did you take the v7 patches? I re-based on top of u-boot-imx/master >>>>>>> and it's compiled for me. >>>>>> >>>>>> I take the v7: >>>>>> >>>>>> http://patchwork.ozlabs.org/bundle/sbabic/jagan/ >>>>>> >>>>>> Applied on top of u-boot-imx master, I get the error. There are just a >>>>>> few of new patches on the tree, I doubt that are guilty for it. >>>>>> >>>>> >>>>> I got the same issue for mx6ull_14x14_evk_plugin after applying Peng's >>>>> patches for plugins. Issue is raised in fsl_esdhc.c driver, that should >>>>> be not yet prepared for DM_MMC_OPS. >>>>> >>>>> Is there a missing patch ? Could you take a look ? >>> >>> Got the issue, we need to update 2 recent changes from Simon >>> patches[1][2] I will fix and update. >>> >>> [1] http://git.denx.de/?p=u-boot/u-boot-imx.git;a=commitdiff;h=896a74f615d6ffcbbcbec1505b19ed3280fe7873 >>> [2] http://git.denx.de/?p=u-boot/u-boot-imx.git;a=commitdiff;h=252788b4eda852e0195e1903e55480b4bf4fea9d >> >> Please try this patches, I've updated the above changes. >> >> [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/engicam-working >> > > No, this is not the correct way to do. We should stick with the patches > sent to ML and included in patchworks. They are documented and I cannot > (that means, all custodians are not allowed) to simply pick up patches > from somewhere else. True, but I am not saying you to just apply the branch. My intention was to check the new changes from your side so-that will send the series to ML again. > > It is enough that we agree about issue and solution. I had already > tested with the following patch: > > diff --git a/configs/imx6qdl_icore_mmc_defconfig > b/configs/imx6qdl_icore_mmc_defconfig > index 221ea7e..c947e10 100644 > --- a/configs/imx6qdl_icore_mmc_defconfig > +++ b/configs/imx6qdl_icore_mmc_defconfig > @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" > CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" > CONFIG_SYS_PROMPT="icorem6qdl> " > CONFIG_SPL=y > +# CONFIG_BLK is not set > +# CONFIG_DM_MMC_OPS is not set > CONFIG_BOOTDELAY=3 > CONFIG_BOARD_EARLY_INIT_F=y > CONFIG_DISPLAY_CPUINFO=y > diff --git a/configs/imx6qdl_icore_nand_defconfig nand_defconfig doesn't require this change. > b/configs/imx6qdl_icore_nand_defconfig > index 8ac3099..55650bb 100644 > --- a/configs/imx6qdl_icore_nand_defconfig > +++ b/configs/imx6qdl_icore_nand_defconfig > @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" > CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" > CONFIG_SYS_PROMPT="icorem6qdl> " > CONFIG_SPL=y > +# CONFIG_BLK is not set > +# CONFIG_DM_MMC_OPS is not set > CONFIG_BOOTDELAY=3 > CONFIG_BOARD_EARLY_INIT_F=y > CONFIG_DISPLAY_CPUINFO=y thanks!
Hi Jagan, On 24/10/2016 13:46, Jagan Teki wrote: >> >> No, this is not the correct way to do. We should stick with the patches >> sent to ML and included in patchworks. They are documented and I cannot >> (that means, all custodians are not allowed) to simply pick up patches >> from somewhere else. > > True, but I am not saying you to just apply the branch. My intention > was to check the new changes from your side so-that will send the > series to ML again. ok > >> >> It is enough that we agree about issue and solution. I had already >> tested with the following patch: >> >> diff --git a/configs/imx6qdl_icore_mmc_defconfig >> b/configs/imx6qdl_icore_mmc_defconfig >> index 221ea7e..c947e10 100644 >> --- a/configs/imx6qdl_icore_mmc_defconfig >> +++ b/configs/imx6qdl_icore_mmc_defconfig >> @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" >> CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" >> CONFIG_SYS_PROMPT="icorem6qdl> " >> CONFIG_SPL=y >> +# CONFIG_BLK is not set >> +# CONFIG_DM_MMC_OPS is not set >> CONFIG_BOOTDELAY=3 >> CONFIG_BOARD_EARLY_INIT_F=y >> CONFIG_DISPLAY_CPUINFO=y >> diff --git a/configs/imx6qdl_icore_nand_defconfig > > nand_defconfig doesn't require this change. > It is what I had expected, but fsl_esdhc is built even in the NAND configuration. There is omething else enabling (not in defconfig). In fact, dropping the two lines, I get the error again for imx6qdl_icore_nand. The reason is that even if MMC is not set, as far I can see, these are automatically set: CONFIG_DM_MMC=y CONFIG_DM_MMC_OPS=y >> b/configs/imx6qdl_icore_nand_defconfig >> index 8ac3099..55650bb 100644 >> --- a/configs/imx6qdl_icore_nand_defconfig >> +++ b/configs/imx6qdl_icore_nand_defconfig >> @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" >> CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" >> CONFIG_SYS_PROMPT="icorem6qdl> " >> CONFIG_SPL=y >> +# CONFIG_BLK is not set >> +# CONFIG_DM_MMC_OPS is not set >> CONFIG_BOOTDELAY=3 >> CONFIG_BOARD_EARLY_INIT_F=y >> CONFIG_DISPLAY_CPUINFO=y > > thanks! >
Hi Stefano, On Mon, Oct 24, 2016 at 5:31 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi Jagan, > > On 24/10/2016 13:46, Jagan Teki wrote: > >>> >>> No, this is not the correct way to do. We should stick with the patches >>> sent to ML and included in patchworks. They are documented and I cannot >>> (that means, all custodians are not allowed) to simply pick up patches >>> from somewhere else. >> >> True, but I am not saying you to just apply the branch. My intention >> was to check the new changes from your side so-that will send the >> series to ML again. > > ok > >> >>> >>> It is enough that we agree about issue and solution. I had already >>> tested with the following patch: >>> >>> diff --git a/configs/imx6qdl_icore_mmc_defconfig >>> b/configs/imx6qdl_icore_mmc_defconfig >>> index 221ea7e..c947e10 100644 >>> --- a/configs/imx6qdl_icore_mmc_defconfig >>> +++ b/configs/imx6qdl_icore_mmc_defconfig >>> @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" >>> CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" >>> CONFIG_SYS_PROMPT="icorem6qdl> " >>> CONFIG_SPL=y >>> +# CONFIG_BLK is not set >>> +# CONFIG_DM_MMC_OPS is not set >>> CONFIG_BOOTDELAY=3 >>> CONFIG_BOARD_EARLY_INIT_F=y >>> CONFIG_DISPLAY_CPUINFO=y >>> diff --git a/configs/imx6qdl_icore_nand_defconfig >> >> nand_defconfig doesn't require this change. >> > > It is what I had expected, but fsl_esdhc is built even in the NAND > configuration. There is omething else enabling (not in defconfig). > > In fact, dropping the two lines, I get the error again for > imx6qdl_icore_nand. > > The reason is that even if MMC is not set, as far I can see, these are > automatically set: > > CONFIG_DM_MMC=y > CONFIG_DM_MMC_OPS=y Ohh, Ok, This is because mx6_common.h by default enabling the MMC. So I will update these on nand support patch[1] and send the single patch, is that OK? [1] [PATCH v8 21/23] imx6: icorem6: Add NAND support
Hi Jagan, On 24/10/2016 14:09, Jagan Teki wrote: > Hi Stefano, > > On Mon, Oct 24, 2016 at 5:31 PM, Stefano Babic <sbabic@denx.de> wrote: >> Hi Jagan, >> >> On 24/10/2016 13:46, Jagan Teki wrote: >> >>>> >>>> No, this is not the correct way to do. We should stick with the patches >>>> sent to ML and included in patchworks. They are documented and I cannot >>>> (that means, all custodians are not allowed) to simply pick up patches >>>> from somewhere else. >>> >>> True, but I am not saying you to just apply the branch. My intention >>> was to check the new changes from your side so-that will send the >>> series to ML again. >> >> ok >> >>> >>>> >>>> It is enough that we agree about issue and solution. I had already >>>> tested with the following patch: >>>> >>>> diff --git a/configs/imx6qdl_icore_mmc_defconfig >>>> b/configs/imx6qdl_icore_mmc_defconfig >>>> index 221ea7e..c947e10 100644 >>>> --- a/configs/imx6qdl_icore_mmc_defconfig >>>> +++ b/configs/imx6qdl_icore_mmc_defconfig >>>> @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" >>>> CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" >>>> CONFIG_SYS_PROMPT="icorem6qdl> " >>>> CONFIG_SPL=y >>>> +# CONFIG_BLK is not set >>>> +# CONFIG_DM_MMC_OPS is not set >>>> CONFIG_BOOTDELAY=3 >>>> CONFIG_BOARD_EARLY_INIT_F=y >>>> CONFIG_DISPLAY_CPUINFO=y >>>> diff --git a/configs/imx6qdl_icore_nand_defconfig >>> >>> nand_defconfig doesn't require this change. >>> >> >> It is what I had expected, but fsl_esdhc is built even in the NAND >> configuration. There is omething else enabling (not in defconfig). >> >> In fact, dropping the two lines, I get the error again for >> imx6qdl_icore_nand. >> >> The reason is that even if MMC is not set, as far I can see, these are >> automatically set: >> >> CONFIG_DM_MMC=y >> CONFIG_DM_MMC_OPS=y > > Ohh, Ok, This is because mx6_common.h by default enabling the MMC. Right. > So I will update these on nand support patch[1] and send the single > patch, is that OK? > > [1] [PATCH v8 21/23] imx6: icorem6: Add NAND support > Please wait, I am confused. You have sent a complete V8, but as far as I understand the oinly changes are reported above (Patch 14/15). However, even if I have not tested, this would break bisecting, because patch 4/23 introduces the board and building with that commit leads to the error. So changes should be respective in patch 4 for mmc and patch 21 for NAND taking as reference V8. Best regards, Stefano Babic
On Mon, Oct 24, 2016 at 6:04 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi Jagan, > > On 24/10/2016 14:09, Jagan Teki wrote: >> Hi Stefano, >> >> On Mon, Oct 24, 2016 at 5:31 PM, Stefano Babic <sbabic@denx.de> wrote: >>> Hi Jagan, >>> >>> On 24/10/2016 13:46, Jagan Teki wrote: >>> >>>>> >>>>> No, this is not the correct way to do. We should stick with the patches >>>>> sent to ML and included in patchworks. They are documented and I cannot >>>>> (that means, all custodians are not allowed) to simply pick up patches >>>>> from somewhere else. >>>> >>>> True, but I am not saying you to just apply the branch. My intention >>>> was to check the new changes from your side so-that will send the >>>> series to ML again. >>> >>> ok >>> >>>> >>>>> >>>>> It is enough that we agree about issue and solution. I had already >>>>> tested with the following patch: >>>>> >>>>> diff --git a/configs/imx6qdl_icore_mmc_defconfig >>>>> b/configs/imx6qdl_icore_mmc_defconfig >>>>> index 221ea7e..c947e10 100644 >>>>> --- a/configs/imx6qdl_icore_mmc_defconfig >>>>> +++ b/configs/imx6qdl_icore_mmc_defconfig >>>>> @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" >>>>> CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" >>>>> CONFIG_SYS_PROMPT="icorem6qdl> " >>>>> CONFIG_SPL=y >>>>> +# CONFIG_BLK is not set >>>>> +# CONFIG_DM_MMC_OPS is not set >>>>> CONFIG_BOOTDELAY=3 >>>>> CONFIG_BOARD_EARLY_INIT_F=y >>>>> CONFIG_DISPLAY_CPUINFO=y >>>>> diff --git a/configs/imx6qdl_icore_nand_defconfig >>>> >>>> nand_defconfig doesn't require this change. >>>> >>> >>> It is what I had expected, but fsl_esdhc is built even in the NAND >>> configuration. There is omething else enabling (not in defconfig). >>> >>> In fact, dropping the two lines, I get the error again for >>> imx6qdl_icore_nand. >>> >>> The reason is that even if MMC is not set, as far I can see, these are >>> automatically set: >>> >>> CONFIG_DM_MMC=y >>> CONFIG_DM_MMC_OPS=y >> >> Ohh, Ok, This is because mx6_common.h by default enabling the MMC. > > Right. > >> So I will update these on nand support patch[1] and send the single >> patch, is that OK? >> >> [1] [PATCH v8 21/23] imx6: icorem6: Add NAND support >> > > Please wait, I am confused. You have sent a complete V8, but as far as I > understand the oinly changes are reported above (Patch 14/15). > > However, even if I have not tested, this would break bisecting, because > patch 4/23 introduces the board and building with that commit leads to > the error. So changes should be respective in patch 4 for mmc and patch > 21 for NAND taking as reference V8. Yes, I've added 14/23 and 15/23 for new changes so the previous 14/21 becomes 16/23 now and the only missing one is 21/23. So I will update this 21/23 for v9. Hope this make clear for you. thanks!
On 24/10/2016 15:15, Jagan Teki wrote: > On Mon, Oct 24, 2016 at 6:04 PM, Stefano Babic <sbabic@denx.de> wrote: >> Hi Jagan, >> >> Please wait, I am confused. You have sent a complete V8, but as far as I >> understand the oinly changes are reported above (Patch 14/15). >> >> However, even if I have not tested, this would break bisecting, because >> patch 4/23 introduces the board and building with that commit leads to >> the error. So changes should be respective in patch 4 for mmc and patch >> 21 for NAND taking as reference V8. > > Yes, I've added 14/23 and 15/23 for new changes so the previous 14/21 > becomes 16/23 now and the only missing one is 21/23. So I will update > this 21/23 for v9. Hope this make clear for you. It is ok if you feel better to send V9 - what I meant it is just that entries for CONFIG_DM_MM and CONFIG_DM_MMC_OPS should be done in the same patch where the defconfig is added to avoid breaking bisecting. Regards, Stefano
On Mon, Oct 24, 2016 at 7:31 PM, Stefano Babic <sbabic@denx.de> wrote: > On 24/10/2016 15:15, Jagan Teki wrote: >> On Mon, Oct 24, 2016 at 6:04 PM, Stefano Babic <sbabic@denx.de> wrote: >>> Hi Jagan, > >>> >>> Please wait, I am confused. You have sent a complete V8, but as far as I >>> understand the oinly changes are reported above (Patch 14/15). >>> >>> However, even if I have not tested, this would break bisecting, because >>> patch 4/23 introduces the board and building with that commit leads to >>> the error. So changes should be respective in patch 4 for mmc and patch >>> 21 for NAND taking as reference V8. >> >> Yes, I've added 14/23 and 15/23 for new changes so the previous 14/21 >> becomes 16/23 now and the only missing one is 21/23. So I will update >> this 21/23 for v9. Hope this make clear for you. > > It is ok if you feel better to send V9 - what I meant it is just that > entries for CONFIG_DM_MM and CONFIG_DM_MMC_OPS should be done in the > same patch where the defconfig is added to avoid breaking bisecting. OK, then I will send the 14/23 with squash of 14/23 and 15/23 to 16/23 and 21/23 v9. Does it fine? thanks!
Hi Stefano, On Mon, Oct 24, 2016 at 7:51 PM, Jagan Teki <jagan@openedev.com> wrote: > On Mon, Oct 24, 2016 at 7:31 PM, Stefano Babic <sbabic@denx.de> wrote: >> On 24/10/2016 15:15, Jagan Teki wrote: >>> On Mon, Oct 24, 2016 at 6:04 PM, Stefano Babic <sbabic@denx.de> wrote: >>>> Hi Jagan, >> >>>> >>>> Please wait, I am confused. You have sent a complete V8, but as far as I >>>> understand the oinly changes are reported above (Patch 14/15). >>>> >>>> However, even if I have not tested, this would break bisecting, because >>>> patch 4/23 introduces the board and building with that commit leads to >>>> the error. So changes should be respective in patch 4 for mmc and patch >>>> 21 for NAND taking as reference V8. >>> >>> Yes, I've added 14/23 and 15/23 for new changes so the previous 14/21 >>> becomes 16/23 now and the only missing one is 21/23. So I will update >>> this 21/23 for v9. Hope this make clear for you. >> >> It is ok if you feel better to send V9 - what I meant it is just that >> entries for CONFIG_DM_MM and CONFIG_DM_MMC_OPS should be done in the >> same patch where the defconfig is added to avoid breaking bisecting. > > OK, then I will send the 14/23 with squash of 14/23 and 15/23 to 16/23 > and 21/23 v9. > Does it fine? Sent these two patches with v9, please let me know for any issues. thanks!
diff --git a/configs/imx6qdl_icore_mmc_defconfig b/configs/imx6qdl_icore_mmc_defconfig index 221ea7e..c947e10 100644 --- a/configs/imx6qdl_icore_mmc_defconfig +++ b/configs/imx6qdl_icore_mmc_defconfig @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" CONFIG_SYS_PROMPT="icorem6qdl> " CONFIG_SPL=y +# CONFIG_BLK is not set +# CONFIG_DM_MMC_OPS is not set CONFIG_BOOTDELAY=3 CONFIG_BOARD_EARLY_INIT_F=y CONFIG_DISPLAY_CPUINFO=y diff --git a/configs/imx6qdl_icore_nand_defconfig b/configs/imx6qdl_icore_nand_defconfig index 8ac3099..55650bb 100644 --- a/configs/imx6qdl_icore_nand_defconfig +++ b/configs/imx6qdl_icore_nand_defconfig @@ -6,6 +6,8 @@ CONFIG_DEFAULT_FDT_FILE="imx6dl-icore.dtb" CONFIG_DEFAULT_DEVICE_TREE="imx6dl-icore" CONFIG_SYS_PROMPT="icorem6qdl> " CONFIG_SPL=y +# CONFIG_BLK is not set +# CONFIG_DM_MMC_OPS is not set CONFIG_BOOTDELAY=3 CONFIG_BOARD_EARLY_INIT_F=y CONFIG_DISPLAY_CPUINFO=y