Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock controller driver
From: Jerome Brunet @ 2024-04-02  9:27 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: Martin Blumenstingl, neil.armstrong, jbrunet, mturquette, sboyd,
	robh+dt, krzysztof.kozlowski+dt, khilman, kernel, rockosov,
	linux-amlogic, linux-clk, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20240401171237.qoewp2pgcdrqvc3e@CAB-WSD-L081021>


On Mon 01 Apr 2024 at 20:12, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:

> Hello Martin,
>
> Thank you for quick response. Please find my thoughts below.
>
> On Sun, Mar 31, 2024 at 11:40:13PM +0200, Martin Blumenstingl wrote:
>> Hi Dmitry,
>> 
>> On Fri, Mar 29, 2024 at 9:59 PM Dmitry Rokosov
>> <ddrokosov@salutedevices.com> wrote:
>> [...]
>> > +static struct clk_regmap cpu_fclk = {
>> > +       .data = &(struct clk_regmap_mux_data) {
>> > +               .offset = CPUCTRL_CLK_CTRL0,
>> > +               .mask = 0x1,
>> > +               .shift = 10,
>> > +       },
>> > +       .hw.init = &(struct clk_init_data) {
>> > +               .name = "cpu_fclk",
>> > +               .ops = &clk_regmap_mux_ops,
>> > +               .parent_hws = (const struct clk_hw *[]) {
>> > +                       &cpu_fsel0.hw,
>> > +                       &cpu_fsel1.hw,
>> Have you considered the CLK_SET_RATE_GATE flag for &cpu_fsel0.hw and
>> &cpu_fsel1.hw and then dropping the clock notifier below?
>> We use that approach with the Mali GPU clock on other SoCs, see for
>> example commit 8daeaea99caa ("clk: meson: meson8b: make the CCF use
>> the glitch-free mali mux").
>> It may differ from what Amlogic does in their BSP,
>
> Amlogic in their BSP takes a different approach, which is slightly
> different from mine. They cleverly change the parent of cpu_clk directly
> by forking the cpufreq driver to a custom version. I must admit, it's
> quite an "interesting and amazing" idea :) but it's not architecturally
> correct totally.

I disagree. Martin's suggestion is correct for the fsel part which is
symetric.

>
>> but I don't think
>> that there's any harm (if it works in general) because CCF (common
>> clock framework) will set all clocks in the "inactive" tree and then
>> as a last step just change the mux (&cpu_fclk.hw). So at no point in
>> time will we get any other rate than a) the original CPU clock rate
>> before the rate change b) the new desired CPU clock rate. This is
>> because we have two symmetric clock trees.
>
> Now, let's dive into the specifics of the issue we're facing. I've
> examined the CLK_SET_RATE_GATE flag, which, to my understanding, blocks
> rate changes for the entire clock chain. However, in this particular
> situation, it doesn't provide the solution we need.
>
> Here's the problem we're dealing with:
>
> 1) The CPU clock can have the following frequency points:
>
>   available frequency steps:  128 MHz, 256 MHz, 512 MHz, 768 MHz, 1.01 GHz, 1.20 GHz
>
> When we run the cpupower, we get the following information:
> # cpupower -c 0,1 frequency-info
> analyzing CPU 0:
>   driver: cpufreq-dt
>   CPUs which run at the same hardware frequency: 0 1
>   CPUs which need to have their frequency coordinated by software: 0 1
>   maximum transition latency: 50.0 us
>   hardware limits: 128 MHz - 1.20 GHz
>   available frequency steps:  128 MHz, 256 MHz, 512 MHz, 768 MHz, 1.01 GHz, 1.20 GHz
>   available cpufreq governors: conservative ondemand userspace performance schedutil
>   current policy: frequency should be within 128 MHz and 128 MHz.
>                   The governor "schedutil" may decide which speed to use
>                   within this range.
>   current CPU frequency: 128 MHz (asserted by call to hardware)
> analyzing CPU 1:
>   driver: cpufreq-dt
>   CPUs which run at the same hardware frequency: 0 1
>   CPUs which need to have their frequency coordinated by software: 0 1
>   maximum transition latency: 50.0 us
>   hardware limits: 128 MHz - 1.20 GHz
>   available frequency steps:  128 MHz, 256 MHz, 512 MHz, 768 MHz, 1.01 GHz, 1.20 GHz
>   available cpufreq governors: conservative ondemand userspace performance schedutil
>   current policy: frequency should be within 128 MHz and 128 MHz.
>                   The governor "schedutil" may decide which speed to use
>                   within this range.
>   current CPU frequency: 128 MHz (asserted by call to hardware)
>
> 2) For the frequency points 128 MHz, 256 MHz, and 512 MHz, the CPU fixed
> clock should be used.

Apparently, you are relying on the SYS PLL lowest possible rate to
enfore this contraint, which I suppose is 24 * 32 = 768MHz. It would be
nice to clearly say so.

> Fortunately, we don't encounter any freeze
> problems when we attempt to change its rate at these frequencies.

That does not sound very solid ...

>
> 3) However, for the frequency points 768 MHz, 1.01 GHz, and 1.20 GHz,
> the sys_pll is used as the clock source because it's a faster option.
> Now, let's imagine that we want to change the CPU clock from 768 MHz to
> 1.01 GHz. Unfortunately, it's not possible due to the broken sys_pll,
> and any execution attempts will result in a hang.

... Because PLL needs to relock, it is going to be off for a while. That
is not "broken", unless there is something else ?

>
> 4) As you can observe, in this case, we actually don't need to lock the
> rate for the sys_pll chain.

In which case ? I'm lost.

> We want to change the rate instead.

... How are you going to do that without relocking the PLL ?

> Hence,
> I'm not aware of any other method to achieve this except by switching
> the cpu_clk parent to a stable clock using clock notifier block.
> Interestingly, I've noticed a similar approach in the CPU clock drivers
> of Rockchip, Qualcomm, and Mediatek.

There is an example of syspll notifier in the g12 clock controller.
You should have a look at it

-- 
Jerome

^ permalink raw reply

* Re: [PATCH v2 1/8] dt-bindings: clock: add Loongson-2K expand clock index
From: Binbin Zhou @ 2024-04-02  9:34 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu,
	loongson-kernel, linux-clk, devicetree, Xuerui Wang, loongarch,
	Conor Dooley
In-Reply-To: <CAAhV-H5L=ff8k4a5PA7vaD5W8QRu3zWQa=-99bq4MsjUz3UQJQ@mail.gmail.com>

On Tue, Apr 2, 2024 at 2:58 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Binbin,
>
> On Mon, Apr 1, 2024 at 4:24 PM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> >
> > In the new Loongson-2K family of SoCs, more clock indexes are needed,
> > such as clock gates.
> > The patch adds these clock indexes
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  include/dt-bindings/clock/loongson,ls2k-clk.h | 56 ++++++++++++-------
> >  1 file changed, 37 insertions(+), 19 deletions(-)
> >
> > diff --git a/include/dt-bindings/clock/loongson,ls2k-clk.h b/include/dt-bindings/clock/loongson,ls2k-clk.h
> > index 3bc4dfc193c2..4e6811eca8c6 100644
> > --- a/include/dt-bindings/clock/loongson,ls2k-clk.h
> > +++ b/include/dt-bindings/clock/loongson,ls2k-clk.h
> > @@ -7,24 +7,42 @@
> >  #ifndef __DT_BINDINGS_CLOCK_LOONGSON2_H
> >  #define __DT_BINDINGS_CLOCK_LOONGSON2_H
> >
> > -#define LOONGSON2_REF_100M                             0
> > -#define LOONGSON2_NODE_PLL                             1
> > -#define LOONGSON2_DDR_PLL                              2
> > -#define LOONGSON2_DC_PLL                               3
> > -#define LOONGSON2_PIX0_PLL                             4
> > -#define LOONGSON2_PIX1_PLL                             5
> > -#define LOONGSON2_NODE_CLK                             6
> > -#define LOONGSON2_HDA_CLK                              7
> > -#define LOONGSON2_GPU_CLK                              8
> > -#define LOONGSON2_DDR_CLK                              9
> > -#define LOONGSON2_GMAC_CLK                             10
> > -#define LOONGSON2_DC_CLK                               11
> > -#define LOONGSON2_APB_CLK                              12
> > -#define LOONGSON2_USB_CLK                              13
> > -#define LOONGSON2_SATA_CLK                             14
> > -#define LOONGSON2_PIX0_CLK                             15
> > -#define LOONGSON2_PIX1_CLK                             16
> > -#define LOONGSON2_BOOT_CLK                             17
> > -#define LOONGSON2_CLK_END                              18
> > +#define LOONGSON2_REF_100M     0
> > +#define LOONGSON2_NODE_PLL     1
> > +#define LOONGSON2_DDR_PLL      2
> > +#define LOONGSON2_DC_PLL       3
> > +#define LOONGSON2_PIX0_PLL     4
> > +#define LOONGSON2_PIX1_PLL     5
> > +#define LOONGSON2_NODE_CLK     6
> > +#define LOONGSON2_HDA_CLK      7
> > +#define LOONGSON2_GPU_CLK      8
> > +#define LOONGSON2_DDR_CLK      9
> > +#define LOONGSON2_GMAC_CLK     10
> > +#define LOONGSON2_DC_CLK       11
> > +#define LOONGSON2_APB_CLK      12
> > +#define LOONGSON2_USB_CLK      13
> > +#define LOONGSON2_SATA_CLK     14
> > +#define LOONGSON2_PIX0_CLK     15
> > +#define LOONGSON2_PIX1_CLK     16
> > +#define LOONGSON2_BOOT_CLK     17
> > +
> > +/* Loongson-2K2000 */
> This line should be removed, because the below definition is not
> specific to Loongson-2K2000.

Yes, it was my mistake, I forgot to drop it.
I will fix it in the next version.

Thanks.
Binbin
>
> Huacai
>
> > +#define LOONGSON2_OUT0_GATE    18
> > +#define LOONGSON2_GMAC_GATE    19
> > +#define LOONGSON2_RIO_GATE     20
> > +#define LOONGSON2_DC_GATE      21
> > +#define LOONGSON2_GPU_GATE     22
> > +#define LOONGSON2_DDR_GATE     23
> > +#define LOONGSON2_HDA_GATE     24
> > +#define LOONGSON2_NODE_GATE    25
> > +#define LOONGSON2_EMMC_GATE    26
> > +#define LOONGSON2_PIX0_GATE    27
> > +#define LOONGSON2_PIX1_GATE    28
> > +#define LOONGSON2_OUT0_CLK     29
> > +#define LOONGSON2_RIO_CLK      30
> > +#define LOONGSON2_EMMC_CLK     31
> > +#define LOONGSON2_DES_CLK      32
> > +#define LOONGSON2_I2S_CLK      33
> > +#define LOONGSON2_MISC_CLK     34
> >
> >  #endif
> > --
> > 2.43.0
> >

^ permalink raw reply

* Re: [PATCH 3/3] arm64: dts: rockchip: Remove UART2 from RGB30
From: Ahmad Fatoum @ 2024-04-02  9:27 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Chris Morgan, linux-rockchip, linux-clk, devicetree, sboyd,
	mturquette, heiko, conor+dt, krzysztof.kozlowski+dt, robh+dt
In-Reply-To: <DM4PR05MB9229E56DE3F587BD222E1402A5392@DM4PR05MB9229.namprd05.prod.outlook.com>

Hello Chris,

On 30.03.24 16:34, Chris Morgan wrote:
> On Sat, Mar 30, 2024 at 02:13:05PM +0100, Ahmad Fatoum wrote:
>> Hello Chris,
>>
>> On 18.10.23 17:33, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> The Powkiddy RGB30 has no onboard UART header, so remove the reference
>>> to it in the device tree. This was left on by mistake in the initial
>>> commit.
>>
>> Do you know if the UART is perhaps available over testpoints?
> 
> There is not one as best I can tell on either the RGB30 or RK2023. The
> Powkiddy X55 does have UART, however. I was able to exploit the fact
> that the RGB30 is extremely similar to all of the Anbernic devices
> (such as the RG353 series) for the purposes of low-level development.
> Once I got a network connection I performed the rest of development
> over SSH, but prior to that I just developed on a different device.

Thanks for the info.

AFAICS, it should be possible to get a console by changing the pinmux
setting on the Game TF-Card:

  SDMMC1_D0/UART6_RX_M0/GPIO2_A3_u
  SDMMC1_D1/UART6_TX_M0/GPIO2_A4_u
  SDMMC1_D2/UART7_RX_M0/GPIO2_A5_u
  SDMMC1_D3/UART7_TX_M0/GPIO2_A6_u
  SDMMC1_CMD/UART9_RX_M0/GPIO2_A7_u
  SDMMC1_CLK/UART9_TX_M0/GPIO2_B0_d

I will give that a try.

Cheers,
Ahmad

