Re: [PATCH 6/9] bus: brcmstb_gisb: correct support for 64-bit address output

[Gregory Fong]

On Fri, Mar 24, 2017 at 7:46 AM, Doug Berger  wrote:
> The 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.
>
> Fixes: 44127b771d9c ("bus: add Broadcom GISB bus arbiter timeout/error 
> handler")
> Signed-off-by: Doug Berger 
> ---
>  drivers/bus/brcmstb_gisb.c | 36 
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
> [snip]
> @@ -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 = (u64)gisb_read(gdev, ARB_ERR_CAP_ADDR);

Unlike the one on the next line, this cast can be omitted.

> +   value |= (u64)gisb_read(gdev, ARB_ERR_CAP_HI_ADDR) << 32;
> +
> +   return value;
> +}
> [snip]

Acked-by: Gregory Fong 

Re: [PATCH 6/9] bus: brcmstb_gisb: correct support for 64-bit address output

[Gregory Fong]

On Fri, Mar 24, 2017 at 7:46 AM, Doug Berger  wrote:
> The 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.
>
> Fixes: 44127b771d9c ("bus: add Broadcom GISB bus arbiter timeout/error 
> handler")
> Signed-off-by: Doug Berger 
> ---
>  drivers/bus/brcmstb_gisb.c | 36 
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
> [snip]
> @@ -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 = (u64)gisb_read(gdev, ARB_ERR_CAP_ADDR);

Unlike the one on the next line, this cast can be omitted.

> +   value |= (u64)gisb_read(gdev, ARB_ERR_CAP_HI_ADDR) << 32;
> +
> +   return value;
> +}
> [snip]

Acked-by: Gregory Fong 

[PATCH 6/9] bus: brcmstb_gisb: correct support for 64-bit address output

[Doug Berger]

The 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.

Fixes: 44127b771d9c ("bus: add Broadcom GISB bus arbiter timeout/error handler")
Signed-off-by: Doug Berger 
---
 drivers/bus/brcmstb_gisb.c | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 9eba0143f1a4..c8d2a61d21ed 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -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_MASK0x3c
 #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 = (u64)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) & 0x;
-#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 at 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" : "",
-- 
2.12.0

[PATCH 6/9] bus: brcmstb_gisb: correct support for 64-bit address output

[Doug Berger]

The 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.

Fixes: 44127b771d9c ("bus: add Broadcom GISB bus arbiter timeout/error handler")
Signed-off-by: Doug Berger 
---
 drivers/bus/brcmstb_gisb.c | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 9eba0143f1a4..c8d2a61d21ed 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -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_MASK0x3c
 #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 = (u64)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) & 0x;
-#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 at 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" : "",
-- 
2.12.0