[Patch v4 0/4] memory: tegra: Add MC channels and error logging

[Patch v4 0/4] memory: tegra: Add MC channels and error logging

[Ashish Mhetre]

From tegra186 onward, memory controllers support multiple channels.
Add memory controller channels in device tree and add support to map
address spaces of these channels in tegra MC driver.
When memory controller interrupt occurs, registers from these channels
are required to be read in order to get error information.
Add error logging support from tegra186 onward for memory controller
interrupts.

Ashish Mhetre (4):
  arm64: tegra: Add memory controller channels
  dt-bindings: memory: Update reg maxitems for tegra186
  memory: tegra: Add memory controller channels support
  memory: tegra: Add MC error logging on tegra186 onward

---
Changes in v4:
- Added memory controller channels support
- Added newlines after every break statement of all switch cases
- Fixed compile error with W=1 build
- Fixed the interrupt mask bit logic

Changes in v3:
- Removed unnecessary ifdefs
- Grouped newly added MC registers with existing MC registers
- Removed unnecessary initialization of variables
- Updated code to use newly added field 'has_addr_hi_reg' instead of ifdefs

Changes in v2:
- Updated patch subject and commit message
- Removed separate irq handlers
- Updated tegra30_mc_handle_irq to be used for tegra186 onwards as well

 .../memory-controllers/nvidia,tegra186-mc.yaml     |   2 +-
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           |   7 +-
 arch/arm64/boot/dts/nvidia/tegra194.dtsi           |  21 +++-
 arch/arm64/boot/dts/nvidia/tegra234.dtsi           |  21 +++-
 drivers/memory/tegra/mc.c                          | 108 ++++++++++++++++++---
 drivers/memory/tegra/mc.h                          |  37 ++++++-
 drivers/memory/tegra/tegra186.c                    |  67 +++++++++++++
 drivers/memory/tegra/tegra194.c                    |  46 +++++++++
 drivers/memory/tegra/tegra234.c                    |   1 +
 include/soc/tegra/mc.h                             |  10 ++
 10 files changed, 296 insertions(+), 24 deletions(-)

-- 
2.7.4

[Patch v4 1/4] arm64: tegra: Add memory controller channels

[Ashish Mhetre]

From tegra186 onwards, memory controller support multiple channels.
During the error interrupts from memory controller, corresponding
channels need to be accessed for logging error info and clearing the
interrupt.
So add address and size of these channels in device tree node of
tegra186, tegra194 and tegra234 memory controller.

Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi |  7 ++++++-
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 21 ++++++++++++++++++---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 21 ++++++++++++++++++---
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index e9b40f5..9c14404 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -521,7 +521,12 @@
 
 	mc: memory-controller@2c00000 {
 		compatible = "nvidia,tegra186-mc";
-		reg = <0x0 0x02c00000 0x0 0xb0000>;
+		reg = <0x0 0x02c00000 0x0 0x10000>,    /* MC-SID */
+		      <0x0 0x02c10000 0x0 0x10000>,    /* Broadcast channel */
+		      <0x0 0x02c20000 0x0 0x10000>,    /* MC0 */
+		      <0x0 0x02c30000 0x0 0x10000>,    /* MC1 */
+		      <0x0 0x02c40000 0x0 0x10000>,    /* MC2 */
+		      <0x0 0x02c50000 0x0 0x10000>;    /* MC3 */
 		interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
 
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index c28bf4d..e19c56c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -604,9 +604,24 @@
 
 		mc: memory-controller@2c00000 {
 			compatible = "nvidia,tegra194-mc";
-			reg = <0x02c00000 0x100000>,
-			      <0x02b80000 0x040000>,
-			      <0x01700000 0x100000>;
+			reg = <0x02c00000 0x10000>,   /* MC-SID */
+			      <0x02c10000 0x10000>,   /* MC Broadcast*/
+			      <0x02c20000 0x10000>,   /* MC0 */
+			      <0x02c30000 0x10000>,   /* MC1 */
+			      <0x02c40000 0x10000>,   /* MC2 */
+			      <0x02c50000 0x10000>,   /* MC3 */
+			      <0x02b80000 0x10000>,   /* MC4 */
+			      <0x02b90000 0x10000>,   /* MC5 */
+			      <0x02ba0000 0x10000>,   /* MC6 */
+			      <0x02bb0000 0x10000>,   /* MC7 */
+			      <0x01700000 0x10000>,   /* MC8 */
+			      <0x01710000 0x10000>,   /* MC9 */
+			      <0x01720000 0x10000>,   /* MC10 */
+			      <0x01730000 0x10000>,   /* MC11 */
+			      <0x01740000 0x10000>,   /* MC12 */
+			      <0x01750000 0x10000>,   /* MC13 */
+			      <0x01760000 0x10000>,   /* MC14 */
+			      <0x01770000 0x10000>;   /* MC15 */
 			interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
 			#interconnect-cells = <1>;
 			status = "disabled";
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index aaace60..6e33d2b 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -507,9 +507,24 @@
 
 		mc: memory-controller@2c00000 {
 			compatible = "nvidia,tegra234-mc";
-			reg = <0x02c00000 0x100000>,
-			      <0x02b80000 0x040000>,
-			      <0x01700000 0x100000>;
+			reg = <0x02c00000 0x10000>,   /* MC-SID */
+			      <0x02c10000 0x10000>,   /* MC Broadcast*/
+			      <0x02c20000 0x10000>,   /* MC0 */
+			      <0x02c30000 0x10000>,   /* MC1 */
+			      <0x02c40000 0x10000>,   /* MC2 */
+			      <0x02c50000 0x10000>,   /* MC3 */
+			      <0x02b80000 0x10000>,   /* MC4 */
+			      <0x02b90000 0x10000>,   /* MC5 */
+			      <0x02ba0000 0x10000>,   /* MC6 */
+			      <0x02bb0000 0x10000>,   /* MC7 */
+			      <0x01700000 0x10000>,   /* MC8 */
+			      <0x01710000 0x10000>,   /* MC9 */
+			      <0x01720000 0x10000>,   /* MC10 */
+			      <0x01730000 0x10000>,   /* MC11 */
+			      <0x01740000 0x10000>,   /* MC12 */
+			      <0x01750000 0x10000>,   /* MC13 */
+			      <0x01760000 0x10000>,   /* MC14 */
+			      <0x01770000 0x10000>;   /* MC15 */
 			interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
 			#interconnect-cells = <1>;
 			status = "okay";
-- 
2.7.4

[Patch v4 2/4] dt-bindings: memory: Update reg maxitems for tegra186

[Ashish Mhetre]

From tegra186 onwards, memory controller support multiple channels.
Reg items are updated with address and size of these channels.
Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234
have overall 17 memory controller channels each.
There is 1 reg item for memory controller stream-id registers.
So update the reg maxItems to 18 in tegra186 devicetree documentation.

Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
---
 .../devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
index 13c4c82..eb7ed00 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
@@ -35,7 +35,7 @@ properties:
 
   reg:
     minItems: 1
-    maxItems: 3
+    maxItems: 18
 
   interrupts:
     items:
-- 
2.7.4

[Patch v4 3/4] memory: tegra: Add memory controller channels support

[Ashish Mhetre]

From tegra186 onwards, memory controller support multiple channels.
Add support for mapping address spaces of these channels.
During error interrupts from memory controller, appropriate registers
from these channels need to be accessed for logging error info.

Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
---
 drivers/memory/tegra/mc.c       |  6 ++++++
 drivers/memory/tegra/tegra186.c | 21 +++++++++++++++++++++
 drivers/memory/tegra/tegra194.c |  1 +
 drivers/memory/tegra/tegra234.c |  1 +
 include/soc/tegra/mc.h          |  7 +++++++
 5 files changed, 36 insertions(+)

diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index bf3abb6..3cda1d9 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -749,6 +749,12 @@ static int tegra_mc_probe(struct platform_device *pdev)
 	if (IS_ERR(mc->regs))
 		return PTR_ERR(mc->regs);
 
+	if (mc->soc->ops && mc->soc->ops->map_regs) {
+		err = mc->soc->ops->map_regs(mc, pdev);
+		if (err < 0)
+			return err;
+	}
+
 	mc->debugfs.root = debugfs_create_dir("mc", NULL);
 
 	if (mc->soc->ops && mc->soc->ops->probe) {
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 3d15388..59a4425 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -139,11 +139,31 @@ static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
 	return 0;
 }
 
+static int tegra186_mc_map_regs(struct tegra_mc *mc,
+				struct platform_device *pdev)
+{
+	struct resource *res;
+	int i;
+
+	mc->mcb_regs = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
+	if (IS_ERR(mc->mcb_regs))
+		return PTR_ERR(mc->mcb_regs);
+
+	for (i = 0; i < mc->soc->num_channels; i++) {
+		mc->mc_regs[i] = devm_platform_get_and_ioremap_resource(pdev, i + 2, &res);
+		if (IS_ERR(mc->mc_regs[i]))
+			return PTR_ERR(mc->mc_regs[i]);
+	}
+
+	return 0;
+}
+
 const struct tegra_mc_ops tegra186_mc_ops = {
 	.probe = tegra186_mc_probe,
 	.remove = tegra186_mc_remove,
 	.resume = tegra186_mc_resume,
 	.probe_device = tegra186_mc_probe_device,
+	.map_regs = tegra186_mc_map_regs,
 };
 
 #if defined(CONFIG_ARCH_TEGRA_186_SOC)
@@ -875,6 +895,7 @@ const struct tegra_mc_soc tegra186_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra186_mc_clients),
 	.clients = tegra186_mc_clients,
 	.num_address_bits = 40,
+	.num_channels = 4,
 	.ops = &tegra186_mc_ops,
 };
 #endif
diff --git a/drivers/memory/tegra/tegra194.c b/drivers/memory/tegra/tegra194.c
index cab998b..9400117 100644
--- a/drivers/memory/tegra/tegra194.c
+++ b/drivers/memory/tegra/tegra194.c
@@ -1347,5 +1347,6 @@ const struct tegra_mc_soc tegra194_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra194_mc_clients),
 	.clients = tegra194_mc_clients,
 	.num_address_bits = 40,