> 
> Thank you,
> Chris.
> 
>>
>> If yes, having a DT-overlay upstream enabling it along with documentation could be useful.
>> If not, how do you do low-level debugging on the RBG30 in absence of the serial console?
>>
>> Thanks,
>> Ahmad 
>>
>>>
>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts
>>> index 3ebc21608213..1ead3c5c24b3 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rgb30.dts
>>> @@ -64,6 +64,10 @@ simple-audio-card,cpu {
>>>  
>>>  /delete-node/ &adc_keys;
>>>  
>>> +&chosen {
>>> +	/delete-property/ stdout-path;
>>> +};
>>> +
>>>  &cru {
>>>  	assigned-clocks = <&pmucru CLK_RTC_32K>, <&cru PLL_GPLL>,
>>>  			  <&pmucru PLL_PPLL>, <&cru PLL_VPLL>;
>>> @@ -149,4 +153,9 @@ rk817_charger: charger {
>>>  	};
>>>  };
>>>  
>>> +/* There is no UART header visible on the board for this device. */
>>> +&uart2 {
>>> +	status = "disabled";
>>> +};
>>> +
>>>  /delete-node/ &vibrator;
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


^ permalink raw reply

* [PATCH v3] clk: starfive: pll: Fix lower rate of CPUfreq by setting PLL0 rate to 1.5GHz
From: Xingyu Wu @ 2024-04-02  9:09 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Conor Dooley,
	Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Hal Feng, Xingyu Wu,
	linux-kernel, linux-clk, linux-riscv, devicetree

CPUfreq supports 4 cpu frequency loads on 375/500/750/1500MHz.
But now PLL0 rate is 1GHz and the cpu frequency loads become
333/500/500/1000MHz in fact.

So PLL0 rate should be default set to 1.5GHz. But setting the
PLL0 rate need certain steps:

1. Change the parent of cpu_root clock to OSC clock.
2. Change the divider of cpu_core if PLL0 rate is higher than
   1.25GHz before CPUfreq boot.
3. Change the parent of cpu_root clock back to PLL0 clock.

Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
Fixes: e2c510d6d630 ("riscv: dts: starfive: Add cpu scaling for JH7110 SoC")
Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---

Hi Stephen and Emil,

This patch fixes the issue about lower rate of CPUfreq[1] by setting PLL0
rate to 1.5GHz.

In order not to affect the cpu operation, setting the PLL0 rate need
certain steps. The cpu_root's parent clock should be changed first. And
the divider of the cpu_core clock should be set to 2 so they won't crash
when setting 1.5GHz without voltage regulation. Due to PLL driver boot
earlier than SYSCRG driver, cpu_core and cpu_root clocks are using by
ioremap(). 

[1]: https://github.com/starfive-tech/VisionFive2/issues/55

Previous patch link:
v2: https://lore.kernel.org/all/20230821152915.208366-1-xingyu.wu@starfivetech.com/
v1: https://lore.kernel.org/all/20230811033631.160912-1-xingyu.wu@starfivetech.com/

Thanks,
Xingyu Wu
---
 .../jh7110-starfive-visionfive-2.dtsi         |   5 +
 .../clk/starfive/clk-starfive-jh7110-pll.c    | 102 ++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index 45b58b6f3df8..0c57d833fb29 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -336,6 +336,11 @@ &pwmdac {
 	status = "okay";
 };
 
+&pllclk {
+	assigned-clocks = <&pllclk JH7110_PLLCLK_PLL0_OUT>;
+	assigned-clock-rates = <1500000000>;
+};
+
 &qspi {
 	#address-cells = <1>;
 	#size-cells = <0>;
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-pll.c b/drivers/clk/starfive/clk-starfive-jh7110-pll.c
index 3598390e8fd0..7a53ded8d526 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-pll.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-pll.c
@@ -24,11 +24,14 @@
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/starfive,jh7110-crg.h>
 
+#include "clk-starfive-jh7110.h"
+
 /* this driver expects a 24MHz input frequency from the oscillator */
 #define JH7110_PLL_OSC_RATE		24000000UL
 
@@ -72,6 +75,9 @@
 #define JH7110_PLL_PREDIV_SHIFT		0
 #define JH7110_PLL_PREDIV_MASK		GENMASK(5, 0)
 
+#define JH7110_CPU_ROOT_MUX_OSC		0
+#define JH7110_CPU_ROOT_MUX_PLL0	1
+
 enum jh7110_pll_mode {
 	JH7110_PLL_MODE_FRACTION,
 	JH7110_PLL_MODE_INTEGER,
@@ -140,6 +146,8 @@ struct jh7110_pll_data {
 struct jh7110_pll_priv {
 	struct device *dev;
 	struct regmap *regmap;
+	void __iomem *syscrg_base;
+	bool is_first_set;
 	struct jh7110_pll_data pll[JH7110_PLLCLK_END];
 };
 
@@ -275,6 +283,25 @@ static struct jh7110_pll_priv *jh7110_pll_priv_from(struct jh7110_pll_data *pll)
 	return container_of(pll, struct jh7110_pll_priv, pll[pll->idx]);
 }
 
+static void jh7110_pll_syscrg_update_div(void __iomem *base,
+					 unsigned int id,
+					 unsigned int div)
+{
+	unsigned int reg = readl(base + id * 4);
+
+	writel((reg & ~JH71X0_CLK_DIV_MASK) | div, (base + id * 4));
+}
+
+static void jh7110_pll_syscrg_update_mux(void __iomem *base,
+					 unsigned int id,
+					 unsigned int mux)
+{
+	unsigned int reg = readl(base + id * 4);
+
+	writel((reg & ~JH71X0_CLK_MUX_MASK) | (mux << JH71X0_CLK_MUX_SHIFT),
+	       (base + id * 4));
+}
+
 static void jh7110_pll_regvals_get(struct regmap *regmap,
 				   const struct jh7110_pll_info *info,
 				   struct jh7110_pll_regvals *ret)
@@ -352,6 +379,47 @@ static int jh7110_pll_determine_rate(struct clk_hw *hw, struct clk_rate_request
 	return 0;
 }
 
+static bool jh7110_pll0_is_assigned_clock(struct device_node *node)
+{
+	struct of_phandle_args clkspec;
+	int ret;
+
+	ret = of_parse_phandle_with_args(node, "assigned-clocks",
+					 "#clock-cells", 0, &clkspec);
+	if (ret < 0 || clkspec.np != node)
+		return false;
+
+	if (clkspec.args[0] == JH7110_PLLCLK_PLL0_OUT)
+		return true;
+
+	return false;
+}
+
+/*
+ * In order to not affect the cpu when the PLL0 rate is changing,
+ * we need to switch the parent of cpu_root clock to osc clock first,
+ * and then switch back after setting the PLL0 rate.
+ *
+ * If cpu rate rather than 1.25GHz, PMIC need to be set higher voltage.
+ * But the PMIC is controlled by CPUfreq and I2C, which boot later than
+ * PLL driver when using assigned_clock to set PLL0 rate. So set the
+ * CPU_CORE divider to 2(default 1) first and make sure the cpu rate is
+ * lower than 1.25G when pll0 rate will be set more than 1.25G.
+ */
+static void jh7110_pll0_rate_preset(struct jh7110_pll_priv *priv,
+				    unsigned long rate)
+{
+	if (rate > 1250000000 && priv->is_first_set &&
+	    jh7110_pll0_is_assigned_clock(priv->dev->of_node))
+		jh7110_pll_syscrg_update_div(priv->syscrg_base,
+					     JH7110_SYSCLK_CPU_CORE, 2);
+
+	jh7110_pll_syscrg_update_mux(priv->syscrg_base,
+				     JH7110_SYSCLK_CPU_ROOT,
+				     JH7110_CPU_ROOT_MUX_OSC);
+	priv->is_first_set = false;
+}
+
 static int jh7110_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 			       unsigned long parent_rate)
 {
@@ -372,6 +440,9 @@ static int jh7110_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 	return -EINVAL;
 
 found:
+	if (pll->idx == JH7110_PLLCLK_PLL0_OUT)
+		jh7110_pll0_rate_preset(priv, rate);
+
 	if (val->mode == JH7110_PLL_MODE_FRACTION)
 		regmap_update_bits(priv->regmap, info->offsets.frac, JH7110_PLL_FRAC_MASK,
 				   val->frac << JH7110_PLL_FRAC_SHIFT);
@@ -387,6 +458,12 @@ static int jh7110_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 	regmap_update_bits(priv->regmap, info->offsets.frac, JH7110_PLL_POSTDIV1_MASK,
 			   (u32)val->postdiv1 << JH7110_PLL_POSTDIV1_SHIFT);
 
+	/* Set parent of cpu_root back to PLL0 */
+	if (pll->idx == JH7110_PLLCLK_PLL0_OUT)
+		jh7110_pll_syscrg_update_mux(priv->syscrg_base,
+					     JH7110_SYSCLK_CPU_ROOT,
+					     JH7110_CPU_ROOT_MUX_PLL0);
+
 	return 0;
 }
 
@@ -458,6 +535,8 @@ static int jh7110_pll_probe(struct platform_device *pdev)
 	struct jh7110_pll_priv *priv;
 	unsigned int idx;
 	int ret;
+	struct device_node *np;
+	struct resource res;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -489,6 +568,29 @@ static int jh7110_pll_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	priv->is_first_set = true;
+	np = of_find_compatible_node(NULL, NULL, "starfive,jh7110-syscrg");
+	if (!np) {
+		ret = PTR_ERR(np);
+		dev_err(priv->dev, "failed to get syscrg node\n");
+		goto np_put;
+	}
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret) {
+		dev_err(priv->dev, "failed to get syscrg resource\n");
+		goto np_put;
+	}
+
+	priv->syscrg_base = ioremap(res.start, resource_size(&res));
+	if (!priv->syscrg_base)
+		ret = -ENOMEM;
+
+np_put:
+	of_node_put(np);
+	if (ret)
+		return ret;
+
 	return devm_of_clk_add_hw_provider(&pdev->dev, jh7110_pll_get, priv);
 }
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 2/2] iio: imu: inv_icm42600: add support of ICM-42688-P
From: inv.git-commit @ 2024-04-02  9:00 UTC (permalink / raw)
  To: jic23, robh, krzysztof.kozlowski+dt, conor+dt
  Cc: lars, linux-iio, devicetree, Jean-Baptiste Maneyrol
In-Reply-To: <20240402090046.764572-1-inv.git-commit@tdk.com>

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_icm42600/inv_icm42600.h      | 2 ++
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 5 +++++
 drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c  | 3 +++
 drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c  | 3 +++
 4 files changed, 13 insertions(+)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
index 0e290c807b0f..0566340b2660 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
@@ -22,6 +22,7 @@ enum inv_icm42600_chip {
        INV_CHIP_ICM42602,
        INV_CHIP_ICM42605,
        INV_CHIP_ICM42622,
+       INV_CHIP_ICM42688,
        INV_CHIP_ICM42631,
        INV_CHIP_NB,
 };
@@ -304,6 +305,7 @@ struct inv_icm42600_state {
 #define INV_ICM42600_WHOAMI_ICM42602                   0x41
 #define INV_ICM42600_WHOAMI_ICM42605                   0x42
 #define INV_ICM42600_WHOAMI_ICM42622                   0x46
+#define INV_ICM42600_WHOAMI_ICM42688                   0x47
 #define INV_ICM42600_WHOAMI_ICM42631                   0x5C

 /* User bank 1 (MSB 0x10) */
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index a5e81906e37e..82e0a2e2ad70 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -87,6 +87,11 @@ static const struct inv_icm42600_hw inv_icm42600_hw[INV_CHIP_NB] = {
                .name = "icm42622",
                .conf = &inv_icm42600_default_conf,
        },
+       [INV_CHIP_ICM42688] = {
+               .whoami = INV_ICM42600_WHOAMI_ICM42688,
+               .name = "icm42688",
+               .conf = &inv_icm42600_default_conf,
+       },
        [INV_CHIP_ICM42631] = {
                .whoami = INV_ICM42600_WHOAMI_ICM42631,
                .name = "icm42631",
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
index 1af559403ba6..ebb28f84ba98 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
@@ -84,6 +84,9 @@ static const struct of_device_id inv_icm42600_of_matches[] = {
        }, {
                .compatible = "invensense,icm42622",
                .data = (void *)INV_CHIP_ICM42622,
+       }, {
+               .compatible = "invensense,icm42688",
+               .data = (void *)INV_CHIP_ICM42688,
        }, {
                .compatible = "invensense,icm42631",
                .data = (void *)INV_CHIP_ICM42631,
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
index 6be4ac794937..50217a10e0bb 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
@@ -80,6 +80,9 @@ static const struct of_device_id inv_icm42600_of_matches[] = {
        }, {
                .compatible = "invensense,icm42622",
                .data = (void *)INV_CHIP_ICM42622,
+       }, {
+               .compatible = "invensense,icm42688",
+               .data = (void *)INV_CHIP_ICM42688,
        }, {
                .compatible = "invensense,icm42631",
                .data = (void *)INV_CHIP_ICM42631,
--
2.34.1

TDK-Micronas GmbH
Company Headquarters / Sitz der Gesellschaft: Freiburg i. Br. - Municipal Court of / Amtsgericht: Freiburg i. Br. HRB 6108. VAT ID / USt-IdNr.: DE 812878184
Management / Geschäftsführung: Sam Maddalena

This e-mail and any files transmitted with it are confidential information of TDK-Micronas and intended solely for the use of the individual or entity to whom they are addressed. If you have received this e-mail in error please notify the sender by return e-mail and delete all copies of this e-mail message along with all attachments. If you are not the named addressee you should not disseminate, distribute or copy this e-mail.

Bitte vermeiden Sie den Ausdruck dieser E-Mail.
Please consider your environmental responsibility before printing this e-mail.

[X]

[https://www.micronas.tdk.com/sites/default/files/header/2024_04_TDK_Tradeshow_Banner_250x135px_96dpi_embeddedworld.png]<https://www.tdk.com/en/news_center/press/20240312_01.html>

^ permalink raw reply related

* Re: [PATCH v2 2/2] arm64: dts: qcom: qcs6490-rb3gen2: Enable various remoteprocs
From: Dmitry Baryshkov @ 2024-04-02  9:17 UTC (permalink / raw)
  To: Komal Bajaj
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel, quic_tsoni
In-Reply-To: <20240402090349.30172-3-quic_kbajaj@quicinc.com>

On Tue, 2 Apr 2024 at 12:04, Komal Bajaj <quic_kbajaj@quicinc.com> wrote:
>
> Enable the ADSP, CDSP and WPSS that are found on qcs6490-rb3gen2.

No MPSS even for GPS?

Anyway,

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 97824c769ba3..a25431ddf922 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -434,6 +434,21 @@ &qupv3_id_0 {
>         status = "okay";
>  };
>
> +&remoteproc_adsp {
> +       firmware-name = "qcom/qcm6490/adsp.mbn";
> +       status = "okay";
> +};
> +
> +&remoteproc_cdsp {
> +       firmware-name = "qcom/qcm6490/cdsp.mbn";
> +       status = "okay";
> +};
> +
> +&remoteproc_wpss {
> +       firmware-name = "qcom/qcm6490/wpss.mbn";
> +       status = "okay";
> +};
> +
>  &tlmm {
>         gpio-reserved-ranges = <32 2>, /* ADSP */
>                                <48 4>; /* NFC */
> --
> 2.42.0
>
>


-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH v2 1/2] arm64: dts: qcom: qcm6490-idp: Enable various remoteprocs
From: Dmitry Baryshkov @ 2024-04-02  9:16 UTC (permalink / raw)
  To: Komal Bajaj
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel, quic_tsoni
In-Reply-To: <20240402090349.30172-2-quic_kbajaj@quicinc.com>

On Tue, 2 Apr 2024 at 12:04, Komal Bajaj <quic_kbajaj@quicinc.com> wrote:
>
> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
>
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

^ permalink raw reply

* [PATCH 2/2] ARM: boot: dts: microchip: at91-sama7g54_curiosity: Replace regulator-suspend-voltage with the valid property
From: Andrei Simion @ 2024-04-02  9:12 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, nicolas.ferre,
	alexandre.belloni, claudiu.beznea, mihai.sain
  Cc: linux-arm-kernel, linux-kernel, devicetree, Andrei Simion
In-Reply-To: <20240402091228.110362-1-andrei.simion@microchip.com>

Replace regulator-suspend-voltage with regulator-suspend-microvolt.

Fixes: ebd6591f8ddb ("ARM: dts: microchip: sama7g54_curiosity: Add initial device tree of the board")
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
 arch/arm/boot/dts/microchip/at91-sama7g54_curiosity.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/microchip/at91-sama7g54_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sama7g54_curiosity.dts
index 4f609e9e510e..009d2c832421 100644
--- a/arch/arm/boot/dts/microchip/at91-sama7g54_curiosity.dts
+++ b/arch/arm/boot/dts/microchip/at91-sama7g54_curiosity.dts
@@ -242,7 +242,7 @@ vddcore: VDD_CORE {
 
 					regulator-state-standby {
 						regulator-on-in-suspend;
-						regulator-suspend-voltage = <1150000>;
+						regulator-suspend-microvolt = <1150000>;
 						regulator-mode = <4>;
 					};
 
@@ -263,7 +263,7 @@ vddcpu: VDD_OTHER {
 
 					regulator-state-standby {
 						regulator-on-in-suspend;
-						regulator-suspend-voltage = <1050000>;
+						regulator-suspend-microvolt = <1050000>;
 						regulator-mode = <4>;
 					};
 
@@ -280,7 +280,7 @@ vldo1: LDO1 {
 					regulator-always-on;
 
 					regulator-state-standby {
-						regulator-suspend-voltage = <1800000>;
+						regulator-suspend-microvolt = <1800000>;
 						regulator-on-in-suspend;
 					};
 
@@ -296,7 +296,7 @@ vldo2: LDO2 {
 					regulator-always-on;
 
 					regulator-state-standby {
-						regulator-suspend-voltage = <3300000>;
+						regulator-suspend-microvolt = <3300000>;
 						regulator-on-in-suspend;
 					};
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH 1/2] ARM: boot: dts: microchip: at91-sama7g5ek: Replace regulator-suspend-voltage with the valid property
From: Andrei Simion @ 2024-04-02  9:12 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, nicolas.ferre,
	alexandre.belloni, claudiu.beznea, mihai.sain
  Cc: linux-arm-kernel, linux-kernel, devicetree, Andrei Simion
In-Reply-To: <20240402091228.110362-1-andrei.simion@microchip.com>

Replace regulator-suspend-voltage with regulator-suspend-microvolt.

Fixes: 85b1304b9daa ("ARM: dts: at91: sama7g5ek: set regulator voltages for standby state")
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
 arch/arm/boot/dts/microchip/at91-sama7g5ek.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts b/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts
index 217e9b96c61e..20b2497657ae 100644
--- a/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts
+++ b/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts
@@ -293,7 +293,7 @@ vddcore: VDD_CORE {
 
 					regulator-state-standby {
 						regulator-on-in-suspend;
-						regulator-suspend-voltage = <1150000>;
+						regulator-suspend-microvolt = <1150000>;
 						regulator-mode = <4>;
 					};
 
@@ -314,7 +314,7 @@ vddcpu: VDD_OTHER {
 
 					regulator-state-standby {
 						regulator-on-in-suspend;
-						regulator-suspend-voltage = <1050000>;
+						regulator-suspend-microvolt = <1050000>;
 						regulator-mode = <4>;
 					};
 
@@ -331,7 +331,7 @@ vldo1: LDO1 {
 					regulator-always-on;
 
 					regulator-state-standby {
-						regulator-suspend-voltage = <1800000>;
+						regulator-suspend-microvolt = <1800000>;
 						regulator-on-in-suspend;
 					};
 
@@ -346,7 +346,7 @@ vldo2: LDO2 {
 					regulator-max-microvolt = <3700000>;
 
 					regulator-state-standby {
-						regulator-suspend-voltage = <1800000>;
+						regulator-suspend-microvolt = <1800000>;
 						regulator-on-in-suspend;
 					};
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH 0/2] Fix the regulator-state-standby definition
From: Andrei Simion @ 2024-04-02  9:12 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, nicolas.ferre,
	alexandre.belloni, claudiu.beznea, mihai.sain
  Cc: linux-arm-kernel, linux-kernel, devicetree, Andrei Simion

make dtbs_check DT_SCHEMA_FILES=microchip,mcp16502.yaml

at91-sama7g5ek.dtb: mcp16502@5b: regulators:VDD_(CORE|OTHER)|LDO[1-2]:
regulator-state-standby 'regulator-suspend-voltage' does not match any of
the regexes 'pinctrl-[0-9]+' from schema
$id: http://devicetree.org/schemas/regulator/microchip,mcp16502.yaml#

at91-sama7g54_curiosity.dtb: pmic@5b: regulators:VDD_(CORE|OTHER)|LDO[1-2]:
regulator-state-standby 'regulator-suspend-voltage' does not match any of
the regexes 'pinctrl-[0-9]+' from schema
$id: http://devicetree.org/schemas/regulator/microchip,mcp16502.yaml#

This patch series proposes to correct the typo that was entered by mistake
into devicetree definition regulator-state-standby by replacing
regulator-suspend-voltage with regulator-suspend-microvolt.

Andrei Simion (2):
  ARM: boot: dts: microchip: at91-sama7g5ek: Replace
    regulator-suspend-voltage with the valid property
  ARM: boot: dts: microchip: at91-sama7g54_curiosity: Replace
    regulator-suspend-voltage with the valid property

 arch/arm/boot/dts/microchip/at91-sama7g54_curiosity.dts | 8 ++++----
 arch/arm/boot/dts/microchip/at91-sama7g5ek.dts          | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.34.1


^ permalink raw reply

* Re: [PATCH v10 09/11] arm64: dts: imx93-11x11-evk: enable usb and typec nodes
From: Shawn Guo @ 2024-04-02  9:07 UTC (permalink / raw)
  To: Xu Yang
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, shawnguo, conor+dt,
	s.hauer, kernel, festevam, linux-imx, peter.chen, jun.li,
	linux-usb, devicetree, linux-arm-kernel, imx, linux-kernel
In-Reply-To: <20240321081439.541799-9-xu.yang_2@nxp.com>

On Thu, Mar 21, 2024 at 04:14:37PM +0800, Xu Yang wrote:
> There are 2 Type-C ports and 2 USB controllers on i.MX93. Enable them.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - remove status property in ptn5110 nodes
>  - fix dt-schema warnings
> Changes in v3:
>  - no changes
> Changes in v4:
>  - no changes
> Changes in v5:
>  - no changes
> Changes in v6:
>  - no changes
> Changes in v7:
>  - no changes
> Changes in v8:
>  - no changes
> Changes in v9:
>  - use compatible "nxp,ptn5110", "tcpci"
> Changes in v10:
>  - no changes
> ---
>  .../boot/dts/freescale/imx93-11x11-evk.dts    | 118 ++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> index 9921ea13ab48..ecc01d872e95 100644
> --- a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> @@ -5,6 +5,7 @@
>  
>  /dts-v1/;
>  
> +#include <dt-bindings/usb/pd.h>
>  #include "imx93.dtsi"
>  
>  / {
> @@ -104,6 +105,80 @@ &mu2 {
>  	status = "okay";
>  };
>  
> +&lpi2c3 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&pinctrl_lpi2c3>;
> +	pinctrl-1 = <&pinctrl_lpi2c3>;

Do you really need "sleep" pinctrl state?

> +	status = "okay";
> +
> +	ptn5110: tcpc@50 {
> +		compatible = "nxp,ptn5110", "tcpci";
> +		reg = <0x50>;
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +
> +		typec1_con: connector {
> +			compatible = "usb-c-connector";
> +			label = "USB-C";
> +			power-role = "dual";
> +			data-role = "dual";
> +			try-power-role = "sink";
> +			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> +			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> +				     PDO_VAR(5000, 20000, 3000)>;
> +			op-sink-microwatt = <15000000>;
> +			self-powered;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;

Have a newline between properties and child node.

Shawn

> +					typec1_dr_sw: endpoint {
> +						remote-endpoint = <&usb1_drd_sw>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +
> +	ptn5110_2: tcpc@51 {
> +		compatible = "nxp,ptn5110", "tcpci";
> +		reg = <0x51>;
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> +
> +		typec2_con: connector {
> +			compatible = "usb-c-connector";
> +			label = "USB-C";
> +			power-role = "dual";
> +			data-role = "dual";
> +			try-power-role = "sink";
> +			source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> +			sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> +				     PDO_VAR(5000, 20000, 3000)>;
> +			op-sink-microwatt = <15000000>;
> +			self-powered;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +					typec2_dr_sw: endpoint {
> +						remote-endpoint = <&usb2_drd_sw>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +};
> +
>  &eqos {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_eqos>;
> @@ -156,6 +231,42 @@ &lpuart5 {
>  	status = "okay";
>  };
>  
> +&usbotg1 {
> +	dr_mode = "otg";
> +	hnp-disable;
> +	srp-disable;
> +	adp-disable;
> +	usb-role-switch;
> +	disable-over-current;
> +	samsung,picophy-pre-emp-curr-control = <3>;
> +	samsung,picophy-dc-vol-level-adjust = <7>;
> +	status = "okay";
> +
> +	port {
> +		usb1_drd_sw: endpoint {
> +			remote-endpoint = <&typec1_dr_sw>;
> +		};
> +	};
> +};
> +
> +&usbotg2 {
> +	dr_mode = "otg";
> +	hnp-disable;
> +	srp-disable;
> +	adp-disable;
> +	usb-role-switch;
> +	disable-over-current;
> +	samsung,picophy-pre-emp-curr-control = <3>;
> +	samsung,picophy-dc-vol-level-adjust = <7>;
> +	status = "okay";
> +
> +	port {
> +		usb2_drd_sw: endpoint {
> +			remote-endpoint = <&typec2_dr_sw>;
> +		};
> +	};
> +};
> +
>  &usdhc1 {
>  	pinctrl-names = "default", "state_100mhz", "state_200mhz";
>  	pinctrl-0 = <&pinctrl_usdhc1>;
> @@ -222,6 +333,13 @@ MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL	0x57e
>  		>;
>  	};
>  
> +	pinctrl_lpi2c3: lpi2c3grp {
> +		fsl,pins = <
> +			MX93_PAD_GPIO_IO28__LPI2C3_SDA			0x40000b9e
> +			MX93_PAD_GPIO_IO29__LPI2C3_SCL			0x40000b9e
> +		>;
> +	};
> +
>  	pinctrl_uart1: uart1grp {
>  		fsl,pins = <
>  			MX93_PAD_UART1_RXD__LPUART1_RX			0x31e
> -- 
> 2.34.1
> 


^ permalink raw reply

* Re: [PATCH v7 2/2] dmaengine: Loongson1: Add Loongson-1 APB DMA driver
From: Huacai Chen @ 2024-04-02  9:04 UTC (permalink / raw)
  To: Keguang Zhang
  Cc: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-mips, dmaengine, devicetree, linux-kernel
In-Reply-To: <CAJhJPsU6agzBR1jOw73SpMoogUMYu0qQT2VaBa+z1DXw2ZPNvw@mail.gmail.com>

On Tue, Apr 2, 2024 at 9:56 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
>
> On Mon, Apr 1, 2024 at 9:24 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > On Mon, Apr 1, 2024 at 7:10 PM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > >
> > > On Mon, Apr 1, 2024 at 5:06 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > >
> > > > On Mon, Apr 1, 2024 at 10:45 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > > > >
> > > > > Hi Huacai,
> > > > >
> > > > > On Sat, Mar 30, 2024 at 9:59 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > >
> > > > > > Hi, Keguang,
> > > > > >
> > > > > > On Fri, Mar 29, 2024 at 7:28 PM Keguang Zhang via B4 Relay
> > > > > > <devnull+keguang.zhang.gmail.com@kernel.org> wrote:
> > > > > > >
> > > > > > > From: Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > >
> > > > > > > This patch adds APB DMA driver for Loongson-1 SoCs.
> > > > > > >
> > > > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > ---
> > > > > > > Changes in v7:
> > > > > > > - Change the comptible to 'loongson,ls1*-apbdma'
> > > > > > > - Update Kconfig and Makefile accordingly
> > > > > > > - Rename the file to loongson1-apb-dma.c to keep the consistency
> > > > > > >
> > > > > > > Changes in v6:
> > > > > > > - Implement .device_prep_dma_cyclic for Loongson1 audio driver,
> > > > > > > - as well as .device_pause and .device_resume.
> > > > > > > - Set the limitation LS1X_DMA_MAX_DESC and put all descriptors
> > > > > > > - into one page to save memory
> > > > > > > - Move dma_pool_zalloc() into ls1x_dma_alloc_desc()
> > > > > > > - Drop dma_slave_config structure
> > > > > > > - Use .remove_new instead of .remove
> > > > > > > - Use KBUILD_MODNAME for the driver name
> > > > > > > - Improve the debug information
> > > > > > >
> > > > > > > Changes in v5:
> > > > > > > - Add DT support
> > > > > > > - Use DT data instead of platform data
> > > > > > > - Use chan_id of struct dma_chan instead of own id
> > > > > > > - Use of_dma_xlate_by_chan_id() instead of ls1x_dma_filter()
> > > > > > > - Update the author information to my official name
> > > > > > >
> > > > > > > Changes in v4:
> > > > > > > - Use dma_slave_map to find the proper channel.
> > > > > > > - Explicitly call devm_request_irq() and tasklet_kill().
> > > > > > > - Fix namespace issue.
> > > > > > > - Some minor fixes and cleanups.
> > > > > > >
> > > > > > > Changes in v3:
> > > > > > > - Rename ls1x_dma_filter_fn to ls1x_dma_filter.
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - Change the config from 'DMA_LOONGSON1' to 'LOONGSON1_DMA',
> > > > > > > - and rearrange it in alphabetical order in Kconfig and Makefile.
> > > > > > > - Fix comment style.
> > > > > > > ---
> > > > > > >  drivers/dma/Kconfig             |   9 +
> > > > > > >  drivers/dma/Makefile            |   1 +
> > > > > > >  drivers/dma/loongson1-apb-dma.c | 665 ++++++++++++++++++++++++++++++++++++++++
> > > > > > >  3 files changed, 675 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > > > > > > index 002a5ec80620..f7b06c4cdf3f 100644
> > > > > > > --- a/drivers/dma/Kconfig
> > > > > > > +++ b/drivers/dma/Kconfig
> > > > > > > @@ -369,6 +369,15 @@ config K3_DMA
> > > > > > >           Support the DMA engine for Hisilicon K3 platform
> > > > > > >           devices.
> > > > > > >
> > > > > > > +config LOONGSON1_APB_DMA
> > > > > > > +       tristate "Loongson1 APB DMA support"
> > > > > > > +       depends on MACH_LOONGSON32 || COMPILE_TEST
> > > > > > > +       select DMA_ENGINE
> > > > > > > +       select DMA_VIRTUAL_CHANNELS
> > > > > > > +       help
> > > > > > > +         This selects support for the APB DMA controller in Loongson1 SoCs,
> > > > > > > +         which is required by Loongson1 NAND and audio support.
> > > > > > Why not rename to LS1X_APB_DMA and put it just before LS2X_APB_DMA
> > > > > > (and also the driver file name)?
> > > > > >
> > > > > So far all Kconfig entries of Loongson-1 drivers are named with the
> > > > > keyword "LOONGSON1".
> > > > > The same is true for these file names.
> > > > > Therefore, I need to keep the consistency.
> > > > But I see LS1X_IRQ in drivers/irqchip/Kconfig
> > > >
> > > Indeed, that's an exception, which was submitted by Jiaxun several years ago.
> > > Actually, most drivers of Loongson family use the keyword "LOONGSON"
> > > for Kconfig and "loongson" for filename.
> > > Thus I take this keywork as the naming convention.
> > But I think keeping consistency in a same subsystem is better than
> > keeping consistency in a same SoC (but cross subsystems).
> >
> In my opinion, "LS*X" is too short and may be confused with other SoCs.
> Meanwhile, there are only four drivers that use this keyword.
>   config I2C_LS2X
>   config LS2K_RESET
>   config LS2X_APB_DMA
>   config LS1X_IRQ
> Then, my suggestion is to change these "LS*X" to "LOONGSON*" to get a
> clear meaning.
We have made a naming conversion some years before with Jiaxun.
1, Use "Loongson" for CPU in arch code;
2, Use "LS7A" or something like this for bridges and devices.
3, For drivers in SoC, if the driver is specific to Loongson-1, use
LS1X, if it is to Loongson-2, use LS2X, if it is shared by both
Loongson-1 and Loongson-2, use LOONGSON.

Huacai

>
> > Huacai
> >
> > >
> > > > Huacai
> > > >
> > > > >
> > > > >
> > > > > > Huacai
> > > > > >
> > > > > > > +
> > > > > > >  config LPC18XX_DMAMUX
> > > > > > >         bool "NXP LPC18xx/43xx DMA MUX for PL080"
> > > > > > >         depends on ARCH_LPC18XX || COMPILE_TEST
> > > > > > > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > > > > > > index dfd40d14e408..b26f6677978a 100644
> > > > > > > --- a/drivers/dma/Makefile
> > > > > > > +++ b/drivers/dma/Makefile
> > > > > > > @@ -47,6 +47,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
> > > > > > >  obj-$(CONFIG_INTEL_IOATDMA) += ioat/
> > > > > > >  obj-y += idxd/
> > > > > > >  obj-$(CONFIG_K3_DMA) += k3dma.o
> > > > > > > +obj-$(CONFIG_LOONGSON1_APB_DMA) += loongson1-apb-dma.o
> > > > > > >  obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
> > > > > > >  obj-$(CONFIG_LS2X_APB_DMA) += ls2x-apb-dma.o
> > > > > > >  obj-$(CONFIG_MILBEAUT_HDMAC) += milbeaut-hdmac.o
> > > > > > > diff --git a/drivers/dma/loongson1-apb-dma.c b/drivers/dma/loongson1-apb-dma.c
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..d474a2601e6e
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/dma/loongson1-apb-dma.c
> > > > > > > @@ -0,0 +1,665 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > > > > +/*
> > > > > > > + * Driver for Loongson-1 APB DMA Controller
> > > > > > > + *
> > > > > > > + * Copyright (C) 2015-2024 Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include <linux/dmapool.h>
> > > > > > > +#include <linux/dma-mapping.h>
> > > > > > > +#include <linux/init.h>
> > > > > > > +#include <linux/interrupt.h>
> > > > > > > +#include <linux/iopoll.h>
> > > > > > > +#include <linux/module.h>
> > > > > > > +#include <linux/of.h>
> > > > > > > +#include <linux/of_dma.h>
> > > > > > > +#include <linux/platform_device.h>
> > > > > > > +#include <linux/slab.h>
> > > > > > > +
> > > > > > > +#include "dmaengine.h"
> > > > > > > +#include "virt-dma.h"
> > > > > > > +
> > > > > > > +/* Loongson-1 DMA Control Register */
> > > > > > > +#define DMA_CTRL                       0x0
> > > > > > > +
> > > > > > > +/* DMA Control Register Bits */
> > > > > > > +#define DMA_STOP                       BIT(4)
> > > > > > > +#define DMA_START                      BIT(3)
> > > > > > > +#define DMA_ASK_VALID                  BIT(2)
> > > > > > > +
> > > > > > > +#define DMA_ADDR_MASK                  GENMASK(31, 6)
> > > > > > > +
> > > > > > > +/* DMA Next Field Bits */
> > > > > > > +#define DMA_NEXT_VALID                 BIT(0)
> > > > > > > +
> > > > > > > +/* DMA Command Field Bits */
> > > > > > > +#define DMA_RAM2DEV                    BIT(12)
> > > > > > > +#define DMA_INT                                BIT(1)
> > > > > > > +#define DMA_INT_MASK                   BIT(0)
> > > > > > > +
> > > > > > > +#define LS1X_DMA_MAX_CHANNELS          3
> > > > > > > +
> > > > > > > +/* Size of allocations for hardware descriptors */
> > > > > > > +#define LS1X_DMA_DESCS_SIZE            PAGE_SIZE
> > > > > > > +#define LS1X_DMA_MAX_DESC              \
> > > > > > > +       (LS1X_DMA_DESCS_SIZE / sizeof(struct ls1x_dma_hwdesc))
> > > > > > > +
> > > > > > > +struct ls1x_dma_hwdesc {
> > > > > > > +       u32 next;               /* next descriptor address */
> > > > > > > +       u32 saddr;              /* memory DMA address */
> > > > > > > +       u32 daddr;              /* device DMA address */
> > > > > > > +       u32 length;
> > > > > > > +       u32 stride;
> > > > > > > +       u32 cycles;
> > > > > > > +       u32 cmd;
> > > > > > > +       u32 stats;
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct ls1x_dma_desc {
> > > > > > > +       struct virt_dma_desc vdesc;
> > > > > > > +       enum dma_transfer_direction dir;
> > > > > > > +       enum dma_transaction_type type;
> > > > > > > +       unsigned int bus_width;
> > > > > > > +
> > > > > > > +       unsigned int nr_descs;  /* number of descriptors */
> > > > > > > +
> > > > > > > +       struct ls1x_dma_hwdesc *hwdesc;
> > > > > > > +       dma_addr_t hwdesc_phys;
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct ls1x_dma_chan {
> > > > > > > +       struct virt_dma_chan vchan;
> > > > > > > +       struct dma_pool *desc_pool;
> > > > > > > +       phys_addr_t src_addr;
> > > > > > > +       phys_addr_t dst_addr;
> > > > > > > +       enum dma_slave_buswidth src_addr_width;
> > > > > > > +       enum dma_slave_buswidth dst_addr_width;
> > > > > > > +
> > > > > > > +       void __iomem *reg_base;
> > > > > > > +       int irq;
> > > > > > > +
> > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > +
> > > > > > > +       struct ls1x_dma_hwdesc *curr_hwdesc;
> > > > > > > +       dma_addr_t curr_hwdesc_phys;
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct ls1x_dma {
> > > > > > > +       struct dma_device ddev;
> > > > > > > +       void __iomem *reg_base;
> > > > > > > +
> > > > > > > +       unsigned int nr_chans;
> > > > > > > +       struct ls1x_dma_chan chan[];
> > > > > > > +};
> > > > > > > +
> > > > > > > +#define to_ls1x_dma_chan(dchan)                \
> > > > > > > +       container_of(dchan, struct ls1x_dma_chan, vchan.chan)
> > > > > > > +
> > > > > > > +#define to_ls1x_dma_desc(vd)           \
> > > > > > > +       container_of(vd, struct ls1x_dma_desc, vdesc)
> > > > > > > +
> > > > > > > +/* macros for registers read/write */
> > > > > > > +#define chan_readl(chan, off)          \
> > > > > > > +       readl((chan)->reg_base + (off))
> > > > > > > +
> > > > > > > +#define chan_writel(chan, off, val)    \
> > > > > > > +       writel((val), (chan)->reg_base + (off))
> > > > > > > +
> > > > > > > +static inline struct device *chan2dev(struct dma_chan *chan)
> > > > > > > +{
> > > > > > > +       return &chan->dev->device;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static inline int ls1x_dma_query(struct ls1x_dma_chan *chan,
> > > > > > > +                                dma_addr_t *hwdesc_phys)
> > > > > > > +{
> > > > > > > +       struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > +       int val, ret;
> > > > > > > +
> > > > > > > +       val = *hwdesc_phys & DMA_ADDR_MASK;
> > > > > > > +       val |= DMA_ASK_VALID;
> > > > > > > +       val |= dchan->chan_id;
> > > > > > > +       chan_writel(chan, DMA_CTRL, val);
> > > > > > > +       ret = readl_poll_timeout_atomic(chan->reg_base + DMA_CTRL, val,
> > > > > > > +                                       !(val & DMA_ASK_VALID), 0, 3000);
> > > > > > > +       if (ret)
> > > > > > > +               dev_err(chan2dev(dchan), "failed to query DMA\n");
> > > > > > > +
> > > > > > > +       return ret;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static inline int ls1x_dma_start(struct ls1x_dma_chan *chan,
> > > > > > > +                                dma_addr_t *hwdesc_phys)
> > > > > > > +{
> > > > > > > +       struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > +       int val, ret;
> > > > > > > +
> > > > > > > +       dev_dbg(chan2dev(dchan), "cookie=%d, starting hwdesc=%x\n",
> > > > > > > +               dchan->cookie, *hwdesc_phys);
> > > > > > > +
> > > > > > > +       val = *hwdesc_phys & DMA_ADDR_MASK;
> > > > > > > +       val |= DMA_START;
> > > > > > > +       val |= dchan->chan_id;
> > > > > > > +       chan_writel(chan, DMA_CTRL, val);
> > > > > > > +       ret = readl_poll_timeout(chan->reg_base + DMA_CTRL, val,
> > > > > > > +                                !(val & DMA_START), 0, 3000);
> > > > > > > +       if (ret)
> > > > > > > +               dev_err(chan2dev(dchan), "failed to start DMA\n");
> > > > > > > +
> > > > > > > +       return ret;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static inline void ls1x_dma_stop(struct ls1x_dma_chan *chan)
> > > > > > > +{
> > > > > > > +       chan_writel(chan, DMA_CTRL, chan_readl(chan, DMA_CTRL) | DMA_STOP);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void ls1x_dma_free_chan_resources(struct dma_chan *dchan)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +
> > > > > > > +       dma_free_coherent(chan2dev(dchan), sizeof(struct ls1x_dma_hwdesc),
> > > > > > > +                         chan->curr_hwdesc, chan->curr_hwdesc_phys);
> > > > > > > +       vchan_free_chan_resources(&chan->vchan);
> > > > > > > +       dma_pool_destroy(chan->desc_pool);
> > > > > > > +       chan->desc_pool = NULL;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_alloc_chan_resources(struct dma_chan *dchan)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +
> > > > > > > +       chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
> > > > > > > +                                         chan2dev(dchan),
> > > > > > > +                                         sizeof(struct ls1x_dma_hwdesc),
> > > > > > > +                                         __alignof__(struct ls1x_dma_hwdesc),
> > > > > > > +                                         0);
> > > > > > > +       if (!chan->desc_pool)
> > > > > > > +               return -ENOMEM;
> > > > > > > +
> > > > > > > +       /* allocate memory for querying current HW descriptor */
> > > > > > > +       dma_set_coherent_mask(chan2dev(dchan), DMA_BIT_MASK(32));
> > > > > > > +       chan->curr_hwdesc = dma_alloc_coherent(chan2dev(dchan),
> > > > > > > +                                              sizeof(struct ls1x_dma_hwdesc),
> > > > > > > +                                              &chan->curr_hwdesc_phys,
> > > > > > > +                                              GFP_KERNEL);
> > > > > > > +       if (!chan->curr_hwdesc)
> > > > > > > +               return -ENOMEM;
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void ls1x_dma_free_desc(struct virt_dma_desc *vdesc)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vdesc);
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(vdesc->tx.chan);
> > > > > > > +
> > > > > > > +       dma_pool_free(chan->desc_pool, desc->hwdesc, desc->hwdesc_phys);
> > > > > > > +       chan->desc = NULL;
> > > > > > > +       kfree(desc);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct ls1x_dma_desc *
> > > > > > > +ls1x_dma_alloc_desc(struct dma_chan *dchan, int sg_len,
> > > > > > > +                   enum dma_transfer_direction direction,
> > > > > > > +                   enum dma_transaction_type type)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > +
> > > > > > > +       if (sg_len > LS1X_DMA_MAX_DESC) {
> > > > > > > +               dev_err(chan2dev(dchan), "sg_len %u exceeds limit %lu",
> > > > > > > +                       sg_len, LS1X_DMA_MAX_DESC);
> > > > > > > +               return NULL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > > > > > > +       if (!desc)
> > > > > > > +               return NULL;
> > > > > > > +
> > > > > > > +       /* allocate HW descriptors */
> > > > > > > +       desc->hwdesc = dma_pool_zalloc(chan->desc_pool, GFP_NOWAIT,
> > > > > > > +                                      &desc->hwdesc_phys);
> > > > > > > +       if (!desc->hwdesc) {
> > > > > > > +               dev_err(chan2dev(dchan), "failed to alloc HW descriptors\n");
> > > > > > > +               ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > +               return NULL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       desc->dir = direction;
> > > > > > > +       desc->type = type;
> > > > > > > +       desc->nr_descs = sg_len;
> > > > > > > +
> > > > > > > +       return desc;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_setup_hwdescs(struct dma_chan *dchan,
> > > > > > > +                                 struct ls1x_dma_desc *desc,
> > > > > > > +                                 struct scatterlist *sgl, unsigned int sg_len)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       dma_addr_t next_hwdesc_phys = desc->hwdesc_phys;
> > > > > > > +
> > > > > > > +       struct scatterlist *sg;
> > > > > > > +       unsigned int dev_addr, cmd, i;
> > > > > > > +
> > > > > > > +       switch (desc->dir) {
> > > > > > > +       case DMA_MEM_TO_DEV:
> > > > > > > +               dev_addr = chan->dst_addr;
> > > > > > > +               desc->bus_width = chan->dst_addr_width;
> > > > > > > +               cmd = DMA_RAM2DEV | DMA_INT;
> > > > > > > +               break;
> > > > > > > +       case DMA_DEV_TO_MEM:
> > > > > > > +               dev_addr = chan->src_addr;
> > > > > > > +               desc->bus_width = chan->src_addr_width;
> > > > > > > +               cmd = DMA_INT;
> > > > > > > +               break;
> > > > > > > +       default:
> > > > > > > +               dev_err(chan2dev(dchan), "unsupported DMA direction: %s\n",
> > > > > > > +                       dmaengine_get_direction_text(desc->dir));
> > > > > > > +               return -EINVAL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       /* setup HW descriptors */
> > > > > > > +       for_each_sg(sgl, sg, sg_len, i) {
> > > > > > > +               dma_addr_t buf_addr = sg_dma_address(sg);
> > > > > > > +               size_t buf_len = sg_dma_len(sg);
> > > > > > > +               struct ls1x_dma_hwdesc *hwdesc = &desc->hwdesc[i];
> > > > > > > +
> > > > > > > +               if (!is_dma_copy_aligned(dchan->device, buf_addr, 0, buf_len)) {
> > > > > > > +                       dev_err(chan2dev(dchan), "buffer is not aligned!\n");
> > > > > > > +                       return -EINVAL;
> > > > > > > +               }
> > > > > > > +
> > > > > > > +               hwdesc->saddr = buf_addr;
> > > > > > > +               hwdesc->daddr = dev_addr;
> > > > > > > +               hwdesc->length = buf_len / desc->bus_width;
> > > > > > > +               hwdesc->stride = 0;
> > > > > > > +               hwdesc->cycles = 1;
> > > > > > > +               hwdesc->cmd = cmd;
> > > > > > > +
> > > > > > > +               if (i) {
> > > > > > > +                       next_hwdesc_phys += sizeof(*hwdesc);
> > > > > > > +                       desc->hwdesc[i - 1].next = next_hwdesc_phys
> > > > > > > +                           | DMA_NEXT_VALID;
> > > > > > > +               }
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       if (desc->type == DMA_CYCLIC)
> > > > > > > +               desc->hwdesc[i - 1].next = desc->hwdesc_phys | DMA_NEXT_VALID;
> > > > > > > +
> > > > > > > +       for_each_sg(sgl, sg, sg_len, i) {
> > > > > > > +               struct ls1x_dma_hwdesc *hwdesc = &desc->hwdesc[i];
> > > > > > > +
> > > > > > > +               print_hex_dump_debug("HW DESC: ", DUMP_PREFIX_OFFSET, 16, 4,
> > > > > > > +                                    hwdesc, sizeof(*hwdesc), false);
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct dma_async_tx_descriptor *
> > > > > > > +ls1x_dma_prep_slave_sg(struct dma_chan *dchan,
> > > > > > > +                      struct scatterlist *sgl, unsigned int sg_len,
> > > > > > > +                      enum dma_transfer_direction direction,
> > > > > > > +                      unsigned long flags, void *context)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > +
> > > > > > > +       dev_dbg(chan2dev(dchan), "sg_len=%u flags=0x%lx dir=%s\n",
> > > > > > > +               sg_len, flags, dmaengine_get_direction_text(direction));
> > > > > > > +
> > > > > > > +       desc = ls1x_dma_alloc_desc(dchan, sg_len, direction, DMA_SLAVE);
> > > > > > > +       if (!desc)
> > > > > > > +               return NULL;
> > > > > > > +
> > > > > > > +       if (ls1x_dma_setup_hwdescs(dchan, desc, sgl, sg_len)) {
> > > > > > > +               ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > +               return NULL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct dma_async_tx_descriptor *
> > > > > > > +ls1x_dma_prep_dma_cyclic(struct dma_chan *dchan,
> > > > > > > +                        dma_addr_t buf_addr, size_t buf_len, size_t period_len,
> > > > > > > +                        enum dma_transfer_direction direction,
> > > > > > > +                        unsigned long flags)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       struct ls1x_dma_desc *desc;
> > > > > > > +       struct scatterlist *sgl;
> > > > > > > +       unsigned int sg_len;
> > > > > > > +       unsigned int i;
> > > > > > > +
> > > > > > > +       dev_dbg(chan2dev(dchan),
> > > > > > > +               "buf_len=%d period_len=%zu flags=0x%lx dir=%s\n", buf_len,
> > > > > > > +               period_len, flags, dmaengine_get_direction_text(direction));
> > > > > > > +
> > > > > > > +       sg_len = buf_len / period_len;
> > > > > > > +       desc = ls1x_dma_alloc_desc(dchan, sg_len, direction, DMA_CYCLIC);
> > > > > > > +       if (!desc)
> > > > > > > +               return NULL;
> > > > > > > +
> > > > > > > +       /* allocate the scatterlist */
> > > > > > > +       sgl = kmalloc_array(sg_len, sizeof(*sgl), GFP_NOWAIT);
> > > > > > > +       if (!sgl)
> > > > > > > +               return NULL;
> > > > > > > +
> > > > > > > +       sg_init_table(sgl, sg_len);
> > > > > > > +       for (i = 0; i < sg_len; ++i) {
> > > > > > > +               sg_set_page(&sgl[i], pfn_to_page(PFN_DOWN(buf_addr)),
> > > > > > > +                           period_len, offset_in_page(buf_addr));
> > > > > > > +               sg_dma_address(&sgl[i]) = buf_addr;
> > > > > > > +               sg_dma_len(&sgl[i]) = period_len;
> > > > > > > +               buf_addr += period_len;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       if (ls1x_dma_setup_hwdescs(dchan, desc, sgl, sg_len)) {
> > > > > > > +               ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > +               return NULL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       kfree(sgl);
> > > > > > > +
> > > > > > > +       return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_slave_config(struct dma_chan *dchan,
> > > > > > > +                                struct dma_slave_config *config)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +
> > > > > > > +       chan->src_addr = config->src_addr;
> > > > > > > +       chan->src_addr_width = config->src_addr_width;
> > > > > > > +       chan->dst_addr = config->dst_addr;
> > > > > > > +       chan->dst_addr_width = config->dst_addr_width;
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_pause(struct dma_chan *dchan)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       unsigned long flags;
> > > > > > > +       int ret;
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > +       ret = ls1x_dma_query(chan, &chan->curr_hwdesc_phys);
> > > > > > > +       if (!ret)
> > > > > > > +               ls1x_dma_stop(chan);
> > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > +
> > > > > > > +       return ret;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_resume(struct dma_chan *dchan)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       unsigned long flags;
> > > > > > > +       int ret;
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > +       ret = ls1x_dma_start(chan, &chan->curr_hwdesc_phys);
> > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > +
> > > > > > > +       return ret;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_terminate_all(struct dma_chan *dchan)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       unsigned long flags;
> > > > > > > +       LIST_HEAD(head);
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > +       ls1x_dma_stop(chan);
> > > > > > > +       vchan_get_all_descriptors(&chan->vchan, &head);
> > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > +
> > > > > > > +       vchan_dma_desc_free_list(&chan->vchan, &head);
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> > > > > > > +                                         dma_cookie_t cookie,
> > > > > > > +                                         struct dma_tx_state *state)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       struct virt_dma_desc *vdesc;
> > > > > > > +       enum dma_status status;
> > > > > > > +       size_t bytes = 0;
> > > > > > > +       unsigned long flags;
> > > > > > > +
> > > > > > > +       status = dma_cookie_status(dchan, cookie, state);
> > > > > > > +       if (status == DMA_COMPLETE)
> > > > > > > +               return status;
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > +       vdesc = vchan_find_desc(&chan->vchan, cookie);
> > > > > > > +       if (chan->desc && cookie == chan->desc->vdesc.tx.cookie) {
> > > > > > > +               struct ls1x_dma_desc *desc = chan->desc;
> > > > > > > +               int i;
> > > > > > > +
> > > > > > > +               if (ls1x_dma_query(chan, &chan->curr_hwdesc_phys))
> > > > > > > +                       return status;
> > > > > > > +
> > > > > > > +               /* locate the current HW descriptor */
> > > > > > > +               for (i = 0; i < desc->nr_descs; i++)
> > > > > > > +                       if (desc->hwdesc[i].next == chan->curr_hwdesc->next)
> > > > > > > +                               break;
> > > > > > > +
> > > > > > > +               /* count the residues */
> > > > > > > +               for (; i < desc->nr_descs; i++)
> > > > > > > +                       bytes += desc->hwdesc[i].length * desc->bus_width;
> > > > > > > +
> > > > > > > +               dma_set_residue(state, bytes);
> > > > > > > +       }
> > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > +
> > > > > > > +       return status;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void ls1x_dma_issue_pending(struct dma_chan *dchan)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > +       struct virt_dma_desc *vdesc;
> > > > > > > +       unsigned long flags;
> > > > > > > +
> > > > > > > +       spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > +       if (vchan_issue_pending(&chan->vchan) && !chan->desc) {
> > > > > > > +               vdesc = vchan_next_desc(&chan->vchan);
> > > > > > > +               if (!vdesc) {
> > > > > > > +                       chan->desc = NULL;
> > > > > > > +                       return;
> > > > > > > +               }
> > > > > > > +               chan->desc = to_ls1x_dma_desc(vdesc);
> > > > > > > +               ls1x_dma_start(chan, &chan->desc->hwdesc_phys);
> > > > > > > +       }
> > > > > > > +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static irqreturn_t ls1x_dma_irq_handler(int irq, void *data)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma_chan *chan = data;
> > > > > > > +       struct ls1x_dma_desc *desc = chan->desc;
> > > > > > > +       struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > +
> > > > > > > +       if (!desc) {
> > > > > > > +               dev_warn(chan2dev(dchan),
> > > > > > > +                        "IRQ %d with no active descriptor on channel %d\n",
> > > > > > > +                        irq, dchan->chan_id);
> > > > > > > +               return IRQ_NONE;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       dev_dbg(chan2dev(dchan), "DMA IRQ %d on channel %d\n", irq,
> > > > > > > +               dchan->chan_id);
> > > > > > > +
> > > > > > > +       spin_lock(&chan->vchan.lock);
> > > > > > > +
> > > > > > > +       if (desc->type == DMA_CYCLIC) {
> > > > > > > +               vchan_cyclic_callback(&desc->vdesc);
> > > > > > > +       } else {
> > > > > > > +               list_del(&desc->vdesc.node);
> > > > > > > +               vchan_cookie_complete(&desc->vdesc);
> > > > > > > +               chan->desc = NULL;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       spin_unlock(&chan->vchan.lock);
> > > > > > > +       return IRQ_HANDLED;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_chan_probe(struct platform_device *pdev,
> > > > > > > +                              struct ls1x_dma *dma, int chan_id)
> > > > > > > +{
> > > > > > > +       struct device *dev = &pdev->dev;
> > > > > > > +       struct ls1x_dma_chan *chan = &dma->chan[chan_id];
> > > > > > > +       char pdev_irqname[4];
> > > > > > > +       char *irqname;
> > > > > > > +       int ret;
> > > > > > > +
> > > > > > > +       sprintf(pdev_irqname, "ch%u", chan_id);
> > > > > > > +       chan->irq = platform_get_irq_byname(pdev, pdev_irqname);
> > > > > > > +       if (chan->irq < 0)
> > > > > > > +               return -ENODEV;
> > > > > > > +
> > > > > > > +       irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:%s",
> > > > > > > +                                dev_name(dev), pdev_irqname);
> > > > > > > +       if (!irqname)
> > > > > > > +               return -ENOMEM;
> > > > > > > +
> > > > > > > +       ret = devm_request_irq(dev, chan->irq, ls1x_dma_irq_handler,
> > > > > > > +                              IRQF_SHARED, irqname, chan);
> > > > > > > +       if (ret)
> > > > > > > +               return dev_err_probe(dev, ret,
> > > > > > > +                                    "failed to request IRQ %u!\n", chan->irq);
> > > > > > > +
> > > > > > > +       chan->reg_base = dma->reg_base;
> > > > > > > +       chan->vchan.desc_free = ls1x_dma_free_desc;
> > > > > > > +       vchan_init(&chan->vchan, &dma->ddev);
> > > > > > > +       dev_info(dev, "%s (irq %d) initialized\n", pdev_irqname, chan->irq);
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void ls1x_dma_chan_remove(struct ls1x_dma *dma, int chan_id)
> > > > > > > +{
> > > > > > > +       struct device *dev = dma->ddev.dev;
> > > > > > > +       struct ls1x_dma_chan *chan = &dma->chan[chan_id];
> > > > > > > +
> > > > > > > +       devm_free_irq(dev, chan->irq, chan);
> > > > > > > +       list_del(&chan->vchan.chan.device_node);
> > > > > > > +       tasklet_kill(&chan->vchan.task);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int ls1x_dma_probe(struct platform_device *pdev)
> > > > > > > +{
> > > > > > > +       struct device *dev = &pdev->dev;
> > > > > > > +       struct dma_device *ddev;
> > > > > > > +       struct ls1x_dma *dma;
> > > > > > > +       int nr_chans, ret, i;
> > > > > > > +
> > > > > > > +       nr_chans = platform_irq_count(pdev);
> > > > > > > +       if (nr_chans <= 0)
> > > > > > > +               return nr_chans;
> > > > > > > +       if (nr_chans > LS1X_DMA_MAX_CHANNELS)
> > > > > > > +               return dev_err_probe(dev, -EINVAL,
> > > > > > > +                                    "nr_chans=%d exceeds the maximum\n",
> > > > > > > +                                    nr_chans);
> > > > > > > +
> > > > > > > +       dma = devm_kzalloc(dev, struct_size(dma, chan, nr_chans), GFP_KERNEL);
> > > > > > > +       if (!dma)
> > > > > > > +               return -ENOMEM;
> > > > > > > +
> > > > > > > +       /* initialize DMA device */
> > > > > > > +       dma->reg_base = devm_platform_ioremap_resource(pdev, 0);
> > > > > > > +       if (IS_ERR(dma->reg_base))
> > > > > > > +               return PTR_ERR(dma->reg_base);
> > > > > > > +
> > > > > > > +       ddev = &dma->ddev;
> > > > > > > +       ddev->dev = dev;
> > > > > > > +       ddev->copy_align = DMAENGINE_ALIGN_4_BYTES;
> > > > > > > +       ddev->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > > > > +           BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > > > > > +       ddev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > > > > +           BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > > > > > +       ddev->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > > > > > > +       ddev->max_sg_burst = LS1X_DMA_MAX_DESC;
> > > > > > > +       ddev->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > > > > > > +       ddev->device_alloc_chan_resources = ls1x_dma_alloc_chan_resources;
> > > > > > > +       ddev->device_free_chan_resources = ls1x_dma_free_chan_resources;
> > > > > > > +       ddev->device_prep_slave_sg = ls1x_dma_prep_slave_sg;
> > > > > > > +       ddev->device_prep_dma_cyclic = ls1x_dma_prep_dma_cyclic;
> > > > > > > +       ddev->device_config = ls1x_dma_slave_config;
> > > > > > > +       ddev->device_pause = ls1x_dma_pause;
> > > > > > > +       ddev->device_resume = ls1x_dma_resume;
> > > > > > > +       ddev->device_terminate_all = ls1x_dma_terminate_all;
> > > > > > > +       ddev->device_tx_status = ls1x_dma_tx_status;
> > > > > > > +       ddev->device_issue_pending = ls1x_dma_issue_pending;
> > > > > > > +
> > > > > > > +       dma_cap_set(DMA_SLAVE, ddev->cap_mask);
> > > > > > > +       INIT_LIST_HEAD(&ddev->channels);
> > > > > > > +
> > > > > > > +       /* initialize DMA channels */
> > > > > > > +       for (i = 0; i < nr_chans; i++) {
> > > > > > > +               ret = ls1x_dma_chan_probe(pdev, dma, i);
> > > > > > > +               if (ret)
> > > > > > > +                       return ret;
> > > > > > > +       }
> > > > > > > +       dma->nr_chans = nr_chans;
> > > > > > > +
> > > > > > > +       ret = dmaenginem_async_device_register(ddev);
> > > > > > > +       if (ret) {
> > > > > > > +               dev_err(dev, "failed to register DMA device! %d\n", ret);
> > > > > > > +               return ret;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       ret =
> > > > > > > +           of_dma_controller_register(dev->of_node, of_dma_xlate_by_chan_id,
> > > > > > > +                                      ddev);
> > > > > > > +       if (ret) {
> > > > > > > +               dev_err(dev, "failed to register DMA controller! %d\n", ret);
> > > > > > > +               return ret;
> > > > > > > +       }
> > > > > > > +
> > > > > > > +       platform_set_drvdata(pdev, dma);
> > > > > > > +       dev_info(dev, "Loongson1 DMA driver registered\n");
> > > > > > > +
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void ls1x_dma_remove(struct platform_device *pdev)
> > > > > > > +{
> > > > > > > +       struct ls1x_dma *dma = platform_get_drvdata(pdev);
> > > > > > > +       int i;
> > > > > > > +
> > > > > > > +       of_dma_controller_free(pdev->dev.of_node);
> > > > > > > +
> > > > > > > +       for (i = 0; i < dma->nr_chans; i++)
> > > > > > > +               ls1x_dma_chan_remove(dma, i);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static const struct of_device_id ls1x_dma_match[] = {
> > > > > > > +       { .compatible = "loongson,ls1b-apbdma" },
> > > > > > > +       { .compatible = "loongson,ls1c-apbdma" },
> > > > > > > +       { /* sentinel */ }
> > > > > > > +};
> > > > > > > +MODULE_DEVICE_TABLE(of, ls1x_dma_match);
> > > > > > > +
> > > > > > > +static struct platform_driver ls1x_dma_driver = {
> > > > > > > +       .probe = ls1x_dma_probe,
> > > > > > > +       .remove_new = ls1x_dma_remove,
> > > > > > > +       .driver = {
> > > > > > > +               .name = KBUILD_MODNAME,
> > > > > > > +               .of_match_table = ls1x_dma_match,
> > > > > > > +       },
> > > > > > > +};
> > > > > > > +
> > > > > > > +module_platform_driver(ls1x_dma_driver);
> > > > > > > +
> > > > > > > +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
> > > > > > > +MODULE_DESCRIPTION("Loongson-1 APB DMA Controller driver");
> > > > > > > +MODULE_LICENSE("GPL");
> > > > > > >
> > > > > > > --
> > > > > > > 2.40.1
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > >
> > > > > Keguang Zhang
> > > > >
> > >
> > >
> > >
> > > --
> > > Best regards,
> > >
> > > Keguang Zhang
>
>
>
> --
> Best regards,
>
> Keguang Zhang

^ permalink raw reply

* Re: [PATCH v1 2/6] clk: meson: a1: pll: support 'syspll' general-purpose PLL for CPU clock
From: Jerome Brunet @ 2024-04-02  9:00 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl, kernel,
	rockosov, linux-amlogic, linux-clk, devicetree, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20240329205904.25002-3-ddrokosov@salutedevices.com>


On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:

> The 'syspll' PLL, also known as the system PLL, is a general and
> essential PLL responsible for generating the CPU clock frequency.
> With its wide-ranging capabilities, it is designed to accommodate
> frequencies within the range of 768MHz to 1536MHz.
>
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> ---
>  drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
>  drivers/clk/meson/a1-pll.h |  6 +++
>  2 files changed, 84 insertions(+)
>
> diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
> index 60b2e53e7e51..02fd2d325cc6 100644
> --- a/drivers/clk/meson/a1-pll.c
> +++ b/drivers/clk/meson/a1-pll.c
> @@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
>  	},
>  };
>  
> +static const struct pll_mult_range sys_pll_mult_range = {
> +	.min = 32,
> +	.max = 64,
> +};
> +
> +/*
> + * We assume that the sys_pll_clk has already been set up by the low-level
> + * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
> + * run the initialization sequence.
> + */

I see no reason to make such assumption.
This clock is no read-only, it apparently is able to re-lock so assuming
anything from the bootloader is just asking from trouble

> +static struct clk_regmap sys_pll = {
> +	.data = &(struct meson_clk_pll_data){
> +		.en = {
> +			.reg_off = ANACTRL_SYSPLL_CTRL0,
> +			.shift   = 28,
> +			.width   = 1,
> +		},
> +		.m = {
> +			.reg_off = ANACTRL_SYSPLL_CTRL0,
> +			.shift   = 0,
> +			.width   = 8,
> +		},
> +		.n = {
> +			.reg_off = ANACTRL_SYSPLL_CTRL0,
> +			.shift   = 10,
> +			.width   = 5,
> +		},
> +		.frac = {
> +			.reg_off = ANACTRL_SYSPLL_CTRL1,
> +			.shift   = 0,
> +			.width   = 19,
> +		},
> +		.l = {
> +			.reg_off = ANACTRL_SYSPLL_STS,
> +			.shift   = 31,
> +			.width   = 1,
> +		},
> +		.current_en = {
> +			.reg_off = ANACTRL_SYSPLL_CTRL0,
> +			.shift   = 26,
> +			.width   = 1,
> +		},
> +		.l_detect = {
> +			.reg_off = ANACTRL_SYSPLL_CTRL2,
> +			.shift   = 6,
> +			.width   = 1,
> +		},
> +		.range = &sys_pll_mult_range,
> +	},
> +	.hw.init = &(struct clk_init_data){
> +		.name = "sys_pll",
> +		.ops = &meson_clk_pll_ops,
> +		.parent_names = (const char *[]){ "syspll_in" },
> +		.num_parents = 1,
> +		/*
> +		 * This clock is used as the main CPU PLL source in low-level
> +		 * bootloaders, and it is necessary to mark it as critical.
> +		 */
> +		.flags = CLK_IS_CRITICAL,

No I don't think so. Downstream consumer maybe critical but that one is
not, unless it is read-only.

A CPU pll, like on the g12 family, is unlikely to be read-only since the
PLL will need to relock to change rates. During this phase, there will
be no reate coming from the PLL so the PLL is not critical and you must
be able to "park" your CPU an another clock while poking this one

> +	},
> +};
> +
> +static struct clk_fixed_factor sys_pll_div16 = {
> +	.mult = 1,
> +	.div = 16,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "sys_pll_div16",
> +		.ops = &clk_fixed_factor_ops,
> +		.parent_hws = (const struct clk_hw *[]) {
> +			&sys_pll.hw
> +		},
> +		.num_parents = 1,
> +	},
> +};
> +
>  static struct clk_fixed_factor fclk_div2_div = {
>  	.mult = 1,
>  	.div = 2,
> @@ -283,6 +358,8 @@ static struct clk_hw *a1_pll_hw_clks[] = {
>  	[CLKID_FCLK_DIV5]	= &fclk_div5.hw,
>  	[CLKID_FCLK_DIV7]	= &fclk_div7.hw,
>  	[CLKID_HIFI_PLL]	= &hifi_pll.hw,
> +	[CLKID_SYS_PLL]		= &sys_pll.hw,
> +	[CLKID_SYS_PLL_DIV16]	= &sys_pll_div16.hw,
>  };
>  
>  static struct clk_regmap *const a1_pll_regmaps[] = {
> @@ -293,6 +370,7 @@ static struct clk_regmap *const a1_pll_regmaps[] = {
>  	&fclk_div5,
>  	&fclk_div7,
>  	&hifi_pll,
> +	&sys_pll,
>  };
>  
>  static struct regmap_config a1_pll_regmap_cfg = {
> diff --git a/drivers/clk/meson/a1-pll.h b/drivers/clk/meson/a1-pll.h
> index 4be17b2bf383..666d9b2137e9 100644
> --- a/drivers/clk/meson/a1-pll.h
> +++ b/drivers/clk/meson/a1-pll.h
> @@ -18,6 +18,12 @@
>  #define ANACTRL_FIXPLL_CTRL0	0x0
>  #define ANACTRL_FIXPLL_CTRL1	0x4
>  #define ANACTRL_FIXPLL_STS	0x14
> +#define ANACTRL_SYSPLL_CTRL0	0x80
> +#define ANACTRL_SYSPLL_CTRL1	0x84
> +#define ANACTRL_SYSPLL_CTRL2	0x88
> +#define ANACTRL_SYSPLL_CTRL3	0x8c
> +#define ANACTRL_SYSPLL_CTRL4	0x90
> +#define ANACTRL_SYSPLL_STS	0x94
>  #define ANACTRL_HIFIPLL_CTRL0	0xc0
>  #define ANACTRL_HIFIPLL_CTRL1	0xc4
>  #define ANACTRL_HIFIPLL_CTRL2	0xc8


-- 
Jerome

^ permalink raw reply

* [PATCH v2 2/2] arm64: dts: qcom: qcs6490-rb3gen2: Enable various remoteprocs
From: Komal Bajaj @ 2024-04-02  9:03 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, quic_tsoni, Komal Bajaj
In-Reply-To: <20240402090349.30172-1-quic_kbajaj@quicinc.com>

Enable the ADSP, CDSP and WPSS that are found on qcs6490-rb3gen2.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 97824c769ba3..a25431ddf922 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -434,6 +434,21 @@ &qupv3_id_0 {
 	status = "okay";
 };

+&remoteproc_adsp {
+	firmware-name = "qcom/qcm6490/adsp.mbn";
+	status = "okay";
+};
+
+&remoteproc_cdsp {
+	firmware-name = "qcom/qcm6490/cdsp.mbn";
+	status = "okay";
+};
+
+&remoteproc_wpss {
+	firmware-name = "qcom/qcm6490/wpss.mbn";
+	status = "okay";
+};
+
 &tlmm {
 	gpio-reserved-ranges = <32 2>, /* ADSP */
 			       <48 4>; /* NFC */
--
2.42.0


^ permalink raw reply related

* [PATCH v2 1/2] arm64: dts: qcom: qcm6490-idp: Enable various remoteprocs
From: Komal Bajaj @ 2024-04-02  9:03 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, quic_tsoni, Komal Bajaj
In-Reply-To: <20240402090349.30172-1-quic_kbajaj@quicinc.com>

Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index e4bfad50a669..3014056a3607 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -452,6 +452,26 @@ &qupv3_id_0 {
 	status = "okay";
 };

+&remoteproc_adsp {
+	firmware-name = "qcom/qcm6490/adsp.mbn";
+	status = "okay";
+};
+
+&remoteproc_cdsp {
+	firmware-name = "qcom/qcm6490/cdsp.mbn";
+	status = "okay";
+};
+
+&remoteproc_mpss {
+	firmware-name = "qcom/qcm6490/modem.mbn";
+	status = "okay";
+};
+
+&remoteproc_wpss {
+	firmware-name = "qcom/qcm6490/wpss.mbn";
+	status = "okay";
+};
+
 &sdhc_1 {
 	non-removable;
 	no-sd;
--
2.42.0


^ permalink raw reply related

* [PATCH v2 0/2] Enable various remoteprocs for qcm6490-idp and qcs6490-rb3gen2
From: Komal Bajaj @ 2024-04-02  9:03 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, quic_tsoni, Komal Bajaj

Enable various applicable remoteproc nodes for qcm6490-idp
and qcs6490-rb3gen2.

Firmwares are not shared at linux-firmware.git, it is under legal approval process.
Meantime, submitting the DT node changes for FW for review.

--------
Changes in v2:
* Updating the firmware name from mdt to mbn
* Link to v1: https://lore.kernel.org/all/20231220114225.26567-1-quic_kbajaj@quicinc.com/

Komal Bajaj (2):
  arm64: dts: qcom: qcm6490-idp: Enable various remoteprocs
  arm64: dts: qcom: qcs6490-rb3gen2: Enable various remoteprocs

 arch/arm64/boot/dts/qcom/qcm6490-idp.dts     | 20 ++++++++++++++++++++
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 15 +++++++++++++++
 2 files changed, 35 insertions(+)

--
2.42.0


^ permalink raw reply

* RE: [PATCH v6 3/5] crypto: tegra: Add Tegra Security Engine driver
From: Akhil R @ 2024-04-02  9:00 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem@davemloft.net, robh@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	thierry.reding@gmail.com, Jon Hunter, catalin.marinas@arm.com,
	will@kernel.org, Mikko Perttunen, airlied@gmail.com,
	daniel@ffwll.ch, linux-crypto@vger.kernel.org,
	devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org
In-Reply-To: <ZgVBAFmfK7GKgmYi@gondor.apana.org.au>

> >
> > +             .alg.skcipher.op.do_one_request = tegra_aes_do_one_req,
> > +             .alg.skcipher.base = {
> > +                     .init = tegra_aes_cra_init,
> > +                     .exit = tegra_aes_cra_exit,
> > +                     .setkey = tegra_aes_setkey,
> > +                     .encrypt = tegra_aes_encrypt,
> > +                     .decrypt = tegra_aes_decrypt,
> > +                     .min_keysize = AES_MIN_KEY_SIZE,
> > +                     .max_keysize = AES_MAX_KEY_SIZE,
> > +                     .ivsize = AES_BLOCK_SIZE,
> > +                     .base = {
> > +                             .cra_name = "ofb(aes)",
> > +                             .cra_driver_name = "ofb-aes-tegra",
> > +                             .cra_priority = 500,
> > +                             .cra_flags = CRYPTO_ALG_TYPE_SKCIPHER |
> CRYPTO_ALG_ASYNC,
> > +                             .cra_blocksize = AES_BLOCK_SIZE,
> > +                             .cra_ctxsize = sizeof(struct tegra_aes_ctx),
> > +                             .cra_alignmask = 0xf,
> > +                             .cra_module = THIS_MODULE,
> > +                     },
> > +             }
> > +     }, {
> 
> OFB no longer exists in the kernel.  Please remove all traces of it from your driver.

Okay. Will remove and post a new version.

> 
> Also please ensure that yuor driver passes the extra fuzz tests.

Yes. It does pass the extra fuzz tests.


Regards,
Akhil

^ permalink raw reply

* Re: [PATCH v2 0/3] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Drive-by fixes
From: Ivan T. Ivanov @ 2024-04-02  8:58 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Stefan Wahren, Peter Robinson, Dave Stevenson, Naushir Patuck,
	Bartosz Golaszewski, Broadcom internal kernel review list,
	Conor Dooley, Florian Fainelli, Krzysztof Kozlowski,
	Linus Walleij, Nicolas Saenz Julienne, Ray Jui, Rob Herring,
	Scott Branden, linux-arm-kernel, devicetree, linux-rpi-kernel,
	u-boot
In-Reply-To: <20240327233700.GA21080@pendragon.ideasonboard.com>


Hi,

On 2024-03-28 01:37, Laurent Pinchart wrote:
> On Wed, Mar 27, 2024 at 07:49:38AM +0100, Stefan Wahren wrote:
>> Hi,
>> 
>> [add Peter and Ivan]
>> 
>> Am 26.03.24 um 20:58 schrieb Laurent Pinchart:
>> > Hello,
>> >
>> > This small series includes a few drive-by fixes for DT validation
>> > errors.
>> >
>> > The first patch has been posted previously in v1 ([1], and now addresses
>> > a small review comment. I think it's good to go.
>> >
>> > The next two patches address the same issue as "[PATCH 1/2] dt-bindings:
>> > arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties" ([2]),
>> > but this time with a (hopefully) correct approach. Patch 2/3 starts by
>> > fixing the raspberrypi-bcm2835-firmware driver, removing the need for DT
>> > properties that are specified in bcm2835-rpi.dtsi but not documented in
>> > the corresponding bindings. Patch 3/3 can then drop those properties,
>> > getting rid of the warnings.
>> 
>> since this series drops properties from the device tree, does anyone
>> have the chance to test it with a recent U-Boot?
> 
> I don't have U-Boot running with my RPi, so I would appreciate if
> someone could help :-)

Sorry for taking me so long to verify this.

I think on RPi U-Boot side we are fine. API used when accessing Mbox
device do not follow DM model and do not use DMA, but just access
device directly using this nice macros phys_to_bus/bus_to_phys.

I build new DTB files with this patch included and U-Boot build
from the latest sources. No obvious issues on RPi3 and RPi4.
Devices boot fine.

Regards,
Ivan

^ permalink raw reply

* Re: [PATCH v2 1/8] dt-bindings: clock: add Loongson-2K expand clock index
From: Huacai Chen @ 2024-04-02  8:58 UTC (permalink / raw)
  To: Binbin Zhou
  Cc: Binbin Zhou, Huacai Chen, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Yinbo Zhu,
	loongson-kernel, linux-clk, devicetree, Xuerui Wang, loongarch,
	Conor Dooley
In-Reply-To: <6c4eb239cbde62e7e1a8c647c945e128a0b78b2b.1711504700.git.zhoubinbin@loongson.cn>

Hi, Binbin,

On Mon, Apr 1, 2024 at 4:24 PM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
>
> In the new Loongson-2K family of SoCs, more clock indexes are needed,
> such as clock gates.
> The patch adds these clock indexes
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  include/dt-bindings/clock/loongson,ls2k-clk.h | 56 ++++++++++++-------
>  1 file changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/include/dt-bindings/clock/loongson,ls2k-clk.h b/include/dt-bindings/clock/loongson,ls2k-clk.h
> index 3bc4dfc193c2..4e6811eca8c6 100644
> --- a/include/dt-bindings/clock/loongson,ls2k-clk.h
> +++ b/include/dt-bindings/clock/loongson,ls2k-clk.h
> @@ -7,24 +7,42 @@
>  #ifndef __DT_BINDINGS_CLOCK_LOONGSON2_H
>  #define __DT_BINDINGS_CLOCK_LOONGSON2_H
>
> -#define LOONGSON2_REF_100M                             0
> -#define LOONGSON2_NODE_PLL                             1
> -#define LOONGSON2_DDR_PLL                              2
> -#define LOONGSON2_DC_PLL                               3
> -#define LOONGSON2_PIX0_PLL                             4
> -#define LOONGSON2_PIX1_PLL                             5
> -#define LOONGSON2_NODE_CLK                             6
> -#define LOONGSON2_HDA_CLK                              7
> -#define LOONGSON2_GPU_CLK                              8
> -#define LOONGSON2_DDR_CLK                              9
> -#define LOONGSON2_GMAC_CLK                             10
> -#define LOONGSON2_DC_CLK                               11
> -#define LOONGSON2_APB_CLK                              12
> -#define LOONGSON2_USB_CLK                              13
> -#define LOONGSON2_SATA_CLK                             14
> -#define LOONGSON2_PIX0_CLK                             15
> -#define LOONGSON2_PIX1_CLK                             16
> -#define LOONGSON2_BOOT_CLK                             17
> -#define LOONGSON2_CLK_END                              18
> +#define LOONGSON2_REF_100M     0
> +#define LOONGSON2_NODE_PLL     1
> +#define LOONGSON2_DDR_PLL      2
> +#define LOONGSON2_DC_PLL       3
> +#define LOONGSON2_PIX0_PLL     4
> +#define LOONGSON2_PIX1_PLL     5
> +#define LOONGSON2_NODE_CLK     6
> +#define LOONGSON2_HDA_CLK      7
> +#define LOONGSON2_GPU_CLK      8
> +#define LOONGSON2_DDR_CLK      9
> +#define LOONGSON2_GMAC_CLK     10
> +#define LOONGSON2_DC_CLK       11
> +#define LOONGSON2_APB_CLK      12
> +#define LOONGSON2_USB_CLK      13
> +#define LOONGSON2_SATA_CLK     14
> +#define LOONGSON2_PIX0_CLK     15
> +#define LOONGSON2_PIX1_CLK     16
> +#define LOONGSON2_BOOT_CLK     17
> +
> +/* Loongson-2K2000 */
This line should be removed, because the below definition is not
specific to Loongson-2K2000.

Huacai

> +#define LOONGSON2_OUT0_GATE    18
> +#define LOONGSON2_GMAC_GATE    19
> +#define LOONGSON2_RIO_GATE     20
> +#define LOONGSON2_DC_GATE      21
> +#define LOONGSON2_GPU_GATE     22
> +#define LOONGSON2_DDR_GATE     23
> +#define LOONGSON2_HDA_GATE     24
> +#define LOONGSON2_NODE_GATE    25
> +#define LOONGSON2_EMMC_GATE    26
> +#define LOONGSON2_PIX0_GATE    27
> +#define LOONGSON2_PIX1_GATE    28
> +#define LOONGSON2_OUT0_CLK     29
> +#define LOONGSON2_RIO_CLK      30
> +#define LOONGSON2_EMMC_CLK     31
> +#define LOONGSON2_DES_CLK      32
> +#define LOONGSON2_I2S_CLK      33
> +#define LOONGSON2_MISC_CLK     34
>
>  #endif
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v7 6/6] spmi: pmic-arb: Add multi bus support
From: Neil Armstrong @ 2024-04-02  8:55 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Srini Kandagatla, Johan Hovold,
	linux-kernel, linux-arm-kernel, linux-arm-msm, linux-mediatek,
	devicetree
In-Reply-To: <ZgvG08kfV2PvzLeb@linaro.org>

On 02/04/2024 10:50, Abel Vesa wrote:
> On 24-04-02 10:25:52, Neil Armstrong wrote:
>> Hi Abel,
>>
>> On 29/03/2024 19:54, Abel Vesa wrote:
>>> Starting with HW version 7, there are actually two separate buses
>>> (with two separate sets of wires). So add support for the second bus.
>>> The first platform that needs this support for the second bus is the
>>> Qualcomm X1 Elite, so add the compatible for it as well.
>>>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> ---
>>>    drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
>>>    1 file changed, 120 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
>>> index 19ff8665f3d9..56f2b3190d82 100644
>>> --- a/drivers/spmi/spmi-pmic-arb.c
>>> +++ b/drivers/spmi/spmi-pmic-arb.c
>>> @@ -13,6 +13,7 @@
>>>    #include <linux/kernel.h>
>>>    #include <linux/module.h>
>>>    #include <linux/of.h>
>>> +#include <linux/of_address.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/slab.h>
>>> @@ -95,6 +96,8 @@ enum pmic_arb_channel {
>>>    	PMIC_ARB_CHANNEL_OBS,
>>>    };
>>> +#define PMIC_ARB_MAX_BUSES		2
>>> +
>>>    /* Maximum number of support PMIC peripherals */
>>>    #define PMIC_ARB_MAX_PERIPHS		512
>>>    #define PMIC_ARB_MAX_PERIPHS_V7		1024
>>> @@ -148,6 +151,7 @@ struct spmi_pmic_arb;
>>>     * @min_apid:		minimum APID (used for bounding IRQ search)
>>>     * @max_apid:		maximum APID
>>>     * @irq:		PMIC ARB interrupt.
>>> + * @id:			unique ID of the bus
>>>     */
>>>    struct spmi_pmic_arb_bus {
>>>    	struct spmi_pmic_arb	*pmic_arb;
>>> @@ -165,6 +169,7 @@ struct spmi_pmic_arb_bus {
>>>    	u16			min_apid;
>>>    	u16			max_apid;
>>>    	int			irq;
>>> +	u8			id;
>>>    };
>>>    /**
>>> @@ -179,7 +184,8 @@ struct spmi_pmic_arb_bus {
>>>     * @ee:			the current Execution Environment
>>>     * @ver_ops:		version dependent operations.
>>>     * @max_periphs:	Number of elements in apid_data[]
>>> - * @bus:		per arbiter bus instance
>>> + * @buses:		per arbiter buses instances
>>> + * @buses_available:	number of buses registered
>>>     */
>>>    struct spmi_pmic_arb {
>>>    	void __iomem		*rd_base;
>>> @@ -191,7 +197,8 @@ struct spmi_pmic_arb {
>>>    	u8			ee;
>>>    	const struct pmic_arb_ver_ops *ver_ops;
>>>    	int			max_periphs;
>>> -	struct spmi_pmic_arb_bus *bus;
>>> +	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
>>> +	int			buses_available;
>>>    };
>>>    /**
>>> @@ -219,7 +226,7 @@ struct spmi_pmic_arb {
>>>    struct pmic_arb_ver_ops {
>>>    	const char *ver_str;
>>>    	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
>>> -	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
>>> +	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
>>>    	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
>>>    	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
>>>    	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
>>> @@ -308,8 +315,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
>>>    			}
>>>    			if (status & PMIC_ARB_STATUS_FAILURE) {
>>> -				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
>>> -					__func__, sid, addr, status);
>>> +				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
>>> +					__func__, sid, addr, status, offset);
>>>    				WARN_ON(1);
>>>    				return -EIO;
>>>    			}
>>> @@ -325,8 +332,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
>>>    		udelay(1);
>>>    	}
>>> -	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
>>> -		__func__, sid, addr, status);
>>> +	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
>>> +		__func__, bus->id, sid, addr, status);
>>>    	return -ETIMEDOUT;
>>>    }
>>> @@ -1005,11 +1012,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
>>>    	return 0;
>>>    }
>>> -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
>>> +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>>>    	u32 *mapping_table;
>>> +	if (index) {
>>> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
>>> +			index);
>>> +		return -EINVAL;
>>> +	}
>>
>> Shouldn't be here
>>
> 
> You're right. Since the DT bindings for HW < v7 doesn't allow multi bus
> support, this check is not needed. Will drop.
> 
>>> +
>>>    	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
>>>    				     sizeof(*mapping_table), GFP_KERNEL);
>>>    	if (!mapping_table)
>>> @@ -1252,11 +1265,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
>>>    	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
>>>    }
>>> -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
>>> +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>>>    	int ret;
>>> +	if (index) {
>>> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
>>> +			index);
>>> +		return -EINVAL;
>>> +	}
>>
>> Shouldn't be here
>>
> 
> Ditto.
> 
>>> +
>>>    	bus->base_apid = 0;
>>>    	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>>>    					   PMIC_ARB_FEATURES_PERIPH_MASK;
>>> @@ -1328,6 +1347,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
>>>    	return pmic_arb_get_obsrvr_chnls_v2(pdev);
>>>    }
>>> +/*
>>> + * Only v7 supports 2 buses. Each bus will get a different apid count, read
>>> + * from different registers.
>>> + */
>>> +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
>>> +{
>>> +	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>>> +	int ret;
>>> +
>>> +	if (index == 0) {
>>> +		bus->base_apid = 0;
>>> +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>>> +						   PMIC_ARB_FEATURES_PERIPH_MASK;
>>> +	} else if (index == 1) {
>>> +		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>>> +						  PMIC_ARB_FEATURES_PERIPH_MASK;
>>> +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
>>> +						   PMIC_ARB_FEATURES_PERIPH_MASK;
>>> +	} else {
>>> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
>>> +			bus->id);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
>>> +		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
>>> +			bus->base_apid + bus->apid_count);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	ret = pmic_arb_init_apid_min_max(bus);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = pmic_arb_read_apid_map_v5(bus);
>>> +	if (ret) {
>>> +		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
>>> +			ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>
>> Shouldn't be here
>>
> 
> Since the apid base and count are different between buses and since v7
> supports 2 buses, we need a v7 specific init_apid. So this one is
> needed.


I know, all those were wrongly removed in patch 5, just let them in place.

Neil

> 
>>> +
>>>    /*
>>>     * v7 offset per ee and per apid for observer channels and per apid for
>>>     * read/write channels.
>>> @@ -1580,7 +1643,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
>>>    static const struct pmic_arb_ver_ops pmic_arb_v7 = {
>>>    	.ver_str		= "v7",
>>>    	.get_core_resources	= pmic_arb_get_core_resources_v7,
>>> -	.init_apid		= pmic_arb_init_apid_v5,
>>> +	.init_apid		= pmic_arb_init_apid_v7,
>>
>> Shouldn't be here
>>
> 
> See above.
> 
>>>    	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
>>>    	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
>>>    	.offset			= pmic_arb_offset_v7,
>>> @@ -1604,6 +1667,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    				  struct device_node *node,
>>>    				  struct spmi_pmic_arb *pmic_arb)
>>>    {
>>> +	int bus_index = pmic_arb->buses_available;
>>>    	struct spmi_pmic_arb_bus *bus;
>>>    	struct device *dev = &pdev->dev;
>>>    	struct spmi_controller *ctrl;
>>> @@ -1622,7 +1686,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    	bus = spmi_controller_get_drvdata(ctrl);
>>> -	pmic_arb->bus = bus;
>>> +	pmic_arb->buses[bus_index] = bus;
>>>    	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
>>>    					 sizeof(*bus->ppid_to_apid),
>>> @@ -1665,12 +1729,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    	bus->cnfg = cnfg;
>>>    	bus->irq = irq;
>>>    	bus->spmic = ctrl;
>>> +	bus->id = bus_index;
>>> -	ret = pmic_arb->ver_ops->init_apid(bus);
>>> +	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
>>>    	if (ret)
>>>    		return ret;
>>> -	dev_dbg(&pdev->dev, "adding irq domain\n");
>>> +	dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
>>>    	bus->domain = irq_domain_add_tree(dev->of_node,
>>>    					  &pmic_arb_irq_domain_ops, bus);
>>> @@ -1683,14 +1748,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    					 pmic_arb_chained_irq, bus);
>>>    	ctrl->dev.of_node = node;
>>> +	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
>>>    	ret = devm_spmi_controller_add(dev, ctrl);
>>>    	if (ret)
>>>    		return ret;
>>> +	pmic_arb->buses_available++;
>>> +
>>>    	return 0;
>>>    }
>>> +static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
>>> +					struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct device_node *node = dev->of_node;
>>> +	struct device_node *child;
>>> +	int ret;
>>> +
>>> +	/* legacy mode doesn't provide child node for the bus */
>>> +	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
>>> +		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
>>> +
>>> +	for_each_available_child_of_node(node, child) {
>>> +		if (of_node_name_eq(child, "spmi")) {
>>> +			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
>>> +		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
>>> +
>>> +		irq_set_chained_handler_and_data(bus->irq,
>>> +						 NULL, NULL);
>>> +		irq_domain_remove(bus->domain);
>>> +	}
>>> +}
>>> +
>>>    static int spmi_pmic_arb_probe(struct platform_device *pdev)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb;
>>> @@ -1761,21 +1865,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>>>    	pmic_arb->ee = ee;
>>> -	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
>>> +	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
>>>    }
>>>    static void spmi_pmic_arb_remove(struct platform_device *pdev)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
>>> -	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
>>> -	irq_set_chained_handler_and_data(bus->irq,
>>> -					 NULL, NULL);
>>> -	irq_domain_remove(bus->domain);
>>> +	spmi_pmic_arb_deregister_buses(pmic_arb);
>>>    }
>>>    static const struct of_device_id spmi_pmic_arb_match_table[] = {
>>>    	{ .compatible = "qcom,spmi-pmic-arb", },
>>> +	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
>>>    	{},
>>>    };
>>>    MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
>>>
>>
>> With issues fixed, it looks fine.
>>
>> Thanks,
>> Neil


^ permalink raw reply

* Re: [PATCH v7 6/6] spmi: pmic-arb: Add multi bus support
From: Abel Vesa @ 2024-04-02  8:50 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Srini Kandagatla, Johan Hovold,
	linux-kernel, linux-arm-kernel, linux-arm-msm, linux-mediatek,
	devicetree
In-Reply-To: <871bc3f2-d4c3-4c83-ad0c-04c65ed15598@linaro.org>

On 24-04-02 10:25:52, Neil Armstrong wrote:
> Hi Abel,
> 
> On 29/03/2024 19:54, Abel Vesa wrote:
> > Starting with HW version 7, there are actually two separate buses
> > (with two separate sets of wires). So add support for the second bus.
> > The first platform that needs this support for the second bus is the
> > Qualcomm X1 Elite, so add the compatible for it as well.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >   drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
> >   1 file changed, 120 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> > index 19ff8665f3d9..56f2b3190d82 100644
> > --- a/drivers/spmi/spmi-pmic-arb.c
> > +++ b/drivers/spmi/spmi-pmic-arb.c
> > @@ -13,6 +13,7 @@
> >   #include <linux/kernel.h>
> >   #include <linux/module.h>
> >   #include <linux/of.h>
> > +#include <linux/of_address.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/slab.h>
> > @@ -95,6 +96,8 @@ enum pmic_arb_channel {
> >   	PMIC_ARB_CHANNEL_OBS,
> >   };
> > +#define PMIC_ARB_MAX_BUSES		2
> > +
> >   /* Maximum number of support PMIC peripherals */
> >   #define PMIC_ARB_MAX_PERIPHS		512
> >   #define PMIC_ARB_MAX_PERIPHS_V7		1024
> > @@ -148,6 +151,7 @@ struct spmi_pmic_arb;
> >    * @min_apid:		minimum APID (used for bounding IRQ search)
> >    * @max_apid:		maximum APID
> >    * @irq:		PMIC ARB interrupt.
> > + * @id:			unique ID of the bus
> >    */
> >   struct spmi_pmic_arb_bus {
> >   	struct spmi_pmic_arb	*pmic_arb;
> > @@ -165,6 +169,7 @@ struct spmi_pmic_arb_bus {
> >   	u16			min_apid;
> >   	u16			max_apid;
> >   	int			irq;
> > +	u8			id;
> >   };
> >   /**
> > @@ -179,7 +184,8 @@ struct spmi_pmic_arb_bus {
> >    * @ee:			the current Execution Environment
> >    * @ver_ops:		version dependent operations.
> >    * @max_periphs:	Number of elements in apid_data[]
> > - * @bus:		per arbiter bus instance
> > + * @buses:		per arbiter buses instances
> > + * @buses_available:	number of buses registered
> >    */
> >   struct spmi_pmic_arb {
> >   	void __iomem		*rd_base;
> > @@ -191,7 +197,8 @@ struct spmi_pmic_arb {
> >   	u8			ee;
> >   	const struct pmic_arb_ver_ops *ver_ops;
> >   	int			max_periphs;
> > -	struct spmi_pmic_arb_bus *bus;
> > +	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
> > +	int			buses_available;
> >   };
> >   /**
> > @@ -219,7 +226,7 @@ struct spmi_pmic_arb {
> >   struct pmic_arb_ver_ops {
> >   	const char *ver_str;
> >   	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
> > -	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
> > +	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
> >   	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
> >   	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
> >   	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> > @@ -308,8 +315,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> >   			}
> >   			if (status & PMIC_ARB_STATUS_FAILURE) {
> > -				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
> > -					__func__, sid, addr, status);
> > +				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
> > +					__func__, sid, addr, status, offset);
> >   				WARN_ON(1);
> >   				return -EIO;
> >   			}
> > @@ -325,8 +332,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> >   		udelay(1);
> >   	}
> > -	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
> > -		__func__, sid, addr, status);
> > +	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
> > +		__func__, bus->id, sid, addr, status);
> >   	return -ETIMEDOUT;
> >   }
> > @@ -1005,11 +1012,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
> >   	return 0;
> >   }
> > -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
> > +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> >   	u32 *mapping_table;
> > +	if (index) {
> > +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> > +			index);
> > +		return -EINVAL;
> > +	}
> 
> Shouldn't be here
> 

You're right. Since the DT bindings for HW < v7 doesn't allow multi bus
support, this check is not needed. Will drop.

> > +
> >   	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
> >   				     sizeof(*mapping_table), GFP_KERNEL);
> >   	if (!mapping_table)
> > @@ -1252,11 +1265,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> >   	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
> >   }
> > -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
> > +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> >   	int ret;
> > +	if (index) {
> > +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> > +			index);
> > +		return -EINVAL;
> > +	}
> 
> Shouldn't be here
> 

Ditto.

> > +
> >   	bus->base_apid = 0;
> >   	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> >   					   PMIC_ARB_FEATURES_PERIPH_MASK;
> > @@ -1328,6 +1347,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
> >   	return pmic_arb_get_obsrvr_chnls_v2(pdev);
> >   }
> > +/*
> > + * Only v7 supports 2 buses. Each bus will get a different apid count, read
> > + * from different registers.
> > + */
> > +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
> > +{
> > +	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> > +	int ret;
> > +
> > +	if (index == 0) {
> > +		bus->base_apid = 0;
> > +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> > +						   PMIC_ARB_FEATURES_PERIPH_MASK;
> > +	} else if (index == 1) {
> > +		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> > +						  PMIC_ARB_FEATURES_PERIPH_MASK;
> > +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
> > +						   PMIC_ARB_FEATURES_PERIPH_MASK;
> > +	} else {
> > +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> > +			bus->id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
> > +		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
> > +			bus->base_apid + bus->apid_count);
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = pmic_arb_init_apid_min_max(bus);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = pmic_arb_read_apid_map_v5(bus);
> > +	if (ret) {
> > +		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> Shouldn't be here
> 

Since the apid base and count are different between buses and since v7
supports 2 buses, we need a v7 specific init_apid. So this one is
needed.

> > +
> >   /*
> >    * v7 offset per ee and per apid for observer channels and per apid for
> >    * read/write channels.
> > @@ -1580,7 +1643,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
> >   static const struct pmic_arb_ver_ops pmic_arb_v7 = {
> >   	.ver_str		= "v7",
> >   	.get_core_resources	= pmic_arb_get_core_resources_v7,
> > -	.init_apid		= pmic_arb_init_apid_v5,
> > +	.init_apid		= pmic_arb_init_apid_v7,
> 
> Shouldn't be here
> 

See above.

> >   	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
> >   	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
> >   	.offset			= pmic_arb_offset_v7,
> > @@ -1604,6 +1667,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   				  struct device_node *node,
> >   				  struct spmi_pmic_arb *pmic_arb)
> >   {
> > +	int bus_index = pmic_arb->buses_available;
> >   	struct spmi_pmic_arb_bus *bus;
> >   	struct device *dev = &pdev->dev;
> >   	struct spmi_controller *ctrl;
> > @@ -1622,7 +1686,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   	bus = spmi_controller_get_drvdata(ctrl);
> > -	pmic_arb->bus = bus;
> > +	pmic_arb->buses[bus_index] = bus;
> >   	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
> >   					 sizeof(*bus->ppid_to_apid),
> > @@ -1665,12 +1729,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   	bus->cnfg = cnfg;
> >   	bus->irq = irq;
> >   	bus->spmic = ctrl;
> > +	bus->id = bus_index;
> > -	ret = pmic_arb->ver_ops->init_apid(bus);
> > +	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
> >   	if (ret)
> >   		return ret;
> > -	dev_dbg(&pdev->dev, "adding irq domain\n");
> > +	dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
> >   	bus->domain = irq_domain_add_tree(dev->of_node,
> >   					  &pmic_arb_irq_domain_ops, bus);
> > @@ -1683,14 +1748,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   					 pmic_arb_chained_irq, bus);
> >   	ctrl->dev.of_node = node;
> > +	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
> >   	ret = devm_spmi_controller_add(dev, ctrl);
> >   	if (ret)
> >   		return ret;
> > +	pmic_arb->buses_available++;
> > +
> >   	return 0;
> >   }
> > +static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
> > +					struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *node = dev->of_node;
> > +	struct device_node *child;
> > +	int ret;
> > +
> > +	/* legacy mode doesn't provide child node for the bus */
> > +	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
> > +		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
> > +
> > +	for_each_available_child_of_node(node, child) {
> > +		if (of_node_name_eq(child, "spmi")) {
> > +			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
> > +		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
> > +
> > +		irq_set_chained_handler_and_data(bus->irq,
> > +						 NULL, NULL);
> > +		irq_domain_remove(bus->domain);
> > +	}
> > +}
> > +
> >   static int spmi_pmic_arb_probe(struct platform_device *pdev)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb;
> > @@ -1761,21 +1865,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> >   	pmic_arb->ee = ee;
> > -	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
> > +	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
> >   }
> >   static void spmi_pmic_arb_remove(struct platform_device *pdev)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> > -	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
> > -	irq_set_chained_handler_and_data(bus->irq,
> > -					 NULL, NULL);
> > -	irq_domain_remove(bus->domain);
> > +	spmi_pmic_arb_deregister_buses(pmic_arb);
> >   }
> >   static const struct of_device_id spmi_pmic_arb_match_table[] = {
> >   	{ .compatible = "qcom,spmi-pmic-arb", },
> > +	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
> >   	{},
> >   };
> >   MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
> > 
> 
> With issues fixed, it looks fine.
> 
> Thanks,
> Neil

^ permalink raw reply

* Re: [PATCH v10 08/11] arm64: dts: imx93: add usb nodes
From: Shawn Guo @ 2024-04-02  8:39 UTC (permalink / raw)
  To: Xu Yang
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, shawnguo, conor+dt,
	s.hauer, kernel, festevam, linux-imx, peter.chen, jun.li,
	linux-usb, devicetree, linux-arm-kernel, imx, linux-kernel
In-Reply-To: <20240321081439.541799-8-xu.yang_2@nxp.com>

On Thu, Mar 21, 2024 at 04:14:36PM +0800, Xu Yang wrote:
> There are 2 USB controllers on i.MX93. Add them.
> 
> Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa9352LA/CA
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - fix format as suggested by Alexander
>  - change compatible from fsl,imx8mm-usb to fsl,imx93-usb
> Changes in v3:
>  - replace deprecated fsl,usbphy with phys as suggested by Alexander
>  - reorder nodes
> Changes in v4:
>  - fix the alignment
> Changes in v5:
>  - rename usb_wakeup_clk to usb_wakeup
> Changes in v6:
>  - rename usb_ctrl_root_clk to usb_ctrl_root
> Changes in v7:
>  - no changes
> Changes in v8:
>  - no changes
> Changes in v9:
>  - no changes
> Changes in v10:
>  - no changes
> ---
>  arch/arm64/boot/dts/freescale/imx93.dtsi | 58 ++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
> index 8f2e7c42ad6e..4a7efccb4f67 100644
> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> @@ -183,6 +183,20 @@ mqs2: mqs2 {
>  		status = "disabled";
>  	};
>  
> +	usbphynop1: usbphynop1 {
> +		compatible = "usb-nop-xceiv";
> +		#phy-cells = <0>;
> +		clocks = <&clk IMX93_CLK_USB_PHY_BURUNIN>;
> +		clock-names = "main_clk";
> +	};
> +
> +	usbphynop2: usbphynop2 {
> +		compatible = "usb-nop-xceiv";
> +		#phy-cells = <0>;
> +		clocks = <&clk IMX93_CLK_USB_PHY_BURUNIN>;
> +		clock-names = "main_clk";
> +	};
> +
>  	soc@0 {
>  		compatible = "simple-bus";
>  		#address-cells = <1>;
> @@ -1167,6 +1181,50 @@ media_blk_ctrl: system-controller@4ac10000 {
>  			status = "disabled";
>  		};
>  
> +		usbotg1: usb@4c100000 {
> +			compatible = "fsl,imx93-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
> +			reg = <0x4c100000 0x200>;
> +			interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk IMX93_CLK_USB_CONTROLLER_GATE>,
> +				 <&clk IMX93_CLK_HSIO_32K_GATE>;
> +			clock-names = "usb_ctrl_root", "usb_wakeup";
> +			assigned-clocks = <&clk IMX93_CLK_HSIO>;
> +			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
> +			assigned-clock-rates = <133000000>;
> +			phys = <&usbphynop1>;
> +			fsl,usbmisc = <&usbmisc1 0>;
> +			status = "disabled";
> +		};
> +
> +		usbmisc1: usbmisc@4c100200 {
> +			compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc",
> +				     "fsl,imx6q-usbmisc";
> +			reg = <0x4c100200 0x200>;
> +			#index-cells = <1>;

Do we still need this '#index-cells' property?  I see it's being marked
as deprecated in bindings doc.

Shawn

> +		};
> +
> +		usbotg2: usb@4c200000 {
> +			compatible = "fsl,imx93-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
> +			reg = <0x4c200000 0x200>;
> +			interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk IMX93_CLK_USB_CONTROLLER_GATE>,
> +				 <&clk IMX93_CLK_HSIO_32K_GATE>;
> +			clock-names = "usb_ctrl_root", "usb_wakeup";
> +			assigned-clocks = <&clk IMX93_CLK_HSIO>;
> +			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
> +			assigned-clock-rates = <133000000>;
> +			phys = <&usbphynop2>;
> +			fsl,usbmisc = <&usbmisc2 0>;
> +			status = "disabled";
> +		};
> +
> +		usbmisc2: usbmisc@4c200200 {
> +			compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc",
> +				     "fsl,imx6q-usbmisc";
> +			reg = <0x4c200200 0x200>;
> +			#index-cells = <1>;
> +		};
> +
>  		ddr-pmu@4e300dc0 {
>  			compatible = "fsl,imx93-ddr-pmu";
>  			reg = <0x4e300dc0 0x200>;
> -- 
> 2.34.1
> 


^ permalink raw reply

* Re: [PATCH v10 03/11] arm64: dts: imx8ulp-evk: enable usb nodes and add ptn5150 nodes
From: Shawn Guo @ 2024-04-02  8:35 UTC (permalink / raw)
  To: Xu Yang
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, shawnguo, conor+dt,
	s.hauer, kernel, festevam, linux-imx, peter.chen, jun.li,
	linux-usb, devicetree, linux-arm-kernel, imx, linux-kernel
In-Reply-To: <20240321081439.541799-3-xu.yang_2@nxp.com>

On Thu, Mar 21, 2024 at 04:14:31PM +0800, Xu Yang wrote:
> Enable 2 USB nodes and add 2 PTN5150 nodes on i.MX8ULP evk board.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - fix format as suggusted by Fabio
>  - add PTN5150 nodes
> Changes in v3:
>  - no changes
> Changes in v4:
>  - no changes
> Changes in v5:
>  - no changes
> Changes in v6:
>  - no changes
> Changes in v7:
>  - no changes
> Changes in v8:
>  - no changes
> Changes in v9:
>  - no changes
> Changes in v10:
>  - no changes
> ---
>  arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> index 69dd8e31027c..bf418af31039 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> @@ -133,6 +133,64 @@ pcal6408: gpio@21 {
>  		gpio-controller;
>  		#gpio-cells = <2>;
>  	};
> +
> +	ptn5150_1: typec@1d {

Could you sort devices in unit-address?

> +		compatible = "nxp,ptn5150";
> +		reg = <0x1d>;
> +		int-gpios = <&gpiof 3 IRQ_TYPE_EDGE_FALLING>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_typec1>;
> +		status = "disabled";
> +	};
> +
> +	ptn5150_2: typec@3d {
> +		compatible = "nxp,ptn5150";
> +		reg = <0x3d>;
> +		int-gpios = <&gpiof 5 IRQ_TYPE_EDGE_FALLING>;
> +			pinctrl-names = "default";

Broken indent?

Shawn

> +		pinctrl-0 = <&pinctrl_typec2>;
> +		status = "disabled";
> +	};
> +};
> +
> +&usbotg1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usb1>;
> +	dr_mode = "otg";
> +	hnp-disable;
> +	srp-disable;
> +	adp-disable;
> +	over-current-active-low;
> +	status = "okay";
> +};
> +
> +&usbphy1 {
> +	fsl,tx-d-cal = <110>;
> +	status = "okay";
> +};
> +
> +&usbmisc1 {
> +	status = "okay";
> +};
> +
> +&usbotg2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usb2>;
> +	dr_mode = "otg";
> +	hnp-disable;
> +	srp-disable;
> +	adp-disable;
> +	over-current-active-low;
> +	status = "okay";
> +};
> +
> +&usbphy2 {
> +	fsl,tx-d-cal = <110>;
> +	status = "okay";
> +};
> +
> +&usbmisc2 {
> +	status = "okay";
>  };
>  
>  &usdhc0 {
> @@ -224,6 +282,32 @@ MX8ULP_PAD_PTE13__LPI2C7_SDA	0x20
>  		>;
>  	};
>  
> +	pinctrl_typec1: typec1grp {
> +		fsl,pins = <
> +			MX8ULP_PAD_PTF3__PTF3           0x3
> +		>;
> +	};
> +
> +	pinctrl_typec2: typec2grp {
> +		fsl,pins = <
> +			MX8ULP_PAD_PTF5__PTF5           0x3
> +		>;
> +	};
> +
> +	pinctrl_usb1: usb1grp {
> +		fsl,pins = <
> +			MX8ULP_PAD_PTF2__USB0_ID	0x10003
> +			MX8ULP_PAD_PTF4__USB0_OC	0x10003
> +		>;
> +	};
> +
> +	pinctrl_usb2: usb2grp {
> +		fsl,pins = <
> +			MX8ULP_PAD_PTD23__USB1_ID	0x10003
> +			MX8ULP_PAD_PTF6__USB1_OC	0x10003
> +		>;
> +	};
> +
>  	pinctrl_usdhc0: usdhc0grp {
>  		fsl,pins = <
>  			MX8ULP_PAD_PTD1__SDHC0_CMD	0x3
> -- 
> 2.34.1
> 


^ permalink raw reply

* Re: [PATCH v10 02/11] arm64: dts: imx8ulp: add usb nodes
From: Shawn Guo @ 2024-04-02  8:31 UTC (permalink / raw)
  To: Xu Yang
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, shawnguo, conor+dt,
	s.hauer, kernel, festevam, linux-imx, peter.chen, jun.li,
	linux-usb, devicetree, linux-arm-kernel, imx, linux-kernel
In-Reply-To: <20240321081439.541799-2-xu.yang_2@nxp.com>

On Thu, Mar 21, 2024 at 04:14:30PM +0800, Xu Yang wrote:
> Add USB nodes on i.MX8ULP platform which has 2 USB controllers.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - no changes
> Changes in v3:
>  - no changes
> Changes in v4:
>  - no changes
> Changes in v5:
>  - no changes
> Changes in v6:
>  - drop usbphy aliases
> Changes in v7:
>  - no changes
> Changes in v8:
>  - no changes
> Changes in v9:
>  - no changes
> Changes in v10:
>  - no changes
> ---
>  arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index c4a0082f30d3..7da9461a5745 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -472,6 +472,68 @@ usdhc2: mmc@298f0000 {
>  				status = "disabled";
>  			};
>  
> +			usbotg1: usb@29900000 {
> +				compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
> +				reg = <0x29900000 0x200>;
> +				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&pcc4 IMX8ULP_CLK_USB0>;
> +				power-domains = <&scmi_devpd IMX8ULP_PD_USB0>;
> +				phys = <&usbphy1>;
> +				fsl,usbmisc = <&usbmisc1 0>;
> +				ahb-burst-config = <0x0>;
> +				tx-burst-size-dword = <0x8>;
> +				rx-burst-size-dword = <0x8>;
> +				status = "disabled";
> +			};
> +
> +			usbmisc1: usbmisc@29900200 {
> +				compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
> +						"fsl,imx6q-usbmisc";
> +				#index-cells = <1>;
> +				reg = <0x29900200 0x200>;

Could you move 'reg' above so that it's after compatible?

> +				status = "disabled";
> +			};
> +
> +			usbphy1: usb-phy@29910000 {
> +				compatible = "fsl,imx8ulp-usbphy", "fsl,imx7ulp-usbphy";
> +				reg = <0x29910000 0x10000>;
> +				interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&pcc4 IMX8ULP_CLK_USB0_PHY>;
> +				#phy-cells = <0>;
> +				status = "disabled";
> +			};
> +
> +			usbotg2: usb@29920000 {
> +				compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
> +				reg = <0x29920000 0x200>;
> +				interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&pcc4 IMX8ULP_CLK_USB1>;
> +				power-domains = <&scmi_devpd IMX8ULP_PD_USDHC2_USB1>;
> +				phys = <&usbphy2>;
> +				fsl,usbmisc = <&usbmisc2 0>;
> +				ahb-burst-config = <0x0>;
> +				tx-burst-size-dword = <0x8>;
> +				rx-burst-size-dword = <0x8>;
> +				status = "disabled";
> +			};
> +
> +			usbmisc2: usbmisc@29920200 {
> +				compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
> +						"fsl,imx6q-usbmisc";
> +				#index-cells = <1>;
> +				reg = <0x29920200 0x200>;

Ditto

Shawn

> +				status = "disabled";
> +			};
> +
> +			usbphy2: usb-phy@29930000 {
> +				compatible = "fsl,imx8ulp-usbphy", "fsl,imx7ulp-usbphy";
> +				reg = <0x29930000 0x10000>;
> +				interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&pcc4 IMX8ULP_CLK_USB1_PHY>;
> +				#phy-cells = <0>;
> +				status = "disabled";
> +			};
> +
>  			fec: ethernet@29950000 {
>  				compatible = "fsl,imx8ulp-fec", "fsl,imx6ul-fec", "fsl,imx6q-fec";
>  				reg = <0x29950000 0x10000>;
> -- 
> 2.34.1
> 


^ permalink raw reply

* Re: [PATCH 0/3] media: i2c: Add imx283 camera sensor driver
From: Umang Jain @ 2024-04-02  8:31 UTC (permalink / raw)
  To: Kieran Bingham, Mauro Carvalho Chehab, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Sakari Ailus
  Cc: linux-media, devicetree, linux-arm-kernel, linux-kernel,
	Rob Herring, Laurent Pinchart
In-Reply-To: <20240402-kernel-name-extraversion-v1-0-57bb38de841b@ideasonboard.com>

Hi all,

PLease ignore the series, I was testing/learning the b4 tool.

I did pass --offline-mode but it has sent the patches anyway :-//

On 02/04/24 1:59 pm, Umang Jain wrote:
> Add a v4l2 subdevice driver for the Sony IMX283 image sensor.
>    
> The IMX283 is a 20MP Diagonal 15.86 mm (Type 1) CMOS Image Sensor with
> Square Pixel for Color Cameras.
>      
> The following features are supported:
> - Manual exposure an gain control support
> - vblank/hblank/link freq control support
> - Test pattern support control
> - Arbitrary horizontal and vertical cropping
> - Supported resolution:
>     - 5472x3648 @ 20fps (SRGGB12)
>     - 5472x3648 @ 25fps (SRGGB10)
>     - 2736x1824 @ 50fps (SRGGB12)
>
> The driver is tested on mainline branch v6.8-rc2 on IMX8MP Debix-SOM-A.
> Additional testing has been done on RPi5 with the downstream BSP.
>
> Changes in v4:
> - fix 32-bit build error around u64 divisions (use do_div)
> - Fix hmax default and minimum values
>
> Changes in v3:
> - fix headers includes
> - Improve #define(s) readability
> - Drop __func__ from error logs
> - Use HZ_PER_MHZ instead of MEGA
> - mdsel* variables should be u8
> - Use container_of_const() instead of container_of()
> - Use clamp() used of clamp_t variant
> - Use streams API imx283_{enable|disable}_streams (**NOTE**)
> - Properly fix PM runtime handling
>    (pm_ptr(), DEFINE_RUNTIME_DEV_PM_OPS,
>     imx283_runtime_suspend, imx283_runtime_resume)
> - Fix format modifiers, signed-ness at various places
>
> changes in v2 (summary):
> - Use u32 wherever possible
> - Use MEGA macro instead of self defined MHZ() macro
> - Properly refine regs using CCI
> - Drop tracking of current mode. Shifted to infer from active state directly.
>    (Laurent's review)
> - Cont. from above: Pass the struct imx283_mode to functions whereever required.
> - Remove unused comments
> - Remove custom mutex. Use control handler one instead.
> - Drop imx283_reset_colorspace() and inline
> - Set colorspace field properly (drop _DEFAULTS)
> - Use __maybe_unused for imx283_power_on() and imx283_power_off()
> - Store controls  v4l2_ctrl handles for those required, not all.
> - Drop imx283_free_controls(). Use v4l2_ctrl_handler_free
> - fix reset-gpios handling and add it to DT schema
> - fix data-lanes property in DT schema
> - fix IMX283 Kconfig
> - Remove unused macros
> - Alphabetical case consistency
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> Kieran Bingham (1):
>        media: i2c: Add imx283 camera sensor driver
>
> Umang Jain (2):
>        media: dt-bindings: media: Add bindings for IMX283
>        fixups
>
>   .../devicetree/bindings/media/i2c/sony,imx283.yaml |  107 ++
>   MAINTAINERS                                        |    9 +
>   drivers/media/i2c/Kconfig                          |   10 +
>   drivers/media/i2c/Makefile                         |    1 +
>   drivers/media/i2c/imx283.c                         | 1605 ++++++++++++++++++++
>   5 files changed, 1732 insertions(+)
> ---
> base-commit: 54ee11761885407056f4ca60309739e2db6b02dc
> change-id: 20240402-kernel-name-extraversion-2b08d441e08c
>
> Best regards,


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox