| Message ID | 20190604173223.4229-1-nsaenzjulienne@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | cpufreq support for the Raspberry Pi | expand |
Hi Nicolas, Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > Hi all, > this series aims at adding cpufreq support to the Raspberry Pi family of > boards. > > The previous revision can be found at: https://lkml.org/lkml/2019/5/20/431 > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > clk driver that operates it over RPi's firmware interface[1]. We are > forced to do so as the firmware 'owns' the pll and we're not allowed to > change through the register interface directly as we might race with the > over-temperature and under-voltage protections provided by the firmware. it would be nice to preserve such design decision in the driver as a comment, because the cover letter usually get lost. > > Next it creates a minimal cpufreq driver that populates the CPU's opp > table, and registers cpufreq-dt. Which is needed as the firmware > controls the max and min frequencies available. I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and manually enable this drivers. During boot with Raspbian rootfs i'm getting the following: [ 1.177009] cpu cpu0: failed to get clock: -2 [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 [ 1.192117] sdhci: Secure Digital Host Controller Interface driver [ 1.198725] sdhci: Copyright(c) Pierre Ossman [ 1.207005] Synopsys Designware Multimedia Card Interface Driver [ 1.319936] sdhost-bcm2835 3f202000.mmc: loaded - DMA enabled (>1) [ 1.326641] sdhci-pltfm: SDHCI platform and OF driver helper [ 1.336568] ledtrig-cpu: registered to indicate activity on CPUs [ 1.343713] usbcore: registered new interface driver usbhid [ 1.350275] usbhid: USB HID core driver [ 1.357639] bcm2835-mbox 3f00b880.mailbox: mailbox enabled [ 1.367490] NET: Registered protocol family 10 [ 1.375013] Segment Routing with IPv6 [ 1.381696] sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver [ 1.388980] NET: Registered protocol family 17 [ 1.395624] can: controller area network core (rev 20170425 abi 9) [ 1.402358] NET: Registered protocol family 29 [ 1.408997] can: raw protocol (rev 20170425) [ 1.415599] can: broadcast manager protocol (rev 20170425 t) [ 1.422219] can: netlink gateway (rev 20170425) max_hops=1 [ 1.429369] Key type dns_resolver registered [ 1.437190] Registering SWP/SWPB emulation handler [ 1.444443] Loading compiled-in X.509 certificates [ 1.455693] 3f201000.serial: ttyAMA0 at MMIO 0x3f201000 (irq = 81, base_baud = 0) is a PL011 rev2 [ 1.462768] serial serial0: tty port ttyAMA0 registered [ 1.478755] mmc0: host does not support reading read-only switch, assuming write-enable [ 1.488792] mmc0: new high speed SDHC card at address 0007 [ 1.495766] raspberrypi-firmware soc:firmware: Attached to firmware from 2019-03-27 15:45 [ 1.496862] mmcblk0: mmc0:0007 SDCIT 14.6 GiB [ 1.512768] raspberrypi-clk raspberrypi-clk: CPU frequency range: min 600000000, max 1400000000 [ 1.513012] mmcblk0: p1 p2 [ 1.558085] dwc2 3f980000.usb: 3f980000.usb supply vusb_d not found, using dummy regulator [ 1.565355] dwc2 3f980000.usb: 3f980000.usb supply vusb_a not found, using dummy regulator [ 1.623246] dwc2 3f980000.usb: DWC OTG Controller [ 1.630318] dwc2 3f980000.usb: new USB bus registered, assigned bus number 1 [ 1.637439] dwc2 3f980000.usb: irq 33, io mem 0x3f980000 [ 1.645268] hub 1-0:1.0: USB hub found [ 1.652317] hub 1-0:1.0: 1 port detected [ 1.665867] sdhci-iproc 3f300000.sdhci: allocated mmc-pwrseq [ 1.704788] mmc1: SDHCI controller on 3f300000.sdhci [3f300000.sdhci] using PIO [ 1.717694] hctosys: unable to open rtc device (rtc0) [ 1.724967] sysfs: cannot create duplicate filename '/devices/platform/cpufreq-dt' [ 1.732120] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc1-00004-g5aa6d98-dirty #2 [ 1.739288] Hardware name: BCM2835 [ 1.746421] [<c0312304>] (unwind_backtrace) from [<c030cc08>] (show_stack+0x10/0x14) [ 1.753636] [<c030cc08>] (show_stack) from [<c0e7d358>] (dump_stack+0xb4/0xc8) [ 1.760840] [<c0e7d358>] (dump_stack) from [<c0503b64>] (sysfs_warn_dup+0x58/0x64) [ 1.768105] [<c0503b64>] (sysfs_warn_dup) from [<c0503c8c>] (sysfs_create_dir_ns+0xd8/0xe8) [ 1.775481] [<c0503c8c>] (sysfs_create_dir_ns) from [<c0e82520>] (kobject_add_internal+0xb0/0x2fc) [ 1.782958] [<c0e82520>] (kobject_add_internal) from [<c0e827c8>] (kobject_add+0x5c/0xc0) [ 1.790534] [<c0e827c8>] (kobject_add) from [<c096b1cc>] (device_add+0xf8/0x608) [ 1.798180] [<c096b1cc>] (device_add) from [<c0971098>] (platform_device_add+0x110/0x214) [ 1.805945] [<c0971098>] (platform_device_add) from [<c0971ae4>] (platform_device_register_full+0x130/0x148) [ 1.813866] [<c0971ae4>] (platform_device_register_full) from [<c15a7a30>] (raspberrypi_cpufreq_driver_init+0x128/0x178) [ 1.821916] [<c15a7a30>] (raspberrypi_cpufreq_driver_init) from [<c0302eec>] (do_one_initcall+0x54/0x21c) [ 1.830099] [<c0302eec>] (do_one_initcall) from [<c15010f8>] (kernel_init_freeable+0x244/0x2e0) [ 1.838312] [<c15010f8>] (kernel_init_freeable) from [<c0e949d4>] (kernel_init+0x8/0x10c) [ 1.846541] [<c0e949d4>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c) [ 1.854783] Exception stack(0xea89dfb0 to 0xea89dff8) [ 1.863036] dfa0: 00000000 00000000 00000000 00000000 [ 1.871450] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 1.879860] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 1.888251] kobject_add_internal failed for cpufreq-dt with -EEXIST, don't try to register things with the same name in the same directory. [ 1.896910] cpu cpu0: Failed to create platform device, -17 > > This was tested on a RPi3b+ and RPI2b which are the boards I have access > to. Until this is tested broadly the cpufreq driver takes care of > filtering out the rest of boards. Unfortunately this makes it harder to test on other boards. So i welcome your decision to remove it. > > That's all, > kind regards, > Nicolas > > [1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface > > --- > > Changes since RFC: > - Addressed Viresh's comments in cpufreq driver > - Addressed Stefan's comments in both cpufreq & clk drivers Just a note for the future, this make it hard for other reviewers to follow. I don't really consist of the credits, it is more important to mention what has changed. But it's not necessary to mention every single change.
Hi Stefan, thanks for the review, I took note of your code comments. On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > Hi all, > > this series aims at adding cpufreq support to the Raspberry Pi family of > > boards. > > > > The previous revision can be found at: https://lkml.org/lkml/2019/5/20/431 > > > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > > clk driver that operates it over RPi's firmware interface[1]. We are > > forced to do so as the firmware 'owns' the pll and we're not allowed to > > change through the register interface directly as we might race with the > > over-temperature and under-voltage protections provided by the firmware. > it would be nice to preserve such design decision in the driver as a > comment, because the cover letter usually get lost. > > Next it creates a minimal cpufreq driver that populates the CPU's opp > > table, and registers cpufreq-dt. Which is needed as the firmware > > controls the max and min frequencies available. > > I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and > manually enable this drivers. During boot with Raspbian rootfs i'm > getting the following: > > [ 1.177009] cpu cpu0: failed to get clock: -2 > [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 This is surprising, who could be creating a platform_device for cpufreq-dt apart from raspberrypi-cpufreq? Just to make things clear, you're using the device tree from v5.2-rc1 (as opposed to the Raspbian one)? > [ 1.192117] sdhci: Secure Digital Host Controller Interface driver > [ 1.198725] sdhci: Copyright(c) Pierre Ossman > [ 1.207005] Synopsys Designware Multimedia Card Interface Driver > [ 1.319936] sdhost-bcm2835 3f202000.mmc: loaded - DMA enabled (>1) > [ 1.326641] sdhci-pltfm: SDHCI platform and OF driver helper > [ 1.336568] ledtrig-cpu: registered to indicate activity on CPUs > [ 1.343713] usbcore: registered new interface driver usbhid > [ 1.350275] usbhid: USB HID core driver > [ 1.357639] bcm2835-mbox 3f00b880.mailbox: mailbox enabled > [ 1.367490] NET: Registered protocol family 10 > [ 1.375013] Segment Routing with IPv6 > [ 1.381696] sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver > [ 1.388980] NET: Registered protocol family 17 > [ 1.395624] can: controller area network core (rev 20170425 abi 9) > [ 1.402358] NET: Registered protocol family 29 > [ 1.408997] can: raw protocol (rev 20170425) > [ 1.415599] can: broadcast manager protocol (rev 20170425 t) > [ 1.422219] can: netlink gateway (rev 20170425) max_hops=1 > [ 1.429369] Key type dns_resolver registered > [ 1.437190] Registering SWP/SWPB emulation handler > [ 1.444443] Loading compiled-in X.509 certificates > [ 1.455693] 3f201000.serial: ttyAMA0 at MMIO 0x3f201000 (irq = 81, > base_baud = 0) is a PL011 rev2 > [ 1.462768] serial serial0: tty port ttyAMA0 registered > [ 1.478755] mmc0: host does not support reading read-only switch, > assuming write-enable > [ 1.488792] mmc0: new high speed SDHC card at address 0007 > [ 1.495766] raspberrypi-firmware soc:firmware: Attached to firmware > from 2019-03-27 15:45 > [ 1.496862] mmcblk0: mmc0:0007 SDCIT 14.6 GiB > [ 1.512768] raspberrypi-clk raspberrypi-clk: CPU frequency range: min > 600000000, max 1400000000 > [ 1.513012] mmcblk0: p1 p2 > [ 1.558085] dwc2 3f980000.usb: 3f980000.usb supply vusb_d not found, > using dummy regulator > [ 1.565355] dwc2 3f980000.usb: 3f980000.usb supply vusb_a not found, > using dummy regulator > [ 1.623246] dwc2 3f980000.usb: DWC OTG Controller > [ 1.630318] dwc2 3f980000.usb: new USB bus registered, assigned bus > number 1 > [ 1.637439] dwc2 3f980000.usb: irq 33, io mem 0x3f980000 > [ 1.645268] hub 1-0:1.0: USB hub found > [ 1.652317] hub 1-0:1.0: 1 port detected > [ 1.665867] sdhci-iproc 3f300000.sdhci: allocated mmc-pwrseq > [ 1.704788] mmc1: SDHCI controller on 3f300000.sdhci [3f300000.sdhci] > using PIO > [ 1.717694] hctosys: unable to open rtc device (rtc0) > [ 1.724967] sysfs: cannot create duplicate filename > '/devices/platform/cpufreq-dt' For the record, this is raspberrypi-cpufreq creating the platform device for cpufreq-dt. > [ 1.732120] CPU: 1 PID: 1 Comm: swapper/0 Not tainted > 5.2.0-rc1-00004-g5aa6d98-dirty #2 > [ 1.739288] Hardware name: BCM2835 > [ 1.746421] [<c0312304>] (unwind_backtrace) from [<c030cc08>] > (show_stack+0x10/0x14) > [ 1.753636] [<c030cc08>] (show_stack) from [<c0e7d358>] > (dump_stack+0xb4/0xc8) > [ 1.760840] [<c0e7d358>] (dump_stack) from [<c0503b64>] > (sysfs_warn_dup+0x58/0x64) > [ 1.768105] [<c0503b64>] (sysfs_warn_dup) from [<c0503c8c>] > (sysfs_create_dir_ns+0xd8/0xe8) > [ 1.775481] [<c0503c8c>] (sysfs_create_dir_ns) from [<c0e82520>] > (kobject_add_internal+0xb0/0x2fc) > [ 1.782958] [<c0e82520>] (kobject_add_internal) from [<c0e827c8>] > (kobject_add+0x5c/0xc0) > [ 1.790534] [<c0e827c8>] (kobject_add) from [<c096b1cc>] > (device_add+0xf8/0x608) > [ 1.798180] [<c096b1cc>] (device_add) from [<c0971098>] > (platform_device_add+0x110/0x214) > [ 1.805945] [<c0971098>] (platform_device_add) from [<c0971ae4>] > (platform_device_register_full+0x130/0x148) > [ 1.813866] [<c0971ae4>] (platform_device_register_full) from > [<c15a7a30>] (raspberrypi_cpufreq_driver_init+0x128/0x178) > [ 1.821916] [<c15a7a30>] (raspberrypi_cpufreq_driver_init) from > [<c0302eec>] (do_one_initcall+0x54/0x21c) > [ 1.830099] [<c0302eec>] (do_one_initcall) from [<c15010f8>] > (kernel_init_freeable+0x244/0x2e0) > [ 1.838312] [<c15010f8>] (kernel_init_freeable) from [<c0e949d4>] > (kernel_init+0x8/0x10c) > [ 1.846541] [<c0e949d4>] (kernel_init) from [<c03010e8>] > (ret_from_fork+0x14/0x2c) > [ 1.854783] Exception stack(0xea89dfb0 to 0xea89dff8) > [ 1.863036] dfa0: 00000000 > 00000000 00000000 00000000 > [ 1.871450] dfc0: 00000000 00000000 00000000 00000000 00000000 > 00000000 00000000 00000000 > [ 1.879860] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ 1.888251] kobject_add_internal failed for cpufreq-dt with -EEXIST, > don't try to register things with the same name in the same directory. > [ 1.896910] cpu cpu0: Failed to create platform device, -17 Regards, Nicolas
Hi, Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: > Hi Stefan, > thanks for the review, I took note of your code comments. > > On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: >> Hi Nicolas, >> >> Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: >>> Hi all, >>> this series aims at adding cpufreq support to the Raspberry Pi family of >>> boards. >>> >>> The previous revision can be found at: https://lkml.org/lkml/2019/5/20/431 >>> >>> The series first factors out 'pllb' from clk-bcm2385 and creates a new >>> clk driver that operates it over RPi's firmware interface[1]. We are >>> forced to do so as the firmware 'owns' the pll and we're not allowed to >>> change through the register interface directly as we might race with the >>> over-temperature and under-voltage protections provided by the firmware. >> it would be nice to preserve such design decision in the driver as a >> comment, because the cover letter usually get lost. >>> Next it creates a minimal cpufreq driver that populates the CPU's opp >>> table, and registers cpufreq-dt. Which is needed as the firmware >>> controls the max and min frequencies available. >> I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and >> manually enable this drivers. During boot with Raspbian rootfs i'm >> getting the following: >> >> [ 1.177009] cpu cpu0: failed to get clock: -2 >> [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 > This is surprising, who could be creating a platform_device for cpufreq-dt > apart from raspberrypi-cpufreq? Just to make things clear, you're using the > device tree from v5.2-rc1 (as opposed to the Raspbian one)? sorry my fault, i thought it already has been replaced. The behavior in this unexpected case is fine, since it doesn't crash. I replaced the the DTB with the mainline one, but now i'm getting this: [ 4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 600000 KHz [ 4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 [ 4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 [ 4.608413] ------------[ cut here ]------------ [ 4.620203] kernel BUG at drivers/cpufreq/cpufreq.c:1348! [ 4.632787] Internal error: Oops - BUG: 0 [#1] SMP ARM [ 4.645062] Modules linked in: [ 4.655147] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G W 5.2.0-rc1-00004-g5aa6d98-dirty #2 [ 4.671891] Hardware name: BCM2835 [ 4.682549] PC is at cpufreq_online+0x690/0x6b0 [ 4.694409] LR is at __wake_up_common_lock+0xa0/0xc8 [ 4.706744] pc : [<c0c696c0>] lr : [<c03860d4>] psr: a0000013 [ 4.720518] sp : ea89dce8 ip : eaa1ace0 fp : c1704d54 [ 4.733288] r10: eaa1acb4 r9 : c1673440 r8 : c1705038 [ 4.746085] r7 : c18e6728 r6 : 000927c0 r5 : 00000000 r4 : eaa1ac00 [ 4.760288] r3 : d89db0bd r2 : d89db0bd r1 : 60000013 r0 : ffffffea [ 4.774541] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 4.789431] Control: 10c5383d Table: 0020406a DAC: 00000051 [ 4.802897] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [ 4.816661] Stack: (0xea89dce8 to 0xea89e000) [ 4.828744] dce0: 00000000 d89db0bd eaa1ac04 00000001 ea89dd28 00000000 [ 4.844876] dd00: c184539c c1704c48 00000000 c186de1c 0000000f 00000000 c15dc83c c0c6975c [ 4.861033] dd20: c186d9e4 c096dbbc ea891358 ea9a5db4 00000000 d89db0bd c186de68 c18e6728 [ 4.877227] dd40: c186d99c c0c66ffc eaff1800 c0c6bda0 00000000 eb80c050 eaff1800 c0c6c148 [ 4.893461] dd60: eaff1810 00000000 c186de1c c097126c c18e16a4 eaff1810 c18e16a8 c096f314 [ 4.909769] dd80: eaff1810 c186de1c c096f800 c1704c48 00000001 00000000 c1667b7c c096f64c [ 4.926077] dda0: c186de1c ea89ddfc eaff1810 00000000 ea89ddfc c096f800 c1704c48 00000001 [ 4.942410] ddc0: 00000000 c1667b7c c15dc83c c096d864 00000000 ea89126c eaf85138 d89db0bd [ 4.958798] dde0: eaff1810 eaff1810 c1704c48 eaff1854 c1704c48 c096f1b0 c1667b7c eaff1810 [ 4.975236] de00: 00000001 d89db0bd eaff1810 eaff1810 c1845308 c1704c48 00000000 c096e574 [ 4.991705] de20: eaff1810 c1845138 00000000 c096b490 00000000 c0e822d8 c1704c48 ea89dea0 [ 5.008233] de40: eaff1800 d89db0bd ea89de60 eaff1800 00000000 eaff1800 eb80c050 00000000 [ 5.024770] de60: eaff1810 c0971098 eaff1800 ea89dea0 c1704c48 eb80c050 00000000 05f5e100 [ 5.041302] de80: c1667b7c c0971ae4 00000000 23c34600 c1704c48 eb80c050 00000000 c15a7a30 [ 5.057796] dea0: 00000000 00000000 00000000 c1361770 ffffffff 00000000 00000000 00000000 [ 5.074269] dec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 d89db0bd [ 5.090693] dee0: 00000167 c1704c48 c188d420 ffffe000 00000000 c15a7908 00000167 c0302eec [ 5.107119] df00: 00000167 c0365a5c c13ff448 c130e700 00000000 00000007 00000007 c124057c [ 5.123539] df20: 00000000 c1704c48 c1251c04 c12405f0 00000000 ebfffc7d 00000000 d89db0bd [ 5.139941] df40: 00000000 c188d420 00000008 d89db0bd c188d420 00000008 c1899800 c1899800 [ 5.156355] df60: c15dc838 c15010f8 00000007 00000007 00000000 c150067c c0e949cc 00000000 [ 5.172779] df80: 00000000 00000000 c0e949cc 00000000 00000000 00000000 00000000 00000000 [ 5.189192] dfa0: 00000000 c0e949d4 00000000 c03010e8 00000000 00000000 00000000 00000000 [ 5.205577] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 5.221915] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [ 5.238205] [<c0c696c0>] (cpufreq_online) from [<c0c6975c>] (cpufreq_add_dev+0x6c/0x78) [ 5.254372] [<c0c6975c>] (cpufreq_add_dev) from [<c096dbbc>] (subsys_interface_register+0xa0/0xec) [ 5.271557] [<c096dbbc>] (subsys_interface_register) from [<c0c66ffc>] (cpufreq_register_driver+0x14c/0x20c) [ 5.289700] [<c0c66ffc>] (cpufreq_register_driver) from [<c0c6c148>] (dt_cpufreq_probe+0x94/0x114) [ 5.307002] [<c0c6c148>] (dt_cpufreq_probe) from [<c097126c>] (platform_drv_probe+0x48/0x98) [ 5.323802] [<c097126c>] (platform_drv_probe) from [<c096f314>] (really_probe+0xf0/0x2c8) [ 5.340390] [<c096f314>] (really_probe) from [<c096f64c>] (driver_probe_device+0x60/0x164) [ 5.357113] [<c096f64c>] (driver_probe_device) from [<c096d864>] (bus_for_each_drv+0x58/0xb8) [ 5.374142] [<c096d864>] (bus_for_each_drv) from [<c096f1b0>] (__device_attach+0xd0/0x13c) [ 5.390937] [<c096f1b0>] (__device_attach) from [<c096e574>] (bus_probe_device+0x84/0x8c) [ 5.407684] [<c096e574>] (bus_probe_device) from [<c096b490>] (device_add+0x3bc/0x608) [ 5.424208] [<c096b490>] (device_add) from [<c0971098>] (platform_device_add+0x110/0x214) [ 5.440983] [<c0971098>] (platform_device_add) from [<c0971ae4>] (platform_device_register_full+0x130/0x148) [ 5.459471] [<c0971ae4>] (platform_device_register_full) from [<c15a7a30>] (raspberrypi_cpufreq_driver_init+0x128/0x178) [ 5.479053] [<c15a7a30>] (raspberrypi_cpufreq_driver_init) from [<c0302eec>] (do_one_initcall+0x54/0x21c) [ 5.497336] [<c0302eec>] (do_one_initcall) from [<c15010f8>] (kernel_init_freeable+0x244/0x2e0) [ 5.514735] [<c15010f8>] (kernel_init_freeable) from [<c0e949d4>] (kernel_init+0x8/0x10c) [ 5.531590] [<c0e949d4>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c) [ 5.547768] Exception stack(0xea89dfb0 to 0xea89dff8) [ 5.561323] dfa0: 00000000 00000000 00000000 00000000 [ 5.578116] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 5.594962] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 5.610133] Code: e59f1024 e34c0136 ebdcbce4 eaffff63 (e7f001f2) [ 5.624789] ---[ end trace 7aaf0f77e232247e ]--- [ 5.637981] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 5.654308] CPU3: stopping [ 5.665609] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G D W 5.2.0-rc1-00004-g5aa6d98-dirty #2 [ 5.683854] Hardware name: BCM2835 [ 5.695927] [<c0312304>] (unwind_backtrace) from [<c030cc08>] (show_stack+0x10/0x14) [ 5.712502] [<c030cc08>] (show_stack) from [<c0e7d358>] (dump_stack+0xb4/0xc8) [ 5.728537] [<c0e7d358>] (dump_stack) from [<c03107c8>] (handle_IPI+0x3bc/0x3dc) [ 5.744710] [<c03107c8>] (handle_IPI) from [<c0301a8c>] (__irq_svc+0x6c/0x90) [ 5.760601] Exception stack(0xea8d7f60 to 0xea8d7fa8) [ 5.774348] 7f60: 00000000 000004e4 eb854ae0 c031d840 ffffe000 c1704c6c c1704cac 00000008 [ 5.791381] 7f80: 00000000 c1704c48 c1673568 00000000 00000002 ea8d7fb0 c0309164 c0309168 [ 5.808421] 7fa0: 60000013 ffffffff [ 5.820668] [<c0301a8c>] (__irq_svc) from [<c0309168>] (arch_cpu_idle+0x38/0x3c) [ 5.836950] [<c0309168>] (arch_cpu_idle) from [<c03747c4>] (do_idle+0x1bc/0x298) [ 5.853198] [<c03747c4>] (do_idle) from [<c0374b3c>] (cpu_startup_entry+0x18/0x1c) [ 5.869667] [<c0374b3c>] (cpu_startup_entry) from [<003025cc>] (0x3025cc) [ 5.885362] CPU1: stopping [ 5.896857] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D W 5.2.0-rc1-00004-g5aa6d98-dirty #2 [ 5.915295] Hardware name: BCM2835 [ 5.927623] [<c0312304>] (unwind_backtrace) from [<c030cc08>] (show_stack+0x10/0x14) [ 5.944490] [<c030cc08>] (show_stack) from [<c0e7d358>] (dump_stack+0xb4/0xc8) [ 5.960861] [<c0e7d358>] (dump_stack) from [<c03107c8>] (handle_IPI+0x3bc/0x3dc) [ 5.977364] [<c03107c8>] (handle_IPI) from [<c0301a8c>] (__irq_svc+0x6c/0x90) [ 5.993562] Exception stack(0xea8d3f60 to 0xea8d3fa8) [ 6.007632] 3f60: 00000000 00002930 eb82cae0 c031d840 ffffe000 c1704c6c c1704cac 00000002 [ 6.024974] 3f80: 00000000 c1704c48 c1673568 00000000 00000000 ea8d3fb0 c0309164 c0309168 [ 6.042285] 3fa0: 60000013 ffffffff [ 6.054772] [<c0301a8c>] (__irq_svc) from [<c0309168>] (arch_cpu_idle+0x38/0x3c) [ 6.071321] [<c0309168>] (arch_cpu_idle) from [<c03747c4>] (do_idle+0x1bc/0x298) [ 6.087880] [<c03747c4>] (do_idle) from [<c0374b3c>] (cpu_startup_entry+0x18/0x1c) [ 6.104610] [<c0374b3c>] (cpu_startup_entry) from [<003025cc>] (0x3025cc) [ 6.120535] CPU0: stopping [ 6.132240] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G D W 5.2.0-rc1-00004-g5aa6d98-dirty #2 [ 6.150859] Hardware name: BCM2835 [ 6.163308] [<c0312304>] (unwind_backtrace) from [<c030cc08>] (show_stack+0x10/0x14) [ 6.180258] [<c030cc08>] (show_stack) from [<c0e7d358>] (dump_stack+0xb4/0xc8) [ 6.196660] [<c0e7d358>] (dump_stack) from [<c03107c8>] (handle_IPI+0x3bc/0x3dc) [ 6.213214] [<c03107c8>] (handle_IPI) from [<c0301a8c>] (__irq_svc+0x6c/0x90) [ 6.229479] Exception stack(0xc1701f10 to 0xc1701f58) [ 6.243636] 1f00: 00000000 0000190c eb818ae0 c031d840 [ 6.261073] 1f20: ffffe000 c1704c6c c1704cac 00000001 00000000 c1704c48 c1673568 00000000 [ 6.278565] 1f40: 00000002 c1701f60 c0309164 c0309168 60000013 ffffffff [ 6.294466] [<c0301a8c>] (__irq_svc) from [<c0309168>] (arch_cpu_idle+0x38/0x3c) [ 6.311211] [<c0309168>] (arch_cpu_idle) from [<c03747c4>] (do_idle+0x1bc/0x298) [ 6.327979] [<c03747c4>] (do_idle) from [<c0374b3c>] (cpu_startup_entry+0x18/0x1c) [ 6.344934] [<c0374b3c>] (cpu_startup_entry) from [<c1500e88>] (start_kernel+0x460/0x48c) [ 6.362527] [<c1500e88>] (start_kernel) from [<00000000>] (0x0) [ 6.377824] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
Hi Stefan, On Wed, 2019-06-05 at 13:34 +0200, Stefan Wahren wrote: > Hi, > > Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: > > Hi Stefan, > > thanks for the review, I took note of your code comments. > > > > On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: > > > Hi Nicolas, > > > > > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > > > Hi all, > > > > this series aims at adding cpufreq support to the Raspberry Pi family of > > > > boards. > > > > > > > > The previous revision can be found at: > > > > https://lkml.org/lkml/2019/5/20/431 > > > > > > > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > > > > clk driver that operates it over RPi's firmware interface[1]. We are > > > > forced to do so as the firmware 'owns' the pll and we're not allowed to > > > > change through the register interface directly as we might race with the > > > > over-temperature and under-voltage protections provided by the firmware. > > > it would be nice to preserve such design decision in the driver as a > > > comment, because the cover letter usually get lost. > > > > Next it creates a minimal cpufreq driver that populates the CPU's opp > > > > table, and registers cpufreq-dt. Which is needed as the firmware > > > > controls the max and min frequencies available. > > > I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and > > > manually enable this drivers. During boot with Raspbian rootfs i'm > > > getting the following: > > > > > > [ 1.177009] cpu cpu0: failed to get clock: -2 > > > [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 > > This is surprising, who could be creating a platform_device for cpufreq-dt > > apart from raspberrypi-cpufreq? Just to make things clear, you're using the > > device tree from v5.2-rc1 (as opposed to the Raspbian one)? > > sorry my fault, i thought it already has been replaced. The behavior in > this unexpected case is fine, since it doesn't crash. > > I replaced the the DTB with the mainline one, but now i'm getting this: > > [ 4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: > 600000 KHz > [ 4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 > [ 4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 Ok, this looks more more like my fault, probably an overflow error somewhere. I saw something similar while testing it on RPI2b. Which board & config was this run with? Could you confirm the clk-raspberrypi.c message verifying the max and min frequencies showed up and was correct. Regards, Nicolas
Am 05.06.19 um 14:27 schrieb Nicolas Saenz Julienne: > Ok, this looks more more like my fault, probably an overflow error somewhere. I > saw something similar while testing it on RPI2b. Which board & config was this > run with? It's an RPi 3B+ with multi_v7_defconfig > Could you confirm the clk-raspberrypi.c message verifying the max and > min frequencies showed up and was correct. [ 4.253294] raspberrypi-firmware soc:firmware: Attached to firmware from 2019-03-27 15:45 [ 4.269727] mmcblk0: mmc0:0007 SDCIT 14.6 GiB [ 4.282464] raspberrypi-clk raspberrypi-clk: CPU frequency range: min 600000000, max 1400000000 > > Regards, > Nicolas >
On Wed, 2019-06-05 at 13:34 +0200, Stefan Wahren wrote: > Hi, > > Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: > > Hi Stefan, > > thanks for the review, I took note of your code comments. > > > > On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: > > > Hi Nicolas, > > > > > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > > > Hi all, > > > > this series aims at adding cpufreq support to the Raspberry Pi family of > > > > boards. > > > > > > > > The previous revision can be found at: > > > > https://lkml.org/lkml/2019/5/20/431 > > > > > > > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > > > > clk driver that operates it over RPi's firmware interface[1]. We are > > > > forced to do so as the firmware 'owns' the pll and we're not allowed to > > > > change through the register interface directly as we might race with the > > > > over-temperature and under-voltage protections provided by the firmware. > > > it would be nice to preserve such design decision in the driver as a > > > comment, because the cover letter usually get lost. > > > > Next it creates a minimal cpufreq driver that populates the CPU's opp > > > > table, and registers cpufreq-dt. Which is needed as the firmware > > > > controls the max and min frequencies available. > > > I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and > > > manually enable this drivers. During boot with Raspbian rootfs i'm > > > getting the following: > > > > > > [ 1.177009] cpu cpu0: failed to get clock: -2 > > > [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 > > This is surprising, who could be creating a platform_device for cpufreq-dt > > apart from raspberrypi-cpufreq? Just to make things clear, you're using the > > device tree from v5.2-rc1 (as opposed to the Raspbian one)? > > sorry my fault, i thought it already has been replaced. The behavior in > this unexpected case is fine, since it doesn't crash. > > I replaced the the DTB with the mainline one, but now i'm getting this: > > [ 4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: > 600000 KHz > [ 4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 > [ 4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 For the record this fixes it: diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index aa51756fd4d6..edb71eefe9cf 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1293,7 +1293,7 @@ static int clk_core_determine_round_nolock(struct clk_core *core, } else if (core->ops->round_rate) { rate = core->ops->round_rate(core->hw, req->rate, &req->best_parent_rate); - if (rate < 0) + if (IS_ERR_VALUE(rate)) return rate; req->rate = rate; round_rate() returns a 'long' value, yet 'pllb' in rpi3b+ goes as high as 2.8GHz, which only fits in an 'unsigned long'. This explains why I didn't see this issue with RPI2b. I'll add the patch to the series. Regards, Nicolas
Hi Nicolas, >> Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: >>> Hi Stefan, >>> thanks for the review, I took note of your code comments. >>> >>> On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: >>>> Hi Nicolas, >>>> >>>> Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: >>>>> Hi all, >>>>> this series aims at adding cpufreq support to the Raspberry Pi family of >>>>> boards. >>>>> >>>>> The previous revision can be found at: >>>>> https://lkml.org/lkml/2019/5/20/431 >>>>> >>>>> The series first factors out 'pllb' from clk-bcm2385 and creates a new >>>>> clk driver that operates it over RPi's firmware interface[1]. We are >>>>> forced to do so as the firmware 'owns' the pll and we're not allowed to >>>>> change through the register interface directly as we might race with the >>>>> over-temperature and under-voltage protections provided by the firmware. >>>> it would be nice to preserve such design decision in the driver as a >>>> comment, because the cover letter usually get lost. >>>>> Next it creates a minimal cpufreq driver that populates the CPU's opp >>>>> table, and registers cpufreq-dt. Which is needed as the firmware >>>>> controls the max and min frequencies available. >>>> I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and >>>> manually enable this drivers. During boot with Raspbian rootfs i'm >>>> getting the following: >>>> >>>> [ 1.177009] cpu cpu0: failed to get clock: -2 >>>> [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 >>> This is surprising, who could be creating a platform_device for cpufreq-dt >>> apart from raspberrypi-cpufreq? Just to make things clear, you're using the >>> device tree from v5.2-rc1 (as opposed to the Raspbian one)? >> sorry my fault, i thought it already has been replaced. The behavior in >> this unexpected case is fine, since it doesn't crash. >> >> I replaced the the DTB with the mainline one, but now i'm getting this: >> >> [ 4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: >> 600000 KHz >> [ 4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 >> [ 4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 > For the record this fixes it: > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index aa51756fd4d6..edb71eefe9cf 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1293,7 +1293,7 @@ static int clk_core_determine_round_nolock(struct clk_core > *core, > } else if (core->ops->round_rate) { > rate = core->ops->round_rate(core->hw, req->rate, > &req->best_parent_rate); > - if (rate < 0) > + if (IS_ERR_VALUE(rate)) > return rate; > > req->rate = rate; > > round_rate() returns a 'long' value, yet 'pllb' in rpi3b+ goes as high as > 2.8GHz, which only fits in an 'unsigned long'. This explains why I didn't see > this issue with RPI2b. Okay, i understand the problem. But the patch doesn't look like the proper fix to me. Maybe the better way is to implement determine_rate instead of round_rate in the clock driver. AFAICS this provides the necessary unsigned long. > > I'll add the patch to the series. > > Regards, > Nicolas