On Mon, Oct 19, 2026 at 12:11 PM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Low] HID: spi-hid: multi-value sysfs attributes and missing ABI > documentation > -- > > commit 4c66f3d16c395b79d0997b29357fb80b5a6f2708 > Author: Angela Czubak <[email protected]> > > HID: spi-hid: add transport driver skeleton for HID over SPI bus > > This commit creates the spi-hid directory with its Kconfig and Makefile, and > adds the initial core driver skeleton including basic device structures, > sysfs attributes, and probe/remove functions. > > > diff --git a/drivers/hid/spi-hid/spi-hid-core.c > > b/drivers/hid/spi-hid/spi-hid-core.c > > new file mode 100644 > > index 0000000000000..02a7608c4b882 > > --- /dev/null > > +++ b/drivers/hid/spi-hid/spi-hid-core.c > > @@ -0,0 +1,213 @@ > > [ ... ] > > > +static ssize_t bus_error_count_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct spi_hid *shid = dev_get_drvdata(dev); > > + > > + return sysfs_emit(buf, "%u (%d)\n", > > + shid->bus_error_count, shid->bus_last_error); > > +} > > +static DEVICE_ATTR_RO(bus_error_count); > > + > > +static ssize_t regulator_error_count_show(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct spi_hid *shid = dev_get_drvdata(dev); > > + > > + return sysfs_emit(buf, "%u (%d)\n", > > + shid->regulator_error_count, > > + shid->regulator_last_error); > > +} > > +static DEVICE_ATTR_RO(regulator_error_count); > > + > > +static ssize_t device_initiated_reset_count_show(struct device *dev, > > + struct device_attribute > > *attr, > > + char *buf) > > +{ > > + struct spi_hid *shid = dev_get_drvdata(dev); > > + > > + return sysfs_emit(buf, "%u\n", shid->dir_count); > > +} > > +static DEVICE_ATTR_RO(device_initiated_reset_count); > > + > > +static struct attribute *spi_hid_attrs[] = { > > + &dev_attr_bus_error_count.attr, > > + &dev_attr_regulator_error_count.attr, > > + &dev_attr_device_initiated_reset_count.attr, > > + NULL /* Terminator */ > > +}; > > + > > +static const struct attribute_group spi_hid_group = { > > + .attrs = spi_hid_attrs, > > +}; > > + > > +const struct attribute_group *spi_hid_groups[] = { > > + &spi_hid_group, > > + NULL > > +}; > > +EXPORT_SYMBOL_GPL(spi_hid_groups); > > [Severity: Low] > In drivers/hid/spi-hid/spi-hid-core.c, bus_error_count_show() and > regulator_error_count_show() format both an unsigned error count and a signed > last error code in parentheses ("%u (%d)\n") into a single sysfs file when > userspace reads bus_error_count or regulator_error_count via spi_hid_groups > (which is attached to .dev_groups in the later ACPI and Device Tree support > commits in this series). > > Per Documentation/filesystems/sysfs.rst, sysfs attributes should export one > value per file without custom formatting or mixing types, and new sysfs > attributes should be documented in Documentation/ABI/. > > Should bus_last_error and regulator_last_error be split into separate sysfs > attributes (or moved to debugfs), and should Documentation/ABI/ entries be > added for the attributes in spi_hid_attrs?
Will drop these attributes in v7. Already reported via dev_err() and tracepoints. > > [ ... ] > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=3
