All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: Jan Dakinevich <jan.dakinevich@salutedevices.com>,
	Jerome Brunet <jbrunet@baylibre.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	linux-kernel@vger.kernel.org, linux-amlogic@lists.infradead.org,
	linux-clk@vger.kernel.org
Subject: Re: [PATCH 1/8] reset: amlogic: convert driver to regmap
Date: Thu, 18 Jul 2024 12:29:13 -0700	[thread overview]
Message-ID: <11e8dd92e07674133d8a291cc016c314.sboyd@kernel.org> (raw)
In-Reply-To: <f5bc9590-f37e-491e-9978-c1eab8914c30@salutedevices.com>

Quoting Jan Dakinevich (2024-07-18 04:01:28)
> 
> 
> On 7/18/24 10:19, Jerome Brunet wrote:
> > On Thu 18 Jul 2024 at 05:39, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> >> and using of regmap_write() (apparently) fixes it.
> > 
> > Nor does that conclusion.
> > > It is perfectly possible I have made mistake somewhere (I have already
> > fixed one in v2), but sending incomplete report like this, with
> > unargumented conclusion is just noise in the end.
> > 
> > No, there is no situation in which `regmap_write` would solve a problem
> > with `regmap_update_bits`.
> > 
> 
> What is the default regs' value of this reset controller? The doc that I
> have doesn't clearly specifies it, but it tells that "the reset will
> auto-cover to 0 by HW". However pr_info() say that there is 0xffffffff
> before write (I am talking about A1).
> 
> Also we know, that reset is triggered by writing 1 to specific bit. So,
> what will happen if 1 will be written where we did not intend to write it?
> 
> > Either send a full analysis of the problem you found, if you did one, or
> > at least send the full dump, explaining that you don't know what is
> > happening.
> > 
> 
> Full analysis is following:
> - using regmap_update_bits() instead of writel() is incorrect because
> this changes the behavior of the driver
> - regmap_update_bits() should not be used here because default value of
> regs isn't taken into account and (_apparently_, the doc is terse) these
> regs could be updated by hw itself.

Maybe use regmap_write_bits() instead.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org>
To: Jan Dakinevich <jan.dakinevich@salutedevices.com>,
	Jerome Brunet <jbrunet@baylibre.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	linux-kernel@vger.kernel.org, linux-amlogic@lists.infradead.org,
	linux-clk@vger.kernel.org
Subject: Re: [PATCH 1/8] reset: amlogic: convert driver to regmap
Date: Thu, 18 Jul 2024 12:29:13 -0700	[thread overview]
Message-ID: <11e8dd92e07674133d8a291cc016c314.sboyd@kernel.org> (raw)
In-Reply-To: <f5bc9590-f37e-491e-9978-c1eab8914c30@salutedevices.com>

Quoting Jan Dakinevich (2024-07-18 04:01:28)
> 
> 
> On 7/18/24 10:19, Jerome Brunet wrote:
> > On Thu 18 Jul 2024 at 05:39, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> >> and using of regmap_write() (apparently) fixes it.
> > 
> > Nor does that conclusion.
> > > It is perfectly possible I have made mistake somewhere (I have already
> > fixed one in v2), but sending incomplete report like this, with
> > unargumented conclusion is just noise in the end.
> > 
> > No, there is no situation in which `regmap_write` would solve a problem
> > with `regmap_update_bits`.
> > 
> 
> What is the default regs' value of this reset controller? The doc that I
> have doesn't clearly specifies it, but it tells that "the reset will
> auto-cover to 0 by HW". However pr_info() say that there is 0xffffffff
> before write (I am talking about A1).
> 
> Also we know, that reset is triggered by writing 1 to specific bit. So,
> what will happen if 1 will be written where we did not intend to write it?
> 
> > Either send a full analysis of the problem you found, if you did one, or
> > at least send the full dump, explaining that you don't know what is
> > happening.
> > 
> 
> Full analysis is following:
> - using regmap_update_bits() instead of writel() is incorrect because
> this changes the behavior of the driver
> - regmap_update_bits() should not be used here because default value of
> regs isn't taken into account and (_apparently_, the doc is terse) these
> regs could be updated by hw itself.

Maybe use regmap_write_bits() instead.

  reply	other threads:[~2024-07-18 19:29 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-10 16:25 [PATCH 0/8] reset: amlogic: move audio reset drivers out of CCF Jerome Brunet
2024-07-10 16:25 ` Jerome Brunet
2024-07-10 16:25 ` [PATCH 1/8] reset: amlogic: convert driver to regmap Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-18  2:39   ` Jan Dakinevich
2024-07-18  2:39     ` Jan Dakinevich
2024-07-18  7:19     ` Jerome Brunet
2024-07-18  7:19       ` Jerome Brunet
2024-07-18 11:01       ` Jan Dakinevich
2024-07-18 11:01         ` Jan Dakinevich
2024-07-18 19:29         ` Stephen Boyd [this message]
2024-07-18 19:29           ` Stephen Boyd
2024-08-08 10:15           ` Jerome Brunet
2024-08-08 10:15             ` Jerome Brunet
2024-07-10 16:25 ` [PATCH 2/8] reset: amlogic: add driver parameters Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-10 22:34   ` Stephen Boyd
2024-07-10 22:34     ` Stephen Boyd
2024-07-10 16:25 ` [PATCH 3/8] reset: amlogic: split the device and platform probe Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-10 22:38   ` Stephen Boyd
2024-07-10 22:38     ` Stephen Boyd
2024-07-15 22:48   ` Jan Dakinevich
2024-07-15 22:48     ` Jan Dakinevich
2024-07-16 12:17     ` Jerome Brunet
2024-07-16 12:17       ` Jerome Brunet
2024-07-10 16:25 ` [PATCH 4/8] reset: amlogic: use reset number instead of register count Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-10 16:25 ` [PATCH 5/8] reset: amlogic: add reset status support Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-10 22:40   ` Stephen Boyd
2024-07-10 22:40     ` Stephen Boyd
2024-07-11  8:41     ` Jerome Brunet
2024-07-11  8:41       ` Jerome Brunet
2024-07-10 16:25 ` [PATCH 6/8] reset: amlogic: add toggle reset support Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-10 16:25 ` [PATCH 7/8] reset: amlogic: add auxiliary reset driver support Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-10 22:49   ` Stephen Boyd
2024-07-10 22:49     ` Stephen Boyd
2024-07-11  9:01     ` Jerome Brunet
2024-07-11  9:01       ` Jerome Brunet
2024-07-15 19:30       ` Stephen Boyd
2024-07-15 19:30         ` Stephen Boyd
2024-07-18  7:05         ` Jerome Brunet
2024-07-18  7:05           ` Jerome Brunet
2024-07-11 13:02   ` kernel test robot
2024-07-11 13:02     ` kernel test robot
2024-07-11 19:03   ` kernel test robot
2024-07-11 19:03     ` kernel test robot
2024-07-10 16:25 ` [PATCH 8/8] clk: amlogic: axg-audio: use the auxiliary reset driver Jerome Brunet
2024-07-10 16:25   ` Jerome Brunet
2024-07-15 22:45 ` [PATCH 0/8] reset: amlogic: move audio reset drivers out of CCF Jan Dakinevich
2024-07-15 22:45   ` Jan Dakinevich

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=11e8dd92e07674133d8a291cc016c314.sboyd@kernel.org \
    --to=sboyd@kernel.org \
    --cc=jan.dakinevich@salutedevices.com \
    --cc=jbrunet@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=p.zabel@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.