> > +static u8 cfg_reg_idx[] = {
> > + /* 0 to 18 */
> > + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
> > + /* 64 to 113 */
> > + 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80,
> > + 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97,
> > + 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111,
> > + 112, 113,
> > +};
> This [1] says "Configure GENI primitive table.", and I have a sneaky suspicion
> that only the first part is actually that (iirc the Linux driver even errors
> out on writes to anything >256? I don't see any bound checking in u-boot,
> which is worrying...).
I see that the same loop is used both in U-Boot [1] and Linux [2].
> Also, these are registers with names.
For some reason, this space is arranged as 2 blocks.
1. 0 - 18 regs, from base to base+0x4c
2. 64 - 113 regs, from base+0x200 to base+0x2c4
These registers have the name GENI_CFG_REG_n with 'n' ranging
from 0 to max as aliases and that has been used. Since we are
dumping the content to these registers don't think names are
needed here. If the registers were in a single block instead of
the above arrangment it is just a memcpy_toio().
> Since the firmware format can't be changed and this just mimics it, I'd say
> it would make sense to improve that comment to explain what it's actually
> doing (since it's setting more than just primitives, whatever that means),
We are not privy to the information that is held in this array.
We just get this code from the hardware block owners.
> and if these indices are going to be in this array anyway, why not replace
> them with defines of the register names? Or are those SoC-specific,
> seems that whatever ones the driver is accessing by name are pretty
> stable at least.
Registers accessed by name are not part of this block.
[1] https://github.com/u-boot/u-boot/blob/master/drivers/misc/qcom_geni.c#L204
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/qcom/qcom-geni-se.c#n1251
Thanks
Varada
> [ . . . ]
