linux-kernel - RE: [PATCH 1/2] drm/adi: axi-hdmi-tx: Add support for AXI HDMI TX IP core

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: <DM6PR03MB52748E16A02E8AE049379D569B1A0@DM6PR03MB5274.namprd03.prod.outlook.com>
Date:   Fri, 23 Oct 2020 08:51:48 +0000
From:   "Togorean, Bogdan" <Bogdan.Togorean@...log.com>
To:     Sam Ravnborg <sam@...nborg.org>
CC:     "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Mike Looijmans <mike.looijmans@...ic.nl>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        David Airlie <airlied@...ux.ie>,
        Rob Herring <robh+dt@...nel.org>,
        "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>
Subject: RE: [PATCH 1/2] drm/adi: axi-hdmi-tx: Add support for AXI HDMI TX IP
 core

Thank you Sam for your review,
I'll send now V2 implementing all your remarks.

Best regards,
Bogdan

> 
> Hi Bogdan
> 
> On Mon, Oct 05, 2020 at 05:12:08PM +0300, Bogdan Togorean wrote:
> > From: Lars-Peter Clausen <lars@...afoo.de>
> >
> > The AXI HDMI HDL driver is the driver for the HDL graphics core which is
> > used on various FPGA designs. It's mostly used to interface with the
> > ADV7511 driver on some Zynq boards (e.g. ZC702 & ZedBoard).
> >
> > Link: https://wiki.analog.com/resources/tools-software/linux-drivers/drm/hdl-
> axi-hdmi
> > Link: https://wiki.analog.com/resources/fpga/docs/axi_hdmi_tx
> 
> Thanks for submitting the driver - a few high level comments after
> browsing the driver:
> 
> - Use drmm_mode_config_init() to utilize new cleanup
> - Look at other uses of drm_driver - there is macros that makes this
>   much simpler / smaller
> - Use devm_drm_dev_alloc() to allocate axi_hdmi_tx_private
>   To do so embed drm_device in axi_hdmi_tx_private - which is the way to
>   do it today
> - Do not use ddev->dev_private, it is deprecated
> - Use dev_err_probe() when you risk to see a PROBE_DEFER
> - In all include blocks sort the include alphabetically
> - Use the new interface to drm_bridge_attach() - where display driver
>   creates the connector
> - See if the Kconfig selects can be trimmed - the framebuffer releated
>   selects looks wrong (others get it wrong too)
> - Check if you can use the simple encoder - see
>   drm_simple_encoder_init()
> 
> If this is a simple one plane, one crtc display driver then it should
> use the drm_simple_* support. Or the changelog should explain why not.
> 
> We want the drivers as simple as we can - and they shall use as much of
> the helper infrastructure as they can.
> 
> We continue to develop the helper infrastructure so it is expected that
> there is some lacking behind as is the case here.
> 
>         Sam
>