spmi: Question about qpnpint_irq_set_type implement

[Axel Lin]

Hi,
Current implementation in qpnpint_irq_set_type() will set (1 << irq) bit
for type.polarity_high/type.polarity_low but never clear this bit.
I'm wondering if it is intentional because the value write to
QPNPINT_REG_SET_TYPE register depends on it's original value.

Maybe it needs below changes, comments?

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 6ea6eab..fe59cf4 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -557,18 +557,26 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
 		type.type |= 1 << irq;
 		if (flow_type & IRQF_TRIGGER_RISING)
 			type.polarity_high |= 1 << irq;
+		else
+			type.polarity_high &= ~(1 << irq);
+
 		if (flow_type & IRQF_TRIGGER_FALLING)
 			type.polarity_low  |= 1 << irq;
+		else
+			type.polarity_low &= ~(1 << irq);
 	} else {
 		if ((flow_type & (IRQF_TRIGGER_HIGH)) &&
 		    (flow_type & (IRQF_TRIGGER_LOW)))
 			return -EINVAL;
 
 		type.type &= ~(1 << irq); /* level trig */
-		if (flow_type & IRQF_TRIGGER_HIGH)
+		if (flow_type & IRQF_TRIGGER_HIGH) {
 			type.polarity_high |= 1 << irq;
-		else
+			type.polarity_low &= ~(1 << irq);
+		} else {
+			type.polarity_high &= ~(1 << irq);
 			type.polarity_low  |= 1 << irq;
+		}
 	}
 
 	qpnpint_spmi_write(d, QPNPINT_REG_SET_TYPE, &type, sizeof(type));


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/