public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: Mark Brown <broonie@kernel.org>,
	Paul Handrigan <paulha@opensource.cirrus.com>
Cc: <lgirdwood@gmail.com>, <linux-sound@vger.kernel.org>,
	<robh@kernel.org>, <krzk+dt@kernel.org>, <conor+dt@kernel.org>,
	<devicetree@vger.kernel.org>, <patches@opensource.cirrus.com>
Subject: Re: [PATCH v3 2/2] ASoC: cs530x: Support for cs530x ADCs
Date: Tue, 18 Jun 2024 15:23:07 +0100	[thread overview]
Message-ID: <4e192a95-bb3d-41d4-b83f-176f0f8aba6b@opensource.cirrus.com> (raw)
In-Reply-To: <96162ff4-8c54-4efa-b06a-dc20e358e712@sirena.org.uk>

On 18/06/2024 15:18, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 02:01:02PM -0500, Paul Handrigan wrote:
> 
>> +static bool cs530x_volatile_register(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	case CS530X_DEVID:
>> +	case CS530X_REVID:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
> 
> Are these really volatile?  I would expect them to have no defaults so
> they must be read from the device, but once read I'd expect we could
> cache the values.
> 
> Otherwise this looks good.

If you mark a register non-volatile but without default, a
regcache_sync() will write it back out to the device. While that
doesn't necessarily do any harm, that depends on what these
registers do on write. Generally it makes me nervous to have
cache syncs writing to registers we don't want to write to.

  reply	other threads:[~2024-06-18 14:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17 19:01 [PATCH v3 1/2] ASoC: dt-bindings: cirrus,cs530x: Add initial DT binding Paul Handrigan
2024-06-17 19:01 ` [PATCH v3 2/2] ASoC: cs530x: Support for cs530x ADCs Paul Handrigan
2024-06-18 14:18   ` Mark Brown
2024-06-18 14:23     ` Richard Fitzgerald [this message]
2024-06-18 14:25       ` Mark Brown
2024-06-18 14:43         ` Richard Fitzgerald
2024-06-18 14:53           ` Mark Brown
2024-06-18  7:30 ` [PATCH v3 1/2] ASoC: dt-bindings: cirrus,cs530x: Add initial DT binding Krzysztof Kozlowski
2024-06-18 16:11   ` Paul Handrigan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4e192a95-bb3d-41d4-b83f-176f0f8aba6b@opensource.cirrus.com \
    --to=rf@opensource.cirrus.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-sound@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=paulha@opensource.cirrus.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox