linux-kernel - Re: [PATCH 2/3] interconnect: qcom: Add QCS404 interconnect provider driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <63aaf5f5-4750-f1e9-f82c-9fff3e6c9214@linaro.org>
Date:   Tue, 9 Apr 2019 13:27:49 +0300
From:   Georgi Djakov <georgi.djakov@...aro.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     robh+dt@...nel.org, vkoul@...nel.org, evgreen@...omium.org,
        daidavid1@...eaurora.org, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/3] interconnect: qcom: Add QCS404 interconnect provider
 driver

Hi Bjorn,

On 4/9/19 06:27, Bjorn Andersson wrote:
> On Mon 08 Apr 07:33 PDT 2019, Georgi Djakov wrote:
>> On 4/5/19 17:57, Bjorn Andersson wrote:
>>> On Fri 05 Apr 10:54 +07 2019, Georgi Djakov wrote:
>>> [..]
> [..]
>>>> diff --git a/drivers/interconnect/qcom/qcs404_ids.h b/drivers/interconnect/qcom/qcs404_ids.h
>>>
>>> You use these defines in the driver, so I think this file should be the
>>> one in include/dt-bindings...
>>
>> The ids in this header are in a single global namespace in order to
>> build the internal topology and could be used for drivers that support
>> only platform data (although not sure if there would be any).
>>
> 
> As you say these numbers could be used by drivers on non-DT enabled
> platforms, but for that this include file should be in
> include/linux/interconnect. That said, there are no such Qualcomm
> platforms, so these numbers will only ever be used internally in the
> qcs404.c provider, so it would be better to just define them in that
> file - to remove the risk of confusion.

Agreed, will put them in the same file.

>>>
>>> [..]
>>>> diff --git a/include/dt-bindings/interconnect/qcom,qcs404.h b/include/dt-bindings/interconnect/qcom,qcs404.h
>>>
>>
>> These header is using per NoC local ids and should be used on DT enabled
>> platforms.
>>
> 
> I had missed that you implemented support for xlating NoC-specific ids,
> so this makes sense now, nice. I presume we won't ever include files in
> a way where these defines collide - so this looks good.
Thanks for the review!
Georgi