[LEDE-DEV] merging the layerscape target
Y.T. Jiang
yutang.jiang at nxp.com
Tue Sep 27 22:34:20 PDT 2016
> -----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/lin
> >> 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
More information about the Lede-dev
mailing list