Re: [PATCH v2 07/14] mfd: zl3073x: Add components versions register defs

From: Ivan Vecera
Date: Thu Apr 10 2025 - 14:45:57 EST




On 10. 04. 25 7:41 odp., Andrew Lunn wrote:
+ /* Take device lock */

What is a device lock? Why do you need to comment standard guards/mutexes?

Just to inform code reader, this is a section that accesses device registers
that are protected by this zl3073x device lock.

I didn't see a reply to my question about the big picture. Why is the
regmap lock not sufficient. Why do you need a device lock.


+ scoped_guard(zl3073x, zldev) {
+ rc = zl3073x_read_id(zldev, &id);
+ if (rc)
+ return rc;
+ rc = zl3073x_read_revision(zldev, &revision);
+ if (rc)
+ return rc;
+ rc = zl3073x_read_fw_ver(zldev, &fw_ver);
+ if (rc)
+ return rc;
+ rc = zl3073x_read_custom_config_ver(zldev, &cfg_ver);
+ if (rc)
+ return rc;
+ }

Nothing improved here. Andrew comments are still valid and do not send
v3 before the discussion is resolved.

I'm accessing device registers here and they are protected by the device
lock. I have to take the lock, register access functions expect this by
lockdep_assert.

Because lockdep asserts is not a useful answer. Locks are there to
protect you from something. What are you protecting yourself from? If
you cannot answer that question, your locking scheme is built on sand
and very probably broken.

Andrew

Hi Andrew,
I have described the locking requirements under different message [v1 05/28]. Could you please take a look?

Thanks,
Ivan