Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230410073134.488762-1-badhri@google.com> <2023041028-irritate-starless-a42f@gregkh>
In-Reply-To: <2023041028-irritate-starless-a42f@gregkh>
From:   Badhri Jagan Sridharan <badhri@google.com>
Date:   Mon, 10 Apr 2023 01:08:55 -0700
Message-ID: <CAPTae5KZ0zkaCWrtPYzyX8oTyvDhBhc-hvxyHn9VHZ32UOqJ3g@mail.gmail.com>
Subject: Re: [PATCH v1] usb: typec: tcpm: Add kernel config to wrap around
 tcpm logs
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     linux@roeck-us.net, heikki.krogerus@linux.intel.com,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 10, 2023 at 12:45=E2=80=AFAM Greg KH <gregkh@linuxfoundation.or=
g> wrote:
>
> On Mon, Apr 10, 2023 at 07:31:34AM +0000, Badhri Jagan Sridharan wrote:
> > This change adds CONFIG_TCPM_LOG_WRAPAROUND which when set allows the
> > logs to be wrapped around. Additionally, when set, does not clear
> > the TCPM logs when dumped.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> >  drivers/usb/typec/tcpm/Kconfig | 6 ++++++
> >  drivers/usb/typec/tcpm/tcpm.c  | 9 +++++++--
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kc=
onfig
> > index e6b88ca4a4b9..4dd2b594dfc9 100644
> > --- a/drivers/usb/typec/tcpm/Kconfig
> > +++ b/drivers/usb/typec/tcpm/Kconfig
> > @@ -18,6 +18,12 @@ config TYPEC_TCPCI
> >       help
> >         Type-C Port Controller driver for TCPCI-compliant controller.
> >
> > +config TCPM_LOG_WRAPAROUND
> > +     bool "Enable TCPM log wraparound"
> > +     help
> > +       When set, wraps around TCPM logs and does not clear the logs wh=
en dumped. TCPM logs by
> > +       default gets cleared when dumped and does not wraparound when f=
ull.
>
> Kconfig help text needs to be wrapped at the properly width.

I assumed that the width is 100 characters, but it looks like it is
80. Will fix it in the next version.
>
> And you do not provide any hint here as to why this is not the default
> option, or why someone would want this.

"TCPM logs by default gets cleared when dumped and does not wraparound
when full." was intended
to convey why someone would want to set this. Perhaps it's not effective.

Does the below look better:
"TCPM logs by default gets cleared when dumped and does not wraparound
when full. This can be overridden by setting this config.
When the config is set, TCPM wraps around logs and does not clear the
logs when dumped."

Also, I could make this default if that's OK with Guenter.

>
> So, why is this not just the default operation anyway?  Why would you
> ever want the logs cleared?

I remember Guenter mentioning that he was finding it useful to not
wrap around the logs to fix problems
during tcpm_register_port (init sequence). IMHO wrapping around the
logs helps to triage interoperability
issues uncovered during testing. So both approaches have their own advantag=
es.

Thanks for taking the time to review !
- Badhri

>
> thanks,
>
> greg k-h