diff mbox series

[RFT,5/9] usb: typec: tipd: Allow to configure irq bits

Message ID 20210918120934.28252-6-sven@svenpeter.dev (mailing list archive)
State Superseded
Headers show
Series usb: typec: tipd: Add Apple M1 support | expand

Commit Message

Sven Peter Sept. 18, 2021, 12:09 p.m. UTC
The Apple variant of the TI TPS6598x chip uses different interrupt
numbers. Prepare for that by allowing those to be configured depending
on the compatible.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
 drivers/usb/typec/tipd/core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Heikki Krogerus Sept. 21, 2021, 1:34 p.m. UTC | #1
On Sat, Sep 18, 2021 at 02:09:30PM +0200, Sven Peter wrote:
> The Apple variant of the TI TPS6598x chip uses different interrupt
> numbers. Prepare for that by allowing those to be configured depending
> on the compatible.

OK, so I think this justifies having a completely separate irq
handler for your board.

> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
>  drivers/usb/typec/tipd/core.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 4a6d66250fef..d191e7435018 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -80,6 +80,10 @@ static const char *const modes[] = {
>  struct tps6598x_hw {
>  	bool use_int1;
>  	bool use_int2;
> +	unsigned int irq_power_status_update;
> +	unsigned int irq_data_status_update;
> +	unsigned int irq_plug_event;
> +	void (*irq_trace)(u64 event1, u64 event2);
>  };

Then I believe you don't need any of that.


thanks,
Sven Peter Sept. 22, 2021, 2:58 p.m. UTC | #2
Hi,


On Tue, Sep 21, 2021, at 15:34, Heikki Krogerus wrote:
> On Sat, Sep 18, 2021 at 02:09:30PM +0200, Sven Peter wrote:
>> The Apple variant of the TI TPS6598x chip uses different interrupt
>> numbers. Prepare for that by allowing those to be configured depending
>> on the compatible.
>
> OK, so I think this justifies having a completely separate irq
> handler for your board.

OK, If you're fine with a bit of code duplication for the irq handler this will
all be quite a bit simpler then.

>
>> Signed-off-by: Sven Peter <sven@svenpeter.dev>
>> ---
>>  drivers/usb/typec/tipd/core.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
>> index 4a6d66250fef..d191e7435018 100644
>> --- a/drivers/usb/typec/tipd/core.c
>> +++ b/drivers/usb/typec/tipd/core.c
>> @@ -80,6 +80,10 @@ static const char *const modes[] = {
>>  struct tps6598x_hw {
>>  	bool use_int1;
>>  	bool use_int2;
>> +	unsigned int irq_power_status_update;
>> +	unsigned int irq_data_status_update;
>> +	unsigned int irq_plug_event;
>> +	void (*irq_trace)(u64 event1, u64 event2);
>>  };
>
> Then I believe you don't need any of that.

Yup, I think I'll really only need something like

struct tps6598x_hw {
	int type;
	irq_handler_t irq_handler;
};

in that case!


Thanks,


Sven
diff mbox series

Patch

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 4a6d66250fef..d191e7435018 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -80,6 +80,10 @@  static const char *const modes[] = {
 struct tps6598x_hw {
 	bool use_int1;
 	bool use_int2;
+	unsigned int irq_power_status_update;
+	unsigned int irq_data_status_update;
+	unsigned int irq_plug_event;
+	void (*irq_trace)(u64 event1, u64 event2);
 };
 static const struct tps6598x_hw ti_tps6598x_data;
 
@@ -431,7 +435,7 @@  static irqreturn_t tps6598x_interrupt(int irq, void *data)
 		dev_err(tps->dev, "%s: failed to read events\n", __func__);
 		goto err_unlock;
 	}
-	trace_tps6598x_irq(event1, event2);
+	tps->hw->irq_trace(event1, event2);
 
 	event = event1 | event2;
 	if (!event)
@@ -444,7 +448,7 @@  static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	}
 	trace_tps6598x_status(status);
 
-	if (event & TPS_REG_INT_POWER_STATUS_UPDATE) {
+	if (event & tps->hw->irq_power_status_update) {
 		ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
 		if (ret < 0) {
 			dev_err(tps->dev, "failed to read power status: %d\n", ret);
@@ -453,7 +457,7 @@  static irqreturn_t tps6598x_interrupt(int irq, void *data)
 		trace_tps6598x_power_status(pwr_status);
 	}
 
-	if (event & TPS_REG_INT_DATA_STATUS_UPDATE) {
+	if (event & tps->hw->irq_data_status_update) {
 		ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &data_status);
 		if (ret < 0) {
 			dev_err(tps->dev, "failed to read data status: %d\n", ret);
@@ -463,7 +467,7 @@  static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	}
 
 	/* Handle plug insert or removal */
-	if (event & TPS_REG_INT_PLUG_EVENT) {
+	if (event & tps->hw->irq_plug_event) {
 		if (status & TPS_STATUS_PLUG_PRESENT) {
 			ret = tps6598x_connect(tps, status);
 			if (ret)
@@ -760,6 +764,10 @@  static int tps6598x_remove(struct i2c_client *client)
 static const struct tps6598x_hw ti_tps6598x_data = {
 	.use_int1 = true,
 	.use_int2 = true,
+	.irq_power_status_update = TPS_REG_INT_POWER_STATUS_UPDATE,
+	.irq_data_status_update = TPS_REG_INT_DATA_STATUS_UPDATE,
+	.irq_plug_event = TPS_REG_INT_PLUG_EVENT,
+	.irq_trace = trace_tps6598x_irq,
 };
 
 static const struct of_device_id tps6598x_of_match[] = {