Hi Greg, On Wed, Aug 26, 2020 at 01:22:46PM +0200, Greg Kroah-Hartman wrote: > > +#define DEBUGFS_ATTR(__space, __write) \ > > +static int __space ## _open(struct inode *inode, struct file *file) \ > > +{ \ > > + return single_open(file, __space ## _show, inode->i_private); \ > > +} \ > > + \ > > +static const struct file_operations __space ## _fops = { \ > > + .owner = THIS_MODULE, \ > > + .open = __space ## _open, \ > > + .release = single_release, \ > > + .read = seq_read, \ > > + .write = __write, \ > > + .llseek = seq_lseek, \ > > +} > > + > > +#define DEBUGFS_ATTR_RO(__space) \ > > + DEBUGFS_ATTR(__space, NULL) > > + > > +#define DEBUGFS_ATTR_RW(__space) \ > > + DEBUGFS_ATTR(__space, __space ## _write) > > We do have DEFINE_SIMPLE_ATTRIBUTE() and DEFINE_DEBUGFS_ATTRIBUTE, do > those work here instead of your custom macro? Unfortunately they do not work as they want u64 for the write side but we actually need to parse (offset, value) pairs here when write is enabled. > Other than that, this series looks fine to me: > > Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> Thanks!