Drop ARCH_BCM2708 and ARCH_BCM2709 #1717

notro · 2016-11-12T21:38:26Z

I have taken another step towards mainline by removing ARCH_BCM2708/ARCH_BCM2709 completely, including 2708/2709/2710 in the compatible property.
I haven't done arm64, because I couldn't get bcmrpi3_defconfig right.

With this PR mkknlimg will tag the kernel for 2835, so if tagged it currently doesn't work.

Tested on Pi1 and Pi2, not Pi3.

It would have been nice if we could move to the mainline dts files as well: bcm283{5,6}-rpi-*.dts
The bootloader could (if not tagged), use 2835 dtb's if present, or fall back to 2708.

What do you think?

ghost · 2016-11-13T06:06:34Z

I combined my needed changes with your's and opened PR #1719.

The bcmrpi3_defconfig looks a bit different. The file from my branch should just replace the existing bcmrpi3_defconfig. I tested the combined changes on a RPI 3.

ghost · 2016-11-13T16:22:38Z

It looks like these PRs have been broken by the recent rebase. Since my changes are small and I can just submit as a separate PR, I'm going to just close my version.

PeterPablo · 2016-11-13T16:24:04Z

You should be able to rebase your branch and force push the changes. This way you wouldn't need to close this PR and open a new one.

popcornmix · 2016-11-13T16:24:36Z

Yes:

git pull --rebase
git push -f

should be all that is needed to fix a PR - no need to open a new one.

pelwell · 2016-11-15T15:08:27Z

drivers/usb/host/dwc_otg/dwc_otg_fiq_fsm.c

