* [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Based on 'usb-next' branch of Greg's usb tree.
devm_ioremap_resource() API is advantageous over devm_ioremap()
and should therefore be preferred to request any ioremap'ed address
for hcd.
Changes from v1:
- Changed the way returned pointer is checked for error value
as pointed out in the review comment in the mailing list.
Vivek Gautam (6):
usb: host: ehci-exynos: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ehci-msm: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
usb: host: ehci-spear: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ehci-tegra: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ohci-exynos: Use devm_ioremap_resource instead of
devm_ioremap
drivers/usb/host/ehci-exynos.c | 7 +++----
drivers/usb/host/ehci-msm.c | 7 +++----
drivers/usb/host/ehci-mv.c | 16 ++++++----------
drivers/usb/host/ehci-spear.c | 13 +++----------
drivers/usb/host/ehci-tegra.c | 7 +++----
drivers/usb/host/ohci-exynos.c | 7 +++----
6 files changed, 21 insertions(+), 36 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Based on 'usb-next' branch of Greg's usb tree.
devm_ioremap_resource() API is advantageous over devm_ioremap()
and should therefore be preferred to request any ioremap'ed address
for hcd.
Changes from v1:
- Changed the way returned pointer is checked for error value
as pointed out in the review comment in the mailing list.
Vivek Gautam (6):
usb: host: ehci-exynos: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ehci-msm: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
usb: host: ehci-spear: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ehci-tegra: Use devm_ioremap_resource instead of
devm_ioremap
usb: host: ohci-exynos: Use devm_ioremap_resource instead of
devm_ioremap
drivers/usb/host/ehci-exynos.c | 7 +++----
drivers/usb/host/ehci-msm.c | 7 +++----
drivers/usb/host/ehci-mv.c | 16 ++++++----------
drivers/usb/host/ehci-spear.c | 13 +++----------
drivers/usb/host/ehci-tegra.c | 7 +++----
drivers/usb/host/ohci-exynos.c | 7 +++----
6 files changed, 21 insertions(+), 36 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 1/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
@ 2014-05-10 12:00 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-exynos.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 7f425ac..c23a6d9 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -135,10 +135,9 @@ skip_phy:
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
- if (!hcd->regs) {
- dev_err(&pdev->dev, "Failed to remap I/O memory\n");
- err = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ err = PTR_ERR(hcd->regs);
goto fail_io;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 1/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-exynos.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 7f425ac..c23a6d9 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -135,10 +135,9 @@ skip_phy:
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
- if (!hcd->regs) {
- dev_err(&pdev->dev, "Failed to remap I/O memory\n");
- err = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ err = PTR_ERR(hcd->regs);
goto fail_io;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 2/6] usb: host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
@ 2014-05-10 12:00 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-msm.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index f341651..982c09b 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -96,10 +96,9 @@ static int ehci_msm_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
- if (!hcd->regs) {
- dev_err(&pdev->dev, "ioremap failed\n");
- ret = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ ret = PTR_ERR(hcd->regs);
goto put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 2/6] usb: host: ehci-msm: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-msm.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index f341651..982c09b 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -96,10 +96,9 @@ static int ehci_msm_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
- if (!hcd->regs) {
- dev_err(&pdev->dev, "ioremap failed\n");
- ret = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ ret = PTR_ERR(hcd->regs);
goto put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 3/6] usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
@ 2014-05-10 12:00 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-mv.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c
index bd61612..08147c3 100644
--- a/drivers/usb/host/ehci-mv.c
+++ b/drivers/usb/host/ehci-mv.c
@@ -176,11 +176,9 @@ static int mv_ehci_probe(struct platform_device *pdev)
goto err_put_hcd;
}
- ehci_mv->phy_regs = devm_ioremap(&pdev->dev, r->start,
- resource_size(r));
- if (!ehci_mv->phy_regs) {
- dev_err(&pdev->dev, "failed to map phy I/O memory\n");
- retval = -EFAULT;
+ ehci_mv->phy_regs = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(ehci_mv->phy_regs)) {
+ retval = PTR_ERR(ehci_mv->phy_regs);
goto err_put_hcd;
}
@@ -191,11 +189,9 @@ static int mv_ehci_probe(struct platform_device *pdev)
goto err_put_hcd;
}
- ehci_mv->cap_regs = devm_ioremap(&pdev->dev, r->start,
- resource_size(r));
- if (ehci_mv->cap_regs == NULL) {
- dev_err(&pdev->dev, "failed to map I/O memory\n");
- retval = -EFAULT;
+ ehci_mv->cap_regs = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(ehci_mv->cap_regs)) {
+ retval = PTR_ERR(ehci_mv->cap_regs);
goto err_put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 3/6] usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-mv.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c
index bd61612..08147c3 100644
--- a/drivers/usb/host/ehci-mv.c
+++ b/drivers/usb/host/ehci-mv.c
@@ -176,11 +176,9 @@ static int mv_ehci_probe(struct platform_device *pdev)
goto err_put_hcd;
}
- ehci_mv->phy_regs = devm_ioremap(&pdev->dev, r->start,
- resource_size(r));
- if (!ehci_mv->phy_regs) {
- dev_err(&pdev->dev, "failed to map phy I/O memory\n");
- retval = -EFAULT;
+ ehci_mv->phy_regs = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(ehci_mv->phy_regs)) {
+ retval = PTR_ERR(ehci_mv->phy_regs);
goto err_put_hcd;
}
@@ -191,11 +189,9 @@ static int mv_ehci_probe(struct platform_device *pdev)
goto err_put_hcd;
}
- ehci_mv->cap_regs = devm_ioremap(&pdev->dev, r->start,
- resource_size(r));
- if (ehci_mv->cap_regs == NULL) {
- dev_err(&pdev->dev, "failed to map I/O memory\n");
- retval = -EFAULT;
+ ehci_mv->cap_regs = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(ehci_mv->cap_regs)) {
+ retval = PTR_ERR(ehci_mv->cap_regs);
goto err_put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 4/6] usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
(?)
@ 2014-05-10 12:00 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, shc_work, swarren, gregkh, linux-kernel,
kgene.kim, stern, Vivek Gautam, linux-tegra, thierry.reding,
linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-spear.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c
index 8bd915b..1d59958 100644
--- a/drivers/usb/host/ehci-spear.c
+++ b/drivers/usb/host/ehci-spear.c
@@ -106,16 +106,9 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- if (!devm_request_mem_region(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len,
- driver->description)) {
- retval = -EBUSY;
- goto err_put_hcd;
- }
-
- hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
- if (hcd->regs == NULL) {
- dev_dbg(&pdev->dev, "error mapping memory\n");
- retval = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ retval = PTR_ERR(hcd->regs);
goto err_put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 4/6] usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-spear.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c
index 8bd915b..1d59958 100644
--- a/drivers/usb/host/ehci-spear.c
+++ b/drivers/usb/host/ehci-spear.c
@@ -106,16 +106,9 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- if (!devm_request_mem_region(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len,
- driver->description)) {
- retval = -EBUSY;
- goto err_put_hcd;
- }
-
- hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
- if (hcd->regs == NULL) {
- dev_dbg(&pdev->dev, "error mapping memory\n");
- retval = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ retval = PTR_ERR(hcd->regs);
goto err_put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 4/6] usb: host: ehci-spear: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-spear.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c
index 8bd915b..1d59958 100644
--- a/drivers/usb/host/ehci-spear.c
+++ b/drivers/usb/host/ehci-spear.c
@@ -106,16 +106,9 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- if (!devm_request_mem_region(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len,
- driver->description)) {
- retval = -EBUSY;
- goto err_put_hcd;
- }
-
- hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
- if (hcd->regs == NULL) {
- dev_dbg(&pdev->dev, "error mapping memory\n");
- retval = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ retval = PTR_ERR(hcd->regs);
goto err_put_hcd;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
@ 2014-05-10 12:00 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-tegra.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 572634c..6fdcb8a 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -411,10 +411,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
}
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!hcd->regs) {
- dev_err(&pdev->dev, "Failed to remap I/O memory\n");
- err = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ err = PTR_ERR(hcd->regs);
goto cleanup_clk_en;
}
ehci->caps = hcd->regs + 0x100;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ehci-tegra.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 572634c..6fdcb8a 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -411,10 +411,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
}
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!hcd->regs) {
- dev_err(&pdev->dev, "Failed to remap I/O memory\n");
- err = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ err = PTR_ERR(hcd->regs);
goto cleanup_clk_en;
}
ehci->caps = hcd->regs + 0x100;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
@ 2014-05-10 12:00 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, swarren, kgene.kim, thierry.reding, shc_work,
Vivek Gautam
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ohci-exynos.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 9cf80cb..dec691d 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -120,10 +120,9 @@ skip_phy:
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
- if (!hcd->regs) {
- dev_err(&pdev->dev, "Failed to remap I/O memory\n");
- err = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ err = PTR_ERR(hcd->regs);
goto fail_io;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-10 12:00 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-10 12:00 UTC (permalink / raw)
To: linux-arm-kernel
Using devm_ioremap_resource() API should actually be preferred over
devm_ioremap(), since the former request the mem region first and then
gives back the ioremap'ed memory pointer.
devm_ioremap_resource() calls request_mem_region(), therby preventing
other drivers to make any overlapping call to the same region.
Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
drivers/usb/host/ohci-exynos.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 9cf80cb..dec691d 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -120,10 +120,9 @@ skip_phy:
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
- if (!hcd->regs) {
- dev_err(&pdev->dev, "Failed to remap I/O memory\n");
- err = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(hcd->regs)) {
+ err = PTR_ERR(hcd->regs);
goto fail_io;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
2014-05-10 12:00 ` Vivek Gautam
(?)
@ 2014-05-10 14:25 ` Alexander Shiyan
-1 siblings, 0 replies; 42+ messages in thread
From: Alexander Shiyan @ 2014-05-10 14:25 UTC (permalink / raw)
To: Vivek Gautam
Cc: linux-samsung-soc, swarren, gregkh, linux-kernel, kgene.kim,
stern, linux-tegra, thierry.reding, linux-arm-kernel, linux-usb
Sat, 10 May 2014 17:30:04 +0530 от Vivek Gautam <gautam.vivek@samsung.com>:
> Based on 'usb-next' branch of Greg's usb tree.
>
> devm_ioremap_resource() API is advantageous over devm_ioremap()
> and should therefore be preferred to request any ioremap'ed address
> for hcd.
>
> Changes from v1:
> - Changed the way returned pointer is checked for error value
> as pointed out in the review comment in the mailing list.
hcd->rsrc_len field can be removed entirely, since I cannot find any reason
how this filed can be used in the drivers now.
This is of course should be in an another series. This one looks good.
---
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-10 14:25 ` Alexander Shiyan
0 siblings, 0 replies; 42+ messages in thread
From: Alexander Shiyan @ 2014-05-10 14:25 UTC (permalink / raw)
To: Vivek Gautam
Cc: linux-samsung-soc, swarren, gregkh, linux-kernel, kgene.kim,
stern, linux-tegra, thierry.reding, linux-arm-kernel, linux-usb
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 788 bytes --]
Sat, 10 May 2014 17:30:04 +0530 Ð¾Ñ Vivek Gautam <gautam.vivek@samsung.com>:
> Based on 'usb-next' branch of Greg's usb tree.
>
> devm_ioremap_resource() API is advantageous over devm_ioremap()
> and should therefore be preferred to request any ioremap'ed address
> for hcd.
>
> Changes from v1:
> - Changed the way returned pointer is checked for error value
> as pointed out in the review comment in the mailing list.
hcd->rsrc_len field can be removed entirely, since I cannot find any reason
how this filed can be used in the drivers now.
This is of course should be in an another series. This one looks good.
---
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-10 14:25 ` Alexander Shiyan
0 siblings, 0 replies; 42+ messages in thread
From: Alexander Shiyan @ 2014-05-10 14:25 UTC (permalink / raw)
To: linux-arm-kernel
Sat, 10 May 2014 17:30:04 +0530 ?? Vivek Gautam <gautam.vivek@samsung.com>:
> Based on 'usb-next' branch of Greg's usb tree.
>
> devm_ioremap_resource() API is advantageous over devm_ioremap()
> and should therefore be preferred to request any ioremap'ed address
> for hcd.
>
> Changes from v1:
> - Changed the way returned pointer is checked for error value
> as pointed out in the review comment in the mailing list.
hcd->rsrc_len field can be removed entirely, since I cannot find any reason
how this filed can be used in the drivers now.
This is of course should be in an another series. This one looks good.
---
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
2014-05-10 14:25 ` Alexander Shiyan
(?)
@ 2014-05-11 14:45 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-11 14:45 UTC (permalink / raw)
To: Alexander Shiyan
Cc: linux-samsung-soc, Stephen Warren, Greg KH, linux-kernel,
Kukjin Kim, Alan Stern, linux-tegra, Thierry Reding,
linux-arm-kernel, Linux USB Mailing List
Hi,
On Sat, May 10, 2014 at 7:55 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Sat, 10 May 2014 17:30:04 +0530 от Vivek Gautam <gautam.vivek@samsung.com>:
>> Based on 'usb-next' branch of Greg's usb tree.
>>
>> devm_ioremap_resource() API is advantageous over devm_ioremap()
>> and should therefore be preferred to request any ioremap'ed address
>> for hcd.
>>
>> Changes from v1:
>> - Changed the way returned pointer is checked for error value
>> as pointed out in the review comment in the mailing list.
>
> hcd->rsrc_len field can be removed entirely, since I cannot find any reason
> how this filed can be used in the drivers now.
> This is of course should be in an another series. This one looks good.
True, even i noticed the same, since after grep'ing for rsrc_len. i
could not find any mention in the
USB layer.
Will send that as a separate patch-series.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-11 14:45 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-11 14:45 UTC (permalink / raw)
To: Alexander Shiyan
Cc: linux-samsung-soc, Stephen Warren, Greg KH, linux-kernel,
Kukjin Kim, Alan Stern, linux-tegra, Thierry Reding,
linux-arm-kernel, Linux USB Mailing List
Hi,
On Sat, May 10, 2014 at 7:55 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Sat, 10 May 2014 17:30:04 +0530 от Vivek Gautam <gautam.vivek@samsung.com>:
>> Based on 'usb-next' branch of Greg's usb tree.
>>
>> devm_ioremap_resource() API is advantageous over devm_ioremap()
>> and should therefore be preferred to request any ioremap'ed address
>> for hcd.
>>
>> Changes from v1:
>> - Changed the way returned pointer is checked for error value
>> as pointed out in the review comment in the mailing list.
>
> hcd->rsrc_len field can be removed entirely, since I cannot find any reason
> how this filed can be used in the drivers now.
> This is of course should be in an another series. This one looks good.
True, even i noticed the same, since after grep'ing for rsrc_len. i
could not find any mention in the
USB layer.
Will send that as a separate patch-series.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-11 14:45 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-05-11 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Sat, May 10, 2014 at 7:55 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Sat, 10 May 2014 17:30:04 +0530 ?? Vivek Gautam <gautam.vivek@samsung.com>:
>> Based on 'usb-next' branch of Greg's usb tree.
>>
>> devm_ioremap_resource() API is advantageous over devm_ioremap()
>> and should therefore be preferred to request any ioremap'ed address
>> for hcd.
>>
>> Changes from v1:
>> - Changed the way returned pointer is checked for error value
>> as pointed out in the review comment in the mailing list.
>
> hcd->rsrc_len field can be removed entirely, since I cannot find any reason
> how this filed can be used in the drivers now.
> This is of course should be in an another series. This one looks good.
True, even i noticed the same, since after grep'ing for rsrc_len. i
could not find any mention in the
USB layer.
Will send that as a separate patch-series.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
2014-05-10 12:00 ` Vivek Gautam
(?)
@ 2014-05-12 14:08 ` Alan Stern
-1 siblings, 0 replies; 42+ messages in thread
From: Alan Stern @ 2014-05-12 14:08 UTC (permalink / raw)
To: Vivek Gautam
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
swarren-3lzwWm7+Weoh9ZMKESR00Q, kgene.kim-Sze3O3UU22JBDgjK7y7TUQ,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, shc_work-JGs/UdohzUI
On Sat, 10 May 2014, Vivek Gautam wrote:
> Based on 'usb-next' branch of Greg's usb tree.
>
> devm_ioremap_resource() API is advantageous over devm_ioremap()
> and should therefore be preferred to request any ioremap'ed address
> for hcd.
>
> Changes from v1:
> - Changed the way returned pointer is checked for error value
> as pointed out in the review comment in the mailing list.
>
> Vivek Gautam (6):
> usb: host: ehci-exynos: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-msm: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
> usb: host: ehci-spear: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-tegra: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ohci-exynos: Use devm_ioremap_resource instead of
> devm_ioremap
>
> drivers/usb/host/ehci-exynos.c | 7 +++----
> drivers/usb/host/ehci-msm.c | 7 +++----
> drivers/usb/host/ehci-mv.c | 16 ++++++----------
> drivers/usb/host/ehci-spear.c | 13 +++----------
> drivers/usb/host/ehci-tegra.c | 7 +++----
> drivers/usb/host/ohci-exynos.c | 7 +++----
> 6 files changed, 21 insertions(+), 36 deletions(-)
For all six patches,
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-12 14:08 ` Alan Stern
0 siblings, 0 replies; 42+ messages in thread
From: Alan Stern @ 2014-05-12 14:08 UTC (permalink / raw)
To: Vivek Gautam
Cc: linux-usb, linux-samsung-soc, linux-tegra, linux-kernel,
linux-arm-kernel, gregkh, swarren, kgene.kim, thierry.reding,
shc_work
On Sat, 10 May 2014, Vivek Gautam wrote:
> Based on 'usb-next' branch of Greg's usb tree.
>
> devm_ioremap_resource() API is advantageous over devm_ioremap()
> and should therefore be preferred to request any ioremap'ed address
> for hcd.
>
> Changes from v1:
> - Changed the way returned pointer is checked for error value
> as pointed out in the review comment in the mailing list.
>
> Vivek Gautam (6):
> usb: host: ehci-exynos: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-msm: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
> usb: host: ehci-spear: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-tegra: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ohci-exynos: Use devm_ioremap_resource instead of
> devm_ioremap
>
> drivers/usb/host/ehci-exynos.c | 7 +++----
> drivers/usb/host/ehci-msm.c | 7 +++----
> drivers/usb/host/ehci-mv.c | 16 ++++++----------
> drivers/usb/host/ehci-spear.c | 13 +++----------
> drivers/usb/host/ehci-tegra.c | 7 +++----
> drivers/usb/host/ohci-exynos.c | 7 +++----
> 6 files changed, 21 insertions(+), 36 deletions(-)
For all six patches,
Acked-by: Alan Stern <stern@rowland.harvard.edu>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory
@ 2014-05-12 14:08 ` Alan Stern
0 siblings, 0 replies; 42+ messages in thread
From: Alan Stern @ 2014-05-12 14:08 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 10 May 2014, Vivek Gautam wrote:
> Based on 'usb-next' branch of Greg's usb tree.
>
> devm_ioremap_resource() API is advantageous over devm_ioremap()
> and should therefore be preferred to request any ioremap'ed address
> for hcd.
>
> Changes from v1:
> - Changed the way returned pointer is checked for error value
> as pointed out in the review comment in the mailing list.
>
> Vivek Gautam (6):
> usb: host: ehci-exynos: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-msm: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-mv: Use devm_ioremap_resource instead of devm_ioremap
> usb: host: ehci-spear: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ehci-tegra: Use devm_ioremap_resource instead of
> devm_ioremap
> usb: host: ohci-exynos: Use devm_ioremap_resource instead of
> devm_ioremap
>
> drivers/usb/host/ehci-exynos.c | 7 +++----
> drivers/usb/host/ehci-msm.c | 7 +++----
> drivers/usb/host/ehci-mv.c | 16 ++++++----------
> drivers/usb/host/ehci-spear.c | 13 +++----------
> drivers/usb/host/ehci-tegra.c | 7 +++----
> drivers/usb/host/ohci-exynos.c | 7 +++----
> 6 files changed, 21 insertions(+), 36 deletions(-)
For all six patches,
Acked-by: Alan Stern <stern@rowland.harvard.edu>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
(?)
@ 2014-05-12 16:58 ` Stephen Warren
-1 siblings, 0 replies; 42+ messages in thread
From: Stephen Warren @ 2014-05-12 16:58 UTC (permalink / raw)
To: Vivek Gautam, linux-usb-u79uwXL29TY76Z2rM5mHXA
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
kgene.kim-Sze3O3UU22JBDgjK7y7TUQ,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, shc_work-JGs/UdohzUI
On 05/10/2014 06:00 AM, Vivek Gautam wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
This patch on its own works OK on Tegra. However, if a similar patch
were to be applied to the Tegra PHY driver, then I expect that would
break USB on Tegra. The reason is that the Tegra USB controller and PHY
registers are interleaved a bit randomly within the same address range,
and rather than call out which individual addresses are relevant to the
controller and the PHY, the DT for both devices just specifies the same
whole range, and the drivers only touch the appropriate registers within
the range. Perhaps we should have described that as an MFD rather than
separate DT nodes and devices, but that's not what we ended up with.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-12 16:58 ` Stephen Warren
0 siblings, 0 replies; 42+ messages in thread
From: Stephen Warren @ 2014-05-12 16:58 UTC (permalink / raw)
To: Vivek Gautam, linux-usb
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
gregkh, stern, kgene.kim, thierry.reding, shc_work
On 05/10/2014 06:00 AM, Vivek Gautam wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
This patch on its own works OK on Tegra. However, if a similar patch
were to be applied to the Tegra PHY driver, then I expect that would
break USB on Tegra. The reason is that the Tegra USB controller and PHY
registers are interleaved a bit randomly within the same address range,
and rather than call out which individual addresses are relevant to the
controller and the PHY, the DT for both devices just specifies the same
whole range, and the drivers only touch the appropriate registers within
the range. Perhaps we should have described that as an MFD rather than
separate DT nodes and devices, but that's not what we ended up with.
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-05-12 16:58 ` Stephen Warren
0 siblings, 0 replies; 42+ messages in thread
From: Stephen Warren @ 2014-05-12 16:58 UTC (permalink / raw)
To: linux-arm-kernel
On 05/10/2014 06:00 AM, Vivek Gautam wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
This patch on its own works OK on Tegra. However, if a similar patch
were to be applied to the Tegra PHY driver, then I expect that would
break USB on Tegra. The reason is that the Tegra USB controller and PHY
registers are interleaved a bit randomly within the same address range,
and rather than call out which individual addresses are relevant to the
controller and the PHY, the DT for both devices just specifies the same
whole range, and the drivers only touch the appropriate registers within
the range. Perhaps we should have described that as an MFD rather than
separate DT nodes and devices, but that's not what we ended up with.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-05-10 12:00 ` Vivek Gautam
(?)
@ 2014-06-04 10:11 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-04 10:11 UTC (permalink / raw)
To: Linux USB Mailing List
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
Greg KH, Alan Stern, Stephen Warren, Kukjin Kim, Thierry Reding,
Alexander Shiyan, Vivek Gautam
Hi,
On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Although this patch and rest in the series are merged.
But i have got a doubt, so making this thread alive.
> ---
> drivers/usb/host/ohci-exynos.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 9cf80cb..dec691d 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -120,10 +120,9 @@ skip_phy:
>
> hcd->rsrc_start = res->start;
> hcd->rsrc_len = resource_size(res);
> - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> - if (!hcd->regs) {
> - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> - err = -ENOMEM;
> + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
which internally
requests the memory region and then does a "devm_ioremap()" or
"devm_ioremap_nocache()"
based on the check for IORESOURCE_CACHEABLE flag.
But this flag is not set for the resource of this device.
So should we be explicitly setting the flag in driver ?
The query goes for other patches too in this series, wherein
devm_ioremap() call is replaced with devm_ioremap_resource().
[snip]
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-04 10:11 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-04 10:11 UTC (permalink / raw)
To: Linux USB Mailing List
Cc: linux-samsung-soc, linux-tegra, linux-kernel, linux-arm-kernel,
Greg KH, Alan Stern, Stephen Warren, Kukjin Kim, Thierry Reding,
Alexander Shiyan, Vivek Gautam
Hi,
On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Although this patch and rest in the series are merged.
But i have got a doubt, so making this thread alive.
> ---
> drivers/usb/host/ohci-exynos.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 9cf80cb..dec691d 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -120,10 +120,9 @@ skip_phy:
>
> hcd->rsrc_start = res->start;
> hcd->rsrc_len = resource_size(res);
> - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> - if (!hcd->regs) {
> - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> - err = -ENOMEM;
> + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
which internally
requests the memory region and then does a "devm_ioremap()" or
"devm_ioremap_nocache()"
based on the check for IORESOURCE_CACHEABLE flag.
But this flag is not set for the resource of this device.
So should we be explicitly setting the flag in driver ?
The query goes for other patches too in this series, wherein
devm_ioremap() call is replaced with devm_ioremap_resource().
[snip]
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-04 10:11 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-04 10:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> Using devm_ioremap_resource() API should actually be preferred over
> devm_ioremap(), since the former request the mem region first and then
> gives back the ioremap'ed memory pointer.
> devm_ioremap_resource() calls request_mem_region(), therby preventing
> other drivers to make any overlapping call to the same region.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Although this patch and rest in the series are merged.
But i have got a doubt, so making this thread alive.
> ---
> drivers/usb/host/ohci-exynos.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 9cf80cb..dec691d 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -120,10 +120,9 @@ skip_phy:
>
> hcd->rsrc_start = res->start;
> hcd->rsrc_len = resource_size(res);
> - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> - if (!hcd->regs) {
> - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> - err = -ENOMEM;
> + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
which internally
requests the memory region and then does a "devm_ioremap()" or
"devm_ioremap_nocache()"
based on the check for IORESOURCE_CACHEABLE flag.
But this flag is not set for the resource of this device.
So should we be explicitly setting the flag in driver ?
The query goes for other patches too in this series, wherein
devm_ioremap() call is replaced with devm_ioremap_resource().
[snip]
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-06-04 10:11 ` Vivek Gautam
(?)
@ 2014-06-04 13:13 ` Thierry Reding
-1 siblings, 0 replies; 42+ messages in thread
From: Thierry Reding @ 2014-06-04 13:13 UTC (permalink / raw)
To: Vivek Gautam
Cc: Linux USB Mailing List, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Greg KH,
Alan Stern, Stephen Warren, Kukjin Kim, Alexander Shiyan
[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]
On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> Hi,
>
>
> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > Using devm_ioremap_resource() API should actually be preferred over
> > devm_ioremap(), since the former request the mem region first and then
> > gives back the ioremap'ed memory pointer.
> > devm_ioremap_resource() calls request_mem_region(), therby preventing
> > other drivers to make any overlapping call to the same region.
> >
> > Signed-off-by: Vivek Gautam <gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> Although this patch and rest in the series are merged.
> But i have got a doubt, so making this thread alive.
>
> > ---
> > drivers/usb/host/ohci-exynos.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > index 9cf80cb..dec691d 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -120,10 +120,9 @@ skip_phy:
> >
> > hcd->rsrc_start = res->start;
> > hcd->rsrc_len = resource_size(res);
> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> > - if (!hcd->regs) {
> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> > - err = -ENOMEM;
> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>
> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> which internally requests the memory region
I guess this could lead to problems if drivers haven't been written to
cleanly split the register ranges that they access, since now two
overlapping regions may be requested and cause the drivers to fail.
> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> the check for IORESOURCE_CACHEABLE flag.
>
> But this flag is not set for the resource of this device.
> So should we be explicitly setting the flag in driver ?
I don't think it makes much sense to map these registers cached anyway.
Drivers will likely expect writes to this region to take effect without
needing any kind of flushing.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-04 13:13 ` Thierry Reding
0 siblings, 0 replies; 42+ messages in thread
From: Thierry Reding @ 2014-06-04 13:13 UTC (permalink / raw)
To: Vivek Gautam
Cc: Linux USB Mailing List, linux-samsung-soc, linux-tegra,
linux-kernel, linux-arm-kernel, Greg KH, Alan Stern,
Stephen Warren, Kukjin Kim, Alexander Shiyan
[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]
On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> Hi,
>
>
> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> > Using devm_ioremap_resource() API should actually be preferred over
> > devm_ioremap(), since the former request the mem region first and then
> > gives back the ioremap'ed memory pointer.
> > devm_ioremap_resource() calls request_mem_region(), therby preventing
> > other drivers to make any overlapping call to the same region.
> >
> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>
> Although this patch and rest in the series are merged.
> But i have got a doubt, so making this thread alive.
>
> > ---
> > drivers/usb/host/ohci-exynos.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > index 9cf80cb..dec691d 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -120,10 +120,9 @@ skip_phy:
> >
> > hcd->rsrc_start = res->start;
> > hcd->rsrc_len = resource_size(res);
> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> > - if (!hcd->regs) {
> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> > - err = -ENOMEM;
> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>
> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> which internally requests the memory region
I guess this could lead to problems if drivers haven't been written to
cleanly split the register ranges that they access, since now two
overlapping regions may be requested and cause the drivers to fail.
> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> the check for IORESOURCE_CACHEABLE flag.
>
> But this flag is not set for the resource of this device.
> So should we be explicitly setting the flag in driver ?
I don't think it makes much sense to map these registers cached anyway.
Drivers will likely expect writes to this region to take effect without
needing any kind of flushing.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-04 13:13 ` Thierry Reding
0 siblings, 0 replies; 42+ messages in thread
From: Thierry Reding @ 2014-06-04 13:13 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> Hi,
>
>
> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> > Using devm_ioremap_resource() API should actually be preferred over
> > devm_ioremap(), since the former request the mem region first and then
> > gives back the ioremap'ed memory pointer.
> > devm_ioremap_resource() calls request_mem_region(), therby preventing
> > other drivers to make any overlapping call to the same region.
> >
> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>
> Although this patch and rest in the series are merged.
> But i have got a doubt, so making this thread alive.
>
> > ---
> > drivers/usb/host/ohci-exynos.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> > index 9cf80cb..dec691d 100644
> > --- a/drivers/usb/host/ohci-exynos.c
> > +++ b/drivers/usb/host/ohci-exynos.c
> > @@ -120,10 +120,9 @@ skip_phy:
> >
> > hcd->rsrc_start = res->start;
> > hcd->rsrc_len = resource_size(res);
> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> > - if (!hcd->regs) {
> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> > - err = -ENOMEM;
> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>
> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> which internally requests the memory region
I guess this could lead to problems if drivers haven't been written to
cleanly split the register ranges that they access, since now two
overlapping regions may be requested and cause the drivers to fail.
> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> the check for IORESOURCE_CACHEABLE flag.
>
> But this flag is not set for the resource of this device.
> So should we be explicitly setting the flag in driver ?
I don't think it makes much sense to map these registers cached anyway.
Drivers will likely expect writes to this region to take effect without
needing any kind of flushing.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140604/b3ac520e/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-06-04 13:13 ` Thierry Reding
(?)
@ 2014-06-06 13:02 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-06 13:02 UTC (permalink / raw)
To: Thierry Reding
Cc: Linux USB Mailing List, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Greg KH,
Alan Stern, Stephen Warren, Kukjin Kim, Alexander Shiyan
Hi,
On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
>> Hi,
>>
>>
>> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> > Using devm_ioremap_resource() API should actually be preferred over
>> > devm_ioremap(), since the former request the mem region first and then
>> > gives back the ioremap'ed memory pointer.
>> > devm_ioremap_resource() calls request_mem_region(), therby preventing
>> > other drivers to make any overlapping call to the same region.
>> >
>> > Signed-off-by: Vivek Gautam <gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>
>> Although this patch and rest in the series are merged.
>> But i have got a doubt, so making this thread alive.
>>
>> > ---
>> > drivers/usb/host/ohci-exynos.c | 7 +++----
>> > 1 file changed, 3 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> > index 9cf80cb..dec691d 100644
>> > --- a/drivers/usb/host/ohci-exynos.c
>> > +++ b/drivers/usb/host/ohci-exynos.c
>> > @@ -120,10 +120,9 @@ skip_phy:
>> >
>> > hcd->rsrc_start = res->start;
>> > hcd->rsrc_len = resource_size(res);
>> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
>> > - if (!hcd->regs) {
>> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>> > - err = -ENOMEM;
>> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>>
>> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
>> which internally requests the memory region
>
> I guess this could lead to problems if drivers haven't been written to
> cleanly split the register ranges that they access, since now two
> overlapping regions may be requested and cause the drivers to fail.
Sorry i did not understand completely. Wouldn't the request_mem_region()
fail for an already busy resource ?
So devm_ioremap_resource() will in fact prevent the drivers from requesting
the same memory region twice until the first request frees the region.
Isn't it ?
>
>> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
>> the check for IORESOURCE_CACHEABLE flag.
>>
>> But this flag is not set for the resource of this device.
>> So should we be explicitly setting the flag in driver ?
>
> I don't think it makes much sense to map these registers cached anyway.
> Drivers will likely expect writes to this region to take effect without
> needing any kind of flushing.
These "hcd->regs" are going to be used by the controller, so wouldn't there be a
a performance difference when the requested address space is
cacheable/non-cacheable ?
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-06 13:02 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-06 13:02 UTC (permalink / raw)
To: Thierry Reding
Cc: Linux USB Mailing List, linux-samsung-soc, linux-tegra,
linux-kernel, linux-arm-kernel, Greg KH, Alan Stern,
Stephen Warren, Kukjin Kim, Alexander Shiyan
Hi,
On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
>> Hi,
>>
>>
>> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
>> > Using devm_ioremap_resource() API should actually be preferred over
>> > devm_ioremap(), since the former request the mem region first and then
>> > gives back the ioremap'ed memory pointer.
>> > devm_ioremap_resource() calls request_mem_region(), therby preventing
>> > other drivers to make any overlapping call to the same region.
>> >
>> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>>
>> Although this patch and rest in the series are merged.
>> But i have got a doubt, so making this thread alive.
>>
>> > ---
>> > drivers/usb/host/ohci-exynos.c | 7 +++----
>> > 1 file changed, 3 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> > index 9cf80cb..dec691d 100644
>> > --- a/drivers/usb/host/ohci-exynos.c
>> > +++ b/drivers/usb/host/ohci-exynos.c
>> > @@ -120,10 +120,9 @@ skip_phy:
>> >
>> > hcd->rsrc_start = res->start;
>> > hcd->rsrc_len = resource_size(res);
>> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
>> > - if (!hcd->regs) {
>> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>> > - err = -ENOMEM;
>> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>>
>> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
>> which internally requests the memory region
>
> I guess this could lead to problems if drivers haven't been written to
> cleanly split the register ranges that they access, since now two
> overlapping regions may be requested and cause the drivers to fail.
Sorry i did not understand completely. Wouldn't the request_mem_region()
fail for an already busy resource ?
So devm_ioremap_resource() will in fact prevent the drivers from requesting
the same memory region twice until the first request frees the region.
Isn't it ?
>
>> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
>> the check for IORESOURCE_CACHEABLE flag.
>>
>> But this flag is not set for the resource of this device.
>> So should we be explicitly setting the flag in driver ?
>
> I don't think it makes much sense to map these registers cached anyway.
> Drivers will likely expect writes to this region to take effect without
> needing any kind of flushing.
These "hcd->regs" are going to be used by the controller, so wouldn't there be a
a performance difference when the requested address space is
cacheable/non-cacheable ?
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-06 13:02 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-06 13:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
>> Hi,
>>
>>
>> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
>> > Using devm_ioremap_resource() API should actually be preferred over
>> > devm_ioremap(), since the former request the mem region first and then
>> > gives back the ioremap'ed memory pointer.
>> > devm_ioremap_resource() calls request_mem_region(), therby preventing
>> > other drivers to make any overlapping call to the same region.
>> >
>> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>>
>> Although this patch and rest in the series are merged.
>> But i have got a doubt, so making this thread alive.
>>
>> > ---
>> > drivers/usb/host/ohci-exynos.c | 7 +++----
>> > 1 file changed, 3 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> > index 9cf80cb..dec691d 100644
>> > --- a/drivers/usb/host/ohci-exynos.c
>> > +++ b/drivers/usb/host/ohci-exynos.c
>> > @@ -120,10 +120,9 @@ skip_phy:
>> >
>> > hcd->rsrc_start = res->start;
>> > hcd->rsrc_len = resource_size(res);
>> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
>> > - if (!hcd->regs) {
>> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>> > - err = -ENOMEM;
>> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>>
>> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
>> which internally requests the memory region
>
> I guess this could lead to problems if drivers haven't been written to
> cleanly split the register ranges that they access, since now two
> overlapping regions may be requested and cause the drivers to fail.
Sorry i did not understand completely. Wouldn't the request_mem_region()
fail for an already busy resource ?
So devm_ioremap_resource() will in fact prevent the drivers from requesting
the same memory region twice until the first request frees the region.
Isn't it ?
>
>> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
>> the check for IORESOURCE_CACHEABLE flag.
>>
>> But this flag is not set for the resource of this device.
>> So should we be explicitly setting the flag in driver ?
>
> I don't think it makes much sense to map these registers cached anyway.
> Drivers will likely expect writes to this region to take effect without
> needing any kind of flushing.
These "hcd->regs" are going to be used by the controller, so wouldn't there be a
a performance difference when the requested address space is
cacheable/non-cacheable ?
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-06-06 13:02 ` Vivek Gautam
(?)
@ 2014-06-06 22:52 ` Thierry Reding
-1 siblings, 0 replies; 42+ messages in thread
From: Thierry Reding @ 2014-06-06 22:52 UTC (permalink / raw)
To: Vivek Gautam
Cc: Linux USB Mailing List, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Greg KH,
Alan Stern, Stephen Warren, Kukjin Kim, Alexander Shiyan
[-- Attachment #1: Type: text/plain, Size: 2828 bytes --]
On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote:
> On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> >> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek-Sze3O3UU22KIwRZHo2/mJg@public.gmane.orgm> wrote:
[...]
> >> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> >> > index 9cf80cb..dec691d 100644
> >> > --- a/drivers/usb/host/ohci-exynos.c
> >> > +++ b/drivers/usb/host/ohci-exynos.c
> >> > @@ -120,10 +120,9 @@ skip_phy:
> >> >
> >> > hcd->rsrc_start = res->start;
> >> > hcd->rsrc_len = resource_size(res);
> >> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> >> > - if (!hcd->regs) {
> >> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> >> > - err = -ENOMEM;
> >> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
> >>
> >> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> >> which internally requests the memory region
> >
> > I guess this could lead to problems if drivers haven't been written to
> > cleanly split the register ranges that they access, since now two
> > overlapping regions may be requested and cause the drivers to fail.
>
> Sorry i did not understand completely. Wouldn't the request_mem_region()
> fail for an already busy resource ?
> So devm_ioremap_resource() will in fact prevent the drivers from requesting
> the same memory region twice until the first request frees the region.
> Isn't it ?
Yes exactly. What I was trying to say is that since drivers weren't
requesting the resources before they may be using overlapping regions.
Now that this patch changes these drivers to also request the resources
they will fail if the regions overlap with those of other drivers.
> >> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> >> the check for IORESOURCE_CACHEABLE flag.
> >>
> >> But this flag is not set for the resource of this device.
> >> So should we be explicitly setting the flag in driver ?
> >
> > I don't think it makes much sense to map these registers cached anyway.
> > Drivers will likely expect writes to this region to take effect without
> > needing any kind of flushing.
>
> These "hcd->regs" are going to be used by the controller, so wouldn't
> there be a performance difference when the requested address space is
> cacheable/non-cacheable ?
The issue here is that if the region is mapped cacheable then register
writes may not immediately take effect and that's almost certainly not
what the driver will expect. I don't think it ever makes sense to map
registers cacheable.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-06 22:52 ` Thierry Reding
0 siblings, 0 replies; 42+ messages in thread
From: Thierry Reding @ 2014-06-06 22:52 UTC (permalink / raw)
To: Vivek Gautam
Cc: Linux USB Mailing List, linux-samsung-soc, linux-tegra,
linux-kernel, linux-arm-kernel, Greg KH, Alan Stern,
Stephen Warren, Kukjin Kim, Alexander Shiyan
[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]
On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote:
> On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> >> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
[...]
> >> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> >> > index 9cf80cb..dec691d 100644
> >> > --- a/drivers/usb/host/ohci-exynos.c
> >> > +++ b/drivers/usb/host/ohci-exynos.c
> >> > @@ -120,10 +120,9 @@ skip_phy:
> >> >
> >> > hcd->rsrc_start = res->start;
> >> > hcd->rsrc_len = resource_size(res);
> >> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> >> > - if (!hcd->regs) {
> >> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> >> > - err = -ENOMEM;
> >> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
> >>
> >> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> >> which internally requests the memory region
> >
> > I guess this could lead to problems if drivers haven't been written to
> > cleanly split the register ranges that they access, since now two
> > overlapping regions may be requested and cause the drivers to fail.
>
> Sorry i did not understand completely. Wouldn't the request_mem_region()
> fail for an already busy resource ?
> So devm_ioremap_resource() will in fact prevent the drivers from requesting
> the same memory region twice until the first request frees the region.
> Isn't it ?
Yes exactly. What I was trying to say is that since drivers weren't
requesting the resources before they may be using overlapping regions.
Now that this patch changes these drivers to also request the resources
they will fail if the regions overlap with those of other drivers.
> >> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> >> the check for IORESOURCE_CACHEABLE flag.
> >>
> >> But this flag is not set for the resource of this device.
> >> So should we be explicitly setting the flag in driver ?
> >
> > I don't think it makes much sense to map these registers cached anyway.
> > Drivers will likely expect writes to this region to take effect without
> > needing any kind of flushing.
>
> These "hcd->regs" are going to be used by the controller, so wouldn't
> there be a performance difference when the requested address space is
> cacheable/non-cacheable ?
The issue here is that if the region is mapped cacheable then register
writes may not immediately take effect and that's almost certainly not
what the driver will expect. I don't think it ever makes sense to map
registers cacheable.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-06 22:52 ` Thierry Reding
0 siblings, 0 replies; 42+ messages in thread
From: Thierry Reding @ 2014-06-06 22:52 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote:
> On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
> >> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
[...]
> >> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> >> > index 9cf80cb..dec691d 100644
> >> > --- a/drivers/usb/host/ohci-exynos.c
> >> > +++ b/drivers/usb/host/ohci-exynos.c
> >> > @@ -120,10 +120,9 @@ skip_phy:
> >> >
> >> > hcd->rsrc_start = res->start;
> >> > hcd->rsrc_len = resource_size(res);
> >> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
> >> > - if (!hcd->regs) {
> >> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
> >> > - err = -ENOMEM;
> >> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
> >>
> >> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
> >> which internally requests the memory region
> >
> > I guess this could lead to problems if drivers haven't been written to
> > cleanly split the register ranges that they access, since now two
> > overlapping regions may be requested and cause the drivers to fail.
>
> Sorry i did not understand completely. Wouldn't the request_mem_region()
> fail for an already busy resource ?
> So devm_ioremap_resource() will in fact prevent the drivers from requesting
> the same memory region twice until the first request frees the region.
> Isn't it ?
Yes exactly. What I was trying to say is that since drivers weren't
requesting the resources before they may be using overlapping regions.
Now that this patch changes these drivers to also request the resources
they will fail if the regions overlap with those of other drivers.
> >> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
> >> the check for IORESOURCE_CACHEABLE flag.
> >>
> >> But this flag is not set for the resource of this device.
> >> So should we be explicitly setting the flag in driver ?
> >
> > I don't think it makes much sense to map these registers cached anyway.
> > Drivers will likely expect writes to this region to take effect without
> > needing any kind of flushing.
>
> These "hcd->regs" are going to be used by the controller, so wouldn't
> there be a performance difference when the requested address space is
> cacheable/non-cacheable ?
The issue here is that if the region is mapped cacheable then register
writes may not immediately take effect and that's almost certainly not
what the driver will expect. I don't think it ever makes sense to map
registers cacheable.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140607/206b0a6d/attachment.sig>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
2014-06-06 22:52 ` Thierry Reding
(?)
@ 2014-06-24 5:49 ` Vivek Gautam
-1 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-24 5:49 UTC (permalink / raw)
To: Thierry Reding
Cc: Linux USB Mailing List, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Greg KH,
Alan Stern, Stephen Warren, Kukjin Kim, Alexander Shiyan
Hi,
On Sat, Jun 7, 2014 at 4:22 AM, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote:
>> On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
>> >> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> [...]
>> >> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> >> > index 9cf80cb..dec691d 100644
>> >> > --- a/drivers/usb/host/ohci-exynos.c
>> >> > +++ b/drivers/usb/host/ohci-exynos.c
>> >> > @@ -120,10 +120,9 @@ skip_phy:
>> >> >
>> >> > hcd->rsrc_start = res->start;
>> >> > hcd->rsrc_len = resource_size(res);
>> >> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
>> >> > - if (!hcd->regs) {
>> >> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>> >> > - err = -ENOMEM;
>> >> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>> >>
>> >> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
>> >> which internally requests the memory region
>> >
>> > I guess this could lead to problems if drivers haven't been written to
>> > cleanly split the register ranges that they access, since now two
>> > overlapping regions may be requested and cause the drivers to fail.
>>
>> Sorry i did not understand completely. Wouldn't the request_mem_region()
>> fail for an already busy resource ?
>> So devm_ioremap_resource() will in fact prevent the drivers from requesting
>> the same memory region twice until the first request frees the region.
>> Isn't it ?
>
> Yes exactly. What I was trying to say is that since drivers weren't
> requesting the resources before they may be using overlapping regions.
> Now that this patch changes these drivers to also request the resources
> they will fail if the regions overlap with those of other drivers.
Thanks for explaining it further.
I understand this fact. And i am sure this case does not arise in exynos.
For Tegra, Stephen noted this fact about the ehci driver and the
corresponding PHY.
So that the PHY does a devm_ioremap() only.
For other platforms too we did not get any concerns raised, so we
moved ahead with the series
for merging.
>
>> >> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
>> >> the check for IORESOURCE_CACHEABLE flag.
>> >>
>> >> But this flag is not set for the resource of this device.
>> >> So should we be explicitly setting the flag in driver ?
>> >
>> > I don't think it makes much sense to map these registers cached anyway.
>> > Drivers will likely expect writes to this region to take effect without
>> > needing any kind of flushing.
>>
>> These "hcd->regs" are going to be used by the controller, so wouldn't
>> there be a performance difference when the requested address space is
>> cacheable/non-cacheable ?
>
> The issue here is that if the region is mapped cacheable then register
> writes may not immediately take effect and that's almost certainly not
> what the driver will expect. I don't think it ever makes sense to map
> registers cacheable.
Ok, this explains things.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-24 5:49 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-24 5:49 UTC (permalink / raw)
To: Thierry Reding
Cc: Linux USB Mailing List, linux-samsung-soc, linux-tegra,
linux-kernel, linux-arm-kernel, Greg KH, Alan Stern,
Stephen Warren, Kukjin Kim, Alexander Shiyan
Hi,
On Sat, Jun 7, 2014 at 4:22 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote:
>> On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
>> >> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> [...]
>> >> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> >> > index 9cf80cb..dec691d 100644
>> >> > --- a/drivers/usb/host/ohci-exynos.c
>> >> > +++ b/drivers/usb/host/ohci-exynos.c
>> >> > @@ -120,10 +120,9 @@ skip_phy:
>> >> >
>> >> > hcd->rsrc_start = res->start;
>> >> > hcd->rsrc_len = resource_size(res);
>> >> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
>> >> > - if (!hcd->regs) {
>> >> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>> >> > - err = -ENOMEM;
>> >> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>> >>
>> >> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
>> >> which internally requests the memory region
>> >
>> > I guess this could lead to problems if drivers haven't been written to
>> > cleanly split the register ranges that they access, since now two
>> > overlapping regions may be requested and cause the drivers to fail.
>>
>> Sorry i did not understand completely. Wouldn't the request_mem_region()
>> fail for an already busy resource ?
>> So devm_ioremap_resource() will in fact prevent the drivers from requesting
>> the same memory region twice until the first request frees the region.
>> Isn't it ?
>
> Yes exactly. What I was trying to say is that since drivers weren't
> requesting the resources before they may be using overlapping regions.
> Now that this patch changes these drivers to also request the resources
> they will fail if the regions overlap with those of other drivers.
Thanks for explaining it further.
I understand this fact. And i am sure this case does not arise in exynos.
For Tegra, Stephen noted this fact about the ehci driver and the
corresponding PHY.
So that the PHY does a devm_ioremap() only.
For other platforms too we did not get any concerns raised, so we
moved ahead with the series
for merging.
>
>> >> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
>> >> the check for IORESOURCE_CACHEABLE flag.
>> >>
>> >> But this flag is not set for the resource of this device.
>> >> So should we be explicitly setting the flag in driver ?
>> >
>> > I don't think it makes much sense to map these registers cached anyway.
>> > Drivers will likely expect writes to this region to take effect without
>> > needing any kind of flushing.
>>
>> These "hcd->regs" are going to be used by the controller, so wouldn't
>> there be a performance difference when the requested address space is
>> cacheable/non-cacheable ?
>
> The issue here is that if the region is mapped cacheable then register
> writes may not immediately take effect and that's almost certainly not
> what the driver will expect. I don't think it ever makes sense to map
> registers cacheable.
Ok, this explains things.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
@ 2014-06-24 5:49 ` Vivek Gautam
0 siblings, 0 replies; 42+ messages in thread
From: Vivek Gautam @ 2014-06-24 5:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Sat, Jun 7, 2014 at 4:22 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote:
>> On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote:
>> >> On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> [...]
>> >> > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
>> >> > index 9cf80cb..dec691d 100644
>> >> > --- a/drivers/usb/host/ohci-exynos.c
>> >> > +++ b/drivers/usb/host/ohci-exynos.c
>> >> > @@ -120,10 +120,9 @@ skip_phy:
>> >> >
>> >> > hcd->rsrc_start = res->start;
>> >> > hcd->rsrc_len = resource_size(res);
>> >> > - hcd->regs = devm_ioremap(&pdev->dev, res->start, hcd->rsrc_len);
>> >> > - if (!hcd->regs) {
>> >> > - dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>> >> > - err = -ENOMEM;
>> >> > + hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>> >>
>> >> Here, we replaced devm_ioremap() call with devm_ioremap_resource(),
>> >> which internally requests the memory region
>> >
>> > I guess this could lead to problems if drivers haven't been written to
>> > cleanly split the register ranges that they access, since now two
>> > overlapping regions may be requested and cause the drivers to fail.
>>
>> Sorry i did not understand completely. Wouldn't the request_mem_region()
>> fail for an already busy resource ?
>> So devm_ioremap_resource() will in fact prevent the drivers from requesting
>> the same memory region twice until the first request frees the region.
>> Isn't it ?
>
> Yes exactly. What I was trying to say is that since drivers weren't
> requesting the resources before they may be using overlapping regions.
> Now that this patch changes these drivers to also request the resources
> they will fail if the regions overlap with those of other drivers.
Thanks for explaining it further.
I understand this fact. And i am sure this case does not arise in exynos.
For Tegra, Stephen noted this fact about the ehci driver and the
corresponding PHY.
So that the PHY does a devm_ioremap() only.
For other platforms too we did not get any concerns raised, so we
moved ahead with the series
for merging.
>
>> >> and then does a "devm_ioremap()" or "devm_ioremap_nocache()" based on
>> >> the check for IORESOURCE_CACHEABLE flag.
>> >>
>> >> But this flag is not set for the resource of this device.
>> >> So should we be explicitly setting the flag in driver ?
>> >
>> > I don't think it makes much sense to map these registers cached anyway.
>> > Drivers will likely expect writes to this region to take effect without
>> > needing any kind of flushing.
>>
>> These "hcd->regs" are going to be used by the controller, so wouldn't
>> there be a performance difference when the requested address space is
>> cacheable/non-cacheable ?
>
> The issue here is that if the region is mapped cacheable then register
> writes may not immediately take effect and that's almost certainly not
> what the driver will expect. I don't think it ever makes sense to map
> registers cacheable.
Ok, this explains things.
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2014-06-24 5:49 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-10 12:00 [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-05-10 12:00 ` [PATCH v2 1/6] usb: host: ehci-exynos: Use devm_ioremap_resource instead of devm_ioremap Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-05-10 12:00 ` [PATCH v2 2/6] usb: host: ehci-msm: " Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-05-10 12:00 ` [PATCH v2 3/6] usb: host: ehci-mv: " Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-05-10 12:00 ` [PATCH v2 4/6] usb: host: ehci-spear: " Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-05-10 12:00 ` [PATCH v2 5/6] usb: host: ehci-tegra: " Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
[not found] ` <1399723210-21569-6-git-send-email-gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2014-05-12 16:58 ` Stephen Warren
2014-05-12 16:58 ` Stephen Warren
2014-05-12 16:58 ` Stephen Warren
2014-05-10 12:00 ` [PATCH v2 6/6] usb: host: ohci-exynos: " Vivek Gautam
2014-05-10 12:00 ` Vivek Gautam
2014-06-04 10:11 ` Vivek Gautam
2014-06-04 10:11 ` Vivek Gautam
2014-06-04 10:11 ` Vivek Gautam
[not found] ` <CAFp+6iFufCh6SVEF-HQA62ZsJGJGSebDSs28eqcrMy1X8e5vjA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-06-04 13:13 ` Thierry Reding
2014-06-04 13:13 ` Thierry Reding
2014-06-04 13:13 ` Thierry Reding
2014-06-06 13:02 ` Vivek Gautam
2014-06-06 13:02 ` Vivek Gautam
2014-06-06 13:02 ` Vivek Gautam
[not found] ` <CAFp+6iEe8fvxboG79=2s=oe4BkQJYAoVUPtYkPdjJHCe+bC=8Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-06-06 22:52 ` Thierry Reding
2014-06-06 22:52 ` Thierry Reding
2014-06-06 22:52 ` Thierry Reding
2014-06-24 5:49 ` Vivek Gautam
2014-06-24 5:49 ` Vivek Gautam
2014-06-24 5:49 ` Vivek Gautam
2014-05-10 14:25 ` [PATCH v2 0/6] usb: host: Cleanup for ioremap'ing hcd memory Alexander Shiyan
2014-05-10 14:25 ` Alexander Shiyan
2014-05-10 14:25 ` Alexander Shiyan
2014-05-11 14:45 ` Vivek Gautam
2014-05-11 14:45 ` Vivek Gautam
2014-05-11 14:45 ` Vivek Gautam
[not found] ` <1399723210-21569-1-git-send-email-gautam.vivek-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2014-05-12 14:08 ` Alan Stern
2014-05-12 14:08 ` Alan Stern
2014-05-12 14:08 ` Alan Stern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.