[PATCH v8 0/2] Imagination Technologies PWM support

[PATCH v8 0/2] Imagination Technologies PWM support

[Ezequiel Garcia]

A new round for the IMG PWM driver.

The IMG PWM controller is muxed with a PDM controller, through a shared
so-called periph register bit, which sets the output as PWM or PDM.
Because this register is not part of the pin controller block, but rather
PWM/PDM specific, and because the register is also used to set the PDM value,
it is simpler to use a regmap-based syscon to deal with it.

This time, I'm removing the PDM driver from the submission and submitting the PWM
alone. The PDM was written as a misc driver, but had some design issues, so for
now I'm proposing to merge the PWM only.

The series is based on v3.19-rc3. If at all possible I'd like to see this merged
for v3.20.

Comments welcome!

Naidu Tellapati (2):
  pwm: Imagination Technologies PWM DAC driver
  DT: pwm: Add binding document for IMG PWM DAC

 Documentation/devicetree/bindings/pwm/img-pwm.txt |  24 +++
 drivers/pwm/Kconfig                               |  13 ++
 drivers/pwm/Makefile                              |   1 +
 drivers/pwm/pwm-img.c                             | 250 ++++++++++++++++++++++
 4 files changed, 288 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/img-pwm.txt
 create mode 100644 drivers/pwm/pwm-img.c

-- 
2.2.1

[PATCH v8 1/2] pwm: Imagination Technologies PWM DAC driver

[Ezequiel Garcia]

From: Naidu Tellapati <naidu.tellapati-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The Pistachio SOC from Imagination Technologies includes a Pulse Width
Modulation DAC which produces 1 to 4 digital bit-outputs which represent
digital waveforms. These PWM outputs are primarily in charge of controlling
backlight LED devices.

Reviewed-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Sai Masarapu <Sai.Masarapu-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 drivers/pwm/Kconfig   |  13 +++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-img.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 264 insertions(+)
 create mode 100644 drivers/pwm/pwm-img.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index a3ecf58..1681cd0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -130,6 +130,19 @@ config PWM_FSL_FTM
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-fsl-ftm.
 
+config PWM_IMG
+	tristate "Imagination Technologies PWM driver"
+	depends on HAS_IOMEM
+	depends on MFD_SYSCON
+	depends on COMMON_CLK
+	depends on MIPS || COMPILE_TEST
+	help
+	  Generic PWM framework driver for Imagination Technologies
+	  PWM block which supports 4 channels.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-img
+
 config PWM_IMX
 	tristate "i.MX PWM support"
 	depends on ARCH_MXC
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 65259ac..85381e4 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_PWM_BFIN)		+= pwm-bfin.o
 obj-$(CONFIG_PWM_CLPS711X)	+= pwm-clps711x.o
 obj-$(CONFIG_PWM_EP93XX)	+= pwm-ep93xx.o
 obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
+obj-$(CONFIG_PWM_IMG)		+= pwm-img.o
 obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
 obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
 obj-$(CONFIG_PWM_LP3943)	+= pwm-lp3943.o
diff --git a/drivers/pwm/pwm-img.c b/drivers/pwm/pwm-img.c
new file mode 100644
index 0000000..0cf9a74
--- /dev/null
+++ b/drivers/pwm/pwm-img.c
@@ -0,0 +1,250 @@
+/*
+ * Imagination Technologies Pulse Width Modulator driver
+ *
+ * Copyright (c) 2014-2015, Imagination Technologies
+ *
+ * Based on drivers/pwm/pwm-tegra.c, Copyright (c) 2010, NVIDIA Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* PWM registers */
+#define PWM_CTRL_CFG				0x0000
+#define PWM_CTRL_CFG_NO_SUB_DIV			0
+#define PWM_CTRL_CFG_SUB_DIV0			1
+#define PWM_CTRL_CFG_SUB_DIV1			2
+#define PWM_CTRL_CFG_SUB_DIV0_DIV1		3
+#define PWM_CTRL_CFG_DIV_SHIFT(ch)		((ch) * 2 + 4)
+#define PWM_CTRL_CFG_DIV_MASK			0x3
+
+#define PWM_CH_CFG(ch)				(0x4 + (ch) * 4)
+#define PWM_CH_CFG_TMBASE_SHIFT			0
+#define PWM_CH_CFG_DUTY_SHIFT			16
+
+#define PERIP_PWM_PDM_CONTROL			0x0140
+#define PERIP_PWM_PDM_CONTROL_CH_MASK		0x1
+#define PERIP_PWM_PDM_CONTROL_CH_SHIFT(ch)	((ch) * 4)
+
+#define MAX_TMBASE_STEPS			65536
+
+struct img_pwm_chip {
+	struct device	*dev;
+	struct pwm_chip	chip;
+	struct clk	*pwm_clk;
+	struct clk	*sys_clk;
+	void __iomem	*base;
+	struct regmap	*periph_regs;
+};
+
+static inline struct img_pwm_chip *to_img_pwm_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct img_pwm_chip, chip);
+}
+
+static inline void img_pwm_writel(struct img_pwm_chip *chip,
+				  u32 reg, u32 val)
+{
+	writel(val, chip->base + reg);
+}
+
+static inline u32 img_pwm_readl(struct img_pwm_chip *chip,
+					 u32 reg)
+{
+	return readl(chip->base + reg);
+}
+
+static int img_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			  int duty_ns, int period_ns)
+{
+	u32 val, div, duty, timebase;
+	unsigned long mul, output_clk_hz, input_clk_hz;
+	struct img_pwm_chip *pwm_chip = to_img_pwm_chip(chip);
+
+	input_clk_hz = clk_get_rate(pwm_chip->pwm_clk);
+	output_clk_hz = DIV_ROUND_UP(NSEC_PER_SEC, period_ns);
+
+	mul = DIV_ROUND_UP(input_clk_hz, output_clk_hz);
+	if (mul <= MAX_TMBASE_STEPS) {
+		div = PWM_CTRL_CFG_NO_SUB_DIV;
+		timebase = DIV_ROUND_UP(mul, 1);
+	} else if (mul <= MAX_TMBASE_STEPS * 8) {
+		div = PWM_CTRL_CFG_SUB_DIV0;
+		timebase = DIV_ROUND_UP(mul, 8);
+	} else if (mul <= MAX_TMBASE_STEPS * 64) {
+		div = PWM_CTRL_CFG_SUB_DIV1;
+		timebase = DIV_ROUND_UP(mul, 64);
+	} else if (mul <= MAX_TMBASE_STEPS * 512) {
+		div = PWM_CTRL_CFG_SUB_DIV0_DIV1;
+		timebase = DIV_ROUND_UP(mul, 512);
+	} else if (mul > MAX_TMBASE_STEPS * 512) {
+		dev_err(chip->dev,
+			"failed to configure timebase steps/divider value\n");
+		return -EINVAL;
+	}
+
+	duty = DIV_ROUND_UP(timebase * duty_ns, period_ns);
+
+	val = img_pwm_readl(pwm_chip, PWM_CTRL_CFG);
+	val &= ~(PWM_CTRL_CFG_DIV_MASK << PWM_CTRL_CFG_DIV_SHIFT(pwm->hwpwm));
+	val |= (div & PWM_CTRL_CFG_DIV_MASK) <<
+		PWM_CTRL_CFG_DIV_SHIFT(pwm->hwpwm);
+	img_pwm_writel(pwm_chip, PWM_CTRL_CFG, val);
+
+	val = (duty << PWM_CH_CFG_DUTY_SHIFT) |
+	      (timebase << PWM_CH_CFG_TMBASE_SHIFT);
+	img_pwm_writel(pwm_chip, PWM_CH_CFG(pwm->hwpwm), val);
+
+	return 0;
+}
+
+static int img_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	u32 val;
+	struct img_pwm_chip *pwm_chip = to_img_pwm_chip(chip);
+
+	val = img_pwm_readl(pwm_chip, PWM_CTRL_CFG);
+	val |= BIT(pwm->hwpwm);
+	img_pwm_writel(pwm_chip, PWM_CTRL_CFG, val);
+
+	regmap_update_bits(pwm_chip->periph_regs, PERIP_PWM_PDM_CONTROL,
+			   PERIP_PWM_PDM_CONTROL_CH_MASK <<
+			   PERIP_PWM_PDM_CONTROL_CH_SHIFT(pwm->hwpwm), 0);
+
+	return 0;
+}
+
+static void img_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	u32 val;
+	struct img_pwm_chip *pwm_chip = to_img_pwm_chip(chip);
+
+	val = img_pwm_readl(pwm_chip, PWM_CTRL_CFG);
+	val &= ~BIT(pwm->hwpwm);
+	img_pwm_writel(pwm_chip, PWM_CTRL_CFG, val);
+}
+
+static const struct pwm_ops img_pwm_ops = {
+	.config = img_pwm_config,
+	.enable = img_pwm_enable,
+	.disable = img_pwm_disable,
+	.owner = THIS_MODULE,
+};
+
+static int img_pwm_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct resource *res;
+	struct img_pwm_chip *pwm;
+
+	pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
+	if (!pwm)
+		return -ENOMEM;
+
+	pwm->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pwm->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pwm->base))
+		return PTR_ERR(pwm->base);
+
+	pwm->periph_regs = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							   "img,cr-periph");
+	if (IS_ERR(pwm->periph_regs))
+		return PTR_ERR(pwm->periph_regs);
+
+	pwm->sys_clk = devm_clk_get(&pdev->dev, "sys");
+	if (IS_ERR(pwm->sys_clk)) {
+		dev_err(&pdev->dev, "failed to get system clock\n");
+		return PTR_ERR(pwm->sys_clk);
+	}
+
+	pwm->pwm_clk = devm_clk_get(&pdev->dev, "pwm");
+	if (IS_ERR(pwm->pwm_clk)) {
+		dev_err(&pdev->dev, "failed to get pwm clock\n");
+		return PTR_ERR(pwm->pwm_clk);
+	}
+
+	ret = clk_prepare_enable(pwm->sys_clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "could not prepare or enable sys clock\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(pwm->pwm_clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "could not prepare or enable pwm clock\n");
+		goto disable_sysclk;
+	}
+
+	pwm->chip.dev = &pdev->dev;
+	pwm->chip.ops = &img_pwm_ops;
+	pwm->chip.base = -1;
+	pwm->chip.npwm = 4;
+
+	ret = pwmchip_add(&pwm->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "pwmchip_add failed: %d\n", ret);
+		goto disable_pwmclk;
+	}
+
+	platform_set_drvdata(pdev, pwm);
+	return 0;
+
+disable_pwmclk:
+	clk_disable_unprepare(pwm->pwm_clk);
+disable_sysclk:
+	clk_disable_unprepare(pwm->sys_clk);
+	return ret;
+}
+
+static int img_pwm_remove(struct platform_device *pdev)
+{
+	struct img_pwm_chip *pwm_chip = platform_get_drvdata(pdev);
+	u32 val;
+	unsigned int i;
+
+	for (i = 0; i < pwm_chip->chip.npwm; i++) {
+		val = img_pwm_readl(pwm_chip, PWM_CTRL_CFG);
+		val &= ~BIT(i);
+		img_pwm_writel(pwm_chip, PWM_CTRL_CFG, val);
+	}
+
+	clk_disable_unprepare(pwm_chip->pwm_clk);
+	clk_disable_unprepare(pwm_chip->sys_clk);
+
+	return pwmchip_remove(&pwm_chip->chip);
+}
+
+static const struct of_device_id img_pwm_of_match[] = {
+	{ .compatible = "img,pistachio-pwm", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, img_pwm_of_match);
+
+static struct platform_driver img_pwm_driver = {
+	.driver = {
+		.name = "img-pwm",
+		.of_match_table = img_pwm_of_match,
+	},
+	.probe = img_pwm_probe,
+	.remove = img_pwm_remove,
+};
+module_platform_driver(img_pwm_driver);
+
+MODULE_AUTHOR("Sai Masarapu <Sai.Masarapu-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Imagination Technologies PWM DAC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 1/2] pwm: Imagination Technologies PWM DAC driver

[One Thousand Gnomes]

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

What if you've not been given a resource ?

Otherwise looks fine to me

Re: [PATCH v8 1/2] pwm: Imagination Technologies PWM DAC driver

[Vladimir Zapolskiy]

Hi Ezequiel,

On 09.01.2015 19:54, Ezequiel Garcia wrote:
> From: Naidu Tellapati <naidu.tellapati@gmail.com>
> 
> The Pistachio SOC from Imagination Technologies includes a Pulse Width
> Modulation DAC which produces 1 to 4 digital bit-outputs which represent
> digital waveforms. These PWM outputs are primarily in charge of controlling
> backlight LED devices.
> 
> Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Naidu Tellapati <Naidu.Tellapati@imgtec.com>
> Signed-off-by: Sai Masarapu <Sai.Masarapu@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> ---
>  drivers/pwm/Kconfig   |  13 +++
>  drivers/pwm/Makefile  |   1 +
>  drivers/pwm/pwm-img.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 264 insertions(+)
>  create mode 100644 drivers/pwm/pwm-img.c
> 

Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

[PATCH v8 2/2] DT: pwm: Add binding document for IMG PWM DAC

[Ezequiel Garcia]

From: Naidu Tellapati <naidu.tellapati@gmail.com>

Add binding document for IMG Pulse Width Modulator (PWM) DAC present on the
Pistachio SOC. The PWM DAC has four channels.

Signed-off-by: Naidu Tellapati <Naidu.Tellapati@imgtec.com>
Signed-off-by: Sai Masarapu <Sai.Masarapu@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 Documentation/devicetree/bindings/pwm/img-pwm.txt | 24 +++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/img-pwm.txt

diff --git a/Documentation/devicetree/bindings/pwm/img-pwm.txt b/Documentation/devicetree/bindings/pwm/img-pwm.txt
new file mode 100644
index 0000000..fade5f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/img-pwm.txt
@@ -0,0 +1,24 @@
+*Imagination Technologies PWM DAC driver
+
+Required properties:
+  - compatible: Should be "img,pistachio-pwm"
+  - reg: Should contain physical base address and length of pwm registers.
+  - clocks: Must contain an entry for each entry in clock-names.
+	See ../clock/clock-bindings.txt for details.
+  - clock-names: Must include the following entries.
+    - pwm: PWM operating clock.
+    - sys: PWM system interface clock.
+  - #pwm-cells: Should be 2. See pwm.txt in this directory for the
+	description of the cells format.
+  - img,cr-periph: Must contain a phandle to the peripheral control
+	syscon node which contains PWM control registers.
+
+Example:
+	pwm: pwm@18101300 {
+		compatible = "img,pistachio-pwm";
+		reg = <0x18101300 0x100>;
+		clocks = <&pwm_clk>, <&system_clk>;
+		clock-names = "pwm", "sys";
+		#pwm-cells = <2>;
+		img,cr-periph = <&cr_periph>;
+	};
-- 
2.2.1

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Ezequiel Garcia]

On 01/09/2015 02:54 PM, Ezequiel Garcia wrote:
> A new round for the IMG PWM driver.
> 
> The IMG PWM controller is muxed with a PDM controller, through a shared
> so-called periph register bit, which sets the output as PWM or PDM.
> Because this register is not part of the pin controller block, but rather
> PWM/PDM specific, and because the register is also used to set the PDM value,
> it is simpler to use a regmap-based syscon to deal with it.
> 
> This time, I'm removing the PDM driver from the submission and submitting the PWM
> alone. The PDM was written as a misc driver, but had some design issues, so for
> now I'm proposing to merge the PWM only.
> 
> The series is based on v3.19-rc3. If at all possible I'd like to see this merged
> for v3.20.
> 

Thierry,

Any comments on this? Any chance we merge it in time for v3.20?
It's -rc5 already and I've started to worry.

Thanks!
-- 
Ezequiel

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Ezequiel Garcia]

On 01/20/2015 07:52 AM, Ezequiel Garcia wrote:
> 
> 
> On 01/09/2015 02:54 PM, Ezequiel Garcia wrote:
>> A new round for the IMG PWM driver.
>>
>> The IMG PWM controller is muxed with a PDM controller, through a shared
>> so-called periph register bit, which sets the output as PWM or PDM.
>> Because this register is not part of the pin controller block, but rather
>> PWM/PDM specific, and because the register is also used to set the PDM value,
>> it is simpler to use a regmap-based syscon to deal with it.
>>
>> This time, I'm removing the PDM driver from the submission and submitting the PWM
>> alone. The PDM was written as a misc driver, but had some design issues, so for
>> now I'm proposing to merge the PWM only.
>>
>> The series is based on v3.19-rc3. If at all possible I'd like to see this merged
>> for v3.20.
>>
> 
> Thierry,
> 
> Any comments on this? Any chance we merge it in time for v3.20?
> It's -rc5 already and I've started to worry.
> 

Thierry,

I'm very sorry to be so bothering, but I got no news from you and it's
-rc6 already. I'd say I'll resend this series to Andrew Morton, hoping
we can merge this for v3.20.

Please let me know if you'll be able to pick this.

Thanks,
-- 
Ezequiel

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]

On Thu, Jan 29, 2015 at 01:32:09PM -0300, Ezequiel Garcia wrote:
> On 01/20/2015 07:52 AM, Ezequiel Garcia wrote:
> > 
> > 
> > On 01/09/2015 02:54 PM, Ezequiel Garcia wrote:
> >> A new round for the IMG PWM driver.
> >>
> >> The IMG PWM controller is muxed with a PDM controller, through a shared
> >> so-called periph register bit, which sets the output as PWM or PDM.
> >> Because this register is not part of the pin controller block, but rather
> >> PWM/PDM specific, and because the register is also used to set the PDM value,
> >> it is simpler to use a regmap-based syscon to deal with it.
> >>
> >> This time, I'm removing the PDM driver from the submission and submitting the PWM
> >> alone. The PDM was written as a misc driver, but had some design issues, so for
> >> now I'm proposing to merge the PWM only.
> >>
> >> The series is based on v3.19-rc3. If at all possible I'd like to see this merged
> >> for v3.20.
> >>
> > 
> > Thierry,
> > 
> > Any comments on this? Any chance we merge it in time for v3.20?
> > It's -rc5 already and I've started to worry.
> > 
> 
> Thierry,
> 
> I'm very sorry to be so bothering, but I got no news from you and it's
> -rc6 already. I'd say I'll resend this series to Andrew Morton, hoping
> we can merge this for v3.20.
> 
> Please let me know if you'll be able to pick this.

