[PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[Heiko Stuebner]

Recent conversions or inclusions of rk3399 and rk3568 as platform-drivers
and subsequently allowing them to be built as modules introduced some
problems.

These two patches try to correct them.

Heiko Stuebner (2):
  Revert "clk: rockchip: use module_platform_driver_probe"
  clk: rockchip: drop module parts from rk3399 and rk3568 drivers

 drivers/clk/rockchip/Kconfig      | 4 ++--
 drivers/clk/rockchip/clk-rk3399.c | 6 +-----
 drivers/clk/rockchip/clk-rk3568.c | 6 +-----
 3 files changed, 4 insertions(+), 12 deletions(-)

-- 
2.30.2

[PATCH 1/2] Revert "clk: rockchip: use module_platform_driver_probe"

[Heiko Stuebner]

This reverts commit 1da80da028fe5accb866c0d6899a292ed86bef45.

Reading recent discussions [0] [1], I realized this change introduces
a number of problems:

- only converting to module_platform_driver creates the issue
  with the existing __init and __initdata attributes.

  When the driver would've been built as a module, all the missing
  clock-definitions (all are initdata) should've turned up as error
  in testing suggesting that the change wasn't at all

- a clock driver is a very core component of soc bringup and making
  this able to be built as a module solely for enabling the soc vendor
  to add out of tree changes for Android implementations is not in our
  interest and also everything except a ramdisk won't probe without a
  clock controller.

  This is especially true when the changes aren't really tested and
  are merely added to move the mainline driver "out of the way".

[0] https://lwn.net/Articles/872209/
[1] https://lore.kernel.org/all/163529604399.15791.378104318036812951@swboyd.mtv.corp.google.com/

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-rk3399.c | 2 +-
 drivers/clk/rockchip/clk-rk3568.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 7924598747b6..53ed5cca335b 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1656,7 +1656,7 @@ static struct platform_driver clk_rk3399_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
-module_platform_driver_probe(clk_rk3399_driver, clk_rk3399_probe);
+builtin_platform_driver_probe(clk_rk3399_driver, clk_rk3399_probe);
 
 MODULE_DESCRIPTION("Rockchip RK3399 Clock Driver");
 MODULE_LICENSE("GPL");
diff --git a/drivers/clk/rockchip/clk-rk3568.c b/drivers/clk/rockchip/clk-rk3568.c
index 939e7079c334..75ca855e720d 100644
--- a/drivers/clk/rockchip/clk-rk3568.c
+++ b/drivers/clk/rockchip/clk-rk3568.c
@@ -1719,7 +1719,7 @@ static struct platform_driver clk_rk3568_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
-module_platform_driver_probe(clk_rk3568_driver, clk_rk3568_probe);
+builtin_platform_driver_probe(clk_rk3568_driver, clk_rk3568_probe);
 
 MODULE_DESCRIPTION("Rockchip RK3568 Clock Driver");
 MODULE_LICENSE("GPL");
-- 
2.30.2

Re: [PATCH 1/2] Revert "clk: rockchip: use module_platform_driver_probe"

[Stephen Boyd]

Quoting Heiko Stuebner (2021-10-27 06:26:15)
> This reverts commit 1da80da028fe5accb866c0d6899a292ed86bef45.
> 
> Reading recent discussions [0] [1], I realized this change introduces
> a number of problems:
> 
> - only converting to module_platform_driver creates the issue
>   with the existing __init and __initdata attributes.
> 
>   When the driver would've been built as a module, all the missing
>   clock-definitions (all are initdata) should've turned up as error
>   in testing suggesting that the change wasn't at all
> 
> - a clock driver is a very core component of soc bringup and making
>   this able to be built as a module solely for enabling the soc vendor
>   to add out of tree changes for Android implementations is not in our
>   interest and also everything except a ramdisk won't probe without a
>   clock controller.
> 
>   This is especially true when the changes aren't really tested and
>   are merely added to move the mainline driver "out of the way".
> 
> [0] https://lwn.net/Articles/872209/
> [1] https://lore.kernel.org/all/163529604399.15791.378104318036812951@swboyd.mtv.corp.google.com/
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

[PATCH 2/2] clk: rockchip: drop module parts from rk3399 and rk3568 drivers

[Heiko Stuebner]

Both of these drivers were converted to real drivers and got a tristate
build option. But them being builtin_platform_drivers, they only ever
should be build-in - as the name suggests.

So adapt the Kconfig symbol and drop the MODULE_* parts from the drivers.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/Kconfig      | 4 ++--
 drivers/clk/rockchip/clk-rk3399.c | 4 ----
 drivers/clk/rockchip/clk-rk3568.c | 4 ----
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/rockchip/Kconfig b/drivers/clk/rockchip/Kconfig
index 2dfd6a383393..3067bdb6e119 100644
--- a/drivers/clk/rockchip/Kconfig
+++ b/drivers/clk/rockchip/Kconfig
@@ -80,14 +80,14 @@ config CLK_RK3368
 	  Build the driver for RK3368 Clock Driver.
 
 config CLK_RK3399
-	tristate "Rockchip RK3399 clock controller support"
+	bool "Rockchip RK3399 clock controller support"
 	depends on ARM64 || COMPILE_TEST
 	default y
 	help
 	  Build the driver for RK3399 Clock Driver.
 
 config CLK_RK3568
-	tristate "Rockchip RK3568 clock controller support"
+	bool "Rockchip RK3568 clock controller support"
 	depends on ARM64 || COMPILE_TEST
 	default y
 	help
diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 53ed5cca335b..306910a3a0d3 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1630,7 +1630,6 @@ static const struct of_device_id clk_rk3399_match_table[] = {
 	},
 	{ }
 };
-MODULE_DEVICE_TABLE(of, clk_rk3399_match_table);
 
 static int __init clk_rk3399_probe(struct platform_device *pdev)
 {
@@ -1657,6 +1656,3 @@ static struct platform_driver clk_rk3399_driver = {
 	},
 };
 builtin_platform_driver_probe(clk_rk3399_driver, clk_rk3399_probe);
-
-MODULE_DESCRIPTION("Rockchip RK3399 Clock Driver");
-MODULE_LICENSE("GPL");
diff --git a/drivers/clk/rockchip/clk-rk3568.c b/drivers/clk/rockchip/clk-rk3568.c
index 75ca855e720d..69a9e8069a48 100644
--- a/drivers/clk/rockchip/clk-rk3568.c
+++ b/drivers/clk/rockchip/clk-rk3568.c
@@ -1693,7 +1693,6 @@ static const struct of_device_id clk_rk3568_match_table[] = {
 	},
 	{ }
 };
-MODULE_DEVICE_TABLE(of, clk_rk3568_match_table);
 
 static int __init clk_rk3568_probe(struct platform_device *pdev)
 {
@@ -1720,6 +1719,3 @@ static struct platform_driver clk_rk3568_driver = {
 	},
 };
 builtin_platform_driver_probe(clk_rk3568_driver, clk_rk3568_probe);
-
-MODULE_DESCRIPTION("Rockchip RK3568 Clock Driver");
-MODULE_LICENSE("GPL");
-- 
2.30.2

Re: [PATCH 2/2] clk: rockchip: drop module parts from rk3399 and rk3568 drivers

[Stephen Boyd]

Quoting Heiko Stuebner (2021-10-27 06:26:16)
> Both of these drivers were converted to real drivers and got a tristate
> build option. But them being builtin_platform_drivers, they only ever
> should be build-in - as the name suggests.
> 
> So adapt the Kconfig symbol and drop the MODULE_* parts from the drivers.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[Stephen Boyd]

Quoting Heiko Stuebner (2021-10-27 06:26:14)
> Recent conversions or inclusions of rk3399 and rk3568 as platform-drivers
> and subsequently allowing them to be built as modules introduced some
> problems.
> 
> These two patches try to correct them.

By removing modular support? Ok.

