[U-Boot,01/27] Provide a generic io.h & address mapping functions

Most architectures currently supported by U-Boot use trivial
implementations of map_to_physmem & virt_to_phys which simply cast a
physical address to a pointer for use a virtual address & vice-versa.
This results in a lot of duplicate implementations of these mapping
functions.

The functions provided by different architectures also differs, with
some having implementations of phys_to_virt & others not. A later patch
in this series will make use of phys_to_virt, so requires that it be
provided for all architectures.

This patch introduces an asm-generic/io.h which provides generic
implementations of address mapping functions, allowing the duplication
of them between architectures to be removed. Once architectures are
converted to make use of this generic header it will also ensure that
all of phys_to_virt, virt_to_phys, map_physmem & unmap_physmem are
provided. The 2 families of functions differ in that map_physmem may
create dynamic mappings whilst phys_to_virt may not & therefore is more
limited in scope but doesn't require information such as a length &
flags.

This patch doesn't convert any architectures to make use of this generic
header - later patches in the series will do so.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>
Cc: Alison Wang <alison.wang@freescale.com>
Cc: Angelo Dureghello <angelo@sysam.it>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
Cc: Francois Retief <fgretief@spaceteq.co.za>
Cc: Macpaul Lin <macpaul@andestech.com>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Cc: Scott McNutt <smcnutt@psyent.com>
Cc: Sonic Zhang <sonic.adi@gmail.com>
Cc: Thomas Chou <thomas@wytron.com.tw>
Cc: Wolfgang Denk <wd@denx.de>
---

 include/asm-generic/io.h | 110 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 include/asm-generic/io.h

Hi Paul,

