Hi Massimo,
Thanks for the patches. Note that the timing is interesting as we were
considering having two variants of the same product, one with DDR3 and
one with DDR4 and we don't want to have with deal with multiple binaries
just for that.
On 8/29/26 7:03 PM, Massimo Pegorer wrote:
Not all the DDR type config options named in the Kconfig are supported
by each one of the Rockchip SoCs. Moreover, the LPDDR4 option has a
different meaning in the context of the RK3399 SoC than in the RV1126
one. Improve clarity splitting config options, using SoC specific help
sections and adding depends on conditionals.
Yeah, we shouldn't do that.
Involved Rockchip SoCs are: RK3399, PX30, RV1126.
Signed-off-by: Massimo Pegorer <[email protected]>
---
drivers/ram/rockchip/Kconfig | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig
index d707d09c1c8..61a3193b3f5 100644
--- a/drivers/ram/rockchip/Kconfig
+++ b/drivers/ram/rockchip/Kconfig
@@ -26,26 +26,53 @@ config RAM_ROCKCHIP_DEBUG
config RAM_ROCKCHIP_DDR4
bool "DDR4 support for Rockchip SoCs"
+ depends on ROCKCHIP_PX30 || ROCKCHIP_RV1126
This kind of change is very welcome.
help
This enables DDR4 sdram support instead of the default DDR3 support
on Rockchip SoCs.
We should get rid of the part that says "instead of the default DDR3
support". We should have instead an option for DDR3 which is enabled by
default and then we don't have to care about that part.
+ Note that even if multiple DDR/LPDDR types are selected, support is
+ enabled only for the selected one that comes first in the following
+ list: LPDDR4, DDR4, LPDDR2, LPDDR3, DDR3.
This one change though is less welcome. I don't want to have this in the
Kconfig help text. I suspect nobody will maintain this in the long term.
+
config RAM_ROCKCHIP_LPDDR2
bool "LPDDR2 support for Rockchip SoCs"
+ depends on ROCKCHIP_PX30
help
This enables LPDDR2 sdram support instead of the default DDR3 support
on Rockchip SoCs.
+ Note that even if multiple DDR/LPDDR types are selected, support is
+ enabled only for the selected one that comes first in the following
+ list: LPDDR4, DDR4, LPDDR2, LPDDR3, DDR3.
+
config RAM_ROCKCHIP_LPDDR3
bool "LPDDR3 support for Rockchip SoCs"
+ depends on ROCKCHIP_PX30
help
This enables LPDDR3 sdram support instead of the default DDR3 support
on Rockchip SoCs.
+ Note that even if multiple DDR/LPDDR types are selected, support is
+ enabled only for the selected one that comes first in the following
+ list: LPDDR4, DDR4, LPDDR2, LPDDR3, DDR3.
+
config RAM_ROCKCHIP_LPDDR4
+ depends on ROCKCHIP_RV1126
bool "LPDDR4 support for Rockchip SoCs"
help
- This enables LPDDR4 sdram code support for the platforms based
+ This enables LPDDR4 sdram support instead of the default DDR3 support
on Rockchip SoCs.
+ Note that even if multiple DDR/LPDDR types are selected, support is
+ enabled only for the selected one that comes first in the following
+ list: LPDDR4, DDR4, LPDDR2, LPDDR3, DDR3.
+
+config RAM_ROCKCHIP_LPDDR4
+ bool "LPDDR4 support for Rockchip SoCs"
+ depends on ROCKCHIP_RK3399
+ help
+ This enables LPDDR4 sdram code support for the platforms based
+ on Rockchip RK3399 SoC.
+
Please do not duplicate the same symbol twice just to change its helptext.
For drivers that support multiple DRAM types but not at the same time,
the easiest answer would be to enforce that outside of Kconfig with
something like:
#if defined(CONFIG_RAM_ROCKCHIP_LPDDR4) +
defined(CONFIG_RAM_ROCKCHIP_DDR4) + defined(CONFIG_RAM_ROCKCHIP_LPDDR2) > 1
#error "This driver only supports one DRAM configuration at a time"
#endif
in the driver. I don't like it too much as this should really be in the
Kconfig symbol but I couldn't think of a way to easily have only one
symbol among a list to be allowed to be selected (that's typically a
choice Kconfig) and also have the same symbols allowed to be selected as
many as possible, depending on the SoC. I think it's a good compromise
for now to fail the build by having this kind of check in the driver.
However, this won't play nice with COMPILE_TEST so not sure if it's
acceptable to Tom.
Ideally, we should also figure out which DRAM type(s) are supported by
DRAM drivers that do not make use of the Kconfig symbols. e.g. RK3066
for example, and have them explicitly enabled in Kconfig for
completeness. That isn't required for this series.
Cheers,
Quentin
