lkml.org 
[lkml]   [2022]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v1 2/3] drivers/perf: add DesignWare PCIe PMU driver
From
On 2022-09-26 14:31, Shuai Xue wrote:
> + Bjorn Helgaas
>
> 在 2022/9/23 PM11:54, Jonathan Cameron 写道:
>>
>>>
>>>>
>>>>> +#define RP_NUM_MAX 32 /* 2die * 4RC * 4Ctrol */
>>>>
>>>> This driver is 'almost' generic. So if you an avoid defines based on a particular
>>>> platform that's definitely good!
>>>
>>> Good idea. How about defining RP_NUM_MAX as 64? As fars as I know,
>>> some platfrom use 2 sockets, 2 die per socket.
>>> Then 2 sockets * 2 dies * 4 Root Complex * 4 root port.
>>
>> Setting a reasonable maximum is fine - but make sure the code then fails with
>> a suitable error message if there are more!
>
> OK, I will add a discovery logic here and count PMU number at runtime.
>
>>
>>
>>>>> +#define DWC_PCIE_LANE_SHIFT 4
>>>>> +#define DWC_PCIE_LANE_MASK GENMASK(9, 4)
>>>>> +
>>>>> +#define DWC_PCIE_EVENT_CNT_CTRL 0x8
>>>>> +#define DWC_PCIE__CNT_EVENT_SELECT_SHIFT 16
>>>>
>>>> Why double __? If point is , then
>>>> naming works better
>>>> DWC_PCIE_EVENT_CNT_CTRL_REG
>>>> DWC_PCIE_EVENT_CNT_CTRL_EV_SELECT_MSK etc
>>>
>>> Yes, I point to use double `__` to indicate it is a field of register,
>>> as CMN and CCN drivers do. I also considered naming with REG explicitly,
>>> but the macro is so long that I often have to wrap code into multilines.
>>> Any way, it's fine to rename if you still suggest to do so.
>>
>> I don't particularly mind. This convention was new to me.
>
> Haha, then I will leave the double `__` as CMN and CCN drivers do.

FWIW I'm not sure there's really any convention. CCN seems to use
double-underscores as distinct separators in a consistent
CCN_REG_NAME__FIELD_NAME__SUFFIX pattern. Conversely in CMN I used it as
an indication of the usual CMN_REG_NAME_FIELD_NAME_VALUE pattern being
abbreviated where it would have been uncomfortably long otherwise (and
particularly where the field name reflects the register name anyway); it
just seemed like a good visual cue to imply that something was missing.

Robin.

\
 
 \ /
  Last update: 2022-09-26 17:48    [W:0.084 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site