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] [day] [month] [year] [list]

Message-ID: <CAK8P3a12JMhEB=qxXqRB0WFyUwvJ_VgwqMi6oWfpcgBr9OWrqQ@mail.gmail.com>
Date:   Mon, 7 Feb 2022 10:33:40 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     gregkh <gregkh@...uxfoundation.org>
Cc:     Tony Huang 黃懷厚 <tony.huang@...plus.com>,
        Arnd Bergmann <arnd@...db.de>,
        Tony Huang <tonyhuang.sunplus@...il.com>,
        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>,
        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 9:39 AM gregkh <gregkh@...uxfoundation.org> wrote:
> On Mon, Feb 07, 2022 at 08:29:40AM +0000, Tony Huang 黃懷厚 wrote:
> > >
> > > 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?
>
> If this is only for debugging, please put it in debugfs and not sysfs.

I don't think that works for the idle mode interface, as this is the only
thing the driver really does at the moment.

In a previous review round, I already asked for the driver to implement
at least two in-kernel interfaces before any custom user interface
is added.

         Arnd