Hi Bjorn, Sorry, I forgot to mention... On Tue, Dec 21, 2021 at 5:21 PM Rajat Jain <rajatja@xxxxxxxxxx> wrote: > > Some devices have an errata such that they only support DWORD accesses > to some registers. > > For e.g. this Bayhub O2 device ([VID:DID] = [0x1217:0x8621]) only > supports DWORD accesses to LTR latency registers and L1 PM substates > control registers: > https://github.com/rajatxjain/public_shared/blob/main/OZ711LV2_appnote.pdf > > Since L1 PM substate control registers are DWORD sized, and hence their > access in the kernel is already DWORD sized, so we don't need to do > anything for them. > > However, the LTR registers being WORD sized, are in need of a solution. > This patch converts the WORD sized accesses to these registers, into > DWORD sized accesses, while saving and restoring them. > > Signed-off-by: Rajat Jain <rajatja@xxxxxxxxxx> ... I have made a more generic proposal here: https://lore.kernel.org/linux-pci/CACK8Z6GR9pR0Oc_G70q2MxTVgrk+PXWTFbWT1EjARnuYCRavLw@xxxxxxxxxxxxxx/T/#t However that is a bigger effort in terms of implementation as well as reaching an agreement and acceptance with the community. So I'd appreciate it if this patch would please be included until a larger framework is put in place. Thanks & Best Regards, Rajat > --- > drivers/pci/pci.c | 24 ++++++++++++++++-------- > drivers/pci/pcie/aspm.c | 1 + > 2 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 3d2fb394986a..efa8cd16827f 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1556,7 +1556,7 @@ static void pci_save_ltr_state(struct pci_dev *dev) > { > int ltr; > struct pci_cap_saved_state *save_state; > - u16 *cap; > + u32 *cap; > > if (!pci_is_pcie(dev)) > return; > @@ -1571,25 +1571,33 @@ static void pci_save_ltr_state(struct pci_dev *dev) > return; > } > > - cap = (u16 *)&save_state->cap.data[0]; > - pci_read_config_word(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap++); > - pci_read_config_word(dev, ltr + PCI_LTR_MAX_NOSNOOP_LAT, cap++); > + /* > + * We deliberately do a dword access to save both PCI_LTR_MAX_SNOOP_LAT > + * and PCI_LTR_MAX_NOSNOOP_LAT together since some devices only support > + * dword accesses to these registers. > + */ > + cap = &save_state->cap.data[0]; > + pci_read_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap); > } > > static void pci_restore_ltr_state(struct pci_dev *dev) > { > struct pci_cap_saved_state *save_state; > int ltr; > - u16 *cap; > + u32 *cap; > > save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR); > ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR); > if (!save_state || !ltr) > return; > > - cap = (u16 *)&save_state->cap.data[0]; > - pci_write_config_word(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, *cap++); > - pci_write_config_word(dev, ltr + PCI_LTR_MAX_NOSNOOP_LAT, *cap++); > + /* > + * We deliberately do a dword access to restore both > + * PCI_LTR_MAX_SNOOP_LAT and PCI_LTR_MAX_NOSNOOP_LAT together since > + * some devices only support dword accesses to these registers. > + */ > + cap = &save_state->cap.data[0]; > + pci_write_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, *cap); > } > > /** > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 52c74682601a..083f47a7b69b 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -496,6 +496,7 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link, > encode_l12_threshold(l1_2_threshold, &scale, &value); > ctl1 |= t_common_mode << 8 | scale << 29 | value << 16; > > + /* Always make DWORD sized accesses to these registers */ > pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, &pctl1); > pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, &pctl2); > pci_read_config_dword(child, child->l1ss + PCI_L1SS_CTL1, &cctl1); > -- > 2.34.1.307.g9b7440fafd-goog >
