RE: [PATCH v3] i386/linux: add support for ext_lfb_base

[Nath, Arindam]

> -----Original Message-----
> From: Daniel Kiper <address@hidden>
> Sent: Friday, July 13, 2018 5:20 PM
> To: Nath, Arindam <address@hidden>
> Cc: address@hidden; Daniel Kiper <address@hidden>; Arindam Nath
> <address@hidden>
> Subject: Re: [PATCH v3] i386/linux: add support for ext_lfb_base
> 
> On Thu, Jul 12, 2018 at 07:02:49PM +0530, Arindam Nath wrote:
> > From: Arindam Nath <address@hidden>
> >
> > Signed-off-by: Arindam Nath <address@hidden>
> > ---
> > Cc: Daniel Kiper <address@hidden>
> > ---
> > v1:
> >
> > The EFI Graphics Output Protocol can return a 64-bit
> > linear frame buffer address in some firmware/BIOS
> > implementations. We currently only store the lower
> > 32-bits in the lfb_base. This will eventually be
> > passed to Linux kernel and the efifb driver will
> > incorrectly interpret the framebuffer address as
> > 32-bit address.
> >
> > The Linux kernel has already added support to handle
> > 64-bit linear framebuffer address in the efifb driver
> > since quite some time now.
> >
> > This patch adds the support for 64-bit linear frame
> > buffer address in GRUB to address the above mentioned
> > scenario.
> >
> > v2: changes suggested by Daniel
> >
> >   - added #if defined (GRUB_MACHINE_EFI) && defined (__x86_64__)
> >   - moved constant definitions to the beginning of header file
> >
> > v3: changes suggested by Daniel
> >
> >   - moved patch version info below SOB
> >   - added empty lines above and below the modified lines
> >   - removed unnecessary #if and #endif from header file
> 
> Code looks good but the commit message is a mess. The commit message
> should look like this in your editor.
> 
> 
> i386/linux: add support for ext_lfb_base
> 
> The EFI Graphics Output Protocol can return a 64-bit
> linear frame buffer address in some firmware/BIOS
> implementations. We currently only store the lower
> 32-bits in the lfb_base. This will eventually be
> passed to Linux kernel and the efifb driver will
> incorrectly interpret the framebuffer address as
> 32-bit address.
> 
> The Linux kernel has already added support to handle
> 64-bit linear framebuffer address in the efifb driver
> since quite some time now.
> 
> This patch adds the support for 64-bit linear frame
> buffer address in GRUB to address the above mentioned
> scenario.
> 
> Signed-off-by: Arindam Nath <address@hidden>
> ---
> Cc: Daniel Kiper <address@hidden>
> 
> v2: changes suggested by Daniel
>   - added #if defined (GRUB_MACHINE_EFI) && defined (__x86_64__)
>   - moved constant definitions to the beginning of header file
> 
> v3: changes suggested by Daniel
>   - moved patch version info below SOB
>   - added empty lines above and below the modified lines
>   - removed unnecessary #if and #endif from header file
> 
> 
> I will fix that before committing the patch. I will do
> that in a week or so if there are no objections.

Thanks.

> 
> Next time please adhere to the commit message layout presented above.

Will do.

Arindam
> 
> Thank you for doing the work.
> 
> Daniel