[LEDE-DEV] merging the layerscape target
Y.T. Jiang
yutang.jiang at nxp.com
Wed Sep 28 02:16:55 PDT 2016
Hi John,
I have submit a new pull request and post a simple test log, please check:
https://github.com/lede-project/source/pull/344
V6 patch update summary:
1.Dropping unnecessary USB_EHCI_FSL related patches.
2.Change BOARDNAME from "layerscape" to "NXP Layerscape"
Thanks & Best Regards
Jiang Yutang
> -----Original Message-----
> From: John Crispin [mailto:john at phrozen.org]
> Sent: Wednesday, September 28, 2016 1:58 PM
> To: Y.T. Jiang
> Subject: Re: [LEDE-DEV] merging the layerscape target
>
> Ok,
>
> please also make this change in target/linux/layerscape/Makefile
>
> BOARDNAME:=layerscape
>
> to
>
> BOARDNAME:=NXP Layerscape
>
>
> John
>
> On 28/09/2016 07:34, Y.T. Jiang wrote:
> >
> >
> >> -----Original Message-----
> >> From: John Crispin [mailto:john at phrozen.org]
> >> Sent: Wednesday, September 28, 2016 1:16 PM
> >> To: Y.T. Jiang
> >> Cc: LEDE Development List
> >> Subject: Re: [LEDE-DEV] merging the layerscape target
> >>
> >>
> >>
> >> On 27/09/2016 13:39, Y.T. Jiang wrote:
> >>> Hi John,
> >>>
> >>> After the survey found, the error are from a old
> >>> usb2.0(USB_EHCI_FSL)
> >> driver which is usually used for powerpc arch, not for layerscape
> >> ls1043ardb(arm64 arch) Soc.
> >>> The default kernel config and packages not enable the old usb2.0
> >>> driver
> >> in my patch. It should be enabled by:
> >>> [x] Select all target specific packages by default [x] Select all
> >>> kernel module packages by default [x] Select all userspace packages
> >>> by
> >>>
> >>> Further investigation found, if remove the kernel patch 8041, can
> >>> avoid
> >> USB_EHCI_FSL be compiled. But the patch hint, USB_EHCI_FSL will
> >> appear in other arch.
> >>> 8041-usb-kconfig-remove-dependency-FSL_SOC-for-ehci-fsl-d.patch
> >>> "usb: kconfig: remove dependency FSL_SOC for ehci fsl driver "
> >>> CONFIG_USB_EHCI_FSL is not dependent on FSL_SOC, it can be built on
> >> non-PPC platforms.
> >>> ...
> >>> 21 config USB_EHCI_FSL
> >>> 22 tristate "Support for Freescale PPC on-chip EHCI USB
> >> controller"
> >>> 23 - depends on FSL_SOC
> >>> 24 + depends on USB_EHCI_HCD
> >>> ...
> >>>
> >>
> >> so this patch is indeed added by your series, yet the module does not
> >> build. i would suggest dropping this patch as it seems totally
> >> unrelated and in fact makes it not work. does your target use FSL EHCI
> support ?
> >>
> >> John
> >>
> > Yes, I double check the patches, it not need indeed. In fact, some
> patches was backported by other colleagues, who not familiar with special
> IP block and mixed with other unnecessary patches. I will drop it and
> create a new patch, after functional validation then submit a new pull
> request. Thank you John.
> > [Y.T. Jiang]
> >
> >>> If have other better ways in dealing with the dependencies in LEDE?
> >>>
> >>>
> >>> Thanks & Best Regards
> >>> Jiang Yutang
> >>>
> >>>> -----Original Message-----
> >>>> From: John Crispin [mailto:john at phrozen.org]
> >>>> Sent: Tuesday, September 27, 2016 2:34 PM
> >>>> To: Y.T. Jiang
> >>>> Cc: LEDE Development List
> >>>> Subject: Re: [LEDE-DEV] merging the layerscape target
> >>>>
> >>>> Hi,
> >>>>
> >>>> i just tried V5 and when building the target with all packages
> >>>> selected i run into some errors
> >>>>
> >>>> make[5]: Entering directory
> >>>> `/home/blogic/source/build_dir/target-aarch64_armv8-a_musl-1.1.15/l
> >>>> in
> >>>> ux-
> >>>> layerscape_64b/linux-4.4.21'
> >>>> CHK include/config/kernel.release
> >>>> CHK include/generated/uapi/linux/version.h
> >>>> CHK include/generated/utsrelease.h
> >>>> CHK include/generated/bounds.h
> >>>> CHK include/generated/timeconst.h
> >>>> CHK include/generated/asm-offsets.h
> >>>> CALL scripts/checksyscalls.sh
> >>>> CC [M] drivers/usb/host/ehci-fsl.o In file included from
> >>>> drivers/usb/host/ehci-fsl.c:39:0:
> >>>> drivers/usb/host/ehci.h: In function 'ehci_readl':
> >>>> drivers/usb/host/ehci.h:741:9: error: implicit declaration of
> >>>> function 'readl' [-Werror=implicit-function-declaration]
> >>>> return readl(regs);
> >>>> ^
> >>>> drivers/usb/host/ehci.h: In function 'ehci_writel':
> >>>> drivers/usb/host/ehci.h:768:3: error: implicit declaration of
> >>>> function 'writel' [-Werror=implicit-function-declaration]
> >>>> writel(val, regs);
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c: In function 'fsl_ehci_drv_probe':
> >>>> drivers/usb/host/ehci-fsl.c:130:3: error: implicit declaration of
> >>>> function 'clrsetbits_be32' [-Werror=implicit-function-declaration]
> >>>> clrsetbits_be32(hcd->regs + FSL_SOC_USB_CTRL,
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c: In function 'ehci_fsl_setup_phy':
> >>>> drivers/usb/host/ehci-fsl.c:205:4: error: implicit declaration of
> >>>> function 'clrbits32' [-Werror=implicit-function-declaration]
> >>>> clrbits32(non_ehci + FSL_SOC_USB_CTRL,
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c:244:9: error: implicit declaration of
> >>>> function 'in_be32' [-Werror=implicit-function-declaration]
> >>>> if (!(in_be32(non_ehci + FSL_SOC_USB_CTRL) & PHY_CLK_VALID)) {
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c: In function 'ehci_fsl_usb_setup':
> >>>> drivers/usb/host/ehci-fsl.c:276:3: error: implicit declaration of
> >>>> function 'out_be32' [-Werror=implicit-function-declaration]
> >>>> out_be32(non_ehci + FSL_SOC_USB_SNOOP1, 0x0 | SNOOP_SIZE_2GB);
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c:293:9: error: implicit declaration of
> >>>> function 'mfspr' [-Werror=implicit-function-declaration]
> >>>> svr = mfspr(SPRN_SVR);
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c:293:15: error: 'SPRN_SVR' undeclared
> >>>> (first use in this function)
> >>>> svr = mfspr(SPRN_SVR);
> >>>> ^
> >>>> drivers/usb/host/ehci-fsl.c:293:15: note: each undeclared
> >>>> identifier is reported only once for each function it appears in
> >>>> cc1: some warnings being treated as errors
> >>>>
> >>>> to reproduce this run menuconfig and then Global build settings --->
> >>>> [x] Select all target specific packages by default
> >>>> [x] Select all kernel module packages by default
> >>>> [x] Select all userspace packages by default
> >>>>
> >>>> John
> >>>>
> >>>>
> >>>>
> >>>> On 21/09/2016 16:21, Y.T. Jiang wrote:
> >>>>> Hi Rafał and John,
> >>>>>
> >>>>> I update the patch and pull a new requests(329), please check and
> >>>> review, thanks!
> >>>>> https://github.com/lede-project/source/pull/329
> >>>>>
> >>>>> V5 patch update summary:
> >>>>> 1.Copyrights assigned to myself.
> >>>>> 2.Introduce DEVICE_TITLE and DEVICE_PACKAGES.
> >>>>> 3.Rename patches prefix with 1xxx,2xxx...
> >>>>> 4.Refresh patches by "make target/linux/refresh V=s"
> >>>>> 5.Move default packages to DEFAULT_PACKAGES.
> >>>>> 6.Optimize Build/mk_firmware.
> >>>>>
> >>>>> Thanks & Best Regards
> >>>>> Jiang Yutang
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Lede-dev [mailto:lede-dev-bounces at lists.infradead.org] On
> >>>>>> Behalf Of Y.T. Jiang
> >>>>>> Sent: Tuesday, September 20, 2016 8:28 PM
> >>>>>> To: Rafa? Mi?ecki
> >>>>>> Cc: LEDE Development List; John Crispin
> >>>>>> Subject: Re: [LEDE-DEV] merging the layerscape target
> >>>>>>
> >>>>>> Hi Rafał,
> >>>>>>
> >>>>>> Thank you for the detailed comment!
> >>>>>>
> >>>>>> Update status:
> >>>>>> prefixed with ">" --done
> >>>>>> Copyright --done
> >>>>>> make target/linux/refresh V=s --done
> >>>>>> Patches prefix with 1xxx,2xxx...refer target/linux/generic/PATCHES
> >>>>>> --done
> >>>>>> using DEVICE_TITLE DEVICE_PACKAGES...refer
> >>>>>> target/linux/bcm53xx/image/Makefile --ongoing
> >>>>>>
> >>>>>> After building and features validate, I will submit a new version
> >>>> patch.
> >>>>>>
> >>>>>>
> >>>>>> Thanks & Best Regards
> >>>>>> Jiang Yutang
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Rafał Miłecki [mailto:zajec5 at gmail.com]
> >>>>>>> Sent: Monday, September 19, 2016 7:55 PM
> >>>>>>> To: Y.T. Jiang
> >>>>>>> Cc: John Crispin; LEDE Development List
> >>>>>>> Subject: Re: [LEDE-DEV] merging the layerscape target
> >>>>>>>
> >>>>>>> On 19 September 2016 at 12:36, Y.T. Jiang <yutang.jiang at nxp.com>
> >>>> wrote:
> >>>>>>>> Thank you for your review and suggestion.
> >>>>>>>
> >>>>>>> Sure. One more note: please take a look at your mailer
> >> configuration.
> >>>>>>> It should keep all quotes prefixed with "> " to keep discussion
> >> clear.
> >>>>>>> https://en.wikipedia.org/wiki/Usenet_quoting
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Rafał Miłecki [mailto:zajec5 at gmail.com]
> >>>>>>>> Sent: Monday, September 19, 2016 4:01 PM
> >>>>>>>> To: John Crispin
> >>>>>>>> Cc: LEDE Development List; Y.T. Jiang
> >>>>>>>> Subject: Re: [LEDE-DEV] merging the layerscape target
> >>>>>>>>
> >>>>>>>> On 18 September 2016 at 14:24, John Crispin <john at phrozen.org>
> >> wrote:
> >>>>>>>>> i have just spent some time reviewing the layerscape PR [1]
> >>>>>>>>> and started a full build of it. its starting to look good and
> >>>>>>>>> i cannot see any blockers. if anyone has any hold on this
> >>>>>>>>> please let me know in the next couple of days. if i dont get
> >>>>>>>>> any vetos i will
> >>>> merge it.
> >>>>>>>>
> >>>>>>>> I can see following Copyright line over and over:
> >>>>>>>> Copyright (C) 2016 OpenWrt.org
> >>>>>>>> Yutang: did you really sign a contract with OpenWrt that
> >>>>>>>> included
> >>>>>>> passing your copyrights to the OpenWrt project? If not, you
> >>>>>>> should just keep Copyrights assigned to yourself.
> >>>>>>>> I really would like assigning copyrights to projects where it
> >>>>>>>> doesn't
> >>>>>>> apply.
> >>>>>>>> [I do not sign a contract with OpenWrt indeed. I refer to some
> >>>>>>>> others target while developing/backporting layerscape, I find
> >>>>>>>> almost of targets included OpenWrt.org Copyright, so I also put
> >>>>>>>> it in my code files. Now should I replace " Copyright (C) 2016
> >>>> OpenWrt.org"
> >>>>>> with "
> >>>>>>>> Copyright (C) 2016 Jiang Yutang <yutang.jiang at nxp.com>" ? or
> >>>>>>>> retain the both copyright: "Copyright (C) LEDE project, Jiang
> >>>>>>>> Yutang <yutang.jiang at nxp.com>" ?]
> >>>>>>>
> >>>>>>> You're correct, current sources are messy about this. I'm trying
> >>>>>>> to stop adding mode incorrectly copyrighted code.
> >>>>>>>
> >>>>>>> You should only have something like:
> >>>>>>> Copyright (C) 2016 Jiang Yutang <yutang.jiang at nxp.com> for the
> >>>>>>> code you have written.
> >>>>>>>
> >>>>>>>
> >>>>>>>> What about using some generic profile only and then using
> >>>>>>>> DEVICE_TITLE
> >>>>>>> DEVICE_PACKAGES to specify modules that should be included on
> >> rootfs?
> >>>>>>>> [I will try to use the two variables.]
> >>>>>>>
> >>>>>>> Thanks! This will allow building images for customized boards
> >>>>>>> with a single "make" call. It's part of recently introduced
> >>>>>>> TARGET_PER_DEVICE_ROOTFS system. You may take a look at
> >>>>>>> target/linux/bcm53xx/image/Makefile as an example. There is only
> >>>>>>> 1 subtarget, but it should give you a hint anyway.
> >>>>>>>
> >>>>>>>
> >>>>>>>> Would that be possible to split patches into accepted ones
> >>>>>>>> (backports)
> >>>>>>> and LEDE-specific ones?
> >>>>>>>> [The kernel patches: dpaa/qbman/fman/etc. it is really too big
> >>>>>>>> and interference review LEDE-specially code. I will split those
> >>>>>>>> kernel patches in folder patches-4.4 as the second, and keep
> >>>>>>>> the rest as fist LEDE-specific, what do you think about it?]
> >>>>>>>
> >>>>>>> For generic patches we have a following guide:
> >>>>>>> target/linux/generic/PATCHES
> >>>>>>>
> >>>>>>> You may try to follow this, if possible. E.g. you could use 0xxx
> >>>>>>> prefix for upstream accepted patches and some other prefix 1xxx,
> >>>>>>> 2xxx, or whatever applicable for other ones.
> >>>>>>>
> >>>>>>> It isn't a strict rule for targets, but it should make your
> >>>>>>> target easier to maintain I believe.
> >>>>>>>
> >>>>>>>
> >>>>>>>> Please refresh all target patches, right now I can see they
> >>>>>>>> contain all
> >>>>>>> these things like:
> >>>>>>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index
> >>>>>>>> 4cb98aa..a8a97bd 100644
> >>>>>>>> 1.7.9.5
> >>>>>>>> [I found it have conflicts in current kernel version with two
> >>>>>>>> patches(arm64/mm related, 0060 and 0061) while backporting the
> >>>>>>>> dpaa/qbman/fman driver, but I'm unacquainted with both mm and
> >>>>>>>> dpaa, our dpaa team are engaged in do upstream work and can't
> >>>>>>>> help me. So I revert the two patch to bypass this issue
> >>>>>>>> temporary, I would like to wait for more leisure time then to
> >>>>>>>> thorough investigate and solve it.]
> >>>>>>>
> >>>>>>> I think you misunderstood me. I don't have anything against your
> >>>>>>> patches, just the format. Please call make target/linux/refresh
> >>>>>>> V=s and that will convert all your patches to the expected
> >>>>>>> format
> >>>>>>> :)
> >>>>>>>
> >>>>>>> --
> >>>>>>> Rafał
> >>>>>> _______________________________________________
> >>>>>> Lede-dev mailing list
> >>>>>> Lede-dev at lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/lede-dev
> > _______________________________________________
> > Lede-dev mailing list
> > Lede-dev at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/lede-dev
> >
More information about the Lede-dev
mailing list