bus: brcmstb_gisb: add support for GISBv7 arbiter
Subsystems (2)
v3 cover bus: brcmstb_gisb: add support for GISBv7 arbiter
This patch set contains changes to enable the GISB arbiter driver on the latest ARM64 architecture Set-Top Box chips from Broadcom.
Since the ARM64 architecture does not support the hooking of low level fault handlers the driver has been adjusted to depend solely on GISB interrupts and notify events to provide diagnostic messaging. The GISB hardware still triggers bus faults for the processor so the default low-level aborts will occur and will be handled based on the architecture specific kernel implementation.
While this tends to obsure the GISB error messaging it is still reasonable so the same approach is applied to the ARM architecture for consistency.
The patches also correct some issues with the existing driver and add the new register map for the GISBv7 hardware first appearing in the BCM7278 device.
Changes since v2 at [2]:
- Removed code associated with enabling the hooking of ARM64 memory faults as well as the code that hooks them for both ARM and ARM64 architectures within the GISB arbiter driver.
- Created separate notifiers for die and panic events to prevent list corruption, and substituted the notifier return name for the previous magic number (Thanks Mark!)
Changes since v1 at [1]:
- Removed code associated with hooking SError handling in favor of a registered notifier (Thanks Mark!)
- Removed an unnecessary explicit cast (Thanks Gregory!)
[1] https://lkml.org/lkml/2017/3/24/413
[2] https://lkml.org/lkml/2017/3/28/1125
Doug Berger (6):
bus: brcmstb_gisb: Use register offsets with writes too
bus: brcmstb_gisb: correct support for 64-bit address output
bus: brcmstb_gisb: add notifier handling
bus: brcmstb_gisb: remove low-level ARM hooks
bus: brcmstb_gisb: enable driver for ARM64 architecture
bus: brcmstb_gisb: update to support new revision
.../devicetree/bindings/bus/brcm,gisb-arb.txt | 3 +- drivers/bus/Kconfig | 2 +- drivers/bus/brcmstb_gisb.c | 121 ++++++++++++--------- 3 files changed, 75 insertions(+), 51 deletions(-)
Signature · 1 line
2.12.0
PATCH v3 1/6 bus: brcmstb_gisb: Use register offsets with writes too
This commit corrects the bug introduced in commit f80835875d3d ("bus: brcmstb_gisb: Look up register offsets in a table") such that gisb_write() translates the register enumeration into an offset from the base address for writes as well as reads.
--- drivers/bus/brcmstb_gisb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/bus/brcmstb_gisb.c +3 -3
@@ -1,5 +1,5 @@/* -* Copyright (C) 2014 Broadcom Corporation +* Copyright (C) 2014-2017 Broadcom * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as
@@ -127,9 +127,9 @@ static void gisb_write(struct brcmstb_gisb_arb_device *gdev, u32 val, int reg)return ;if (gdev->big_endian) -iowrite32be (val, gdev->base + reg); +iowrite32be (val, gdev->base + offset);else -iowrite32 (val, gdev->base + reg); +iowrite32 (val, gdev->base + offset); } static ssize_t gisb_arb_get_timeout(struct device *dev,
Signature · 1 line
2.12.0
PATCH v3 2/6 bus: brcmstb_gisb: correct support for 64-bit address output
In reply to Doug BergerThe GISB bus can support addresses beyond 32-bits. So this commit corrects support for reading a captured 64-bit address into a 64-bit variable by obtaining the high bits from the ARB_ERR_CAP_HI_ADDR register (when present) and then outputting the full 64-bit value.
It also removes unused definitions.
--- drivers/bus/brcmstb_gisb.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)
drivers/bus/brcmstb_gisb.c +20 -16
@@ -37,8 +37,6 @@#define ARB_ERR_CAP_CLEAR (1 << 0)#define ARB_ERR_CAP_STATUS_TIMEOUT (1 << 12)#define ARB_ERR_CAP_STATUS_TEA (1 << 11) -#define ARB_ERR_CAP_STATUS_BS_SHIFT (1 << 2) -#define ARB_ERR_CAP_STATUS_BS_MASK 0x3c#define ARB_ERR_CAP_STATUS_WRITE (1 << 1)#define ARB_ERR_CAP_STATUS_VALID (1 << 0)
@@ -47,7 +45,6 @@ enum {ARB_ERR_CAP_CLR ,ARB_ERR_CAP_HI_ADDR ,ARB_ERR_CAP_ADDR , -ARB_ERR_CAP_DATA ,ARB_ERR_CAP_STATUS , ARB_ERR_CAP_MASTER, };
@@ -57,7 +54,6 @@ static const int gisb_offsets_bcm7038[] = { [ARB_ERR_CAP_CLR] = 0x0c4, [ARB_ERR_CAP_HI_ADDR] = -1, [ARB_ERR_CAP_ADDR] = 0x0c8, - [ARB_ERR_CAP_DATA] = 0x0cc, [ARB_ERR_CAP_STATUS] = 0x0d0, [ARB_ERR_CAP_MASTER] = -1, };
@@ -67,7 +63,6 @@ static const int gisb_offsets_bcm7400[] = { [ARB_ERR_CAP_CLR] = 0x0c8, [ARB_ERR_CAP_HI_ADDR] = -1, [ARB_ERR_CAP_ADDR] = 0x0cc, - [ARB_ERR_CAP_DATA] = 0x0d0, [ARB_ERR_CAP_STATUS] = 0x0d4, [ARB_ERR_CAP_MASTER] = 0x0d8, };
@@ -77,7 +72,6 @@ static const int gisb_offsets_bcm7435[] = { [ARB_ERR_CAP_CLR] = 0x168, [ARB_ERR_CAP_HI_ADDR] = -1, [ARB_ERR_CAP_ADDR] = 0x16c, - [ARB_ERR_CAP_DATA] = 0x170, [ARB_ERR_CAP_STATUS] = 0x174, [ARB_ERR_CAP_MASTER] = 0x178, };
@@ -87,7 +81,6 @@ static const int gisb_offsets_bcm7445[] = { [ARB_ERR_CAP_CLR] = 0x7e4, [ARB_ERR_CAP_HI_ADDR] = 0x7e8, [ARB_ERR_CAP_ADDR] = 0x7ec, - [ARB_ERR_CAP_DATA] = 0x7f0, [ARB_ERR_CAP_STATUS] = 0x7f4, [ARB_ERR_CAP_MASTER] = 0x7f8, };
@@ -109,9 +102,13 @@ static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg) {int offset = gdev->gisb_offsets[reg]; -/* return 1 if the hardware doesn't have ARB_ERR_CAP_MASTER */ -if (offset ==-1 ) -return 1 ; +if (offset <0 ) { +/* return 1 if the hardware doesn't have ARB_ERR_CAP_MASTER */ +if (reg ==ARB_ERR_CAP_MASTER ) +return 1 ; +else +return 0 ; + }if (gdev->big_endian)return ioread32be (gdev->base + offset);
@@ -119,6 +116,16 @@ static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg)return ioread32 (gdev->base + offset); } +static u64 gisb_read_address (struct brcmstb_gisb_arb_device *gdev) +{ +u64 value; + + value =gisb_read (gdev,ARB_ERR_CAP_ADDR ); + value |= (u64 )gisb_read (gdev,ARB_ERR_CAP_HI_ADDR ) <<32 ; + +return value; +} +static void gisb_write (struct brcmstb_gisb_arb_device *gdev,u32 val,int reg) {int offset = gdev->gisb_offsets[reg];
@@ -185,7 +192,7 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev, const char *reason) {u32 cap_status; -unsigned long arb_addr; +u64 arb_addr;u32 master;const char *m_name;char m_fmt[11 ];
@@ -197,10 +204,7 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev,return 1 ;/* Read the address and master */ - arb_addr =gisb_read (gdev,ARB_ERR_CAP_ADDR ) &0xffffffff ; -#if (IS_ENABLED (CONFIG_PHYS_ADDR_T_64BIT )) - arb_addr |= (u64 )gisb_read (gdev,ARB_ERR_CAP_HI_ADDR ) <<32 ; -#endif + arb_addr =gisb_read_address (gdev); master =gisb_read (gdev,ARB_ERR_CAP_MASTER ); m_name =brcmstb_gisb_master_to_str (gdev, master);
@@ -209,7 +213,7 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev, m_name = m_fmt; } - pr_crit("%s: %s at 0x%lx [%c %s], core: %s\n", + pr_crit("%s: %s@0x%llx [%c %s], core: %s\n", __func__, reason, arb_addr, cap_status & ARB_ERR_CAP_STATUS_WRITE ? 'W' : 'R', cap_status & ARB_ERR_CAP_STATUS_TIMEOUT ? "timeout" : "",
Signature · 1 line
2.12.0
PATCH v3 3/6 bus: brcmstb_gisb: add notifier handling
In reply to Doug BergerCheck for GISB arbitration errors through a chained notifier when a process dies or a kernel panic occurs. This allows a meaningful GISB diagnostic message to occur along with other diagnostic information from the event.
--- drivers/bus/brcmstb_gisb.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
drivers/bus/brcmstb_gisb.c +39 -0
@@ -24,6 +24,9 @@#include <linux/of.h> #include <linux/bitops.h> #include <linux/pm.h> +#include <linux/kernel.h> +#include <linux/kdebug.h> +#include <linux/notifier.h> #ifdef CONFIG_ARM #include <asm/bug.h>
@@ -283,6 +286,36 @@ static irqreturn_t brcmstb_gisb_tea_handler(int irq, void *dev_id)return IRQ_HANDLED ; } +/* +* Dump out gisb errors on die or panic. +*/ +static int dump_gisb_error (struct notifier_block *self,unsigned long v, +void *p); + +static struct notifier_block gisb_die_notifier = { + .notifier_call = dump_gisb_error, +}; + +static struct notifier_block gisb_panic_notifier = { + .notifier_call = dump_gisb_error, +}; + +static int dump_gisb_error (struct notifier_block *self,unsigned long v, +void *p) +{ +struct brcmstb_gisb_arb_device *gdev; +const char *reason ="panic" ; + +if (self == &gisb_die_notifier) + reason ="die" ; + +/* iterate over each GISB arb registered handlers */ +list_for_each_entry (gdev, &brcmstb_gisb_arb_device_list, next) +brcmstb_gisb_arb_decode_addr (gdev, reason); + +return NOTIFY_DONE ; +} + static DEVICE_ATTR(gisb_arb_timeout, S_IWUSR | S_IRUGO, gisb_arb_get_timeout, gisb_arb_set_timeout);
@@ -390,6 +423,12 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev) board_be_handler = brcmstb_bus_error_handler;#endif +if (list_is_singular (&brcmstb_gisb_arb_device_list)) { +register_die_notifier (&gisb_die_notifier); +atomic_notifier_chain_register (&panic_notifier_list, + &gisb_panic_notifier); + } +dev_info (&pdev->dev,"registered mem: %p, irqs: %d, %d\n" , gdev->base, timeout_irq, tea_irq);
Signature · 1 line
2.12.0
PATCH v3 4/6 bus: brcmstb_gisb: remove low-level ARM hooks
In reply to Doug BergerThe ARM architecture can provide meaningful diagnostic output from the GISB arbiter solely from interrupts and notifiers without the need to hook the low level fault handlers.
--- drivers/bus/brcmstb_gisb.c | 30 ------------------------------ 1 file changed, 30 deletions(-)
drivers/bus/brcmstb_gisb.c +0 -30
@@ -28,11 +28,6 @@#include <linux/kdebug.h> #include <linux/notifier.h> -#ifdef CONFIG_ARM -#include <asm/bug.h> -#include <asm/signal.h> -#endif -#ifdef CONFIG_MIPS #include <asm/traps.h> #endif
@@ -228,27 +223,6 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev,return 0 ; } -#ifdef CONFIG_ARM -static int brcmstb_bus_error_handler (unsigned long addr,unsigned int fsr, -struct pt_regs *regs) -{ -int ret =0 ; -struct brcmstb_gisb_arb_device *gdev; - -/* iterate over each GISB arb registered handlers */ -list_for_each_entry (gdev, &brcmstb_gisb_arb_device_list, next) - ret |=brcmstb_gisb_arb_decode_addr (gdev,"bus error" ); -/* -* If it was an imprecise abort, then we need to correct the -* return address to be _after_ the instruction. -*/ -if (fsr & (1 <<10 )) - regs->ARM_pc +=4 ; - -return ret; -} -#endif -#ifdef CONFIG_MIPS static int brcmstb_bus_error_handler (struct pt_regs *regs,int is_fixup) {
@@ -415,10 +389,6 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev)list_add_tail (&gdev->next, &brcmstb_gisb_arb_device_list); -#ifdef CONFIG_ARM -hook_fault_code (22 , brcmstb_bus_error_handler,SIGBUS ,0 , -"imprecise external abort" ); -#endif #ifdef CONFIG_MIPS board_be_handler = brcmstb_bus_error_handler;#endif
Signature · 1 line
2.12.0
PATCH v3 5/6 bus: brcmstb_gisb: enable driver for ARM64 architecture
In reply to Doug BergerThe ARM64 architecture can provide meaningful diagnostic output from the GISB arbiter solely from interrupts and notifiers without the need to hook the low level fault handlers.
--- drivers/bus/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/Kconfig +1 -1
@@ -57,7 +57,7 @@ config ARM_CCN config BRCMSTB_GISB_ARB bool "Broadcom STB GISB bus arbiter" - depends on ARM || MIPS + depends on ARM || ARM64 || MIPS default ARCH_BRCMSTB || BMIPS_GENERIC help Driver for the Broadcom Set Top Box System-on-a-chip internal bus
Signature · 1 line
2.12.0
PATCH v3 6/6 bus: brcmstb_gisb: update to support new revision
In reply to Doug BergerThe 7278 introduces a new version of this core. This commit adds support for that revision.
--- Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt | 3 ++- drivers/bus/brcmstb_gisb.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt +2 -1
@@ -3,7 +3,8 @@ Broadcom GISB bus Arbiter controller Required properties: - compatible: - "brcm,gisb-arb" or "brcm,bcm7445-gisb-arb" for 28nm chips + "brcm,bcm7278-gisb-arb" for V7 28nm chips + "brcm,gisb-arb" or "brcm,bcm7445-gisb-arb" for other 28nm chips "brcm,bcm7435-gisb-arb" for newer 40nm chips "brcm,bcm7400-gisb-arb" for older 40nm chips and all 65nm chips "brcm,bcm7038-gisb-arb" for 130nm chips
drivers/bus/brcmstb_gisb.c +10 -0
@@ -56,6 +56,15 @@ static const int gisb_offsets_bcm7038[] = { [ARB_ERR_CAP_MASTER] = -1, }; +static const int gisb_offsets_bcm7278[] = { + [ARB_TIMER ] =0x008 , + [ARB_ERR_CAP_CLR ] =0x7f8 , + [ARB_ERR_CAP_HI_ADDR ] =-1 , + [ARB_ERR_CAP_ADDR ] =0x7e0 , + [ARB_ERR_CAP_STATUS ] =0x7f0 , + [ARB_ERR_CAP_MASTER ] =0x7f4 , +}; +static const int gisb_offsets_bcm7400[] = { [ARB_TIMER ] =0x00c , [ARB_ERR_CAP_CLR ] =0x0c8 ,
@@ -307,6 +316,7 @@ static const struct of_device_id brcmstb_gisb_arb_of_match[] = { { .compatible = "brcm,bcm7445-gisb-arb", .data = gisb_offsets_bcm7445 }, { .compatible = "brcm,bcm7435-gisb-arb", .data = gisb_offsets_bcm7435 }, { .compatible = "brcm,bcm7400-gisb-arb", .data = gisb_offsets_bcm7400 }, + { .compatible = "brcm,bcm7278-gisb-arb", .data = gisb_offsets_bcm7278 }, { .compatible = "brcm,bcm7038-gisb-arb", .data = gisb_offsets_bcm7038 }, { }, };
Signature · 1 line
2.12.0
Quoted 10 lines · Doug Berger
This patch set contains changes to enable the GISB arbiter driver on the latest ARM64 architecture Set-Top Box chips from Broadcom.
Since the ARM64 architecture does not support the hooking of low level fault handlers the driver has been adjusted to depend solely on GISB interrupts and notify events to provide diagnostic messaging. The GISB hardware still triggers bus faults for the processor so the default low-level aborts will occur and will be handled based on the architecture specific kernel implementation.
Quoted 7 lines
While this tends to obsure the GISB error messaging it is still reasonable so the same approach is applied to the ARM architecture for consistency.
The patches also correct some issues with the existing driver and add the new register map for the GISBv7 hardware first appearing in the BCM7278 device.
This all looks great to me, and since there are no more ARM64 dependencies I can take this via the Broadcom ARM/ARM64 SoC pull requests.
Mark, are you also happy with this v3?
Thanks Doug!
Quoted 30 lines
Changes since v2 at [2]:
- Removed code associated with enabling the hooking of ARM64 memory faults as well as the code that hooks them for both ARM and ARM64 architectures within the GISB arbiter driver.
- Created separate notifiers for die and panic events to prevent list corruption, and substituted the notifier return name for the previous magic number (Thanks Mark!)Changes since v1 at [1]:
- Removed code associated with hooking SError handling in favor of a registered notifier (Thanks Mark!)
- Removed an unnecessary explicit cast (Thanks Gregory!)[1] https://lkml.org/lkml/2017/3/24/413
[2] https://lkml.org/lkml/2017/3/28/1125Doug Berger (6):
bus:brcmstb_gisb: Use register offsets with writes too
bus:brcmstb_gisb: correct support for 64-bit address output
bus:brcmstb_gisb: add notifier handling
bus:brcmstb_gisb: remove low-level ARM hooks
bus:brcmstb_gisb: enable driver for ARM64 architecture
bus:brcmstb_gisb: update to support new revision.../devicetree/bindings/bus/brcm,gisb-arb.txt | 3 +- drivers/bus/Kconfig | 2 +- drivers/bus/brcmstb_gisb.c | 121 ++++++++++++--------- 3 files changed, 75 insertions(+), 51 deletions(-)
Signature · 1 line
Florian
Quoted 15 lines · Florian Fainelli
Quoted 14 lines · Doug Berger
This patch set contains changes to enable the GISB arbiter driver on the latest ARM64 architecture Set-Top Box chips from Broadcom.
Since the ARM64 architecture does not support the hooking of low level fault handlers the driver has been adjusted to depend solely on GISB interrupts and notify events to provide diagnostic messaging. The GISB hardware still triggers bus faults for the processor so the default low-level aborts will occur and will be handled based on the architecture specific kernel implementation.
While this tends to obsure the GISB error messaging it is still reasonable so the same approach is applied to the ARM architecture for consistency.
Quoted 8 lines
Quoted 3 lines
The patches also correct some issues with the existing driver and add the new register map for the GISBv7 hardware first appearing in the BCM7278 device.
This all looks great to me, and since there are no more ARM64 dependencies I can take this via the Broadcom ARM/ARM64 SoC pull requests.
Mark, are you also happy with this v3?
It looks fine by me.
Thanks,
Mark.
Quoted 20 lines · Mark Rutland
Quoted 19 lines · Florian Fainelli
Quoted 17 lines · Doug Berger
This patch set contains changes to enable the GISB arbiter driver on the latest ARM64 architecture Set-Top Box chips from Broadcom.
Since the ARM64 architecture does not support the hooking of low level fault handlers the driver has been adjusted to depend solely on GISB interrupts and notify events to provide diagnostic messaging. The GISB hardware still triggers bus faults for the processor so the default low-level aborts will occur and will be handled based on the architecture specific kernel implementation.
While this tends to obsure the GISB error messaging it is still reasonable so the same approach is applied to the ARM architecture for consistency.
The patches also correct some issues with the existing driver and add the new register map for the GISBv7 hardware first appearing in the BCM7278 device.
Quoted 6 lines
Quoted 4 lines
This all looks great to me, and since there are no more ARM64 dependencies I can take this via the Broadcom ARM/ARM64 SoC pull requests.
Mark, are you also happy with this v3?
It looks fine by me.
Series applied to drivers/next, thanks everyone!
Signature · 1 line
Florian