Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <VI1PR0402MB39170322F313A03DC0018A8089D30@VI1PR0402MB3917.eurprd04.prod.outlook.com>
References: <1520301032-10109-1-git-send-email-leechu729@gmail.com> <VI1PR0402MB39170322F313A03DC0018A8089D30@VI1PR0402MB3917.eurprd04.prod.outlook.com>
From:   =?UTF-8?B?5p2O5pu45biG?= <leechu729@gmail.com>
Date:   Mon, 12 Mar 2018 13:22:10 +0800
Message-ID: <CAFaeLdQDHSdeoG9fPVrBheohBGMkG-KirFtF-ra48AXfMWhVNw@mail.gmail.com>
Subject: Re: [PATCH v7] staging: typec: handle vendor defined part and modify
 drp toggling flow
To:     Jun Li <jun.li@nxp.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "heikki.krogerus@linux.intel.com" <heikki.krogerus@linux.intel.com>,
        "linux@roeck-us.net" <linux@roeck-us.net>,
        "greg@kroah.com" <greg@kroah.com>,
        "shufan_lee@richtek.com" <shufan_lee@richtek.com>,
        "cy_huang@richtek.com" <cy_huang@richtek.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jun,

  Thank you.

2018-03-12 12:33 GMT+08:00 Jun Li <jun.li@nxp.com>:
> Hi,
>
>> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
>> +     struct tcpci *tcpci =3D dev_id;
>> +
>> +     return tcpci_irq(tcpci);
>> +}
>>
> ...
>
>> +     err =3D devm_request_threaded_irq(&client->dev, client->irq, NULL,
>> +                                     _tcpci_irq,
>>                                       IRQF_ONESHOT | IRQF_TRIGGER_LOW,
>> -                                     dev_name(tcpci->dev), tcpci);
>> +                                     dev_name(&client->dev), chip);
>
> - dev_name(&client->dev), chip);
> + dev_name(&client->dev), chip->tcpci);
>
> Did you ever test this patch?
I've tested this patch with tcpci_rt1711h.c that will be sent out for
reviewing in the next patch after tcpci's modification is passed.
Because interrupt handler is registered in tcpci_rt1711h.c, here is
the place I didn't notice.
The interrupt handler for tcpci.c should be modified as following:
 static irqreturn_t _tcpci_irq(int irq, void *dev_id)
 {
-       struct tcpci *tcpci =3D dev_id;
+       struct tcpci_chip *chip =3D dev_id;

-       return tcpci_irq(tcpci);
+       return tcpci_irq(chip->tcpci);
 }

>
> I noticed Greg already picked this patch[1]:
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit=
/?h=3Dusb-testing&id=3D8f94390226487bcb86c554ddc12eef0d27bdec3b
>
> One more minor comment below.
>
> Jun Li
>
>> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci=
.h index
>> fdfb06c..a2c1754 100644
>> --- a/drivers/staging/typec/tcpci.h
>> +++ b/drivers/staging/typec/tcpci.h
>> @@ -59,6 +59,7 @@
>>  #define TCPC_POWER_CTRL_VCONN_ENABLE BIT(0)
>>
>>  #define TCPC_CC_STATUS                       0x1d
>> +#define TCPC_CC_STATUS_DRPRST                BIT(5)
>
> Defined but not used.
This definition can be removed for now.
>
>>  #define TCPC_CC_STATUS_TERM          BIT(4)
>>  #define TCPC_CC_STATUS_CC2_SHIFT     2
>>  #define TCPC_CC_STATUS_CC2_MASK              0x3
>> @@ -121,4 +122,18 @@
>>  #define TCPC_VBUS_VOLTAGE_ALARM_HI_CFG               0x76
>>  #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG               0x78
>>
>> +struct tcpci;
>> +struct tcpci_data {
>> +     struct regmap *regmap;
>> +     int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
>> +     int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
>> +                      bool enable);
>> +     int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *=
data,
>> +                               enum typec_cc_status cc);
>> +};
>> +
>> +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data
>> +*data); void tcpci_unregister_port(struct tcpci *tcpci); irqreturn_t
>> +tcpci_irq(struct tcpci *tcpci);
>> +
>>  #endif /* __LINUX_USB_TCPCI_H */
>> --
>> 1.9.1
>


--=20
Best Regards,
=E6=9B=B8=E5=B8=86