Hi Hans,
Thanks for the express comments!
<cut>
>> +
>> +struct vidc_core {
>> + struct list_head list;
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clks[VIDC_CLKS_NUM_MAX];
>> + struct mutex lock;
>> + struct hfi_core hfi;
>> + struct video_device vdev_dec;
>> + struct video_device vdev_enc;
>
> I know that many drivers embed struct video_device, but this can cause subtle
> refcounting problems. I recommend changing this to a pointer and using
> video_device_alloc().
>
> I have plans to reorganize the way video_devices are allocated and registered
> in
> the near future, and you might just as well prepare this driver for that by
> switching
> to a pointer.
OK, thanks for the info, I will change to pointers.
<cut>
>> +
>> +int vidc_set_color_format(struct vidc_inst *inst, u32 type, u32 pixfmt)
>> +{
>> + struct hfi_uncompressed_format_select fmt;
>> + struct hfi_core *hfi = &inst->core->hfi;
>> + u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SELECT;
>> + int ret;
>> +
>> + fmt.buffer_type = type;
>> +
>> + switch (pixfmt) {
>> + case V4L2_PIX_FMT_NV12:
>> + fmt.format = HFI_COLOR_FORMAT_NV12;
>> + break;
>> + case V4L2_PIX_FMT_NV21:
>> + fmt.format = HFI_COLOR_FORMAT_NV21;
>> + break;
>> + default:
>> + return -ENOTSUPP;
>
> I'm not really sure how this error code is used, but normally -EINVAL is
> returned
> for invalid pixel formats. -ENOTSUPP is not used by V4L2.
>
you are right, I need to change this to EINVAL.
--
regards,
Stan
