Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

media: adv7180: Remove the s_power callback

The s_power callback is used a bit oddly in adv7180, the callback
adv7180_set_power() do not control power to the chip itself, but
rather the power to the chips decoder.

When the decoder is powered the chip process video data, or if no video
source is available freeruns. When the decoder is off the device i2c
registers are still powered and the device can be configured.

In the current s_power implementation the device starts to transmit
video data as soon as it's powered, and the s_stream operation only
tracks if s_stream have been called or not.

The actual configuration of the device happens when the configuration
IOCTLs are called. Sometimes with very odd implementations where the
decoder have to first be power off, the device configured, and then
unconditionally power on, see adv7180_set_pad_format() for an example.

As a first step to remedy this remove the s_power callback altogether
and instead completely initialize the device from s_stream. Future work
will clean up the IOCTL callbacks that directly configures the device
that is also done by init_device().

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

authored by

Niklas Söderlund and committed by
Hans Verkuil
a67e0eed b918cbcb

+19 -24
+19 -24
drivers/media/i2c/adv7180.c
··· 545 545 } 546 546 } 547 547 548 - static int adv7180_s_power(struct v4l2_subdev *sd, int on) 549 - { 550 - struct adv7180_state *state = to_state(sd); 551 - int ret; 552 - 553 - ret = mutex_lock_interruptible(&state->mutex); 554 - if (ret) 555 - return ret; 556 - 557 - ret = adv7180_set_power(state, on); 558 - 559 - mutex_unlock(&state->mutex); 560 - return ret; 561 - } 562 - 563 548 static const char * const test_pattern_menu[] = { 564 549 "Single color", 565 550 "Color bars", ··· 945 960 struct adv7180_state *state = to_state(sd); 946 961 int ret; 947 962 948 - /* It's always safe to stop streaming, no need to take the lock */ 949 - if (!enable) { 950 - state->streaming = enable; 951 - return 0; 952 - } 953 - 954 963 /* Must wait until querystd released the lock */ 955 - ret = mutex_lock_interruptible(&state->mutex); 964 + guard(mutex)(&state->mutex); 965 + 966 + /* 967 + * Always power off the decoder even if streaming is to be enabled, the 968 + * decoder needs to be off for the device to be configured. 969 + */ 970 + ret = adv7180_set_power(state, false); 956 971 if (ret) 957 972 return ret; 973 + 974 + if (enable) { 975 + ret = init_device(state); 976 + if (ret) 977 + return ret; 978 + 979 + ret = adv7180_set_power(state, true); 980 + if (ret) 981 + return ret; 982 + } 983 + 958 984 state->streaming = enable; 959 - mutex_unlock(&state->mutex); 985 + 960 986 return 0; 961 987 } 962 988 ··· 996 1000 }; 997 1001 998 1002 static const struct v4l2_subdev_core_ops adv7180_core_ops = { 999 - .s_power = adv7180_s_power, 1000 1003 .subscribe_event = adv7180_subscribe_event, 1001 1004 .unsubscribe_event = v4l2_event_subdev_unsubscribe, 1002 1005 };