Re: [PATCH v3 4/5] i2c: tegra: Add support for SW mutex register

Thierry Reding
2025-07-07T15:29:51Z

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

On Mon, Jun 16, 2025 at 10:25:18AM +0000, Kartik Rajput wrote:
[...]
> > > diff --git a/drivers/i2c/busses/i2c-tegra.c
[...]
> > > + if (!num_retries)
> > > + dev_warn(i2c_dev->dev, "timeout while acquiring
> > > mutex, proceeding anyway\n");
> > > +}
> >
> > I take it there's no way to refuse operations since there's no return
> > value for this function? I wonder if it's the right interface for
> > this,
> > then. If there's no mechanism to enforce the lock in hardware, then
> > we
> > must somehow respect it in software. Proceeding even after failing to
> > acquire the lock seems like a recipe for breaking things.
>
> Should I move the lock/unlock operations to
> tegra_i2c_runtime_resume/suspend functions?
>
> That way we can propagate the error correctly and fail in case we do
> not acquire the mutex.

suspend/resume isn't the proper place for this. On one hand, returning
an error from suspend/resume typically indicates that something went
wrong with the power management bits, and the SW mutex is not that.

Also, runtime PM support can be disabled, so those callbacks are not
guaranteed to be called in the proper sequence.

I think we want to add calls for these to tegra_i2c_xfer(), after the
pm_runtime_get_sync() and before pm_runtime_put(), respectively.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]