Subject: Re: [RFC/PATCH 4/4] Add support for MSI on Axon-based Cell systems

From: Arnd Bergmann <[email-masked]>
Date: Mon, 4 Jun 2007 19:09:54 +0200

On Monday 04 June 2007, Michael Ellerman wrote:
> This patch adds support for the setup and decoding of MSIs
> on Axon-based Cell systems.

Looks pretty good, just two details I noticed:

> +/* Flags */
> +#define MSIC_ENABLE		0x0001	/* Bit 31 */
> +#define MSIC_FIFO_FULL_ENABLE	0x0002	/* Bit 30 */
> +#define MSIC_IRQ_ENABLE		0x0008	/* Bit 28 */
> +#define MSIC_FULL_STOP_ENABLE	0x0010	/* Bit 27 */

The comments behind each definition look rather bogus, as much as
we all love to have documentation normally.

If you have the specification for the hardware, the bit numbers
are in there, for everyone else, the IBM numbering scheme can
only confuse the reader...

> +#define MSIC_DCR_BASE		MSIC_CTRL_REG_ADDR
> +#define MSIC_DCR_SIZE		(MSIC_WRITE_OFFSET_REG - MSIC_CTRL_REG_ADDR)

shouldn't that come from the device tree?

	Arnd <><