[PATCH v2 RESEND 0/5] x86: clk: Add config option to enable 48MHz fixed fch clk

[PATCH v2 RESEND 0/5] x86: clk: Add config option to enable 48MHz fixed fch clk

[Ajit Kumar Pandey]

changes since v1:
- Remove unsused variable

Ajit Kumar Pandey (5):
  x86: clk: Add config option to enable 48MHz fixed fch clk
  drivers: acpi: acpi_apd: Remove unused device property "is-rv"
  ACPI: APD: Add a fmw property clk-name
  clk: x86: Use dynamic con_id string during clk registration
  clk: x86: Fix clk_gate_flags for RV_CLK_GATE

 drivers/acpi/acpi_apd.c               | 11 +++++++++--
 drivers/clk/x86/Kconfig               |  5 +++++
 drivers/clk/x86/clk-fch.c             | 26 +++++++++++++-------------
 include/linux/platform_data/clk-fch.h |  2 +-
 4 files changed, 28 insertions(+), 16 deletions(-)

-- 
2.25.1

[PATCH v2 RESEND 1/5] x86: clk: Add config option to enable 48MHz fixed fch clk

[Ajit Kumar Pandey]

At present 48MHz clk support is only enabled for RV architecture
using "is-rv" device property initialized from boot loader. This
limit 48MHz fixed clock gate support to RV platform unless we add
similar device property in boot loader for other architecture.

Add Kernel config option to enable 48MHz fixed clk gate registration
with clock framework. This enahanced flexibility to enable 48MHz fch
clock support on any platforms by simply enabling kernel config. Also
replace RV with FIXED as generic naming convention across platforms.

Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
---
 drivers/clk/x86/Kconfig   |  5 +++++
 drivers/clk/x86/clk-fch.c | 22 +++++++++++-----------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/x86/Kconfig b/drivers/clk/x86/Kconfig
index 69642e15fcc1..c10081774cd6 100644
--- a/drivers/clk/x86/Kconfig
+++ b/drivers/clk/x86/Kconfig
@@ -6,3 +6,8 @@ config CLK_LGM_CGU
 	help
 	  Clock Generation Unit(CGU) driver for Intel Lightning Mountain(LGM)
 	  network processor SoC.
+
+config CLK_FIXED_FCH
+	bool "AMD FCH controller fixed 48MHz CLK support"
+	help
+	  Enable this option for 48MHz fixed mclk support on AMD platforms.
diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c
index 8f7c5142b0f0..72d2c7497234 100644
--- a/drivers/clk/x86/clk-fch.c
+++ b/drivers/clk/x86/clk-fch.c
@@ -26,9 +26,9 @@
 #define ST_CLK_GATE	3
 #define ST_MAX_CLKS	4
 
-#define RV_CLK_48M	0
-#define RV_CLK_GATE	1
-#define RV_MAX_CLKS	2
+#define CLK_48M_FIXED	0
+#define CLK_GATE_FIXED	1
+#define CLK_MAX_FIXED	2
 
 static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" };
 static struct clk_hw *hws[ST_MAX_CLKS];
@@ -41,7 +41,7 @@ static int fch_clk_probe(struct platform_device *pdev)
 	if (!fch_data || !fch_data->base)
 		return -EINVAL;
 
