Update on NOKIA-IP530

[mbertens]

Hi all,

I have updated the point that where passed to me.

- Moved the register CONFIG items for the drivers to the board Kconfig.
- Placed the PCMCIA./CardBus drivers all to one file, with #if(s) to
make sure that only included in the code that was selected for a board.
- Unfortunate we still need ti dirty hack in arch\i386\boot
\pirq_routing.c this should be fixed within the code of coreboot. But my
unfamiliarly with that code its not a good idea that i make that fix.
But i'm of cource willing to patch it on the Nokia code and test it. I
hope that you will consider applying the patch for the mean while, its
guarded by the #if CONFIG_BOARD_NOKIA_IP530 so nobody shall have any
problems with it.

Marc

[Myles Watson]

> Unfortunate we still need ti dirty hack in arch\i386\boot
>
> \pirq_routing.c this should be fixed within the code of coreboot. But my
> unfamiliarly with that code its not a good idea that i make that fix.
> But i'm of cource willing to patch it on the Nokia code and test it. I
> hope that you will consider applying the patch for the mean while, its
> guarded by the #if CONFIG_BOARD_NOKIA_IP530 so nobody shall have any
> problems with it.
> From your earlier mail:

a last update the dirty hack in the pirq_routing is not needed, therefor
i made a new diff for it.

Was it just a mistake that you didn't need it?

Thanks,
Myles

[mbertens]

On Thu, 2010-06-03 at 13:29 -0600, Myles Watson wrote:
> > Unfortunate we still need ti dirty hack in arch\i386\boot
> >
> > \pirq_routing.c this should be fixed within the code of coreboot. But my
> > unfamiliarly with that code its not a good idea that i make that fix.
> > But i'm of cource willing to patch it on the Nokia code and test it. I
> > hope that you will consider applying the patch for the mean while, its
> > guarded by the #if CONFIG_BOARD_NOKIA_IP530 so nobody shall have any
> > problems with it.
> > From your earlier mail:
>
> a last update the dirty hack in the pirq_routing is not needed, therefor
> i made a new diff for it.
>
> Was it just a mistake that you didn't need it?

No, the ip530 needs it.
> Thanks,
> Myles

[Myles Watson]

Marc,

Here's your patch with a couple of changes:
1. took out white space changes so that I could see the devicetree.cb changes
 - I understood most of the changes, but why remove the register
settings for the keyboard and mouse?
 - there's no reason we can' t use tabs when it gets committed, I just
couldn't see the differences.

2. changed the Kconfig a little for your drivers
 - Kconfig of drivers shouldn't show up in the menu.  If you build
this board, you want it to have all the drivers.

Signed-off-by: Myles Watson

Thanks,
Myles

[Myles Watson]

Marc,

A couple of picky details I forgot to mention:
- Since Nokia isn't an acronym, it should probably stay Nokia, not
NOKIA in the board string.
- The purpose of HAVE_ vs. GENERATE_ variables is:
  - HAVE_PIRQ_TABLES specifies that the board has a PIRQ table
  - GENERATE_PIRQ_TABLES lets the user decide if it should be built
Since GENERATE_ defaults to HAVE_, board Kconfigs should just set HAVE_

Thanks,
Myles

[Myles Watson]

On Thu, Jun 3, 2010 at 3:02 PM, Myles Watson  wrote:
> Marc,
>
> Here's your patch with a couple of changes:
> took out white space changes so that I could see the devicetree.cb changes
>
> - I understood most of the changes, but why remove the register
> settings for the keyboard and mouse?
>  - there's no reason we can' t use tabs when it gets committed, I just
> couldn't see the differences.
> changed the Kconfig a little for your drivers
>
> - Kconfig of drivers shouldn't show up in the menu.  If you build
> this board, you want it to have all the drivers.
>
> Signed-off-by: Myles Watson

I forgot a final refresh of the patches.  Sorry.

Thanks,
Myles

[mbertens]

Signed-off-by: Marc Bertens

On Thu, 2010-06-03 at 15:12 -0600, Myles Watson wrote:
> On Thu, Jun 3, 2010 at 3:02 PM, Myles Watson  wrote:
> > Marc,
> >
> > Here's your patch with a couple of changes:
> > took out white space changes so that I could see the devicetree.cb changes
> > I understood most of the changes, but why remove the register
> >
> > settings for the keyboard and mouse?
> > there's no reason we can' t use tabs when it gets committed, I just
> >
> > couldn't see the differences.
> > changed the Kconfig a little for your drivers
> > Kconfig of drivers shouldn't show up in the menu.  If you build
> >
> > this board, you want it to have all the drivers.
> >
> > Signed-off-by: Myles Watson
>
> I forgot a final refresh of the patches.  Sorry.
>
> Thanks,
> Myles
> -- 
> coreboot mailing list: [e-mail]
> http://www.coreboot.org/mailman/listinfo/coreboot

[Myles Watson]

> Signed-off-by: Marc Bertens

Acked-by: is the tag you want if you agree with the patch and want it to be
committed.  I'm hoping that means you tested it.
> > > I understood most of the changes, but why remove the register
> > >
> > > settings for the keyboard and mouse?

