| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 14 Apr 2022 09:37:33 +0530
From: Ashish Mhetre <amhetre@...dia.com>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>,
Rob Herring <robh@...nel.org>
Cc: digetx@...il.com, krzysztof.kozlowski@...aro.org,
thierry.reding@...il.com, jonathanh@...dia.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-tegra@...r.kernel.org, krzysztof.kozlowski+dt@...aro.org,
vdumpa@...dia.com, Snikam@...dia.com
Subject: Re: [Patch v7 3/4] dt-bindings: memory: Update reg/reg-names
validation
On 4/14/2022 2:39 AM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> On 4/13/22 19:17, Ashish Mhetre wrote:
>>
>>
>> On 4/13/2022 7:34 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 4/13/22 16:37, Rob Herring wrote:
>>>> On Wed, Apr 13, 2022 at 03:10:11PM +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.
>>>>> Also update validation for reg-names added for these corresponding reg
>>>>> items.
>>>>
>>>> Somehow your subject should indicate this is for Tegra.
>>>>
>>>>>
>>>>> Signed-off-by: Ashish Mhetre <amhetre@...dia.com>
>>>>> ---
>>>>> .../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
>>>>
>>>> You just broke current users.
>>>>
>>>>> + 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
>>>>
>>>> New, added properties cannot be required. That's an ABI break.
>>>>
>>>>> - interrupts
>>>>> - "#address-cells"
>>>>> - "#size-cells"
>>>>> @@ -182,7 +244,13 @@ examples:
>>>>>
>>>>> memory-controller@...0000 {
>>>>> 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
>>>>>
>>>
>>> Oh, wait.. I didn't notice that the new reg ranges are only splitting up
>>> the old ranges. Previously it appeared to me that these are the new
>>> ranges.
>>> > Ashish, in this case you don't need to change the regs in the DT at
>>> all.
>>> Instead, you need to specify the per-channel reg-base offsets in the
>>> driver code.
>>
>> Yes, it's kind of splitting up the old ranges and straight forward for
>> Tegra186. But on Tegra194 and Tegra234 the old address is not in single
>> range. It's already split across 3 ranges. We have to choose right range
>> and add channel offsets to that range in order to read interrupts.
>> So I went with the approach of splitting the regs in DT itself as per
>> the channels because that way they can be mapped in a single loop and
>> used easily.
>> If we want to specify per-channel reg-base offsets then that would be
>> per-SOC. Also we would need to choose correct reg-range for Tegra194 and
>> Tegra234 and have a way to maintain offsets of channels from those
>> respective reg-ranges.
>
> That is not nice too. Should be better to switch to the new DT scheme,
> since those channels weren't used by older kernels. It's okay to change
> the binding ABI in this case then, driver will continue to work for the
> older dtbs.
So the current DTS and binding changes are fine?
> Have you tested driver using the older dtbs?
Yes, the driver is tested with old dtb and it's working fine.
Powered by blists - more mailing lists