All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Mon, 27 May 2024 09:47:16 +0200	[thread overview]
Message-ID: <87ikyzae7v.fsf@geanix.com> (raw)
In-Reply-To: <87msobaes6.fsf@geanix.com> (Esben Haabendal's message of "Mon, 27 May 2024 09:35:05 +0200")

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
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
Date: Mon, 27 May 2024 09:47:16 +0200	[thread overview]
Message-ID: <87ikyzae7v.fsf@geanix.com> (raw)
In-Reply-To: <87msobaes6.fsf@geanix.com> (Esben Haabendal's message of "Mon, 27 May 2024 09:35:05 +0200")

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

  reply	other threads:[~2024-05-27  7:47 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-23 13:58 [PATCH] memory: fsl_ifc: Make FSL_IFC config visible and selectable Esben Haabendal
2024-05-23 13:58 ` Esben Haabendal
2024-05-23 14:01 ` Krzysztof Kozlowski
2024-05-23 14:01   ` Krzysztof Kozlowski
2024-05-23 14:32   ` Esben Haabendal
2024-05-23 14:32     ` Esben Haabendal
2024-05-23 14:36     ` Krzysztof Kozlowski
2024-05-23 14:36       ` Krzysztof Kozlowski
2024-05-24  8:47       ` Esben Haabendal
2024-05-24  8:47         ` Esben Haabendal
2024-05-25 16:41         ` Krzysztof Kozlowski
2024-05-25 16:41           ` Krzysztof Kozlowski
2024-05-27  6:55           ` Esben Haabendal
2024-05-27  6:55             ` Esben Haabendal
2024-05-27  7:19             ` Krzysztof Kozlowski
2024-05-27  7:19               ` Krzysztof Kozlowski
2024-05-27  7:35               ` Esben Haabendal
2024-05-27  7:35                 ` Esben Haabendal
2024-05-27  7:47                 ` Esben Haabendal [this message]
2024-05-27  7:47                   ` Esben Haabendal
2024-05-28  6:53                   ` Krzysztof Kozlowski
2024-05-28  6:53                     ` Krzysztof Kozlowski
2024-05-28 10:03                     ` Esben Haabendal
2024-05-28 10:03                       ` Esben Haabendal
2024-05-28 10:21                       ` Krzysztof Kozlowski
2024-05-28 10:21                         ` Krzysztof Kozlowski
2024-05-28 11:01                         ` Esben Haabendal
2024-05-28 11:01                           ` Esben Haabendal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ikyzae7v.fsf@geanix.com \
    --to=esben@geanix.com \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mwalle@kernel.org \
    --cc=pratyush@kernel.org \
    --cc=tudor.ambarus@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.