On 2017-01-16 04:11, Phil Reid wrote:
> 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 0x3 while
> delay the irq being enabled until both devices are ready.
>
> Acked-by: Peter Rosin <peda@xxxxxxxxxx>
> Signed-off-by: Phil Reid <preid@xxxxxxxxxxxxxxxxx>
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index f55da88..66f7ed8 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -76,6 +76,19 @@ struct chip_desc {
> } muxtype;
> };
>
> +/*
> + * irq_mask_enable: Provides a mechanism to work around hardware that asserts
> + * their irq immediately on power on. It allows the enabling of the irq to be
> + * delayed until the corresponding bits in the the irq_mask are set thru
> + * irq_unmask.
> + * For example 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 0x3 while
> + * delay the irq being enabled until both devices are ready.
> + * This workaround will not work if two devices share an interrupt on the
> + * same bus segment.
> + */
> struct pca954x {
> const struct chip_desc *chip;
>
> @@ -84,6 +97,7 @@ struct pca954x {
> struct i2c_client *client;
>
> struct irq_domain *irq;
> + unsigned int irq_mask_enable;
> unsigned int irq_mask;
> spinlock_t lock;
> };
> @@ -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...
Cheers,
peda
> spin_unlock_irqrestore(&data->lock, flags);
> }
> @@ -409,6 +426,9 @@ static int pca954x_probe(struct i2c_client *client,
> idle_disconnect_dt = of_node &&
> of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>
> + of_property_read_u32(of_node, "nxp,irq-mask-enable",
> + &data->irq_mask_enable);
> +
> ret = pca954x_irq_setup(muxc);
> if (ret)
> goto fail_del_adapters;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
