Hi All,
This is just a minor change to promote using of this api
devm_platform_ioremap_resource() and nothing else functional
change it does.
Also, there are patches where only ioremap is used without requesting
the mem region so this devm_platform_ioremap_resource() take cares all
of that in single call.
Mukesh Ojha (7):
irqchip/irq-uniphier-aidet: Use devm_platform_ioremap_resource()
irqchip/irq-imgpdc.c: Use devm_platform_ioremap_resource()
irqchip/irq-ts4800.c: Use devm_platform_ioremap_resource()
irqchip/irq-mbigen.c: Use devm_platform_ioremap_resource()
irqchip/irq-mvebu-icu: Use devm_platform_ioremap_resource()
irqchip/irq-mvebu-pic: Use devm_platform_ioremap_resource()
irqchip/irq-mvebu-sei: Use devm_platform_ioremap_resource()
drivers/irqchip/irq-imgpdc.c | 10 +---------
drivers/irqchip/irq-ls-scfg-msi.c | 4 +---
drivers/irqchip/irq-mbigen.c | 9 +--------
drivers/irqchip/irq-mvebu-icu.c | 4 +---
drivers/irqchip/irq-mvebu-pic.c | 4 +---
drivers/irqchip/irq-mvebu-sei.c | 3 +--
drivers/irqchip/irq-ts4800.c | 4 +---
drivers/irqchip/irq-uniphier-aidet.c | 4 +---
8 files changed, 8 insertions(+), 34 deletions(-)
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-uniphier-aidet.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-uniphier-aidet.c b/drivers/irqchip/irq-uniphier-aidet.c
index 7ba7f25..1a77268 100644
--- a/drivers/irqchip/irq-uniphier-aidet.c
+++ b/drivers/irqchip/irq-uniphier-aidet.c
@@ -174,7 +174,6 @@ static int uniphier_aidet_probe(struct platform_device *pdev)
struct device_node *parent_np;
struct irq_domain *parent_domain;
struct uniphier_aidet_priv *priv;
- struct resource *res;
parent_np = of_irq_find_parent(dev->of_node);
if (!parent_np)
@@ -189,8 +188,7 @@ static int uniphier_aidet_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- priv->reg_base = devm_ioremap_resource(dev, res);
+ priv->reg_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(priv->reg_base))
return PTR_ERR(priv->reg_base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-imgpdc.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c
index d00489a..8904a5f 100644
--- a/drivers/irqchip/irq-imgpdc.c
+++ b/drivers/irqchip/irq-imgpdc.c
@@ -307,13 +307,6 @@ static int pdc_intc_probe(struct platform_device *pdev)
if (!node)
return -ENOENT;
- /* Get registers */
- res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res_regs == NULL) {
- dev_err(&pdev->dev, "cannot find registers resource\n");
- return -ENOENT;
- }
-
/* Allocate driver data */
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
@@ -324,8 +317,7 @@ static int pdc_intc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, priv);
/* Ioremap the registers */
- priv->pdc_base = devm_ioremap(&pdev->dev, res_regs->start,
- resource_size(res_regs));
+ priv->pdc_base = devm_platform_ioremap_resource(pdev, 0);
if (!priv->pdc_base)
return -EIO;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-ts4800.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-ts4800.c b/drivers/irqchip/irq-ts4800.c
index 2325fb3..589a8a1 100644
--- a/drivers/irqchip/irq-ts4800.c
+++ b/drivers/irqchip/irq-ts4800.c
@@ -94,15 +94,13 @@ static int ts4800_ic_probe(struct platform_device *pdev)
struct device_node *node = pdev->dev.of_node;
struct ts4800_irq_data *data;
struct irq_chip *irq_chip;
- struct resource *res;
int parent_irq;
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->base = devm_ioremap_resource(&pdev->dev, res);
+ data->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(data->base))
return PTR_ERR(data->base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-mvebu-pic.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-mvebu-pic.c b/drivers/irqchip/irq-mvebu-pic.c
index eec6395..d6ade62 100644
--- a/drivers/irqchip/irq-mvebu-pic.c
+++ b/drivers/irqchip/irq-mvebu-pic.c
@@ -124,14 +124,12 @@ static int mvebu_pic_probe(struct platform_device *pdev)
struct device_node *node = pdev->dev.of_node;
struct mvebu_pic *pic;
struct irq_chip *irq_chip;
- struct resource *res;
pic = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_pic), GFP_KERNEL);
if (!pic)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pic->base = devm_ioremap_resource(&pdev->dev, res);
+ pic->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pic->base))
return PTR_ERR(pic->base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-mvebu-icu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-mvebu-icu.c b/drivers/irqchip/irq-mvebu-icu.c
index 547045d..a54e4cd 100644
--- a/drivers/irqchip/irq-mvebu-icu.c
+++ b/drivers/irqchip/irq-mvebu-icu.c
@@ -347,7 +347,6 @@ static int mvebu_icu_subset_probe(struct platform_device *pdev)
static int mvebu_icu_probe(struct platform_device *pdev)
{
struct mvebu_icu *icu;
- struct resource *res;
int i;
icu = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_icu),
@@ -357,8 +356,7 @@ static int mvebu_icu_probe(struct platform_device *pdev)
icu->dev = &pdev->dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- icu->base = devm_ioremap_resource(&pdev->dev, res);
+ icu->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(icu->base)) {
dev_err(&pdev->dev, "Failed to map icu base address.\n");
return PTR_ERR(icu->base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-ls-scfg-msi.c | 4 +---
drivers/irqchip/irq-mbigen.c | 9 +--------
2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
index c671b32..aec5a2f 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -336,7 +336,6 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
{
const struct of_device_id *match;
struct ls_scfg_msi *msi_data;
- struct resource *res;
int i, ret;
match = of_match_device(ls_scfg_msi_id, &pdev->dev);
@@ -349,8 +348,7 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
msi_data->cfg = (struct ls_scfg_msi_cfg *) match->data;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
+ msi_data->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(msi_data->regs)) {
dev_err(&pdev->dev, "failed to initialize 'regs'\n");
return PTR_ERR(msi_data->regs);
diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 98b6e1d..c9aedfb 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -327,7 +327,6 @@ static inline int mbigen_acpi_create_domain(struct platform_device *pdev,
static int mbigen_device_probe(struct platform_device *pdev)
{
struct mbigen_device *mgn_chip;
- struct resource *res;
int err;
mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
@@ -335,13 +334,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
return -ENOMEM;
mgn_chip->pdev = pdev;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -EINVAL;
-
- mgn_chip->base = devm_ioremap(&pdev->dev, res->start,
- resource_size(res));
+ mgn_chip->base = devm_platform_ioremap_resource(pdev, 0);
if (!mgn_chip->base) {
dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
return -ENOMEM;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
devm_platform_ioremap_resource() internally have platform_get_resource()
and devm_ioremap_resource() in it. So instead of calling them separately
use devm_platform_ioremap_resource() directly.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/irqchip/irq-mvebu-sei.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-mvebu-sei.c b/drivers/irqchip/irq-mvebu-sei.c
index 18832cc..0a09dcc 100644
--- a/drivers/irqchip/irq-mvebu-sei.c
+++ b/drivers/irqchip/irq-mvebu-sei.c
@@ -382,8 +382,7 @@ static int mvebu_sei_probe(struct platform_device *pdev)
mutex_init(&sei->cp_msi_lock);
raw_spin_lock_init(&sei->mask_lock);
- sei->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- sei->base = devm_ioremap_resource(sei->dev, sei->res);
+ sei->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(sei->base)) {
dev_err(sei->dev, "Failed to remap SEI resource\n");
return PTR_ERR(sei->base);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
On 04/04/2019 08:20, Mukesh Ojha wrote:
> devm_platform_ioremap_resource() internally have platform_get_resource()
> and devm_ioremap_resource() in it. So instead of calling them separately
> use devm_platform_ioremap_resource() directly.
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
> drivers/irqchip/irq-imgpdc.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c
> index d00489a..8904a5f 100644
> --- a/drivers/irqchip/irq-imgpdc.c
> +++ b/drivers/irqchip/irq-imgpdc.c
> @@ -307,13 +307,6 @@ static int pdc_intc_probe(struct platform_device *pdev)
> if (!node)
> return -ENOENT;
>
> - /* Get registers */
> - res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (res_regs == NULL) {
> - dev_err(&pdev->dev, "cannot find registers resource\n");
> - return -ENOENT;
> - }
> -
> /* Allocate driver data */
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> if (!priv) {
> @@ -324,8 +317,7 @@ static int pdc_intc_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, priv);
>
> /* Ioremap the registers */
> - priv->pdc_base = devm_ioremap(&pdev->dev, res_regs->start,
> - resource_size(res_regs));
> + priv->pdc_base = devm_platform_ioremap_resource(pdev, 0);
> if (!priv->pdc_base)
> return -EIO;
>
>
What happens to the res_regs variable then?
Also, and more importantly, devm_platform_ioremap_resource doesn't
return NULL on error, but an ERR_PTR. Yes, the bug was already there,
but since you're changing it, you might as well fix the thing.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 04/04/2019 08:20, Mukesh Ojha wrote:
> devm_platform_ioremap_resource() internally have platform_get_resource()
> and devm_ioremap_resource() in it. So instead of calling them separately
> use devm_platform_ioremap_resource() directly.
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
> drivers/irqchip/irq-ls-scfg-msi.c | 4 +---
> drivers/irqchip/irq-mbigen.c | 9 +--------
> 2 files changed, 2 insertions(+), 11 deletions(-)
You're changing two drivers, and yet only mention one. Bad.
>
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
> index c671b32..aec5a2f 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -336,7 +336,6 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
> {
> const struct of_device_id *match;
> struct ls_scfg_msi *msi_data;
> - struct resource *res;
> int i, ret;
>
> match = of_match_device(ls_scfg_msi_id, &pdev->dev);
> @@ -349,8 +348,7 @@ static int ls_scfg_msi_probe(struct platform_device *pdev)
>
> msi_data->cfg = (struct ls_scfg_msi_cfg *) match->data;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - msi_data->regs = devm_ioremap_resource(&pdev->dev, res);
> + msi_data->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(msi_data->regs)) {
> dev_err(&pdev->dev, "failed to initialize 'regs'\n");
> return PTR_ERR(msi_data->regs);
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 98b6e1d..c9aedfb 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -327,7 +327,6 @@ static inline int mbigen_acpi_create_domain(struct platform_device *pdev,
> static int mbigen_device_probe(struct platform_device *pdev)
> {
> struct mbigen_device *mgn_chip;
> - struct resource *res;
> int err;
>
> mgn_chip = devm_kzalloc(&pdev->dev, sizeof(*mgn_chip), GFP_KERNEL);
> @@ -335,13 +334,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> mgn_chip->pdev = pdev;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res)
> - return -EINVAL;
> -
> - mgn_chip->base = devm_ioremap(&pdev->dev, res->start,
> - resource_size(res));
> + mgn_chip->base = devm_platform_ioremap_resource(pdev, 0);
> if (!mgn_chip->base) {
Now you are introducing a bug, as devm_ioremap and
devm_platform_ioremap_resource do not return the same thing.
> dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
> return -ENOMEM;
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 04/04/2019 08:20, Mukesh Ojha wrote:
> devm_platform_ioremap_resource() internally have platform_get_resource()
> and devm_ioremap_resource() in it. So instead of calling them separately
> use devm_platform_ioremap_resource() directly.
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
> drivers/irqchip/irq-mvebu-sei.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-mvebu-sei.c b/drivers/irqchip/irq-mvebu-sei.c
> index 18832cc..0a09dcc 100644
> --- a/drivers/irqchip/irq-mvebu-sei.c
> +++ b/drivers/irqchip/irq-mvebu-sei.c
> @@ -382,8 +382,7 @@ static int mvebu_sei_probe(struct platform_device *pdev)
> mutex_init(&sei->cp_msi_lock);
> raw_spin_lock_init(&sei->mask_lock);
>
> - sei->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - sei->base = devm_ioremap_resource(sei->dev, sei->res);
> + sei->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(sei->base)) {
> dev_err(sei->dev, "Failed to remap SEI resource\n");
> return PTR_ERR(sei->base);
>
Again, what about sei->res?
M.
--
Jazz is not dead. It just smells funny...
On 29/04/2019 09:13, Marc Zyngier wrote:
> On 04/04/2019 08:20, Mukesh Ojha wrote:
>> devm_platform_ioremap_resource() internally have platform_get_resource()
>> and devm_ioremap_resource() in it. So instead of calling them separately
>> use devm_platform_ioremap_resource() directly.
>>
>> Signed-off-by: Mukesh Ojha <[email protected]>
>> ---
>> drivers/irqchip/irq-imgpdc.c | 10 +---------
>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c
>> index d00489a..8904a5f 100644
>> --- a/drivers/irqchip/irq-imgpdc.c
>> +++ b/drivers/irqchip/irq-imgpdc.c
>> @@ -307,13 +307,6 @@ static int pdc_intc_probe(struct platform_device *pdev)
>> if (!node)
>> return -ENOENT;
>>
>> - /* Get registers */
>> - res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - if (res_regs == NULL) {
>> - dev_err(&pdev->dev, "cannot find registers resource\n");
>> - return -ENOENT;
>> - }
>> -
>> /* Allocate driver data */
>> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> if (!priv) {
>> @@ -324,8 +317,7 @@ static int pdc_intc_probe(struct platform_device *pdev)
>> platform_set_drvdata(pdev, priv);
>>
>> /* Ioremap the registers */
>> - priv->pdc_base = devm_ioremap(&pdev->dev, res_regs->start,
>> - resource_size(res_regs));
>> + priv->pdc_base = devm_platform_ioremap_resource(pdev, 0);
>> if (!priv->pdc_base)
>> return -EIO;
>>
>>
>
> What happens to the res_regs variable then?
>
> Also, and more importantly, devm_platform_ioremap_resource doesn't
> return NULL on error, but an ERR_PTR. Yes, the bug was already there,
> but since you're changing it, you might as well fix the thing.
Actually, the current code is right, and you're actively breaking it. Boo.
M.
--
Jazz is not dead. It just smells funny...
On 04/04/2019 08:20, Mukesh Ojha wrote:
> Hi All,
>
> This is just a minor change to promote using of this api
> devm_platform_ioremap_resource() and nothing else functional
> change it does.
>
> Also, there are patches where only ioremap is used without requesting
> the mem region so this devm_platform_ioremap_resource() take cares all
> of that in single call.
This series has a number of problems:
- It seems to introduce bugs rather than fixing anything
- It doesn't do a complete job of cleaning up the drivers
- Patch 4 does more than it is advertises doing
- You haven't cc'd the authors of these drivers, which would be common
courtesy
For a series that is a "minor change", that's a bit much. Please do not
send such blanket changes without understanding what you are doing.
Thanks,
M.
--
Jazz is not dead. It just smells funny...