On Thu, May 29, 2008 at 2:11 PM, Kenneth Johansson <kenneth@...> wrote:
>
> On Thu, 2008-05-29 at 15:37 -0400, Martha Marx wrote:
>> Signed-off-by: Martha Marx <mmarx@...>
Acked-by: Grant Likely <grant.likely@...>
>> ---
>> board/ads5121/Makefile | 2 +-
>> board/ads5121/ads5121.c | 36 +-------
>> board/ads5121/iopin.c | 96 ++++++++++++++++++++
>> board/ads5121/iopin.h | 220 +++++++++++++++++++++++++++++++++++++++++++++
>> cpu/mpc512x/fec.c | 15 ---
>> include/configs/ads5121.h | 1 +
>> 6 files changed, 321 insertions(+), 49 deletions(-)
>> create mode 100644 board/ads5121/iopin.c
>> create mode 100644 board/ads5121/iopin.h
>
> It's good that you remove iomux setting from the drivers like fec it
> should not be there.
>
> It's NOT good that you now put them in another "driver" iopin.
>
> The pins should be in board specific files. iopin should only contain
> helper functions not data that everybody that do a new board needs to go
> in and change/add.
>
> put the table in ads5121.c and I'm happy with it. well I'm not sure we
> need this fancy stuff only gets harder to read but I take that over
> having the config spread out over different files like it's now.
I think this approach is okay. For now it is isolated within the
board directory, but it can be factored out into common code where
there are a few more 5121 board ports.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
|