[PATCH v2 02/12] ARM: defconfigs: add MTD_SPI_NOR (new dependency for M25P80)
horms at verge.net.au
Sat May 10 17:43:44 PDT 2014
On Tue, May 06, 2014 at 11:12:40AM -0700, Brian Norris wrote:
> On Thu, May 01, 2014 at 05:54:47PM +0900, Simon Horman wrote:
> > [ CC linux-sh and Magnus Damm (shmobile co-maintainer) ]
> > On Wed, Apr 30, 2014 at 11:26:37PM -0700, Brian Norris wrote:
> > > These defconfigs contain the CONFIG_M25P80 symbol, which is now
> > > dependent on the MTD_SPI_NOR symbol. Add CONFIG_MTD_SPI_NOR to satisfy
> > > the new dependency.
> > >
> > > At the same time, drop the now-nonexistent CONFIG_MTD_CHAR symbol.
> > >
> > > Signed-off-by: Brian Norris <computersforpeace at gmail.com>
> > > Cc: Russell King <linux at arm.linux.org.uk>
> > > Cc: Arnd Bergmann <arnd at arndb.de>
> > > Cc: Olof Johansson <olof at lixom.net>
> > > Cc: linux-arm-kernel at lists.infradead.org
> > > Cc: linux-kernel at vger.kernel.org
> > > ---
> > > This patch catches all the configs I couldn't find a sub-arch for, plus the ARM
> > > multiplatform defconfigs
> > >
> > > arch/arm/configs/bockw_defconfig | 2 +-
> > > arch/arm/configs/koelsch_defconfig | 1 +
> > > arch/arm/configs/lager_defconfig | 1 +
> > The above changes are for shmobile SoC defconfigs which I maintain
> > (as is one other patch in the series).
> > While the below ones are not.
> This is admittedly a confusing process. I have to parse each defconfig
> to figure out what the mach-* is, then read MAINTAINERS. Perhaps you can
> update your MAINTAINERS entries? This is a wide problem, that makes
> patch submission for defconfigs even more difficult.
U understand it is confusing.
I'll see about updating my MAINTAINERS entries accordingly.
> Olof, other ARM SOC maintainers,
> What do you recommend? Should I send another couple of patches just to
> split this trivial patch? Olof mentioned taking some patch(es) directly,
> and handling merge conflicts when they come.
> > With regards to updating shmobile configuration options,
> > we have recently moved towards using Kconfig rather than
> > defconfig in cases where selecting A means we really ought
> > to select B too. Something along the lines of:
> > select CONFIG_MTD_SPI_NOR if CONFIG_MTD_M25P80
> That seems like you're just avoiding touching defconfig for the sake of
> not touching defconfig. Additionally, this method only really makes
> sense for an upgrade path (otherwise, how would you configure MTD_M25P80
> && !MTD_SPI_NOR?). Is this approach consistent across other mach-*?
I do not fully understand your patchset. But from my point of view
it seems to me that the key question is "is it desirable to configure
MTD_M25P80 && !MTD_SPI_NOR". I suspect not as below you confirm
that MTD_SPI_NOR should always be selected if MTD_M25P80 if selected.
> > Do you consider that CONFIG_MTD_SPI_NOR should always
> > be selected if CONFIG_MTD_M25P80 if selected?
> Yes. That's what "depends on" means.
In that case I think that dependency should be expressed using Kconfig.
That way anyone (or any defconfig) that selects MTD_M25P80 will
also get MTD_SPI_NOR selected without needing to know that the former
depends on the later.
> BTW, M25P80 will likely be moved under the SPI-NOR submenu eventually
> (its driver is not really a "Standalone MTD" anymore), but currently
> this is just a "depends on".
> > If so, perhaps it would be best to update the CONFIG_MTD_M25P80 Kconfig
> > node to select CONFIG_MTD_M25P80. This would probably imply that most
> > if not all of this series would no longer be needed.
> I do not understand this paragraph. How does M25P80 select M25P80?
Sorry, I meant to say "... update the MTD_M25P80 Kconfig node
to select MTD_SPI_NOR".
> > If not, would you be able to make a patch to add something like
> > the above snippet to arch/arm/shmobile/Kconfig (probably more than once)
> > and drop the changes to shmobile defconfigs from this series?
> Feel free to submit your own patch if you don't want mine.
> > > arch/arm/configs/multi_v5_defconfig | 1 +
> > > arch/arm/configs/multi_v7_defconfig | 1 +
> > > 5 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/configs/bockw_defconfig b/arch/arm/configs/bockw_defconfig
> > > index e816140d81c5..28339e072a71 100644
> > > --- a/arch/arm/configs/bockw_defconfig
> > > +++ b/arch/arm/configs/bockw_defconfig
> > > @@ -50,11 +50,11 @@ CONFIG_DEVTMPFS_MOUNT=y
> > > # CONFIG_PREVENT_FIRMWARE_BUILD is not set
> > > # CONFIG_FW_LOADER is not set
> > > CONFIG_MTD=y
> > > -CONFIG_MTD_CHAR=y
> > The above change seems unrelated to the subject of the patch.
> > If its valid then I'd prefer it submitted as a separate patch.
> Seriously? It's mentioned in the commit description, and it's really
> really trivial.
Sorry, I missed it in the changelog.
More information about the linux-arm-kernel