Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1539583 > unrolled thread

[PATCH v6 0/3] Add platform clock for BayTrail platforms

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/3] Add platform clock for BayTrail platforms Irina Tirdea <irina.tirdea@intel.com> - 2016-12-09 19:10 +0100
    [PATCH v6 2/3] arch/x86/platform/atom: Move pmc_atom to drivers/platform/x86 Irina Tirdea <irina.tirdea@intel.com> - 2016-12-09 19:10 +0100
      Re: [PATCH v6 2/3] arch/x86/platform/atom: Move pmc_atom to drivers/platform/x86 Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-13 00:50 +0100
        Re: [PATCH v6 2/3] arch/x86/platform/atom: Move pmc_atom to  drivers/platform/x86 Darren Hart <dvhart@infradead.org> - 2016-12-16 19:30 +0100
          Re: [PATCH v6 2/3] arch/x86/platform/atom: Move pmc_atom to drivers/platform/x86 Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-16 19:40 +0100
    [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Irina Tirdea <irina.tirdea@intel.com> - 2016-12-09 19:10 +0100
      Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-13 00:40 +0100
        Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> - 2016-12-13 01:20 +0100
          Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-13 01:30 +0100
            Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Darren Hart <dvhart@infradead.org> - 2016-12-16 19:40 +0100
              Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-16 19:50 +0100
                Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Darren Hart <dvhart@infradead.org> - 2016-12-16 20:30 +0100
                  Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-16 23:30 +0100
                    Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Darren Hart <dvhart@infradead.org> - 2016-12-17 00:00 +0100
              Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Mark Brown <broonie@kernel.org> - 2016-12-19 12:10 +0100
          Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-13 02:20 +0100
        Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Stephen Boyd <sboyd@codeaurora.org> - 2016-12-14 00:30 +0100
          Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> - 2016-12-16 06:20 +0100
            Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-16 09:50 +0100
              Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> - 2016-12-16 16:00 +0100
            Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Stephen Boyd <sboyd@codeaurora.org> - 2016-12-17 02:40 +0100
              Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-17 15:00 +0100
                Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> - 2016-12-19 17:20 +0100
                  Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Stephen Boyd <sboyd@codeaurora.org> - 2016-12-22 00:10 +0100
                    Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> - 2016-12-22 02:10 +0100
                      Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform  clocks Stephen Boyd <sboyd@codeaurora.org> - 2016-12-22 19:30 +0100
                      Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-22 19:50 +0100
    [PATCH v6 3/3] platform/x86: Enable Atom PMC platform clocks Irina Tirdea <irina.tirdea@intel.com> - 2016-12-09 19:10 +0100
      Re: [PATCH v6 3/3] platform/x86: Enable Atom PMC platform clocks Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-12-13 01:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1543906 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On 12/15, Pierre-Louis Bossart wrote:
> I am not sure I understand this last comment.
> init.name is not a constant, it's made of the "pmc_plt_clk_" string
> concatenated with an id which directly maps to which hardware clock
> is registered.

That's all fine. We need globally unique strings for clk names in
the framework so things work.

>Clients use devm_clk_get() with a "pmc_plt_clk_<n>"
> argument.

This is the problem. Clients should be calling clk_get() like:

	clk_get(dev, "signal name in datasheet")

where the first argument is the device and the second argument is
some string that is meaningful to the device, not the system as a
whole. The way clkdev is intended is so that the dev argument's
dev_name() is combined with the con_id that matches some signale
name in the datasheet. This way when the same IP is put into some
other chip, the globally unique name doesn't need to change, just
the device name that's registered with the lookup. Obviously this
breaks down quite badly when dev_name() isn't stable. Is that
happening here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1543973 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On Sat, Dec 17, 2016 at 3:33 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 12/15, Pierre-Louis Bossart wrote:

>>Clients use devm_clk_get() with a "pmc_plt_clk_<n>"
>> argument.
>
> This is the problem. Clients should be calling clk_get() like:
>
>         clk_get(dev, "signal name in datasheet")
>
> where the first argument is the device and the second argument is
> some string that is meaningful to the device, not the system as a
> whole. The way clkdev is intended is so that the dev argument's
> dev_name() is combined with the con_id that matches some signale
> name in the datasheet. This way when the same IP is put into some
> other chip, the globally unique name doesn't need to change, just
> the device name that's registered with the lookup. Obviously this
> breaks down quite badly when dev_name() isn't stable. Is that
> happening here?

PMC Atom is a PCI device and thus each platform would have different
dev_name(). Do you want to list all in each consumer if consumer wants
to work on all of them or I missed something?

So, the question is how clock getting will look like to work on
currently both CherryTrail and BayTrail.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1544643 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On 12/17/16 7:57 AM, Andy Shevchenko wrote:
> On Sat, Dec 17, 2016 at 3:33 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 12/15, Pierre-Louis Bossart wrote:
>
>>> Clients use devm_clk_get() with a "pmc_plt_clk_<n>"
>>> argument.
>>
>> This is the problem. Clients should be calling clk_get() like:
>>
>>         clk_get(dev, "signal name in datasheet")
>>
>> where the first argument is the device and the second argument is
>> some string that is meaningful to the device, not the system as a
>> whole. The way clkdev is intended is so that the dev argument's
>> dev_name() is combined with the con_id that matches some signale
>> name in the datasheet. This way when the same IP is put into some
>> other chip, the globally unique name doesn't need to change, just
>> the device name that's registered with the lookup. Obviously this
>> breaks down quite badly when dev_name() isn't stable. Is that
>> happening here?
>
> PMC Atom is a PCI device and thus each platform would have different
> dev_name(). Do you want to list all in each consumer if consumer wants
> to work on all of them or I missed something?
>
> So, the question is how clock getting will look like to work on
> currently both CherryTrail and BayTrail.

The name pmc_plt_clk_<n> follows the data sheet specification, where 
this convention is suggested:
   PLT_CLK[2:0] - Camera
   PLT_CLK[3] - Audio Codec
   PLT_CLK[4] -
   PLT_CLK[5] - COMMs

These clocks are not internal but are made available to external 
components through dedicated physical pins on the package, this external 
visibility limits the scope for confusions, variations. I have not seen 
any skews where these clocks and pins were changed at all.

[toc] | [prev] | [next] | [standalone]

#1546031 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On 12/19, Pierre-Louis Bossart wrote:
> On 12/17/16 7:57 AM, Andy Shevchenko wrote:
> >On Sat, Dec 17, 2016 at 3:33 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >>On 12/15, Pierre-Louis Bossart wrote:
> >
> >>>Clients use devm_clk_get() with a "pmc_plt_clk_<n>"
> >>>argument.
> >>
> >>This is the problem. Clients should be calling clk_get() like:
> >>
> >>        clk_get(dev, "signal name in datasheet")
> >>
> >>where the first argument is the device and the second argument is
> >>some string that is meaningful to the device, not the system as a
> >>whole. The way clkdev is intended is so that the dev argument's
> >>dev_name() is combined with the con_id that matches some signale
> >>name in the datasheet. This way when the same IP is put into some
> >>other chip, the globally unique name doesn't need to change, just
> >>the device name that's registered with the lookup. Obviously this
> >>breaks down quite badly when dev_name() isn't stable. Is that
> >>happening here?
> >
> >PMC Atom is a PCI device and thus each platform would have different
> >dev_name(). Do you want to list all in each consumer if consumer wants
> >to work on all of them or I missed something?
> >
> >So, the question is how clock getting will look like to work on
> >currently both CherryTrail and BayTrail.
> 
> The name pmc_plt_clk_<n> follows the data sheet specification, where
> this convention is suggested:
>   PLT_CLK[2:0] - Camera
>   PLT_CLK[3] - Audio Codec
>   PLT_CLK[4] -
>   PLT_CLK[5] - COMMs
> 
> These clocks are not internal but are made available to external
> components through dedicated physical pins on the package, this
> external visibility limits the scope for confusions, variations. I
> have not seen any skews where these clocks and pins were changed at
> all.

Ok, by clkdev design if a device is passed but there isn't a
match in the lookup table it allows it to match based solely on
the connection id. Given that the connection id is globally
unique this will work.

Hopefully we don't have two of these devices with pmc_plt_clk_<n>
signals in a single system though. Then having the device name
would help differentiate between the two. And then it may make
sense to have some sort of ACPI lookup system, similar to how we
have lookups for clks in DT.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1546097 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On 12/21/16 5:05 PM, Stephen Boyd wrote:
> On 12/19, Pierre-Louis Bossart wrote:
>> On 12/17/16 7:57 AM, Andy Shevchenko wrote:
>>> On Sat, Dec 17, 2016 at 3:33 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> On 12/15, Pierre-Louis Bossart wrote:
>>>
>>>>> Clients use devm_clk_get() with a "pmc_plt_clk_<n>"
>>>>> argument.
>>>>
>>>> This is the problem. Clients should be calling clk_get() like:
>>>>
>>>>        clk_get(dev, "signal name in datasheet")
>>>>
>>>> where the first argument is the device and the second argument is
>>>> some string that is meaningful to the device, not the system as a
>>>> whole. The way clkdev is intended is so that the dev argument's
>>>> dev_name() is combined with the con_id that matches some signale
>>>> name in the datasheet. This way when the same IP is put into some
>>>> other chip, the globally unique name doesn't need to change, just
>>>> the device name that's registered with the lookup. Obviously this
>>>> breaks down quite badly when dev_name() isn't stable. Is that
>>>> happening here?
>>>
>>> PMC Atom is a PCI device and thus each platform would have different
>>> dev_name(). Do you want to list all in each consumer if consumer wants
>>> to work on all of them or I missed something?
>>>
>>> So, the question is how clock getting will look like to work on
>>> currently both CherryTrail and BayTrail.
>>
>> The name pmc_plt_clk_<n> follows the data sheet specification, where
>> this convention is suggested:
>>   PLT_CLK[2:0] - Camera
>>   PLT_CLK[3] - Audio Codec
>>   PLT_CLK[4] -
>>   PLT_CLK[5] - COMMs
>>
>> These clocks are not internal but are made available to external
>> components through dedicated physical pins on the package, this
>> external visibility limits the scope for confusions, variations. I
>> have not seen any skews where these clocks and pins were changed at
>> all.
>
> Ok, by clkdev design if a device is passed but there isn't a
> match in the lookup table it allows it to match based solely on
> the connection id. Given that the connection id is globally
> unique this will work.
>
> Hopefully we don't have two of these devices with pmc_plt_clk_<n>
> signals in a single system though. Then having the device name
> would help differentiate between the two. And then it may make
> sense to have some sort of ACPI lookup system, similar to how we
> have lookups for clks in DT.

So in short we keep the existing solution for now and will only use the 
device name if and when the pmc_plt_clk_<n> identifier is no longer 
unique due to hardware changes. Did I get this right?

[toc] | [prev] | [next] | [standalone]

#1546531 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On 12/21/2016 05:07 PM, Pierre-Louis Bossart wrote:
> On 12/21/16 5:05 PM, Stephen Boyd wrote:
>>
>> Ok, by clkdev design if a device is passed but there isn't a
>> match in the lookup table it allows it to match based solely on
>> the connection id. Given that the connection id is globally
>> unique this will work.
>>
>> Hopefully we don't have two of these devices with pmc_plt_clk_<n>
>> signals in a single system though. Then having the device name
>> would help differentiate between the two. And then it may make
>> sense to have some sort of ACPI lookup system, similar to how we
>> have lookups for clks in DT.
>
> So in short we keep the existing solution for now and will only use
> the device name if and when the pmc_plt_clk_<n> identifier is no
> longer unique due to hardware changes. Did I get this right?

Ok.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1546541 — Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On Thu, Dec 22, 2016 at 3:07 AM, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> On 12/21/16 5:05 PM, Stephen Boyd wrote:

>>> The name pmc_plt_clk_<n> follows the data sheet specification, where
>>> this convention is suggested:
>>>   PLT_CLK[2:0] - Camera
>>>   PLT_CLK[3] - Audio Codec
>>>   PLT_CLK[4] -
>>>   PLT_CLK[5] - COMMs

By the way, would I suggest to use same prefix as provider, i.e.
pmc_atom_plt_clk_%d ?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1539590 — [PATCH v6 3/3] platform/x86: Enable Atom PMC platform clocks

The BayTrail and CherryTrail platforms provide platform clocks
through their Power Management Controller (PMC).

The SoC supports up to 6 clocks (PMC_PLT_CLK[5:0]) with a
frequency of either 19.2 MHz (PLL) or 25 MHz (XTAL) for BayTrail
an a frequency of 19.2 MHz (XTAL) for CherryTrail. These clocks
are available for general system use, where appropriate. For example,
the usage for platform clocks suggested in the datasheet is the
following:
  PLT_CLK[2:0] - Camera
  PLT_CLK[3] - Audio Codec
  PLT_CLK[4] -
  PLT_CLK[5] - COMMs

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/platform/x86/Kconfig               |  1 +
 drivers/platform/x86/pmc_atom.c            | 78 ++++++++++++++++++++++++++++--
 include/linux/platform_data/x86/pmc_atom.h |  3 ++
 3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 21dce1e..c1b07ed 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1032,3 +1032,4 @@ endif # X86_PLATFORM_DEVICES
 config PMC_ATOM
 	def_bool y
         depends on PCI
+	select COMMON_CLK
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index b53fbc1..324c44f 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -22,6 +22,8 @@
 #include <linux/seq_file.h>
 #include <linux/io.h>
 #include <linux/platform_data/x86/pmc_atom.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/x86/clk-byt-plt.h>
 
 struct pmc_bit_map {
 	const char *name;
@@ -36,6 +38,11 @@ struct pmc_reg_map {
 	const struct pmc_bit_map *pss;
 };
 
+struct pmc_data {
+	const struct pmc_reg_map *map;
+	const struct pmc_clk *clks;
+};
+
 struct pmc_dev {
 	u32 base_addr;
 	void __iomem *regmap;
@@ -49,6 +56,29 @@ struct pmc_dev {
 static struct pmc_dev pmc_device;
 static u32 acpi_base_addr;
 
+static const struct pmc_clk byt_clks[] = {
+	{
+		.name = "xtal",
+		.freq = 25000000,
+		.parent_name = NULL,
+	},
+	{
+		.name = "pll",
+		.freq = 19200000,
+		.parent_name = "xtal",
+	},
+	{},
+};
+
+static const struct pmc_clk cht_clks[] = {
+	{
+		.name = "xtal",
+		.freq = 19200000,
+		.parent_name = NULL,
+	},
+	{},
+};
+
 static const struct pmc_bit_map d3_sts_0_map[] = {
 	{"LPSS1_F0_DMA",	BIT_LPSS1_F0_DMA},
 	{"LPSS1_F1_PWM1",	BIT_LPSS1_F1_PWM1},
@@ -168,6 +198,16 @@ struct pmc_dev {
 	.pss		= cht_pss_map,
 };
 
+static const struct pmc_data byt_data = {
+	.map = &byt_reg_map,
+	.clks = byt_clks,
+};
+
+static const struct pmc_data cht_data = {
+	.map = &cht_reg_map,
+	.clks = cht_clks,
+};
+
 static inline u32 pmc_reg_read(struct pmc_dev *pmc, int reg_offset)
 {
 	return readl(pmc->regmap + reg_offset);
@@ -381,10 +421,36 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc)
 }
 #endif /* CONFIG_DEBUG_FS */
 
+static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
+			  const struct pmc_data *pmc_data)
+{
+	struct platform_device *clkdev;
+	struct pmc_clk_data *clk_data;
+
+	clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->base = pmc_regmap + PMC_CLK_CTL_0;
+	clk_data->clks = pmc_data->clks;
+
+	clkdev = platform_device_register_data(&pdev->dev, "clk-byt-plt", -1,
+					       clk_data, sizeof(*clk_data));
+	if (IS_ERR(clkdev)) {
+		kfree(clk_data);
+		return PTR_ERR(clkdev);
+	}
+
+	kfree(clk_data);
+
+	return 0;
+}
+
 static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct pmc_dev *pmc = &pmc_device;
-	const struct pmc_reg_map *map = (struct pmc_reg_map *)ent->driver_data;
+	const struct pmc_data *data = (struct pmc_data *)ent->driver_data;
+	const struct pmc_reg_map *map = data->map;
 	int ret;
 
 	/* Obtain ACPI base address */
@@ -413,6 +479,12 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		dev_warn(&pdev->dev, "debugfs register failed\n");
 
+	/* Register platform clocks - PMC_PLT_CLK [5:0] */
+	ret = pmc_setup_clks(pdev, pmc->regmap, data);
+	if (ret)
+		dev_warn(&pdev->dev, "platform clocks register failed: %d\n",
+			 ret);
+
 	pmc->init = true;
 	return ret;
 }
@@ -423,8 +495,8 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
  * used by pci_match_id() call below.
  */
 static const struct pci_device_id pmc_pci_ids[] = {
-	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_VLV_PMC), (kernel_ulong_t)&byt_reg_map },
-	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_CHT_PMC), (kernel_ulong_t)&cht_reg_map },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_VLV_PMC), (kernel_ulong_t)&byt_data },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_CHT_PMC), (kernel_ulong_t)&cht_data },
 	{ 0, },
 };
 
diff --git a/include/linux/platform_data/x86/pmc_atom.h b/include/linux/platform_data/x86/pmc_atom.h
index aa8744c..2d310cf 100644
--- a/include/linux/platform_data/x86/pmc_atom.h
+++ b/include/linux/platform_data/x86/pmc_atom.h
@@ -50,6 +50,9 @@
 				BIT_ORED_DEDICATED_IRQ_GPSC | \
 				BIT_SHARED_IRQ_GPSS)
 
+/* Platform clock control registers */
+#define PMC_CLK_CTL_0		0x60
+
 /* The timers acumulate time spent in sleep state */
 #define	PMC_S0IR_TMR		0x80
 #define	PMC_S0I1_TMR		0x84
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1540751 — Re: [PATCH v6 3/3] platform/x86: Enable Atom PMC platform clocks

On Fri, Dec 9, 2016 at 8:01 PM, Irina Tirdea <irina.tirdea@intel.com> wrote:
> The BayTrail and CherryTrail platforms provide platform clocks
> through their Power Management Controller (PMC).
>
> The SoC supports up to 6 clocks (PMC_PLT_CLK[5:0]) with a
> frequency of either 19.2 MHz (PLL) or 25 MHz (XTAL) for BayTrail
> an a frequency of 19.2 MHz (XTAL) for CherryTrail. These clocks
> are available for general system use, where appropriate. For example,
> the usage for platform clocks suggested in the datasheet is the
> following:
>   PLT_CLK[2:0] - Camera
>   PLT_CLK[3] - Audio Codec
>   PLT_CLK[4] -
>   PLT_CLK[5] - COMMs
>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Same comments as per patch 1/3.

> ---
>  drivers/platform/x86/Kconfig               |  1 +
>  drivers/platform/x86/pmc_atom.c            | 78 ++++++++++++++++++++++++++++--

>  include/linux/platform_data/x86/pmc_atom.h |  3 ++

Same.

>  3 files changed, 79 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 21dce1e..c1b07ed 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1032,3 +1032,4 @@ endif # X86_PLATFORM_DEVICES
>  config PMC_ATOM
>         def_bool y
>          depends on PCI
> +       select COMMON_CLK
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index b53fbc1..324c44f 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -22,6 +22,8 @@
>  #include <linux/seq_file.h>
>  #include <linux/io.h>
>  #include <linux/platform_data/x86/pmc_atom.h>

> +#include <linux/platform_device.h>

It should go after (e comes after a).

> +#include <linux/platform_data/x86/clk-byt-plt.h>
>
>  struct pmc_bit_map {
>         const char *name;
> @@ -36,6 +38,11 @@ struct pmc_reg_map {
>         const struct pmc_bit_map *pss;
>  };
>
> +struct pmc_data {
> +       const struct pmc_reg_map *map;
> +       const struct pmc_clk *clks;

clks -> clocks

> +};
> +
>  struct pmc_dev {
>         u32 base_addr;
>         void __iomem *regmap;
> @@ -49,6 +56,29 @@ struct pmc_dev {
>  static struct pmc_dev pmc_device;
>  static u32 acpi_base_addr;
>
> +static const struct pmc_clk byt_clks[] = {
> +       {
> +               .name = "xtal",
> +               .freq = 25000000,
> +               .parent_name = NULL,
> +       },
> +       {
> +               .name = "pll",
> +               .freq = 19200000,
> +               .parent_name = "xtal",
> +       },
> +       {},
> +};
> +
> +static const struct pmc_clk cht_clks[] = {
> +       {
> +               .name = "xtal",
> +               .freq = 19200000,
> +               .parent_name = NULL,
> +       },
> +       {},
> +};
> +

Okay, this is definition of clock trees. It means that clk-x86-pmc can
be used basically on any of PMC that provides similar clock tree,
right?

>  static const struct pmc_bit_map d3_sts_0_map[] = {
>         {"LPSS1_F0_DMA",        BIT_LPSS1_F0_DMA},
>         {"LPSS1_F1_PWM1",       BIT_LPSS1_F1_PWM1},
> @@ -168,6 +198,16 @@ struct pmc_dev {
>         .pss            = cht_pss_map,
>  };
>
> +static const struct pmc_data byt_data = {
> +       .map = &byt_reg_map,
> +       .clks = byt_clks,
> +};
> +
> +static const struct pmc_data cht_data = {
> +       .map = &cht_reg_map,
> +       .clks = cht_clks,
> +};
> +
>  static inline u32 pmc_reg_read(struct pmc_dev *pmc, int reg_offset)
>  {
>         return readl(pmc->regmap + reg_offset);
> @@ -381,10 +421,36 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc)
>  }
>  #endif /* CONFIG_DEBUG_FS */
>
> +static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,

_clks -> _clocks

> +                         const struct pmc_data *pmc_data)
> +{
> +       struct platform_device *clkdev;
> +       struct pmc_clk_data *clk_data;
> +
> +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);

Can we just use stack for that?

> +       if (!clk_data)
> +               return -ENOMEM;
> +
> +       clk_data->base = pmc_regmap + PMC_CLK_CTL_0;
> +       clk_data->clks = pmc_data->clks;
> +
> +       clkdev = platform_device_register_data(&pdev->dev, "clk-byt-plt", -1,

-1 has a definition in this case. AUTO smth... or NONE, I don't
remember which one.

> +                                              clk_data, sizeof(*clk_data));
> +       if (IS_ERR(clkdev)) {
> +               kfree(clk_data);
> +               return PTR_ERR(clkdev);
> +       }
> +
> +       kfree(clk_data);
> +
> +       return 0;
> +}
> +
>  static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>         struct pmc_dev *pmc = &pmc_device;
> -       const struct pmc_reg_map *map = (struct pmc_reg_map *)ent->driver_data;
> +       const struct pmc_data *data = (struct pmc_data *)ent->driver_data;
> +       const struct pmc_reg_map *map = data->map;
>         int ret;
>
>         /* Obtain ACPI base address */
> @@ -413,6 +479,12 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
>         if (ret)
>                 dev_warn(&pdev->dev, "debugfs register failed\n");
>
> +       /* Register platform clocks - PMC_PLT_CLK [5:0] */
> +       ret = pmc_setup_clks(pdev, pmc->regmap, data);
> +       if (ret)
> +               dev_warn(&pdev->dev, "platform clocks register failed: %d\n",
> +                        ret);
> +
>         pmc->init = true;
>         return ret;
>  }
> @@ -423,8 +495,8 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent)
>   * used by pci_match_id() call below.
>   */
>  static const struct pci_device_id pmc_pci_ids[] = {
> -       { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_VLV_PMC), (kernel_ulong_t)&byt_reg_map },
> -       { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_CHT_PMC), (kernel_ulong_t)&cht_reg_map },
> +       { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_VLV_PMC), (kernel_ulong_t)&byt_data },
> +       { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_CHT_PMC), (kernel_ulong_t)&cht_data },
>         { 0, },
>  };
>
> diff --git a/include/linux/platform_data/x86/pmc_atom.h b/include/linux/platform_data/x86/pmc_atom.h
> index aa8744c..2d310cf 100644
> --- a/include/linux/platform_data/x86/pmc_atom.h
> +++ b/include/linux/platform_data/x86/pmc_atom.h
> @@ -50,6 +50,9 @@
>                                 BIT_ORED_DEDICATED_IRQ_GPSC | \
>                                 BIT_SHARED_IRQ_GPSS)
>
> +/* Platform clock control registers */
> +#define PMC_CLK_CTL_0          0x60
> +
>  /* The timers acumulate time spent in sleep state */

Typo:
accumulate

>  #define        PMC_S0IR_TMR            0x80
>  #define        PMC_S0I1_TMR            0x84

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web