Re: [PATCH 1/2] MIPS: Loongson: Add support for Loongson3.

[Andrei Borzenkov <arvidjaar@gmail.com>]

В Fri, 12 Jun 2015 19:52:31 +0800
Heiher <r@hev.cc> пишет:

> Hi,
> 
> Thanks for reivew.
> 
> On Fri, Jun 12, 2015 at 6:02 PM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> 
> > В Mon, 8 Jun 2015 17:25:16 +0800
> > Heiher <r@hev.cc> пишет:
> >
> > > --- a/grub-core/bus/pci.c
> > > +++ b/grub-core/bus/pci.c
> > > @@ -72,7 +72,7 @@ grub_dma_get_virt (struct grub_pci_dma_chunk *ch)
> > >  grub_uint32_t
> > >  grub_dma_get_phys (struct grub_pci_dma_chunk *ch)
> > >  {
> > > -  return (((grub_uint32_t) ch) & 0x1fffffff) | 0x80000000;
> > > +  return (((grub_uint32_t) ch) & 0x1fffffff);
> >
> > I do not have experience with hardware but that does not look right.
> > Either this chunk breaks existing hardware or this part was not needed
> > in the first place. I suspect the former because later you re-add the
> > same code in subsequent patch.
> >
> 
> I think this is a old mistake that caused grub on loongson3 broken. The dma
> chunk address (ch & 0x1fffffff) is physical address. and it or 0x80000000
> is cached virtual address.
> 

So if this is not needed for existing platforms please make it separate
patch to make it obvious and facilitate regression search. Do not hide
it inside additional platform support.

> My next subsequent patch fixed grub_dma_get_virt (not grub_dma_get_phys)
> function,  because the Loongson 3B has a dma cached coherent issue, just
> only access memory by uncached for dma. so i add a grub_dma_coherent flag
> to control grub_dma_get_virt return cached virtual address or uncached.
> 
> 
> >
> > This makes it hard to bisect later as it leaves some commits in
> > non-functional state.
> >
> > Could you please rearrange your patches so that they do not impact
> > existing platforms?
> >
> 
> I have tested on Loongson 2F and Loongson 3A, 3B platforms. I have not
> tested on qemu mips, and i will do that next. thanks!
>