linux-kernel - Re: [PATCH] drm: bridge: dw-hdmi: support i2c extended read mode

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:   Thu, 23 Feb 2017 16:50:27 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Nickey Yang <nickey.yang@...k-chips.com>
Cc:     architt@...eaurora.org, David Airlie <airlied@...ux.ie>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
        joabreu@...opsys.com,
        闫孝军 <andy.yan@...k-chips.com>
Subject: Re: [PATCH] drm: bridge: dw-hdmi: support i2c extended read mode

Hi,

On Tue, Feb 21, 2017 at 11:45 PM, Nickey Yang
<nickey.yang@...k-chips.com> wrote:
> "I2C Master Interface Extended Read Mode" implements a segment
> pointer-based read operation using the Special Register configuration.
>
> This patch fix https://patchwork.kernel.org/patch/7098101/ mentioned
> "The current implementation does not support "I2C Master Interface
> Extended Read Mode" to read data addressed by non-zero segment
> pointer, this means that if EDID has more than 1 extension blocks"
>
> With this patch,dw-hdmi can read EDID data with 1/2/4 blocks.
>
> Signed-off-by: Nickey Yang <nickey.yang@...k-chips.com>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 38 ++++++++++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 14 deletions(-)

I don't have any deep expertise on how the DW HDMI's i2c controller is
supposed to work and the truly correct way to specify the segment
address.  However, this looks fairly sane to me and review feedback I
provided on http://crosreview.com/442308 has been addressed.

I would agree that this appears to handle the types of messages that
the kernel seems to construct, basically noticing the first write to
DDC_SEGMENT_ADDR and stashing the data that the kernel provides in
HDMI_I2CM_SEGPTR.  It then tells dw-hdmi to do a
HDMI_I2CM_OPERATION_READ_EXT instead of HDMI_I2CM_OPERATION_READ.

Anyway, presuming that's the right way to tell dw-hdmi, I'd say:

Reviewed-by: Douglas Anderson <dianders@...omium.org>