Hi Uwe, On 03/02/2015 08:00 AM, Uwe Kleine-König wrote: > Since 39b2bbe3d715 (gpio: add flags argument to gpiod_get*() functions) > which appeared in v3.17-rc1, the gpiod_get* functions take an additional > parameter that allows to specify direction and initial value for output. > Simplify accordingly. > > Moreover use devm_gpiod_get_index_optional instead of > devm_gpiod_get_index with ignoring all errors. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > --- > BTW, sparse fails to check this file with many errors like: > > drivers/media/i2c/adv7604.c:311:11: error: unknown field name in initializer > > Didn't look into that. That's a sparse bug that's been fixed in the sparse repo, but not in the 0.5.0 release (they really should make a new sparse release IMHO). Some comments below: > --- > drivers/media/i2c/adv7604.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c > index 5a7c9389a605..ddeeb6695a4b 100644 > --- a/drivers/media/i2c/adv7604.c > +++ b/drivers/media/i2c/adv7604.c > @@ -537,12 +537,8 @@ static void adv7604_set_hpd(struct adv7604_state *state, unsigned int hpd) > { > unsigned int i; > > - for (i = 0; i < state->info->num_dv_ports; ++i) { > - if (IS_ERR(state->hpd_gpio[i])) > - continue; Why this change? See also below: > - > + for (i = 0; i < state->info->num_dv_ports; ++i) > gpiod_set_value_cansleep(state->hpd_gpio[i], hpd & BIT(i)); > - } > > v4l2_subdev_notify(&state->sd, ADV7604_HOTPLUG, &hpd); > } > @@ -2720,13 +2716,13 @@ static int adv7604_probe(struct i2c_client *client, > /* Request GPIOs. */ > for (i = 0; i < state->info->num_dv_ports; ++i) { > state->hpd_gpio[i] = > - devm_gpiod_get_index(&client->dev, "hpd", i); > + devm_gpiod_get_index_optional(&client->dev, "hpd", i, > + GPIOD_OUT_LOW); > if (IS_ERR(state->hpd_gpio[i])) > - continue; > - > - gpiod_direction_output(state->hpd_gpio[i], 0); > + return PTR_ERR(state->hpd_gpio[i]); This isn't correct. The use of gpio is optional, on some boards a different mechanism is used to control the hpd, and there devm_gpiod_get_index will just return an error. That's OK, we just continue in that case. Regards, Hans > > - v4l_info(client, "Handling HPD %u GPIO\n", i); > + if (state->hpd_gpio[i]) > + v4l_info(client, "Handling HPD %u GPIO\n", i); > } > > state->timings = cea640x480; > -- 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