Messages in this thread | | | Date | Fri, 19 Oct 2007 14:09:18 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 4/8] drivers-edac-add Cell MC driver |
| |
On Fri, 19 Oct 2007 13:17:43 -0600 dougthompson@xmission.com wrote:
> + /* The procedure for clearing FIR bits is a bit ... weird */ > + if (clear) { > + fir &= ~(CBE_MIC_FIR_ECC_ERR_MASK | CBE_MIC_FIR_ECC_SET_MASK); > + fir |= CBE_MIC_FIR_ECC_RESET_MASK; > + fir &= ~clear; > + out_be64(&priv->regs->mic_fir, fir); > + (void)in_be64(&priv->regs->mic_fir);
The (void) cast isn't particularly popular practice. Did you find that it actually does anything useful?
> + > + mb(); /* sync up */
What's this here for? It could do with a more usful comment.
If it's trying to perform some synchronisation of device register access then I suspect it didn't work. Or maybe it happens to work because of how ppc implements mb(), in which case a direct use of the appropriate ppc primitive might be clearer.
> +#ifdef DEBUG > + fir = in_be64(&priv->regs->mic_fir); > + dev_dbg(mci->dev, "fir clear : 0x%016lx\n", fir); > +#endif > + } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |