LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Chen-Yu Tsai <wenst@chromium.org>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Chun-Jie Chen <chun-jie.chen@mediatek.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Nicolas Boichat <drinkcat@chromium.org>,
	Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	srv_heupstream@mediatek.com,
	Project_Global_Chrome_Upstream_Group@mediatek.com
Subject: Re: [PATCH 03/22] clk: mediatek: Fix corner case of tuner_en_reg
Date: Thu, 1 Jul 2021 12:02:17 +0800	[thread overview]
Message-ID: <CAGXv+5FSq-cCRR-wB_kp2s+59273r0nrhtkH9006ezN-sUtNzQ@mail.gmail.com> (raw)
In-Reply-To: <be824462-4c2f-3bde-0a3d-c5470a5b0fbb@gmail.com>

"On Wed, Jun 30, 2021 at 7:43 PM Matthias Brugger
<matthias.bgg@gmail.com> wrote:
> On 30/06/2021 13:09, Chen-Yu Tsai wrote:
> > On Wed, Jun 30, 2021 at 6:53 PM Matthias Brugger <matthias.bgg@gmail.com> wrote:
> >> On 30/06/2021 09:31, Chen-Yu Tsai wrote:
> >>> On Thu, Jun 17, 2021 at 7:01 AM Chun-Jie Chen
> >>> <chun-jie.chen@mediatek.com> wrote:
> >>>>
> >>>> On MT8195, tuner_en_reg is moved to register offest 0x0.
> >>>> If we only judge by tuner_en_reg, it may lead to wrong address.
> >>>> Add tuner_en_bit to the check condition. And it has been confirmed,
> >>>> on all the MediaTek SoCs, bit0 of offset 0x0 is always occupied by
> >>>> clock square control.
> >>>>
> >>>> Signed-off-by: Chun-Jie Chen <chun-jie.chen@mediatek.com>
> >>>
> >>> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>
> >>> Though you might want to consider converting these types of checks into feature
> >>> flags.
> >>>
> >>
> >> Yes I think adding a feature flag is the way to go. Luckily there are only a few
> >> SoCs that will need updates at the same time.
> >
> > I also see that the different clock modules are tied together using only clock
> > names written in the drivers, instead of clock references in the device tree.
> >
>
> Not sure I understand what you mean. Do you refer to something like [1]? That's
> because the clock is probed by the DRM driver, as they share the same compatible
> and IP block.

In the example driver you mentioned, most of the registered clocks have the same
parent clock, "mm_sel". This clock is from another hardware block,
"topckgen" [1].

The two are linked together by looking up the clock name. The link should be
explicitly described in the device tree, instead of implicitly by some name
found in two drivers. The consuming driver can fetch the clock name via
of_clk_get_parent_name(), or be migrated to use `struct clk_parent_data`,
which allows specifying local (to the DT node) clock names or clk indices
as parent clk references.

What's more confusing is that the mmsys node actually has "assigned-clocks"
properties [2] referencing the "mm_sel" clock, but not "clock" properties
referencing the same clock. On the surface this looks like the hardware
is trying to configure clocks that it doesn't use.

Also, Maxime Ripard made the argument before that "assigned-clock-rates"
doesn't give any real guarantees that the clock rate won't change. A
better method is to request and "lock" the clock rate in the consuming
driver.

So overall I think there are many improvements that can be made to the
Mediatek clk drivers. They aren't real blockers to new drivers though,
and I think each would take some effort and coordination across all
the SoCs.


Regards
ChenYu

[1] https://elixir.bootlin.com/linux/latest/source/drivers/clk/mediatek/clk-mt8173.c#L545
[2] https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L996


