[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-doc
Subject:    Re: [PATCH v5 03/14] coresight: cti: Add sysfs access to program function regs
From:       Mike Leach <mike.leach () linaro ! org>
Date:       2019-11-29 12:50:30
Message-ID: CAJ9a7Vh=J8QbPbng0OMmH6uNjhTZJMfm9hn63zw8rNcr00msbw () mail ! gmail ! com
[Download RAW message or body]

Hi Suzuki, Mathieu,

On Thu, 28 Nov 2019 at 10:54, Suzuki Kuruppassery Poulose
<suzuki.poulose@arm.com> wrote:
>
> On 19/11/2019 23:19, Mike Leach wrote:
> > Adds in sysfs programming support for the CTI function register sets.
> > Allows direct manipulation of channel / trigger association registers.
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
>
>
> > +/*
> > + * #define CTI_DEBUG_INTEGRATION_CTRL to enable the access to the integration
> > + * control registers. Normally only used to investigate connection data.
> > + */
>
> On a second thought, I have some comments on this symbol.
>
> Given that the integration control registers may be useful for people to
> find the device connections, I strongly feel that this is provided
> via a CONFIG symbol rather than a  debug symbol within the code.
>
> i.e, CONFIG_CTI_DEBUG_INTEGRATION_CTRL, to help the people better.
> Codewise this doesn't make much difference, but it certainly makes
> it more easier for people to use it.
>
> We have used debug symbols elsewhere in the drivers for pure functional
> debugging purposes. However I feel this is case is superior.
>
>
> Cheers
> Suzuki

Per the comment above, and the discussions following, I would agree
that using a config symbol makes it easier for users to select the
feature and gives us an opportunity to put in some explanation as to
what it does.

Thanks

Mike

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
[prev in list] [next in list] [prev in thread] [next in thread]