linux-kernel - Re: [PATCH v7] staging: typec: handle vendor defined part and modify drp toggling flow

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAFaeLdQDHSdeoG9fPVrBheohBGMkG-KirFtF-ra48AXfMWhVNw@mail.gmail.com>
Date:   Mon, 12 Mar 2018 13:22:10 +0800
From:   李書帆 <leechu729@...il.com>
To:     Jun Li <jun.li@....com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "heikki.krogerus@...ux.intel.com" <heikki.krogerus@...ux.intel.com>,
        "linux@...ck-us.net" <linux@...ck-us.net>,
        "greg@...ah.com" <greg@...ah.com>,
        "shufan_lee@...htek.com" <shufan_lee@...htek.com>,
        "cy_huang@...htek.com" <cy_huang@...htek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v7] staging: typec: handle vendor defined part and modify
 drp toggling flow

Hi Jun,

  Thank you.

2018-03-12 12:33 GMT+08:00 Jun Li <jun.li@....com>:
> Hi,
>
>> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
>> +     struct tcpci *tcpci = dev_id;
>> +
>> +     return tcpci_irq(tcpci);
>> +}
>>
> ...
>
>> +     err = 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 = dev_id;
+       struct tcpci_chip *chip = 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=usb-testing&id=8f94390226487bcb86c554ddc12eef0d27bdec3b
>
> 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
>



-- 
Best Regards,
書帆