[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2b2e5eb3-1786-c772-fb60-d1f9c3ad9036@infradead.org>
Date: Thu, 25 Jan 2018 15:27:20 -0800
From: Randy Dunlap <rdunlap@...radead.org>
To: richard.gong@...ux.intel.com, arnd@...db.de,
gregkh@...uxfoundation.org
Cc: linux-kernel@...r.kernel.org, richard.gong@...el.com
Subject: Re: [PATCHv1] driver: misc: add Intel Stratix10 service layer driver
On 01/25/2018 08:39 AM, richard.gong@...ux.intel.com wrote:
> From: Richard Gong <richard.gong@...el.com>
>
>
> Signed-off-by: Richard Gong <richard.gong@...el.com>
> ---
> drivers/misc/Kconfig | 3 +-
> drivers/misc/Makefile | 3 +-
> drivers/misc/intel-service/Kconfig | 9 +
> drivers/misc/intel-service/Makefile | 2 +
> drivers/misc/intel-service/intel_service.c | 703 +++++++++++++++++++++++++++++
> include/linux/intel-service-client.h | 227 ++++++++++
> include/linux/intel-service.h | 122 +++++
> include/linux/intel-smc.h | 246 ++++++++++
> 8 files changed, 1313 insertions(+), 2 deletions(-)
> create mode 100644 drivers/misc/intel-service/Kconfig
> create mode 100644 drivers/misc/intel-service/Makefile
> create mode 100644 drivers/misc/intel-service/intel_service.c
> create mode 100644 include/linux/intel-service-client.h
> create mode 100644 include/linux/intel-service.h
> create mode 100644 include/linux/intel-smc.h
>
> diff --git a/drivers/misc/intel-service/intel_service.c b/drivers/misc/intel-service/intel_service.c
> new file mode 100644
> index 0000000..90f883c
> --- /dev/null
> +++ b/drivers/misc/intel-service/intel_service.c
> @@ -0,0 +1,703 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017-2018, Intel Corporation
> +
> +#include <linux/bitops.h>
> +#include <linux/completion.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/intel-service.h>
> +#include <linux/intel-service-client.h>
> +#include <linux/intel-smc.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kthread.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_address.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define SVC_FO_LEN 32
> +#define SVC_MAX_CHANNEL 2
> +
> +static LIST_HEAD(svc_cons);
> +static LIST_HEAD(svc_private_mem);
> +static DEFINE_MUTEX(svc_mutex);
> +
> +svc_invoke_fn *invoke_fn;
Is that calling into firmware or what?
> + /* timeout for polling FPGA configuration status
> + * it is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC
> + */
Fix multi-line comment style (several places).
> diff --git a/include/linux/intel-service-client.h b/include/linux/intel-service-client.h
> new file mode 100644
> index 0000000..5bb6513
> --- /dev/null
> +++ b/include/linux/intel-service-client.h
> @@ -0,0 +1,227 @@
> +
> +#ifndef __INTEL_SERVICE_CLIENT_H
> +#define __INTEL_SERVICE_CLIENT_H
> +
> +#include <linux/of.h>
> +#include <linux/device.h>
> +
> +/**
In kernel source files, /** means that the following is a kernel-doc formatted
comment. This is close, but not quite. Others also are just close.
> + * Service driver supports client names
> + * @fpga: for FPGA configuration
> + * @dummy: for integration/debug/trouble-shooting
> + */
> +#define SVC_CLIENT_FPGA "fpga"
> +#define SVC_CLIENT_DUMMY "dummy"
> +
> +/**
Hm, these comments would be close to kernel-doc notation for an enum type,
but not for a list of #defines.
> + * Status of the sent command, in bit number
> + * @SVC_COMMAND_STATUS_RECONFIG_REQUEST_OK
> + * SDM accepts the request of FPGA reconfiguration
> + * @SVC_STATUS_RECONFIG_BUFFER_SUBMITTED
> + * Service client successfully submits FPGA configuration
> + * data buffer to SDM
> + * @SVC_COMMAND_STATUS_RECONFIG_BUFFER_DONE
> + * SDM completes data process, ready to accept the
> + * next WRITE transaction
> + * @SVC_COMMAND_STATUS_RECONFIG_COMPLETED
> + * SDM completes FPGA configuration successfully, FPGA should
> + * be in user mode
> + * @SVC_COMMAND_STATUS_RECONFIG_BUSY
> + * FPGA configuration is still in process
> + * @SVC_COMMAND_STATUS_RECONFIG_ERROR
> + * Error encountered during FPGA configuration
> + */
> +#define SVC_STATUS_RECONFIG_REQUEST_OK 0
> +#define SVC_STATUS_RECONFIG_BUFFER_SUBMITTED 1
> +#define SVC_STATUS_RECONFIG_BUFFER_DONE 2
> +#define SVC_STATUS_RECONFIG_COMPLETED 3
> +#define SVC_STATUS_RECONFIG_BUSY 4
> +#define SVC_STATUS_RECONFIG_ERROR 5
> +
> +struct intel_svc_chan;
> +
> diff --git a/include/linux/intel-service.h b/include/linux/intel-service.h
> new file mode 100644
> index 0000000..fdf21b3
> --- /dev/null
> +++ b/include/linux/intel-service.h
> @@ -0,0 +1,122 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __INTEL_SERVICE_H
> +#define __INTEL_SERVICE_H
> +
> +#include <linux/of.h>
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/kfifo.h>
> +#include <linux/genalloc.h>
> +#include <linux/arm-smccc.h>
> +
> +
> +/**
Ah, this one is formatted correctly as kernel-doc.
> + * struct intel_svc_sh_memory - service shared memory info
> + *
> + * @sync_complete: maintain state for a completion
> + * @start_addr: starting address of shared memory
> + * @size: size of shared memory
> + */
> +struct intel_svc_sh_memory {
> + struct completion sync_complete;
> + unsigned long start_addr;
> + unsigned long size;
> +};
> +
> +/**
> + * struct intel_svc_private_mem - service private memory info
> + *
> + * @kaddr: virtual address
> + * @paddr: physical address
> + * @size: size of memory
> + * @node: link list head node
> + */
> +struct intel_svc_private_mem {
> + void *kaddr;
> + phys_addr_t paddr;
> + size_t size;
> + struct list_head node;
> +};
> +
> +/**
> + * struct intel_svc_private_data - service private data
> + *
> + * @chan: service channel
> + * @paddr: play-load physical address
> + * @size: play-load size
> + * @ommand: service request command
@command:
> + */
> +struct intel_svc_private_data {
> + struct intel_svc_chan *chan;
> + phys_addr_t paddr;
> + size_t size;
> + u32 command;
> +};
> +
> +/**
> + * struct intel_svc_controller - service controller
> + *
> + * @dev: device backing this controller
> + * @chans: array of service channels
> + * $num_chans: number of channels in 'chans' array
> + * @node: list management
missing 2 entries.
> + * @intel_svc_fifo: a queue for storing service message data
> + * @svc_fifo_lock: protect access to service message data queue
> + */
> +struct intel_svc_controller {
> + struct device *dev;
> + struct intel_svc_chan *chans;
> + int num_chans;
> +
> + struct list_head node;
> +
> + struct gen_pool *genpool;
> + struct intel_svc_prviate_mem *private_mem;
> +
> + DECLARE_KFIFO_PTR(intel_svc_fifo, struct intel_svc_private_data *);
> + /* protect access to the service message data queue*/
> + spinlock_t svc_fifo_lock;
> +};
> +
> +/**
> + * struct intel_svc_chan - service communication channel
> + *
> + * @ctrl: pointer to the provider of this channel
> + * @scl: pointer to service client which owns this channel
> + * $name: name associated with this service channel
@name:
> + * @lock: protect access to the channel
> + */
> +struct intel_svc_chan {
> + struct intel_svc_controller *ctrl;
> + struct intel_svc_client *scl;
> + char *name;
> + /* protect access to the channel*/
> + spinlock_t lock;
> +};
> +
> +static int intel_svc_smc_thread(void *data);
> +
> +#endif
--
~Randy
Powered by blists - more mailing lists