On Fri, 31 Oct 2008 09:31:19 +0100
Stefan Roese <[EMAIL PROTECTED]> wrote:
> On Wednesday 29 October 2008, Josh Boyer wrote:
> > Looks pretty good. Just a few minor comments/questions below.
>
> Thanks. I also added some comments below.
>
> > Also, do you have a patch for a DTS file that gives an example of how to
> > instantiate the SPI stuff in the device tree?
>
> OK, I'll add a DTS patch to the next version.
Ok thanks.
> > >+ /* send the first byte */
> > >+ data = hw->tx ? hw->tx[0] : 0;
> > >+ out_8(&hw->regs->txd, data);
> > >+ out_8(&hw->regs->cr, SPI_PPC4XX_CR_STR);
> >
> > Maybe iowrite8? Same comment elsewhere.
>
> Why? We use the in_/out_xxx() accessor function for all other 4xx driver as
> well.
Yeah. We used to have all of 4xx in arch/ppc too. ;)
Seriously though, it doesn't really bother me too much. The
io{read,write} functions do have a bit better clarity as to endian-ness
though.
> > >+ wait_for_completion(&hw->done);
> > >+
> > >+ return hw->count;
> > >+}
> > >+
> >
> > <snip>
> >
> > >+static struct of_device_id spi_ppc4xx_of_match[] = {
> > >+ { .compatible = "ibm,spi", },
> > >+ {},
> > >+};
> >
> > I'm wondering if that is too generic of a match. In theory,
> > IBM could have another SPI controller that isn't for 4xx.
> > Maybe "ibm,spi-4xx" ?
>
> Right. I was doing it the same way as already done before, e.g. "ibm,iic".
> For
> the gpio driver we already switched to "ibm,ppc4xx-gpio". So how
> about "ibm,ppc4xx-spi"?
Sounds fine to me.
josh
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
