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

[Andrew Lunn]

> > > +	/* 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