linux-kernel - Re: [PATCH v9 16/22] drm/mediatek: add ETHDR support for MT8195

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>] [day] [month] [year] [list]

Message-ID: <ea057e8b080a7f553ee45f282459090dca4fb4c6.camel@pengutronix.de>
Date:   Tue, 30 Nov 2021 10:46:26 +0100
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     "Nancy.Lin" <nancy.lin@...iatek.com>, CK Hu <ck.hu@...iatek.com>
Cc:     Chun-Kuang Hu <chunkuang.hu@...nel.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "jason-jh . lin" <jason-jh.lin@...iatek.com>,
        Yongqiang Niu <yongqiang.niu@...iatek.com>,
        dri-devel@...ts.freedesktop.org,
        linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        singo.chang@...iatek.com, srv_heupstream@...iatek.com
Subject: Re: [PATCH v9 16/22] drm/mediatek: add ETHDR support for MT8195

Hi Nancy,

On Tue, 2021-11-30 at 11:35 +0800, Nancy.Lin wrote:
[...]
> +void mtk_ethdr_stop(struct device *dev)
> +{
> +	struct mtk_ethdr *priv = dev_get_drvdata(dev);
> +	struct mtk_ethdr_comp *mixer = &priv->ethdr_comp[ETHDR_MIXER];
> +
> +	writel(0, mixer->regs + MIX_EN);
> +	writel(1, mixer->regs + MIX_RST);
> +	reset_control_reset(devm_reset_control_array_get(dev, true, true));

Are these reset lines really shared with other hardware units, and more
specifically, are there other drivers that also try to control them?

If so, please use devm_reset_control_array_get_optional_shared().

Otherwise use devm_reset_control_array_get_optional_exclusive() instead.

If you really need to share the reset line with other hardware and have
to trigger it more than once, the only use case supported by the reset
framework is to use reset_control_reset() before starting the hardware
for each user and then reset_control_rearm() after stopping it for all
users (see [1]). That would both stop the reset from being triggered
while the hardware is active (before last _rearm) and allow another
reset once after all hardware is stopped.

[1] https://www.kernel.org/doc/html/latest/driver-api/reset.htm#triggering

Also, request the reset control once in the probe function, not every
time ETHDR is stopped. Otherwise you'll end up leaking devres memory

regards
Philipp