Re: [PATCH 1/2] linux/cmem.inc: Support reserving memory for CMEM.

[Jacob Stiffler <j-stiffler@ti.com>]

On 5/19/2015 1:58 PM, Denys Dmytriyenko wrote:
> On Tue, May 19, 2015 at 01:52:43PM -0400, Jacob Stiffler wrote:
>>
>> On 5/19/2015 1:47 PM, Denys Dmytriyenko wrote:
>>> On Tue, May 19, 2015 at 01:39:58PM -0400, Jacob Stiffler wrote:
>>>> On 5/19/2015 1:32 PM, Denys Dmytriyenko wrote:
>>>>> On Tue, May 19, 2015 at 01:26:05PM -0400, Jacob Stiffler wrote:
>>>>>> On 5/19/2015 1:25 PM, Denys Dmytriyenko wrote:
>>>>>>> On Tue, May 19, 2015 at 01:23:06PM -0400, Jacob Stiffler wrote:
>>>>>>>> On 5/18/2015 3:13 PM, Denys Dmytriyenko wrote:
>>>>>>>>> On Mon, May 18, 2015 at 03:03:18PM -0400, Jacob Stiffler wrote:
>>>>>>>>>> On 5/18/2015 2:11 PM, Denys Dmytriyenko wrote:
>>>>>>>>>>> On Mon, May 18, 2015 at 08:20:58AM -0400, Jacob Stiffler wrote:
>>>>>>>>>>>> To reserve contiguous memory for CMEM:
>>>>>>>>>>>> * include the "recipes-kernel/linux/cmem.inc"
>>>>>>>>>>>> * Set CMEM_BASE and CMEM_SIZE to the physical memory address and size,
>>>>>>>>>>>>    respectively, to reserve for CMEM.
>>>>>>>>>>>>
>>>>>>>>>>>> Currently limited to reserving a single memory region used to create
>>>>>>>>>>>> a single buffer pool of a single buffer.
>>>>>>>>>>> Looks good. 2 comments below.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Jacob Stiffler <j-stiffler@ti.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>   recipes-kernel/linux/cmem.inc        |   22 ++++++++++++++++++++++
>>>>>>>>>>>>   recipes-kernel/linux/linux/cmem.dtsi |   24 ++++++++++++++++++++++++
>>>>>>>>>>>>   2 files changed, 46 insertions(+)
>>>>>>>>>>>>   create mode 100644 recipes-kernel/linux/cmem.inc
>>>>>>>>>>>>   create mode 100644 recipes-kernel/linux/linux/cmem.dtsi
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/recipes-kernel/linux/cmem.inc b/recipes-kernel/linux/cmem.inc
>>>>>>>>>>>> new file mode 100644
>>>>>>>>>>>> index 0000000..207bdc6
>>>>>>>>>>>> --- /dev/null
>>>>>>>>>>>> +++ b/recipes-kernel/linux/cmem.inc
>>>>>>>>>>>> @@ -0,0 +1,22 @@
>>>>>>>>>>>> +FILESEXTRAPATHS_append := ":${THISDIR}/linux"
>>>>>>>>>>> Move the file into standard "files" directory and drop above line.
>>>>>>>>>>>
>>>>>>>>>> Ok.
>>>>>>>>>>
>>>>>>>>>>>> +SRC_URI += "file://cmem.dtsi"
>>>>>>>>>>>> +
>>>>>>>>>>>> +CMEM_BASE ?= ""
>>>>>>>>>>>> +CMEM_SIZE ?= ""
>>>>>>>>>>>> +
>>>>>>>>>>>> +do_compileconfigs_prepend() {
>>>>>>>>>>> Should this be do_configure_append() instead? It would probably be safer, as
>>>>>>>>>>> do_compileconfigs() is specific to multi-kernel.inc and some kernel recipes
>>>>>>>>>>> may not include it...
>>>>>>>>>>>
>>>>>>>>>> I had noticed that do_configure() gets invoked multiple times.
>>>>>>>>>>
>>>>>>>>>> I'll try it as a do_configure_append() and make sure it will work.
>>>>>>>>> Ah, you are right. Then you'd need to call this function uniquely like
>>>>>>>>> do_setup_cmem() and then addtask it after do_patch before do_configure. It
>>>>>>>>> looks like it should be safe to do it even before do_configure. It would be
>>>>>>>>> nice to do it after do_configure, but it needs to be before do_compile and
>>>>>>>>> also do_compileconfigs, while the latter one is only defined by multi-kernel
>>>>>>>>>
>>>>>>>> This should be called before do_create_srcipk(). But I suppose it
>>>>>>>> should not be assumed that do_create_srcipk() will be run for a
>>>>>>>> general kernel recipe.
>>>>>>>>
>>>>>>>> Should do_setup_cmem() be a postfunc of do_configure()? For example:
>>>>>>>>
>>>>>>>> do_configure[postfuncs] += "do_setup_cmem"
>>>>>>> Will it be called multiple times after each do_configure?
>>>>>>>
>>>>>> Sorry, I had meant do_patch().
>>>>> BTW, are you sure you want it before create_srcipk? It won't package the
>>>>> changes then...
>>>>>
>>>> I thought it would have to be done before create_srcipk, since
>>>> create_srcipk pulls the ${S} directory into the sourceipk. If this
>>>> is done after create_srcipk, then there will not be any way for
>>>> these changes to be packaged, correct?
>>> Oops, you are correct. I was thinking backwards...
>>> So, do_create_srcipk is also being re-called in do_compile_prepend, so it
>>> would pick up those changes.
>>>
>> But that is only for the case with the meta-arago bbappend, through
>> copy-defconfig.inc.
> And srcipk is specific to meta-arago as well...
>
>
>> Any reason it would be better to make this change after do_configure()?
> Nope, nothing specifically depending on do_configure here, AFAICS.
>

Then it seems that having as a do_patch() postfunc would be safest.

>>>>>>>>>>>> +    if [ ! -z "${CMEM_BASE}" ]
>>>>>>>>>>>> +    then
>>>>>>>>>>>> +        cp ${WORKDIR}/cmem.dtsi ${S}/arch/arm/boot/dts/${MACHINE}-cmem.dtsi
>>>>>>>>>>>> +
>>>>>>>>>>>> +        sed -i -e "s|__CMEM_BASE__|${CMEM_BASE}|g" \
>>>>>>>>>>>> +               -e "s|__CMEM_SIZE__|${CMEM_SIZE}|g" \
>>>>>>>>>>>> +               ${S}/arch/arm/boot/dts/${MACHINE}-cmem.dtsi
>>>>>>>>>>>> +
>>>>>>>>>>>> +        for dts in ${KERNEL_DEVICETREE}
>>>>>>>>>>>> +        do
>>>>>>>>>>>> +            echo "#include \"${MACHINE}-cmem.dtsi\"" >> ${S}/arch/arm/boot/dts/${dts%.dtb}.dts
> BTW, is that bashism at the end of the line ^^^^^^^^^^^
>

The "${dts%.dtb}"? Yes. I am doing this to remove the .dtb extension and 
then append the .dts extension, since KERNEL_DEVICE tree contains the 
dtb extended filenames.

Is this an issue?

>>>>>>>>>>>> +        done
>>>>>>>>>>>> +    fi
>>>>>>>>>>>> +}
>>>>>>>>>>>> diff --git a/recipes-kernel/linux/linux/cmem.dtsi b/recipes-kernel/linux/linux/cmem.dtsi
>>>>>>>>>>>> new file mode 100644
>>>>>>>>>>>> index 0000000..6b1da99
>>>>>>>>>>>> --- /dev/null
>>>>>>>>>>>> +++ b/recipes-kernel/linux/linux/cmem.dtsi
>>>>>>>>>>>> @@ -0,0 +1,24 @@
>>>>>>>>>>>> +/ {
>>>>>>>>>>>> +        reserved-memory {
>>>>>>>>>>>> +                cmem_block_mem_0: cmem_block_mem@__CMEM_BASE__ {
>>>>>>>>>>>> +                        reg = <0x__CMEM_BASE__ 0x__CMEM_SIZE__>;
>>>>>>>>>>>> +                        no-map;
>>>>>>>>>>>> +                        status = "okay";
>>>>>>>>>>>> +                };
>>>>>>>>>>>> +        };
>>>>>>>>>>>> +
>>>>>>>>>>>> +        cmem {
>>>>>>>>>>>> +                compatible = "ti,cmem";
>>>>>>>>>>>> +                #address-cells = <1>;
>>>>>>>>>>>> +                #size-cells = <0>;
>>>>>>>>>>>> +
>>>>>>>>>>>> +                status = "okay";
>>>>>>>>>>>> +
>>>>>>>>>>>> +                cmem_block_0: cmem_block@0 {
>>>>>>>>>>>> +                        reg = <0>;
>>>>>>>>>>>> +                        memory-region = <&cmem_block_mem_0>;
>>>>>>>>>>>> +                        cmem-buf-pools = <1 0x__CMEM_SIZE__>;
>>>>>>>>>>>> +                };
>>>>>>>>>>>> +        };
>>>>>>>>>>>> +
>>>>>>>>>>>> +};
>>>>>>>>>>>> -- 
>>>>>>>>>>>> 1.7.9.5
>>>>>>>>>>>>
>>>>>>>>>>>> -- 
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> meta-ti mailing list
>>>>>>>>>>>> meta-ti@yoctoproject.org
>>>>>>>>>>>> https://lists.yoctoproject.org/listinfo/meta-ti