Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bcm270x: Convert to ARCH_MULTIPLATFORM #1699

Merged
merged 10 commits into from Oct 28, 2016

Conversation

notro
Copy link
Contributor

@notro notro commented Oct 27, 2016

This PR converts ARCH_BCM2708 and ARCH_BCM2709 to ARCH_MULTIPLATFORM.

Both ARCH_BCM2708 and ARCH_BCM2709 are now based on ARCH_BCM2835, removing the need for the mach-bcm2708 and mach-bcm2709 directories. In addition to that it removes the need to patch many Kconfig files just to add depends ARCH_BCM2708 || ARCH_BCM2708 || ....

Using ARCH_BCM2835 means that the 2835 dtb's are also built, but they won't work with this config.
And scripts/mkknlimg will add the tag: 283x: y

Tested on Pi1 and Pi2.

@popcornmix please check with the GPU debugger to make sure that this doesn't give L2 bad address exception like it did when I tried this last year: #1178 (comment)

Here's the .config diff introduced by this PR:

Of all the PR's I have submitted, this is the one I have the least confidence in. I have had so much problems and lockups working with this ARCH_MULTIPLATFORM convertion, both now and last year, that I started to think that I just couldn't do it.
That being said, I haven't had any issues with this particular PR, just with all my efforts to try and build a stable kernel that can boot on both Pi1 and Pi2.

This isn't used anymore now that the watchdog driver does restart/poweroff.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
NEED_MACH_IO_H isn't necessary since we don't have
PC card/PCI/ISA IO space.
The __io macro is only used in the {in,out}[bwl] macros.

arch/arm/include/asm/io.h will give these defaults now:

define __io(a)		__typesafe_io((a) & IO_SPACE_LIMIT)
define IO_SPACE_LIMIT ((resource_size_t)0)

This is the same as ARCH_BCM2835.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
This makes it possible to get the bus address from Device Tree.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
This makes it possible to get the bus address from Device Tree.
At the same time move the call to log_init() after getting the clock
to avoid allocating twice due to deferred probing.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Drop NEED_MACH_MEMORY_H and use dma-ranges from the Device Tree to
get the bus address, like ARCH_BCM2835 does.

This means that we go from this:

arch/arm/mach-bcm270x/include/mach/memory.h:

define __virt_to_bus(x)    ((x) + (BUS_OFFSET - PAGE_OFFSET))
define __bus_to_virt(x)    ((x) - (BUS_OFFSET - PAGE_OFFSET))
define __pfn_to_bus(x)     (__pfn_to_phys(x) + BUS_OFFSET)
define __bus_to_pfn(x)     __phys_to_pfn((x) - BUS_OFFSET

To this:

arch/arm/include/asm/memory.h:

define __virt_to_bus   __virt_to_phys
define __bus_to_virt   __phys_to_virt
define __pfn_to_bus(x) __pfn_to_phys(x)
define __bus_to_pfn(x) __phys_to_pfn(x)

Drivers now have to use the DMA API to get to the bus address.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
This file doesn't build anymore and isn't used so remove it.
It was added as part of my ARCH_BCM2835 work last year, but the future
didn't pan out as expected.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Convert to multi platform and base it on ARCH_BCM2835.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Convert to multi platform and base it on ARCH_BCM2835.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
ARCH_BCM2708 and ARCH_BCM2709 selects ARCH_BCM2835 now, so the
dependencies can be simplified.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
@notro
Copy link
Contributor Author

notro commented Oct 27, 2016

Unified kernel image

It is now possible to build a kernel image that can/should boot on Pi1 and Pi2.
I based the kernel on bcm2709_defconfig with these changes:

CONFIG_ARCH_MULTI_V6=y
CONFIG_ARCH_BCM2708=y
# CONFIG_MACH_BCM2708 is not set
# CONFIG_NEON is not set

It seems to work on Pi1 except that vchiq_test -p doesn't work, but vchiq_test -f does.

On Pi2 it isn't good. Tagging the kernel from a network driver always fails like this:

$ mount -t nfs 192.168.10.92:/home/pi /home/pi/work -o rw
$ ~/work/notro-raspberrypi-linux/notro-raspberrypi-linux/scripts/mkknlimg --dtok ~/work/notro-raspberrypi-linux/notro-raspberrypi-linux/arch/arm/boot/zImage ~/kernel.img
Illegal instruction

And every other boot drops me into emergency console (eventually):

[  *** ] (1 of 1) A start job is running for dev-mmcblk0p1.de... 1s / 1min 30s)
<...>
[ TIME ] Timed out waiting for device dev-mmcblk0p1.device.
[DEPEND] Dependency failed for /boot.
[DEPEND] Dependency failed for Local File Systems.
[DEPEND] Dependency failed for File System Check on /dev/mmcblk0p1.

Welcome to emergesulogin: root account is locked, starting shell
root@raspberrypi:~#

I had this problem last year also when I tried this, and I haven't figured out the cause.
I have tried switching to dwc2 and disabling UACCESS_WITH_MEMCPY and BCM2708_VCHIQ.

Oh well, I wasn't able to make that unified image, but hopefully this PR survives review and testing, with the result that downstream for all pratical purposes will be using ARCH_BCM2835!

@popcornmix
Copy link
Collaborator

Looks good.
@P33M dwc_otg commit looks okay to me - any objections?

@pelwell
Copy link
Contributor

pelwell commented Oct 27, 2016

One interesting aspect of having 270x and 283x support in the same image is that the mkknlimg trailer (if you add one) sets both flags. The firmware currently sees the 283x flag and chooses to load bcm283x dtb files. If they aren't there, as is the case with my (remote) Pi, it won't boot (not falling back to the bcm270x variants). Omitting the trailer should get it working again.

@notro
Copy link
Contributor Author

notro commented Oct 27, 2016

I see now that there are two flags: 283x and 283X. But in my case it loads the right dtb:

$ ~/work/notro-raspberrypi-linux/notro-raspberrypi-linux/scripts/knlinfo /boot/kernel.img
Kernel trailer found at 4341168/0x423db0:
  KVer: "Linux version 4.8.4+ (pi@raspi2) (gcc version 4.8.3 20140106 (prerelease) (crosstool-NG linaro-1.13.1-4.8-2014.01 - Linaro GCC 2013.11) ) #1 Tue Oct 25 19:24:32 CEST 2016"
  DTOK: y
  DDTK: y
  270X: y
  283X: y
  283x: n

$ sudo vcdbg log msg
001413.403: Loading 'kernel.img' to 0x8000 size 0x423eb4
001417.054: Kernel trailer DTOK property says yes
001417.068: Kernel trailer DDTK property says yes
001419.353: Loading 'bcm2708-rpi-b-plus.dtb' to 0x42beb4 size 0x31a7

@pelwell
Copy link
Contributor

pelwell commented Oct 28, 2016

That was a false alarm - "283x" is the old single-platform tag, and if set will result in the bcm283x DTBs being loaded . "283X" says the image includes upstream support, but not to use it automatically because it also includes "270X" support ("283x" means "283X && !270X").

On a Pi 2 I get complaints about timer interrupts:

