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
