* Re: [PATCH v4 00/10] Allwinner sunxi message box support
From: Ondřej Jirman @ 2019-09-09 3:22 UTC (permalink / raw)
To: Samuel Holland
Cc: Mark Rutland, devicetree, linux-sunxi, Maxime Ripard,
Michael Turquette, Jassi Brar, linux-kernel, Stephen Boyd,
Chen-Yu Tsai, Rob Herring, Corentin Labbe, linux-clk,
linux-arm-kernel
In-Reply-To: <20190820032311.6506-1-samuel@sholland.org>
Hello Samuel,
On Mon, Aug 19, 2019 at 10:23:01PM -0500, Samuel Holland wrote:
> This series adds support for the "hardware message box" in sun8i, sun9i,
> and sun50i SoCs, used for communication with the ARISC management
> processor (the platform's equivalent of the ARM SCP). The end goal is to
> use the arm_scpi driver as a client, communicating with firmware running
> on the AR100 CPU, or to use the mailbox to forward NMIs that the
> firmware picks up from R_INTC.
>
> Unfortunately, the ARM SCPI client no longer works with this driver
> since it now exposes all 8 hardware FIFOs individually. The SCPI client
> could be made to work (and I posted proof-of-concept code to that effect
> with v1 of this series), but that is a low priority, as Linux does not
> directly use SCPI with the current firmware version; all SCPI use goes
> through ATF via PSCI.
>
> As requested in the comments to v3 of this patchset, a demo client is
> provided in the final patch. This demo goes along with a toy firmware
> which shows that the driver does indeed work for two-way communication
> on all channels. To build the firmware component, run:
I've tried using this driver with mainline arm_scpi driver (which is probably
an expected future use, since crust provides SCPI interface).
The problem I've found is that arm_scpi expects message box to be
bi-directional, but this driver provides uni-directional interface.
What do you think about making this driver provide bi-directional interface?
We could halve the number of channels to 4 and mandate TX/RX configuration
(from main CPU's PoV) as ABI.
Otherwise it's impossible to use it with the arm_scpi driver.
Or do you have any other ideas? I guess arm_scpi can be fixed to add a
property that would make it possible to use single shmem with two
mailboxes, one for rx and one for tx, but making sun6i mailbox have
bi-directional interface sounds easier.
regards,
o.
> git clone https://github.com/crust-firmware/meta meta
> git clone -b mailbox-demo https://github.com/crust-firmware/crust meta/crust
> cd meta
> make
>
> That will by default produce a U-Boot + ATF + SCP firmware image in
> [meta/]build/pinebook/u-boot-sunxi-with-spl.bin. See the top-level
> README.md for more information, such as cross-compiler setup.
>
> I've now used this driver with three separate clients over the past two
> years, and they all work. If there are no remaining concerns with the
> driver, I'd like it to get merged.
>
> Even without the driver, the clock patches (1-2) can go in at any time.
>
> Changes from v3:
> - Rebased on sunxi-next
> - Added Rob's Reviewed-by for patch 3
> - Fixed a crash when receiving a message on a disabled channel
> - Cleaned up some comments/formatting in the driver
> - Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
> - Removed the irqchip example (no longer relevant to the fw design)
> - Added a demo/example client that uses the driver and a toy firmware
>
> Changes from v2:
> - Merge patches 1-3
> - Add a comment in the code explaining the CLK_IS_CRITICAL usage
> - Add a patch to mark the AR100 clocks as critical
> - Use YAML for the device tree binding
> - Include a not-for-merge example usage of the mailbox
>
> Changes from v1:
> - Marked message box clocks as critical instead of hacks in the driver
> - 8 unidirectional channels instead of 4 bidirectional pairs
> - Use per-SoC compatible strings and an A31 fallback compatible
> - Dropped the mailbox framework patch
> - Include DT patches for SoCs that document the message box
>
> Samuel Holland (10):
> clk: sunxi-ng: Mark msgbox clocks as critical
> clk: sunxi-ng: Mark AR100 clocks as critical
> dt-bindings: mailbox: Add a sunxi message box binding
> mailbox: sunxi-msgbox: Add a new mailbox driver
> ARM: dts: sunxi: a80: Add msgbox node
> ARM: dts: sunxi: a83t: Add msgbox node
> ARM: dts: sunxi: h3/h5: Add msgbox node
> arm64: dts: allwinner: a64: Add msgbox node
> arm64: dts: allwinner: h6: Add msgbox node
> [DO NOT MERGE] drivers: firmware: msgbox demo
>
> .../mailbox/allwinner,sunxi-msgbox.yaml | 79 +++++
> arch/arm/boot/dts/sun8i-a83t.dtsi | 10 +
> arch/arm/boot/dts/sun9i-a80.dtsi | 10 +
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 34 ++
> arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 24 ++
> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 +
> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 +-
> drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c | 2 +-
> drivers/clk/sunxi-ng/ccu-sun50i-h6.c | 3 +-
> drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 3 +-
> drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 3 +-
> drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 3 +-
> drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 3 +-
> drivers/clk/sunxi-ng/ccu-sun8i-r.c | 2 +-
> drivers/clk/sunxi-ng/ccu-sun9i-a80.c | 3 +-
> drivers/firmware/Kconfig | 6 +
> drivers/firmware/Makefile | 1 +
> drivers/firmware/sunxi_msgbox_demo.c | 307 +++++++++++++++++
> drivers/mailbox/Kconfig | 10 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/sunxi-msgbox.c | 323 ++++++++++++++++++
> 22 files changed, 842 insertions(+), 9 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
> create mode 100644 drivers/firmware/sunxi_msgbox_demo.c
> create mode 100644 drivers/mailbox/sunxi-msgbox.c
>
> --
> 2.21.0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V3 0/4] clk: imx8m: fix glitch/mux
From: Peng Fan @ 2019-09-09 3:39 UTC (permalink / raw)
To: mturquette@baylibre.com, sboyd@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, festevam@gmail.com
Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jacky Bai
From: Peng Fan <peng.fan@nxp.com>
V3:
Add cover-letter
V2:
Added patch [2,3,4]/4 and avoid glitch when prepare
There is two bypass bit in the pll, BYPASS and EXT_BYPASS.
There is also a restriction that to avoid glitch, need set BYPASS
bit when RESETB changed from 0 to 1, otherwise there will be glitch.
However the BYPASS bit is also used as mux bit in imx8mm/imx8mn clk driver.
This means two paths touch the same bit which is wrong. So switch to use
EXT_BYPASS bit as the mux.
Peng Fan (4):
clk: imx: pll14xx: avoid glitch when set rate
clk: imx: clk-pll14xx: unbypass PLL by default
clk: imx: imx8mm: fix pll mux bit
clk: imx: imx8mn: fix pll mux bit
drivers/clk/imx/clk-imx8mm.c | 32 ++++++++++----------------------
drivers/clk/imx/clk-imx8mn.c | 32 ++++++++++----------------------
drivers/clk/imx/clk-pll14xx.c | 27 ++++++++++++++++++++++++++-
3 files changed, 46 insertions(+), 45 deletions(-)
--
2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V3 1/4] clk: imx: pll14xx: avoid glitch when set rate
From: Peng Fan @ 2019-09-09 3:39 UTC (permalink / raw)
To: mturquette@baylibre.com, sboyd@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, festevam@gmail.com
Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jacky Bai
In-Reply-To: <1568043491-20680-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
According to PLL1443XA and PLL1416X spec,
"When BYPASS is 0 and RESETB is changed from 0 to 1, FOUT starts to
output unstable clock until lock time passes. PLL1416X/PLL1443XA may
generate a glitch at FOUT."
So set BYPASS when RESETB is changed from 0 to 1 to avoid glitch.
In the end of set rate, BYPASS will be cleared.
When prepare clock, also need to take care to avoid glitch. So
we also follow Spec to set BYPASS before RESETB changed from 0 to 1.
And add a check if the RESETB is already 0, directly return 0;
Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
V3:
None
V2:
Avoid glitch when prepare
update commit log
drivers/clk/imx/clk-pll14xx.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index b7213023b238..656f48b002dd 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -191,6 +191,10 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
tmp &= ~RST_MASK;
writel_relaxed(tmp, pll->base);
+ /* Enable BYPASS */
+ tmp |= BYPASS_MASK;
+ writel(tmp, pll->base);
+
div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
(rate->sdiv << SDIV_SHIFT);
writel_relaxed(div_val, pll->base + 0x4);
@@ -250,6 +254,10 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
tmp &= ~RST_MASK;
writel_relaxed(tmp, pll->base);
+ /* Enable BYPASS */
+ tmp |= BYPASS_MASK;
+ writel_relaxed(tmp, pll->base);
+
div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
(rate->sdiv << SDIV_SHIFT);
writel_relaxed(div_val, pll->base + 0x4);
@@ -283,16 +291,28 @@ static int clk_pll14xx_prepare(struct clk_hw *hw)
{
struct clk_pll14xx *pll = to_clk_pll14xx(hw);
u32 val;
+ int ret;
/*
* RESETB = 1 from 0, PLL starts its normal
* operation after lock time
*/
val = readl_relaxed(pll->base + GNRL_CTL);
+ if (val & RST_MASK)
+ return 0;
+ val |= BYPASS_MASK;
+ writel_relaxed(val, pll->base + GNRL_CTL);
val |= RST_MASK;
writel_relaxed(val, pll->base + GNRL_CTL);
- return clk_pll14xx_wait_lock(pll);
+ ret = clk_pll14xx_wait_lock(pll);
+ if (ret)
+ return ret;
+
+ val &= ~BYPASS_MASK;
+ writel_relaxed(val, pll->base + GNRL_CTL);
+
+ return 0;
}
static int clk_pll14xx_is_prepared(struct clk_hw *hw)
--
2.16.4
_______________________________________________
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 V3 2/4] clk: imx: clk-pll14xx: unbypass PLL by default
From: Peng Fan @ 2019-09-09 3:39 UTC (permalink / raw)
To: mturquette@baylibre.com, sboyd@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, festevam@gmail.com
Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jacky Bai
In-Reply-To: <1568043491-20680-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
When registering the PLL, unbypass the PLL.
The PLL has two bypass control bit, BYPASS and EXT_BYPASS.
we will expose EXT_BYPASS to clk driver for mux usage, and keep
BYPASS inside pll14xx usage. The PLL has a restriction that
when M/P change, need to RESET/BYPASS pll to avoid glitch, so
we could not expose BYPASS.
To make it easy for clk driver usage, unbypass PLL which does
not hurt current function.
Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
V3:
None
V2:
New patch
drivers/clk/imx/clk-pll14xx.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 656f48b002dd..7a815ec76aa5 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -368,6 +368,7 @@ struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
struct clk_pll14xx *pll;
struct clk *clk;
struct clk_init_data init;
+ u32 val;
pll = kzalloc(sizeof(*pll), GFP_KERNEL);
if (!pll)
@@ -399,6 +400,10 @@ struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
pll->rate_table = pll_clk->rate_table;
pll->rate_count = pll_clk->rate_count;
+ val = readl_relaxed(pll->base + GNRL_CTL);
+ val &= ~BYPASS_MASK;
+ writel_relaxed(val, pll->base + GNRL_CTL);
+
clk = clk_register(NULL, &pll->hw);
if (IS_ERR(clk)) {
pr_err("%s: failed to register pll %s %lu\n",
--
2.16.4
_______________________________________________
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 V3 3/4] clk: imx: imx8mm: fix pll mux bit
From: Peng Fan @ 2019-09-09 3:39 UTC (permalink / raw)
To: mturquette@baylibre.com, sboyd@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, festevam@gmail.com
Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jacky Bai
In-Reply-To: <1568043491-20680-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
pll BYPASS bit should be kept inside pll driver for glitchless freq
setting following spec. If exposing the bit, that means pll driver and
clk driver has two paths to touch this bit, which is wrong.
So use EXT_BYPASS bit here.
And drop uneeded set parent, because EXT_BYPASS default is 0.
Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
Suggested-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
V3:
None
V2:
New patch
drivers/clk/imx/clk-imx8mm.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 2758e3f0d15d..067ab876911d 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -408,28 +408,16 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
clks[IMX8MM_SYS_PLL3] = imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel", base + 0x114, &imx8mm_sys_pll);
/* PLL bypass out */
- clks[IMX8MM_AUDIO_PLL1_BYPASS] = imx_clk_mux_flags("audio_pll1_bypass", base, 4, 1, audio_pll1_bypass_sels, ARRAY_SIZE(audio_pll1_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_AUDIO_PLL2_BYPASS] = imx_clk_mux_flags("audio_pll2_bypass", base + 0x14, 4, 1, audio_pll2_bypass_sels, ARRAY_SIZE(audio_pll2_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_VIDEO_PLL1_BYPASS] = imx_clk_mux_flags("video_pll1_bypass", base + 0x28, 4, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_DRAM_PLL_BYPASS] = imx_clk_mux_flags("dram_pll_bypass", base + 0x50, 4, 1, dram_pll_bypass_sels, ARRAY_SIZE(dram_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_GPU_PLL_BYPASS] = imx_clk_mux_flags("gpu_pll_bypass", base + 0x64, 4, 1, gpu_pll_bypass_sels, ARRAY_SIZE(gpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_VPU_PLL_BYPASS] = imx_clk_mux_flags("vpu_pll_bypass", base + 0x74, 4, 1, vpu_pll_bypass_sels, ARRAY_SIZE(vpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_ARM_PLL_BYPASS] = imx_clk_mux_flags("arm_pll_bypass", base + 0x84, 4, 1, arm_pll_bypass_sels, ARRAY_SIZE(arm_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_SYS_PLL1_BYPASS] = imx_clk_mux_flags("sys_pll1_bypass", base + 0x94, 4, 1, sys_pll1_bypass_sels, ARRAY_SIZE(sys_pll1_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_SYS_PLL2_BYPASS] = imx_clk_mux_flags("sys_pll2_bypass", base + 0x104, 4, 1, sys_pll2_bypass_sels, ARRAY_SIZE(sys_pll2_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MM_SYS_PLL3_BYPASS] = imx_clk_mux_flags("sys_pll3_bypass", base + 0x114, 4, 1, sys_pll3_bypass_sels, ARRAY_SIZE(sys_pll3_bypass_sels), CLK_SET_RATE_PARENT);
-
- /* unbypass all the plls */
- clk_set_parent(clks[IMX8MM_AUDIO_PLL1_BYPASS], clks[IMX8MM_AUDIO_PLL1]);
- clk_set_parent(clks[IMX8MM_AUDIO_PLL2_BYPASS], clks[IMX8MM_AUDIO_PLL2]);
- clk_set_parent(clks[IMX8MM_VIDEO_PLL1_BYPASS], clks[IMX8MM_VIDEO_PLL1]);
- clk_set_parent(clks[IMX8MM_DRAM_PLL_BYPASS], clks[IMX8MM_DRAM_PLL]);
- clk_set_parent(clks[IMX8MM_GPU_PLL_BYPASS], clks[IMX8MM_GPU_PLL]);
- clk_set_parent(clks[IMX8MM_VPU_PLL_BYPASS], clks[IMX8MM_VPU_PLL]);
- clk_set_parent(clks[IMX8MM_ARM_PLL_BYPASS], clks[IMX8MM_ARM_PLL]);
- clk_set_parent(clks[IMX8MM_SYS_PLL1_BYPASS], clks[IMX8MM_SYS_PLL1]);
- clk_set_parent(clks[IMX8MM_SYS_PLL2_BYPASS], clks[IMX8MM_SYS_PLL2]);
- clk_set_parent(clks[IMX8MM_SYS_PLL3_BYPASS], clks[IMX8MM_SYS_PLL3]);
+ clks[IMX8MM_AUDIO_PLL1_BYPASS] = imx_clk_mux_flags("audio_pll1_bypass", base, 16, 1, audio_pll1_bypass_sels, ARRAY_SIZE(audio_pll1_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_AUDIO_PLL2_BYPASS] = imx_clk_mux_flags("audio_pll2_bypass", base + 0x14, 16, 1, audio_pll2_bypass_sels, ARRAY_SIZE(audio_pll2_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_VIDEO_PLL1_BYPASS] = imx_clk_mux_flags("video_pll1_bypass", base + 0x28, 16, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_DRAM_PLL_BYPASS] = imx_clk_mux_flags("dram_pll_bypass", base + 0x50, 16, 1, dram_pll_bypass_sels, ARRAY_SIZE(dram_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_GPU_PLL_BYPASS] = imx_clk_mux_flags("gpu_pll_bypass", base + 0x64, 28, 1, gpu_pll_bypass_sels, ARRAY_SIZE(gpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_VPU_PLL_BYPASS] = imx_clk_mux_flags("vpu_pll_bypass", base + 0x74, 28, 1, vpu_pll_bypass_sels, ARRAY_SIZE(vpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_ARM_PLL_BYPASS] = imx_clk_mux_flags("arm_pll_bypass", base + 0x84, 28, 1, arm_pll_bypass_sels, ARRAY_SIZE(arm_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_SYS_PLL1_BYPASS] = imx_clk_mux_flags("sys_pll1_bypass", base + 0x94, 28, 1, sys_pll1_bypass_sels, ARRAY_SIZE(sys_pll1_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_SYS_PLL2_BYPASS] = imx_clk_mux_flags("sys_pll2_bypass", base + 0x104, 28, 1, sys_pll2_bypass_sels, ARRAY_SIZE(sys_pll2_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MM_SYS_PLL3_BYPASS] = imx_clk_mux_flags("sys_pll3_bypass", base + 0x114, 28, 1, sys_pll3_bypass_sels, ARRAY_SIZE(sys_pll3_bypass_sels), CLK_SET_RATE_PARENT);
/* PLL out gate */
clks[IMX8MM_AUDIO_PLL1_OUT] = imx_clk_gate("audio_pll1_out", "audio_pll1_bypass", base, 13);
--
2.16.4
_______________________________________________
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 V3 4/4] clk: imx: imx8mn: fix pll mux bit
From: Peng Fan @ 2019-09-09 3:39 UTC (permalink / raw)
To: mturquette@baylibre.com, sboyd@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, festevam@gmail.com
Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jacky Bai
In-Reply-To: <1568043491-20680-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
pll BYPASS bit should be kept inside pll driver for glitchless freq
setting following spec. If exposing the bit, that means pll driver and
clk driver has two paths to touch this bit, which is wrong.
So use EXT_BYPASS bit here.
And drop uneeded set parent, because EXT_BYPASS default is 0.
Suggested-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
V3:
None
V2:
New patch
drivers/clk/imx/clk-imx8mn.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index f41116d59749..f767d18679ea 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -421,28 +421,16 @@ static int imx8mn_clocks_probe(struct platform_device *pdev)
clks[IMX8MN_SYS_PLL3] = imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel", base + 0x114, &imx8mn_sys_pll);
/* PLL bypass out */
- clks[IMX8MN_AUDIO_PLL1_BYPASS] = imx_clk_mux_flags("audio_pll1_bypass", base, 4, 1, audio_pll1_bypass_sels, ARRAY_SIZE(audio_pll1_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_AUDIO_PLL2_BYPASS] = imx_clk_mux_flags("audio_pll2_bypass", base + 0x14, 4, 1, audio_pll2_bypass_sels, ARRAY_SIZE(audio_pll2_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_VIDEO_PLL1_BYPASS] = imx_clk_mux_flags("video_pll1_bypass", base + 0x28, 4, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_DRAM_PLL_BYPASS] = imx_clk_mux_flags("dram_pll_bypass", base + 0x50, 4, 1, dram_pll_bypass_sels, ARRAY_SIZE(dram_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_GPU_PLL_BYPASS] = imx_clk_mux_flags("gpu_pll_bypass", base + 0x64, 4, 1, gpu_pll_bypass_sels, ARRAY_SIZE(gpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_VPU_PLL_BYPASS] = imx_clk_mux_flags("vpu_pll_bypass", base + 0x74, 4, 1, vpu_pll_bypass_sels, ARRAY_SIZE(vpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_ARM_PLL_BYPASS] = imx_clk_mux_flags("arm_pll_bypass", base + 0x84, 4, 1, arm_pll_bypass_sels, ARRAY_SIZE(arm_pll_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_SYS_PLL1_BYPASS] = imx_clk_mux_flags("sys_pll1_bypass", base + 0x94, 4, 1, sys_pll1_bypass_sels, ARRAY_SIZE(sys_pll1_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_SYS_PLL2_BYPASS] = imx_clk_mux_flags("sys_pll2_bypass", base + 0x104, 4, 1, sys_pll2_bypass_sels, ARRAY_SIZE(sys_pll2_bypass_sels), CLK_SET_RATE_PARENT);
- clks[IMX8MN_SYS_PLL3_BYPASS] = imx_clk_mux_flags("sys_pll3_bypass", base + 0x114, 4, 1, sys_pll3_bypass_sels, ARRAY_SIZE(sys_pll3_bypass_sels), CLK_SET_RATE_PARENT);
-
- /* unbypass all the plls */
- clk_set_parent(clks[IMX8MN_AUDIO_PLL1_BYPASS], clks[IMX8MN_AUDIO_PLL1]);
- clk_set_parent(clks[IMX8MN_AUDIO_PLL2_BYPASS], clks[IMX8MN_AUDIO_PLL2]);
- clk_set_parent(clks[IMX8MN_VIDEO_PLL1_BYPASS], clks[IMX8MN_VIDEO_PLL1]);
- clk_set_parent(clks[IMX8MN_DRAM_PLL_BYPASS], clks[IMX8MN_DRAM_PLL]);
- clk_set_parent(clks[IMX8MN_GPU_PLL_BYPASS], clks[IMX8MN_GPU_PLL]);
- clk_set_parent(clks[IMX8MN_VPU_PLL_BYPASS], clks[IMX8MN_VPU_PLL]);
- clk_set_parent(clks[IMX8MN_ARM_PLL_BYPASS], clks[IMX8MN_ARM_PLL]);
- clk_set_parent(clks[IMX8MN_SYS_PLL1_BYPASS], clks[IMX8MN_SYS_PLL1]);
- clk_set_parent(clks[IMX8MN_SYS_PLL2_BYPASS], clks[IMX8MN_SYS_PLL2]);
- clk_set_parent(clks[IMX8MN_SYS_PLL3_BYPASS], clks[IMX8MN_SYS_PLL3]);
+ clks[IMX8MN_AUDIO_PLL1_BYPASS] = imx_clk_mux_flags("audio_pll1_bypass", base, 16, 1, audio_pll1_bypass_sels, ARRAY_SIZE(audio_pll1_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_AUDIO_PLL2_BYPASS] = imx_clk_mux_flags("audio_pll2_bypass", base + 0x14, 16, 1, audio_pll2_bypass_sels, ARRAY_SIZE(audio_pll2_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_VIDEO_PLL1_BYPASS] = imx_clk_mux_flags("video_pll1_bypass", base + 0x28, 16, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_DRAM_PLL_BYPASS] = imx_clk_mux_flags("dram_pll_bypass", base + 0x50, 16, 1, dram_pll_bypass_sels, ARRAY_SIZE(dram_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_GPU_PLL_BYPASS] = imx_clk_mux_flags("gpu_pll_bypass", base + 0x64, 28, 1, gpu_pll_bypass_sels, ARRAY_SIZE(gpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_VPU_PLL_BYPASS] = imx_clk_mux_flags("vpu_pll_bypass", base + 0x74, 28, 1, vpu_pll_bypass_sels, ARRAY_SIZE(vpu_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_ARM_PLL_BYPASS] = imx_clk_mux_flags("arm_pll_bypass", base + 0x84, 28, 1, arm_pll_bypass_sels, ARRAY_SIZE(arm_pll_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_SYS_PLL1_BYPASS] = imx_clk_mux_flags("sys_pll1_bypass", base + 0x94, 28, 1, sys_pll1_bypass_sels, ARRAY_SIZE(sys_pll1_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_SYS_PLL2_BYPASS] = imx_clk_mux_flags("sys_pll2_bypass", base + 0x104, 28, 1, sys_pll2_bypass_sels, ARRAY_SIZE(sys_pll2_bypass_sels), CLK_SET_RATE_PARENT);
+ clks[IMX8MN_SYS_PLL3_BYPASS] = imx_clk_mux_flags("sys_pll3_bypass", base + 0x114, 28, 1, sys_pll3_bypass_sels, ARRAY_SIZE(sys_pll3_bypass_sels), CLK_SET_RATE_PARENT);
/* PLL out gate */
clks[IMX8MN_AUDIO_PLL1_OUT] = imx_clk_gate("audio_pll1_out", "audio_pll1_bypass", base, 13);
--
2.16.4
_______________________________________________
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 V2 1/4] clk: imx: pll14xx: avoid glitch when set rate
From: Peng Fan @ 2019-09-09 3:40 UTC (permalink / raw)
To: Stephen Boyd, festevam@gmail.com, mturquette@baylibre.com,
s.hauer@pengutronix.de, shawnguo@kernel.org
Cc: Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Jacky Bai
In-Reply-To: <20190906172145.CAD3C20838@mail.kernel.org>
Hi Stephen,
> Subject: Re: [PATCH V2 1/4] clk: imx: pll14xx: avoid glitch when set rate
>
> Quoting Peng Fan (2019-08-26 02:42:14)
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > According to PLL1443XA and PLL1416X spec, "When BYPASS is 0 and RESETB
> > is changed from 0 to 1, FOUT starts to output unstable clock until
> > lock time passes. PLL1416X/PLL1443XA may generate a glitch at FOUT."
> >
> > So set BYPASS when RESETB is changed from 0 to 1 to avoid glitch.
> > In the end of set rate, BYPASS will be cleared.
> >
> > When prepare clock, also need to take care to avoid glitch. So we also
> > follow Spec to set BYPASS before RESETB changed from 0 to 1.
> > And add a check if the RESETB is already 0, directly return 0;
> >
> > Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
> > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
>
> Please make cover letters for multi-patch series.
Just sent out v3 to include cover-letter, no other changes.
Thanks,
Peng.
_______________________________________________
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 v4 00/10] Allwinner sunxi message box support
From: Samuel Holland @ 2019-09-09 3:54 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
linux-clk, linux-arm-kernel
In-Reply-To: <20190909032208.rlorx2ppytymtyej@core.my.home>
On 9/8/19 10:22 PM, Ondřej Jirman wrote:
> Hello Samuel,
>
> On Mon, Aug 19, 2019 at 10:23:01PM -0500, Samuel Holland wrote:
>> This series adds support for the "hardware message box" in sun8i, sun9i,
>> and sun50i SoCs, used for communication with the ARISC management
>> processor (the platform's equivalent of the ARM SCP). The end goal is to
>> use the arm_scpi driver as a client, communicating with firmware running
>> on the AR100 CPU, or to use the mailbox to forward NMIs that the
>> firmware picks up from R_INTC.
>>
>> Unfortunately, the ARM SCPI client no longer works with this driver
>> since it now exposes all 8 hardware FIFOs individually. The SCPI client
>> could be made to work (and I posted proof-of-concept code to that effect
>> with v1 of this series), but that is a low priority, as Linux does not
>> directly use SCPI with the current firmware version; all SCPI use goes
>> through ATF via PSCI.
>>
>> As requested in the comments to v3 of this patchset, a demo client is
>> provided in the final patch. This demo goes along with a toy firmware
>> which shows that the driver does indeed work for two-way communication
>> on all channels. To build the firmware component, run:
>
> I've tried using this driver with mainline arm_scpi driver (which is probably
> an expected future use, since crust provides SCPI interface).
If you've verified in some way that this driver works on A83T, I'd appreciate
your Tested-by, so I can send a patch for the A83T device tree node.
> The problem I've found is that arm_scpi expects message box to be
> bi-directional, but this driver provides uni-directional interface.
>
> What do you think about making this driver provide bi-directional interface?
> We could halve the number of channels to 4 and mandate TX/RX configuration
> (from main CPU's PoV) as ABI.
Funny you mention that. That's what I did originally for v1, but it got NAKed by
Maxime, Andre, and Jassi:
https://lkml.org/lkml/2018/2/28/125
https://lkml.org/lkml/2018/2/28/944
> Otherwise it's impossible to use it with the arm_scpi driver.
>
> Or do you have any other ideas? I guess arm_scpi can be fixed to add a
> property that would make it possible to use single shmem with two
> mailboxes, one for rx and one for tx, but making sun6i mailbox have
> bi-directional interface sounds easier.
Yes, you can use the existence of the mbox-names property to determine if the
driver needs one mailbox or two, as I did in this driver:
https://lkml.org/lkml/2019/3/1/789
I'll have a patch available soon that implements this for arm_scpi.
Cheers,
Samuel
> regards,
> o.
>
>> git clone https://github.com/crust-firmware/meta meta
>> git clone -b mailbox-demo https://github.com/crust-firmware/crust meta/crust
>> cd meta
>> make
>>
>> That will by default produce a U-Boot + ATF + SCP firmware image in
>> [meta/]build/pinebook/u-boot-sunxi-with-spl.bin. See the top-level
>> README.md for more information, such as cross-compiler setup.
>>
>> I've now used this driver with three separate clients over the past two
>> years, and they all work. If there are no remaining concerns with the
>> driver, I'd like it to get merged.
>>
>> Even without the driver, the clock patches (1-2) can go in at any time.
>>
>> Changes from v3:
>> - Rebased on sunxi-next
>> - Added Rob's Reviewed-by for patch 3
>> - Fixed a crash when receiving a message on a disabled channel
>> - Cleaned up some comments/formatting in the driver
>> - Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
>> - Removed the irqchip example (no longer relevant to the fw design)
>> - Added a demo/example client that uses the driver and a toy firmware
>>
>> Changes from v2:
>> - Merge patches 1-3
>> - Add a comment in the code explaining the CLK_IS_CRITICAL usage
>> - Add a patch to mark the AR100 clocks as critical
>> - Use YAML for the device tree binding
>> - Include a not-for-merge example usage of the mailbox
>>
>> Changes from v1:
>> - Marked message box clocks as critical instead of hacks in the driver
>> - 8 unidirectional channels instead of 4 bidirectional pairs
>> - Use per-SoC compatible strings and an A31 fallback compatible
>> - Dropped the mailbox framework patch
>> - Include DT patches for SoCs that document the message box
>>
>> Samuel Holland (10):
>> clk: sunxi-ng: Mark msgbox clocks as critical
>> clk: sunxi-ng: Mark AR100 clocks as critical
>> dt-bindings: mailbox: Add a sunxi message box binding
>> mailbox: sunxi-msgbox: Add a new mailbox driver
>> ARM: dts: sunxi: a80: Add msgbox node
>> ARM: dts: sunxi: a83t: Add msgbox node
>> ARM: dts: sunxi: h3/h5: Add msgbox node
>> arm64: dts: allwinner: a64: Add msgbox node
>> arm64: dts: allwinner: h6: Add msgbox node
>> [DO NOT MERGE] drivers: firmware: msgbox demo
>>
>> .../mailbox/allwinner,sunxi-msgbox.yaml | 79 +++++
>> arch/arm/boot/dts/sun8i-a83t.dtsi | 10 +
>> arch/arm/boot/dts/sun9i-a80.dtsi | 10 +
>> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
>> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 34 ++
>> arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 24 ++
>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 +
>> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 +-
>> drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c | 2 +-
>> drivers/clk/sunxi-ng/ccu-sun50i-h6.c | 3 +-
>> drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 3 +-
>> drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 3 +-
>> drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 3 +-
>> drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 3 +-
>> drivers/clk/sunxi-ng/ccu-sun8i-r.c | 2 +-
>> drivers/clk/sunxi-ng/ccu-sun9i-a80.c | 3 +-
>> drivers/firmware/Kconfig | 6 +
>> drivers/firmware/Makefile | 1 +
>> drivers/firmware/sunxi_msgbox_demo.c | 307 +++++++++++++++++
>> drivers/mailbox/Kconfig | 10 +
>> drivers/mailbox/Makefile | 2 +
>> drivers/mailbox/sunxi-msgbox.c | 323 ++++++++++++++++++
>> 22 files changed, 842 insertions(+), 9 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
>> create mode 100644 drivers/firmware/sunxi_msgbox_demo.c
>> create mode 100644 drivers/mailbox/sunxi-msgbox.c
>>
>> --
>> 2.21.0
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856
From: Dongchun Zhu @ 2019-09-09 3:55 UTC (permalink / raw)
To: Sakari Ailus
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
Ben Kao, tfiga, louis.kuo, sj.huang, robh+dt, linux-mediatek,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190808165416.GA1845@kekkonen.localdomain>
Hi Sakari,
On Thu, 2019-08-08 at 19:54 +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> (Cc'ing Ben, too.)
>
> On Thu, Aug 08, 2019 at 05:22:14PM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> > Add device tree binding documentation for the ov8856 camera sensor.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov8856.txt | 41 ++++++++++++++++++++++
> > MAINTAINERS | 1 +
> > 2 files changed, 42 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.txt b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
> > new file mode 100644
> > index 0000000..96b10e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
> > @@ -0,0 +1,41 @@
> > +* Omnivision OV8856 MIPI CSI-2 sensor
> > +
> > +Required Properties:
> > +- compatible: shall be "ovti,ov8856"
> > +- clocks: reference to the xvclk input clock
> > +- clock-names: shall be "xvclk"
>
> Could you add "clock-frequency" property here, please, and specify the
> upper and lower limits?
>
Sorry, this frequency of the external clock to the sensor and
link-frequencies would be added in DT in next release.
> > +- avdd-supply: Analog voltage supply, 2.8 volts
> > +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> > +- dvdd-supply: Digital core voltage supply, 1.2 volts
> > +- reset-gpios: Low active reset gpio
> > +
> > +The device node shall contain one 'port' child node with an
> > +'endpoint' subnode for its digital output video port,
> > +in accordance with the video interface bindings defined in
> > +Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +The endpoint optional property 'data-lanes' shall be "<0 1 3 4>".
>
> If you don't support lane reordering, then monotonically incrementing lane
> numbers from 1 onwards are recommended.
>
> Please also make the property mandatory.
>
data-lanes would be required property on endpoint in next release.
> > +
> > +Example:
> > +&i2c7 {
> > + ov8856: camera-sensor@10 {
> > + compatible = "ovti,ov8856";
> > + reg = <0x10>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&clk_24m_cam>;
> > +
> > + clocks = <&cru SCLK_TESTCLKOUT1>;
> > + clock-names = "xvclk";
> > +
> > + avdd-supply = <&mt6358_vcama2_reg>;
> > + dvdd-supply = <&mt6358_vcamd_reg>;
> > + dovdd-supply = <&mt6358_vcamio_reg>;
> > + reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>;
> > +
> > + port {
> > + wcam_out: endpoint {
> > + remote-endpoint = <&mipi_in_wcam>;
> > + data-lanes = <0 1 3 4>;
> > + };
> > + };
> > + };
> > +};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 783569e..7746c6b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11889,6 +11889,7 @@ L: linux-media@vger.kernel.org
> > T: git git://linuxtv.org/media_tree.git
> > S: Maintained
> > F: drivers/media/i2c/ov8856.c
> > +F: Documentation/devicetree/bindings/media/i2c/ov8856.txt
> >
> > OMNIVISION OV9650 SENSOR DRIVER
> > M: Sakari Ailus <sakari.ailus@linux.intel.com>
>
_______________________________________________
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 2/2] vhost: re-introducing metadata acceleration through kernel virtual address
From: Michael S. Tsirkin @ 2019-09-09 4:45 UTC (permalink / raw)
To: Jason Wang
Cc: aarcange, Christoph Hellwig, linux-parisc, kvm, netdev,
linux-kernel, virtualization, James Bottomley, linux-mm, jglisse,
jgg, David Miller, linux-arm-kernel
In-Reply-To: <1cb5aa8d-6213-5fce-5a77-fcada572c882@redhat.com>
On Mon, Sep 09, 2019 at 10:18:57AM +0800, Jason Wang wrote:
>
> On 2019/9/8 下午7:05, Michael S. Tsirkin wrote:
> > On Thu, Sep 05, 2019 at 08:27:36PM +0800, Jason Wang wrote:
> > > This is a rework on the commit 7f466032dc9e ("vhost: access vq
> > > metadata through kernel virtual address").
> > >
> > > It was noticed that the copy_to/from_user() friends that was used to
> > > access virtqueue metdata tends to be very expensive for dataplane
> > > implementation like vhost since it involves lots of software checks,
> > > speculation barriers,
> > So if we drop speculation barrier,
> > there's a problem here in access will now be speculated.
> > This effectively disables the defence in depth effect of
> > b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd
> > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec
> >
> >
> > So now we need to sprinkle array_index_nospec or barrier_nospec over the
> > code whenever we use an index we got from userspace.
> > See below for some examples.
> >
> >
> > > hardware feature toggling (e.g SMAP). The
> > > extra cost will be more obvious when transferring small packets since
> > > the time spent on metadata accessing become more significant.
> > >
> > > This patch tries to eliminate those overheads by accessing them
> > > through direct mapping of those pages. Invalidation callbacks is
> > > implemented for co-operation with general VM management (swap, KSM,
> > > THP or NUMA balancing). We will try to get the direct mapping of vq
> > > metadata before each round of packet processing if it doesn't
> > > exist. If we fail, we will simplely fallback to copy_to/from_user()
> > > friends.
> > >
> > > This invalidation, direct mapping access and set are synchronized
> > > through spinlock. This takes a step back from the original commit
> > > 7f466032dc9e ("vhost: access vq metadata through kernel virtual
> > > address") which tries to RCU which is suspicious and hard to be
> > > reviewed. This won't perform as well as RCU because of the atomic,
> > > this could be addressed by the future optimization.
> > >
> > > This method might does not work for high mem page which requires
> > > temporary mapping so we just fallback to normal
> > > copy_to/from_user() and may not for arch that has virtual tagged cache
> > > since extra cache flushing is needed to eliminate the alias. This will
> > > result complex logic and bad performance. For those archs, this patch
> > > simply go for copy_to/from_user() friends. This is done by ruling out
> > > kernel mapping codes through ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE.
> > >
> > > Note that this is only done when device IOTLB is not enabled. We
> > > could use similar method to optimize IOTLB in the future.
> > >
> > > Tests shows at most about 22% improvement on TX PPS when using
> > > virtio-user + vhost_net + xdp1 + TAP on 4.0GHz Kaby Lake.
> > >
> > > SMAP on | SMAP off
> > > Before: 4.9Mpps | 6.9Mpps
> > > After: 6.0Mpps | 7.5Mpps
> > >
> > > On a elder CPU Sandy Bridge without SMAP support. TX PPS doesn't see
> > > any difference.
> > Why is not Kaby Lake with SMAP off the same as Sandy Bridge?
>
>
> I don't know, I guess it was because the atomic is l
>
>
> >
> >
> > > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > > Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
> > > Cc: Christoph Hellwig <hch@infradead.org>
> > > Cc: David Miller <davem@davemloft.net>
> > > Cc: Jerome Glisse <jglisse@redhat.com>
> > > Cc: Jason Gunthorpe <jgg@mellanox.com>
> > > Cc: linux-mm@kvack.org
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: linux-parisc@vger.kernel.org
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > drivers/vhost/vhost.c | 551 +++++++++++++++++++++++++++++++++++++++++-
> > > drivers/vhost/vhost.h | 41 ++++
> > > 2 files changed, 589 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index 791562e03fe0..f98155f28f02 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -298,6 +298,182 @@ static void vhost_vq_meta_reset(struct vhost_dev *d)
> > > __vhost_vq_meta_reset(d->vqs[i]);
> > > }
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > +static void vhost_map_unprefetch(struct vhost_map *map)
> > > +{
> > > + kfree(map->pages);
> > > + kfree(map);
> > > +}
> > > +
> > > +static void vhost_set_map_dirty(struct vhost_virtqueue *vq,
> > > + struct vhost_map *map, int index)
> > > +{
> > > + struct vhost_uaddr *uaddr = &vq->uaddrs[index];
> > > + int i;
> > > +
> > > + if (uaddr->write) {
> > > + for (i = 0; i < map->npages; i++)
> > > + set_page_dirty(map->pages[i]);
> > > + }
> > > +}
> > > +
> > > +static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq)
> > > +{
> > > + struct vhost_map *map[VHOST_NUM_ADDRS];
> > > + int i;
> > > +
> > > + spin_lock(&vq->mmu_lock);
> > > + for (i = 0; i < VHOST_NUM_ADDRS; i++) {
> > > + map[i] = vq->maps[i];
> > > + if (map[i]) {
> > > + vhost_set_map_dirty(vq, map[i], i);
> > > + vq->maps[i] = NULL;
> > > + }
> > > + }
> > > + spin_unlock(&vq->mmu_lock);
> > > +
> > > + /* No need for synchronization since we are serialized with
> > > + * memory accessors (e.g vq mutex held).
> > > + */
> > > +
> > > + for (i = 0; i < VHOST_NUM_ADDRS; i++)
> > > + if (map[i])
> > > + vhost_map_unprefetch(map[i]);
> > > +
> > > +}
> > > +
> > > +static void vhost_reset_vq_maps(struct vhost_virtqueue *vq)
> > > +{
> > > + int i;
> > > +
> > > + vhost_uninit_vq_maps(vq);
> > > + for (i = 0; i < VHOST_NUM_ADDRS; i++)
> > > + vq->uaddrs[i].size = 0;
> > > +}
> > > +
> > > +static bool vhost_map_range_overlap(struct vhost_uaddr *uaddr,
> > > + unsigned long start,
> > > + unsigned long end)
> > > +{
> > > + if (unlikely(!uaddr->size))
> > > + return false;
> > > +
> > > + return !(end < uaddr->uaddr || start > uaddr->uaddr - 1 + uaddr->size);
> > > +}
> > > +
> > > +static void inline vhost_vq_access_map_begin(struct vhost_virtqueue *vq)
> > > +{
> > > + spin_lock(&vq->mmu_lock);
> > > +}
> > > +
> > > +static void inline vhost_vq_access_map_end(struct vhost_virtqueue *vq)
> > > +{
> > > + spin_unlock(&vq->mmu_lock);
> > > +}
> > > +
> > > +static int vhost_invalidate_vq_start(struct vhost_virtqueue *vq,
> > > + int index,
> > > + unsigned long start,
> > > + unsigned long end,
> > > + bool blockable)
> > > +{
> > > + struct vhost_uaddr *uaddr = &vq->uaddrs[index];
> > > + struct vhost_map *map;
> > > +
> > > + if (!vhost_map_range_overlap(uaddr, start, end))
> > > + return 0;
> > > + else if (!blockable)
> > > + return -EAGAIN;
> > > +
> > > + spin_lock(&vq->mmu_lock);
> > > + ++vq->invalidate_count;
> > > +
> > > + map = vq->maps[index];
> > > + if (map)
> > > + vq->maps[index] = NULL;
> > > + spin_unlock(&vq->mmu_lock);
> > > +
> > > + if (map) {
> > > + vhost_set_map_dirty(vq, map, index);
> > > + vhost_map_unprefetch(map);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void vhost_invalidate_vq_end(struct vhost_virtqueue *vq,
> > > + int index,
> > > + unsigned long start,
> > > + unsigned long end)
> > > +{
> > > + if (!vhost_map_range_overlap(&vq->uaddrs[index], start, end))
> > > + return;
> > > +
> > > + spin_lock(&vq->mmu_lock);
> > > + --vq->invalidate_count;
> > > + spin_unlock(&vq->mmu_lock);
> > > +}
> > > +
> > > +static int vhost_invalidate_range_start(struct mmu_notifier *mn,
> > > + const struct mmu_notifier_range *range)
> > > +{
> > > + struct vhost_dev *dev = container_of(mn, struct vhost_dev,
> > > + mmu_notifier);
> > > + bool blockable = mmu_notifier_range_blockable(range);
> > > + int i, j, ret;
> > > +
> > > + for (i = 0; i < dev->nvqs; i++) {
> > > + struct vhost_virtqueue *vq = dev->vqs[i];
> > > +
> > > + for (j = 0; j < VHOST_NUM_ADDRS; j++) {
> > > + ret = vhost_invalidate_vq_start(vq, j,
> > > + range->start,
> > > + range->end, blockable);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void vhost_invalidate_range_end(struct mmu_notifier *mn,
> > > + const struct mmu_notifier_range *range)
> > > +{
> > > + struct vhost_dev *dev = container_of(mn, struct vhost_dev,
> > > + mmu_notifier);
> > > + int i, j;
> > > +
> > > + for (i = 0; i < dev->nvqs; i++) {
> > > + struct vhost_virtqueue *vq = dev->vqs[i];
> > > +
> > > + for (j = 0; j < VHOST_NUM_ADDRS; j++)
> > > + vhost_invalidate_vq_end(vq, j,
> > > + range->start,
> > > + range->end);
> > > + }
> > > +}
> > > +
> > > +static const struct mmu_notifier_ops vhost_mmu_notifier_ops = {
> > > + .invalidate_range_start = vhost_invalidate_range_start,
> > > + .invalidate_range_end = vhost_invalidate_range_end,
> > > +};
> > > +
> > > +static void vhost_init_maps(struct vhost_dev *dev)
> > > +{
> > > + struct vhost_virtqueue *vq;
> > > + int i, j;
> > > +
> > > + dev->mmu_notifier.ops = &vhost_mmu_notifier_ops;
> > > +
> > > + for (i = 0; i < dev->nvqs; ++i) {
> > > + vq = dev->vqs[i];
> > > + for (j = 0; j < VHOST_NUM_ADDRS; j++)
> > > + vq->maps[j] = NULL;
> > > + }
> > > +}
> > > +#endif
> > > +
> > > static void vhost_vq_reset(struct vhost_dev *dev,
> > > struct vhost_virtqueue *vq)
> > > {
> > > @@ -326,7 +502,11 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> > > vq->busyloop_timeout = 0;
> > > vq->umem = NULL;
> > > vq->iotlb = NULL;
> > > + vq->invalidate_count = 0;
> > > __vhost_vq_meta_reset(vq);
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + vhost_reset_vq_maps(vq);
> > > +#endif
> > > }
> > > static int vhost_worker(void *data)
> > > @@ -471,12 +651,15 @@ void vhost_dev_init(struct vhost_dev *dev,
> > > dev->iov_limit = iov_limit;
> > > dev->weight = weight;
> > > dev->byte_weight = byte_weight;
> > > + dev->has_notifier = false;
> > > init_llist_head(&dev->work_list);
> > > init_waitqueue_head(&dev->wait);
> > > INIT_LIST_HEAD(&dev->read_list);
> > > INIT_LIST_HEAD(&dev->pending_list);
> > > spin_lock_init(&dev->iotlb_lock);
> > > -
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + vhost_init_maps(dev);
> > > +#endif
> > > for (i = 0; i < dev->nvqs; ++i) {
> > > vq = dev->vqs[i];
> > > @@ -485,6 +668,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> > > vq->heads = NULL;
> > > vq->dev = dev;
> > > mutex_init(&vq->mutex);
> > > + spin_lock_init(&vq->mmu_lock);
> > > vhost_vq_reset(dev, vq);
> > > if (vq->handle_kick)
> > > vhost_poll_init(&vq->poll, vq->handle_kick,
> > > @@ -564,7 +748,19 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
> > > if (err)
> > > goto err_cgroup;
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + err = mmu_notifier_register(&dev->mmu_notifier, dev->mm);
> > > + if (err)
> > > + goto err_mmu_notifier;
> > > +#endif
> > > + dev->has_notifier = true;
> > > +
> > > return 0;
> > > +
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > +err_mmu_notifier:
> > > + vhost_dev_free_iovecs(dev);
> > > +#endif
> > > err_cgroup:
> > > kthread_stop(worker);
> > > dev->worker = NULL;
> > > @@ -655,6 +851,107 @@ static void vhost_clear_msg(struct vhost_dev *dev)
> > > spin_unlock(&dev->iotlb_lock);
> > > }
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > +static void vhost_setup_uaddr(struct vhost_virtqueue *vq,
> > > + int index, unsigned long uaddr,
> > > + size_t size, bool write)
> > > +{
> > > + struct vhost_uaddr *addr = &vq->uaddrs[index];
> > > +
> > > + addr->uaddr = uaddr;
> > > + addr->size = size;
> > > + addr->write = write;
> > > +}
> > > +
> > > +static void vhost_setup_vq_uaddr(struct vhost_virtqueue *vq)
> > > +{
> > > + vhost_setup_uaddr(vq, VHOST_ADDR_DESC,
> > > + (unsigned long)vq->desc,
> > > + vhost_get_desc_size(vq, vq->num),
> > > + false);
> > > + vhost_setup_uaddr(vq, VHOST_ADDR_AVAIL,
> > > + (unsigned long)vq->avail,
> > > + vhost_get_avail_size(vq, vq->num),
> > > + false);
> > > + vhost_setup_uaddr(vq, VHOST_ADDR_USED,
> > > + (unsigned long)vq->used,
> > > + vhost_get_used_size(vq, vq->num),
> > > + true);
> > > +}
> > > +
> > > +static int vhost_map_prefetch(struct vhost_virtqueue *vq,
> > > + int index)
> > > +{
> > > + struct vhost_map *map;
> > > + struct vhost_uaddr *uaddr = &vq->uaddrs[index];
> > > + struct page **pages;
> > > + int npages = DIV_ROUND_UP(uaddr->size, PAGE_SIZE);
> > > + int npinned;
> > > + void *vaddr, *v;
> > > + int err;
> > > + int i;
> > > +
> > > + spin_lock(&vq->mmu_lock);
> > > +
> > > + err = -EFAULT;
> > > + if (vq->invalidate_count)
> > > + goto err;
> > > +
> > > + err = -ENOMEM;
> > > + map = kmalloc(sizeof(*map), GFP_ATOMIC);
> > > + if (!map)
> > > + goto err;
> > > +
> > > + pages = kmalloc_array(npages, sizeof(struct page *), GFP_ATOMIC);
> > > + if (!pages)
> > > + goto err_pages;
> > > +
> > > + err = EFAULT;
> > > + npinned = __get_user_pages_fast(uaddr->uaddr, npages,
> > > + uaddr->write, pages);
> > > + if (npinned > 0)
> > > + release_pages(pages, npinned);
> > > + if (npinned != npages)
> > > + goto err_gup;
> > > +
> > > + for (i = 0; i < npinned; i++)
> > > + if (PageHighMem(pages[i]))
> > > + goto err_gup;
> > > +
> > > + vaddr = v = page_address(pages[0]);
> > > +
> > > + /* For simplicity, fallback to userspace address if VA is not
> > > + * contigious.
> > > + */
> > > + for (i = 1; i < npinned; i++) {
> > > + v += PAGE_SIZE;
> > > + if (v != page_address(pages[i]))
> > > + goto err_gup;
> > > + }
> > > +
> > > + map->addr = vaddr + (uaddr->uaddr & (PAGE_SIZE - 1));
> > > + map->npages = npages;
> > > + map->pages = pages;
> > > +
> > > + vq->maps[index] = map;
> > > + /* No need for a synchronize_rcu(). This function should be
> > > + * called by dev->worker so we are serialized with all
> > > + * readers.
> > > + */
> > > + spin_unlock(&vq->mmu_lock);
> > > +
> > > + return 0;
> > > +
> > > +err_gup:
> > > + kfree(pages);
> > > +err_pages:
> > > + kfree(map);
> > > +err:
> > > + spin_unlock(&vq->mmu_lock);
> > > + return err;
> > > +}
> > > +#endif
> > > +
> > > void vhost_dev_cleanup(struct vhost_dev *dev)
> > > {
> > > int i;
> > > @@ -684,8 +981,20 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> > > kthread_stop(dev->worker);
> > > dev->worker = NULL;
> > > }
> > > - if (dev->mm)
> > > + if (dev->mm) {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + if (dev->has_notifier) {
> > > + mmu_notifier_unregister(&dev->mmu_notifier,
> > > + dev->mm);
> > > + dev->has_notifier = false;
> > > + }
> > > +#endif
> > > mmput(dev->mm);
> > > + }
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + for (i = 0; i < dev->nvqs; i++)
> > > + vhost_uninit_vq_maps(dev->vqs[i]);
> > > +#endif
> > > dev->mm = NULL;
> > > }
> > > EXPORT_SYMBOL_GPL(vhost_dev_cleanup);
> > > @@ -914,6 +1223,26 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
> > > static inline int vhost_put_avail_event(struct vhost_virtqueue *vq)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_used *used;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_USED];
> > > + if (likely(map)) {
> > > + used = map->addr;
> > > + *((__virtio16 *)&used->ring[vq->num]) =
> > > + cpu_to_vhost16(vq, vq->avail_idx);
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
> > > vhost_avail_event(vq));
> > > }
> > > @@ -922,6 +1251,27 @@ static inline int vhost_put_used(struct vhost_virtqueue *vq,
> > > struct vring_used_elem *head, int idx,
> > > int count)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_used *used;
> > > + size_t size;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_USED];
> > > + if (likely(map)) {
> > > + used = map->addr;
> > > + size = count * sizeof(*head);
> > > + memcpy(used->ring + idx, head, size);
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_copy_to_user(vq, vq->used->ring + idx, head,
> > > count * sizeof(*head));
> > > }
> > > @@ -929,6 +1279,25 @@ static inline int vhost_put_used(struct vhost_virtqueue *vq,
> > > static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_used *used;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_USED];
> > > + if (likely(map)) {
> > > + used = map->addr;
> > > + used->flags = cpu_to_vhost16(vq, vq->used_flags);
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_put_user(vq, cpu_to_vhost16(vq, vq->used_flags),
> > > &vq->used->flags);
> > > }
> > > @@ -936,6 +1305,25 @@ static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
> > > static inline int vhost_put_used_idx(struct vhost_virtqueue *vq)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_used *used;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_USED];
> > > + if (likely(map)) {
> > > + used = map->addr;
> > > + used->idx = cpu_to_vhost16(vq, vq->last_used_idx);
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
> > > &vq->used->idx);
> > > }
> > > @@ -981,12 +1369,50 @@ static void vhost_dev_unlock_vqs(struct vhost_dev *d)
> > > static inline int vhost_get_avail_idx(struct vhost_virtqueue *vq,
> > > __virtio16 *idx)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_avail *avail;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_AVAIL];
> > > + if (likely(map)) {
> > > + avail = map->addr;
> > > + *idx = avail->idx;
> > index can now be speculated.
>
> [...]
>
>
> > + vhost_vq_access_map_begin(vq);
> > +
> > + map = vq->maps[VHOST_ADDR_AVAIL];
> > + if (likely(map)) {
> > + avail = map->addr;
> > + *head = avail->ring[idx & (vq->num - 1)];
> >
> > Since idx can be speculated, I guess we need array_index_nospec here?
>
>
> So we have
>
> ACQUIRE(mmu_lock)
>
> get idx
>
> RELEASE(mmu_lock)
>
> ACQUIRE(mmu_lock)
>
> read array[idx]
>
> RELEASE(mmu_lock)
>
> Then I think idx can't be speculated consider we've passed RELEASE +
> ACQUIRE?
I don't think memory barriers have anything to do with speculation,
they are architectural.
>
> >
> >
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_get_avail(vq, *head,
> > > &vq->avail->ring[idx & (vq->num - 1)]);
> > > }
> > > @@ -994,24 +1420,98 @@ static inline int vhost_get_avail_head(struct vhost_virtqueue *vq,
> > > static inline int vhost_get_avail_flags(struct vhost_virtqueue *vq,
> > > __virtio16 *flags)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_avail *avail;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_AVAIL];
> > > + if (likely(map)) {
> > > + avail = map->addr;
> > > + *flags = avail->flags;
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_get_avail(vq, *flags, &vq->avail->flags);
> > > }
> > > static inline int vhost_get_used_event(struct vhost_virtqueue *vq,
> > > __virtio16 *event)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_avail *avail;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > + map = vq->maps[VHOST_ADDR_AVAIL];
> > > + if (likely(map)) {
> > > + avail = map->addr;
> > > + *event = (__virtio16)avail->ring[vq->num];
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_get_avail(vq, *event, vhost_used_event(vq));
> > > }
> > > static inline int vhost_get_used_idx(struct vhost_virtqueue *vq,
> > > __virtio16 *idx)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_used *used;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_USED];
> > > + if (likely(map)) {
> > > + used = map->addr;
> > > + *idx = used->idx;
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_get_used(vq, *idx, &vq->used->idx);
> > > }
> >
> > This seems to be used during init. Why do we bother
> > accelerating this?
>
>
> Ok, I can remove this part in next version.
>
>
> >
> >
> > > static inline int vhost_get_desc(struct vhost_virtqueue *vq,
> > > struct vring_desc *desc, int idx)
> > > {
> > > +#if VHOST_ARCH_CAN_ACCEL_UACCESS
> > > + struct vhost_map *map;
> > > + struct vring_desc *d;
> > > +
> > > + if (!vq->iotlb) {
> > > + vhost_vq_access_map_begin(vq);
> > > +
> > > + map = vq->maps[VHOST_ADDR_DESC];
> > > + if (likely(map)) {
> > > + d = map->addr;
> > > + *desc = *(d + idx);
> >
> > Since idx can be speculated, I guess we need array_index_nospec here?
>
>
> This is similar to the above avail idx case.
>
>
> >
> >
> > > + vhost_vq_access_map_end(vq);
> > > + return 0;
> > > + }
> > > +
> > > + vhost_vq_access_map_end(vq);
> > > + }
> > > +#endif
> > > +
> > > return vhost_copy_from_user(vq, desc, vq->desc + idx, sizeof(*desc));
> > > }
> > I also wonder about the userspace address we get eventualy.
> > It would seem that we need to prevent that from speculating -
> > and that seems like a good idea even if this patch isn't
> > applied. As you are playing with micro-benchmarks, maybe
> > you could the below patch?
>
>
> Let me test it.
>
> Thanks
>
>
> > It's unfortunately untested.
> > Thanks a lot in advance!
> >
> > ===>
> > vhost: block speculation of translated descriptors
> >
> > iovec addresses coming from vhost are assumed to be
> > pre-validated, but in fact can be speculated to a value
> > out of range.
> >
> > Userspace address are later validated with array_index_nospec so we can
> > be sure kernel info does not leak through these addresses, but vhost
> > must also not leak userspace info outside the allowed memory table to
> > guests.
> >
> > Following the defence in depth principle, make sure
> > the address is not validated out of node range.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ---
> >
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 5dc174ac8cac..863e25011ef6 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2072,7 +2076,9 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
> > size = node->size - addr + node->start;
> > _iov->iov_len = min((u64)len - s, size);
> > _iov->iov_base = (void __user *)(unsigned long)
> > - (node->userspace_addr + addr - node->start);
> > + (node->userspace_addr +
> > + array_index_nospec(addr - node->start,
> > + node->size));
> > s += size;
> > addr += size;
> > ++ret;
_______________________________________________
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 02/14] soc: ti: k3: add navss ringacc driver
From: Tero Kristo @ 2019-09-09 6:09 UTC (permalink / raw)
To: Peter Ujfalusi, vkoul, robh+dt, nm, ssantosh
Cc: devicetree, grygorii.strashko, lokeshvutla, j-keerthy,
linux-kernel, tony, dmaengine, dan.j.williams, linux-arm-kernel
In-Reply-To: <20190730093450.12664-3-peter.ujfalusi@ti.com>
Hi,
Mostly some cosmetic comments below, other than that seems fine to me.
On 30/07/2019 12:34, Peter Ujfalusi wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> The Ring Accelerator (RINGACC or RA) provides hardware acceleration to
> enable straightforward passing of work between a producer and a consumer.
> There is one RINGACC module per NAVSS on TI AM65x SoCs.
>
> The RINGACC converts constant-address read and write accesses to equivalent
> read or write accesses to a circular data structure in memory. The RINGACC
> eliminates the need for each DMA controller which needs to access ring
> elements from having to know the current state of the ring (base address,
> current offset). The DMA controller performs a read or write access to a
> specific address range (which maps to the source interface on the RINGACC)
> and the RINGACC replaces the address for the transaction with a new address
> which corresponds to the head or tail element of the ring (head for reads,
> tail for writes). Since the RINGACC maintains the state, multiple DMA
> controllers or channels are allowed to coherently share the same rings as
> applicable. The RINGACC is able to place data which is destined towards
> software into cached memory directly.
>
> Supported ring modes:
> - Ring Mode
> - Messaging Mode
> - Credentials Mode
> - Queue Manager Mode
>
> TI-SCI integration:
>
> Texas Instrument's System Control Interface (TI-SCI) Message Protocol now
> has control over Ringacc module resources management (RM) and Rings
> configuration.
>
> The corresponding support of TI-SCI Ringacc module RM protocol
> introduced as option through DT parameters:
> - ti,sci: phandle on TI-SCI firmware controller DT node
> - ti,sci-dev-id: TI-SCI device identifier as per TI-SCI firmware spec
>
> if both parameters present - Ringacc driver will configure/free/reset Rings
> using TI-SCI Message Ringacc RM Protocol.
>
> The Ringacc driver manages Rings allocation by itself now and requests
> TI-SCI firmware to allocate and configure specific Rings only. It's done
> this way because, Linux driver implements two stage Rings allocation and
> configuration (allocate ring and configure ring) while I-SCI Message
I-SCI should be TI-SCI I believe.
> Protocol supports only one combined operation (allocate+configure).
>
> Grygorii Strashko <grygorii.strashko@ti.com>
Above seems to be missing SoB?
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> drivers/soc/ti/Kconfig | 17 +
> drivers/soc/ti/Makefile | 1 +
> drivers/soc/ti/k3-ringacc.c | 1191 +++++++++++++++++++++++++++++
> include/linux/soc/ti/k3-ringacc.h | 262 +++++++
> 4 files changed, 1471 insertions(+)
> create mode 100644 drivers/soc/ti/k3-ringacc.c
> create mode 100644 include/linux/soc/ti/k3-ringacc.h
>
> diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
> index cf545f428d03..10c76faa503e 100644
> --- a/drivers/soc/ti/Kconfig
> +++ b/drivers/soc/ti/Kconfig
> @@ -80,6 +80,23 @@ config TI_SCI_PM_DOMAINS
> called ti_sci_pm_domains. Note this is needed early in boot before
> rootfs may be available.
>
> +config TI_K3_RINGACC
> + tristate "K3 Ring accelerator Sub System"
> + depends on ARCH_K3 || COMPILE_TEST
> + depends on TI_SCI_INTA_IRQCHIP
> + default y
> + help
> + Say y here to support the K3 Ring accelerator module.
> + The Ring Accelerator (RINGACC or RA) provides hardware acceleration
> + to enable straightforward passing of work between a producer
> + and a consumer. There is one RINGACC module per NAVSS on TI AM65x SoCs
> + If unsure, say N.
> +
> +config TI_K3_RINGACC_DEBUG
> + tristate "K3 Ring accelerator Sub System tests and debug"
> + depends on TI_K3_RINGACC
> + default n
> +
> endif # SOC_TI
>
> config TI_SCI_INTA_MSI_DOMAIN
> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
> index b3868d392d4f..cc4bc8b08bf5 100644
> --- a/drivers/soc/ti/Makefile
> +++ b/drivers/soc/ti/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_AMX3_PM) += pm33xx.o
> obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
> obj-$(CONFIG_TI_SCI_PM_DOMAINS) += ti_sci_pm_domains.o
> obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN) += ti_sci_inta_msi.o
> +obj-$(CONFIG_TI_K3_RINGACC) += k3-ringacc.o
> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> new file mode 100644
> index 000000000000..401dfc963319
> --- /dev/null
> +++ b/drivers/soc/ti/k3-ringacc.c
> @@ -0,0 +1,1191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI K3 NAVSS Ring Accelerator subsystem driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/soc/ti/k3-ringacc.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +#include <linux/soc/ti/ti_sci_inta_msi.h>
> +#include <linux/of_irq.h>
> +#include <linux/irqdomain.h>
> +
> +static LIST_HEAD(k3_ringacc_list);
> +static DEFINE_MUTEX(k3_ringacc_list_lock);
> +
> +#ifdef CONFIG_TI_K3_RINGACC_DEBUG
> +#define k3_nav_dbg(dev, arg...) dev_err(dev, arg)
dev_err seems exaggeration for debug purposes, maybe just dev_info.
> +static void dbg_writel(u32 v, void __iomem *reg)
> +{
> + pr_err("WRITEL(32): v(%08X)-->reg(%p)\n", v, reg);
Again, maybe just pr_info.
> + writel(v, reg);
> +}
> +
> +static u32 dbg_readl(void __iomem *reg)
> +{
> + u32 v;
> +
> + v = readl(reg);
> + pr_err("READL(32): v(%08X)<--reg(%p)\n", v, reg);
> + return v;
> +}
> +#else
> +#define k3_nav_dbg(dev, arg...) dev_dbg(dev, arg)
> +#define dbg_writel(v, reg) writel(v, reg)
Do you need to use hard writel, writel_relaxed is not enough?
> +
> +#define dbg_readl(reg) readl(reg)
Same as above but for read?
> +#endif
> +
> +#define K3_RINGACC_CFG_RING_SIZE_ELCNT_MASK GENMASK(19, 0)
> +
> +/**
> + * struct k3_ring_rt_regs - The RA Control/Status Registers region
> + */
> +struct k3_ring_rt_regs {
> + u32 resv_16[4];
> + u32 db; /* RT Ring N Doorbell Register */
> + u32 resv_4[1];
> + u32 occ; /* RT Ring N Occupancy Register */
> + u32 indx; /* RT Ring N Current Index Register */
> + u32 hwocc; /* RT Ring N Hardware Occupancy Register */
> + u32 hwindx; /* RT Ring N Current Index Register */
> +};
> +
> +#define K3_RINGACC_RT_REGS_STEP 0x1000
> +
> +/**
> + * struct k3_ring_fifo_regs - The Ring Accelerator Queues Registers region
> + */
> +struct k3_ring_fifo_regs {
> + u32 head_data[128]; /* Ring Head Entry Data Registers */
> + u32 tail_data[128]; /* Ring Tail Entry Data Registers */
> + u32 peek_head_data[128]; /* Ring Peek Head Entry Data Regs */
> + u32 peek_tail_data[128]; /* Ring Peek Tail Entry Data Regs */
> +};
> +
> +/**
> + * struct k3_ringacc_proxy_gcfg_regs - RA Proxy Global Config MMIO Region
> + */
> +struct k3_ringacc_proxy_gcfg_regs {
> + u32 revision; /* Revision Register */
> + u32 config; /* Config Register */
> +};
> +
> +#define K3_RINGACC_PROXY_CFG_THREADS_MASK GENMASK(15, 0)
> +
> +/**
> + * struct k3_ringacc_proxy_target_regs - Proxy Datapath MMIO Region
> + */
> +struct k3_ringacc_proxy_target_regs {
> + u32 control; /* Proxy Control Register */
> + u32 status; /* Proxy Status Register */
> + u8 resv_512[504];
> + u32 data[128]; /* Proxy Data Register */
> +};
> +
> +#define K3_RINGACC_PROXY_TARGET_STEP 0x1000
> +#define K3_RINGACC_PROXY_NOT_USED (-1)
> +
> +enum k3_ringacc_proxy_access_mode {
> + PROXY_ACCESS_MODE_HEAD = 0,
> + PROXY_ACCESS_MODE_TAIL = 1,
> + PROXY_ACCESS_MODE_PEEK_HEAD = 2,
> + PROXY_ACCESS_MODE_PEEK_TAIL = 3,
> +};
> +
> +#define K3_RINGACC_FIFO_WINDOW_SIZE_BYTES (512U)
> +#define K3_RINGACC_FIFO_REGS_STEP 0x1000
> +#define K3_RINGACC_MAX_DB_RING_CNT (127U)
> +
> +/**
> + * struct k3_ring_ops - Ring operations
> + */
> +struct k3_ring_ops {
> + int (*push_tail)(struct k3_ring *ring, void *elm);
> + int (*push_head)(struct k3_ring *ring, void *elm);
> + int (*pop_tail)(struct k3_ring *ring, void *elm);
> + int (*pop_head)(struct k3_ring *ring, void *elm);
> +};
> +
> +/**
> + * struct k3_ring - RA Ring descriptor
> + *
> + * @rt - Ring control/status registers
> + * @fifos - Ring queues registers
> + * @proxy - Ring Proxy Datapath registers
> + * @ring_mem_dma - Ring buffer dma address
> + * @ring_mem_virt - Ring buffer virt address
> + * @ops - Ring operations
> + * @size - Ring size in elements
> + * @elm_size - Size of the ring element
> + * @mode - Ring mode
> + * @flags - flags
> + * @free - Number of free elements
> + * @occ - Ring occupancy
> + * @windex - Write index (only for @K3_RINGACC_RING_MODE_RING)
> + * @rindex - Read index (only for @K3_RINGACC_RING_MODE_RING)
> + * @ring_id - Ring Id
> + * @parent - Pointer on struct @k3_ringacc
> + * @use_count - Use count for shared rings
> + * @proxy_id - RA Ring Proxy Id (only if @K3_RINGACC_RING_USE_PROXY)
> + */
> +struct k3_ring {
> + struct k3_ring_rt_regs __iomem *rt;
> + struct k3_ring_fifo_regs __iomem *fifos;
> + struct k3_ringacc_proxy_target_regs __iomem *proxy;
> + dma_addr_t ring_mem_dma;
> + void *ring_mem_virt;
> + struct k3_ring_ops *ops;
> + u32 size;
> + enum k3_ring_size elm_size;
> + enum k3_ring_mode mode;
> + u32 flags;
> +#define K3_RING_FLAG_BUSY BIT(1)
> +#define K3_RING_FLAG_SHARED BIT(2)
> + u32 free;
> + u32 occ;
> + u32 windex;
> + u32 rindex;
> + u32 ring_id;
> + struct k3_ringacc *parent;
> + u32 use_count;
> + int proxy_id;
> +};
> +
> +/**
> + * struct k3_ringacc - Rings accelerator descriptor
> + *
> + * @dev - pointer on RA device
> + * @proxy_gcfg - RA proxy global config registers
> + * @proxy_target_base - RA proxy datapath region
> + * @num_rings - number of ring in RA
> + * @rm_gp_range - general purpose rings range from tisci
> + * @dma_ring_reset_quirk - DMA reset w/a enable
> + * @num_proxies - number of RA proxies
> + * @rings - array of rings descriptors (struct @k3_ring)
> + * @list - list of RAs in the system
> + * @tisci - pointer ti-sci handle
> + * @tisci_ring_ops - ti-sci rings ops
> + * @tisci_dev_id - ti-sci device id
> + */
> +struct k3_ringacc {
> + struct device *dev;
> + struct k3_ringacc_proxy_gcfg_regs __iomem *proxy_gcfg;
> + void __iomem *proxy_target_base;
> + u32 num_rings; /* number of rings in Ringacc module */
> + unsigned long *rings_inuse;
> + struct ti_sci_resource *rm_gp_range;
> +
> + bool dma_ring_reset_quirk;
> + u32 num_proxies;
> + unsigned long *proxy_inuse;
proxy_inuse is not documented above.
> +
> + struct k3_ring *rings;
> + struct list_head list;
> + struct mutex req_lock; /* protect rings allocation */
> +
> + const struct ti_sci_handle *tisci;
> + const struct ti_sci_rm_ringacc_ops *tisci_ring_ops;
> + u32 tisci_dev_id;
> +};
> +
> +static long k3_ringacc_ring_get_fifo_pos(struct k3_ring *ring)
> +{
> + return K3_RINGACC_FIFO_WINDOW_SIZE_BYTES -
> + (4 << ring->elm_size);
> +}
> +
> +static void *k3_ringacc_get_elm_addr(struct k3_ring *ring, u32 idx)
> +{
> + return (idx * (4 << ring->elm_size) + ring->ring_mem_virt);
The arithmetic here seems backwards compared to most other code I've
seen. It would be more readable if you have it like:
ring->ring_mem_virt + idx * (4 << ring->elm_size);
> +}
> +
> +static int k3_ringacc_ring_push_mem(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_pop_mem(struct k3_ring *ring, void *elem);
> +
> +static struct k3_ring_ops k3_ring_mode_ring_ops = {
> + .push_tail = k3_ringacc_ring_push_mem,
> + .pop_head = k3_ringacc_ring_pop_mem,
> +};
> +
> +static int k3_ringacc_ring_push_io(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_pop_io(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_push_head_io(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_pop_tail_io(struct k3_ring *ring, void *elem);
> +
> +static struct k3_ring_ops k3_ring_mode_msg_ops = {
> + .push_tail = k3_ringacc_ring_push_io,
> + .push_head = k3_ringacc_ring_push_head_io,
> + .pop_tail = k3_ringacc_ring_pop_tail_io,
> + .pop_head = k3_ringacc_ring_pop_io,
> +};
> +
> +static int k3_ringacc_ring_push_head_proxy(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_push_tail_proxy(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_pop_head_proxy(struct k3_ring *ring, void *elem);
> +static int k3_ringacc_ring_pop_tail_proxy(struct k3_ring *ring, void *elem);
> +
> +static struct k3_ring_ops k3_ring_mode_proxy_ops = {
> + .push_tail = k3_ringacc_ring_push_tail_proxy,
> + .push_head = k3_ringacc_ring_push_head_proxy,
> + .pop_tail = k3_ringacc_ring_pop_tail_proxy,
> + .pop_head = k3_ringacc_ring_pop_head_proxy,
> +};
> +
> +#ifdef CONFIG_TI_K3_RINGACC_DEBUG
> +void k3_ringacc_ring_dump(struct k3_ring *ring)
> +{
> + struct device *dev = ring->parent->dev;
> +
> + k3_nav_dbg(dev, "dump ring: %d\n", ring->ring_id);
> + k3_nav_dbg(dev, "dump mem virt %p, dma %pad\n",
> + ring->ring_mem_virt, &ring->ring_mem_dma);
> + k3_nav_dbg(dev, "dump elmsize %d, size %d, mode %d, proxy_id %d\n",
> + ring->elm_size, ring->size, ring->mode, ring->proxy_id);
> +
> + k3_nav_dbg(dev, "dump ring_rt_regs: db%08x\n",
> + readl(&ring->rt->db));
Why not use readl_relaxed in this func?
> + k3_nav_dbg(dev, "dump occ%08x\n",
> + readl(&ring->rt->occ));
> + k3_nav_dbg(dev, "dump indx%08x\n",
> + readl(&ring->rt->indx));
> + k3_nav_dbg(dev, "dump hwocc%08x\n",
> + readl(&ring->rt->hwocc));
> + k3_nav_dbg(dev, "dump hwindx%08x\n",
> + readl(&ring->rt->hwindx));
> +
> + if (ring->ring_mem_virt)
> + print_hex_dump(KERN_ERR, "dump ring_mem_virt ",
> + DUMP_PREFIX_NONE, 16, 1,
> + ring->ring_mem_virt, 16 * 8, false);
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_dump);
Do you really need to export a debug function?
> +#endif
> +
> +struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
> + int id, u32 flags)
> +{
> + int proxy_id = K3_RINGACC_PROXY_NOT_USED;
> +
> + mutex_lock(&ringacc->req_lock);
> +
> + if (id == K3_RINGACC_RING_ID_ANY) {
> + /* Request for any general purpose ring */
> + struct ti_sci_resource_desc *gp_rings =
> + &ringacc->rm_gp_range->desc[0];
> + unsigned long size;
> +
> + size = gp_rings->start + gp_rings->num;
> + id = find_next_zero_bit(ringacc->rings_inuse, size,
> + gp_rings->start);
> + if (id == size)
> + goto error;
> + } else if (id < 0) {
> + goto error;
> + }
> +
> + if (test_bit(id, ringacc->rings_inuse) &&
> + !(ringacc->rings[id].flags & K3_RING_FLAG_SHARED))
> + goto error;
> + else if (ringacc->rings[id].flags & K3_RING_FLAG_SHARED)
> + goto out;
> +
> + if (flags & K3_RINGACC_RING_USE_PROXY) {
> + proxy_id = find_next_zero_bit(ringacc->proxy_inuse,
> + ringacc->num_proxies, 0);
> + if (proxy_id == ringacc->num_proxies)
> + goto error;
> + }
> +
> + if (!try_module_get(ringacc->dev->driver->owner))
> + goto error;
> +
> + if (proxy_id != K3_RINGACC_PROXY_NOT_USED) {
> + set_bit(proxy_id, ringacc->proxy_inuse);
> + ringacc->rings[id].proxy_id = proxy_id;
> + k3_nav_dbg(ringacc->dev, "Giving ring#%d proxy#%d\n",
> + id, proxy_id);
> + } else {
> + k3_nav_dbg(ringacc->dev, "Giving ring#%d\n", id);
> + }
> +
> + set_bit(id, ringacc->rings_inuse);
> +out:
> + ringacc->rings[id].use_count++;
> + mutex_unlock(&ringacc->req_lock);
> + return &ringacc->rings[id];
> +
> +error:
> + mutex_unlock(&ringacc->req_lock);
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
> +
> +static void k3_ringacc_ring_reset_sci(struct k3_ring *ring)
> +{
> + struct k3_ringacc *ringacc = ring->parent;
> + int ret;
> +
> + ret = ringacc->tisci_ring_ops->config(
> + ringacc->tisci,
> + TI_SCI_MSG_VALUE_RM_RING_COUNT_VALID,
> + ringacc->tisci_dev_id,
> + ring->ring_id,
> + 0,
> + 0,
> + ring->size,
> + 0,
> + 0,
> + 0);
> + if (ret)
> + dev_err(ringacc->dev, "TISCI reset ring fail (%d) ring_idx %d\n",
> + ret, ring->ring_id);
Return value of sci ops is masked, why not return it and let the caller
handle it properly?
Same comment for anything similar that follows.
> +}
> +
> +void k3_ringacc_ring_reset(struct k3_ring *ring)
> +{
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return;
> +
> + ring->occ = 0;
> + ring->free = 0;
> + ring->rindex = 0;
> + ring->windex = 0;
> +
> + k3_ringacc_ring_reset_sci(ring);
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_reset);
> +
> +static void k3_ringacc_ring_reconfig_qmode_sci(struct k3_ring *ring,
> + enum k3_ring_mode mode)
> +{
> + struct k3_ringacc *ringacc = ring->parent;
> + int ret;
> +
> + ret = ringacc->tisci_ring_ops->config(
> + ringacc->tisci,
> + TI_SCI_MSG_VALUE_RM_RING_MODE_VALID,
> + ringacc->tisci_dev_id,
> + ring->ring_id,
> + 0,
> + 0,
> + 0,
> + mode,
> + 0,
> + 0);
> + if (ret)
> + dev_err(ringacc->dev, "TISCI reconf qmode fail (%d) ring_idx %d\n",
> + ret, ring->ring_id);
>+}
> +
> +void k3_ringacc_ring_reset_dma(struct k3_ring *ring, u32 occ)
> +{
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return;
> +
> + if (!ring->parent->dma_ring_reset_quirk)
> + return;
> +
> + if (!occ)
> + occ = dbg_readl(&ring->rt->occ);
> +
> + if (occ) {
> + u32 db_ring_cnt, db_ring_cnt_cur;
> +
> + k3_nav_dbg(ring->parent->dev, "%s %u occ: %u\n", __func__,
> + ring->ring_id, occ);
> + /* 2. Reset the ring */
2? Where is 1?
> + k3_ringacc_ring_reset_sci(ring);
> +
> + /*
> + * 3. Setup the ring in ring/doorbell mode
> + * (if not already in this mode)
> + */
> + if (ring->mode != K3_RINGACC_RING_MODE_RING)
> + k3_ringacc_ring_reconfig_qmode_sci(
> + ring, K3_RINGACC_RING_MODE_RING);
> + /*
> + * 4. Ring the doorbell 2**22 – ringOcc times.
> + * This will wrap the internal UDMAP ring state occupancy
> + * counter (which is 21-bits wide) to 0.
> + */
> + db_ring_cnt = (1U << 22) - occ;
> +
> + while (db_ring_cnt != 0) {
> + /*
> + * Ring the doorbell with the maximum count each
> + * iteration if possible to minimize the total
> + * of writes
> + */
> + if (db_ring_cnt > K3_RINGACC_MAX_DB_RING_CNT)
> + db_ring_cnt_cur = K3_RINGACC_MAX_DB_RING_CNT;
> + else
> + db_ring_cnt_cur = db_ring_cnt;
> +
> + writel(db_ring_cnt_cur, &ring->rt->db);
> + db_ring_cnt -= db_ring_cnt_cur;
> + }
> +
> + /* 5. Restore the original ring mode (if not ring mode) */
> + if (ring->mode != K3_RINGACC_RING_MODE_RING)
> + k3_ringacc_ring_reconfig_qmode_sci(ring, ring->mode);
> + }
> +
> + /* 2. Reset the ring */
Again 2?
> + k3_ringacc_ring_reset(ring);
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_reset_dma);
> +
> +static void k3_ringacc_ring_free_sci(struct k3_ring *ring)
> +{
> + struct k3_ringacc *ringacc = ring->parent;
> + int ret;
> +
> + ret = ringacc->tisci_ring_ops->config(
> + ringacc->tisci,
> + TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER,
> + ringacc->tisci_dev_id,
> + ring->ring_id,
> + 0,
> + 0,
> + 0,
> + 0,
> + 0,
> + 0);
> + if (ret)
> + dev_err(ringacc->dev, "TISCI ring free fail (%d) ring_idx %d\n",
> + ret, ring->ring_id);
> +}
> +
> +int k3_ringacc_ring_free(struct k3_ring *ring)
> +{
> + struct k3_ringacc *ringacc;
> +
> + if (!ring)
> + return -EINVAL;
> +
> + ringacc = ring->parent;
> +
> + k3_nav_dbg(ring->parent->dev, "flags: 0x%08x\n", ring->flags);
> +
> + if (!test_bit(ring->ring_id, ringacc->rings_inuse))
> + return -EINVAL;
> +
> + mutex_lock(&ringacc->req_lock);
> +
> + if (--ring->use_count)
> + goto out;
> +
> + if (!(ring->flags & K3_RING_FLAG_BUSY))
> + goto no_init;
> +
> + k3_ringacc_ring_free_sci(ring);
> +
> + dma_free_coherent(ringacc->dev,
> + ring->size * (4 << ring->elm_size),
> + ring->ring_mem_virt, ring->ring_mem_dma);
> + ring->flags = 0;
> + ring->ops = NULL;
> + if (ring->proxy_id != K3_RINGACC_PROXY_NOT_USED) {
> + clear_bit(ring->proxy_id, ringacc->proxy_inuse);
> + ring->proxy = NULL;
> + ring->proxy_id = K3_RINGACC_PROXY_NOT_USED;
> + }
> +
> +no_init:
> + clear_bit(ring->ring_id, ringacc->rings_inuse);
> +
> + module_put(ringacc->dev->driver->owner);
> +
> +out:
> + mutex_unlock(&ringacc->req_lock);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_free);
> +
> +u32 k3_ringacc_get_ring_id(struct k3_ring *ring)
> +{
> + if (!ring)
> + return -EINVAL;
> +
> + return ring->ring_id;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_get_ring_id);
> +
> +u32 k3_ringacc_get_tisci_dev_id(struct k3_ring *ring)
> +{
> + if (!ring)
> + return -EINVAL;
> +
What if parent is NULL? Can it ever be here?
> + return ring->parent->tisci_dev_id;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_get_tisci_dev_id);
> +
> +int k3_ringacc_get_ring_irq_num(struct k3_ring *ring)
> +{
> + int irq_num;
> +
> + if (!ring)
> + return -EINVAL;
> +
> + irq_num = ti_sci_inta_msi_get_virq(ring->parent->dev, ring->ring_id);
> + if (irq_num <= 0)
> + irq_num = -EINVAL;
> + return irq_num;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_get_ring_irq_num);
> +
> +static int k3_ringacc_ring_cfg_sci(struct k3_ring *ring)
> +{
> + struct k3_ringacc *ringacc = ring->parent;
> + u32 ring_idx;
> + int ret;
> +
> + if (!ringacc->tisci)
> + return -EINVAL;
> +
> + ring_idx = ring->ring_id;
> + ret = ringacc->tisci_ring_ops->config(
> + ringacc->tisci,
> + TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER,
> + ringacc->tisci_dev_id,
> + ring_idx,
> + lower_32_bits(ring->ring_mem_dma),
> + upper_32_bits(ring->ring_mem_dma),
> + ring->size,
> + ring->mode,
> + ring->elm_size,
> + 0);
> + if (ret)
> + dev_err(ringacc->dev, "TISCI config ring fail (%d) ring_idx %d\n",
> + ret, ring_idx);
> +
> + return ret;
> +}
> +
> +int k3_ringacc_ring_cfg(struct k3_ring *ring, struct k3_ring_cfg *cfg)
> +{
> + struct k3_ringacc *ringacc = ring->parent;
> + int ret = 0;
> +
> + if (!ring || !cfg)
> + return -EINVAL;
> + if (cfg->elm_size > K3_RINGACC_RING_ELSIZE_256 ||
> + cfg->mode > K3_RINGACC_RING_MODE_QM ||
> + cfg->size & ~K3_RINGACC_CFG_RING_SIZE_ELCNT_MASK ||
> + !test_bit(ring->ring_id, ringacc->rings_inuse))
> + return -EINVAL;
> +
> + if (ring->use_count != 1)
Hmm, isn't this a failure actually?
> + return 0;
> +
> + ring->size = cfg->size;
> + ring->elm_size = cfg->elm_size;
> + ring->mode = cfg->mode;
> + ring->occ = 0;
> + ring->free = 0;
> + ring->rindex = 0;
> + ring->windex = 0;
> +
> + if (ring->proxy_id != K3_RINGACC_PROXY_NOT_USED)
> + ring->proxy = ringacc->proxy_target_base +
> + ring->proxy_id * K3_RINGACC_PROXY_TARGET_STEP;
> +
> + switch (ring->mode) {
> + case K3_RINGACC_RING_MODE_RING:
> + ring->ops = &k3_ring_mode_ring_ops;
> + break;
> + case K3_RINGACC_RING_MODE_QM:
> + /*
> + * In Queue mode elm_size can be 8 only and each operation
> + * uses 2 element slots
> + */
> + if (cfg->elm_size != K3_RINGACC_RING_ELSIZE_8 ||
> + cfg->size % 2)
> + goto err_free_proxy;
> + /* else, fall through */
> + case K3_RINGACC_RING_MODE_MESSAGE:
> + if (ring->proxy)
> + ring->ops = &k3_ring_mode_proxy_ops;
> + else
> + ring->ops = &k3_ring_mode_msg_ops;
> + break;
> + default:
> + ring->ops = NULL;
> + ret = -EINVAL;
> + goto err_free_proxy;
> + };
> +
> + ring->ring_mem_virt =
> + dma_alloc_coherent(ringacc->dev,
> + ring->size * (4 << ring->elm_size),
> + &ring->ring_mem_dma, GFP_KERNEL);
> + if (!ring->ring_mem_virt) {
> + dev_err(ringacc->dev, "Failed to alloc ring mem\n");
> + ret = -ENOMEM;
> + goto err_free_ops;
> + }
> +
> + ret = k3_ringacc_ring_cfg_sci(ring);
> +
> + if (ret)
> + goto err_free_mem;
> +
> + ring->flags |= K3_RING_FLAG_BUSY;
> + ring->flags |= (cfg->flags & K3_RINGACC_RING_SHARED) ?
> + K3_RING_FLAG_SHARED : 0;
> +
> + k3_ringacc_ring_dump(ring);
> +
> + return 0;
> +
> +err_free_mem:
> + dma_free_coherent(ringacc->dev,
> + ring->size * (4 << ring->elm_size),
> + ring->ring_mem_virt,
> + ring->ring_mem_dma);
> +err_free_ops:
> + ring->ops = NULL;
> +err_free_proxy:
> + ring->proxy = NULL;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_cfg);
> +
> +u32 k3_ringacc_ring_get_size(struct k3_ring *ring)
> +{
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + return ring->size;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_get_size);
> +
> +u32 k3_ringacc_ring_get_free(struct k3_ring *ring)
> +{
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + if (!ring->free)
> + ring->free = ring->size - dbg_readl(&ring->rt->occ);
> +
> + return ring->free;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_get_free);
> +
> +u32 k3_ringacc_ring_get_occ(struct k3_ring *ring)
> +{
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + return dbg_readl(&ring->rt->occ);
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_get_occ);
> +
> +u32 k3_ringacc_ring_is_full(struct k3_ring *ring)
> +{
> + return !k3_ringacc_ring_get_free(ring);
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_is_full);
> +
> +enum k3_ringacc_access_mode {
> + K3_RINGACC_ACCESS_MODE_PUSH_HEAD,
> + K3_RINGACC_ACCESS_MODE_POP_HEAD,
> + K3_RINGACC_ACCESS_MODE_PUSH_TAIL,
> + K3_RINGACC_ACCESS_MODE_POP_TAIL,
> + K3_RINGACC_ACCESS_MODE_PEEK_HEAD,
> + K3_RINGACC_ACCESS_MODE_PEEK_TAIL,
> +};
> +
> +static int k3_ringacc_ring_cfg_proxy(struct k3_ring *ring,
> + enum k3_ringacc_proxy_access_mode mode)
> +{
> + u32 val;
> +
> + val = ring->ring_id;
> + val |= mode << 16;
> + val |= ring->elm_size << 24;
Would be nice to have these magic shifts as defines.
> + dbg_writel(val, &ring->proxy->control);
> + return 0;
> +}
> +
> +static int k3_ringacc_ring_access_proxy(struct k3_ring *ring, void *elem,
> + enum k3_ringacc_access_mode access_mode)
> +{
> + void __iomem *ptr;
> +
> + ptr = (void __iomem *)&ring->proxy->data;
> +
> + switch (access_mode) {
> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
> + k3_ringacc_ring_cfg_proxy(ring, PROXY_ACCESS_MODE_HEAD);
> + break;
> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
> + k3_ringacc_ring_cfg_proxy(ring, PROXY_ACCESS_MODE_TAIL);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ptr += k3_ringacc_ring_get_fifo_pos(ring);
> +
> + switch (access_mode) {
> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
> + k3_nav_dbg(ring->parent->dev, "proxy:memcpy_fromio(x): --> ptr(%p), mode:%d\n",
> + ptr, access_mode);
> + memcpy_fromio(elem, ptr, (4 << ring->elm_size));
> + ring->occ--;
> + break;
> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
> + k3_nav_dbg(ring->parent->dev, "proxy:memcpy_toio(x): --> ptr(%p), mode:%d\n",
> + ptr, access_mode);
> + memcpy_toio(ptr, elem, (4 << ring->elm_size));
> + ring->free--;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + k3_nav_dbg(ring->parent->dev, "proxy: free%d occ%d\n",
> + ring->free, ring->occ);
> + return 0;
> +}
> +
> +static int k3_ringacc_ring_push_head_proxy(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_proxy(ring, elem,
> + K3_RINGACC_ACCESS_MODE_PUSH_HEAD);
> +}
> +
> +static int k3_ringacc_ring_push_tail_proxy(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_proxy(ring, elem,
> + K3_RINGACC_ACCESS_MODE_PUSH_TAIL);
> +}
> +
> +static int k3_ringacc_ring_pop_head_proxy(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_proxy(ring, elem,
> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
> +}
> +
> +static int k3_ringacc_ring_pop_tail_proxy(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_proxy(ring, elem,
> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
> +}
> +
> +static int k3_ringacc_ring_access_io(struct k3_ring *ring, void *elem,
> + enum k3_ringacc_access_mode access_mode)
> +{
> + void __iomem *ptr;
> +
> + switch (access_mode) {
> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
> + ptr = (void __iomem *)&ring->fifos->head_data;
> + break;
> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
> + ptr = (void __iomem *)&ring->fifos->tail_data;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ptr += k3_ringacc_ring_get_fifo_pos(ring);
> +
> + switch (access_mode) {
> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
> + k3_nav_dbg(ring->parent->dev, "memcpy_fromio(x): --> ptr(%p), mode:%d\n",
> + ptr, access_mode);
> + memcpy_fromio(elem, ptr, (4 << ring->elm_size));
> + ring->occ--;
> + break;
> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
> + k3_nav_dbg(ring->parent->dev, "memcpy_toio(x): --> ptr(%p), mode:%d\n",
> + ptr, access_mode);
> + memcpy_toio(ptr, elem, (4 << ring->elm_size));
> + ring->free--;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + k3_nav_dbg(ring->parent->dev, "free%d index%d occ%d index%d\n",
> + ring->free, ring->windex, ring->occ, ring->rindex);
> + return 0;
> +}
> +
> +static int k3_ringacc_ring_push_head_io(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_io(ring, elem,
> + K3_RINGACC_ACCESS_MODE_PUSH_HEAD);
> +}
> +
> +static int k3_ringacc_ring_push_io(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_io(ring, elem,
> + K3_RINGACC_ACCESS_MODE_PUSH_TAIL);
> +}
> +
> +static int k3_ringacc_ring_pop_io(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_io(ring, elem,
> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
> +}
> +
> +static int k3_ringacc_ring_pop_tail_io(struct k3_ring *ring, void *elem)
> +{
> + return k3_ringacc_ring_access_io(ring, elem,
> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
> +}
> +
> +static int k3_ringacc_ring_push_mem(struct k3_ring *ring, void *elem)
> +{
> + void *elem_ptr;
> +
> + elem_ptr = k3_ringacc_get_elm_addr(ring, ring->windex);
> +
> + memcpy(elem_ptr, elem, (4 << ring->elm_size));
> +
> + ring->windex = (ring->windex + 1) % ring->size;
> + ring->free--;
> + dbg_writel(1, &ring->rt->db);
> +
> + k3_nav_dbg(ring->parent->dev, "ring_push_mem: free%d index%d\n",
> + ring->free, ring->windex);
> +
> + return 0;
> +}
> +
> +static int k3_ringacc_ring_pop_mem(struct k3_ring *ring, void *elem)
> +{
> + void *elem_ptr;
> +
> + elem_ptr = k3_ringacc_get_elm_addr(ring, ring->rindex);
> +
> + memcpy(elem, elem_ptr, (4 << ring->elm_size));
> +
> + ring->rindex = (ring->rindex + 1) % ring->size;
> + ring->occ--;
> + dbg_writel(-1, &ring->rt->db);
> +
> + k3_nav_dbg(ring->parent->dev, "ring_pop_mem: occ%d index%d pos_ptr%p\n",
> + ring->occ, ring->rindex, elem_ptr);
> + return 0;
> +}
> +
> +int k3_ringacc_ring_push(struct k3_ring *ring, void *elem)
> +{
> + int ret = -EOPNOTSUPP;
> +
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + k3_nav_dbg(ring->parent->dev, "ring_push: free%d index%d\n",
> + ring->free, ring->windex);
> +
> + if (k3_ringacc_ring_is_full(ring))
> + return -ENOMEM;
> +
> + if (ring->ops && ring->ops->push_tail)
> + ret = ring->ops->push_tail(ring, elem);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_push);
> +
> +int k3_ringacc_ring_push_head(struct k3_ring *ring, void *elem)
> +{
> + int ret = -EOPNOTSUPP;
> +
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + k3_nav_dbg(ring->parent->dev, "ring_push_head: free%d index%d\n",
> + ring->free, ring->windex);
> +
> + if (k3_ringacc_ring_is_full(ring))
> + return -ENOMEM;
> +
> + if (ring->ops && ring->ops->push_head)
> + ret = ring->ops->push_head(ring, elem);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_push_head);
> +
> +int k3_ringacc_ring_pop(struct k3_ring *ring, void *elem)
> +{
> + int ret = -EOPNOTSUPP;
> +
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + if (!ring->occ)
> + ring->occ = k3_ringacc_ring_get_occ(ring);
> +
> + k3_nav_dbg(ring->parent->dev, "ring_pop: occ%d index%d\n",
> + ring->occ, ring->rindex);
> +
> + if (!ring->occ)
> + return -ENODATA;
> +
> + if (ring->ops && ring->ops->pop_head)
> + ret = ring->ops->pop_head(ring, elem);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_pop);
> +
> +int k3_ringacc_ring_pop_tail(struct k3_ring *ring, void *elem)
> +{
> + int ret = -EOPNOTSUPP;
> +
> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
> + return -EINVAL;
> +
> + if (!ring->occ)
> + ring->occ = k3_ringacc_ring_get_occ(ring);
> +
> + k3_nav_dbg(ring->parent->dev, "ring_pop_tail: occ%d index%d\n",
> + ring->occ, ring->rindex);
> +
> + if (!ring->occ)
> + return -ENODATA;
> +
> + if (ring->ops && ring->ops->pop_tail)
> + ret = ring->ops->pop_tail(ring, elem);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_pop_tail);
> +
> +struct k3_ringacc *of_k3_ringacc_get_by_phandle(struct device_node *np,
> + const char *property)
> +{
> + struct device_node *ringacc_np;
> + struct k3_ringacc *ringacc = ERR_PTR(-EPROBE_DEFER);
> + struct k3_ringacc *entry;
> +
> + ringacc_np = of_parse_phandle(np, property, 0);
> + if (!ringacc_np)
> + return ERR_PTR(-ENODEV);
> +
> + mutex_lock(&k3_ringacc_list_lock);
> + list_for_each_entry(entry, &k3_ringacc_list, list)
> + if (entry->dev->of_node == ringacc_np) {
> + ringacc = entry;
> + break;
> + }
> + mutex_unlock(&k3_ringacc_list_lock);
> + of_node_put(ringacc_np);
> +
> + return ringacc;
> +}
> +EXPORT_SYMBOL_GPL(of_k3_ringacc_get_by_phandle);
> +
> +static int k3_ringacc_probe_dt(struct k3_ringacc *ringacc)
> +{
> + struct device_node *node = ringacc->dev->of_node;
> + struct device *dev = ringacc->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + int ret;
> +
> + if (!node) {
> + dev_err(dev, "device tree info unavailable\n");
> + return -ENODEV;
> + }
> +
> + ret = of_property_read_u32(node, "ti,num-rings", &ringacc->num_rings);
> + if (ret) {
> + dev_err(dev, "ti,num-rings read failure %d\n", ret);
> + return ret;
> + }
> +
> + ringacc->dma_ring_reset_quirk =
> + of_property_read_bool(node, "ti,dma-ring-reset-quirk");
> +
> + ringacc->tisci = ti_sci_get_by_phandle(node, "ti,sci");
> + if (IS_ERR(ringacc->tisci)) {
> + ret = PTR_ERR(ringacc->tisci);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "ti,sci read fail %d\n", ret);
> + ringacc->tisci = NULL;
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "ti,sci-dev-id",
> + &ringacc->tisci_dev_id);
> + if (ret) {
> + dev_err(dev, "ti,sci-dev-id read fail %d\n", ret);
> + return ret;
> + }
> +
> + pdev->id = ringacc->tisci_dev_id;
> +
> + ringacc->rm_gp_range = devm_ti_sci_get_of_resource(ringacc->tisci, dev,
> + ringacc->tisci_dev_id,
> + "ti,sci-rm-range-gp-rings");
> + if (IS_ERR(ringacc->rm_gp_range)) {
> + dev_err(dev, "Failed to allocate MSI interrupts\n");
> + return PTR_ERR(ringacc->rm_gp_range);
> + }
> +
> + return ti_sci_inta_msi_domain_alloc_irqs(ringacc->dev,
> + ringacc->rm_gp_range);
> +}
> +
> +static int k3_ringacc_probe(struct platform_device *pdev)
> +{
> + struct k3_ringacc *ringacc;
> + void __iomem *base_fifo, *base_rt;
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + int ret, i;
> +
> + ringacc = devm_kzalloc(dev, sizeof(*ringacc), GFP_KERNEL);
> + if (!ringacc)
> + return -ENOMEM;
> +
> + ringacc->dev = dev;
> + mutex_init(&ringacc->req_lock);
> +
> + dev->msi_domain = of_msi_get_domain(dev, dev->of_node,
> + DOMAIN_BUS_TI_SCI_INTA_MSI);
> + if (!dev->msi_domain) {
> + dev_err(dev, "Failed to get MSI domain\n");
> + return -EPROBE_DEFER;
> + }
> +
> + ret = k3_ringacc_probe_dt(ringacc);
> + if (ret)
> + return ret;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rt");
> + base_rt = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base_rt))
> + return PTR_ERR(base_rt);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fifos");
> + base_fifo = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base_fifo))
> + return PTR_ERR(base_fifo);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "proxy_gcfg");
> + ringacc->proxy_gcfg = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ringacc->proxy_gcfg))
> + return PTR_ERR(ringacc->proxy_gcfg);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "proxy_target");
> + ringacc->proxy_target_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ringacc->proxy_target_base))
> + return PTR_ERR(ringacc->proxy_target_base);
> +
> + ringacc->num_proxies = dbg_readl(&ringacc->proxy_gcfg->config) &
> + K3_RINGACC_PROXY_CFG_THREADS_MASK;
> +
> + ringacc->rings = devm_kzalloc(dev,
> + sizeof(*ringacc->rings) *
> + ringacc->num_rings,
> + GFP_KERNEL);
> + ringacc->rings_inuse = devm_kcalloc(dev,
> + BITS_TO_LONGS(ringacc->num_rings),
> + sizeof(unsigned long), GFP_KERNEL);
> + ringacc->proxy_inuse = devm_kcalloc(dev,
> + BITS_TO_LONGS(ringacc->num_proxies),
> + sizeof(unsigned long), GFP_KERNEL);
> +
> + if (!ringacc->rings || !ringacc->rings_inuse || !ringacc->proxy_inuse)
> + return -ENOMEM;
> +
> + for (i = 0; i < ringacc->num_rings; i++) {
> + ringacc->rings[i].rt = base_rt +
> + K3_RINGACC_RT_REGS_STEP * i;
> + ringacc->rings[i].fifos = base_fifo +
> + K3_RINGACC_FIFO_REGS_STEP * i;
> + ringacc->rings[i].parent = ringacc;
> + ringacc->rings[i].ring_id = i;
> + ringacc->rings[i].proxy_id = K3_RINGACC_PROXY_NOT_USED;
> + }
> + dev_set_drvdata(dev, ringacc);
> +
> + ringacc->tisci_ring_ops = &ringacc->tisci->ops.rm_ring_ops;
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(dev);
> + dev_err(dev, "Failed to enable pm %d\n", ret);
> + goto err;
> + }
> +
> + mutex_lock(&k3_ringacc_list_lock);
> + list_add_tail(&ringacc->list, &k3_ringacc_list);
> + mutex_unlock(&k3_ringacc_list_lock);
> +
> + dev_info(dev, "Ring Accelerator probed rings:%u, gp-rings[%u,%u] sci-dev-id:%u\n",
> + ringacc->num_rings,
> + ringacc->rm_gp_range->desc[0].start,
> + ringacc->rm_gp_range->desc[0].num,
> + ringacc->tisci_dev_id);
> + dev_info(dev, "dma-ring-reset-quirk: %s\n",
> + ringacc->dma_ring_reset_quirk ? "enabled" : "disabled");
> + dev_info(dev, "RA Proxy rev. %08x, num_proxies:%u\n",
> + dbg_readl(&ringacc->proxy_gcfg->revision),
> + ringacc->num_proxies);
> + return 0;
> +
> +err:
> + pm_runtime_disable(dev);
> + return ret;
> +}
> +
> +static int k3_ringacc_remove(struct platform_device *pdev)
> +{
> + struct k3_ringacc *ringacc = dev_get_drvdata(&pdev->dev);
> +
> + pm_runtime_put_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> +
> + mutex_lock(&k3_ringacc_list_lock);
> + list_del(&ringacc->list);
> + mutex_unlock(&k3_ringacc_list_lock);
> + return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id k3_ringacc_of_match[] = {
> + { .compatible = "ti,am654-navss-ringacc", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, k3_ringacc_of_match);
> +
> +static struct platform_driver k3_ringacc_driver = {
> + .probe = k3_ringacc_probe,
> + .remove = k3_ringacc_remove,
> + .driver = {
> + .name = "k3-ringacc",
> + .of_match_table = k3_ringacc_of_match,
> + },
> +};
> +module_platform_driver(k3_ringacc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("TI Ringacc driver for K3 SOCs");
> +MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
> diff --git a/include/linux/soc/ti/k3-ringacc.h b/include/linux/soc/ti/k3-ringacc.h
> new file mode 100644
> index 000000000000..debffba48ac9
> --- /dev/null
> +++ b/include/linux/soc/ti/k3-ringacc.h
> @@ -0,0 +1,262 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * K3 Ring Accelerator (RA) subsystem interface
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
> + */
> +
> +#ifndef __SOC_TI_K3_RINGACC_API_H_
> +#define __SOC_TI_K3_RINGACC_API_H_
> +
> +#include <linux/types.h>
> +
> +struct device_node;
> +
> +/**
> + * enum k3_ring_mode - &struct k3_ring_cfg mode
> + *
> + * RA ring operational modes
> + *
> + * @K3_RINGACC_RING_MODE_RING: Exposed Ring mode for SW direct access
> + * @K3_RINGACC_RING_MODE_MESSAGE: Messaging mode. Messaging mode requires
> + * that all accesses to the queue must go through this IP so that all
> + * accesses to the memory are controlled and ordered. This IP then
> + * controls the entire state of the queue, and SW has no directly control,
> + * such as through doorbells and cannot access the storage memory directly.
> + * This is particularly useful when more than one SW or HW entity can be
> + * the producer and/or consumer at the same time
> + * @K3_RINGACC_RING_MODE_CREDENTIALS: Credentials mode is message mode plus
> + * stores credentials with each message, requiring the element size to be
> + * doubled to fit the credentials. Any exposed memory should be protected
> + * by a firewall from unwanted access
> + * @K3_RINGACC_RING_MODE_QM: Queue manager mode. This takes the credentials
> + * mode and adds packet length per element, along with additional read only
> + * fields for element count and accumulated queue length. The QM mode only
> + * operates with an 8 byte element size (any other element size is
> + * illegal), and like in credentials mode each operation uses 2 element
> + * slots to store the credentials and length fields
> + */
> +enum k3_ring_mode {
> + K3_RINGACC_RING_MODE_RING = 0,
> + K3_RINGACC_RING_MODE_MESSAGE,
> + K3_RINGACC_RING_MODE_CREDENTIALS,
> + K3_RINGACC_RING_MODE_QM,
> + K3_RINGACC_RING_MODE_INVALID
> +};
> +
> +/**
> + * enum k3_ring_size - &struct k3_ring_cfg elm_size
> + *
> + * RA ring element's sizes in bytes.
> + */
> +enum k3_ring_size {
> + K3_RINGACC_RING_ELSIZE_4 = 0,
> + K3_RINGACC_RING_ELSIZE_8,
> + K3_RINGACC_RING_ELSIZE_16,
> + K3_RINGACC_RING_ELSIZE_32,
> + K3_RINGACC_RING_ELSIZE_64,
> + K3_RINGACC_RING_ELSIZE_128,
> + K3_RINGACC_RING_ELSIZE_256,
> + K3_RINGACC_RING_ELSIZE_INVALID
> +};
> +
> +struct k3_ringacc;
> +struct k3_ring;
> +
> +/**
> + * enum k3_ring_cfg - RA ring configuration structure
> + *
> + * @size: Ring size, number of elements
> + * @elm_size: Ring element size
> + * @mode: Ring operational mode
> + * @flags: Ring configuration flags. Possible values:
> + * @K3_RINGACC_RING_SHARED: when set allows to request the same ring
> + * few times. It's usable when the same ring is used as Free Host PD ring
> + * for different flows, for example.
> + * Note: Locking should be done by consumer if required
> + */
> +struct k3_ring_cfg {
> + u32 size;
> + enum k3_ring_size elm_size;
> + enum k3_ring_mode mode;
> +#define K3_RINGACC_RING_SHARED BIT(1)
> + u32 flags;
> +};
> +
> +#define K3_RINGACC_RING_ID_ANY (-1)
> +
> +/**
> + * of_k3_ringacc_get_by_phandle - find a RA by phandle property
> + * @np: device node
> + * @propname: property name containing phandle on RA node
> + *
> + * Returns pointer on the RA - struct k3_ringacc
> + * or -ENODEV if not found,
> + * or -EPROBE_DEFER if not yet registered
> + */
> +struct k3_ringacc *of_k3_ringacc_get_by_phandle(struct device_node *np,
> + const char *property);
> +
> +#define K3_RINGACC_RING_USE_PROXY BIT(1)
> +
> +/**
> + * k3_ringacc_request_ring - request ring from ringacc
> + * @ringacc: pointer on ringacc
> + * @id: ring id or K3_RINGACC_RING_ID_ANY for any general purpose ring
> + * @flags:
> + * @K3_RINGACC_RING_USE_PROXY: if set - proxy will be allocated and
> + * used to access ring memory. Sopported only for rings in
> + * Message/Credentials/Queue mode.
> + *
> + * Returns pointer on the Ring - struct k3_ring
> + * or NULL in case of failure.
> + */
> +struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
> + int id, u32 flags);
> +
> +/**
> + * k3_ringacc_ring_reset - ring reset
> + * @ring: pointer on Ring
> + *
> + * Resets ring internal state ((hw)occ, (hw)idx).
> + * TODO_GS: ? Ring can be reused without reconfiguration
> + */
> +void k3_ringacc_ring_reset(struct k3_ring *ring);
> +/**
> + * k3_ringacc_ring_reset - ring reset for DMA rings
> + * @ring: pointer on Ring
> + *
> + * Resets ring internal state ((hw)occ, (hw)idx). Should be used for rings
> + * which are read by K3 UDMA, like TX or Free Host PD rings.
> + */
> +void k3_ringacc_ring_reset_dma(struct k3_ring *ring, u32 occ);
> +
> +/**
> + * k3_ringacc_ring_free - ring free
> + * @ring: pointer on Ring
> + *
> + * Resets ring and free all alocated resources.
> + */
> +int k3_ringacc_ring_free(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_get_ring_id - Get the Ring ID
> + * @ring: pointer on ring
> + *
> + * Returns the Ring ID
> + */
> +u32 k3_ringacc_get_ring_id(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_get_ring_irq_num - Get the irq number for the ring
> + * @ring: pointer on ring
> + *
> + * Returns the interrupt number which can be used to request the interrupt
> + */
> +int k3_ringacc_get_ring_irq_num(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_ring_cfg - ring configure
> + * @ring: pointer on ring
> + * @cfg: Ring configuration parameters (see &struct k3_ring_cfg)
> + *
> + * Configures ring, including ring memory allocation.
> + * Returns 0 on success, errno otherwise.
> + */
> +int k3_ringacc_ring_cfg(struct k3_ring *ring, struct k3_ring_cfg *cfg);
> +
> +/**
> + * k3_ringacc_ring_get_size - get ring size
> + * @ring: pointer on ring
> + *
> + * Returns ring size in number of elements.
> + */
> +u32 k3_ringacc_ring_get_size(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_ring_get_free - get free elements
> + * @ring: pointer on ring
> + *
> + * Returns number of free elements in the ring.
> + */
> +u32 k3_ringacc_ring_get_free(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_ring_get_occ - get ring occupancy
> + * @ring: pointer on ring
> + *
> + * Returns total number of valid entries on the ring
> + */
> +u32 k3_ringacc_ring_get_occ(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_ring_is_full - checks if ring is full
> + * @ring: pointer on ring
> + *
> + * Returns true if the ring is full
> + */
> +u32 k3_ringacc_ring_is_full(struct k3_ring *ring);
> +
> +/**
> + * k3_ringacc_ring_push - push element to the ring tail
> + * @ring: pointer on ring
> + * @elem: pointer on ring element buffer
> + *
> + * Push one ring element to the ring tail. Size of the ring element is
> + * determined by ring configuration &struct k3_ring_cfg elm_size.
> + *
> + * Returns 0 on success, errno otherwise.
> + */
> +int k3_ringacc_ring_push(struct k3_ring *ring, void *elem);
> +
> +/**
> + * k3_ringacc_ring_pop - pop element from the ring head
> + * @ring: pointer on ring
> + * @elem: pointer on ring element buffer
> + *
> + * Push one ring element from the ring head. Size of the ring element is
> + * determined by ring configuration &struct k3_ring_cfg elm_size..
> + *
> + * Returns 0 on success, errno otherwise.
> + */
> +int k3_ringacc_ring_pop(struct k3_ring *ring, void *elem);
> +
> +/**
> + * k3_ringacc_ring_push_head - push element to the ring head
> + * @ring: pointer on ring
> + * @elem: pointer on ring element buffer
> + *
> + * Push one ring element to the ring head. Size of the ring element is
> + * determined by ring configuration &struct k3_ring_cfg elm_size.
> + *
> + * Returns 0 on success, errno otherwise.
> + * Not Supported by ring modes: K3_RINGACC_RING_MODE_RING
> + */
> +int k3_ringacc_ring_push_head(struct k3_ring *ring, void *elem);
> +
> +/**
> + * k3_ringacc_ring_pop_tail - pop element from the ring tail
> + * @ring: pointer on ring
> + * @elem: pointer on ring element buffer
> + *
> + * Push one ring element from the ring tail. Size of the ring element is
> + * determined by ring configuration &struct k3_ring_cfg elm_size.
> + *
> + * Returns 0 on success, errno otherwise.
> + * Not Supported by ring modes: K3_RINGACC_RING_MODE_RING
> + */
> +int k3_ringacc_ring_pop_tail(struct k3_ring *ring, void *elem);
> +
> +u32 k3_ringacc_get_tisci_dev_id(struct k3_ring *ring);
> +
> +/**
> + * Debugging definitions
> + * TODO: might be removed
> + */
> +#ifdef CONFIG_TI_K3_RINGACC_DEBUG
> +void k3_ringacc_ring_dump(struct k3_ring *ring);
> +#else
> +static inline void k3_ringacc_ring_dump(struct k3_ring *ring) {};
> +#endif
> +
> +#endif /* __SOC_TI_K3_RINGACC_API_H_ */
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
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/1] mm/pgtable/debug: Add test validating architecture page table helpers
From: Anshuman Khandual @ 2019-09-09 6:26 UTC (permalink / raw)
To: Gerald Schaefer
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, linux-mm, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390,
Michael Ellerman, x86, Russell King - ARM Linux, Matthew Wilcox,
Steven Price, Jason Gunthorpe, linux-arm-kernel, linux-snps-arc,
Kees Cook, Masahiro Yamada, Mark Brown, Dan Williams,
Vlastimil Babka, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, linux-mips, Ralf Baechle, linux-kernel,
Paul Burton, Mike Rapoport, Vineet Gupta, Martin Schwidefsky,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20190906210346.5ecbff01@thinkpad>
On 09/07/2019 12:33 AM, Gerald Schaefer wrote:
> On Fri, 6 Sep 2019 11:58:59 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>
>> On 09/05/2019 10:36 PM, Gerald Schaefer wrote:
>>> On Thu, 5 Sep 2019 14:48:14 +0530
>>> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>
>>>>> [...]
>>>>>> +
>>>>>> +#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
>>>>>> +static void pud_clear_tests(pud_t *pudp)
>>>>>> +{
>>>>>> + memset(pudp, RANDOM_NZVALUE, sizeof(pud_t));
>>>>>> + pud_clear(pudp);
>>>>>> + WARN_ON(!pud_none(READ_ONCE(*pudp)));
>>>>>> +}
>>>>>
>>>>> For pgd/p4d/pud_clear(), we only clear if the page table level is present
>>>>> and not folded. The memset() here overwrites the table type bits, so
>>>>> pud_clear() will not clear anything on s390 and the pud_none() check will
>>>>> fail.
>>>>> Would it be possible to OR a (larger) random value into the table, so that
>>>>> the lower 12 bits would be preserved?
>>>>
>>>> So the suggestion is instead of doing memset() on entry with RANDOM_NZVALUE,
>>>> it should OR a large random value preserving lower 12 bits. Hmm, this should
>>>> still do the trick for other platforms, they just need non zero value. So on
>>>> s390, the lower 12 bits on the page table entry already has valid value while
>>>> entering this function which would make sure that pud_clear() really does
>>>> clear the entry ?
>>>
>>> Yes, in theory the table entry on s390 would have the type set in the last
>>> 4 bits, so preserving those would be enough. If it does not conflict with
>>> others, I would still suggest preserving all 12 bits since those would contain
>>> arch-specific flags in general, just to be sure. For s390, the pte/pmd tests
>>> would also work with the memset, but for consistency I think the same logic
>>> should be used in all pxd_clear_tests.
>>
>> Makes sense but..
>>
>> There is a small challenge with this. Modifying individual bits on a given
>> page table entry from generic code like this test case is bit tricky. That
>> is because there are not enough helpers to create entries with an absolute
>> value. This would have been easier if all the platforms provided functions
>> like __pxx() which is not the case now. Otherwise something like this should
>> have worked.
>>
>>
>> pud_t pud = READ_ONCE(*pudp);
>> pud = __pud(pud_val(pud) | RANDOM_VALUE (keeping lower 12 bits 0))
>> WRITE_ONCE(*pudp, pud);
>>
>> But __pud() will fail to build in many platforms.
>
> Hmm, I simply used this on my system to make pud_clear_tests() work, not
> sure if it works on all archs:
>
> pud_val(*pudp) |= RANDOM_NZVALUE;
Which compiles on arm64 but then fails on x86 because of the way pmd_val()
has been defined there. on arm64 and s390 (with many others) pmd_val() is
a macro which still got the variable that can be used as lvalue but that is
not true for some other platforms like x86.
arch/arm64/include/asm/pgtable-types.h: #define pmd_val(x) ((x).pmd)
arch/s390/include/asm/page.h: #define pmd_val(x) ((x).pmd)
arch/x86/include/asm/pgtable.h: #define pmd_val(x) native_pmd_val(x)
static inline pmdval_t native_pmd_val(pmd_t pmd)
{
return pmd.pmd;
}
Unless I am mistaken, the return value from this function can not be used as
lvalue for future assignments.
mm/arch_pgtable_test.c: In function ‘pud_clear_tests’:
mm/arch_pgtable_test.c:156:17: error: lvalue required as left operand of assignment
pud_val(*pudp) |= RANDOM_ORVALUE;
^~
AFAICS pxx_val() were never intended to be used as lvalue and using it that way
might just happen to work on all those platforms which define them as macros.
They meant to just provide values for an entry as being determined by the platform.
In principle pxx_val() on an entry was not supposed to be modified directly from
generic code without going through (again) platform helpers for any specific state
change (write, old, dirty, special, huge etc). The current use case is a deviation
for that.
I originally went with memset() just to load up the entries with non-zero value so
that we know pxx_clear() are really doing the clearing. The same is being followed
for all pxx_same() checks.
Another way for fixing the problem would be to mark them with known attributes
like write/young/huge etc instead which for sure will create non-zero entries.
We can do that for pxx_clear() and pxx_same() tests and drop RANDOM_NZVALUE
completely. Does that sound good ?
>
>>
>> The other alternative will be to make sure memset() happens on all other
>> bits except the lower 12 bits which will depend on endianness. If s390
>> has a fixed endianness, we can still use either of them which will hold
>> good for others as well.
>>
>> memset(pudp, RANDOM_NZVALUE, sizeof(pud_t) - 3);
>>
>> OR
>>
>> memset(pudp + 3, RANDOM_NZVALUE, sizeof(pud_t) - 3);
>>
>>>
>>> However, there is another issue on s390 which will make this only work
>>> for pud_clear_tests(), and not for the p4d/pgd_tests. The problem is that
>>> mm_alloc() will only give you a 3-level page table initially on s390.
>>> This means that pudp == p4dp == pgdp, and so the p4d/pgd_tests will
>>> both see the pud level (of course this also affects other tests).
>>
>> Got it.
>>
>>>
>>> Not sure yet how to fix this, i.e. how to initialize/update the page table
>>> to 5 levels. We can handle 5 level page tables, and it would be good if
>>> all levels could be tested, but using mm_alloc() to establish the page
>>> tables might not work on s390. One option could be to provide an arch-hook
>>> or weak function to allocate/initialize the mm.
>>
>> Sure, got it. Though I plan to do add some arch specific tests or init sequence
>> like the above later on but for now the idea is to get the smallest possible set
>> of test cases which builds and runs on all platforms without requiring any arch
>> specific hooks or special casing (#ifdef) to be agreed upon broadly and accepted.
>>
>> Do you think this is absolutely necessary on s390 for the very first set of test
>> cases or we can add this later on as an improvement ?
>
> It can be added later, no problem. I did not expect this to work flawlessly
> on s390 right from the start anyway, with all our peculiarities, so don't
> let this hinder you. I might come up with an add-on patch later.
Sure.
>
> Actually, using get_unmapped_area() as suggested by Kirill could also
> solve this issue. We do create a new mm with 3-level page tables on s390,
> and the dynamic upgrade to 4 or 5 levels is then triggered exactly by
> arch_get_unmapped_area(), depending on the addr. But I currently don't
> see how / where arch_get_unmapped_area() is set up for such a dummy mm
> created by mm_alloc().
Normally they are set during program loading but we can set it up explicitly
for the test mm_struct if we need to but there are some other challenges.
load_[aout|elf|flat|..]_binary()
setup_new_exec()
arch_pick_mmap_layout().
I did some initial experiments around get_unmapped_area(). Seems bit tricky
to get it working on a pure 'test' mm_struct. It expects a real user context
in the form of current->mm.
get_unmapped_area()
{
....
get_area = current->mm->get_unmapped_area;
....
addr = get_area(file, addr, len, pgoff, flags); {
....
struct mm_struct *mm = current->mm;
....
if (addr) {
...
vma = find_vma_prev(mm, addr, &prev);
}
....
vm_unmapped_area() {
struct mm_struct *mm = current->mm;
....
/* Walks across mm->mm_rb.rb_node */
}
}
....
}
Simple call like get_unmapped_area(NULL, 0, PAGE_SIZE, 0, 0) to get an
address fails right away on current->mm->get_unmapped_area which does
not have a valid value in the kernel context.
There might be two methods to get around this problem
1) Write a custom get_unmapped_area() imitating the real one but going
around the problem by taking an appropriately initialized mm_struct
instead of current->mm.
2) Create dummy user task with dummy mm, switch 'current' context before
calling into get_unmapped_area() and switch back again. Dont know if
this is even possible.
Wondering if this might deviate too much from the original goal of
testing the page table helpers.
Looking back again at the proposed test vaddr, wondering what will be the
real problem in case it goes beyond user address range ? Will pxx_alloc()
fail to create page table ranges at required level ? Apart from skipping
pgtable_page_ctor/dtor for page table pages, it might not really affect
any helpers as such.
VADDR_TEST (PGDIR_SIZE + [P4D_SIZE] + PUD_SIZE + PMD_SIZE + PAGE_SIZE)
OR
A random page aligned address in [FIRST_USER_ADDRESS..TASK_SIZE] range ?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V4, 1/2] media: dt-bindings: media: i2c: Add bindings for OV02A10
From: Sakari Ailus @ 2019-09-09 6:46 UTC (permalink / raw)
To: dongchun.zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <20190907092728.23897-2-dongchun.zhu@mediatek.com>
Hi Dongchun,
On Sat, Sep 07, 2019 at 05:27:27PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
>
> This patch adds device tree bindings documentation for OV02A10 camera sensor.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
Please remember to add acks you get.
> ---
> .../devicetree/bindings/media/i2c/ov02a10.txt | 54 ++++++++++++++++++++++
> MAINTAINERS | 7 +++
> 2 files changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov02a10.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov02a10.txt b/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> new file mode 100644
> index 0000000..18acc4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> @@ -0,0 +1,54 @@
> +* Omnivision OV02A10 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: shall be "ovti,ov02a10"
> +- clocks: reference to the eclk input clock
> +- clock-names: shall be "eclk"
> +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> +- avdd-supply: Analog voltage supply, 2.8 volts
> +- dvdd-supply: Digital core voltage supply, 1.8 volts
> +- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
> + if any. This is an active low signal to the OV02A10.
> +- reset-gpios: reference to the GPIO connected to the reset pin, if any.
> + This is an active high signal to the OV02A10.
> +
> +Optional Properties:
> +- rotation: as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt,
> + valid values are 0 (sensor mounted upright) and 180 (sensor
> + mounted upside down).
This needs to be either mandatory or the default needs to be documented. I
think it could be simply:
Documentation/devicetree/bindings/media/video-interfaces.txt,
valid values are 0 (sensor mounted upright; the default) and
180 (sensor mounted upside down).
> +
> +The device node shall contain one 'port' child node with an
> +'endpoint' subnode for its digital output video port,
> +in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +&i2c4 {
> + ov02a10: camera-sensor@3d {
> + compatible = "ovti,ov02a10";
> + reg = <0x3d>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&camera_pins_cam1_mclk_on>;
> +
> + clocks = <&topckgen CLK_TOP_MUX_CAMTG2>,
> + <&topckgen CLK_TOP_UNIVP_192M_D8>;
> + clock-names = "eclk", "freq_mux";
> + clock-frequency = <24000000>;
> +
> + dovdd-supply = <&mt6358_vcamio_reg>;
> + avdd-supply = <&mt6358_vcama1_reg>;
> + dvdd-supply = <&mt6358_vcn18_reg>;
> + powerdown-gpios = <&pio 107 GPIO_ACTIVE_LOW>;
> + reset-gpios = <&pio 109 GPIO_ACTIVE_HIGH>;
> + rotation = <180>;
> +
> + port {
> + /* MIPI CSI-2 bus endpoint */
> + ov02a10_core: endpoint {
> + remote-endpoint = <&ov02a10_0>;
> + link-frequencies = /bits/ 64 <390000000>;
> + };
> + };
> + };
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 783569e..41734fb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11816,6 +11816,13 @@ T: git git://linuxtv.org/media_tree.git
> S: Maintained
> F: drivers/media/i2c/ov13858.c
>
> +OMNIVISION OV02A10 SENSOR DRIVER
> +M: Dongchun Zhu <dongchun.zhu@mediatek.com>
> +L: linux-media@vger.kernel.org
> +T: git git://linuxtv.org/media_tree.git
> +S: Maintained
> +F: Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> +
> OMNIVISION OV2680 SENSOR DRIVER
> M: Rui Miguel Silva <rmfrfs@gmail.com>
> L: linux-media@vger.kernel.org
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
_______________________________________________
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 04/14] dmaengine: Add metadata_ops for dma_async_tx_descriptor
From: Peter Ujfalusi @ 2019-09-09 6:52 UTC (permalink / raw)
To: Vinod Koul, Radhey Shyam Pandey
Cc: Menon, Nishanth, devicetree, grygorii.strashko, lokeshvutla,
j-keerthy, linux-kernel, t-kristo, tony, robh+dt, ssantosh,
dmaengine, dan.j.williams, linux-arm-kernel
In-Reply-To: <20190908141207.GO2672@vkoul-mobl>
On 08/09/2019 17.12, Vinod Koul wrote:
> On 30-07-19, 12:34, Peter Ujfalusi wrote:
>> The metadata is best described as side band data or parameters traveling
>> alongside the data DMAd by the DMA engine. It is data
>> which is understood by the peripheral and the peripheral driver only, the
>> DMA engine see it only as data block and it is not interpreting it in any
>> way.
>>
>> The metadata can be different per descriptor as it is a parameter for the
>> data being transferred.
>>
>> If the DMA supports per descriptor metadata it can implement the attach,
>> get_ptr/set_len callbacks.
>>
>> Client drivers must only use either attach or get_ptr/set_len to avoid
>> misconfiguration.
>>
>> Client driver can check if a given metadata mode is supported by the
>> channel during probe time with
>> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
>> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_ENGINE);
>>
>> and based on this information can use either mode.
>>
>> Wrappers are also added for the metadata_ops.
>>
>> To be used in DESC_METADATA_CLIENT mode:
>> dmaengine_desc_attach_metadata()
>>
>> To be used in DESC_METADATA_ENGINE mode:
>> dmaengine_desc_get_metadata_ptr()
>> dmaengine_desc_set_metadata_len()
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>> drivers/dma/dmaengine.c | 73 ++++++++++++++++++++++++++
>> include/linux/dmaengine.h | 108 ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 181 insertions(+)
>>
>> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>> index 03ac4b96117c..6baddf7dcbfd 100644
>> --- a/drivers/dma/dmaengine.c
>> +++ b/drivers/dma/dmaengine.c
>> @@ -1302,6 +1302,79 @@ void dma_async_tx_descriptor_init(struct dma_async_tx_descriptor *tx,
>> }
>> EXPORT_SYMBOL(dma_async_tx_descriptor_init);
>>
>> +static inline int desc_check_and_set_metadata_mode(
>> + struct dma_async_tx_descriptor *desc, enum dma_desc_metadata_mode mode)
>> +{
>> + /* Make sure that the metadata mode is not mixed */
>> + if (!desc->desc_metadata_mode) {
>> + if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
>> + desc->desc_metadata_mode = mode;
>
> So do we have different descriptors supporting different modes or is it
> controlled based? For latter we can do this check at controller
> registration!
It is actually on channel basis (in UDMAP):
TR channel does not support metadata at all.
Packet Mode channel have support for metadata, but it might not be
available for certain remote peripherals. PDMAs for example does not use
metadata.
Any channel can be configured as TR or Packet Mode, any channel can
service a peripheral which needs or does not need metadata.
The reason we ended up per descriptor callbacks with Radhey (added to
CC) is that all functions operate on the descriptor and it was natural
to have them attached to the descriptor rather than add channel based
callbacks which must also take the descriptor pointer in addition. The
descriptor have pointer to the channel it is issued on.
I only know if metadata is going to be supported when the channel is
requested, based on the psil-config of the remote thread.
Clients still can check and plan ahead on how to use the metadata.
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V4, 2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Sakari Ailus @ 2019-09-09 6:55 UTC (permalink / raw)
To: dongchun.zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <20190907092728.23897-3-dongchun.zhu@mediatek.com>
Hi Dongchun,
Thanks for the update. A few more comments below.
On Sat, Sep 07, 2019 at 05:27:28PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
>
> This patch adds a V4L2 sub-device driver for OV02A10 image sensor.
> The OV02A10 is a 1/5" CMOS sensor from Omnivision,
> supporting output format: 10-bit Raw.
>
> The OV02A10 has a single MIPI lane interface and use the I2C bus
> for control and the CSI-2 bus for data.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 11 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/ov02a10.c | 1062 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1075 insertions(+)
> create mode 100644 drivers/media/i2c/ov02a10.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 41734fb..4b714a2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11821,6 +11821,7 @@ M: Dongchun Zhu <dongchun.zhu@mediatek.com>
> L: linux-media@vger.kernel.org
> T: git git://linuxtv.org/media_tree.git
> S: Maintained
> +F: drivers/media/i2c/ov02a10.c
> F: Documentation/devicetree/bindings/media/i2c/ov02a10.txt
>
> OMNIVISION OV2680 SENSOR DRIVER
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 79ce9ec..22e0db4 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -617,6 +617,17 @@ config VIDEO_IMX355
> To compile this driver as a module, choose M here: the
> module will be called imx355.
>
> +config VIDEO_OV02A10
> + tristate "OmniVision OV02A10 sensor support"
> + depends on VIDEO_V4L2 && I2C
> + depends on MEDIA_CAMERA_SUPPORT
Please add VIDEO_V4L2_SUBDEV_API
> + help
> + This is a Video4Linux2 sensor driver for the OmniVision
> + OV02A10 camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called ov02a10.
> +
> config VIDEO_OV2640
> tristate "OmniVision OV2640 sensor support"
> depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index fd4ea86..d27f4e2 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
> obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
> obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
> obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
> +obj-$(CONFIG_VIDEO_OV02A10) += ov02a10.o
> obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
> obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
> obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
> diff --git a/drivers/media/i2c/ov02a10.c b/drivers/media/i2c/ov02a10.c
> new file mode 100644
> index 0000000..fb677d5
> --- /dev/null
> +++ b/drivers/media/i2c/ov02a10.c
> @@ -0,0 +1,1062 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 MediaTek Inc.
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-fwnode.h>
> +
> +#define CHIP_ID 0x2509
> +#define OV02A10_REG_CHIP_ID_H 0x02
> +#define OV02A10_REG_CHIP_ID_L 0x03
> +#define OV02A10_ID(_msb, _lsb) ((_msb) << 8 | (_lsb))
> +
> +/* Bit[1] vertical upside down */
> +/* Bit[0] horizontal mirror */
> +#define REG_MIRROR_FLIP_CONTROL 0x3f
> +
> +/* Orientation */
> +#define REG_MIRROR_FLIP_ENABLE 0x03
> +
> +#define REG_PAGE_SWITCH 0xfd
> +#define REG_GLOBAL_EFFECTIVE 0x01
> +#define REG_ENABLE BIT(0)
> +#define OV02A10_MASK_8_BITS 0xff
> +
> +#define REG_SC_CTRL_MODE 0xac
> +#define SC_CTRL_MODE_STANDBY 0x00
> +#define SC_CTRL_MODE_STREAMING 0x01
> +
> +#define OV02A10_EXP_SHIFT 8
> +#define OV02A10_REG_EXPOSURE_H 0x03
> +#define OV02A10_REG_EXPOSURE_L 0x04
> +#define OV02A10_EXPOSURE_MIN 4
> +#define OV02A10_EXPOSURE_MAX_MARGIN 4
> +#define OV02A10_EXPOSURE_STEP 1
> +
> +#define OV02A10_VTS_SHIFT 8
> +#define OV02A10_REG_VTS_H 0x05
> +#define OV02A10_REG_VTS_L 0x06
> +#define OV02A10_VTS_MAX 0x209f
> +#define OV02A10_VTS_MIN 0x04cf
> +#define OV02A10_BASIC_LINE 1224
> +
> +#define OV02A10_REG_GAIN 0x24
> +#define OV02A10_GAIN_MIN 0x10
> +#define OV02A10_GAIN_MAX 0xf8
> +#define OV02A10_GAIN_STEP 0x01
> +#define OV02A10_GAIN_DEFAULT 0x40
> +
> +/* Test pattern control */
> +#define OV02A10_REG_TEST_PATTERN 0xb6
> +#define OV02A10_TEST_PATTERN_ENABLE BIT(0)
> +
> +#define OV02A10_LINK_FREQ_390MHZ 390000000ULL
> +#define OV02A10_ECLK_FREQ 24000000
> +#define OV02A10_DATA_LANES 1
> +#define OV02A10_BITS_PER_SAMPLE 10
> +
> +static const char * const ov02a10_supply_names[] = {
> + "dovdd", /* Digital I/O power */
> + "avdd", /* Analog power */
> + "dvdd", /* Digital core power */
> +};
> +
> +#define OV02A10_NUM_SUPPLIES ARRAY_SIZE(ov02a10_supply_names)
> +
> +struct ov02a10_reg {
> + u8 addr;
> + u8 val;
> +};
> +
> +struct ov02a10_reg_list {
> + u32 num_of_regs;
> + const struct ov02a10_reg *regs;
> +};
> +
> +struct ov02a10_mode {
> + u32 width;
> + u32 height;
> + u32 exp_def;
> + u32 hts_def;
> + u32 vts_def;
> + const struct ov02a10_reg_list reg_list;
> +};
> +
> +struct ov02a10 {
> + struct clk *eclk;
> + u32 eclk_freq;
> +
> + struct gpio_desc *pd_gpio;
> + struct gpio_desc *n_rst_gpio;
> + struct regulator_bulk_data supplies[OV02A10_NUM_SUPPLIES];
> +
> + bool streaming;
> + bool upside_down;
> +
> + /*
> + * Serialize control access, get/set format, get selection
> + * and start streaming.
> + */
> + struct mutex mutex;
> + struct v4l2_subdev subdev;
> + struct media_pad pad;
> + struct v4l2_ctrl *anal_gain;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *hblank;
> + struct v4l2_ctrl *vblank;
> + struct v4l2_ctrl *hflip;
> + struct v4l2_ctrl *vflip;
> + struct v4l2_ctrl *test_pattern;
Of the above references to controls, only exposure appears to be actually
used. Please drop the others.
> + struct v4l2_mbus_framefmt fmt;
> + struct v4l2_ctrl_handler ctrl_handler;
> +
> + const struct ov02a10_mode *cur_mode;
> +};
> +
> +static inline struct ov02a10 *to_ov02a10(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct ov02a10, subdev);
> +}
> +
> +/*
> + * eclk 24Mhz
> + * pclk 39Mhz
> + * linelength 934(0x3a6)
> + * framelength 1390(0x56E)
> + * grabwindow_width 1600
> + * grabwindow_height 1200
> + * max_framerate 30fps
> + * mipi_datarate per lane 780Mbps
> + */
> +static const struct ov02a10_reg ov02a10_1600x1200_regs[] = {
> + {0xfd, 0x01},
> + {0xac, 0x00},
> + {0xfd, 0x00},
> + {0x2f, 0x29},
> + {0x34, 0x00},
> + {0x35, 0x21},
> + {0x30, 0x15},
> + {0x33, 0x01},
> + {0xfd, 0x01},
> + {0x44, 0x00},
> + {0x2a, 0x4c},
> + {0x2b, 0x1e},
> + {0x2c, 0x60},
> + {0x25, 0x11},
> + {0x03, 0x01},
> + {0x04, 0xae},
> + {0x09, 0x00},
> + {0x0a, 0x02},
> + {0x06, 0xa6},
> + {0x31, 0x00},
> + {0x24, 0x40},
> + {0x01, 0x01},
> + {0xfb, 0x73},
> + {0xfd, 0x01},
> + {0x16, 0x04},
> + {0x1c, 0x09},
> + {0x21, 0x42},
> + {0x12, 0x04},
> + {0x13, 0x10},
> + {0x11, 0x40},
> + {0x33, 0x81},
> + {0xd0, 0x00},
> + {0xd1, 0x01},
> + {0xd2, 0x00},
> + {0x50, 0x10},
> + {0x51, 0x23},
> + {0x52, 0x20},
> + {0x53, 0x10},
> + {0x54, 0x02},
> + {0x55, 0x20},
> + {0x56, 0x02},
> + {0x58, 0x48},
> + {0x5d, 0x15},
> + {0x5e, 0x05},
> + {0x66, 0x66},
> + {0x68, 0x68},
> + {0x6b, 0x00},
> + {0x6c, 0x00},
> + {0x6f, 0x40},
> + {0x70, 0x40},
> + {0x71, 0x0a},
> + {0x72, 0xf0},
> + {0x73, 0x10},
> + {0x75, 0x80},
> + {0x76, 0x10},
> + {0x84, 0x00},
> + {0x85, 0x10},
> + {0x86, 0x10},
> + {0x87, 0x00},
> + {0x8a, 0x22},
> + {0x8b, 0x22},
> + {0x19, 0xf1},
> + {0x29, 0x01},
> + {0xfd, 0x01},
> + {0x9d, 0xd6},
> + {0xa0, 0x29},
> + {0xa1, 0x03},
> + {0xad, 0x62},
> + {0xae, 0x00},
> + {0xaf, 0x85},
> + {0xb1, 0x01},
> + {0x8e, 0x06},
> + {0x8f, 0x40},
> + {0x90, 0x04},
> + {0x91, 0xb0},
> + {0x45, 0x01},
> + {0x46, 0x00},
> + {0x47, 0x6c},
> + {0x48, 0x03},
> + {0x49, 0x8b},
> + {0x4a, 0x00},
> + {0x4b, 0x07},
> + {0x4c, 0x04},
> + {0x4d, 0xb7},
> + {0xf0, 0x40},
> + {0xf1, 0x40},
> + {0xf2, 0x40},
> + {0xf3, 0x40},
> + {0x3f, 0x00},
> + {0xfd, 0x01},
> + {0x05, 0x00},
> + {0x06, 0xa6},
> + {0xfd, 0x01},
> +};
> +
> +static const char * const ov02a10_test_pattern_menu[] = {
> + "Disabled",
> + "Color Bar",
> +};
> +
> +static const s64 link_freq_menu_items[] = {
> + OV02A10_LINK_FREQ_390MHZ
> +};
> +
> +static u64 to_pixel_rate(u32 f_index)
> +{
> + u64 pixel_rate = link_freq_menu_items[f_index] * 2 * OV02A10_DATA_LANES;
> +
> + do_div(pixel_rate, OV02A10_BITS_PER_SAMPLE);
> +
> + return pixel_rate;
> +}
> +
> +static const struct ov02a10_mode supported_modes[] = {
> + {
> + .width = 1600,
> + .height = 1200,
> + .exp_def = 0x01ae,
> + .hts_def = 0x03a6,
> + .vts_def = 0x056e,
> + .reg_list = {
> + .num_of_regs = ARRAY_SIZE(ov02a10_1600x1200_regs),
> + .regs = ov02a10_1600x1200_regs,
> + },
> + },
> +};
> +
> +static int ov02a10_write_array(struct ov02a10 *ov02a10,
> + const struct ov02a10_reg_list *r_list)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < r_list->num_of_regs; i++) {
> + ret = i2c_smbus_write_byte_data(client,
> + r_list->regs[i].addr,
> + r_list->regs[i].val);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ov02a10_read_smbus(struct ov02a10 *ov02a10, unsigned char reg,
> + unsigned char *val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret >= 0) {
> + *val = (unsigned char)ret;
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +static void ov02a10_fill_fmt(const struct ov02a10_mode *mode,
> + struct v4l2_mbus_framefmt *fmt)
> +{
> + fmt->width = mode->width;
> + fmt->height = mode->height;
> + fmt->field = V4L2_FIELD_NONE;
> +}
> +
> +static int ov02a10_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> +
> + mutex_lock(&ov02a10->mutex);
> +
> + if (ov02a10->streaming) {
> + mutex_unlock(&ov02a10->mutex);
> + return -EBUSY;
> + }
> +
> + /* Only one sensor mode supported */
> + mbus_fmt->code = ov02a10->fmt.code;
> + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> + ov02a10->fmt = fmt->format;
> +
> + mutex_unlock(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static int ov02a10_get_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> +
> + mutex_lock(&ov02a10->mutex);
> +
> + fmt->format = ov02a10->fmt;
> + mbus_fmt->code = ov02a10->fmt.code;
> + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> +
> + mutex_unlock(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static int ov02a10_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + if (code->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + code->code = ov02a10->fmt.code;
> +
> + return 0;
> +}
> +
> +static int ov02a10_enum_frame_sizes(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + if (fse->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + fse->min_width = supported_modes[fse->index].width;
> + fse->max_width = supported_modes[fse->index].width;
> + fse->max_height = supported_modes[fse->index].height;
> + fse->min_height = supported_modes[fse->index].height;
> +
> + return 0;
> +}
> +
> +static int __ov02a10_power_on(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + struct device *dev = &client->dev;
> + int ret;
> +
> + gpiod_set_value_cansleep(ov02a10->n_rst_gpio, GPIOD_OUT_LOW);
> + gpiod_set_value_cansleep(ov02a10->pd_gpio, GPIOD_OUT_HIGH);
> +
> + ret = clk_prepare_enable(ov02a10->eclk);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable eclk\n");
> + return ret;
> + }
> +
> + ret = regulator_bulk_enable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + goto disable_clk;
> + }
> + usleep_range(5000, 6000);
> +
> + gpiod_set_value_cansleep(ov02a10->pd_gpio, GPIOD_OUT_LOW);
> + usleep_range(5000, 6000);
> +
> + gpiod_set_value_cansleep(ov02a10->n_rst_gpio, GPIOD_OUT_HIGH);
> + usleep_range(5000, 6000);
> +
> + return 0;
> +
> +disable_clk:
> + clk_disable_unprepare(ov02a10->eclk);
> +
> + return ret;
> +}
> +
> +static void __ov02a10_power_off(struct ov02a10 *ov02a10)
> +{
> + gpiod_set_value_cansleep(ov02a10->n_rst_gpio, 1);
> + clk_disable_unprepare(ov02a10->eclk);
> + gpiod_set_value_cansleep(ov02a10->pd_gpio, 1);
> + regulator_bulk_disable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> +}
> +
> +static int __ov02a10_start_stream(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + const struct ov02a10_reg_list *reg_list;
> + int ret;
> +
> + /* Apply default values of current mode */
> + reg_list = &ov02a10->cur_mode->reg_list;
> + ret = ov02a10_write_array(ov02a10, reg_list);
> + if (ret)
> + return ret;
> +
> + /* Apply customized values from user */
> + ret = __v4l2_ctrl_handler_setup(ov02a10->subdev.ctrl_handler);
> + if (ret)
> + return ret;
> +
> + /* Set orientation to 180 degree */
> + if (ov02a10->upside_down) {
> + ret = i2c_smbus_write_byte_data(client, REG_MIRROR_FLIP_CONTROL,
> + REG_MIRROR_FLIP_ENABLE);
> + if (ret) {
> + dev_err(&client->dev, "failed to set orientation\n");
> + return ret;
> + }
> + ret = i2c_smbus_write_byte_data(client, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* Set stream on register */
> + return i2c_smbus_write_byte_data(client,
> + REG_SC_CTRL_MODE, SC_CTRL_MODE_STREAMING);
> +}
> +
> +static int __ov02a10_stop_stream(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> +
> + return i2c_smbus_write_byte_data(client,
> + REG_SC_CTRL_MODE, SC_CTRL_MODE_STANDBY);
Indentation.
> +}
> +
> +static int ov02a10_entity_init_cfg(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct v4l2_subdev_format fmt = { 0 };
> +
> + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> + fmt.format.width = ov02a10->cur_mode->width;
> + fmt.format.height = ov02a10->cur_mode->width;
> +
> + ov02a10_set_fmt(sd, cfg, &fmt);
> +
> + return 0;
> +}
> +
> +static int ov02a10_s_stream(struct v4l2_subdev *sd, int on)
> +{
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret = 0;
> +
> + dev_dbg(&client->dev, "ov02a10 s_stream (%d)\n", on);
> + mutex_lock(&ov02a10->mutex);
> +
> + if (ov02a10->streaming == on)
> + goto unlock_and_return;
> +
> + if (on) {
> + ret = pm_runtime_get_sync(&client->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(&client->dev);
> + goto unlock_and_return;
> + }
> +
> + ret = __ov02a10_start_stream(ov02a10);
> + if (ret) {
> + __ov02a10_stop_stream(ov02a10);
> + ov02a10->streaming = !on;
> + goto err_rpm_put;
> + }
> + } else {
> + __ov02a10_stop_stream(ov02a10);
> + pm_runtime_put(&client->dev);
> + }
> +
> + ov02a10->streaming = on;
> + mutex_unlock(&ov02a10->mutex);
> +
> + return ret;
> +
> +err_rpm_put:
> + pm_runtime_put(&client->dev);
> +unlock_and_return:
> + mutex_unlock(&ov02a10->mutex);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused ov02a10_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + return __ov02a10_power_on(ov02a10);
> +}
> +
> +static int __maybe_unused ov02a10_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + __ov02a10_power_off(ov02a10);
> +
> + return 0;
> +}
Please move the content of the both functions here; you don't need another
pair with just different arguments.
> +
> +static const struct dev_pm_ops ov02a10_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(ov02a10_runtime_suspend,
> + ov02a10_runtime_resume, NULL)
> +};
> +
> +/*
> + * ov02a10_set_exposure - Function called when setting exposure time
> + * @priv: Pointer to device structure
> + * @val: Variable for exposure time, in the unit of micro-second
> + *
> + * Set exposure time based on input value.
> + *
> + * Return: 0 on success
> + */
> +static int ov02a10_set_exposure(struct ov02a10 *ov02a10, int val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OV02A10_REG_EXPOSURE_H,
> + ((val >> OV02A10_EXP_SHIFT) &
> + OV02A10_MASK_8_BITS));
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OV02A10_REG_EXPOSURE_L,
> + (val & OV02A10_MASK_8_BITS));
> + if (ret < 0)
> + return ret;
> +
> + return i2c_smbus_write_byte_data(client, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> +}
> +
> +static int ov02a10_set_gain(struct ov02a10 *ov02a10, int val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OV02A10_REG_GAIN,
> + (val & OV02A10_MASK_8_BITS));
> + if (ret < 0)
> + return ret;
> +
> + return i2c_smbus_write_byte_data(client, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> +}
> +
> +static int ov02a10_set_vblank(struct ov02a10 *ov02a10, int val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OV02A10_REG_VTS_H,
> + (((val + ov02a10->cur_mode->height -
> + OV02A10_BASIC_LINE) >>
> + OV02A10_VTS_SHIFT) &
> + OV02A10_MASK_8_BITS));
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OV02A10_REG_VTS_L,
> + ((val + ov02a10->cur_mode->height -
> + OV02A10_BASIC_LINE) &
> + OV02A10_MASK_8_BITS));
> + if (ret < 0)
> + return ret;
> +
> + return i2c_smbus_write_byte_data(client, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> +}
> +
> +static int ov02a10_set_test_pattern(struct ov02a10 *ov02a10, int pattern)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + int ret;
> +
> + if (pattern)
> + pattern = OV02A10_TEST_PATTERN_ENABLE;
> +
> + ret = i2c_smbus_write_byte_data(client, REG_PAGE_SWITCH, REG_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OV02A10_REG_TEST_PATTERN,
> + pattern);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, REG_GLOBAL_EFFECTIVE,
> + REG_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + return i2c_smbus_write_byte_data(client, REG_SC_CTRL_MODE,
> + SC_CTRL_MODE_STREAMING);
> +}
> +
> +static int ov02a10_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct ov02a10 *ov02a10 = container_of(ctrl->handler,
> + struct ov02a10, ctrl_handler);
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + s64 max_expo;
> + int ret;
> +
> + /* Propagate change of current control to all related controls */
> + if (ctrl->id == V4L2_CID_VBLANK) {
> + /* Update max exposure while meeting expected vblanking */
> + max_expo = ov02a10->cur_mode->height + ctrl->val -
> + OV02A10_EXPOSURE_MAX_MARGIN;
> + __v4l2_ctrl_modify_range(ov02a10->exposure,
> + ov02a10->exposure->minimum, max_expo,
> + ov02a10->exposure->step,
> + ov02a10->exposure->default_value);
> + }
> +
> + /* V4L2 controls values will be applied only when power is already up */
> + if (!pm_runtime_get_if_in_use(&client->dev))
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_EXPOSURE:
> + ret = ov02a10_set_exposure(ov02a10, ctrl->val);
> + break;
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = ov02a10_set_gain(ov02a10, ctrl->val);
> + break;
> + case V4L2_CID_VBLANK:
> + ret = ov02a10_set_vblank(ov02a10, ctrl->val);
> + break;
> + case V4L2_CID_TEST_PATTERN:
> + ret = ov02a10_set_test_pattern(ov02a10, ctrl->val);
> + break;
> + };
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +}
> +
> +static const struct v4l2_subdev_video_ops ov02a10_video_ops = {
> + .s_stream = ov02a10_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops ov02a10_pad_ops = {
> + .init_cfg = ov02a10_entity_init_cfg,
> + .enum_mbus_code = ov02a10_enum_mbus_code,
> + .enum_frame_size = ov02a10_enum_frame_sizes,
> + .get_fmt = ov02a10_get_fmt,
> + .set_fmt = ov02a10_set_fmt,
> +};
> +
> +static const struct v4l2_subdev_ops ov02a10_subdev_ops = {
> + .video = &ov02a10_video_ops,
> + .pad = &ov02a10_pad_ops,
> +};
> +
> +static const struct media_entity_operations ov02a10_subdev_entity_ops = {
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static const struct v4l2_ctrl_ops ov02a10_ctrl_ops = {
> + .s_ctrl = ov02a10_set_ctrl,
> +};
> +
> +static int ov02a10_initialize_controls(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + const struct ov02a10_mode *mode;
> + struct v4l2_ctrl_handler *handler;
> + struct v4l2_ctrl *ctrl;
> + u64 exposure_max;
> + u32 pixel_rate, h_blank;
> + int ret;
> +
> + handler = &ov02a10->ctrl_handler;
> + mode = ov02a10->cur_mode;
> + ret = v4l2_ctrl_handler_init(handler, 7);
> + if (ret)
> + return ret;
> +
> + handler->lock = &ov02a10->mutex;
> +
> + ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ,
> + 0, 0, link_freq_menu_items);
> + if (ctrl)
> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + pixel_rate = to_pixel_rate(0);
> + v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE,
> + 0, pixel_rate, 1, pixel_rate);
> +
> + h_blank = mode->hts_def - mode->width;
> + ov02a10->hblank = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK,
> + h_blank, h_blank, 1, h_blank);
> + if (ov02a10->hblank)
> + ov02a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + ov02a10->vblank = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> + V4L2_CID_VBLANK, mode->vts_def -
> + mode->height,
> + OV02A10_VTS_MAX - mode->height, 1,
> + mode->vts_def - mode->height);
> +
> + exposure_max = mode->vts_def - 4;
> + ov02a10->exposure = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> + V4L2_CID_EXPOSURE,
> + OV02A10_EXPOSURE_MIN,
> + exposure_max,
> + OV02A10_EXPOSURE_STEP,
> + mode->exp_def);
> +
> + ov02a10->anal_gain = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> + V4L2_CID_ANALOGUE_GAIN,
> + OV02A10_GAIN_MIN,
> + OV02A10_GAIN_MAX,
> + OV02A10_GAIN_STEP,
> + OV02A10_GAIN_DEFAULT);
> +
> + ov02a10->test_pattern =
> + v4l2_ctrl_new_std_menu_items(handler,
> + &ov02a10_ctrl_ops,
> + V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(ov02a10_test_pattern_menu) -
> + 1, 0, 0,
> + ov02a10_test_pattern_menu);
> +
> + if (handler->error) {
> + ret = handler->error;
> + dev_err(&client->dev, "failed to init controls(%d)\n", ret);
> + goto err_free_handler;
> + }
> +
> + ov02a10->subdev.ctrl_handler = handler;
> +
> + return 0;
> +
> +err_free_handler:
> + v4l2_ctrl_handler_free(handler);
> +
> + return ret;
> +}
> +
> +static int ov02a10_check_hwcfg(struct device *dev)
> +{
> + struct fwnode_handle *ep;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + struct v4l2_fwnode_endpoint bus_cfg = {
> + .bus_type = V4L2_MBUS_CSI2_DPHY
> + };
> + unsigned int i, j;
> + int ret;
> +
> + if (!fwnode)
> + return -ENXIO;
> +
> + ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> + if (!ep)
> + return -ENXIO;
> +
> + ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> + fwnode_handle_put(ep);
> + if (ret)
> + return ret;
> +
> + if (!bus_cfg.nr_of_link_frequencies) {
> + dev_err(dev, "no link frequencies defined");
> + ret = -EINVAL;
> + goto check_hwcfg_error;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(link_freq_menu_items); i++) {
> + for (j = 0; j < bus_cfg.nr_of_link_frequencies; j++) {
> + if (link_freq_menu_items[i] ==
> + bus_cfg.link_frequencies[j])
> + break;
> + }
> +
> + if (j == bus_cfg.nr_of_link_frequencies) {
> + dev_err(dev, "no link frequency %lld supported",
> + link_freq_menu_items[i]);
> + ret = -EINVAL;
> + goto check_hwcfg_error;
> + }
> + }
> +
> +check_hwcfg_error:
> + v4l2_fwnode_endpoint_free(&bus_cfg);
> +
> + return ret;
> +}
> +
> +static int ov02a10_check_sensor_id(struct ov02a10 *ov02a10)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> + u16 id;
> + u8 pid = 0;
> + u8 ver = 0;
> + int ret;
> +
> + /* Check sensor revision */
> + ret = ov02a10_read_smbus(ov02a10, OV02A10_REG_CHIP_ID_H, &pid);
> + if (ret)
> + return ret;
> +
> + ret = ov02a10_read_smbus(ov02a10, OV02A10_REG_CHIP_ID_L, &ver);
> + if (ret)
> + return ret;
> +
> + id = OV02A10_ID(pid, ver);
> + if (id != CHIP_ID) {
> + dev_err(&client->dev, "Unexpected sensor id(%04x)\n", id);
> + return ret;
> + }
> +
> + dev_info(&client->dev, "Detected OV%04X sensor\n", id);
> +
> + return 0;
> +}
> +
> +static int ov02a10_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct ov02a10 *ov02a10;
> + unsigned int rotation;
> + unsigned int i;
> + int ret;
> +
> + dev_info(dev, "ov02a10 probe aaaa++\n");
Please remove.
> +
> + ret = ov02a10_check_hwcfg(dev);
> + if (ret) {
> + dev_err(dev, "failed to check HW configuration: %d",
> + ret);
> + return ret;
> + }
> +
> + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> + if (!ov02a10)
> + return -ENOMEM;
> +
> + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +
> + /* Optional indication of physical rotation of sensor */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation",
> + &rotation);
> + if (!ret) {
> + switch (rotation) {
> + case 180:
> + ov02a10->upside_down = true;
> + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> + break;
> + case 0:
> + break;
> + default:
> + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
> + rotation);
> + }
> + }
> +
> + /* Get system clock (eclk) */
> + ov02a10->eclk = devm_clk_get(dev, "eclk");
> + if (IS_ERR(ov02a10->eclk)) {
> + dev_err(dev, "failed to get eclk\n");
> + return -EINVAL;
> + }
> +
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> + &ov02a10->eclk_freq);
> + if (ret) {
> + dev_err(dev, "failed to get eclk frequency\n");
> + return ret;
> + }
> +
> + ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> + if (ret) {
> + dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> + return ret;
> + }
> +
> + if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> + dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> + ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> + return -EINVAL;
> + }
> +
> + ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> + if (IS_ERR(ov02a10->pd_gpio)) {
> + dev_err(dev, "failed to get powerdown-gpios\n");
> + return -EINVAL;
> + }
> +
> + ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ov02a10->n_rst_gpio)) {
> + dev_err(dev, "failed to get reset-gpios\n");
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < OV02A10_NUM_SUPPLIES; i++)
> + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev,
> + OV02A10_NUM_SUPPLIES,
Fits on a single line.
> + ov02a10->supplies);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + mutex_init(&ov02a10->mutex);
> + ov02a10->cur_mode = &supported_modes[0];
> + ret = ov02a10_initialize_controls(ov02a10);
> + if (ret) {
> + dev_err(dev, "failed to initialize controls\n");
> + goto err_destroy_mutex;
> + }
> +
> + ret = __ov02a10_power_on(ov02a10);
> + if (ret)
> + goto err_free_handler;
> +
> + ret = ov02a10_check_sensor_id(ov02a10);
> + if (ret)
> + goto err_power_off;
> +
> + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> + if (ret < 0) {
> + dev_err(dev, "failed to init entity pads: %d", ret);
> + goto err_power_off;
> + }
> +
> + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> + if (ret) {
> + dev_err(dev, "failed to register V4L2 subdev: %d",
> + ret);
> + goto err_clean_entity;
> + }
> +
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_idle(dev);
> +
> + dev_info(dev, "ov02a10 probe --\n");
Ditto.
> + return 0;
> +
> +err_clean_entity:
> + media_entity_cleanup(&ov02a10->subdev.entity);
> +err_power_off:
> + __ov02a10_power_off(ov02a10);
> +err_free_handler:
> + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> +err_destroy_mutex:
> + mutex_destroy(&ov02a10->mutex);
> +
> + return ret;
> +}
> +
> +static int ov02a10_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_ctrl_handler_free(sd->ctrl_handler);
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + __ov02a10_power_off(ov02a10);
> + pm_runtime_set_suspended(&client->dev);
> + mutex_destroy(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ov02a10_of_match[] = {
> + { .compatible = "ovti,ov02a10" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> +
> +static struct i2c_driver ov02a10_i2c_driver = {
> + .driver = {
> + .name = "ov02a10",
> + .pm = &ov02a10_pm_ops,
> + .of_match_table = ov02a10_of_match,
> + },
> + .probe_new = &ov02a10_probe,
> + .remove = &ov02a10_remove,
> +};
> +
> +module_i2c_driver(ov02a10_i2c_driver);
> +
> +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> +MODULE_LICENSE("GPL v2");
> +
--
Sakari Ailus
sakari.ailus@linux.intel.com
_______________________________________________
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 2/2] vhost: re-introducing metadata acceleration through kernel virtual address
From: Jason Wang @ 2019-09-09 7:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange, Christoph Hellwig, linux-parisc, kvm, netdev,
linux-kernel, virtualization, James Bottomley, linux-mm, jglisse,
jgg, David Miller, linux-arm-kernel
In-Reply-To: <20190909004504-mutt-send-email-mst@kernel.org>
On 2019/9/9 下午12:45, Michael S. Tsirkin wrote:
>>> Since idx can be speculated, I guess we need array_index_nospec here?
>> So we have
>>
>> ACQUIRE(mmu_lock)
>>
>> get idx
>>
>> RELEASE(mmu_lock)
>>
>> ACQUIRE(mmu_lock)
>>
>> read array[idx]
>>
>> RELEASE(mmu_lock)
>>
>> Then I think idx can't be speculated consider we've passed RELEASE +
>> ACQUIRE?
> I don't think memory barriers have anything to do with speculation,
> they are architectural.
>
Oh right. Let me add array_index_nospec() in next version.
Thanks
_______________________________________________
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 02/14] soc: ti: k3: add navss ringacc driver
From: Vignesh Raghavendra @ 2019-09-09 7:25 UTC (permalink / raw)
To: Tero Kristo, Peter Ujfalusi, vkoul, robh+dt, nm, ssantosh
Cc: devicetree, grygorii.strashko, lokeshvutla, j-keerthy,
linux-kernel, tony, dmaengine, dan.j.williams, linux-arm-kernel
In-Reply-To: <13e5c02f-7060-3a30-56cb-a9caca9fc85b@ti.com>
Hi,
On 09/09/19 11:39 AM, Tero Kristo wrote:
[...]
>> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
>> new file mode 100644
>> index 000000000000..401dfc963319
>> --- /dev/null
>> +++ b/drivers/soc/ti/k3-ringacc.c
>> @@ -0,0 +1,1191 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * TI K3 NAVSS Ring Accelerator subsystem driver
>> + *
>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
>> + */
>> +
>> +#include <linux/dma-mapping.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/soc/ti/k3-ringacc.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +#include <linux/soc/ti/ti_sci_inta_msi.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/irqdomain.h>
>> +
>> +static LIST_HEAD(k3_ringacc_list);
>> +static DEFINE_MUTEX(k3_ringacc_list_lock);
>> +
>> +#ifdef CONFIG_TI_K3_RINGACC_DEBUG
>> +#define k3_nav_dbg(dev, arg...) dev_err(dev, arg)
>
> dev_err seems exaggeration for debug purposes, maybe just dev_info.
If you make this dev_dbg(), it should be possible to just enable
CONFIG_DYNAMIC_DEBUG[1] and control whether or not debug prints are
enabled for this module. Have you explored that option? If that works we
may not need CONFIG_TI_K3_RINGACC_DEBUG at all.
[1] Documentation/admin-guide/dynamic-debug-howto.rst
Regards
Vignesh
>
>> +static void dbg_writel(u32 v, void __iomem *reg)
>> +{
>> + pr_err("WRITEL(32): v(%08X)-->reg(%p)\n", v, reg);
>
> Again, maybe just pr_info.
>
>> + writel(v, reg);
>> +}
>> +
>> +static u32 dbg_readl(void __iomem *reg)
>> +{
>> + u32 v;
>> +
>> + v = readl(reg);
>> + pr_err("READL(32): v(%08X)<--reg(%p)\n", v, reg);
>> + return v;
>> +}
>> +#else
>> +#define k3_nav_dbg(dev, arg...) dev_dbg(dev, arg)
>> +#define dbg_writel(v, reg) writel(v, reg)
>
> Do you need to use hard writel, writel_relaxed is not enough?
>
>> +
>> +#define dbg_readl(reg) readl(reg)
>
> Same as above but for read?
>
>> +#endif
>> +
>> +#define K3_RINGACC_CFG_RING_SIZE_ELCNT_MASK GENMASK(19, 0)
>> +
>> +/**
>> + * struct k3_ring_rt_regs - The RA Control/Status Registers region
>> + */
>> +struct k3_ring_rt_regs {
>> + u32 resv_16[4];
>> + u32 db; /* RT Ring N Doorbell Register */
>> + u32 resv_4[1];
>> + u32 occ; /* RT Ring N Occupancy Register */
>> + u32 indx; /* RT Ring N Current Index Register */
>> + u32 hwocc; /* RT Ring N Hardware Occupancy Register */
>> + u32 hwindx; /* RT Ring N Current Index Register */
>> +};
>> +
>> +#define K3_RINGACC_RT_REGS_STEP 0x1000
>> +
>> +/**
>> + * struct k3_ring_fifo_regs - The Ring Accelerator Queues Registers
>> region
>> + */
>> +struct k3_ring_fifo_regs {
>> + u32 head_data[128]; /* Ring Head Entry Data Registers */
>> + u32 tail_data[128]; /* Ring Tail Entry Data Registers */
>> + u32 peek_head_data[128]; /* Ring Peek Head Entry Data Regs */
>> + u32 peek_tail_data[128]; /* Ring Peek Tail Entry Data Regs */
>> +};
>> +
>> +/**
>> + * struct k3_ringacc_proxy_gcfg_regs - RA Proxy Global Config MMIO
>> Region
>> + */
>> +struct k3_ringacc_proxy_gcfg_regs {
>> + u32 revision; /* Revision Register */
>> + u32 config; /* Config Register */
>> +};
>> +
>> +#define K3_RINGACC_PROXY_CFG_THREADS_MASK GENMASK(15, 0)
>> +
>> +/**
>> + * struct k3_ringacc_proxy_target_regs - Proxy Datapath MMIO Region
>> + */
>> +struct k3_ringacc_proxy_target_regs {
>> + u32 control; /* Proxy Control Register */
>> + u32 status; /* Proxy Status Register */
>> + u8 resv_512[504];
>> + u32 data[128]; /* Proxy Data Register */
>> +};
>> +
>> +#define K3_RINGACC_PROXY_TARGET_STEP 0x1000
>> +#define K3_RINGACC_PROXY_NOT_USED (-1)
>> +
>> +enum k3_ringacc_proxy_access_mode {
>> + PROXY_ACCESS_MODE_HEAD = 0,
>> + PROXY_ACCESS_MODE_TAIL = 1,
>> + PROXY_ACCESS_MODE_PEEK_HEAD = 2,
>> + PROXY_ACCESS_MODE_PEEK_TAIL = 3,
>> +};
>> +
>> +#define K3_RINGACC_FIFO_WINDOW_SIZE_BYTES (512U)
>> +#define K3_RINGACC_FIFO_REGS_STEP 0x1000
>> +#define K3_RINGACC_MAX_DB_RING_CNT (127U)
>> +
>> +/**
>> + * struct k3_ring_ops - Ring operations
>> + */
>> +struct k3_ring_ops {
>> + int (*push_tail)(struct k3_ring *ring, void *elm);
>> + int (*push_head)(struct k3_ring *ring, void *elm);
>> + int (*pop_tail)(struct k3_ring *ring, void *elm);
>> + int (*pop_head)(struct k3_ring *ring, void *elm);
>> +};
>> +
>> +/**
>> + * struct k3_ring - RA Ring descriptor
>> + *
>> + * @rt - Ring control/status registers
>> + * @fifos - Ring queues registers
>> + * @proxy - Ring Proxy Datapath registers
>> + * @ring_mem_dma - Ring buffer dma address
>> + * @ring_mem_virt - Ring buffer virt address
>> + * @ops - Ring operations
>> + * @size - Ring size in elements
>> + * @elm_size - Size of the ring element
>> + * @mode - Ring mode
>> + * @flags - flags
>> + * @free - Number of free elements
>> + * @occ - Ring occupancy
>> + * @windex - Write index (only for @K3_RINGACC_RING_MODE_RING)
>> + * @rindex - Read index (only for @K3_RINGACC_RING_MODE_RING)
>> + * @ring_id - Ring Id
>> + * @parent - Pointer on struct @k3_ringacc
>> + * @use_count - Use count for shared rings
>> + * @proxy_id - RA Ring Proxy Id (only if @K3_RINGACC_RING_USE_PROXY)
>> + */
>> +struct k3_ring {
>> + struct k3_ring_rt_regs __iomem *rt;
>> + struct k3_ring_fifo_regs __iomem *fifos;
>> + struct k3_ringacc_proxy_target_regs __iomem *proxy;
>> + dma_addr_t ring_mem_dma;
>> + void *ring_mem_virt;
>> + struct k3_ring_ops *ops;
>> + u32 size;
>> + enum k3_ring_size elm_size;
>> + enum k3_ring_mode mode;
>> + u32 flags;
>> +#define K3_RING_FLAG_BUSY BIT(1)
>> +#define K3_RING_FLAG_SHARED BIT(2)
>> + u32 free;
>> + u32 occ;
>> + u32 windex;
>> + u32 rindex;
>> + u32 ring_id;
>> + struct k3_ringacc *parent;
>> + u32 use_count;
>> + int proxy_id;
>> +};
>> +
>> +/**
>> + * struct k3_ringacc - Rings accelerator descriptor
>> + *
>> + * @dev - pointer on RA device
>> + * @proxy_gcfg - RA proxy global config registers
>> + * @proxy_target_base - RA proxy datapath region
>> + * @num_rings - number of ring in RA
>> + * @rm_gp_range - general purpose rings range from tisci
>> + * @dma_ring_reset_quirk - DMA reset w/a enable
>> + * @num_proxies - number of RA proxies
>> + * @rings - array of rings descriptors (struct @k3_ring)
>> + * @list - list of RAs in the system
>> + * @tisci - pointer ti-sci handle
>> + * @tisci_ring_ops - ti-sci rings ops
>> + * @tisci_dev_id - ti-sci device id
>> + */
>> +struct k3_ringacc {
>> + struct device *dev;
>> + struct k3_ringacc_proxy_gcfg_regs __iomem *proxy_gcfg;
>> + void __iomem *proxy_target_base;
>> + u32 num_rings; /* number of rings in Ringacc module */
>> + unsigned long *rings_inuse;
>> + struct ti_sci_resource *rm_gp_range;
>> +
>> + bool dma_ring_reset_quirk;
>> + u32 num_proxies;
>> + unsigned long *proxy_inuse;
>
> proxy_inuse is not documented above.
>
>> +
>> + struct k3_ring *rings;
>> + struct list_head list;
>> + struct mutex req_lock; /* protect rings allocation */
>> +
>> + const struct ti_sci_handle *tisci;
>> + const struct ti_sci_rm_ringacc_ops *tisci_ring_ops;
>> + u32 tisci_dev_id;
>> +};
>> +
>> +static long k3_ringacc_ring_get_fifo_pos(struct k3_ring *ring)
>> +{
>> + return K3_RINGACC_FIFO_WINDOW_SIZE_BYTES -
>> + (4 << ring->elm_size);
>> +}
>> +
>> +static void *k3_ringacc_get_elm_addr(struct k3_ring *ring, u32 idx)
>> +{
>> + return (idx * (4 << ring->elm_size) + ring->ring_mem_virt);
>
> The arithmetic here seems backwards compared to most other code I've
> seen. It would be more readable if you have it like:
>
> ring->ring_mem_virt + idx * (4 << ring->elm_size);
>
>> +}
>> +
>> +static int k3_ringacc_ring_push_mem(struct k3_ring *ring, void *elem);
>> +static int k3_ringacc_ring_pop_mem(struct k3_ring *ring, void *elem);
>> +
>> +static struct k3_ring_ops k3_ring_mode_ring_ops = {
>> + .push_tail = k3_ringacc_ring_push_mem,
>> + .pop_head = k3_ringacc_ring_pop_mem,
>> +};
>> +
>> +static int k3_ringacc_ring_push_io(struct k3_ring *ring, void *elem);
>> +static int k3_ringacc_ring_pop_io(struct k3_ring *ring, void *elem);
>> +static int k3_ringacc_ring_push_head_io(struct k3_ring *ring, void
>> *elem);
>> +static int k3_ringacc_ring_pop_tail_io(struct k3_ring *ring, void
>> *elem);
>> +
>> +static struct k3_ring_ops k3_ring_mode_msg_ops = {
>> + .push_tail = k3_ringacc_ring_push_io,
>> + .push_head = k3_ringacc_ring_push_head_io,
>> + .pop_tail = k3_ringacc_ring_pop_tail_io,
>> + .pop_head = k3_ringacc_ring_pop_io,
>> +};
>> +
>> +static int k3_ringacc_ring_push_head_proxy(struct k3_ring *ring, void
>> *elem);
>> +static int k3_ringacc_ring_push_tail_proxy(struct k3_ring *ring, void
>> *elem);
>> +static int k3_ringacc_ring_pop_head_proxy(struct k3_ring *ring, void
>> *elem);
>> +static int k3_ringacc_ring_pop_tail_proxy(struct k3_ring *ring, void
>> *elem);
>> +
>> +static struct k3_ring_ops k3_ring_mode_proxy_ops = {
>> + .push_tail = k3_ringacc_ring_push_tail_proxy,
>> + .push_head = k3_ringacc_ring_push_head_proxy,
>> + .pop_tail = k3_ringacc_ring_pop_tail_proxy,
>> + .pop_head = k3_ringacc_ring_pop_head_proxy,
>> +};
>> +
>> +#ifdef CONFIG_TI_K3_RINGACC_DEBUG
>> +void k3_ringacc_ring_dump(struct k3_ring *ring)
>> +{
>> + struct device *dev = ring->parent->dev;
>> +
>> + k3_nav_dbg(dev, "dump ring: %d\n", ring->ring_id);
>> + k3_nav_dbg(dev, "dump mem virt %p, dma %pad\n",
>> + ring->ring_mem_virt, &ring->ring_mem_dma);
>> + k3_nav_dbg(dev, "dump elmsize %d, size %d, mode %d, proxy_id %d\n",
>> + ring->elm_size, ring->size, ring->mode, ring->proxy_id);
>> +
>> + k3_nav_dbg(dev, "dump ring_rt_regs: db%08x\n",
>> + readl(&ring->rt->db));
>
> Why not use readl_relaxed in this func?
>
>> + k3_nav_dbg(dev, "dump occ%08x\n",
>> + readl(&ring->rt->occ));
>> + k3_nav_dbg(dev, "dump indx%08x\n",
>> + readl(&ring->rt->indx));
>> + k3_nav_dbg(dev, "dump hwocc%08x\n",
>> + readl(&ring->rt->hwocc));
>> + k3_nav_dbg(dev, "dump hwindx%08x\n",
>> + readl(&ring->rt->hwindx));
>> +
>> + if (ring->ring_mem_virt)
>> + print_hex_dump(KERN_ERR, "dump ring_mem_virt ",
>> + DUMP_PREFIX_NONE, 16, 1,
>> + ring->ring_mem_virt, 16 * 8, false);
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_dump);
>
> Do you really need to export a debug function?
>
>> +#endif
>> +
>> +struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>> + int id, u32 flags)
>> +{
>> + int proxy_id = K3_RINGACC_PROXY_NOT_USED;
>> +
>> + mutex_lock(&ringacc->req_lock);
>> +
>> + if (id == K3_RINGACC_RING_ID_ANY) {
>> + /* Request for any general purpose ring */
>> + struct ti_sci_resource_desc *gp_rings =
>> + &ringacc->rm_gp_range->desc[0];
>> + unsigned long size;
>> +
>> + size = gp_rings->start + gp_rings->num;
>> + id = find_next_zero_bit(ringacc->rings_inuse, size,
>> + gp_rings->start);
>> + if (id == size)
>> + goto error;
>> + } else if (id < 0) {
>> + goto error;
>> + }
>> +
>> + if (test_bit(id, ringacc->rings_inuse) &&
>> + !(ringacc->rings[id].flags & K3_RING_FLAG_SHARED))
>> + goto error;
>> + else if (ringacc->rings[id].flags & K3_RING_FLAG_SHARED)
>> + goto out;
>> +
>> + if (flags & K3_RINGACC_RING_USE_PROXY) {
>> + proxy_id = find_next_zero_bit(ringacc->proxy_inuse,
>> + ringacc->num_proxies, 0);
>> + if (proxy_id == ringacc->num_proxies)
>> + goto error;
>> + }
>> +
>> + if (!try_module_get(ringacc->dev->driver->owner))
>> + goto error;
>> +
>> + if (proxy_id != K3_RINGACC_PROXY_NOT_USED) {
>> + set_bit(proxy_id, ringacc->proxy_inuse);
>> + ringacc->rings[id].proxy_id = proxy_id;
>> + k3_nav_dbg(ringacc->dev, "Giving ring#%d proxy#%d\n",
>> + id, proxy_id);
>> + } else {
>> + k3_nav_dbg(ringacc->dev, "Giving ring#%d\n", id);
>> + }
>> +
>> + set_bit(id, ringacc->rings_inuse);
>> +out:
>> + ringacc->rings[id].use_count++;
>> + mutex_unlock(&ringacc->req_lock);
>> + return &ringacc->rings[id];
>> +
>> +error:
>> + mutex_unlock(&ringacc->req_lock);
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
>> +
>> +static void k3_ringacc_ring_reset_sci(struct k3_ring *ring)
>> +{
>> + struct k3_ringacc *ringacc = ring->parent;
>> + int ret;
>> +
>> + ret = ringacc->tisci_ring_ops->config(
>> + ringacc->tisci,
>> + TI_SCI_MSG_VALUE_RM_RING_COUNT_VALID,
>> + ringacc->tisci_dev_id,
>> + ring->ring_id,
>> + 0,
>> + 0,
>> + ring->size,
>> + 0,
>> + 0,
>> + 0);
>> + if (ret)
>> + dev_err(ringacc->dev, "TISCI reset ring fail (%d) ring_idx
>> %d\n",
>> + ret, ring->ring_id);
>
> Return value of sci ops is masked, why not return it and let the caller
> handle it properly?
>
> Same comment for anything similar that follows.
>
>> +}
>> +
>> +void k3_ringacc_ring_reset(struct k3_ring *ring)
>> +{
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return;
>> +
>> + ring->occ = 0;
>> + ring->free = 0;
>> + ring->rindex = 0;
>> + ring->windex = 0;
>> +
>> + k3_ringacc_ring_reset_sci(ring);
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_reset);
>> +
>> +static void k3_ringacc_ring_reconfig_qmode_sci(struct k3_ring *ring,
>> + enum k3_ring_mode mode)
>> +{
>> + struct k3_ringacc *ringacc = ring->parent;
>> + int ret;
>> +
>> + ret = ringacc->tisci_ring_ops->config(
>> + ringacc->tisci,
>> + TI_SCI_MSG_VALUE_RM_RING_MODE_VALID,
>> + ringacc->tisci_dev_id,
>> + ring->ring_id,
>> + 0,
>> + 0,
>> + 0,
>> + mode,
>> + 0,
>> + 0);
>> + if (ret)
>> + dev_err(ringacc->dev, "TISCI reconf qmode fail (%d) ring_idx
>> %d\n",
>> + ret, ring->ring_id);
>> +}
>> +
>> +void k3_ringacc_ring_reset_dma(struct k3_ring *ring, u32 occ)
>> +{
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return;
>> +
>> + if (!ring->parent->dma_ring_reset_quirk)
>> + return;
>> +
>> + if (!occ)
>> + occ = dbg_readl(&ring->rt->occ);
>> +
>> + if (occ) {
>> + u32 db_ring_cnt, db_ring_cnt_cur;
>> +
>> + k3_nav_dbg(ring->parent->dev, "%s %u occ: %u\n", __func__,
>> + ring->ring_id, occ);
>> + /* 2. Reset the ring */
>
> 2? Where is 1?
>
>> + k3_ringacc_ring_reset_sci(ring);
>> +
>> + /*
>> + * 3. Setup the ring in ring/doorbell mode
>> + * (if not already in this mode)
>> + */
>> + if (ring->mode != K3_RINGACC_RING_MODE_RING)
>> + k3_ringacc_ring_reconfig_qmode_sci(
>> + ring, K3_RINGACC_RING_MODE_RING);
>> + /*
>> + * 4. Ring the doorbell 2**22 – ringOcc times.
>> + * This will wrap the internal UDMAP ring state occupancy
>> + * counter (which is 21-bits wide) to 0.
>> + */
>> + db_ring_cnt = (1U << 22) - occ;
>> +
>> + while (db_ring_cnt != 0) {
>> + /*
>> + * Ring the doorbell with the maximum count each
>> + * iteration if possible to minimize the total
>> + * of writes
>> + */
>> + if (db_ring_cnt > K3_RINGACC_MAX_DB_RING_CNT)
>> + db_ring_cnt_cur = K3_RINGACC_MAX_DB_RING_CNT;
>> + else
>> + db_ring_cnt_cur = db_ring_cnt;
>> +
>> + writel(db_ring_cnt_cur, &ring->rt->db);
>> + db_ring_cnt -= db_ring_cnt_cur;
>> + }
>> +
>> + /* 5. Restore the original ring mode (if not ring mode) */
>> + if (ring->mode != K3_RINGACC_RING_MODE_RING)
>> + k3_ringacc_ring_reconfig_qmode_sci(ring, ring->mode);
>> + }
>> +
>> + /* 2. Reset the ring */
>
> Again 2?
>
>> + k3_ringacc_ring_reset(ring);
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_reset_dma);
>> +
>> +static void k3_ringacc_ring_free_sci(struct k3_ring *ring)
>> +{
>> + struct k3_ringacc *ringacc = ring->parent;
>> + int ret;
>> +
>> + ret = ringacc->tisci_ring_ops->config(
>> + ringacc->tisci,
>> + TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER,
>> + ringacc->tisci_dev_id,
>> + ring->ring_id,
>> + 0,
>> + 0,
>> + 0,
>> + 0,
>> + 0,
>> + 0);
>> + if (ret)
>> + dev_err(ringacc->dev, "TISCI ring free fail (%d) ring_idx %d\n",
>> + ret, ring->ring_id);
>> +}
>> +
>> +int k3_ringacc_ring_free(struct k3_ring *ring)
>> +{
>> + struct k3_ringacc *ringacc;
>> +
>> + if (!ring)
>> + return -EINVAL;
>> +
>> + ringacc = ring->parent;
>> +
>> + k3_nav_dbg(ring->parent->dev, "flags: 0x%08x\n", ring->flags);
>> +
>> + if (!test_bit(ring->ring_id, ringacc->rings_inuse))
>> + return -EINVAL;
>> +
>> + mutex_lock(&ringacc->req_lock);
>> +
>> + if (--ring->use_count)
>> + goto out;
>> +
>> + if (!(ring->flags & K3_RING_FLAG_BUSY))
>> + goto no_init;
>> +
>> + k3_ringacc_ring_free_sci(ring);
>> +
>> + dma_free_coherent(ringacc->dev,
>> + ring->size * (4 << ring->elm_size),
>> + ring->ring_mem_virt, ring->ring_mem_dma);
>> + ring->flags = 0;
>> + ring->ops = NULL;
>> + if (ring->proxy_id != K3_RINGACC_PROXY_NOT_USED) {
>> + clear_bit(ring->proxy_id, ringacc->proxy_inuse);
>> + ring->proxy = NULL;
>> + ring->proxy_id = K3_RINGACC_PROXY_NOT_USED;
>> + }
>> +
>> +no_init:
>> + clear_bit(ring->ring_id, ringacc->rings_inuse);
>> +
>> + module_put(ringacc->dev->driver->owner);
>> +
>> +out:
>> + mutex_unlock(&ringacc->req_lock);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_free);
>> +
>> +u32 k3_ringacc_get_ring_id(struct k3_ring *ring)
>> +{
>> + if (!ring)
>> + return -EINVAL;
>> +
>> + return ring->ring_id;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_get_ring_id);
>> +
>> +u32 k3_ringacc_get_tisci_dev_id(struct k3_ring *ring)
>> +{
>> + if (!ring)
>> + return -EINVAL;
>> +
>
> What if parent is NULL? Can it ever be here?
>
>> + return ring->parent->tisci_dev_id;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_get_tisci_dev_id);
>> +
>> +int k3_ringacc_get_ring_irq_num(struct k3_ring *ring)
>> +{
>> + int irq_num;
>> +
>> + if (!ring)
>> + return -EINVAL;
>> +
>> + irq_num = ti_sci_inta_msi_get_virq(ring->parent->dev,
>> ring->ring_id);
>> + if (irq_num <= 0)
>> + irq_num = -EINVAL;
>> + return irq_num;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_get_ring_irq_num);
>> +
>> +static int k3_ringacc_ring_cfg_sci(struct k3_ring *ring)
>> +{
>> + struct k3_ringacc *ringacc = ring->parent;
>> + u32 ring_idx;
>> + int ret;
>> +
>> + if (!ringacc->tisci)
>> + return -EINVAL;
>> +
>> + ring_idx = ring->ring_id;
>> + ret = ringacc->tisci_ring_ops->config(
>> + ringacc->tisci,
>> + TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER,
>> + ringacc->tisci_dev_id,
>> + ring_idx,
>> + lower_32_bits(ring->ring_mem_dma),
>> + upper_32_bits(ring->ring_mem_dma),
>> + ring->size,
>> + ring->mode,
>> + ring->elm_size,
>> + 0);
>> + if (ret)
>> + dev_err(ringacc->dev, "TISCI config ring fail (%d) ring_idx
>> %d\n",
>> + ret, ring_idx);
>> +
>> + return ret;
>> +}
>> +
>> +int k3_ringacc_ring_cfg(struct k3_ring *ring, struct k3_ring_cfg *cfg)
>> +{
>> + struct k3_ringacc *ringacc = ring->parent;
>> + int ret = 0;
>> +
>> + if (!ring || !cfg)
>> + return -EINVAL;
>> + if (cfg->elm_size > K3_RINGACC_RING_ELSIZE_256 ||
>> + cfg->mode > K3_RINGACC_RING_MODE_QM ||
>> + cfg->size & ~K3_RINGACC_CFG_RING_SIZE_ELCNT_MASK ||
>> + !test_bit(ring->ring_id, ringacc->rings_inuse))
>> + return -EINVAL;
>> +
>> + if (ring->use_count != 1)
>
> Hmm, isn't this a failure actually?
>
>> + return 0;
>> +
>> + ring->size = cfg->size;
>> + ring->elm_size = cfg->elm_size;
>> + ring->mode = cfg->mode;
>> + ring->occ = 0;
>> + ring->free = 0;
>> + ring->rindex = 0;
>> + ring->windex = 0;
>> +
>> + if (ring->proxy_id != K3_RINGACC_PROXY_NOT_USED)
>> + ring->proxy = ringacc->proxy_target_base +
>> + ring->proxy_id * K3_RINGACC_PROXY_TARGET_STEP;
>> +
>> + switch (ring->mode) {
>> + case K3_RINGACC_RING_MODE_RING:
>> + ring->ops = &k3_ring_mode_ring_ops;
>> + break;
>> + case K3_RINGACC_RING_MODE_QM:
>> + /*
>> + * In Queue mode elm_size can be 8 only and each operation
>> + * uses 2 element slots
>> + */
>> + if (cfg->elm_size != K3_RINGACC_RING_ELSIZE_8 ||
>> + cfg->size % 2)
>> + goto err_free_proxy;
>> + /* else, fall through */
>> + case K3_RINGACC_RING_MODE_MESSAGE:
>> + if (ring->proxy)
>> + ring->ops = &k3_ring_mode_proxy_ops;
>> + else
>> + ring->ops = &k3_ring_mode_msg_ops;
>> + break;
>> + default:
>> + ring->ops = NULL;
>> + ret = -EINVAL;
>> + goto err_free_proxy;
>> + };
>> +
>> + ring->ring_mem_virt =
>> + dma_alloc_coherent(ringacc->dev,
>> + ring->size * (4 << ring->elm_size),
>> + &ring->ring_mem_dma, GFP_KERNEL);
>> + if (!ring->ring_mem_virt) {
>> + dev_err(ringacc->dev, "Failed to alloc ring mem\n");
>> + ret = -ENOMEM;
>> + goto err_free_ops;
>> + }
>> +
>> + ret = k3_ringacc_ring_cfg_sci(ring);
>> +
>> + if (ret)
>> + goto err_free_mem;
>> +
>> + ring->flags |= K3_RING_FLAG_BUSY;
>> + ring->flags |= (cfg->flags & K3_RINGACC_RING_SHARED) ?
>> + K3_RING_FLAG_SHARED : 0;
>> +
>> + k3_ringacc_ring_dump(ring);
>> +
>> + return 0;
>> +
>> +err_free_mem:
>> + dma_free_coherent(ringacc->dev,
>> + ring->size * (4 << ring->elm_size),
>> + ring->ring_mem_virt,
>> + ring->ring_mem_dma);
>> +err_free_ops:
>> + ring->ops = NULL;
>> +err_free_proxy:
>> + ring->proxy = NULL;
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_cfg);
>> +
>> +u32 k3_ringacc_ring_get_size(struct k3_ring *ring)
>> +{
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + return ring->size;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_get_size);
>> +
>> +u32 k3_ringacc_ring_get_free(struct k3_ring *ring)
>> +{
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + if (!ring->free)
>> + ring->free = ring->size - dbg_readl(&ring->rt->occ);
>> +
>> + return ring->free;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_get_free);
>> +
>> +u32 k3_ringacc_ring_get_occ(struct k3_ring *ring)
>> +{
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + return dbg_readl(&ring->rt->occ);
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_get_occ);
>> +
>> +u32 k3_ringacc_ring_is_full(struct k3_ring *ring)
>> +{
>> + return !k3_ringacc_ring_get_free(ring);
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_is_full);
>> +
>> +enum k3_ringacc_access_mode {
>> + K3_RINGACC_ACCESS_MODE_PUSH_HEAD,
>> + K3_RINGACC_ACCESS_MODE_POP_HEAD,
>> + K3_RINGACC_ACCESS_MODE_PUSH_TAIL,
>> + K3_RINGACC_ACCESS_MODE_POP_TAIL,
>> + K3_RINGACC_ACCESS_MODE_PEEK_HEAD,
>> + K3_RINGACC_ACCESS_MODE_PEEK_TAIL,
>> +};
>> +
>> +static int k3_ringacc_ring_cfg_proxy(struct k3_ring *ring,
>> + enum k3_ringacc_proxy_access_mode mode)
>> +{
>> + u32 val;
>> +
>> + val = ring->ring_id;
>> + val |= mode << 16;
>> + val |= ring->elm_size << 24;
>
> Would be nice to have these magic shifts as defines.
>
>> + dbg_writel(val, &ring->proxy->control);
>> + return 0;
>> +}
>> +
>> +static int k3_ringacc_ring_access_proxy(struct k3_ring *ring, void
>> *elem,
>> + enum k3_ringacc_access_mode access_mode)
>> +{
>> + void __iomem *ptr;
>> +
>> + ptr = (void __iomem *)&ring->proxy->data;
>> +
>> + switch (access_mode) {
>> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
>> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
>> + k3_ringacc_ring_cfg_proxy(ring, PROXY_ACCESS_MODE_HEAD);
>> + break;
>> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
>> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
>> + k3_ringacc_ring_cfg_proxy(ring, PROXY_ACCESS_MODE_TAIL);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ptr += k3_ringacc_ring_get_fifo_pos(ring);
>> +
>> + switch (access_mode) {
>> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
>> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
>> + k3_nav_dbg(ring->parent->dev, "proxy:memcpy_fromio(x): -->
>> ptr(%p), mode:%d\n",
>> + ptr, access_mode);
>> + memcpy_fromio(elem, ptr, (4 << ring->elm_size));
>> + ring->occ--;
>> + break;
>> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
>> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
>> + k3_nav_dbg(ring->parent->dev, "proxy:memcpy_toio(x): -->
>> ptr(%p), mode:%d\n",
>> + ptr, access_mode);
>> + memcpy_toio(ptr, elem, (4 << ring->elm_size));
>> + ring->free--;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + k3_nav_dbg(ring->parent->dev, "proxy: free%d occ%d\n",
>> + ring->free, ring->occ);
>> + return 0;
>> +}
>> +
>> +static int k3_ringacc_ring_push_head_proxy(struct k3_ring *ring, void
>> *elem)
>> +{
>> + return k3_ringacc_ring_access_proxy(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_PUSH_HEAD);
>> +}
>> +
>> +static int k3_ringacc_ring_push_tail_proxy(struct k3_ring *ring, void
>> *elem)
>> +{
>> + return k3_ringacc_ring_access_proxy(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_PUSH_TAIL);
>> +}
>> +
>> +static int k3_ringacc_ring_pop_head_proxy(struct k3_ring *ring, void
>> *elem)
>> +{
>> + return k3_ringacc_ring_access_proxy(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
>> +}
>> +
>> +static int k3_ringacc_ring_pop_tail_proxy(struct k3_ring *ring, void
>> *elem)
>> +{
>> + return k3_ringacc_ring_access_proxy(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
>> +}
>> +
>> +static int k3_ringacc_ring_access_io(struct k3_ring *ring, void *elem,
>> + enum k3_ringacc_access_mode access_mode)
>> +{
>> + void __iomem *ptr;
>> +
>> + switch (access_mode) {
>> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
>> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
>> + ptr = (void __iomem *)&ring->fifos->head_data;
>> + break;
>> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
>> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
>> + ptr = (void __iomem *)&ring->fifos->tail_data;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ptr += k3_ringacc_ring_get_fifo_pos(ring);
>> +
>> + switch (access_mode) {
>> + case K3_RINGACC_ACCESS_MODE_POP_HEAD:
>> + case K3_RINGACC_ACCESS_MODE_POP_TAIL:
>> + k3_nav_dbg(ring->parent->dev, "memcpy_fromio(x): --> ptr(%p),
>> mode:%d\n",
>> + ptr, access_mode);
>> + memcpy_fromio(elem, ptr, (4 << ring->elm_size));
>> + ring->occ--;
>> + break;
>> + case K3_RINGACC_ACCESS_MODE_PUSH_TAIL:
>> + case K3_RINGACC_ACCESS_MODE_PUSH_HEAD:
>> + k3_nav_dbg(ring->parent->dev, "memcpy_toio(x): --> ptr(%p),
>> mode:%d\n",
>> + ptr, access_mode);
>> + memcpy_toio(ptr, elem, (4 << ring->elm_size));
>> + ring->free--;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + k3_nav_dbg(ring->parent->dev, "free%d index%d occ%d index%d\n",
>> + ring->free, ring->windex, ring->occ, ring->rindex);
>> + return 0;
>> +}
>> +
>> +static int k3_ringacc_ring_push_head_io(struct k3_ring *ring, void
>> *elem)
>> +{
>> + return k3_ringacc_ring_access_io(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_PUSH_HEAD);
>> +}
>> +
>> +static int k3_ringacc_ring_push_io(struct k3_ring *ring, void *elem)
>> +{
>> + return k3_ringacc_ring_access_io(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_PUSH_TAIL);
>> +}
>> +
>> +static int k3_ringacc_ring_pop_io(struct k3_ring *ring, void *elem)
>> +{
>> + return k3_ringacc_ring_access_io(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
>> +}
>> +
>> +static int k3_ringacc_ring_pop_tail_io(struct k3_ring *ring, void *elem)
>> +{
>> + return k3_ringacc_ring_access_io(ring, elem,
>> + K3_RINGACC_ACCESS_MODE_POP_HEAD);
>> +}
>> +
>> +static int k3_ringacc_ring_push_mem(struct k3_ring *ring, void *elem)
>> +{
>> + void *elem_ptr;
>> +
>> + elem_ptr = k3_ringacc_get_elm_addr(ring, ring->windex);
>> +
>> + memcpy(elem_ptr, elem, (4 << ring->elm_size));
>> +
>> + ring->windex = (ring->windex + 1) % ring->size;
>> + ring->free--;
>> + dbg_writel(1, &ring->rt->db);
>> +
>> + k3_nav_dbg(ring->parent->dev, "ring_push_mem: free%d index%d\n",
>> + ring->free, ring->windex);
>> +
>> + return 0;
>> +}
>> +
>> +static int k3_ringacc_ring_pop_mem(struct k3_ring *ring, void *elem)
>> +{
>> + void *elem_ptr;
>> +
>> + elem_ptr = k3_ringacc_get_elm_addr(ring, ring->rindex);
>> +
>> + memcpy(elem, elem_ptr, (4 << ring->elm_size));
>> +
>> + ring->rindex = (ring->rindex + 1) % ring->size;
>> + ring->occ--;
>> + dbg_writel(-1, &ring->rt->db);
>> +
>> + k3_nav_dbg(ring->parent->dev, "ring_pop_mem: occ%d index%d
>> pos_ptr%p\n",
>> + ring->occ, ring->rindex, elem_ptr);
>> + return 0;
>> +}
>> +
>> +int k3_ringacc_ring_push(struct k3_ring *ring, void *elem)
>> +{
>> + int ret = -EOPNOTSUPP;
>> +
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + k3_nav_dbg(ring->parent->dev, "ring_push: free%d index%d\n",
>> + ring->free, ring->windex);
>> +
>> + if (k3_ringacc_ring_is_full(ring))
>> + return -ENOMEM;
>> +
>> + if (ring->ops && ring->ops->push_tail)
>> + ret = ring->ops->push_tail(ring, elem);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_push);
>> +
>> +int k3_ringacc_ring_push_head(struct k3_ring *ring, void *elem)
>> +{
>> + int ret = -EOPNOTSUPP;
>> +
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + k3_nav_dbg(ring->parent->dev, "ring_push_head: free%d index%d\n",
>> + ring->free, ring->windex);
>> +
>> + if (k3_ringacc_ring_is_full(ring))
>> + return -ENOMEM;
>> +
>> + if (ring->ops && ring->ops->push_head)
>> + ret = ring->ops->push_head(ring, elem);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_push_head);
>> +
>> +int k3_ringacc_ring_pop(struct k3_ring *ring, void *elem)
>> +{
>> + int ret = -EOPNOTSUPP;
>> +
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + if (!ring->occ)
>> + ring->occ = k3_ringacc_ring_get_occ(ring);
>> +
>> + k3_nav_dbg(ring->parent->dev, "ring_pop: occ%d index%d\n",
>> + ring->occ, ring->rindex);
>> +
>> + if (!ring->occ)
>> + return -ENODATA;
>> +
>> + if (ring->ops && ring->ops->pop_head)
>> + ret = ring->ops->pop_head(ring, elem);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_pop);
>> +
>> +int k3_ringacc_ring_pop_tail(struct k3_ring *ring, void *elem)
>> +{
>> + int ret = -EOPNOTSUPP;
>> +
>> + if (!ring || !(ring->flags & K3_RING_FLAG_BUSY))
>> + return -EINVAL;
>> +
>> + if (!ring->occ)
>> + ring->occ = k3_ringacc_ring_get_occ(ring);
>> +
>> + k3_nav_dbg(ring->parent->dev, "ring_pop_tail: occ%d index%d\n",
>> + ring->occ, ring->rindex);
>> +
>> + if (!ring->occ)
>> + return -ENODATA;
>> +
>> + if (ring->ops && ring->ops->pop_tail)
>> + ret = ring->ops->pop_tail(ring, elem);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_ring_pop_tail);
>> +
>> +struct k3_ringacc *of_k3_ringacc_get_by_phandle(struct device_node *np,
>> + const char *property)
>> +{
>> + struct device_node *ringacc_np;
>> + struct k3_ringacc *ringacc = ERR_PTR(-EPROBE_DEFER);
>> + struct k3_ringacc *entry;
>> +
>> + ringacc_np = of_parse_phandle(np, property, 0);
>> + if (!ringacc_np)
>> + return ERR_PTR(-ENODEV);
>> +
>> + mutex_lock(&k3_ringacc_list_lock);
>> + list_for_each_entry(entry, &k3_ringacc_list, list)
>> + if (entry->dev->of_node == ringacc_np) {
>> + ringacc = entry;
>> + break;
>> + }
>> + mutex_unlock(&k3_ringacc_list_lock);
>> + of_node_put(ringacc_np);
>> +
>> + return ringacc;
>> +}
>> +EXPORT_SYMBOL_GPL(of_k3_ringacc_get_by_phandle);
>> +
>> +static int k3_ringacc_probe_dt(struct k3_ringacc *ringacc)
>> +{
>> + struct device_node *node = ringacc->dev->of_node;
>> + struct device *dev = ringacc->dev;
>> + struct platform_device *pdev = to_platform_device(dev);
>> + int ret;
>> +
>> + if (!node) {
>> + dev_err(dev, "device tree info unavailable\n");
>> + return -ENODEV;
>> + }
>> +
>> + ret = of_property_read_u32(node, "ti,num-rings",
>> &ringacc->num_rings);
>> + if (ret) {
>> + dev_err(dev, "ti,num-rings read failure %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ringacc->dma_ring_reset_quirk =
>> + of_property_read_bool(node, "ti,dma-ring-reset-quirk");
>> +
>> + ringacc->tisci = ti_sci_get_by_phandle(node, "ti,sci");
>> + if (IS_ERR(ringacc->tisci)) {
>> + ret = PTR_ERR(ringacc->tisci);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "ti,sci read fail %d\n", ret);
>> + ringacc->tisci = NULL;
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(node, "ti,sci-dev-id",
>> + &ringacc->tisci_dev_id);
>> + if (ret) {
>> + dev_err(dev, "ti,sci-dev-id read fail %d\n", ret);
>> + return ret;
>> + }
>> +
>> + pdev->id = ringacc->tisci_dev_id;
>> +
>> + ringacc->rm_gp_range =
>> devm_ti_sci_get_of_resource(ringacc->tisci, dev,
>> + ringacc->tisci_dev_id,
>> + "ti,sci-rm-range-gp-rings");
>> + if (IS_ERR(ringacc->rm_gp_range)) {
>> + dev_err(dev, "Failed to allocate MSI interrupts\n");
>> + return PTR_ERR(ringacc->rm_gp_range);
>> + }
>> +
>> + return ti_sci_inta_msi_domain_alloc_irqs(ringacc->dev,
>> + ringacc->rm_gp_range);
>> +}
>> +
>> +static int k3_ringacc_probe(struct platform_device *pdev)
>> +{
>> + struct k3_ringacc *ringacc;
>> + void __iomem *base_fifo, *base_rt;
>> + struct device *dev = &pdev->dev;
>> + struct resource *res;
>> + int ret, i;
>> +
>> + ringacc = devm_kzalloc(dev, sizeof(*ringacc), GFP_KERNEL);
>> + if (!ringacc)
>> + return -ENOMEM;
>> +
>> + ringacc->dev = dev;
>> + mutex_init(&ringacc->req_lock);
>> +
>> + dev->msi_domain = of_msi_get_domain(dev, dev->of_node,
>> + DOMAIN_BUS_TI_SCI_INTA_MSI);
>> + if (!dev->msi_domain) {
>> + dev_err(dev, "Failed to get MSI domain\n");
>> + return -EPROBE_DEFER;
>> + }
>> +
>> + ret = k3_ringacc_probe_dt(ringacc);
>> + if (ret)
>> + return ret;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rt");
>> + base_rt = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base_rt))
>> + return PTR_ERR(base_rt);
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fifos");
>> + base_fifo = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base_fifo))
>> + return PTR_ERR(base_fifo);
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> "proxy_gcfg");
>> + ringacc->proxy_gcfg = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(ringacc->proxy_gcfg))
>> + return PTR_ERR(ringacc->proxy_gcfg);
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> + "proxy_target");
>> + ringacc->proxy_target_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(ringacc->proxy_target_base))
>> + return PTR_ERR(ringacc->proxy_target_base);
>> +
>> + ringacc->num_proxies = dbg_readl(&ringacc->proxy_gcfg->config) &
>> + K3_RINGACC_PROXY_CFG_THREADS_MASK;
>> +
>> + ringacc->rings = devm_kzalloc(dev,
>> + sizeof(*ringacc->rings) *
>> + ringacc->num_rings,
>> + GFP_KERNEL);
>> + ringacc->rings_inuse = devm_kcalloc(dev,
>> + BITS_TO_LONGS(ringacc->num_rings),
>> + sizeof(unsigned long), GFP_KERNEL);
>> + ringacc->proxy_inuse = devm_kcalloc(dev,
>> + BITS_TO_LONGS(ringacc->num_proxies),
>> + sizeof(unsigned long), GFP_KERNEL);
>> +
>> + if (!ringacc->rings || !ringacc->rings_inuse ||
>> !ringacc->proxy_inuse)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < ringacc->num_rings; i++) {
>> + ringacc->rings[i].rt = base_rt +
>> + K3_RINGACC_RT_REGS_STEP * i;
>> + ringacc->rings[i].fifos = base_fifo +
>> + K3_RINGACC_FIFO_REGS_STEP * i;
>> + ringacc->rings[i].parent = ringacc;
>> + ringacc->rings[i].ring_id = i;
>> + ringacc->rings[i].proxy_id = K3_RINGACC_PROXY_NOT_USED;
>> + }
>> + dev_set_drvdata(dev, ringacc);
>> +
>> + ringacc->tisci_ring_ops = &ringacc->tisci->ops.rm_ring_ops;
>> +
>> + pm_runtime_enable(dev);
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0) {
>> + pm_runtime_put_noidle(dev);
>> + dev_err(dev, "Failed to enable pm %d\n", ret);
>> + goto err;
>> + }
>> +
>> + mutex_lock(&k3_ringacc_list_lock);
>> + list_add_tail(&ringacc->list, &k3_ringacc_list);
>> + mutex_unlock(&k3_ringacc_list_lock);
>> +
>> + dev_info(dev, "Ring Accelerator probed rings:%u, gp-rings[%u,%u]
>> sci-dev-id:%u\n",
>> + ringacc->num_rings,
>> + ringacc->rm_gp_range->desc[0].start,
>> + ringacc->rm_gp_range->desc[0].num,
>> + ringacc->tisci_dev_id);
>> + dev_info(dev, "dma-ring-reset-quirk: %s\n",
>> + ringacc->dma_ring_reset_quirk ? "enabled" : "disabled");
>> + dev_info(dev, "RA Proxy rev. %08x, num_proxies:%u\n",
>> + dbg_readl(&ringacc->proxy_gcfg->revision),
>> + ringacc->num_proxies);
>> + return 0;
>> +
>> +err:
>> + pm_runtime_disable(dev);
>> + return ret;
>> +}
>> +
>> +static int k3_ringacc_remove(struct platform_device *pdev)
>> +{
>> + struct k3_ringacc *ringacc = dev_get_drvdata(&pdev->dev);
>> +
>> + pm_runtime_put_sync(&pdev->dev);
>> + pm_runtime_disable(&pdev->dev);
>> +
>> + mutex_lock(&k3_ringacc_list_lock);
>> + list_del(&ringacc->list);
>> + mutex_unlock(&k3_ringacc_list_lock);
>> + return 0;
>> +}
>> +
>> +/* Match table for of_platform binding */
>> +static const struct of_device_id k3_ringacc_of_match[] = {
>> + { .compatible = "ti,am654-navss-ringacc", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, k3_ringacc_of_match);
>> +
>> +static struct platform_driver k3_ringacc_driver = {
>> + .probe = k3_ringacc_probe,
>> + .remove = k3_ringacc_remove,
>> + .driver = {
>> + .name = "k3-ringacc",
>> + .of_match_table = k3_ringacc_of_match,
>> + },
>> +};
>> +module_platform_driver(k3_ringacc_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("TI Ringacc driver for K3 SOCs");
>> +MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
>> diff --git a/include/linux/soc/ti/k3-ringacc.h
>> b/include/linux/soc/ti/k3-ringacc.h
>> new file mode 100644
>> index 000000000000..debffba48ac9
>> --- /dev/null
>> +++ b/include/linux/soc/ti/k3-ringacc.h
>> @@ -0,0 +1,262 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * K3 Ring Accelerator (RA) subsystem interface
>> + *
>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com
>> + */
>> +
>> +#ifndef __SOC_TI_K3_RINGACC_API_H_
>> +#define __SOC_TI_K3_RINGACC_API_H_
>> +
>> +#include <linux/types.h>
>> +
>> +struct device_node;
>> +
>> +/**
>> + * enum k3_ring_mode - &struct k3_ring_cfg mode
>> + *
>> + * RA ring operational modes
>> + *
>> + * @K3_RINGACC_RING_MODE_RING: Exposed Ring mode for SW direct access
>> + * @K3_RINGACC_RING_MODE_MESSAGE: Messaging mode. Messaging mode
>> requires
>> + * that all accesses to the queue must go through this IP so that all
>> + * accesses to the memory are controlled and ordered. This IP then
>> + * controls the entire state of the queue, and SW has no directly
>> control,
>> + * such as through doorbells and cannot access the storage memory
>> directly.
>> + * This is particularly useful when more than one SW or HW entity
>> can be
>> + * the producer and/or consumer at the same time
>> + * @K3_RINGACC_RING_MODE_CREDENTIALS: Credentials mode is message
>> mode plus
>> + * stores credentials with each message, requiring the element
>> size to be
>> + * doubled to fit the credentials. Any exposed memory should be
>> protected
>> + * by a firewall from unwanted access
>> + * @K3_RINGACC_RING_MODE_QM: Queue manager mode. This takes the
>> credentials
>> + * mode and adds packet length per element, along with additional
>> read only
>> + * fields for element count and accumulated queue length. The QM
>> mode only
>> + * operates with an 8 byte element size (any other element size is
>> + * illegal), and like in credentials mode each operation uses 2
>> element
>> + * slots to store the credentials and length fields
>> + */
>> +enum k3_ring_mode {
>> + K3_RINGACC_RING_MODE_RING = 0,
>> + K3_RINGACC_RING_MODE_MESSAGE,
>> + K3_RINGACC_RING_MODE_CREDENTIALS,
>> + K3_RINGACC_RING_MODE_QM,
>> + K3_RINGACC_RING_MODE_INVALID
>> +};
>> +
>> +/**
>> + * enum k3_ring_size - &struct k3_ring_cfg elm_size
>> + *
>> + * RA ring element's sizes in bytes.
>> + */
>> +enum k3_ring_size {
>> + K3_RINGACC_RING_ELSIZE_4 = 0,
>> + K3_RINGACC_RING_ELSIZE_8,
>> + K3_RINGACC_RING_ELSIZE_16,
>> + K3_RINGACC_RING_ELSIZE_32,
>> + K3_RINGACC_RING_ELSIZE_64,
>> + K3_RINGACC_RING_ELSIZE_128,
>> + K3_RINGACC_RING_ELSIZE_256,
>> + K3_RINGACC_RING_ELSIZE_INVALID
>> +};
>> +
>> +struct k3_ringacc;
>> +struct k3_ring;
>> +
>> +/**
>> + * enum k3_ring_cfg - RA ring configuration structure
>> + *
>> + * @size: Ring size, number of elements
>> + * @elm_size: Ring element size
>> + * @mode: Ring operational mode
>> + * @flags: Ring configuration flags. Possible values:
>> + * @K3_RINGACC_RING_SHARED: when set allows to request the same ring
>> + * few times. It's usable when the same ring is used as Free Host
>> PD ring
>> + * for different flows, for example.
>> + * Note: Locking should be done by consumer if required
>> + */
>> +struct k3_ring_cfg {
>> + u32 size;
>> + enum k3_ring_size elm_size;
>> + enum k3_ring_mode mode;
>> +#define K3_RINGACC_RING_SHARED BIT(1)
>> + u32 flags;
>> +};
>> +
>> +#define K3_RINGACC_RING_ID_ANY (-1)
>> +
>> +/**
>> + * of_k3_ringacc_get_by_phandle - find a RA by phandle property
>> + * @np: device node
>> + * @propname: property name containing phandle on RA node
>> + *
>> + * Returns pointer on the RA - struct k3_ringacc
>> + * or -ENODEV if not found,
>> + * or -EPROBE_DEFER if not yet registered
>> + */
>> +struct k3_ringacc *of_k3_ringacc_get_by_phandle(struct device_node *np,
>> + const char *property);
>> +
>> +#define K3_RINGACC_RING_USE_PROXY BIT(1)
>> +
>> +/**
>> + * k3_ringacc_request_ring - request ring from ringacc
>> + * @ringacc: pointer on ringacc
>> + * @id: ring id or K3_RINGACC_RING_ID_ANY for any general purpose ring
>> + * @flags:
>> + * @K3_RINGACC_RING_USE_PROXY: if set - proxy will be allocated and
>> + * used to access ring memory. Sopported only for rings in
>> + * Message/Credentials/Queue mode.
>> + *
>> + * Returns pointer on the Ring - struct k3_ring
>> + * or NULL in case of failure.
>> + */
>> +struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>> + int id, u32 flags);
>> +
>> +/**
>> + * k3_ringacc_ring_reset - ring reset
>> + * @ring: pointer on Ring
>> + *
>> + * Resets ring internal state ((hw)occ, (hw)idx).
>> + * TODO_GS: ? Ring can be reused without reconfiguration
>> + */
>> +void k3_ringacc_ring_reset(struct k3_ring *ring);
>> +/**
>> + * k3_ringacc_ring_reset - ring reset for DMA rings
>> + * @ring: pointer on Ring
>> + *
>> + * Resets ring internal state ((hw)occ, (hw)idx). Should be used for
>> rings
>> + * which are read by K3 UDMA, like TX or Free Host PD rings.
>> + */
>> +void k3_ringacc_ring_reset_dma(struct k3_ring *ring, u32 occ);
>> +
>> +/**
>> + * k3_ringacc_ring_free - ring free
>> + * @ring: pointer on Ring
>> + *
>> + * Resets ring and free all alocated resources.
>> + */
>> +int k3_ringacc_ring_free(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_get_ring_id - Get the Ring ID
>> + * @ring: pointer on ring
>> + *
>> + * Returns the Ring ID
>> + */
>> +u32 k3_ringacc_get_ring_id(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_get_ring_irq_num - Get the irq number for the ring
>> + * @ring: pointer on ring
>> + *
>> + * Returns the interrupt number which can be used to request the
>> interrupt
>> + */
>> +int k3_ringacc_get_ring_irq_num(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_ring_cfg - ring configure
>> + * @ring: pointer on ring
>> + * @cfg: Ring configuration parameters (see &struct k3_ring_cfg)
>> + *
>> + * Configures ring, including ring memory allocation.
>> + * Returns 0 on success, errno otherwise.
>> + */
>> +int k3_ringacc_ring_cfg(struct k3_ring *ring, struct k3_ring_cfg *cfg);
>> +
>> +/**
>> + * k3_ringacc_ring_get_size - get ring size
>> + * @ring: pointer on ring
>> + *
>> + * Returns ring size in number of elements.
>> + */
>> +u32 k3_ringacc_ring_get_size(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_ring_get_free - get free elements
>> + * @ring: pointer on ring
>> + *
>> + * Returns number of free elements in the ring.
>> + */
>> +u32 k3_ringacc_ring_get_free(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_ring_get_occ - get ring occupancy
>> + * @ring: pointer on ring
>> + *
>> + * Returns total number of valid entries on the ring
>> + */
>> +u32 k3_ringacc_ring_get_occ(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_ring_is_full - checks if ring is full
>> + * @ring: pointer on ring
>> + *
>> + * Returns true if the ring is full
>> + */
>> +u32 k3_ringacc_ring_is_full(struct k3_ring *ring);
>> +
>> +/**
>> + * k3_ringacc_ring_push - push element to the ring tail
>> + * @ring: pointer on ring
>> + * @elem: pointer on ring element buffer
>> + *
>> + * Push one ring element to the ring tail. Size of the ring element is
>> + * determined by ring configuration &struct k3_ring_cfg elm_size.
>> + *
>> + * Returns 0 on success, errno otherwise.
>> + */
>> +int k3_ringacc_ring_push(struct k3_ring *ring, void *elem);
>> +
>> +/**
>> + * k3_ringacc_ring_pop - pop element from the ring head
>> + * @ring: pointer on ring
>> + * @elem: pointer on ring element buffer
>> + *
>> + * Push one ring element from the ring head. Size of the ring element is
>> + * determined by ring configuration &struct k3_ring_cfg elm_size..
>> + *
>> + * Returns 0 on success, errno otherwise.
>> + */
>> +int k3_ringacc_ring_pop(struct k3_ring *ring, void *elem);
>> +
>> +/**
>> + * k3_ringacc_ring_push_head - push element to the ring head
>> + * @ring: pointer on ring
>> + * @elem: pointer on ring element buffer
>> + *
>> + * Push one ring element to the ring head. Size of the ring element is
>> + * determined by ring configuration &struct k3_ring_cfg elm_size.
>> + *
>> + * Returns 0 on success, errno otherwise.
>> + * Not Supported by ring modes: K3_RINGACC_RING_MODE_RING
>> + */
>> +int k3_ringacc_ring_push_head(struct k3_ring *ring, void *elem);
>> +
>> +/**
>> + * k3_ringacc_ring_pop_tail - pop element from the ring tail
>> + * @ring: pointer on ring
>> + * @elem: pointer on ring element buffer
>> + *
>> + * Push one ring element from the ring tail. Size of the ring element is
>> + * determined by ring configuration &struct k3_ring_cfg elm_size.
>> + *
>> + * Returns 0 on success, errno otherwise.
>> + * Not Supported by ring modes: K3_RINGACC_RING_MODE_RING
>> + */
>> +int k3_ringacc_ring_pop_tail(struct k3_ring *ring, void *elem);
>> +
>> +u32 k3_ringacc_get_tisci_dev_id(struct k3_ring *ring);
>> +
>> +/**
>> + * Debugging definitions
>> + * TODO: might be removed
>> + */
>> +#ifdef CONFIG_TI_K3_RINGACC_DEBUG
>> +void k3_ringacc_ring_dump(struct k3_ring *ring);
>> +#else
>> +static inline void k3_ringacc_ring_dump(struct k3_ring *ring) {};
>> +#endif
>> +
>> +#endif /* __SOC_TI_K3_RINGACC_API_H_ */
>>
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Regards
Vignesh
_______________________________________________
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 v5 0/5] Add HDMI jack support on RK3288
From: Neil Armstrong @ 2019-09-09 7:37 UTC (permalink / raw)
To: Cheng-yi Chiang, Tzung-Bi Shih
Cc: Jernej Skrabec, ALSA development, Douglas Anderson,
Heiko Stuebner, Liam Girdwood, David Airlie, dri-devel,
Jonas Karlman, Takashi Iwai, tzungbi, Linux Kernel Mailing List,
Hans Verkuil, Andrzej Hajda, Russell King, Mark Brown,
Laurent Pinchart, Daniel Vetter, linux-rockchip, Dylan Reid,
Jaroslav Kysela, linux-arm-kernel
In-Reply-To: <CAFv8NwJjG4mwfnYO=M3O9nZN48D6aY72nQuqEFpZL68dh5727w@mail.gmail.com>
Hi,
On 08/09/2019 15:51, Cheng-yi Chiang wrote:
> On Fri, Aug 30, 2019 at 10:55 AM Cheng-yi Chiang <cychiang@chromium.org> wrote:
>>
>> On Wed, Jul 17, 2019 at 6:28 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
>>>
>>> On Wed, Jul 17, 2019 at 4:33 PM Cheng-Yi Chiang <cychiang@chromium.org> wrote:
>>>>
>>>> This patch series supports HDMI jack reporting on RK3288, which uses
>>>> DRM dw-hdmi driver and hdmi-codec codec driver.
>>>>
>>>> The previous discussion about reporting jack status using hdmi-notifier
>>>> and drm_audio_component is at
>>>>
>>>> https://lore.kernel.org/patchwork/patch/1083027/
>>>>
>>>> The new approach is to use a callback mechanism that is
>>>> specific to hdmi-codec.
>>>>
>>>> Changes from v4 to v5:
>>>> - synopsys/Kconfig: Remove the incorrect dependency change in v4.
>>>> - rockchip/Kconfig: Add dependency of hdmi-codec when it is really need
>>>> for jack support.
>>>>
>>>> Cheng-Yi Chiang (5):
>>>> ASoC: hdmi-codec: Add an op to set callback function for plug event
>>>> drm: bridge: dw-hdmi: Report connector status using callback
>>>> drm: dw-hdmi-i2s: Use fixed id for codec device
>>>> ASoC: rockchip_max98090: Add dai_link for HDMI
>>>> ASoC: rockchip_max98090: Add HDMI jack support
>>>>
>>> LGTM.
>>>
>>> Reviewed-by: Tzung-Bi Shih <tzungbi@google.com>
>>
>> Hi Daniel,
>> Do you have further concern on this patch series related to hdmi-codec
>> and drm part ?
>> We would like to merge this patch series if possible.
>> They will be needed in many future chrome projects for HDMI audio jack
>> detection.
>> Thanks a lot!
>
> Hi Neil,
> I am not sure if this patch series is under your radar.
> Would you mind taking a look ?
> This patch series has been following great suggestions from various
> reviewers, so I hope it is fine now.
I'd like some review from ASoC people and other drm bridge reviewers,
Jernej, Jonas & Andrzej.
Jonas could have some comments on the overall patchset.
>
> Audio jack reporting for HDMI might not be needed for other OS, but it
> is a must on ChromeOS.
> We have many previous projects using different local patch sets to
> achieve HDMI jack reporting.
> I hope that we can achieve a proper way and really get the patches
> merged to mainline.
> Thanks a lot!
>
Sure,
Don't forget to put Jernej, Jonas, and Jerome Brunet <jbrunet@baylibre.com> who is
working on integrating audio on the Amlogic SoCs.
Neil
_______________________________________________
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 4/5] ARM: davinci: support multiplatform build for ARM v5
From: Sekhar Nori @ 2019-09-09 7:38 UTC (permalink / raw)
To: Bartosz Golaszewski, Kevin Hilman, Arnd Bergmann, David Lechner
Cc: Bartosz Golaszewski, linux-kernel, linux-arm-kernel
In-Reply-To: <20190725131257.6142-5-brgl@bgdev.pl>
On 25/07/19 6:42 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add modifications necessary to make davinci part of the ARM v5
> multiplatform build.
>
> Move the arch-specific configuration out of arch/arm/Kconfig and
> into mach-davinci/Kconfig. Remove the sub-menu for DaVinci
> implementations (they'll be visible directly under the system type.
> Select all necessary options not already selected by ARCH_MULTI_V5.
> Update davinci_all_defconfig. Explicitly include the mach-specific
> headers in mach-davinci/Makefile.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Sekhar Nori <nsekhar@ti.com>
Thanks,
Sekhar
_______________________________________________
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 5/5] ARM: multi_v5_defconfig: make DaVinci part of the ARM v5 multiplatform build
From: Sekhar Nori @ 2019-09-09 7:38 UTC (permalink / raw)
To: Bartosz Golaszewski, Kevin Hilman, Arnd Bergmann, David Lechner
Cc: Bartosz Golaszewski, linux-kernel, linux-arm-kernel
In-Reply-To: <20190725131257.6142-6-brgl@bgdev.pl>
On 25/07/19 6:42 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add all DaVinci boards to multi_v5_defconfig.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Sekhar Nori <nsekhar@ti.com>
Thanks,
Sekhar
_______________________________________________
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] crypto: arm64: Use PTR_ERR_OR_ZERO rather than its implementation.
From: Herbert Xu @ 2019-09-09 7:48 UTC (permalink / raw)
To: zhong jiang
Cc: catalin.marinas, linux-kernel, linux-crypto, will, davem,
linux-arm-kernel
In-Reply-To: <1567493656-19916-1-git-send-email-zhongjiang@huawei.com>
On Tue, Sep 03, 2019 at 02:54:16PM +0800, zhong jiang wrote:
> PTR_ERR_OR_ZERO contains if(IS_ERR(...)) + PTR_ERR. It is better to
> use it directly. hence just replace it.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> arch/arm64/crypto/aes-glue.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
_______________________________________________
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 0/5] ARM: make DaVinci part of the ARM v5 multiplatform build
From: Sekhar Nori @ 2019-09-09 7:53 UTC (permalink / raw)
To: Bartosz Golaszewski, Arnd Bergmann
Cc: David Lechner, Kevin Hilman, Linux Kernel Mailing List,
Bartosz Golaszewski, Hans Verkuil, Mauro Carvalho Chehab,
Linux ARM, Linux Media Mailing List
In-Reply-To: <CAMRc=Mdsfbh1nF1a23Anig=w42s7=WzS3Uz7KK1P1aRNP2kFvg@mail.gmail.com>
On 08/09/19 7:01 PM, Bartosz Golaszewski wrote:
> sob., 7 wrz 2019 o 10:21 Arnd Bergmann <arnd@arndb.de> napisał(a):
>>
>> On Wed, Aug 28, 2019 at 9:55 AM Bartosz Golaszewski
>> <bgolaszewski@baylibre.com> wrote:
>>> śr., 28 sie 2019 o 09:44 Sekhar Nori <nsekhar@ti.com> napisał(a):
>>>
>>> Actually I tested this without the clocksource conversion and it works
>>> - the previous driver still selects relevant config options. But I
>>> think you're right - it's worth picking up all the bug fixes from this
>>> series and then merging the rest once dm365 issue is fixed.
>>
>> I just had another look at the series and found that the driver fixes
>> (patches 1 and 2) are queued in linux-next, and patch 3 was merged.
>>
>> If you like, I could put the remaining two patches into the arm/late branch
>> and send that after the media and staging trees are merged into mainline.
>>
>> Arnd
>
> Sure! Makes sense.
>
> Sekhar: this series doesn't break current mainline (i.e. without the
> clocksource series) so I think we're safe even for dm365.
Yes, I boot/NFS tested the two patches applied on latest linux-next on
on all 6 DaVinci SoCs.
I have acked the patches in case Arnd will apply them directly to ARM-SoC.
Thanks,
Sekhar
_______________________________________________
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 v9 1/8] mm: Add per-cpu logic to page shuffling
From: David Hildenbrand @ 2019-09-09 8:14 UTC (permalink / raw)
To: Alexander Duyck, virtio-dev, kvm, mst, catalin.marinas,
dave.hansen, linux-kernel, willy, mhocko, linux-mm, akpm, will,
linux-arm-kernel, osalvador
Cc: yang.zhang.wz, pagupta, riel, konrad.wilk, ying.huang,
lcapitulino, wei.w.wang, aarcange, nitesh, pbonzini,
dan.j.williams, fengguang.wu, alexander.h.duyck, kirill.shutemov
In-Reply-To: <20190907172512.10910.74435.stgit@localhost.localdomain>
On 07.09.19 19:25, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
> Change the logic used to generate randomness in the suffle path so that we
> can avoid cache line bouncing. The previous logic was sharing the offset
> and entropy word between all CPUs. As such this can result in cache line
> bouncing and will ultimately hurt performance when enabled.
So, usually we perform such changes if there is real evidence. Do you
have any such performance numbers to back your claims?
>
> To resolve this I have moved to a per-cpu logic for maintaining a unsigned
> long containing some amount of bits, and an offset value for which bit we
> can use for entropy with each call.
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
> mm/shuffle.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/mm/shuffle.c b/mm/shuffle.c
> index 3ce12481b1dc..9ba542ecf335 100644
> --- a/mm/shuffle.c
> +++ b/mm/shuffle.c
> @@ -183,25 +183,38 @@ void __meminit __shuffle_free_memory(pg_data_t *pgdat)
> shuffle_zone(z);
> }
>
> +struct batched_bit_entropy {
> + unsigned long entropy_bool;
> + int position;
> +};
> +
> +static DEFINE_PER_CPU(struct batched_bit_entropy, batched_entropy_bool);
> +
> void add_to_free_area_random(struct page *page, struct free_area *area,
> int migratetype)
> {
> - static u64 rand;
> - static u8 rand_bits;
> + struct batched_bit_entropy *batch;
> + unsigned long entropy;
> + int position;
>
> /*
> - * The lack of locking is deliberate. If 2 threads race to
> - * update the rand state it just adds to the entropy.
> + * We shouldn't need to disable IRQs as the only caller is
> + * __free_one_page and it should only be called with the zone lock
> + * held and either from IRQ context or with local IRQs disabled.
> */
> - if (rand_bits == 0) {
> - rand_bits = 64;
> - rand = get_random_u64();
> + batch = raw_cpu_ptr(&batched_entropy_bool);
> + position = batch->position;
> +
> + if (--position < 0) {
> + batch->entropy_bool = get_random_long();
> + position = BITS_PER_LONG - 1;
> }
>
> - if (rand & 1)
> + batch->position = position;
> + entropy = batch->entropy_bool;
> +
> + if (1ul & (entropy >> position))
> add_to_free_area(page, area, migratetype);
> else
> add_to_free_area_tail(page, area, migratetype);
> - rand_bits--;
> - rand >>= 1;
> }
>
>
--
Thanks,
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] arm64: dts: renesas: Add /soc dma-ranges
From: Geert Uytterhoeven @ 2019-09-09 8:19 UTC (permalink / raw)
To: Marek Vasut
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, Wolfram Sang, Linux-Renesas, Linux ARM,
Marek Vasut
In-Reply-To: <20190907161634.27378-1-marek.vasut@gmail.com>
Hi Marek,
On Sat, Sep 7, 2019 at 6:16 PM <marek.vasut@gmail.com> wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> Add dma-ranges property into /soc node to describe the DMA capabilities
> of the bus. This is currently needed to translate PCI DMA ranges, which
> are limited to 32bit addresses.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Thanks for your patch!
> NOTE: This is needed for the following patches to work correctly:
> https://patchwork.ozlabs.org/patch/1144870/
> https://patchwork.ozlabs.org/patch/1144871/
What happens with the above patches applied, and without this one?
As PCI/OF driver patches go in through different trees, is it safe to apply
this patch now?
Should they go in together?
> arch/arm64/boot/dts/renesas/r8a7795.dtsi | 1 +
> arch/arm64/boot/dts/renesas/r8a7796.dtsi | 1 +
> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 +
Do we need similar patches for the other R-Car Gen3 and RZ/G2 DTS files?
What about R-Car Gen2 and RZ/G1?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
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 v9 2/8] mm: Adjust shuffle code to allow for future coalescing
From: David Hildenbrand @ 2019-09-09 8:19 UTC (permalink / raw)
To: Alexander Duyck, virtio-dev, kvm, mst, catalin.marinas,
dave.hansen, linux-kernel, willy, mhocko, linux-mm, akpm, will,
linux-arm-kernel, osalvador
Cc: yang.zhang.wz, pagupta, riel, konrad.wilk, ying.huang,
lcapitulino, wei.w.wang, aarcange, nitesh, pbonzini,
dan.j.williams, fengguang.wu, alexander.h.duyck, kirill.shutemov
In-Reply-To: <20190907172520.10910.83100.stgit@localhost.localdomain>
On 07.09.19 19:25, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
> Move the head/tail adding logic out of the shuffle code and into the
> __free_one_page function since ultimately that is where it is really
> needed anyway. By doing this we should be able to reduce the overhead
> and can consolidate all of the list addition bits in one spot.
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
> include/linux/mmzone.h | 12 --------
> mm/page_alloc.c | 70 +++++++++++++++++++++++++++---------------------
> mm/shuffle.c | 9 +-----
> mm/shuffle.h | 12 ++++++++
> 4 files changed, 53 insertions(+), 50 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index bda20282746b..125f300981c6 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -116,18 +116,6 @@ static inline void add_to_free_area_tail(struct page *page, struct free_area *ar
> area->nr_free++;
> }
>
> -#ifdef CONFIG_SHUFFLE_PAGE_ALLOCATOR
> -/* Used to preserve page allocation order entropy */
> -void add_to_free_area_random(struct page *page, struct free_area *area,
> - int migratetype);
> -#else
> -static inline void add_to_free_area_random(struct page *page,
> - struct free_area *area, int migratetype)
> -{
> - add_to_free_area(page, area, migratetype);
> -}
> -#endif
> -
> /* Used for pages which are on another list */
> static inline void move_to_free_area(struct page *page, struct free_area *area,
> int migratetype)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c5d62f1c2851..4e4356ba66c7 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -878,6 +878,36 @@ static inline struct capture_control *task_capc(struct zone *zone)
> #endif /* CONFIG_COMPACTION */
>
> /*
> + * If this is not the largest possible page, check if the buddy
> + * of the next-highest order is free. If it is, it's possible
> + * that pages are being freed that will coalesce soon. In case,
> + * that is happening, add the free page to the tail of the list
> + * so it's less likely to be used soon and more likely to be merged
> + * as a higher order page
> + */
> +static inline bool
> +buddy_merge_likely(unsigned long pfn, unsigned long buddy_pfn,
> + struct page *page, unsigned int order)
> +{
> + struct page *higher_page, *higher_buddy;
> + unsigned long combined_pfn;
> +
> + if (order >= MAX_ORDER - 2)
> + return false;
> +
> + if (!pfn_valid_within(buddy_pfn))
> + return false;
> +
> + combined_pfn = buddy_pfn & pfn;
> + higher_page = page + (combined_pfn - pfn);
> + buddy_pfn = __find_buddy_pfn(combined_pfn, order + 1);
> + higher_buddy = higher_page + (buddy_pfn - combined_pfn);
> +
> + return pfn_valid_within(buddy_pfn) &&
> + page_is_buddy(higher_page, higher_buddy, order + 1);
> +}
> +
> +/*
> * Freeing function for a buddy system allocator.
> *
> * The concept of a buddy system is to maintain direct-mapped table
> @@ -906,11 +936,12 @@ static inline void __free_one_page(struct page *page,
> struct zone *zone, unsigned int order,
> int migratetype)
> {
> - unsigned long combined_pfn;
> + struct capture_control *capc = task_capc(zone);
> unsigned long uninitialized_var(buddy_pfn);
> - struct page *buddy;
> + unsigned long combined_pfn;
> + struct free_area *area;
> unsigned int max_order;
> - struct capture_control *capc = task_capc(zone);
> + struct page *buddy;
>
> max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1);
>
> @@ -979,35 +1010,12 @@ static inline void __free_one_page(struct page *page,
> done_merging:
> set_page_order(page, order);
>
> - /*
> - * If this is not the largest possible page, check if the buddy
> - * of the next-highest order is free. If it is, it's possible
> - * that pages are being freed that will coalesce soon. In case,
> - * that is happening, add the free page to the tail of the list
> - * so it's less likely to be used soon and more likely to be merged
> - * as a higher order page
> - */
> - if ((order < MAX_ORDER-2) && pfn_valid_within(buddy_pfn)
> - && !is_shuffle_order(order)) {
> - struct page *higher_page, *higher_buddy;
> - combined_pfn = buddy_pfn & pfn;
> - higher_page = page + (combined_pfn - pfn);
> - buddy_pfn = __find_buddy_pfn(combined_pfn, order + 1);
> - higher_buddy = higher_page + (buddy_pfn - combined_pfn);
> - if (pfn_valid_within(buddy_pfn) &&
> - page_is_buddy(higher_page, higher_buddy, order + 1)) {
> - add_to_free_area_tail(page, &zone->free_area[order],
> - migratetype);
> - return;
> - }
> - }
> -
> - if (is_shuffle_order(order))
> - add_to_free_area_random(page, &zone->free_area[order],
> - migratetype);
> + area = &zone->free_area[order];
> + if (is_shuffle_order(order) ? shuffle_pick_tail() :
> + buddy_merge_likely(pfn, buddy_pfn, page, order))
> + add_to_free_area_tail(page, area, migratetype);
> else
> - add_to_free_area(page, &zone->free_area[order], migratetype);
> -
> + add_to_free_area(page, area, migratetype);
> }
>
> /*
> diff --git a/mm/shuffle.c b/mm/shuffle.c
> index 9ba542ecf335..345cb4347455 100644
> --- a/mm/shuffle.c
> +++ b/mm/shuffle.c
> @@ -4,7 +4,6 @@
> #include <linux/mm.h>
> #include <linux/init.h>
> #include <linux/mmzone.h>
> -#include <linux/random.h>
> #include <linux/moduleparam.h>
> #include "internal.h"
> #include "shuffle.h"
> @@ -190,8 +189,7 @@ struct batched_bit_entropy {
>
> static DEFINE_PER_CPU(struct batched_bit_entropy, batched_entropy_bool);
>
> -void add_to_free_area_random(struct page *page, struct free_area *area,
> - int migratetype)
> +bool __shuffle_pick_tail(void)
> {
> struct batched_bit_entropy *batch;
> unsigned long entropy;
> @@ -213,8 +211,5 @@ void add_to_free_area_random(struct page *page, struct free_area *area,
> batch->position = position;
> entropy = batch->entropy_bool;
>
> - if (1ul & (entropy >> position))
> - add_to_free_area(page, area, migratetype);
> - else
> - add_to_free_area_tail(page, area, migratetype);
> + return 1ul & (entropy >> position);
> }
> diff --git a/mm/shuffle.h b/mm/shuffle.h
> index 777a257a0d2f..0723eb97f22f 100644
> --- a/mm/shuffle.h
> +++ b/mm/shuffle.h
> @@ -3,6 +3,7 @@
> #ifndef _MM_SHUFFLE_H
> #define _MM_SHUFFLE_H
> #include <linux/jump_label.h>
> +#include <linux/random.h>
>
> /*
> * SHUFFLE_ENABLE is called from the command line enabling path, or by
> @@ -22,6 +23,7 @@ enum mm_shuffle_ctl {
> DECLARE_STATIC_KEY_FALSE(page_alloc_shuffle_key);
> extern void page_alloc_shuffle(enum mm_shuffle_ctl ctl);
> extern void __shuffle_free_memory(pg_data_t *pgdat);
> +extern bool __shuffle_pick_tail(void);
> static inline void shuffle_free_memory(pg_data_t *pgdat)
> {
> if (!static_branch_unlikely(&page_alloc_shuffle_key))
> @@ -43,6 +45,11 @@ static inline bool is_shuffle_order(int order)
> return false;
> return order >= SHUFFLE_ORDER;
> }
> +
> +static inline bool shuffle_pick_tail(void)
> +{
> + return __shuffle_pick_tail();
> +}
> #else
> static inline void shuffle_free_memory(pg_data_t *pgdat)
> {
> @@ -60,5 +67,10 @@ static inline bool is_shuffle_order(int order)
> {
> return false;
> }
> +
> +static inline bool shuffle_pick_tail(void)
> +{
> + return false;
> +}
> #endif
> #endif /* _MM_SHUFFLE_H */
>
>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
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
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