On 01/10/2016 16:19, Paul Burton wrote:
> Most architectures currently supported by U-Boot use trivial
> implementations of map_to_physmem & virt_to_phys which simply cast a
> physical address to a pointer for use a virtual address & vice-versa.
> This results in a lot of duplicate implementations of these mapping
> functions.
>
> The functions provided by different architectures also differs, with
> some having implementations of phys_to_virt & others not. A later patch
> in this series will make use of phys_to_virt, so requires that it be
> provided for all architectures.
>
> This patch introduces an asm-generic/io.h which provides generic
> implementations of address mapping functions, allowing the duplication
> of them between architectures to be removed. Once architectures are
> converted to make use of this generic header it will also ensure that
> all of phys_to_virt, virt_to_phys, map_physmem & unmap_physmem are
> provided. The 2 families of functions differ in that map_physmem may
> create dynamic mappings whilst phys_to_virt may not & therefore is more
> limited in scope but doesn't require information such as a length &
> flags.
>
> This patch doesn't convert any architectures to make use of this generic
> header - later patches in the series will do so.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>
> Cc: Alison Wang <alison.wang@freescale.com>
> Cc: Angelo Dureghello <angelo@sysam.it>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> Cc: Francois Retief <fgretief@spaceteq.co.za>
> Cc: Macpaul Lin <macpaul@andestech.com>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Scott McNutt <smcnutt@psyent.com>
> Cc: Sonic Zhang <sonic.adi@gmail.com>
> Cc: Thomas Chou <thomas@wytron.com.tw>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>
>  include/asm-generic/io.h | 110 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 include/asm-generic/io.h
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> new file mode 100644
> index 0000000..dd3a46d
> --- /dev/null
> +++ b/include/asm-generic/io.h
> @@ -0,0 +1,110 @@
> +/*
> + * Generic I/O functions.
> + *
> + * Copyright (c) 2016 Imagination Technologies Ltd.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __ASM_GENERIC_IO_H__
> +#define __ASM_GENERIC_IO_H__
> +
> +/*
> + * This file should be included at the end of each architecture-specific
> + * asm/io.h such that we may provide generic implementations without
> + * conflicting with architecture-specific code.
> + */
> +
> +#ifndef __ASSEMBLY__
> +
> +/**
> + * phys_to_virt() - Return a virtual address mapped to a given physical address
> + * @paddr: the physical address
> + *
> + * Returns a virtual address which the CPU can access that maps to the physical
> + * address @paddr. This should only be used where it is known that no dynamic
> + * mapping is required. In general, map_physmem should be used instead.
> + *
> + * Returns: a virtual address which maps to @paddr
> + */
> +#ifndef phys_to_virt
> +static inline void *phys_to_virt(phys_addr_t paddr)
> +{
> +	return (void *)(unsigned long)paddr;
> +}
> +#endif
> +
> +/**
> + * virt_to_phys() - Return the physical address that a virtual address maps to
> + * @vaddr: the virtual address
> + *
> + * Returns the physical address which the CPU-accessible virtual address @vaddr
> + * maps to.
> + *
> + * Returns: the physical address which @vaddr maps to
> + */
> +#ifndef virt_to_phys
> +static inline phys_addr_t virt_to_phys(void *vaddr)
> +{
> +	return (phys_addr_t)((unsigned long)vaddr);
> +}
> +#endif
> +
> +/*
> + * Flags for use with map_physmem() & unmap_physmem(). Architectures need not
> + * support all of these, in which case they will be defined as zero here &
> + * ignored. Callers that may run on multiple architectures should therefore
> + * treat them as hints rather than requirements.
> + */
> +#ifndef MAP_NOCACHE
> +# define MAP_NOCACHE	0	/* Produce an uncached mapping */
> +#endif
> +#ifndef MAP_WRCOMBINE
> +# define MAP_WRCOMBINE	0	/* Allow write-combining on the mapping */
> +#endif
> +#ifndef MAP_WRBACK
> +# define MAP_WRBACK	0	/* Map using write-back caching */
> +#endif
> +#ifndef MAP_WRTHROUGH
> +# define MAP_WRTHROUGH	0	/* Map using write-through caching */
> +#endif
> +
> +/**
> + * map_physmem() - Return a virtual address mapped to a given physical address
> + * @paddr: the physical address
> + * @len: the length of the required mapping
> + * @flags: flags affecting the type of mapping
> + *
> + * Return a virtual address through which the CPU may access the memory at
> + * physical address @paddr. The mapping will be valid for at least @len bytes,
> + * and may be affected by flags passed to the @flags argument. This function
> + * may create new mappings, so should generally be paired with a matching call
> + * to unmap_physmem once the caller is finished with the memory in question.
> + *
> + * Returns: a virtual address suitably mapped to @paddr
> + */
> +#ifndef map_physmem
> +static inline void *
> +map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
> +{
> +	return phys_to_virt(paddr);
> +}
> +#endif
> +
> +/**
> + * unmap_physmem() - Remove mappings created by a prior call to map_physmem()
> + * @vaddr: the virtual address which map_physmem() previously returned
> + * @flags: flags matching those originally passed to map_physmem()
> + *
> + * Unmap memory which was previously mapped by a call to map_physmem(). If
> + * map_physmem() dynamically created a mapping for the memory in question then
> + * unmap_physmem() will remove that mapping.
> + */
> +#ifndef unmap_physmem
> +static inline void unmap_physmem(void *vaddr, unsigned long flags)
> +{
> +}
> +#endif
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* __ASM_GENERIC_IO_H__ */
>

tested on mcf5307 amcore board, (that btw is not using those functions),
tested buildall for m68k boards,

Acked-by: Angelo Dureghello <angelo at sysam.it>
Tested-by: Angelo Dureghello <angelo at sysam.it>

many thanks,
Best regards,
Angelo Dureghello

Hi Paul,

