Patch Set 1:

Patch Set 1:

Patch Set 1:

also another suggestion, could be another patch, this is already so much better than just raw constants, but would be nice to just specify GPP_A10 for example in the bias_control fields; that would mean decoding the bank and pin numbers though.

ya, i was looking for a way to decode the pin numbers. is that done on the FSP side?

I don't know of a great way, other than
```
if (gpio_num >= GPP_A0 && gpio_num <= GPP_A24) {
      bank = GPIO_BANK_A;
} else if (gpio_num >= GPP_B0 && gpio_num <= GPP_B23) {
      bank = GPIO_BANK_B;
}
```

or the equivalent as a table + loop. I haven't looked much beyond the UPD itself TBH

We have most of the required code already present in soc/intel/common/block/gpio/gpio.c. I would recommend looking into that:

const struct pad_community *comm = gpio_get_community(pad); // pad would be what you input from the mainboard e.g. GPP_E10.
int rel_offset = relative_pad_in_comm(comm, pad);

That should give you the relative offset that needs to be filled in. For the group(bank) encoding, has anyone checked with Intel how GPP_E translates to 0xe? I don't think there is any hardware register that defines this. I fear that we are leaking FSP assumptions/macros into coreboot here. Anyways, this can be handled by:

int group_idx = gpio_group_index(comm, rel_offset);

and then have a helper function that can return the bank # expected by FSP. Either you can have that implemented only for TGL now or update pad_group to add a field for fsp_bank_number. We should bring this up with FSP team as well because we cannot just keep expecting users to understand/know the internal assumptions of FSP w.r.t. macros or ids.

View Change

To view, visit change 47860. To unsubscribe, or for help writing mail filters, visit settings.