> Regards,
> Matthias
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.13#n139
>
> > Unfortunately reworking this would likely require a lot more work. I previously
> > did a bit of internal reworking for the sunxi drivers. While not the same, I
> > think the plumbing required is comparable.
> >
> > ChenYu
> >

  reply	other threads:[~2021-07-01  4:02 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16 22:47 [PATCH 00/22] Mediatek MT8195 clock support Chun-Jie Chen
2021-06-16 22:47 ` [PATCH 01/22] dt-bindings: ARM: Mediatek: Add new document bindings of MT8195 clock Chun-Jie Chen
2021-06-24 21:21   ` Rob Herring
2021-07-12  9:32   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 02/22] clk: mediatek: Add dt-bindings of MT8195 clocks Chun-Jie Chen
2021-06-24 21:22   ` Rob Herring
2021-07-12  9:37   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 03/22] clk: mediatek: Fix corner case of tuner_en_reg Chun-Jie Chen
2021-06-30  7:31   ` Chen-Yu Tsai
2021-06-30 10:53     ` Matthias Brugger
2021-06-30 11:09       ` Chen-Yu Tsai
2021-06-30 11:43         ` Matthias Brugger
2021-07-01  4:02           ` Chen-Yu Tsai [this message]
2021-06-16 22:47 ` [PATCH 04/22] clk: mediatek: Add MT8195 basic clocks support Chun-Jie Chen
2021-07-02 11:44   ` Chen-Yu Tsai
     [not found]     ` <1626913060.1546.4.camel@mtksdaap41>
2021-07-22  7:44       ` Chen-Yu Tsai
2021-08-11  4:31         ` Chun-Jie Chen
2021-06-16 22:47 ` [PATCH 05/22] clk: mediatek: Add MT8195 audio clock support Chun-Jie Chen
2021-07-05  9:03   ` Chen-Yu Tsai
2021-07-12  1:26     ` Chun-Jie Chen
2021-07-12  2:09       ` Chen-Yu Tsai
2021-07-12  4:35         ` Chun-Jie Chen
2021-07-12  7:06           ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 06/22] clk: mediatek: Add MT8195 audio src " Chun-Jie Chen
2021-07-05 10:07   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 07/22] clk: mediatek: Add MT8195 camsys " Chun-Jie Chen
2021-07-06  8:53   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 08/22] clk: mediatek: Add MT8195 ccusys " Chun-Jie Chen
2021-07-06  9:00   ` Chen-Yu Tsai
2021-08-17  0:56     ` Chun-Jie Chen
2021-06-16 22:47 ` [PATCH 09/22] clk: mediatek: Add MT8195 imgsys " Chun-Jie Chen
2021-07-06  9:07   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 10/22] clk: mediatek: Add MT8195 ipesys " Chun-Jie Chen
2021-07-06  9:11   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 11/22] clk: mediatek: Add MT8195 mfgcfg " Chun-Jie Chen
2021-07-09  6:29   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 12/22] clk: mediatek: Add MT8195 scp adsp " Chun-Jie Chen
2021-07-09  6:39   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 13/22] clk: mediatek: Add MT8195 nnasys " Chun-Jie Chen
2021-07-09  8:24   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 14/22] clk: mediatek: Add MT8195 vdecsys " Chun-Jie Chen
2021-07-09  8:40   ` Chen-Yu Tsai
2021-07-12  1:34     ` Chun-Jie Chen
2021-06-16 22:47 ` [PATCH 15/22] clk: mediatek: Add MT8195 vdosys0 " Chun-Jie Chen
2021-07-09  8:51   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 16/22] clk: mediatek: Add MT8195 vdosys1 " Chun-Jie Chen
2021-07-09  9:30   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 17/22] clk: mediatek: Add MT8195 vencsys " Chun-Jie Chen
2021-07-09 10:26   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 18/22] clk: mediatek: Add MT8195 vppsys0 " Chun-Jie Chen
2021-07-09 10:38   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 19/22] clk: mediatek: Add MT8195 vppsys1 " Chun-Jie Chen
2021-07-09 10:45   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 20/22] clk: mediatek: Add MT8195 wpesys " Chun-Jie Chen
2021-08-25 11:26   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 21/22] clk: mediatek: Add MT8195 imp i2c wrapper " Chun-Jie Chen
2021-07-12  8:34   ` Chen-Yu Tsai
2021-06-16 22:47 ` [PATCH 22/22] clk: mediatek: Add MT8195 apusys " Chun-Jie Chen
2021-07-12  8:51   ` Chen-Yu Tsai

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=CAGXv+5FSq-cCRR-wB_kp2s+59273r0nrhtkH9006ezN-sUtNzQ@mail.gmail.com \
    --to=wenst@chromium.org \
    --cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
    --cc=chun-jie.chen@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=drinkcat@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=srv_heupstream@mediatek.com \
    --subject='Re: [PATCH 03/22] clk: mediatek: Fix corner case of tuner_en_reg' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).