Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Simon, On Tue, Aug 13, 2019 at 10:38:22PM +0200, Simon Goldschmidt wrote: > Am 07.08.2019 um 23:23 schrieb Andreas Dannenberg: > > Hi Simon, > > thanks for your patience waiting for a response. Please see comments > > inlined... > > > > On Thu, Jul 25, 2019 at 11:52:55AM +0200, Simon Goldschmidt wrote: > > > On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: > > > > > > > > Hi Simon, > > > > > > > > On 25/07/19 12:31 PM, Simon Goldschmidt wrote: > > > > > Hi Lokesh, > > > > > > > > > > thanks for following up on this. > > > > > > > > > > On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla > > > > > wrote: > > > > > > > > > > > > Hi Tom, > > > > > > > > > > > > On 20/07/19 9:21 PM, Tom Rini wrote: > > > > > > > On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > > > > > > > > On Fri, Jul 19, 2019 at 2:29 AM Tom Rini > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > In order to be able to use more advanced driver > > > > > > > > > > functionality which often > > > > > > > > > > relies on having BSS initialized during early boot prior to > > > > > > > > > > relocation > > > > > > > > > > several things need to be in place: > > > > > > > > > > > > > > > > > > > > 1) Memory needs to be available for BSS to use. For this, > > > > > > > > > > we locate BSS > > > > > > > > > > at the top of the MCU SRAM area, with the stack > > > > > > > > > > starting right below > > > > > > > > > > it, > > > > > > > > > > 2) We need to move the initialization of BSS prior to > > > > > > > > > > entering > > > > > > > > > > board_init_f(). We will do this with a separate commit > > > > > > > > > > by turning on > > > > > > > > > > the respective CONFIG option. > > > > > > > > > > > > > > > > > > > > In this commit we also clean up the assignment of the > > > > > > > > > > initial SP address > > > > > > > > > > as part of the refactoring, taking into account the > > > > > > > > > > pre-decrement post- > > > > > > > > > > increment nature in which the SP is used on ARM. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Andreas Dannenberg > > > > > > > > > > > > > > > > > > Applied to u-boot/master, thanks! > > > > > > > > > > > > > > > > Wait, why has this been merged? Unfortunately, I haven't > > > > > > > > followed this series, > > > > > > > > but in a discussion about a similar patch I sent [1], using BSS > > > > > > > > from > > > > > > > > board_init_f > > > > > > > > was turned down. And Simon Glass rather convinced me that this > > > > > > > > is the current > > > > > > > > API U-Boot has (and is documented in README). > > > > > > > > > > > > > > > > So either we must change this API and its documentation (and I > > > > > > > > would expect the > > > > > > > > author of this patch to combine the README change with the code > > > > > > > > change), or this > > > > > > > > patch would have to be rejected. > > > > > > > > > > > > > > > > Again, I'm sorry I only see this now. In thought to remember a > > > > > > > > discussion in this > > > > > > > > thread, but I clearly remember that wrong... > > > > > > > > > > > > > > > > [1] https://patchwork.ozlabs.org/patch/1057237/ > > > > > > > > > > > > > > And I had missed that other thread. Lokesh, since I think > > > > > > > Andreas is > > > > > > > out currently can you expand a little on what we can/can't do on > > > > > > > this > > > > > > > platform? Thanks! > > > > > > > > > > > > The reason why BSS is needed very early in this platform is for the > > > > > > following > > > > > > reasons: > > > > > > - System co-processor is the central resource manager in SoC and > > > > > > should be > > > > > > loaded and started very early in the boot process. Without that no > > > > > > peripheral or > > > > > > memory can be initialized. So for loading system co-processor > > > > > > image, we only > > > > > > have limited SRAM and a peripheral initialized by ROM. > > > > > > - System co-processor(DMSC) is being represented as remote-core in > > > > > > Device-tree(We are strictly following DM and DT model for the > > > > > > entire SoC). > > > > > > - Since DM is also followed by each peripheral device and remote > > > > > > core, DM should > > > > > > be enabled very early and many peripheral drivers are dependent on > > > > > > BSS usage. > > > > > > So, BSS has been made available very early. > > > > > > > > > > > > Hope this is clear. Let me know if more details are required, I > > > > > > will be happy to > > > > > > explain. > > > > > > > > > > Don't get me wrong: I'm not against using BSS early. I just want to > > > > > ensure this > > > > > stays consistent throught U-Boot. > > > > > > > > I understand and agree that it should be consistent. Just discussed > > > > this with > > > > Andreas, who is courteous enough to update the details in his vacation.
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Am 07.08.2019 um 23:23 schrieb Andreas Dannenberg: Hi Simon, thanks for your patience waiting for a response. Please see comments inlined... On Thu, Jul 25, 2019 at 11:52:55AM +0200, Simon Goldschmidt wrote: On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: Hi Simon, On 25/07/19 12:31 PM, Simon Goldschmidt wrote: Hi Lokesh, thanks for following up on this. On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: Hi Tom, On 20/07/19 9:21 PM, Tom Rini wrote: On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: In order to be able to use more advanced driver functionality which often relies on having BSS initialized during early boot prior to relocation several things need to be in place: 1) Memory needs to be available for BSS to use. For this, we locate BSS at the top of the MCU SRAM area, with the stack starting right below it, 2) We need to move the initialization of BSS prior to entering board_init_f(). We will do this with a separate commit by turning on the respective CONFIG option. In this commit we also clean up the assignment of the initial SP address as part of the refactoring, taking into account the pre-decrement post- increment nature in which the SP is used on ARM. Signed-off-by: Andreas Dannenberg Applied to u-boot/master, thanks! Wait, why has this been merged? Unfortunately, I haven't followed this series, but in a discussion about a similar patch I sent [1], using BSS from board_init_f was turned down. And Simon Glass rather convinced me that this is the current API U-Boot has (and is documented in README). So either we must change this API and its documentation (and I would expect the author of this patch to combine the README change with the code change), or this patch would have to be rejected. Again, I'm sorry I only see this now. In thought to remember a discussion in this thread, but I clearly remember that wrong... [1] https://patchwork.ozlabs.org/patch/1057237/ And I had missed that other thread. Lokesh, since I think Andreas is out currently can you expand a little on what we can/can't do on this platform? Thanks! The reason why BSS is needed very early in this platform is for the following reasons: - System co-processor is the central resource manager in SoC and should be loaded and started very early in the boot process. Without that no peripheral or memory can be initialized. So for loading system co-processor image, we only have limited SRAM and a peripheral initialized by ROM. - System co-processor(DMSC) is being represented as remote-core in Device-tree(We are strictly following DM and DT model for the entire SoC). - Since DM is also followed by each peripheral device and remote core, DM should be enabled very early and many peripheral drivers are dependent on BSS usage. So, BSS has been made available very early. Hope this is clear. Let me know if more details are required, I will be happy to explain. Don't get me wrong: I'm not against using BSS early. I just want to ensure this stays consistent throught U-Boot. I understand and agree that it should be consistent. Just discussed this with Andreas, who is courteous enough to update the details in his vacation. We don't have to rush here, I don't have a problem waiting for Andreas to answer when he's back. The reasons you stated still don't make it clear to me *why* bss is needed early. There are other boards using DM early that don't need this. In my opinion, DM drivers normally don't rely on BSS but keep all their state in This statement doesn't hold true for all the drviers. At least the mmc driver uses "initialized" variable stored in BSS to avoid initializing mmc multiple times[0]. In the past we en counted other drivers using it. I guess the idea here is to enable the BSS support generically instead of fixing each of every driver. So this driver is generally not usable in pre-relocation phase? The README document is pretty clear about BSS not being available in board_init_f. I know this text is old, but it seems still valid. And if this is really a workaround because it's easier to use this workaround instead of fixing drivers that invalidly use BSS, is this what we want? heap memory. If you only need BSS early because drivers rely on BSS, you might have to fix those drivers? So, correct me here, why should driver be restricted to not use BSS? Post-relocation drivers might be free to use BSS (although you lose the per-instance storage when using BSS instead of the driver's priv data), but pre-relocation drivers are not. That's the current definition in U-Boot. This patch changes it by adding the option to use BSS early. This bears the danger of code being changed in a way that it requires BSS to be available early and might not work on other boards that actually cannot provide BSS early (e.g. before SDRAM is up or
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Am 08.08.2019 um 21:43 schrieb Andreas Dannenberg: Hi Simon, On Thu, Aug 08, 2019 at 09:01:03PM +0200, Simon Goldschmidt wrote: Am 08.08.2019 um 20:29 schrieb Andreas Dannenberg: Ok back to my specific scenario, hopefully I'm adding at least some new aspects now rather than repeating what was discussed already in different threads... From SPL I'm required to load (and start) our "System Firmware" which is a prerequisite for bringing up DDR. We know that DDR bringup itself should happen in SPL's board_init_f(), hence the need for loading stuff from board_init_f() when no DDR is yet available (only on-chip memory). I'm using the same loader framework to do the loading from board_init_f() that SPL later uses from a board_init_r() context to load U-Boot proper, ATF, and other files depending on the platform. > Now let's focus on two static variables that play a role in this context, 'fat_registered' spl_fat.c and '*mmc' in spl_mmc.c. Those static variables are essentially used to remember the initialization state of the FAT driver and the MMC loader, so that it doesn't get re-initialized the second time those get called (during SPL's main usage of loading U-Boot, etc.). So essentially the desire is to carry this initialization state from SPL's board_init_f() to board_init_r(). OK, so essentially, you've added CONFIG_SPL_EARLY_BSS because FAT and MMC contain variables in BSS? That would mean we could drop that config option after fixing those two (given that they can be fixed)? Yes I think I could drop it in this case, I'm not married to CONFIG_SPL_EARLY_BSS in any way. But we still have all the other platforms that use memset() to directly clear BSS from SPL's board_init_f()... Ideally any improvements should be made across the board. (And I just need to throw that out there) one could also imagine moving said platforms to also use CONFIG_SPL_EARLY_BSS (replacing their custom memset approach) to at least unify the approach... That could well be a valid approach. However, ideally, we'd first check why these boards really need that memset. I fixed socfgpa_gen5 by fixing the sdram driver to not use BSS... (a rather simple fix that even ended up reducing code size by keeping the data on the stack). it), but how could this play in here? Sure I can reserve some memory from board_init_f(), or the drivers under discussion, and store the initialization state there, but now I'd have the need to carry the pointer to that initialization data forward somehow. spl_fat.c is not a DM driver, it inherently doesn't have anything I can "tack on" additional data fields. I don't quite see how I can make this work more elegantly but I'm open to suggestions... No, sorry, what I wrote was probably a bit confusing. I wrote that as a result of my work on socfpga_a10. There, we have the firmware loader framework loading things e.g. from MMC. During my test of unifying the socfpga config header files (both gen5 and a10 combined), I stumbled accross the fact that you cannot use standard malloc in SPL when the devicetree is initialized during board_init_f as dlmalloc.c makes heavy use of BSS. You can only use simple malloc there, because its state is kept in 'gd'. But it seems that wasn't your problem? A next problem with simple malloc is that you can't free anything and I think I remember code passages around file system loading that make heavy use of malloc/free. But that again doesn't seem to be your problem here? I had this exact problem initially, as I was loading two files from board_init_f (the "system firmware" and it's config data), which I since consolidated into a FIT image avoiding the issue altogether. The FAT driver specifically is very wasteful allocating 64KB chunks of memory (you can reduce the sector size to alleviate some of that) and with simple malloc it would eat through the little RAM I had too quickly... So for a while I was using full malloc from board_init_f() to make the FAT driver happy. And I had also found I need BSS for that after a little pain and suffering with the JTAG debugger. But this is no longer n issue when only loading one file. Yeah, well, that issue remains even if we'd fix the already mentioned problems... (Mr. Glass had suggested in one of the threads why I don't do the DDR initialization in board_init_r() then, which I experimented with, but the changes I had to make to common U-Boot files were rather drastic so I abandoned this attempt). Yes, I can understand that that's not an ideal way to move forward... To come back on the original issue, I'd still propose to add these static variables to 'gd' or to some sub-struct referenced from 'gd'. I see a high risk for others to run into these issues that you have hidden for your platform by enabling CONFIG_SPL_EARLY_BSS. I was always hesitant even thinking about adding stuff to gd due to the large impact so I haven't really considered this as a feasible path. But if we were to go down this road
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Simon, On Thu, Aug 08, 2019 at 09:01:03PM +0200, Simon Goldschmidt wrote: > Am 08.08.2019 um 20:29 schrieb Andreas Dannenberg: > > Ok back to my specific scenario, hopefully I'm adding at least some new > > aspects now rather than repeating what was discussed already in different > > threads... > > > > From SPL I'm required to load (and start) our "System Firmware" which is > > a prerequisite for bringing up DDR. We know that DDR bringup itself > > should happen in SPL's board_init_f(), hence the need for loading stuff > > from board_init_f() when no DDR is yet available (only on-chip memory). > > > > I'm using the same loader framework to do the loading from > > board_init_f() that SPL later uses from a board_init_r() context to > > load U-Boot proper, ATF, and other files depending on the platform. > > > Now let's focus on two static variables that play a role in this > > context, 'fat_registered' spl_fat.c and '*mmc' in spl_mmc.c. Those > > static variables are essentially used to remember the initialization > > state of the FAT driver and the MMC loader, so that it doesn't get > > re-initialized the second time those get called (during SPL's main usage > > of loading U-Boot, etc.). So essentially the desire is to carry this > > initialization state from SPL's board_init_f() to board_init_r(). > > OK, so essentially, you've added CONFIG_SPL_EARLY_BSS because FAT and MMC > contain variables in BSS? That would mean we could drop that config option > after fixing those two (given that they can be fixed)? Yes I think I could drop it in this case, I'm not married to CONFIG_SPL_EARLY_BSS in any way. But we still have all the other platforms that use memset() to directly clear BSS from SPL's board_init_f()... Ideally any improvements should be made across the board. (And I just need to throw that out there) one could also imagine moving said platforms to also use CONFIG_SPL_EARLY_BSS (replacing their custom memset approach) to at least unify the approach... > > it), but how could this play in here? Sure I can reserve some memory > > from board_init_f(), or the drivers under discussion, and store the > > initialization state there, but now I'd have the need to carry the > > pointer to that initialization data forward somehow. spl_fat.c is not a > > DM driver, it inherently doesn't have anything I can "tack on" additional > > data fields. I don't quite see how I can make this work more elegantly > > but I'm open to suggestions... > > No, sorry, what I wrote was probably a bit confusing. I wrote that as a > result of my work on socfpga_a10. There, we have the firmware loader > framework loading things e.g. from MMC. During my test of unifying the > socfpga config header files (both gen5 and a10 combined), I stumbled accross > the fact that you cannot use standard malloc in SPL when the devicetree is > initialized during board_init_f as dlmalloc.c makes heavy use of BSS. You > can only use simple malloc there, because its state is kept in 'gd'. > > But it seems that wasn't your problem? > > A next problem with simple malloc is that you can't free anything and I > think I remember code passages around file system loading that make heavy > use of malloc/free. But that again doesn't seem to be your problem here? I had this exact problem initially, as I was loading two files from board_init_f (the "system firmware" and it's config data), which I since consolidated into a FIT image avoiding the issue altogether. The FAT driver specifically is very wasteful allocating 64KB chunks of memory (you can reduce the sector size to alleviate some of that) and with simple malloc it would eat through the little RAM I had too quickly... So for a while I was using full malloc from board_init_f() to make the FAT driver happy. And I had also found I need BSS for that after a little pain and suffering with the JTAG debugger. But this is no longer n issue when only loading one file. > > (Mr. Glass had suggested in one of the threads why I don't do the > > DDR initialization in board_init_r() then, which I experimented with, > > but the changes I had to make to common U-Boot files were rather drastic > > so I abandoned this attempt). > > Yes, I can understand that that's not an ideal way to move forward... > > To come back on the original issue, I'd still propose to add these static > variables to 'gd' or to some sub-struct referenced from 'gd'. I see a high > risk for others to run into these issues that you have hidden for your > platform by enabling CONFIG_SPL_EARLY_BSS. I was always hesitant even thinking about adding stuff to gd due to the large impact so I haven't really considered this as a feasible path. But if we were to go down this road possibly we could add a bitfield variable of some type (to be considerate of memory use) containing a collection of "initialized" flags used by different drivers that really need it? -- Andreas Dannenberg Texas Instruments Inc
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Am 08.08.2019 um 20:29 schrieb Andreas Dannenberg: Simon, On Thu, Aug 08, 2019 at 09:29:03AM +0200, Simon Goldschmidt wrote: Hi Andreas, On Wed, Aug 7, 2019 at 11:24 PM Andreas Dannenberg wrote: Hi Simon, thanks for your patience waiting for a response. Please see comments inlined... On Thu, Jul 25, 2019 at 11:52:55AM +0200, Simon Goldschmidt wrote: On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: Hi Simon, On 25/07/19 12:31 PM, Simon Goldschmidt wrote: Hi Lokesh, thanks for following up on this. On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: Hi Tom, On 20/07/19 9:21 PM, Tom Rini wrote: On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: In order to be able to use more advanced driver functionality which often relies on having BSS initialized during early boot prior to relocation several things need to be in place: 1) Memory needs to be available for BSS to use. For this, we locate BSS at the top of the MCU SRAM area, with the stack starting right below it, 2) We need to move the initialization of BSS prior to entering board_init_f(). We will do this with a separate commit by turning on the respective CONFIG option. In this commit we also clean up the assignment of the initial SP address as part of the refactoring, taking into account the pre-decrement post- increment nature in which the SP is used on ARM. Signed-off-by: Andreas Dannenberg Applied to u-boot/master, thanks! Wait, why has this been merged? Unfortunately, I haven't followed this series, but in a discussion about a similar patch I sent [1], using BSS from board_init_f was turned down. And Simon Glass rather convinced me that this is the current API U-Boot has (and is documented in README). So either we must change this API and its documentation (and I would expect the author of this patch to combine the README change with the code change), or this patch would have to be rejected. Again, I'm sorry I only see this now. In thought to remember a discussion in this thread, but I clearly remember that wrong... [1] https://patchwork.ozlabs.org/patch/1057237/ And I had missed that other thread. Lokesh, since I think Andreas is out currently can you expand a little on what we can/can't do on this platform? Thanks! The reason why BSS is needed very early in this platform is for the following reasons: - System co-processor is the central resource manager in SoC and should be loaded and started very early in the boot process. Without that no peripheral or memory can be initialized. So for loading system co-processor image, we only have limited SRAM and a peripheral initialized by ROM. - System co-processor(DMSC) is being represented as remote-core in Device-tree(We are strictly following DM and DT model for the entire SoC). - Since DM is also followed by each peripheral device and remote core, DM should be enabled very early and many peripheral drivers are dependent on BSS usage. So, BSS has been made available very early. Hope this is clear. Let me know if more details are required, I will be happy to explain. Don't get me wrong: I'm not against using BSS early. I just want to ensure this stays consistent throught U-Boot. I understand and agree that it should be consistent. Just discussed this with Andreas, who is courteous enough to update the details in his vacation. We don't have to rush here, I don't have a problem waiting for Andreas to answer when he's back. The reasons you stated still don't make it clear to me *why* bss is needed early. There are other boards using DM early that don't need this. In my opinion, DM drivers normally don't rely on BSS but keep all their state in This statement doesn't hold true for all the drviers. At least the mmc driver uses "initialized" variable stored in BSS to avoid initializing mmc multiple times[0]. In the past we en counted other drivers using it. I guess the idea here is to enable the BSS support generically instead of fixing each of every driver. So this driver is generally not usable in pre-relocation phase? The README document is pretty clear about BSS not being available in board_init_f. I know this text is old, but it seems still valid. And if this is really a workaround because it's easier to use this workaround instead of fixing drivers that invalidly use BSS, is this what we want? heap memory. If you only need BSS early because drivers rely on BSS, you might have to fix those drivers? So, correct me here, why should driver be restricted to not use BSS? Post-relocation drivers might be free to use BSS (although you lose the per-instance storage when using BSS instead of the driver's priv data), but pre-relocation drivers are not. That's the current definition in U-Boot. This patch changes it by adding the option to use BSS early. This bears the danger of code being changed in a
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Simon, On Thu, Aug 08, 2019 at 09:29:03AM +0200, Simon Goldschmidt wrote: > Hi Andreas, > > On Wed, Aug 7, 2019 at 11:24 PM Andreas Dannenberg wrote: > > > > Hi Simon, > > thanks for your patience waiting for a response. Please see comments > > inlined... > > > > On Thu, Jul 25, 2019 at 11:52:55AM +0200, Simon Goldschmidt wrote: > > > On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: > > > > > > > > Hi Simon, > > > > > > > > On 25/07/19 12:31 PM, Simon Goldschmidt wrote: > > > > > Hi Lokesh, > > > > > > > > > > thanks for following up on this. > > > > > > > > > > On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla > > > > > wrote: > > > > >> > > > > >> Hi Tom, > > > > >> > > > > >> On 20/07/19 9:21 PM, Tom Rini wrote: > > > > >>> On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > > > > On Fri, Jul 19, 2019 at 2:29 AM Tom Rini > > > > wrote: > > > > > > > > > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg > > > > > wrote: > > > > > > > > > >> In order to be able to use more advanced driver functionality > > > > >> which often > > > > >> relies on having BSS initialized during early boot prior to > > > > >> relocation > > > > >> several things need to be in place: > > > > >> > > > > >> 1) Memory needs to be available for BSS to use. For this, we > > > > >> locate BSS > > > > >>at the top of the MCU SRAM area, with the stack starting > > > > >> right below > > > > >>it, > > > > >> 2) We need to move the initialization of BSS prior to entering > > > > >>board_init_f(). We will do this with a separate commit by > > > > >> turning on > > > > >>the respective CONFIG option. > > > > >> > > > > >> In this commit we also clean up the assignment of the initial SP > > > > >> address > > > > >> as part of the refactoring, taking into account the > > > > >> pre-decrement post- > > > > >> increment nature in which the SP is used on ARM. > > > > >> > > > > >> Signed-off-by: Andreas Dannenberg > > > > > > > > > > Applied to u-boot/master, thanks! > > > > > > > > Wait, why has this been merged? Unfortunately, I haven't followed > > > > this series, > > > > but in a discussion about a similar patch I sent [1], using BSS > > > > from > > > > board_init_f > > > > was turned down. And Simon Glass rather convinced me that this is > > > > the current > > > > API U-Boot has (and is documented in README). > > > > > > > > So either we must change this API and its documentation (and I > > > > would expect the > > > > author of this patch to combine the README change with the code > > > > change), or this > > > > patch would have to be rejected. > > > > > > > > Again, I'm sorry I only see this now. In thought to remember a > > > > discussion in this > > > > thread, but I clearly remember that wrong... > > > > > > > > [1] https://patchwork.ozlabs.org/patch/1057237/ > > > > >>> > > > > >>> And I had missed that other thread. Lokesh, since I think Andreas > > > > >>> is > > > > >>> out currently can you expand a little on what we can/can't do on > > > > >>> this > > > > >>> platform? Thanks! > > > > >> > > > > >> The reason why BSS is needed very early in this platform is for the > > > > >> following > > > > >> reasons: > > > > >> - System co-processor is the central resource manager in SoC and > > > > >> should be > > > > >> loaded and started very early in the boot process. Without that no > > > > >> peripheral or > > > > >> memory can be initialized. So for loading system co-processor image, > > > > >> we only > > > > >> have limited SRAM and a peripheral initialized by ROM. > > > > >> - System co-processor(DMSC) is being represented as remote-core in > > > > >> Device-tree(We are strictly following DM and DT model for the entire > > > > >> SoC). > > > > >> - Since DM is also followed by each peripheral device and remote > > > > >> core, DM should > > > > >> be enabled very early and many peripheral drivers are dependent on > > > > >> BSS usage. > > > > >> So, BSS has been made available very early. > > > > >> > > > > >> Hope this is clear. Let me know if more details are required, I will > > > > >> be happy to > > > > >> explain. > > > > > > > > > > Don't get me wrong: I'm not against using BSS early. I just want to > > > > > ensure this > > > > > stays consistent throught U-Boot. > > > > > > > > I understand and agree that it should be consistent. Just discussed > > > > this with > > > > Andreas, who is courteous enough to update the details in his vacation. > > > > > > We don't have to rush here, I don't have a problem waiting for Andreas to > > > answer when he's back. > > > > > > > > > > > > > > > > > The reasons you stated still don't make it clear to me *why* bss is > > > > > needed > > > > > early. There are other
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Andreas, On Wed, Aug 7, 2019 at 11:24 PM Andreas Dannenberg wrote: > > Hi Simon, > thanks for your patience waiting for a response. Please see comments > inlined... > > On Thu, Jul 25, 2019 at 11:52:55AM +0200, Simon Goldschmidt wrote: > > On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: > > > > > > Hi Simon, > > > > > > On 25/07/19 12:31 PM, Simon Goldschmidt wrote: > > > > Hi Lokesh, > > > > > > > > thanks for following up on this. > > > > > > > > On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: > > > >> > > > >> Hi Tom, > > > >> > > > >> On 20/07/19 9:21 PM, Tom Rini wrote: > > > >>> On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > > > On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > > > > > > > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > > > > > > > >> In order to be able to use more advanced driver functionality > > > >> which often > > > >> relies on having BSS initialized during early boot prior to > > > >> relocation > > > >> several things need to be in place: > > > >> > > > >> 1) Memory needs to be available for BSS to use. For this, we > > > >> locate BSS > > > >>at the top of the MCU SRAM area, with the stack starting right > > > >> below > > > >>it, > > > >> 2) We need to move the initialization of BSS prior to entering > > > >>board_init_f(). We will do this with a separate commit by > > > >> turning on > > > >>the respective CONFIG option. > > > >> > > > >> In this commit we also clean up the assignment of the initial SP > > > >> address > > > >> as part of the refactoring, taking into account the pre-decrement > > > >> post- > > > >> increment nature in which the SP is used on ARM. > > > >> > > > >> Signed-off-by: Andreas Dannenberg > > > > > > > > Applied to u-boot/master, thanks! > > > > > > Wait, why has this been merged? Unfortunately, I haven't followed > > > this series, > > > but in a discussion about a similar patch I sent [1], using BSS from > > > board_init_f > > > was turned down. And Simon Glass rather convinced me that this is > > > the current > > > API U-Boot has (and is documented in README). > > > > > > So either we must change this API and its documentation (and I would > > > expect the > > > author of this patch to combine the README change with the code > > > change), or this > > > patch would have to be rejected. > > > > > > Again, I'm sorry I only see this now. In thought to remember a > > > discussion in this > > > thread, but I clearly remember that wrong... > > > > > > [1] https://patchwork.ozlabs.org/patch/1057237/ > > > >>> > > > >>> And I had missed that other thread. Lokesh, since I think Andreas is > > > >>> out currently can you expand a little on what we can/can't do on this > > > >>> platform? Thanks! > > > >> > > > >> The reason why BSS is needed very early in this platform is for the > > > >> following > > > >> reasons: > > > >> - System co-processor is the central resource manager in SoC and > > > >> should be > > > >> loaded and started very early in the boot process. Without that no > > > >> peripheral or > > > >> memory can be initialized. So for loading system co-processor image, > > > >> we only > > > >> have limited SRAM and a peripheral initialized by ROM. > > > >> - System co-processor(DMSC) is being represented as remote-core in > > > >> Device-tree(We are strictly following DM and DT model for the entire > > > >> SoC). > > > >> - Since DM is also followed by each peripheral device and remote core, > > > >> DM should > > > >> be enabled very early and many peripheral drivers are dependent on BSS > > > >> usage. > > > >> So, BSS has been made available very early. > > > >> > > > >> Hope this is clear. Let me know if more details are required, I will > > > >> be happy to > > > >> explain. > > > > > > > > Don't get me wrong: I'm not against using BSS early. I just want to > > > > ensure this > > > > stays consistent throught U-Boot. > > > > > > I understand and agree that it should be consistent. Just discussed this > > > with > > > Andreas, who is courteous enough to update the details in his vacation. > > > > We don't have to rush here, I don't have a problem waiting for Andreas to > > answer when he's back. > > > > > > > > > > > > > The reasons you stated still don't make it clear to me *why* bss is > > > > needed > > > > early. There are other boards using DM early that don't need this. In my > > > > opinion, DM drivers normally don't rely on BSS but keep all their state > > > > in > > > > > > This statement doesn't hold true for all the drviers. At least the mmc > > > driver > > > uses "initialized" variable stored in BSS to avoid initializing mmc > > > multiple > > > times[0]. In the past we en counted other drivers using
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Simon, thanks for your patience waiting for a response. Please see comments inlined... On Thu, Jul 25, 2019 at 11:52:55AM +0200, Simon Goldschmidt wrote: > On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: > > > > Hi Simon, > > > > On 25/07/19 12:31 PM, Simon Goldschmidt wrote: > > > Hi Lokesh, > > > > > > thanks for following up on this. > > > > > > On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: > > >> > > >> Hi Tom, > > >> > > >> On 20/07/19 9:21 PM, Tom Rini wrote: > > >>> On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > > On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > > > > > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > > > > > >> In order to be able to use more advanced driver functionality which > > >> often > > >> relies on having BSS initialized during early boot prior to > > >> relocation > > >> several things need to be in place: > > >> > > >> 1) Memory needs to be available for BSS to use. For this, we locate > > >> BSS > > >>at the top of the MCU SRAM area, with the stack starting right > > >> below > > >>it, > > >> 2) We need to move the initialization of BSS prior to entering > > >>board_init_f(). We will do this with a separate commit by turning > > >> on > > >>the respective CONFIG option. > > >> > > >> In this commit we also clean up the assignment of the initial SP > > >> address > > >> as part of the refactoring, taking into account the pre-decrement > > >> post- > > >> increment nature in which the SP is used on ARM. > > >> > > >> Signed-off-by: Andreas Dannenberg > > > > > > Applied to u-boot/master, thanks! > > > > Wait, why has this been merged? Unfortunately, I haven't followed this > > series, > > but in a discussion about a similar patch I sent [1], using BSS from > > board_init_f > > was turned down. And Simon Glass rather convinced me that this is the > > current > > API U-Boot has (and is documented in README). > > > > So either we must change this API and its documentation (and I would > > expect the > > author of this patch to combine the README change with the code > > change), or this > > patch would have to be rejected. > > > > Again, I'm sorry I only see this now. In thought to remember a > > discussion in this > > thread, but I clearly remember that wrong... > > > > [1] https://patchwork.ozlabs.org/patch/1057237/ > > >>> > > >>> And I had missed that other thread. Lokesh, since I think Andreas is > > >>> out currently can you expand a little on what we can/can't do on this > > >>> platform? Thanks! > > >> > > >> The reason why BSS is needed very early in this platform is for the > > >> following > > >> reasons: > > >> - System co-processor is the central resource manager in SoC and should > > >> be > > >> loaded and started very early in the boot process. Without that no > > >> peripheral or > > >> memory can be initialized. So for loading system co-processor image, we > > >> only > > >> have limited SRAM and a peripheral initialized by ROM. > > >> - System co-processor(DMSC) is being represented as remote-core in > > >> Device-tree(We are strictly following DM and DT model for the entire > > >> SoC). > > >> - Since DM is also followed by each peripheral device and remote core, > > >> DM should > > >> be enabled very early and many peripheral drivers are dependent on BSS > > >> usage. > > >> So, BSS has been made available very early. > > >> > > >> Hope this is clear. Let me know if more details are required, I will be > > >> happy to > > >> explain. > > > > > > Don't get me wrong: I'm not against using BSS early. I just want to > > > ensure this > > > stays consistent throught U-Boot. > > > > I understand and agree that it should be consistent. Just discussed this > > with > > Andreas, who is courteous enough to update the details in his vacation. > > We don't have to rush here, I don't have a problem waiting for Andreas to > answer when he's back. > > > > > > > > > The reasons you stated still don't make it clear to me *why* bss is needed > > > early. There are other boards using DM early that don't need this. In my > > > opinion, DM drivers normally don't rely on BSS but keep all their state in > > > > This statement doesn't hold true for all the drviers. At least the mmc > > driver > > uses "initialized" variable stored in BSS to avoid initializing mmc multiple > > times[0]. In the past we en counted other drivers using it. I guess the idea > > here is to enable the BSS support generically instead of fixing each of > > every > > driver. > > So this driver is generally not usable in pre-relocation phase? The README > document is pretty clear about BSS not being available in board_init_f. I know > this text is old, but it seems still valid. > >
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
On Thu, Jul 25, 2019 at 10:23 AM Lokesh Vutla wrote: > > Hi Simon, > > On 25/07/19 12:31 PM, Simon Goldschmidt wrote: > > Hi Lokesh, > > > > thanks for following up on this. > > > > On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: > >> > >> Hi Tom, > >> > >> On 20/07/19 9:21 PM, Tom Rini wrote: > >>> On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > > > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > > > >> In order to be able to use more advanced driver functionality which > >> often > >> relies on having BSS initialized during early boot prior to relocation > >> several things need to be in place: > >> > >> 1) Memory needs to be available for BSS to use. For this, we locate BSS > >>at the top of the MCU SRAM area, with the stack starting right below > >>it, > >> 2) We need to move the initialization of BSS prior to entering > >>board_init_f(). We will do this with a separate commit by turning on > >>the respective CONFIG option. > >> > >> In this commit we also clean up the assignment of the initial SP > >> address > >> as part of the refactoring, taking into account the pre-decrement post- > >> increment nature in which the SP is used on ARM. > >> > >> Signed-off-by: Andreas Dannenberg > > > > Applied to u-boot/master, thanks! > > Wait, why has this been merged? Unfortunately, I haven't followed this > series, > but in a discussion about a similar patch I sent [1], using BSS from > board_init_f > was turned down. And Simon Glass rather convinced me that this is the > current > API U-Boot has (and is documented in README). > > So either we must change this API and its documentation (and I would > expect the > author of this patch to combine the README change with the code change), > or this > patch would have to be rejected. > > Again, I'm sorry I only see this now. In thought to remember a > discussion in this > thread, but I clearly remember that wrong... > > [1] https://patchwork.ozlabs.org/patch/1057237/ > >>> > >>> And I had missed that other thread. Lokesh, since I think Andreas is > >>> out currently can you expand a little on what we can/can't do on this > >>> platform? Thanks! > >> > >> The reason why BSS is needed very early in this platform is for the > >> following > >> reasons: > >> - System co-processor is the central resource manager in SoC and should be > >> loaded and started very early in the boot process. Without that no > >> peripheral or > >> memory can be initialized. So for loading system co-processor image, we > >> only > >> have limited SRAM and a peripheral initialized by ROM. > >> - System co-processor(DMSC) is being represented as remote-core in > >> Device-tree(We are strictly following DM and DT model for the entire SoC). > >> - Since DM is also followed by each peripheral device and remote core, DM > >> should > >> be enabled very early and many peripheral drivers are dependent on BSS > >> usage. > >> So, BSS has been made available very early. > >> > >> Hope this is clear. Let me know if more details are required, I will be > >> happy to > >> explain. > > > > Don't get me wrong: I'm not against using BSS early. I just want to ensure > > this > > stays consistent throught U-Boot. > > I understand and agree that it should be consistent. Just discussed this with > Andreas, who is courteous enough to update the details in his vacation. We don't have to rush here, I don't have a problem waiting for Andreas to answer when he's back. > > > > > The reasons you stated still don't make it clear to me *why* bss is needed > > early. There are other boards using DM early that don't need this. In my > > opinion, DM drivers normally don't rely on BSS but keep all their state in > > This statement doesn't hold true for all the drviers. At least the mmc driver > uses "initialized" variable stored in BSS to avoid initializing mmc multiple > times[0]. In the past we en counted other drivers using it. I guess the idea > here is to enable the BSS support generically instead of fixing each of every > driver. So this driver is generally not usable in pre-relocation phase? The README document is pretty clear about BSS not being available in board_init_f. I know this text is old, but it seems still valid. And if this is really a workaround because it's easier to use this workaround instead of fixing drivers that invalidly use BSS, is this what we want? > > > heap memory. If you only need BSS early because drivers rely on BSS, you > > might > > have to fix those drivers? > > So, correct me here, why should driver be restricted to not use BSS? Post-relocation drivers might be free to use BSS (although you lose the per-instance storage when using BSS instead of the
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Simon, On 25/07/19 12:31 PM, Simon Goldschmidt wrote: > Hi Lokesh, > > thanks for following up on this. > > On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: >> >> Hi Tom, >> >> On 20/07/19 9:21 PM, Tom Rini wrote: >>> On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > >> In order to be able to use more advanced driver functionality which often >> relies on having BSS initialized during early boot prior to relocation >> several things need to be in place: >> >> 1) Memory needs to be available for BSS to use. For this, we locate BSS >>at the top of the MCU SRAM area, with the stack starting right below >>it, >> 2) We need to move the initialization of BSS prior to entering >>board_init_f(). We will do this with a separate commit by turning on >>the respective CONFIG option. >> >> In this commit we also clean up the assignment of the initial SP address >> as part of the refactoring, taking into account the pre-decrement post- >> increment nature in which the SP is used on ARM. >> >> Signed-off-by: Andreas Dannenberg > > Applied to u-boot/master, thanks! Wait, why has this been merged? Unfortunately, I haven't followed this series, but in a discussion about a similar patch I sent [1], using BSS from board_init_f was turned down. And Simon Glass rather convinced me that this is the current API U-Boot has (and is documented in README). So either we must change this API and its documentation (and I would expect the author of this patch to combine the README change with the code change), or this patch would have to be rejected. Again, I'm sorry I only see this now. In thought to remember a discussion in this thread, but I clearly remember that wrong... [1] https://patchwork.ozlabs.org/patch/1057237/ >>> >>> And I had missed that other thread. Lokesh, since I think Andreas is >>> out currently can you expand a little on what we can/can't do on this >>> platform? Thanks! >> >> The reason why BSS is needed very early in this platform is for the following >> reasons: >> - System co-processor is the central resource manager in SoC and should be >> loaded and started very early in the boot process. Without that no >> peripheral or >> memory can be initialized. So for loading system co-processor image, we only >> have limited SRAM and a peripheral initialized by ROM. >> - System co-processor(DMSC) is being represented as remote-core in >> Device-tree(We are strictly following DM and DT model for the entire SoC). >> - Since DM is also followed by each peripheral device and remote core, DM >> should >> be enabled very early and many peripheral drivers are dependent on BSS usage. >> So, BSS has been made available very early. >> >> Hope this is clear. Let me know if more details are required, I will be >> happy to >> explain. > > Don't get me wrong: I'm not against using BSS early. I just want to ensure > this > stays consistent throught U-Boot. I understand and agree that it should be consistent. Just discussed this with Andreas, who is courteous enough to update the details in his vacation. > > The reasons you stated still don't make it clear to me *why* bss is needed > early. There are other boards using DM early that don't need this. In my > opinion, DM drivers normally don't rely on BSS but keep all their state in This statement doesn't hold true for all the drviers. At least the mmc driver uses "initialized" variable stored in BSS to avoid initializing mmc multiple times[0]. In the past we en counted other drivers using it. I guess the idea here is to enable the BSS support generically instead of fixing each of every driver. > heap memory. If you only need BSS early because drivers rely on BSS, you might > have to fix those drivers? So, correct me here, why should driver be restricted to not use BSS? Also doing a grep for bss usage very early in board_init_f produced many results: ➜ u-boot git:(master) git grep -in "memset(__bss_start" | cut -d : -f 1 arch/arm/mach-socfpga/spl_gen5.c arch/arm/mach-zynqmp/spl.c arch/mips/mach-jz47xx/jz4780/jz4780.c board/barco/platinum/spl_picon.c board/barco/platinum/spl_titanium.c board/compulab/cl-som-imx7/spl.c board/congatec/cgtqmx6eval/cgtqmx6eval.c board/dhelectronics/dh_imx6/dh_imx6_spl.c board/el/el6x/el6x.c board/freescale/imx8mq_evk/spl.c board/freescale/imx8qm_mek/spl.c board/freescale/imx8qxp_mek/spl.c board/freescale/ls1021aiot/ls1021aiot.c board/freescale/ls1021aqds/ls1021aqds.c board/freescale/ls1021atwr/ls1021atwr.c board/freescale/mx6sabreauto/mx6sabreauto.c board/freescale/mx6sabresd/mx6sabresd.c board/freescale/mx6slevk/mx6slevk.c board/freescale/mx6sxsabresd/mx6sxsabresd.c
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Lokesh, thanks for following up on this. On Thu, Jul 25, 2019 at 6:36 AM Lokesh Vutla wrote: > > Hi Tom, > > On 20/07/19 9:21 PM, Tom Rini wrote: > > On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > >> On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > >>> > >>> On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > >>> > In order to be able to use more advanced driver functionality which often > relies on having BSS initialized during early boot prior to relocation > several things need to be in place: > > 1) Memory needs to be available for BSS to use. For this, we locate BSS > at the top of the MCU SRAM area, with the stack starting right below > it, > 2) We need to move the initialization of BSS prior to entering > board_init_f(). We will do this with a separate commit by turning on > the respective CONFIG option. > > In this commit we also clean up the assignment of the initial SP address > as part of the refactoring, taking into account the pre-decrement post- > increment nature in which the SP is used on ARM. > > Signed-off-by: Andreas Dannenberg > >>> > >>> Applied to u-boot/master, thanks! > >> > >> Wait, why has this been merged? Unfortunately, I haven't followed this > >> series, > >> but in a discussion about a similar patch I sent [1], using BSS from > >> board_init_f > >> was turned down. And Simon Glass rather convinced me that this is the > >> current > >> API U-Boot has (and is documented in README). > >> > >> So either we must change this API and its documentation (and I would > >> expect the > >> author of this patch to combine the README change with the code change), > >> or this > >> patch would have to be rejected. > >> > >> Again, I'm sorry I only see this now. In thought to remember a > >> discussion in this > >> thread, but I clearly remember that wrong... > >> > >> [1] https://patchwork.ozlabs.org/patch/1057237/ > > > > And I had missed that other thread. Lokesh, since I think Andreas is > > out currently can you expand a little on what we can/can't do on this > > platform? Thanks! > > The reason why BSS is needed very early in this platform is for the following > reasons: > - System co-processor is the central resource manager in SoC and should be > loaded and started very early in the boot process. Without that no peripheral > or > memory can be initialized. So for loading system co-processor image, we only > have limited SRAM and a peripheral initialized by ROM. > - System co-processor(DMSC) is being represented as remote-core in > Device-tree(We are strictly following DM and DT model for the entire SoC). > - Since DM is also followed by each peripheral device and remote core, DM > should > be enabled very early and many peripheral drivers are dependent on BSS usage. > So, BSS has been made available very early. > > Hope this is clear. Let me know if more details are required, I will be happy > to > explain. Don't get me wrong: I'm not against using BSS early. I just want to ensure this stays consistent throught U-Boot. The reasons you stated still don't make it clear to me *why* bss is needed early. There are other boards using DM early that don't need this. In my opinion, DM drivers normally don't rely on BSS but keep all their state in heap memory. If you only need BSS early because drivers rely on BSS, you might have to fix those drivers? Further, allowing BSS early is still against what the main README says, so I want to raise the question again: shouldn't this main README be changed if we suddenly allow BSS to be used early (because that main README says we can'that do that)? Regards, Simon > > Thanks and regards, > Lokesh ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
Hi Tom, On 20/07/19 9:21 PM, Tom Rini wrote: > On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: >> On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: >>> >>> On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: >>> In order to be able to use more advanced driver functionality which often relies on having BSS initialized during early boot prior to relocation several things need to be in place: 1) Memory needs to be available for BSS to use. For this, we locate BSS at the top of the MCU SRAM area, with the stack starting right below it, 2) We need to move the initialization of BSS prior to entering board_init_f(). We will do this with a separate commit by turning on the respective CONFIG option. In this commit we also clean up the assignment of the initial SP address as part of the refactoring, taking into account the pre-decrement post- increment nature in which the SP is used on ARM. Signed-off-by: Andreas Dannenberg >>> >>> Applied to u-boot/master, thanks! >> >> Wait, why has this been merged? Unfortunately, I haven't followed this >> series, >> but in a discussion about a similar patch I sent [1], using BSS from >> board_init_f >> was turned down. And Simon Glass rather convinced me that this is the current >> API U-Boot has (and is documented in README). >> >> So either we must change this API and its documentation (and I would expect >> the >> author of this patch to combine the README change with the code change), or >> this >> patch would have to be rejected. >> >> Again, I'm sorry I only see this now. In thought to remember a >> discussion in this >> thread, but I clearly remember that wrong... >> >> [1] https://patchwork.ozlabs.org/patch/1057237/ > > And I had missed that other thread. Lokesh, since I think Andreas is > out currently can you expand a little on what we can/can't do on this > platform? Thanks! The reason why BSS is needed very early in this platform is for the following reasons: - System co-processor is the central resource manager in SoC and should be loaded and started very early in the boot process. Without that no peripheral or memory can be initialized. So for loading system co-processor image, we only have limited SRAM and a peripheral initialized by ROM. - System co-processor(DMSC) is being represented as remote-core in Device-tree(We are strictly following DM and DT model for the entire SoC). - Since DM is also followed by each peripheral device and remote core, DM should be enabled very early and many peripheral drivers are dependent on BSS usage. So, BSS has been made available very early. Hope this is clear. Let me know if more details are required, I will be happy to explain. Thanks and regards, Lokesh ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
On Fri, Jul 19, 2019 at 07:29:37AM +0200, Simon Goldschmidt wrote: > On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > > > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > > > > > In order to be able to use more advanced driver functionality which often > > > relies on having BSS initialized during early boot prior to relocation > > > several things need to be in place: > > > > > > 1) Memory needs to be available for BSS to use. For this, we locate BSS > > >at the top of the MCU SRAM area, with the stack starting right below > > >it, > > > 2) We need to move the initialization of BSS prior to entering > > >board_init_f(). We will do this with a separate commit by turning on > > >the respective CONFIG option. > > > > > > In this commit we also clean up the assignment of the initial SP address > > > as part of the refactoring, taking into account the pre-decrement post- > > > increment nature in which the SP is used on ARM. > > > > > > Signed-off-by: Andreas Dannenberg > > > > Applied to u-boot/master, thanks! > > Wait, why has this been merged? Unfortunately, I haven't followed this series, > but in a discussion about a similar patch I sent [1], using BSS from > board_init_f > was turned down. And Simon Glass rather convinced me that this is the current > API U-Boot has (and is documented in README). > > So either we must change this API and its documentation (and I would expect > the > author of this patch to combine the README change with the code change), or > this > patch would have to be rejected. > > Again, I'm sorry I only see this now. In thought to remember a > discussion in this > thread, but I clearly remember that wrong... > > [1] https://patchwork.ozlabs.org/patch/1057237/ And I had missed that other thread. Lokesh, since I think Andreas is out currently can you expand a little on what we can/can't do on this platform? Thanks! -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
On Fri, Jul 19, 2019 at 2:29 AM Tom Rini wrote: > > On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > > > In order to be able to use more advanced driver functionality which often > > relies on having BSS initialized during early boot prior to relocation > > several things need to be in place: > > > > 1) Memory needs to be available for BSS to use. For this, we locate BSS > >at the top of the MCU SRAM area, with the stack starting right below > >it, > > 2) We need to move the initialization of BSS prior to entering > >board_init_f(). We will do this with a separate commit by turning on > >the respective CONFIG option. > > > > In this commit we also clean up the assignment of the initial SP address > > as part of the refactoring, taking into account the pre-decrement post- > > increment nature in which the SP is used on ARM. > > > > Signed-off-by: Andreas Dannenberg > > Applied to u-boot/master, thanks! Wait, why has this been merged? Unfortunately, I haven't followed this series, but in a discussion about a similar patch I sent [1], using BSS from board_init_f was turned down. And Simon Glass rather convinced me that this is the current API U-Boot has (and is documented in README). So either we must change this API and its documentation (and I would expect the author of this patch to combine the README change with the code change), or this patch would have to be rejected. Again, I'm sorry I only see this now. In thought to remember a discussion in this thread, but I clearly remember that wrong... [1] https://patchwork.ozlabs.org/patch/1057237/ Regards, Simon > > -- > Tom ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
On Tue, Jun 04, 2019 at 05:55:48PM -0500, Andreas Dannenberg wrote: > In order to be able to use more advanced driver functionality which often > relies on having BSS initialized during early boot prior to relocation > several things need to be in place: > > 1) Memory needs to be available for BSS to use. For this, we locate BSS >at the top of the MCU SRAM area, with the stack starting right below >it, > 2) We need to move the initialization of BSS prior to entering >board_init_f(). We will do this with a separate commit by turning on >the respective CONFIG option. > > In this commit we also clean up the assignment of the initial SP address > as part of the refactoring, taking into account the pre-decrement post- > increment nature in which the SP is used on ARM. > > Signed-off-by: Andreas Dannenberg Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 05/12] armV7R: K3: am654: Allow using SPL BSS pre-relocation
In order to be able to use more advanced driver functionality which often relies on having BSS initialized during early boot prior to relocation several things need to be in place: 1) Memory needs to be available for BSS to use. For this, we locate BSS at the top of the MCU SRAM area, with the stack starting right below it, 2) We need to move the initialization of BSS prior to entering board_init_f(). We will do this with a separate commit by turning on the respective CONFIG option. In this commit we also clean up the assignment of the initial SP address as part of the refactoring, taking into account the pre-decrement post- increment nature in which the SP is used on ARM. Signed-off-by: Andreas Dannenberg --- include/configs/am65x_evm.h | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/include/configs/am65x_evm.h b/include/configs/am65x_evm.h index 51abab3943..1319745963 100644 --- a/include/configs/am65x_evm.h +++ b/include/configs/am65x_evm.h @@ -19,6 +19,29 @@ #define CONFIG_SYS_SDRAM_BASE1 0x88000 /* SPL Loader Configuration */ +#ifdef CONFIG_TARGET_AM654_A53_EVM +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SPL_TEXT_BASE +\ +CONFIG_SYS_K3_NON_SECURE_MSRAM_SIZE) +#else +/* + * Maximum size in memory allocated to the SPL BSS. Keep it as tight as + * possible (to allow the build to go through), as this directly affects + * our memory footprint. The less we use for BSS the more we have available + * for everything else. + */ +#define CONFIG_SPL_BSS_MAX_SIZE0x5000 +/* + * Link BSS to be within SPL in a dedicated region located near the top of + * the MCU SRAM, this way making it available also before relocation. Note + * that we are not using the actual top of the MCU SRAM as there is a memory + * location filled in by the boot ROM that we want to read out without any + * interference from the C context. + */ +#define CONFIG_SPL_BSS_START_ADDR (CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX -\ +CONFIG_SPL_BSS_MAX_SIZE) +/* Set the stack right below the SPL BSS section */ +#define CONFIG_SYS_INIT_SP_ADDR CONFIG_SPL_BSS_START_ADDR +#endif #ifdef CONFIG_SYS_K3_SPL_ATF #define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME"tispl.bin" @@ -29,8 +52,6 @@ #endif #define CONFIG_SPL_MAX_SIZECONFIG_SYS_K3_MAX_DOWNLODABLE_IMAGE_SIZE -#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SPL_TEXT_BASE +\ - CONFIG_SYS_K3_NON_SECURE_MSRAM_SIZE - 4) #define CONFIG_SYS_BOOTM_LEN SZ_64M -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
