lkml.org 
[lkml]   [2012]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 6/7] remoteproc: Support custom firmware handlers
    Hi Sjur,

    On Wed, Jun 6, 2012 at 4:38 PM, <sjur.brandeland@stericsson.com> wrote:
    > From: Sjur Brændeland <sjur.brandeland@stericsson.com>
    >
    > Firmware handling is made customizable.
    > This is done by creating a separate ops structure for the
    > firmware functions that depends on a particular firmware
    > format (such as ELF). The ELF specific functions are exported
    > by the structure rproc_elf_fw_ops.
    >
    > Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
    > ---
    > @@ -171,6 +171,7 @@ static struct rproc_ops omap_rproc_ops = {
    >        .start          = omap_rproc_start,
    >        .stop           = omap_rproc_stop,
    >        .kick           = omap_rproc_kick,
    > +       .fw             = &rproc_elf_fw_ops

    It might be more appropriate to provide the binary format handlers as
    a separate argument to rproc_alloc, instead of adding it to the
    rproc_ops struct (rproc_ops is dedicated to platform-specific handlers
    that are implemented by the low-level driver).

    >
    >        rproc = kzalloc(sizeof(struct rproc) + len, GFP_KERNEL);
    > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
    > index 2017ae3..9a53871 100644
    > --- a/drivers/remoteproc/remoteproc_elf_loader.c
    > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
    > @@ -33,8 +33,8 @@
    >  #include "remoteproc_internal.h"
    >
    >  /* make sure this fw image is sane */
    > -int
    > -rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
    > +static int
    > +elf_sanity_check(struct rproc *rproc, const struct firmware *fw)

    Do we want to keep the "rproc" prefix for the function names (e.g.
    rproc_elf_sanity_check) ? we'll be better namespace citizens that way.

    > -int
    > -rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
    > -struct resource_table *
    > -rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
    > -                                                               int *tablesz);
    > -int
    > -rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
    > -void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw);

    Oh, great, you removed those. In that case we don't really care they'd
    stay in remoteproc.h for the transition.. (but that probably doesn't
    apply to rproc_da_to_va).

    > +static inline int
    > +rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
    > +{
    > +       if (rproc->ops->fw_ops->sanity_check)
    > +               return rproc->ops->fw_ops->sanity_check(rproc, fw);
    > +       return 0;
    > +}
    > +
    > +static inline
    > +void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
    > +{
    > +       if (rproc->ops->fw_ops->set_boot_addr)
    > +               rproc->ops->fw_ops->set_boot_addr(rproc, fw);
    > +}
    > +
    > +static inline int
    > +rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
    > +{
    > +       if (rproc->ops->fw_ops->load)
    > +               return rproc->ops->fw_ops->load(rproc, fw);
    > +       return -EINVAL;
    > +}
    > +static inline struct resource_table *
    > +rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
    > +                                                       int *tablesz)
    > +{
    > +       if (rproc->ops->fw_ops->find_rsc_table)
    > +               return rproc->ops->fw_ops->find_rsc_table(rproc, fw, tablesz);
    > +       return NULL;
    > +}
    > +
    > +extern struct rproc_fw_ops rproc_elf_fw_ops;

    I'd keep those too in remoteproc_internal.h, so they won't be
    accessible to code outside of drivers/remoteproc.

    Thanks!
    Ohad.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-06-10 19:41    [W:3.717 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site