[PATCH v9 16/22] drm/mediatek: add ETHDR support for MT8195

[Nancy.Lin]

Hi Philipp,

Thanks for the review.

On Tue, 2021-11-30 at 10:46 +0100, Philipp Zabel wrote:
> Hi Nancy,
> 
> On Tue, 2021-11-30 at 11:35 +0800, Nancy.Lin wrote:
> [...]
> > +void mtk_ethdr_stop(struct device *dev)
> > +{
> > +	struct mtk_ethdr *priv = dev_get_drvdata(dev);
> > +	struct mtk_ethdr_comp *mixer = &priv->ethdr_comp[ETHDR_MIXER];
> > +
> > +	writel(0, mixer->regs + MIX_EN);
> > +	writel(1, mixer->regs + MIX_RST);
> > +	reset_control_reset(devm_reset_control_array_get(dev, true,
> > true));
> 
> Are these reset lines really shared with other hardware units, and
> more
> specifically, are there other drivers that also try to control them?
> 
> If so, please use devm_reset_control_array_get_optional_shared().
> 
> Otherwise use devm_reset_control_array_get_optional_exclusive()
> instead.
> 
> If you really need to share the reset line with other hardware and
> have
> to trigger it more than once, the only use case supported by the
> reset
> framework is to use reset_control_reset() before starting the
> hardware
> for each user and then reset_control_rearm() after stopping it for
> all
> users (see [1]). That would both stop the reset from being triggered
> while the hardware is active (before last _rearm) and allow another
> reset once after all hardware is stopped.
> 
> [1] 
> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/driver-api/reset.htm*triggering__;Iw!!CTRNKA9wMg0ARbw!zETfuA55cH_3uiTFEknFoWXKkApsSKFbemms9qLwluMdylp-FVK3jn5E9Ld1ZCwv$
>  
> 
> Also, request the reset control once in the probe function, not every
> time ETHDR is stopped. Otherwise you'll end up leaking devres memory
> 
> regards
> Philipp

These reset lines doesn't share with other hardware units. I will use
devm_reset_control_array_get_optional_exclusive instead and and reqest
reset control in the probe function.

Regards,
Nancy