netdev - Re: [PATCH] ptp: ocp: don't allow on S390

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]

Message-ID: <20210825204042.5v7ad3ntor6s3pq3@bsd-mbp.dhcp.thefacebook.com>
Date:   Wed, 25 Aug 2021 13:40:42 -0700
From:   Jonathan Lemon <jonathan.lemon@...il.com>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     Arnd Bergmann <arnd@...nel.org>,
        Richard Cochran <richardcochran@...il.com>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH] ptp: ocp: don't allow on S390

On Wed, Aug 25, 2021 at 10:29:51AM -0700, Randy Dunlap wrote:
> On 8/25/21 10:08 AM, Jonathan Lemon wrote:
> > On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote:
> > > On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap <rdunlap@...radead.org> wrote:
> > > > 
> > > > On 8/20/21 8:31 AM, Richard Cochran wrote:
> > > > > On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote:
> > > > > 
> > > > > > I would also suggest removing all the 'imply' statements, they
> > > > > > usually don't do what the original author intended anyway.
> > > > > > If there is a compile-time dependency with those drivers,
> > > > > > it should be 'depends on', otherwise they can normally be
> > > > > > left out.
> > > > > 
> > > > > +1
> > > > 
> > > > Hi,
> > > > 
> > > > Removing the "imply" statements is simple enough and the driver
> > > > still builds cleanly without them, so Yes, they aren't needed here.
> > > > 
> > > > Removing the SPI dependency is also clean.
> > > > 
> > > > The driver does use I2C, MTD, and SERIAL_8250 interfaces, so they
> > > > can't be removed without some other driver changes, like using
> > > > #ifdef/#endif (or #if IS_ENABLED()) blocks and some function stubs.
> > > 
> > > If the SERIAL_8250 dependency is actually required, then using
> > > 'depends on' for this is probably better than an IS_ENABLED() check.
> > > The 'select' is definitely misplaced here, that doesn't even work when
> > > the dependencies fo 8250 itself are not met, and it does force-enable
> > > the entire TTY subsystem.
> > 
> > So, something like the following (untested) patch?
> > I admit to not fully understanding all the nuances around Kconfig.
> 
> Hi,
> 
> You can also remove the "select NET_DEVLINK". The driver builds fine
> without it. And please drop the "default n" while at it.

I had to add this one because devlink is a dependency and the kbuild
robot generated a config without it.

The 'imply' statements were added because while the driver builds
without them, the resources don't show up unless the platform
modules are also present.  This was really confusing users, since
they selected the OCP driver and then were not able to use the 
flash since the XILINX modules had not been selected.

Is there a better way of specifying these type of dependencies?
-- 
Jonathan