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

From: ææå
Date: Mon Mar 12 2018 - 01:22:20 EST


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,
æå