Re: [PATCH v1 2/9] misc: Support Asus Transformer's EC access device

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Feb 03, 2026 at 01:54:58PM +0200, Svyatoslav Ryhel wrote:
> вт, 3 лют. 2026 р. о 13:41 Greg Kroah-Hartman <gregkh@linuxfoundation.org> пише:
> >
> > On Sun, Feb 01, 2026 at 12:43:36PM +0200, Svyatoslav Ryhel wrote:
> > > --- /dev/null
> > > +++ b/drivers/misc/asus-dockram.c
> > > @@ -0,0 +1,327 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * ASUS EC: DockRAM
> > > + */
> > > +
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/mfd/asus-ec.h>
> > > +#include <linux/mod_devicetable.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/string.h>
> > > +#include <linux/sysfs.h>
> > > +#include <linux/types.h>
> > > +#include <linux/unaligned.h>
> > > +
> > > +struct dockram_ec_data {
> > > +     struct mutex ctl_lock; /* prevent simultaneous access */
> > > +     char ctl_data[DOCKRAM_ENTRY_BUFSIZE];
> > > +};
> > > +
> > > +int asus_dockram_read(struct i2c_client *client, int reg, char *buf)
> > > +{
> > > +     int rc;
> > > +
> > > +     memset(buf, 0, DOCKRAM_ENTRY_BUFSIZE);
> > > +     rc = i2c_smbus_read_i2c_block_data(client, reg, DOCKRAM_ENTRY_BUFSIZE, buf);
> > > +     if (rc < 0)
> > > +             return rc;
> > > +
> > > +     if (buf[0] > DOCKRAM_ENTRY_SIZE) {
> > > +             dev_err(&client->dev, "bad data len; buffer: %*ph; rc: %d\n",
> > > +                     DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> > > +             return -EPROTO;
> > > +     }
> > > +
> > > +     dev_dbg(&client->dev, "got data; buffer: %*ph; rc: %d\n",
> > > +             DOCKRAM_ENTRY_BUFSIZE, buf, rc);
> > > +
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(asus_dockram_read);
> >
> > No documentation for these new public symbols?
> >
> 
> These functions are mainly used in communication between the dockram
> device, asus-ec and its subdevices. Export is used here because all
> mentioned devices can be built as modules. I can add descriptions of
> functions into header if needed, but they should never be used outside
> of dockram-EC complex. Same applies to 2 export functions in the EC
> MFD.

Then you should properly document this :)

> > > +static BIN_ATTR_RW(dockram, DOCKRAM_ENTRIES * DOCKRAM_ENTRY_SIZE);
> > > +static DEVICE_ATTR_RW(control_reg);
> >
> > You did not document your new sysfs files in Documentation/ABI/ which is
> > required.
> >
> > Also, why do you need a brand new user/kernel api at all?  Who is going
> > to use this and for what?
> >
> 
> These api were used mainly for debugging/logging purposes and descend
> from original downstream EC driver. I can both add documentation into
> ABI or remove them if that is absolutely necessary.

Debugging should not be in sysfs, please put this type of stuff into
debugfs instead if you really need it.

thanks,

greg k-h