[U-Boot,2/3] remoteproc: Introduce a sandbox dummy driver

Introduce a dummy driver for sandbox that allows us to verify basic
functionality. This is not meant to do anything functional - but is
more or less meant as a framework plumbing debug helper.

The sandbox remoteproc driver maintains absolutey no states and is a
simple driver which just is filled with empty hooks. Idea being to give
an approximate idea to implement own remoteproc driver using this as a
template.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/remoteproc/Kconfig            |   9 ++
 drivers/remoteproc/Makefile           |   3 +
 drivers/remoteproc/sandbox_testproc.c | 243 ++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+)
 create mode 100644 drivers/remoteproc/sandbox_testproc.c

Hi Nishanth,

On 24 August 2015 at 11:28, Nishanth Menon <nm@ti.com> wrote:
> Introduce a dummy driver for sandbox that allows us to verify basic
> functionality. This is not meant to do anything functional - but is
> more or less meant as a framework plumbing debug helper.
>
> The sandbox remoteproc driver maintains absolutey no states and is a
> simple driver which just is filled with empty hooks. Idea being to give
> an approximate idea to implement own remoteproc driver using this as a
> template.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  drivers/remoteproc/Kconfig            |   9 ++
>  drivers/remoteproc/Makefile           |   3 +
>  drivers/remoteproc/sandbox_testproc.c | 243 ++++++++++++++++++++++++++++++++++
>  3 files changed, 255 insertions(+)
>  create mode 100644 drivers/remoteproc/sandbox_testproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 700f52caf1dc..61cc506d6baa 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -12,4 +12,13 @@ config DM_REMOTEPROC
>         bool
>         depends on DM
>
> +# Please keep the configuration alphabetically sorted.
> +config DM_TESTPROC_SANDBOX

Should this be REMOTEPROC_SANDBOX?

> +       bool "Support for Test processor for Sandbox"
> +       select DM_REMOTEPROC
> +       depends on DM
> +       depends on SANDBOX
> +       help
> +         Say 'y' here to add support for test processor which does dummy
> +         operations for sandbox platform.
>  endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index af90ace95899..57c04d0e024a 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -5,3 +5,6 @@
>  #
>
>  obj-$(CONFIG_DM_REMOTEPROC) += rproc-uclass.o
> +
> +# Remote proc drivers - Please keep this list alphabetically sorted.
> +obj-$(CONFIG_DM_TESTPROC_SANDBOX) += sandbox_testproc.o
> diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
> new file mode 100644
> index 000000000000..e942f66fc2dc
> --- /dev/null
> +++ b/drivers/remoteproc/sandbox_testproc.c
> @@ -0,0 +1,243 @@
> +/*
> + * (C) Copyright 2015
> + * Texas Instruments Incorporated - http://www.ti.com/
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <remoteproc.h>
> +
> +enum sandbox_state {
> +       sb_entry,
> +       sb_init,
> +       sb_reset,
> +       sb_loaded,
> +       sb_running
> +};
> +
> +struct sandbox_test_devdata {
> +       enum sandbox_state current_state;
> +};
> +
> +static int sandbox_dev_move_to_state(struct udevice *dev,
> +                                    enum sandbox_state next_state)
> +{
> +       struct sandbox_test_devdata *ddata = dev_get_priv(dev);
> +
> +       /* No state transition is OK */
> +       if (ddata->current_state == next_state)
> +               return 0;
> +
> +       debug("current_state=%d, next_state=%d\n", ddata->current_state, next_state);
> +       switch (ddata->current_state) {
> +       case sb_entry:
> +               if (next_state == sb_init)
> +                       goto ok_state;
> +               break;
> +
> +       case sb_init:
> +               if (next_state == sb_reset || next_state == sb_loaded)
> +                       goto ok_state;
> +               break;
> +
> +       case sb_reset:
> +               if (next_state == sb_loaded || next_state == sb_init)
> +                       goto ok_state;
> +               break;
> +
> +       case sb_loaded:
> +               if (next_state == sb_reset || next_state == sb_init ||
> +                   next_state == sb_running)
> +                       goto ok_state;
> +               break;
> +
> +       case sb_running:
> +               if (next_state == sb_reset || next_state == sb_init)
> +                       goto ok_state;
> +               break;
> +       };
> +       return -EINVAL;
> +
> +ok_state:
> +       ddata->current_state = next_state;
> +       return 0;
> +}
> +
> +static int sandbox_testproc_probe(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       struct sandbox_test_devdata *ddata;
> +       int ret;
> +
> +       ddata = dev_get_priv(dev);
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +       if (!uc_pdata || !ddata) {
> +               printf("%s: platform data %p OR driver data %p missing\n",
> +                      dev->name, uc_pdata, ddata);
> +               return -EINVAL;
> +       }
> +       ret = sandbox_dev_move_to_state(dev, sb_entry);
> +
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> +       return ret;
> +}
> +
> +static int sandbox_testproc_init(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       int ret;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +
> +       ret = sandbox_dev_move_to_state(dev, sb_init);
> +
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> +       if (ret)
> +               printf("%s init failed\n", uc_pdata->name);
> +
> +       return ret;
> +}
> +
> +static int sandbox_testproc_reset(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       int ret;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +
> +       ret = sandbox_dev_move_to_state(dev, sb_reset);
> +
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> +       if (ret)
> +               printf("%s reset failed\n", uc_pdata->name);
> +       return ret;
> +}
> +
> +static int sandbox_testproc_load(struct udevice *dev, ulong addr, ulong size)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       int ret;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +
> +       ret = sandbox_dev_move_to_state(dev, sb_loaded);
> +
> +       debug("%s: called(%d) Loading to %08lX %lu size\n",
> +             uc_pdata->name, ret, addr, size);
> +
> +       if (ret)
> +               printf("%s load failed\n", uc_pdata->name);
> +       return ret;
> +}
> +
> +static int sandbox_testproc_start(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       int ret;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +
> +       ret = sandbox_dev_move_to_state(dev, sb_running);
> +
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> +       if (ret)
> +               printf("%s start failed\n", uc_pdata->name);
> +       return ret;
> +}
> +
> +static int sandbox_testproc_stop(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       int ret;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +
> +       ret = sandbox_dev_move_to_state(dev, sb_init);
> +
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> +       if (ret)
> +               printf("%s stop failed\n", uc_pdata->name);
> +       return ret;
> +}
> +
> +static int sandbox_testproc_is_running(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       struct sandbox_test_devdata *ddata;
> +       int ret = 1;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +       ddata = dev_get_priv(dev);
> +
> +       if (ddata->current_state == sb_running)
> +               ret = 0;
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);

should that say "is_running called" / do you need __func__?

> +
> +       return ret;
> +}
> +
> +static int sandbox_testproc_ping(struct udevice *dev)
> +{
> +       struct dm_rproc_uclass_pdata *uc_pdata;
> +       struct sandbox_test_devdata *ddata;
> +       int ret = 1;
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +       ddata = dev_get_priv(dev);
> +
> +       if (ddata->current_state == sb_running)
> +               ret = 0;
> +       else
> +               ret = -EINVAL;
> +
> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> +       if (ret) {
> +               printf("%s: No response.(Not started?)\n", uc_pdata->name);
> +       }
> +
> +       return ret;
> +}
> +
> +static const struct dm_rproc_ops sandbox_testproc_ops = {
> +       .init = sandbox_testproc_init,
> +       .reset = sandbox_testproc_reset,
> +       .load = sandbox_testproc_load,
> +       .start = sandbox_testproc_start,
> +       .stop = sandbox_testproc_stop,
> +       .is_running = sandbox_testproc_is_running,
> +       .ping = sandbox_testproc_ping,
> +};
> +
> +static const struct udevice_id sandbox_ids[] = {
> +       {.compatible = "sandbox,test-processor"},
> +       {}
> +};
> +
> +U_BOOT_DRIVER(sandbox_testproc) = {
> +       /* *INDENT-OFF* */

What is that for? ^^

> +       .name = "sandbox_test_proc",
> +       .of_match = sandbox_ids,
> +       .id = UCLASS_RPROC,
> +       .ops = &sandbox_testproc_ops,
> +       .probe = sandbox_testproc_probe,
> +       .priv_auto_alloc_size = sizeof(struct sandbox_test_devdata),
> +       /* *INDENT-ON* */
> +};
> +
> +static struct dm_rproc_uclass_pdata proc_3_test = {
> +       .name = "proc_3_legacy",
> +       .mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
> +};
> +
> +U_BOOT_DEVICE(proc_3_demo) = {
> +       /* *INDENT-OFF* */
> +       .name = "sandbox_test_proc",
> +       .platdata = &proc_3_test,
> +       /* *INDENT-ON* */
> +};
> --
> 2.1.4
>

Regards,
Simon

On 08/25/2015 12:04 AM, Simon Glass wrote:
[...]
>> +# Please keep the configuration alphabetically sorted.
>> +config DM_TESTPROC_SANDBOX
> 
> Should this be REMOTEPROC_SANDBOX?

Yep - will do.

>> diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
>> new file mode 100644
>> index 000000000000..e942f66fc2dc
>> --- /dev/null
>> +++ b/drivers/remoteproc/sandbox_testproc.c
>> @@ -0,0 +1,243 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Texas Instruments Incorporated - http://www.ti.com/
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +#define pr_fmt(fmt) "%s: " fmt, __func__
^^

>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <remoteproc.h>

[...]

>> +
>> +static int sandbox_testproc_is_running(struct udevice *dev)
>> +{
>> +       struct dm_rproc_uclass_pdata *uc_pdata;
>> +       struct sandbox_test_devdata *ddata;
>> +       int ret = 1;
>> +
>> +       uc_pdata = dev_get_uclass_platdata(dev);
>> +       ddata = dev_get_priv(dev);
>> +
>> +       if (ddata->current_state == sb_running)
>> +               ret = 0;
>> +       debug("%s: called(%d)\n", uc_pdata->name, ret);
> 
> should that say "is_running called" / do you need __func__?

pr_fmt (at the start of the file) should take care of it.

[...]

>> +U_BOOT_DRIVER(sandbox_testproc) = {
>> +       /* *INDENT-OFF* */
> 
> What is that for? ^^

trying to keep indent sane - will drop.


Thanks once again for the review.