On 06/01/2022 21.37, Arend van Spriel wrote:
> On 1/4/2022 8:26 AM, Hector Martin wrote:
>> +static int brcmf_pcie_read_otp(struct brcmf_pciedev_info *devinfo)
>> +{
>> + const struct pci_dev *pdev = devinfo->pdev;
>> + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev);
>> + u32 coreid, base, words, idx, sromctl;
>> + u16 *otp;
>> + struct brcmf_core *core;
>> + int ret;
>> +
>> + switch (devinfo->ci->chip) {
>> + default:
>> + /* OTP not supported on this chip */
>> + return 0;
>> + }
>
> Does not seem this code is put to work yet. Will dive into it later on.
The specific OTP ranges and cores are added by the subsequent patches
that add support for individual chips, once all the scaffolding is in place.
>
>> + core = brcmf_chip_get_core(devinfo->ci, coreid);
>> + if (!core) {
>> + brcmf_err(bus, "No OTP core\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (coreid == BCMA_CORE_CHIPCOMMON) {
>> + /* Chips with OTP accessed via ChipCommon need additional
>> + * handling to access the OTP
>> + */
>> + brcmf_pcie_select_core(devinfo, coreid);
>> + sromctl = READCC32(devinfo, sromcontrol);
>> +
>> + if (!(sromctl & BCMA_CC_SROM_CONTROL_OTP_PRESENT)) {
>> + /* Chip lacks OTP, try without it... */
>> + brcmf_err(bus,
>> + "OTP unavailable, using default firmware\n");
>> + return 0;
>> + }
>> +
>> + /* Map OTP to shadow area */
>> + WRITECC32(devinfo, sromcontrol,
>> + sromctl | BCMA_CC_SROM_CONTROL_OTPSEL);
>> + }
>> +
>> + otp = kzalloc(sizeof(u16) * words, GFP_KERNEL);
>> +
>> + /* Map bus window to SROM/OTP shadow area in core */
>> + base = brcmf_pcie_buscore_prep_addr(devinfo->pdev, base + core->base);
>
> I guess this changes the bar window...
>
>> + brcmf_dbg(PCIE, "OTP data:\n");
>> + for (idx = 0; idx < words; idx++) {
>> + otp[idx] = brcmf_pcie_read_reg16(devinfo, base + 2 * idx);
>> + brcmf_dbg(PCIE, "[%8x] 0x%04x\n", base + 2 * idx, otp[idx]);
>> + }
>> +
>> + if (coreid == BCMA_CORE_CHIPCOMMON) {
>> + brcmf_pcie_select_core(devinfo, coreid);
>
> ... which is why you need to reselect the core. Otherwise it makes no
> sense to me.
Yes; *technically* with the BCMA_CORE_CHIPCOMMON core the OTP is always
within the first 0x1000 and so I wouldn't have to reselect it, since
it'd end up with the same window, but that is not the case with
BCMA_CORE_GCI used on other chips (where the OTP offset is >0x1000),
although those don't hit this code path. So while this line could be
removed without causing any issues, I find it more orthogonal and safer
to keep the pattern where I select the core before accessing
core-relative fixed registers, and treat brcmf_pcie_buscore_prep_addr as
invalidating the BAR window for all intents and purposes.
--
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub
