On Wed, Sep 10, 2025 at 11:24:20AM -0500, Bowman, Terry wrote:
> On 8/28/2025 3:18 AM, Alejandro Lucero Palau wrote:
> > On 8/27/25 02:35, Terry Bowman wrote:
> >> +static void set_pcie_cxl(struct pci_dev *dev)
> >> +{
> >> + struct pci_dev *parent;
> >> + u16 dvsec = pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
> >> + PCI_DVSEC_CXL_FLEXBUS_PORT);
> >> + if (dvsec) {
> >> + u16 cap;
> >> +
> >> + pci_read_config_word(dev, dvsec + PCI_DVSEC_CXL_FLEXBUS_STATUS_OFFSET, &cap);
> >> +
> >> + dev->is_cxl = FIELD_GET(PCI_DVSEC_CXL_FLEXBUS_STATUS_CACHE_MASK, cap) ||
> >> + FIELD_GET(PCI_DVSEC_CXL_FLEXBUS_STATUS_MEM_MASK, cap);
> >> + }
> >> +
> >> + if (!pci_is_pcie(dev) ||
> >> + !(pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT ||
> >> + pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM))
> >> + return;
> >> +
> >> + parent = pci_upstream_bridge(dev);
> >> + set_pcie_cxl(parent);
> >
> > This recursion is confusing to me.
> >
> > Is it not the parent already having this set from its own pci setup? Or
> > maybe do we expect that to change after a reset and this is a sanity check?
>
> Right. The upstream parent bus state is already set but could change after
> reset.
Please add a code comment as this is non-obvious at least to me.
Thanks,
Lukas
