On Thu, Jul 17, 2014 at 02:08:14PM +0800, caesar wrote: > Signed-off-by: caesar <[email protected]>
Hi Caesar, just a couple of comments below. > --- > drivers/pwm/pwm-rockchip.c | 108 > ++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 88 insertions(+), 20 deletions(-) > > diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c > index eec2145..59b0380 100644 > --- a/drivers/pwm/pwm-rockchip.c > +++ b/drivers/pwm/pwm-rockchip.c > @@ -2,6 +2,7 @@ > * PWM driver for Rockchip SoCs > * > * Copyright (C) 2014 Beniamino Galvani <[email protected]> > + * Copyright (C) 2014 Caesar Wang <[email protected]> > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > @@ -12,6 +13,8 @@ > #include <linux/io.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_address.h> These two should be swapped to keep the alphabetical order of the includes. > #include <linux/platform_device.h> > #include <linux/pwm.h> > #include <linux/time.h> > @@ -23,14 +26,37 @@ > #define PWM_CTRL_TIMER_EN (1 << 0) > #define PWM_CTRL_OUTPUT_EN (1 << 3) > + [...] > static int rockchip_pwm_probe(struct platform_device *pdev) > { > + const struct of_device_id *of_id = > + of_match_device(rockchip_pwm_dt_ids, &pdev->dev); > + struct device_node *np = pdev->dev.of_node; > struct rockchip_pwm_chip *pc; > struct resource *r; > int ret; > @@ -119,9 +185,12 @@ static int rockchip_pwm_probe(struct platform_device > *pdev) > return -ENOMEM; > > r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - pc->base = devm_ioremap_resource(&pdev->dev, r); > - if (IS_ERR(pc->base)) > - return PTR_ERR(pc->base); > + pc->base = of_iomap(np, 0); > + if (!pc->base) { > + dev_err(&pdev->dev, "failed to map controller\n"); > + ret = -ENOMEM; > + goto fail_map; > + } I think that this change is not needed. devm_ioremap_resource() guarantees an automatic unmapping when the device is destroyed. Moreover, when of_iomap() fails you don't need to call iounmap(). Beniamino > > pc->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(pc->clk)) > @@ -133,6 +202,7 @@ static int rockchip_pwm_probe(struct platform_device > *pdev) > > platform_set_drvdata(pdev, pc); > > + pc->data = of_id->data; > pc->chip.dev = &pdev->dev; > pc->chip.ops = &rockchip_pwm_ops; > pc->chip.base = -1; > @@ -145,6 +215,10 @@ static int rockchip_pwm_probe(struct platform_device > *pdev) > } > > return ret; > + > +fail_map: > + iounmap(pc->base); > + return ret; > } > > static int rockchip_pwm_remove(struct platform_device *pdev) > @@ -156,12 +230,6 @@ static int rockchip_pwm_remove(struct platform_device > *pdev) > return pwmchip_remove(&pc->chip); > } > > -static const struct of_device_id rockchip_pwm_dt_ids[] = { > - { .compatible = "rockchip,rk2928-pwm" }, > - { /* sentinel */ } > -}; > -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > - > static struct platform_driver rockchip_pwm_driver = { > .driver = { > .name = "rockchip-pwm", > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pwm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
