[U-Boot] [PATCH v3 0/3] sunxi: add PWM driver for H3 and A64
This series introduces a PWM driver for Allwinner H3 and A64 and makes some adjustments to pwm_backlight driver that are neccessary to get backlight working on Pinebook.
This commit adds basic support for PWM found on Allwinner A64 and H3 It can be used for pwm_backlight driver (e.g. for Pinebook)
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com --- v2: - move pinmux config into enable function to make driver more friendly to the boards with ethernet - drop 'sun50i-a64-pwm' compatible string and use 'sun8i-h3-pwm' instead, since A64 PWM is compatible with one on H3
arch/arm/include/asm/arch-sunxi/gpio.h | 1 + arch/arm/include/asm/arch-sunxi/pwm.h | 12 +++ drivers/pwm/Kconfig | 7 ++ drivers/pwm/Makefile | 1 + drivers/pwm/sunxi_pwm.c | 184 +++++++++++++++++++++++++++++++++ 5 files changed, 205 insertions(+) create mode 100644 drivers/pwm/sunxi_pwm.c
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h index 24f85206c8..7265d18099 100644 --- a/arch/arm/include/asm/arch-sunxi/gpio.h +++ b/arch/arm/include/asm/arch-sunxi/gpio.h @@ -173,6 +173,7 @@ enum sunxi_gpio_number { #define SUN8I_GPD_SDC1 3 #define SUNXI_GPD_LCD0 2 #define SUNXI_GPD_LVDS0 3 +#define SUNXI_GPD_PWM 2
#define SUN5I_GPE_SDC2 3 #define SUN8I_GPE_TWI2 3 diff --git a/arch/arm/include/asm/arch-sunxi/pwm.h b/arch/arm/include/asm/arch-sunxi/pwm.h index 5884b5dbe7..673e0eb7b5 100644 --- a/arch/arm/include/asm/arch-sunxi/pwm.h +++ b/arch/arm/include/asm/arch-sunxi/pwm.h @@ -11,8 +11,15 @@ #define SUNXI_PWM_CH0_PERIOD (SUNXI_PWM_BASE + 4)
#define SUNXI_PWM_CTRL_PRESCALE0(x) ((x) & 0xf) +#define SUNXI_PWM_CTRL_PRESCALE0_MASK (0xf) #define SUNXI_PWM_CTRL_ENABLE0 (0x5 << 4) #define SUNXI_PWM_CTRL_POLARITY0(x) ((x) << 5) +#define SUNXI_PWM_CTRL_POLARITY0_MASK (1 << 5) +#define SUNXI_PWM_CTRL_CLK_GATE (1 << 6) + +#define SUNXI_PWM_CH0_PERIOD_MAX (0xffff) +#define SUNXI_PWM_CH0_PERIOD_PRD(x) ((x & 0xffff) << 16) +#define SUNXI_PWM_CH0_PERIOD_DUTY(x) ((x) & 0xffff)
#define SUNXI_PWM_PERIOD_80PCT 0x04af03c0
@@ -31,4 +38,9 @@ #define SUNXI_PWM_MUX SUN8I_GPH_PWM #endif
+struct sunxi_pwm { + u32 ctrl; + u32 ch0_period; +}; + #endif diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index e827558052..2984b79766 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -43,3 +43,10 @@ config PWM_TEGRA four channels with a programmable period and duty cycle. Only a 32KHz clock is supported by the driver but the duty cycle is configurable. + +config PWM_SUNXI + bool "Enable support for the Allwinner Sunxi PWM" + depends on DM_PWM + help + This PWM is found on H3, A64 and other Allwinner SoCs. It supports a + programmable period and duty cycle. A 16-bit counter is used. diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 29d59916cb..1a8f8a58bc 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_PWM_IMX) += pwm-imx.o pwm-imx-util.o obj-$(CONFIG_PWM_ROCKCHIP) += rk_pwm.o obj-$(CONFIG_PWM_SANDBOX) += sandbox_pwm.o obj-$(CONFIG_PWM_TEGRA) += tegra_pwm.o +obj-$(CONFIG_PWM_SUNXI) += sunxi_pwm.o diff --git a/drivers/pwm/sunxi_pwm.c b/drivers/pwm/sunxi_pwm.c new file mode 100644 index 0000000000..cfea7d69f3 --- /dev/null +++ b/drivers/pwm/sunxi_pwm.c @@ -0,0 +1,184 @@ +/* + * Copyright (c) 2017 Vasily Khoruzhick anarsoul@gmail.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <div64.h> +#include <dm.h> +#include <pwm.h> +#include <regmap.h> +#include <syscon.h> +#include <asm/io.h> +#include <asm/arch/pwm.h> +#include <asm/arch/gpio.h> +#include <power/regulator.h> + +DECLARE_GLOBAL_DATA_PTR; + +struct sunxi_pwm_priv { + struct sunxi_pwm *regs; + ulong freq; + bool invert; + uint32_t prescaler; +}; + +static const uint32_t prescaler_table[] = { + 120, /* 0000 */ + 180, /* 0001 */ + 240, /* 0010 */ + 360, /* 0011 */ + 480, /* 0100 */ + 0, /* 0101 */ + 0, /* 0110 */ + 0, /* 0111 */ + 12000, /* 1000 */ + 24000, /* 1001 */ + 36000, /* 1010 */ + 48000, /* 1011 */ + 72000, /* 1100 */ + 0, /* 1101 */ + 0, /* 1110 */ + 1, /* 1111 */ +}; + +static const uint64_t nsecs_per_sec = 1000000000L; + +static int sunxi_pwm_config_pinmux(void) +{ +#ifdef CONFIG_MACH_SUN50I + sunxi_gpio_set_cfgpin(SUNXI_GPD(22), SUNXI_GPD_PWM); +#endif + return 0; +} + +static int sunxi_pwm_set_invert(struct udevice *dev, uint channel, bool polarity) +{ + struct sunxi_pwm_priv *priv = dev_get_priv(dev); + + debug("%s: polarity=%u\n", __func__, polarity); + priv->invert = polarity; + + return 0; +} + +static int sunxi_pwm_set_config(struct udevice *dev, uint channel, uint period_ns, + uint duty_ns) +{ + struct sunxi_pwm_priv *priv = dev_get_priv(dev); + struct sunxi_pwm *regs = priv->regs; + int prescaler; + u32 v, period, duty; + uint64_t div = 0, pval = 0, scaled_freq = 0; + + debug("%s: period_ns=%u, duty_ns=%u\n", __func__, period_ns, duty_ns); + + for (prescaler = 0; prescaler < SUNXI_PWM_CTRL_PRESCALE0_MASK; prescaler++) { + if (!prescaler_table[prescaler]) + continue; + div = priv->freq; + pval = prescaler_table[prescaler]; + scaled_freq = lldiv(div, pval); + div = scaled_freq * period_ns; + div = lldiv(div, nsecs_per_sec); + if (div - 1 <= SUNXI_PWM_CH0_PERIOD_MAX) + break; + } + + if (div - 1 > SUNXI_PWM_CH0_PERIOD_MAX) { + debug("%s: failed to find prescaler value\n", __func__); + return -EINVAL; + } + + period = div; + div = scaled_freq * duty_ns; + div = lldiv(div, nsecs_per_sec); + duty = div; + + if (priv->prescaler != prescaler) { + /* Mask clock to update prescaler */ + v = readl(®s->ctrl); + v &= ~SUNXI_PWM_CTRL_CLK_GATE; + writel(v, ®s->ctrl); + v &= ~SUNXI_PWM_CTRL_PRESCALE0_MASK; + v |= (priv->prescaler & SUNXI_PWM_CTRL_PRESCALE0_MASK); + writel(v, ®s->ctrl); + v |= SUNXI_PWM_CTRL_CLK_GATE; + writel(v, ®s->ctrl); + priv->prescaler = prescaler; + } + + writel(SUNXI_PWM_CH0_PERIOD_PRD(period) | + SUNXI_PWM_CH0_PERIOD_DUTY(duty), ®s->ch0_period); + + debug("%s: prescaler: %d, period: %d, duty: %d\n", __func__, priv->prescaler, + period, duty); + + return 0; +} + +static int sunxi_pwm_set_enable(struct udevice *dev, uint channel, bool enable) +{ + struct sunxi_pwm_priv *priv = dev_get_priv(dev); + struct sunxi_pwm *regs = priv->regs; + uint32_t v; + + debug("%s: Enable '%s'\n", __func__, dev->name); + + v = readl(®s->ctrl); + if (!enable) { + v &= ~SUNXI_PWM_CTRL_ENABLE0; + writel(v, ®s->ctrl); + return 0; + } + + sunxi_pwm_config_pinmux(); + + v &= ~SUNXI_PWM_CTRL_POLARITY0_MASK; + v |= priv->invert ? SUNXI_PWM_CTRL_POLARITY0(0) : + SUNXI_PWM_CTRL_POLARITY0(1); + v |= SUNXI_PWM_CTRL_ENABLE0; + writel(v, ®s->ctrl); + + return 0; +} + +static int sunxi_pwm_ofdata_to_platdata(struct udevice *dev) +{ + struct sunxi_pwm_priv *priv = dev_get_priv(dev); + + priv->regs = (struct sunxi_pwm *)devfdt_get_addr(dev); + + return 0; +} + +static int sunxi_pwm_probe(struct udevice *dev) +{ + struct sunxi_pwm_priv *priv = dev_get_priv(dev); + + priv->freq = 24000000; + + return 0; +} + +static const struct pwm_ops sunxi_pwm_ops = { + .set_invert = sunxi_pwm_set_invert, + .set_config = sunxi_pwm_set_config, + .set_enable = sunxi_pwm_set_enable, +}; + +static const struct udevice_id sunxi_pwm_ids[] = { + { .compatible = "allwinner,sun8i-h3-pwm" }, + { } +}; + +U_BOOT_DRIVER(sunxi_pwm) = { + .name = "sunxi_pwm", + .id = UCLASS_PWM, + .of_match = sunxi_pwm_ids, + .ops = &sunxi_pwm_ops, + .ofdata_to_platdata = sunxi_pwm_ofdata_to_platdata, + .probe = sunxi_pwm_probe, + .priv_auto_alloc_size = sizeof(struct sunxi_pwm_priv), +};
Hi Vasily,
On 21/09/17 07:07, Vasily Khoruzhick wrote:
This commit adds basic support for PWM found on Allwinner A64 and H3 It can be used for pwm_backlight driver (e.g. for Pinebook)
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
v2: - move pinmux config into enable function to make driver more friendly to the boards with ethernet - drop 'sun50i-a64-pwm' compatible string and use 'sun8i-h3-pwm' instead, since A64 PWM is compatible with one on H3
arch/arm/include/asm/arch-sunxi/gpio.h | 1 + arch/arm/include/asm/arch-sunxi/pwm.h | 12 +++ drivers/pwm/Kconfig | 7 ++ drivers/pwm/Makefile | 1 + drivers/pwm/sunxi_pwm.c | 184 +++++++++++++++++++++++++++++++++ 5 files changed, 205 insertions(+) create mode 100644 drivers/pwm/sunxi_pwm.c
diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h index 24f85206c8..7265d18099 100644 --- a/arch/arm/include/asm/arch-sunxi/gpio.h +++ b/arch/arm/include/asm/arch-sunxi/gpio.h @@ -173,6 +173,7 @@ enum sunxi_gpio_number { #define SUN8I_GPD_SDC1 3 #define SUNXI_GPD_LCD0 2 #define SUNXI_GPD_LVDS0 3 +#define SUNXI_GPD_PWM 2
#define SUN5I_GPE_SDC2 3 #define SUN8I_GPE_TWI2 3 diff --git a/arch/arm/include/asm/arch-sunxi/pwm.h b/arch/arm/include/asm/arch-sunxi/pwm.h index 5884b5dbe7..673e0eb7b5 100644 --- a/arch/arm/include/asm/arch-sunxi/pwm.h +++ b/arch/arm/include/asm/arch-sunxi/pwm.h @@ -11,8 +11,15 @@ #define SUNXI_PWM_CH0_PERIOD (SUNXI_PWM_BASE + 4)
#define SUNXI_PWM_CTRL_PRESCALE0(x) ((x) & 0xf) +#define SUNXI_PWM_CTRL_PRESCALE0_MASK (0xf)
No need for the brackets around just a number.
Actually I wonder why we would need those new constants in a header file, when they are actually internals only relevant to the driver. I see we have it here already, because we also hack something in sunxi_display.c, but that should not affect the new driver. Leave it up to you ...
#define SUNXI_PWM_CTRL_ENABLE0 (0x5 << 4) #define SUNXI_PWM_CTRL_POLARITY0(x) ((x) << 5) +#define SUNXI_PWM_CTRL_POLARITY0_MASK (1 << 5)
MASK sounds a bit confusing, what about: SUNXI_PWM_CTRL_INV_POLARITY or SUNXI_PWM_CTRL_ACTIVE_LOW or SUNXI_PWM_CH0_ACT_STA (to match the spec)
+#define SUNXI_PWM_CTRL_CLK_GATE (1 << 6)
+#define SUNXI_PWM_CH0_PERIOD_MAX (0xffff) +#define SUNXI_PWM_CH0_PERIOD_PRD(x) ((x & 0xffff) << 16) +#define SUNXI_PWM_CH0_PERIOD_DUTY(x) ((x) & 0xffff)
#define SUNXI_PWM_PERIOD_80PCT 0x04af03c0
@@ -31,4 +38,9 @@ #define SUNXI_PWM_MUX SUN8I_GPH_PWM #endif
+struct sunxi_pwm {
- u32 ctrl;
- u32 ch0_period;
+};
#endif diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index e827558052..2984b79766 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -43,3 +43,10 @@ config PWM_TEGRA four channels with a programmable period and duty cycle. Only a 32KHz clock is supported by the driver but the duty cycle is configurable.
+config PWM_SUNXI
- bool "Enable support for the Allwinner Sunxi PWM"
- depends on DM_PWM
- help
This PWM is found on H3, A64 and other Allwinner SoCs. It supports aprogrammable period and duty cycle. A 16-bit counter is used.diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 29d59916cb..1a8f8a58bc 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_PWM_IMX) += pwm-imx.o pwm-imx-util.o obj-$(CONFIG_PWM_ROCKCHIP) += rk_pwm.o obj-$(CONFIG_PWM_SANDBOX) += sandbox_pwm.o obj-$(CONFIG_PWM_TEGRA) += tegra_pwm.o +obj-$(CONFIG_PWM_SUNXI) += sunxi_pwm.o diff --git a/drivers/pwm/sunxi_pwm.c b/drivers/pwm/sunxi_pwm.c new file mode 100644 index 0000000000..cfea7d69f3 --- /dev/null +++ b/drivers/pwm/sunxi_pwm.c @@ -0,0 +1,184 @@ +/*
- Copyright (c) 2017 Vasily Khoruzhick anarsoul@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <div64.h> +#include <dm.h> +#include <pwm.h> +#include <regmap.h> +#include <syscon.h> +#include <asm/io.h> +#include <asm/arch/pwm.h> +#include <asm/arch/gpio.h> +#include <power/regulator.h>
+DECLARE_GLOBAL_DATA_PTR;
+struct sunxi_pwm_priv {
- struct sunxi_pwm *regs;
- ulong freq;
What do you need this "freq" for? I see it's only set once (with the constant oscillator frequency), then never changed.
- bool invert;
- uint32_t prescaler;
+};
+static const uint32_t prescaler_table[] = {
- 120, /* 0000 */
- 180, /* 0001 */
- 240, /* 0010 */
- 360, /* 0011 */
- 480, /* 0100 */
- 0, /* 0101 */
- 0, /* 0110 */
- 0, /* 0111 */
- 12000, /* 1000 */
- 24000, /* 1001 */
- 36000, /* 1010 */
- 48000, /* 1011 */
- 72000, /* 1100 */
- 0, /* 1101 */
- 0, /* 1110 */
- 1, /* 1111 */
+};
+static const uint64_t nsecs_per_sec = 1000000000L;
Any reason you made this a variable here? Also the type is wrong, it's used as a uint32_t (lldiv() divisor) below and fits well into 32-bit, so also no need for the L suffix (U would be sufficient to not blow it up to 64-bit on arm64). You might want to put this into some generic header file and let fs/ubifs/ubifs.h and drivers/i2c/stm32f7_i2c.c use it as well, if you like.
+static int sunxi_pwm_config_pinmux(void) +{ +#ifdef CONFIG_MACH_SUN50I
- sunxi_gpio_set_cfgpin(SUNXI_GPD(22), SUNXI_GPD_PWM);
+#endif
- return 0;
+}
+static int sunxi_pwm_set_invert(struct udevice *dev, uint channel, bool polarity) +{
- struct sunxi_pwm_priv *priv = dev_get_priv(dev);
- debug("%s: polarity=%u\n", __func__, polarity);
- priv->invert = polarity;
- return 0;
+}
+static int sunxi_pwm_set_config(struct udevice *dev, uint channel, uint period_ns,
uint duty_ns)
indentation?
+{
- struct sunxi_pwm_priv *priv = dev_get_priv(dev);
- struct sunxi_pwm *regs = priv->regs;
- int prescaler;
- u32 v, period, duty;
- uint64_t div = 0, pval = 0, scaled_freq = 0;
- debug("%s: period_ns=%u, duty_ns=%u\n", __func__, period_ns, duty_ns);
- for (prescaler = 0; prescaler < SUNXI_PWM_CTRL_PRESCALE0_MASK; prescaler++) {
if (!prescaler_table[prescaler])continue;div = priv->freq;pval = prescaler_table[prescaler];scaled_freq = lldiv(div, pval);
This is a bit hard to read. What about: scaled_freq = lldiv(OSC_24MHZ, prescaler_table[prescaler]); Let's you get rid of pval and priv->freq as well and avoids the confusing usage of "div" here.
div = scaled_freq * period_ns;div = lldiv(div, nsecs_per_sec);
Same here, div seems to be abused. period = lldiv(scaled_freq * period_ns, NSECS_PER_SEC);
That should allow you to get rid of "div" at all, I believe.
if (div - 1 <= SUNXI_PWM_CH0_PERIOD_MAX)break;- }
- if (div - 1 > SUNXI_PWM_CH0_PERIOD_MAX) {
debug("%s: failed to find prescaler value\n", __func__);return -EINVAL;- }
- period = div;
- div = scaled_freq * duty_ns;
- div = lldiv(div, nsecs_per_sec);
- duty = div;
Can all be shortened to: duty = lldiv(scaled_freq * duty_ns, NSECS_PER_SEC);
- if (priv->prescaler != prescaler) {
/* Mask clock to update prescaler */v = readl(®s->ctrl);v &= ~SUNXI_PWM_CTRL_CLK_GATE;writel(v, ®s->ctrl);v &= ~SUNXI_PWM_CTRL_PRESCALE0_MASK;v |= (priv->prescaler & SUNXI_PWM_CTRL_PRESCALE0_MASK);writel(v, ®s->ctrl);v |= SUNXI_PWM_CTRL_CLK_GATE;writel(v, ®s->ctrl);priv->prescaler = prescaler;- }
- writel(SUNXI_PWM_CH0_PERIOD_PRD(period) |
SUNXI_PWM_CH0_PERIOD_DUTY(duty), ®s->ch0_period);- debug("%s: prescaler: %d, period: %d, duty: %d\n", __func__, priv->prescaler,
period, duty);- return 0;
+}
+static int sunxi_pwm_set_enable(struct udevice *dev, uint channel, bool enable) +{
- struct sunxi_pwm_priv *priv = dev_get_priv(dev);
- struct sunxi_pwm *regs = priv->regs;
- uint32_t v;
- debug("%s: Enable '%s'\n", __func__, dev->name);
- v = readl(®s->ctrl);
- if (!enable) {
v &= ~SUNXI_PWM_CTRL_ENABLE0;writel(v, ®s->ctrl);return 0;- }
- sunxi_pwm_config_pinmux();
- v &= ~SUNXI_PWM_CTRL_POLARITY0_MASK;
- v |= priv->invert ? SUNXI_PWM_CTRL_POLARITY0(0) :
SUNXI_PWM_CTRL_POLARITY0(1);
I think: if (priv->invert) v |= SUNXI_PWM_CTRL_INV_POLARITY; else v &= ~SUNXI_PWM_CTRL_INV_POLARITY;
would be easier to read. The SUNXI_PWM_CTRL_POLARITY0() macro looks like there is some magic behind it, where it actually is just one bit.
Cheers, Andre.
- v |= SUNXI_PWM_CTRL_ENABLE0;
- writel(v, ®s->ctrl);
- return 0;
+}
+static int sunxi_pwm_ofdata_to_platdata(struct udevice *dev) +{
- struct sunxi_pwm_priv *priv = dev_get_priv(dev);
- priv->regs = (struct sunxi_pwm *)devfdt_get_addr(dev);
- return 0;
+}
+static int sunxi_pwm_probe(struct udevice *dev) +{
- struct sunxi_pwm_priv *priv = dev_get_priv(dev);
- priv->freq = 24000000;
- return 0;
+}
+static const struct pwm_ops sunxi_pwm_ops = {
- .set_invert = sunxi_pwm_set_invert,
- .set_config = sunxi_pwm_set_config,
- .set_enable = sunxi_pwm_set_enable,
+};
+static const struct udevice_id sunxi_pwm_ids[] = {
- { .compatible = "allwinner,sun8i-h3-pwm" },
- { }
+};
+U_BOOT_DRIVER(sunxi_pwm) = {
- .name = "sunxi_pwm",
- .id = UCLASS_PWM,
- .of_match = sunxi_pwm_ids,
- .ops = &sunxi_pwm_ops,
- .ofdata_to_platdata = sunxi_pwm_ofdata_to_platdata,
- .probe = sunxi_pwm_probe,
- .priv_auto_alloc_size = sizeof(struct sunxi_pwm_priv),
+};
u-boot doesn't have dummy regulators, so pwm_backlight probe will fail if regulator is missing. Make it optional to get this driver working on platforms where there's no backlight regultor.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com --- v2: - no changes
drivers/video/pwm_backlight.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/video/pwm_backlight.c b/drivers/video/pwm_backlight.c index fbd7bf7838..05e56ffead 100644 --- a/drivers/video/pwm_backlight.c +++ b/drivers/video/pwm_backlight.c @@ -32,16 +32,21 @@ static int pwm_backlight_enable(struct udevice *dev) uint duty_cycle; int ret;
- plat = dev_get_uclass_platdata(priv->reg); - debug("%s: Enable '%s', regulator '%s'/'%s'\n", __func__, dev->name, - priv->reg->name, plat->name); - ret = regulator_set_enable(priv->reg, true); - if (ret) { - debug("%s: Cannot enable regulator for PWM '%s'\n", __func__, - dev->name); - return ret; + if (priv->reg) { + plat = dev_get_uclass_platdata(priv->reg); + debug("%s: Enable '%s', regulator '%s'/'%s'\n", __func__, dev->name, + priv->reg->name, plat->name); + ret = regulator_set_enable(priv->reg, true); + if (ret) { + debug("%s: Cannot enable regulator for PWM '%s'\n", __func__, + dev->name); + return ret; + } + mdelay(120); } - mdelay(120); + + debug("%s: default: %d, min: %d, max: %d\n", __func__, + priv->default_level, priv->min_level, priv->max_level);
duty_cycle = priv->period_ns * (priv->default_level - priv->min_level) / (priv->max_level - priv->min_level + 1); @@ -68,10 +73,9 @@ static int pwm_backlight_ofdata_to_platdata(struct udevice *dev) debug("%s: start\n", __func__); ret = uclass_get_device_by_phandle(UCLASS_REGULATOR, dev, "power-supply", &priv->reg); - if (ret) { + if (ret) debug("%s: Cannot get power supply: ret=%d\n", __func__, ret); - return ret; - } + ret = gpio_request_by_name(dev, "enable-gpios", 0, &priv->enable, GPIOD_IS_OUT); if (ret) {
Add PWM definition to sun50i-a64.dtsi - it's compatible with PWM found on H3
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com --- v2: - drop 'sun50i-a64-pwm' compatible string and use 'sun8i-h3-pwm' instead, since A64 PWM is compatible with one on H3
arch/arm/dts/sun50i-a64.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/sun50i-a64.dtsi b/arch/arm/dts/sun50i-a64.dtsi index 65a344d9ce..00132855ff 100644 --- a/arch/arm/dts/sun50i-a64.dtsi +++ b/arch/arm/dts/sun50i-a64.dtsi @@ -319,6 +319,14 @@ }; };
+ pwm: pwm@01c21400 { + compatible = "allwinner,sun8i-h3-pwm"; + reg = <0x01c21400 0x8>; + clocks = <&osc24M>; + #pwm-cells = <3>; + status = "disabled"; + }; + uart0: serial@1c28000 { compatible = "snps,dw-apb-uart"; reg = <0x01c28000 0x400>;
On Thu, Sep 21, 2017 at 06:07:04AM +0000, Vasily Khoruzhick wrote:
Add PWM definition to sun50i-a64.dtsi - it's compatible with PWM found on H3
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
v2: - drop 'sun50i-a64-pwm' compatible string and use 'sun8i-h3-pwm' instead, since A64 PWM is compatible with one on H3
arch/arm/dts/sun50i-a64.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/sun50i-a64.dtsi b/arch/arm/dts/sun50i-a64.dtsi index 65a344d9ce..00132855ff 100644 --- a/arch/arm/dts/sun50i-a64.dtsi +++ b/arch/arm/dts/sun50i-a64.dtsi @@ -319,6 +319,14 @@ }; };
pwm: pwm@01c21400 {compatible = "allwinner,sun8i-h3-pwm";
You'd need both compatible. The A64 you used to have, plus that one.
Maxime
On Thu, Sep 21, 2017 at 12:27 AM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Thu, Sep 21, 2017 at 06:07:04AM +0000, Vasily Khoruzhick wrote:
Add PWM definition to sun50i-a64.dtsi - it's compatible with PWM found on H3
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
v2: - drop 'sun50i-a64-pwm' compatible string and use 'sun8i-h3-pwm' instead, since A64 PWM is compatible with one on H3
arch/arm/dts/sun50i-a64.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/sun50i-a64.dtsi b/arch/arm/dts/sun50i-a64.dtsi index 65a344d9ce..00132855ff 100644 --- a/arch/arm/dts/sun50i-a64.dtsi +++ b/arch/arm/dts/sun50i-a64.dtsi @@ -319,6 +319,14 @@ }; };
pwm: pwm@01c21400 {compatible = "allwinner,sun8i-h3-pwm";You'd need both compatible. The A64 you used to have, plus that one.
Why? Hardware seems to be identical and I see a number of non-A64 compatibles in sun50i-a64.dtsi. What's rationale behind adding a new string?
Maxime
-- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Hi Vasily,
On 22/09/17 05:45, Vasily Khoruzhick wrote:
On Thu, Sep 21, 2017 at 12:27 AM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Thu, Sep 21, 2017 at 06:07:04AM +0000, Vasily Khoruzhick wrote:
Add PWM definition to sun50i-a64.dtsi - it's compatible with PWM found on H3
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
v2: - drop 'sun50i-a64-pwm' compatible string and use 'sun8i-h3-pwm' instead, since A64 PWM is compatible with one on H3
arch/arm/dts/sun50i-a64.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/sun50i-a64.dtsi b/arch/arm/dts/sun50i-a64.dtsi index 65a344d9ce..00132855ff 100644 --- a/arch/arm/dts/sun50i-a64.dtsi +++ b/arch/arm/dts/sun50i-a64.dtsi @@ -319,6 +319,14 @@ }; };
pwm: pwm@01c21400 {compatible = "allwinner,sun8i-h3-pwm";You'd need both compatible. The A64 you used to have, plus that one.
Why? Hardware seems to be identical and I see a number of non-A64 compatibles in sun50i-a64.dtsi. What's rationale behind adding a new string?
Two reasons: 1) The DT should say what it is, namely the PWM controller in the A64 SoC. 2) We don't know *for sure* that's really identical. It seems to be from *our* driver's perspective, but there might be bugs in this particular implementation or some specialities. Should we discover this (in the future), we can add the special handling to the driver and bind it to this compatible string. But the DT does not need any change. Which is good, because it's supposed to live in the firmware.
So it should read: compatible = "allwinner,sun50i-a64-pwm", "allwinner,sun8i-h3-pwm";
The DT parsing code won't find a match for the first string, but then fall back on trying the second (or third, ...).
The only thing that would need to be done is to add this new string to the binding documentation, mainly to reserve the name.
Cheers, Andre.
Typo in cover letter subject, it's actually v2 series.
On Wed, Sep 20, 2017 at 11:07 PM, Vasily Khoruzhick anarsoul@gmail.com wrote:
This series introduces a PWM driver for Allwinner H3 and A64 and makes some adjustments to pwm_backlight driver that are neccessary to get backlight working on Pinebook.
Hi,
On 21/09/17 07:22, Vasily Khoruzhick wrote:
Typo in cover letter subject, it's actually v2 series.
$ git format-patch --cover-letter ....
automatically creates a 0/x template (with the right prefix), also gives you some nice diffstat in it.
Cheers, Andre.
On Wed, Sep 20, 2017 at 11:07 PM, Vasily Khoruzhick anarsoul@gmail.com wrote:
This series introduces a PWM driver for Allwinner H3 and A64 and makes some adjustments to pwm_backlight driver that are neccessary to get backlight working on Pinebook.
participants (3)
-
Andre Przywara -
Maxime Ripard -
Vasily Khoruzhick