+	.num_channels = 16,
 	.ops = &tegra186_mc_ops,
 };
diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
index e22824a..6335a13 100644
--- a/drivers/memory/tegra/tegra234.c
+++ b/drivers/memory/tegra/tegra234.c
@@ -97,5 +97,6 @@ const struct tegra_mc_soc tegra234_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra234_mc_clients),
 	.clients = tegra234_mc_clients,
 	.num_address_bits = 40,
+	.num_channels = 16,
 	.ops = &tegra186_mc_ops,
 };
diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
index 1066b11..92f810c 100644
--- a/include/soc/tegra/mc.h
+++ b/include/soc/tegra/mc.h
@@ -13,6 +13,9 @@
 #include <linux/irq.h>
 #include <linux/reset-controller.h>
 #include <linux/types.h>
+#include <linux/platform_device.h>
+
+#define MC_MAX_CHANNELS 16
 
 struct clk;
 struct device;
@@ -181,6 +184,7 @@ struct tegra_mc_ops {
 	int (*resume)(struct tegra_mc *mc);
 	irqreturn_t (*handle_irq)(int irq, void *data);
 	int (*probe_device)(struct tegra_mc *mc, struct device *dev);
+	int (*map_regs)(struct tegra_mc *mc, struct platform_device *pdev);
 };
 
 struct tegra_mc_soc {
@@ -194,6 +198,7 @@ struct tegra_mc_soc {
 	unsigned int atom_size;
 
 	u8 client_id_mask;
+	u8 num_channels;
 
 	const struct tegra_smmu_soc *smmu;
 
@@ -212,6 +217,8 @@ struct tegra_mc {
 	struct tegra_smmu *smmu;
 	struct gart_device *gart;
 	void __iomem *regs;
+	void __iomem *mcb_regs;
+	void __iomem *mc_regs[MC_MAX_CHANNELS];
 	struct clk *clk;
 	int irq;
 
-- 
2.7.4

[Patch v4 4/4] memory: tegra: Add MC error logging on tegra186 onward

[Ashish Mhetre]

Add new function 'get_int_channel' in tegra_mc_soc struture which is
implemented by tegra SOCs which support multiple MC channels. This
function returns the channel which should be used to get the information
of interrupts.
Remove static from tegra30_mc_handle_irq and use it as interrupt handler
for MC interrupts on tegra186, tegra194 and tegra234 to log the errors.
Add error specific MC status and address register bits and use them on
tegra186, tegra194 and tegra234.
Add error logging for generalized carveout interrupt on tegra186, tegra194
and tegra234.
Add error logging for route sanity interrupt on tegra194 an tegra234.
Add register for higher bits of error address which is available on
tegra194 and tegra234.
Add a boolean variable 'has_addr_hi_reg' in tegra_mc_soc struture which
will be true if soc has register for higher bits of memory controller
error address. Set it true for tegra194 and tegra234.

Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
---
 drivers/memory/tegra/mc.c       | 102 ++++++++++++++++++++++++++++++++++------
 drivers/memory/tegra/mc.h       |  37 ++++++++++++++-
 drivers/memory/tegra/tegra186.c |  45 ++++++++++++++++++
 drivers/memory/tegra/tegra194.c |  44 +++++++++++++++++
 drivers/memory/tegra/tegra234.c |  59 +++++++++++++++++++++++
 include/soc/tegra/mc.h          |   4 ++
 6 files changed, 275 insertions(+), 16 deletions(-)

diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 3cda1d9..bb861a8 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -508,14 +508,32 @@ int tegra30_mc_probe(struct tegra_mc *mc)
 	return 0;
 }
 
-static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
+const struct tegra_mc_ops tegra30_mc_ops = {
+	.probe = tegra30_mc_probe,
+	.handle_irq = tegra30_mc_handle_irq,
+};
+#endif
+
+irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
 {
 	struct tegra_mc *mc = data;
 	unsigned long status;
 	unsigned int bit;
+	int channel;
+
+	if (mc->soc->num_channels && mc->soc->get_int_channel) {
+		int err;
+
+		err = mc->soc->get_int_channel(mc, &channel);
+		if (err < 0)
+			return IRQ_NONE;
+
+		/* mask all interrupts to avoid flooding */
+		status = mc_ch_readl(mc, channel, MC_INTSTATUS) & mc->soc->intmask;
+	} else {
+		status = mc_readl(mc, MC_INTSTATUS) & mc->soc->intmask;
+	}
 
-	/* mask all interrupts to avoid flooding */
-	status = mc_readl(mc, MC_INTSTATUS) & mc->soc->intmask;
 	if (!status)
 		return IRQ_NONE;
 
@@ -523,18 +541,66 @@ static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
 		const char *error = tegra_mc_status_names[bit] ?: "unknown";
 		const char *client = "unknown", *desc;
 		const char *direction, *secure;
+		u32 status_reg, addr_reg;
+		u32 intmask = BIT(bit);
 		phys_addr_t addr = 0;
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+		u32 addr_hi_reg = 0;
+#endif
 		unsigned int i;
 		char perm[7];
 		u8 id, type;
 		u32 value;
 
-		value = mc_readl(mc, MC_ERR_STATUS);
+		switch (intmask) {
+		case MC_INT_DECERR_VPR:
+			status_reg = MC_ERR_VPR_STATUS;
+			addr_reg = MC_ERR_VPR_ADR;
+			break;
+
+		case MC_INT_SECERR_SEC:
+			status_reg = MC_ERR_SEC_STATUS;
+			addr_reg = MC_ERR_SEC_ADR;
+			break;
+
+		case MC_INT_DECERR_MTS:
+			status_reg = MC_ERR_MTS_STATUS;
+			addr_reg = MC_ERR_MTS_ADR;
+			break;
+
+		case MC_INT_DECERR_GENERALIZED_CARVEOUT:
+			status_reg = MC_ERR_GENERALIZED_CARVEOUT_STATUS;
+			addr_reg = MC_ERR_GENERALIZED_CARVEOUT_ADR;
+			break;
+
+		case MC_INT_DECERR_ROUTE_SANITY:
+			status_reg = MC_ERR_ROUTE_SANITY_STATUS;
+			addr_reg = MC_ERR_ROUTE_SANITY_ADR;
+			break;
+
+		default:
+			status_reg = MC_ERR_STATUS;
+			addr_reg = MC_ERR_ADR;
+
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+			if (mc->soc->has_addr_hi_reg)
+				addr_hi_reg = MC_ERR_ADR_HI;
+#endif
+			break;
+		}
+
+		if (mc->soc->num_channels)
+			value = mc_ch_readl(mc, channel, status_reg);
+		else
+			value = mc_readl(mc, status_reg);
 
 #ifdef CONFIG_PHYS_ADDR_T_64BIT
 		if (mc->soc->num_address_bits > 32) {
-			addr = ((value >> MC_ERR_STATUS_ADR_HI_SHIFT) &
-				MC_ERR_STATUS_ADR_HI_MASK);
+			if (addr_hi_reg)
+				addr = mc_ch_readl(mc, channel, addr_hi_reg);
+			else
+				addr = ((value >> MC_ERR_STATUS_ADR_HI_SHIFT) &
+					MC_ERR_STATUS_ADR_HI_MASK);
 			addr <<= 32;
 		}
 #endif
@@ -591,7 +657,10 @@ static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
 			break;
 		}
 
-		value = mc_readl(mc, MC_ERR_ADR);
+		if (mc->soc->num_channels)
+			value = mc_ch_readl(mc, channel, addr_reg);
+		else
+			value = mc_readl(mc, addr_reg);
 		addr |= value;
 
 		dev_err_ratelimited(mc->dev, "%s: %s%s @%pa: %s (%s%s)\n",
@@ -600,17 +669,14 @@ static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
 	}
 
 	/* clear interrupts */
-	mc_writel(mc, status, MC_INTSTATUS);
+	if (mc->soc->num_channels)
+		mc_ch_writel(mc, channel, status, MC_INTSTATUS);
+	else
+		mc_writel(mc, status, MC_INTSTATUS);
 
 	return IRQ_HANDLED;
 }
 
-const struct tegra_mc_ops tegra30_mc_ops = {
-	.probe = tegra30_mc_probe,
-	.handle_irq = tegra30_mc_handle_irq,
-};
-#endif
-
 const char *const tegra_mc_status_names[32] = {
 	[ 1] = "External interrupt",
 	[ 6] = "EMEM address decode error",
@@ -622,6 +688,8 @@ const char *const tegra_mc_status_names[32] = {
 	[12] = "VPR violation",
 	[13] = "Secure carveout violation",
 	[16] = "MTS carveout violation",
+	[17] = "Generalized carveout violation",
+	[20] = "Route Sanity error",
 };
 
 const char *const tegra_mc_error_names[8] = {
@@ -770,7 +838,11 @@ static int tegra_mc_probe(struct platform_device *pdev)
 
 		WARN(!mc->soc->client_id_mask, "missing client ID mask for this SoC\n");
 
-		mc_writel(mc, mc->soc->intmask, MC_INTMASK);
+		if (mc->soc->num_channels)
+			mc_ch_writel(mc, MC_BROADCAST_CHANNEL, mc->soc->intmask,
+				  MC_INTMASK);
+		else
+			mc_writel(mc, mc->soc->intmask, MC_INTMASK);
 
 		err = devm_request_irq(&pdev->dev, mc->irq, mc->soc->ops->handle_irq, 0,
 				       dev_name(&pdev->dev), mc);
diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
index 062886e..3836c35 100644
--- a/drivers/memory/tegra/mc.h
+++ b/drivers/memory/tegra/mc.h
@@ -43,7 +43,21 @@
 #define MC_EMEM_ARB_OVERRIDE				0xe8
 #define MC_TIMING_CONTROL_DBG				0xf8
 #define MC_TIMING_CONTROL				0xfc
-
+#define MC_ERR_VPR_STATUS				0x654
+#define MC_ERR_VPR_ADR					0x658
+#define MC_ERR_SEC_STATUS				0x67c
+#define MC_ERR_SEC_ADR					0x680
+#define MC_ERR_MTS_STATUS				0x9b0
+#define MC_ERR_MTS_ADR					0x9b4
+#define MC_ERR_ROUTE_SANITY_STATUS			0x9c0
+#define MC_ERR_ROUTE_SANITY_ADR				0x9c4
+#define MC_ERR_GENERALIZED_CARVEOUT_STATUS		0xc00
+#define MC_ERR_GENERALIZED_CARVEOUT_ADR			0xc04
+#define MC_GLOBAL_INTSTATUS				0xf24
+#define MC_ERR_ADR_HI					0x11fc
+
+#define MC_INT_DECERR_ROUTE_SANITY			BIT(20)
+#define MC_INT_DECERR_GENERALIZED_CARVEOUT		BIT(17)
 #define MC_INT_DECERR_MTS				BIT(16)
 #define MC_INT_SECERR_SEC				BIT(13)
 #define MC_INT_DECERR_VPR				BIT(12)
@@ -78,6 +92,8 @@
 
 #define MC_TIMING_UPDATE				BIT(0)
 
+#define MC_BROADCAST_CHANNEL				~0
+
 static inline u32 tegra_mc_scale_percents(u64 val, unsigned int percents)
 {
 	val = val * percents;
@@ -92,6 +108,24 @@ icc_provider_to_tegra_mc(struct icc_provider *provider)
 	return container_of(provider, struct tegra_mc, provider);
 }
 
+static inline u32 mc_ch_readl(const struct tegra_mc *mc, int ch,
+			       unsigned long offset)
+{
+	if (ch == MC_BROADCAST_CHANNEL)
+		return readl_relaxed(mc->mcb_regs + offset);
+
+	return readl_relaxed(mc->mc_regs[ch] + offset);
+}
+
+static inline void mc_ch_writel(const struct tegra_mc *mc, int ch,
+				u32 value, unsigned long offset)
+{
+	if (ch == MC_BROADCAST_CHANNEL)
+		writel_relaxed(value, mc->mcb_regs + offset);
+	else
+		writel_relaxed(value, mc->mc_regs[ch] + offset);
+}
+
 static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
 {
 	return readl_relaxed(mc->regs + offset);
@@ -156,6 +190,7 @@ extern const struct tegra_mc_ops tegra30_mc_ops;
 extern const struct tegra_mc_ops tegra186_mc_ops;
 #endif
 
+irqreturn_t tegra30_mc_handle_irq(int irq, void *data);
 extern const char * const tegra_mc_status_names[32];
 extern const char * const tegra_mc_error_names[8];
 
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 59a4425..bce0237 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -16,6 +16,8 @@
 #include <dt-bindings/memory/tegra186-mc.h>
 #endif
 
+#include "mc.h"
+
 #define MC_SID_STREAMID_OVERRIDE_MASK GENMASK(7, 0)
 #define MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED BIT(16)
 #define MC_SID_STREAMID_SECURITY_OVERRIDE BIT(8)
@@ -164,6 +166,7 @@ const struct tegra_mc_ops tegra186_mc_ops = {
 	.resume = tegra186_mc_resume,
 	.probe_device = tegra186_mc_probe_device,
 	.map_regs = tegra186_mc_map_regs,
+	.handle_irq = tegra30_mc_handle_irq,
 };
 
 #if defined(CONFIG_ARCH_TEGRA_186_SOC)
@@ -891,11 +894,53 @@ static const struct tegra_mc_client tegra186_mc_clients[] = {
 	},
 };
 
+static int tegra186_mc_get_channel(struct tegra_mc *mc, int *mc_channel)
+{
+	u32 g_intstatus;
+
+	g_intstatus = mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
+				  MC_GLOBAL_INTSTATUS);
+
+	switch (g_intstatus & mc->soc->int_channel_mask) {
+	case BIT(0):
+		*mc_channel = 0;
+		break;
+
+	case BIT(1):
+		*mc_channel = 1;
+		break;
+
+	case BIT(2):
+		*mc_channel = 2;
+		break;
+
+	case BIT(3):
+		*mc_channel = 3;
+		break;
+
+	case BIT(24):
+		*mc_channel = MC_BROADCAST_CHANNEL;
+		break;
+
+	default:
+		pr_err("Unknown interrupt source\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 const struct tegra_mc_soc tegra186_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra186_mc_clients),
 	.clients = tegra186_mc_clients,
 	.num_address_bits = 40,
 	.num_channels = 4,
+	.client_id_mask = 0xff,
+	.intmask = MC_INT_DECERR_GENERALIZED_CARVEOUT | MC_INT_DECERR_MTS |
+		   MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
+		   MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
 	.ops = &tegra186_mc_ops,
+	.int_channel_mask = 0x100000f,
+	.get_int_channel = tegra186_mc_get_channel,
 };
 #endif
diff --git a/drivers/memory/tegra/tegra194.c b/drivers/memory/tegra/tegra194.c
index 9400117..bc16567 100644
--- a/drivers/memory/tegra/tegra194.c
+++ b/drivers/memory/tegra/tegra194.c
@@ -1343,10 +1343,54 @@ static const struct tegra_mc_client tegra194_mc_clients[] = {
 	},
 };
 
+static int tegra194_mc_get_channel(struct tegra_mc *mc, int *mc_channel)
+{
+	u32 g_intstatus;
+
+	g_intstatus = mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
+				  MC_GLOBAL_INTSTATUS);
+
+	switch (g_intstatus & mc->soc->int_channel_mask) {
+	case BIT(8):
+		*mc_channel = 0;
+		break;
+
+	case BIT(9):
+		*mc_channel = 1;
+		break;
+
+	case BIT(10):
+		*mc_channel = 2;
+		break;
+
+	case BIT(11):
+		*mc_channel = 3;
+		break;
+
+	case BIT(25):
+		*mc_channel = MC_BROADCAST_CHANNEL;
+		break;
+
+	default:
+		pr_err("Unknown interrupt source\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 const struct tegra_mc_soc tegra194_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra194_mc_clients),
 	.clients = tegra194_mc_clients,
 	.num_address_bits = 40,
 	.num_channels = 16,
+	.client_id_mask = 0xff,
+	.intmask = MC_INT_DECERR_ROUTE_SANITY |
+		   MC_INT_DECERR_GENERALIZED_CARVEOUT | MC_INT_DECERR_MTS |
+		   MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
+		   MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
+	.has_addr_hi_reg = true,
 	.ops = &tegra186_mc_ops,
+	.int_channel_mask = 0x2000f00,
+	.get_int_channel = tegra194_mc_get_channel,
 };
diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
index 6335a13..8e09fc4 100644
--- a/drivers/memory/tegra/tegra234.c
+++ b/drivers/memory/tegra/tegra234.c
@@ -93,10 +93,69 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
 	},
 };
 
+static int tegra234_mc_get_channel(struct tegra_mc *mc, int *mc_channel)
+{
+	u32 g_intstatus;
+
+	g_intstatus = mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
+				  MC_GLOBAL_INTSTATUS);
+
+	switch (g_intstatus & mc->soc->int_channel_mask) {
+	case BIT(8):
+		*mc_channel = 0;
+		break;
+
+	case BIT(9):
+		*mc_channel = 1;
+		break;
+
+	case BIT(10):
+		*mc_channel = 2;
+		break;
+
+	case BIT(11):
+		*mc_channel = 3;
+		break;
+
+	case BIT(12):
+		*mc_channel = 4;
+		break;
+
+	case BIT(13):
+		*mc_channel = 5;
+		break;
+
+	case BIT(14):
+		*mc_channel = 6;
+		break;
+
+	case BIT(15):
+		*mc_channel = 7;
+		break;
+
+	case BIT(25):
+		*mc_channel = MC_BROADCAST_CHANNEL;
+		break;
+
+	default:
+		pr_err("Unknown interrupt source\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 const struct tegra_mc_soc tegra234_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra234_mc_clients),
 	.clients = tegra234_mc_clients,
 	.num_address_bits = 40,
 	.num_channels = 16,
+	.intmask = MC_INT_DECERR_ROUTE_SANITY |
+		   MC_INT_DECERR_GENERALIZED_CARVEOUT | MC_INT_DECERR_MTS |
+		   MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
+		   MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
+	.has_addr_hi_reg = true,
 	.ops = &tegra186_mc_ops,
+	.int_channel_mask = 0x200ff00,
+	.get_int_channel = tegra234_mc_get_channel,
 };
diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
index 92f810c..1fe6a62 100644
--- a/include/soc/tegra/mc.h
+++ b/include/soc/tegra/mc.h
@@ -203,6 +203,8 @@ struct tegra_mc_soc {
 	const struct tegra_smmu_soc *smmu;
 
 	u32 intmask;
+	u32 int_channel_mask;
+	bool has_addr_hi_reg;
 
 	const struct tegra_mc_reset_ops *reset_ops;
 	const struct tegra_mc_reset *resets;
@@ -210,6 +212,8 @@ struct tegra_mc_soc {
 
 	const struct tegra_mc_icc_ops *icc_ops;
 	const struct tegra_mc_ops *ops;
+
+	int (*get_int_channel)(struct tegra_mc *mc, int *mc_channel);
 };
 
 struct tegra_mc {
-- 
2.7.4

Re: [Patch v4 2/4] dt-bindings: memory: Update reg maxitems for tegra186

[Rob Herring]

On Wed, Mar 02, 2022 at 02:13:27PM +0530, Ashish Mhetre wrote:
> >From tegra186 onwards, memory controller support multiple channels.
> Reg items are updated with address and size of these channels.
> Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234
> have overall 17 memory controller channels each.
> There is 1 reg item for memory controller stream-id registers.
> So update the reg maxItems to 18 in tegra186 devicetree documentation.

Some of this needs to be in 'description' for 'reg'.

> 
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> ---
>  .../devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml      | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
> index 13c4c82..eb7ed00 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
> @@ -35,7 +35,7 @@ properties:
>  
>    reg:
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 18
>  
>    interrupts:
>      items:
> -- 
> 2.7.4
> 

Re: [Patch v4 2/4] dt-bindings: memory: Update reg maxitems for tegra186

[Krzysztof Kozlowski]

On 02/03/2022 18:51, Rob Herring wrote:
> On Wed, Mar 02, 2022 at 02:13:27PM +0530, Ashish Mhetre wrote:
>> >From tegra186 onwards, memory controller support multiple channels.
>> Reg items are updated with address and size of these channels.
>> Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234
>> have overall 17 memory controller channels each.
>> There is 1 reg item for memory controller stream-id registers.
>> So update the reg maxItems to 18 in tegra186 devicetree documentation.
> 
> Some of this needs to be in 'description' for 'reg'.
> 
>>
>> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
>> ---
>>  .../devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml      | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
>> index 13c4c82..eb7ed00 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
>> @@ -35,7 +35,7 @@ properties:
>>  
>>    reg:
>>      minItems: 1
>> -    maxItems: 3
>> +    maxItems: 18
>>  

...and with "if:" block constraining these on different hardware.


Best regards,
Krzysztof

Re: [Patch v4 1/4] arm64: tegra: Add memory controller channels

[Krzysztof Kozlowski]

On 02/03/2022 09:43, Ashish Mhetre wrote:
> From tegra186 onwards, memory controller support multiple channels.
> During the error interrupts from memory controller, corresponding
> channels need to be accessed for logging error info and clearing the
> interrupt.
> So add address and size of these channels in device tree node of
> tegra186, tegra194 and tegra234 memory controller.
> 
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra186.dtsi |  7 ++++++-
>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 21 ++++++++++++++++++---
>  arch/arm64/boot/dts/nvidia/tegra234.dtsi | 21 ++++++++++++++++++---
>  3 files changed, 42 insertions(+), 7 deletions(-)
> 

You send DTS patch before, which is wrong order and actually it points
my attention to probably ABI break.

DTS goes always separately from driver changes so it is not only ABI
break, but also non-bisectable.

Best regards,
Krzysztof

Re: [Patch v4 3/4] memory: tegra: Add memory controller channels support

[Krzysztof Kozlowski]

On 02/03/2022 09:43, Ashish Mhetre wrote:
> From tegra186 onwards, memory controller support multiple channels.
> Add support for mapping address spaces of these channels.
> During error interrupts from memory controller, appropriate registers
> from these channels need to be accessed for logging error info.
> 
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> ---
>  drivers/memory/tegra/mc.c       |  6 ++++++
>  drivers/memory/tegra/tegra186.c | 21 +++++++++++++++++++++
>  drivers/memory/tegra/tegra194.c |  1 +
>  drivers/memory/tegra/tegra234.c |  1 +
>  include/soc/tegra/mc.h          |  7 +++++++
>  5 files changed, 36 insertions(+)
> 
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index bf3abb6..3cda1d9 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -749,6 +749,12 @@ static int tegra_mc_probe(struct platform_device *pdev)
>  	if (IS_ERR(mc->regs))
>  		return PTR_ERR(mc->regs);
>  
> +	if (mc->soc->ops && mc->soc->ops->map_regs) {
> +		err = mc->soc->ops->map_regs(mc, pdev);
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	mc->debugfs.root = debugfs_create_dir("mc", NULL);
>  
>  	if (mc->soc->ops && mc->soc->ops->probe) {
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index 3d15388..59a4425 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -139,11 +139,31 @@ static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
>  	return 0;
>  }
>  
> +static int tegra186_mc_map_regs(struct tegra_mc *mc,
> +				struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	int i;
> +
> +	mc->mcb_regs = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> +	if (IS_ERR(mc->mcb_regs))
> +		return PTR_ERR(mc->mcb_regs);
> +
> +	for (i = 0; i < mc->soc->num_channels; i++) {
> +		mc->mc_regs[i] = devm_platform_get_and_ioremap_resource(pdev, i + 2, &res);
> +		if (IS_ERR(mc->mc_regs[i]))
> +			return PTR_ERR(mc->mc_regs[i]);

This breaks the ABI, so I need Thierry's ack that such ABI break is
perfectly ok.

> +	}
> +
> +	return 0;
> +}
> +


Best regards,
Krzysztof

Re: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186 onward

[Krzysztof Kozlowski]

On 02/03/2022 09:43, Ashish Mhetre wrote:
> Add new function 'get_int_channel' in tegra_mc_soc struture which is
> implemented by tegra SOCs which support multiple MC channels. This
> function returns the channel which should be used to get the information
> of interrupts.
> Remove static from tegra30_mc_handle_irq and use it as interrupt handler
> for MC interrupts on tegra186, tegra194 and tegra234 to log the errors.
> Add error specific MC status and address register bits and use them on
> tegra186, tegra194 and tegra234.
> Add error logging for generalized carveout interrupt on tegra186, tegra194
> and tegra234.
> Add error logging for route sanity interrupt on tegra194 an tegra234.
> Add register for higher bits of error address which is available on
> tegra194 and tegra234.
> Add a boolean variable 'has_addr_hi_reg' in tegra_mc_soc struture which
> will be true if soc has register for higher bits of memory controller
> error address. Set it true for tegra194 and tegra234.
> 
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> ---
>  drivers/memory/tegra/mc.c       | 102 ++++++++++++++++++++++++++++++++++------
>  drivers/memory/tegra/mc.h       |  37 ++++++++++++++-
>  drivers/memory/tegra/tegra186.c |  45 ++++++++++++++++++
>  drivers/memory/tegra/tegra194.c |  44 +++++++++++++++++
>  drivers/memory/tegra/tegra234.c |  59 +++++++++++++++++++++++
>  include/soc/tegra/mc.h          |   4 ++
>  6 files changed, 275 insertions(+), 16 deletions(-)
> 

(...)

>  
> +static int tegra186_mc_get_channel(struct tegra_mc *mc, int *mc_channel)
> +{
> +	u32 g_intstatus;
> +
> +	g_intstatus = mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
> +				  MC_GLOBAL_INTSTATUS);
> +
> +	switch (g_intstatus & mc->soc->int_channel_mask) {
> +	case BIT(0):
> +		*mc_channel = 0;
> +		break;
> +
> +	case BIT(1):
> +		*mc_channel = 1;
> +		break;
> +
> +	case BIT(2):
> +		*mc_channel = 2;
> +		break;
> +
> +	case BIT(3):
> +		*mc_channel = 3;
> +		break;
> +
> +	case BIT(24):
> +		*mc_channel = MC_BROADCAST_CHANNEL;
> +		break;
> +
> +	default:
> +		pr_err("Unknown interrupt source\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  const struct tegra_mc_soc tegra186_mc_soc = {
>  	.num_clients = ARRAY_SIZE(tegra186_mc_clients),
>  	.clients = tegra186_mc_clients,
>  	.num_address_bits = 40,
>  	.num_channels = 4,
> +	.client_id_mask = 0xff,
> +	.intmask = MC_INT_DECERR_GENERALIZED_CARVEOUT | MC_INT_DECERR_MTS |
> +		   MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
> +		   MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
>  	.ops = &tegra186_mc_ops,
> +	.int_channel_mask = 0x100000f,
> +	.get_int_channel = tegra186_mc_get_channel,
>  };
>  #endif
> diff --git a/drivers/memory/tegra/tegra194.c b/drivers/memory/tegra/tegra194.c
> index 9400117..bc16567 100644
> --- a/drivers/memory/tegra/tegra194.c
> +++ b/drivers/memory/tegra/tegra194.c
> @@ -1343,10 +1343,54 @@ static const struct tegra_mc_client tegra194_mc_clients[] = {
>  	},
>  };
>  
> +static int tegra194_mc_get_channel(struct tegra_mc *mc, int *mc_channel)

Looks like 'mc' could be a pointer to const.

> +{
> +	u32 g_intstatus;

Variable name just "status" because it looks like some
hungarian-notation-style...

The same in other places like this.


Best regards,
Krzysztof

Re: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186 onward

[Dan Carpenter]

Hi Ashish,

url:    https://github.com/0day-ci/linux/commits/Ashish-Mhetre/memory-tegra-Add-MC-channels-and-error-logging/20220302-164625
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
config: openrisc-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031247.0bBX70B3-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/memory/tegra/mc.c:593 tegra30_mc_handle_irq() error: uninitialized symbol 'channel'.

vim +/channel +593 drivers/memory/tegra/mc.c

cc84c62c96f257 Ashish Mhetre   2022-03-02  516  
cc84c62c96f257 Ashish Mhetre   2022-03-02  517  irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
89184651631713 Thierry Reding  2014-04-16  518  {
89184651631713 Thierry Reding  2014-04-16  519  	struct tegra_mc *mc = data;
1c74d5c0de0c2c Dmitry Osipenko 2018-04-09  520  	unsigned long status;
89184651631713 Thierry Reding  2014-04-16  521  	unsigned int bit;
cc84c62c96f257 Ashish Mhetre   2022-03-02  522  	int channel;
cc84c62c96f257 Ashish Mhetre   2022-03-02  523  
cc84c62c96f257 Ashish Mhetre   2022-03-02  524  	if (mc->soc->num_channels && mc->soc->get_int_channel) {

Let's assume "mc->soc->num_channels" is non-zero but there is no
mc->soc->get_int_channel() function.

cc84c62c96f257 Ashish Mhetre   2022-03-02  525  		int err;
cc84c62c96f257 Ashish Mhetre   2022-03-02  526  
cc84c62c96f257 Ashish Mhetre   2022-03-02  527  		err = mc->soc->get_int_channel(mc, &channel);
cc84c62c96f257 Ashish Mhetre   2022-03-02  528  		if (err < 0)
cc84c62c96f257 Ashish Mhetre   2022-03-02  529  			return IRQ_NONE;
89184651631713 Thierry Reding  2014-04-16  530  
89184651631713 Thierry Reding  2014-04-16  531  		/* mask all interrupts to avoid flooding */
cc84c62c96f257 Ashish Mhetre   2022-03-02  532  		status = mc_ch_readl(mc, channel, MC_INTSTATUS) & mc->soc->intmask;
cc84c62c96f257 Ashish Mhetre   2022-03-02  533  	} else {
1c74d5c0de0c2c Dmitry Osipenko 2018-04-09  534  		status = mc_readl(mc, MC_INTSTATUS) & mc->soc->intmask;
cc84c62c96f257 Ashish Mhetre   2022-03-02  535  	}
cc84c62c96f257 Ashish Mhetre   2022-03-02  536  
bf3fbdfbec947c Dmitry Osipenko 2018-04-09  537  	if (!status)
bf3fbdfbec947c Dmitry Osipenko 2018-04-09  538  		return IRQ_NONE;
89184651631713 Thierry Reding  2014-04-16  539  
89184651631713 Thierry Reding  2014-04-16  540  	for_each_set_bit(bit, &status, 32) {
1079a66bc32ff0 Thierry Reding  2021-06-02  541  		const char *error = tegra_mc_status_names[bit] ?: "unknown";
89184651631713 Thierry Reding  2014-04-16  542  		const char *client = "unknown", *desc;
89184651631713 Thierry Reding  2014-04-16  543  		const char *direction, *secure;
cc84c62c96f257 Ashish Mhetre   2022-03-02  544  		u32 status_reg, addr_reg;
cc84c62c96f257 Ashish Mhetre   2022-03-02  545  		u32 intmask = BIT(bit);
89184651631713 Thierry Reding  2014-04-16  546  		phys_addr_t addr = 0;
cc84c62c96f257 Ashish Mhetre   2022-03-02  547  #ifdef CONFIG_PHYS_ADDR_T_64BIT
cc84c62c96f257 Ashish Mhetre   2022-03-02  548  		u32 addr_hi_reg = 0;
cc84c62c96f257 Ashish Mhetre   2022-03-02  549  #endif
89184651631713 Thierry Reding  2014-04-16  550  		unsigned int i;
89184651631713 Thierry Reding  2014-04-16  551  		char perm[7];
89184651631713 Thierry Reding  2014-04-16  552  		u8 id, type;
89184651631713 Thierry Reding  2014-04-16  553  		u32 value;
89184651631713 Thierry Reding  2014-04-16  554  
cc84c62c96f257 Ashish Mhetre   2022-03-02  555  		switch (intmask) {
cc84c62c96f257 Ashish Mhetre   2022-03-02  556  		case MC_INT_DECERR_VPR:
cc84c62c96f257 Ashish Mhetre   2022-03-02  557  			status_reg = MC_ERR_VPR_STATUS;
cc84c62c96f257 Ashish Mhetre   2022-03-02  558  			addr_reg = MC_ERR_VPR_ADR;
cc84c62c96f257 Ashish Mhetre   2022-03-02  559  			break;
cc84c62c96f257 Ashish Mhetre   2022-03-02  560  
cc84c62c96f257 Ashish Mhetre   2022-03-02  561  		case MC_INT_SECERR_SEC:
cc84c62c96f257 Ashish Mhetre   2022-03-02  562  			status_reg = MC_ERR_SEC_STATUS;
cc84c62c96f257 Ashish Mhetre   2022-03-02  563  			addr_reg = MC_ERR_SEC_ADR;
cc84c62c96f257 Ashish Mhetre   2022-03-02  564  			break;
cc84c62c96f257 Ashish Mhetre   2022-03-02  565  
cc84c62c96f257 Ashish Mhetre   2022-03-02  566  		case MC_INT_DECERR_MTS:
cc84c62c96f257 Ashish Mhetre   2022-03-02  567  			status_reg = MC_ERR_MTS_STATUS;
cc84c62c96f257 Ashish Mhetre   2022-03-02  568  			addr_reg = MC_ERR_MTS_ADR;
cc84c62c96f257 Ashish Mhetre   2022-03-02  569  			break;
cc84c62c96f257 Ashish Mhetre   2022-03-02  570  
cc84c62c96f257 Ashish Mhetre   2022-03-02  571  		case MC_INT_DECERR_GENERALIZED_CARVEOUT:
cc84c62c96f257 Ashish Mhetre   2022-03-02  572  			status_reg = MC_ERR_GENERALIZED_CARVEOUT_STATUS;
cc84c62c96f257 Ashish Mhetre   2022-03-02  573  			addr_reg = MC_ERR_GENERALIZED_CARVEOUT_ADR;
cc84c62c96f257 Ashish Mhetre   2022-03-02  574  			break;
cc84c62c96f257 Ashish Mhetre   2022-03-02  575  
cc84c62c96f257 Ashish Mhetre   2022-03-02  576  		case MC_INT_DECERR_ROUTE_SANITY:
cc84c62c96f257 Ashish Mhetre   2022-03-02  577  			status_reg = MC_ERR_ROUTE_SANITY_STATUS;
cc84c62c96f257 Ashish Mhetre   2022-03-02  578  			addr_reg = MC_ERR_ROUTE_SANITY_ADR;
cc84c62c96f257 Ashish Mhetre   2022-03-02  579  			break;
cc84c62c96f257 Ashish Mhetre   2022-03-02  580  
cc84c62c96f257 Ashish Mhetre   2022-03-02  581  		default:
cc84c62c96f257 Ashish Mhetre   2022-03-02  582  			status_reg = MC_ERR_STATUS;
cc84c62c96f257 Ashish Mhetre   2022-03-02  583  			addr_reg = MC_ERR_ADR;
cc84c62c96f257 Ashish Mhetre   2022-03-02  584  
cc84c62c96f257 Ashish Mhetre   2022-03-02  585  #ifdef CONFIG_PHYS_ADDR_T_64BIT
cc84c62c96f257 Ashish Mhetre   2022-03-02  586  			if (mc->soc->has_addr_hi_reg)
cc84c62c96f257 Ashish Mhetre   2022-03-02  587  				addr_hi_reg = MC_ERR_ADR_HI;
cc84c62c96f257 Ashish Mhetre   2022-03-02  588  #endif
cc84c62c96f257 Ashish Mhetre   2022-03-02  589  			break;
cc84c62c96f257 Ashish Mhetre   2022-03-02  590  		}
cc84c62c96f257 Ashish Mhetre   2022-03-02  591  
cc84c62c96f257 Ashish Mhetre   2022-03-02  592  		if (mc->soc->num_channels)
cc84c62c96f257 Ashish Mhetre   2022-03-02 @593  			value = mc_ch_readl(mc, channel, status_reg);

Then "channel" is uninitialized here.

cc84c62c96f257 Ashish Mhetre   2022-03-02  594  		else
cc84c62c96f257 Ashish Mhetre   2022-03-02  595  			value = mc_readl(mc, status_reg);
89184651631713 Thierry Reding  2014-04-16  596  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186 onward

[Krzysztof Kozlowski]

On 03/03/2022 13:31, Dan Carpenter wrote:
> Hi Ashish,
> 
> url:    https://github.com/0day-ci/linux/commits/Ashish-Mhetre/memory-tegra-Add-MC-channels-and-error-logging/20220302-164625
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
> config: openrisc-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031247.0bBX70B3-lkp@intel.com/config)
> compiler: or1k-linux-gcc (GCC) 11.2.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> New smatch warnings:
> drivers/memory/tegra/mc.c:593 tegra30_mc_handle_irq() error: uninitialized symbol 'channel'.

Ashish,

I mentioned with your v3 that it is expected for submitter to run
certain automatic tools:
"We not only expect to compile it but also compile with W=1, run sparse,
smatch and coccicheck. Then also test."

Judging by the output here, it could be that either you used old
compiler or did not run the checks.

Can you please confirm that you performed all the activities mentioned
before?

Best regards,
Krzysztof

RE: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186 onward

[Ashish Mhetre]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: Thursday, March 3, 2022 1:14 AM
> To: Ashish Mhetre <amhetre@nvidia.com>; robh+dt@kernel.org;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> digetx@gmail.com; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> linux-tegra@vger.kernel.org
> Cc: Krishna Reddy <vdumpa@nvidia.com>; Sachin Nikam
> <Snikam@nvidia.com>
> Subject: Re: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186
> onward
> 
> External email: Use caution opening links or attachments
> 
> 
> On 02/03/2022 09:43, Ashish Mhetre wrote:
> > Add new function 'get_int_channel' in tegra_mc_soc struture which is
> > implemented by tegra SOCs which support multiple MC channels. This
> > function returns the channel which should be used to get the
> > information of interrupts.
> > Remove static from tegra30_mc_handle_irq and use it as interrupt
> > handler for MC interrupts on tegra186, tegra194 and tegra234 to log the
> errors.
> > Add error specific MC status and address register bits and use them on
> > tegra186, tegra194 and tegra234.
> > Add error logging for generalized carveout interrupt on tegra186,
> > tegra194 and tegra234.
> > Add error logging for route sanity interrupt on tegra194 an tegra234.
> > Add register for higher bits of error address which is available on
> > tegra194 and tegra234.
> > Add a boolean variable 'has_addr_hi_reg' in tegra_mc_soc struture
> > which will be true if soc has register for higher bits of memory
> > controller error address. Set it true for tegra194 and tegra234.
> >
> > Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> > ---
> >  drivers/memory/tegra/mc.c       | 102
> ++++++++++++++++++++++++++++++++++------
> >  drivers/memory/tegra/mc.h       |  37 ++++++++++++++-
> >  drivers/memory/tegra/tegra186.c |  45 ++++++++++++++++++
> > drivers/memory/tegra/tegra194.c |  44 +++++++++++++++++
> > drivers/memory/tegra/tegra234.c |  59 +++++++++++++++++++++++
> >  include/soc/tegra/mc.h          |   4 ++
> >  6 files changed, 275 insertions(+), 16 deletions(-)
> >
> 
> (...)
> 
> >
> > +static int tegra186_mc_get_channel(struct tegra_mc *mc, int
> > +*mc_channel) {
> > +     u32 g_intstatus;
> > +
> > +     g_intstatus = mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
> > +                               MC_GLOBAL_INTSTATUS);
> > +
> > +     switch (g_intstatus & mc->soc->int_channel_mask) {
> > +     case BIT(0):
> > +             *mc_channel = 0;
> > +             break;
> > +
> > +     case BIT(1):
> > +             *mc_channel = 1;
> > +             break;
> > +
> > +     case BIT(2):
> > +             *mc_channel = 2;
> > +             break;
> > +
> > +     case BIT(3):
> > +             *mc_channel = 3;
> > +             break;
> > +
> > +     case BIT(24):
> > +             *mc_channel = MC_BROADCAST_CHANNEL;
> > +             break;
> > +
> > +     default:
> > +             pr_err("Unknown interrupt source\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  const struct tegra_mc_soc tegra186_mc_soc = {
> >       .num_clients = ARRAY_SIZE(tegra186_mc_clients),
> >       .clients = tegra186_mc_clients,
> >       .num_address_bits = 40,
> >       .num_channels = 4,
> > +     .client_id_mask = 0xff,
> > +     .intmask = MC_INT_DECERR_GENERALIZED_CARVEOUT |
> MC_INT_DECERR_MTS |
> > +                MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
> > +                MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
> >       .ops = &tegra186_mc_ops,
> > +     .int_channel_mask = 0x100000f,
> > +     .get_int_channel = tegra186_mc_get_channel,
> >  };
> >  #endif
> > diff --git a/drivers/memory/tegra/tegra194.c
> > b/drivers/memory/tegra/tegra194.c index 9400117..bc16567 100644
> > --- a/drivers/memory/tegra/tegra194.c
> > +++ b/drivers/memory/tegra/tegra194.c
> > @@ -1343,10 +1343,54 @@ static const struct tegra_mc_client
> tegra194_mc_clients[] = {
> >       },
> >  };
> >
> > +static int tegra194_mc_get_channel(struct tegra_mc *mc, int
> > +*mc_channel)
> 
> Looks like 'mc' could be a pointer to const.
> 
> > +{
> > +     u32 g_intstatus;
> 
> Variable name just "status" because it looks like some hungarian-notation-
> style...
> 
> The same in other places like this.
> 
Okay, I will update this in next version.

> 
> Best regards,
> Krzysztof

RE: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186 onward

[Ashish Mhetre]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: Thursday, March 3, 2022 6:33 PM
> To: Dan Carpenter <dan.carpenter@oracle.com>; kbuild@lists.01.org; Ashish
> Mhetre <amhetre@nvidia.com>; robh+dt@kernel.org;
> thierry.reding@gmail.com; Jonathan Hunter <jonathanh@nvidia.com>;
> digetx@gmail.com; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> linux-tegra@vger.kernel.org
> Cc: lkp@intel.com; kbuild-all@lists.01.org; Krishna Reddy
> <vdumpa@nvidia.com>; Sachin Nikam <Snikam@nvidia.com>
> Subject: Re: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186
> onward
> 
> External email: Use caution opening links or attachments
> 
> 
> On 03/03/2022 13:31, Dan Carpenter wrote:
> > Hi Ashish,
> >
> > url:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2F0day-ci%2Flinux%2Fcommits%2FAshish-Mhetre%2Fmemory-tegra-
> Add-MC-channels-and-error-logging%2F20220302-
> 164625&amp;data=04%7C01%7Camhetre%40nvidia.com%7C448e9570ac274b
> 7ed2f408d9fd162da7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C
> 637819094016979779%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=
> dzcWTAMPikKWLFc4mkD%2FJPWQckiYrUzI9OOEEGvvDAA%3D&amp;reserved
> =0
> > base:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftegra%2Flinux.git&amp;dat
> a=04%7C01%7Camhetre%40nvidia.com%7C448e9570ac274b7ed2f408d9fd162
> da7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63781909401697
> 9779%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=pbpW4cT7C%2Fa
> qP8FJClKKdG4NdXpEGh0yBZPPk%2FeCSvU%3D&amp;reserved=0 for-next
> > config: openrisc-randconfig-m031-20220302
> > (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdow
> > nload.01.org%2F0day-
> ci%2Farchive%2F20220303%2F202203031247.0bBX70B3-lk
> >
> p%40intel.com%2Fconfig&amp;data=04%7C01%7Camhetre%40nvidia.com%7C
> 448e9
> >
> 570ac274b7ed2f408d9fd162da7%7C43083d15727340c1b7db39efd9ccc17a%7C
> 0%7C0
> >
> %7C637819094016979779%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQ
> >
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=TO1bX5
> %2FM
> > PhUpf%2BnwSuHkB%2ByLEe4Mdn6Or%2BiZUrbeHpY%3D&amp;reserved=0)
> > compiler: or1k-linux-gcc (GCC) 11.2.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > New smatch warnings:
> > drivers/memory/tegra/mc.c:593 tegra30_mc_handle_irq() error: uninitialized
> symbol 'channel'.
> 
> Ashish,
> 
> I mentioned with your v3 that it is expected for submitter to run certain
> automatic tools:
> "We not only expect to compile it but also compile with W=1, run sparse,
> smatch and coccicheck. Then also test."
> 
> Judging by the output here, it could be that either you used old compiler or did
> not run the checks.
> 
> Can you please confirm that you performed all the activities mentioned before?
> 
Hi Krzysztof,

I had tested the code and verified that MC errors are getting logged as expected
with my changes.
I had also compiled kernel with W=1, ran sparse and made sure that there
aren't any warnings/errors with my changes.
However, I didn't run smatch and coccicheck because I was facing difficulties
in setting up these tools.
I'll make sure that patches in future are scrutinized by all of these tools.

> Best regards,
> Krzysztof

Re: [Patch v4 3/4] memory: tegra: Add memory controller channels support

[Jon Hunter]

On 02/03/2022 19:35, Krzysztof Kozlowski wrote:
> On 02/03/2022 09:43, Ashish Mhetre wrote:
>>  From tegra186 onwards, memory controller support multiple channels.
>> Add support for mapping address spaces of these channels.
>> During error interrupts from memory controller, appropriate registers
>> from these channels need to be accessed for logging error info.
>>
>> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
>> ---
>>   drivers/memory/tegra/mc.c       |  6 ++++++
>>   drivers/memory/tegra/tegra186.c | 21 +++++++++++++++++++++
>>   drivers/memory/tegra/tegra194.c |  1 +
>>   drivers/memory/tegra/tegra234.c |  1 +
>>   include/soc/tegra/mc.h          |  7 +++++++
>>   5 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
>> index bf3abb6..3cda1d9 100644
>> --- a/drivers/memory/tegra/mc.c
>> +++ b/drivers/memory/tegra/mc.c
>> @@ -749,6 +749,12 @@ static int tegra_mc_probe(struct platform_device *pdev)
>>   	if (IS_ERR(mc->regs))
>>   		return PTR_ERR(mc->regs);
>>   
>> +	if (mc->soc->ops && mc->soc->ops->map_regs) {
>> +		err = mc->soc->ops->map_regs(mc, pdev);
>> +		if (err < 0)
>> +			return err;
>> +	}
>> +
>>   	mc->debugfs.root = debugfs_create_dir("mc", NULL);
>>   
>>   	if (mc->soc->ops && mc->soc->ops->probe) {
>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>> index 3d15388..59a4425 100644
>> --- a/drivers/memory/tegra/tegra186.c
>> +++ b/drivers/memory/tegra/tegra186.c
>> @@ -139,11 +139,31 @@ static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
>>   	return 0;
>>   }
>>   
>> +static int tegra186_mc_map_regs(struct tegra_mc *mc,
>> +				struct platform_device *pdev)
>> +{
>> +	struct resource *res;
>> +	int i;
>> +
>> +	mc->mcb_regs = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
>> +	if (IS_ERR(mc->mcb_regs))
>> +		return PTR_ERR(mc->mcb_regs);
>> +
>> +	for (i = 0; i < mc->soc->num_channels; i++) {
>> +		mc->mc_regs[i] = devm_platform_get_and_ioremap_resource(pdev, i + 2, &res);
>> +		if (IS_ERR(mc->mc_regs[i]))
>> +			return PTR_ERR(mc->mc_regs[i]);
> 
> This breaks the ABI, so I need Thierry's ack that such ABI break is
> perfectly ok.


We should not break the DT ABI and so if all the reg entries it should 
still be able to work.

Jon

-- 
nvpublic