| Message ID | 1324377138-32129-4-git-send-email-thierry.reding@avionic-design.de |
|---|---|
| State | New, archived |
| Headers | show |
Thierry Reding wrote at Tuesday, December 20, 2011 3:32 AM: > This patch adds helpers to support device-tree bindings for the generic > PWM API. > diff --git a/drivers/of/pwm.c b/drivers/of/pwm.c > +/** > + * of_get_named_pwm() - get a PWM number and period to use with the PWM API > + * @np: device node to get the PWM from > + * @propname: property name containing PWM specifier(s) > + * @index: index of the PWM > + * @period_ns: a pointer to the PWM period (in ns) to fill in > + * > + * Returns PWM number to use with the Linux generic PWM API or a negative > + * error code on failure. If @period_ns is not NULL the function fills in > + * the value parsed from the period-ns property. > + */ > +int of_get_named_pwm(struct device_node *np, const char *propname, > + int index, unsigned int *period_ns) > +{ ... > + if ((cells > 1) && period_ns) > + *period_ns = be32_to_cpu(spec[1]); What if the client needs to know period_ns, yet the DT doesn't provide it? I think a better approach would be to use an "of_xlate" function like GPIO and IRQ do. This way, the PWM device's own bindings get to define what the extra cells mean, and the definition of of_xlate can be such that it must return a period_ns value in all cases; in some cases, the driver may return a hard-coded value if the HW doesn't support the feature, whereas in most cases the value would be parsed from the extra cells. Without seeing the complete binding documentation and an example, it's difficult to think about whether it's a good idea to include period_ns in the PWM specifier or not. An alternative might be a property in the PWM node itself. > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > @@ -65,6 +65,10 @@ struct pwm_chip { > +#ifdef CONFIG_OF_PWM > + struct device_node *of_node; > +#endif > }; Can we remove the conditionals here? That would allow a PWM driver to unconditionally assign this field, rather than having to wrap the assignment in ifdefs. Actually, even better would be for struct pwm_chip to contain a struct device * instead. That'd allow the PWM core to use that for e.g. dev_err calls, and it includes the of_node for use in the match function above.
* Stephen Warren wrote: > Thierry Reding wrote at Tuesday, December 20, 2011 3:32 AM: > > This patch adds helpers to support device-tree bindings for the generic > > PWM API. > > > diff --git a/drivers/of/pwm.c b/drivers/of/pwm.c > > > +/** > > + * of_get_named_pwm() - get a PWM number and period to use with the PWM API > > + * @np: device node to get the PWM from > > + * @propname: property name containing PWM specifier(s) > > + * @index: index of the PWM > > + * @period_ns: a pointer to the PWM period (in ns) to fill in > > + * > > + * Returns PWM number to use with the Linux generic PWM API or a negative > > + * error code on failure. If @period_ns is not NULL the function fills in > > + * the value parsed from the period-ns property. > > + */ > > +int of_get_named_pwm(struct device_node *np, const char *propname, > > + int index, unsigned int *period_ns) > > +{ > ... > > + if ((cells > 1) && period_ns) > > + *period_ns = be32_to_cpu(spec[1]); > > What if the client needs to know period_ns, yet the DT doesn't provide > it? > > I think a better approach would be to use an "of_xlate" function like > GPIO and IRQ do. This way, the PWM device's own bindings get to define > what the extra cells mean, and the definition of of_xlate can be such > that it must return a period_ns value in all cases; in some cases, the > driver may return a hard-coded value if the HW doesn't support the > feature, whereas in most cases the value would be parsed from the > extra cells. > > Without seeing the complete binding documentation and an example, it's > difficult to think about whether it's a good idea to include period_ns > in the PWM specifier or not. An alternative might be a property in the > PWM node itself. I like the "of_xlate" alternative better. Adding a property to the PWM node would hard-code the period regardless of the user. That doesn't really reflect the PWM API. I will play around with it a bit and make sure to include PWM binding documentation in the next version. > Actually, even better would be for struct pwm_chip to contain a struct > device * instead. That'd allow the PWM core to use that for e.g. dev_err > calls, and it includes the of_node for use in the match function above. I like that idea. I'll address that in the next version. Thierry
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index cac63c9..62a2073 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -47,6 +47,12 @@ config OF_GPIO help OpenFirmware GPIO accessors +config OF_PWM + def_bool y + depends on PWM + help + OpenFirmware PWM accessors + config OF_I2C def_tristate I2C depends on I2C && !SPARC diff --git a/drivers/of/Makefile b/drivers/of/Makefile index dccb117..c3acf27 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_OF_ADDRESS) += address.o obj-$(CONFIG_OF_IRQ) += irq.o obj-$(CONFIG_OF_DEVICE) += device.o platform.o obj-$(CONFIG_OF_GPIO) += gpio.o +obj-$(CONFIG_OF_PWM) += pwm.o obj-$(CONFIG_OF_I2C) += of_i2c.o obj-$(CONFIG_OF_NET) += of_net.o obj-$(CONFIG_OF_SPI) += of_spi.o diff --git a/drivers/of/pwm.c b/drivers/of/pwm.c new file mode 100644 index 0000000..c0ef302 --- /dev/null +++ b/drivers/of/pwm.c @@ -0,0 +1,88 @@ +/* + * OF helpers for the PWM API + * + * Copyright (c) 2011 Avionic Design GmbH + * + * 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/errno.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pwm.h> + +static int of_pwmchip_is_match(struct pwm_chip *chip, void *data) +{ + return chip->of_node == data; +} + +/** + * of_node_to_pwmchip() - finds the PWM chip associated with a device node + * @np: device node of the PWM chip + * + * Returns a pointer to the PWM chip associated with the specified device + * node or NULL if the device node doesn't represent a PWM chip. + */ +struct pwm_chip *of_node_to_pwmchip(struct device_node *np) +{ + return pwmchip_find(np, of_pwmchip_is_match); +} + +/** + * of_get_named_pwm() - get a PWM number and period to use with the PWM API + * @np: device node to get the PWM from + * @propname: property name containing PWM specifier(s) + * @index: index of the PWM + * @period_ns: a pointer to the PWM period (in ns) to fill in + * + * Returns PWM number to use with the Linux generic PWM API or a negative + * error code on failure. If @period_ns is not NULL the function fills in + * the value parsed from the period-ns property. + */ +int of_get_named_pwm(struct device_node *np, const char *propname, + int index, unsigned int *period_ns) +{ + struct device_node *pwm_np; + const void *pwm_spec; + struct pwm_chip *pc; + const __be32 *spec; + u32 cells; + u32 pwm; + int ret; + + ret = of_parse_phandles_with_args(np, propname, "#pwm-cells", index, + &pwm_np, &pwm_spec); + if (ret) { + pr_debug("%s(): can't parse pwms property\n", __func__); + goto out; + } + + pc = of_node_to_pwmchip(pwm_np); + if (!pc) { + ret = -ENODEV; + goto put; + } + + ret = of_property_read_u32(pwm_np, "#pwm-cells", &cells); + if (ret < 0) + goto put; + + spec = (const __be32 *)pwm_spec; + pwm = be32_to_cpup(pwm_spec); + + if ((cells > 1) && period_ns) + *period_ns = be32_to_cpu(spec[1]); + + ret = pc->base + pwm; + +put: + of_node_put(pwm_np); +out: + pr_debug("%s() exited with status %d\n", __func__, ret); + return ret; +} +EXPORT_SYMBOL(of_get_named_pwm); diff --git a/include/linux/of_pwm.h b/include/linux/of_pwm.h new file mode 100644 index 0000000..8d6d569 --- /dev/null +++ b/include/linux/of_pwm.h @@ -0,0 +1,41 @@ +/* + * OF helpers for the PWM API + * + * Copyright (c) 2011 Avionic Design GmbH + * + * 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. + */ + +#ifndef __LINUX_OF_PWM_H +#define __LINUX_OF_PWM_H + +#include <linux/pwm.h> + +struct device_node; + +#ifdef CONFIG_OF_PWM + +struct pwm_chip *of_node_to_pwmchip(struct device_node *np); +int of_get_named_pwm(struct device_node *np, const char *propname, + int index, unsigned int *period_ns); + +#else + +static inline struct pwm_chip *of_node_to_pwmchip(struct device_node *np) +{ + return NULL; +} + +static inline int of_get_named_pwm(struct device_node *np, + const char *propname, int index, + unsigned int *period_ns) +{ + return -ENOSYS; +} + +#endif /* CONFIG_OF_PWM */ + +#endif /* __LINUX_OF_PWM_H */ diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 01c0153..d6be4e4 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -65,6 +65,10 @@ struct pwm_chip { struct pwm_ops *ops; int base; unsigned int npwm; + +#ifdef CONFIG_OF_PWM + struct device_node *of_node; +#endif }; int pwmchip_add(struct pwm_chip *chip);
This patch adds helpers to support device-tree bindings for the generic PWM API. Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de> --- drivers/of/Kconfig | 6 +++ drivers/of/Makefile | 1 + drivers/of/pwm.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_pwm.h | 41 ++++++++++++++++++++++ include/linux/pwm.h | 4 ++ 5 files changed, 140 insertions(+), 0 deletions(-) create mode 100644 drivers/of/pwm.c create mode 100644 include/linux/of_pwm.h