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: <20220801101447.86207-1-gene.chen.richtek@gmail.com>
 <20220801101447.86207-3-gene.chen.richtek@gmail.com> <YujZATquqh0srgHm@kuha.fi.intel.com>
In-Reply-To: <YujZATquqh0srgHm@kuha.fi.intel.com>
From:   Gene Chen <gene.chen.richtek@gmail.com>
Date:   Wed, 3 Aug 2022 14:08:50 +0800
Message-ID: <CAE+NS37qqAUON=GZV1Q9HisgxpzJGQai6QFBJi8Ss_rhpTjzBg@mail.gmail.com>
Subject: Re: [PATCH v3 2/7] usb: typec: tcpci_rt1711h: Fix vendor setting when
 set vconn
To:     Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc:     Guenter Roeck <linux@roeck-us.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rob Herring <robh+dt@kernel.org>,
        krzysztof.kozlowski+dt@linaro.org, linux-usb@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        Gene Chen <gene_chen@richtek.com>,
        ChiYuan Huang <cy_huang@richtek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Heikki Krogerus <heikki.krogerus@linux.intel.com> =E6=96=BC 2022=E5=B9=B48=
=E6=9C=882=E6=97=A5 =E9=80=B1=E4=BA=8C =E4=B8=8B=E5=8D=883:57=E5=AF=AB=E9=
=81=93=EF=BC=9A
>
> Hi Gene,
>
> On Mon, Aug 01, 2022 at 06:14:42PM +0800, Gene Chen wrote:
> > From: Gene Chen <gene_chen@richtek.com>
> >
> > replace overwrite whole register with update bits
> >
> > Signed-off-by: Gene Chen <gene_chen@richtek.com>
> > ---
> >  drivers/usb/typec/tcpm/tcpci_rt1711h.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec=
/tcpm/tcpci_rt1711h.c
> > index b56a0880a044..6197d9a05d36 100644
> > --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > @@ -5,13 +5,15 @@
> >   * Richtek RT1711H Type-C Chip Driver
> >   */
> >
> > -#include <linux/kernel.h>
> > -#include <linux/module.h>
> > +#include <linux/bits.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/i2c.h>
> >  #include <linux/interrupt.h>
> > -#include <linux/gpio/consumer.h>
> > -#include <linux/usb/tcpm.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> >  #include <linux/regmap.h>
> > +#include <linux/usb/tcpm.h>
>
> That header reshuffling is not necessary for this change - at least you
> are not giving any reason for it in your commit message.
>
> If there is no real need for that in this patch, then please leave the
> headers as they are. You can propose changing the order of the headers
> in a separate patch. Though, I would not bother with it unless there
> is some real benefit in doing so, and I'm pretty sure there isn't any.
>

ACK, I will remove reshuffling.
It was suggested coding style by other reviewer in other driver.

> >  #include "tcpci.h"
> >
> >  #define RT1711H_VID          0x29CF
> > @@ -23,6 +25,7 @@
> >  #define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
> >                           (((ck300) << 7) | ((ship_off) << 5) | \
> >                           ((auto_idle) << 3) | ((tout) & 0x07))
> > +#define RT1711H_AUTOIDLEEN   BIT(3)
> >
> >  #define RT1711H_RTCTRL11     0x9E
> >
> > @@ -109,8 +112,8 @@ static int rt1711h_set_vconn(struct tcpci *tcpci, s=
truct tcpci_data *tdata,
> >  {
> >       struct rt1711h_chip *chip =3D tdata_to_rt1711h(tdata);
> >
> > -     return rt1711h_write8(chip, RT1711H_RTCTRL8,
> > -                           RT1711H_RTCTRL8_SET(0, 1, !enable, 2));
> > +     return regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL8,
> > +                               RT1711H_AUTOIDLEEN, enable ? 0 : RT1711=
H_AUTOIDLEEN);
> >  }
> >
> >  static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
> > --
> > 2.25.1
>
> thanks,
>
> --
> heikki