Re: [PATCH v3 1/3] noon010pc30: Conversion to the media controller API

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hi Laurent,

thanks for the review.

On 09/21/2011 12:18 AM, Laurent Pinchart wrote:
> Hi Sylwester,
> 
> Thanks for the patch.
> 
> On Friday 16 September 2011 17:59:54 Sylwester Nawrocki wrote:
>> Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations.
>> Add media entity initialization and set subdev flags so the host driver
>> creates a subdev device node for the driver.
>> A mutex was added for serializing the subdev operations. When setting
>> format is attempted during streaming an (EBUSY) error will be returned.
>>
>> After the device is powered up it will now remain in "power sleep"
>> mode until s_stream(1) is called. The "power sleep" mode is used
>> to suspend/resume frame generation at the sensor's output through
>> s_stream op.
>>
>> While at here simplify the colorspace parameter handling.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> 
> [snip]
> 
>> diff --git a/drivers/media/video/noon010pc30.c
>> b/drivers/media/video/noon010pc30.c index 35f722a..115d976 100644
>> --- a/drivers/media/video/noon010pc30.c
>> +++ b/drivers/media/video/noon010pc30.c
> 
> [snip]
> 
>> @@ -599,6 +641,22 @@ static int noon010_log_status(struct v4l2_subdev *sd)
>>  	return 0;
>>  }
>>
>> +static int noon010_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> +	struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0);
>> +	struct noon010_info *info = to_noon010(sd);
>> +
>> +	mutex_lock(&info->lock);
>> +	noon010_get_current_fmt(to_noon010(sd), mf);
> 
> Should you initialize mf with a constant default format instead of retrieving 
> the current format from the sensor ? A non-constant default would probably 
> confuse userspace application.

Sure, I could change to it. But I don't quite see the problem, the applications
should call set_fmt(TRY) anyway, rather than relying on the format gotten right
after opening the device, shouldn't they ? Anyway I guess it's better to have
all drivers behaving consistently.


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center