| Message ID | 20240523-fsl-ifc-config-v1-1-6eff73bdc7e6@geanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | memory: fsl_ifc: Make FSL_IFC config visible and selectable | expand |
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? Which DTS? Best regards, Krzysztof
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. > Which DTS? It is for "fsl,ifc" compatible devices. /Esben
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. > >> Which DTS? > > It is for "fsl,ifc" compatible devices. That's not a NOR flash. Best regards, Krzysztof
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". /Esben
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. Best regards, Krzysztof
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
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
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"; >> }; >> }; >> }; >> >> 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... I don't have anything out of tree other than the DTS. Is the kernel only supposed to support in-tree DTS? I thought that keeping (dtb) device-tree in bootloader was very much considered a supported setup. Or are you saying that I should send patches with all downstream DTS files, even if it is for hardware that is not generally available? /Esben
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
On 27/05/2024 09:47, Esben Haabendal wrote: > > 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>; > }; > }; > I don't understand why it took so many emails to answer that (my first) question... Sounds good, however you did not update the existing select. Drivers are not supposed to select user-visible symbols (leads to issues), so you need to change it to depends and update defconfigs. Best regards, Krzysztof
Krzysztof Kozlowski <krzk@kernel.org> writes: > On 27/05/2024 09:47, Esben Haabendal wrote: >> >> 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>; >> }; >> }; >> > > I don't understand why it took so many emails to answer that (my first) > question... Because I did not understand the question. Primarely because I was (and is) surprised that out-of-tree DTS is not supported. I was convinced that out-of-tree DTS was the right way for hardware which is not commonly available. > Sounds good, however you did not update the existing select. > Drivers are not supposed to select user-visible symbols (leads to > issues), so you need to change it to depends and update defconfigs. Do you wan this split into multiple commits, or a single commit changing the Kconfig to make FSL_IFC user-visible, and changing select of it to DEPENDS, and updating the related defconfig(s)? /Esben
On 28/05/2024 12:03, Esben Haabendal wrote: > Krzysztof Kozlowski <krzk@kernel.org> writes: > >> On 27/05/2024 09:47, Esben Haabendal wrote: >>> >>> 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>; >>> }; >>> }; >>> >> >> I don't understand why it took so many emails to answer that (my first) >> question... > > Because I did not understand the question. Primarely because I was (and > is) surprised that out-of-tree DTS is not supported. I was convinced > that out-of-tree DTS was the right way for hardware which is not > commonly available. Even some non-GA hardware could, and IMHO should, be upstreamed, at least some parts of it. This gives the user/upstreamer reason to do changes. Otherwise you might get questions for contributions: why you are doing and why this is worth? Downstream or any fork is not really answer to such questions, because they are allowed to make whatever stupid choices they want (not saying it was done here, but in general), which should not be a reason to do anything upstream. If downstream creates wrong DTS files, shall we create wrong device drivers or bindings for them? No. > >> Sounds good, however you did not update the existing select. >> Drivers are not supposed to select user-visible symbols (leads to >> issues), so you need to change it to depends and update defconfigs. > > Do you wan this split into multiple commits, or a single commit changing > the Kconfig to make FSL_IFC user-visible, and changing select of it to > DEPENDS, and updating the related defconfig(s)? One commit for Kconfigs (nand and memory), additional commits for defconfigs, one per each arch, because defconfigs might go via arch tree. Best regards, Krzysztof
Krzysztof Kozlowski <krzk@kernel.org> writes: > On 28/05/2024 12:03, Esben Haabendal wrote: >> Krzysztof Kozlowski <krzk@kernel.org> writes: >> >>> On 27/05/2024 09:47, Esben Haabendal wrote: >>>> >>>> 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>; >>>> }; >>>> }; >>>> >>> >>> I don't understand why it took so many emails to answer that (my first) >>> question... >> >> Because I did not understand the question. Primarely because I was (and >> is) surprised that out-of-tree DTS is not supported. I was convinced >> that out-of-tree DTS was the right way for hardware which is not >> commonly available. > > Even some non-GA hardware could, and IMHO should, be upstreamed, at > least some parts of it. This gives the user/upstreamer reason to do > changes. Otherwise you might get questions for contributions: why you > are doing and why this is worth? > > Downstream or any fork is not really answer to such questions, because > they are allowed to make whatever stupid choices they want (not saying > it was done here, but in general), which should not be a reason to do > anything upstream. If downstream creates wrong DTS files, shall we > create wrong device drivers or bindings for them? No. Of-course not. But to be fair, in this case, there has not been any new bindings created, and no creative usage of existing bindings. Only straight forward usage of existing drivers (IFC and physmap MTD driver), which I am surprised no one else had seen the need to fix in upstream. But do you/we really want to open the flood gates for DTS maintenance of a gazillion embedded boards that only a few in the world have an interest in? I suspect it would create quite a lot of patches to flow through maintainers, with very litle benefit to most people. But for me (as in the companies doing such these kind of embedded hardware), it is a clear benefit to put upstream DTS files. But what about boards where device-tree is created/modified dynamically by bootloader? Is that also not really supported? >>> Sounds good, however you did not update the existing select. >>> Drivers are not supposed to select user-visible symbols (leads to >>> issues), so you need to change it to depends and update defconfigs. >> >> Do you wan this split into multiple commits, or a single commit changing >> the Kconfig to make FSL_IFC user-visible, and changing select of it to >> DEPENDS, and updating the related defconfig(s)? > > One commit for Kconfigs (nand and memory), additional commits for > defconfigs, one per each arch, because defconfigs might go via arch tree. Sounds good. I will send a v2 with that. /Esben
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig index 8efdd1f97139..c82d8d8a16ea 100644 --- a/drivers/memory/Kconfig +++ b/drivers/memory/Kconfig @@ -167,7 +167,7 @@ config FSL_CORENET_CF represents a coherency violation. config FSL_IFC - bool "Freescale IFC driver" if COMPILE_TEST + bool "Freescale IFC driver" depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST depends on HAS_IOMEM
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. Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers") Signed-off-by: Esben Haabendal <esben@geanix.com> --- drivers/memory/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6 change-id: 20240523-fsl-ifc-config-c877902b297e Best regards,