of/platform: Don't include reserved memory compatibles that aren't needed
PATCH 1/1 of/platform: Don't include reserved memory compatibles that aren't needed
There are a number of very platform specific compatibles for reserved memory that the vast majority of people don't need and they waste 196/200 bytes each.
Wrap the platform specific compatibles so that they are only included if needed.
--- drivers/of/platform.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/of/platform.c +10 -0
@@ -489,12 +489,22 @@ EXPORT_SYMBOL_GPL(of_platform_default_populate);static const struct of_device_id reserved_mem_matches[] = { { .compatible ="phram" }, +#if IS_ENABLED(CONFIG_QCOM_RMTFS_MEM) { .compatible ="qcom,rmtfs-mem" }, +#endif +#if IS_ENABLED(CONFIG_QCOM_COMMAND_DB) { .compatible ="qcom,cmd-db" }, +#endif +#if IS_ENABLED(CONFIG_QCOM_SMEM) { .compatible ="qcom,smem" }, +#endif { .compatible ="ramoops" }, +#if IS_ENABLED(CONFIG_NVMEM_RMEM) { .compatible ="nvmem-rmem" }, +#endif +#if IS_ENABLED(CONFIG_OPEN_DICE) { .compatible ="google,open-dice" }, +#endif {} };
Signature · 1 line
2.51.0
Quoted 3 lines · Daniel Palmer
There are a number of very platform specific compatibles for reserved memory that the vast majority of people don't need and they waste 196/200 bytes each.
We don't want #ifdefs in the code because they are more difficult to maintain and review. Following your approach we would need to add such ifdefs in multiple other places, so I am not convinced it is worth.
And this file should not be having different rules than rest of drivers.
Best regards,
Krzysztof
Hi Krzysztof,
Quoted 9 lines · Krzysztof Kozlowski
Quoted 3 lines · Daniel Palmer
There are a number of very platform specific compatibles for reserved memory that the vast majority of people don't need and they waste 196/200 bytes each.
We don't want #ifdefs in the code because they are more difficult to maintain and review. Following your approach we would need to add such ifdefs in multiple other places, so I am not convinced it is worth.
I sort of disagree here. At the moment these compatibles are not coupled to the code that uses them at all.
If that code gets deleted or moved for some reason, the Kconfig symbol they belong to will disappear but since these compatibles aren't coupled to them and are in a file on the other side of the kernel it'll be easy to miss them.
If they are wrapped in something that links them back to where they are used grep or whatever indexer can spot them.
Quoted 1 line
And this file should not be having different rules than rest of drivers.
There are other places with #ifdefs in match tables. It would be nice if there was a nicer way to do it in C but there doesn't seem to be. It would be nice if vendor specific stuff wasn't in the core code but I don't see a nice way of fixing that either.
And if you want to see some of the more crazy examples of match tables take a look here:
https://elixir.bootlin.com/linux/v7.0-rc6/source/drivers/cpufreq/cpufreq-dt-platdev.c
Since these matches are not the size of the compatible string but actually 200 bytes on 64 bit machines the two megatables in that driver will consume about 40K. It doesn't seem like much but DT is being used by systems that have megabytes of RAM like a RISC-V softcore on an FPGA. I think they might prefer to have a 40K of usable memory over 40K of tables that don't contain any values that are actually used on their machine ever... and it could be cleaned up easily by wrapping each of the vendor specific chunks in the Kconfig symbol for their platform.
Cheers,
Daniel
Rob Herring
maintainerQuoted 32 lines · Daniel Palmer
Hi Krzysztof,
Quoted 9 lines · Krzysztof Kozlowski
Quoted 3 lines · Daniel Palmer
There are a number of very platform specific compatibles for reserved memory that the vast majority of people don't need and they waste 196/200 bytes each.
We don't want #ifdefs in the code because they are more difficult to maintain and review. Following your approach we would need to add such ifdefs in multiple other places, so I am not convinced it is worth.
I sort of disagree here. At the moment these compatibles are not coupled to the code that uses them at all.
If that code gets deleted or moved for some reason, theKconfigsymbol they belong to will disappear but since these compatibles aren't coupled to them and are in a file on the other side of the kernel it'll be easy to miss them.
If they are wrapped in something that links them back to where they are used grep or whatever indexer can spot them.Quoted 1 line
And this file should not be having different rules than rest of drivers.
There are other places with #ifdefs in match tables. It would be nice if there was a nicer way to do it in C but there doesn't seem to be. It would be nice if vendor specific stuff wasn't in the core code but I don't see a nice way of fixing that either.
And if you want to see some of the more crazy examples of match tables take a look here:
https://elixir.bootlin.com/linux/v7.0-rc6/source/drivers/cpufreq/cpufreq-dt-platdev.c
Quoted 9 lines
Since these matches are not the size of the compatible string but actually 200 bytes on 64 bit machines the two megatables in that driver will consume about 40K. It doesn't seem like much but DT is being used by systems that have megabytes of RAM like a RISC-V softcore on an FPGA. I think they might prefer to have a 40K of usable memory over 40K of tables that don't contain any values that are actually used on their machine ever... and it could be cleaned up easily by wrapping each of the vendor specific chunks in the
Kconfigsymbol for their platform.
40K is more convincing, but the case here isn't 40K.
Maybe we would be better off just creating a device for every reserved memory node with a compatible which is what we do for the rest of the DT. Then there's never any of these strings. It would only cost memory if you have nodes with compatibles and you don't want a device created. Looks like the only ones we have are the 2 NVIDIA ones.
Rob