[    0.000000] Hierarchical RCU implementation.
[    0.000000]  Build-time adjustment of leaf fanout to 32.
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] arm_arch_timer: WARNING: Invalid trigger for IRQ16, assuming level low
[    0.000000] arm_arch_timer: WARNING: Please fix your firmware
[    0.000000] arm_arch_timer: WARNING: Invalid trigger for IRQ17, assuming level low
[    0.000000] arm_arch_timer: WARNING: Please fix your firmware
[    0.000000] arm_arch_timer: Architected cp15 timer(s) running at 19.20MHz (phys).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x46d987e47, max_idle_ns: 440795202767 ns
[    0.000010] sched_clock: 56 bits at 19MHz, resolution 52ns, wraps every 4398046511078ns
[    0.000030] Switching to timer-based delay loop, resolution 52ns
[    0.000377] Console: colour dummy device 80x30
[    0.001872] console [tty1] enabled
[    0.001927] Calibrating delay loop (skipped), value calculated using timer frequency.. 38.40 BogoMIPS (lpj=192000)
[    0.002004] pid_max: default: 32768 minimum: 301
[    0.002419] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[    0.002473] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes)
[    0.003762] Disabling cpuset control group subsystem
[    0.004028] CPU: Testing write buffer coherency: ok
[    0.004124] ftrace: allocating 21951 entries in 65 pages
[    0.058851] CPU0: update cpu_capacity 1024
[    0.058932] CPU0: thread -1, cpu 0, socket 15, mpidr 80000f00
[    0.059031] Setting up static identity map for 0x100000 - 0x100034
[    0.062111] arm_arch_timer: WARNING: Invalid trigger for IRQ16, assuming level low
[    0.062117] arm_arch_timer: WARNING: Please fix your firmware
[    0.062129] arm_arch_timer: WARNING: Invalid trigger for IRQ17, assuming level low
[    0.062133] arm_arch_timer: WARNING: Please fix your firmware
[    0.062151] CPU1: update cpu_capacity 1024
[    0.062159] CPU1: thread -1, cpu 1, socket 15, mpidr 80000f01
[    0.063174] arm_arch_timer: WARNING: Invalid trigger for IRQ16, assuming level low
[    0.063179] arm_arch_timer: WARNING: Please fix your firmware
[    0.063186] arm_arch_timer: WARNING: Invalid trigger for IRQ17, assuming level low
[    0.063189] arm_arch_timer: WARNING: Please fix your firmware
[    0.063203] CPU2: update cpu_capacity 1024
[    0.063210] CPU2: thread -1, cpu 2, socket 15, mpidr 80000f02
[    0.064127] arm_arch_timer: WARNING: Invalid trigger for IRQ16, assuming level low
[    0.064132] arm_arch_timer: WARNING: Please fix your firmware
[    0.064139] arm_arch_timer: WARNING: Invalid trigger for IRQ17, assuming level low
[    0.064142] arm_arch_timer: WARNING: Please fix your firmware
[    0.064156] CPU3: update cpu_capacity 1024
[    0.064164] CPU3: thread -1, cpu 3, socket 15, mpidr 80000f03
[    0.064273] Brought up 4 CPUs
[    0.064880] SMP: Total of 4 processors activated (153.60 BogoMIPS).

My Pi was failing to boot because it's a Pi3, and both USB and the UART seem to be hosed with this build. More digging required.

@notro
Copy link
Contributor Author

notro commented Oct 28, 2016

#1669 has info about the warnings.

@pelwell
Copy link
Contributor

pelwell commented Oct 28, 2016

vchiq isn't connecting. Still digging.

@popcornmix
Copy link
Collaborator

I have the same problem. Build with bcm2709_defconfig works on Pi2.
On Pi3 usb doesn't work. Hub is detected but devices aren't:
https://www.dropbox.com/s/ud8g277qnu41ktq/IMG_1718.JPG?dl=0

@P33M
Copy link
Contributor

P33M commented Oct 28, 2016

I see no issues with the dwc_otg changes.

See: raspberrypi#1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
@pelwell
Copy link
Contributor

pelwell commented Oct 28, 2016

The problem was caused by the lack of a dma-ranges property in the bcm2710 dtb. I've added a patch that includes it, and the reg property for the intc node which was also missing.

@@ -160,6 +160,20 @@ config ARCH_BCM2835
This enables support for the Broadcom BCM2835 and BCM2836 SoCs.
This SoC is used in the Raspberry Pi and Roku 2 devices.

config MACH_BCM2708
bool "Enable optimized __copy_to_user and __copy_from_user"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text doesn't seem to match the config name. A c/p error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was tired and didn't know what to call it, so I just kept the name.
The only place CONFIG_MACH_BCM2708 is used is in the optimization code: 5fb515e

