Re: [PATCH v5 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for nxp,irq-mask-enable

On 2017-01-17 10:43, Peter Rosin wrote:

On 2017-01-17 10:28, Phil Reid wrote:

quoted

On 17/01/2017 16:57, Peter Rosin wrote:

quoted

On 2017-01-17 09:00, Phil Reid wrote:

quoted

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to <1 1> will
delay the irq being enabled until both devices are ready.

G'day Peter,

quoted

Hang on, does this suggestion I made make any sense at all? Maybe it does,
but does the pca954x driver even get notified of any but the first irq client
that unmasks interrupts on a mux segment? How can it count the number of
active irq clients if not?

Good question.

So what I did to test is setup my 2 ltc1760s to use the same irq on the pca954x.
Using the latest patch series.

Adding a log message into the irq_unmask function got the following.
	dev_err(&data->client->dev, "irq_unmask %d %x %d", pos, data->irq_mask, data->irq_enabled);

dmesg | grep irq_unmask
[    4.392098] pca954x 4-0070: irq_unmask 0 1 1


But Looks like both got registered ok to the same irq.
cat /proc/interrupts
161:          0          0  i2c-mux-pca954x   0 Level     15-000a, 16-000a

So from this testing, it doesn't look like it gets called multiple times.

As I suspected, thanks for verifying!

quoted

So back to the bitmask for the dt do you think.

Looking at kernel/irq/chip.c:irq_enable (and struct irq_chip docs), I get the
feeling that if you provide the irq_enable operation (and maybe irq_disable too?),
that might get us more info?

No, I no longer think that. I think we need to get at the irq descriptor "depth".
But that feels like poking at the wrong level. Crap.

And to answer the question if I think we should go back to a dt bitmask, then
no I do not think that. The array describes what we want to do, it's the linux
implementation that gives us difficulties. Agreed?

quoted

I think the interrupt enablelogic is correct now.

quoted

I'm truly sorry for the trouble I'm causing by not just saying how it should
be done from the start, but I feel like I've been thrown in at the deep end
when it comes to interrupt controllers...

No problem. I'm learning a couple things as we go.
Should help me out on other drivers :)

Yes, I'm also picking up a few bits here and there...

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

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