Re: [PATCH 1/3] platform-device driver for MQ11xx graphics chip

[Andrew Morton <akpm@osdl.org>]

<jamey@crl.dec.com> wrote:
>
> 
> This patch adds platform_device driver for MQ11xx system-on-chip
> graphics chip.  This chip is used in several non-PCI ARM and MIPS
> platforms such as the iPAQ H5550.  Two subsequent patches add
> support for the framebuffer and USB gadget subdevices.  This patch
> adds the toplevel driver to drivers/platform because it does not
> provide any specific functionality (e.g., framebuffer) and it not tied
> to a named physical bus.  In these platforms, the MQ11xx is tied
> directly to the host bus.
> 
> ...
> +
> +static void
> +mq_free (struct mediaq11xx_base *zis, u32 addr, unsigned size)
> +{
> +	int i, j;
> +	u32 eaddr;
> +	struct mq_data *mqdata = to_mq_data (zis);
> +
> +	size = (size + MEMBLOCK_ALIGN - 1) & ~(MEMBLOCK_ALIGN - 1);
> +	eaddr = addr + size;
> +
> +	spin_lock (&mqdata->mem_lock);

It's unusual for a memory allocate/free function to not be IRQ-safe.  Will
there never be a requirement that mq_free() or mq_alloc() be called from
interrupts or softirq's?

> +	spin_unlock (&mqdata->mem_lock);

You've religiously used the wrong coding style throughout these patches:
neither core kernel nor arch/arm places a space before the opening paren. 
A little thing, but easy to get right.

There are eight-spaces where tabs should be in various places too..

-
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/