linux-kernel - Re: [PATCH v3 2/7] usb: typec: tcpci_rt1711h: Fix vendor setting when set vconn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 3 Aug 2022 14:08:50 +0800
From:   Gene Chen <gene.chen.richtek@...il.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, linux-usb@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Gene Chen <gene_chen@...htek.com>,
        ChiYuan Huang <cy_huang@...htek.com>
Subject: Re: [PATCH v3 2/7] usb: typec: tcpci_rt1711h: Fix vendor setting when
 set vconn

Heikki Krogerus <heikki.krogerus@...ux.intel.com> 於 2022年8月2日 週二 下午3:57寫道：
>
> Hi Gene,
>
> On Mon, Aug 01, 2022 at 06:14:42PM +0800, Gene Chen wrote:
> > From: Gene Chen <gene_chen@...htek.com>
> >
> > replace overwrite whole register with update bits
> >
> > Signed-off-by: Gene Chen <gene_chen@...htek.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, struct tcpci_data *tdata,
> >  {
> >       struct rt1711h_chip *chip = 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 : RT1711H_AUTOIDLEEN);
> >  }
> >
> >  static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
> > --
> > 2.25.1
>
> thanks,
>
> --
> heikki