On 1 October 2016 at 08:19, Paul Burton <paul.burton@imgtec.com> wrote:
> Most architectures currently supported by U-Boot use trivial
> implementations of map_to_physmem & virt_to_phys which simply cast a
> physical address to a pointer for use a virtual address & vice-versa.
> This results in a lot of duplicate implementations of these mapping
> functions.
>
> The functions provided by different architectures also differs, with
> some having implementations of phys_to_virt & others not. A later patch
> in this series will make use of phys_to_virt, so requires that it be
> provided for all architectures.
>
> This patch introduces an asm-generic/io.h which provides generic
> implementations of address mapping functions, allowing the duplication
> of them between architectures to be removed. Once architectures are
> converted to make use of this generic header it will also ensure that
> all of phys_to_virt, virt_to_phys, map_physmem & unmap_physmem are
> provided. The 2 families of functions differ in that map_physmem may
> create dynamic mappings whilst phys_to_virt may not & therefore is more
> limited in scope but doesn't require information such as a length &
> flags.
>
> This patch doesn't convert any architectures to make use of this generic
> header - later patches in the series will do so.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>
> Cc: Alison Wang <alison.wang@freescale.com>
> Cc: Angelo Dureghello <angelo@sysam.it>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> Cc: Francois Retief <fgretief@spaceteq.co.za>
> Cc: Macpaul Lin <macpaul@andestech.com>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Scott McNutt <smcnutt@psyent.com>
> Cc: Sonic Zhang <sonic.adi@gmail.com>
> Cc: Thomas Chou <thomas@wytron.com.tw>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>
>  include/asm-generic/io.h | 110 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 include/asm-generic/io.h

Reviewed-by: Simon Glass <sjg@chromium.org>

Question and nits below.

>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> new file mode 100644
> index 0000000..dd3a46d
> --- /dev/null
> +++ b/include/asm-generic/io.h
> @@ -0,0 +1,110 @@
> +/*
> + * Generic I/O functions.
> + *
> + * Copyright (c) 2016 Imagination Technologies Ltd.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __ASM_GENERIC_IO_H__
> +#define __ASM_GENERIC_IO_H__
> +
> +/*
> + * This file should be included at the end of each architecture-specific
> + * asm/io.h such that we may provide generic implementations without
> + * conflicting with architecture-specific code.
> + */
> +
> +#ifndef __ASSEMBLY__
> +
> +/**
> + * phys_to_virt() - Return a virtual address mapped to a given physical address
> + * @paddr: the physical address
> + *

@return

> + * Returns a virtual address which the CPU can access that maps to the physical
> + * address @paddr. This should only be used where it is known that no dynamic
> + * mapping is required. In general, map_physmem should be used instead.
> + *
> + * Returns: a virtual address which maps to @paddr

Why two Returns?

> + */
> +#ifndef phys_to_virt
> +static inline void *phys_to_virt(phys_addr_t paddr)
> +{
> +       return (void *)(unsigned long)paddr;
> +}
> +#endif
> +
> +/**
> + * virt_to_phys() - Return the physical address that a virtual address maps to
> + * @vaddr: the virtual address
> + *

@return

> + * Returns the physical address which the CPU-accessible virtual address @vaddr
> + * maps to.
> + *
> + * Returns: the physical address which @vaddr maps to

Why two Returns?

> + */
> +#ifndef virt_to_phys
> +static inline phys_addr_t virt_to_phys(void *vaddr)
> +{
> +       return (phys_addr_t)((unsigned long)vaddr);
> +}
> +#endif
> +
> +/*
> + * Flags for use with map_physmem() & unmap_physmem(). Architectures need not
> + * support all of these, in which case they will be defined as zero here &
> + * ignored. Callers that may run on multiple architectures should therefore
> + * treat them as hints rather than requirements.
> + */
> +#ifndef MAP_NOCACHE
> +# define MAP_NOCACHE   0       /* Produce an uncached mapping */
> +#endif
> +#ifndef MAP_WRCOMBINE
> +# define MAP_WRCOMBINE 0       /* Allow write-combining on the mapping */
> +#endif
> +#ifndef MAP_WRBACK
> +# define MAP_WRBACK    0       /* Map using write-back caching */
> +#endif
> +#ifndef MAP_WRTHROUGH
> +# define MAP_WRTHROUGH 0       /* Map using write-through caching */
> +#endif

It seems odd to make these 0 when not supported. How could an arch
know that it was requested, and then complain when an unsupported flag
is passed?

> +
> +/**
> + * map_physmem() - Return a virtual address mapped to a given physical address
> + * @paddr: the physical address
> + * @len: the length of the required mapping
> + * @flags: flags affecting the type of mapping

@return

> + *
> + * Return a virtual address through which the CPU may access the memory at
> + * physical address @paddr. The mapping will be valid for at least @len bytes,
> + * and may be affected by flags passed to the @flags argument. This function
> + * may create new mappings, so should generally be paired with a matching call
> + * to unmap_physmem once the caller is finished with the memory in question.
> + *

@return

> + * Returns: a virtual address suitably mapped to @paddr
> + */
> +#ifndef map_physmem
> +static inline void *
> +map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)

Can you join those two lines and wrap the args if needed??

> +{
> +       return phys_to_virt(paddr);
> +}
> +#endif
> +
> +/**
> + * unmap_physmem() - Remove mappings created by a prior call to map_physmem()
> + * @vaddr: the virtual address which map_physmem() previously returned
> + * @flags: flags matching those originally passed to map_physmem()
> + *
> + * Unmap memory which was previously mapped by a call to map_physmem(). If
> + * map_physmem() dynamically created a mapping for the memory in question then
> + * unmap_physmem() will remove that mapping.
> + */
> +#ifndef unmap_physmem
> +static inline void unmap_physmem(void *vaddr, unsigned long flags)
> +{
> +}
> +#endif
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* __ASM_GENERIC_IO_H__ */
> --
> 2.10.0
>
REgards,
Simon

On Monday, 3 October 2016 15:49:33 GMT Simon Glass wrote:
> Hi Paul,
> 
> On 1 October 2016 at 08:19, Paul Burton <paul.burton@imgtec.com> wrote:
> > Most architectures currently supported by U-Boot use trivial
> > implementations of map_to_physmem & virt_to_phys which simply cast a
> > physical address to a pointer for use a virtual address & vice-versa.
> > This results in a lot of duplicate implementations of these mapping
> > functions.
> > 
> > The functions provided by different architectures also differs, with
> > some having implementations of phys_to_virt & others not. A later patch
> > in this series will make use of phys_to_virt, so requires that it be
> > provided for all architectures.
> > 
> > This patch introduces an asm-generic/io.h which provides generic
> > implementations of address mapping functions, allowing the duplication
> > of them between architectures to be removed. Once architectures are
> > converted to make use of this generic header it will also ensure that
> > all of phys_to_virt, virt_to_phys, map_physmem & unmap_physmem are
> > provided. The 2 families of functions differ in that map_physmem may
> > create dynamic mappings whilst phys_to_virt may not & therefore is more
> > limited in scope but doesn't require information such as a length &
> > flags.
> > 
> > This patch doesn't convert any architectures to make use of this generic
> > header - later patches in the series will do so.
> > 
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>
> > Cc: Alison Wang <alison.wang@freescale.com>
> > Cc: Angelo Dureghello <angelo@sysam.it>
> > Cc: Bin Meng <bmeng.cn@gmail.com>
> > Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> > Cc: Francois Retief <fgretief@spaceteq.co.za>
> > Cc: Macpaul Lin <macpaul@andestech.com>
> > Cc: Michal Simek <monstr@monstr.eu>
> > Cc: Mike Frysinger <vapier@gentoo.org>
> > Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> > Cc: Scott McNutt <smcnutt@psyent.com>
> > Cc: Sonic Zhang <sonic.adi@gmail.com>
> > Cc: Thomas Chou <thomas@wytron.com.tw>
> > Cc: Wolfgang Denk <wd@denx.de>
> > ---
> > 
> >  include/asm-generic/io.h | 110
> >  +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110
> >  insertions(+)
> >  create mode 100644 include/asm-generic/io.h
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Question and nits below.
> 
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > new file mode 100644
> > index 0000000..dd3a46d
> > --- /dev/null
> > +++ b/include/asm-generic/io.h
> > @@ -0,0 +1,110 @@
> > +/*
> > + * Generic I/O functions.
> > + *
> > + * Copyright (c) 2016 Imagination Technologies Ltd.
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#ifndef __ASM_GENERIC_IO_H__
> > +#define __ASM_GENERIC_IO_H__
> > +
> > +/*
> > + * This file should be included at the end of each architecture-specific
> > + * asm/io.h such that we may provide generic implementations without
> > + * conflicting with architecture-specific code.
> > + */
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +/**
> > + * phys_to_virt() - Return a virtual address mapped to a given physical
> > address + * @paddr: the physical address
> > + *
> 
> @return

Hi Simon,

I was under the impression that we're following the kernel-doc style, both 
based on the style of existing comments & the statement from the CodingStyle 
page of the wiki:

  U-Boot adopted the kernel-doc annotation style, this is the only exception
  from multi-line comment rule of Coding Style. While not mandatory, adding
  documentation is strongly advised. The Linux kernel kernel-doc document
  applies with no changes.

  (From http://www.denx.de/wiki/U-Boot/CodingStyle)

The kernel-doc-nano-HOWTO.txt file linked to from that wiki paragraph & 
included in the Linux kernel source shows this example:

  /**
   * foobar() - short function description of foobar
   * @arg1:	Describe the first argument to foobar.
   * @arg2:	Describe the second argument to foobar.
   *		One can provide multiple line descriptions
   *		for arguments.
   *
   * A longer description, with more discussion of the function foobar()
   * that might be useful to those using or modifying it.  Begins with
   * empty comment line, and may include additional embedded empty
   * comment lines.
   *
   * The longer description can have multiple paragraphs.
   *
   * Return: Describe the return value of foobar.
   */

Nowhere does it use @return & that's not what's done in Linux, so my belief is 
that having a "Return:" line at the end of the comment is the right way.

> 
> > + * Returns a virtual address which the CPU can access that maps to the
> > physical + * address @paddr. This should only be used where it is known
> > that no dynamic + * mapping is required. In general, map_physmem should
> > be used instead. + *
> > + * Returns: a virtual address which maps to @paddr
> 
> Why two Returns?

The first is just a part of the paragraph explaining what the function does. I 
could start it with "This function returns" if you really want, but I don't 
see as that makes it any clearer. The second is the kernel-doc style 
description of what the function returns, as described above.

> 
> > + */
> > +#ifndef phys_to_virt
> > +static inline void *phys_to_virt(phys_addr_t paddr)
> > +{
> > +       return (void *)(unsigned long)paddr;
> > +}
> > +#endif
> > +
> > +/**
> > + * virt_to_phys() - Return the physical address that a virtual address
> > maps to + * @vaddr: the virtual address
> > + *
> 
> @return

Ditto.

> 
> > + * Returns the physical address which the CPU-accessible virtual address
> > @vaddr + * maps to.
> > + *
> > + * Returns: the physical address which @vaddr maps to
> 
> Why two Returns?

Ditto.

> 
> > + */
> > +#ifndef virt_to_phys
> > +static inline phys_addr_t virt_to_phys(void *vaddr)
> > +{
> > +       return (phys_addr_t)((unsigned long)vaddr);
> > +}
> > +#endif
> > +
> > +/*
> > + * Flags for use with map_physmem() & unmap_physmem(). Architectures need
> > not + * support all of these, in which case they will be defined as zero
> > here & + * ignored. Callers that may run on multiple architectures should
> > therefore + * treat them as hints rather than requirements.
> > + */
> > +#ifndef MAP_NOCACHE
> > +# define MAP_NOCACHE   0       /* Produce an uncached mapping */
> > +#endif
> > +#ifndef MAP_WRCOMBINE
> > +# define MAP_WRCOMBINE 0       /* Allow write-combining on the mapping */
> > +#endif
> > +#ifndef MAP_WRBACK
> > +# define MAP_WRBACK    0       /* Map using write-back caching */
> > +#endif
> > +#ifndef MAP_WRTHROUGH
> > +# define MAP_WRTHROUGH 0       /* Map using write-through caching */
> > +#endif
> 
> It seems odd to make these 0 when not supported. How could an arch
> know that it was requested, and then complain when an unsupported flag
> is passed?

That's true, it couldn't unless it explicitly defined ones that it doesn't 
support. Given that architectures already #define each of the above to 0 in 
the cases that they don't support this doesn't change that problem though. I'd 
suggest that we improve this separately, otherwise we'd risk this change 
breaking anything which sets flags that some architectures might simply want 
to ignore.

> 
> > +
> > +/**
> > + * map_physmem() - Return a virtual address mapped to a given physical
> > address + * @paddr: the physical address
> > + * @len: the length of the required mapping
> > + * @flags: flags affecting the type of mapping
> 
> @return

See above.

> 
> > + *
> > + * Return a virtual address through which the CPU may access the memory
> > at
> > + * physical address @paddr. The mapping will be valid for at least @len
> > bytes, + * and may be affected by flags passed to the @flags argument.
> > This function + * may create new mappings, so should generally be paired
> > with a matching call + * to unmap_physmem once the caller is finished
> > with the memory in question. + *
> 
> @return

Ditto.

> 
> > + * Returns: a virtual address suitably mapped to @paddr
> > + */
> > +#ifndef map_physmem
> > +static inline void *
> > +map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
> 
> Can you join those two lines and wrap the args if needed??

Sure.

Thanks,
    Paul

> 
> > +{
> > +       return phys_to_virt(paddr);
> > +}
> > +#endif
> > +
> > +/**
> > + * unmap_physmem() - Remove mappings created by a prior call to
> > map_physmem() + * @vaddr: the virtual address which map_physmem()
> > previously returned + * @flags: flags matching those originally passed to
> > map_physmem() + *
> > + * Unmap memory which was previously mapped by a call to map_physmem().
> > If
> > + * map_physmem() dynamically created a mapping for the memory in question
> > then + * unmap_physmem() will remove that mapping.
> > + */
> > +#ifndef unmap_physmem
> > +static inline void unmap_physmem(void *vaddr, unsigned long flags)
> > +{
> > +}
> > +#endif
> > +
> > +#endif /* !__ASSEMBLY__ */
> > +#endif /* __ASM_GENERIC_IO_H__ */
> > --
> > 2.10.0
> 
> REgards,
> Simon

Hi Paul,

On 17 November 2016 at 08:32, Paul Burton <paul.burton@imgtec.com> wrote:
> On Monday, 3 October 2016 15:49:33 GMT Simon Glass wrote:
>> Hi Paul,
>>
>> On 1 October 2016 at 08:19, Paul Burton <paul.burton@imgtec.com> wrote:
>> > Most architectures currently supported by U-Boot use trivial
>> > implementations of map_to_physmem & virt_to_phys which simply cast a
>> > physical address to a pointer for use a virtual address & vice-versa.
>> > This results in a lot of duplicate implementations of these mapping
>> > functions.
>> >
>> > The functions provided by different architectures also differs, with
>> > some having implementations of phys_to_virt & others not. A later patch
>> > in this series will make use of phys_to_virt, so requires that it be
>> > provided for all architectures.
>> >
>> > This patch introduces an asm-generic/io.h which provides generic
>> > implementations of address mapping functions, allowing the duplication
>> > of them between architectures to be removed. Once architectures are
>> > converted to make use of this generic header it will also ensure that
>> > all of phys_to_virt, virt_to_phys, map_physmem & unmap_physmem are
>> > provided. The 2 families of functions differ in that map_physmem may
>> > create dynamic mappings whilst phys_to_virt may not & therefore is more
>> > limited in scope but doesn't require information such as a length &
>> > flags.
>> >
>> > This patch doesn't convert any architectures to make use of this generic
>> > header - later patches in the series will do so.
>> >
>> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>> > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>> > Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>
>> > Cc: Alison Wang <alison.wang@freescale.com>
>> > Cc: Angelo Dureghello <angelo@sysam.it>
>> > Cc: Bin Meng <bmeng.cn@gmail.com>
>> > Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>> > Cc: Francois Retief <fgretief@spaceteq.co.za>
>> > Cc: Macpaul Lin <macpaul@andestech.com>
>> > Cc: Michal Simek <monstr@monstr.eu>
>> > Cc: Mike Frysinger <vapier@gentoo.org>
>> > Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
>> > Cc: Scott McNutt <smcnutt@psyent.com>
>> > Cc: Sonic Zhang <sonic.adi@gmail.com>
>> > Cc: Thomas Chou <thomas@wytron.com.tw>
>> > Cc: Wolfgang Denk <wd@denx.de>
>> > ---
>> >
>> >  include/asm-generic/io.h | 110
>> >  +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110
>> >  insertions(+)
>> >  create mode 100644 include/asm-generic/io.h
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Question and nits below.
>>
>> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>> > new file mode 100644
>> > index 0000000..dd3a46d
>> > --- /dev/null
>> > +++ b/include/asm-generic/io.h
>> > @@ -0,0 +1,110 @@
>> > +/*
>> > + * Generic I/O functions.
>> > + *
>> > + * Copyright (c) 2016 Imagination Technologies Ltd.
>> > + *
>> > + * SPDX-License-Identifier:    GPL-2.0+
>> > + */
>> > +
>> > +#ifndef __ASM_GENERIC_IO_H__
>> > +#define __ASM_GENERIC_IO_H__
>> > +
>> > +/*
>> > + * This file should be included at the end of each architecture-specific
>> > + * asm/io.h such that we may provide generic implementations without
>> > + * conflicting with architecture-specific code.
>> > + */
>> > +
>> > +#ifndef __ASSEMBLY__
>> > +
>> > +/**
>> > + * phys_to_virt() - Return a virtual address mapped to a given physical
>> > address + * @paddr: the physical address
>> > + *
>>
>> @return
>
> Hi Simon,
>
> I was under the impression that we're following the kernel-doc style, both
> based on the style of existing comments & the statement from the CodingStyle
> page of the wiki:
>
>   U-Boot adopted the kernel-doc annotation style, this is the only exception
>   from multi-line comment rule of Coding Style. While not mandatory, adding
>   documentation is strongly advised. The Linux kernel kernel-doc document
>   applies with no changes.
>
>   (From http://www.denx.de/wiki/U-Boot/CodingStyle)
>
> The kernel-doc-nano-HOWTO.txt file linked to from that wiki paragraph &
> included in the Linux kernel source shows this example:
>
>   /**
>    * foobar() - short function description of foobar
>    * @arg1:     Describe the first argument to foobar.
>    * @arg2:     Describe the second argument to foobar.
>    *            One can provide multiple line descriptions
>    *            for arguments.
>    *
>    * A longer description, with more discussion of the function foobar()
>    * that might be useful to those using or modifying it.  Begins with
>    * empty comment line, and may include additional embedded empty
>    * comment lines.
>    *
>    * The longer description can have multiple paragraphs.
>    *
>    * Return: Describe the return value of foobar.
>    */
>
> Nowhere does it use @return & that's not what's done in Linux, so my belief is
> that having a "Return:" line at the end of the comment is the right way.

Well there are about 900 instances of @return in Linux and U-Boot has
both as well. Does the docbook tool decode 'Returns' just as well as
@return?

Regards,
Simon