[linux-yocto] [PATCH 1/1] meta: add baytrail scc & config files

Tue Sep 24 20:23:01 PDT 2013

Hi Darren,

Please see my replies below.

> -----Original Message-----
> From: Darren Hart [mailto:dvhart at linux.intel.com]
> Sent: 25 September, 2013 3:13 AM
> To: Development list for the linux-yocto*.git Linux kernel repositories
> Cc: Chang, Rebecca Swee Fun
> Subject: Re: [linux-yocto] [PATCH 1/1] meta: add baytrail scc & config files
> 
> Hi Rebecca,
> 
> Please see my comments below regarding several of the CONFIG options in
> the baytrail.cfg fragment.
> 
> On Tue, 2013-09-24 at 11:11 +0800, rebecca.swee.fun.chang at intel.com
> wrote:
> > From: "Chang, Rebecca Swee Fun" <rebecca.swee.fun.chang at intel.com>
> >
> > To create baytrail & its associated scc & cfg files under meta branch
> >
> > Signed-off-by: Chang, Rebecca Swee Fun
> > <rebecca.swee.fun.chang at intel.com>
> > ---
> >  .../bsp/baytrail/baytrail-preempt-rt.scc           |   16 +++++
> >  .../bsp/baytrail/baytrail-standard.scc             |   16 +++++
> >  meta/cfg/kernel-cache/bsp/baytrail/baytrail.cfg    |   71
> ++++++++++++++++++++
> >  meta/cfg/kernel-cache/bsp/baytrail/baytrail.scc    |   16 +++++
> >  .../bsp/baytrail/baytrail32-preempt-rt.scc         |   16 +++++
> >  .../bsp/baytrail/baytrail32-standard.scc           |   16 +++++
> >  meta/cfg/kernel-cache/bsp/baytrail/baytrail32.scc  |   14 ++++
> >  7 files changed, 165 insertions(+)
> >  create mode 100644
> > meta/cfg/kernel-cache/bsp/baytrail/baytrail-preempt-rt.scc
> >  create mode 100644
> > meta/cfg/kernel-cache/bsp/baytrail/baytrail-standard.scc
> >  create mode 100644 meta/cfg/kernel-cache/bsp/baytrail/baytrail.cfg
> >  create mode 100644 meta/cfg/kernel-cache/bsp/baytrail/baytrail.scc
> >  create mode 100644
> > meta/cfg/kernel-cache/bsp/baytrail/baytrail32-preempt-rt.scc
> >  create mode 100644
> > meta/cfg/kernel-cache/bsp/baytrail/baytrail32-standard.scc
> >  create mode 100644 meta/cfg/kernel-cache/bsp/baytrail/baytrail32.scc
> >
> > diff --git
> > a/meta/cfg/kernel-cache/bsp/baytrail/baytrail-preempt-rt.scc
> > b/meta/cfg/kernel-cache/bsp/baytrail/baytrail-preempt-rt.scc
> > new file mode 100644
> > index 0000000..0513dab
> > --- /dev/null
> > +++ b/meta/cfg/kernel-cache/bsp/baytrail/baytrail-preempt-rt.scc
> > @@ -0,0 +1,16 @@
> > +define KMACHINE baytrail
> > +define KTYPE preempt-rt
> > +define KARCH x86_64
> > +
> > +# no new branch required, re-use the ktypes/preempt-rt/preempt-rt.scc
> > +branch include ktypes/preempt-rt/preempt-rt.scc
> > +
> > +include baytrail.scc
> > +
> > +include cfg/efi-ext.scc
> > +
> > +# default policy for preempt-rt kernels include
> > +cfg/usb-mass-storage.scc include cfg/boot-live.scc include
> > +features/latencytop/latencytop.scc
> > +include features/profiling/profiling.scc
> > diff --git a/meta/cfg/kernel-cache/bsp/baytrail/baytrail-standard.scc
> > b/meta/cfg/kernel-cache/bsp/baytrail/baytrail-standard.scc
> > new file mode 100644
> > index 0000000..609b580
> > --- /dev/null
> > +++ b/meta/cfg/kernel-cache/bsp/baytrail/baytrail-standard.scc
> > @@ -0,0 +1,16 @@
> > +define KMACHINE baytrail
> > +define KTYPE standard
> > +define KARCH x86_64
> > +
> > +include ktypes/standard/standard.scc
> > +branch baytrail
> > +
> > +include baytrail.scc
> > +
> > +include cfg/efi-ext.scc
> > +
> > +# default policy for standard kernels include
> > +cfg/usb-mass-storage.scc include cfg/boot-live.scc include
> > +features/latencytop/latencytop.scc
> > +include features/profiling/profiling.scc
> > diff --git a/meta/cfg/kernel-cache/bsp/baytrail/baytrail.cfg
> > b/meta/cfg/kernel-cache/bsp/baytrail/baytrail.cfg
> > new file mode 100644
> > index 0000000..bb6f2b8
> > --- /dev/null
> > +++ b/meta/cfg/kernel-cache/bsp/baytrail/baytrail.cfg
> > @@ -0,0 +1,71 @@
> > +CONFIG_MCORE2=y
> > +CONFIG_X86_INTEL_LPSS=y
> > +CONFIG_BYT_LPSS_BRD=y
> > +CONFIG_PRINTK=y
> > +CONFIG_PRINTK_TIME=y
> > +
> > +# Basic hardware support for the box - network, USB, PCI, sound
> > +CONFIG_PM=y CONFIG_ASPM=y
> > +
> > +CONFIG_PCI=y
> > +CONFIG_PCIEPORTBUS=y
> > +
> > +CONFIG_INPUT=y
> > +CONFIG_INPUT_EVDEV=y
> > +
> > +CONFIG_NET=y
> > + CONFIG_NETDEVICES=y
> 
> These are part of base.cfg and should not be duplicated here.

Noted. I will remove CONFIG_NET=y and CONFIG_NETDEVICES=y as they were duplicated.

> > +
> > +CONFIG_ATA=y
> > +CONFIG_ATA_GENERIC=y
> 
> Is this needed?

Yes.

> 
> > +CONFIG_ATA_SFF=y
> 
> Do you want to support legacy IDE and PATA devices on BayTrail SoCs ?

Noted and realized only CONFIG_SATA_AHCI is needed . I will remove CONFIG_ATA_SFF.

> 
> > +CONFIG_ATA_PIIX=y
> 
> Why do you need ATA_PIIX?

Noted as it is redundant, will remove it.

> 
> 
> > +CONFIG_CHR_DEV_SG=y
> > +CONFIG_SATA_AHCI=y
> > +
> > +CONFIG_SOUND=y
> > +CONFIG_SND=y
> > +CONFIG_SND_HDA_INTEL=y
> 
> This is fine, but ultimately this should be abstracted into a feature as it is
> duplicated in at least 16 cfg files.

For baytrail, we are supporting HAD and LPE Asoc. In future, yes, we will have a plan to move these to feature. 

> 
> > +
> > +CONFIG_BACKLIGHT_LCD_SUPPORT=y
> > +CONFIG_BACKLIGHT_CLASS_DEVICE=y
> > +
> > +# Make sure these are on, otherwise the bootup won't be fun
> > +CONFIG_UNIX=y CONFIG_INET=y CONFIG_MODULES=y
> 
> These are part of base.cfg and should not be duplicated here.

Noted. Will remove CONFIG_UNIX=y CONFIG_INET=y CONFIG_MODULES=y.

> 
> > +CONFIG_SHMEM=y
> 
> This is part of standard and should not be duplicated here
> 
> > +CONFIG_TMPFS=y
> 
> Part of base
> 
> > +CONFIG_PACKET=y
> 
> Part of base
> 
> > +
> > +CONFIG_AGP=y
> > +CONFIG_AGP_INTEL=y
> 
> Part of i915.cfg which should be enabled for Intel graphics platforms.

Noted. Will remove CONFIG_SHMEM=y CONFIG_TMPFS=y CONFIG_PACKET=y CONFIG_AGP=y and CONFIG_AGP_INTEL=y.

> 
> > +CONFIG_DRM=y
> > +CONFIG_DRM_EMGD=y
> 
> There is a feature for EMGD, do not duplicate here.

Feature for EMGD found in feature/drm-emgd is not the configurations that I want for baytrail. 
In drm-emgd.cfg, there is CONFIG_DRM_EGD but not CONFIG_DRM_EMGD.
That is why I put it here.

> 
> > +
> > +CONFIG_DW_DMAC=y
> > +CONFIG_DW_DMAC_PCI=y
> 
> I'm not familiar with the DesignWare AHB DMA platform driver.... this is
> something needed for BayTrail?

Yes.

> 
> > +
> > +CONFIG_SERIAL_8250_DW=y
> 
> This too?

Yes

> 
> > +
> > +CONFIG_SPI=y
> > +CONFIG_SPI_SPIDEV=y
> 
> There are spi and spidev features, please use those.

Noted. I will do the changes and test it first so that it won't breaks the driver functionality.

> 
> > +CONFIG_SPI_PXA2XX_DMA=y
> > +CONFIG_SPI_PXA2XX=y
> > +CONFIG_SPI_PXA2XX_PCI=y
> 
> 
> 
> 
> > +
> > +CONFIG_I2C=y
> > +CONFIG_I2C_CHARDEV=y
> 
> See features i2c and i2cdbg

Noted. I will do the changes and test it first so that it won't breaks the driver functionality.

> 
> > +CONFIG_I2C_DESIGNWARE_CORE=y
> > +CONFIG_I2C_DESIGNWARE_PLATFORM=y
> > +CONFIG_I2C_DESIGNWARE_PCI=y
> 
> 
> Again, specific to BayTrail? If not, perhaps these designware bits should be
> added as a fragment which specific BSPs can pull in.

Yes, it is baytrail specific.

> 
> > +
> > +CONFIG_GPIOLIB=y
> > +CONFIG_GPIO_BYT=y
> > +CONFIG_GPIO_BYT_DEVICE=y
> > +
> 
> 
> GPIO_BYT* configs do not exist in the sources nor do they exist in the
> upstream kernel.

We will have internal discussion regarding this.
For Darren, please refer to Ong Boon Leong's email reply in the internal review email thread. He will comment on this.

> 
> 
> > +CONFIG_MMC=y
> > +CONFIG_MMC_SDHCI=y
> > +CONFIG_MMC_SDHCI_PCI=y
> > +CONFIG_MMC_SDHCI_ACPI=y
> 
> 
> Ideally, this would be a feature fragment and used in all the BSPs currently
> using this block. But... it's OK as is given the precedent.

OK.

Thanks a lot.

Regards
Rebecca

> 
> Regards,
> 
> --
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Linux Kernel
> 