On Tue, May 12, 2015 at 04:34:03PM +1000, Gavin Shan wrote:
>>
>>>>+ /* Disable Completion Timeout */
>>>>+ if (pcie_cap) {
>>>>+ pnv_pci_cfg_read(pdn, pcie_cap + PCI_EXP_DEVCAP2, 4, &cap2);
>>>>+ if (cap2 & 0x10) {
>>>>+ pnv_pci_cfg_read(pdn, pcie_cap + PCI_EXP_DEVCTL2, 4, &cap2);
>>>>+ cap2 |= 0x10;
>>>>+ pnv_pci_cfg_write(pdn, pcie_cap + PCI_EXP_DEVCTL2, 4, cap2);
>>>>+ }
>>>>+ }
>>>>+
>>>>+ /* Enable SERR and parity checking */
>>>>+ pnv_pci_cfg_read(pdn, PCI_COMMAND, 2, &cmd);
>>>>+ cmd |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>>>>+ pnv_pci_cfg_write(pdn, PCI_COMMAND, 2, cmd);
>>>>+
>>>>+ /* Enable report various errors */
>>>>+ if (pcie_cap) {
>>>>+ pnv_pci_cfg_read(pdn, pcie_cap + PCI_EXP_DEVCTL, 2, &devctl);
>>>>+ devctl &= ~PCI_EXP_DEVCTL_CERE;
>>>>+ devctl |= (PCI_EXP_DEVCTL_NFERE |
>>>>+ PCI_EXP_DEVCTL_FERE |
>>>>+ PCI_EXP_DEVCTL_URRE);
>>>>+ pnv_pci_cfg_write(pdn, pcie_cap + PCI_EXP_DEVCTL, 2, devctl);
>>>>+ }
>>>>+
>>>>+ /* Enable ECRC generation and check */
>>>>+ if (pcie_cap) {
>>>>+ aer_cap = pnv_eeh_find_ecap(pdn, PCI_EXT_CAP_ID_ERR);
>>>>+ pnv_pci_cfg_read(pdn, aer_cap + PCI_ERR_CAP, 4, &aer_capctl);
>>>>+ aer_capctl |= (PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
>>>>+ pnv_pci_cfg_write(pdn, aer_cap + PCI_ERR_CAP, 4, aer_capctl);
>>>>+ }
>>>>+
>>>>+ return 0;
>>>>+}
>>>>+#endif /* CONFIG_PCI_IOV */
>>>>+
>>>
>>>The code is copied over from skiboot firmware. I still dislike the fact that
>>>we have to maintain two sets of similar functions in skiboot/kernel. I still
>>>believe the way I suggested can help: the firmware exports the error routing
>>>rules and kernel has support it based on the rules. With it, the skiboot is
>>>the source of the information to avoid mismatching between kernel/firmware.
>>
>>Yes, it looks we have duplicate code in kernel and skiboot.
>>
>>As you suggest, if we export some bit map from device node, we still have the
>>real logic in kernel, until we remove that part in skiboot.
>>
>>By removing that part in skiboot, we may have some compatibility problem. For
>>example, an old kernel may not run on the new version of skiboot.
>>
>
>It's fine to keep two set code which bear with same rule, which is exported
>from skiboot. In that case, the rule is the only thing we have to care. We
>don't need care the code any more to avoid mismatch between kernel/firmware.
>
Ok, duplication is reasonable, then the major point for this is we need to
have a clear rule for restoring configuration for a device.
Than I suggest we could have another patch set to handle this. Define the rule
clearly and restore the configuration in kernel when skiboot firmware export
such rules.
--
Richard Yang
Help you, Help me
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
