| Message ID | 20210918120934.28252-7-sven@svenpeter.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: tipd: Add Apple M1 support | expand |
This seems reasonable (and has my Tested-by on the M1) but I don't know enough about non-apple TPS to feel comfortable reviewing.
On Sat, Sep 18, 2021 at 02:09:31PM +0200, Sven Peter wrote: > Right now the code relies on the bootloader to set up the interrupt mask > correctly. This usually works but let's make sure to do it explicitly to > guarantee it will always work. > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > --- > drivers/usb/typec/tipd/core.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index d191e7435018..2058e8cca631 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -728,6 +728,24 @@ static int tps6598x_probe(struct i2c_client *client) > dev_err(&client->dev, "failed to register partner\n"); > } > > + if (tps->hw->use_int1) { > + ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, > + tps->hw->irq_power_status_update | > + tps->hw->irq_data_status_update | > + tps->hw->irq_plug_event); > + if (ret) > + goto err_role_put; > + } > + > + if (tps->hw->use_int2) { > + ret = tps6598x_write64(tps, TPS_REG_INT_MASK2, > + tps->hw->irq_power_status_update | > + tps->hw->irq_data_status_update | > + tps->hw->irq_plug_event); > + if (ret) > + goto err_role_put; > + } You should first only set the mask on your board. We can then see if it's something that should be done on other boards as well later. thanks,
On Tue, Sep 21, 2021, at 15:40, Heikki Krogerus wrote: > On Sat, Sep 18, 2021 at 02:09:31PM +0200, Sven Peter wrote: >> Right now the code relies on the bootloader to set up the interrupt mask >> correctly. This usually works but let's make sure to do it explicitly to >> guarantee it will always work. >> >> Signed-off-by: Sven Peter <sven@svenpeter.dev> >> --- >> drivers/usb/typec/tipd/core.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c >> index d191e7435018..2058e8cca631 100644 >> --- a/drivers/usb/typec/tipd/core.c >> +++ b/drivers/usb/typec/tipd/core.c >> @@ -728,6 +728,24 @@ static int tps6598x_probe(struct i2c_client *client) >> dev_err(&client->dev, "failed to register partner\n"); >> } >> >> + if (tps->hw->use_int1) { >> + ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, >> + tps->hw->irq_power_status_update | >> + tps->hw->irq_data_status_update | >> + tps->hw->irq_plug_event); >> + if (ret) >> + goto err_role_put; >> + } >> + >> + if (tps->hw->use_int2) { >> + ret = tps6598x_write64(tps, TPS_REG_INT_MASK2, >> + tps->hw->irq_power_status_update | >> + tps->hw->irq_data_status_update | >> + tps->hw->irq_plug_event); >> + if (ret) >> + goto err_role_put; >> + } > > You should first only set the mask on your board. We can then see if > it's something that should be done on other boards as well later. > Make sense, I'll only call this when the cd321x variant is probed then. Sven
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index d191e7435018..2058e8cca631 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -728,6 +728,24 @@ static int tps6598x_probe(struct i2c_client *client) dev_err(&client->dev, "failed to register partner\n"); } + if (tps->hw->use_int1) { + ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, + tps->hw->irq_power_status_update | + tps->hw->irq_data_status_update | + tps->hw->irq_plug_event); + if (ret) + goto err_role_put; + } + + if (tps->hw->use_int2) { + ret = tps6598x_write64(tps, TPS_REG_INT_MASK2, + tps->hw->irq_power_status_update | + tps->hw->irq_data_status_update | + tps->hw->irq_plug_event); + if (ret) + goto err_role_put; + } + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL, tps6598x_interrupt, IRQF_SHARED | IRQF_ONESHOT,
Right now the code relies on the bootloader to set up the interrupt mask correctly. This usually works but let's make sure to do it explicitly to guarantee it will always work. Signed-off-by: Sven Peter <sven@svenpeter.dev> --- drivers/usb/typec/tipd/core.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)