I can pick it up if you make up your mind about the license. The header
comment says GPL v2 or later, but MODULE_LICENSE has "GPL v2", which
does not include the "or later" part.

Also you're making it especially difficult to build-test by not
providing even the basic bits of your SoC support first. All even
linux-next seems to have for the Pistachio SoC is the addition of a
compatible string to the dw-mmc driver.

I'll take the PWM driver, but I'll assume that you'll eventually have
more pieces available, in which case I'd appreciate a note so I can
update my build scripts.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Andrew Bresticker]

> Also you're making it especially difficult to build-test by not
> providing even the basic bits of your SoC support first. All even
> linux-next seems to have for the Pistachio SoC is the addition of a
> compatible string to the dw-mmc driver.
>
> I'll take the PWM driver, but I'll assume that you'll eventually have
> more pieces available, in which case I'd appreciate a note so I can
> update my build scripts.

FYI, I'm hoping to post Pistachio platform support for 3.21.

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

On Fri, Jan 30, 2015 at 09:50:46AM +0000, Andrew Bresticker wrote:
> > Also you're making it especially difficult to build-test by not
> > providing even the basic bits of your SoC support first. All even
> > linux-next seems to have for the Pistachio SoC is the addition of a
> > compatible string to the dw-mmc driver.
> >
> > I'll take the PWM driver, but I'll assume that you'll eventually have
> > more pieces available, in which case I'd appreciate a note so I can
> > update my build scripts.
> 
> FYI, I'm hoping to post Pistachio platform support for 3.21.

That'd be great. I can switch over to a proper defconfig then and not
jump through extra hoops to build test patches.

Also, I'm seeing a bunch of weird errors from building MIPS, mostly
things like this:

	  CC      net/ipv4/netfilter/arp_tables.mod.o
	{standard input}: Assembler messages:
	{standard input}: Warning: .gnu_attribute 4,3 requires `softfloat'

or this later on:

	mips-linux-gnu-ld: Warning: vmlinuz uses -mhard-float (set by arch/mips/boot/compressed/head.o), arch/mips/boot/compressed/decompress.o uses -msoft-float

This is essentially a gpr_defconfig (because it select MIPS_ALCHEMY,
which in turns pulls in COMMON_CLK that PWM_IMG depends on) and then
enabling MFD_SYSCON on top so that all dependencies are met.

What am I doing wrong?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Andrew Bresticker]

+linux-mips

On Fri, Jan 30, 2015 at 10:26 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Jan 30, 2015 at 09:50:46AM +0000, Andrew Bresticker wrote:
>> > Also you're making it especially difficult to build-test by not
>> > providing even the basic bits of your SoC support first. All even
>> > linux-next seems to have for the Pistachio SoC is the addition of a
>> > compatible string to the dw-mmc driver.
>> >
>> > I'll take the PWM driver, but I'll assume that you'll eventually have
>> > more pieces available, in which case I'd appreciate a note so I can
>> > update my build scripts.
>>
>> FYI, I'm hoping to post Pistachio platform support for 3.21.
>
> That'd be great. I can switch over to a proper defconfig then and not
> jump through extra hoops to build test patches.
>
> Also, I'm seeing a bunch of weird errors from building MIPS, mostly
> things like this:
>
>           CC      net/ipv4/netfilter/arp_tables.mod.o
>         {standard input}: Assembler messages:
>         {standard input}: Warning: .gnu_attribute 4,3 requires `softfloat'
>
> or this later on:
>
>         mips-linux-gnu-ld: Warning: vmlinuz uses -mhard-float (set by arch/mips/boot/compressed/head.o), arch/mips/boot/compressed/decompress.o uses -msoft-float
>
> This is essentially a gpr_defconfig (because it select MIPS_ALCHEMY,
> which in turns pulls in COMMON_CLK that PWM_IMG depends on) and then
> enabling MFD_SYSCON on top so that all dependencies are met.
>
> What am I doing wrong?

