Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 24 Apr 2020 11:50:53 +0100
From:   Lee Jones <lee.jones@linaro.org>
To:     Marek Vasut <marex@denx.de>
Cc:     Christophe Kerello <christophe.kerello@st.com>,
        miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
        robh+dt@kernel.org, mark.rutland@arm.com, tony@atomide.com,
        linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        devicetree@vger.kernel.org,
        Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH v2 02/12] mfd: stm32-fmc2: add STM32 FMC2 controller
 driver
Message-ID: <20200424105053.GC8414@dell>
References: <1586966256-29548-1-git-send-email-christophe.kerello@st.com>
 <1586966256-29548-3-git-send-email-christophe.kerello@st.com>
 <20200424074517.GN3612@dell>
 <8b625f1c-9ded-c07a-a20e-8cd44c1ca46d@denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <8b625f1c-9ded-c07a-a20e-8cd44c1ca46d@denx.de>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 24 Apr 2020, Marek Vasut wrote:

> On 4/24/20 9:45 AM, Lee Jones wrote:
> > On Wed, 15 Apr 2020, Christophe Kerello wrote:
> > 
> >> The driver adds the support for the STMicroelectronics FMC2 controller
> >> found on STM32MP SOCs.
> >>
> >> The FMC2 functional block makes the interface with: synchronous and
> >> asynchronous static memories (such as PSNOR, PSRAM or other
> >> memory-mapped peripherals) and NAND flash memories.
> >>
> >> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> >> ---
> >> Changes in v2:
> >>  - remove ops from stm32_fmc2 structure
> >>  - add 2 APIs to manage FMC2 enable/disable
> >>  - add 2 APIs to manage FMC2 NWAIT shared signal
> >>
> >>  drivers/mfd/Kconfig            |  12 +++
> >>  drivers/mfd/Makefile           |   1 +
> >>  drivers/mfd/stm32-fmc2.c       | 136 +++++++++++++++++++++++++
> >>  include/linux/mfd/stm32-fmc2.h | 225 +++++++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 374 insertions(+)
> >>  create mode 100644 drivers/mfd/stm32-fmc2.c
> >>  create mode 100644 include/linux/mfd/stm32-fmc2.h
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >> index 2b20329..5260582 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -1922,6 +1922,18 @@ config MFD_ROHM_BD71828
> >>  	  Also included is a Coulomb counter, a real-time clock (RTC), and
> >>  	  a 32.768 kHz clock gate.
> >>  
> >> +config MFD_STM32_FMC2
> >> +	tristate "Support for FMC2 controllers on STM32MP SoCs"
> >> +	depends on MACH_STM32MP157 || COMPILE_TEST
> >> +	select MFD_CORE
> >> +	select REGMAP
> >> +	select REGMAP_MMIO
> >> +	help
> >> +	  Select this option to enable STM32 FMC2 driver used for FMC2 External
> >> +	  Bus Interface controller and FMC2 NAND flash controller. This driver
> >> +	  provides core support for the STM32 FMC2 controllers, in order to use
> >> +	  the actual functionality of the device other drivers must be enabled.
> > 
> > Not sure how many times I have to say this before people stop
> > attempting to pass these kinds of relationships off as MFDs:
> > 
> > A memory device and its bus is not an MFD.  In a similar vain to the
> > thousands of USB, I2C, SPI, PCI and the like devices that aren't MFDs
> > either.
> > 
> > Please find another way to associate your device with its bus.
> 
> This FMC2 is however an IP which can either operate external devices
> (like ethernet chip on this parallel bus) or external flashes (like NOR
> and NAND chips).

I'm sure that it *can*.  Although that's not its main purpose.  The
clue is in the nomenclature ("Flexible *Memory* Controller").  Nor is
it how the device is being used in this submission:

  "The FMC2 functional block makes the interface with: synchronous and
   asynchronous static memories (such as PSNOR, PSRAM or other
   memory-mapped peripherals) and NAND flash memories."

As I mentioned, this is just another memory device and its bus.

> Can you provide a suggestion how this should be handled, if not as MFD?
> It seems to me, that this is a Multi-Function Device .

Simply move it into the MTD or Memory subsystems and set up the
dependencies via Kconfig.

> If this discussion is a recurring topic, is there some documentation
> which explains how such devices should be handled ?

Not that I'm aware of.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog