On 10/25/2019 5:34 PM, Andrew Murray wrote:
+/*
+ * Link width change on the fly is not always successful.
+ * It also depends on the partner.
+ */
+static ssize_t pcie_width_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ lpp = dev_get_drvdata(dev);
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > lpp->max_width)
+ return -EINVAL;
+
+ /* HW auto bandwidth negotiation must be enabled */
+ pcie_rc_cfg_wr_mask(lpp, PCI_EXP_LNKCTL_HAWD, 0,
+ PCIE_CAP_OFST + PCI_EXP_LNKCTL);
+ dw_pcie_link_width_resize(&lpp->pci, val);
+
+ return len;
+}
+static DEVICE_ATTR_WO(pcie_width);
+
+static struct attribute *pcie_cfg_attrs[] = {
+ &dev_attr_pcie_link_status.attr,
+ &dev_attr_pcie_speed.attr,
+ &dev_attr_pcie_width.attr,
+ NULL,
+};
Is there a reason that these are limited only to the Intel driver and
not the wider set of DWC drivers?
Is there anything specific here about the Intel GW driver?
Yes, they need intel_pcie_max_speed_setup() and pcie_link_gen_to_str().
Once intel_pcie_max_speed_setup() moved to DesignWare framework (as per
Bjorn Helgaas inputs) and use pcie_link_speed[] array instead of
pcie_link_gen_to_str() (as per gustavo pimentel inputs) we can move this to
PCIe DesignWare framework or to pci sysfs file.
I think the key concern here is this: If you introduce sysfs controls that
represent generic PCI concepts (such as changing the link speed) - the concept
isn't limited to a particular host controller, it's limited to PCI. Therefore
the sysfs control should also apply more widely to all PCI controllers. This
is important as otherwise you may end up getting a slightly different user
interface to achieve the same consequence depending on the host-controller in
use.
If each controller driver has a different way of doing things, then it lends
itself to having some set of ops that they can all implement. Or perhaps there
is a middle-ground solution where this applies just to DWC devices and not all
devices.
I see the better way is to move the implementation to PCIe DesignWare
framework because of
the registers programming.
Regards,
Dilip
Thanks,
Andrew Murray
