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

On 8/25/21 2:14 PM, Jonathan Lemon wrote:

On Wed, Aug 25, 2021 at 01:45:57PM -0700, Randy Dunlap wrote:

quoted

On 8/25/21 1:40 PM, Jonathan Lemon wrote:

quoted

On Wed, Aug 25, 2021 at 10:29:51AM -0700, Randy Dunlap wrote:

quoted

On 8/25/21 10:08 AM, Jonathan Lemon wrote:

quoted

On Wed, Aug 25, 2021 at 12:55:25PM +0200, Arnd Bergmann wrote:

quoted

On Tue, Aug 24, 2021 at 11:48 PM Randy Dunlap [off-list ref] wrote:

quoted

On 8/20/21 8:31 AM, Richard Cochran wrote:

quoted

On Fri, Aug 20, 2021 at 12:45:42PM +0200, Arnd Bergmann wrote:

quoted

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.

What kind of dependency is devlink?
The driver builds without NET_DEVLINK.

It really doesn't.  Odds are one of the network drivers is also
selecting this as well, so it is hidden.

OK, my mistake. Thanks.

-- 
~Randy