linux-kernel - Re: [RFC PATCH 1/5] soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZAkXcyIlAhgdU3lF@gmail.com>
Date:   Wed, 8 Mar 2023 23:17:07 +0000
From:   Eric Biggers <ebiggers@...nel.org>
To:     Abel Vesa <abel.vesa@...aro.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        "James E . J . Bottomley" <jejb@...ux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@...cle.com>,
        Manivannan Sadhasivam <mani@...nel.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-mmc@...r.kernel.org, linux-scsi@...r.kernel.org
Subject: Re: [RFC PATCH 1/5] soc: qcom: Make the Qualcomm UFS/SDCC ICE a
 dedicated driver

On Wed, Mar 08, 2023 at 11:54:25PM +0200, Abel Vesa wrote:
> > Also, in both cases, how will the ICE clock be controlled?  Currently the ICE
> > clock gets turned on and off by the UFS and eMMC drivers.  I don't see any logic
> > in your new driver that turns the clock on and off.
> 
> I added clock enablement in v2. We can decide later on if the clocks
> need to be disabled and when.

To reduce power usage, the ICE clock should be disabled when the UFS (or eMMC)
host controller clocks are disabled, as is currently the case.

> > > +struct qcom_ice {
> > > +	struct device *dev;
> > > +	struct device_node *np;
> > > +	void __iomem *base;
> > > +
> > > +	struct clk *core_clk;
> > > +
> > > +	bool supported;
> > > +};
> > 
> > Shouldn't struct qcom_ice be private to the driver?
> 
> Nope. If the QCOM_INLINE_CRYPTO_ENGINE is not set, the consumer still
> need to be able to call the ICE API (which in this case does really
> nothing). Again, this allows the consumer drivers to keep clean of
> unnecessary #ifdefs.

But isn't struct qcom_ice only dereferenced by drivers/soc/qcom/ice.c?  If other
.c files do not need it, the definition should be private to that file.

- Eric