Functional testing of mainline vchiq driver

[Michael Zoran]

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?