Yes - I got this one wrong. Nevermind, looks good. Just the punctuation NIT. On Wed, Jun 09, 2021 at 05:36:26PM -0500, Shanker R Donthineni wrote: > Hi Raphael, > > On 6/9/21 4:57 PM, Raphael Norwitz wrote: > >> +static ssize_t reset_method_show(struct device *dev, > >> + struct device_attribute *attr, > >> + char *buf) > >> +{ > >> + struct pci_dev *pdev = to_pci_dev(dev); > >> + ssize_t len = 0; > >> + int i, prio; > >> + > >> + for (prio = PCI_RESET_METHODS_NUM; prio; prio--) { > >> + for (i = 0; i < PCI_RESET_METHODS_NUM; i++) { > >> + if (prio == pdev->reset_methods[i]) { > >> + len += sysfs_emit_at(buf, len, "%s%s", > >> + len ? "," : "", > >> + pci_reset_fn_methods[i].name); > >> + break; > >> + } > >> + } > >> + > >> + if (i == PCI_RESET_METHODS_NUM) > >> + break; > >> + } > >> + > > Don't you still need to ensure you add the newline even if there are no > > reset methods set? If the len is zero why don't we need the newline? > > > > Otherwise looks good. > > > > sysfs entry 'reset_method' will not be visible if there are no reset methods. > >