Date:   Wed, 09 Aug 2023 10:56:50 +0200
From:   Florian Eckert <fe@....tdt.de>
To:     Yi xin Zhu <yzhu@...linear.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        mturquette@...libre.com, sboyd@...nel.org,
        Rahul Tanwar <rtanwar@...linear.com>, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, Eckert.Florian@...glemail.com
Subject: Re: [PATCH 2/2] dt-bindings: clock: intel,cgu-lgm: add
 mxl,control-gate option

>> I didn't want to always change the source of the driver when it has to
>> take
>> care of the GATE, so I wanted to map this via the dts.
>> 
>> I have a board support package from Maxlinear for the Lightning 
>> Mountain
>> Soc
>> with other drivers that are not upstream now. Some of them use the
>> clock framework some of them does not.
>> 
>> Due to missing documents it is not possible to send these drivers
>> upstream.
>> Strictly speaking, this is about the gptc and the watchdog.
>> 
>> Since it is a buildin_platform driver, it can also not work via
>> module parameters.
> 
> Could you please give more details on your target?

We are currently putting our own board with the new Maxlinear URX85x
into operation. From Maxlinear we have received a board support
package BSP named UGW 9.1.45.

Since we not only have Maxlinear devices, but also other SoC from
other manufacturers, we cannot use the BSP. We therefore need to use
OpenWrt vanilla (master, openwrt-23.05) to support all our devices
with the same software stack.

We have therefore picked all the relevant software components
from UGW and ported them to the next openwrt-23.05 stable release.

Due to the last rebasing of the kernel by openwrt to 5.15.123 [1],
this patch [2] has been included. After that change, the gptc and
watchdog drivers can't find the cgu device tree handlers and
stopped working!

> In what kind of condition, you want to change the flag?

Since we don't have access to the latest internal MxL code base,
we don't know what has been changed since then. Therefore we ended
up with reverting your last change [2] to get the gptc and watchdog
driver working again.

> In LGM SoC,  some gate clocks can be covered by EPU (power management
> module).

We have already seen that in your BSP. But this driver is not integrated
upstream and is unfortunately only maintained in the BSP :-(. So no one
knows about that.

> that is the reason clock driver introduced the HW/SW flag definition.

Since we don't have a hardware description, but only your drivers, it
is difficult for us to find out how everything fits together. In 
addition,
not all drivers are upstream, which makes it even more confusing for us.

> However gptc and watchdog are not covered by EPU.  it can only be
> controlled via clock
> driver.   So I'm not quite sure the target to change the flag for these
> two clocks.

So only Maxlinear knows the internals, then this is up to you to make
the right choice.

The major problem is that not all relevant drivers are upstreamed.
Mixing upstreamed and proprietary drivers definitely leads to 
regressions.

By the way, up to now the SoC is not selectable. There are patches 
needed
from your BSP for the kernel to make the SoC URX85x selectable with
'make menuconfig'

Can you *please* integrate them upstream, so that the patches do not 
always have
to be extracted and rebased from the BSP!

We can discuss this in a separate thread.
Thanks for your feedback

Best regards

Florian

[1] 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=7efec0acca80b231ab8e69729a4bdaf11ef84541
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/clk/x86/clk-cgu.c?h=linux-5.15.y&id=a0583edea4fdb7b5b87a077263dddab476e9f138

Powered by blists - more mailing lists