* [PATCH v2 2/3] clk: samsung: gs101: propagate PERIC1 USI SPI clock rate
From: Tudor Ambarus @ 2024-03-26 17:28 UTC (permalink / raw)
To: peter.griffin, krzysztof.kozlowski
Cc: alim.akhtar, linux-arm-kernel, linux-samsung-soc, linux-kernel,
andre.draszik, willmcvicker, kernel-team, s.nawrocki, cw00.choi,
mturquette, sboyd, semen.protsenko, linux-clk, jaewon02.kim,
Tudor Ambarus
In-Reply-To: <20240326172813.801470-1-tudor.ambarus@linaro.org>
When SPI transfer is being prepared, the spi-s3c64xx driver will call
clk_set_rate() to change the rate of SPI source clock (IPCLK). But IPCLK
is a gate (leaf) clock, so it must propagate the rate change up the
clock tree, so that corresponding MUX/DIV clocks can actually change
their values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
all USI instances in GS101 PERIC1: USI{0, 9, 10, 11, 12, 13}. This change
involves the following clocks:
PERIC1 USI*:
Clock Div range MUX Selection
-------------------------------------------------------------------
gout_peric1_peric1_top0_ipclk_* - -
dout_peric1_usi*_usi /1..16 -
mout_peric1_usi*_usi_user - {24.5 MHz, 400 MHz}
With input clock of 400 MHz this scheme provides the following IPCLK
rate range, for each USI block:
PERIC1 USI*: 1.5 MHz ... 400 MHz
Accounting for internal /4 divider in SPI blocks, and because the max
SPI frequency is limited at 50 MHz, it gives us next SPI SCK rates:
PERIC1 USI_SPI*: 384 KHz ... 49.9 MHz
Which shall be fine for the applications of the SPI bus.
Note that with this we allow the reparenting of the MUX_USIx clocks to
OSCCLK. Each instance of the USI IP has its own MUX_USI clock, thus the
reparenting of a MUX_USI clock corresponds to a single instance of the
USI IP. The datasheet mentions OSCCLK just in the low-power mode
context, but the downstream driver reparents too the MUX_USI clocks to
OSCCLK. Follow the downstream driver and do the same.
Fixes: 63b4bd1259d9 ("clk: samsung: gs101: add support for cmu_peric1")
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/clk/samsung/clk-gs101.c | 90 ++++++++++++++++++---------------
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index d065e343a85d..ddf2d57eed68 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -3230,47 +3230,53 @@ static const struct samsung_mux_clock peric1_mux_clks[] __initconst = {
MUX(CLK_MOUT_PERIC1_I3C_USER,
"mout_peric1_i3c_user", mout_peric1_nonbususer_p,
PLL_CON0_MUX_CLKCMU_PERIC1_I3C_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI0_USI_USER,
- "mout_peric1_usi0_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI0_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI10_USI_USER,
- "mout_peric1_usi10_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI10_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI11_USI_USER,
- "mout_peric1_usi11_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI11_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI12_USI_USER,
- "mout_peric1_usi12_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI12_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI13_USI_USER,
- "mout_peric1_usi13_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI13_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC1_USI9_USI_USER,
- "mout_peric1_usi9_usi_user", mout_peric1_nonbususer_p,
- PLL_CON0_MUX_CLKCMU_PERIC1_USI9_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI0_USI_USER,
+ "mout_peric1_usi0_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI0_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI10_USI_USER,
+ "mout_peric1_usi10_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI10_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI11_USI_USER,
+ "mout_peric1_usi11_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI11_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI12_USI_USER,
+ "mout_peric1_usi12_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI12_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI13_USI_USER,
+ "mout_peric1_usi13_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI13_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC1_USI9_USI_USER,
+ "mout_peric1_usi9_usi_user", mout_peric1_nonbususer_p,
+ PLL_CON0_MUX_CLKCMU_PERIC1_USI9_USI_USER, 4, 1),
};
static const struct samsung_div_clock peric1_div_clks[] __initconst = {
DIV(CLK_DOUT_PERIC1_I3C, "dout_peric1_i3c", "mout_peric1_i3c_user",
CLK_CON_DIV_DIV_CLK_PERIC1_I3C, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI0_USI,
- "dout_peric1_usi0_usi", "mout_peric1_usi0_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI0_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI10_USI,
- "dout_peric1_usi10_usi", "mout_peric1_usi10_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI10_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI11_USI,
- "dout_peric1_usi11_usi", "mout_peric1_usi11_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI11_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI12_USI,
- "dout_peric1_usi12_usi", "mout_peric1_usi12_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI12_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI13_USI,
- "dout_peric1_usi13_usi", "mout_peric1_usi13_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI13_USI, 0, 4),
- DIV(CLK_DOUT_PERIC1_USI9_USI,
- "dout_peric1_usi9_usi", "mout_peric1_usi9_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC1_USI9_USI, 0, 4),
+ DIV_F(CLK_DOUT_PERIC1_USI0_USI,
+ "dout_peric1_usi0_usi", "mout_peric1_usi0_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI0_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI10_USI,
+ "dout_peric1_usi10_usi", "mout_peric1_usi10_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI10_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI11_USI,
+ "dout_peric1_usi11_usi", "mout_peric1_usi11_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI11_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI12_USI,
+ "dout_peric1_usi12_usi", "mout_peric1_usi12_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI12_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI13_USI,
+ "dout_peric1_usi13_usi", "mout_peric1_usi13_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI13_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC1_USI9_USI,
+ "dout_peric1_usi9_usi", "mout_peric1_usi9_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC1_USI9_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
@@ -3305,27 +3311,27 @@ static const struct samsung_gate_clock peric1_gate_clks[] __initconst = {
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_1,
"gout_peric1_peric1_top0_ipclk_1", "dout_peric1_usi0_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_1,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_2,
"gout_peric1_peric1_top0_ipclk_2", "dout_peric1_usi9_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_2,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_3,
"gout_peric1_peric1_top0_ipclk_3", "dout_peric1_usi10_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_3,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_4,
"gout_peric1_peric1_top0_ipclk_4", "dout_peric1_usi11_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_4,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_5,
"gout_peric1_peric1_top0_ipclk_5", "dout_peric1_usi12_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_5,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_6,
"gout_peric1_peric1_top0_ipclk_6", "dout_peric1_usi13_usi",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_6,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC1_PERIC1_TOP0_IPCLK_8,
"gout_peric1_peric1_top0_ipclk_8", "dout_peric1_i3c",
CLK_CON_GAT_GOUT_BLK_PERIC1_UID_PERIC1_TOP0_IPCLKPORT_IPCLK_8,
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 1/3] clk: samsung: introduce nMUX for MUX clks that can reparented
From: Tudor Ambarus @ 2024-03-26 17:28 UTC (permalink / raw)
To: peter.griffin, krzysztof.kozlowski
Cc: alim.akhtar, linux-arm-kernel, linux-samsung-soc, linux-kernel,
andre.draszik, willmcvicker, kernel-team, s.nawrocki, cw00.choi,
mturquette, sboyd, semen.protsenko, linux-clk, jaewon02.kim,
Tudor Ambarus
In-Reply-To: <20240326172813.801470-1-tudor.ambarus@linaro.org>
All samsung MUX clocks that are defined with MUX() set the
CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
reparented during clk_set_rate().
Introduce nMUX() for MUX clocks that can be reparented. One user of
nMUX() will be GS101. GS101 defines MUX clocks that are dedicated for
each instance of an IP (see MUX USI). The reparenting of these MUX clocks
will not affect other instances of the same IP or different IPs
altogether.
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/clk/samsung/clk.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index a70bd7cce39f..01f58b7686db 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -146,6 +146,26 @@ struct samsung_mux_clock {
#define MUX_F(_id, cname, pnames, o, s, w, f, mf) \
__MUX(_id, cname, pnames, o, s, w, f, mf)
+/* Used by MUX clocks where reparenting is allowed. */
+#define __nMUX(_id, cname, pnames, o, s, w, f, mf) \
+ { \
+ .id = _id, \
+ .name = cname, \
+ .parent_names = pnames, \
+ .num_parents = ARRAY_SIZE(pnames), \
+ .flags = f, \
+ .offset = o, \
+ .shift = s, \
+ .width = w, \
+ .mux_flags = mf, \
+ }
+
+#define nMUX(_id, cname, pnames, o, s, w) \
+ __nMUX(_id, cname, pnames, o, s, w, 0, 0)
+
+#define nMUX_F(_id, cname, pnames, o, s, w, f, mf) \
+ __nMUX(_id, cname, pnames, o, s, w, f, mf)
+
/**
* struct samsung_div_clock - information about div clock
* @id: platform specific id of the clock
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node
From: Laurent Pinchart @ 2024-03-26 17:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski, Eric Anholt, Stefan Wahren
In-Reply-To: <8f4ef49a-74a1-4e05-a2fa-f6bc29e80c5c@linaro.org>
On Tue, Mar 26, 2024 at 08:09:46AM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 01:49, Laurent Pinchart wrote:
> > Unlike the other child nodes of the raspberrypi,bcm2835-firmware device,
> > the gpio child is documented in a legacy text-based binding in
> > gpio/raspberrypi,firmware-gpio.txt. This causes DT validation failures:
>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + compatible:
> > + const: raspberrypi,firmware-gpio
> > +
> > + gpio-controller: true
> > +
> > + "#gpio-cells":
> > + const: 2
> > + description:
> > + The first cell is the pin number, and the second cell is used to
> > + specify the gpio polarity (GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW).
> > +
> > + gpio-line-names: true
>
> You could provide here maxItems, if this is known, but it's fine as is
> as well.
The number of items seems to be always 8, so I'll use that value.
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 3/3] clk: samsung: gs101: propagate PERIC0 USI SPI clock rate
From: Tudor Ambarus @ 2024-03-26 17:28 UTC (permalink / raw)
To: peter.griffin, krzysztof.kozlowski
Cc: alim.akhtar, linux-arm-kernel, linux-samsung-soc, linux-kernel,
andre.draszik, willmcvicker, kernel-team, s.nawrocki, cw00.choi,
mturquette, sboyd, semen.protsenko, linux-clk, jaewon02.kim,
Tudor Ambarus
In-Reply-To: <20240326172813.801470-1-tudor.ambarus@linaro.org>
When SPI transfer is being prepared, the spi-s3c64xx driver will call
clk_set_rate() to change the rate of SPI source clock (IPCLK). But IPCLK
is a gate (leaf) clock, so it must propagate the rate change up the
clock tree, so that corresponding MUX/DIV clocks can actually change
their values. Add CLK_SET_RATE_PARENT flag to corresponding clocks for
all USI instances in GS101 PERIC0: USI{1-8, 14}. This change involves the
following clocks:
PERIC0 USI*:
Clock Div range MUX Selection
-------------------------------------------------------------------
gout_peric0_peric0_top0_ipclk_* - -
dout_peric0_usi*_usi /1..16 -
mout_peric0_usi*_usi_user - {24.5 MHz, 400 MHz}
With input clock of 400 MHz this scheme provides the following IPCLK
rate range, for each USI block:
PERIC0 USI*: 1.5 MHz ... 400 MHz
Accounting for internal /4 divider in SPI blocks, and because the max
SPI frequency is limited at 50 MHz, it gives us next SPI SCK rates:
PERIC0 USI_SPI*: 384 KHz ... 49.9 MHz
Which shall be fine for the applications of the SPI bus.
Note that with this we allow the reparenting of the MUX_USIx clocks to
OSCCLK. Each instance of the USI IP has its own MUX_USI clock, thus the
reparenting of a MUX_USI clock corresponds to a single instance of the
USI IP. The datasheet mentions OSCCLK just in the low-power mode
context, but the downstream driver reparents too the MUX_USI clocks to
OSCCLK. Follow the downstream driver and do the same.
Fixes: 893f133a040b ("clk: samsung: gs101: add support for cmu_peric0")
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/clk/samsung/clk-gs101.c | 135 +++++++++++++++++---------------
1 file changed, 72 insertions(+), 63 deletions(-)
diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
index ddf2d57eed68..bd3c1b02715b 100644
--- a/drivers/clk/samsung/clk-gs101.c
+++ b/drivers/clk/samsung/clk-gs101.c
@@ -2763,33 +2763,33 @@ static const struct samsung_mux_clock peric0_mux_clks[] __initconst = {
MUX(CLK_MOUT_PERIC0_USI0_UART_USER,
"mout_peric0_usi0_uart_user", mout_peric0_usi0_uart_user_p,
PLL_CON0_MUX_CLKCMU_PERIC0_USI0_UART_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI14_USI_USER,
- "mout_peric0_usi14_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI14_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI1_USI_USER,
- "mout_peric0_usi1_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI1_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI2_USI_USER,
- "mout_peric0_usi2_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI2_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI3_USI_USER,
- "mout_peric0_usi3_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI3_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI4_USI_USER,
- "mout_peric0_usi4_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI4_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI5_USI_USER,
- "mout_peric0_usi5_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI5_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI6_USI_USER,
- "mout_peric0_usi6_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI6_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI7_USI_USER,
- "mout_peric0_usi7_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI7_USI_USER, 4, 1),
- MUX(CLK_MOUT_PERIC0_USI8_USI_USER,
- "mout_peric0_usi8_usi_user", mout_peric0_usi_usi_user_p,
- PLL_CON0_MUX_CLKCMU_PERIC0_USI8_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI14_USI_USER,
+ "mout_peric0_usi14_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI14_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI1_USI_USER,
+ "mout_peric0_usi1_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI1_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI2_USI_USER,
+ "mout_peric0_usi2_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI2_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI3_USI_USER,
+ "mout_peric0_usi3_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI3_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI4_USI_USER,
+ "mout_peric0_usi4_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI4_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI5_USI_USER,
+ "mout_peric0_usi5_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI5_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI6_USI_USER,
+ "mout_peric0_usi6_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI6_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI7_USI_USER,
+ "mout_peric0_usi7_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI7_USI_USER, 4, 1),
+ nMUX(CLK_MOUT_PERIC0_USI8_USI_USER,
+ "mout_peric0_usi8_usi_user", mout_peric0_usi_usi_user_p,
+ PLL_CON0_MUX_CLKCMU_PERIC0_USI8_USI_USER, 4, 1),
};
static const struct samsung_div_clock peric0_div_clks[] __initconst = {
@@ -2798,33 +2798,42 @@ static const struct samsung_div_clock peric0_div_clks[] __initconst = {
DIV(CLK_DOUT_PERIC0_USI0_UART,
"dout_peric0_usi0_uart", "mout_peric0_usi0_uart_user",
CLK_CON_DIV_DIV_CLK_PERIC0_USI0_UART, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI14_USI,
- "dout_peric0_usi14_usi", "mout_peric0_usi14_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI14_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI1_USI,
- "dout_peric0_usi1_usi", "mout_peric0_usi1_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI1_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI2_USI,
- "dout_peric0_usi2_usi", "mout_peric0_usi2_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI2_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI3_USI,
- "dout_peric0_usi3_usi", "mout_peric0_usi3_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI3_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI4_USI,
- "dout_peric0_usi4_usi", "mout_peric0_usi4_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI4_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI5_USI,
- "dout_peric0_usi5_usi", "mout_peric0_usi5_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI5_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI6_USI,
- "dout_peric0_usi6_usi", "mout_peric0_usi6_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI6_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI7_USI,
- "dout_peric0_usi7_usi", "mout_peric0_usi7_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI7_USI, 0, 4),
- DIV(CLK_DOUT_PERIC0_USI8_USI,
- "dout_peric0_usi8_usi", "mout_peric0_usi8_usi_user",
- CLK_CON_DIV_DIV_CLK_PERIC0_USI8_USI, 0, 4),
+ DIV_F(CLK_DOUT_PERIC0_USI14_USI,
+ "dout_peric0_usi14_usi", "mout_peric0_usi14_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI14_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI1_USI,
+ "dout_peric0_usi1_usi", "mout_peric0_usi1_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI1_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI2_USI,
+ "dout_peric0_usi2_usi", "mout_peric0_usi2_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI2_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI3_USI,
+ "dout_peric0_usi3_usi", "mout_peric0_usi3_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI3_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI4_USI,
+ "dout_peric0_usi4_usi", "mout_peric0_usi4_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI4_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI5_USI,
+ "dout_peric0_usi5_usi", "mout_peric0_usi5_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI5_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI6_USI,
+ "dout_peric0_usi6_usi", "mout_peric0_usi6_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI6_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI7_USI,
+ "dout_peric0_usi7_usi", "mout_peric0_usi7_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI7_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
+ DIV_F(CLK_DOUT_PERIC0_USI8_USI,
+ "dout_peric0_usi8_usi", "mout_peric0_usi8_usi_user",
+ CLK_CON_DIV_DIV_CLK_PERIC0_USI8_USI, 0, 4,
+ CLK_SET_RATE_PARENT, 0),
};
static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
@@ -2857,11 +2866,11 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0,
"gout_peric0_peric0_top0_ipclk_0", "dout_peric0_usi1_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_0,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_1,
"gout_peric0_peric0_top0_ipclk_1", "dout_peric0_usi2_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_1,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_10,
"gout_peric0_peric0_top0_ipclk_10", "dout_peric0_i3c",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_10,
@@ -2889,27 +2898,27 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_2,
"gout_peric0_peric0_top0_ipclk_2", "dout_peric0_usi3_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_2,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_3,
"gout_peric0_peric0_top0_ipclk_3", "dout_peric0_usi4_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_3,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_4,
"gout_peric0_peric0_top0_ipclk_4", "dout_peric0_usi5_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_4,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_5,
"gout_peric0_peric0_top0_ipclk_5", "dout_peric0_usi6_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_5,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_6,
"gout_peric0_peric0_top0_ipclk_6", "dout_peric0_usi7_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_6,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7,
"gout_peric0_peric0_top0_ipclk_7", "dout_peric0_usi8_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_7,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
GATE(CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_8,
"gout_peric0_peric0_top0_ipclk_8", "dout_peric0_i3c",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP0_IPCLKPORT_IPCLK_8,
@@ -2990,7 +2999,7 @@ static const struct samsung_gate_clock peric0_gate_clks[] __initconst = {
GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_2,
"gout_peric0_peric0_top1_ipclk_2", "dout_peric0_usi14_usi",
CLK_CON_GAT_GOUT_BLK_PERIC0_UID_PERIC0_TOP1_IPCLKPORT_IPCLK_2,
- 21, 0, 0),
+ 21, CLK_SET_RATE_PARENT, 0),
/* Disabling this clock makes the system hang. Mark the clock as critical. */
GATE(CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_0,
"gout_peric0_peric0_top1_pclk_0", "mout_peric0_bus_user",
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH v1 0/4] Reduce cost of ptep_get_lockless on arm64
From: Ryan Roberts @ 2024-03-26 17:32 UTC (permalink / raw)
To: David Hildenbrand, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <c1218cdb-905b-4896-8e17-109700577cec@redhat.com>
On 26/03/2024 17:04, David Hildenbrand wrote:
>>>>>
>>>>> Likely, we just want to read "the real deal" on both sides of the pte_same()
>>>>> handling.
>>>>
>>>> Sorry I'm not sure I understand? You mean read the full pte including
>>>> access/dirty? That's the same as dropping the patch, right? Of course if we do
>>>> that, we still have to keep pte_get_lockless() around for this case. In an
>>>> ideal
>>>> world we would convert everything over to ptep_get_lockless_norecency() and
>>>> delete ptep_get_lockless() to remove the ugliness from arm64.
>>>
>>> Yes, agreed. Patch #3 does not look too crazy and it wouldn't really affect any
>>> architecture.
>>>
>>> I do wonder if pte_same_norecency() should be defined per architecture and the
>>> default would be pte_same(). So we could avoid the mkold etc on all other
>>> architectures.
>>
>> Wouldn't that break it's semantics? The "norecency" of
>> ptep_get_lockless_norecency() means "recency information in the returned pte may
>> be incorrect". But the "norecency" of pte_same_norecency() means "ignore the
>> access and dirty bits when you do the comparison".
>
> My idea was that ptep_get_lockless_norecency() would return the actual result on
> these architectures. So e.g., on x86, there would be no actual change in
> generated code.
I think this is a bad plan... You'll end up with subtle differences between
architectures.
>
> But yes, the documentation of these functions would have to be improved.
>
> Now I wonder if ptep_get_lockless_norecency() should actively clear
> dirty/accessed bits to more easily find any actual issues where the bits still
> matter ...
I did a version that took that approach. Decided it was not as good as this way
though. Now for the life of me, I can't remember my reasoning.
>
>>
>> I think you could only do the optimization you describe if you required that
>> pte_same_norecency() would only be given values returned by
>> ptep_get_lockless_norecency() (or ptep_get_norecency()). Even then, its not
>> quite the same; if a page is accessed between gets one will return true and the
>> other false.
>
> Right.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH RESEND v6 0/5] spmi: pmic-arb: Add support for multiple buses
From: Abel Vesa @ 2024-03-26 16:28 UTC (permalink / raw)
To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Abel Vesa
This RFC prepares for and adds support for 2 buses, which is supported
in HW starting with version 7. Until now, none of the currently
supported platforms in upstream have used the second bus. The X1E80100
platform, on the other hand, needs the second bus for the USB2.0 to work
as there are 3 SMB2360 PMICs which provide eUSB2 repeaters and they are
all found on the second bus.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Changes in v6:
- Changed the compatible to platform specific (X1E80100) along with the
schema. Fixed the spmi buses unit addresses and added the empty ranges
property. Added missing properties to the spmi buses and the
"unevaluatedProperties: false".
- Deprecated the "qcom,bus-id" in the legacy schema.
- Changed the driver to check for legacy compatible first
- Link to v5: https://lore.kernel.org/r/20240221-spmi-multi-master-support-v5-0-3255ca413a0b@linaro.org
Changes in v5:
- Dropped the RFC as there aren't any concerns about the approach anymore
- Dropped the unused dev and res variables from pmic_arb_get_obsrvr_chnls_v2
- Link to v4: https://lore.kernel.org/r/20240220-spmi-multi-master-support-v4-0-dc813c878ba8@linaro.org
Changes in v4:
- Fixed comment above pmic_arb_init_apid_v7 by dropping the extra "bus" word
- Swicthed to devm_platform_ioremap_resource_byname for obsrvr and chnls.
The core remains with platform_get_resource_byname as we need the core size.
- Dropped comment from probe related to the need of platform_get_resource_byname
as it not true anymore.
- Dropped the qcom,bus-id optional property.
- Link to v3: https://lore.kernel.org/r/20240214-spmi-multi-master-support-v3-0-0bae0ef04faf@linaro.org
Changes in v3:
- Split the change into 3 separate patches. First 2 patches are moving
apid init and core resources into version specific ops. Third one is
adding the support for 2 buses and dedicated compatible.
- Added separate bindings patch
- Link to v2: https://lore.kernel.org/r/20240213-spmi-multi-master-support-v2-1-b3b102326906@linaro.org
Changes in v2:
- Reworked it so that it registers a spmi controller for each bus
rather than relying on the generic framework to pass on the bus
(master) id.
- Link to v1: https://lore.kernel.org/r/20240207-spmi-multi-master-support-v1-0-ce57f301c7fd@linaro.org
---
Abel Vesa (5):
dt-bindings: spmi: Add X1E80100 SPMI PMIC ARB schema
dt-bindings: spmi: Deprecate qcom,bus-id
spmi: pmic-arb: Make the APID init a version operation
spmi: pmic-arb: Make core resources acquiring a version operation
spmi: pmic-arb: Add multi bus support
.../bindings/spmi/qcom,spmi-pmic-arb.yaml | 1 +
.../bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml | 136 +++
drivers/spmi/spmi-pmic-arb.c | 948 +++++++++++++--------
3 files changed, 715 insertions(+), 370 deletions(-)
---
base-commit: 4893c639cc3659cefaa675bf1e59f4e7571afb5c
change-id: 20240207-spmi-multi-master-support-832a704b779b
Best regards,
--
Abel Vesa <abel.vesa@linaro.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH RESEND v6 1/5] dt-bindings: spmi: Add X1E80100 SPMI PMIC ARB schema
From: Abel Vesa @ 2024-03-26 16:28 UTC (permalink / raw)
To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Abel Vesa
In-Reply-To: <20240326-spmi-multi-master-support-v6-0-1c87d8306c5b@linaro.org>
Add dedicated schema for X1E80100 PMIC ARB (v7) as it allows multiple
buses by declaring them as child nodes.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
.../bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml | 136 +++++++++++++++++++++
1 file changed, 136 insertions(+)
diff --git a/Documentation/devicetree/bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml
new file mode 100644
index 000000000000..bdf362c4a147
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spmi/qcom,x1e80100-spmi-pmic-arb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm X1E80100 SPMI Controller (PMIC Arbiter v7)
+
+maintainers:
+ - Stephen Boyd <sboyd@kernel.org>
+
+description: |
+ The X1E80100 SPMI PMIC Arbiter implements HW version 7 and it's an SPMI
+ controller with wrapping arbitration logic to allow for multiple on-chip
+ devices to control up to 2 SPMI separate buses.
+
+ The PMIC Arbiter can also act as an interrupt controller, providing interrupts
+ to slave devices.
+
+properties:
+ compatible:
+ const: qcom,x1e80100-spmi-pmic-arb
+
+ reg:
+ items:
+ - description: core registers
+ - description: tx-channel per virtual slave regosters
+ - description: rx-channel (called observer) per virtual slave registers
+
+ reg-names:
+ items:
+ - const: core
+ - const: chnls
+ - const: obsrvr
+
+ ranges: true
+
+ '#address-cells':
+ const: 2
+
+ '#size-cells':
+ const: 2
+
+ qcom,ee:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 5
+ description: >
+ indicates the active Execution Environment identifier
+
+ qcom,channel:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 5
+ description: >
+ which of the PMIC Arb provided channels to use for accesses
+
+patternProperties:
+ "spmi@[a-f0-9]+$":
+ type: object
+ $ref: /schemas/spmi/spmi.yaml
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ items:
+ - description: configuration registers
+ - description: interrupt controller registers
+
+ reg-names:
+ items:
+ - const: cnfg
+ - const: intr
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ const: periph_irq
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 4
+ description: |
+ cell 1: slave ID for the requested interrupt (0-15)
+ cell 2: peripheral ID for requested interrupt (0-255)
+ cell 3: the requested peripheral interrupt (0-7)
+ cell 4: interrupt flags indicating level-sense information,
+ as defined in dt-bindings/interrupt-controller/irq.h
+
+required:
+ - compatible
+ - reg-names
+ - qcom,ee
+ - qcom,channel
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ spmi: arbiter@c400000 {
+ compatible = "qcom,x1e80100-spmi-pmic-arb";
+ reg = <0 0x0c400000 0 0x3000>,
+ <0 0x0c500000 0 0x4000000>,
+ <0 0x0c440000 0 0x80000>;
+ reg-names = "core", "chnls", "obsrvr";
+
+ qcom,ee = <0>;
+ qcom,channel = <0>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ spmi_bus0: spmi@c42d000 {
+ reg = <0 0x0c42d000 0 0x4000>,
+ <0 0x0c4c0000 0 0x10000>;
+ reg-names = "cnfg", "intr";
+
+ interrupt-names = "periph_irq";
+ interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-controller;
+ #interrupt-cells = <4>;
+
+ #address-cells = <2>;
+ #size-cells = <0>;
+ };
+ };
+ };
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH RESEND v6 3/5] spmi: pmic-arb: Make the APID init a version operation
From: Abel Vesa @ 2024-03-26 16:28 UTC (permalink / raw)
To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Abel Vesa
In-Reply-To: <20240326-spmi-multi-master-support-v6-0-1c87d8306c5b@linaro.org>
Rather than using conditionals in probe function, add the APID init
as a version specific operation. Due to v7, which supports multiple
buses, pass on the bus index to be used for sorting out the apid base
and count.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
drivers/spmi/spmi-pmic-arb.c | 199 +++++++++++++++++++++++++++----------------
1 file changed, 124 insertions(+), 75 deletions(-)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 9ed1180fe31f..38fed8a585fe 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -183,6 +183,7 @@ struct spmi_pmic_arb {
* struct pmic_arb_ver_ops - version dependent functionality.
*
* @ver_str: version string.
+ * @init_apid: finds the apid base and count
* @ppid_to_apid: finds the apid for a given ppid.
* @non_data_cmd: on v1 issues an spmi non-data command.
* on v2 no HW support, returns -EOPNOTSUPP.
@@ -202,6 +203,7 @@ struct spmi_pmic_arb {
*/
struct pmic_arb_ver_ops {
const char *ver_str;
+ int (*init_apid)(struct spmi_pmic_arb *pmic_arb, int index);
int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
/* spmi commands (read_cmd, write_cmd, cmd) functionality */
int (*offset)(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
@@ -942,6 +944,38 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
return 0;
}
+static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
+{
+ /*
+ * Initialize max_apid/min_apid to the opposite bounds, during
+ * the irq domain translation, we are sure to update these
+ */
+ pmic_arb->max_apid = 0;
+ pmic_arb->min_apid = pmic_arb->max_periphs - 1;
+
+ return 0;
+}
+
+static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb, int index)
+{
+ u32 *mapping_table;
+
+ if (index) {
+ dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
+ index);
+ return -EINVAL;
+ }
+
+ mapping_table = devm_kcalloc(&pmic_arb->spmic->dev, pmic_arb->max_periphs,
+ sizeof(*mapping_table), GFP_KERNEL);
+ if (!mapping_table)
+ return -ENOMEM;
+
+ pmic_arb->mapping_table = mapping_table;
+
+ return pmic_arb_init_apid_min_max(pmic_arb);
+}
+
static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
{
u32 *mapping_table = pmic_arb->mapping_table;
@@ -1144,6 +1178,40 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
return 0x1000 * pmic_arb->ee + 0x8000 * apid;
}
+static int pmic_arb_init_apid_v5(struct spmi_pmic_arb *pmic_arb, int index)
+{
+ int ret;
+
+ if (index) {
+ dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
+ index);
+ return -EINVAL;
+ }
+
+ pmic_arb->base_apid = 0;
+ pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+ PMIC_ARB_FEATURES_PERIPH_MASK;
+
+ if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
+ dev_err(&pmic_arb->spmic->dev, "Unsupported APID count %d detected\n",
+ pmic_arb->base_apid + pmic_arb->apid_count);
+ return -EINVAL;
+ }
+
+ ret = pmic_arb_init_apid_min_max(pmic_arb);
+ if (ret)
+ return ret;
+
+ ret = pmic_arb_read_apid_map_v5(pmic_arb);
+ if (ret) {
+ dev_err(&pmic_arb->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* v5 offset per ee and per apid for observer channels and per apid for
* read/write channels.
@@ -1178,6 +1246,49 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
return offset;
}
+/*
+ * 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 *pmic_arb, int index)
+{
+ int ret;
+
+ if (index == 0) {
+ pmic_arb->base_apid = 0;
+ pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+ PMIC_ARB_FEATURES_PERIPH_MASK;
+ } else if (index == 1) {
+ pmic_arb->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+ PMIC_ARB_FEATURES_PERIPH_MASK;
+ pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
+ PMIC_ARB_FEATURES_PERIPH_MASK;
+ } else {
+ dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
+ index);
+ return -EINVAL;
+ }
+
+ if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
+ dev_err(&pmic_arb->spmic->dev, "Unsupported APID count %d detected\n",
+ pmic_arb->base_apid + pmic_arb->apid_count);
+ return -EINVAL;
+ }
+
+ ret = pmic_arb_init_apid_min_max(pmic_arb);
+ if (ret)
+ return ret;
+
+ ret = pmic_arb_read_apid_map_v5(pmic_arb);
+ if (ret) {
+ dev_err(&pmic_arb->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
/*
* v7 offset per ee and per apid for observer channels and per apid for
* read/write channels.
@@ -1358,6 +1469,7 @@ pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
static const struct pmic_arb_ver_ops pmic_arb_v1 = {
.ver_str = "v1",
+ .init_apid = pmic_arb_init_apid_v1,
.ppid_to_apid = pmic_arb_ppid_to_apid_v1,
.non_data_cmd = pmic_arb_non_data_cmd_v1,
.offset = pmic_arb_offset_v1,
@@ -1372,6 +1484,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v1 = {
static const struct pmic_arb_ver_ops pmic_arb_v2 = {
.ver_str = "v2",
+ .init_apid = pmic_arb_init_apid_v1,
.ppid_to_apid = pmic_arb_ppid_to_apid_v2,
.non_data_cmd = pmic_arb_non_data_cmd_v2,
.offset = pmic_arb_offset_v2,
@@ -1386,6 +1499,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v2 = {
static const struct pmic_arb_ver_ops pmic_arb_v3 = {
.ver_str = "v3",
+ .init_apid = pmic_arb_init_apid_v1,
.ppid_to_apid = pmic_arb_ppid_to_apid_v2,
.non_data_cmd = pmic_arb_non_data_cmd_v2,
.offset = pmic_arb_offset_v2,
@@ -1400,6 +1514,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v3 = {
static const struct pmic_arb_ver_ops pmic_arb_v5 = {
.ver_str = "v5",
+ .init_apid = pmic_arb_init_apid_v5,
.ppid_to_apid = pmic_arb_ppid_to_apid_v5,
.non_data_cmd = pmic_arb_non_data_cmd_v2,
.offset = pmic_arb_offset_v5,
@@ -1414,6 +1529,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
static const struct pmic_arb_ver_ops pmic_arb_v7 = {
.ver_str = "v7",
+ .init_apid = pmic_arb_init_apid_v7,
.ppid_to_apid = pmic_arb_ppid_to_apid_v5,
.non_data_cmd = pmic_arb_non_data_cmd_v2,
.offset = pmic_arb_offset_v7,
@@ -1439,7 +1555,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
struct spmi_controller *ctrl;
struct resource *res;
void __iomem *core;
- u32 *mapping_table;
u32 channel, ee, hw_ver;
int err;
@@ -1467,12 +1582,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
pmic_arb->core_size = resource_size(res);
- pmic_arb->ppid_to_apid = devm_kcalloc(&ctrl->dev, PMIC_ARB_MAX_PPID,
- sizeof(*pmic_arb->ppid_to_apid),
- GFP_KERNEL);
- if (!pmic_arb->ppid_to_apid)
- return -ENOMEM;
-
hw_ver = readl_relaxed(core + PMIC_ARB_VERSION);
if (hw_ver < PMIC_ARB_VERSION_V2_MIN) {
@@ -1506,58 +1615,17 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
return PTR_ERR(pmic_arb->wr_base);
}
- pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
+ dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
+ pmic_arb->ver_ops->ver_str, hw_ver);
- if (hw_ver >= PMIC_ARB_VERSION_V7_MIN) {
+ if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
+ pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
+ else
pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
- /* Optional property for v7: */
- of_property_read_u32(pdev->dev.of_node, "qcom,bus-id",
- &pmic_arb->bus_instance);
- if (pmic_arb->bus_instance > 1) {
- dev_err(&pdev->dev, "invalid bus instance (%u) specified\n",
- pmic_arb->bus_instance);
- return -EINVAL;
- }
-
- if (pmic_arb->bus_instance == 0) {
- pmic_arb->base_apid = 0;
- pmic_arb->apid_count =
- readl_relaxed(core + PMIC_ARB_FEATURES) &
- PMIC_ARB_FEATURES_PERIPH_MASK;
- } else {
- pmic_arb->base_apid =
- readl_relaxed(core + PMIC_ARB_FEATURES) &
- PMIC_ARB_FEATURES_PERIPH_MASK;
- pmic_arb->apid_count =
- readl_relaxed(core + PMIC_ARB_FEATURES1) &
- PMIC_ARB_FEATURES_PERIPH_MASK;
- }
- if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
- dev_err(&pdev->dev, "Unsupported APID count %d detected\n",
- pmic_arb->base_apid + pmic_arb->apid_count);
- return -EINVAL;
- }
- } else if (hw_ver >= PMIC_ARB_VERSION_V5_MIN) {
- pmic_arb->base_apid = 0;
- pmic_arb->apid_count = readl_relaxed(core + PMIC_ARB_FEATURES) &
- PMIC_ARB_FEATURES_PERIPH_MASK;
-
- if (pmic_arb->apid_count > pmic_arb->max_periphs) {
- dev_err(&pdev->dev, "Unsupported APID count %d detected\n",
- pmic_arb->apid_count);
- return -EINVAL;
- }
- }
-
- pmic_arb->apid_data = devm_kcalloc(&ctrl->dev, pmic_arb->max_periphs,
- sizeof(*pmic_arb->apid_data),
- GFP_KERNEL);
- if (!pmic_arb->apid_data)
- return -ENOMEM;
-
- dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
- pmic_arb->ver_ops->ver_str, hw_ver);
+ err = pmic_arb->ver_ops->init_apid(pmic_arb, 0);
+ if (err)
+ return err;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
pmic_arb->intr = devm_ioremap_resource(&ctrl->dev, res);
@@ -1599,16 +1667,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
}
pmic_arb->ee = ee;
- mapping_table = devm_kcalloc(&ctrl->dev, pmic_arb->max_periphs,
- sizeof(*mapping_table), GFP_KERNEL);
- if (!mapping_table)
- return -ENOMEM;
-
- pmic_arb->mapping_table = mapping_table;
- /* Initialize max_apid/min_apid to the opposite bounds, during
- * the irq domain translation, we are sure to update these */
- pmic_arb->max_apid = 0;
- pmic_arb->min_apid = pmic_arb->max_periphs - 1;
platform_set_drvdata(pdev, ctrl);
raw_spin_lock_init(&pmic_arb->lock);
@@ -1617,15 +1675,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
ctrl->read_cmd = pmic_arb_read_cmd;
ctrl->write_cmd = pmic_arb_write_cmd;
- if (hw_ver >= PMIC_ARB_VERSION_V5_MIN) {
- err = pmic_arb_read_apid_map_v5(pmic_arb);
- if (err) {
- dev_err(&pdev->dev, "could not read APID->PPID mapping table, rc= %d\n",
- err);
- return err;
- }
- }
-
dev_dbg(&pdev->dev, "adding irq domain\n");
pmic_arb->domain = irq_domain_add_tree(pdev->dev.of_node,
&pmic_arb_irq_domain_ops, pmic_arb);
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 1/1] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: Ryan Roberts @ 2024-03-26 17:37 UTC (permalink / raw)
To: Barry Song, catalin.marinas, will, akpm, hughd, linux-mm,
linux-arm-kernel
Cc: chrisl, mark.rutland, steven.price, david, willy, Barry Song,
Kemeng Shi, Anshuman Khandual, Peter Collingbourne, Yosry Ahmed,
Peter Xu, Lorenzo Stoakes, Mike Rapoport (IBM), Aneesh Kumar K.V,
Rick Edgecombe
In-Reply-To: <20240322114136.61386-2-21cnbao@gmail.com>
On 22/03/2024 11:41, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
>
> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
> MTE as the MTE code works with the assumption tags save/restore is
> always handling a folio with only one page.
>
> The limitation should be removed as more and more ARM64 SoCs have
> this feature. Co-existence of MTE and THP_SWAP becomes more and
> more important.
>
> This patch makes MTE tags saving support large folios, then we don't
> need to split large folios into base pages for swapping out on ARM64
> SoCs with MTE any more.
>
> arch_prepare_to_swap() should take folio rather than page as parameter
> because we support THP swap-out as a whole. It saves tags for all
> pages in a large folio.
>
> As now we are restoring tags based-on folio, in arch_swap_restore(),
> we may increase some extra loops and early-exitings while refaulting
> a large folio which is still in swapcache in do_swap_page(). In case
> a large folio has nr pages, do_swap_page() will only set the PTE of
> the particular page which is causing the page fault.
> Thus do_swap_page() runs nr times, and each time, arch_swap_restore()
> will loop nr times for those subpages in the folio. So right now the
> algorithmic complexity becomes O(nr^2).
>
> Once we support mapping large folios in do_swap_page(), extra loops
> and early-exitings will decrease while not being completely removed
> as a large folio might get partially tagged in corner cases such as,
> 1. a large folio in swapcache can be partially unmapped, thus, MTE
> tags for the unmapped pages will be invalidated;
> 2. users might use mprotect() to set MTEs on a part of a large folio.
>
> arch_thp_swp_supported() is dropped since ARM64 MTE was the only one
> who needed it.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Kemeng Shi <shikemeng@huaweicloud.com>
> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Peter Collingbourne <pcc@google.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Yosry Ahmed <yosryahmed@google.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Lorenzo Stoakes <lstoakes@gmail.com>
> Cc: "Mike Rapoport (IBM)" <rppt@kernel.org>
> Cc: Hugh Dickins <hughd@google.com>
> CC: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> Reviewed-by: Steven Price <steven.price@arm.com>
> Acked-by: Chris Li <chrisl@kernel.org>
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
It would be really great if we can get this in for v6.10!
> ---
> arch/arm64/include/asm/pgtable.h | 19 ++------------
> arch/arm64/mm/mteswap.c | 45 ++++++++++++++++++++++++++++++++
> include/linux/huge_mm.h | 12 ---------
> include/linux/pgtable.h | 2 +-
> mm/internal.h | 14 ++++++++++
> mm/memory.c | 2 +-
> mm/page_io.c | 2 +-
> mm/shmem.c | 2 +-
> mm/swap_slots.c | 2 +-
> mm/swapfile.c | 2 +-
> 10 files changed, 67 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index afdd56d26ad7..259325e6b7e8 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -49,12 +49,6 @@
> __flush_tlb_range(vma, addr, end, PUD_SIZE, false, 1)
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> -static inline bool arch_thp_swp_supported(void)
> -{
> - return !system_supports_mte();
> -}
> -#define arch_thp_swp_supported arch_thp_swp_supported
> -
> /*
> * Outside of a few very special situations (e.g. hibernation), we always
> * use broadcast TLB invalidation instructions, therefore a spurious page
> @@ -1280,12 +1274,7 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
> #ifdef CONFIG_ARM64_MTE
>
> #define __HAVE_ARCH_PREPARE_TO_SWAP
> -static inline int arch_prepare_to_swap(struct page *page)
> -{
> - if (system_supports_mte())
> - return mte_save_tags(page);
> - return 0;
> -}
> +extern int arch_prepare_to_swap(struct folio *folio);
>
> #define __HAVE_ARCH_SWAP_INVALIDATE
> static inline void arch_swap_invalidate_page(int type, pgoff_t offset)
> @@ -1301,11 +1290,7 @@ static inline void arch_swap_invalidate_area(int type)
> }
>
> #define __HAVE_ARCH_SWAP_RESTORE
> -static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
> -{
> - if (system_supports_mte())
> - mte_restore_tags(entry, &folio->page);
> -}
> +extern void arch_swap_restore(swp_entry_t entry, struct folio *folio);
>
> #endif /* CONFIG_ARM64_MTE */
>
> diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
> index a31833e3ddc5..63e8d72f202a 100644
> --- a/arch/arm64/mm/mteswap.c
> +++ b/arch/arm64/mm/mteswap.c
> @@ -68,6 +68,13 @@ void mte_invalidate_tags(int type, pgoff_t offset)
> mte_free_tag_storage(tags);
> }
>
> +static inline void __mte_invalidate_tags(struct page *page)
> +{
> + swp_entry_t entry = page_swap_entry(page);
> +
> + mte_invalidate_tags(swp_type(entry), swp_offset(entry));
> +}
> +
> void mte_invalidate_tags_area(int type)
> {
> swp_entry_t entry = swp_entry(type, 0);
> @@ -83,3 +90,41 @@ void mte_invalidate_tags_area(int type)
> }
> xa_unlock(&mte_pages);
> }
> +
> +int arch_prepare_to_swap(struct folio *folio)
> +{
> + long i, nr;
> + int err;
> +
> + if (!system_supports_mte())
> + return 0;
> +
> + nr = folio_nr_pages(folio);
> +
> + for (i = 0; i < nr; i++) {
> + err = mte_save_tags(folio_page(folio, i));
> + if (err)
> + goto out;
> + }
> + return 0;
> +
> +out:
> + while (i--)
> + __mte_invalidate_tags(folio_page(folio, i));
> + return err;
> +}
> +
> +void arch_swap_restore(swp_entry_t entry, struct folio *folio)
> +{
> + long i, nr;
> +
> + if (!system_supports_mte())
> + return;
> +
> + nr = folio_nr_pages(folio);
> +
> + for (i = 0; i < nr; i++) {
> + mte_restore_tags(entry, folio_page(folio, i));
> + entry.val++;
> + }
> +}
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index de0c89105076..e04b93c43965 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -535,16 +535,4 @@ static inline int split_folio_to_order(struct folio *folio, int new_order)
> #define split_folio_to_list(f, l) split_folio_to_list_to_order(f, l, 0)
> #define split_folio(f) split_folio_to_order(f, 0)
>
> -/*
> - * archs that select ARCH_WANTS_THP_SWAP but don't support THP_SWP due to
> - * limitations in the implementation like arm64 MTE can override this to
> - * false
> - */
> -#ifndef arch_thp_swp_supported
> -static inline bool arch_thp_swp_supported(void)
> -{
> - return true;
> -}
> -#endif
> -
> #endif /* _LINUX_HUGE_MM_H */
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 85fc7554cd52..b10a7dd615bd 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1052,7 +1052,7 @@ static inline int arch_unmap_one(struct mm_struct *mm,
> * prototypes must be defined in the arch-specific asm/pgtable.h file.
> */
> #ifndef __HAVE_ARCH_PREPARE_TO_SWAP
> -static inline int arch_prepare_to_swap(struct page *page)
> +static inline int arch_prepare_to_swap(struct folio *folio)
> {
> return 0;
> }
> diff --git a/mm/internal.h b/mm/internal.h
> index 7e486f2c502c..2551e93dd885 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -76,6 +76,20 @@ static inline int folio_nr_pages_mapped(struct folio *folio)
> return atomic_read(&folio->_nr_pages_mapped) & FOLIO_PAGES_MAPPED;
> }
>
> +/*
> + * Retrieve the first entry of a folio based on a provided entry within the
> + * folio. We cannot rely on folio->swap as there is no guarantee that it has
> + * been initialized. Used for calling arch_swap_restore()
> + */
> +static inline swp_entry_t folio_swap(swp_entry_t entry, struct folio *folio)
> +{
> + swp_entry_t swap = {
> + .val = ALIGN_DOWN(entry.val, folio_nr_pages(folio)),
> + };
> +
> + return swap;
> +}
> +
> static inline void *folio_raw_mapping(struct folio *folio)
> {
> unsigned long mapping = (unsigned long)folio->mapping;
> diff --git a/mm/memory.c b/mm/memory.c
> index f2bc6dd15eb8..b7cab8be8632 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4188,7 +4188,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> * when reading from swap. This metadata may be indexed by swap entry
> * so this must be called before swap_free().
> */
> - arch_swap_restore(entry, folio);
> + arch_swap_restore(folio_swap(entry, folio), folio);
>
> /*
> * Remove the swap entry and conditionally try to free up the swapcache.
> diff --git a/mm/page_io.c b/mm/page_io.c
> index ae2b49055e43..a9a7c236aecc 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -189,7 +189,7 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
> * Arch code may have to preserve more data than just the page
> * contents, e.g. memory tags.
> */
> - ret = arch_prepare_to_swap(&folio->page);
> + ret = arch_prepare_to_swap(folio);
> if (ret) {
> folio_mark_dirty(folio);
> folio_unlock(folio);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 0aad0d9a621b..44c1519ba881 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1913,7 +1913,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> * Some architectures may have to restore extra metadata to the
> * folio after reading from swap.
> */
> - arch_swap_restore(swap, folio);
> + arch_swap_restore(folio_swap(swap, folio), folio);
>
> if (shmem_should_replace_folio(folio, gfp)) {
> error = shmem_replace_folio(&folio, gfp, info, index);
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index 90973ce7881d..53abeaf1371d 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -310,7 +310,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio)
> entry.val = 0;
>
> if (folio_test_large(folio)) {
> - if (IS_ENABLED(CONFIG_THP_SWAP) && arch_thp_swp_supported())
> + if (IS_ENABLED(CONFIG_THP_SWAP))
> get_swap_pages(1, &entry, folio_nr_pages(folio));
> goto out;
> }
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 4919423cce76..5e6d2304a2a4 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1806,7 +1806,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
> * when reading from swap. This metadata may be indexed by swap entry
> * so this must be called before swap_free().
> */
> - arch_swap_restore(entry, folio);
> + arch_swap_restore(folio_swap(entry, folio), folio);
>
> dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
> inc_mm_counter(vma->vm_mm, MM_ANONPAGES);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 3/4] mm/memory: Use ptep_get_lockless_norecency() for orig_pte
From: David Hildenbrand @ 2024-03-26 17:38 UTC (permalink / raw)
To: Ryan Roberts, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <febd0c97-8869-4ce5-bd37-cbbdf5be0a43@arm.com>
On 26.03.24 18:27, Ryan Roberts wrote:
> On 26/03/2024 17:02, David Hildenbrand wrote:
>> On 15.02.24 13:17, Ryan Roberts wrote:
>>> Let's convert handle_pte_fault()'s use of ptep_get_lockless() to
>>> ptep_get_lockless_norecency() to save orig_pte.
>>>
>>> There are a number of places that follow this model:
>>>
>>> orig_pte = ptep_get_lockless(ptep)
>>> ...
>>> <lock>
>>> if (!pte_same(orig_pte, ptep_get(ptep)))
>>> // RACE!
>>> ...
>>> <unlock>
>>>
>>> So we need to be careful to convert all of those to use
>>> pte_same_norecency() so that the access and dirty bits are excluded from
>>> the comparison.
>>>
>>> Additionally there are a couple of places that genuinely rely on the
>>> access and dirty bits of orig_pte, but with some careful refactoring, we
>>> can use ptep_get() once we are holding the lock to achieve equivalent
>>> logic.
>>
>> We really should document that changed behavior somewhere where it can be easily
>> found: that orig_pte might have incomplete/stale accessed/dirty information.
>
> I could add it to the orig_pte definition in the `struct vm_fault`?
>
>>
>>
>>> @@ -5343,7 +5356,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>> vmf->address, &vmf->ptl);
>>> if (unlikely(!vmf->pte))
>>> return 0;
>>> - vmf->orig_pte = ptep_get_lockless(vmf->pte);
>>> + vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte);
>>> vmf->flags |= FAULT_FLAG_ORIG_PTE_VALID;
>>>
>>> if (pte_none(vmf->orig_pte)) {
>>> @@ -5363,7 +5376,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>>
>>> spin_lock(vmf->ptl);
>>> entry = vmf->orig_pte;
>>> - if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
>>> + if (unlikely(!pte_same_norecency(ptep_get(vmf->pte), entry))) {
>>> update_mmu_tlb(vmf->vma, vmf->address, vmf->pte);
>>> goto unlock;
>>
>> I was wondering about the following:
>>
>> Assume the PTE is not dirty.
>>
>> Thread 1 does
>
> Sorry not sure what threads have to do with this? How is the vmf shared between
> threads? What have I misunderstood...
Assume we have a HW that does not have HW-managed access/dirty bits. One
that ends up using generic ptep_set_access_flags(). Access/dirty bits
are always updated under PT lock.
Then, imagine two threads. One is the the fault path here. another
thread performs some other magic that sets the PTE dirty under PTL.
>
>>
>> vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte)
>> /* not dirty */
>>
>> /* Now, thread 2 ends up setting the PTE dirty under PT lock. */
>>
>> spin_lock(vmf->ptl);
>> entry = vmf->orig_pte;
>> if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
>> ...
>> }
>> ...
>> entry = pte_mkyoung(entry);
>
> Do you mean pte_mkdirty() here? You're talking about dirty everywhere else.
No, that is just thread 1 seeing "oh, nothing to do" and then goes ahead
and unconditionally does that in handle_pte_fault().
>
>> if (ptep_set_access_flags(vmf->vma, ...)
>> ...
>> pte_unmap_unlock(vmf->pte, vmf->ptl);
>>
>>
>> Generic ptep_set_access_flags() will do another pte_same() check and realize
>> "hey, there was a change!" let's update the PTE!
>>
>> set_pte_at(vma->vm_mm, address, ptep, entry);
>
> This is called from the generic ptep_set_access_flags() in your example, right?
>
Yes.
>>
>> would overwrite the dirty bit set by thread 2.
>
> I'm not really sure what you are getting at... Is your concern that there is a
> race where the page could become dirty in the meantime and it now gets lost? I
> think that's why arm64 overrides ptep_set_access_flags(); since the hw can
> update access/dirty we have to deal with the races.
My concern is that your patch can in subtle ways lead to use losing PTE
dirty bits on architectures that don't have the HW-managed dirty bit.
They do exist ;)
Arm64 should be fine in that regard.
--
Cheers,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 4/4] kprobes: Remove core dependency on modules
From: Mark Rutland @ 2024-03-26 17:38 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-kernel, agordeev, anil.s.keshavamurthy, aou, bp,
catalin.marinas, dave.hansen, davem, gor, hca, jcalvinowens,
linux-arm-kernel, mhiramat, mingo, mpe, naveen.n.rao, palmer,
paul.walmsley, tglx, will
In-Reply-To: <D03UMKC71414.2D6NN1BIWD5TZ@kernel.org>
On Tue, Mar 26, 2024 at 07:13:51PM +0200, Jarkko Sakkinen wrote:
> On Tue Mar 26, 2024 at 6:36 PM EET, Mark Rutland wrote:
> > +#ifdef CONFIG_MODULES
> > /* Check if 'p' is probing a module. */
> > *probed_mod = __module_text_address((unsigned long) p->addr);
> > if (*probed_mod) {
> > @@ -1605,6 +1606,8 @@ static int check_kprobe_address_safe(struct kprobe *p,
> > ret = -ENOENT;
> > }
> > }
> > +#endif
>
> This can be scoped a bit more (see v7 of my patch set).
> > +#ifdef CONFIG_MODULES
> > static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> > {
> > char *p;
> > @@ -129,6 +130,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> >
> > return ret;
> > }
> > +#else
> > +#define trace_kprobe_module_exist(tk) false /* aka a module never exists */
> > +#endif /* CONFIG_MODULES */
> >
> > static bool trace_kprobe_is_busy(struct dyn_event *ev)
> > {
> > @@ -670,6 +674,7 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
> > return ret;
> > }
> >
> > +#ifdef CONFIG_MODULES
> > /* Module notifier call back, checking event on the module */
> > static int trace_kprobe_module_callback(struct notifier_block *nb,
> > unsigned long val, void *data)
> > @@ -699,6 +704,9 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
> >
> > return NOTIFY_DONE;
> > }
> > +#else
> > +#define trace_kprobe_module_callback (NULL)
> > +#endif /* CONFIG_MODULES */
>
> The last two CONFIG_MODULES sections could be combined. This was also in
> v7.
> Other than lgtm.
Great! I've folded your v7 changes in, and pushed that out to:
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=kprobes/without-modules
I'll hold off sending that out to the list until other folk have had a chance
to comment.
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 0/4] Reduce cost of ptep_get_lockless on arm64
From: David Hildenbrand @ 2024-03-26 17:39 UTC (permalink / raw)
To: Ryan Roberts, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <a41b0534-b841-42c2-8c06-41337c35347d@arm.com>
On 26.03.24 18:32, Ryan Roberts wrote:
> On 26/03/2024 17:04, David Hildenbrand wrote:
>>>>>>
>>>>>> Likely, we just want to read "the real deal" on both sides of the pte_same()
>>>>>> handling.
>>>>>
>>>>> Sorry I'm not sure I understand? You mean read the full pte including
>>>>> access/dirty? That's the same as dropping the patch, right? Of course if we do
>>>>> that, we still have to keep pte_get_lockless() around for this case. In an
>>>>> ideal
>>>>> world we would convert everything over to ptep_get_lockless_norecency() and
>>>>> delete ptep_get_lockless() to remove the ugliness from arm64.
>>>>
>>>> Yes, agreed. Patch #3 does not look too crazy and it wouldn't really affect any
>>>> architecture.
>>>>
>>>> I do wonder if pte_same_norecency() should be defined per architecture and the
>>>> default would be pte_same(). So we could avoid the mkold etc on all other
>>>> architectures.
>>>
>>> Wouldn't that break it's semantics? The "norecency" of
>>> ptep_get_lockless_norecency() means "recency information in the returned pte may
>>> be incorrect". But the "norecency" of pte_same_norecency() means "ignore the
>>> access and dirty bits when you do the comparison".
>>
>> My idea was that ptep_get_lockless_norecency() would return the actual result on
>> these architectures. So e.g., on x86, there would be no actual change in
>> generated code.
>
> I think this is a bad plan... You'll end up with subtle differences between
> architectures.
>
>>
>> But yes, the documentation of these functions would have to be improved.
>>
>> Now I wonder if ptep_get_lockless_norecency() should actively clear
>> dirty/accessed bits to more easily find any actual issues where the bits still
>> matter ...
>
> I did a version that took that approach. Decided it was not as good as this way
> though. Now for the life of me, I can't remember my reasoning.
Maybe because there are some code paths that check accessed/dirty
without "correctness" implications? For example, if the PTE is already
dirty, no need to set it dirty etc?
--
Cheers,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [linux-next:master] BUILD REGRESSION 084c8e315db34b59d38d06e684b1a0dd07d30287
From: kernel test robot @ 2024-03-26 17:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Linux Memory Management List, amd-gfx, bpf, linux-arm-kernel,
linux-mtd, linux-omap, linux-sunxi, netdev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 084c8e315db34b59d38d06e684b1a0dd07d30287 Add linux-next specific files for 20240326
Error/Warning: (recently discovered and may have been fixed)
ERROR: modpost: "memcpy" [crypto/cast6_generic.ko] undefined!
ERROR: modpost: "memcpy" [crypto/ccm.ko] undefined!
ERROR: modpost: "memcpy" [crypto/ecdsa_generic.ko] undefined!
ERROR: modpost: "memcpy" [crypto/md5.ko] undefined!
ERROR: modpost: "memcpy" [crypto/nhpoly1305.ko] undefined!
ERROR: modpost: "memcpy" [crypto/poly1305_generic.ko] undefined!
ERROR: modpost: "memcpy" [crypto/rmd160.ko] undefined!
ERROR: modpost: "memcpy" [fs/bfs/bfs.ko] undefined!
ERROR: modpost: "memcpy" [fs/coda/coda.ko] undefined!
ERROR: modpost: "memcpy" [fs/dlm/dlm.ko] undefined!
ERROR: modpost: "memcpy" [fs/ecryptfs/ecryptfs.ko] undefined!
ERROR: modpost: "memcpy" [fs/exfat/exfat.ko] undefined!
ERROR: modpost: "memcpy" [fs/fat/fat.ko] undefined!
ERROR: modpost: "memcpy" [fs/minix/minix.ko] undefined!
ERROR: modpost: "memcpy" [fs/overlayfs/overlay.ko] undefined!
ERROR: modpost: "memcpy" [fs/pstore/pstore.ko] undefined!
ERROR: modpost: "memcpy" [fs/pstore/ramoops.ko] undefined!
Unverified Error/Warning (likely false positive, please contact us if interested):
drivers/dma/sun6i-dma.c:777 sun6i_dma_prep_slave_sg() error: dereferencing freed memory 'v_lli'
drivers/dma/sun6i-dma.c:848 sun6i_dma_prep_dma_cyclic() error: dereferencing freed memory 'v_lli'
drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_dscl.c:400:42-43: WARNING opportunity for min()
drivers/gpu/drm/amd/display/dc/dpp/dcn20/dcn20_dpp.c:269:42-43: WARNING opportunity for min()
drivers/gpu/drm/amd/display/dc/dpp/dcn32/dcn32_dpp.c:43:42-43: WARNING opportunity for min()
Error/Warning ids grouped by kconfigs:
gcc_recent_errors
|-- alpha-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- alpha-randconfig-r002-20220208
| |-- ERROR:memcpy-crypto-ecdsa_generic.ko-undefined
| |-- ERROR:memcpy-crypto-md5.ko-undefined
| |-- ERROR:memcpy-crypto-rmd160.ko-undefined
| |-- ERROR:memcpy-fs-dlm-dlm.ko-undefined
| |-- ERROR:memcpy-fs-ecryptfs-ecryptfs.ko-undefined
| |-- ERROR:memcpy-fs-overlayfs-overlay.ko-undefined
| |-- ERROR:memcpy-fs-pstore-pstore.ko-undefined
| `-- ERROR:memcpy-fs-pstore-ramoops.ko-undefined
|-- alpha-randconfig-r034-20211016
| |-- ERROR:memcpy-fs-bfs-bfs.ko-undefined
| |-- ERROR:memcpy-fs-coda-coda.ko-undefined
| |-- ERROR:memcpy-fs-exfat-exfat.ko-undefined
| |-- ERROR:memcpy-fs-fat-fat.ko-undefined
| `-- ERROR:memcpy-fs-minix-minix.ko-undefined
|-- alpha-randconfig-r036-20211224
| |-- ERROR:memcpy-crypto-cast6_generic.ko-undefined
| |-- ERROR:memcpy-crypto-ccm.ko-undefined
| |-- ERROR:memcpy-crypto-nhpoly1305.ko-undefined
| `-- ERROR:memcpy-crypto-poly1305_generic.ko-undefined
|-- arc-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arc-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm-allmodconfig
| |-- arch-arm-mach-omap2-prm33xx.c:warning:expecting-prototype-for-am33xx_prm_global_warm_sw_reset().-Prototype-was-for-am33xx_prm_global_sw_reset()-instead
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm-allyesconfig
| |-- arch-arm-mach-omap2-prm33xx.c:warning:expecting-prototype-for-am33xx_prm_global_warm_sw_reset().-Prototype-was-for-am33xx_prm_global_sw_reset()-instead
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm-randconfig-003-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm64-defconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- csky-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- csky-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-buildonly-randconfig-004-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-buildonly-randconfig-006-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-randconfig-051-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- loongarch-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- loongarch-defconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- loongarch-randconfig-r062-20240326
| `-- drivers-firmware-arm_scmi-raw_mode.c:WARNING:scmi_dbg_raw_mode_reset_fops:write()-has-stream-semantic-safe-to-change-nonseekable_open-stream_open.
|-- m68k-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- m68k-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- m68k-randconfig-r071-20240326
| |-- drivers-dma-sun6i-dma.c-sun6i_dma_prep_dma_cyclic()-error:dereferencing-freed-memory-v_lli
| `-- drivers-dma-sun6i-dma.c-sun6i_dma_prep_slave_sg()-error:dereferencing-freed-memory-v_lli
|-- microblaze-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- microblaze-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- mips-allyesconfig
| |-- (.ref.text):relocation-truncated-to-fit:R_MIPS_26-against-start_secondary
| |-- (.text):relocation-truncated-to-fit:R_MIPS_26-against-kernel_entry
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- nios2-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- nios2-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- openrisc-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- parisc-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- parisc-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- powerpc-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- powerpc64-randconfig-001-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- powerpc64-randconfig-r064-20240326
| `-- drivers-firmware-arm_scmi-raw_mode.c:WARNING:scmi_dbg_raw_mode_reset_fops:write()-has-stream-semantic-safe-to-change-nonseekable_open-stream_open.
|-- s390-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sh-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sh-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sh-randconfig-002-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sparc-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sparc-randconfig-r051-20240326
| |-- drivers-firmware-arm_scmi-raw_mode.c:WARNING:scmi_dbg_raw_mode_reset_fops:write()-has-stream-semantic-safe-to-change-nonseekable_open-stream_open.
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sparc64-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- sparc64-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- um-allyesconfig
| |-- collect2:error:ld-returned-exit-status
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- um-i386_defconfig
| `-- collect2:error:ld-returned-exit-status
|-- um-randconfig-002-20240326
| `-- collect2:error:ld-returned-exit-status
|-- x86_64-buildonly-randconfig-004-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- x86_64-buildonly-randconfig-005-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- x86_64-randconfig-002-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
`-- x86_64-randconfig-102-20240326
|-- drivers-gpu-drm-amd-display-dc-dpp-dcn10-dcn10_dpp_dscl.c:WARNING-opportunity-for-min()
|-- drivers-gpu-drm-amd-display-dc-dpp-dcn20-dcn20_dpp.c:WARNING-opportunity-for-min()
`-- drivers-gpu-drm-amd-display-dc-dpp-dcn32-dcn32_dpp.c:WARNING-opportunity-for-min()
clang_recent_errors
|-- arm-defconfig
| |-- arch-arm-mach-omap2-prm33xx.c:warning:expecting-prototype-for-am33xx_prm_global_warm_sw_reset().-Prototype-was-for-am33xx_prm_global_sw_reset()-instead
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- arm64-allmodconfig
| |-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- arm64-randconfig-004-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- hexagon-allmodconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- hexagon-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-randconfig-006-20240326
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- i386-randconfig-061-20240327
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
|-- mips-randconfig-r061-20240326
| `-- drivers-firmware-arm_scmi-raw_mode.c:WARNING:scmi_dbg_raw_mode_reset_fops:write()-has-stream-semantic-safe-to-change-nonseekable_open-stream_open.
|-- powerpc-allyesconfig
| |-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- riscv-allmodconfig
| |-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- riscv-allyesconfig
| |-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- s390-allmodconfig
| |-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- s390-defconfig
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- s390-randconfig-002-20240326
| `-- kernel-bpf-bpf_struct_ops.c:warning:bitwise-operation-between-different-enumeration-types-(-enum-bpf_type_flag-and-enum-bpf_reg_type-)
|-- x86_64-allyesconfig
| `-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
`-- x86_64-randconfig-016-20240326
`-- fs-ubifs-journal.c:warning:expecting-prototype-for-wake_up_reservation().-Prototype-was-for-add_or_start_queue()-instead
elapsed time: 733m
configs tested: 180
configs skipped: 3
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20240326 gcc
arc randconfig-002-20240326 gcc
arm allmodconfig gcc
arm allnoconfig clang
arm allyesconfig gcc
arm defconfig clang
arm randconfig-001-20240326 gcc
arm randconfig-002-20240326 gcc
arm randconfig-003-20240326 gcc
arm randconfig-004-20240326 gcc
arm s3c6400_defconfig gcc
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 defconfig gcc
arm64 randconfig-001-20240326 clang
arm64 randconfig-002-20240326 clang
arm64 randconfig-003-20240326 gcc
arm64 randconfig-004-20240326 clang
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240326 gcc
csky randconfig-002-20240326 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240326 clang
hexagon randconfig-002-20240326 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20240326 gcc
i386 buildonly-randconfig-002-20240326 clang
i386 buildonly-randconfig-003-20240326 clang
i386 buildonly-randconfig-004-20240326 gcc
i386 buildonly-randconfig-005-20240326 gcc
i386 buildonly-randconfig-006-20240326 gcc
i386 defconfig clang
i386 randconfig-001-20240326 gcc
i386 randconfig-002-20240326 gcc
i386 randconfig-003-20240326 gcc
i386 randconfig-004-20240326 clang
i386 randconfig-005-20240326 gcc
i386 randconfig-006-20240326 clang
i386 randconfig-011-20240326 clang
i386 randconfig-012-20240326 gcc
i386 randconfig-013-20240326 clang
i386 randconfig-014-20240326 clang
i386 randconfig-015-20240326 clang
i386 randconfig-016-20240326 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240326 gcc
loongarch randconfig-002-20240326 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
m68k mvme147_defconfig gcc
m68k sun3_defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips decstation_r4k_defconfig gcc
mips fuloong2e_defconfig gcc
mips malta_defconfig gcc
mips malta_qemu_32r6_defconfig gcc
mips rt305x_defconfig clang
mips vocore2_defconfig clang
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240326 gcc
nios2 randconfig-002-20240326 gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc alldefconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-001-20240326 gcc
parisc randconfig-002-20240326 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc microwatt_defconfig gcc
powerpc randconfig-001-20240326 clang
powerpc randconfig-002-20240326 gcc
powerpc randconfig-003-20240326 clang
powerpc walnut_defconfig gcc
powerpc64 randconfig-001-20240326 gcc
powerpc64 randconfig-002-20240326 gcc
powerpc64 randconfig-003-20240326 gcc
riscv allmodconfig clang
riscv allnoconfig gcc
riscv allyesconfig clang
riscv defconfig clang
riscv nommu_k210_sdcard_defconfig gcc
riscv randconfig-001-20240326 gcc
riscv randconfig-002-20240326 clang
s390 allmodconfig clang
s390 allnoconfig clang
s390 allyesconfig gcc
s390 defconfig clang
s390 randconfig-001-20240326 clang
s390 randconfig-002-20240326 clang
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh lboxre2_defconfig gcc
sh polaris_defconfig gcc
sh randconfig-001-20240326 gcc
sh randconfig-002-20240326 gcc
sh rsk7269_defconfig gcc
sh shx3_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240326 gcc
sparc64 randconfig-002-20240326 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig gcc
um defconfig clang
um i386_defconfig gcc
um randconfig-001-20240326 gcc
um randconfig-002-20240326 gcc
um x86_64_defconfig clang
x86_64 allnoconfig clang
x86_64 allyesconfig clang
x86_64 buildonly-randconfig-001-20240326 clang
x86_64 buildonly-randconfig-002-20240326 gcc
x86_64 buildonly-randconfig-003-20240326 clang
x86_64 buildonly-randconfig-004-20240326 gcc
x86_64 buildonly-randconfig-005-20240326 gcc
x86_64 buildonly-randconfig-006-20240326 gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20240326 gcc
x86_64 randconfig-002-20240326 gcc
x86_64 randconfig-003-20240326 gcc
x86_64 randconfig-004-20240326 clang
x86_64 randconfig-005-20240326 gcc
x86_64 randconfig-006-20240326 clang
x86_64 randconfig-011-20240326 gcc
x86_64 randconfig-012-20240326 clang
x86_64 randconfig-013-20240326 gcc
x86_64 randconfig-014-20240326 gcc
x86_64 randconfig-015-20240326 clang
x86_64 randconfig-016-20240326 clang
x86_64 randconfig-071-20240326 clang
x86_64 randconfig-072-20240326 gcc
x86_64 randconfig-073-20240326 gcc
x86_64 randconfig-074-20240326 gcc
x86_64 randconfig-075-20240326 gcc
x86_64 randconfig-076-20240326 gcc
x86_64 rhel-8.3-rust clang
xtensa allnoconfig gcc
xtensa randconfig-001-20240326 gcc
xtensa randconfig-002-20240326 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Stefan Wahren @ 2024-03-26 17:40 UTC (permalink / raw)
To: Laurent Pinchart, Krzysztof Kozlowski
Cc: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio,
Dave Stevenson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Florian Fainelli, Broadcom internal kernel review list, Ray Jui,
Scott Branden, Linus Walleij, Bartosz Golaszewski
In-Reply-To: <20240326171837.GB17067@pendragon.ideasonboard.com>
Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
>> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
>> opinion about this]
>>
>> [drop Emma Anholt old address since she is not involved anymore]
>>
>> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
>>> On 26/03/2024 01:49, Laurent Pinchart wrote:
>>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
>>>> and, as a result, also needs to specify #address-cells and #size-cells.
>>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
>>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
>>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
>>>> limitations"), but the DT bindings haven't been updated, resulting in
>>>> validation errors:
>>>>
>>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
>>>> from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
>>>>
>>>> Fix this by adding the properties to the bindings.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Children do not perform any IO on their own, because everything is
>>> handled by parent. It is really odd to see dma-ranges without ranges.
>>> Referenced commits might be also wrong.
> Comunication with the firmware goes through a mailbox interface, which
> uses DMA transfers. See for instance
>
> rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> {
> u32 message = MBOX_MSG(chan, data);
> int ret;
>
> WARN_ON(data & 0xf);
>
> mutex_lock(&transaction_lock);
> reinit_completion(&fw->c);
> ret = mbox_send_message(fw->chan, &message);
> if (ret >= 0) {
> if (wait_for_completion_timeout(&fw->c, HZ)) {
> ret = 0;
> } else {
> ret = -ETIMEDOUT;
> WARN_ONCE(1, "Firmware transaction timeout");
> }
> } else {
> dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> }
> mutex_unlock(&transaction_lock);
>
> return ret;
> }
>
> int rpi_firmware_property_list(struct rpi_firmware *fw,
> void *data, size_t tag_size)
> {
> size_t size = tag_size + 12;
> u32 *buf;
> dma_addr_t bus_addr;
> int ret;
>
> /* Packets are processed a dword at a time. */
> if (size & 3)
> return -EINVAL;
>
> buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> GFP_ATOMIC);
> if (!buf)
> return -ENOMEM;
>
> /* The firmware will error out without parsing in this case. */
> WARN_ON(size >= 1024 * 1024);
>
> buf[0] = size;
> buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> memcpy(&buf[2], data, tag_size);
> buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> wmb();
>
> ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
>
> rmb();
> memcpy(data, &buf[2], tag_size);
> if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> /*
> * The tag name here might not be the one causing the
> * error, if there were multiple tags in the request.
> * But single-tag is the most common, so go with it.
> */
> dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> buf[2], buf[1]);
> ret = -EINVAL;
> }
>
> dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
>
> return ret;
> }
>
> fw->cl.dev is the device for the firmware child node. That may be where
> the problem comes from, shouldn't we use the mailbox device for DMA
> mapping ?
>
From devicetree perspective this is the mailbox DT part [1] and this
the matching dt-binding [2].
[1] -
https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
[2] -
https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/3] clk: samsung: introduce nMUX for MUX clks that can reparented
From: André Draszik @ 2024-03-26 17:43 UTC (permalink / raw)
To: Tudor Ambarus, peter.griffin, krzysztof.kozlowski
Cc: alim.akhtar, linux-arm-kernel, linux-samsung-soc, linux-kernel,
willmcvicker, kernel-team, s.nawrocki, cw00.choi, mturquette,
sboyd, semen.protsenko, linux-clk, jaewon02.kim
In-Reply-To: <20240326172813.801470-2-tudor.ambarus@linaro.org>
Hi Tudor,
On Tue, 2024-03-26 at 17:28 +0000, Tudor Ambarus wrote:
> All samsung MUX clocks that are defined with MUX() set the
> CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
> reparented during clk_set_rate().
>
> Introduce nMUX() for MUX clocks that can be reparented.
What does n in nMUX stand for?
> [...]
>
> +/* Used by MUX clocks where reparenting is allowed. */
> +#define __nMUX(_id, cname, pnames, o, s, w, f, mf) \
> + { \
> + .id = _id, \
> + .name = cname, \
> + .parent_names = pnames, \
> + .num_parents = ARRAY_SIZE(pnames), \
> + .flags = f, \
> + .offset = o, \
> + .shift = s, \
> + .width = w, \
> + .mux_flags = mf, \
> + }
You've duplicated __MUX() and removed the CLK_SET_RATE_NO_REPARENT
from flags - I think it would make sense to instead drop the flag
from the existing __MUX(), and adjust the only two existing users
of the macro, i.e. to add it in MUX() and MUX_F().
Cheers,
Andre'
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 3/4] mm/memory: Use ptep_get_lockless_norecency() for orig_pte
From: Ryan Roberts @ 2024-03-26 17:48 UTC (permalink / raw)
To: David Hildenbrand, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <f6996c4f-da60-4267-bcf1-09e4fd40c91b@redhat.com>
On 26/03/2024 17:38, David Hildenbrand wrote:
> On 26.03.24 18:27, Ryan Roberts wrote:
>> On 26/03/2024 17:02, David Hildenbrand wrote:
>>> On 15.02.24 13:17, Ryan Roberts wrote:
>>>> Let's convert handle_pte_fault()'s use of ptep_get_lockless() to
>>>> ptep_get_lockless_norecency() to save orig_pte.
>>>>
>>>> There are a number of places that follow this model:
>>>>
>>>> orig_pte = ptep_get_lockless(ptep)
>>>> ...
>>>> <lock>
>>>> if (!pte_same(orig_pte, ptep_get(ptep)))
>>>> // RACE!
>>>> ...
>>>> <unlock>
>>>>
>>>> So we need to be careful to convert all of those to use
>>>> pte_same_norecency() so that the access and dirty bits are excluded from
>>>> the comparison.
>>>>
>>>> Additionally there are a couple of places that genuinely rely on the
>>>> access and dirty bits of orig_pte, but with some careful refactoring, we
>>>> can use ptep_get() once we are holding the lock to achieve equivalent
>>>> logic.
>>>
>>> We really should document that changed behavior somewhere where it can be easily
>>> found: that orig_pte might have incomplete/stale accessed/dirty information.
>>
>> I could add it to the orig_pte definition in the `struct vm_fault`?
>>
>>>
>>>
>>>> @@ -5343,7 +5356,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>>> vmf->address, &vmf->ptl);
>>>> if (unlikely(!vmf->pte))
>>>> return 0;
>>>> - vmf->orig_pte = ptep_get_lockless(vmf->pte);
>>>> + vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte);
>>>> vmf->flags |= FAULT_FLAG_ORIG_PTE_VALID;
>>>>
>>>> if (pte_none(vmf->orig_pte)) {
>>>> @@ -5363,7 +5376,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>>>>
>>>> spin_lock(vmf->ptl);
>>>> entry = vmf->orig_pte;
>>>> - if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
>>>> + if (unlikely(!pte_same_norecency(ptep_get(vmf->pte), entry))) {
>>>> update_mmu_tlb(vmf->vma, vmf->address, vmf->pte);
>>>> goto unlock;
>>>
>>> I was wondering about the following:
>>>
>>> Assume the PTE is not dirty.
>>>
>>> Thread 1 does
>>
>> Sorry not sure what threads have to do with this? How is the vmf shared between
>> threads? What have I misunderstood...
>
> Assume we have a HW that does not have HW-managed access/dirty bits. One that
> ends up using generic ptep_set_access_flags(). Access/dirty bits are always
> updated under PT lock.
>
> Then, imagine two threads. One is the the fault path here. another thread
> performs some other magic that sets the PTE dirty under PTL.
>
>>
>>>
>>> vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte)
>>> /* not dirty */
>>>
>>> /* Now, thread 2 ends up setting the PTE dirty under PT lock. */
Ahh, this comment about thread 2 is not referring to the code immediately below
it. It all makes much more sense now. :)
>>>
>>> spin_lock(vmf->ptl);
>>> entry = vmf->orig_pte;
>>> if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
>>> ...
>>> }
>>> ...
>>> entry = pte_mkyoung(entry);
>>
>> Do you mean pte_mkdirty() here? You're talking about dirty everywhere else.
>
> No, that is just thread 1 seeing "oh, nothing to do" and then goes ahead and
> unconditionally does that in handle_pte_fault().
>
>>
>>> if (ptep_set_access_flags(vmf->vma, ...)
>>> ...
>>> pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>
>>>
>>> Generic ptep_set_access_flags() will do another pte_same() check and realize
>>> "hey, there was a change!" let's update the PTE!
>>>
>>> set_pte_at(vma->vm_mm, address, ptep, entry);
>>
>> This is called from the generic ptep_set_access_flags() in your example, right?
>>
>
> Yes.
>
>>>
>>> would overwrite the dirty bit set by thread 2.
>>
>> I'm not really sure what you are getting at... Is your concern that there is a
>> race where the page could become dirty in the meantime and it now gets lost? I
>> think that's why arm64 overrides ptep_set_access_flags(); since the hw can
>> update access/dirty we have to deal with the races.
>
> My concern is that your patch can in subtle ways lead to use losing PTE dirty
> bits on architectures that don't have the HW-managed dirty bit. They do exist ;)
But I think the example you give can already happen today? Thread 1 reads
orig_pte = ptep_get_lockless(). So that's already racy, if thread 2 is going to
set dirty just after the get, then thread 1 is going to set the PTE back to (a
modified version of) orig_pte. Isn't it already broken?
>
> Arm64 should be fine in that regard.
>
There is plenty of arm64 HW that doesn't do HW access/dirty update. But our
ptep_set_access_flags() can always deal with a racing update, even if that
update originates from SW.
Why do I have the feeling you're about to explain (very patiently) exactly why
I'm wrong?... :)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] dt-bindings: display: rockchip: add missing #sound-dai-cells to dw-hdmi
From: Krzysztof Kozlowski @ 2024-03-26 17:50 UTC (permalink / raw)
To: Heiko Stuebner, hjc, andy.yan, robh, krzysztof.kozlowski+dt,
conor+dt
Cc: dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
In-Reply-To: <20240326172801.1163200-1-heiko@sntech.de>
On 26/03/2024 18:28, Heiko Stuebner wrote:
> The #sound-dai-cells DT property is required to describe link between
> the HDMI IP block and the SoC's audio subsystem.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> .../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> index af638b6c0d21..3768df80ca7a 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> @@ -124,6 +124,9 @@ properties:
> description:
> phandle to the GRF to mux vopl/vopb.
>
> + "#sound-dai-cells":
> + const: 0
> +
Then you miss $ref in allOf to /schemas/sound/dai-common.yaml
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] dt-bindings: display: rockchip: add missing #sound-dai-cells to dw-hdmi
From: Krzysztof Kozlowski @ 2024-03-26 17:50 UTC (permalink / raw)
To: Heiko Stuebner, hjc, andy.yan, robh, krzysztof.kozlowski+dt,
conor+dt
Cc: dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
In-Reply-To: <edc0bfa2-7984-4d9d-ba25-4805ee0be679@linaro.org>
On 26/03/2024 18:50, Krzysztof Kozlowski wrote:
> On 26/03/2024 18:28, Heiko Stuebner wrote:
>> The #sound-dai-cells DT property is required to describe link between
>> the HDMI IP block and the SoC's audio subsystem.
>>
>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> .../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
>> index af638b6c0d21..3768df80ca7a 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
>> @@ -124,6 +124,9 @@ properties:
>> description:
>> phandle to the GRF to mux vopl/vopb.
>>
>> + "#sound-dai-cells":
>> + const: 0
>> +
>
> Then you miss $ref in allOf to /schemas/sound/dai-common.yaml
I meant, except your change you should add also above $ref.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 0/4] Reduce cost of ptep_get_lockless on arm64
From: Ryan Roberts @ 2024-03-26 17:51 UTC (permalink / raw)
To: David Hildenbrand, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <8bd9e136-8575-4c40-bae2-9b015d823916@redhat.com>
On 26/03/2024 17:39, David Hildenbrand wrote:
> On 26.03.24 18:32, Ryan Roberts wrote:
>> On 26/03/2024 17:04, David Hildenbrand wrote:
>>>>>>>
>>>>>>> Likely, we just want to read "the real deal" on both sides of the pte_same()
>>>>>>> handling.
>>>>>>
>>>>>> Sorry I'm not sure I understand? You mean read the full pte including
>>>>>> access/dirty? That's the same as dropping the patch, right? Of course if
>>>>>> we do
>>>>>> that, we still have to keep pte_get_lockless() around for this case. In an
>>>>>> ideal
>>>>>> world we would convert everything over to ptep_get_lockless_norecency() and
>>>>>> delete ptep_get_lockless() to remove the ugliness from arm64.
>>>>>
>>>>> Yes, agreed. Patch #3 does not look too crazy and it wouldn't really affect
>>>>> any
>>>>> architecture.
>>>>>
>>>>> I do wonder if pte_same_norecency() should be defined per architecture and the
>>>>> default would be pte_same(). So we could avoid the mkold etc on all other
>>>>> architectures.
>>>>
>>>> Wouldn't that break it's semantics? The "norecency" of
>>>> ptep_get_lockless_norecency() means "recency information in the returned pte
>>>> may
>>>> be incorrect". But the "norecency" of pte_same_norecency() means "ignore the
>>>> access and dirty bits when you do the comparison".
>>>
>>> My idea was that ptep_get_lockless_norecency() would return the actual result on
>>> these architectures. So e.g., on x86, there would be no actual change in
>>> generated code.
>>
>> I think this is a bad plan... You'll end up with subtle differences between
>> architectures.
>>
>>>
>>> But yes, the documentation of these functions would have to be improved.
>>>
>>> Now I wonder if ptep_get_lockless_norecency() should actively clear
>>> dirty/accessed bits to more easily find any actual issues where the bits still
>>> matter ...
>>
>> I did a version that took that approach. Decided it was not as good as this way
>> though. Now for the life of me, I can't remember my reasoning.
>
> Maybe because there are some code paths that check accessed/dirty without
> "correctness" implications? For example, if the PTE is already dirty, no need to
> set it dirty etc?
I think I decided I was penalizing the architectures that don't care because all
their ptep_get_norecency() and ptep_get_lockless_norecency() need to explicitly
clear access/dirty. And I would have needed ptep_get_norecency() from day 1 so
that I could feed its result into pte_same().
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v1 3/4] mm/memory: Use ptep_get_lockless_norecency() for orig_pte
From: David Hildenbrand @ 2024-03-26 17:58 UTC (permalink / raw)
To: Ryan Roberts, Mark Rutland, Catalin Marinas, Will Deacon,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Andrew Morton, Muchun Song
Cc: linux-arm-kernel, linux-mm, linux-kernel
In-Reply-To: <fb101fd1-e28a-4278-bf5f-e2dca0215a90@arm.com>
>>>>
>>>> vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte)
>>>> /* not dirty */
>>>>
>>>> /* Now, thread 2 ends up setting the PTE dirty under PT lock. */
>
> Ahh, this comment about thread 2 is not referring to the code immediately below
> it. It all makes much more sense now. :)
Sorry :)
>
>>>>
>>>> spin_lock(vmf->ptl);
>>>> entry = vmf->orig_pte;
>>>> if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
>>>> ...
>>>> }
>>>> ...
>>>> entry = pte_mkyoung(entry);
>>>
>>> Do you mean pte_mkdirty() here? You're talking about dirty everywhere else.
>>
>> No, that is just thread 1 seeing "oh, nothing to do" and then goes ahead and
>> unconditionally does that in handle_pte_fault().
>>
>>>
>>>> if (ptep_set_access_flags(vmf->vma, ...)
>>>> ...
>>>> pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>>
>>>>
>>>> Generic ptep_set_access_flags() will do another pte_same() check and realize
>>>> "hey, there was a change!" let's update the PTE!
>>>>
>>>> set_pte_at(vma->vm_mm, address, ptep, entry);
>>>
>>> This is called from the generic ptep_set_access_flags() in your example, right?
>>>
>>
>> Yes.
>>
>>>>
>>>> would overwrite the dirty bit set by thread 2.
>>>
>>> I'm not really sure what you are getting at... Is your concern that there is a
>>> race where the page could become dirty in the meantime and it now gets lost? I
>>> think that's why arm64 overrides ptep_set_access_flags(); since the hw can
>>> update access/dirty we have to deal with the races.
>>
>> My concern is that your patch can in subtle ways lead to use losing PTE dirty
>> bits on architectures that don't have the HW-managed dirty bit. They do exist ;)
>
> But I think the example you give can already happen today? Thread 1 reads
> orig_pte = ptep_get_lockless(). So that's already racy, if thread 2 is going to
> set dirty just after the get, then thread 1 is going to set the PTE back to (a
> modified version of) orig_pte. Isn't it already broken?
No, because the pte_same() check under PTL would have detected it, and
we would have backed out. And I think the problem comes to live when we
convert pte_same()->pte_same_norecency(), because we fail to protect PTE
access/dirty changes that happend under PTL from another thread.
But could be I am missing something :)
>> Arm64 should be fine in that regard.
>>
>
> There is plenty of arm64 HW that doesn't do HW access/dirty update. But our
> ptep_set_access_flags() can always deal with a racing update, even if that
> update originates from SW.
>
> Why do I have the feeling you're about to explain (very patiently) exactly why
> I'm wrong?... :)
heh ... or you'll tell me (vary patiently) why I am wrong :)
--
Cheers,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Laurent Pinchart @ 2024-03-26 18:00 UTC (permalink / raw)
To: Stefan Wahren
Cc: Krzysztof Kozlowski, devicetree, linux-rpi-kernel,
linux-arm-kernel, linux-gpio, Dave Stevenson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <e6334490-501c-4967-a48b-168868acfd39@gmx.net>
On Tue, Mar 26, 2024 at 06:40:52PM +0100, Stefan Wahren wrote:
> Am 26.03.24 um 18:18 schrieb Laurent Pinchart:
> > On Tue, Mar 26, 2024 at 12:47:34PM +0100, Stefan Wahren wrote:
> >> [add Dave since he's working on DMA for Raspberry Pi 4 and maybe have a
> >> opinion about this]
> >>
> >> [drop Emma Anholt old address since she is not involved anymore]
> >>
> >> Am 26.03.24 um 08:06 schrieb Krzysztof Kozlowski:
> >>> On 26/03/2024 01:49, Laurent Pinchart wrote:
> >>>> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> >>>> and, as a result, also needs to specify #address-cells and #size-cells.
> >>>> Those properties have been added to thebcm2835-rpi.dtsi in commits
> >>>> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> >>>> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> >>>> limitations"), but the DT bindings haven't been updated, resulting in
> >>>> validation errors:
> >>>>
> >>>> arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
> >>>> from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
> >>>>
> >>>> Fix this by adding the properties to the bindings.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Children do not perform any IO on their own, because everything is
> >>> handled by parent. It is really odd to see dma-ranges without ranges.
> >>> Referenced commits might be also wrong.
> >
> > Comunication with the firmware goes through a mailbox interface, which
> > uses DMA transfers. See for instance
> >
> > rpi_firmware_transaction(struct rpi_firmware *fw, u32 chan, u32 data)
> > {
> > u32 message = MBOX_MSG(chan, data);
> > int ret;
> >
> > WARN_ON(data & 0xf);
> >
> > mutex_lock(&transaction_lock);
> > reinit_completion(&fw->c);
> > ret = mbox_send_message(fw->chan, &message);
> > if (ret >= 0) {
> > if (wait_for_completion_timeout(&fw->c, HZ)) {
> > ret = 0;
> > } else {
> > ret = -ETIMEDOUT;
> > WARN_ONCE(1, "Firmware transaction timeout");
> > }
> > } else {
> > dev_err(fw->cl.dev, "mbox_send_message returned %d\n", ret);
> > }
> > mutex_unlock(&transaction_lock);
> >
> > return ret;
> > }
> >
> > int rpi_firmware_property_list(struct rpi_firmware *fw,
> > void *data, size_t tag_size)
> > {
> > size_t size = tag_size + 12;
> > u32 *buf;
> > dma_addr_t bus_addr;
> > int ret;
> >
> > /* Packets are processed a dword at a time. */
> > if (size & 3)
> > return -EINVAL;
> >
> > buf = dma_alloc_coherent(fw->cl.dev, PAGE_ALIGN(size), &bus_addr,
> > GFP_ATOMIC);
> > if (!buf)
> > return -ENOMEM;
> >
> > /* The firmware will error out without parsing in this case. */
> > WARN_ON(size >= 1024 * 1024);
> >
> > buf[0] = size;
> > buf[1] = RPI_FIRMWARE_STATUS_REQUEST;
> > memcpy(&buf[2], data, tag_size);
> > buf[size / 4 - 1] = RPI_FIRMWARE_PROPERTY_END;
> > wmb();
> >
> > ret = rpi_firmware_transaction(fw, MBOX_CHAN_PROPERTY, bus_addr);
> >
> > rmb();
> > memcpy(data, &buf[2], tag_size);
> > if (ret == 0 && buf[1] != RPI_FIRMWARE_STATUS_SUCCESS) {
> > /*
> > * The tag name here might not be the one causing the
> > * error, if there were multiple tags in the request.
> > * But single-tag is the most common, so go with it.
> > */
> > dev_err(fw->cl.dev, "Request 0x%08x returned status 0x%08x\n",
> > buf[2], buf[1]);
> > ret = -EINVAL;
> > }
> >
> > dma_free_coherent(fw->cl.dev, PAGE_ALIGN(size), buf, bus_addr);
> >
> > return ret;
> > }
> >
> > fw->cl.dev is the device for the firmware child node. That may be where
> > the problem comes from, shouldn't we use the mailbox device for DMA
> > mapping ?
>
> From devicetree perspective this is the mailbox DT part [1] and this
> the matching dt-binding [2].
>
> [1] - https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm/boot/dts/broadcom/bcm283x.dtsi#L100
> [2] - https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.yaml
That's the device performing DMA, so I think it should be used for DMA
mapping.
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v11 28/28] drm/sun4i: hdmi: Switch to HDMI connector
From: Maxime Ripard @ 2024-03-26 15:40 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard, Sui Jingfeng
In-Reply-To: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
The new HDMI connector infrastructure allows to remove some boilerplate,
especially to generate infoframes. Let's switch to it.
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/sun4i/Kconfig | 1 +
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 80 ++++++++++++++++++++++------------
2 files changed, 52 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 4741d9f6544c..cef3af4bd3a6 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -16,10 +16,11 @@ config DRM_SUN4I
if DRM_SUN4I
config DRM_SUN4I_HDMI
tristate "Allwinner A10/A10s/A20/A31 HDMI Controller Support"
depends on ARM || COMPILE_TEST
+ depends on DRM_HDMI_STATE_HELPER
default DRM_SUN4I
help
Choose this option if you have an Allwinner A10/A10s/A20/A31
SoC with an HDMI controller.
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 2d1880c61b50..7ac085aa0a35 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -35,34 +35,28 @@
container_of_const(e, struct sun4i_hdmi, encoder)
#define drm_connector_to_sun4i_hdmi(c) \
container_of_const(c, struct sun4i_hdmi, connector)
-static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
- struct drm_display_mode *mode)
+static int sun4i_hdmi_write_infoframe(struct drm_connector *connector,
+ enum hdmi_infoframe_type type,
+ const u8 *buffer, size_t len)
{
- struct hdmi_avi_infoframe frame;
- u8 buffer[17];
- int i, ret;
+ struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
+ int i;
- ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
- &hdmi->connector, mode);
- if (ret < 0) {
- DRM_ERROR("Failed to get infoframes from mode\n");
- return ret;
+ if (type != HDMI_INFOFRAME_TYPE_AVI) {
+ drm_err(connector->dev,
+ "Unsupported infoframe type: %u\n", type);
+ return 0;
}
- ret = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
- if (ret < 0) {
- DRM_ERROR("Failed to pack infoframes\n");
- return ret;
- }
-
- for (i = 0; i < sizeof(buffer); i++)
+ for (i = 0; i < len; i++)
writeb(buffer[i], hdmi->base + SUN4I_HDMI_AVI_INFOFRAME_REG(i));
return 0;
+
}
static void sun4i_hdmi_disable(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
@@ -81,18 +75,22 @@ static void sun4i_hdmi_disable(struct drm_encoder *encoder,
static void sun4i_hdmi_enable(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
- struct drm_display_info *display = &hdmi->connector.display_info;
+ struct drm_connector *connector = &hdmi->connector;
+ struct drm_display_info *display = &connector->display_info;
+ struct drm_connector_state *conn_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ unsigned long long tmds_rate = conn_state->hdmi.tmds_char_rate;
unsigned int x, y;
u32 val = 0;
DRM_DEBUG_DRIVER("Enabling the HDMI Output\n");
- clk_set_rate(hdmi->mod_clk, mode->crtc_clock * 1000);
- clk_set_rate(hdmi->tmds_clk, mode->crtc_clock * 1000);
+ clk_set_rate(hdmi->mod_clk, tmds_rate);
+ clk_set_rate(hdmi->tmds_clk, tmds_rate);
/* Set input sync enable */
writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC,
hdmi->base + SUN4I_HDMI_UNKNOWN_REG);
@@ -141,11 +139,12 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder,
writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);
clk_prepare_enable(hdmi->tmds_clk);
- sun4i_hdmi_setup_avi_infoframes(hdmi, mode);
+ drm_atomic_helper_connector_hdmi_update_infoframes(connector, state);
+
val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END);
writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0));
val = SUN4I_HDMI_VID_CTRL_ENABLE;
@@ -194,23 +193,26 @@ static int sun4i_hdmi_connector_atomic_check(struct drm_connector *connector,
struct drm_crtc_state *crtc_state = crtc->state;
struct drm_display_mode *mode = &crtc_state->adjusted_mode;
enum drm_mode_status status;
status = sun4i_hdmi_connector_clock_valid(connector, mode,
- mode->clock * 1000);
+ conn_state->hdmi.tmds_char_rate);
if (status != MODE_OK)
return -EINVAL;
return 0;
}
static enum drm_mode_status
sun4i_hdmi_connector_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
{
- return sun4i_hdmi_connector_clock_valid(connector, mode,
- mode->clock * 1000);
+ unsigned long long rate =
+ drm_connector_hdmi_compute_mode_clock(mode, 8,
+ HDMI_COLORSPACE_RGB);
+
+ return sun4i_hdmi_connector_clock_valid(connector, mode, rate);
}
static int sun4i_hdmi_get_modes(struct drm_connector *connector)
{
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
@@ -252,10 +254,15 @@ static struct i2c_adapter *sun4i_hdmi_get_ddc(struct device *dev)
return ERR_PTR(-EPROBE_DEFER);
return ddc;
}
+static const struct drm_connector_hdmi_funcs sun4i_hdmi_hdmi_connector_funcs = {
+ .tmds_char_rate_valid = sun4i_hdmi_connector_clock_valid,
+ .write_infoframe = sun4i_hdmi_write_infoframe,
+};
+
static const struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = {
.atomic_check = sun4i_hdmi_connector_atomic_check,
.mode_valid = sun4i_hdmi_connector_mode_valid,
.get_modes = sun4i_hdmi_get_modes,
};
@@ -273,15 +280,21 @@ sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force)
}
return connector_status_connected;
}
+static void sun4i_hdmi_connector_reset(struct drm_connector *connector)
+{
+ drm_atomic_helper_connector_reset(connector);
+ __drm_atomic_helper_connector_hdmi_reset(connector, connector->state);
+}
+
static const struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
.detect = sun4i_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = drm_connector_cleanup,
- .reset = drm_atomic_helper_connector_reset,
+ .reset = sun4i_hdmi_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
#ifdef CONFIG_DRM_SUN4I_HDMI_CEC
@@ -636,14 +649,23 @@ static int sun4i_hdmi_bind(struct device *dev, struct device *master,
hdmi->base + SUN4I_HDMI_CEC);
#endif
drm_connector_helper_add(&hdmi->connector,
&sun4i_hdmi_connector_helper_funcs);
- ret = drm_connector_init_with_ddc(drm, &hdmi->connector,
- &sun4i_hdmi_connector_funcs,
- DRM_MODE_CONNECTOR_HDMIA,
- hdmi->ddc_i2c);
+ ret = drmm_connector_hdmi_init(drm, &hdmi->connector,
+ /*
+ * NOTE: Those are likely to be
+ * wrong, but I couldn't find the
+ * actual ones in the BSP.
+ */
+ "AW", "HDMI",
+ &sun4i_hdmi_connector_funcs,
+ &sun4i_hdmi_hdmi_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ hdmi->ddc_i2c,
+ BIT(HDMI_COLORSPACE_RGB),
+ 8);
if (ret) {
dev_err(dev,
"Couldn't initialise the HDMI connector\n");
goto err_cleanup_connector;
}
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v11 26/28] drm/vc4: tests: Convert to plane creation helper
From: Maxime Ripard @ 2024-03-26 15:40 UTC (permalink / raw)
To: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Daniel Vetter,
Jonathan Corbet, Sandy Huang, Heiko Stübner, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: Hans Verkuil, Sebastian Wick, Ville Syrjälä, dri-devel,
linux-arm-kernel, linux-doc, linux-kernel, linux-media,
linux-rockchip, linux-sunxi, Maxime Ripard, Maíra Canal
In-Reply-To: <20240326-kms-hdmi-connector-state-v11-0-c5680ffcf261@kernel.org>
Now that we have a plane create helper for kunit mocked drivers, let's
convert to it in vc4.
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/gpu/drm/vc4/tests/vc4_mock_plane.c | 34 +++++++-----------------------
1 file changed, 8 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c b/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c
index 973f5f929097..14357db82238 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_mock_plane.c
@@ -1,43 +1,25 @@
// SPDX-License-Identifier: GPL-2.0
-#include <drm/drm_atomic_state_helper.h>
-#include <drm/drm_fourcc.h>
-#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_kunit_helpers.h>
#include <drm/drm_plane.h>
#include <kunit/test.h>
#include "vc4_mock.h"
-static const struct drm_plane_helper_funcs vc4_dummy_plane_helper_funcs = {
-};
-
-static const struct drm_plane_funcs vc4_dummy_plane_funcs = {
- .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
- .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
- .reset = drm_atomic_helper_plane_reset,
-};
-
-static const uint32_t vc4_dummy_plane_formats[] = {
- DRM_FORMAT_XRGB8888,
-};
-
struct drm_plane *vc4_dummy_plane(struct kunit *test, struct drm_device *drm,
enum drm_plane_type type)
{
struct drm_plane *plane;
- plane = __drmm_universal_plane_alloc(drm, sizeof(struct drm_plane), 0,
- 0,
- &vc4_dummy_plane_funcs,
- vc4_dummy_plane_formats,
- ARRAY_SIZE(vc4_dummy_plane_formats),
- NULL,
- DRM_PLANE_TYPE_PRIMARY,
- NULL);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane);
+ KUNIT_ASSERT_EQ(test, type, DRM_PLANE_TYPE_PRIMARY);
- drm_plane_helper_add(plane, &vc4_dummy_plane_helper_funcs);
+ plane = drm_kunit_helper_create_primary_plane(test, drm,
+ NULL,
+ NULL,
+ NULL, 0,
+ NULL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane);
return plane;
}
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 04/25] clk: meson: a1: add the audio clock controller driver
From: Jerome Brunet @ 2024-03-26 15:26 UTC (permalink / raw)
To: Jan Dakinevich
Cc: Jerome Brunet, Neil Armstrong, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Kevin Hilman, Martin Blumenstingl, Liam Girdwood, Mark Brown,
Linus Walleij, Jaroslav Kysela, Takashi Iwai, linux-amlogic,
linux-clk, devicetree, linux-kernel, linux-arm-kernel, alsa-devel,
linux-sound, linux-gpio, kernel
In-Reply-To: <0f4b2ec3-204f-44ea-a1b2-e47ea05def1f@salutedevices.com>
On Sat 23 Mar 2024 at 21:02, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> Jerome, I have reworked my driver reusing axg-audio code as most as I
> could and now I have one more question. Lets see on this definition from
> axg-audio:
>
> #define AUD_MST_MUX(_name, _reg, _flag) \
> AUD_MUX(_name##_sel, _reg, 0x7, 24, _flag, \
> mst_mux_parent_data, 0)
>
> #define AUD_MST_MCLK_MUX(_name, _reg) \
> AUD_MST_MUX(_name, _reg, CLK_MUX_ROUND_CLOSEST)
>
> CLK_SET_RATE_PARENT is not set here. But why? It means, that topmost pll
> clock will not be reconfigured at runtime to satisfy the rate that was
> requested from axg-tdm.
>
Yes, that is by design. It is another area where mainline audio differs
greatly from AML vendor code. The PLLs are expected be to fixed and the
audio master clock will reparent to the most adequate PLL source
depending on the use case.
This is how we manage to satisfy all audio interfaces with a very
limited number of PLLs
On AXG/G12 there is at most 6 concurrent interfaces (3 FRDDR/TODDR) - 8
on sm1 - and we can satisfy on that with 3 PLLs. That would not be
possible if interfaces were having their way with the PLLs, reseting it
everytime a stream is started.
The PLL rate should be carefully chosen so it can be derived easily. On
AXG/G12/SM1 that is:
* one PLL per rate family, to maximize clock precision
* x24 x32: to handle different sample sizes
* x2 until we reach the PLL limits to allow higher rates such as 384kHz
or even higher
If you have less PLLs on A1, you'll have to make compromises, like a less
precise clock to support multiple family with one PLL.
This is why the PLLs are set for each platform in DT because that choice
may depend on the platform use case.
>
> On 3/19/24 11:30, Jerome Brunet wrote:
>>
>> On Tue 19 Mar 2024 at 04:47, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>
>>> Let's start from the end:
>>>
>>>> No - Looks to me you just have two clock controllers you are trying
>>> force into one.
>>>
>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>
>>> Most of fishy workarounds that you commented is caused the fact the mmio
>>> of this clock controller is divided into two parts. Compare it with
>>> axg-audio driver, things that was part of contigous memory region (like
>>> pdm) here are moved to second region. Is this enough to make a guess
>>> that these are two devices?
>>
>> I see obsolutely no reason to think it is a single device nor to add all the quirks
>> you have the way you did. So yes, in that case, 2 zones, 2 devices.
>>
>>>
>>> Concerning AUD2_CLKID_AUDIOTOP clock, as it turned out, it must be
>>> enabled before enabling of clocks from second region too. That is
>>> AUD2_CLKID_AUDIOTOP clock feeds both parts of this clock controller.
>>>
>>
>> Yes. I understood the first time around and already commented on that.
>>
>>>
>>> On 3/15/24 12:20, Jerome Brunet wrote:
>>>>
>>>> On Fri 15 Mar 2024 at 02:21, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>>
>>>>> This controller provides clocks and reset functionality for audio
>>>>> peripherals on Amlogic A1 SoC family.
>>>>>
>>>>> The driver is almost identical to 'axg-audio', however it would be better
>>>>> to keep it separate due to following reasons:
>>>>>
>>>>> - significant amount of bits has another definition. I will bring there
>>>>> a mess of new defines with A1_ suffixes.
>>>>>
>>>>> - registers of this controller are located in two separate regions. It
>>>>> will give a lot of complications for 'axg-audio' to support this.
>>>>>
>>>>> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>> ---
>>>>> drivers/clk/meson/Kconfig | 13 +
>>>>> drivers/clk/meson/Makefile | 1 +
>>>>> drivers/clk/meson/a1-audio.c | 556 +++++++++++++++++++++++++++++++++++
>>>>> drivers/clk/meson/a1-audio.h | 58 ++++
>>>>> 4 files changed, 628 insertions(+)
>>>>> create mode 100644 drivers/clk/meson/a1-audio.c
>>>>> create mode 100644 drivers/clk/meson/a1-audio.h
>>>>>
>>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>>> index d6a2fa5f7e88..80c4a18c83d2 100644
>>>>> --- a/drivers/clk/meson/Kconfig
>>>>> +++ b/drivers/clk/meson/Kconfig
>>>>> @@ -133,6 +133,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>>> controller to work.
>>>>>
>>>>> +config COMMON_CLK_A1_AUDIO
>>>>> + tristate "Amlogic A1 SoC Audio clock controller support"
>>>>> + depends on ARM64
>>>>> + select COMMON_CLK_MESON_REGMAP
>>>>> + select COMMON_CLK_MESON_CLKC_UTILS
>>>>> + select COMMON_CLK_MESON_PHASE
>>>>> + select COMMON_CLK_MESON_SCLK_DIV
>>>>> + select COMMON_CLK_MESON_AUDIO_RSTC
>>>>> + help
>>>>> + Support for the Audio clock controller on Amlogic A113L based
>>>>> + device, A1 SoC Family. Say Y if you want A1 Audio clock controller
>>>>> + to work.
>>>>> +
>>>>> config COMMON_CLK_G12A
>>>>> tristate "G12 and SM1 SoC clock controllers support"
>>>>> depends on ARM64
>>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>>> index 88d94921a4dc..4968fc7ad555 100644
>>>>> --- a/drivers/clk/meson/Makefile
>>>>> +++ b/drivers/clk/meson/Makefile
>>>>> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>>>> +obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
>>>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>>>>> diff --git a/drivers/clk/meson/a1-audio.c b/drivers/clk/meson/a1-audio.c
>>>>> new file mode 100644
>>>>> index 000000000000..6039116c93ba
>>>>> --- /dev/null
>>>>> +++ b/drivers/clk/meson/a1-audio.c
>>>>> @@ -0,0 +1,556 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>>> +/*
>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>> + *
>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>> + */
>>>>> +
>>>>> +#include <linux/clk.h>
>>>>> +#include <linux/clk-provider.h>
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/of_device.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/regmap.h>
>>>>> +#include <linux/reset.h>
>>>>> +#include <linux/reset-controller.h>
>>>>> +#include <linux/slab.h>
>>>>> +
>>>>> +#include "meson-clkc-utils.h"
>>>>> +#include "meson-audio-rstc.h"
>>>>> +#include "clk-regmap.h"
>>>>> +#include "clk-phase.h"
>>>>> +#include "sclk-div.h"
>>>>> +#include "a1-audio.h"
>>>>> +
>>>>> +#define AUDIO_PDATA(_name) \
>>>>> + ((const struct clk_parent_data[]) { { .hw = &(_name).hw } })
>>>>
>>>> Not a fan - yet another level of macro.
>>>>
>>>>> +
>>>>> +#define AUDIO_MUX(_name, _reg, _mask, _shift, _pdata) \
>>>>> +static struct clk_regmap _name = { \
>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>> + .data = &(struct clk_regmap_mux_data){ \
>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>> + .mask = (_mask), \
>>>>> + .shift = (_shift), \
>>>>> + }, \
>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>> + .name = #_name, \
>>>>> + .ops = &clk_regmap_mux_ops, \
>>>>> + .parent_data = (_pdata), \
>>>>> + .num_parents = ARRAY_SIZE(_pdata), \
>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>> + }, \
>>>>> +}
>>>>> +
>>>>> +#define AUDIO_DIV(_name, _reg, _shift, _width, _pdata) \
>>>>> +static struct clk_regmap _name = { \
>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>> + .data = &(struct clk_regmap_div_data){ \
>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_shift), \
>>>>> + .width = (_width), \
>>>>> + }, \
>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>> + .name = #_name, \
>>>>> + .ops = &clk_regmap_divider_ops, \
>>>>> + .parent_data = (_pdata), \
>>>>> + .num_parents = 1, \
>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>> + }, \
>>>>> +}
>>>>> +
>>>>> +#define AUDIO_GATE(_name, _reg, _bit, _pdata) \
>>>>> +static struct clk_regmap _name = { \
>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>> + .data = &(struct clk_regmap_gate_data){ \
>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>> + .bit_idx = (_bit), \
>>>>> + }, \
>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>> + .name = #_name, \
>>>>> + .ops = &clk_regmap_gate_ops, \
>>>>> + .parent_data = (_pdata), \
>>>>> + .num_parents = 1, \
>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>> + }, \
>>>>> +}
>>>>> +
>>>>> +#define AUDIO_SCLK_DIV(_name, _reg, _div_shift, _div_width, \
>>>>> + _hi_shift, _hi_width, _pdata, _set_rate_parent) \
>>>>> +static struct clk_regmap _name = { \
>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>> + .data = &(struct meson_sclk_div_data) { \
>>>>> + .div = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_div_shift), \
>>>>> + .width = (_div_width), \
>>>>> + }, \
>>>>> + .hi = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_hi_shift), \
>>>>> + .width = (_hi_width), \
>>>>> + }, \
>>>>> + }, \
>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>> + .name = #_name, \
>>>>> + .ops = &meson_sclk_div_ops, \
>>>>> + .parent_data = (_pdata), \
>>>>> + .num_parents = 1, \
>>>>> + .flags = (_set_rate_parent) ? CLK_SET_RATE_PARENT : 0, \
>>>>
>>>> Does not help readeability. Just pass the flag as axg-audio does.
>>>>
>>>>> + }, \
>>>>> +}
>>>>> +
>>>>> +#define AUDIO_TRIPHASE(_name, _reg, _width, _shift0, _shift1, _shift2, \
>>>>> + _pdata) \
>>>>> +static struct clk_regmap _name = { \
>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>> + .data = &(struct meson_clk_triphase_data) { \
>>>>> + .ph0 = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_shift0), \
>>>>> + .width = (_width), \
>>>>> + }, \
>>>>> + .ph1 = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_shift1), \
>>>>> + .width = (_width), \
>>>>> + }, \
>>>>> + .ph2 = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_shift2), \
>>>>> + .width = (_width), \
>>>>> + }, \
>>>>> + }, \
>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>> + .name = #_name, \
>>>>> + .ops = &meson_clk_triphase_ops, \
>>>>> + .parent_data = (_pdata), \
>>>>> + .num_parents = 1, \
>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>> + }, \
>>>>> +}
>>>>> +
>>>>> +#define AUDIO_SCLK_WS(_name, _reg, _width, _shift_ph, _shift_ws, \
>>>>> + _pdata) \
>>>>> +static struct clk_regmap _name = { \
>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>> + .data = &(struct meson_sclk_ws_inv_data) { \
>>>>> + .ph = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_shift_ph), \
>>>>> + .width = (_width), \
>>>>> + }, \
>>>>> + .ws = { \
>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>> + .shift = (_shift_ws), \
>>>>> + .width = (_width), \
>>>>> + }, \
>>>>> + }, \
>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>> + .name = #_name, \
>>>>> + .ops = &meson_sclk_ws_inv_ops, \
>>>>> + .parent_data = (_pdata), \
>>>>> + .num_parents = 1, \
>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>> + }, \
>>>>> +}
>>>>
>>>> All the above does essentially the same things as the macro of
>>>> axg-audio, to some minor differences. Yet it is another set to maintain.
>>>>
>>>
>>> Except one thing... Here I keep memory identifier to which this clock
>>> belongs:
>>>
>>> .map = AUDIO_REG_MAP(_reg),
>>>
>>> It is workaround, but ->map the only common field in clk_regmap that
>>> could be used for this purpose.
>>>
>>>
>>>> I'd much prefer if you put the axg-audio macro in a header a re-used
>>>> those. There would a single set to maintain. You may then specialize the
>>>> included in the driver C file, to avoid redundant parameters
>>>>
>>>> Rework axg-audio to use clk_parent_data if you must, but not in the same
>>>> series please.
>>>>
>>>>> +
>>>>> +static const struct clk_parent_data a1_pclk_pdata[] = {
>>>>> + { .fw_name = "pclk", },
>>>>> +};
>>>>> +
>>>>> +AUDIO_GATE(audio_ddr_arb, AUDIO_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_tdmin_a, AUDIO_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_tdmin_b, AUDIO_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_tdmin_lb, AUDIO_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_loopback, AUDIO_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_tdmout_a, AUDIO_CLK_GATE_EN0, 5, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_tdmout_b, AUDIO_CLK_GATE_EN0, 6, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_frddr_a, AUDIO_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_frddr_b, AUDIO_CLK_GATE_EN0, 8, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_toddr_a, AUDIO_CLK_GATE_EN0, 9, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_toddr_b, AUDIO_CLK_GATE_EN0, 10, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_spdifin, AUDIO_CLK_GATE_EN0, 11, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_resample, AUDIO_CLK_GATE_EN0, 12, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_eqdrc, AUDIO_CLK_GATE_EN0, 13, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio_audiolocker, AUDIO_CLK_GATE_EN0, 14, a1_pclk_pdata);
>>>> This is what I mean by redundant parameter ^
>>>>
>>>
>>> Yep. I could define something like AUDIO_PCLK_GATE().
>>>
>>>>> +
>>>>> +AUDIO_GATE(audio2_ddr_arb, AUDIO2_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio2_pdm, AUDIO2_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio2_tdmin_vad, AUDIO2_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio2_toddr_vad, AUDIO2_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio2_vad, AUDIO2_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>> +AUDIO_GATE(audio2_audiotop, AUDIO2_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>> +
>>>>> +static const struct clk_parent_data a1_mst_pdata[] = {
>>>>> + { .fw_name = "dds_in" },
>>>>> + { .fw_name = "fclk_div2" },
>>>>> + { .fw_name = "fclk_div3" },
>>>>> + { .fw_name = "hifi_pll" },
>>>>> + { .fw_name = "xtal" },
>>>>> +};
>>>>> +
>>>>> +#define AUDIO_MST_MCLK(_name, _reg) \
>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0x7, 24, a1_mst_pdata); \
>>>>> + AUDIO_DIV(_name##_div, (_reg), 0, 16, \
>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>> + AUDIO_GATE(_name, (_reg), 31, AUDIO_PDATA(_name##_div))
>>>>> +
>>>>> +AUDIO_MST_MCLK(audio_mst_a_mclk, AUDIO_MCLK_A_CTRL);
>>>>> +AUDIO_MST_MCLK(audio_mst_b_mclk, AUDIO_MCLK_B_CTRL);
>>>>> +AUDIO_MST_MCLK(audio_mst_c_mclk, AUDIO_MCLK_C_CTRL);
>>>>> +AUDIO_MST_MCLK(audio_mst_d_mclk, AUDIO_MCLK_D_CTRL);
>>>>> +AUDIO_MST_MCLK(audio_spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>>>>> +AUDIO_MST_MCLK(audio_eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>>>>> +
>>>>> +AUDIO_MUX(audio_resample_clk_mux, AUDIO_CLK_RESAMPLE_CTRL, 0xf, 24,
>>>>> + a1_mst_pdata);
>>>>> +AUDIO_DIV(audio_resample_clk_div, AUDIO_CLK_RESAMPLE_CTRL, 0, 8,
>>>>> + AUDIO_PDATA(audio_resample_clk_mux));
>>>>> +AUDIO_GATE(audio_resample_clk, AUDIO_CLK_RESAMPLE_CTRL, 31,
>>>>> + AUDIO_PDATA(audio_resample_clk_div));
>>>>> +
>>>>> +AUDIO_MUX(audio_locker_in_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 8,
>>>>> + a1_mst_pdata);
>>>>> +AUDIO_DIV(audio_locker_in_clk_div, AUDIO_CLK_LOCKER_CTRL, 0, 8,
>>>>> + AUDIO_PDATA(audio_locker_in_clk_mux));
>>>>> +AUDIO_GATE(audio_locker_in_clk, AUDIO_CLK_LOCKER_CTRL, 15,
>>>>> + AUDIO_PDATA(audio_locker_in_clk_div));
>>>>> +
>>>>> +AUDIO_MUX(audio_locker_out_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 24,
>>>>> + a1_mst_pdata);
>>>>> +AUDIO_DIV(audio_locker_out_clk_div, AUDIO_CLK_LOCKER_CTRL, 16, 8,
>>>>> + AUDIO_PDATA(audio_locker_out_clk_mux));
>>>>> +AUDIO_GATE(audio_locker_out_clk, AUDIO_CLK_LOCKER_CTRL, 31,
>>>>> + AUDIO_PDATA(audio_locker_out_clk_div));
>>>>> +
>>>>> +AUDIO_MST_MCLK(audio2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>>>>> +AUDIO_MST_MCLK(audio2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>>>>> +AUDIO_MST_MCLK(audio2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>>>>> +AUDIO_MST_MCLK(audio2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>>>>> +
>>>>> +#define AUDIO_MST_SCLK(_name, _reg0, _reg1, _pdata) \
>>>>> + AUDIO_GATE(_name##_pre_en, (_reg0), 31, (_pdata)); \
>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 20, 10, 0, 0, \
>>>>> + AUDIO_PDATA(_name##_pre_en), true); \
>>>>> + AUDIO_GATE(_name##_post_en, (_reg0), 30, \
>>>>> + AUDIO_PDATA(_name##_div)); \
>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 0, 2, 4, \
>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>> +
>>>>
>>>> Again, I'm not a fan of this many levels of macro. I can live with it
>>>> but certainly don't want the burden of reviewing and maintaining for
>>>> clock driver. AXG / G12 and A1 are obviously closely related, so make it common.
>>>>
>>>>> +#define AUDIO_MST_LRCLK(_name, _reg0, _reg1, _pdata) \
>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 0, 10, 10, 10, \
>>>>> + (_pdata), false); \
>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 1, 3, 5, \
>>>>> + AUDIO_PDATA(_name##_div))
>>>>> +
>>>>> +AUDIO_MST_SCLK(audio_mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_a_mclk));
>>>>> +AUDIO_MST_SCLK(audio_mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_b_mclk));
>>>>> +AUDIO_MST_SCLK(audio_mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_c_mclk));
>>>>> +AUDIO_MST_SCLK(audio_mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_d_mclk));
>>>>> +
>>>>> +AUDIO_MST_LRCLK(audio_mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_a_sclk_post_en));
>>>>> +AUDIO_MST_LRCLK(audio_mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_b_sclk_post_en));
>>>>> +AUDIO_MST_LRCLK(audio_mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_c_sclk_post_en));
>>>>> +AUDIO_MST_LRCLK(audio_mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>> + AUDIO_PDATA(audio_mst_d_sclk_post_en));
>>>>> +
>>>>> +static const struct clk_parent_data a1_mst_sclk_pdata[] = {
>>>>> + { .hw = &audio_mst_a_sclk.hw },
>>>>> + { .hw = &audio_mst_b_sclk.hw },
>>>>> + { .hw = &audio_mst_c_sclk.hw },
>>>>> + { .hw = &audio_mst_d_sclk.hw },
>>>>> + { .fw_name = "slv_sclk0" },
>>>>> + { .fw_name = "slv_sclk1" },
>>>>> + { .fw_name = "slv_sclk2" },
>>>>> + { .fw_name = "slv_sclk3" },
>>>>> + { .fw_name = "slv_sclk4" },
>>>>> + { .fw_name = "slv_sclk5" },
>>>>> + { .fw_name = "slv_sclk6" },
>>>>> + { .fw_name = "slv_sclk7" },
>>>>> + { .fw_name = "slv_sclk8" },
>>>>> + { .fw_name = "slv_sclk9" },
>>>>> +};
>>>>> +
>>>>> +static const struct clk_parent_data a1_mst_lrclk_pdata[] = {
>>>>> + { .hw = &audio_mst_a_lrclk.hw },
>>>>> + { .hw = &audio_mst_b_lrclk.hw },
>>>>> + { .hw = &audio_mst_c_lrclk.hw },
>>>>> + { .hw = &audio_mst_d_lrclk.hw },
>>>>> + { .fw_name = "slv_lrclk0" },
>>>>> + { .fw_name = "slv_lrclk1" },
>>>>> + { .fw_name = "slv_lrclk2" },
>>>>> + { .fw_name = "slv_lrclk3" },
>>>>> + { .fw_name = "slv_lrclk4" },
>>>>> + { .fw_name = "slv_lrclk5" },
>>>>> + { .fw_name = "slv_lrclk6" },
>>>>> + { .fw_name = "slv_lrclk7" },
>>>>> + { .fw_name = "slv_lrclk8" },
>>>>> + { .fw_name = "slv_lrclk9" },
>>>>> +};
>>>>> +
>>>>> +#define AUDIO_TDM_SCLK(_name, _reg) \
>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0xf, 24, a1_mst_sclk_pdata); \
>>>>> + AUDIO_GATE(_name##_pre_en, (_reg), 31, \
>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>> + AUDIO_GATE(_name##_post_en, (_reg), 30, \
>>>>> + AUDIO_PDATA(_name##_pre_en)); \
>>>>> + AUDIO_SCLK_WS(_name, (_reg), 1, 29, 28, \
>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>> +
>>>>> +#define AUDIO_TDM_LRCLK(_name, _reg) \
>>>>> + AUDIO_MUX(_name, (_reg), 0xf, 20, a1_mst_lrclk_pdata)
>>>>> +
>>>>> +AUDIO_TDM_SCLK(audio_tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>> +AUDIO_TDM_SCLK(audio_tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>> +AUDIO_TDM_SCLK(audio_tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>> +AUDIO_TDM_SCLK(audio_tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>> +AUDIO_TDM_SCLK(audio_tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>> +
>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_a_lrclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_b_lrclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_lb_lrclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_a_lrclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_b_lrclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>> +
>>>>> +static struct clk_hw *a1_audio_hw_clks[] = {
>>>>> + [AUD_CLKID_DDR_ARB] = &audio_ddr_arb.hw,
>>>>> + [AUD_CLKID_TDMIN_A] = &audio_tdmin_a.hw,
>>>>> + [AUD_CLKID_TDMIN_B] = &audio_tdmin_b.hw,
>>>>> + [AUD_CLKID_TDMIN_LB] = &audio_tdmin_lb.hw,
>>>>> + [AUD_CLKID_LOOPBACK] = &audio_loopback.hw,
>>>>> + [AUD_CLKID_TDMOUT_A] = &audio_tdmout_a.hw,
>>>>> + [AUD_CLKID_TDMOUT_B] = &audio_tdmout_b.hw,
>>>>> + [AUD_CLKID_FRDDR_A] = &audio_frddr_a.hw,
>>>>> + [AUD_CLKID_FRDDR_B] = &audio_frddr_b.hw,
>>>>> + [AUD_CLKID_TODDR_A] = &audio_toddr_a.hw,
>>>>> + [AUD_CLKID_TODDR_B] = &audio_toddr_b.hw,
>>>>> + [AUD_CLKID_SPDIFIN] = &audio_spdifin.hw,
>>>>> + [AUD_CLKID_RESAMPLE] = &audio_resample.hw,
>>>>> + [AUD_CLKID_EQDRC] = &audio_eqdrc.hw,
>>>>> + [AUD_CLKID_LOCKER] = &audio_audiolocker.hw,
>>>>> + [AUD_CLKID_MST_A_MCLK_SEL] = &audio_mst_a_mclk_mux.hw,
>>>>> + [AUD_CLKID_MST_A_MCLK_DIV] = &audio_mst_a_mclk_div.hw,
>>>>> + [AUD_CLKID_MST_A_MCLK] = &audio_mst_a_mclk.hw,
>>>>> + [AUD_CLKID_MST_B_MCLK_SEL] = &audio_mst_b_mclk_mux.hw,
>>>>> + [AUD_CLKID_MST_B_MCLK_DIV] = &audio_mst_b_mclk_div.hw,
>>>>> + [AUD_CLKID_MST_B_MCLK] = &audio_mst_b_mclk.hw,
>>>>> + [AUD_CLKID_MST_C_MCLK_SEL] = &audio_mst_c_mclk_mux.hw,
>>>>> + [AUD_CLKID_MST_C_MCLK_DIV] = &audio_mst_c_mclk_div.hw,
>>>>> + [AUD_CLKID_MST_C_MCLK] = &audio_mst_c_mclk.hw,
>>>>> + [AUD_CLKID_MST_D_MCLK_SEL] = &audio_mst_d_mclk_mux.hw,
>>>>> + [AUD_CLKID_MST_D_MCLK_DIV] = &audio_mst_d_mclk_div.hw,
>>>>> + [AUD_CLKID_MST_D_MCLK] = &audio_mst_d_mclk.hw,
>>>>> + [AUD_CLKID_RESAMPLE_CLK_SEL] = &audio_resample_clk_mux.hw,
>>>>> + [AUD_CLKID_RESAMPLE_CLK_DIV] = &audio_resample_clk_div.hw,
>>>>> + [AUD_CLKID_RESAMPLE_CLK] = &audio_resample_clk.hw,
>>>>> + [AUD_CLKID_LOCKER_IN_CLK_SEL] = &audio_locker_in_clk_mux.hw,
>>>>> + [AUD_CLKID_LOCKER_IN_CLK_DIV] = &audio_locker_in_clk_div.hw,
>>>>> + [AUD_CLKID_LOCKER_IN_CLK] = &audio_locker_in_clk.hw,
>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_SEL] = &audio_locker_out_clk_mux.hw,
>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_DIV] = &audio_locker_out_clk_div.hw,
>>>>> + [AUD_CLKID_LOCKER_OUT_CLK] = &audio_locker_out_clk.hw,
>>>>> + [AUD_CLKID_SPDIFIN_CLK_SEL] = &audio_spdifin_clk_mux.hw,
>>>>> + [AUD_CLKID_SPDIFIN_CLK_DIV] = &audio_spdifin_clk_div.hw,
>>>>> + [AUD_CLKID_SPDIFIN_CLK] = &audio_spdifin_clk.hw,
>>>>> + [AUD_CLKID_EQDRC_CLK_SEL] = &audio_eqdrc_clk_mux.hw,
>>>>> + [AUD_CLKID_EQDRC_CLK_DIV] = &audio_eqdrc_clk_div.hw,
>>>>> + [AUD_CLKID_EQDRC_CLK] = &audio_eqdrc_clk.hw,
>>>>> + [AUD_CLKID_MST_A_SCLK_PRE_EN] = &audio_mst_a_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_MST_A_SCLK_DIV] = &audio_mst_a_sclk_div.hw,
>>>>> + [AUD_CLKID_MST_A_SCLK_POST_EN] = &audio_mst_a_sclk_post_en.hw,
>>>>> + [AUD_CLKID_MST_A_SCLK] = &audio_mst_a_sclk.hw,
>>>>> + [AUD_CLKID_MST_B_SCLK_PRE_EN] = &audio_mst_b_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_MST_B_SCLK_DIV] = &audio_mst_b_sclk_div.hw,
>>>>> + [AUD_CLKID_MST_B_SCLK_POST_EN] = &audio_mst_b_sclk_post_en.hw,
>>>>> + [AUD_CLKID_MST_B_SCLK] = &audio_mst_b_sclk.hw,
>>>>> + [AUD_CLKID_MST_C_SCLK_PRE_EN] = &audio_mst_c_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_MST_C_SCLK_DIV] = &audio_mst_c_sclk_div.hw,
>>>>> + [AUD_CLKID_MST_C_SCLK_POST_EN] = &audio_mst_c_sclk_post_en.hw,
>>>>> + [AUD_CLKID_MST_C_SCLK] = &audio_mst_c_sclk.hw,
>>>>> + [AUD_CLKID_MST_D_SCLK_PRE_EN] = &audio_mst_d_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_MST_D_SCLK_DIV] = &audio_mst_d_sclk_div.hw,
>>>>> + [AUD_CLKID_MST_D_SCLK_POST_EN] = &audio_mst_d_sclk_post_en.hw,
>>>>> + [AUD_CLKID_MST_D_SCLK] = &audio_mst_d_sclk.hw,
>>>>> + [AUD_CLKID_MST_A_LRCLK_DIV] = &audio_mst_a_lrclk_div.hw,
>>>>> + [AUD_CLKID_MST_A_LRCLK] = &audio_mst_a_lrclk.hw,
>>>>> + [AUD_CLKID_MST_B_LRCLK_DIV] = &audio_mst_b_lrclk_div.hw,
>>>>> + [AUD_CLKID_MST_B_LRCLK] = &audio_mst_b_lrclk.hw,
>>>>> + [AUD_CLKID_MST_C_LRCLK_DIV] = &audio_mst_c_lrclk_div.hw,
>>>>> + [AUD_CLKID_MST_C_LRCLK] = &audio_mst_c_lrclk.hw,
>>>>> + [AUD_CLKID_MST_D_LRCLK_DIV] = &audio_mst_d_lrclk_div.hw,
>>>>> + [AUD_CLKID_MST_D_LRCLK] = &audio_mst_d_lrclk.hw,
>>>>> + [AUD_CLKID_TDMIN_A_SCLK_SEL] = &audio_tdmin_a_sclk_mux.hw,
>>>>> + [AUD_CLKID_TDMIN_A_SCLK_PRE_EN] = &audio_tdmin_a_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_TDMIN_A_SCLK_POST_EN] = &audio_tdmin_a_sclk_post_en.hw,
>>>>> + [AUD_CLKID_TDMIN_A_SCLK] = &audio_tdmin_a_sclk.hw,
>>>>> + [AUD_CLKID_TDMIN_A_LRCLK] = &audio_tdmin_a_lrclk.hw,
>>>>> + [AUD_CLKID_TDMIN_B_SCLK_SEL] = &audio_tdmin_b_sclk_mux.hw,
>>>>> + [AUD_CLKID_TDMIN_B_SCLK_PRE_EN] = &audio_tdmin_b_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_TDMIN_B_SCLK_POST_EN] = &audio_tdmin_b_sclk_post_en.hw,
>>>>> + [AUD_CLKID_TDMIN_B_SCLK] = &audio_tdmin_b_sclk.hw,
>>>>> + [AUD_CLKID_TDMIN_B_LRCLK] = &audio_tdmin_b_lrclk.hw,
>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_SEL] = &audio_tdmin_lb_sclk_mux.hw,
>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_PRE_EN] = &audio_tdmin_lb_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_POST_EN] = &audio_tdmin_lb_sclk_post_en.hw,
>>>>> + [AUD_CLKID_TDMIN_LB_SCLK] = &audio_tdmin_lb_sclk.hw,
>>>>> + [AUD_CLKID_TDMIN_LB_LRCLK] = &audio_tdmin_lb_lrclk.hw,
>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_SEL] = &audio_tdmout_a_sclk_mux.hw,
>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_PRE_EN] = &audio_tdmout_a_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_POST_EN] = &audio_tdmout_a_sclk_post_en.hw,
>>>>> + [AUD_CLKID_TDMOUT_A_SCLK] = &audio_tdmout_a_sclk.hw,
>>>>> + [AUD_CLKID_TDMOUT_A_LRCLK] = &audio_tdmout_a_lrclk.hw,
>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_SEL] = &audio_tdmout_b_sclk_mux.hw,
>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_PRE_EN] = &audio_tdmout_b_sclk_pre_en.hw,
>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_POST_EN] = &audio_tdmout_b_sclk_post_en.hw,
>>>>> + [AUD_CLKID_TDMOUT_B_SCLK] = &audio_tdmout_b_sclk.hw,
>>>>> + [AUD_CLKID_TDMOUT_B_LRCLK] = &audio_tdmout_b_lrclk.hw,
>>>>> +
>>>>> + [AUD2_CLKID_DDR_ARB] = &audio2_ddr_arb.hw,
>>>>> + [AUD2_CLKID_PDM] = &audio2_pdm.hw,
>>>>> + [AUD2_CLKID_TDMIN_VAD] = &audio2_tdmin_vad.hw,
>>>>> + [AUD2_CLKID_TODDR_VAD] = &audio2_toddr_vad.hw,
>>>>> + [AUD2_CLKID_VAD] = &audio2_vad.hw,
>>>>> + [AUD2_CLKID_AUDIOTOP] = &audio2_audiotop.hw,
>>>>> + [AUD2_CLKID_VAD_MCLK_SEL] = &audio2_vad_mclk_mux.hw,
>>>>> + [AUD2_CLKID_VAD_MCLK_DIV] = &audio2_vad_mclk_div.hw,
>>>>> + [AUD2_CLKID_VAD_MCLK] = &audio2_vad_mclk.hw,
>>>>> + [AUD2_CLKID_VAD_CLK_SEL] = &audio2_vad_clk_mux.hw,
>>>>> + [AUD2_CLKID_VAD_CLK_DIV] = &audio2_vad_clk_div.hw,
>>>>> + [AUD2_CLKID_VAD_CLK] = &audio2_vad_clk.hw,
>>>>> + [AUD2_CLKID_PDM_DCLK_SEL] = &audio2_pdm_dclk_mux.hw,
>>>>> + [AUD2_CLKID_PDM_DCLK_DIV] = &audio2_pdm_dclk_div.hw,
>>>>> + [AUD2_CLKID_PDM_DCLK] = &audio2_pdm_dclk.hw,
>>>>> + [AUD2_CLKID_PDM_SYSCLK_SEL] = &audio2_pdm_sysclk_mux.hw,
>>>>> + [AUD2_CLKID_PDM_SYSCLK_DIV] = &audio2_pdm_sysclk_div.hw,
>>>>> + [AUD2_CLKID_PDM_SYSCLK] = &audio2_pdm_sysclk.hw,
>>>>> +};
>>>>> +
>>>>> +static struct meson_clk_hw_data a1_audio_clks = {
>>>>> + .hws = a1_audio_hw_clks,
>>>>> + .num = ARRAY_SIZE(a1_audio_hw_clks),
>>>>> +};
>>>>> +
>>>>> +static struct regmap *a1_audio_map(struct platform_device *pdev,
>>>>> + unsigned int index)
>>>>> +{
>>>>> + char name[32];
>>>>> + const struct regmap_config cfg = {
>>>>> + .reg_bits = 32,
>>>>> + .val_bits = 32,
>>>>> + .reg_stride = 4,
>>>>> + .name = name,
>>>>
>>>> Not necessary
>>>>
>>>
>>> This implementation uses two regmaps, and this field allow to avoid
>>> errors like this:
>>>
>>> [ 0.145530] debugfs: Directory 'fe050000.audio-clock-controller' with
>>> parent 'regmap' already present!
>>>
>>>>> + };
>>>>> + void __iomem *base;
>>>>> +
>>>>> + base = devm_platform_ioremap_resource(pdev, index);
>>>>> + if (IS_ERR(base))
>>>>> + return base;
>>>>> +
>>>>> + scnprintf(name, sizeof(name), "%d", index);
>>>>> + return devm_regmap_init_mmio(&pdev->dev, base, &cfg);
>>>>> +}
>>>>
>>>> That is overengineered. Please keep it simple. Declare the regmap_config
>>>> as static const global, and do it like axg-audio please.
>>>>
>>>
>>> This only reason why it is not "static const" because I need to set
>>> unique name for each regmap.
>>>
>>>>> +
>>>>> +static int a1_register_clk(struct platform_device *pdev,
>>>>> + struct regmap *map0, struct regmap *map1,
>>>>> + struct clk_hw *hw)
>>>>> +{
>>>>> + struct clk_regmap *clk = container_of(hw, struct clk_regmap, hw);
>>>>> +
>>>>> + if (!hw)
>>>>> + return 0;
>>>>> +
>>>>> + switch ((unsigned long)clk->map) {
>>>>> + case AUDIO_RANGE_0:
>>>>> + clk->map = map0;
>>>>> + break;
>>>>> + case AUDIO_RANGE_1:
>>>>> + clk->map = map1;
>>>>> + break;
>>>>
>>>> ... fishy
>>>>
>>>>> + default:
>>>>> + WARN_ON(1);
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + return devm_clk_hw_register(&pdev->dev, hw);
>>>>> +}
>>>>> +
>>>>> +static int a1_audio_clkc_probe(struct platform_device *pdev)
>>>>> +{
>>>>> + struct regmap *map0, *map1;
>>>>> + struct clk *clk;
>>>>> + unsigned int i;
>>>>> + int ret;
>>>>> +
>>>>> + clk = devm_clk_get_enabled(&pdev->dev, "pclk");
>>>>> + if (WARN_ON(IS_ERR(clk)))
>>>>> + return PTR_ERR(clk);
>>>>> +
>>>>> + map0 = a1_audio_map(pdev, 0);
>>>>> + if (IS_ERR(map0))
>>>>> + return PTR_ERR(map0);
>>>>> +
>>>>> + map1 = a1_audio_map(pdev, 1);
>>>>> + if (IS_ERR(map1))
>>>>> + return PTR_ERR(map1);
>>>>
>>>> No - Looks to me you just have two clock controllers you are trying
>>>> force into one.
>>>>
>>>
>>> See the begining.
>>>
>>>>> +
>>>>> + /*
>>>>> + * Register and enable AUD2_CLKID_AUDIOTOP clock first. Unless
>>>>> + * it is enabled any read/write to 'map0' hangs the CPU.
>>>>> + */
>>>>> +
>>>>> + ret = a1_register_clk(pdev, map0, map1,
>>>>> + a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ret = clk_prepare_enable(a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]->clk);
>>>>> + if (ret)
>>>>> + return ret;
>>>>
>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>
>>>
>>> See the begining.
>>>
>>>>> +
>>>>> + for (i = 0; i < a1_audio_clks.num; i++) {
>>>>> + if (i == AUD2_CLKID_AUDIOTOP)
>>>>> + continue;
>>>>> +
>>>>> + ret = a1_register_clk(pdev, map0, map1, a1_audio_clks.hws[i]);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + ret = devm_of_clk_add_hw_provider(&pdev->dev, meson_clk_hw_get,
>>>>> + &a1_audio_clks);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + BUILD_BUG_ON((unsigned long)AUDIO_REG_MAP(AUDIO_SW_RESET0) !=
>>>>> + AUDIO_RANGE_0);
>>>>
>>>> Why is that necessary ?
>>>>
>>>
>>> A little paranoia. Here AUDIO_SW_RESET0 is handled as map0's register,
>>> and I want to assert it.
>>>
>>>>> + return meson_audio_rstc_register(&pdev->dev, map0,
>>>>> + AUDIO_REG_OFFSET(AUDIO_SW_RESET0), 32);
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id a1_audio_clkc_match_table[] = {
>>>>> + { .compatible = "amlogic,a1-audio-clkc", },
>>>>> + {}
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, a1_audio_clkc_match_table);
>>>>> +
>>>>> +static struct platform_driver a1_audio_clkc_driver = {
>>>>> + .probe = a1_audio_clkc_probe,
>>>>> + .driver = {
>>>>> + .name = "a1-audio-clkc",
>>>>> + .of_match_table = a1_audio_clkc_match_table,
>>>>> + },
>>>>> +};
>>>>> +module_platform_driver(a1_audio_clkc_driver);
>>>>> +
>>>>> +MODULE_DESCRIPTION("Amlogic A1 Audio Clock driver");
>>>>> +MODULE_AUTHOR("Jan Dakinevich <jan.dakinevich@salutedevices.com>");
>>>>> +MODULE_LICENSE("GPL");
>>>>> diff --git a/drivers/clk/meson/a1-audio.h b/drivers/clk/meson/a1-audio.h
>>>>> new file mode 100644
>>>>> index 000000000000..f994e87276cd
>>>>> --- /dev/null
>>>>> +++ b/drivers/clk/meson/a1-audio.h
>>>>> @@ -0,0 +1,58 @@
>>>>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>>>>> +/*
>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>> + *
>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>> + */
>>>>> +
>>>>> +#ifndef __A1_AUDIO_H
>>>>> +#define __A1_AUDIO_H
>>>>> +
>>>>> +#define AUDIO_RANGE_0 0xa
>>>>> +#define AUDIO_RANGE_1 0xb
>>>>> +#define AUDIO_RANGE_SHIFT 16
>>>>> +
>>>>> +#define AUDIO_REG(_range, _offset) \
>>>>> + (((_range) << AUDIO_RANGE_SHIFT) + (_offset))
>>>>> +
>>>>> +#define AUDIO_REG_OFFSET(_reg) \
>>>>> + ((_reg) & ((1 << AUDIO_RANGE_SHIFT) - 1))
>>>>> +
>>>>> +#define AUDIO_REG_MAP(_reg) \
>>>>> + ((void *)((_reg) >> AUDIO_RANGE_SHIFT))
>>>>
>>>> That is seriouly overengineered.
>>>> The following are offset. Just write what they are.
>>>>
>>>
>>> This is all in order to keep range's identifier together with offset and
>>> then use it to store the identifier in clk_regmaps.
>>>
>>>> There is not reason to put that into a header. It is only going to be
>>>> used by a single driver.
>>>>>> +
>>>>> +#define AUDIO_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_0, 0x000)
>>>>> +#define AUDIO_MCLK_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x008)
>>>>> +#define AUDIO_MCLK_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x00c)
>>>>> +#define AUDIO_MCLK_C_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x010)
>>>>> +#define AUDIO_MCLK_D_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x014)
>>>>> +#define AUDIO_MCLK_E_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x018)
>>>>> +#define AUDIO_MCLK_F_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x01c)
>>>>> +#define AUDIO_SW_RESET0 AUDIO_REG(AUDIO_RANGE_0, 0x028)
>>>>> +#define AUDIO_MST_A_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x040)
>>>>> +#define AUDIO_MST_A_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x044)
>>>>> +#define AUDIO_MST_B_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x048)
>>>>> +#define AUDIO_MST_B_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x04c)
>>>>> +#define AUDIO_MST_C_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x050)
>>>>> +#define AUDIO_MST_C_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x054)
>>>>> +#define AUDIO_MST_D_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x058)
>>>>> +#define AUDIO_MST_D_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x05c)
>>>>> +#define AUDIO_CLK_TDMIN_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x080)
>>>>> +#define AUDIO_CLK_TDMIN_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x084)
>>>>> +#define AUDIO_CLK_TDMIN_LB_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x08c)
>>>>> +#define AUDIO_CLK_TDMOUT_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x090)
>>>>> +#define AUDIO_CLK_TDMOUT_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x094)
>>>>> +#define AUDIO_CLK_SPDIFIN_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x09c)
>>>>> +#define AUDIO_CLK_RESAMPLE_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a4)
>>>>> +#define AUDIO_CLK_LOCKER_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a8)
>>>>> +#define AUDIO_CLK_EQDRC_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0c0)
>>>>> +
>>>>> +#define AUDIO2_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_1, 0x00c)
>>>>> +#define AUDIO2_MCLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x040)
>>>>> +#define AUDIO2_CLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x044)
>>>>> +#define AUDIO2_CLK_PDMIN_CTRL0 AUDIO_REG(AUDIO_RANGE_1, 0x058)
>>>>> +#define AUDIO2_CLK_PDMIN_CTRL1 AUDIO_REG(AUDIO_RANGE_1, 0x05c)
>>>>> +
>>>>> +#include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
>>>>> +
>>>>> +#endif /* __A1_AUDIO_H */
>>>>
>>>>
>>
>>
--
Jerome
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 19/23] dt-bindings: crypto: meson: support new SoC's
From: Conor Dooley @ 2024-03-26 18:04 UTC (permalink / raw)
To: Alexey Romanov
Cc: neil.armstrong, clabbe, herbert, davem, robh+dt,
krzysztof.kozlowski+dt, conor+dt, khilman, jbrunet,
martin.blumenstingl, vadim.fedorenko, linux-crypto, linux-amlogic,
linux-kernel, devicetree, linux-arm-kernel, kernel
In-Reply-To: <20240326153219.2915080-20-avromanov@salutedevices.com>
[-- Attachment #1.1: Type: text/plain, Size: 1543 bytes --]
On Tue, Mar 26, 2024 at 06:32:15PM +0300, Alexey Romanov wrote:
> Now crypto module available at G12A/G12B/S4/A1/SM1/AXG.
>
> 1. Add new compatibles:
> - amlogic,g12a-crypto
> - amlogic,axg-crypto
> - amlogic,a1-crypto
> - amlogic,s4-crypto (uses a1-crypto as fallback)
>
> 2. Add power-domains in schema.
>
> Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
> ---
> .../bindings/crypto/amlogic,gxl-crypto.yaml | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> index d3af7b4d5f39..c92edde314aa 100644
> --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml
> @@ -11,8 +11,16 @@ maintainers:
>
> properties:
> compatible:
> - items:
> - - const: amlogic,gxl-crypto
> + oneOf:
> + - items:
> + - enum:
> + - amlogic,s4-crypto
> + - const: amlogic,a1-crypto
> + - enum:
> + - amlogic,gxl-crypto
> + - amlogic,axg-crypto
> + - amlogic,g12a-crypto
> + - amlogic,a1-crypto
>
> reg:
> maxItems: 1
> @@ -21,6 +29,9 @@ properties:
> items:
> - description: Interrupt for flow 0
>
> + power-domains:
> + maxItems: 1
Is power-domains valid for the devices that existed prior to your patch?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox