Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH RESEND 0/2] Common protected-clocks implementation
To:     Samuel Holland <samuel@sholland.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Maxime Ripard <mripard@kernel.org>
Cc:     linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>
References: <20200903040015.5627-1-samuel@sholland.org>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
Message-ID: <9363f63f-8584-2d84-71fd-baca13e16164@rasmusvillemoes.dk>
Date:   Tue, 9 Mar 2021 09:03:14 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20200903040015.5627-1-samuel@sholland.org>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/09/2020 06.00, Samuel Holland wrote:
> Stephen, Maxime,
> 
> You previously asked me to implement the protected-clocks property in a
> driver-independent way:
> 
> https://www.spinics.net/lists/arm-kernel/msg753832.html
> 
> I provided an implementation 6 months ago, which I am resending now:
> 
> https://patchwork.kernel.org/patch/11398629/
> 
> Do you have any comments on it?

I'm also interested [1] in getting something like this supported in a
generic fashion - i.e., being able to mark a clock as
protected/critical/whatnot by just adding an appropriate property in the
clock provider's DT node, but without modifying the driver to opt-in to
handling it.

Now, as to this implementation, the commit 48d7f160b1 which added the
common protected-clocks binding says

  For example, on some Qualcomm firmwares reading or writing certain clk
  registers causes the entire system to reboot,

so I'm not sure handling protected-clocks by translating it to
CLK_CRITICAL and thus calling prepare/enable on it is the right thing to
do - clks that behave like above are truly "hands off, kernel", so the
current driver-specific implementation of simply not registering those
clocks seems to be the right thing to do - or at least the clk framework
would need to be taught to not actually call any methods on such
protected clocks.

For my use case, either "hands off kernel" or "make sure this clock is
enabled" would work since the bootloader anyway enables the clock.

Rasmus

[1]
https://lore.kernel.org/lkml/20210226141411.2517368-1-linux@rasmusvillemoes.dk/