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

Krzysztof Kozlowski
2024-05-27T07:19:59Z

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.

>
> &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?

If there is no user for such option, why changing it? To clarify:
whatever is out of tree, does not really exist...

Best regards,
Krzysztof