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

[Krzysztof Kozlowski <krzk@kernel.org>]

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