Messages in this thread | | | Date | Mon, 26 Sep 2022 15:32:47 +0100 | Subject | Re: [PATCH v1 2/3] drivers/perf: add DesignWare PCIe PMU driver | From | Robin Murphy <> |
| |
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.
| |