Message-ID: <20200220154521.GB25745@shell.armlinux.org.uk>
Date:   Thu, 20 Feb 2020 15:45:21 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Joel Johnson <mrjoel@...il.net>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Baruch Siach <baruch@...s.co.il>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Rob Herring <robh@...nel.org>, netdev@...r.kernel.org
Subject: Re: mvneta: comphy regression with SolidRun ClearFog

On Thu, Feb 20, 2020 at 08:34:07AM -0700, Joel Johnson wrote:
> On 2020-02-20 03:12, Russell King - ARM Linux admin wrote:
> > On Wed, Feb 19, 2020 at 06:49:51AM -0700, Joel Johnson wrote:
> > > On 2020-02-19 02:22, Russell King - ARM Linux admin wrote:
> > > > On Tue, Feb 18, 2020 at 10:14:48PM -0700, Joel Johnson wrote:
> > > > Does debian have support for the comphy enabled in their kernel,
> > > > which is controlled by CONFIG_PHY_MVEBU_A38X_COMPHY ?
> > > 
> > > Well, doh! I stared at the patch series for way to long, but the added
> > > Kconfig symbol failed to register mentally somehow. I had been using
> > > the
> > > last known good Debian config with make olddefconfig, but it obviously
> > > wasn't included in earlier configs and not enabled by default.
> > > 
> > > Many thanks to you and Willy Tarreau for pointing out my glaring
> > > omission!
> > 
> > Thanks for letting us know that you've fixed it now.
> 
> Sure thing, I've submitted a Debian patch, and was pointed to an existing
> Debian bug with the same issue and patch, so hopefully that will get
> incorporated soon. I'll also keep an eye on OpenWRT when they move to an
> affected kernel version to make sure it's included.
> 
> One lingering question that wasn't clear to me is the apparent inconsistency
> in default enablement for PHYs in drivers/phy/marvell/Kconfig. Is there a
> technical reason why PHY_MVEBU_A3700_COMPHY defaults to 'y' but
> PHY_MVEBU_A38X_COMPHY (and PHY_MVEBU_CP110_COMPHY) default to 'n', or is it
> just an artifact of being added at different times? Similarly, is there a
> reason that PHY_MVEBU_A3700_COMPHY and PHY_MVEBU_A3700_UTMI default to 'y'
> instead of 'm' for all ARCH_MVEBU builds? In my testing, building with
> PHY_MVEBU_A38X_COMPHY as a module still seemed to autoload the module as
> needed on boot, so modules for different platforms seems off-hand more
> lightweight that building the driver in for all MVEBU boards which don't use
> all drivers.
> 
> With the current defaults, it seems like PHY_MVEBU_CP110_COMPHY may be
> affected in Debian the same way as PHY_MVEBU_A38X_COMPHY, but I don't have
> available Armada 7K/8K hardware yet to confirm.

There is no clear answer to whether should something default to Y,
M or N - different people have different ideas and different levels
of frustration with which-ever are picked.

The root problem is that there are way too many configuration
options that it's become quite time consuming to put together the
proper kernel configuration for any particular platform, and things
get even more interesting when it comes to a kernel supporting
multiple platforms, where one may wish to avoid having a huge
kernel image, but want to use modules for the platform specific
bits.

I wonder whether we ought to be considering something like:

menuconfig ARCH_MVEBU_DEFAULTS
	tristate "Marvell Engineering Business Unit (MVEBU) SoCs"

config ARCH_MVEBU
	def_bool y if ARCH_MVEBU_DEFAULTS
	...

and then have mvebu drivers default to the state of
ARCH_MVEBU_DEFAULTS.  That means, if you want to build the
platform with modular drivers, or built-in drivers there's one top
level config that will default all the appropriate options that way,
and any new drivers added later can pick up on the state of that
option.

Just a thought.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Powered by blists - more mailing lists