| Message ID | 1349685556-23718-3-git-send-email-t.figa@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa <t.figa@samsung.com> wrote: > This patch converts the pinctrl-exynos driver to parse wakeup interrupt > count and register offsets from device tree. It reduces the amount of > static platform-specific data and facilitates adding further SoC > variants to pinctrl-samsung driver. So these are: > + ret = of_property_read_u32(wkup_np, "samsung,weint-count", &val); > + ret = of_property_read_u32(wkup_np, "samsung,weint-con", &val); > + ret = of_property_read_u32(wkup_np, "samsung,weint-mask", &val); > + ret = of_property_read_u32(wkup_np, "samsung,weint-pend", &val); Are these all four register offsets? I don't think it's proper for the device tree to contain register offsets. Base address, "regs" property, yes. Individual registers, no. That just makes the code hard to read and compare to the datasheet. Or what are you aiming at here? Linus Walleij
On Wednesday 10 of October 2012 09:18:51 Linus Walleij wrote: > On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa <t.figa@samsung.com> wrote: > > This patch converts the pinctrl-exynos driver to parse wakeup interrupt > > count and register offsets from device tree. It reduces the amount of > > static platform-specific data and facilitates adding further SoC > > variants to pinctrl-samsung driver. > > So these are: > > + ret = of_property_read_u32(wkup_np, "samsung,weint-count", > > &val); + ret = of_property_read_u32(wkup_np, > > "samsung,weint-con", &val); + ret = > > of_property_read_u32(wkup_np, "samsung,weint-mask", &val); + ret > > = of_property_read_u32(wkup_np, "samsung,weint-pend", &val); > Are these all four register offsets? > > I don't think it's proper for the device tree to contain register > offsets. > > Base address, "regs" property, yes. Individual registers, no. That just > makes the code hard to read and compare to the datasheet. > > Or what are you aiming at here? See my reply to your comments for patch 1. I think it should explain how these values are used. One thing worth mentioning is that registers for GPIO interrupts and wakeup interrupts can be located at different areas of pin controller address space, so these offsets have to be specified for both (using geint-* and weint-* properties). Best regards,
diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c index 21362f4..b4b58d4 100644 --- a/drivers/pinctrl/pinctrl-exynos.c +++ b/drivers/pinctrl/pinctrl-exynos.c @@ -435,6 +435,8 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) struct device_node *np; struct exynos_weint_data *weint_data; int idx, irq; + u32 val; + int ret; for_each_child_of_node(dev->of_node, np) { if (of_match_node(exynos_wkup_irq_ids, np)) { @@ -445,6 +447,26 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) if (!wkup_np) return -ENODEV; + ret = of_property_read_u32(wkup_np, "samsung,weint-count", &val); + if (ret) + return -EINVAL; + d->ctrl->nr_wint = val; + + ret = of_property_read_u32(wkup_np, "samsung,weint-con", &val); + if (ret) + return -EINVAL; + d->ctrl->weint_con = val; + + ret = of_property_read_u32(wkup_np, "samsung,weint-mask", &val); + if (ret) + return -EINVAL; + d->ctrl->weint_mask = val; + + ret = of_property_read_u32(wkup_np, "samsung,weint-pend", &val); + if (ret) + return -EINVAL; + d->ctrl->weint_pend = val; + d->wkup_irqd = irq_domain_add_linear(wkup_np, d->ctrl->nr_wint, &exynos_wkup_irqd_ops, d); if (!d->wkup_irqd) {
This patch converts the pinctrl-exynos driver to parse wakeup interrupt count and register offsets from device tree. It reduces the amount of static platform-specific data and facilitates adding further SoC variants to pinctrl-samsung driver. Signed-off-by: Tomasz Figa <t.figa@samsung.com> --- drivers/pinctrl/pinctrl-exynos.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)