Re: [PATCH] memory: fsl_ifc: Make FSL_IFC config visible and selectable

Esben Haabendal
2024-05-27T06:55:55Z

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)

The DTS used to specify the NOR flash is

&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";
};
};
};

So as mentioned, it is a quite standard "cfi-flash" compatible device
declaration.

Do you think there is a way that ti enable CONFIG_FSL_IFC for my
situation without the config being visible? Something like automatically
selecting CONFIG_FSL_IFC when CONFIG_MTD_PHYSMAP and
CONFIG_MTD_PHYSMAP_OF is enabled? Will that not include a risk of
pulling in CONFIG_FSL_IFC in some cases where it is not desired?

Something like

config MTD_PHYSMAP
tristate "Flash device in physical memory map"
depends on MTD_CFI || MTD_JEDECPROBE || MTD_ROM || MTD_RAM || MTD_LPDDR
select FSL_IFC if SOC_LS1021A

But that looks like a road somewhere not so nice. The generic
MTD_PHYSMAP being littered with architecture specific selects.

Or is there something completely obvious (not for me then) that I am
missing?

/Esben