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

[Vitaly Bordug <vitb@kernel.crashing.org>]

On Tue, 18 Mar 2008 09:04:14 +0100
Heiko Schocher wrote:

> 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?
> 
This has been a disputable question some time ago, and decided (or it looks like decided) that devtree describes hardware, and not the way it is configured at the moment. Therefor, best way for pin stuff is considered, as Scott mentioned, to set up stuff inside the firmware.

-Vitaly

> bye,
> Heiko
>