Scott Wood <scottw...@freescale.com> wrote on 08/21/2013 09:20:03 AM:
> From: Scott Wood <scottw...@freescale.com>
> To: Stephen N Chivers <schiv...@csc.com.au>
> Cc: <b...@kernel.crashing.org>, Chris Proctor <cproc...@csc.com.au>,
> <linuxppc-dev@lists.ozlabs.org>, <pau...@samba.org>
> Date: 08/21/2013 09:20 AM
> Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for
> Motorola/Emerson MVME5100.
>
> On Tue, 2013-08-20 at 13:28 +1100, Stephen N Chivers wrote:
> > Scott Wood <scottw...@freescale.com> wrote on 08/09/2013 11:35:20 AM:
> >
> > > From: Scott Wood <scottw...@freescale.com>
> > > To: Stephen N Chivers <schiv...@csc.com.au>
> > > Cc: <b...@kernel.crashing.org>, <pau...@samba.org>, Chris Proctor
> > > <cproc...@csc.com.au>, <linuxppc-dev@lists.ozlabs.org>
> > > Date: 08/09/2013 11:36 AM
> > > Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for
> > > Motorola/Emerson MVME5100.
> > >
> > > simple-bus may be applicable here (in addition to a specific
> > > compatible).
> > >
> > The HAWK ASIC is a difficult beast. I still cannot get a positive
> > identification as to what it is (Motorola/Freescale part number
> > unknown, not even the part number on the chip on the board helps....).
> > The best I can come up with is that it is a tsi108 without
> > the ethenets.
> > So device_type will be tsi-bridge and compatible will be
> > tsi108-bridge.
>
> Don't use device_type. compatible should include "hawk" in the name
> (especially if you're not sure what's really in it), and/or the part
> number on the chip. If you're convinced it's fully compatible with
> tsi108-bridge you can add that as a second compatible value, though
> given the uncertainty it's probably better to just teach Linux to look
> for the new compatible.
>
> If devices on the bus can be used without any special bus setup or
> knowledge, then you can add a compatible of "simple-bus" to the end.
>
> > > Why not just look for a chrp,iic node directly?
> > >
> > I was following the model used in other places, like chrp/setup.c.
>
> Not all examples are good examples. :-)
>
> > > > + if ((np = of_find_compatible_node(NULL, "pci",
"mpc10x-pci")))
> > {
> > >
> > > Why insist on the device_type?
> > >
> > Following the model in the linkstation (kurobox) platform support.
>
> Drop the device_type check.
>
> > > > +static void
> > > > +mvme5100_restart(char *cmd)
> > > > +{
> > > > + volatile ulong i = 10000000;
> > > > +
> > > > +
> > > > + local_irq_disable();
> > > > + _nmask_and_or_msr(0, MSR_IP);
> > >
> > > Does "mtmsr(mfmsr() | MSR_IP)" not work?
> > >
> > Don't know. Is from the original code by Matt Porter.
>
> It actually appears that there are no callers remaining that use the
> "and" portion of the functionality. In fact there are no callers that
> use it for anything other than setting MSR_IP. :-P
>
> > > > + out_8((u_char *) BOARD_MODRST_REG, 0x01);
> > > > +
> > > > + while (i-- > 0);
> > >
> > > Do not use a loop to implement a delay.
> > >
> > Taken from the original code. But at this point the board
> > is going to reset and reboot via firmware, as /sbin/reboot
> > or /sbin/halt has been invoked.
>
> Still, it's just a bad idea. What's wrong with udelay()?
>
> Or just use an infinite loop. How much value is there really in timing
> out here?
>
> > > > +static void __init
> > > > +mvme5100_set_bat(void)
> > > > +{
> > > > +
> > > > +
> > > > + mb();
> > > > + mtspr(SPRN_DBAT1U, 0xf0001ffe);
> > > > + mtspr(SPRN_DBAT1L, 0xf000002a);
> > > > + mb();
> > > > + setbat(1, 0xfe000000, 0xfe000000, 0x02000000,
> > PAGE_KERNEL_NCG);
> > > > +}
> > >
> > > It is no longer allowed to squat on random virtual address space
like
> > > this. If you really need a BAT you'll have to allocate the virtual
> > > address properly.
> > >
> > Yes. I found that this was an anathema when researching the port in
> > 2010 but I couldn't find any practical solution at the time.
> > The code is called early to ensure that the hawk registers are
available.
> > sysdev/cpm_common.c does the same thing.
>
> > What is the correct solution?
>
> ioremap() has special code to function early (using ioremap_bot).
>
> If you still need to use a BAT that early, reserve the space with
> asm/fixmap.h or by adding a function to the early ioremap code to just
> reserve the space. Or better, improve the ioremap code to be capable of
> creating a BAT (or equivalent) when requested.
>
It is really interesting. Given that the UART implementation on the
HAWK is such that legacy_serial will not set up an early console it
is very likely that the address translation set up by the bat is not
required.
I can probably replace the physical addresses used in:
setup_indirec_pci(hose, 0, 0xfe000cf8, 0xfe000cfc, 0);
with remapped equivalents. But, with the setbat eliminated, the
line:
pcibios_alloc_controller(dev);
silently (remember no early console, due to UART reg-shift) panics.
It is happening at the point where the newly allocated PHB
structure is being added to the "hose_list" in pci-common.c.
So, I think there is some side effect due to the call to the setbat with
the
PAGE_KERNEL_NCG parameter that I do not yet understand.
> -Scott
>
>
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
