Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1363316

Re: [PATCH 03/14] coresight: tmc: re-implementing tmc_read_prepare/unprepare() functions

From "Suzuki K. Poulose" <Suzuki.Poulose@arm.com>
Newsgroups linux.kernel
Subject Re: [PATCH 03/14] coresight: tmc: re-implementing tmc_read_prepare/unprepare() functions
Date 2016-03-23 11:40 +0100
Message-ID <rfOmK-22n-7@gated-at.bofh.it> (permalink)
References <rfB69-1hK-1@gated-at.bofh.it> <rfB6b-1hK-35@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 22/03/16 20:23, Mathieu Poirier wrote:
> In their current implementation the tmc_read_prepare/unprepare()
> are a lump of if/else that is difficult to read.  This patch is
> alleviating that by using a switch statement.  The latter also
> allows for a better control on the error path.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   drivers/hwtracing/coresight/coresight-tmc.c | 56 ++++++++++++++++++-----------
>   1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index f4ba837a0810..208d47dd3083 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -430,7 +430,7 @@ static const struct coresight_ops tmc_etf_cs_ops = {
>

> -	if (drvdata->config_type == TMC_CONFIG_TYPE_ETB) {
> +	switch (drvdata->config_type) {
> +	case TMC_CONFIG_TYPE_ETB:
>   		tmc_etb_disable_hw(drvdata);
> -	} else if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) {
> -		tmc_etr_disable_hw(drvdata);
> -	} else {
> +		break;
> +	case TMC_CONFIG_TYPE_ETF:
> +		/* There is no point in reading a TMC in HW FIFO mode */
>   		mode = readl_relaxed(drvdata->base + TMC_MODE);
> -		if (mode == TMC_MODE_CIRCULAR_BUFFER) {
> -			tmc_etb_disable_hw(drvdata);
> -		} else {
> -			ret = -ENODEV;
> +		if (mode != TMC_MODE_CIRCULAR_BUFFER) {
> +			ret = -EINVAL;
>   			goto err;
>   		}
> +
> +		tmc_etb_disable_hw(drvdata);
> +		break;
> +	case TMC_CONFIG_TYPE_ETR:
> +		tmc_etr_disable_hw(drvdata);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto err;
>   	}

We seem to be doing this switch at different places in the code just for enable_hw/disable_hw.
e.g, tmc_enable, tmc_disable

Could we make this a bit more cleaner by introducing something like this ?

struct tmc_hw_ops {
	int (*enable_hw)(struct tmc_drvdata *drvdata, enum tmc_mode mode);
	int (*disable_hw)(struct tmc_drvdata *drvdata, enum tmc_mode mode);
};

struct tmc_hw_ops tmc_etf_ops = {
	tmc_etf_enable_hw,
	tmc_etf_disable_hw,
};

similiarly for etb and etr and then add struct tmc_hw_ops *hw_ops to tmc_drvdata, initialised
at probe time (while reading the config_type).


Suzuki

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/14] coresight: tmc: make driver usable by Perf  Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 12/14] coresight: tmc: implementing TMC-ETF AUX space API Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 14/14] coresight: configuring ETF in FIFO mode when acting as link Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 11/14] coresight: tmc: keep track of memory width Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 07/14] coresight: tmc: making disable function reusable Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 08/14] coresight: tmc: allocating memory when needed Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 05/14] coresight: tmc: splitting driver in ETB/ETF and ETR components Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 09/14] coresight: tmc: adding mode of operation for link/sinks Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 02/14] coresight: tmc: waiting for TMCReady bit before programming Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 01/14] coresight: tmc: modifying naming convention Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
    Re: [PATCH 01/14] coresight: tmc: modifying naming convention "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2016-03-23 11:40 +0100
  [PATCH 03/14] coresight: tmc: re-implementing tmc_read_prepare/unprepare() functions Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
    Re: [PATCH 03/14] coresight: tmc: re-implementing  tmc_read_prepare/unprepare() functions "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2016-03-23 11:40 +0100
      Re: [PATCH 03/14] coresight: tmc: re-implementing tmc_read_prepare/unprepare()  functions Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-24 17:40 +0100
      Re: [PATCH 03/14] coresight: tmc: re-implementing tmc_read_prepare/unprepare()  functions Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-24 20:20 +0100
  [PATCH 13/14] coresight: tmc: implementing TMC-ETR AUX space API Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 04/14] coresight: tmc: introducing new header file Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 10/14] coresight: tmc: make sysFS and Perf mode mutually exclusive Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100
  [PATCH 06/14] coresight: tmc: making prepare/unprepare functions generic Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-03-22 21:30 +0100

csiph-web