Re: [PATCH v2 2/4] mmc: Mediatek: enable crypto hardware engine

From: Ulf Hansson <ulf.hansson@linaro.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "Peng.Zhou" <peng.zhou@mediatek.com>,
	Eric Biggers <ebiggers@kernel.org>,
	linux-block <linux-block@vger.kernel.org>,
	Chaotian Jing <chaotian.jing@mediatek.com>,
	linux-mmc <linux-mmc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Satya Tangirala <satyat@google.com>,
	Wulin Li <wulin.li@mediatek.com>, Arnd Bergmann <arnd@arndb.de>,
	Sudeep Holla <sudeep.holla@arm.com>
Subject: Re: [PATCH v2 2/4] mmc: Mediatek: enable crypto hardware engine
Date: Tue, 23 Mar 2021 14:37:35 +0100	[thread overview]
Message-ID: <CAPDyKFoCjv7S_2yyVZOAgwbEEZV5H_dmQxQGFVTt7awSE3Ss8Q@mail.gmail.com> (raw)
In-Reply-To: <CACRpkdapAOq7NtZDOgnugvTmO0+Yh+EoCVod-s_akPfs2=Sj9Q@mail.gmail.com>

On Mon, 22 Mar 2021 at 14:45, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Mar 16, 2021 at 2:56 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> > It looks like we have a couple of options to support this. I suggest
> > we consider the two below, but perhaps others (Arnd/Linus?) have
> > better ideas?
>
> Admittedly it's a bit hard to shoehorn this in as it is not a standard
> resource (clk, regulator, genpd, reset, gpio...)

In my opinion, I wouldn't object if we would model this as phy, simply
because I think it would be the easiest way. Although, I agree, it's
not a perfect fit.

>
> There is drivers/soc and then you end up with the same custom
> abstraction that qcom is using. The upside to using that
> is that we can #ifdef it to static stubs in the .h file if this SoC
> is not used, so I would go for that.
>
> See for example qcom_scm_ice_invalidate_key() used from
> drivers/firmware/qcom_scm.c, header is at
> include/linux/qcom_scm.h and here you find:
> #if IS_ENABLED(CONFIG_QCOM_SCM)
> and if not, there are some stubs.

Please, no. As discussed and also pointed out by Arnd in another
thread, generic drivers must remain portable and must not get
sprinkled with SoC specific code. If not, we would be moving backwards
and increasing the fragmentation of the kernel.

The qcom case is about programming a crypto key, which seems rather
specific to me. I can't figure out another generic way to support
this, but using the SoC specific calls.

The Mediatek case is about turning on/off a resource to activate the
device. If the phy framework doesn't work for us (or another), then at
least we should fall back to use runtime PM + PM domain provider
(genpd), because this would solve the problem. SoC specific code, like
the SMC call can then be called from the genpd provider driver and
abstracted from generic drivers.

Additionally, in this case the mmc driver has already runtime PM
support deployed, which means some of the work has already been
completed.

>
> Yours,
> Linus Walleij

Kind regards
Uffe