lkml.org 
[lkml]   [2018]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv3 3/7] driver, misc: add Intel Stratix10 service layer driver
On Tue, Mar 27, 2018 at 03:20:10PM -0500, richard.gong@linux.intel.com wrote:
> +config INTEL_SERVICE

Naming is hard, but this is a _very_ generic name, don't you agree?

> + tristate "Intel Service Layer"

As is this. Can you make this a bit more specific to what hardware is
being controlled here?

> +++ b/drivers/misc/intel-service.c

Same for the file name, why not stratix10.c?

or intel_svc.c? That makes it a _bit_ more generic. Well, not really,
but it does hide the "genericness" a bit more, right?

> --- /dev/null
> +++ b/include/linux/intel-service-client.h
> @@ -0,0 +1,188 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2017-2018, Intel Corporation
> + */
> +
> +#ifndef __INTEL_SERVICE_CLIENT_H
> +#define __INTEL_SERVICE_CLIENT_H
> +
> +/*
> + * Service layer driver supports client names
> + *
> + * fpga: for FPGA configuration
> + * dummy: for integration/debug/trouble-shooting
> + */
> +#define SVC_CLIENT_FPGA "fpga"
> +#define SVC_CLIENT_DUMMY "dummy"
> +
> +/*
> + * Status of the sent command, in bit number
> + *
> + * SVC_COMMAND_STATUS_RECONFIG_REQUEST_OK:
> + * Secure firmware accepts the request of FPGA reconfiguration.
> + *
> + * SVC_STATUS_RECONFIG_BUFFER_SUBMITTED:
> + * Service client successfully submits FPGA configuration
> + * data buffer to secure firmware.
> + *
> + * SVC_COMMAND_STATUS_RECONFIG_BUFFER_DONE:
> + * Secure firmware completes data process, ready to accept the
> + * next WRITE transaction.
> + *
> + * SVC_COMMAND_STATUS_RECONFIG_COMPLETED:
> + * Secure firmware 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.
> + */

You have an odd mix of kernel-doc formatting and non-kernel-doc
formatting in this file. Pick one and stick with it :)

Also, if you use kernel-doc (as you should), please hook it up to the
kernel documentation build process to take advantage of it.

thanks,

greg k-h

\
 
 \ /
  Last update: 2018-04-23 14:03    [W:0.167 / U:0.528 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site