Functional testing of mainline vchiq driver

[Phil Elwell]

On 30/10/2016 17:00, Michael Zoran wrote:
> On Fri, 2016-10-28 at 20:14 +0200, Noralf Trønnes wrote:
>> Den 28.10.2016 19:53, skrev Michael Zoran:
>>> On Fri, 2016-10-28 at 19:50 +0200, Noralf Trønnes wrote:
>>>> Den 28.10.2016 18:01, skrev Eric Anholt:
>>>>> Stefan Wahren <stefan.wahren at i2se.com> writes:
>>>>>
>>>>>> Am 28.10.2016 um 09:27 schrieb Michael Zoran:
>>>>>>> On Wed, 2016-10-26 at 20:00 +0200, Stefan Wahren wrote:
>>>>>>>> Hi Michael,
>>>>>>>>
>>>>>>>>> Michael Zoran <mzoran at crowfest.net> hat am 26. Oktober
>>>>>>>>> 2016
>>>>>>>>> um
>>>>>>>>> 01:22
>>>>>>>>> geschrieben:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, 2016-10-25 at 14:52 -0700, Michael Zoran wrote:
>>>>>>>>>> On Tue, 2016-10-25 at 14:45 -0700, Michael Zoran
>>>>>>>>>> wrote:
>>>>>>>>>>> On Tue, 2016-10-25 at 23:20 +0200, Stefan Wahren
>>>>>>>>>>> wrote:
>>>>>>>>>>>> thanks, here are my steps to get the driver
>>>>>>>>>>>> working:
>>>>>>>>>>>>
>>>>>>>>>>>> * checkout Greg's staging-testing tree
>>>>>>>>>>>> * applied "staging: vc04_services: Remove
>>>>>>>>>>>> dmac_map_area"
>>>>>>>>>>>> * remove BROKEN in
>>>>>>>>>>>> drivers/staging/vc04_services/Kconfig
>>>>>>>>>>>> * add the following node to
>>>>>>>>>>>> arch/arm/boot/dts/bcm283x.dtsi
>>>>>>>>>>>>
>>>>>>>>>>>>                   vchiq: vchiq at 7e00b840 {
>>>>>>>>>>>>                           compatible =
>>>>>>>>>>>> "brcm,bcm2835-
>>>>>>>>>>>> vchiq";
>>>>>>>>>>>>                           reg = <0x7e00b840 0xf>;
>>>>>>>>>>>>                           interrupts = <0 2>;
>>>>>>>>>>>>                           cache-line-size = <32>;
>>>>>>>>>>>>                           firmware = <&firmware>;
>>>>>>>>>>>>                   };
>>>>>>>>>>>>
>>>>>>>>>>>> * build kernel and dtb for Raspberry Pi B
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>> BTW, the downstream tree has an override for the
>>>>>>>>>> cache-
>>>>>>>>>> line-
>>>>>>>>>> size.  Did
>>>>>>>>>> you perhaps try and add it to the device tree as
>>>>>>>>>> well?
>>>>>>>>>>
>>>>>>>>>> bcm2708_common.dtsi:
>>>>>>>>>>
>>>>>>>>>> 	__overrides__ {
>>>>>>>>>> 		cache_line_size = <&vchiq>, "cache-
>>>>>>>>>> line-
>>>>>>>>>> size:0";
>>>>>>>>>> 	};
>>>>>>>> i applied the second version of your patch "staging:
>>>>>>>> vc04_services:
>>>>>>>> Remove
>>>>>>>> dmac_map_area" and rebuild the whole driver. Now
>>>>>>>> vchiq_test
>>>>>>>> works
>>>>>>>> without any
>>>>>>>> issues. The __overrides__ wasn't necessary.
>>>>>>> I just tested an out of the box built of the staging-
>>>>>>> testing
>>>>>>> branch
>>>>>>> with all the latest patches.  For me, if I have the
>>>>>>> __overrides__ in
>>>>>>> the DT vchiq_test -f works fine.  If I remove it I get data
>>>>>>> corruption
>>>>>>> on my RPI 2.
>>>>>> Yes, Phil wrote that the RPI 2 & 3 requires a cache-line-size
>>>>>> of
>>>>>> 64.
>>>>>>
>>>>>> Currently i'm searching for a good solution but it seems
>>>>>> get_cpu_cacheinfo() which contains the cache line size isn't
>>>>>> implemented
>>>>>> for ARM (only for ARM64).
>>>>> Aren't you just looking for linux/cache.h's cache_line_size()?
>>>> cache_line_size() is a static value which means it won't be
>>>> possible
>>>> to
>>>> build a kernel image that works on both Pi1 and Pi2.
>>>> Has anyone tried to build such a kernel?
>>>> I tried to do it in the downstream repo and failed miserably.
>>>>
>>>> Noralf.
>>>>
>>> I tried and also failed.  I guess were both failures...
>>>
>>> One of the issues I hit was that the Pi2 requires symetric
>>> multiprocessing to be enable.  If it isn't included the Pi2 will
>>> hang
>>> very early in the boot sequence.  I found that out recently.
>>>
>>> Doesn't the PI Foundation distribute separate kernel binaries for
>>> v6
>>> and v7?
>>>
>> Yes they have separate binaries.
>>
>> I just did some work downstream that made it possible to build a
>> single
>> binary, but I got strange things happening when booting the image on
>> Pi2.
>>
>> If testing goes well, downstream will be based on ARCH_BCM2835 with
>> 4.9:
>> https://github.com/raspberrypi/linux/pull/1699
> Going back to the cache line size issue.
>
> I'm wondering if it's important to people to not have an override, if
> it would just make sense to use a reasonably large value in the DT that
> works on all the platforms.  Say 64 or 128.  I mean, I think the line
> size is sent over to the firmware in the driver init so making the
> number too big is just a performance issue and not a correctness issue,
> right?
The override is a non-issue - 2709 & 2710 already have different DTBs from 2708, so the correct value can be hard-coded. It was added to cover a transition period, but it can be removed now.