Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 6 Nov 2020 09:09:17 +0000
From:   Will Deacon <will@kernel.org>
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     Prasad Sodagudi <psodagud@codeaurora.org>, rostedt@goodmis.org,
        mingo@redhat.com, keescook@chromium.org, catalin.marinas@arm.com,
        saiprakash.ranjan@codeaurora.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, anton@enomsg.org, arnd@arndb.de,
        ccross@android.com, jbaron@akamai.com, jim.cromie@gmail.com,
        joe@perches.com, joel@joelfernandes.org
Subject: Re: [PATCH v2] tracing: Add register read and write tracing support
Message-ID: <20201106090916.GB9496@willie-the-truck>
References: <1604631386-178312-1-git-send-email-psodagud@codeaurora.org>
 <1604631386-178312-2-git-send-email-psodagud@codeaurora.org>
 <20201106064929.GC697514@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201106064929.GC697514@kroah.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Fri, Nov 06, 2020 at 07:49:29AM +0100, Greg KH wrote:
> On Thu, Nov 05, 2020 at 06:56:26PM -0800, Prasad Sodagudi wrote:
> > Add register read/write operations tracing support.
> > ftrace events helps to trace register read and write
> > location details of memory mapped IO registers.
> 
> This sentance does not parse for me, can you please rework it?
> 
> > These trace logs helps to debug un clocked access
> > of peripherals.
> 
> "un clocked"?  What does that mean?
> 
> And you do have 72 columns to fill, please use it :)
> 
> > 
> > Signed-off-by: Prasad Sodagudi <psodagud@codeaurora.org>
> > ---
> >  arch/arm64/include/asm/io.h    |  9 ++++++++
> >  arch/arm64/kernel/image-vars.h |  8 +++++++
> 
> You are only adding it for arm64, why not add the generic support first,
> and then add it for all "major" arches afterwards?

Yeah, or just hook the higher-level accessors rather than the arch-specific
bowels. Less work for more coverage that way!

Will