[coreboot-gerrit] Change in coreboot[master]: soc/intel/jasperlake: Correct sequence for gpio pad group

[Maulik V Vaghela (Code Review)]

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46842 )

Change subject: soc/intel/jasperlake: Correct sequence for gpio pad group
......................................................................

Patch Set 2:

Hi Karthik,

Regarding gpio register offset calculation, you're right. It won't matter much from ACPI perspective.

Issue which we're seeing is regarding "Host Software Pad Ownership" offset calculation. Now, here is the snippet of code which does this calculation.
      hostsw_own_offset = comm->host_own_reg_0;
      hostsw_own_offset += gpio_group_index_scaled(comm, pin, sizeof(uint32_t));

host_own_reg_0 is base which is defined in src/soc/intel/jasperlake/gpio.c which is 0xC0

now, we need to calculate gpio group index which will determine correct offset.
      gpio_group_index_scaled = gpio_group_index(comm, relative_pad) * scale;  // here scale = 4

Here is the logic which determines gpio group index within community.
       for (i = 0; i < comm->num_groups; i++) {
                 if (relative_pad >= comm->groups[i].first_pad &&
                         relative_pad < comm->groups[i].first_pad +
                         comm->groups[i].size) {
                         return i;
                 }
         }

Now since this is for VGPIO community, we had defined community group array for comm1 as below
  static const struct pad_group jsl_community1_groups[] = {
         INTEL_GPP_BASE(GPP_H0, GPP_H0, GPP_H23, 160),           /* GPP_H */
         INTEL_GPP_BASE(GPP_H0, GPP_D0, GPP_D23, 192),   /* GPP_D */
         INTEL_GPP(GPP_H0, GPIO_RSVD_12, GPIO_RSVD_13),
         INTEL_GPP_BASE(GPP_H0, VGPIO_0, VGPIO_39, 224),         /* VGPIO */
         INTEL_GPP_BASE(GPP_H0, GPP_C0, GPP_C23, 256),           /* GPP_C */
   };

Because of this, VGPIO index will be returned as 3 (not as 2), which will calculate gpio_group_index_scaled = 4 *3 = 0xC
This will result in offset 0xCC but actual offset of VGPIO community is 0xC8 as per EDS. This is the reason we want to move reserved GPIO at last to prevent miscalculations of host own GPIO register offset. Ideally it would only affect gpios which are configured in GPIO_DRIVER mode but correcting for all communities so that we don't introduce this issue unknowingly in future.

Sorry for long reply but I hope it clarifies it properly.😊