All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Jinlong <quic_jinlmao@quicinc.com>
Cc: Tao Zhang <quic_taozha@quicinc.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Tingwei Zhang <quic_tingweiz@quicinc.com>,
	Yuanfang Zhang <quic_yuanfang@quicinc.com>,
	Trilok Soni <quic_tsoni@quicinc.com>
Subject: Re: [PATCH 04/10] Coresight: Enable BC and GPR for TPDM driver
Date: Fri, 5 Nov 2021 09:14:25 -0600	[thread overview]
Message-ID: <20211105151425.GA827656@p14s> (raw)
In-Reply-To: <20211105081750.GD25738@jinlmao-gv.ap.qualcomm.com>

On Fri, Nov 05, 2021 at 04:17:54PM +0800, Jinlong wrote:
> On Thu, Nov 04, 2021 at 11:02:24AM -0600, Mathieu Poirier wrote:
> > [...]
> > 
> > > > > +
> > > > > +static ssize_t reset_store(struct device *dev,
> > > > > +					  struct device_attribute *attr,
> > > > > +					  const char *buf,
> > > > > +					  size_t size)
> > > > > +{
> > > > > +	int ret = 0;
> > > > > +	unsigned long val;
> > > > > +	struct mcmb_dataset *mcmb_temp = NULL;
> > > > > +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > > > > +
> > > > > +	ret = kstrtoul(buf, 10, &val);
> > > > 
> > > > The coresight subsystem normally uses the hexadecimal base.
> > > > 
> > > 
> > > We will address you comments.
> > > 
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > 
> > > > Shouldn't this be "if (!ret)" ? 
> > > >
> > > 
> > > When ret is not 0, it need to return.
> > 
> > I would expect something like this:
> > 
> > $ echo 1 > /sys/path/to/tpdm/device/reset
> > 
> > and not
> > 
> > $ echo 0 > /sys/path/to/tpdm/device/reset
> > 
> > The latter is what the code does.
> > 
> > Thanks,
> > Mathieu
> > 
> 
> Hi Mathieu,
> 
> The ret is the result of kstrtoul not the val.
>

Ah yes, you are correct.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Jinlong <quic_jinlmao@quicinc.com>
Cc: Tao Zhang <quic_taozha@quicinc.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Tingwei Zhang <quic_tingweiz@quicinc.com>,
	Yuanfang Zhang <quic_yuanfang@quicinc.com>,
	Trilok Soni <quic_tsoni@quicinc.com>
Subject: Re: [PATCH 04/10] Coresight: Enable BC and GPR for TPDM driver
Date: Fri, 5 Nov 2021 09:14:25 -0600	[thread overview]
Message-ID: <20211105151425.GA827656@p14s> (raw)
In-Reply-To: <20211105081750.GD25738@jinlmao-gv.ap.qualcomm.com>

On Fri, Nov 05, 2021 at 04:17:54PM +0800, Jinlong wrote:
> On Thu, Nov 04, 2021 at 11:02:24AM -0600, Mathieu Poirier wrote:
> > [...]
> > 
> > > > > +
> > > > > +static ssize_t reset_store(struct device *dev,
> > > > > +					  struct device_attribute *attr,
> > > > > +					  const char *buf,
> > > > > +					  size_t size)
> > > > > +{
> > > > > +	int ret = 0;
> > > > > +	unsigned long val;
> > > > > +	struct mcmb_dataset *mcmb_temp = NULL;
> > > > > +	struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > > > > +
> > > > > +	ret = kstrtoul(buf, 10, &val);
> > > > 
> > > > The coresight subsystem normally uses the hexadecimal base.
> > > > 
> > > 
> > > We will address you comments.
> > > 
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > 
> > > > Shouldn't this be "if (!ret)" ? 
> > > >
> > > 
> > > When ret is not 0, it need to return.
> > 
> > I would expect something like this:
> > 
> > $ echo 1 > /sys/path/to/tpdm/device/reset
> > 
> > and not
> > 
> > $ echo 0 > /sys/path/to/tpdm/device/reset
> > 
> > The latter is what the code does.
> > 
> > Thanks,
> > Mathieu
> > 
> 
> Hi Mathieu,
> 
> The ret is the result of kstrtoul not the val.
>

Ah yes, you are correct.


  reply	other threads:[~2021-11-05 15:16 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-21  7:38 [PATCH 00/10] Add support for TPDM and TPDA Tao Zhang
2021-10-21  7:38 ` Tao Zhang
2021-10-21  7:38 ` [PATCH 01/10] coresight: add support to enable more coresight paths Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-10-28 18:06   ` Mathieu Poirier
2021-10-28 18:06     ` Mathieu Poirier
2021-11-22 15:12     ` Jinlong Mao
2021-11-22 15:12       ` Jinlong Mao
2021-11-22 16:51       ` Mathieu Poirier
2021-11-22 16:51         ` Mathieu Poirier
2021-10-21  7:38 ` [PATCH 02/10] coresight: funnel: add support for multiple output ports Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-10-29 17:48   ` Mathieu Poirier
2021-10-29 17:48     ` Mathieu Poirier
2021-10-21  7:38 ` [PATCH 03/10] Coresight: Add driver to support Coresight device TPDM Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-11-02 17:59   ` Mathieu Poirier
2021-11-02 17:59     ` Mathieu Poirier
2021-11-04  8:56     ` Jinlong
2021-11-04  8:56       ` Jinlong
2021-11-04 16:55       ` Mathieu Poirier
2021-11-04 16:55         ` Mathieu Poirier
2021-11-05  8:15         ` Jinlong
2021-11-05  8:15           ` Jinlong
2021-11-04  9:37     ` Suzuki K Poulose
2021-11-04  9:37       ` Suzuki K Poulose
2021-11-05  8:12       ` Jinlong
2021-11-05  8:12         ` Jinlong
2021-10-21  7:38 ` [PATCH 04/10] Coresight: Enable BC and GPR for TPDM driver Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-11-03 19:43   ` Mathieu Poirier
2021-11-03 19:43     ` Mathieu Poirier
2021-11-04 11:13     ` Jinlong
2021-11-04 11:13       ` Jinlong
2021-11-04 17:02       ` Mathieu Poirier
2021-11-04 17:02         ` Mathieu Poirier
2021-11-05  8:17         ` Jinlong
2021-11-05  8:17           ` Jinlong
2021-11-05 15:14           ` Mathieu Poirier [this message]
2021-11-05 15:14             ` Mathieu Poirier
2021-10-21  7:38 ` [PATCH 05/10] Coresight: Add interface for TPDM BC subunit Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-11-04 18:01   ` Mathieu Poirier
2021-11-04 18:01     ` Mathieu Poirier
2021-11-05  8:26     ` Jinlong
2021-11-05  8:26       ` Jinlong
2021-11-12  8:42       ` Jinlong
2021-11-12  8:42         ` Jinlong
2021-11-12  9:10         ` Jinlong
2021-11-12  9:10           ` Jinlong
2021-11-12 16:37           ` Mathieu Poirier
2021-11-12 16:37             ` Mathieu Poirier
2021-10-21  7:38 ` [PATCH 06/10] Coresight: Enable and add interface for TPDM TC subunit Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-10-21  7:38 ` [PATCH 07/10] Coresight: Enable DSB subunit for TPDM Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-10-21  7:38 ` [PATCH 08/10] Coresight: Enable CMB " Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-10-21  7:38 ` [PATCH 09/10] coresight: Add driver to support Coresight device TPDA Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-10-21  7:38 ` [PATCH 10/10] ARM: dts: msm: Add TPDA and TPDM support to DTS for RB5 Tao Zhang
2021-10-21  7:38   ` Tao Zhang
2021-11-02 18:02   ` Mathieu Poirier
2021-11-02 18:02     ` Mathieu Poirier
2021-11-03  8:14     ` Tao Zhang
2021-11-03  8:14       ` Tao Zhang
2021-11-04  9:45   ` Suzuki K Poulose
2021-11-04  9:45     ` Suzuki K Poulose
2021-11-05  8:07     ` Jinlong
2021-11-05  8:07       ` Jinlong
2021-10-28 17:16 ` [PATCH 00/10] Add support for TPDM and TPDA Mathieu Poirier
2021-10-28 17:16   ` Mathieu Poirier
2021-10-29 15:11   ` Tao Zhang
2021-10-29 15:11     ` Tao Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211105151425.GA827656@p14s \
    --to=mathieu.poirier@linaro.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=coresight@lists.linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.leach@linaro.org \
    --cc=quic_jinlmao@quicinc.com \
    --cc=quic_taozha@quicinc.com \
    --cc=quic_tingweiz@quicinc.com \
    --cc=quic_tsoni@quicinc.com \
    --cc=quic_yuanfang@quicinc.com \
    --cc=suzuki.poulose@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.