On Thu, Jun 02, 2005 at 03:35:40PM -0700, Andrew Morton wrote:
> Matt Porter <mporter at kernel.crashing.org> wrote:
> >
> > +++ uncommitted/drivers/serial/cpm_uart/cpm_uart_cpm2.c (mode:100644)
> > @@ -134,12 +134,21 @@
> >
> > void scc2_lineif(struct uart_cpm_port *pinfo)
> > {
> > + /*
> > + * STx GP3 uses the SCC2 secondary option pin assignment
> > + * which this driver doesn't account for in the static
> > + * pin assignments. This kind of board specific info
> > + * really has to get out of the driver so boards can
> > + * be supported in a sane fashion.
> > + */
> > +#ifndef CONFIG_STX_GP3
> > volatile iop_cpm2_t *io = &cpm2_immr->im_ioport;
> > io->iop_pparb |= 0x008b0000;
>
> Silly question: why is this driver using a volatile pointer to
> memory-mapped I/O rather than readl and writel?
readl/writel just won't work. They are byte swapped on PPC since
they are specifically for PCI access. In other "on-chip" drivers
for PPC32, we typically ioremap() to a non-volatile type and then
use out_be32()/in_be32() since everything except PCI on PPC is
in right-endian (big endian) form. out_be*()/in_be*() contain
an eieio instruction to guarantee proper ordering.
I know this driver was recently rewritten but the authors may have
missed the past threads about why volatile use is discouraged. We
do something similar with register overlay structures in
drivers/net/ibm_emac. However, we don't use a volatile type and do
use PPC32-specific (these are PPC-specific drivers anyway)
out_be32()/in_be32() for correctness.
-Matt