@notro
Copy link
Contributor Author

notro commented Oct 28, 2016

Thanks for fixing that up @pelwell, I've been down so many rabbit holes with this, that I forgot about 2710.

@popcornmix
Copy link
Collaborator

FYI I've updated 4.9 tree with these changes, rebasing them into original commits.
Looks quite a lot cleaner. :-)

@popcornmix
Copy link
Collaborator

@pelwell okay to merge?

@notro
Copy link
Contributor Author

notro commented Oct 28, 2016

Yes the Main bcm2708/bcm2709 linux port commit is significantly smaller now!

I have some i2c-bcm2835 patches that awaits i2c maintainer review. With those in place, I hope we can switch to use i2c-bcm2835 by default. AFAIK that's the last of the mainline drivers which has a downstream 2708 version.

@popcornmix
Copy link
Collaborator

@notro great - we always look forward to your PR's - keep them coming!

@ghost
Copy link

ghost commented Oct 28, 2016

As a Raspberry Pi enthusiast I love threads like this, despite understanding only the most general aspects. Thanks to all involved, and please keep up the great work.

@pelwell
Copy link
Contributor

pelwell commented Oct 28, 2016

@notro Don't forget the bcm2708-usb - we'll be holding on to that for a while, I think. And thank you for another significant contribution.

Yes - merge away.

@popcornmix popcornmix merged commit 7ede3d5 into raspberrypi:rpi-4.8.y Oct 28, 2016
@notro
Copy link
Contributor Author

notro commented Oct 28, 2016

Yes you're right of course, I forgot that one. I had a look at the dwc_otg patch and it touches core files for the fiq support, so it will probably stay a long time. IIRC there was a comment in an arm64 thread about dwc_otg outperforming dwc2 even without fiq.

@clivem
Copy link

clivem commented Oct 30, 2016

I chuckled earlier today, when I came across this from Dec 2012.... #170

@clivem
Copy link

clivem commented Oct 30, 2016

BTW, I've been using "coherent_pool=1M" on the 2x Zero's, 4x 2B's and 3x 3B's, that I've now got running 4.8.y HEAD with ARCH_MULTIPLATFORM config. Perhaps that is a sensible compromise between 256k and 4M?

@popcornmix
Copy link
Collaborator

@clivem #170 was before 67fd2c5 so coherent memory requirements are less now then they once were.
However I agree it needs increasing, or dwc_otg needs to reduce its requirement. We'll make a decision about this tomorrow.

@clivem
Copy link

clivem commented Oct 30, 2016

@popcornmix Yes.... Understand..... I just chuckled at the headline, "256 KiB atomic DMA coherent pool is too small!", 4 years ago, rather than what was causing it to be too small or where it fits in terms of timelines of what happened, when, and other commits.

@ghost
Copy link

ghost commented Oct 30, 2016

OK, silly question here. I'm new to this whole thing....

Isn't ALL the ram on the RPI DMA coherent? It's just a matter of getting the mappings correct, right?

@ghost
Copy link

ghost commented Oct 30, 2016

I mean, couldn't the memory be allocated with kmalloc or friends, then mapped into DMA space with dma_map_single/dma_map_sg/friends when it's actually sent to the USB controller?

@clivem
Copy link

clivem commented Oct 30, 2016

I'll leave the technical discussion to the experts.....
I've now migrated 18 instances of various Pi hardware to 4.8.y HEAD with MULTIPLATFORM config and "coherent_pool=1M". I'll keep an eye on the logs.......

@P33M
Copy link
Contributor

P33M commented Oct 30, 2016

Coherent mappings require no explicit cache flushing between hardware changing values in RAM and the CPU reading them and vice versa. Bits are set in the page entries that effectively bypass cache on these pages.

dwc_otg is suboptimal in that regard - an unaligned memcpy from coherent memory is going to be a bit slow, but the majority of transfers that fall into the "unaligned" bucket are of the order of 16 bytes.

