Re: [PATCH v4 2/8] media: adv7604: automatic "default-input" selection

[Hans Verkuil]

On 05/11/2016 04:02 PM, Ulrich Hecht wrote:
> From: William Towle 
> 
> Add logic such that the "default-input" property becomes unnecessary
> for chips that only have one suitable input (ADV7611 by design, and
> ADV7612 due to commit 7111cddd518f ("[media] media: adv7604: reduce
> support to first (digital) input").
> 
> Additionally, Ian's documentation in commit bf9c82278c34 ("[media]
> media: adv7604: ability to read default input port from DT") states
> that the "default-input" property should reside directly in the node
> for adv7612. Hence, also adjust the parsing to make the implementation
> consistent with this.
> 
> Signed-off-by: William Towle 
> Signed-off-by: Ulrich Hecht 
> ---
>  drivers/media/i2c/adv7604.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 41a1bfc..d722c16 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -2788,7 +2788,7 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
>   struct device_node *np;
>   unsigned int flags;
>   int ret;
> - u32 v;
> + u32 v = -1;
>  
>   np = state->i2c_clients[ADV76XX_PAGE_IO]->dev.of_node;
>  
> @@ -2810,6 +2810,22 @@ static int adv76xx_parse_dt(struct adv76xx_state 
> *state)
>  
>   of_node_put(endpoint);
>  
> + if (of_property_read_u32(np, "default-input", )) {
> + /* not specified ... can we choose automatically? */
> + switch (state->info->type) {
> + case ADV7611:
> + v = 0;
> + break;
> + case ADV7612:
> + if (state->info->max_port == ADV76XX_PAD_HDMI_PORT_A)
> + v = 0;
> + /* else is unhobbled, leave unspecified */

Please add a break here, don't fall through.

What happens when the default_input is unspecified? I don't really like this,
I think that if nothing is specified, then it should just fall back to
input 0.

Note that neither include/media/i2c/adv7604.h nor 
Documentation/devicetree/bindings/media/i2c/adv7604.txt
say anything about this either.

Regards,

Hans

> + default:
> + break;
> + }
> + }
> + state->pdata.default_input = v;
> +
>   flags = bus_cfg.bus.parallel.flags;
>  
>   if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/8] media: adv7604: automatic "default-input" selection

[Ulrich Hecht]

From: William Towle 

Add logic such that the "default-input" property becomes unnecessary
for chips that only have one suitable input (ADV7611 by design, and
ADV7612 due to commit 7111cddd518f ("[media] media: adv7604: reduce
support to first (digital) input").

Additionally, Ian's documentation in commit bf9c82278c34 ("[media]
media: adv7604: ability to read default input port from DT") states
that the "default-input" property should reside directly in the node
for adv7612. Hence, also adjust the parsing to make the implementation
consistent with this.

Signed-off-by: William Towle 
Signed-off-by: Ulrich Hecht 
---
 drivers/media/i2c/adv7604.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 41a1bfc..d722c16 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2788,7 +2788,7 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
struct device_node *np;
unsigned int flags;
int ret;
-   u32 v;
+   u32 v = -1;
 
np = state->i2c_clients[ADV76XX_PAGE_IO]->dev.of_node;
 
@@ -2810,6 +2810,22 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
 
of_node_put(endpoint);
 
+   if (of_property_read_u32(np, "default-input", )) {
+   /* not specified ... can we choose automatically? */
+   switch (state->info->type) {
+   case ADV7611:
+   v = 0;
+   break;
+   case ADV7612:
+   if (state->info->max_port == ADV76XX_PAD_HDMI_PORT_A)
+   v = 0;
+   /* else is unhobbled, leave unspecified */
+   default:
+   break;
+   }
+   }
+   state->pdata.default_input = v;
+
flags = bus_cfg.bus.parallel.flags;
 
if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
-- 
2.7.4

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