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

[Simon Glass]

Hi Paul,

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