lkml.org 
[lkml]   [2019]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/3] interconnect: qcom: Add QCS404 interconnect provider driver
From
Date
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

\
 
 \ /
  Last update: 2019-04-09 12:28    [W:0.053 / U:1.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site