3 comments:
File src/soc/mediatek/mt8192/devapc.c:
Patch Set #46, Line 6: static unsigned long devapc_ao_base;
I think relying on this static variable is not a good idea.
Let's change this to:
static uint32_t *getreg(uintptr_t base, unsigned offset)
{
return (uint32_t *)(base + offset);
}
Then you can do the init as:
static void infra_master_init(uintptr_t base)
{
SET32_BITFLIELDS(getreg(base, MAS_SEC), ...);
...
}
That's more clear to read and understand.
Patch Set #46, Line 43: devapc_ao_base
this can be a local variable.
File src/soc/mediatek/mt8192/include/soc/devapc.h:
#define AO_APC_CON (devapc_ao_base + (unsigned long)OFF_AO_APC_CON)
#define MAS_SEC (devapc_ao_base + (unsigned long)OFF_MAS_SEC_0)
#define MAS_DOM_0 (devapc_ao_base + (unsigned long)OFF_MAS_DOM_0)
#define MAS_DOM_1 (devapc_ao_base + (unsigned long)OFF_MAS_DOM_1)
I think this coding style (have define in header that uses some static variable inside c, and didn't use the variable explicitly in c) is confusing. Let's move them to C file.
To view, visit change 46402. To unsubscribe, or for help writing mail filters, visit settings.