@@ -78,7 +78,7 @@ void notrace _fiq_print(enum fiq_debug_level dbg_lvl, volatile struct fiq_state
 * fiq_fsm_spin_lock() - ARMv6+ bare bones spinlock
 * Must be called with local interrupts and FIQ disabled.
 */
-#if defined(CONFIG_ARCH_BCM2709) && defined(CONFIG_SMP)
+#if defined(CONFIG_SMP)


There are now people building our downstream tree with random configurations and emailing us about build errors, so I'd be happier if we kept this implementation with its inline assembler conditional on CONFIG_ARCH_BCM2836.

pelwell · 2016-11-15T16:02:37Z

I've appended a commit that keeps mkknlimg working as it used to, i.e. detecting builds from the RPi tree as wanting the downstread bcm27xx DTBs.

Both are based on ARCH_BCM2835 so use that instead. Signed-off-by: Noralf Trønnes <[email protected]>

They are not necessary anymore since both are based on ARCH_BCM2835. Also use the compatible strings "brcm,bcm2835", "brcm,bcm2836" and "brcm,bcm2837". Signed-off-by: Noralf Trønnes <[email protected]>

With the death of ARCH_BCM2708 and ARCH_BCM2709, a new way is needed to determine if this is a "downstream" build that wants the firmware to load a bcm27xx .dtb. The vc_cma driver is used downstream but not upstream, making vc_cma_init a suitable predicate symbol.

notro · 2016-11-15T17:23:16Z

There is no CONFIG_ARCH_BCM2836 so I used CONFIG_ARCH_BCM2835.
Pi1 and Pi2 are differentiated with ARCH_MULTI_V6 and ARCH_MULTI_V7.

pelwell · 2016-11-15T17:40:40Z

One final thought, on something I should have spotted earlier - shouldn't bcm2836.dtsi set model = "BCM2836"; for completeness?

notro · 2016-11-15T19:04:56Z

The files that include it, overwrites the property, so that means patching an upstream file without actually changing anything.
But I agree that it should have been set for completness as you say, since bcm283x.dtsi sets model = "BCM2835";

What's your thoughts on my suggestion to switch to mainline dts files?
One of the main benefits that I see, is that the bootloader will treat downstream and upstream kernels the same. No need for u-boot or config.txt changes.
Another is that we will do it some day, so why not do it now, bundled with all the other changes we have made in this iteration?
It would put us in a place where the only difference between upstream and downstream are the downstream drivers (I know there are more differences but they are related to drivers and performance).

pelwell · 2016-11-15T19:44:11Z

What's your thoughts on my suggestion to switch to mainline dts files?

Not unless we modify the upstream DTBs to include FIQ-enhanced USB, the sdhost driver, DT parameters and the labels (and perhaps a few others).

notro · 2016-11-15T20:03:09Z

Yes of course, in that case we would patch the upstream files.

The dtsi files would be patched by applying the diff-files we have:
bcm283x.dtsi <<- bcm270x.dtsi
bcm2835.dtsi <<- bcm2708.dtsi
bcm2836.dtsi <<- bcm2709.dtsi
bcm2835-rpi.dtsi <<- bcm2708-rpi.dtsi

And the downstream dts files will in reality just change names and the upstream version dropped:
bcm2708-rpi-b-plus.dts -> bcm2835-rpi-b-plus.dts
etc.

pelwell · 2016-11-15T20:41:07Z

That plan is logically equivalent to deleting the upstream files. How is that a step forwards?

Apart from all of the downstream-only code, the primary difference to upstream is now the choice of DTB. By retaining the upstream files we make the differences plain, and by keeping them unmodified we reduce the merge conflicts when switching branches.

notro · 2016-11-15T21:16:43Z

All the PR's I have made was done with the purpose of closing the gap to mainline. To minimize the difference. DTB names are one obvious difference.
That being said, this issue is not important for me personally. The downside is ofc as you say patches and merge conflicts.
Since I'm not the one doing that, I thought I'd ask before actually implementing it in this PR.

Anyways, if this PR goes in, it will mean that we have fully moved to ARCH_BCM2835 which is what I have been working towards for some time now. At that's a milestone for me personally :-)

pelwell · 2016-11-16T08:59:52Z

Anyways, if this PR goes in, it will mean that we have fully moved to ARCH_BCM2835 which is what I have been working towards for some time now. At that's a milestone for me personally :-)

And for that, and all your contributions, you have our thanks.

I'm happy for use to merge this PR. @popcornmix?

kukabu · 2016-11-16T09:48:39Z

@notro I'm not sure but I think this patch breaks some overlays when compatible string contains "brcm,bcm27XX" only.

pelwell · 2016-11-16T09:52:26Z

Really? I'll let you into a secret - currently nothing is checking the top-level compatible string in overlays.

kukabu · 2016-11-16T09:59:31Z

It's ok then

notro · 2016-11-16T14:04:17Z

I have updated the Upstreaming page.

notro · 2016-11-16T17:04:41Z

I did a fresh 4.8 Pi1 build, and get this:

$ sudo vcdbg log msg
debug_sym: OpenVideoCoreMemoryFileWithOffset: newHandle->vcSymbolTableOffset (0x20 - 0x1ec00000) > 8MB

Unable to open videocore memory access: -5

$ dmesg
[    0.000000] OF: fdt:Machine model: Raspberry Pi Model B Plus Rev 1.2

[    0.650445] raspberrypi-firmware soc:firmware: Attached to firmware from 2016-11-16 12:24

[    1.856713] vc_mem_init: called
[    1.856731] vc-mem: phys_addr:0x00000000 mem_base=0x1ec00000 mem_size:0x20000000(512 MiB)

[   65.689449] vc_mem_open: called file = 0xd898d960
[   65.689511] vc_mem_ioctl: called file = 0xd898d960
[   65.689525] vc_mem_ioctl: VC_MEM_IOC_MEM_SIZE=536870912
[   65.689537] vc_mem_ioctl: file = 0xd898d960 returning 0
[   65.689547] vc_mem_ioctl: called file = 0xd898d960
[   65.689554] vc_mem_ioctl: VC_MEM_IOC_MEM_BASE=515899392
[   65.689561] vc_mem_ioctl: file = 0xd898d960 returning 0
[   65.689568] vc_mem_ioctl: called file = 0xd898d960
[   65.689574] vc_mem_ioctl: VC_MEM_IOC_MEM_LOAD=515899392
[   65.689581] vc_mem_ioctl: file = 0xd898d960 returning 0
[   65.689589] vc_mem_ioctl: called file = 0xd898d960
[   65.689598] vc_mem_ioctl: VC_MEM_IOC_MEM_PHYS_ADDR=0x  (null)
[   65.689606] vc_mem_ioctl: file = 0xd898d960 returning 0
[   65.690616] vc_mem_release: called file = 0xd898d960

notro · 2016-11-16T19:29:04Z

vchiq ping test doesn't work either:

$ vchiq_test -p
Ping test - service:echo, iters:1000, version 3
^C

Works fine on Pi2.

popcornmix · 2016-11-16T21:30:08Z

@notro you are correct. There is a bug in the code that affects Pi1 more than Pi2 (bonus points for explaining why Pi2 wasn't broken...)
I've pushed a fix. (for the vcdbg issue - that won't be affecting vchiq)

See: raspberrypi/linux#1717 (comment)

popcornmix · 2016-11-16T21:51:37Z

@pelwell vchiq_test -p seems to hang on 4.4 kernel too (on Pi1). Any ideas?

notro · 2016-11-16T22:45:52Z

Yes, vcdbg is working now.

bonus points for explaining why Pi2 wasn't broken...

I suck at this low level memory stuff. Some L2 cache alias thing maybe :-)

struct device is missing here:

static long vc_mem_copy()

    buf = dma_alloc_coherent(NULL, PAGE_ALIGN(size), &bus_addr,
                 GFP_ATOMIC);

This means that bus_addr contains the ARM physical address instead of the bus address on rpi-4.8.y.

popcornmix · 2016-11-17T16:50:08Z

Added.

pelwell · 2016-11-17T16:54:49Z

I'm still trying to get to the bottom of the vchiq_test -p stall. The main protocol handling looks OK, but something is going wrong within the client library.

