Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-sun4i.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 581d23287333..f2afd312f77c 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -344,7 +344,6 @@ MODULE_DEVICE_TABLE(of, sun4i_pwm_dt_ids);
static int sun4i_pwm_probe(struct platform_device *pdev)
{
struct sun4i_pwm_chip *pwm;
- struct resource *res;
int ret;
pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
@@ -355,8 +354,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
if (!pwm->data)
return -ENODEV;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pwm->base))
return PTR_ERR(pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-rcar.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index 852eb2347954..7bd67f69db84 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -203,15 +203,13 @@ static const struct pwm_ops rcar_pwm_ops = {
static int rcar_pwm_probe(struct platform_device *pdev)
{
struct rcar_pwm_chip *rcar_pwm;
- struct resource *res;
int ret;
rcar_pwm = devm_kzalloc(&pdev->dev, sizeof(*rcar_pwm), GFP_KERNEL);
if (rcar_pwm == NULL)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- rcar_pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ rcar_pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(rcar_pwm->base))
return PTR_ERR(rcar_pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-fsl-ftm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index 59272a920479..2a6801226aba 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -399,7 +399,6 @@ static const struct regmap_config fsl_pwm_regmap_config = {
static int fsl_pwm_probe(struct platform_device *pdev)
{
struct fsl_pwm_chip *fpc;
- struct resource *res;
void __iomem *base;
int ret;
@@ -412,8 +411,7 @@ static int fsl_pwm_probe(struct platform_device *pdev)
fpc->soc = of_device_get_match_data(&pdev->dev);
fpc->chip.dev = &pdev->dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(&pdev->dev, res);
+ base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-renesas-tpu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 4a855a21b782..9f4695cc8e72 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -383,7 +383,6 @@ static const struct pwm_ops tpu_pwm_ops = {
static int tpu_probe(struct platform_device *pdev)
{
struct tpu_device *tpu;
- struct resource *res;
int ret;
tpu = devm_kzalloc(&pdev->dev, sizeof(*tpu), GFP_KERNEL);
@@ -394,8 +393,7 @@ static int tpu_probe(struct platform_device *pdev)
tpu->pdev = pdev;
/* Map memory, get clock and pin control. */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- tpu->base = devm_ioremap_resource(&pdev->dev, res);
+ tpu->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(tpu->base))
return PTR_ERR(tpu->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-mediatek.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index b94e0d09c300..e4a715d3860c 100644
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -207,7 +207,6 @@ static const struct pwm_ops pwm_mediatek_ops = {
static int pwm_mediatek_probe(struct platform_device *pdev)
{
struct pwm_mediatek_chip *pc;
- struct resource *res;
unsigned int i;
int ret;
@@ -217,8 +216,7 @@ static int pwm_mediatek_probe(struct platform_device *pdev)
pc->soc = of_device_get_match_data(&pdev->dev);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pc->regs = devm_ioremap_resource(&pdev->dev, res);
+ pc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->regs))
return PTR_ERR(pc->regs);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-rockchip.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 73352e6fbccb..f0549b82338d 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -292,7 +292,6 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
{
const struct of_device_id *id;
struct rockchip_pwm_chip *pc;
- struct resource *r;
int ret, count;
id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
@@ -303,8 +302,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
if (!pc)
return -ENOMEM;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pc->base = devm_ioremap_resource(&pdev->dev, r);
+ pc->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->base))
return PTR_ERR(pc->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-samsung.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index 87a886f7dc2f..645d0066ff0a 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -510,7 +510,6 @@ static int pwm_samsung_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct samsung_pwm_chip *chip;
- struct resource *res;
unsigned int chan;
int ret;
@@ -541,8 +540,7 @@ static int pwm_samsung_probe(struct platform_device *pdev)
sizeof(chip->variant));
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- chip->base = devm_ioremap_resource(&pdev->dev, res);
+ chip->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(chip->base))
return PTR_ERR(chip->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-puv3.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-puv3.c b/drivers/pwm/pwm-puv3.c
index 9d0bd87a425e..2367dad7cf95 100644
--- a/drivers/pwm/pwm-puv3.c
+++ b/drivers/pwm/pwm-puv3.c
@@ -100,7 +100,6 @@ static const struct pwm_ops puv3_pwm_ops = {
static int pwm_probe(struct platform_device *pdev)
{
struct puv3_pwm_chip *puv3;
- struct resource *r;
int ret;
puv3 = devm_kzalloc(&pdev->dev, sizeof(*puv3), GFP_KERNEL);
@@ -111,8 +110,7 @@ static int pwm_probe(struct platform_device *pdev)
if (IS_ERR(puv3->clk))
return PTR_ERR(puv3->clk);
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- puv3->base = devm_ioremap_resource(&pdev->dev, r);
+ puv3->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(puv3->base))
return PTR_ERR(puv3->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-brcmstb.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-brcmstb.c b/drivers/pwm/pwm-brcmstb.c
index fea612c45f20..8b66f9d2f589 100644
--- a/drivers/pwm/pwm-brcmstb.c
+++ b/drivers/pwm/pwm-brcmstb.c
@@ -234,7 +234,6 @@ MODULE_DEVICE_TABLE(of, brcmstb_pwm_of_match);
static int brcmstb_pwm_probe(struct platform_device *pdev)
{
struct brcmstb_pwm *p;
- struct resource *res;
int ret;
p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
@@ -262,8 +261,7 @@ static int brcmstb_pwm_probe(struct platform_device *pdev)
p->chip.base = -1;
p->chip.npwm = 2;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- p->base = devm_ioremap_resource(&pdev->dev, res);
+ p->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(p->base)) {
ret = PTR_ERR(p->base);
goto out_clk;
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-meson.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 6245bbdb6e6c..62fc79092038 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -537,15 +537,13 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
static int meson_pwm_probe(struct platform_device *pdev)
{
struct meson_pwm *meson;
- struct resource *regs;
int err;
meson = devm_kzalloc(&pdev->dev, sizeof(*meson), GFP_KERNEL);
if (!meson)
return -ENOMEM;
- regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- meson->base = devm_ioremap_resource(&pdev->dev, regs);
+ meson->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(meson->base))
return PTR_ERR(meson->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-sifive.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index cc63f9baa481..cb9134d7197b 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -232,7 +232,6 @@ static int pwm_sifive_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct pwm_sifive_ddata *ddata;
struct pwm_chip *chip;
- struct resource *res;
int ret;
ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
@@ -248,8 +247,7 @@ static int pwm_sifive_probe(struct platform_device *pdev)
chip->base = -1;
chip->npwm = 4;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- ddata->regs = devm_ioremap_resource(dev, res);
+ ddata->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(ddata->regs))
return PTR_ERR(ddata->regs);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
'i' and 'ret' are variables of the same type and there is no
need to use two lines.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-hibvt.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-hibvt.c b/drivers/pwm/pwm-hibvt.c
index ad205fdad372..a1900d0a872e 100644
--- a/drivers/pwm/pwm-hibvt.c
+++ b/drivers/pwm/pwm-hibvt.c
@@ -190,9 +190,7 @@ static int hibvt_pwm_probe(struct platform_device *pdev)
const struct hibvt_pwm_soc *soc =
of_device_get_match_data(&pdev->dev);
struct hibvt_pwm_chip *pwm_chip;
- struct resource *res;
- int ret;
- int i;
+ int ret, i;
pwm_chip = devm_kzalloc(&pdev->dev, sizeof(*pwm_chip), GFP_KERNEL);
if (pwm_chip == NULL)
@@ -213,8 +211,7 @@ static int hibvt_pwm_probe(struct platform_device *pdev)
pwm_chip->chip.of_pwm_n_cells = 3;
pwm_chip->soc = soc;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pwm_chip->base = devm_ioremap_resource(&pdev->dev, res);
+ pwm_chip->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pwm_chip->base))
return PTR_ERR(pwm_chip->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-img.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-img.c b/drivers/pwm/pwm-img.c
index c9e57bd109fb..e9b25440b808 100644
--- a/drivers/pwm/pwm-img.c
+++ b/drivers/pwm/pwm-img.c
@@ -238,7 +238,6 @@ static int img_pwm_probe(struct platform_device *pdev)
int ret;
u64 val;
unsigned long clk_rate;
- struct resource *res;
struct img_pwm_chip *pwm;
const struct of_device_id *of_dev_id;
@@ -248,8 +247,7 @@ static int img_pwm_probe(struct platform_device *pdev)
pwm->dev = &pdev->dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pwm->base))
return PTR_ERR(pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-tegra.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index aa12fb3ed92e..7f77a93eb4b5 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -170,7 +170,6 @@ static const struct pwm_ops tegra_pwm_ops = {
static int tegra_pwm_probe(struct platform_device *pdev)
{
struct tegra_pwm_chip *pwm;
- struct resource *r;
int ret;
pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
@@ -180,8 +179,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
pwm->soc = of_device_get_match_data(&pdev->dev);
pwm->dev = &pdev->dev;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pwm->regs = devm_ioremap_resource(&pdev->dev, r);
+ pwm->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pwm->regs))
return PTR_ERR(pwm->regs);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-vt8500.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index 11d45e56a923..6e36851a22bb 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -193,7 +193,6 @@ MODULE_DEVICE_TABLE(of, vt8500_pwm_dt_ids);
static int vt8500_pwm_probe(struct platform_device *pdev)
{
struct vt8500_chip *chip;
- struct resource *r;
struct device_node *np = pdev->dev.of_node;
int ret;
@@ -219,8 +218,7 @@ static int vt8500_pwm_probe(struct platform_device *pdev)
return PTR_ERR(chip->clk);
}
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- chip->base = devm_ioremap_resource(&pdev->dev, r);
+ chip->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(chip->base))
return PTR_ERR(chip->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-clps711x.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 924d39a797cf..e69a5877cec6 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -113,14 +113,12 @@ static struct pwm_device *clps711x_pwm_xlate(struct pwm_chip *chip,
static int clps711x_pwm_probe(struct platform_device *pdev)
{
struct clps711x_chip *priv;
- struct resource *res;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- priv->pmpcon = devm_ioremap_resource(&pdev->dev, res);
+ priv->pmpcon = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(priv->pmpcon))
return PTR_ERR(priv->pmpcon);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-bcm2835.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index 91e24f01b54e..9923cd2f069e 100644
--- a/drivers/pwm/pwm-bcm2835.c
+++ b/drivers/pwm/pwm-bcm2835.c
@@ -137,7 +137,6 @@ static const struct pwm_ops bcm2835_pwm_ops = {
static int bcm2835_pwm_probe(struct platform_device *pdev)
{
struct bcm2835_pwm *pc;
- struct resource *res;
int ret;
pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
@@ -146,8 +145,7 @@ static int bcm2835_pwm_probe(struct platform_device *pdev)
pc->dev = &pdev->dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pc->base = devm_ioremap_resource(&pdev->dev, res);
+ pc->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->base))
return PTR_ERR(pc->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-mtk-disp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 83b8be0209b7..87c6b4bc5d43 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -172,7 +172,6 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
static int mtk_disp_pwm_probe(struct platform_device *pdev)
{
struct mtk_disp_pwm *mdp;
- struct resource *r;
int ret;
mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
@@ -181,8 +180,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
mdp->data = of_device_get_match_data(&pdev->dev);
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- mdp->base = devm_ioremap_resource(&pdev->dev, r);
+ mdp->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(mdp->base))
return PTR_ERR(mdp->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-tiecap.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index ab38c8203b79..3a9885cce386 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -196,7 +196,6 @@ static int ecap_pwm_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct ecap_pwm_chip *pc;
- struct resource *r;
struct clk *clk;
int ret;
@@ -230,8 +229,7 @@ static int ecap_pwm_probe(struct platform_device *pdev)
pc->chip.base = -1;
pc->chip.npwm = 1;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pc->mmio_base = devm_ioremap_resource(&pdev->dev, r);
+ pc->mmio_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->mmio_base))
return PTR_ERR(pc->mmio_base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-tiehrpwm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-tiehrpwm.c b/drivers/pwm/pwm-tiehrpwm.c
index 7b4c770ce9d6..3a1313caa5fe 100644
--- a/drivers/pwm/pwm-tiehrpwm.c
+++ b/drivers/pwm/pwm-tiehrpwm.c
@@ -421,7 +421,6 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct ehrpwm_pwm_chip *pc;
- struct resource *r;
struct clk *clk;
int ret;
@@ -455,8 +454,7 @@ static int ehrpwm_pwm_probe(struct platform_device *pdev)
pc->chip.base = -1;
pc->chip.npwm = NUM_PWM_CHANNEL;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pc->mmio_base = devm_ioremap_resource(&pdev->dev, r);
+ pc->mmio_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->mmio_base))
return PTR_ERR(pc->mmio_base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-lpc18xx-sct.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 5ff11145c1a3..dc5133bec3e7 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -325,7 +325,6 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
{
struct lpc18xx_pwm_chip *lpc18xx_pwm;
struct pwm_device *pwm;
- struct resource *res;
int ret, i;
u64 val;
@@ -336,8 +335,7 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
lpc18xx_pwm->dev = &pdev->dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- lpc18xx_pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ lpc18xx_pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(lpc18xx_pwm->base))
return PTR_ERR(lpc18xx_pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-lpc32xx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index 710d9a207d2b..6b4090436c06 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -98,7 +98,6 @@ static const struct pwm_ops lpc32xx_pwm_ops = {
static int lpc32xx_pwm_probe(struct platform_device *pdev)
{
struct lpc32xx_pwm_chip *lpc32xx;
- struct resource *res;
int ret;
u32 val;
@@ -106,8 +105,7 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev)
if (!lpc32xx)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- lpc32xx->base = devm_ioremap_resource(&pdev->dev, res);
+ lpc32xx->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(lpc32xx->base))
return PTR_ERR(lpc32xx->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-bcm-kona.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
index 81da91df2529..aa451987733b 100644
--- a/drivers/pwm/pwm-bcm-kona.c
+++ b/drivers/pwm/pwm-bcm-kona.c
@@ -259,7 +259,6 @@ static const struct pwm_ops kona_pwm_ops = {
static int kona_pwmc_probe(struct platform_device *pdev)
{
struct kona_pwmc *kp;
- struct resource *res;
unsigned int chan;
unsigned int value = 0;
int ret = 0;
@@ -277,8 +276,7 @@ static int kona_pwmc_probe(struct platform_device *pdev)
kp->chip.of_xlate = of_pwm_xlate_with_flags;
kp->chip.of_pwm_n_cells = 3;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- kp->base = devm_ioremap_resource(&pdev->dev, res);
+ kp->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(kp->base))
return PTR_ERR(kp->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-atmel.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 9ba733467e26..86cc5ccaa46c 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -340,7 +340,6 @@ MODULE_DEVICE_TABLE(of, atmel_pwm_dt_ids);
static int atmel_pwm_probe(struct platform_device *pdev)
{
struct atmel_pwm_chip *atmel_pwm;
- struct resource *res;
int ret;
atmel_pwm = devm_kzalloc(&pdev->dev, sizeof(*atmel_pwm), GFP_KERNEL);
@@ -351,8 +350,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
atmel_pwm->data = of_device_get_match_data(&pdev->dev);
atmel_pwm->updated_pwms = 0;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- atmel_pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ atmel_pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(atmel_pwm->base))
return PTR_ERR(atmel_pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-spear.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c
index 6c6b44fd3f43..f63b54aae1b4 100644
--- a/drivers/pwm/pwm-spear.c
+++ b/drivers/pwm/pwm-spear.c
@@ -174,7 +174,6 @@ static int spear_pwm_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct spear_pwm_chip *pc;
- struct resource *r;
int ret;
u32 val;
@@ -182,8 +181,7 @@ static int spear_pwm_probe(struct platform_device *pdev)
if (!pc)
return -ENOMEM;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pc->mmio_base = devm_ioremap_resource(&pdev->dev, r);
+ pc->mmio_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->mmio_base))
return PTR_ERR(pc->mmio_base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-berlin.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
index b91c477cc84b..fe405289e582 100644
--- a/drivers/pwm/pwm-berlin.c
+++ b/drivers/pwm/pwm-berlin.c
@@ -186,15 +186,13 @@ MODULE_DEVICE_TABLE(of, berlin_pwm_match);
static int berlin_pwm_probe(struct platform_device *pdev)
{
struct berlin_pwm_chip *pwm;
- struct resource *res;
int ret;
pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
if (!pwm)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pwm->base))
return PTR_ERR(pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-ep93xx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-ep93xx.c b/drivers/pwm/pwm-ep93xx.c
index 4bab73073ad7..c9fc6f223640 100644
--- a/drivers/pwm/pwm-ep93xx.c
+++ b/drivers/pwm/pwm-ep93xx.c
@@ -169,15 +169,13 @@ static const struct pwm_ops ep93xx_pwm_ops = {
static int ep93xx_pwm_probe(struct platform_device *pdev)
{
struct ep93xx_pwm *ep93xx_pwm;
- struct resource *res;
int ret;
ep93xx_pwm = devm_kzalloc(&pdev->dev, sizeof(*ep93xx_pwm), GFP_KERNEL);
if (!ep93xx_pwm)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- ep93xx_pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ ep93xx_pwm->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(ep93xx_pwm->base))
return PTR_ERR(ep93xx_pwm->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-imx1.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-imx1.c b/drivers/pwm/pwm-imx1.c
index f8b2c2e001a7..1a60bfd7d659 100644
--- a/drivers/pwm/pwm-imx1.c
+++ b/drivers/pwm/pwm-imx1.c
@@ -136,7 +136,6 @@ MODULE_DEVICE_TABLE(of, pwm_imx1_dt_ids);
static int pwm_imx1_probe(struct platform_device *pdev)
{
struct pwm_imx1_chip *imx;
- struct resource *r;
imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
if (!imx)
@@ -168,8 +167,7 @@ static int pwm_imx1_probe(struct platform_device *pdev)
imx->chip.base = -1;
imx->chip.npwm = 1;
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
+ imx->mmio_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(imx->mmio_base))
return PTR_ERR(imx->mmio_base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-sti.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c
index 1508616d794c..eaeb38c0c0c7 100644
--- a/drivers/pwm/pwm-sti.c
+++ b/drivers/pwm/pwm-sti.c
@@ -540,7 +540,6 @@ static int sti_pwm_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct sti_pwm_compat_data *cdata;
struct sti_pwm_chip *pc;
- struct resource *res;
unsigned int i;
int irq, ret;
@@ -552,9 +551,7 @@ static int sti_pwm_probe(struct platform_device *pdev)
if (!cdata)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
- pc->mmio = devm_ioremap_resource(dev, res);
+ pc->mmio = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pc->mmio))
return PTR_ERR(pc->mmio);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-bcm-iproc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c
index 1f829edd8ee7..52860df7d9b7 100644
--- a/drivers/pwm/pwm-bcm-iproc.c
+++ b/drivers/pwm/pwm-bcm-iproc.c
@@ -193,7 +193,6 @@ static const struct pwm_ops iproc_pwm_ops = {
static int iproc_pwmc_probe(struct platform_device *pdev)
{
struct iproc_pwmc *ip;
- struct resource *res;
unsigned int i;
u32 value;
int ret;
@@ -211,8 +210,7 @@ static int iproc_pwmc_probe(struct platform_device *pdev)
ip->chip.of_xlate = of_pwm_xlate_with_flags;
ip->chip.of_pwm_n_cells = 3;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- ip->base = devm_ioremap_resource(&pdev->dev, res);
+ ip->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(ip->base))
return PTR_ERR(ip->base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-pxa.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index a2a0912c2dcd..d06cf60e6575 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -166,7 +166,6 @@ static int pwm_probe(struct platform_device *pdev)
{
const struct platform_device_id *id = platform_get_device_id(pdev);
struct pxa_pwm_chip *pwm;
- struct resource *r;
int ret = 0;
if (IS_ENABLED(CONFIG_OF) && id == NULL)
@@ -193,8 +192,7 @@ static int pwm_probe(struct platform_device *pdev)
pwm->chip.of_pwm_n_cells = 1;
}
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pwm->mmio_base = devm_ioremap_resource(&pdev->dev, r);
+ pwm->mmio_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pwm->mmio_base))
return PTR_ERR(pwm->mmio_base);
--
2.17.1
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <[email protected]>
---
drivers/pwm/pwm-zx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-zx.c b/drivers/pwm/pwm-zx.c
index e2c21cc34a96..0f5e8919b034 100644
--- a/drivers/pwm/pwm-zx.c
+++ b/drivers/pwm/pwm-zx.c
@@ -196,7 +196,6 @@ static const struct pwm_ops zx_pwm_ops = {
static int zx_pwm_probe(struct platform_device *pdev)
{
struct zx_pwm_chip *zpc;
- struct resource *res;
unsigned int i;
int ret;
@@ -204,8 +203,7 @@ static int zx_pwm_probe(struct platform_device *pdev)
if (!zpc)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- zpc->base = devm_ioremap_resource(&pdev->dev, res);
+ zpc->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(zpc->base))
return PTR_ERR(zpc->base);
--
2.17.1
Am Sonntag, 29. Dezember 2019, 09:05:53 CET schrieb Yangtao Li:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
> ---
> drivers/pwm/pwm-rockchip.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index 73352e6fbccb..f0549b82338d 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -292,7 +292,6 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> {
> const struct of_device_id *id;
> struct rockchip_pwm_chip *pc;
> - struct resource *r;
> int ret, count;
>
> id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev);
> @@ -303,8 +302,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> if (!pc)
> return -ENOMEM;
>
> - r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - pc->base = devm_ioremap_resource(&pdev->dev, r);
> + pc->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(pc->base))
> return PTR_ERR(pc->base);
>
>
On Sun, Dec 29, 2019 at 9:16 AM Yangtao Li <[email protected]> wrote:
>
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Martin Blumenstingl <[email protected]>
thank you for taking care of this cleanup!
Martin
On 29.12.2019 10:06, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Acked-by: Claudiu Beznea <[email protected]>
> ---
> drivers/pwm/pwm-atmel.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 9ba733467e26..86cc5ccaa46c 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -340,7 +340,6 @@ MODULE_DEVICE_TABLE(of, atmel_pwm_dt_ids);
> static int atmel_pwm_probe(struct platform_device *pdev)
> {
> struct atmel_pwm_chip *atmel_pwm;
> - struct resource *res;
> int ret;
>
> atmel_pwm = devm_kzalloc(&pdev->dev, sizeof(*atmel_pwm), GFP_KERNEL);
> @@ -351,8 +350,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
> atmel_pwm->data = of_device_get_match_data(&pdev->dev);
> atmel_pwm->updated_pwms = 0;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - atmel_pwm->base = devm_ioremap_resource(&pdev->dev, res);
> + atmel_pwm->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(atmel_pwm->base))
> return PTR_ERR(atmel_pwm->base);
>
> --
> 2.17.1
>
>
On Sun, 2019-12-29 at 08:06 +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
Reviewed-by: Nicolas Saenz Julienne <[email protected]>
Thanks!
> drivers/pwm/pwm-brcmstb.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-brcmstb.c b/drivers/pwm/pwm-brcmstb.c
> index fea612c45f20..8b66f9d2f589 100644
> --- a/drivers/pwm/pwm-brcmstb.c
> +++ b/drivers/pwm/pwm-brcmstb.c
> @@ -234,7 +234,6 @@ MODULE_DEVICE_TABLE(of, brcmstb_pwm_of_match);
> static int brcmstb_pwm_probe(struct platform_device *pdev)
> {
> struct brcmstb_pwm *p;
> - struct resource *res;
> int ret;
>
> p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> @@ -262,8 +261,7 @@ static int brcmstb_pwm_probe(struct platform_device *pdev)
> p->chip.base = -1;
> p->chip.npwm = 2;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - p->base = devm_ioremap_resource(&pdev->dev, res);
> + p->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(p->base)) {
> ret = PTR_ERR(p->base);
> goto out_clk;
On Sun, Dec 29, 2019 at 08:06:08AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 'i' and 'ret' are variables of the same type and there is no
> need to use two lines.
I think I wouldn't have merged these two lines, but I don't feel strong
here. The other 31 patches are clean replacements.
But I also don't think respining just for this minor thing is worth the
effort, so:
Acked-by: Uwe Kleine-K?nig <[email protected]>
for the whole series. (Not sure it is sensible to ack each patch
individually, @Thierry, tell me if this simplifies things for you.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 29/12/2019 09:06, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
> ---
> drivers/pwm/pwm-mtk-disp.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> index 83b8be0209b7..87c6b4bc5d43 100644
> --- a/drivers/pwm/pwm-mtk-disp.c
> +++ b/drivers/pwm/pwm-mtk-disp.c
> @@ -172,7 +172,6 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> static int mtk_disp_pwm_probe(struct platform_device *pdev)
> {
> struct mtk_disp_pwm *mdp;
> - struct resource *r;
> int ret;
>
> mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> @@ -181,8 +180,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>
> mdp->data = of_device_get_match_data(&pdev->dev);
>
> - r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - mdp->base = devm_ioremap_resource(&pdev->dev, r);
> + mdp->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(mdp->base))
> return PTR_ERR(mdp->base);
>
>
On Sun, Dec 29, 2019 at 08:05:39AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Sun, Dec 29, 2019 at 08:05:42AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:07AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:09AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:06AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Sun, Dec 29, 2019 at 08:06:02AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:05AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:59AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:01AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:57AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:00AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:51AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:50AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:58AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:47AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:56AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:54AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:49AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:48AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:55AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:45AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:44AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:43AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:41AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:40AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:53AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:46AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:05:52AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:03AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Sun, Dec 29, 2019 at 08:06:10AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Feb 20, 2020 at 09:41:52PM +0100, Uwe Kleine-K?nig wrote:
> On Sun, Dec 29, 2019 at 08:06:08AM +0000, Yangtao Li wrote:
> > Use devm_platform_ioremap_resource() to simplify code.
> > 'i' and 'ret' are variables of the same type and there is no
> > need to use two lines.
>
> I think I wouldn't have merged these two lines, but I don't feel strong
> here. The other 31 patches are clean replacements.
>
> But I also don't think respining just for this minor thing is worth the
> effort, so:
>
> Acked-by: Uwe Kleine-K?nig <[email protected]>
>
> for the whole series. (Not sure it is sensible to ack each patch
> individually, @Thierry, tell me if this simplifies things for you.)
I took a deeper look now and added Reviewed-by for all other patches to
ease application. So doing the same here:
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
FTR: I'd do the following now:
for patch in 1216003 1216065 1216063 1216005 1216062 1216061 1216059 1216057 1216054 1216056 1216051 1216050 1216048 1216010 1216044 1216046 1216042 1216041 1216036 1216037 1216034 1216032 1216030 1216013 1216029 1216025 1216026 1216024 1216015 1216021 1216017 1216019; do
pwclient git-am -m -s $patch && pwclient update -s "Accepted" -c "$(git rev-parse HEAD)" $patch || break
done
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello Thierry,
On Sun, Dec 29, 2019 at 08:05:39AM +0000, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> drivers/pwm/pwm-sun4i.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 581d23287333..f2afd312f77c 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -344,7 +344,6 @@ MODULE_DEVICE_TABLE(of, sun4i_pwm_dt_ids);
> static int sun4i_pwm_probe(struct platform_device *pdev)
> {
> struct sun4i_pwm_chip *pwm;
> - struct resource *res;
> int ret;
>
> pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> @@ -355,8 +354,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> if (!pwm->data)
> return -ENODEV;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - pwm->base = devm_ioremap_resource(&pdev->dev, res);
> + pwm->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(pwm->base))
> return PTR_ERR(pwm->base);
Can you please comment why you don't apply this series?
My point of view is:
devm_platform_ioremap_resource is the designated wrapper to replace
platform_get_resource() and devm_ioremap_resource(). So I don't see a
good reason to continue open coding it.
The patch series doesn't apply to 5.10-rc1 as is. (pwm-puv3 was removed
and a simple conflict in the pwm-rockchip driver.) The overall diffstat
(of the fixed series applied on top of 5.10-rc1) is
31 files changed, 32 insertions(+), 96 deletions(-)
and it converts all of drivers/pwm but a single instance of
platform_get_resource() + devm_ioremap_resource() (for pwm-lpss where
platform_get_resource and devm_ioremap_resource are in different
functions (different files even)) which isn't trivial to fix.
So in my eyes applying this series is the right thing to do.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Nov 12, 2020 at 05:13:46PM +0100, Uwe Kleine-König wrote:
> Hello Thierry,
>
> On Sun, Dec 29, 2019 at 08:05:39AM +0000, Yangtao Li wrote:
> > Use devm_platform_ioremap_resource() to simplify code.
> >
> > Signed-off-by: Yangtao Li <[email protected]>
> > ---
> > drivers/pwm/pwm-sun4i.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 581d23287333..f2afd312f77c 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -344,7 +344,6 @@ MODULE_DEVICE_TABLE(of, sun4i_pwm_dt_ids);
> > static int sun4i_pwm_probe(struct platform_device *pdev)
> > {
> > struct sun4i_pwm_chip *pwm;
> > - struct resource *res;
> > int ret;
> >
> > pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> > @@ -355,8 +354,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> > if (!pwm->data)
> > return -ENODEV;
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - pwm->base = devm_ioremap_resource(&pdev->dev, res);
> > + pwm->base = devm_platform_ioremap_resource(pdev, 0);
> > if (IS_ERR(pwm->base))
> > return PTR_ERR(pwm->base);
>
> Can you please comment why you don't apply this series?
I did in fact apply this yesterday, but I now see that I didn't reply to
the thread to report that.
> My point of view is:
>
> devm_platform_ioremap_resource is the designated wrapper to replace
> platform_get_resource() and devm_ioremap_resource(). So I don't see a
> good reason to continue open coding it.
>
> The patch series doesn't apply to 5.10-rc1 as is. (pwm-puv3 was removed
> and a simple conflict in the pwm-rockchip driver.) The overall diffstat
> (of the fixed series applied on top of 5.10-rc1) is
>
> 31 files changed, 32 insertions(+), 96 deletions(-)
>
> and it converts all of drivers/pwm but a single instance of
> platform_get_resource() + devm_ioremap_resource() (for pwm-lpss where
> platform_get_resource and devm_ioremap_resource are in different
> functions (different files even)) which isn't trivial to fix.
>
> So in my eyes applying this series is the right thing to do.
For the record, I personally think this helper is a bit over the top. I
do agree that it's nice to create helpers for common code sequences, but
this is a *lot* of churn all across the kernel to save just two lines,
which I don't think is worth it in this case. Often these helpers allow
common mistakes to be avoided while at the same time reducing lines of
code, but devm_ioremap_resource() was already created to address the
pitfalls (like returning all sorts of weird and inconsistent error
codes). So this helper doesn't actually add any value other than saving
a few lines, which I don't think justifies the churn. I would've been
sold on this if the ratio had been slightly higher, like maybe saving a
dozen or so lines, but as it is, I just don't think it's worth the churn
that it's causing.
I also think that it's overly narrow is scope, so you can't actually
"blindly" use this helper and I've seen quite a few cases where this was
unknowingly used for cases where it shouldn't have been used and then
broke things (because some drivers must not do the request_mem_region()
for example). And then there are cases where the driver needs the
resource for some other purpose, so you can't use the helper either, or
at least it looses all of its advantages in those cases.
That said, the helper is there and has been widely accepted, so my
opinion has been overruled by the majority.
Thierry
Hello Thierry,
On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote:
> I also think that it's overly narrow is scope, so you can't actually
> "blindly" use this helper and I've seen quite a few cases where this was
> unknowingly used for cases where it shouldn't have been used and then
> broke things (because some drivers must not do the request_mem_region()
> for example).
You have a link to such an accident?
> And then there are cases where the driver needs the
> resource for some other purpose, so you can't use the helper either, or
> at least it looses all of its advantages in those cases.
There is devm_platform_get_and_ioremap_resource() for (some of) these
cases.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 12/29/19 10:06 AM, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Acked-by: Vladimir Zapolskiy <[email protected]>
--
Best wishes,
Vladimir
On 12/29/19 10:05 AM, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
>
> Signed-off-by: Yangtao Li <[email protected]>
Acked-by: Vladimir Zapolskiy <[email protected]>
--
Best wishes,
Vladimir
Hello,
[Added lkml and the people involved in commit 7945f929f1a7
("drivers: provide devm_platform_ioremap_resource()") to Cc:. For the
new readers: This is about patches making use of
devm_platform_ioremap_resource() instead of open coding it. Full context
at https://lore.kernel.org/r/20201112190649.GA908613@ulmo]
On Thu, Nov 12, 2020 at 10:14:29PM +0100, Uwe Kleine-K?nig wrote:
> On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote:
> > I also think that it's overly narrow is scope, so you can't actually
> > "blindly" use this helper and I've seen quite a few cases where this was
> > unknowingly used for cases where it shouldn't have been used and then
> > broke things (because some drivers must not do the request_mem_region()
> > for example).
>
> You have a link to such an accident?
I got a hint in private here: https://lore.kernel.org/r/[email protected]
devm_platform_ioremap_resource() is platform_get_resource() +
devm_ioremap_resource() and here it was used to replace
platform_get_resource() + devm_ioremap().
IMHO the unlucky thing in this situation is that devm_ioremap_resource()
and devm_ioremap() are different by more than just how they get the area
to remap. (i.e. devm_ioremap_resource() also does
devm_request_mem_region().)
So the problem is not the added wrapper, but unclear semantics in the
functions it uses. In my eyes devm_ioremap() and
devm_platform_ioremap_resource() should better be named
devm_request_ioremap() and devm_platform_request_ioremap_resource()
respectively. Is it worth to rename these for clearity?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
here comes a patch set that implements the suggestion in the previous
mail so we have something to discuss about.
Best regards
Uwe
Uwe Kleine-König (2):
base: Rename devm_ioremap_resource to make the implicit request_mem
explicit
platform: Rename devm_platform_ioremap_resource to make the implicit
request_mem explicit
.../driver-api/driver-model/devres.rst | 6 +--
drivers/base/platform.c | 26 +++++------
include/linux/device.h | 32 ++++++++++++--
include/linux/platform_device.h | 43 +++++++++++++++++--
lib/devres.c | 18 ++++----
5 files changed, 93 insertions(+), 32 deletions(-)
base-commit: 3650b228f83adda7e5ee532e2b90429c03f7b9ec
--
2.28.0
devm_ioremap_resource() and so devm_platform_ioremap_resource() et al also
include a call to devm_request_mem(). Make this explicit in their name
to make this difference compared to devm_ioremap() more obvious.
This follows the similar rename of devm_ioremap_resource in the previous
commit.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
.../driver-api/driver-model/devres.rst | 2 +-
drivers/base/platform.c | 26 +++++------
include/linux/device.h | 3 +-
include/linux/platform_device.h | 43 +++++++++++++++++--
4 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 65f9f44d5c39..6dd5c219f11e 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -311,7 +311,7 @@ IOMAP
devm_ioremap_wc()
devm_request_ioremap_resource() : checks resource, requests memory region, ioremaps
devm_request_ioremap_resource_wc()
- devm_platform_ioremap_resource() : calls devm_ioremap_resource() for platform device
+ devm_platform_request_ioremap_resource() : calls devm_request_ioremap_resource() for platform device
devm_platform_ioremap_resource_wc()
devm_platform_ioremap_resource_byname()
devm_platform_get_and_ioremap_resource()
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 88aef93eb4dd..5a451121a9e0 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -65,8 +65,8 @@ EXPORT_SYMBOL_GPL(platform_get_resource);
#ifdef CONFIG_HAS_IOMEM
/**
- * devm_platform_get_and_ioremap_resource - call devm_ioremap_resource() for a
- * platform device and get resource
+ * devm_platform_get_request_and_ioremap_resource - call devm_ioremap_resource() for a
+ * platform device and get resource
*
* @pdev: platform device to use both for memory resource lookup as well as
* resource management
@@ -77,17 +77,17 @@ EXPORT_SYMBOL_GPL(platform_get_resource);
* on failure.
*/
void __iomem *
-devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
- unsigned int index, struct resource **res)
+devm_platform_get_request_and_ioremap_resource(struct platform_device *pdev,
+ unsigned int index, struct resource **res)
{
struct resource *r;
r = platform_get_resource(pdev, IORESOURCE_MEM, index);
if (res)
*res = r;
- return devm_ioremap_resource(&pdev->dev, r);
+ return devm_request_ioremap_resource(&pdev->dev, r);
}
-EXPORT_SYMBOL_GPL(devm_platform_get_and_ioremap_resource);
+EXPORT_SYMBOL_GPL(devm_platform_get_request_and_ioremap_resource);
/**
* devm_platform_ioremap_resource - call devm_ioremap_resource() for a platform
@@ -100,12 +100,12 @@ EXPORT_SYMBOL_GPL(devm_platform_get_and_ioremap_resource);
* Return: a pointer to the remapped memory or an ERR_PTR() encoded error code
* on failure.
*/
-void __iomem *devm_platform_ioremap_resource(struct platform_device *pdev,
- unsigned int index)
+void __iomem *devm_platform_request_ioremap_resource(struct platform_device *pdev,
+ unsigned int index)
{
- return devm_platform_get_and_ioremap_resource(pdev, index, NULL);
+ return devm_platform_get_request_and_ioremap_resource(pdev, index, NULL);
}
-EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
+EXPORT_SYMBOL_GPL(devm_platform_request_ioremap_resource);
/**
* devm_platform_ioremap_resource_wc - write-combined variant of
@@ -118,13 +118,13 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource);
* Return: a pointer to the remapped memory or an ERR_PTR() encoded error code
* on failure.
*/
-void __iomem *devm_platform_ioremap_resource_wc(struct platform_device *pdev,
- unsigned int index)
+void __iomem *devm_platform_request_ioremap_resource_wc(struct platform_device *pdev,
+ unsigned int index)
{
struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, index);
- return devm_ioremap_resource_wc(&pdev->dev, res);
+ return devm_request_ioremap_resource_wc(&pdev->dev, res);
}
/**
diff --git a/include/linux/device.h b/include/linux/device.h
index 927992549db9..3679a42f94a9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -245,7 +245,8 @@ void __iomem *devm_request_ioremap_resource(struct device *dev,
/*
* devm_ioremap_resource() was the initial name chosen for
* devm_request_ioremap_resource(). Please stick to the latter for clearer
- * semantics.
+ * semantics. When converting consider using
+ * devm_platform_request_ioremap_resource().
*/
static inline void __iomem *
devm_ioremap_resource(struct device *dev, const struct resource *res)
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 77a2aada106d..87b21f8c7daa 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -55,15 +55,52 @@ extern struct resource *platform_get_resource(struct platform_device *,
extern struct device *
platform_find_device_by_driver(struct device *start,
const struct device_driver *drv);
+
extern void __iomem *
+devm_platform_get_request_and_ioremap_resource(struct platform_device *pdev,
+ unsigned int index, struct resource **res);
+/*
+ * devm_platform_get_and_ioremap_resource() was the initial name chosen for
+ * devm_platform_get_request_and_ioremap_resource(). Please stick to the latter
+ * for clearer semantics.
+ */
+static inline void __iomem *
devm_platform_get_and_ioremap_resource(struct platform_device *pdev,
- unsigned int index, struct resource **res);
+ unsigned int index, struct resource **res)
+{
+ return devm_platform_get_request_and_ioremap_resource(pdev, index, res);
+}
+
extern void __iomem *
+devm_platform_request_ioremap_resource(struct platform_device *pdev,
+ unsigned int index);
+/*
+ * devm_platform_ioremap_resource() was the initial name chosen for
+ * devm_platform_request_ioremap_resource(). Please stick to the latter for
+ * clearer semantics.
+ */
+static inline void __iomem *
devm_platform_ioremap_resource(struct platform_device *pdev,
- unsigned int index);
+ unsigned int index)
+{
+ return devm_platform_request_ioremap_resource(pdev, index);
+}
+
extern void __iomem *
+devm_platform_request_ioremap_resource_wc(struct platform_device *pdev,
+ unsigned int index);
+/*
+ * devm_platform_ioremap_resource_wc() was the initial name chosen for
+ * devm_platform_request_ioremap_resource_wc(). Please stick to the latter for
+ * clearer semantics.
+ */
+static inline void __iomem *
devm_platform_ioremap_resource_wc(struct platform_device *pdev,
- unsigned int index);
+ unsigned int index)
+{
+ return devm_platform_request_ioremap_resource_wc(pdev, index);
+}
+
extern void __iomem *
devm_platform_ioremap_resource_byname(struct platform_device *pdev,
const char *name);
--
2.28.0
The main difference between devm_ioremap() and devm_ioremap_resource()
(apart from the different way to pass the area to map) is that the latter
also calls devm_request_mem() which is unintuitive and yields problems
like https://lore.kernel.org/r/[email protected] .
So rename devm_ioremap_resource and it's relative devm_ioremap_resource_wc
to include "request" in their name.
Until all users are converted, provide wrappers with the old name.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
.../driver-api/driver-model/devres.rst | 4 +--
include/linux/device.h | 31 ++++++++++++++++---
lib/devres.c | 18 +++++------
3 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index bb676570acc3..65f9f44d5c39 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -309,8 +309,8 @@ IOMAP
devm_ioremap()
devm_ioremap_uc()
devm_ioremap_wc()
- devm_ioremap_resource() : checks resource, requests memory region, ioremaps
- devm_ioremap_resource_wc()
+ devm_request_ioremap_resource() : checks resource, requests memory region, ioremaps
+ devm_request_ioremap_resource_wc()
devm_platform_ioremap_resource() : calls devm_ioremap_resource() for platform device
devm_platform_ioremap_resource_wc()
devm_platform_ioremap_resource_byname()
diff --git a/include/linux/device.h b/include/linux/device.h
index 5ed101be7b2e..927992549db9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -240,10 +240,33 @@ unsigned long devm_get_free_pages(struct device *dev,
gfp_t gfp_mask, unsigned int order);
void devm_free_pages(struct device *dev, unsigned long addr);
-void __iomem *devm_ioremap_resource(struct device *dev,
- const struct resource *res);
-void __iomem *devm_ioremap_resource_wc(struct device *dev,
- const struct resource *res);
+void __iomem *devm_request_ioremap_resource(struct device *dev,
+ const struct resource *res);
+/*
+ * devm_ioremap_resource() was the initial name chosen for
+ * devm_request_ioremap_resource(). Please stick to the latter for clearer
+ * semantics.
+ */
+static inline void __iomem *
+devm_ioremap_resource(struct device *dev, const struct resource *res)
+{
+ return devm_request_ioremap_resource(dev, res);
+}
+
+void __iomem *devm_request_ioremap_resource_wc(struct device *dev,
+ const struct resource *res);
+
+/*
+ * devm_ioremap_resource_wc() was the initial name chosen for
+ * devm_request_ioremap_resource_wc(). Please stick to the latter for clearer
+ * semantics.
+ */
+static inline void __iomem *
+devm_ioremap_resource_wc(struct device *dev,
+ const struct resource *res)
+{
+ return devm_request_ioremap_resource_wc(dev, res);
+}
void __iomem *devm_of_iomap(struct device *dev,
struct device_node *node, int index,
diff --git a/lib/devres.c b/lib/devres.c
index 2a4ff5d64288..907588f1a5b7 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -114,8 +114,8 @@ void devm_iounmap(struct device *dev, void __iomem *addr)
EXPORT_SYMBOL(devm_iounmap);
static void __iomem *
-__devm_ioremap_resource(struct device *dev, const struct resource *res,
- enum devm_ioremap_type type)
+__devm_request_ioremap_resource(struct device *dev, const struct resource *res,
+ enum devm_ioremap_type type)
{
resource_size_t size;
void __iomem *dest_ptr;
@@ -172,12 +172,12 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res,
* Return: a pointer to the remapped memory or an ERR_PTR() encoded error code
* on failure.
*/
-void __iomem *devm_ioremap_resource(struct device *dev,
- const struct resource *res)
+void __iomem *devm_request_ioremap_resource(struct device *dev,
+ const struct resource *res)
{
- return __devm_ioremap_resource(dev, res, DEVM_IOREMAP);
+ return __devm_request_ioremap_resource(dev, res, DEVM_IOREMAP);
}
-EXPORT_SYMBOL(devm_ioremap_resource);
+EXPORT_SYMBOL(devm_request_ioremap_resource);
/**
* devm_ioremap_resource_wc() - write-combined variant of
@@ -188,10 +188,10 @@ EXPORT_SYMBOL(devm_ioremap_resource);
* Return: a pointer to the remapped memory or an ERR_PTR() encoded error code
* on failure.
*/
-void __iomem *devm_ioremap_resource_wc(struct device *dev,
- const struct resource *res)
+void __iomem *devm_request_ioremap_resource_wc(struct device *dev,
+ const struct resource *res)
{
- return __devm_ioremap_resource(dev, res, DEVM_IOREMAP_WC);
+ return __devm_request_ioremap_resource(dev, res, DEVM_IOREMAP_WC);
}
/*
--
2.28.0
On Fri, Nov 13, 2020 at 8:04 AM Uwe Kleine-König
<[email protected]> wrote:
>
> Hello,
>
> [Added lkml and the people involved in commit 7945f929f1a7
> ("drivers: provide devm_platform_ioremap_resource()") to Cc:. For the
> new readers: This is about patches making use of
> devm_platform_ioremap_resource() instead of open coding it. Full context
> at https://lore.kernel.org/r/20201112190649.GA908613@ulmo]
>
> On Thu, Nov 12, 2020 at 10:14:29PM +0100, Uwe Kleine-König wrote:
> > On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote:
> > > I also think that it's overly narrow is scope, so you can't actually
> > > "blindly" use this helper and I've seen quite a few cases where this was
> > > unknowingly used for cases where it shouldn't have been used and then
> > > broke things (because some drivers must not do the request_mem_region()
> > > for example).
> >
> > You have a link to such an accident?
>
> I got a hint in private here: https://lore.kernel.org/r/[email protected]
>
> devm_platform_ioremap_resource() is platform_get_resource() +
> devm_ioremap_resource() and here it was used to replace
> platform_get_resource() + devm_ioremap().
>
> IMHO the unlucky thing in this situation is that devm_ioremap_resource()
> and devm_ioremap() are different by more than just how they get the area
> to remap. (i.e. devm_ioremap_resource() also does
> devm_request_mem_region().)
>
> So the problem is not the added wrapper, but unclear semantics in the
> functions it uses. In my eyes devm_ioremap() and
> devm_platform_ioremap_resource() should better be named
> devm_request_ioremap() and devm_platform_request_ioremap_resource()
> respectively. Is it worth to rename these for clearity?
But devm_ioremap() doesn't request the region. Did you mean
devm_ioremap_resource() should become devm_request_ioremap_resource()?
Bartosz
Signed-off-by: Uwe Kleine-König <[email protected]>
---
Hello,
this can also be squashed into the respective patches instead.
Best regards
Uwe
scripts/checkpatch.pl | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..5abb87256d4c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -615,6 +615,11 @@ our %deprecated_apis = (
"rcu_barrier_sched" => "rcu_barrier",
"get_state_synchronize_sched" => "get_state_synchronize_rcu",
"cond_synchronize_sched" => "cond_synchronize_rcu",
+ "devm_platform_get_and_ioremap_resource" => "devm_platform_get_request_and_ioremap_resource",
+ "devm_platform_ioremap_resource" => "devm_platform_request_ioremap_resource",
+ "devm_platform_ioremap_resource_wc" => "devm_platform_request_ioremap_resource_wc",
+ "devm_ioremap_resource" => "devm_request_ioremap_resource",
+ "devm_ioremap_resource_wc" => "devm_request_ioremap_resource_wc",
);
#Create a search pattern for all these strings to speed up a loop below
--
2.28.0
On Fri, Nov 13, 2020 at 10:12:46AM +0100, Bartosz Golaszewski wrote:
> On Fri, Nov 13, 2020 at 8:04 AM Uwe Kleine-K?nig
> <[email protected]> wrote:
> >
> > Hello,
> >
> > [Added lkml and the people involved in commit 7945f929f1a7
> > ("drivers: provide devm_platform_ioremap_resource()") to Cc:. For the
> > new readers: This is about patches making use of
> > devm_platform_ioremap_resource() instead of open coding it. Full context
> > at https://lore.kernel.org/r/20201112190649.GA908613@ulmo]
> >
> > On Thu, Nov 12, 2020 at 10:14:29PM +0100, Uwe Kleine-K?nig wrote:
> > > On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote:
> > > > I also think that it's overly narrow is scope, so you can't actually
> > > > "blindly" use this helper and I've seen quite a few cases where this was
> > > > unknowingly used for cases where it shouldn't have been used and then
> > > > broke things (because some drivers must not do the request_mem_region()
> > > > for example).
> > >
> > > You have a link to such an accident?
> >
> > I got a hint in private here: https://lore.kernel.org/r/[email protected]
> >
> > devm_platform_ioremap_resource() is platform_get_resource() +
> > devm_ioremap_resource() and here it was used to replace
> > platform_get_resource() + devm_ioremap().
> >
> > IMHO the unlucky thing in this situation is that devm_ioremap_resource()
> > and devm_ioremap() are different by more than just how they get the area
> > to remap. (i.e. devm_ioremap_resource() also does
> > devm_request_mem_region().)
> >
> > So the problem is not the added wrapper, but unclear semantics in the
> > functions it uses. In my eyes devm_ioremap() and
> > devm_platform_ioremap_resource() should better be named
> > devm_request_ioremap() and devm_platform_request_ioremap_resource()
> > respectively. Is it worth to rename these for clearity?
>
> But devm_ioremap() doesn't request the region. Did you mean
> devm_ioremap_resource() should become devm_request_ioremap_resource()?
Yes indeed. The last paragraph should be:
So the problem is not the added wrapper, but unclear semantics in the
functions it uses. In my eyes devm_ioremap_resource() and
devm_platform_ioremap_resource() should better be named
devm_request_ioremap_resource() and
devm_platform_request_ioremap_resource().
(Note that I created a patch series that implements this suggestion, but
you were not on Cc: as I extensively trimmed the recipents assuming most
people are not interested. See https://lore.kernel.org/r/[email protected])
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Fri, Nov 13, 2020 at 10:11:57AM +0100, Uwe Kleine-K?nig wrote:
> Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> ---
> Hello,
>
> this can also be squashed into the respective patches instead.
Hm, how? Please just resend the series, or just provide a changelog
text for this patch and I'll be glad to take it that way.
thanks,
greg k-h
On Fri, Nov 13, 2020 at 09:53:25AM +0100, Uwe Kleine-König wrote:
> Hello,
>
> here comes a patch set that implements the suggestion in the previous
> mail so we have something to discuss about.
>
> Best regards
> Uwe
>
> Uwe Kleine-König (2):
> base: Rename devm_ioremap_resource to make the implicit request_mem
> explicit
> platform: Rename devm_platform_ioremap_resource to make the implicit
> request_mem explicit
>
> .../driver-api/driver-model/devres.rst | 6 +--
> drivers/base/platform.c | 26 +++++------
> include/linux/device.h | 32 ++++++++++++--
> include/linux/platform_device.h | 43 +++++++++++++++++--
> lib/devres.c | 18 ++++----
> 5 files changed, 93 insertions(+), 32 deletions(-)
To be honest, this is getting a bit eccentric for my taste. Yes,
devm_ioremap_resource() does more than just devm_ioremap(), but that's
why it's called devm_ioremap_resource(). It's a compromise between the
new functionality and practical symbol length.
The kerneldoc for devm_ioremap_resource() is very explicit about what it
does, so I think it's fine to omit some of the details from the symbol
name for the sake of brevity.
Things get out of hand pretty quickly if we start incorporating every
single aspect of what a function does into its name.
Thierry
On Fri, Nov 13, 2020 at 08:03:43AM +0100, Uwe Kleine-König wrote:
> Hello,
>
> [Added lkml and the people involved in commit 7945f929f1a7
> ("drivers: provide devm_platform_ioremap_resource()") to Cc:. For the
> new readers: This is about patches making use of
> devm_platform_ioremap_resource() instead of open coding it. Full context
> at https://lore.kernel.org/r/20201112190649.GA908613@ulmo]
>
> On Thu, Nov 12, 2020 at 10:14:29PM +0100, Uwe Kleine-König wrote:
> > On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote:
> > > I also think that it's overly narrow is scope, so you can't actually
> > > "blindly" use this helper and I've seen quite a few cases where this was
> > > unknowingly used for cases where it shouldn't have been used and then
> > > broke things (because some drivers must not do the request_mem_region()
> > > for example).
> >
> > You have a link to such an accident?
>
> I got a hint in private here: https://lore.kernel.org/r/[email protected]
>
> devm_platform_ioremap_resource() is platform_get_resource() +
> devm_ioremap_resource() and here it was used to replace
> platform_get_resource() + devm_ioremap().
>
> IMHO the unlucky thing in this situation is that devm_ioremap_resource()
> and devm_ioremap() are different by more than just how they get the area
> to remap. (i.e. devm_ioremap_resource() also does
> devm_request_mem_region().)
>
> So the problem is not the added wrapper, but unclear semantics in the
> functions it uses.
The semantics aren't unclear. It's just that the symbol name doesn't
spell out every detail that the function implements, which, frankly, no
function name ever does, at least not for anything beyond simple
instructional examples. That's what we have documentation for and why
people should read the documentation before they use a function and make
(potentially wrong) assumption about what it does.
> In my eyes devm_ioremap() and
> devm_platform_ioremap_resource() should better be named
> devm_request_ioremap() and devm_platform_request_ioremap_resource()
> respectively. Is it worth to rename these for clearity?
I think function names are always a compromise between giving you the
gist of what the implementation does and being short enough so it
doesn't become difficult to read or use.
One of the reasons why I dislike the addition of helpers for every
common special case (like devm_platform_ioremap_resource()) is because
it doesn't (always) actually make things easier for developers and/or
maintainers. Replacing three lines of code with one is a minor
improvement, even though there may be many callsites and therefore in
the sum this being a fairly sizeable reduction. The flip side is that
now we've got an extra symbol with an unwieldy name that people need
to become familiar with, and then, like the link above shows, it doesn't
work in all cases, so you either need to fall back to the open-coded
version or you keep adding helpers until you've covered all cases. And
then we end up with a bunch of helpers that you actually have to go and
read the documentation for in order to find out which one exactly fits
your use-case.
Without the helpers it's pretty simple to write, even if a little
repetitive:
1) get the resource you want to map
2) request the resource
3) map the resource
2) & 3) are very commonly done together, so it makes sense to have a
generic helper for them. If you look at the implementation, the
devm_ioremap_request() implementation does quite a bit of things in
addition to just requesting and remapping, and that's the reason why
that helper makes sense.
For me personally, devm_platform_ioremap_resource() is just not adding
enough value to justify its existence. And then we get all these other
variants that operate on the resource name (_byname) and those which
remap write-combined (_wc). But don't we also need a _byname_wc()
variant for the combination? Where does it stop?
Thierry
On Fri, 2020-11-13 at 10:11 +0100, Uwe Kleine-K?nig wrote:
> Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> ---
> Hello,
>
> this can also be squashed into the respective patches instead.
>
> Best regards
> Uwe
>
> ?scripts/checkpatch.pl | 5 +++++
> ?1 file changed, 5 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -615,6 +615,11 @@ our %deprecated_apis = (
> ? "rcu_barrier_sched" => "rcu_barrier",
> ? "get_state_synchronize_sched" => "get_state_synchronize_rcu",
> ? "cond_synchronize_sched" => "cond_synchronize_rcu",
> + "devm_platform_get_and_ioremap_resource" => "devm_platform_get_request_and_ioremap_resource",
Do we really need 46 character length function names?
> + "devm_platform_ioremap_resource" => "devm_platform_request_ioremap_resource",
> + "devm_platform_ioremap_resource_wc" => "devm_platform_request_ioremap_resource_wc",
> + "devm_ioremap_resource" => "devm_request_ioremap_resource",
> + "devm_ioremap_resource_wc" => "devm_request_ioremap_resource_wc",
> ?);
> ?
>
> ?#Create a search pattern for all these strings to speed up a loop below
And do please send your proposed patches to the appropriate maintainers.
On Fri, Nov 13, 2020 at 08:36:44AM -0800, Joe Perches wrote:
> On Fri, 2020-11-13 at 10:11 +0100, Uwe Kleine-K?nig wrote:
> > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> > ---
> > Hello,
> >
> > this can also be squashed into the respective patches instead.
> >
> > Best regards
> > Uwe
> >
> > ?scripts/checkpatch.pl | 5 +++++
> > ?1 file changed, 5 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -615,6 +615,11 @@ our %deprecated_apis = (
> > ? "rcu_barrier_sched" => "rcu_barrier",
> > ? "get_state_synchronize_sched" => "get_state_synchronize_rcu",
> > ? "cond_synchronize_sched" => "cond_synchronize_rcu",
> > + "devm_platform_get_and_ioremap_resource" => "devm_platform_get_request_and_ioremap_resource",
>
> Do we really need 46 character length function names?
I can drop the "_and" and maybe "_get", so we're down to 38 "only".
Other than that I think all name parts are relevant.
> > + "devm_platform_ioremap_resource" => "devm_platform_request_ioremap_resource",
> > + "devm_platform_ioremap_resource_wc" => "devm_platform_request_ioremap_resource_wc",
> > + "devm_ioremap_resource" => "devm_request_ioremap_resource",
> > + "devm_ioremap_resource_wc" => "devm_request_ioremap_resource_wc",
> > ?);
> > ?
> >
> > ?#Create a search pattern for all these strings to speed up a loop below
>
> And do please send your proposed patches to the appropriate maintainers.
Yes, sure. This patch 3/2 was only a quick shot and it was already clear
to me that I have to redo it. I want to squash this change in the patch
that does the actual renaming, I assume that's fine for you?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Fri, 2020-11-13 at 18:00 +0100, Uwe Kleine-K?nig wrote:
> On Fri, Nov 13, 2020 at 08:36:44AM -0800, Joe Perches wrote:
> > On Fri, 2020-11-13 at 10:11 +0100, Uwe Kleine-K?nig wrote:
> > > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> > > ---
> > > Hello,
> > >
> > > this can also be squashed into the respective patches instead.
> > >
> > > Best regards
> > > Uwe
> > >
> > > ?scripts/checkpatch.pl | 5 +++++
> > > ?1 file changed, 5 insertions(+)
> > >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -615,6 +615,11 @@ our %deprecated_apis = (
> > > ? "rcu_barrier_sched" => "rcu_barrier",
> > > ? "get_state_synchronize_sched" => "get_state_synchronize_rcu",
> > > ? "cond_synchronize_sched" => "cond_synchronize_rcu",
> > > + "devm_platform_get_and_ioremap_resource" => "devm_platform_get_request_and_ioremap_resource",
> >
> > Do we really need 46 character length function names?
>
> I can drop the "_and" and maybe "_get", so we're down to 38 "only".
> Other than that I think all name parts are relevant.
>
> > > + "devm_platform_ioremap_resource" => "devm_platform_request_ioremap_resource",
> > > + "devm_platform_ioremap_resource_wc" => "devm_platform_request_ioremap_resource_wc",
> > > + "devm_ioremap_resource" => "devm_request_ioremap_resource",
> > > + "devm_ioremap_resource_wc" => "devm_request_ioremap_resource_wc",
> > > ?);
> > > ?
> > >
> > > ?#Create a search pattern for all these strings to speed up a loop below
> >
> > And do please send your proposed patches to the appropriate maintainers.
>
> Yes, sure. This patch 3/2 was only a quick shot and it was already clear
> to me that I have to redo it. I want to squash this change in the patch
> that does the actual renaming, I assume that's fine for you?!
Sure.
But please do take Thierry Reding's comment about overall
API complexity into account.
All wrapper macro/functions aren't always obviously good.
They can be useful, but can make knowing which of many
possible wrappers to use and when to use them appropriately
difficult.
Wrappers also add complexity to documentation.
On 2020-11-13 16:11, Thierry Reding wrote:
> On Fri, Nov 13, 2020 at 08:03:43AM +0100, Uwe Kleine-K?nig wrote:
>> Hello,
>>
>> [Added lkml and the people involved in commit 7945f929f1a7
>> ("drivers: provide devm_platform_ioremap_resource()") to Cc:. For the
>> new readers: This is about patches making use of
>> devm_platform_ioremap_resource() instead of open coding it. Full context
>> at https://lore.kernel.org/r/20201112190649.GA908613@ulmo]
>>
>> On Thu, Nov 12, 2020 at 10:14:29PM +0100, Uwe Kleine-K?nig wrote:
>>> On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote:
>>>> I also think that it's overly narrow is scope, so you can't actually
>>>> "blindly" use this helper and I've seen quite a few cases where this was
>>>> unknowingly used for cases where it shouldn't have been used and then
>>>> broke things (because some drivers must not do the request_mem_region()
>>>> for example).
>>>
>>> You have a link to such an accident?
>>
>> I got a hint in private here: https://lore.kernel.org/r/[email protected]
>>
>> devm_platform_ioremap_resource() is platform_get_resource() +
>> devm_ioremap_resource() and here it was used to replace
>> platform_get_resource() + devm_ioremap().
>>
>> IMHO the unlucky thing in this situation is that devm_ioremap_resource()
>> and devm_ioremap() are different by more than just how they get the area
>> to remap. (i.e. devm_ioremap_resource() also does
>> devm_request_mem_region().)
>>
>> So the problem is not the added wrapper, but unclear semantics in the
>> functions it uses.
>
> The semantics aren't unclear. It's just that the symbol name doesn't
> spell out every detail that the function implements, which, frankly, no
> function name ever does, at least not for anything beyond simple
> instructional examples. That's what we have documentation for and why
> people should read the documentation before they use a function and make
> (potentially wrong) assumption about what it does.
>
>> In my eyes devm_ioremap() and
>> devm_platform_ioremap_resource() should better be named
>> devm_request_ioremap() and devm_platform_request_ioremap_resource()
>> respectively. Is it worth to rename these for clearity?
>
> I think function names are always a compromise between giving you the
> gist of what the implementation does and being short enough so it
> doesn't become difficult to read or use.
>
> One of the reasons why I dislike the addition of helpers for every
> common special case (like devm_platform_ioremap_resource()) is because
> it doesn't (always) actually make things easier for developers and/or
> maintainers. Replacing three lines of code with one is a minor
> improvement, even though there may be many callsites and therefore in
> the sum this being a fairly sizeable reduction. The flip side is that
> now we've got an extra symbol with an unwieldy name that people need
> to become familiar with, and then, like the link above shows, it doesn't
> work in all cases, so you either need to fall back to the open-coded
> version or you keep adding helpers until you've covered all cases. And
> then we end up with a bunch of helpers that you actually have to go and
> read the documentation for in order to find out which one exactly fits
> your use-case.
>
> Without the helpers it's pretty simple to write, even if a little
> repetitive:
>
> 1) get the resource you want to map
> 2) request the resource
> 3) map the resource
>
> 2) & 3) are very commonly done together, so it makes sense to have a
> generic helper for them. If you look at the implementation, the
> devm_ioremap_request() implementation does quite a bit of things in
> addition to just requesting and remapping, and that's the reason why
> that helper makes sense.
>
> For me personally, devm_platform_ioremap_resource() is just not adding
> enough value to justify its existence. And then we get all these other
> variants that operate on the resource name (_byname) and those which
> remap write-combined (_wc). But don't we also need a _byname_wc()
> variant for the combination? Where does it stop?
Arguably the worst thing about devm_platform_ioremap_resource() is that
it was apparently the gateway drug to a belief that
devm_platform_get_and_ioremap_resource() is anything other than a
hideous way to obfuscate an assignment...
Robin.
Hello,
On Fri, Nov 13, 2020 at 05:11:53PM +0100, Thierry Reding wrote:
> I think function names are always a compromise between giving you the
> gist of what the implementation does and being short enough so it
> doesn't become difficult to read or use.
Right. In my eyes if you have
- devm_platform_ioremap_resource
- devm_platform_get_and_ioremap_resource
- devm_ioremap_resource
- devm_ioremap
(to list just a few) with the current semantics, the compromise is badly
shifted into the "short name" direction however; and that was the
motivation for this patch set. In my eyes it must be more obvious which
of these functions include devm_request_mem_region() and which don't.
And note, my patch series doesn't introduce new helpers, just renames
them to have a better name (and adds compat glue for the old names).
> One of the reasons why I dislike the addition of helpers for every
> common special case (like devm_platform_ioremap_resource()) is because
> it doesn't (always) actually make things easier for developers and/or
> maintainers. Replacing three lines of code with one is a minor
> improvement, even though there may be many callsites and therefore in
> the sum this being a fairly sizeable reduction. The flip side is that
> now we've got an extra symbol with an unwieldy name that people need
> to become familiar with, and then, like the link above shows, it doesn't
> work in all cases, so you either need to fall back to the open-coded
> version or you keep adding helpers until you've covered all cases. And
> then we end up with a bunch of helpers that you actually have to go and
> read the documentation for in order to find out which one exactly fits
> your use-case.
This is indeed a relevant point. An alternative is to make the helper
more flexible. This complicates the API, too, however, so this isn't
always gold, either.
> Without the helpers it's pretty simple to write, even if a little
> repetitive:
>
> 1) get the resource you want to map
> 2) request the resource
> 3) map the resource
>
> 2) & 3) are very commonly done together, so it makes sense to have a
> generic helper for them. If you look at the implementation, the
> devm_ioremap_request() implementation does quite a bit of things in
> addition to just requesting and remapping, and that's the reason why
> that helper makes sense.
>
> For me personally, devm_platform_ioremap_resource() is just not adding
> enough value to justify its existence. And then we get all these other
> variants that operate on the resource name (_byname) and those which
> remap write-combined (_wc). But don't we also need a _byname_wc()
> variant for the combination? Where does it stop?
I'm on your side for the _wc stuff, looking at next-20201119:
- devm_ioremap_resource_wc has a single user:
devm_platform_ioremap_resource_wc
- devm_platform_ioremap_resource_wc has a single user:
drivers/misc/sram.c
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |