2022-04-14 08:36:03

by Ashish Mhetre

[permalink] [raw]
Subject: [Patch v7 0/4] memory: tegra: Add MC channels and error logging

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):
memory: tegra: Add memory controller channels support
memory: tegra: Add MC error logging on tegra186 onward
dt-bindings: memory: Update reg/reg-names validation
arm64: tegra: Add memory controller channels

---
Changes in v7:
- Updated reg-names as per comments on v6
- Removed use of of_property_count_elems_of_size() and used broadcast
reg for checking whether old or new DTS is getting used
- Updated variable names as per comments on v6
- Added helper function for getting global_intstatus bit from channel
- Used to_platform_device() instead of passing pdev pointer to
map_regs()
- Allocated ch_regs at runtime
- Updated DT binding documentation to add validation for reg-names

Changes in v6:
- Added reg-names for each reg item of memory controller node
- Added logging for interrupts on multiple memory controller channels
- Added clearing interrupt support for global intstatus
- Updated DT binding documentation to work with existing DTS as well
- Updated function to get MC channels
- Updated variable names

Changes in v5:
- Updated patch sequence such that driver patches are before DT patches
- Fixed DT ABI break from v4
- Fixed smatch bug
- Updated description in DT binding documentation
- Updated variable names

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

.../nvidia,tegra186-mc.yaml | 80 +++++++++-
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 8 +-
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 24 ++-
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 24 ++-
drivers/memory/tegra/mc.c | 140 ++++++++++++++++--
drivers/memory/tegra/mc.h | 37 ++++-
drivers/memory/tegra/tegra186.c | 47 ++++++
drivers/memory/tegra/tegra194.c | 9 ++
drivers/memory/tegra/tegra234.c | 8 +
include/soc/tegra/mc.h | 7 +
10 files changed, 354 insertions(+), 30 deletions(-)

--
2.17.1


2022-04-14 09:56:46

by Ashish Mhetre

[permalink] [raw]
Subject: [Patch v7 3/4] dt-bindings: memory: Update reg/reg-names validation

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.
Also update validation for reg-names added for these corresponding reg
items.

Signed-off-by: Ashish Mhetre <[email protected]>
---
.../nvidia,tegra186-mc.yaml | 80 +++++++++++++++++--
1 file changed, 74 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
index 13c4c82fd0d3..c7cfa6c2cd81 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
@@ -34,8 +34,12 @@ properties:
- nvidia,tegra234-mc

reg:
- minItems: 1
- maxItems: 3
+ minItems: 6
+ maxItems: 18
+
+ reg-names:
+ minItems: 6
+ maxItems: 18

interrupts:
items:
@@ -142,7 +146,18 @@ allOf:
then:
properties:
reg:
- maxItems: 1
+ maxItems: 6
+ description: 5 memory controller channels and 1 for stream-id registers
+
+ reg-names:
+ maxItems: 6
+ items:
+ - const: sid
+ - const: broadcast
+ - const: ch0
+ - const: ch1
+ - const: ch2
+ - const: ch3

- if:
properties:
@@ -151,7 +166,30 @@ allOf:
then:
properties:
reg:
- minItems: 3
+ minItems: 18
+ description: 17 memory controller channels and 1 for stream-id registers
+
+ reg-names:
+ minItems: 18
+ items:
+ - const: sid
+ - const: broadcast
+ - const: ch0
+ - const: ch1
+ - const: ch2
+ - const: ch3
+ - const: ch4
+ - const: ch5
+ - const: ch6
+ - const: ch7
+ - const: ch8
+ - const: ch9
+ - const: ch10
+ - const: ch11
+ - const: ch12
+ - const: ch13
+ - const: ch14
+ - const: ch15

- if:
properties:
@@ -160,13 +198,37 @@ allOf:
then:
properties:
reg:
- minItems: 3
+ minItems: 18
+ description: 17 memory controller channels and 1 for stream-id registers
+
+ reg-names:
+ minItems: 18
+ items:
+ - const: sid
+ - const: broadcast
+ - const: ch0
+ - const: ch1
+ - const: ch2
+ - const: ch3
+ - const: ch4
+ - const: ch5
+ - const: ch6
+ - const: ch7
+ - const: ch8
+ - const: ch9
+ - const: ch10
+ - const: ch11
+ - const: ch12
+ - const: ch13
+ - const: ch14
+ - const: ch15

additionalProperties: false

required:
- compatible
- reg
+ - reg-names
- interrupts
- "#address-cells"
- "#size-cells"
@@ -182,7 +244,13 @@ examples:

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 */
+ reg-names = "sid", "broadcast", "ch0", "ch1", "ch2", "ch3";
interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;

#address-cells = <2>;
--
2.17.1

2022-04-24 16:55:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [Patch v7 3/4] dt-bindings: memory: Update reg/reg-names validation

On 24/04/2022 07:20, Ashish Mhetre wrote:
>>>
>>> New, added properties cannot be required. That's an ABI break.
>>>
>> This is handled in driver code to make sure driver works with old dts
>> as well. So is this bindings change fine or shall I change it such that
>> dt bindings check shall pass with older dts as well?
>> Or as mentioned by Dmitry, I can update the commit message to reflect
>> that ABI change is intended and driver is compatible with older DTBs as
>> well.
>>
> Hi Rob,
> Can you please confirm how shall I go in next version?
> Is it fine for dt bindings check to fail if driver is compatible with
> old as well as new dts? Or dt bindings check shall pass with old as
> well as new dts?
The driver works fine without reg-names and accepts old DTB, right? In
such case, just mention this in commit msg, that the bindings require
reg-names but backwards compatibility will be preserved in the driver. I
think it's fine to alter bindings such way.

Best regards,
Krzysztof

2022-04-25 09:46:14

by Ashish Mhetre

[permalink] [raw]
Subject: Re: [Patch v7 3/4] dt-bindings: memory: Update reg/reg-names validation



On 4/24/2022 8:04 PM, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 24/04/2022 07:20, Ashish Mhetre wrote:
>>>>
>>>> New, added properties cannot be required. That's an ABI break.
>>>>
>>> This is handled in driver code to make sure driver works with old dts
>>> as well. So is this bindings change fine or shall I change it such that
>>> dt bindings check shall pass with older dts as well?
>>> Or as mentioned by Dmitry, I can update the commit message to reflect
>>> that ABI change is intended and driver is compatible with older DTBs as
>>> well.
>>>
>> Hi Rob,
>> Can you please confirm how shall I go in next version?
>> Is it fine for dt bindings check to fail if driver is compatible with
>> old as well as new dts? Or dt bindings check shall pass with old as
>> well as new dts?
> The driver works fine without reg-names and accepts old DTB, right? In
> such case, just mention this in commit msg, that the bindings require
> reg-names but backwards compatibility will be preserved in the driver. I
> think it's fine to alter bindings such way.
>
Thanks for confirming Krzysztof. I'll send v8 with this information.

> Best regards,
> Krzysztof