Patchwork Trying to add Kconfig support for M57SLI - builderror

On Friday 21 August 2009 20:21:27 Myles Watson wrote:
> > > OK, I am going to apply Cristi's patch, and, if it looks good, commit.
> > > Then we can do nvidia south.
> >
> > Okay, sounds good.
>
> Lets not create a new CONFIG variable SOUTHBRIDGE_NVIDIA_MCP55.  I think
> that whole file can go.  And the files that source it.
> Index: nvidia/mcp55/Kconfig
> Index: nvidia/Kconfig

Are you sure that this will work as in the mainboard/{vendor}/{model}/Kconfig 
it is used to select which sb is used. (Maybe I'm wrong with this, but I used 
the amd_serengeti target as starting point, and there it's done the same way.)

For example here on my board it looks like:
config BOARD_GIGABYTE_M57SLI
        bool "M57SLI"
        select ARCH_X86
        select CPU_AMD_K8
        select CPU_AMD_SOCKET_AM2
        select NORTHBRIDGE_AMD_AMDK8
        select NORTHBRIDGE_AMD_AMDK8_ROOT_COMPLEX
        select SOUTHBRIDGE_NVIDIA_MCP55
        select SUPERIO_WINBOND_IT8716F
        select PIRQ_TABLE
        select USE_PRINTK_IN_CAR
        help
        Gigabyte M57SLI mainboard
endchoice

>
> I'd rather you didn't remove the ck804 references, since my patch will have
> to put them back.
Done.

> +driver-$(CONFIG_HAVE_ACPI_TABLES) += mcp55_fadt.o
> I think you mean obj here.
Yes.

v2 of the patch attached, with some more (little) modifications.

> Thanks,
> Myles

Thanks,
Harald

On Fri, Aug 21, 2009 at 12:33 PM, Harald Gutmann<harald.gutmann@gmx.net> wrote:
> On Friday 21 August 2009 20:21:27 Myles Watson wrote:
>> > > OK, I am going to apply Cristi's patch, and, if it looks good, commit.
>> > > Then we can do nvidia south.
>> >
>> > Okay, sounds good.
>>
>> Lets not create a new CONFIG variable SOUTHBRIDGE_NVIDIA_MCP55.  I think
>> that whole file can go.  And the files that source it.
>> Index: nvidia/mcp55/Kconfig
>> Index: nvidia/Kconfig
>
> Are you sure that this will work as in the mainboard/{vendor}/{model}/Kconfig
> it is used to select which sb is used. (Maybe I'm wrong with this, but I used
> the amd_serengeti target as starting point, and there it's done the same way.)
You're right.  The reason I was questioning it was that I grepped the
tree and didn't find any CONFIG_SOUTHBRIDGE_NVIDIA*

I think the correct fix for that problem is to change
src/southbridge/nvidia/Makefile.inc
subdirs-y should be subdirs-$(CONFIG_SOUTHBRIDGE_NVIDIA_MCP55).

Does that sound better?

>> I'd rather you didn't remove the ck804 references, since my patch will have
>> to put them back.
> Done.
>
>> +driver-$(CONFIG_HAVE_ACPI_TABLES) += mcp55_fadt.o
>> I think you mean obj here.
> Yes.
>
> v2 of the patch attached, with some more (little) modifications.
Thanks,
Myles

On Fri, Aug 21, 2009 at 12:14 PM, Myles Watson<mylesgw@gmail.com> wrote:

> I think the correct fix for that problem is to change
> src/southbridge/nvidia/Makefile.inc
> subdirs-y should be subdirs-$(CONFIG_SOUTHBRIDGE_NVIDIA_MCP55).
>

yes!

This is the general rule. It's even in my small Kconfig.tex doco :-)

ron

On Fri, Aug 21, 2009 at 2:41 PM, ron minnich<rminnich@gmail.com> wrote:
> On Fri, Aug 21, 2009 at 12:14 PM, Myles Watson<mylesgw@gmail.com> wrote:
>
>> I think the correct fix for that problem is to change
>> src/southbridge/nvidia/Makefile.inc
>> subdirs-y should be subdirs-$(CONFIG_SOUTHBRIDGE_NVIDIA_MCP55).
>>
>
> yes!
>
> This is the general rule. It's even in my small Kconfig.tex doco :-)

Unfortunately it fits into the category of "the tree speaks louder
than the docs."  We shouldn't have missed it, but we did.

Myles