Were they causing a problem?
> > > there's no reason we can' t use tabs when it gets committed, I just
> > >
> > > couldn't see the differences.

Did you have a preference here?

Thanks,
Myles

[mbertens]

On Fri, 2010-06-04 at 12:29 -0600, Myles Watson wrote:
> > Signed-off-by: Marc Bertens
>
> Acked-by: is the tag you want if you agree with the patch and want it to be
> committed.  I'm hoping that means you tested it.

Ok, i mis understand you then, because i was not seeing any real
changes. But i will test the patches.
> > > > I understood most of the changes, but why remove the register
> > > >
> > > > settings for the keyboard and mouse?
>
> Were they causing a problem?

The IP530 doesn't have a mouse or keyboard, they where not a problem,
but i dont like any thing these that is not nessary.
> > > > there's no reason we can' t use tabs when it gets committed, I just
> > > >
> > > > couldn't see the differences.
>
> Did you have a preference here?

Regarding the tabs, i was just folling the coding rules, nothing more or
less :-)
> Thanks,
> Myles

I will send you an ack when i tested it.

Marc

[Myles Watson]

> On Fri, 2010-06-04 at 12:29 -0600, Myles Watson wrote:
> > > Signed-off-by: Marc Bertens
> >
> > Acked-by: is the tag you want if you agree with the patch and want it to
>
> be
> > committed.  I'm hoping that means you tested it.
>
> Ok, i mis understand you then, because i was not seeing any real
> changes. But i will test the patches.

I didn't mean for there to be any, but I wanted to make sure I didn't break
it.
> > > > > I understood most of the changes, but why remove the register
> > > > >
> > > > > settings for the keyboard and mouse?
> >
> > Were they causing a problem?
>
> The IP530 doesn't have a mouse or keyboard, they where not a problem,
> but i dont like any thing these that is not nessary.

Then should they be turned off?  I agree that simpler is better.
> > > > > there's no reason we can' t use tabs when it gets committed, I
>
> just
> > > > > couldn't see the differences.
> >
> > Did you have a preference here?
>
> Regarding the tabs, i was just folling the coding rules, nothing more or
> less :-)

Sorry about that.  It is an unofficial exception because the device trees
get so deeply nested.  I agree that we should make it official or use tabs.
> I will send you an ack when i tested it.

Great.  You could also test turning off the floppy and sign off an updated
patch.  I'll Ack and commit it.

Thanks,
Myles

[mbertens]

Acked-by: Marc Bertens

I tested the patch at it worked fine, when i added the patch on the
\arch\i386\boot\pirq_routing.c too :-)

Marc

On Fri, 2010-06-04 at 12:46 -0600, Myles Watson wrote:
> > On Fri, 2010-06-04 at 12:29 -0600, Myles Watson wrote:
> > > > Signed-off-by: Marc Bertens
> > >
> > > Acked-by: is the tag you want if you agree with the patch and want it to
> >
> > be
> > > committed.  I'm hoping that means you tested it.
> >
> > Ok, i mis understand you then, because i was not seeing any real
> > changes. But i will test the patches.
>
> I didn't mean for there to be any, but I wanted to make sure I didn't break
> it.
> > > > > > I understood most of the changes, but why remove the register
> > > > > >
> > > > > > settings for the keyboard and mouse?
> > >
> > > Were they causing a problem?
> >
> > The IP530 doesn't have a mouse or keyboard, they where not a problem,
> > but i dont like any thing these that is not nessary.
>
> Then should they be turned off?  I agree that simpler is better.
> > > > > > there's no reason we can' t use tabs when it gets committed, I
> >
> > just
> > > > > > couldn't see the differences.
> > >
> > > Did you have a preference here?
> >
> > Regarding the tabs, i was just folling the coding rules, nothing more or
> > less :-)
>
> Sorry about that.  It is an unofficial exception because the device trees
> get so deeply nested.  I agree that we should make it official or use tabs.
> > I will send you an ack when i tested it.
>
> Great.  You could also test turning off the floppy and sign off an updated
> patch.  I'll Ack and commit it.
>
> Thanks,
> Myles

[Myles Watson]

> Acked-by: Marc Bertens
>
> I tested the patch at it worked fine, when i added the patch on the
> \arch\i386\boot\pirq_routing.c too :-)

Rev 5609.

Thanks for testing it.  Peter said he didn't think the ugly hack was ready
to go in, and I've never done anything with PIRQ tables.  Maybe he can help
you figure out a better long-term solution.

Thanks,
Myles

[mbertens]

On Fri, 2010-06-04 at 13:57 -0600, Myles Watson wrote:
> > Acked-by: Marc Bertens
> >
> > I tested the patch at it worked fine, when i added the patch on the
> > \arch\i386\boot\pirq_routing.c too :-)
>
> Rev 5609.

Great
> Thanks for testing it.  Peter said he didn't think the ugly hack was ready
> to go in, and I've never done anything with PIRQ tables.  Maybe he can help
> you figure out a better long-term solution.

Yeah, its an ugly hack, my problem at this time is de unfamiliarty with
that part of the code and the impact on all other configurations. But i
will investigate how that code works and see if i can make a proposition
for it...
> Thanks,
> Myles

Marc