linux-kernel - Re: [Patch v6 1/4] memory: tegra: Add memory controller channels support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <16d5c86b-cb04-5f57-7923-724850ce2633@nvidia.com>
Date:   Mon, 11 Apr 2022 12:58:19 +0530
From:   Ashish Mhetre <amhetre@...dia.com>
To:     Dmitry Osipenko <dmitry.osipenko@...labora.com>,
        Dmitry Osipenko <digetx@...il.com>,
        krzysztof.kozlowski@...aro.org, thierry.reding@...il.com,
        jonathanh@...dia.com, robh+dt@...nel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-tegra@...r.kernel.org, krzysztof.kozlowski+dt@...aro.org
Cc:     vdumpa@...dia.com, Snikam@...dia.com
Subject: Re: [Patch v6 1/4] memory: tegra: Add memory controller channels
 support



On 4/11/2022 12:03 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 4/11/22 09:05, Ashish Mhetre wrote:
>>
>>
>> On 4/10/2022 7:48 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 06.04.2022 08:24, Ashish Mhetre пишет:
>>>> +     num_dt_channels =
>>>> of_property_count_elems_of_size(pdev->dev.of_node, "reg",
>>>> +                                                       reg_cells *
>>>> sizeof(u32));
>>>> +     /*
>>>> +      * On tegra186 onwards, memory controller support multiple
>>>> channels.
>>>> +      * Apart from regular memory controller channels, there is one
>>>> broadcast
>>>> +      * channel and one for stream-id registers.
>>>> +      */
>>>> +     if (num_dt_channels < mc->soc->num_channels + 2) {
>>>> +             dev_warn(&pdev->dev, "MC channels are missing, please
>>>> update memory controller DT node with MC channels\n");
>>>> +             return 0;
>>>> +     }
>>>> +
>>>> +     mc->bcast_ch_regs = devm_platform_ioremap_resource_byname(pdev,
>>>> "mc-broadcast");
>>>> +     if (IS_ERR(mc->bcast_ch_regs))
>>>> +             return PTR_ERR(mc->bcast_ch_regs);
>>>
>>> Looks to me that you don't need to use of_property_count_elems_of_size()
>>> and could only check the "mc-broadcast" presence to decide whether this
>>> is an older DT.
>>>
>> Now that we are using reg-names in new DT, yes it'd be fine to just
>> check mc-broadcast to decide it's a new or old DT.
>>
>>> mc->bcast_ch_regs = devm_platform_ioremap_resource_byname(pdev,
>>> "broadcast");
>>> if (IS_ERR(mc->bcast_ch_regs)) {
>>>           dev_warn(&pdev->dev, "Broadcast channel is missing, please
>>> update your
>>> device-tree\n");
>>>           return PTR_ERR(mc->bcast_ch_regs);
>>> }
>>
>> return 0;
>>
>> to avoid DT ABI break, right?
> 
> Yes, it should be "return 0".

But if we "return 0" from here, then what about the case when ioremap()
actually fails with new DT i.e. when broadcast reg is present in DT?
In that case error should be returned and probe should be failed, right?