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@xxxxxxx>:
> 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,
æå