* [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe()
@ 2023-12-17 19:31 Uwe Kleine-König
2023-12-17 19:31 ` [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS Uwe Kleine-König
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2023-12-17 19:31 UTC (permalink / raw)
To: Santosh Shilimkar, Krzysztof Kozlowski; +Cc: kernel, linux-kernel
Hello,
while preparing the series to convert the platform drivers below
drivers/memory to use .remove_new()[1], I noticed that the emif driver
unnecessarily uses some #ifdefs and doesn't use platform_driver_probe()
correctly. Note there is a conflict between these series. Tell me if you
need support to resolve this.
The alternative to the third patch is to do s/__init_or_module/__init/
in the driver and mark emif_driver with __refdata.
Best regards
Uwe
[1] https://lore.kernel.org/r/cover.1702822744.git.u.kleine-koenig@pengutronix.de
Uwe Kleine-König (3):
memory: emif: Simplify code handling CONFIG_DEBUG_FS
memory: emif: Simplify code handling CONFIG_OF
memory: emif: Drop usage of platform_driver_probe()
drivers/memory/emif.c | 63 ++++++++++++++++---------------------------
1 file changed, 23 insertions(+), 40 deletions(-)
base-commit: 17cb8a20bde66a520a2ca7aad1063e1ce7382240
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS
2023-12-17 19:31 [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Uwe Kleine-König
@ 2023-12-17 19:31 ` Uwe Kleine-König
2024-01-23 15:18 ` Krzysztof Kozlowski
2023-12-17 19:31 ` [PATCH 2/3] memory: emif: Simplify code handling CONFIG_OF Uwe Kleine-König
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-12-17 19:31 UTC (permalink / raw)
To: Santosh Shilimkar, Krzysztof Kozlowski; +Cc: kernel, linux-kernel
Instead of using #ifdef make use of IS_ENABLED().
According to bloat-o-meter this patch doesn't change code sizes with
CONFIG_DEBUG_FS=n.
Also change emif_debugfs_init() to return void. The only caller doesn't
check the return value anyhow.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/memory/emif.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index f305643209f0..dd1d87f8fdc8 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -72,7 +72,6 @@ static DEFINE_SPINLOCK(emif_lock);
static unsigned long irq_state;
static LIST_HEAD(device_list);
-#ifdef CONFIG_DEBUG_FS
static void do_emif_regdump_show(struct seq_file *s, struct emif_data *emif,
struct emif_regs *regs)
{
@@ -140,31 +139,24 @@ static int emif_mr4_show(struct seq_file *s, void *unused)
DEFINE_SHOW_ATTRIBUTE(emif_mr4);
-static int __init_or_module emif_debugfs_init(struct emif_data *emif)
+static void __init_or_module emif_debugfs_init(struct emif_data *emif)
{
- emif->debugfs_root = debugfs_create_dir(dev_name(emif->dev), NULL);
- debugfs_create_file("regcache_dump", S_IRUGO, emif->debugfs_root, emif,
- &emif_regdump_fops);
- debugfs_create_file("mr4", S_IRUGO, emif->debugfs_root, emif,
- &emif_mr4_fops);
- return 0;
+ if (IS_ENABLED(CONFIG_DEBUG_FS)) {
+ emif->debugfs_root = debugfs_create_dir(dev_name(emif->dev), NULL);
+ debugfs_create_file("regcache_dump", S_IRUGO, emif->debugfs_root, emif,
+ &emif_regdump_fops);
+ debugfs_create_file("mr4", S_IRUGO, emif->debugfs_root, emif,
+ &emif_mr4_fops);
+ }
}
static void __exit emif_debugfs_exit(struct emif_data *emif)
{
- debugfs_remove_recursive(emif->debugfs_root);
- emif->debugfs_root = NULL;
+ if (IS_ENABLED(CONFIG_DEBUG_FS)) {
+ debugfs_remove_recursive(emif->debugfs_root);
+ emif->debugfs_root = NULL;
+ }
}
-#else
-static inline int __init_or_module emif_debugfs_init(struct emif_data *emif)
-{
- return 0;
-}
-
-static inline void __exit emif_debugfs_exit(struct emif_data *emif)
-{
-}
-#endif
/*
* Get bus width used by EMIF. Note that this may be different from the
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] memory: emif: Simplify code handling CONFIG_OF
2023-12-17 19:31 [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Uwe Kleine-König
2023-12-17 19:31 ` [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS Uwe Kleine-König
@ 2023-12-17 19:31 ` Uwe Kleine-König
2024-01-23 15:18 ` Krzysztof Kozlowski
2023-12-17 19:31 ` [PATCH 3/3] memory: emif: Drop usage of platform_driver_probe() Uwe Kleine-König
2023-12-21 20:46 ` [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Krzysztof Kozlowski
3 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-12-17 19:31 UTC (permalink / raw)
To: Santosh Shilimkar, Krzysztof Kozlowski; +Cc: kernel, linux-kernel
The first thing that of_get_memory_device_details() does is calling
of_parse_phandle(). With CONFIG_OF=n this returns NULL in a static
inline function. So the compiler can determine that
of_get_memory_device_details() also returns NULL. bloat-o-meter confirms
that this patch has no effects on the size of the generated code for
CONFIG_OF=n builds.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/memory/emif.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index dd1d87f8fdc8..5a3194b9b3bc 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -826,7 +826,6 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
return valid;
}
-#if defined(CONFIG_OF)
static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
struct emif_data *emif)
{
@@ -983,15 +982,6 @@ static struct emif_data * __init_or_module of_get_memory_device_details(
return emif;
}
-#else
-
-static struct emif_data * __init_or_module of_get_memory_device_details(
- struct device_node *np_emif, struct device *dev)
-{
- return NULL;
-}
-#endif
-
static struct emif_data *__init_or_module get_device_details(
struct platform_device *pdev)
{
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] memory: emif: Drop usage of platform_driver_probe()
2023-12-17 19:31 [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Uwe Kleine-König
2023-12-17 19:31 ` [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS Uwe Kleine-König
2023-12-17 19:31 ` [PATCH 2/3] memory: emif: Simplify code handling CONFIG_OF Uwe Kleine-König
@ 2023-12-17 19:31 ` Uwe Kleine-König
2024-01-23 15:17 ` Krzysztof Kozlowski
2023-12-21 20:46 ` [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Krzysztof Kozlowski
3 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2023-12-17 19:31 UTC (permalink / raw)
To: Santosh Shilimkar, Krzysztof Kozlowski; +Cc: kernel, linux-kernel
There are considerations to drop platform_driver_probe() as a concept
that isn't relevant any more today. It comes with an added complexity
that makes many users hold it wrong. (E.g. this driver should have
better used __init instead of __init_or_module to mark functions only
relevant to .probe() and mark the driver struct with __refdata.)
This fixes a W=1 build warning:
WARNING: modpost: drivers/memory/emif: section mismatch in reference: emif_driver+0x4 (section: .data) -> emif_remove (section: .exit.text)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/memory/emif.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
index 5a3194b9b3bc..84973d7133f4 100644
--- a/drivers/memory/emif.c
+++ b/drivers/memory/emif.c
@@ -139,7 +139,7 @@ static int emif_mr4_show(struct seq_file *s, void *unused)
DEFINE_SHOW_ATTRIBUTE(emif_mr4);
-static void __init_or_module emif_debugfs_init(struct emif_data *emif)
+static void emif_debugfs_init(struct emif_data *emif)
{
if (IS_ENABLED(CONFIG_DEBUG_FS)) {
emif->debugfs_root = debugfs_create_dir(dev_name(emif->dev), NULL);
@@ -150,7 +150,7 @@ static void __init_or_module emif_debugfs_init(struct emif_data *emif)
}
}
-static void __exit emif_debugfs_exit(struct emif_data *emif)
+static void emif_debugfs_exit(struct emif_data *emif)
{
if (IS_ENABLED(CONFIG_DEBUG_FS)) {
debugfs_remove_recursive(emif->debugfs_root);
@@ -671,7 +671,7 @@ static void disable_and_clear_all_interrupts(struct emif_data *emif)
clear_all_interrupts(emif);
}
-static int __init_or_module setup_interrupts(struct emif_data *emif, u32 irq)
+static int setup_interrupts(struct emif_data *emif, u32 irq)
{
u32 interrupts, type;
void __iomem *base = emif->base;
@@ -702,7 +702,7 @@ static int __init_or_module setup_interrupts(struct emif_data *emif, u32 irq)
}
-static void __init_or_module emif_onetime_settings(struct emif_data *emif)
+static void emif_onetime_settings(struct emif_data *emif)
{
u32 pwr_mgmt_ctrl, zq, temp_alert_cfg;
void __iomem *base = emif->base;
@@ -826,7 +826,7 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
return valid;
}
-static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
+static void of_get_custom_configs(struct device_node *np_emif,
struct emif_data *emif)
{
struct emif_custom_configs *cust_cfgs = NULL;
@@ -875,7 +875,7 @@ static void __init_or_module of_get_custom_configs(struct device_node *np_emif,
emif->plat_data->custom_configs = cust_cfgs;
}
-static void __init_or_module of_get_ddr_info(struct device_node *np_emif,
+static void of_get_ddr_info(struct device_node *np_emif,
struct device_node *np_ddr,
struct ddr_device_info *dev_info)
{
@@ -909,7 +909,7 @@ static void __init_or_module of_get_ddr_info(struct device_node *np_emif,
dev_info->io_width = __fls(io_width) - 1;
}
-static struct emif_data * __init_or_module of_get_memory_device_details(
+static struct emif_data *of_get_memory_device_details(
struct device_node *np_emif, struct device *dev)
{
struct emif_data *emif = NULL;
@@ -1086,7 +1086,7 @@ static struct emif_data *__init_or_module get_device_details(
return NULL;
}
-static int __init_or_module emif_probe(struct platform_device *pdev)
+static int emif_probe(struct platform_device *pdev)
{
struct emif_data *emif;
int irq, ret;
@@ -1141,7 +1141,7 @@ static int __init_or_module emif_probe(struct platform_device *pdev)
return -ENODEV;
}
-static int __exit emif_remove(struct platform_device *pdev)
+static int emif_remove(struct platform_device *pdev)
{
struct emif_data *emif = platform_get_drvdata(pdev);
@@ -1167,7 +1167,8 @@ MODULE_DEVICE_TABLE(of, emif_of_match);
#endif
static struct platform_driver emif_driver = {
- .remove = __exit_p(emif_remove),
+ .probe = emif_probe,
+ .remove = emif_remove,
.shutdown = emif_shutdown,
.driver = {
.name = "emif",
@@ -1175,7 +1176,7 @@ static struct platform_driver emif_driver = {
},
};
-module_platform_driver_probe(emif_driver, emif_probe);
+module_platform_driver(emif_driver);
MODULE_DESCRIPTION("TI EMIF SDRAM Controller Driver");
MODULE_LICENSE("GPL");
--
2.42.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe()
2023-12-17 19:31 [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Uwe Kleine-König
` (2 preceding siblings ...)
2023-12-17 19:31 ` [PATCH 3/3] memory: emif: Drop usage of platform_driver_probe() Uwe Kleine-König
@ 2023-12-21 20:46 ` Krzysztof Kozlowski
2023-12-21 21:25 ` Uwe Kleine-König
3 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-12-21 20:46 UTC (permalink / raw)
To: Uwe Kleine-König, Santosh Shilimkar; +Cc: kernel, linux-kernel
On 17/12/2023 20:31, Uwe Kleine-König wrote:
> Hello,
>
> while preparing the series to convert the platform drivers below
> drivers/memory to use .remove_new()[1], I noticed that the emif driver
> unnecessarily uses some #ifdefs and doesn't use platform_driver_probe()
> correctly. Note there is a conflict between these series. Tell me if you
> need support to resolve this.
>
I was waiting here for some Reviews or Tested-by. Time passed, reviews
did not happen but it is too late for me to take it for the next merge
window. I will take it after the merge window.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe()
2023-12-21 20:46 ` [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Krzysztof Kozlowski
@ 2023-12-21 21:25 ` Uwe Kleine-König
0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2023-12-21 21:25 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: Santosh Shilimkar, kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
Hello Krzysztof,
On Thu, Dec 21, 2023 at 09:46:29PM +0100, Krzysztof Kozlowski wrote:
> On 17/12/2023 20:31, Uwe Kleine-König wrote:
> > while preparing the series to convert the platform drivers below
> > drivers/memory to use .remove_new()[1], I noticed that the emif driver
> > unnecessarily uses some #ifdefs and doesn't use platform_driver_probe()
> > correctly. Note there is a conflict between these series. Tell me if you
> > need support to resolve this.
>
> I was waiting here for some Reviews or Tested-by. Time passed, reviews
> did not happen but it is too late for me to take it for the next merge
> window. I will take it after the merge window.
Thanks for the status update. That's very appreciated. (And taking it
for the next development cycle is entirely fine, that's just a drive-by
series, nothing on my side depends on it.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] memory: emif: Drop usage of platform_driver_probe()
2023-12-17 19:31 ` [PATCH 3/3] memory: emif: Drop usage of platform_driver_probe() Uwe Kleine-König
@ 2024-01-23 15:17 ` Krzysztof Kozlowski
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-23 15:17 UTC (permalink / raw)
To: Uwe Kleine-König, Santosh Shilimkar; +Cc: kernel, linux-kernel
On 17/12/2023 20:31, Uwe Kleine-König wrote:
> There are considerations to drop platform_driver_probe() as a concept
> that isn't relevant any more today. It comes with an added complexity
> that makes many users hold it wrong. (E.g. this driver should have
> better used __init instead of __init_or_module to mark functions only
> relevant to .probe() and mark the driver struct with __refdata.)
>
> This fixes a W=1 build warning:
>
> WARNING: modpost: drivers/memory/emif: section mismatch in reference: emif_driver+0x4 (section: .data) -> emif_remove (section: .exit.text)
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/memory/emif.c | 23 ++++++++++++-----------
I applied #1 and #2. This does not apply. Please rebase and resend.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS
2023-12-17 19:31 ` [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS Uwe Kleine-König
@ 2024-01-23 15:18 ` Krzysztof Kozlowski
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-23 15:18 UTC (permalink / raw)
To: Uwe Kleine-König, Santosh Shilimkar; +Cc: kernel, linux-kernel
On 17/12/2023 20:31, Uwe Kleine-König wrote:
> Instead of using #ifdef make use of IS_ENABLED().
>
> According to bloat-o-meter this patch doesn't change code sizes with
> CONFIG_DEBUG_FS=n.
>
> Also change emif_debugfs_init() to return void. The only caller doesn't
> check the return value anyhow.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
Applied.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] memory: emif: Simplify code handling CONFIG_OF
2023-12-17 19:31 ` [PATCH 2/3] memory: emif: Simplify code handling CONFIG_OF Uwe Kleine-König
@ 2024-01-23 15:18 ` Krzysztof Kozlowski
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-23 15:18 UTC (permalink / raw)
To: Uwe Kleine-König, Santosh Shilimkar; +Cc: kernel, linux-kernel
On 17/12/2023 20:31, Uwe Kleine-König wrote:
> The first thing that of_get_memory_device_details() does is calling
> of_parse_phandle(). With CONFIG_OF=n this returns NULL in a static
> inline function. So the compiler can determine that
> of_get_memory_device_details() also returns NULL. bloat-o-meter confirms
> that this patch has no effects on the size of the generated code for
> CONFIG_OF=n builds.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/memory/emif.c | 10 ----------
> 1 file changed, 10 deletions(-)
Applied.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-01-23 15:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-17 19:31 [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Uwe Kleine-König
2023-12-17 19:31 ` [PATCH 1/3] memory: emif: Simplify code handling CONFIG_DEBUG_FS Uwe Kleine-König
2024-01-23 15:18 ` Krzysztof Kozlowski
2023-12-17 19:31 ` [PATCH 2/3] memory: emif: Simplify code handling CONFIG_OF Uwe Kleine-König
2024-01-23 15:18 ` Krzysztof Kozlowski
2023-12-17 19:31 ` [PATCH 3/3] memory: emif: Drop usage of platform_driver_probe() Uwe Kleine-König
2024-01-23 15:17 ` Krzysztof Kozlowski
2023-12-21 20:46 ` [PATCH 0/3] memory: emif: ifdefs and platform_driver_probe() Krzysztof Kozlowski
2023-12-21 21:25 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).