linux-kernel - Re: [PATCH v2 1/3] v4l: common: v4l2_get_link

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <b9bc3f15-ef8e-af4d-32dc-83cb82c1197b@linaro.org>
Date:   Thu, 18 Feb 2021 20:14:10 +0300
From:   Andrey Konovalov <andrey.konovalov@...aro.org>
To:     Jacopo Mondi <jacopo@...ndi.org>
Cc:     junak.pub@...il.com, robert.foss@...aro.org,
        sakari.ailus@...ux.intel.com, todor.too@...il.com,
        agross@...nel.org, bjorn.andersson@...aro.org, mchehab@...nel.org,
        laurent.pinchart@...asonboard.com, linux-media@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] v4l: common: v4l2_get_link_freq: add printing a
 warning

Hi Jacopo,

On 18.02.2021 10:55, Jacopo Mondi wrote:
> Hi Andrey,
> 
> On Thu, Feb 18, 2021 at 01:11:32AM +0300, Andrey Konovalov wrote:
>> Print a warning if V4L2_CID_LINK_FREQ control is not implemented.
>>
>> Signed-off-by: Andrey Konovalov <andrey.konovalov@...aro.org>
>> ---
>>   drivers/media/v4l2-core/v4l2-common.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>> index 133d20e40f82..f1abdf2ab4ec 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -461,6 +461,8 @@ s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul,
>>
>>   		freq = qm.value;
>>   	} else {
>> +		pr_warn("%s: V4L2_CID_LINK_FREQ not implemented\n", __func__);
>> +
> 
> It's a shame we can't access a struct device * somehow :(

Right. Otherwise the message could be even more useful.

> Also, nitpicking (please bear with me here) it is absolutely correct
> that V4L2_CID_LINK_FREQ is not implemented, but I think the real deal
> here is that the link rate is estimanted from PIXEL_RATE and that
> might be wrong.

Correct.

> What about (insipired from the error message in match_fwnode() which I
> find useful)
> 
>                  pr_warn("%s: Link frequency estimanted using pixel rate: result might be inaccurate\n",
>                          __func__);
>                  pr_warn("%s: Consider implementing support for V4L2_CID_LINK_FREQ in the transmitter driver\n",
>                          __func___);

This text is fine for me.
Just the both of these pr_warn's wouldn't fit into the 80 characters limit.
I tried to think out something more descriptive within this limit, but
gave up soon.

Let me post the v2.1 of this single patch with the longer warning message.
Then the maintainers could probably merge whatever they prefer.
Or I could post the v3 of the whole patchset if the [PATCH v2.1 1/3]
gets positive feedback.

Thanks,
Andrey

> Anyway, whatever works
> Reviewed-by: Jacopo Mondi <jacopo@...ndi.org>
> 
> Thanks
>     j
> 
>>   		if (!mul || !div)
>>   			return -ENOENT;
>>
>> --
>> 2.17.1
>>