dwc2 upstream allocates an aligned buffer and overwrites urb->transfer_buffer to allow the usb-related DMA map-unmap functions to work, but frees/allocates on submission and retire of each URB with unaligned buffers. I don't know which would be slower on a Pi 1 - the coherent unaligned memcpy or the allocate/deallocate for each URB.

@popcornmix
Copy link
Collaborator

@clivem can you identify the USB device that results in the dwc_otg allocations?
Either unplug one USB device at a time and see if number of caller=__DWC_DMA_ALLOC_ATOMIC lines reduces, or add a WARN to the current logging to get a backtrace. Interested to know if you have a single device that results in 4 x 64K allocations, or you have a number of devices with one or two each.

@clivem
Copy link

clivem commented Oct 31, 2016

@popcornmix Only USB device plugged into that board (which generated the earlier log I posted) is an Edimax wifi dongle using the mt7601u kernel module.

Feb 11 16:28:02 MamboLSPlus2B1 kernel: usb 1-1.5: new high-speed USB device number 4 using dwc_otg
Feb 11 16:28:02 MamboLSPlus2B1 kernel: usb 1-1.5: New USB device found, idVendor=7392, idProduct=7710
Feb 11 16:28:02 MamboLSPlus2B1 kernel: usb 1-1.5: New USB device strings: Mfr=1, Product=2, SerialNumber=3
Feb 11 16:28:09 MamboLSPlus2B1 kernel: usb 1-1.5: reset high-speed USB device number 4 using dwc_otg
Feb 11 16:28:09 MamboLSPlus2B1 kernel: mt7601u 1-1.5:1.0: ASIC revision: 76010001 MAC revision: 76010500
Feb 11 16:28:09 MamboLSPlus2B1 kernel: mt7601u 1-1.5:1.0: Firmware Version: 0.1.00 Build: 7640 Build time: 201302052146____
Feb 11 16:28:09 MamboLSPlus2B1 kernel: mt7601u 1-1.5:1.0: Warning: unsupported EEPROM version 0d
Feb 11 16:28:09 MamboLSPlus2B1 kernel: mt7601u 1-1.5:1.0: EEPROM ver:0d fae:00
Feb 11 16:28:09 MamboLSPlus2B1 kernel: mt7601u 1-1.5:1.0: EEPROM country region 01 (channels 1-13)
Feb 11 16:28:10 MamboLSPlus2B1 kernel: usbcore: registered new interface driver mt7601u

@popcornmix
Copy link
Collaborator

Okay, I've bumped default coherent size to 1M and added a WARN_ON to failure path when allocating a coherent buffer from dwc_otg, so we might catch users with multiple USB devices that use unaligned transfers.

@clivem
Copy link

clivem commented Oct 31, 2016

@popcornmix Cool! Any chance of rebase 4.8.4 -> 4.8.5 before I run another local build. ;)

popcornmix pushed a commit that referenced this pull request Oct 31, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
@popcornmix
Copy link
Collaborator

Rebased. 4.8.6 now.

@clivem
Copy link

clivem commented Oct 31, 2016

@popcornmix Ta.

popcornmix pushed a commit that referenced this pull request Nov 13, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Nov 15, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Nov 19, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Nov 22, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Nov 27, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Dec 2, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Dec 9, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Dec 15, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Dec 19, 2016
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
popcornmix pushed a commit that referenced this pull request Jan 8, 2017
See: #1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
mseaster-wr pushed a commit to WindRiver-Labs/kernel-4.8.x that referenced this pull request Jul 13, 2017
See: raspberrypi/linux#1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
[Xulin: Original patch taken from
https://github.com/raspberrypi/linux.git branch rpi-4.8.y]
Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
mseaster-wr pushed a commit to WindRiver-Labs/kernel-4.8.x that referenced this pull request Jul 13, 2017
See: raspberrypi/linux#1699

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
[Xulin: Original patch taken from
https://github.com/raspberrypi/linux.git branch rpi-4.8.y]
Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants