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.
next prev parent 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.