Re: [PATCH v4 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 16/01/2017 20:08, Peter Rosin wrote:

@@ -280,9 +294,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)

 	spin_lock_irqsave(&data->lock, flags);

-	if (!data->irq_mask)
+	if (!data->irq_mask_enable && !data->irq_mask)
 		enable_irq(data->client->irq);
 	data->irq_mask |= BIT(pos);
+	if (data->irq_mask_enable &&
+		(data->irq_mask & data->irq_mask) == data->irq_mask_enable)

Hmm, I see that some apparently incompetent person :-) already acked this,
but the (data->irq_mask & data->irq_mask) part doesn't make sense at all.

+		enable_irq(data->client->irq);


Hmm2, if you have a problematic device (like the ltc1760) on mux segment 0
and sane devices on other segments I'd be inclined to specify irq-mask-enable
as 0x1. But then this is possible:

1. ltc1760 registers its irq
2. enable_irq(data->client->irq) is called because irq_mask_enable is "fulfilled"
3. a sane irq register an irq on some other segment
4. enable_irq(...) is called again (which the code appears to try to avoid)

As I read the code, there will be problems with specifying irq-mask-enable
whenever there are more than one irq-client on a mux segment.

So, I'm removing my ack until the above is resolved...


G'day Peter,

Yes that is obviously wrong.
I think I've got it right this time in v5.
Also implemented your suggest on the array for the dt binding.
But stuck with the bitfield implementation for the moment.
Hopefully that's ok.

--
Regards
Phil Reid

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux