On Fri, Nov 25, 2022 at 01:12:18PM +0100, Krzysztof Kozlowski wrote: > On 25/11/2022 11:34, Lad, Prabhakar wrote: > >>> +/* Device, Non-bufferable */ > >>> +#define AX45MP_PMACFG_MTYP_DEV_NON_BUF (0 << 2) > >>> +/* Device, bufferable */ > >>> +#define AX45MP_PMACFG_MTYP_DEV_BUF (1 << 2) > >>> +/* Memory, Non-cacheable, Non-bufferable */ > >>> +#define AX45MP_PMACFG_MTYP_MEM_NON_CACHE_NON_BUF (2 << 2) > >>> +/* Memory, Non-cacheable, Bufferable */ > >>> +#define AX45MP_PMACFG_MTYP_MEM_NON_CACHE_BUF (3 << 2) > >> > >> What are all these? They don't look like flags, because 3 = 1 | 2... > >> they don't look like constants, because we do not use shifts in > >> constants. Are these some register values? I also do not see the header > >> being used in the code, so why having a bindings header if it is not > >> used (DTS is not usage...)? > >> > > These are register bit values for the MTYP[5:2] field. The DTS example > > in the binding doc (above) uses these macros. I haven't included the > > DTS/I patches with this patchset yet do think I should? > > Then why storing it as bindings? Bindings headers describe the interface > implemented by drivers and used by DTS, but this is not implemented by > drivers. IIUC, some of these properties are non-discoverable attributes of the cache controller. I see two things that could be done here that are "better" than #defining bits: - add an RZ/Five specific compatible and use match data to set the attributes which is only possible if the pma-regions are set on a per SoC basis - make pma-regions into a child node, in which andestech,non-cacheable andestech,non-bufferable etc are properties of the child node Prabhakar, does that make sense or am I off with my understanding of the attributes? Thanks, Conor.
