linux-kernel - Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140604131306.GB28484@ulmo>
Date:	Wed, 4 Jun 2014 15:13:07 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Vivek Gautam <gautam.vivek@...sung.com>
Cc:	Linux USB Mailing List <linux-usb@...r.kernel.org>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>,
	linux-tegra <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Greg KH <gregkh@...uxfoundation.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Stephen Warren <swarren@...dotorg.org>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Alexander Shiyan <shc_work@...l.ru>
Subject: Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource
 instead of devm_ioremap

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@...sung.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@...sung.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

Content of type "application/pgp-signature" skipped