linux-kernel - Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox 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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Aug 2019 14:30:35 +0200
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Chen-Yu Tsai <wens@...e.org>,
        Jassi Brar <jassisinghbrar@...il.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Corentin Labbe <clabbe.montjoie@...il.com>,
        Vasily Khoruzhick <anarsoul@...il.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-sunxi@...glegroups.com, linux-clk@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver

On Tue, Aug 20, 2019 at 08:07:53AM -0500, Samuel Holland wrote:
> On 8/20/19 6:18 AM, Ondřej Jirman wrote:
> >> +	reset = devm_reset_control_get(dev, NULL);
> >> +	if (IS_ERR(reset)) {
> >> +		ret = PTR_ERR(reset);
> >> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	ret = reset_control_deassert(reset);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >
> > You need to assert the reset again from now on, in error paths. devm
> > will not do that for you.
>
> I know, and that's intentional. This same message box device is used for ATF to
> communicate with SCP firmware (on a different channel). This could be happening
> on a different core while Linux is running. So Linux is not allowed to deassert
> the reset. clk_disable_unprepare() is only okay because the clock is marked as
> critical.

I agree with Ondrej that since this is clearly not the standard use of
the API, this must have a big comment explaining why we're doing it
this way.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)