On Monday 31 March 2008 21:10, Scott Wood wrote: > Laurent Pinchart wrote: > > This patch relocates the buffer descriptors and the SMC parameter RAM at the > > end of the first CPM muram chunk, as described in the device tree. This > > allows > > device trees to stop excluding SMC parameter ram allocated by the boot > > loader > > from the CPM muram node. > > It's usually a good idea to state that something is untested if that's > the case. :-)
Sorry. I'll state it clearly next time.
> This patch cannot work as is.
>
> > +static int cpm_get_virtual_address(void *devp, void **addr, int ncells)
> > +{
> > + unsigned long xaddr;
> > + int n;
> > +
> > + n = getprop(devp, "virtual-reg", addr, ncells * sizeof *addr);
> > + if (n < ncells * sizeof *addr) {
>
> You must cast the sizeof to a signed int; otherwise, a negative return
> from getprop will be "bigger" than the unsigned size, and you'll return
> garbage as the address.
>
> > + for (n = 0; n < ncells; n++) {
> > + if (!dt_xlate_reg(devp, n, &xaddr, NULL))
> > + return -1;
> > +
> > + addr[n] = (void*)xaddr;
>
> (void *)
>
> > + }
> > + }
> > +
> > + return ncells;
> > +}
>
> This could be a generic bootwrapper function. It should return the
> number of resources (ncells is a misnomer) actually found, though,
> rather than failing if there are fewer than asked for. Let the caller
> decide if it's fatal.
Ok. I'll move it to devtree.c.
> > @@ -202,63 +243,62 @@ int cpm_console_init(void *devp, struct
> > serial_console_data *scdp)
> > else
> > do_cmd = cpm1_cmd;
> >
> > - n = getprop(devp, "fsl,cpm-command", &cpm_cmd, 4);
> > - if (n < 4)
> > + if (getprop(devp, "fsl,cpm-command", &cpm_cmd, 4) < sizeof cpm_cmd)
> > return -1;
>
> Standard kernel style is sizeof(foo), not sizeof foo.
Ok.
> Plus, if you're going to replace 4 with sizeof(cpm_cmd), do it both
> places. I don't really see the need, though; a cell is always 4 bytes.
Ok, I'll keep 4.
> > - n = getprop(parent, "virtual-reg", reg_virt, sizeof(reg_virt));
> > - if (n < (int)sizeof(reg_virt)) {
> > - if (!dt_xlate_reg(parent, 0, ®_phys, NULL))
> > - return -1;
> > -
> > - reg_virt[0] = (void *)reg_phys;
> > - }
> > -
> > - cpcr = reg_virt[0];
> > + if (cpm_get_virtual_address(devp, &cpcr, 1) < 0)
> > + return -1;
>
> s/devp/parent/
>
> > muram = finddevice("/soc/cpm/muram/data");
> > if (!muram)
> > return -1;
> >
> > /* For bootwrapper-compatible device trees, we assume that the first
> > - * entry has at least 18 bytes, and that #address-cells/#data-cells
> > + * entry has at least 128 bytes, and that #address-cells/#data-cells
> > * is one for both parent and child.
> > */
> >
> > - n = getprop(muram, "virtual-reg", reg_virt, sizeof(reg_virt));
> > - if (n < (int)sizeof(reg_virt)) {
> > - if (!dt_xlate_reg(muram, 0, ®_phys, NULL))
> > - return -1;
> > + if (cpm_get_virtual_address(devp, &muram_addr, 1) < 0)
> > + return -1;
>
> s/devp/muram/
>
> > +
> > + if (getprop(muram, "reg", reg, sizeof reg) < sizeof reg)
> > + return -1;
>
> Should read into array of u32, not void *.
Ok.
> > + if (is_cpm2 && is_smc) {
> > + u16 *smc_base = (u16*)param;
>
> (u16 *)
>
> > + u16 pram_offset;
> >
> > - muram_start = reg_virt[0];
> > + pram_offset = cbd_offset - 64;
> > + pram_offset = _ALIGN_DOWN(pram_offset, 64);
> > + *smc_base = pram_offset;
>
> Use out_be16().
>
> The SMC should be stopped before you do this.
Right.
New patch comming.
--
Laurent Pinchart
CSE Semaphore Belgium
Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium
T +32 (2) 387 42 59
F +32 (2) 387 42 75
pgpJvihx22QTP.pgp
Description: PGP signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
