of_iomap() matched with plan iounmap()

of_iomap() matched with plan iounmap()

[David Brown]

The SPARC target contains of_ioremap() and of_iounmap(), which various
drivers use (generally inside of CONFIG_SBUS).

include/linux/of_address.h contains a definition for of_iomap(), but
not corresponding unmap call.  Code using this calls the regular
iounmap().

Is it safe to assume that of_iomap() will always be based on ioremap()
and therefore it is safe to use iounmap(), or would it be better to
define another name for drivers to use as the inverse of of_iomap().
I'm not sure what to call it, since of_iounmap() is already taken by
SPARC.

Thanks,
David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: of_iomap() matched with plan iounmap()

[David Miller]

From: David Brown <davidb@codeaurora.org>
Date: Thu, 18 Aug 2011 10:02:26 -0700

> The SPARC target contains of_ioremap() and of_iounmap(), which various
> drivers use (generally inside of CONFIG_SBUS).
> 
> include/linux/of_address.h contains a definition for of_iomap(), but
> not corresponding unmap call.  Code using this calls the regular
> iounmap().
> 
> Is it safe to assume that of_iomap() will always be based on ioremap()
> and therefore it is safe to use iounmap(), or would it be better to
> define another name for drivers to use as the inverse of of_iomap().
> I'm not sure what to call it, since of_iounmap() is already taken by
> SPARC.

It's better to define a matching of_iounmap() interface, even if for
now it is exactly iounmap()

Re: of_iomap() matched with plan iounmap()

[Arnd Bergmann]

On Friday 19 August 2011, David Miller wrote:
> From: David Brown <davidb@codeaurora.org>
> Date: Thu, 18 Aug 2011 10:02:26 -0700
> 
> > The SPARC target contains of_ioremap() and of_iounmap(), which various
> > drivers use (generally inside of CONFIG_SBUS).
> > 
> > include/linux/of_address.h contains a definition for of_iomap(), but
> > not corresponding unmap call.  Code using this calls the regular
> > iounmap().
> > 
> > Is it safe to assume that of_iomap() will always be based on ioremap()
> > and therefore it is safe to use iounmap(), or would it be better to
> > define another name for drivers to use as the inverse of of_iomap().
> > I'm not sure what to call it, since of_iounmap() is already taken by
> > SPARC.
> 
> It's better to define a matching of_iounmap() interface, even if for
> now it is exactly iounmap()

But the problem is that we need conflicting prototypes for of_iounmap.
Sparc currently has

extern void of_iounmap(struct resource *res, void __iomem *base, unsigned long size);
(as the reverse of of_ioremap)

While we would probably want the generic prototype to be
extern void of_iounmap(void __iomem *base);
(as the reverse of of_iomap)

We could of course change all existing users of of_iounmap on sparc to use
the simpler prototype, because it also just calls iounmap.

	Arnd

Re: of_iomap() matched with plan iounmap()

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 19 Aug 2011 14:26:18 +0200

> We could of course change all existing users of of_iounmap on sparc to use
> the simpler prototype, because it also just calls iounmap.

Check again, on sparc64 it needs the resource to release it.

Only the 32-bit version on sparc evaluates to just a plain iounmap().

Re: of_iomap() matched with plan iounmap()

[Arnd Bergmann]

On Friday 19 August 2011, David Miller wrote:
> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Date: Fri, 19 Aug 2011 14:26:18 +0200
> 
> > We could of course change all existing users of of_iounmap on sparc to use
> > the simpler prototype, because it also just calls iounmap.
> 
> Check again, on sparc64 it needs the resource to release it.
> 
> Only the 32-bit version on sparc evaluates to just a plain iounmap().

Ok, I see.

On Friday 19 August 2011, David Brown wrote:
> What if we left the SPARC calls alone, and changed of_iomap() into
> of_dt_iomap() and could then make of_dt_iounmap().  Or, it could just
> be of_dt_map(), and of_dt_unmap().

I think at some point in the future, we will have a mass-renaming
of of_* to dt_* or similar, which would result in a silly name.

Also, the of_iomap() name is modeled after pci_iomap/pci_iomap.

How about renaming the sparc of_ioremap/of_iounmap pair to
resource_iomap/resource_iounmap? I believe that it is not
(any more) tied to device tree based probing at all, and also
could be useful for other subsystems on non-sparc architectures.

	Arnd

Re: of_iomap() matched with plan iounmap()

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 22 Aug 2011 16:07:25 +0200

> How about renaming the sparc of_ioremap/of_iounmap pair to
> resource_iomap/resource_iounmap? I believe that it is not
> (any more) tied to device tree based probing at all, and also
> could be useful for other subsystems on non-sparc architectures.

Sure, but it would be even nicer if other platforms used pre-computed
resources :-)

It would make OF devices match how PCI devices are managed, and that's
one of the main reasons I implemented it this way.

I know someone is going to say that OF resource resolution is "hard" or
difficult to untangle with how drivers on other platforms work.  But
that situation only exists because other platforms didn't do preresolved
OF resources from the beginning.

Having resource resolution handling and/or knowledge scattered throughout
all of the non-sparc OF drivers is very unwise.

Re: of_iomap() matched with plan iounmap()

[David Brown]

On Fri, Aug 19, 2011 at 02:26:18PM +0200, Arnd Bergmann wrote:
> On Friday 19 August 2011, David Miller wrote:
> > From: David Brown <davidb@codeaurora.org>
> > Date: Thu, 18 Aug 2011 10:02:26 -0700
> > 
> > > The SPARC target contains of_ioremap() and of_iounmap(), which various
> > > drivers use (generally inside of CONFIG_SBUS).
> > > 
> > > include/linux/of_address.h contains a definition for of_iomap(), but
> > > not corresponding unmap call.  Code using this calls the regular
> > > iounmap().
> > > 
> > > Is it safe to assume that of_iomap() will always be based on ioremap()
> > > and therefore it is safe to use iounmap(), or would it be better to
> > > define another name for drivers to use as the inverse of of_iomap().
> > > I'm not sure what to call it, since of_iounmap() is already taken by
> > > SPARC.
> > 
> > It's better to define a matching of_iounmap() interface, even if for
> > now it is exactly iounmap()
> 
> But the problem is that we need conflicting prototypes for of_iounmap.

What if we left the SPARC calls alone, and changed of_iomap() into
of_dt_iomap() and could then make of_dt_iounmap().  Or, it could just
be of_dt_map(), and of_dt_unmap().

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.