| Message ID | 20200307232220.111052-1-sjg@chromium.org |
|---|---|
| Headers | show |
| Series | x86: Improve support for chain-loading U-Boot | expand |
On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > This little series adds a few checks into the code to allow better > operation when booting a build from a previous-state loader such as > coreboot. > > At present we have a 'coreboot' target but this runs very different code > from the bare-metal targets, such as coral. There is very little in common > between them. > > It is useful to be able to boot the same U-Boot on a device, with or > without a first-stage bootloader. For example, with chromebook_coral, it > is helpful for testing to be able to boot the same U-Boot (complete with > FSP) on bare metal and from coreboot. It allows checking of things like > CPU speed, comparing registers, ACPI tables and the like. > > This series allows U-Boot to detect that it ran from coreboot and > automatically do the right thing. > > This series makes the most important changes to allow the same u-boot.bin > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL). I see only this and one patch out of 7 (seven?). Please, (re)send it correctly.
Hi Andy, On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko <andriy.shevchenko at linux.intel.com> wrote: > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > > This little series adds a few checks into the code to allow better > > operation when booting a build from a previous-state loader such as > > coreboot. > > > > At present we have a 'coreboot' target but this runs very different code > > from the bare-metal targets, such as coral. There is very little in common > > between them. > > > > It is useful to be able to boot the same U-Boot on a device, with or > > without a first-stage bootloader. For example, with chromebook_coral, it > > is helpful for testing to be able to boot the same U-Boot (complete with > > FSP) on bare metal and from coreboot. It allows checking of things like > > CPU speed, comparing registers, ACPI tables and the like. > > > > This series allows U-Boot to detect that it ran from coreboot and > > automatically do the right thing. > > > > This series makes the most important changes to allow the same u-boot.bin > > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL). > > I see only this and one patch out of 7 (seven?). > Please, (re)send it correctly. It looks like it went to the mailing list...are you subscribed? Regards, Simon
On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote: > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko > <andriy.shevchenko at linux.intel.com> wrote: > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > > > This little series adds a few checks into the code to allow better > > > operation when booting a build from a previous-state loader such as > > > coreboot. > > > > > > At present we have a 'coreboot' target but this runs very different code > > > from the bare-metal targets, such as coral. There is very little in common > > > between them. > > > > > > It is useful to be able to boot the same U-Boot on a device, with or > > > without a first-stage bootloader. For example, with chromebook_coral, it > > > is helpful for testing to be able to boot the same U-Boot (complete with > > > FSP) on bare metal and from coreboot. It allows checking of things like > > > CPU speed, comparing registers, ACPI tables and the like. > > > > > > This series allows U-Boot to detect that it ran from coreboot and > > > automatically do the right thing. > > > > > > This series makes the most important changes to allow the same u-boot.bin > > > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL). > > > > I see only this and one patch out of 7 (seven?). > > Please, (re)send it correctly. > > It looks like it went to the mailing list... > are you subscribed? It doesn't matter if you want people to pay an attention (by putting them to Cc list), so, since it was one patch I can't see the rest and know if they affect or not the one, you Cc'ed me to.
Hi Andy, On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko <andriy.shevchenko at linux.intel.com> wrote: > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote: > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko > > <andriy.shevchenko at linux.intel.com> wrote: > > > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > > > > This little series adds a few checks into the code to allow better > > > > operation when booting a build from a previous-state loader such as > > > > coreboot. > > > > > > > > At present we have a 'coreboot' target but this runs very different code > > > > from the bare-metal targets, such as coral. There is very little in common > > > > between them. > > > > > > > > It is useful to be able to boot the same U-Boot on a device, with or > > > > without a first-stage bootloader. For example, with chromebook_coral, it > > > > is helpful for testing to be able to boot the same U-Boot (complete with > > > > FSP) on bare metal and from coreboot. It allows checking of things like > > > > CPU speed, comparing registers, ACPI tables and the like. > > > > > > > > This series allows U-Boot to detect that it ran from coreboot and > > > > automatically do the right thing. > > > > > > > > This series makes the most important changes to allow the same u-boot.bin > > > > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL). > > > > > > I see only this and one patch out of 7 (seven?). > > > Please, (re)send it correctly. > > > > It looks like it went to the mailing list... > > > are you subscribed? > > It doesn't matter if you want people to pay an attention (by putting them to Cc > list), so, since it was one patch I can't see the rest and know if they affect > or not the one, you Cc'ed me to. I don't fully understand this, but are you saying you did not get the patches in your email? If you subscribe to the U-Boot mailing list you should receive all patches even if not cc'd to you. I did not specifically cc you on this series but I can do that if I send it again. But patman has added you to one patch (number 3) and therefore you are also cc'd on the cover letter. Can you check whether you are subscribed to the mailing list? Regards, Simon
On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote: > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko > <andriy.shevchenko at linux.intel.com> wrote: > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote: > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko > > > <andriy.shevchenko at linux.intel.com> wrote: > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: ... > > > It looks like it went to the mailing list... > > > > > are you subscribed? > > > > It doesn't matter if you want people to pay an attention (by putting them to Cc > > list), so, since it was one patch I can't see the rest and know if they affect > > or not the one, you Cc'ed me to. > > I don't fully understand this, but are you saying you did not get the > patches in your email? If you subscribe to the U-Boot mailing list you > should receive all patches even if not cc'd to you. Probably I got to another email, not this one. > I did not specifically cc you on this series but I can do that if I > send it again. But patman has added you to one patch (number 3) and > therefore you are also cc'd on the cover letter. Yes, and this one I got without seeing full picture. > Can you check whether you are subscribed to the mailing list? As I said, I am not subscribed by email patman uses to Cc me. Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to the entire series. Of course there are cases, when patches are independent and no need to spam everybody with tons of emails, but this should be clarified in cover letter. Did I miss that?
Hi Andy On Wed, Mar 11, 2020 at 3:04 PM Andy Shevchenko <andriy.shevchenko at linux.intel.com> wrote: > > On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote: > > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko > > <andriy.shevchenko at linux.intel.com> wrote: > > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote: > > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko > > > > <andriy.shevchenko at linux.intel.com> wrote: > > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > > ... > > > > > It looks like it went to the mailing list... > > > > > > > are you subscribed? > > > > > > It doesn't matter if you want people to pay an attention (by putting them to Cc > > > list), so, since it was one patch I can't see the rest and know if they affect > > > or not the one, you Cc'ed me to. > > > > I don't fully understand this, but are you saying you did not get the > > patches in your email? If you subscribe to the U-Boot mailing list you > > should receive all patches even if not cc'd to you. > > Probably I got to another email, not this one. > > > I did not specifically cc you on this series but I can do that if I > > send it again. But patman has added you to one patch (number 3) and > > therefore you are also cc'd on the cover letter. > > Yes, and this one I got without seeing full picture. > > > Can you check whether you are subscribed to the mailing list? > > As I said, I am not subscribed by email patman uses to Cc me. > > > Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to > the entire series. Of course there are cases, when patches are independent and > no need to spam everybody with tons of emails, but this should be clarified in > cover letter. Did I miss that? AFAIR, last time you were not happy about "irrelevant" patches that were CC-ed to you "by mistake" [1]. This time, as I see, is vice-versa. The algo is simple - if the change touches a file where you're marked as one of commiters/maintainers - it will add your email to CC, otherwise it won't (however you can still find the full list of patches in the ML, it takes a couple of seconds only). Unfortunately current state of tooling in U-Boot is not too good to be able to read thoughts of people and guess their wishes. But if you know how to improve that, patches are always welcome! [1] https://patchwork.ozlabs.org/cover/1225901/ > > -- > With Best Regards, > Andy Shevchenko > >
On Wed, Mar 11, 2020 at 4:54 PM Igor Opaniuk <igor.opaniuk at gmail.com> wrote: > On Wed, Mar 11, 2020 at 3:04 PM Andy Shevchenko > <andriy.shevchenko at linux.intel.com> wrote: > > On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote: > > > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko > > > <andriy.shevchenko at linux.intel.com> wrote: > > > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote: > > > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko > > > > > <andriy.shevchenko at linux.intel.com> wrote: > > > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > > > > ... > > > > > > > It looks like it went to the mailing list... > > > > > > > > > are you subscribed? > > > > > > > > It doesn't matter if you want people to pay an attention (by putting them to Cc > > > > list), so, since it was one patch I can't see the rest and know if they affect > > > > or not the one, you Cc'ed me to. > > > > > > I don't fully understand this, but are you saying you did not get the > > > patches in your email? If you subscribe to the U-Boot mailing list you > > > should receive all patches even if not cc'd to you. > > > > Probably I got to another email, not this one. > > > > > I did not specifically cc you on this series but I can do that if I > > > send it again. But patman has added you to one patch (number 3) and > > > therefore you are also cc'd on the cover letter. > > > > Yes, and this one I got without seeing full picture. > > > > > Can you check whether you are subscribed to the mailing list? > > > > As I said, I am not subscribed by email patman uses to Cc me. > > > > > > Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to > > the entire series. Of course there are cases, when patches are independent and > > no need to spam everybody with tons of emails, but this should be clarified in > > cover letter. Did I miss that? > > AFAIR, last time you were not happy about "irrelevant" patches that were CC-ed > to you "by mistake" [1]. This time, as I see, is vice-versa. Nope, it's not. It maybe that I missed cover letter to tell that the rest is irrelevant to me. Otherwise Cc list is not filled correctly. > The algo is simple - if the change touches a file where you're marked as one > of commiters/maintainers - it will add your email to CC, otherwise it won't > (however you can still find the full list of patches in the ML, it takes > a couple of seconds only). Yes, but here is obvious flaw. If the change is done in the file dependent to the preparatory patch(es), then I need to be Cc'ed in all relevant. For example, patch 1 does introduce nice helper function foo(), patches 2-(N-1) does change some irrelevant files, patch N touches my stuff, I have to be Cc'ed to the 1) cover letter, 2) patch 1, 3) patch N. > Unfortunately current state of tooling in U-Boot is not too good to be able > to read thoughts of people and guess their wishes. See above. *Algo is simple*. > But if you know how to improve that, patches are always welcome! > > [1] https://patchwork.ozlabs.org/cover/1225901/
Dear Andy, dear Igor, In message <CAHp75Venz1ZSpfkK6werib1csKXLk=tzqA25TE4iJRdSOXGbHA at mail.gmail.com> you wrote: > > > Unfortunately current state of tooling in U-Boot is not too good to be able > > to read thoughts of people and guess their wishes. > > See above. *Algo is simple*. Come on, guys, in theory none of the Cc:s should be needed at all as you get everything on the mailing list in first place; so the Cc: is already a friendly reminder. Getting one for a patch in a series should be sufficient to identify the whole series without problems, right? So please, instead of fighting over such things rather invest the energy in writing/reviewing code... Thanks.
Hi Andy, On Wed, 11 Mar 2020 at 07:04, Andy Shevchenko < andriy.shevchenko at linux.intel.com> wrote: > > On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote: > > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko > > <andriy.shevchenko at linux.intel.com> wrote: > > > On Tue, Mar 10, 2020 at 08:28:29PM -0600, Simon Glass wrote: > > > > On Tue, 10 Mar 2020 at 08:51, Andy Shevchenko > > > > <andriy.shevchenko at linux.intel.com> wrote: > > > > > On Sat, Mar 07, 2020 at 04:22:13PM -0700, Simon Glass wrote: > > ... > > > > > It looks like it went to the mailing list... > > > > > > > are you subscribed? > > > > > > It doesn't matter if you want people to pay an attention (by putting them to Cc > > > list), so, since it was one patch I can't see the rest and know if they affect > > > or not the one, you Cc'ed me to. > > > > I don't fully understand this, but are you saying you did not get the > > patches in your email? If you subscribe to the U-Boot mailing list you > > should receive all patches even if not cc'd to you. > > Probably I got to another email, not this one. > > > I did not specifically cc you on this series but I can do that if I > > send it again. But patman has added you to one patch (number 3) and > > therefore you are also cc'd on the cover letter. > > Yes, and this one I got without seeing full picture. > > > Can you check whether you are subscribed to the mailing list? > > As I said, I am not subscribed by email patman uses to Cc me. Please consider subscribing to the mailing list. You'll miss a lot of things otherwise. You can add an email filter. > > > Basically it's a bug in patman. Either it shouldn't tell me, or should Cc to > the entire series. Of course there are cases, when patches are independent and > no need to spam everybody with tons of emails, but this should be clarified in > cover letter. Did I miss that? It is that latter case that happened here. A series may have various patches that are relevant to different maintainers, so they want to see just those patches and the cover letter (for context). Perhaps this doesn't happen so often with Linux? Regards, Simon
On Wed, Mar 11, 2020 at 09:22:49PM -0600, Simon Glass wrote: > On Wed, 11 Mar 2020 at 07:04, Andy Shevchenko < > andriy.shevchenko at linux.intel.com> wrote: > > On Wed, Mar 11, 2020 at 06:24:07AM -0600, Simon Glass wrote: > > > On Wed, 11 Mar 2020 at 06:09, Andy Shevchenko > > > <andriy.shevchenko at linux.intel.com> wrote: ... > > > I don't fully understand this, but are you saying you did not get the > > > patches in your email? If you subscribe to the U-Boot mailing list you > > > should receive all patches even if not cc'd to you. > > > > Probably I got to another email, not this one. > > > > > I did not specifically cc you on this series but I can do that if I > > > send it again. But patman has added you to one patch (number 3) and > > > therefore you are also cc'd on the cover letter. > > > > Yes, and this one I got without seeing full picture. > > > > > Can you check whether you are subscribed to the mailing list? > > > > As I said, I am not subscribed by email patman uses to Cc me. > > Please consider subscribing to the mailing list. You'll miss a lot of > things otherwise. You can add an email filter. As I told above, I have two addresses in use, and patman chose one, which I'm not using for mailing list subscription. > > Basically it's a bug in patman. Either it shouldn't tell me, or should Cc > to > > the entire series. Of course there are cases, when patches are > independent and > > no need to spam everybody with tons of emails, but this should be > clarified in > > cover letter. Did I miss that? > > It is that latter case that happened here. A series may have various > patches that are relevant to different maintainers, so they want to see > just those patches and the cover letter (for context). Perhaps this doesn't > happen so often with Linux? It happens, but at least I can get the idea. I re-read the cover letter and it sounds to me (in light of Edison chain loader) that I have to see entire series. Unfortunately my main priorities is not a U-Boot work, so, if you think it's not breaking existing platforms, go ahead, we will notice breakage sooner or later. More important is what you are doing in your ACPI series, which I (slowly) has been reviewing.
Hi Bin, On Sat, 7 Mar 2020 at 16:22, Simon Glass <sjg at chromium.org> wrote: > > This little series adds a few checks into the code to allow better > operation when booting a build from a previous-state loader such as > coreboot. > > At present we have a 'coreboot' target but this runs very different code > from the bare-metal targets, such as coral. There is very little in common > between them. > > It is useful to be able to boot the same U-Boot on a device, with or > without a first-stage bootloader. For example, with chromebook_coral, it > is helpful for testing to be able to boot the same U-Boot (complete with > FSP) on bare metal and from coreboot. It allows checking of things like > CPU speed, comparing registers, ACPI tables and the like. > > This series allows U-Boot to detect that it ran from coreboot and > automatically do the right thing. > > This series makes the most important changes to allow the same u-boot.bin > for coral to boot after coreboot (by itself) or bare metal (via TPL->SPL). > > Changes in v3: > - Add a new patch with a gd flag for chain loading > - Add new patch to detect running from coreboot > > Changes in v2: > - Drop the other check in interrupt_init() which is not needed now > - Drop patch 'dm: Avoid initing built-in devices when chain loading' > > Simon Glass (7): > x86: fsp: Allow skipping init code when chain loading > x86: apl: Skip init code when chain loading > x86: cpu: Skip init code when chain loading > pci: Avoid auto-config when chain loading > board: Add a gd flag for chain loading > x86: Add a way to detect running from coreboot > x86: Use the existing stack when chain-loading > > arch/x86/cpu/apollolake/fsp_s.c | 2 ++ > arch/x86/cpu/cpu.c | 4 +++- > arch/x86/cpu/i386/cpu.c | 15 +++++++++++++++ > arch/x86/cpu/i386/interrupt.c | 6 ++++-- > arch/x86/cpu/start_from_spl.S | 16 ++++++++++++++-- > arch/x86/include/asm/u-boot-x86.h | 7 +++++++ > arch/x86/lib/fsp/fsp_dram.c | 8 ++++++++ > arch/x86/lib/fsp/fsp_graphics.c | 3 +++ > arch/x86/lib/fsp2/fsp_dram.c | 10 ++++++++++ > arch/x86/lib/fsp2/fsp_init.c | 2 +- > arch/x86/lib/init_helpers.c | 3 +++ > drivers/pci/pci-uclass.c | 4 ++-- > include/asm-generic/global_data.h | 1 + > include/init.h | 2 +- > 14 files changed, 74 insertions(+), 9 deletions(-) > > -- > 2.25.1.481.gfbce0eb801-goog > Any comments on this series? Regards, Simon