> -----Original Message----- > From: Dmytriyenko, Denys > Sent: Monday, October 08, 2012 3:08 PM > To: Maupin, Chase > Cc: Koen Kooi; Reizer, Eyal; [email protected]; Algranati, > EYAL > Subject: Re: [meta-ti] [PATCH] bt-enable: Add bt-enable recipe > > On Mon, Oct 08, 2012 at 01:31:26PM +0000, Maupin, Chase wrote: > > > -----Original Message----- > > > From: Koen Kooi [mailto:[email protected]] > > > Sent: Monday, October 08, 2012 8:05 AM > > > To: Maupin, Chase > > > Cc: Franklin S. Cooper Jr; Reizer, Eyal; Algranati, EYAL; > meta- > > > [email protected] > > > Subject: Re: [meta-ti] [PATCH] bt-enable: Add bt-enable > recipe > > > > > > > > > Op 8 okt. 2012, om 14:09 heeft "Maupin, Chase" > > > <[email protected]> het volgende geschreven: > > > > > > > Looping in wireless team who can answer that question. > > > > > > > > However, I don't see tis as blocking the acceptance of this > > > recipe but more of feedback on the driver implementation. Do > you > > > see a reason to block the recipe? > > > > > > I see tons of reasons to block this recipe: duplicate > DEPENDS, > > > duplicate PACKAGE_ARCH, wrong ordering of variables, > inconsistent > > > whitespace, overriding EXTRA_OEMAKE instead of appending it, > > > introducing another dependency on meta-oe. And that's even > > > without the issue I raised earlier: why do it different from > > > mainline? It looks like this is an 'opkg install unbreak- > > > bluetooth' type of thing and you can't even tell me *why*. > > > > I looped in the driver maintainers to tell you *why*, so please > give them a > > chance to respond. > > > > As for the other feedback, thank you. I'm sure Franklin will > find these > > specifics helpful for correcting his recipe. > > > > But my initial statement stands. The structure of the driver > should not be > > what blocks this recipe. Feedback to the driver owner is > useful and > > appreciated so don't think I'm saying we should ignore that. > Let's focus on > > the recipe here and let the driver owner have a chance to > address issues you > > see with their driver. > > > > > Do I need to go on? > > > > If you have more useful feedback then please do. Perhaps some > specifics on > > "wrong ordering of variables" and the new "dependency on meta- > oe" would be > > useful. I see that PLATFORM could possibly be done differently > (and should > > not have both omapl138 and da850-omapl138-evm defined), but I'm > afraid I > > don't see the meta-oe dependency. Any help you can give here > would be > > appreciated. > > Chase, > > Franklin already sent the new revision of the patch (although, > the last one > was v2 and the new one is still v2...) with most of the issues > fixed. > > For the record, here are some specifics you asked: > > 1. duplicate DEPENDS - module.bbclass already sets it for you > 2. duplicate PACKAGE_ARCH - same as above, set in module.bbclass > 3. wrong ordering of variables - there are some hard exceptions, > but even > Yocto guys and Richard are not that strict on this matter... > 4. inconsistent whitespace - EXTRA_OEMAKE is multi-line, but uses > mix of > spaces and tabs > 4a. please note, it is "acceptable" to use tabs for shell-code > indentation, > but not in Python _code_. Also, it is acceptable to use tabs in > multi-line > variables. But please be consistent in using one or another. For > simplicity it > may just be easier to use only spaces... > 5. overriding EXTRA_OEMAKE - you might lose framework's default > make flags, > although by default it's just "-e MAKEFLGAS=" > 6. another dependency on meta-oe - while not obvious, > MACHINE_KERNEL_PR is > really being used by meta-oe: > http://cgit.openembedded.org/meta-openembedded/tree/meta- > oe/classes/kernel.bbclass#n24
Denys, Thanks for the clarification. So should we not be using MACHINE_KERNEL_PR in meta-ti if we do not want a dependency on meta-oe? > > -- > Denys > > > > > >> -----Original Message----- > > > >> From: [email protected] [mailto:meta-ti- > > > >> [email protected]] On Behalf Of Koen Kooi > > > >> Sent: Saturday, October 06, 2012 2:43 AM > > > >> To: Franklin S. Cooper Jr > > > >> Cc: [email protected] > > > >> Subject: Re: [meta-ti] [PATCH] bt-enable: Add bt-enable > recipe > > > >> > > > >> Why isn't this using the standard kim platformdata in the > > > kernel? > > > >> That also gives you a nice rfkill interface. That's how > all > > > the > > > >> wl127x boards in mainline do it. > > > >> > > > >> Op 5 okt. 2012, om 21:27 heeft Franklin S. Cooper Jr > > > >> <[email protected]> het volgende geschreven: > > > >> > > > >>> * Port bt-enable recipe from arago. > > > >>> * Bt-enable is used to enable the BT module on the TI > wl12xx > > > >>> Wi-Fi + Bluetooth module. > > > >>> > > > >>> Signed-off-by: Franklin S. Cooper Jr <[email protected]> > > > >>> --- > > > >>> Version 2 changes: > > > >>> Loosen up COMPATIBLE_MACHINE restriction. Using generic > SOC > > > >> family when > > > >>> possible. > > > >>> > > > >>> ...kefile-Update-makefile-to-work-well-in-OE.patch | 78 > > > >> ++++++++++++++++++++ > > > >>> recipes-bsp/bt-enable/bt-enable_1.0.bb | 41 > > > >> ++++++++++ > > > >>> 2 files changed, 119 insertions(+), 0 deletions(-) > > > >>> create mode 100644 recipes-bsp/bt-enable/bt-enable/0001- > > > >> Makefile-Update-makefile-to-work-well-in-OE.patch > > > >>> create mode 100644 recipes-bsp/bt-enable/bt-enable_1.0.bb > > > >>> > > > >>> diff --git a/recipes-bsp/bt-enable/bt-enable/0001- > Makefile- > > > >> Update-makefile-to-work-well-in-OE.patch b/recipes-bsp/bt- > > > >> enable/bt-enable/0001-Makefile-Update-makefile-to-work- > well- > > > in- > > > >> OE.patch > > > >>> new file mode 100644 > > > >>> index 0000000..7e6756c > > > >>> --- /dev/null > > > >>> +++ b/recipes-bsp/bt-enable/bt-enable/0001-Makefile- > Update- > > > >> makefile-to-work-well-in-OE.patch > > > >>> @@ -0,0 +1,78 @@ > > > >>> +From a400ac3d83023a66a356d056899d6b380cb30473 Mon Sep 17 > > > >> 00:00:00 2001 > > > >>> +From: Chase Maupin <[email protected]> > > > >>> +Date: Wed, 7 Mar 2012 10:51:43 -0600 > > > >>> +Subject: [PATCH] Makefile: Update makefile to work with > OE > > > >>> + > > > >>> +* Updated the makefile to with OE > > > >>> +* Use the kernel install target for installing the > module > > > >>> + > > > >>> +Upstream-Status: Pending > > > >>> + * will be in next release > > > >>> + > > > >>> +Signed-off-by: Chase Maupin <[email protected]> > > > >>> +--- > > > >>> + Makefile | 45 > ++++++++++++++++++++++++++++++++++++++++--- > > > -- > > > >>> + 1 files changed, 40 insertions(+), 5 deletions(-) > > > >>> + > > > >>> +diff --git a/Makefile b/Makefile > > > >>> +index ebbcd11..b17d33e 100755 > > > >>> +--- a/Makefile > > > >>> ++++ b/Makefile > > > >>> +@@ -7,15 +7,50 @@ else > > > >>> + EXTRA_CFLAGS += -O2 > > > >>> + endif > > > >>> + > > > >>> ++-include ../../../Rules.make > > > >>> ++ > > > >>> ++# If KERNEL_DIR is not set then use the default in > > > Rules.make > > > >>> ++KERNEL_DIR ?= ${LINUXKERNEL_INSTALL_DIR} > > > >>> ++DEST_DIR ?= ${DESTDIR} > > > >>> ++ > > > >>> ++PLATFORM ?= "unknown" > > > >>> ++MACHINE_NAME ?= "unknown" > > > >>> ++ > > > >>> ++# Use the PLATFORM value from the Rules.make if it was > > > >> sourced > > > >>> ++ifeq ($(PLATFORM), am335x-evm) > > > >>> ++ MACHINE_NAME := "am335x" > > > >>> ++endif > > > >>> ++ifeq ($(PLATFORM), am180x-evm) > > > >>> ++ MACHINE_NAME := "am1808" > > > >>> ++endif > > > >>> ++ifeq ($(PLATFORM), da850-omapl138-evm) > > > >>> ++ MACHINE_NAME := "am1808" > > > >>> ++endif > > > >>> ++ifeq ($(PLATFORM), am37x-evm) > > > >>> ++ MACHINE_NAME := "omap3evm" > > > >>> ++endif > > > >>> ++ > > > >>> ++# If CROSS_COMPILE is not set by Rules.make then set a > sane > > > >> default > > > >>> ++CROSS_COMPILE ?= arm-arago-linux-gnueabi- > > > >>> ++export CROSS_COMPILE > > > >>> ++ > > > >>> ++# set the INSTALL_MOD_DIR so that the executables won't > be > > > >> placed in extra > > > >>> ++INSTALL_MOD_DIR = kernel/drivers/bt_enable > > > >>> ++export INSTALL_MOD_DIR > > > >>> ++ > > > >>> + obj-m := gpio_en.o > > > >>> + > > > >>> ++MAKE_ENV = ARCH=arm > > > >>> ++ > > > >>> + PWD := $(shell pwd) > > > >>> + all: > > > >>> +- pwd > > > >>> +- @echo EXTRA_CFLAGS = $(EXTRA_CFLAGS) > > > >>> +- $(MAKE) CROSS_COMPILE=$(CROSS_COMPILE) ARCH=$(ARCH) > > > >> EXTRA_CFLAGS="$(EXTRA_CFLAGS)" -C $(KERNEL_DIR) M=$(PWD) > > > modules > > > >>> ++ @cp -f gpio_en_${MACHINE_NAME}.c gpio_en.c > > > >>> ++ $(MAKE) EXTRA_CFLAGS="$(EXTRA_CFLAGS)" -C > > > $(KERNEL_DIR) > > > >> $(MAKE_ENV) \ > > > >>> ++ M=$(PWD) modules > > > >>> ++ > > > >>> + install: > > > >>> +- install -d > > > >> > > > > ${DEST_DIR}${BASE_LIB_DIR}/modules/${KRNL_VER}/kernel/drivers/bt_ > > > >> enable > > > >>> +- install -m 0755 ./gpio_en.ko > > > >> > > > > ${DEST_DIR}${BASE_LIB_DIR}/modules/${KRNL_VER}/kernel/drivers/bt_ > > > >> enable > > > >>> ++ $(MAKE) EXTRA_CFLAGS="$(EXTRA_CFLAGS)" -C > > > $(KERNEL_DIR) > > > >> $(MAKE_ENV) \ > > > >>> ++ M=$(PWD) INSTALL_MOD_PATH="${DEST_DIR}" > modules_install > > > >>> ++ > > > >>> + clean: > > > >>> + rm -rf *.o *~ core .depend .*.cmd *.ko *.mod.c > > > >> .tmp_versions *.symvers > > > >>> +-- > > > >>> +1.7.0.4 > > > >>> diff --git a/recipes-bsp/bt-enable/bt-enable_1.0.bb > > > b/recipes- > > > >> bsp/bt-enable/bt-enable_1.0.bb > > > >>> new file mode 100644 > > > >>> index 0000000..b6f1ee7 > > > >>> --- /dev/null > > > >>> +++ b/recipes-bsp/bt-enable/bt-enable_1.0.bb > > > >>> @@ -0,0 +1,41 @@ > > > >>> +DESCRIPTION = "BT GPIO Enable" > > > >>> +LICENSE = "BSD" > > > >>> +COMPATIBLE_MACHINE = "omap3|omapl138|da850-omapl138- > > > evm|ti33x" > > > >>> +LIC_FILES_CHKSUM = > > > >> > > > > "file://gpio_en_am1808.c;beginline=1;endline=34;md5=fe94639d8f61c > > > >> 867d1bc4bf61473d3cd \ > > > >>> + > > > >> > > > > file://gpio_en_am335x.c;beginline=1;endline=34;md5=fe94639d8f61c8 > > > >> 67d1bc4bf61473d3cd \ > > > >>> + > > > >> > > > > file://gpio_en_omap3evm.c;beginline=1;endline=34;md5=fe94639d8f61 > > > >> c867d1bc4bf61473d3cd \ > > > >>> +" > > > >>> + > > > >>> +DEPENDS = "virtual/kernel" > > > >>> + > > > >>> +PACKAGE_ARCH = "${MACHINE_ARCH}" > > > >>> + > > > >>> +PACKAGE_STRIP = "no" > > > >>> + > > > >>> +inherit module > > > >>> + > > > >>> +MACHINE_KERNEL_PR_append = "a" > > > >>> +PR = "r0+gitr${SRCREV}" > > > >>> + > > > >>> +SRCREV = "97c4600ff7d39f1cc6079939248cd9ed15100db4" > > > >>> + > > > >>> +SRC_URI = "git://github.com/TI- > > > ECS/bt_enable.git;protocol=git > > > >> \ > > > >>> + file://0001-Makefile-Update-makefile-to-work- > > > well- > > > >> in-OE.patch \ > > > >>> + " > > > >>> + > > > >>> +PLATFORM = "unknown" > > > >>> +PLATFORM_ti33x = "am335x-evm" > > > >>> +PLATFORM_omap3 = "am37x-evm" > > > >>> +PLATFORM_omapl138 = "am180x-evm" > > > >>> +PLATFORM_da850-omapl138-evm = "am180x-evm" > > > >>> + > > > >>> +S = "${WORKDIR}/git" > > > >>> + > > > >>> +EXTRA_OEMAKE = " \ > > > >>> + KERNEL_DIR=${STAGING_KERNEL_DIR} \ > > > >>> + PLATFORM=${PLATFORM} \ > > > >>> +" > > > >>> + > > > >>> +do_install () { > > > >>> + oe_runmake 'DEST_DIR=${D}' install > > > >>> +} > > > >>> -- > > > >>> 1.7.0.4 > > > >>> > > > >>> _______________________________________________ > > > >>> meta-ti mailing list > > > >>> [email protected] > > > >>> https://lists.yoctoproject.org/listinfo/meta-ti > > > >> > > > >> _______________________________________________ > > > >> meta-ti mailing list > > > >> [email protected] > > > >> https://lists.yoctoproject.org/listinfo/meta-ti > > > > _______________________________________________ > > meta-ti mailing list > > [email protected] > > https://lists.yoctoproject.org/listinfo/meta-ti _______________________________________________ meta-ti mailing list [email protected] https://lists.yoctoproject.org/listinfo/meta-ti
