linux-kernel - Re: [RFC PATCH 5/5] media: platform: Add Chrome OS EC CEC driver

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]

Date:   Tue, 15 May 2018 10:30:57 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Neil Armstrong <narmstrong@...libre.com>, airlied@...ux.ie,
        hans.verkuil@...co.com, lee.jones@...aro.org, olof@...om.net,
        seanpaul@...gle.com
Cc:     sadolfsson@...gle.com, felixe@...gle.com, bleung@...gle.com,
        darekm@...gle.com, marcheu@...omium.org, fparent@...libre.com,
        dri-devel@...ts.freedesktop.org, linux-media@...r.kernel.org,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 5/5] media: platform: Add Chrome OS EC CEC driver

On 05/15/18 10:28, Neil Armstrong wrote:
>>>>> +	int ret;
>>>>> +
>>>>> +	cros_ec_cec = devm_kzalloc(&pdev->dev, sizeof(*cros_ec_cec),
>>>>> +				   GFP_KERNEL);
>>>>> +	if (!cros_ec_cec)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	platform_set_drvdata(pdev, cros_ec_cec);
>>>>> +	cros_ec_cec->cros_ec = cros_ec;
>>>>> +
>>>>> +	ret = cros_ec_cec_get_notifier(&cros_ec_cec->notify);
>>>>> +	if (ret) {
>>>>> +		dev_warn(&pdev->dev, "no CEC notifier available\n");
>>>>> +		cec_caps |= CEC_CAP_PHYS_ADDR;
>>>>
>>>> Can this happen? What hardware has this? I am strongly opposed to CEC drivers
>>>> using this capability unless there is no other option. It's a pain for userspace.
>>>
>>> It's in case an HW having a CEC capable FW but not in the cec_dmi_match_table, in this case
>>> it won't fail but still enable the CEC interface without a notifier.
>>
>> I don't think that's a good idea. CAP_PHYS_ADDR should *only* be used in situations
>> where it is truly impossible to tell which output is connected to the CEC adapter.
>> That's the case with e.g. USB CEC dongles where you have no idea how the user
>> connected the HDMI cables.
>>
>> But I assume that in this case it just means that the cec_dmi_match_table needs
>> to be updated, i.e. it is a driver bug.
> 
> Yep, maybe a dev_warn should be added to notify this bug ?

Yes, a dev_warn and then return -ENODEV.

> 
>>
>> Another thing: this driver assumes that there is only one CEC adapter for only
>> one HDMI output. But what if there are more HDMI outputs? Will there be one
>> CEC adapter for each output? Or does the hardware have no provisions for that?
> 
> The current EC interface only exposes a single CEC interface for now, there is no
> plan yet for multiple HDMI outputs handling.
> 
>>
>> Something should be mentioned about this in a comment.
> 
> Ok

Thanks!

	Hans