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
| ||
|
Date: Fri, 5 Jul 2019 11:44:52 +0200 From: Andrzej Hajda <a.hajda@...sung.com> To: Thomas Zimmermann <tzimmermann@...e.de>, Andrzej Pietrasiewicz <andrzej.p@...labora.com>, dri-devel@...ts.freedesktop.org Cc: kernel@...labora.com, Alex Deucher <alexander.deucher@....com>, Christian König <christian.koenig@....com>, "David (ChunMing) Zhou" <David1.Zhou@....com>, David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>, Dave Airlie <airlied@...hat.com>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <maxime.ripard@...tlin.com>, Sean Paul <sean@...rly.run>, Inki Dae <inki.dae@...sung.com>, Seung-Woo Kim <sw0312.kim@...sung.com>, Krzysztof Kozlowski <krzk@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, amd-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, linux-mediatek@...ts.infradead.org, linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org, linux-rockchip@...ts.infradead.org, linux-tegra@...r.kernel.org Subject: Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector On 01.07.2019 16:41, Thomas Zimmermann wrote: > Hi > > Am 01.07.19 um 15:27 schrieb Andrzej Pietrasiewicz: >> Hi Thomas, >> >> Thank you for your comments. Please see inline. >> >> W dniu 30.06.2019 o 10:12, Thomas Zimmermann pisze: >>> Hi >>> >>> I like the idea, but would prefer a more structured approach. >>> >>> Setting connector->ddc before calling drm_sysfs_connector_add() seems >>> error prone. The dependency is not really clear from the code or >>> interfaces. >>> >>> The other thing is that drivers I mostly work on, ast and mgag200, have >>> code like this: >>> >>> struct ast_i2c_chan { >>> struct i2c_adapter adapter; >>> struct drm_device *dev; >>> struct i2c_algo_bit_data bit; >>> }; >>> >>> struct ast_connector { >>> struct drm_connector base; >>> struct ast_i2c_chan *i2c; >>> }; >>> >>> It already encodes the connection between connector and ddc adapter. >>> >>> I suggest to introduce struct drm_i2c_adapter >>> >>> struct drm_i2c_adapter { >>> struct i2c_adapter base; >>> struct drm_connector *connector; >>> }; >>> >>> and convert drivers over to it. Ast would look like this: >>> >>> struct ast_i2c_chan { >>> struct drm_i2c_adapter adapter; >>> struct i2c_algo_bit_data bit; >>> }; >>> >> There are few flavors of these drivers. In some of them >> the i2c_adapter for ddc is allocated and initialized by >> the driver, which must provide a place in memory to hold >> the adapter. ast is an example of this approach. >> >> But there are others, such as for example exynos_hdmi.c. >> It gets its ddc adapter with of_find_i2c_adapter_by_node() >> and merely stores a pointer to it, while not managing the >> memory needed to hold the i2c_adapter. > I see. > >> Do you have any idea how to accommodate these various >> flavors of drivers in the scheme you propose? > Something like > > struct drm_i2c_adapter { > struct i2c_adapter *adapter; > struct drm_connector *connector; > }; > > with adapter either being allocated dynamically or acquired via > of_find_i2c_adapter_by_node(); with separate init and finish functions > for each variant. But it looks like over-abstraction to me. Without > prototyping the idea, I cannot say if it's worth the effort. > > For something more simple, maybe just have a function to attach an i2c > adapter to a connector: > > drm_connector_attach_i2c_adapter(connector, i2c_adapter) > > which sets the connector's ddc pointer and creates the sysfs entry if > the connector's entry is already present. I am not sure if such function is really necessary. Assigning ddc field before connector registration seems to be much simpler and quite standard - many fields are initialized this way. Reviewed-by: Andrzej Hajda <a.hajda@...sung.com> -- Regards Andrzej > Best regards > Thomas > >> Andrzej >> >>
Powered by blists - more mailing lists