linux-kernel - Re: [PATCH 2/8] media: vidc: adding core part and helper functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 22 Aug 2016 19:03:25 +0300
From:   Stanimir Varbanov <stanimir.varbanov@...aro.org>
To:     Hans Verkuil <hverkuil@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/8] media: vidc: adding core part and helper functions

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