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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPVz0n0TMOCYnMiVUZ7xx-1SqrXuaVCOY-o4-x9L=f-xSMDj8g@mail.gmail.com>
Date: Tue, 3 Feb 2026 18:50:01 +0200
From: Svyatoslav Ryhel <clamor95@...il.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Dmitry Torokhov <dmitry.torokhov@...il.com>, Pavel Machek <pavel@...nel.org>, 
	Arnd Bergmann <arnd@...db.de>, Sebastian Reichel <sre@...nel.org>, 
	Michał Mirosław <mirq-linux@...e.qmqm.pl>, 
	Ion Agorria <ion@...rria.com>, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-input@...r.kernel.org, linux-leds@...r.kernel.org, 
	linux-pm@...r.kernel.org
Subject: Re: [PATCH v1 2/9] misc: Support Asus Transformer's EC access device

вт, 3 лют. 2026 р. о 18:45 Greg Kroah-Hartman <gregkh@...uxfoundation.org> пише:
>
> On Tue, Feb 03, 2026 at 06:28:11PM +0200, Svyatoslav Ryhel wrote:
> > вт, 3 лют. 2026 р. о 14:00 Greg Kroah-Hartman <gregkh@...uxfoundation.org> пише:
> > >
> > > On Tue, Feb 03, 2026 at 01:54:58PM +0200, Svyatoslav Ryhel wrote:
> > > > вт, 3 лют. 2026 р. о 13:41 Greg Kroah-Hartman <gregkh@...uxfoundation.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.
> > >
> >
> > There is no similar way to handle BIN_ATTR_RW in the debugfs (), may I
> > preserve  dockram_read/write with __maybe_unused instead of removing
> > them? I will add comment with explanation
>
> debugfs allows you to do much much more than simple stuff like
> BIN_ATTR_RW().  Go wild there, but don't put debugging stuff in sysfs,
> that is NOT what it is there for at all, but rather, that is exactly
> what debugfs is for.
>

I am removing said stuff from sysfs, that is not what I am asking.
Debugs does not allow to upload register values in a form of binary
block. It allows only dumping via debugfs_create_blob or
debugfs_create_regset32 but not writing. If you know examples of
reading and writing register sets as binary data, please point me to
it.

I am asking if it is possible only to preserve dockram_read/write
functions in the code, without exposing it to sysfs.

> thanks,
>
> greg k-h