On Fri, Apr 29, 2016 at 11:20:26AM -0500, Bjorn Helgaas wrote:
> On Tue, Apr 19, 2016 at 06:29:18AM +0000, Yong, Jonathan wrote:
> > ...
> > +static ssize_t ptm_status_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + u16 word;
> > + int pos;
> > +
> > + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
> > + if (!pos)
> > + return -ENXIO;
> > +
> > + pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
>
> This is a 32-bit register; read it as a dword, even though the upper
> bits are currently reserved.
>
> > + return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
> > +}
> > +
> > +static ssize_t ptm_status_store(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + unsigned long val;
> > + ssize_t ret;
> > +
> > + ret = kstrtoul(buf, 0, &val);
> > + if (ret)
> > + return ret;
> > + if (val)
> > + return pci_enable_ptm(pdev);
> > + pci_disable_ptm(pdev);
> > + return 0;
> > +}
> > +
> > +static DEVICE_ATTR_RW(ptm_status);
Thinking about this sysfs interface some more... Do you think we
really need a knob in sysfs? What's the use case for it? I suppose
there's potentially a performance impact because we'll send more
messages across the link, but I'd be surprised if it were measurable.
If it's basically free, there shouldn't be a need to enable/disable it
individually, except maybe to work around hardware bugs. A kernel
boot parameter might be enough for that. Even a kernel boot parameter
should be regarded as a debug/testing tool, not a solution. If we
find broken devices, we should add quirks for them so users don't need
to use a boot parameter.
Bjorn
--
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