Re: [PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[Heiko Stübner]

Am Mittwoch, 27. Oktober 2021, 22:27:46 CEST schrieb Stephen Boyd:
> Quoting Heiko Stuebner (2021-10-27 06:26:14)
> > Recent conversions or inclusions of rk3399 and rk3568 as platform-drivers
> > and subsequently allowing them to be built as modules introduced some
> > problems.
> > 
> > These two patches try to correct them.
> 
> By removing modular support? Ok.

It looked like the easiest way to go for now.

With all the clock-definitions as init-data still in both drivers, I don't think
anybody ever tested running this as a real module.

We could of course also remove all the __init + __initdata attributes, but
still wouldn't know if it actually runs as a module, without someone
building a real test environment for it.


Heiko

Re: [PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[Stephen Boyd]

Quoting Heiko Stübner (2021-10-27 16:20:17)
> Am Mittwoch, 27. Oktober 2021, 22:27:46 CEST schrieb Stephen Boyd:
> > Quoting Heiko Stuebner (2021-10-27 06:26:14)
> > > Recent conversions or inclusions of rk3399 and rk3568 as platform-drivers
> > > and subsequently allowing them to be built as modules introduced some
> > > problems.
> > > 
> > > These two patches try to correct them.
> > 
> > By removing modular support? Ok.
> 
> It looked like the easiest way to go for now.
> 
> With all the clock-definitions as init-data still in both drivers, I don't think
> anybody ever tested running this as a real module.
> 
> We could of course also remove all the __init + __initdata attributes, but
> still wouldn't know if it actually runs as a module, without someone
> building a real test environment for it.
> 
> 

Sure. Do I need to pick these up directly?

Re: [PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[Heiko Stübner]

Am Freitag, 29. Oktober 2021, 08:43:05 CEST schrieb Stephen Boyd:
> Quoting Heiko Stübner (2021-10-27 16:20:17)
> > Am Mittwoch, 27. Oktober 2021, 22:27:46 CEST schrieb Stephen Boyd:
> > > Quoting Heiko Stuebner (2021-10-27 06:26:14)
> > > > Recent conversions or inclusions of rk3399 and rk3568 as platform-drivers
> > > > and subsequently allowing them to be built as modules introduced some
> > > > problems.
> > > > 
> > > > These two patches try to correct them.
> > > 
> > > By removing modular support? Ok.
> > 
> > It looked like the easiest way to go for now.
> > 
> > With all the clock-definitions as init-data still in both drivers, I don't think
> > anybody ever tested running this as a real module.
> > 
> > We could of course also remove all the __init + __initdata attributes, but
> > still wouldn't know if it actually runs as a module, without someone
> > building a real test environment for it.
> > 
> > 
> 
> Sure. Do I need to pick these up directly?

I don't have anything more for 5.16 and the 5.15 release should happen
on sunday, so yeah just pick them if you like - no need for doing a separate
pull request.

Thanks
Heiko

Re: [PATCH 0/2] clk: rockchip: cleanup errors in (module-)driver handling

[Stephen Boyd]

Quoting Heiko Stübner (2021-10-29 05:37:06)
> Am Freitag, 29. Oktober 2021, 08:43:05 CEST schrieb Stephen Boyd:
> > Quoting Heiko St�bner (2021-10-27 16:20:17)
> > > Am Mittwoch, 27. Oktober 2021, 22:27:46 CEST schrieb Stephen Boyd:
> > > > Quoting Heiko Stuebner (2021-10-27 06:26:14)
> > > > > Recent conversions or inclusions of rk3399 and rk3568 as platform-drivers
> > > > > and subsequently allowing them to be built as modules introduced some
> > > > > problems.
> > > > > 
> > > > > These two patches try to correct them.
> > > > 
> > > > By removing modular support? Ok.
> > > 
> > > It looked like the easiest way to go for now.
> > > 
> > > With all the clock-definitions as init-data still in both drivers, I don't think
> > > anybody ever tested running this as a real module.
> > > 
> > > We could of course also remove all the __init + __initdata attributes, but
> > > still wouldn't know if it actually runs as a module, without someone
> > > building a real test environment for it.
> > > 
> > > 
> > 
> > Sure. Do I need to pick these up directly?
> 
> I don't have anything more for 5.16 and the 5.15 release should happen
> on sunday, so yeah just pick them if you like - no need for doing a separate
> pull request.
> 

Ok I picked them up and I'll send them off next week.