[U-Boot] [PATCH] ARM: phytec: pcm051: select board revision by Kconfig

[Masahiro Yamada]

Hi.

Sorry for my late reply.

2015-06-04 17:07 GMT+09:00 Lars Poeschel <poeschel at lemonage.de>:
> On Wed, Jun 03, 2015 at 11:20:25AM -0400, Tom Rini wrote:
>> On Wed, Jun 03, 2015 at 04:36:06PM +0200, Lars Poeschel wrote:
>> > On Tue, Jun 02, 2015 at 10:34:34AM -0400, Tom Rini wrote:
>> > > On Mon, Jun 01, 2015 at 05:09:11PM +0200, poeschel at lemonage.de wrote:
>> > >
>> > > > From: Lars Poeschel <poeschel at lemonage.de>
>> > > >
>> > > > This add a Kconfig entry that allows to set the board revision in
>> > > > menuconfig. So the deprecated CONFIG_SYS_EXTRA_OPTIONS is no longer
>> > > > needed for this boad.
>> > > >
>> > > > Signed-off-by: Lars Poeschel <poeschel at lemonage.de>
>> > >
>> > > I like the concept but CONFIG_REVx is way too generic.  Can we maybe
>> > > re-work things as CONFIG_TARGET_PCM051_REV1 / CONFIG_TARGET_PCM051_REV3
>> > > (and those select CONFIG_TARGET_PCM051) ?  Masahiro?  Thanks!
>> >
>> > Agree: CONFIG_REVx is too generic. I will send a version 2 of the patch,
>> > but I don't understand why you want CONFIG_TARGET_PCM051_REV1 /
>> > CONFIG_TARGET_PCM051_REV3 to select CONFIG_TARGET_PCM051. The
>> > CONFIG_TARGET_PCM051_REVx's are inside an
>> >
>> > if TARGET_PCM051
>> > ...
>> > endif
>> >
>> > That means, that CONFIG_TARGET_PCM051 must already be selected to make
>> > the *_REVx's visible and selectable.
>>
>> Right.  I mean since we must select one of these boards at build-time,
>> why not just ask about them up-front in arch/arm/Kconfig as rev1/rev3,
>
> I wanted the initial list you choose your board from not to grow too
> big. The (unsorted) list you scroll in menuconfig is already huge! I
> like the aproach "Atmel AT91" takes more, and I'd propably even go
> one step further an select a manufacturer as a first step and then
> in a second step select a specific board from this manufacturer.

Right.
Refactoring arch/arm/Kconfig is still under way.

I think the biggest part of the remainder is Freescale boards.


> But if you prefer the other way - no problem. I tried that:
>
>> and then have the main symbol be a hidden one, ie roughly:
>>
>> config TARGET_PCM051
>>   bool
>>
>> config TARGET_PCM051_REV1
>>   bool "Enable pcm051 rev1"
>>   select TARGET_PCM051
>>   help
>>     ...
>>
>> config TARGET_PCM051_REV3
>>   bool "Enable pcm051 rev3"
>>   select TARGET_PCM051
>>   help
>>     ...
>
> I could not get this approach to work. It seems Kconfig does not support
> hidden config options. I took a slightly different approach. I'll send a
> PATCH V2 in a second. Please have a look, if that is what what you mean.


Why do we need to "select" the board?

The problem here is that CONFIG_REV1/_REV3 are too generic,
so I think just renaming them into CONFIG_PCM051_REV1/_REV3 is enough
and better.



choice
prompt "pcm051 revision select"
default REV3

config PCM051_REV1
       bool "pcm051 revision 1 or 2"
       help
         If you have 1358.1 written on the pcb of your pcm051, you
         have a revision 1 board. Likewise if you have 1358.2 on your
         board, it is a revision 2 board and this entry is for you.

config PCM051_REV3
       bool "pcm051 revision 3"
       help
         If you have 1358.3 written on the pcb of your pcm051, you
         have a revision 3 board and you have to select this entry.

endchoice




-- 
Best Regards
Masahiro Yamada