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]
Date:   Fri, 24 Apr 2020 18:42:59 +0200
From:   Christophe Kerello <christophe.kerello@...com>
To:     Boris Brezillon <boris.brezillon@...labora.com>
CC:     Marek Vasut <marex@...x.de>, Lee Jones <lee.jones@...aro.org>,
        <mark.rutland@....com>, <devicetree@...r.kernel.org>,
        <vigneshr@...com>, <tony@...mide.com>, <richard@....at>,
        <linux-kernel@...r.kernel.org>, <robh+dt@...nel.org>,
        <linux-mtd@...ts.infradead.org>, <miquel.raynal@...tlin.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH v2 02/12] mfd: stm32-fmc2: add STM32 FMC2 controller
 driver



On 4/24/20 5:14 PM, Boris Brezillon wrote:
> On Fri, 24 Apr 2020 13:47:34 +0200
> Christophe Kerello <christophe.kerello@...com> wrote:
> 
>> On 4/24/20 1:06 PM, Marek Vasut wrote:
>>> On 4/24/20 12:50 PM, Lee Jones wrote:
>>>> 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@...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.
>>>
>>> I use it to operate KSZ8851-16MLL ethernet chip, which has async bus
>>> interface. Linux just didn't have support for that mode of operation
>>> thus far and the FMC was used to operate NANDs and NORs only. This
>>> series, or rather, the first three patches in this series, add support
>>> for operating other bus devices, like this ethernet controller.
>>>    
>>>> 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.
>>>
>>> I don't think it's _just_ a memory controller, it's more universal than
>>> that, see above. Note that SRAM interface basically boils down to
>>> anything which has external parallel bus, e.g. Davicom DM9000, that
>>> KSZ8851-16MLL etc.
>>>    
>>>>> 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.
>>>
>>> I see.
>>>   
>>
>> Hi Lee, Marek,
>>
>> I will move this source code in the FMC2 bus driver. I think that I
>> should be able to manage the 2 controllers with 2 drivers (the FMC2 bus
>> driver and the FMC2 raw NAND driver).
> 
> FWIW, that's what I did for the Atmel EBI (External Bus Interface)
> controller (see [1]).
> 
> [1]https://elixir.bootlin.com/linux/v5.6/source/drivers/memory/atmel-ebi.c
> 

Hi Boris,

Thanks for your help.

I was thinking about the bindings and I think that the bindings below 
are close to what has been done for Atmel EBI/Raw NAND bindings (in 
terms of structure of bindings if I have well understood).
I think that these proposed bindings are very close to the first 
proposed version (V1/V2).

     fmc@...02000 {
       #address-cells = <2>;
       #size-cells = <1>;
       compatible = "st,stm32mp1-fmc2";
       reg = <0x58002000 0x1000>;
       clocks = <&rcc FMC_K>;
       resets = <&rcc FMC_R>;

       ranges = <0 0 0x60000000 0x4000000>, /* EBI bank 1 */
                <1 0 0x64000000 0x4000000>, /* EBI bank 2 */
                <2 0 0x68000000 0x4000000>, /* EBI bank 3 */
                <3 0 0x6c000000 0x4000000>, /* EBI bank 4 */
                <4 0 0x80000000 0x4000000>, /* NAND common memory space */
                <5 0 0x88000000 0x4000000>; /* NAND attribute memory 
space */
	
       psram@0 {
         compatible = "mtd-ram";
         reg = <0 0x00000000 0x100000>;
         bank-width = <2>;

         st,fmc2_ebi_cs_transaction_type = <1>;
         st,fmc2_ebi_cs_address_setup = <60>;
         st,fmc2_ebi_cs_data_setup = <30>;
         st,fmc2_ebi_cs_bus_turnaround = <5>;
       };

       nand-controller@4 {
         #address-cells = <1>;
         #size-cells = <0>;
         compatible = "st,stm32mp15-fmc2";
         reg = <4 0x00000000 0x1000>,
               <5 0x00010000 0x1000>,
               <5 0x00020000 0x1000>,
               <4 0x01000000 0x1000>,
               <5 0x01010000 0x1000>,
               <5 0x01020000 0x1000>;
         interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
         dmas = <&mdma1 20 0x2 0x12000a02 0x0 0x0>,
                <&mdma1 20 0x2 0x12000a08 0x0 0x0>,
                <&mdma1 21 0x2 0x12000a0a 0x0 0x0>;
         dma-names = "tx", "rx", "ecc";

         nand@0 {
           reg = <0>;
           nand-on-flash-bbt;
           #address-cells = <1>;
           #size-cells = <1>;
         };
       };
     };

Regards,
Christophe Kerello.

Powered by blists - more mailing lists