Hi Paul,
On 1 October 2016 at 08:19, Paul Burton <paul.bur...@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.bur...@imgtec.com>
> Cc: Albert Aribaud <albert.u.b...@aribaud.net>
> Cc: Alexey Brodkin <alexey.brod...@synopsys.com>
> Cc: Alison Wang <alison.w...@freescale.com>
> Cc: Angelo Dureghello <ang...@sysam.it>
> Cc: Bin Meng <bmeng...@gmail.com>
> Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com>
> Cc: Francois Retief <fgret...@spaceteq.co.za>
> Cc: Macpaul Lin <macp...@andestech.com>
> Cc: Michal Simek <mon...@monstr.eu>
> Cc: Mike Frysinger <vap...@gentoo.org>
> Cc: Nobuhiro Iwamatsu <iwama...@nigauri.org>
> Cc: Scott McNutt <smcn...@psyent.com>
> Cc: Sonic Zhang <sonic....@gmail.com>
> Cc: Thomas Chou <tho...@wytron.com.tw>
> Cc: Wolfgang Denk <w...@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 <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
