diff mbox series

[1/2] soc: samsung: exynos-pmu: instantiate clkout driver as MFD

Message ID 20201001165646.32279-2-krzk@kernel.org
State Accepted
Headers show
Series samsung: exynos: convert clkout to module driver | expand

Commit Message

Krzysztof Kozlowski Oct. 1, 2020, 4:56 p.m. UTC
The Exynos clock output (clkout) driver uses same register address space
(Power Management Unit address space) as Exynos PMU driver and same set
of compatibles.  It was modeled as clock provider instantiated with
CLK_OF_DECLARE_DRIVE().

This however brings ordering problems and lack of probe deferral,
therefore clkout driver should be converted to a regular module and
instantiated as a child of PMU driver to be able to use existing
compatibles and address space.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/soc/samsung/exynos-pmu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Stephen Boyd Oct. 14, 2020, 2:44 a.m. UTC | #1
Quoting Krzysztof Kozlowski (2020-10-01 09:56:45)
> The Exynos clock output (clkout) driver uses same register address space
> (Power Management Unit address space) as Exynos PMU driver and same set
> of compatibles.  It was modeled as clock provider instantiated with
> CLK_OF_DECLARE_DRIVE().
> 
> This however brings ordering problems and lack of probe deferral,
> therefore clkout driver should be converted to a regular module and
> instantiated as a child of PMU driver to be able to use existing
> compatibles and address space.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Sylwester Nawrocki Oct. 23, 2020, 11:34 a.m. UTC | #2
Hi,

On 10/1/20 18:56, Krzysztof Kozlowski wrote:
> The Exynos clock output (clkout) driver uses same register address space
> (Power Management Unit address space) as Exynos PMU driver and same set
> of compatibles.  It was modeled as clock provider instantiated with
> CLK_OF_DECLARE_DRIVE().
> 
> This however brings ordering problems and lack of probe deferral,
> therefore clkout driver should be converted to a regular module and
> instantiated as a child of PMU driver to be able to use existing
> compatibles and address space.

It might have been cleaner to have the CLKOUT device as a PMU subnode in DT, 
then device instantiation would be already covered by devm_of_platform_populate().
But it gets a bit complicated to make such a change in a backward compatible way.

I have tested both patches on Trats2, where CLKOUT provides master clock for
the audio codec.

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
 

> @@ -128,6 +134,11 @@ static int exynos_pmu_probe(struct platform_device *pdev)
>   
>   	platform_set_drvdata(pdev, pmu_context);
>   
> +	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, exynos_pmu_devs,
> +				   ARRAY_SIZE(exynos_pmu_devs), NULL, 0, NULL);
> +	if (ret)
> +		return ret;
> +
>   	if (devm_of_platform_populate(dev))
>   		dev_err(dev, "Error populating children, reboot and poweroff might not work properly\n");
Krzysztof Kozlowski Oct. 23, 2020, 11:37 a.m. UTC | #3
On Fri, Oct 23, 2020 at 01:34:19PM +0200, Sylwester Nawrocki wrote:
> Hi,
> 
> On 10/1/20 18:56, Krzysztof Kozlowski wrote:
> > The Exynos clock output (clkout) driver uses same register address space
> > (Power Management Unit address space) as Exynos PMU driver and same set
> > of compatibles.  It was modeled as clock provider instantiated with
> > CLK_OF_DECLARE_DRIVE().
> > 
> > This however brings ordering problems and lack of probe deferral,
> > therefore clkout driver should be converted to a regular module and
> > instantiated as a child of PMU driver to be able to use existing
> > compatibles and address space.
> 
> It might have been cleaner to have the CLKOUT device as a PMU subnode in DT, 
> then device instantiation would be already covered by devm_of_platform_populate().
> But it gets a bit complicated to make such a change in a backward compatible way.

Yes, I agree, but the backward compatibility would be here a pain.
Optionally the driver could check for new DTB and skip adding MFD
children... but this is just simpler.

> 
> I have tested both patches on Trats2, where CLKOUT provides master clock for
> the audio codec.
> 
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Thanks!

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 28, 2020, 10:02 p.m. UTC | #4
On Thu, Oct 01, 2020 at 06:56:45PM +0200, Krzysztof Kozlowski wrote:
> The Exynos clock output (clkout) driver uses same register address space
> (Power Management Unit address space) as Exynos PMU driver and same set
> of compatibles.  It was modeled as clock provider instantiated with
> CLK_OF_DECLARE_DRIVE().
> 
> This however brings ordering problems and lack of probe deferral,
> therefore clkout driver should be converted to a regular module and
> instantiated as a child of PMU driver to be able to use existing
> compatibles and address space.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/soc/samsung/exynos-pmu.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Applied with all tags.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/soc/samsung/exynos-pmu.c b/drivers/soc/samsung/exynos-pmu.c
index 17304fa18429..a18c93a4646c 100644
--- a/drivers/soc/samsung/exynos-pmu.c
+++ b/drivers/soc/samsung/exynos-pmu.c
@@ -8,6 +8,7 @@ 
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/mfd/core.h>
 #include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
@@ -97,6 +98,10 @@  static const struct of_device_id exynos_pmu_of_device_ids[] = {
 	{ /*sentinel*/ },
 };
 
+static const struct mfd_cell exynos_pmu_devs[] = {
+	{ .name = "exynos-clkout", },
+};
+
 struct regmap *exynos_get_pmu_regmap(void)
 {
 	struct device_node *np = of_find_matching_node(NULL,
@@ -110,6 +115,7 @@  EXPORT_SYMBOL_GPL(exynos_get_pmu_regmap);
 static int exynos_pmu_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	int ret;
 
 	pmu_base_addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pmu_base_addr))
@@ -128,6 +134,11 @@  static int exynos_pmu_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pmu_context);
 
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, exynos_pmu_devs,
+				   ARRAY_SIZE(exynos_pmu_devs), NULL, 0, NULL);
+	if (ret)
+		return ret;
+
 	if (devm_of_platform_populate(dev))
 		dev_err(dev, "Error populating children, reboot and poweroff might not work properly\n");