| Message ID | 1468297308-32102-7-git-send-email-afaerber@suse.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
On 12.07.16 06:21, Andreas Färber wrote: > We do so for the EFI binary already and it aids debugging. > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > include/config_distro_bootcmd.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h > index 8f14457..0cf74e2 100644 > --- a/include/config_distro_bootcmd.h > +++ b/include/config_distro_bootcmd.h > @@ -149,6 +149,10 @@ > "${prefix}${dtb_prefix}" \ > "${dtb_vendor_prefix}" \ > "${efi_fdtfile}; then " \ > + "echo Found ${prefix}" \ As mentioned in the other reply, I think this message is very useful, but should contain the target device name as well, so that we don't need to print out a message for every single partition we scan :). Alex
Am 12.07.2016 um 09:30 schrieb Alexander Graf: > On 12.07.16 06:21, Andreas Färber wrote: >> We do so for the EFI binary already and it aids debugging. >> >> Cc: Alexander Graf <agraf@suse.de> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> --- >> include/config_distro_bootcmd.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h >> index 8f14457..0cf74e2 100644 >> --- a/include/config_distro_bootcmd.h >> +++ b/include/config_distro_bootcmd.h >> @@ -149,6 +149,10 @@ >> "${prefix}${dtb_prefix}" \ >> "${dtb_vendor_prefix}" \ >> "${efi_fdtfile}; then " \ >> + "echo Found ${prefix}" \ > > As mentioned in the other reply, I think this message is very useful, > but should contain the target device name as well, so that we don't need > to print out a message for every single partition we scan :). I originally had it that way, but remember that issues like with patches 4 and 5 will not lead to any Found message at all. Therefore I still prefer decoupling the two. Btw where does the "reading efi/boot/bootaa64.efi" message come from? That one could be dropped as duplicate instead! :) Regards, Andreas
On 12.07.16 14:45, Andreas Färber wrote: > Am 12.07.2016 um 09:30 schrieb Alexander Graf: >> On 12.07.16 06:21, Andreas Färber wrote: >>> We do so for the EFI binary already and it aids debugging. >>> >>> Cc: Alexander Graf <agraf@suse.de> >>> Signed-off-by: Andreas Färber <afaerber@suse.de> >>> --- >>> include/config_distro_bootcmd.h | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h >>> index 8f14457..0cf74e2 100644 >>> --- a/include/config_distro_bootcmd.h >>> +++ b/include/config_distro_bootcmd.h >>> @@ -149,6 +149,10 @@ >>> "${prefix}${dtb_prefix}" \ >>> "${dtb_vendor_prefix}" \ >>> "${efi_fdtfile}; then " \ >>> + "echo Found ${prefix}" \ >> >> As mentioned in the other reply, I think this message is very useful, >> but should contain the target device name as well, so that we don't need >> to print out a message for every single partition we scan :). > > I originally had it that way, but remember that issues like with patches > 4 and 5 will not lead to any Found message at all. Therefore I still > prefer decoupling the two. We do detect the case where we don't have a working fdt loaded already, so we could probably print out the search paths there if we don't find a working fdt? > Btw where does the "reading efi/boot/bootaa64.efi" message come from? > That one could be dropped as duplicate instead! :) Phew. The "load" command maybe? Alex
Am 12.07.2016 um 16:25 schrieb Alexander Graf: > On 12.07.16 14:45, Andreas Färber wrote: >> Am 12.07.2016 um 09:30 schrieb Alexander Graf: >>> On 12.07.16 06:21, Andreas Färber wrote: >>>> We do so for the EFI binary already and it aids debugging. >>>> >>>> Cc: Alexander Graf <agraf@suse.de> >>>> Signed-off-by: Andreas Färber <afaerber@suse.de> >>>> --- >>>> include/config_distro_bootcmd.h | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h >>>> index 8f14457..0cf74e2 100644 >>>> --- a/include/config_distro_bootcmd.h >>>> +++ b/include/config_distro_bootcmd.h >>>> @@ -149,6 +149,10 @@ >>>> "${prefix}${dtb_prefix}" \ >>>> "${dtb_vendor_prefix}" \ >>>> "${efi_fdtfile}; then " \ >>>> + "echo Found ${prefix}" \ >>> >>> As mentioned in the other reply, I think this message is very useful, >>> but should contain the target device name as well, so that we don't need >>> to print out a message for every single partition we scan :). >> >> I originally had it that way, but remember that issues like with patches >> 4 and 5 will not lead to any Found message at all. Therefore I still >> prefer decoupling the two. > > We do detect the case where we don't have a working fdt loaded already, > so we could probably print out the search paths there if we don't find a > working fdt? Feel free to make a follow-up patch... >> Btw where does the "reading efi/boot/bootaa64.efi" message come from? >> That one could be dropped as duplicate instead! :) > > Phew. The "load" command maybe? Then why isn't it printing the same for the .dtb files? :) Maybe it's some FAT debug thing? Andreas
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 8f14457..0cf74e2 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -149,6 +149,10 @@ "${prefix}${dtb_prefix}" \ "${dtb_vendor_prefix}" \ "${efi_fdtfile}; then " \ + "echo Found ${prefix}" \ + "${dtb_prefix}" \ + "${dtb_vendor_prefix}" \ + "${efi_fdtfile}; " \ "run load_efi_dtb; " \ "fi; " \ "done; " \
We do so for the EFI binary already and it aids debugging. Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Andreas Färber <afaerber@suse.de> --- include/config_distro_bootcmd.h | 4 ++++ 1 file changed, 4 insertions(+)