Subject: Re: [PATCH 1/1] powerpc: mpc85xx: Add board support for ucp1020

From: Scott Wood <[email-masked]>
Date: Thu, 7 May 2015 16:33:19 -0500

On Thu, 2015-05-07 at 15:29 -0400, Oleksandr G Zhadan wrote:
> On 05/07/2015 02:18 PM, Scott Wood wrote:
> > On Thu, 2015-05-07 at 12:31 -0400, Oleksandr G Zhadan wrote:
> >>>> diff --git a/arch/powerpc/configs/ucp1020_defconfig b/arch/powerpc/configs/ucp1020_defconfig
> >>>> new file mode 100644
> >>>> index 0000000..62f99aa
> >>>> --- /dev/null
> >>>> +++ b/arch/powerpc/configs/ucp1020_defconfig
> >>>
> >>> Please explain why your board needs its own defconfig.
> >>>
> >>
> >> Because, it's our own board and it has some specific to board
> >> definitions like CONFIG_DEFAULT_HOSTNAME and some specific to product
> >> definitions.
> >>
> >> If I can do it in some other way could you please give me some example
> >> if it's possible.
> >
> > I don't think stuff like CONFIG_DEFAULT_HOSTNAME belongs upstream.
> > Could you list what you need to be set that mpc85xx_smp_defconfig
> > doesn't set?
> 
> I make diff "mpc85xx_smp_defconfig" vs "ucp1020_defconfig after make 
> savedefconfig" and it's some differences like:
> 
> - mpc85xx_smp_defconfig has:
> CONFIG_PHYS_64BIT=y
> CONFIG_NR_CPUS=8

These won't prevent your board from working.  If you want
CONFIG_PHYS_64BIT disabled for performance, I could see a fragment being
used for that as per the recent defconfig discussions.  I wouldn't
expect NR_CPUS being 8 instead of 2 to be noticeable.

> - it enabled almost all boards to build. What for ?

Because that's what the common defconfigs are for.  We don't want a
defconfig for each board (most of the board-specific configs that are
currently there were added long ago).  If you want a config that
contains nothing your board doesn't need, you can maintain that locally.

> - it has MTD related differences (doesn't enabled spi flashes support we 
> need):
> -CONFIG_MTD_M25P80=y
> -CONFIG_MTD_SST25L=y

So add them to the existing defconfig.

> - It includes some PHY support, but not phy we are using

This should not harm your board.

>  and we need include intel wifi support:
> -CONFIG_MICREL_PHY=y
> -CONFIG_IWLWIFI=y

So add them to the existing defconfig.

> - It doesn't enable EXT4 fs support.

I think this would be a reasonable thing to add.

> Etc...
> 
> You can see it yourself below:

That doesn't show me the set of changes that you *need*, only the set of
changes that you have.

-Scott