Re: [PATCH] Add Magic Panel R2 SH7720 board support

[Paul M. <le...@li...>]

On Mon, Jul 23, 2007 at 09:19:39AM +0200, Markus Brunner wrote:
> The board is currently under development and not publicly available. We
> thought that it is better to have a template than nothing to get the
> SH7720 running on another board. Maybe somebody with a Solution Engine
> Light (MS7720RP01) volunteers to write a BSP for this board as a
> template.
> 
This is not a problem, it's never been a requirement that a board be mass
produced or even readily available. As long as there are folks testing
and keeping it maintained, we'll do our best to accomodate it.

> +static struct ipr_data mpr2_ipr_map[] = {
> +	/* IRQ, IPR-idx, shift, priority */
> +	{ 32, 2,  0, 13 },	/* IRQ0 CAN1 */
> +	{ 33, 2,  4, 13 },	/* IRQ1 CAN2 */
> +	{ 34, 2,  8, 13 },	/* IRQ2 CAN3 */
> +	{ 35, 2, 12,  6 },	/* IRQ3 SMSC9115 */
> +	{ 84, 7, 12, 14 },	/* PINTA (FLASH_READY, /PWR1, /PWR2) */
> +};
> +
This will probably want to be re-rolled for INTC.

> +	/* CURRENT_SINK=off,	PWR_SWITCH=off, LAN_SPEED=100MBit,
> +	 * LAN_RESET=off,	BUZZER=off,	LCD_BL=off
> +	 */
> +	#if (CONFIG_SH_MAGIC_PANEL_R2_VERSION == 2)
> +		ctrl_outb(0x30, PORT_PMDR);
> +	#elif (CONFIG_SH_MAGIC_PANEL_R2_VERSION == 3)
> +		ctrl_outb(0xF0, PORT_PMDR);
> +	#else
> +	#error Unknown revision of PLATFORM_MP_R2
> +	#endif
> +
Please keep pre-processor macros on the far left margin, not indented. If
you run your patch through scripts/checkpatch.pl it should complain about
this and give you an idea of what stylistic things you may want to
address before submission.


> +struct sh_machine_vector mv_mpr2 __initmv = {
> +	.mv_name		= "Magic Panel Release 2",

Note that with the recent machvec changes, mv_name is something we can
overload sh_mv with, so you might want to give it a more practical name
(ie, mpr2 or so), so you can place this on the kernel command line
without raising eyebrows.

Also as far as the various board revisions go, please throw these in a
board-specific Kconfig. Quite a few boards do this already, and it keeps
the upper-level Kconfig a bit tidier, thanks.