Received-SPF: pass (google.com: domain of linux-kernel+bounces-190164-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Esben Haabendal <esben@geanix.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Tudor Ambarus <tudor.ambarus@linaro.org>,  Pratyush Yadav
 <pratyush@kernel.org>,  Michael Walle <mwalle@kernel.org>,
  linux-kernel@vger.kernel.org,  linux-mtd@lists.infradead.org
Subject: Re: [PATCH] memory: fsl_ifc: Make FSL_IFC config visible and
 selectable
In-Reply-To: <87msobaes6.fsf@geanix.com> (Esben Haabendal's message of "Mon,
	27 May 2024 09:35:05 +0200")
References: <20240523-fsl-ifc-config-v1-1-6eff73bdc7e6@geanix.com>
	<979fd913-050b-445d-9ca8-0ec6906ce3ea@kernel.org>
	<87cypc38gu.fsf@geanix.com>
	<9a7f73f4-f5dc-4342-855b-08df6a839bb5@kernel.org>
	<87le3zoatn.fsf@geanix.com>
	<6c166ad5-8004-4bc4-9107-a47ba9a72161@kernel.org>
	<87ttijaglp.fsf@geanix.com>
	<c045f1a4-9ddf-4c53-a69b-22ceb68a1ce8@kernel.org>
	<87msobaes6.fsf@geanix.com>
Date: Mon, 27 May 2024 09:47:16 +0200
Message-ID: <87ikyzae7v.fsf@geanix.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Esben Haabendal <esben@geanix.com> writes:

> Krzysztof Kozlowski <krzk@kernel.org> writes:
>
>> On 27/05/2024 08:55, Esben Haabendal wrote:
>>> Krzysztof Kozlowski <krzk@kernel.org> writes:
>>> 
>>>> On 24/05/2024 10:47, Esben Haabendal wrote:
>>>>> Krzysztof Kozlowski <krzk@kernel.org> writes:
>>>>>
>>>>>> On 23/05/2024 16:32, Esben Haabendal wrote:
>>>>>>> Krzysztof Kozlowski <krzk@kernel.org> writes:
>>>>>>>
>>>>>>>> On 23/05/2024 15:58, Esben Haabendal wrote:
>>>>>>>>> While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
>>>>>>>>> driver selects FSL_IFC automatically, we need the option to be selectable
>>>>>>>>> for platforms using fsl_ifc with NOR flash.
>>>>>>>>
>>>>>>>> Which driver is that?
>>>>>>>
>>>>>>> This is drivers/memory/fsl_ifc.o driver. It is for Integrated Flash
>>>>>>> Controller (IFC) from NXP. It is used in various Layerscape socs.
>>>>>>
>>>>>> ? I know that, I mean the NOR flash working here.
>>>>>
>>>>> Ah, sorry. The NOR flash I am using here is a Spansion S29AL016J.
>>>>>
>>>>>>>> Which DTS?
>>>>>>>
>>>>>>> It is for "fsl,ifc" compatible devices.
>>>>>>
>>>>>> That's not a NOR flash.
>>>>>
>>>>> Nope.  The binding used for the NOR flash is "cfi-flash".
>>>>
>>>> And now let's get back to my original question. I asked for driver, not
>>>> device, and for DTS not compatible.
>>> 
>>> You got me really confused now. I am not sure what you are asking me
>>> for, and why.
>>> 
>>> I am sending a patch which changes to Kconfig for a memory controller
>>> driver. The change is AFAICS quite similar to commit be34f45f0d4a
>>> ("memory: omap-gpmc: Make OMAP_GPMC config visible and selectable").
>>> 
>>> As for the NOR flash that in this situation is attached to the IFC
>>> controller, it is (as mentioned) Spansion S29AL016J. It is handled by
>>> the drivers/mtd/maps/physmap.o driver (CONFIG_MTD_PHYSMAP +
>>> CONFIG_MTD_PHYSMAP_OF)
>>
>> Thanks.
>>
>>> 
>>> The DTS used to specify the NOR flash is
>>
>> Upstream DTS. I don't care about downstream.
>
> Upstream DTS is arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
>
> Which has
>
>                 ifc: memory-controller@1530000 {
>                         compatible = "fsl,ifc";
>                         reg = <0x0 0x1530000 0x0 0x10000>;
>                         interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
>                         status = "disabled";
>                 };
>
> The NOR flash is not specified in upstream DTS, as it is board and not
> SoC specific.
>
> I assume it is not considered bad practise to keep downstream DTS out of
> tree.
>
>>> &ifc {
>>>         status = "okay";
>>>         #address-cells = <2>;
>>>         #size-cells = <1>;
>>>         ranges = <0x0 0x0 0x0 0x60000000 0x00200000>;
>>> 
>>>         nor_flash: nor@0,0 {
>>>                 #address-cells = <1>;
>>>                 #size-cells = <1>;
>>>                 compatible = "cfi-flash";
>>>                 reg = <0x0 0x0 0x200000>;
>>>                 bank-width = <2>;
>>> 
>>>                 partition@0 {
>>>                         reg = <0x000000 0x0f0000>;
>>>                         label = "boot0";
>>>                 };
>>>         };
>>> };

Ok, I seem to still be confused as to what you want from me. If you are
saying that the kernel is not supposed to care about out-of-tree DTS
(and thereby any bootloader provided DTB), I would like to bring your
attention to arch/arm/boot/dts/nxp/ls/ls1021a-twr.dts in upstream:

&ifc {
        #address-cells = <2>;
        #size-cells = <1>;
        /* NOR Flash on board */
        ranges = <0x0 0x0 0x0 0x60000000 0x08000000>;
        status = "okay";

        nor@0,0 {
                #address-cells = <1>;
                #size-cells = <1>;
                compatible = "cfi-flash";
                reg = <0x0 0x0 0x8000000>;
                big-endian;
                bank-width = <2>;
                device-width = <1>;
        };
};

This is basically identical to what I am using, and it will have the
same problem. It requires CONFIG_FSL_IFC enabled, which is not possible
(without enabling CONFIG_MTD_NAND_FSL_IFC). This board does not have
NAND, so I don't think it is reasonable to require keeping support for
it enabled to be able to use NOR flash.

/Esben