What version of binutils are you using?  I believe the latter error
should be fixed by commit 842dfc11ea9a ("MIPS: Fix build with binutils
2.24.51+"), but perhaps the decompressor Makefile requires a fix as
well?  Unfortunately I'm traveling for the next couple of days, but I
may be able to take a look at it on Monday.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

On Fri, Jan 30, 2015 at 11:08:46AM +0000, Andrew Bresticker wrote:
> +linux-mips
> 
> On Fri, Jan 30, 2015 at 10:26 AM, Thierry Reding
> <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Fri, Jan 30, 2015 at 09:50:46AM +0000, Andrew Bresticker wrote:
> >> > Also you're making it especially difficult to build-test by not
> >> > providing even the basic bits of your SoC support first. All even
> >> > linux-next seems to have for the Pistachio SoC is the addition of a
> >> > compatible string to the dw-mmc driver.
> >> >
> >> > I'll take the PWM driver, but I'll assume that you'll eventually have
> >> > more pieces available, in which case I'd appreciate a note so I can
> >> > update my build scripts.
> >>
> >> FYI, I'm hoping to post Pistachio platform support for 3.21.
> >
> > That'd be great. I can switch over to a proper defconfig then and not
> > jump through extra hoops to build test patches.
> >
> > Also, I'm seeing a bunch of weird errors from building MIPS, mostly
> > things like this:
> >
> >           CC      net/ipv4/netfilter/arp_tables.mod.o
> >         {standard input}: Assembler messages:
> >         {standard input}: Warning: .gnu_attribute 4,3 requires `softfloat'
> >
> > or this later on:
> >
> >         mips-linux-gnu-ld: Warning: vmlinuz uses -mhard-float (set by arch/mips/boot/compressed/head.o), arch/mips/boot/compressed/decompress.o uses -msoft-float
> >
> > This is essentially a gpr_defconfig (because it select MIPS_ALCHEMY,
> > which in turns pulls in COMMON_CLK that PWM_IMG depends on) and then
> > enabling MFD_SYSCON on top so that all dependencies are met.
> >
> > What am I doing wrong?
> 
> What version of binutils are you using?  I believe the latter error
> should be fixed by commit 842dfc11ea9a ("MIPS: Fix build with binutils
> 2.24.51+"), but perhaps the decompressor Makefile requires a fix as
> well?  Unfortunately I'm traveling for the next couple of days, but I
> may be able to take a look at it on Monday.

I use binutils 2.25, and indeed building the correct branch gets rid of
those errors. Sorry for the noise.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Ezequiel Garcia]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi Thierry,

On 01/30/2015 06:18 AM, Thierry Reding wrote:
> On Thu, Jan 29, 2015 at 01:32:09PM -0300, Ezequiel Garcia wrote:
>> On 01/20/2015 07:52 AM, Ezequiel Garcia wrote:
>>> 
>>> 
>>> On 01/09/2015 02:54 PM, Ezequiel Garcia wrote:
>>>> A new round for the IMG PWM driver.
>>>> 
>>>> The IMG PWM controller is muxed with a PDM controller,
>>>> through a shared so-called periph register bit, which sets
>>>> the output as PWM or PDM. Because this register is not part
>>>> of the pin controller block, but rather PWM/PDM specific, and
>>>> because the register is also used to set the PDM value, it is
>>>> simpler to use a regmap-based syscon to deal with it.
>>>> 
>>>> This time, I'm removing the PDM driver from the submission
>>>> and submitting the PWM alone. The PDM was written as a misc
>>>> driver, but had some design issues, so for now I'm proposing
>>>> to merge the PWM only.
>>>> 
>>>> The series is based on v3.19-rc3. If at all possible I'd like
>>>> to see this merged for v3.20.
>>>> 
>>> 
>>> Thierry,
>>> 
>>> Any comments on this? Any chance we merge it in time for
>>> v3.20? It's -rc5 already and I've started to worry.
>>> 
>> 
>> Thierry,
>> 
>> I'm very sorry to be so bothering, but I got no news from you and
>> it's -rc6 already. I'd say I'll resend this series to Andrew
>> Morton, hoping we can merge this for v3.20.
>> 
>> Please let me know if you'll be able to pick this.
> 
> I can pick it up if you make up your mind about the license. The
> header comment says GPL v2 or later, but MODULE_LICENSE has "GPL
> v2", which does not include the "or later" part.
> 

License should be GPL v2, sorry about that. Need me to fix it and
resend or can you amend it before pushing this?

> Also you're making it especially difficult to build-test by not 
> providing even the basic bits of your SoC support first. All even 
> linux-next seems to have for the Pistachio SoC is the addition of
> a compatible string to the dw-mmc driver.
> 

Well, we've added COMPILE_TEST for precisely this reason, so you only
need to select COMPILE_TEST on any arch and you'll be able to build
test the driver.

> I'll take the PWM driver, but I'll assume that you'll eventually
> have more pieces available, in which case I'd appreciate a note so
> I can update my build scripts.
> 

If you can pick it, it would be great. I understand it's hard to
accept patches for drivers, when there's little testing possibilities.
But on the other side, isn't it a positive thing that a vendor is
pushing drivers this early?

Thanks a lot,
- -- 
Ezequiel
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJUy2RhAAoJEIOKbhOEIHKiWB0QAKc4h+YLjxLLa7y0/NfoEiqV
gwipBbrfO4leAB6RUiDOGil53Ir0Pxg87IbCHV5A0Gzxahv47GG41DYPVzbU3lHm
DGkGqyXDf0BNQqJkCXmDWV9/8pnjFfUMDlSrs15pxXTaRjZ7m6CrxCoZof5UVt91
4cmeDYHg4lr+YzT3oXuyiKFlrIRrT5heD3gfyL5IB9qQypR2KPnpDrCwMcmT6+ag
zI6sg6OAi8de7ZvSWEZSK7WkymhgduR9Rt0WRQBC3EKF2dCsUxhmZt14RJsDcpEK
LWn5z1FxImtJq8vWOq/E2F9AWtQ/o9gUeII2myNDaL2SIrVoggUP+yKV51J8bidl
G2HXU8NEIHCaqcKl5sdbeEbB4diXVqGJu+d9SOM2lrW2PlwWN63jgnGfyYc2RadH
60adajBgNjs9Ujfo5nFy+aYXJtJZk+4b1M0UMI+2qKV4G9PBfDrrpUMBRCRt8il4
A15lUoHTwTeLQp8m7YmNuoKLhLmeVrBgbzBagFLUZgpJs7TEYIWefrh/jos37n2s
aNlVW55c5CXDMDX9Bef4ar6Ch95TOxPqiED70e2/lNM0ckRzd7viQandUxquvc9q
mSWhSxYuT62/B6innSbuWLdifKtCsqX+QQIo2NBC3AB+NVybEmC89gB8jx+Py7pX
YS0wpCGXivYm3AIWhhyT
=qi1N
-----END PGP SIGNATURE-----

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3941 bytes --]

On Fri, Jan 30, 2015 at 08:01:00AM -0300, Ezequiel Garcia wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Hi Thierry,
> 
> On 01/30/2015 06:18 AM, Thierry Reding wrote:
> > On Thu, Jan 29, 2015 at 01:32:09PM -0300, Ezequiel Garcia wrote:
> >> On 01/20/2015 07:52 AM, Ezequiel Garcia wrote:
> >>> 
> >>> 
> >>> On 01/09/2015 02:54 PM, Ezequiel Garcia wrote:
> >>>> A new round for the IMG PWM driver.
> >>>> 
> >>>> The IMG PWM controller is muxed with a PDM controller,
> >>>> through a shared so-called periph register bit, which sets
> >>>> the output as PWM or PDM. Because this register is not part
> >>>> of the pin controller block, but rather PWM/PDM specific, and
> >>>> because the register is also used to set the PDM value, it is
> >>>> simpler to use a regmap-based syscon to deal with it.
> >>>> 
> >>>> This time, I'm removing the PDM driver from the submission
> >>>> and submitting the PWM alone. The PDM was written as a misc
> >>>> driver, but had some design issues, so for now I'm proposing
> >>>> to merge the PWM only.
> >>>> 
> >>>> The series is based on v3.19-rc3. If at all possible I'd like
> >>>> to see this merged for v3.20.
> >>>> 
> >>> 
> >>> Thierry,
> >>> 
> >>> Any comments on this? Any chance we merge it in time for
> >>> v3.20? It's -rc5 already and I've started to worry.
> >>> 
> >> 
> >> Thierry,
> >> 
> >> I'm very sorry to be so bothering, but I got no news from you and
> >> it's -rc6 already. I'd say I'll resend this series to Andrew
> >> Morton, hoping we can merge this for v3.20.
> >> 
> >> Please let me know if you'll be able to pick this.
> > 
> > I can pick it up if you make up your mind about the license. The
> > header comment says GPL v2 or later, but MODULE_LICENSE has "GPL
> > v2", which does not include the "or later" part.
> > 
> 
> License should be GPL v2, sorry about that. Need me to fix it and
> resend or can you amend it before pushing this?

I've fixed it up.

> > Also you're making it especially difficult to build-test by not 
> > providing even the basic bits of your SoC support first. All even 
> > linux-next seems to have for the Pistachio SoC is the addition of
> > a compatible string to the dw-mmc driver.
> > 
> 
> Well, we've added COMPILE_TEST for precisely this reason, so you only
> need to select COMPILE_TEST on any arch and you'll be able to build
> test the driver.

I'm not too big a fan of COMPILE_TEST. Sometimes build errors happen
only on one architecture, so if I build test your driver for ARM I may
miss issues and people will later complain to me that my tree broke
their build. So I prefer building on the target architecture if at all
possible.

> > I'll take the PWM driver, but I'll assume that you'll eventually
> > have more pieces available, in which case I'd appreciate a note so
> > I can update my build scripts.
> > 
> 
> If you can pick it, it would be great. I understand it's hard to
> accept patches for drivers, when there's little testing possibilities.
> But on the other side, isn't it a positive thing that a vendor is
> pushing drivers this early?

I wonder what's keeping the basic SoC support from being merged. As it
is the driver is completely unused, so I'm merging dead code. I'm going
to trust Andrew when he says that he plans on merging the SoC bits soon
so it's okay for now.

The problem with merging drivers first is that you have no possibility
of testing them in an upstream environment. You can't even test them
yourself even if you have local patches to make things actually boot.
I've seen that lead to unnecessary churn later on.

Yes, vendors contributing early is a good thing, but submitting code
that can't be tested in an upstream environment isn't very useful. You
can never be entirely certain that it's going to work before you have
the basic SoC support merged first.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v8 0/2] Imagination Technologies PWM support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On Fri, Jan 09, 2015 at 02:54:46PM -0300, Ezequiel Garcia wrote:
> A new round for the IMG PWM driver.
> 
> The IMG PWM controller is muxed with a PDM controller, through a shared
> so-called periph register bit, which sets the output as PWM or PDM.
> Because this register is not part of the pin controller block, but rather
> PWM/PDM specific, and because the register is also used to set the PDM value,
> it is simpler to use a regmap-based syscon to deal with it.
> 
> This time, I'm removing the PDM driver from the submission and submitting the PWM
> alone. The PDM was written as a misc driver, but had some design issues, so for
> now I'm proposing to merge the PWM only.
> 
> The series is based on v3.19-rc3. If at all possible I'd like to see this merged
> for v3.20.
> 
> Comments welcome!
> 
> Naidu Tellapati (2):
>   pwm: Imagination Technologies PWM DAC driver
>   DT: pwm: Add binding document for IMG PWM DAC

Applied the series with Vladimir's Reviewed-by on patch 1/2 and the
license text updated as discussed in this thread.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]