On Thu, May 13, 2021 at 01:00:28PM +0200, Maciej Kwapulinski wrote:
> +void gna_start_scoring(struct gna_private *gna_priv,
> + struct gna_compute_cfg *compute_cfg)
> +{
> + u32 ctrl = gna_reg_read(gna_priv, GNA_MMIO_CTRL);
> +
> + ctrl |= GNA_CTRL_START_ACCEL | GNA_CTRL_COMP_INT_EN | GNA_CTRL_ERR_INT_EN;
> +
> + ctrl &= ~GNA_CTRL_COMP_STATS_EN;
> + ctrl |= FIELD_PREP(GNA_CTRL_COMP_STATS_EN,
> + compute_cfg->hw_perf_encoding & FIELD_MAX(GNA_CTRL_COMP_STATS_EN));
> +
> + ctrl &= ~GNA_CTRL_ACTIVE_LIST_EN;
> + ctrl |= FIELD_PREP(GNA_CTRL_ACTIVE_LIST_EN,
> + compute_cfg->active_list_on & FIELD_MAX(GNA_CTRL_ACTIVE_LIST_EN));
> +
> + ctrl &= ~GNA_CTRL_OP_MODE;
> + ctrl |= FIELD_PREP(GNA_CTRL_OP_MODE,
> + compute_cfg->gna_mode & FIELD_MAX(GNA_CTRL_OP_MODE));
> +
> + gna_reg_write(gna_priv, GNA_MMIO_CTRL, ctrl);
> +
> + dev_dbg(gna_dev(gna_priv), "scoring started...\n");
ftrace is your friend, no need for lines like this.
> +void gna_abort_hw(struct gna_private *gna_priv)
> +{
> + u32 val;
> + int ret;
> +
> + /* saturation bit in the GNA status register needs
> + * to be explicitly cleared.
> + */
> + gna_clear_saturation(gna_priv);
> +
> + val = gna_reg_read(gna_priv, GNA_MMIO_STS);
> + dev_dbg(gna_dev(gna_priv), "status before abort: %#x\n", val);
> +
> + val = gna_reg_read(gna_priv, GNA_MMIO_CTRL);
> + val |= GNA_CTRL_ABORT_CLR_ACCEL;
> + gna_reg_write(gna_priv, GNA_MMIO_CTRL, val);
> +
> + ret = readl_poll_timeout(gna_priv->iobase + GNA_MMIO_STS, val,
> + !(val & 0x1),
> + 0, 1000);
> +
> + if (ret)
> + dev_err(gna_dev(gna_priv), "abort did not complete\n");
> +}
If "abort_hw" can fail, then return an error. What could a user do with
an error message in the kernel log like the above one? The driver
obviously did not recover from it, so what can they do?
> --- /dev/null
> +++ b/include/uapi/misc/intel/gna.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> +/* Copyright(c) 2017-2021 Intel Corporation */
> +
> +#ifndef _UAPI_GNA_H_
> +#define _UAPI_GNA_H_
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif
These C++ things should not be needed in kernel uapi header files,
right?
thanks,
greg k-h
