Hi Ulrich,
Thank you for the patch.
On Tuesday 09 February 2016 17:40:19 Ulrich Hecht wrote:
> When probed from device tree, the i2c client driver can handle the
> interrupt on its own.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@xxxxxxxxx>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> v4: As suggested by Hans and Lars-Peter, this revision attempts to parse the
> interrupts node to determine polarity, and passes the appropriate flags to
> devm_request_threaded_irq().
I should have replied before you posted v4, sorry about that, but I think this
is unneeded.
The I2C core retrieves the interrupt with of_irq_get_byname() or of_irq_get().
The former calls the latter, and the call stack continues with
irq_create_of_mapping() and irq_create_fwspec_mapping(). That function will
call irq_domain_translate() to translate the IRQ spec retrieved from DT to an
IRQ type, and then set the type with irq_set_irq_type().
If my understanding is correct all the work is done for you already.
The real problem is how to configure the adv7604 interrupt polarity. You can
simply call irq_get_trigger_type() to retrieve the trigger type, but that
would leave open the question of what to do if the board includes an inverter
on the IRQ line. I wonder whether it wouldn't be better to hardcode the IRQ
polarity on the adv7604 side, and document the DT IRQ flag as the polarity at
the SoC interrupt pin to match the hardcoded (and documented) polarity at the
adv7604 pin.
> v3: uses IRQ_RETVAL
>
> v2: implements the suggested style changes and drops the IRQF_TRIGGER_LOW
> flag, which is handled in the device tree.
>
>
> drivers/media/i2c/adv7604.c | 50 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 66dbe86..2a1ae6d 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -31,6 +31,8 @@
> #include <linux/gpio/consumer.h>
> #include <linux/hdmi.h>
> #include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> @@ -1971,6 +1973,16 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32
> status, bool *handled) return 0;
> }
>
> +static irqreturn_t adv76xx_irq_handler(int irq, void *devid)
> +{
> + struct adv76xx_state *state = devid;
> + bool handled;
> +
> + adv76xx_isr(&state->sd, 0, &handled);
> +
> + return IRQ_RETVAL(handled);
> +}
> +
> static int adv76xx_get_edid(struct v4l2_subdev *sd, struct v4l2_edid *edid)
> {
> struct adv76xx_state *state = to_state(sd);
> @@ -2799,6 +2811,7 @@ static int adv76xx_parse_dt(struct adv76xx_state
> *state) struct device_node *endpoint;
> struct device_node *np;
> unsigned int flags;
> + u32 irq[2];
> u32 v= -1;
>
> np = state->i2c_clients[ADV76XX_PAGE_IO]->dev.of_node;
> @@ -2844,8 +2857,20 @@ static int adv76xx_parse_dt(struct adv76xx_state
> *state) state->pdata.op_656_range = 1;
> }
>
> - /* Disable the interrupt for now as no DT-based board uses it. */
> state->pdata.int1_config = ADV76XX_INT1_CONFIG_DISABLED;
> + if (!of_property_read_u32_array(np, "interrupts", irq, 2)) {
> + switch (irq[1]) {
> + case IRQ_TYPE_LEVEL_LOW:
> + state->pdata.int1_config = ADV76XX_INT1_CONFIG_ACTIVE_LOW;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + state->pdata.int1_config = ADV76XX_INT1_CONFIG_ACTIVE_HIGH;
> + break;
> + default:
> + WARN(1, "Unsupported interrupt configuration.");
> + break;
> + }
> + }
>
> /* Use the default I2C addresses. */
> state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
> @@ -3235,6 +3260,29 @@ static int adv76xx_probe(struct i2c_client *client,
> v4l2_info(sd, "%s found @ 0x%x (%s)\n", client->name,
> client->addr << 1, client->adapter->name);
>
> + if (client->irq) {
> + unsigned long flags = IRQF_ONESHOT;
> +
> + switch (state->pdata.int1_config) {
> + case ADV76XX_INT1_CONFIG_ACTIVE_LOW:
> + flags |= IRQF_TRIGGER_LOW;
> + break;
> + case ADV76XX_INT1_CONFIG_ACTIVE_HIGH:
> + flags |= IRQF_TRIGGER_HIGH;
> + break;
> + default:
> + break;
> + }
> +
> + err = devm_request_threaded_irq(&client->dev,
> + client->irq,
> + NULL, adv76xx_irq_handler,
> + flags,
> + dev_name(&client->dev), state);
> + if (err)
> + goto err_entity;
> + }
> +
> err = v4l2_async_register_subdev(sd);
> if (err)
> goto err_entity;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
