Re: [PATCH] cryptodev kernel module recipe

[Darren Hart <dvhart@linux.intel.com>]

On 10/18/2012 04:33 PM, McClintock Matthew-B29882 wrote:
> On Thu, Oct 18, 2012 at 3:16 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>
>>
>> On 10/18/2012 05:57 AM, Yashpal Dutta wrote:
>>> This is a /dev/crypto device driver, equivalent to those in OpenBSD or FreeBSD.
>>> The main idea is to access of existing ciphers in kernel space from userspace,
>>> thus enabling re-use of a hardware implementation of a cipher.
>>>
>>> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
>>> ---
>>>  meta/recipes-kernel/cryptodev/cryptodev_1.5.bb     |   18 +++++++++++++
>>>  .../cryptodev/files/makefile_fixup.patch           |   26 ++++++++++++++++++++
>>>  2 files changed, 44 insertions(+), 0 deletions(-)
>>>  create mode 100644 meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>>  create mode 100644 meta/recipes-kernel/cryptodev/files/makefile_fixup.patch
>>>
>>> diff --git a/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>> new file mode 100644
>>> index 0000000..5125710
>>> --- /dev/null
>>> +++ b/meta/recipes-kernel/cryptodev/cryptodev_1.5.bb
>>> @@ -0,0 +1,18 @@
>>> +SECTION = "devel"
>>> +SUMMARY = "Linux Cryptodev KERNEL MODULE"
>>> +DESCRIPTION = "The Cryptodev package contains the kernel /dev/crypto module"
>>> +LICENSE = "GPLv2"
>>> +LIC_FILES_CHKSUM = "file://COPYING;md5=b234ee4d69f5fce4486a80fdaf4a4263"
>>> +
>>> +DEPENDS = "virtual/kernel"
>>
>> This DEPENDS in inherited from the module.bbclass, no need to duplicate
>>
>>> +
>>> +inherit module
>>> +
>>> +SRCREV = "1c24a0aa996630518d47826a2e3fea129ea094c7"
>>> +
>>> +SRC_URI = "git://repo.or.cz/cryptodev-linux.git;protocol=git \
>>> +       file://makefile_fixup.patch"
>>
>> Tabs to indent, spaces to align. Spaces here please.
>>
>>> +
>>> +EXTRA_OEMAKE='KERNEL_DIR="${STAGING_KERNEL_DIR}" PREFIX="${D}"'
>>
>> modules.bbclass already sets KERNEL_PATH and KERNEL_SRC, perhaps you
>> could use one of those?
> 
> cryptodev Makefile does not use these it uses KERNEL_DIR in it's
> Makefile for whatever reason. Getting an upstream project to change is
> more difficult.

I think this is the second reference to KERNEL_DIR in an external module,
perhaps module.bbclass should add that to it's list of predefined names for
the STAGING_KERNEL_DIR.

> 
> Is changing upstream cryptodev is one thing (e.g. SUBDIR= vs. M=), how
> far do you want a patch to a Makefile to change the way a project
> builds?

Yeah, you're correct about the Makefile changes. I got carried away
during a rapid review.


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Technical Lead - Linux Kernel