There is a potential race between the callback thread and the main test thread when switching between asynchronous and synchronous modes. The vchi_clnt_callback function tries to rapidly swallow zero-length pong packets in asynchronous mode by looping in the callback until the message queue is emptied. There is a danger that if the main test thread switches to synchronous mode before the callback thread notices the queue is empty that the callback thread will erroneously receive the synchronous pong packet. Remove the race by forcing the callback thread to leave dequeue loop when a sync point (non-zero length reply) is received. See: raspberrypi/linux#1717

pelwell · 2016-11-17T17:40:19Z

Got it - there was a race between the VCHI callback thread and the main test thread. This race is much more likely to occur on an SoC with a single ARM core, i.e. a Pi1.

I've pushed a fix to the userland repo.

See: raspberrypi/linux#1722 kernel: fb: Use struct device for dma_alloc_coherent See: raspberrypi/linux#1717 (comment)

popcornmix force-pushed the rpi-4.8.y branch from 235e57a to 438f9d5 Compare November 13, 2016 16:12

pelwell reviewed Nov 15, 2016

View reviewed changes

popcornmix force-pushed the rpi-4.8.y branch 2 times, most recently from 14f6d97 to 875e8ce Compare November 15, 2016 17:16

notro and others added 3 commits November 15, 2016 18:18

BCM270x_DT: Drop using ARCH_BCM2708 and ARCH_BCM2709

b36810b

Both are based on ARCH_BCM2835 so use that instead. Signed-off-by: Noralf Trønnes <[email protected]>

Drop ARCH_BCM2708 and ARCH_BCM2709

c26b00c

They are not necessary anymore since both are based on ARCH_BCM2835. Also use the compatible strings "brcm,bcm2835", "brcm,bcm2836" and "brcm,bcm2837". Signed-off-by: Noralf Trønnes <[email protected]>

popcornmix merged commit 9fce336 into raspberrypi:rpi-4.8.y Nov 16, 2016

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Nov 16, 2016

fb: Use correct bus address for dest of dma memcpy

6aef553

See: raspberrypi/linux#1717 (comment)

popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Nov 16, 2016

fb: Use correct bus address for dest of dma memcpy

21c41dd

See: raspberrypi/linux#1717 (comment)

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Nov 18, 2016

kernel: Merge drm-vc4-next

70068cf

See: raspberrypi/linux#1722 kernel: fb: Use struct device for dma_alloc_coherent See: raspberrypi/linux#1717 (comment)

popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Nov 18, 2016

kernel: Merge drm-vc4-next

c618ec2

See: raspberrypi/linux#1722 kernel: fb: Use struct device for dma_alloc_coherent See: raspberrypi/linux#1717 (comment)

MilhouseVH mentioned this pull request Nov 19, 2016

linux: update to linux-4.8.9 LibreELEC/LibreELEC.tv#956

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop ARCH_BCM2708 and ARCH_BCM2709 #1717

Drop ARCH_BCM2708 and ARCH_BCM2709 #1717

notro commented Nov 12, 2016

ghost commented Nov 13, 2016

ghost commented Nov 13, 2016

PeterPablo commented Nov 13, 2016

popcornmix commented Nov 13, 2016

pelwell Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 16, 2016

kukabu commented Nov 16, 2016

pelwell commented Nov 16, 2016

kukabu commented Nov 16, 2016

notro commented Nov 16, 2016

notro commented Nov 16, 2016 •

edited

Loading

notro commented Nov 16, 2016

popcornmix commented Nov 16, 2016 •

edited

Loading

popcornmix commented Nov 16, 2016 •

edited

Loading

notro commented Nov 16, 2016

popcornmix commented Nov 17, 2016

pelwell commented Nov 17, 2016

pelwell commented Nov 17, 2016

Drop ARCH_BCM2708 and ARCH_BCM2709 #1717

Drop ARCH_BCM2708 and ARCH_BCM2709 #1717

Conversation

notro commented Nov 12, 2016

ghost commented Nov 13, 2016

ghost commented Nov 13, 2016

PeterPablo commented Nov 13, 2016

popcornmix commented Nov 13, 2016

pelwell Nov 15, 2016

Choose a reason for hiding this comment

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 15, 2016

notro commented Nov 15, 2016

pelwell commented Nov 16, 2016

kukabu commented Nov 16, 2016

pelwell commented Nov 16, 2016

kukabu commented Nov 16, 2016

notro commented Nov 16, 2016

notro commented Nov 16, 2016 • edited Loading

notro commented Nov 16, 2016

popcornmix commented Nov 16, 2016 • edited Loading

popcornmix commented Nov 16, 2016 • edited Loading

notro commented Nov 16, 2016

popcornmix commented Nov 17, 2016

pelwell commented Nov 17, 2016

pelwell commented Nov 17, 2016

notro commented Nov 16, 2016 •

edited

Loading

popcornmix commented Nov 16, 2016 •

edited

Loading

popcornmix commented Nov 16, 2016 •

edited

Loading