lkml.org 
[lkml]   [2011]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/2] ramoops: use pstore interface
Date
On Wednesday 16 November 2011, Kees Cook wrote:
> Instead of using /dev/mem directly, use the common pstore infrastructure
> to handle Oops gathering and extraction.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Sounds like a very good plan to me. It probably makes sense to move the
entire driver into fs/pstore after this. Otherwise, I have only trivial
style comments:

> +static int ramoops_pstore_open(struct pstore_info *psi);
> +static int ramoops_pstore_close(struct pstore_info *psi);
> +static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> + struct timespec *time,
> + char **buf,
> + struct pstore_info *psi);
> +static int ramoops_pstore_write(enum pstore_type_id type,
> + enum kmsg_dump_reason reason, u64 *id,
> + unsigned int part,
> + size_t size, struct pstore_info *psi);
> +static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
> + struct pstore_info *psi);

Can you do it without forward declarations? Many people find code
more readable if it is structure in the natural order that avoids
these.

> +static int ramoops_pstore_close(struct pstore_info *psi)
> +{
> + return 0;
> +}

Do you actually have to provide this if it's empty?

If yes, it might make sense to change the pstore code so that
it works without a close function.

Arnd


\
 
 \ /
  Last update: 2011-11-17 16:09    [from the cache]
©2003-2011 Jasper Spaans