| Message ID | 20211125110447.1188073-2-AjitKumar.Pandey@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support for newer family of AMD's SOC. | expand |
On 11/25/2021 05:04, Ajit Kumar Pandey wrote: > FCH controller clock configuration slightly differs across AMD's > SOC architectures. Newer family of SOC only support a 48MHz fixed > clock while older family has a clk_mux to choose 48MHz and 25MHz. > At present fixed 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 pci_device_id table with Raven platform id and replace "is-rv" > device property check with pci id match to support 48MHz fixed clk > support. This enhanced flexibility to enable fixed 48MHz fch clock > framework on other architectures by simply adding new entries into > pci_device_id table. Also replace RV with FIXED as generic naming > convention across all platforms. > > Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> > --- > drivers/clk/x86/clk-fch.c | 41 ++++++++++++++++++++++++++++++--------- > 1 file changed, 32 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c > index 8f7c5142b0f0..de556b03e184 100644 > --- a/drivers/clk/x86/clk-fch.c > +++ b/drivers/clk/x86/clk-fch.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: MIT > /* > - * clock framework for AMD Stoney based clocks > + * clock framework for AMD FCH controller block > * > * Copyright 2018 Advanced Micro Devices, Inc. > */ > @@ -8,6 +8,7 @@ > #include <linux/clk.h> > #include <linux/clkdev.h> > #include <linux/clk-provider.h> > +#include <linux/pci.h> > #include <linux/platform_data/clk-fch.h> > #include <linux/platform_device.h> > > @@ -26,22 +27,37 @@ > #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 > + > +/* List of supported CPU ids for fixed clk */ > +#define AMD_CPU_ID_RV 0x15D0 > > static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" }; > static struct clk_hw *hws[ST_MAX_CLKS]; > > +static const struct pci_device_id soc_pci_ids[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) }, > + { } > +}; > + Have you considered inverting it? The number of ASICs using the "older" design with the mux and multiple clock sources is a fixed value, but we'll keep adding new ASICs in the "new" design of just 48Mhz. Notably; I see that this series is missing the Yellow Carp ID for example. We'll keep having more designs with the 48Mhz that need to be added to this list. > static int fch_clk_probe(struct platform_device *pdev) > { > struct fch_clk_data *fch_data; > + struct pci_dev *rdev; > > fch_data = dev_get_platdata(&pdev->dev); > if (!fch_data || !fch_data->base) > return -EINVAL; > > - if (!fch_data->is_rv) { > + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0)); > + if (!rdev) { > + dev_err(&pdev->dev, "FCH device not found\n"); > + return -ENODEV; > + } > + > + if (!pci_match_id(soc_pci_ids, rdev)) { > 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,17 +77,18 @@ 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); > } > > + pci_dev_put(rdev); > return 0; > } > > @@ -79,14 +96,20 @@ static int fch_clk_remove(struct platform_device *pdev) > { > int i, clks; > struct fch_clk_data *fch_data; > + struct pci_dev *rdev; > > fch_data = dev_get_platdata(&pdev->dev); > > - clks = fch_data->is_rv ? RV_MAX_CLKS : ST_MAX_CLKS; > + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0)); > + if (!rdev) > + return -ENODEV; > + > + clks = pci_match_id(soc_pci_ids, rdev) ? CLK_MAX_FIXED : ST_MAX_CLKS; > > for (i = 0; i < clks; i++) > clk_hw_unregister(hws[i]); > > + pci_dev_put(rdev); > return 0; > } > >
Quoting Limonciello, Mario (2021-11-30 11:41:30) > On 11/25/2021 05:04, Ajit Kumar Pandey wrote: > > @@ -26,22 +27,37 @@ > > #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 > > + > > +/* List of supported CPU ids for fixed clk */ > > +#define AMD_CPU_ID_RV 0x15D0 > > > > static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" }; > > static struct clk_hw *hws[ST_MAX_CLKS]; > > > > +static const struct pci_device_id soc_pci_ids[] = { > > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) }, > > + { } > > +}; > > + > > Have you considered inverting it? The number of ASICs using the "older" > design with the mux and multiple clock sources is a fixed value, but > we'll keep adding new ASICs in the "new" design of just 48Mhz. > > Notably; I see that this series is missing the Yellow Carp ID for > example. We'll keep having more designs with the 48Mhz that need to be > added to this list. +1 Let's not keep adding to a list.
diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c index 8f7c5142b0f0..de556b03e184 100644 --- a/drivers/clk/x86/clk-fch.c +++ b/drivers/clk/x86/clk-fch.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT /* - * clock framework for AMD Stoney based clocks + * clock framework for AMD FCH controller block * * Copyright 2018 Advanced Micro Devices, Inc. */ @@ -8,6 +8,7 @@ #include <linux/clk.h> #include <linux/clkdev.h> #include <linux/clk-provider.h> +#include <linux/pci.h> #include <linux/platform_data/clk-fch.h> #include <linux/platform_device.h> @@ -26,22 +27,37 @@ #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 + +/* List of supported CPU ids for fixed clk */ +#define AMD_CPU_ID_RV 0x15D0 static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" }; static struct clk_hw *hws[ST_MAX_CLKS]; +static const struct pci_device_id soc_pci_ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) }, + { } +}; + static int fch_clk_probe(struct platform_device *pdev) { struct fch_clk_data *fch_data; + struct pci_dev *rdev; fch_data = dev_get_platdata(&pdev->dev); if (!fch_data || !fch_data->base) return -EINVAL; - if (!fch_data->is_rv) { + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0)); + if (!rdev) { + dev_err(&pdev->dev, "FCH device not found\n"); + return -ENODEV; + } + + if (!pci_match_id(soc_pci_ids, rdev)) { 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,17 +77,18 @@ 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); } + pci_dev_put(rdev); return 0; } @@ -79,14 +96,20 @@ static int fch_clk_remove(struct platform_device *pdev) { int i, clks; struct fch_clk_data *fch_data; + struct pci_dev *rdev; fch_data = dev_get_platdata(&pdev->dev); - clks = fch_data->is_rv ? RV_MAX_CLKS : ST_MAX_CLKS; + rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0)); + if (!rdev) + return -ENODEV; + + clks = pci_match_id(soc_pci_ids, rdev) ? CLK_MAX_FIXED : ST_MAX_CLKS; for (i = 0; i < clks; i++) clk_hw_unregister(hws[i]); + pci_dev_put(rdev); return 0; }
FCH controller clock configuration slightly differs across AMD's SOC architectures. Newer family of SOC only support a 48MHz fixed clock while older family has a clk_mux to choose 48MHz and 25MHz. At present fixed 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 pci_device_id table with Raven platform id and replace "is-rv" device property check with pci id match to support 48MHz fixed clk support. This enhanced flexibility to enable fixed 48MHz fch clock framework on other architectures by simply adding new entries into pci_device_id table. Also replace RV with FIXED as generic naming convention across all platforms. Signed-off-by: Ajit Kumar Pandey <AjitKumar.Pandey@amd.com> --- drivers/clk/x86/clk-fch.c | 41 ++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)