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