-	if (!fch_data->is_rv) {
+	if (!IS_ENABLED(CONFIG_CLK_FIXED_FCH)) {
 		hws[ST_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz",
 			NULL, 0, 48000000);
 		hws[ST_CLK_25M] = clk_hw_register_fixed_rate(NULL, "clk25MHz",
@@ -61,14 +61,14 @@ static int fch_clk_probe(struct platform_device *pdev)
 		devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE],
 			"oscout1", NULL);
 	} else {
-		hws[RV_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz",
+		hws[CLK_48M_FIXED] = clk_hw_register_fixed_rate(NULL, "clk48MHz",
 			NULL, 0, 48000000);
 
-		hws[RV_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1",
+		hws[CLK_GATE_FIXED] = clk_hw_register_gate(NULL, "oscout1",
 			"clk48MHz", 0, fch_data->base + MISCCLKCNTL1,
 			OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL);
 
-		devm_clk_hw_register_clkdev(&pdev->dev, hws[RV_CLK_GATE],
+		devm_clk_hw_register_clkdev(&pdev->dev, hws[CLK_GATE_FIXED],
 			"oscout1", NULL);
 	}
 
@@ -78,11 +78,11 @@ static int fch_clk_probe(struct platform_device *pdev)
 static int fch_clk_remove(struct platform_device *pdev)
 {
 	int i, clks;
-	struct fch_clk_data *fch_data;
-
-	fch_data = dev_get_platdata(&pdev->dev);
 
-	clks = fch_data->is_rv ? RV_MAX_CLKS : ST_MAX_CLKS;
+	if (IS_ENABLED(CONFIG_CLK_FIXED_FCH))
+		clks = CLK_MAX_FIXED;
+	else
+		clks = ST_MAX_CLKS;
 
 	for (i = 0; i < clks; i++)
 		clk_hw_unregister(hws[i]);
-- 
2.25.1

[PATCH v2 RESEND 2/5] drivers: acpi: acpi_apd: Remove unused device property "is-rv"

[Ajit Kumar Pandey]

Initially "is-rv" device property is added for 48MHz fixed clock
support on Raven or RV architecture. It's unused now as we moved
to clock config based selection to extend such support on others
architecture. This change removed unused code from acpi driver.

Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
---
 drivers/acpi/acpi_apd.c               | 3 ---
 include/linux/platform_data/clk-fch.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
index 6e02448d15d9..6913e9712852 100644
--- a/drivers/acpi/acpi_apd.c
+++ b/drivers/acpi/acpi_apd.c
@@ -87,9 +87,6 @@ static int fch_misc_setup(struct apd_private_data *pdata)
 	if (ret < 0)
 		return -ENOENT;
 
-	if (!acpi_dev_get_property(adev, "is-rv", ACPI_TYPE_INTEGER, &obj))
-		clk_data->is_rv = obj->integer.value;
-
 	list_for_each_entry(rentry, &resource_list, node) {
 		clk_data->base = devm_ioremap(&adev->dev, rentry->res->start,
 					      resource_size(rentry->res));
diff --git a/include/linux/platform_data/clk-fch.h b/include/linux/platform_data/clk-fch.h
index b9f682459f08..850ca776156d 100644
--- a/include/linux/platform_data/clk-fch.h
+++ b/include/linux/platform_data/clk-fch.h
@@ -12,7 +12,6 @@
 
 struct fch_clk_data {
 	void __iomem *base;
-	u32 is_rv;
 };
 
 #endif /* __CLK_FCH_H */
-- 
2.25.1

[PATCH v2 RESEND 3/5] ACPI: APD: Add a fmw property clk-name

[Ajit Kumar Pandey]

Add a new device property to fetch clk-name from firmware.

Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
---
 drivers/acpi/acpi_apd.c               | 10 ++++++++++
 include/linux/platform_data/clk-fch.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
index 6913e9712852..2b958b426b03 100644
--- a/drivers/acpi/acpi_apd.c
+++ b/drivers/acpi/acpi_apd.c
@@ -87,6 +87,16 @@ static int fch_misc_setup(struct apd_private_data *pdata)
 	if (ret < 0)
 		return -ENOENT;
 
+	if (!acpi_dev_get_property(adev, "clk-name", ACPI_TYPE_STRING, &obj)) {
+		clk_data->name = devm_kzalloc(&adev->dev, obj->string.length,
+					      GFP_KERNEL);
+
+		strcpy(clk_data->name, obj->string.pointer);
+	} else {
+		/* Set default name to mclk if entry missing in firmware */
+		clk_data->name = "mclk";
+	}
+
 	list_for_each_entry(rentry, &resource_list, node) {
 		clk_data->base = devm_ioremap(&adev->dev, rentry->res->start,
 					      resource_size(rentry->res));
diff --git a/include/linux/platform_data/clk-fch.h b/include/linux/platform_data/clk-fch.h
index 850ca776156d..11a2a23fd9b2 100644
--- a/include/linux/platform_data/clk-fch.h
+++ b/include/linux/platform_data/clk-fch.h
@@ -12,6 +12,7 @@
 
 struct fch_clk_data {
 	void __iomem *base;
+	char *name;
 };
 
 #endif /* __CLK_FCH_H */
-- 
2.25.1

[PATCH v2 RESEND 4/5] clk: x86: Use dynamic con_id string during clk registration

[Ajit Kumar Pandey]

Replace hard coded con_id string with fch_data->name. We have clk
consumers looking up with different clock names, hence use dynamic
con_id string during clk lookup registration. fch_data->name will
be initialized in acpi driver based on fmw property value.

Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
---
 drivers/clk/x86/clk-fch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c
index 72d2c7497234..19a5c8cf1b3f 100644
--- a/drivers/clk/x86/clk-fch.c
+++ b/drivers/clk/x86/clk-fch.c
@@ -69,7 +69,7 @@ static int fch_clk_probe(struct platform_device *pdev)
 			OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL);
 
 		devm_clk_hw_register_clkdev(&pdev->dev, hws[CLK_GATE_FIXED],
-			"oscout1", NULL);
+					    fch_data->name, NULL);
 	}
 
 	return 0;
-- 
2.25.1

[PATCH v2 RESEND 5/5] clk: x86: Fix clk_gate_flags for RV_CLK_GATE

[Ajit Kumar Pandey]

In AMD's SoC we have to clear bit for disabling 48MHz oscillator
clock gate. Remove CLK_GATE_SET_TO_DISABLE flag for proper enable
and disable of 48MHz clock.

Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
---
 drivers/clk/x86/clk-fch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c
index 19a5c8cf1b3f..517061fc771c 100644
--- a/drivers/clk/x86/clk-fch.c
+++ b/drivers/clk/x86/clk-fch.c
@@ -66,7 +66,7 @@ static int fch_clk_probe(struct platform_device *pdev)
 
 		hws[CLK_GATE_FIXED] = clk_hw_register_gate(NULL, "oscout1",
 			"clk48MHz", 0, fch_data->base + MISCCLKCNTL1,
-			OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL);
+			OSCCLKENB, 0, NULL);
 
 		devm_clk_hw_register_clkdev(&pdev->dev, hws[CLK_GATE_FIXED],
 					    fch_data->name, NULL);
-- 
2.25.1

Re: [PATCH v2 RESEND 2/5] drivers: acpi: acpi_apd: Remove unused device property "is-rv"

[Rafael J. Wysocki]

On Fri, Oct 29, 2021 at 8:33 AM Ajit Kumar Pandey
<AjitKumar.Pandey@amd.com> wrote:
>
> Initially "is-rv" device property is added for 48MHz fixed clock
> support on Raven or RV architecture. It's unused now as we moved
> to clock config based selection to extend such support on others
> architecture. This change removed unused code from acpi driver.
>
> Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>

Is this and the [3/5] applicable without the [1/5]?

If so, I can pick them up once they are approved by some other AMD
folks, preferably Mario.

> ---
>  drivers/acpi/acpi_apd.c               | 3 ---
>  include/linux/platform_data/clk-fch.h | 1 -
>  2 files changed, 4 deletions(-)
>
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> index 6e02448d15d9..6913e9712852 100644
> --- a/drivers/acpi/acpi_apd.c
> +++ b/drivers/acpi/acpi_apd.c
> @@ -87,9 +87,6 @@ static int fch_misc_setup(struct apd_private_data *pdata)
>         if (ret < 0)
>                 return -ENOENT;
>
> -       if (!acpi_dev_get_property(adev, "is-rv", ACPI_TYPE_INTEGER, &obj))
> -               clk_data->is_rv = obj->integer.value;
> -
>         list_for_each_entry(rentry, &resource_list, node) {
>                 clk_data->base = devm_ioremap(&adev->dev, rentry->res->start,
>                                               resource_size(rentry->res));
> diff --git a/include/linux/platform_data/clk-fch.h b/include/linux/platform_data/clk-fch.h
> index b9f682459f08..850ca776156d 100644
> --- a/include/linux/platform_data/clk-fch.h
> +++ b/include/linux/platform_data/clk-fch.h
> @@ -12,7 +12,6 @@
>
>  struct fch_clk_data {
>         void __iomem *base;
> -       u32 is_rv;
>  };
>
>  #endif /* __CLK_FCH_H */
> --
> 2.25.1
>

Re: [PATCH v2 RESEND 2/5] drivers: acpi: acpi_apd: Remove unused device property "is-rv"

[Ajit Kumar Pandey]

On 11/6/2021 3:52 AM, Stephen Boyd wrote:
> [CAUTION: External Email]
> 
> Quoting Rafael J. Wysocki (2021-11-05 08:02:26)
>> On Fri, Oct 29, 2021 at 8:33 AM Ajit Kumar Pandey
>> <AjitKumar.Pandey@amd.com> wrote:
>>>
>>> Initially "is-rv" device property is added for 48MHz fixed clock
>>> support on Raven or RV architecture. It's unused now as we moved
>>> to clock config based selection to extend such support on others
>>> architecture. This change removed unused code from acpi driver.
>>>
>>> Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
>>
>> Is this and the [3/5] applicable without the [1/5]?
>>
>> If so, I can pick them up once they are approved by some other AMD
>> folks, preferably Mario.
> 
> I'm still lost why we're putting this decision to have some clk or not
> into Kconfig.
> 
I agree it's kind of weird to select clock based on Kconfig but only 
reason to put Kconfig based selection is that this clk support required 
on some older platforms where BIOS or bootloader change isn't feasible.
We have only one consumer for this 48MHzclk i.e audio drivers and idea 
is that this config shouldn't be user-selectable one and automatically
select from audio machine driver which uses this clk.

RE: [PATCH v2 RESEND 2/5] drivers: acpi: acpi_apd: Remove unused device property "is-rv"

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Friday, November 5, 2021 10:02
> To: Pandey, Ajit Kumar <AjitKumar.Pandey@amd.com>
> Cc: Stephen Boyd <sboyd@kernel.org>; linux-clk <linux-clk@vger.kernel.org>;
> Mukunda, Vijendar <Vijendar.Mukunda@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Hiregoudar, Basavaraj
> <Basavaraj.Hiregoudar@amd.com>; Dommati, Sunil-kumar <Sunil-
> kumar.Dommati@amd.com>; Rafael J. Wysocki <rafael@kernel.org>; Len
> Brown <lenb@kernel.org>; open list:ACPI <linux-acpi@vger.kernel.org>; open
> list <linux-kernel@vger.kernel.org>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: Re: [PATCH v2 RESEND 2/5] drivers: acpi: acpi_apd: Remove unused
> device property "is-rv"
> 
> On Fri, Oct 29, 2021 at 8:33 AM Ajit Kumar Pandey
> <AjitKumar.Pandey@amd.com> wrote:
> >
> > Initially "is-rv" device property is added for 48MHz fixed clock
> > support on Raven or RV architecture. It's unused now as we moved
> > to clock config based selection to extend such support on others
> > architecture. This change removed unused code from acpi driver.
> >
> > Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>
> 
> Is this and the [3/5] applicable without the [1/5]?
> 
> If so, I can pick them up once they are approved by some other AMD
> folks, preferably Mario.

I had some offline conversations with Ajit, and gave them some other
ideas instead of Kconfig based clock settings.  Let's wait for the result
of those tries.

> 
> > ---
> >  drivers/acpi/acpi_apd.c               | 3 ---
> >  include/linux/platform_data/clk-fch.h | 1 -
> >  2 files changed, 4 deletions(-)
> >
> > diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> > index 6e02448d15d9..6913e9712852 100644
> > --- a/drivers/acpi/acpi_apd.c
> > +++ b/drivers/acpi/acpi_apd.c
> > @@ -87,9 +87,6 @@ static int fch_misc_setup(struct apd_private_data
> *pdata)
> >         if (ret < 0)
> >                 return -ENOENT;
> >
> > -       if (!acpi_dev_get_property(adev, "is-rv", ACPI_TYPE_INTEGER, &obj))
> > -               clk_data->is_rv = obj->integer.value;
> > -
> >         list_for_each_entry(rentry, &resource_list, node) {
> >                 clk_data->base = devm_ioremap(&adev->dev, rentry->res->start,
> >                                               resource_size(rentry->res));
> > diff --git a/include/linux/platform_data/clk-fch.h
> b/include/linux/platform_data/clk-fch.h
> > index b9f682459f08..850ca776156d 100644
> > --- a/include/linux/platform_data/clk-fch.h
> > +++ b/include/linux/platform_data/clk-fch.h
> > @@ -12,7 +12,6 @@
> >
> >  struct fch_clk_data {
> >         void __iomem *base;
> > -       u32 is_rv;
> >  };
> >
> >  #endif /* __CLK_FCH_H */
> > --
> > 2.25.1
> >