On Wed, May 11, 2022 at 10:59:42AM +0300, Heikki Krogerus wrote:
> On Tue, May 10, 2022 at 10:54:37AM +0530, Sanket Goswami wrote:
> > The current implementation supports only Level trigger with ACTIVE HIGH.
> > Some of the AMD platforms have different PD firmware implementation which
> > needs different polarity. This patch checks the polarity and type based
> > on the device properties set and registers the interrupt handler
> > accordingly.
> >
> > Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
> > ---
> > drivers/usb/typec/ucsi/ucsi_ccg.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > index 7585599bacfd..0db935bd8473 100644
> > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> > @@ -20,6 +20,7 @@
> >
> > #include <asm/unaligned.h>
> > #include "ucsi.h"
> > +#define INTR_POL_TYPE BIT(0)
> >
> > enum enum_fw_mode {
> > BOOT, /* bootloader */
> > @@ -1324,6 +1325,8 @@ static int ucsi_ccg_probe(struct i2c_client *client,
> > struct device *dev = &client->dev;
> > struct ucsi_ccg *uc;
> > int status;
> > + /* Keep the IRQ type and polarity default as Level trigger Active High */
> > + int irqtype = IRQF_TRIGGER_HIGH;
> >
> > uc = devm_kzalloc(dev, sizeof(*uc), GFP_KERNEL);
> > if (!uc)
> > @@ -1366,8 +1369,12 @@ static int ucsi_ccg_probe(struct i2c_client *client,
> >
> > ucsi_set_drvdata(uc->ucsi, uc);
> >
> > + status = (uintptr_t)device_get_match_data(dev);
> > + if (status & INTR_POL_TYPE)
> > + irqtype = IRQF_TRIGGER_FALLING;
> > +
> > status = request_threaded_irq(client->irq, NULL, ccg_irq_handler,
> > - IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> > + IRQF_ONESHOT | irqtype,
> > dev_name(dev), uc);
>
> Please note that you would need to update ccg_restart() as well, but
> there is something else wrong here...
>
> > if (status < 0) {
> > dev_err(uc->dev, "request_threaded_irq failed - %d\n", status);
> > @@ -1419,7 +1426,7 @@ static const struct i2c_device_id ucsi_ccg_device_id[] = {
> > MODULE_DEVICE_TABLE(i2c, ucsi_ccg_device_id);
> >
> > static const struct acpi_device_id amd_i2c_ucsi_match[] = {
> > - {"AMDI0042"},
> > + {"AMDI0042", INTR_POL_TYPE},
> > {}
> > };
>
> This should not be necessary. That information comes from the ACPI
> tables.
>
> I don't think that you need to set the polarity/level flags at all in
> case of ACPI. I'll double check that, but if that is the case, then you
> need to make the case where the device is not ACPI enumerated the
> special case instead.
Actually, can you just test if this works for you:
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 6db7c8ddd51cd..f13c10e815d7d 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -1251,8 +1251,7 @@ static int ccg_restart(struct ucsi_ccg *uc)
}
status = request_threaded_irq(uc->irq, NULL, ccg_irq_handler,
- IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
- dev_name(dev), uc);
+ IRQF_ONESHOT, dev_name(dev), uc);
if (status < 0) {
dev_err(dev, "request_threaded_irq failed - %d\n", status);
return status;
@@ -1367,8 +1366,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
ucsi_set_drvdata(uc->ucsi, uc);
status = request_threaded_irq(client->irq, NULL, ccg_irq_handler,
- IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
- dev_name(dev), uc);
+ IRQF_ONESHOT, dev_name(dev), uc);
if (status < 0) {
dev_err(uc->dev, "request_threaded_irq failed - %d\n", status);
goto out_ucsi_destroy;
thanks,
--
heikki
