Re: [PATCH v2 2/3] i2c: k1: add reset support

Troy Mitchell
2025-12-30T14:12:57Z

On Sun, Dec 28, 2025 at 08:42:47AM +0800, Guodong Xu wrote:
> Hi, Andi
>
> On Sun, Dec 28, 2025 at 3:26 AM Andi Shyti <andi.shyti@kernel.org> wrote:
> >
> > Hi Guodong,
> >
> > On Fri, Dec 26, 2025 at 07:38:22AM +0800, Guodong Xu wrote:
> > > On Fri, Dec 26, 2025 at 5:01 AM Andi Shyti <andi.shyti@kernel.org> wrote:
> > > > > + rst = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
> > > > > + if (IS_ERR(rst))
> > > > > + return dev_err_probe(dev, PTR_ERR(rst),
> > > > > + "failed to acquire deasserted reset\n");
> > > >
> > > > If this is optional, why are we returning with error?
> > > >
> > >
> > > According to include/linux/reset.h, if the requested reset is not
> > > specified in the device tree, this function returns NULL instead of
> > > an error. Therefore, IS_ERR(rst) will only be true for actual
> > > errors (e.g probe deferral).
> >
> > And this is quite obvious, but you haven't answered my qestion.
> >
> > Why do we care of internal failures in reset? If reset fails on
> > an optional reset control function why should we kill our driver?
>
> Thanks for the clarification. I see your point now.
>
> My reasoning is that if the resets property is explicitly listed in the
> Device Tree, the driver must respect it.
It's not required.

>
> If the property is present but
> we encounter an error (like -EPROBE_DEFER), ignoring that failure could
> put the hardware in an undefined or dirty state.
Then why it's optional?

The real reason:
"Optional" here means the reset line is allowed to be absent from the
Device Tree. It does not mean we can ignore the failure when it is
defined in the DT but fails to be acquired.

If devm_reset_control_get_optional_* returns an error (e.g.,
-EPROBE_DEFER), it indicates the hardware description expects a reset
control, but the system is not yet ready to provide it. Ignoring this
error would break the probe deferral mechanism and potentially cause the
driver to access hardware in an invalid state.

- Troy