Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230403193250.108693-1-sebastian.reichel@collabora.com>
 <20230403193250.108693-2-sebastian.reichel@collabora.com> <CA+VMnFz4USPcXmQMyUB9n5EVmvQrJARDvnpO7iBrXZ8q2xcyAA@mail.gmail.com>
 <20230713193812.px7q4sdfdppuywsg@mercury.elektranox.org>
In-Reply-To: <20230713193812.px7q4sdfdppuywsg@mercury.elektranox.org>
From:   Jagan Teki <jagan@edgeble.ai>
Date:   Fri, 14 Jul 2023 11:03:12 +0530
Message-ID: <CA+VMnFwjFx5+h9=KGR-1o0ejSaxbE=-5wH3iqEEkn6k4q_he7Q@mail.gmail.com>
Subject: Re: [PATCHv2 1/2] clk: rockchip: rk3588: make gate linked clocks
 ignore unused
To:     Sebastian Reichel <sebastian.reichel@collabora.com>
Cc:     Heiko Stuebner <heiko@sntech.de>,
        linux-rockchip@lists.infradead.org,
        Peter Geis <pgwipeout@gmail.com>,
        Elaine Zhang <zhangqing@rock-chips.com>,
        Finley Xiao <finley.xiao@rock-chips.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Boris Brezillon <boris.brezillon@collabora.com>,
        linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, kernel@collabora.com,
        Vincent Legoll <vincent.legoll@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Sebastian,

On Fri, 14 Jul 2023 at 01:08, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hello Jagan,
>
> On Thu, Jul 13, 2023 at 08:25:03PM +0530, Jagan Teki wrote:
> > On Tue, 4 Apr 2023 at 01:03, Sebastian Reichel
> > <sebastian.reichel@collabora.com> wrote:
> > [...]
> > > + * Recent Rockchip SoCs have a new hardware block called Native Interface
> > > + * Unit (NIU), which gates clocks to devices behind them. These effectively
> > > + * need two parent clocks.
> > > + *
> > > + * Downstream enables the linked clock via runtime PM whenever the gate is
> > > + * enabled. This implementation uses separate clock nodes for each of the
> > > + * linked gate clocks, which leaks parts of the clock tree into DT.
> > > + *
> > > + * The GATE_LINK macro instead takes the second parent via 'linkname', but
> > > + * ignores the information. Once the clock framework is ready to handle it, the
> > > + * information should be passed on here. But since these clocks are required to
> > > + * access multiple relevant IP blocks, such as PCIe or USB, we mark all linked
> > > + * clocks critical until a better solution is available. This will waste some
> > > + * power, but avoids leaking implementation details into DT or hanging the
> > > + * system.
> > >   */
> >
> > Does it mean the clk-link topology in the downstream kernel can be
> > reused the same as normal clock notation?
>
> Yes.
>
> > For example, I'm trying to add HCLK_VO1 directly to VO1 syscon instead
> > of routing to pclk_vo1_grf(done downstream)
> >       vo1_grf: syscon@fd5a8000 {
> >                compatible = "rockchip,rk3588-vo-grf", "syscon";
> >                reg = <0x0 0xfd5a8000 0x0 0x100>;
> >              clocks = <&cru HCLK_VO1>;
>
> You need PCLK_VO1 (which is currently not exposed; I somehow missed
> it).
>
> >       };
> >
> > This seems breaking syscon for vo1_grf and observed a bus error
> > while accessing regmap.
>
> I investigated the issue you are seeing some weeks ago when my
> co-workers started to work on HDMI RX and TX. You are probably
> just missing this patch:
>
> https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/ecc6415344957fa88356cec10f8b75a9da603a7b

In fact, I tried this solution as well, by connecting the PCLK_VO1GRF.

        vo1_grf: syscon@fd5a8000 {
                compatible = "rockchip,rk3588-vo-grf", "syscon";
                reg = <0x0 0xfd5a8000 0x0 0x100>;
                clocks = <&cru PCLK_VO1GRF>;
        };

But the result seems the same, accessing vo1_grf triggers an abort [1]

[1] https://gist.github.com/openedev/e241da8180341ffbf4dc6a26de7efa31

Thanks,
Jagan.