Re: [PATCH v2] 8xx: Add support for the MPC852 based board from keymile.

[Heiko Schocher <hs@denx.de>]

Hello Stephen,

Stephen Rothwell wrote:
> On Tue, 18 Mar 2008 08:13:06 +0100 Heiko Schocher <hs@denx.de> wrote:
>> Stephen Rothwell wrote:
>>> On Fri, 14 Mar 2008 10:24:30 +0100 Heiko Schocher <hs@denx.de> wrote:
>> [...]
>>>> +struct cpm_pin {
>>>> +		int port, pin, flags;
>>>> +};
>>> I wish someone would consolidate all these definitions of cpm_pin.
>> Hmm... do you mean something like, moving this struct
>> in cpm1.h and ...
> 
> Yes or somewhere else appropriate.
> 
>>>> +	for (i = 0; i < ARRAY_SIZE(mgsuvd_pins); i++) {
>>>> +		struct cpm_pin *pin = &mgsuvd_pins[i];
>>>> +		cpm1_set_pin(pin->port, pin->pin, pin->flags);
>>>> +	}
>>> And the code that uses them ...
>> making in arch/powerpc/sysdev/cpm1.c a function
>> cpm1_setup_pins (struct cpm_pin *pins) ?
> 
> Yes.
> 
> This is not necessary for your patch, but would be a nice cleanup later.
> N.B. this struct is alos used by users of cpm2_set_pin().

OK. Another thought about this. Shouldnt this table go in the dts?
A device node like

cpm_pin {
	pins = <port pin flags>;
};

would be nice, or?

bye,
Heiko

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany