From: Hans Verkuil <hverkuil@xs4all.nl>
To: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>,
linux-media@vger.kernel.org, linux-sh@vger.kernel.org
Cc: magnus.damm@gmail.com, laurent.pinchart@ideasonboard.com,
hans.verkuil@cisco.com, ian.molton@codethink.co.uk,
lars@metafoo.de, william.towle@codethink.co.uk,
sergei.shtylyov@cogentembedded.com
Subject: Re: [PATCH v3] adv7604: add direct interrupt handling
Date: Mon, 18 Jan 2016 11:10:25 +0000 [thread overview]
Message-ID: <569CC821.6040905@xs4all.nl> (raw)
In-Reply-To: <1452698143-31897-1-git-send-email-ulrich.hecht+renesas@gmail.com>
On 01/13/2016 04:15 PM, 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@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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 | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 5bd81bd..ab4cb25 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -31,6 +31,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/hdmi.h>
> #include <linux/i2c.h>
> +#include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> @@ -1971,6 +1972,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);
> @@ -2844,8 +2855,7 @@ 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;
> + state->pdata.int1_config = ADV76XX_INT1_CONFIG_ACTIVE_LOW;
Hmm, this hardcodes the interrupt to active low. Can you use the DT to determine
whether it should be active low or high?
Regards,
Hans
>
> /* Use the default I2C addresses. */
> state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
> @@ -3235,6 +3245,16 @@ 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) {
> + err = devm_request_threaded_irq(&client->dev,
> + client->irq,
> + NULL, adv76xx_irq_handler,
> + IRQF_ONESHOT,
> + dev_name(&client->dev), state);
> + if (err)
> + goto err_entity;
> + }
> +
> err = v4l2_async_register_subdev(sd);
> if (err)
> goto err_entity;
>
WARNING: multiple messages have this Message-ID (diff)
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>,
linux-media@vger.kernel.org, linux-sh@vger.kernel.org
Cc: magnus.damm@gmail.com, laurent.pinchart@ideasonboard.com,
hans.verkuil@cisco.com, ian.molton@codethink.co.uk,
lars@metafoo.de, william.towle@codethink.co.uk,
sergei.shtylyov@cogentembedded.com
Subject: Re: [PATCH v3] adv7604: add direct interrupt handling
Date: Mon, 18 Jan 2016 12:10:25 +0100 [thread overview]
Message-ID: <569CC821.6040905@xs4all.nl> (raw)
In-Reply-To: <1452698143-31897-1-git-send-email-ulrich.hecht+renesas@gmail.com>
On 01/13/2016 04:15 PM, 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@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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 | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 5bd81bd..ab4cb25 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -31,6 +31,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/hdmi.h>
> #include <linux/i2c.h>
> +#include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> @@ -1971,6 +1972,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);
> @@ -2844,8 +2855,7 @@ 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;
> + state->pdata.int1_config = ADV76XX_INT1_CONFIG_ACTIVE_LOW;
Hmm, this hardcodes the interrupt to active low. Can you use the DT to determine
whether it should be active low or high?
Regards,
Hans
>
> /* Use the default I2C addresses. */
> state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
> @@ -3235,6 +3245,16 @@ 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) {
> + err = devm_request_threaded_irq(&client->dev,
> + client->irq,
> + NULL, adv76xx_irq_handler,
> + IRQF_ONESHOT,
> + dev_name(&client->dev), state);
> + if (err)
> + goto err_entity;
> + }
> +
> err = v4l2_async_register_subdev(sd);
> if (err)
> goto err_entity;
>
next prev parent reply other threads:[~2016-01-18 11:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-13 15:15 [PATCH v3] adv7604: add direct interrupt handling Ulrich Hecht
2016-01-13 15:15 ` Ulrich Hecht
2016-01-18 11:10 ` Hans Verkuil [this message]
2016-01-18 11:10 ` Hans Verkuil
2016-02-08 9:24 ` Hans Verkuil
2016-02-08 9:24 ` Hans Verkuil
2016-02-08 9:29 ` Lars-Peter Clausen
2016-02-08 9:29 ` Lars-Peter Clausen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=569CC821.6040905@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=hans.verkuil@cisco.com \
--cc=ian.molton@codethink.co.uk \
--cc=lars@metafoo.de \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=sergei.shtylyov@cogentembedded.com \
--cc=ulrich.hecht+renesas@gmail.com \
--cc=william.towle@codethink.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.