[04/11] persistent_ram: Introduce persistent_ram_new()

The routine just creates a persistent ram zone at a specified address.

For persistent_ram_init_ringbuffer() we'd need to add a
'struct persistent_ram' to the global list, and associate it with a
device. We don't need all this complexity in pstore_ram, so we introduce
the simple function.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 drivers/staging/android/persistent_ram.c |   26 ++++++++++++++++++++++++++
 drivers/staging/android/persistent_ram.h |    4 ++++
 2 files changed, 30 insertions(+)

On Fri, May 11, 2012 at 5:17 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> The routine just creates a persistent ram zone at a specified address.
>
> For persistent_ram_init_ringbuffer() we'd need to add a
> 'struct persistent_ram' to the global list, and associate it with a
> device. We don't need all this complexity in pstore_ram, so we introduce
> the simple function.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
>  drivers/staging/android/persistent_ram.c |   26 ++++++++++++++++++++++++++
>  drivers/staging/android/persistent_ram.h |    4 ++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
> index ec23822..c0c3d32 100644
> --- a/drivers/staging/android/persistent_ram.c
> +++ b/drivers/staging/android/persistent_ram.c
> @@ -412,6 +412,32 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
>        return 0;
>  }
>
> +struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
> +                                                      size_t size,
> +                                                      bool ecc)
> +{
> +       struct persistent_ram_zone *prz;
> +       int ret = -ENOMEM;
> +
> +       prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
> +       if (!prz) {
> +               pr_err("persistent_ram: failed to allocate persistent ram zone\n");
> +               goto err;
> +       }
> +
> +       ret = persistent_ram_buffer_map(start, size, prz);
> +       if (ret)
> +               goto err;
> +
> +       persistent_ram_post_init(prz, ecc);
> +       persistent_ram_update_header_ecc(prz);
> +
> +       return prz;
> +err:
> +       kfree(prz);
> +       return ERR_PTR(ret);
> +}
> +
>  static  __init
>  struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
>  {
> diff --git a/drivers/staging/android/persistent_ram.h b/drivers/staging/android/persistent_ram.h
> index 5635355..8154d15 100644
> --- a/drivers/staging/android/persistent_ram.h
> +++ b/drivers/staging/android/persistent_ram.h
> @@ -19,6 +19,7 @@
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/types.h>
> +#include <linux/init.h>
>
>  struct persistent_ram_buffer;
>
> @@ -62,6 +63,9 @@ struct persistent_ram_zone {
>
>  int persistent_ram_early_init(struct persistent_ram *ram);
>
> +struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
> +                                                      size_t size,
> +                                                      bool ecc);
>  struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
>                bool ecc);
>
> --
> 1.7.9.2
>

Overall I like this series, but I'm not sure I agree with
persistent_ram_new().  The point of persistent_ram_early_init with the
call to memblock_reserve and the persistent_ram_descriptor list was to
have a single pool of persistent memory that could be parcelled out to
whatever drivers needed it, keeping the code out of the board file.
With persistent_ram_new, the board file is now responsible for making
sure that the memory has been reserved with memblock_reserve(), or,
even worse, mem= from the bootloader.  Mixing the two methods together
would be confusing.  Either persistent_ram_early_init should be
removed completely (or replaced with something that is easier to
register ramoops into), or ramoops should use
persistent_ram_init_ringbuffer like ram_console does.

Hello Colin,

On Mon, May 14, 2012 at 05:37:56PM -0700, Colin Cross wrote:
[...]
> even worse, mem= from the bootloader.  Mixing the two methods together
> would be confusing.

Yes, mixing is discouraged. The mem= hack is mostly useful for
developers, for hacking random kernels. Even on x86 it is
useful, when you want to grab an oops, but you don't have say
netconsole, or HW really screwed up and you don't have any
means to get the oops log, ramoops may become quite useful.

But in the Android phone scenario, if you want to have this
feature into production kernels, platforms should register
the ramoops platform driver, as they were doing before.

> Either persistent_ram_early_init should be
> removed completely (or replaced with something that is easier to
> register ramoops into), or ramoops should use
> persistent_ram_init_ringbuffer like ram_console does.

Yep, this was indeed my original idea: persistent_ram_early_init
should go.

Boards (or generic arch/ or arch/mach-* code that knows memory
layout) will have to just do two things:

1. Wisely and early call memblock_reserve().
2. Register a ramoops platform device pointing to the reserved
   memory.

This is actually exactly the same as you were doing with
ram_console:

1. Platform were adding an entry to the global list of persistent
   ram zones, and then were calling persistent_ram_early_init()
   somewhere in the arch/ code (at least that's how I understood
   the idea of the code, as there are currently no in-tree users).
2. Then platforms were registering a ram_console platform device,
   and the driver would find out the needed zone by matching on
   the device name.

Thinking about it, the whole thing was actually abusing
the device-driver model a little bit. So things are just easier
now.

Thanks!