linux-kernel - RE: [PATCH v8 2/2] misc: Add iop driver for Sunplus SP7021

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Feb 2022 08:29:40 +0000
From:   Tony Huang 黃懷厚 <tony.huang@...plus.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Tony Huang <tonyhuang.sunplus@...il.com>
CC:     Rob Herring <robh+dt@...nel.org>,
        DTML <devicetree@...r.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        Derek Kiernan <derek.kiernan@...inx.com>,
        Dragan Cvetic <dragan.cvetic@...inx.com>,
        gregkh <gregkh@...uxfoundation.org>,
        Wells Lu 呂芳騰 <wells.lu@...plus.com>
Subject: RE: [PATCH v8 2/2] misc: Add iop driver for Sunplus SP7021

Dear Arnd:

> -----Original Message-----
> From: Arnd Bergmann <arnd@...db.de>
> Sent: Monday, February 7, 2022 3:48 PM
> To: Tony Huang <tonyhuang.sunplus@...il.com>
> Cc: Rob Herring <robh+dt@...nel.org>; DTML <devicetree@...r.kernel.org>;
> Linux Kernel Mailing List <linux-kernel@...r.kernel.org>; Derek Kiernan
> <derek.kiernan@...inx.com>; Dragan Cvetic <dragan.cvetic@...inx.com>; Arnd
> Bergmann <arnd@...db.de>; gregkh <gregkh@...uxfoundation.org>; Tony
> Huang 黃懷厚 <tony.huang@...plus.com>; Wells Lu 呂芳騰
> <wells.lu@...plus.com>
> Subject: Re: [PATCH v8 2/2] misc: Add iop driver for Sunplus SP7021
> 
> On Mon, Feb 7, 2022 at 7:30 AM Tony Huang
> <tonyhuang.sunplus@...il.com> wrote:
> >
> > IOP(8051) embedded inside SP7021 which is used as Processor for I/O
> > control, monitor RTC interrupt and cooperation with CPU & PMC in power
> > management purpose.
> > The IOP core is DQ8051, so also named IOP8051, it supports dedicated
> > JTAG debug pins which share with SP7021.
> > In standby mode operation, the power spec reach 400uA.
> >
> > Signed-off-by: Tony Huang <tonyhuang.sunplus@...il.com>
> > ---
> > Changes in v8:
> >  - Addressed comments from Greg KH.
> >
> >  Documentation/ABI/testing/sysfs-platform-soc@B |  28 ++
> >  MAINTAINERS                                    |   2 +
> >  drivers/misc/Kconfig                           |  20 ++
> >  drivers/misc/Makefile                          |   1 +
> >  drivers/misc/sunplus_iop.c                     | 463
> +++++++++++++++++++++++++
> >  5 files changed, 514 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-platform-soc@B
> >  create mode 100644 drivers/misc/sunplus_iop.c
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-soc@B
> > b/Documentation/ABI/testing/sysfs-platform-soc@B
> > new file mode 100644
> > index 0000000..d26d6f5
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-platform-soc@B
> > @@ -0,0 +1,28 @@
> > +What:
> /sys/devices/platform/soc@...c000400.iop/sp_iop_mailbox
> > +Date:          January 2022
> > +KernelVersion: 5.16
> > +Contact:       Tony Huang <tonyhuang.sunplus@...il.com>
> > +Description:
> > +               Show IOP's mailbox0 register data.
> > +               Format: %x
> > +
> > +What:
> /sys/devices/platform/soc@...c000400.iop/sp_iop_mode
> > +Date:          January 2022
> > +KernelVersion: 5.16
> > +Contact:       Tony Huang <tonyhuang.sunplus@...il.com>
> > +Description:
> > +               Read-Write.
> > +
> > +               Write this file.
> > +               Operation mode of IOP is switched to standby mode by
> writing
> > +               "1" to sysfs.
> > +               Operation mode of IOP is switched to normal mode by
> writing
> > +               "0" to sysfs.
> > +               Writing of other values is invalid.
> > +
> > +               Read this file.
> > +               Show operation mode of IOP. "0" is normal mode. "1" is
> standby
> > +               mode.
> > +               Format: %x
> 
> As discussed before, I would suggest leaving out all custom attributes for now,
> and first hooking up the driver to all the in-kernel subsystems.
> 
> The mailbox0 register data definitely feels like an implementation detail, not
> something that should be exposed to user space as an interface.
> 
> For standby mode, this would normally be handled by the power management
> subsystem in the kernel. not a custom interface. From your earlier description,
> I assume this interface puts the main CPU into standby mode, not the IOP,
> right?
> 
> CPU standby is handled by the cpuidle subsystem, so you need a driver in
> drivers/cpuidle/ to replace your sysfs attribute.
> If you plan to hook up the driver to multiple subsystems, keeping a generic
> driver file is ok, so  you'll end up with two driver modules, with one of them
> calling into the other, using
> EXPORT_SYMBOL() to link between them.
> 

The purpose of adding sysfs is only for users to debug.
So this is not needed?

Thanks