* Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Krzysztof Kozlowski @ 2024-04-06 10:57 UTC (permalink / raw)
To: Peng Fan (OSS), Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-1-9fc9186856c2@nxp.com>
On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> When adding vendor extension protocols, there is dt-schema warning:
> "
> imx,scmi.example.dtb: scmi: 'protocol@81', 'protocol@84' do not match any
> of the regexes: 'pinctrl-[0-9]+'
> "
>
> Set additionalProperties to true to address the issue.
I do not see anything addressed here, except making the binding
accepting anything anywhere...
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 4/4] clk: en7523: Add EN7581 support
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce EN7581 clock support to clk-en7523 driver.
Add hw_init callback to en_clk_soc_data data structure.
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/clk/clk-en7523.c | 158 +++++++++++++++++++++++++++++++++++++--
1 file changed, 153 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 7eee921ab575..381605be333f 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -4,13 +4,16 @@
#include <linux/clk-provider.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <dt-bindings/clock/en7523-clk.h>
#define REG_PCI_CONTROL 0x88
#define REG_PCI_CONTROL_PERSTOUT BIT(29)
#define REG_PCI_CONTROL_PERSTOUT1 BIT(26)
+#define REG_PCI_CONTROL_REFCLK_EN0 BIT(23)
#define REG_PCI_CONTROL_REFCLK_EN1 BIT(22)
+#define REG_PCI_CONTROL_PERSTOUT2 BIT(16)
#define REG_GSW_CLK_DIV_SEL 0x1b4
#define REG_EMI_CLK_DIV_SEL 0x1b8
#define REG_BUS_CLK_DIV_SEL 0x1bc
@@ -18,10 +21,25 @@
#define REG_SPI_CLK_FREQ_SEL 0x1c8
#define REG_NPU_CLK_DIV_SEL 0x1fc
#define REG_CRYPTO_CLKSRC 0x200
-#define REG_RESET_CONTROL 0x834
+#define REG_RESET_CONTROL2 0x830
+#define REG_RESET2_CONTROL_PCIE2 BIT(27)
+#define REG_RESET_CONTROL1 0x834
#define REG_RESET_CONTROL_PCIEHB BIT(29)
#define REG_RESET_CONTROL_PCIE1 BIT(27)
#define REG_RESET_CONTROL_PCIE2 BIT(26)
+/* EN7581 */
+#define REG_PCIE0_MEM 0x00
+#define REG_PCIE0_MEM_MASK 0x04
+#define REG_PCIE1_MEM 0x08
+#define REG_PCIE1_MEM_MASK 0x0c
+#define REG_PCIE2_MEM 0x10
+#define REG_PCIE2_MEM_MASK 0x14
+#define REG_PCIE_RESET_OPEN_DRAIN 0x018c
+#define REG_PCIE_RESET_OPEN_DRAIN_MASK GENMASK(2, 0)
+#define REG_NP_SCU_PCIC 0x88
+#define REG_NP_SCU_SSTR 0x9c
+#define REG_PCIE_XSI0_SEL_MASK GENMASK(14, 13)
+#define REG_PCIE_XSI1_SEL_MASK GENMASK(12, 11)
struct en_clk_desc {
int id;
@@ -49,6 +67,8 @@ struct en_clk_gate {
struct en_clk_soc_data {
const struct clk_ops pcie_ops;
+ int (*hw_init)(struct platform_device *pdev, void __iomem *base,
+ void __iomem *np_base);
};
static const u32 gsw_base[] = { 400000000, 500000000 };
@@ -211,14 +231,14 @@ static int en7523_pci_prepare(struct clk_hw *hw)
usleep_range(1000, 2000);
/* Reset to default */
- val = readl(np_base + REG_RESET_CONTROL);
+ val = readl(np_base + REG_RESET_CONTROL1);
mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
REG_RESET_CONTROL_PCIEHB;
- writel(val & ~mask, np_base + REG_RESET_CONTROL);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
usleep_range(1000, 2000);
- writel(val | mask, np_base + REG_RESET_CONTROL);
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
msleep(100);
- writel(val & ~mask, np_base + REG_RESET_CONTROL);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
usleep_range(5000, 10000);
/* Release device */
@@ -259,6 +279,9 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
cg->base = np_base;
cg->hw.init = &init;
+
+ if (init.ops->disable)
+ init.ops->disable(&cg->hw);
init.ops->unprepare(&cg->hw);
if (clk_hw_register(dev, &cg->hw))
@@ -267,6 +290,111 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
return &cg->hw;
}
+static int en7581_pci_is_enabled(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
+ val = readl(cg->base + REG_PCI_CONTROL);
+ return (val & mask) == mask;
+}
+
+static int en7581_pci_prepare(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
+ REG_RESET_CONTROL_PCIEHB;
+ val = readl(np_base + REG_RESET_CONTROL1);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
+ val = readl(np_base + REG_RESET_CONTROL2);
+ writel(val & ~REG_RESET2_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
+ usleep_range(5000, 10000);
+
+ return 0;
+}
+
+static int en7581_pci_enable(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
+ REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
+ REG_PCI_CONTROL_PERSTOUT;
+ val = readl(np_base + REG_PCI_CONTROL);
+ writel(val | mask, np_base + REG_PCI_CONTROL);
+ msleep(250);
+
+ return 0;
+}
+
+static void en7581_pci_unprepare(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
+ REG_RESET_CONTROL_PCIEHB;
+ val = readl(np_base + REG_RESET_CONTROL1);
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2;
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
+ val = readl(np_base + REG_RESET_CONTROL2);
+ writel(val | REG_RESET_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
+ msleep(100);
+}
+
+static void en7581_pci_disable(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
+ REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
+ REG_PCI_CONTROL_PERSTOUT;
+ val = readl(np_base + REG_PCI_CONTROL);
+ writel(val & ~mask, np_base + REG_PCI_CONTROL);
+ usleep_range(1000, 2000);
+}
+
+static int en7581_clk_hw_init(struct platform_device *pdev,
+ void __iomem *base,
+ void __iomem *np_base)
+{
+ void __iomem *pb_base;
+ u32 val;
+
+ pb_base = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(pb_base))
+ return PTR_ERR(pb_base);
+
+ val = readl(np_base + REG_NP_SCU_SSTR);
+ val &= ~(REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
+ writel(val, np_base + REG_NP_SCU_SSTR);
+ val = readl(np_base + REG_NP_SCU_PCIC);
+ writel(val | 3, np_base + REG_NP_SCU_PCIC);
+
+ writel(0x20000000, pb_base + REG_PCIE0_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE0_MEM_MASK);
+ writel(0x24000000, pb_base + REG_PCIE1_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE1_MEM_MASK);
+ writel(0x28000000, pb_base + REG_PCIE2_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE2_MEM_MASK);
+
+ val = readl(base + REG_PCIE_RESET_OPEN_DRAIN);
+ writel(val | REG_PCIE_RESET_OPEN_DRAIN_MASK,
+ base + REG_PCIE_RESET_OPEN_DRAIN);
+
+ return 0;
+}
+
static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
void __iomem *base, void __iomem *np_base)
{
@@ -299,6 +427,7 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
static int en7523_clk_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
+ const struct en_clk_soc_data *soc_data;
struct clk_hw_onecell_data *clk_data;
void __iomem *base, *np_base;
int r;
@@ -311,6 +440,13 @@ static int en7523_clk_probe(struct platform_device *pdev)
if (IS_ERR(np_base))
return PTR_ERR(np_base);
+ soc_data = of_device_get_match_data(&pdev->dev);
+ if (soc_data->hw_init) {
+ r = soc_data->hw_init(pdev, base, np_base);
+ if (r)
+ return r;
+ }
+
clk_data = devm_kzalloc(&pdev->dev,
struct_size(clk_data, hws, EN7523_NUM_CLOCKS),
GFP_KERNEL);
@@ -336,8 +472,20 @@ static const struct en_clk_soc_data en7523_data = {
},
};
+static const struct en_clk_soc_data en7581_data = {
+ .pcie_ops = {
+ .is_enabled = en7581_pci_is_enabled,
+ .prepare = en7581_pci_prepare,
+ .enable = en7581_pci_enable,
+ .unprepare = en7581_pci_unprepare,
+ .disable = en7581_pci_disable,
+ },
+ .hw_init = en7581_clk_hw_init,
+};
+
static const struct of_device_id of_match_clk_en7523[] = {
{ .compatible = "airoha,en7523-scu", .data = &en7523_data },
+ { .compatible = "airoha,en7581-scu", .data = &en7581_data },
{ /* sentinel */ }
};
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 3/4] clk: en7523: Add en_clk_soc_data data structure
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce en_clk_soc_data data structure in order to define multiple
clk_ops for each supported SoC. This is a preliminary patch to
introduce EN7581 clock support.
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/clk/clk-en7523.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 7cde328495e2..7eee921ab575 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -47,6 +47,10 @@ struct en_clk_gate {
struct clk_hw hw;
};
+struct en_clk_soc_data {
+ const struct clk_ops pcie_ops;
+};
+
static const u32 gsw_base[] = { 400000000, 500000000 };
static const u32 emi_base[] = { 333000000, 400000000 };
static const u32 bus_base[] = { 500000000, 540000000 };
@@ -145,11 +149,6 @@ static const struct en_clk_desc en7523_base_clks[] = {
}
};
-static const struct of_device_id of_match_clk_en7523[] = {
- { .compatible = "airoha,en7523-scu", },
- { /* sentinel */ }
-};
-
static unsigned int en7523_get_base_rate(void __iomem *base, unsigned int i)
{
const struct en_clk_desc *desc = &en7523_base_clks[i];
@@ -247,14 +246,10 @@ static void en7523_pci_unprepare(struct clk_hw *hw)
static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
void __iomem *np_base)
{
- static const struct clk_ops pcie_gate_ops = {
- .is_enabled = en7523_pci_is_enabled,
- .prepare = en7523_pci_prepare,
- .unprepare = en7523_pci_unprepare,
- };
+ const struct en_clk_soc_data *soc_data = of_device_get_match_data(dev);
struct clk_init_data init = {
.name = "pcie",
- .ops = &pcie_gate_ops,
+ .ops = &soc_data->pcie_ops,
};
struct en_clk_gate *cg;
@@ -264,7 +259,7 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
cg->base = np_base;
cg->hw.init = &init;
- en7523_pci_unprepare(&cg->hw);
+ init.ops->unprepare(&cg->hw);
if (clk_hw_register(dev, &cg->hw))
return NULL;
@@ -333,6 +328,19 @@ static int en7523_clk_probe(struct platform_device *pdev)
return r;
}
+static const struct en_clk_soc_data en7523_data = {
+ .pcie_ops = {
+ .is_enabled = en7523_pci_is_enabled,
+ .prepare = en7523_pci_prepare,
+ .unprepare = en7523_pci_unprepare,
+ },
+};
+
+static const struct of_device_id of_match_clk_en7523[] = {
+ { .compatible = "airoha,en7523-scu", .data = &en7523_data },
+ { /* sentinel */ }
+};
+
static struct platform_driver clk_en7523_drv = {
.probe = en7523_clk_probe,
.driver = {
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/4] arm64: dts: airoha: Add EN7581 clock node
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
index 55eb1762fb11..5c4bfe3e1e5a 100644
--- a/arch/arm64/boot/dts/airoha/en7581.dtsi
+++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
@@ -2,6 +2,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/en7523-clk.h>
/ {
interrupt-parent = <&gic>;
@@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <1843200>;
};
+
+ scuclk: clock-controller@1fa20000 {
+ compatible = "airoha,en7581-scu";
+ reg = <0x0 0x1fa20000 0x0 0x400>,
+ <0x0 0x1fb00000 0x0 0x1000>,
+ <0x0 0x1fbe3400 0x0 0xfc>;
+ #clock-cells = <1>;
+ };
};
};
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
.../bindings/clock/airoha,en7523-scu.yaml | 31 +++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index 79b0752faa91..3f4266637733 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -29,10 +29,13 @@ description: |
properties:
compatible:
items:
- - const: airoha,en7523-scu
+ - enum:
+ - airoha,en7523-scu
+ - airoha,en7581-scu
reg:
- maxItems: 2
+ minItems: 2
+ maxItems: 3
"#clock-cells":
description:
@@ -45,6 +48,30 @@ required:
- reg
- '#clock-cells'
+allOf:
+ - if:
+ properties:
+ compatible:
+ const: airoha,en7523-scu
+ then:
+ properties:
+ reg:
+ items:
+ - description: scu base address
+ - description: misc scu base address
+
+ - if:
+ properties:
+ compatible:
+ const: airoha,en7581-scu
+ then:
+ properties:
+ reg:
+ items:
+ - description: scu base address
+ - description: misc scu base address
+ - description: pb scu base address
+
additionalProperties: false
examples:
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/4] Introduce clock support for Airoha EN7581 SoC
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
this series is based on the following series:
https://patchwork.kernel.org/project/linux-arm-kernel/cover/cover.1709975956.git.lorenzo@kernel.org/
Changes since v1:
- add .enable()/disable() callbacks to pcie ops
- introduce en_clk_soc_data data structure in order to define multiple clk_ops
for each supported SoC
- rename clock node from system-controller to clock-controller
- add missing item descriptions in dt-binding
Lorenzo Bianconi (4):
dt-bindings: clock: airoha: add EN7581 binding
arm64: dts: airoha: Add EN7581 clock node
clk: en7523: Add en_clk_soc_data data structure
clk: en7523: Add EN7581 support
.../bindings/clock/airoha,en7523-scu.yaml | 31 ++-
arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +
drivers/clk/clk-en7523.c | 190 ++++++++++++++++--
3 files changed, 211 insertions(+), 19 deletions(-)
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: (subset) [PATCH 00/17] HSI2, UFS & UFS phy support for Tensor GS101
From: Vinod Koul @ 2024-04-06 9:19 UTC (permalink / raw)
To: mturquette, sboyd, robh, krzk+dt, conor+dt, kishon, alim.akhtar,
avri.altman, bvanassche, s.nawrocki, cw00.choi, jejb,
martin.petersen, chanho61.park, ebiggers, Peter Griffin
Cc: linux-scsi, linux-phy, devicetree, linux-clk, linux-samsung-soc,
linux-kernel, linux-arm-kernel, tudor.ambarus, andre.draszik,
saravanak, willmcvicker
In-Reply-To: <20240404122559.898930-1-peter.griffin@linaro.org>
On Thu, 04 Apr 2024 13:25:42 +0100, Peter Griffin wrote:
> This series adds support for the High Speed Interface (HSI) 2 clock
> management unit, UFS controller and UFS phy calibration/tuning for GS101.
>
> With this series applied, UFS is now functional! The SKhynix HN8T05BZGKX015
> can be enumerated, partitions mounted etc. This then allows us to move away
> from the initramfs rootfs we have been using for development so far.
>
> [...]
Applied, thanks!
[04/17] dt-bindings: phy: samsung,ufs-phy: Add dedicated gs101-ufs-phy compatible
commit: 724e4fc053fe217d0ed477517ae68db11feab1f5
[09/17] phy: samsung-ufs: use exynos_get_pmu_regmap_by_phandle() to obtain PMU regmap
commit: f2c6d0fa197a1558f4ef50162bb87e6644af232d
[10/17] phy: samsung-ufs: ufs: Add SoC callbacks for calibration and clk data recovery
commit: a4de58a9096b471f9dc1c2bc6bfaa8aa48110c31
[11/17] phy: samsung-ufs: ufs: Add support for gs101 UFS phy tuning
commit: c1cf725db1065153459f0deb69bd4d497a5fd183
[17/17] MAINTAINERS: Add phy-gs101-ufs file to Tensor GS101.
commit: 0338e1d2f933a4ec7ae96ed1f40c39b899e357d7
Best regards,
--
~Vinod
_______________________________________________
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/4] Speed up boot with faster linear map creation
From: Itaru Kitayama @ 2024-04-06 10:31 UTC (permalink / raw)
To: Ryan Roberts
Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
David Hildenbrand, Donald Dutile, Eric Chanudet, linux-arm-kernel,
linux-kernel
In-Reply-To: <fd4aed3a-42be-44e0-b3bb-12f77c5911a1@arm.com>
Hi Ryan,
On Sat, Apr 06, 2024 at 09:32:34AM +0100, Ryan Roberts wrote:
> Hi Itaru,
>
> On 05/04/2024 08:39, Itaru Kitayama wrote:
> > On Thu, Apr 04, 2024 at 03:33:04PM +0100, Ryan Roberts wrote:
> >> Hi All,
> >>
> >> It turns out that creating the linear map can take a significant proportion of
> >> the total boot time, especially when rodata=full. And most of the time is spent
> >> waiting on superfluous tlb invalidation and memory barriers. This series reworks
> >> the kernel pgtable generation code to significantly reduce the number of those
> >> TLBIs, ISBs and DSBs. See each patch for details.
> >>
> >> The below shows the execution time of map_mem() across a couple of different
> >> systems with different RAM configurations. We measure after applying each patch
> >> and show the improvement relative to base (v6.9-rc2):
> >>
> >> | Apple M2 VM | Ampere Altra| Ampere Altra| Ampere Altra
> >> | VM, 16G | VM, 64G | VM, 256G | Metal, 512G
> >> ---------------|-------------|-------------|-------------|-------------
> >> | ms (%) | ms (%) | ms (%) | ms (%)
> >> ---------------|-------------|-------------|-------------|-------------
> >> base | 153 (0%) | 2227 (0%) | 8798 (0%) | 17442 (0%)
> >> no-cont-remap | 77 (-49%) | 431 (-81%) | 1727 (-80%) | 3796 (-78%)
> >> batch-barriers | 13 (-92%) | 162 (-93%) | 655 (-93%) | 1656 (-91%)
> >> no-alloc-remap | 11 (-93%) | 109 (-95%) | 449 (-95%) | 1257 (-93%)
> >> lazy-unmap | 6 (-96%) | 61 (-97%) | 257 (-97%) | 838 (-95%)
> >>
> >> This series applies on top of v6.9-rc2. All mm selftests pass. I've compile and
> >> boot tested various PAGE_SIZE and VA size configs.
> >>
> >> ---
> >>
> >> Changes since v1 [1]
> >> ====================
> >>
> >> - Added Tested-by tags (thanks to Eric and Itaru)
> >> - Renamed ___set_pte() -> __set_pte_nosync() (per Ard)
> >> - Reordered patches (biggest impact & least controversial first)
> >> - Reordered alloc/map/unmap functions in mmu.c to aid reader
> >> - pte_clear() -> __pte_clear() in clear_fixmap_nosync()
> >> - Reverted generic p4d_index() which caused x86 build error. Replaced with
> >> unconditional p4d_index() define under arm64.
> >>
> >>
> >> [1] https://lore.kernel.org/linux-arm-kernel/20240326101448.3453626-1-ryan.roberts@arm.com/
> >>
> >> Thanks,
> >> Ryan
> >>
> >>
> >> Ryan Roberts (4):
> >> arm64: mm: Don't remap pgtables per-cont(pte|pmd) block
> >> arm64: mm: Batch dsb and isb when populating pgtables
> >> arm64: mm: Don't remap pgtables for allocate vs populate
> >> arm64: mm: Lazily clear pte table mappings from fixmap
> >>
> >> arch/arm64/include/asm/fixmap.h | 5 +-
> >> arch/arm64/include/asm/mmu.h | 8 +
> >> arch/arm64/include/asm/pgtable.h | 13 +-
> >> arch/arm64/kernel/cpufeature.c | 10 +-
> >> arch/arm64/mm/fixmap.c | 11 +
> >> arch/arm64/mm/mmu.c | 377 +++++++++++++++++++++++--------
> >> 6 files changed, 319 insertions(+), 105 deletions(-)
> >>
> >> --
> >> 2.25.1
> >>
> >
> > I've build and boot tested the v2 on FVP, base is taken from your
> > linux-rr repo. Running run_vmtests.sh on v2 left some gup longterm not oks, would you take a look at it? The mm ksefltests used is from your linux-rr repo too.
>
> Thanks for taking a look at this.
>
> I can't reproduce your issue unfortunately; steps as follows on Apple M2 VM:
>
> Config: arm64 defconfig + the following:
>
> # Squashfs for snaps, xfs for large file folios.
> ./scripts/config --enable CONFIG_SQUASHFS_LZ4
> ./scripts/config --enable CONFIG_SQUASHFS_LZO
> ./scripts/config --enable CONFIG_SQUASHFS_XZ
> ./scripts/config --enable CONFIG_SQUASHFS_ZSTD
> ./scripts/config --enable CONFIG_XFS_FS
>
> # For general mm debug.
> ./scripts/config --enable CONFIG_DEBUG_VM
> ./scripts/config --enable CONFIG_DEBUG_VM_MAPLE_TREE
> ./scripts/config --enable CONFIG_DEBUG_VM_RB
> ./scripts/config --enable CONFIG_DEBUG_VM_PGFLAGS
> ./scripts/config --enable CONFIG_DEBUG_VM_PGTABLE
> ./scripts/config --enable CONFIG_PAGE_TABLE_CHECK
>
> # For mm selftests.
> ./scripts/config --enable CONFIG_USERFAULTFD
> ./scripts/config --enable CONFIG_TEST_VMALLOC
> ./scripts/config --enable CONFIG_GUP_TEST
>
> Running on VM with 12G memory, split across 2 (emulated) NUMA nodes (needed by
> some mm selftests), with kernel command line to reserve hugetlbs and other
> features required by some mm selftests:
>
> "
> transparent_hugepage=madvise earlycon root=/dev/vda2 secretmem.enable
> hugepagesz=1G hugepages=0:2,1:2 hugepagesz=32M hugepages=0:2,1:2
> default_hugepagesz=2M hugepages=0:64,1:64 hugepagesz=64K hugepages=0:2,1:2
> "
>
> Ubuntu userspace running off XFS rootfs. Build and run mm selftests from same
> git tree.
>
>
> Although I don't think any of this config should make a difference to gup_longterm.
>
> Looks like your errors are all "ftruncate() failed". I've seen this problem on
> our CI system. There it is due to running the tests from NFS file system. What
> filesystem are you using? Perhaps you are sharing into the FVP using 9p? That
> might also be problematic.
That was it. This time I booted up the kernel including your series on
QEMU on my M1 and executed the gup_longterm program without the ftruncate
failures. When testing your kernel on FVP, I was executing the script from the FVP's host filesystem using 9p.
Thanks,
Itaru.
>
> Does this problem reproduce with v6.9-rc2, without my patches? I except it
> probably does?
>
> Thanks,
> Ryan
>
> >
> > Thanks,
> > Itaru.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESPIN PATCH] irqchip/gic-v4.1:Fix VSYNC referencing an unmapped VPE
From: Marc Zyngier @ 2024-04-06 9:58 UTC (permalink / raw)
To: Nianyao Tang, Thomas Gleixner
Cc: yuzenghui, linux-arm-kernel, linux-kernel, guoyang2, wangwudi
In-Reply-To: <20240406022737.3898763-1-tangnianyao@huawei.com>
On Sat, 06 Apr 2024 03:27:37 +0100,
Nianyao Tang <tangnianyao@huawei.com> wrote:
>
> As per the GICv4.1 spec (Arm IHI 0069H, 5.3.19):
>
> "A VMAPP with {V, Alloc}=={0, x} is self-synchronizing, This means the
> ITS command queue does not show the command as consumed until all of
> its effects are completed."
>
> Furthermore, VSYNC is allowed to deliver an SError when referencing a
> non existent VPE.
>
> By these definitions, a VMAPP followed by a VSYNC is a bug, as the
> later references a VPE that has been unmapped by the former.
>
> Fix it by eliding the VSYNC in this scenario.
>
> Fixes: 64edfaa9a234 ("irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP")
>
> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
Thanks for having taken my remarks into account, it looks good now.
Thomas, can you please take this as a fix for 6.9?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
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] phy: freescale: imx8m-pcie: fix pcie link-up instability
From: Vinod Koul @ 2024-04-06 9:18 UTC (permalink / raw)
To: linux-phy, Marcel Ziswiler
Cc: linux-imx, linux-kernel, kernel, Richard Zhu, Lucas Stach,
linux-arm-kernel, stable, Marcel Ziswiler, Fabio Estevam,
Heiko Stuebner, Kishon Vijay Abraham I, Rob Herring, Sascha Hauer,
Sergio Paracuellos, Shawn Guo, Tim Harvey, Yang Li, imx
In-Reply-To: <20240327071946.8869-1-marcel@ziswiler.com>
On Wed, 27 Mar 2024 08:19:37 +0100, Marcel Ziswiler wrote:
> On the i.MX 8M Mini, the AUX_PLL_REFCLK_SEL has to be left at its reset
> default of AUX_IN (PLL clock).
>
> Background Information:
> In our automated testing setup, we use Delock Mini-PCIe SATA cards [1].
> While this setup has proven very stable overall we noticed upstream on
> the i.MX 8M Mini fails quite regularly (about 50/50) to bring up the
> PCIe link while with NXP's downstream BSP 5.15.71_2.2.2 it always works.
> As that old downstream stuff was quite different, I first also tried
> NXP's latest downstream BSP 6.1.55_2.2.0 which from a PCIe point of view
> is fairly vanilla, however, also there the PCIe link-up was not stable.
> Comparing and debugging I noticed that upstream explicitly configures
> the AUX_PLL_REFCLK_SEL to I_PLL_REFCLK_FROM_SYSPLL while working
> downstream [2] leaving it at reset defaults of AUX_IN (PLL clock).
> Unfortunately, the TRM does not mention any further details about this
> register (both for the i.MX 8M Mini as well as the Plus).
> NXP confirmed their validation codes for the i.MX8MM PCIe doesn't
> configure cmn_reg063 (offset: 0x18C).
> BTW: On the i.MX 8M Plus we have not seen any issues with PCIe with the
> exact same setup which is why I left it unchanged.
>
> [...]
Applied, thanks!
[1/1] phy: freescale: imx8m-pcie: fix pcie link-up instability
commit: 3a161017f1de55cc48be81f6156004c151f32677
Best regards,
--
~Vinod
_______________________________________________
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 v1 0/1] phy: freescale: imx8m-pcie: facing pcie link-up instability
From: Vinod Koul @ 2024-04-06 9:18 UTC (permalink / raw)
To: linux-phy, Marcel Ziswiler
Cc: linux-imx, Lucas Stach, linux-arm-kernel, kernel, Richard Zhu,
linux-kernel, Marcel Ziswiler, Fabio Estevam, Heiko Stuebner,
Kishon Vijay Abraham I, Marc Kleine-Budde, Rob Herring,
Sascha Hauer, Shawn Guo, Tim Harvey, Yang Li, imx
In-Reply-To: <20240322130646.1016630-1-marcel@ziswiler.com>
On Fri, 22 Mar 2024 14:06:31 +0100, Marcel Ziswiler wrote:
> In our automated testing setup, we use Delock Mini-PCIe SATA cards [1].
> While this setup has proven very stable overall we noticed upstream on
> the i.MX8M Mini fails quite regularly (about 50/50) to bring up the PCIe
> link while with NXP's downstream BSP 5.15.71_2.2.2 it always works. As
> that old downstream stuff was quite different, I first also tried NXP's
> latest downstream BSP 6.1.55_2.2.0 which from a PCIe point of view is
> fairly vanilla, however, also there the PCIe link-up was not stable.
> Comparing and debugging I noticed that upstream explicitly configures
> the AUX_PLL_REFCLK_SEL to I_PLL_REFCLK_FROM_SYSPLL while working
> downstream [2] leaving it at reset defaults of AUX_IN (PLL clock).
> Unfortunately, the TRM does not mention any further details about this
> register (both for the i.MX 8M Mini as well as the Plus). Maybe somebody
> from NXP could further comment on this?
>
> [...]
Applied, thanks!
[1/1] phy: freescale: imx8m-pcie: fix pcie link-up instability
commit: 3a161017f1de55cc48be81f6156004c151f32677
Best regards,
--
~Vinod
_______________________________________________
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/4] Speed up boot with faster linear map creation
From: Ryan Roberts @ 2024-04-06 8:32 UTC (permalink / raw)
To: Itaru Kitayama
Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel,
David Hildenbrand, Donald Dutile, Eric Chanudet, linux-arm-kernel,
linux-kernel
In-Reply-To: <Zg+qwooaWFNL7KIg@vm3>
Hi Itaru,
On 05/04/2024 08:39, Itaru Kitayama wrote:
> On Thu, Apr 04, 2024 at 03:33:04PM +0100, Ryan Roberts wrote:
>> Hi All,
>>
>> It turns out that creating the linear map can take a significant proportion of
>> the total boot time, especially when rodata=full. And most of the time is spent
>> waiting on superfluous tlb invalidation and memory barriers. This series reworks
>> the kernel pgtable generation code to significantly reduce the number of those
>> TLBIs, ISBs and DSBs. See each patch for details.
>>
>> The below shows the execution time of map_mem() across a couple of different
>> systems with different RAM configurations. We measure after applying each patch
>> and show the improvement relative to base (v6.9-rc2):
>>
>> | Apple M2 VM | Ampere Altra| Ampere Altra| Ampere Altra
>> | VM, 16G | VM, 64G | VM, 256G | Metal, 512G
>> ---------------|-------------|-------------|-------------|-------------
>> | ms (%) | ms (%) | ms (%) | ms (%)
>> ---------------|-------------|-------------|-------------|-------------
>> base | 153 (0%) | 2227 (0%) | 8798 (0%) | 17442 (0%)
>> no-cont-remap | 77 (-49%) | 431 (-81%) | 1727 (-80%) | 3796 (-78%)
>> batch-barriers | 13 (-92%) | 162 (-93%) | 655 (-93%) | 1656 (-91%)
>> no-alloc-remap | 11 (-93%) | 109 (-95%) | 449 (-95%) | 1257 (-93%)
>> lazy-unmap | 6 (-96%) | 61 (-97%) | 257 (-97%) | 838 (-95%)
>>
>> This series applies on top of v6.9-rc2. All mm selftests pass. I've compile and
>> boot tested various PAGE_SIZE and VA size configs.
>>
>> ---
>>
>> Changes since v1 [1]
>> ====================
>>
>> - Added Tested-by tags (thanks to Eric and Itaru)
>> - Renamed ___set_pte() -> __set_pte_nosync() (per Ard)
>> - Reordered patches (biggest impact & least controversial first)
>> - Reordered alloc/map/unmap functions in mmu.c to aid reader
>> - pte_clear() -> __pte_clear() in clear_fixmap_nosync()
>> - Reverted generic p4d_index() which caused x86 build error. Replaced with
>> unconditional p4d_index() define under arm64.
>>
>>
>> [1] https://lore.kernel.org/linux-arm-kernel/20240326101448.3453626-1-ryan.roberts@arm.com/
>>
>> Thanks,
>> Ryan
>>
>>
>> Ryan Roberts (4):
>> arm64: mm: Don't remap pgtables per-cont(pte|pmd) block
>> arm64: mm: Batch dsb and isb when populating pgtables
>> arm64: mm: Don't remap pgtables for allocate vs populate
>> arm64: mm: Lazily clear pte table mappings from fixmap
>>
>> arch/arm64/include/asm/fixmap.h | 5 +-
>> arch/arm64/include/asm/mmu.h | 8 +
>> arch/arm64/include/asm/pgtable.h | 13 +-
>> arch/arm64/kernel/cpufeature.c | 10 +-
>> arch/arm64/mm/fixmap.c | 11 +
>> arch/arm64/mm/mmu.c | 377 +++++++++++++++++++++++--------
>> 6 files changed, 319 insertions(+), 105 deletions(-)
>>
>> --
>> 2.25.1
>>
>
> I've build and boot tested the v2 on FVP, base is taken from your
> linux-rr repo. Running run_vmtests.sh on v2 left some gup longterm not oks, would you take a look at it? The mm ksefltests used is from your linux-rr repo too.
Thanks for taking a look at this.
I can't reproduce your issue unfortunately; steps as follows on Apple M2 VM:
Config: arm64 defconfig + the following:
# Squashfs for snaps, xfs for large file folios.
./scripts/config --enable CONFIG_SQUASHFS_LZ4
./scripts/config --enable CONFIG_SQUASHFS_LZO
./scripts/config --enable CONFIG_SQUASHFS_XZ
./scripts/config --enable CONFIG_SQUASHFS_ZSTD
./scripts/config --enable CONFIG_XFS_FS
# For general mm debug.
./scripts/config --enable CONFIG_DEBUG_VM
./scripts/config --enable CONFIG_DEBUG_VM_MAPLE_TREE
./scripts/config --enable CONFIG_DEBUG_VM_RB
./scripts/config --enable CONFIG_DEBUG_VM_PGFLAGS
./scripts/config --enable CONFIG_DEBUG_VM_PGTABLE
./scripts/config --enable CONFIG_PAGE_TABLE_CHECK
# For mm selftests.
./scripts/config --enable CONFIG_USERFAULTFD
./scripts/config --enable CONFIG_TEST_VMALLOC
./scripts/config --enable CONFIG_GUP_TEST
Running on VM with 12G memory, split across 2 (emulated) NUMA nodes (needed by
some mm selftests), with kernel command line to reserve hugetlbs and other
features required by some mm selftests:
"
transparent_hugepage=madvise earlycon root=/dev/vda2 secretmem.enable
hugepagesz=1G hugepages=0:2,1:2 hugepagesz=32M hugepages=0:2,1:2
default_hugepagesz=2M hugepages=0:64,1:64 hugepagesz=64K hugepages=0:2,1:2
"
Ubuntu userspace running off XFS rootfs. Build and run mm selftests from same
git tree.
Although I don't think any of this config should make a difference to gup_longterm.
Looks like your errors are all "ftruncate() failed". I've seen this problem on
our CI system. There it is due to running the tests from NFS file system. What
filesystem are you using? Perhaps you are sharing into the FVP using 9p? That
might also be problematic.
Does this problem reproduce with v6.9-rc2, without my patches? I except it
probably does?
Thanks,
Ryan
>
> Thanks,
> Itaru.
_______________________________________________
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] phy: freescale: imx8m-pcie: fix pcie link-up instability
From: Vinod Koul @ 2024-04-06 7:18 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: linux-phy, linux-imx, linux-kernel, kernel, Richard Zhu,
Lucas Stach, linux-arm-kernel, stable, Marcel Ziswiler,
Fabio Estevam, Heiko Stuebner, Kishon Vijay Abraham I,
Rob Herring, Sascha Hauer, Sergio Paracuellos, Shawn Guo,
Tim Harvey, Yang Li, imx
In-Reply-To: <20240327071946.8869-1-marcel@ziswiler.com>
On 27-03-24, 08:19, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> On the i.MX 8M Mini, the AUX_PLL_REFCLK_SEL has to be left at its reset
> default of AUX_IN (PLL clock).
>
> Background Information:
> In our automated testing setup, we use Delock Mini-PCIe SATA cards [1].
> While this setup has proven very stable overall we noticed upstream on
> the i.MX 8M Mini fails quite regularly (about 50/50) to bring up the
> PCIe link while with NXP's downstream BSP 5.15.71_2.2.2 it always works.
> As that old downstream stuff was quite different, I first also tried
> NXP's latest downstream BSP 6.1.55_2.2.0 which from a PCIe point of view
> is fairly vanilla, however, also there the PCIe link-up was not stable.
> Comparing and debugging I noticed that upstream explicitly configures
> the AUX_PLL_REFCLK_SEL to I_PLL_REFCLK_FROM_SYSPLL while working
> downstream [2] leaving it at reset defaults of AUX_IN (PLL clock).
> Unfortunately, the TRM does not mention any further details about this
> register (both for the i.MX 8M Mini as well as the Plus).
> NXP confirmed their validation codes for the i.MX8MM PCIe doesn't
> configure cmn_reg063 (offset: 0x18C).
> BTW: On the i.MX 8M Plus we have not seen any issues with PCIe with the
> exact same setup which is why I left it unchanged.
This does not apply on phy/fixes, pls rebase
--
~Vinod
_______________________________________________
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 v3 11/25] media: i2c: imx258: Add get_selection for pixel array information
From: Luis Garcia @ 2024-04-06 6:36 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2kHYc6kdiZEAFD@duo.ucw.cz>
On 4/3/24 12:46, Pavel Machek wrote:
> Hi!
>
>> Libcamera requires the cropping information for each mode, so
>> add this information to the driver.
>
>> @@ -116,6 +124,9 @@ struct imx258_mode {
>> u32 link_freq_index;
>> /* Default register values */
>> struct imx258_reg_list reg_list;
>> +
>> + /* Analog crop rectangle. */
>
> No need for "." at the end, as it is not above.
>
Done
>> + struct v4l2_rect crop;
>> };
>
> If the crop is same in all modes, should we have it in common place?
>
> Best regards,
> Pavel
I see the imx219 made a similar change where it was changed from using
that .crop format to just setting it in the set_pad_format function
so we can do something similar here.
_______________________________________________
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 v1 1/1] phy: freescale: imx8m-pcie: fix pcie link-up instability
From: Vinod Koul @ 2024-04-06 6:30 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: linux-phy, linux-imx, Lucas Stach, linux-arm-kernel, kernel,
Richard Zhu, linux-kernel, Marcel Ziswiler, Fabio Estevam,
Heiko Stuebner, Kishon Vijay Abraham I, Marc Kleine-Budde,
Rob Herring, Sascha Hauer, Shawn Guo, Tim Harvey, Yang Li, imx
In-Reply-To: <20240322130646.1016630-2-marcel@ziswiler.com>
On 22-03-24, 14:06, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> Leaving AUX_PLL_REFCLK_SEL at its reset default of AUX_IN (PLL clock)
> proves to be more stable on the i.MX 8M Mini.
This doesn't apply on phy/fixes
Please rebase
--
~Vinod
_______________________________________________
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 v1 1/1] phy: freescale: imx8m-pcie: fix pcie link-up instability
From: Vinod Koul @ 2024-04-06 6:28 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: linux-phy, linux-imx, Lucas Stach, linux-arm-kernel, kernel,
Richard Zhu, linux-kernel, Marcel Ziswiler, Fabio Estevam,
Heiko Stuebner, Kishon Vijay Abraham I, Marc Kleine-Budde,
Rob Herring, Sascha Hauer, Shawn Guo, Tim Harvey, Yang Li, imx
In-Reply-To: <20240322130646.1016630-2-marcel@ziswiler.com>
On 22-03-24, 14:06, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> Leaving AUX_PLL_REFCLK_SEL at its reset default of AUX_IN (PLL clock)
> proves to be more stable on the i.MX 8M Mini.
>
> Fixes: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
>
Please dont keep an empty line b/w Fixes and s-o-b line...
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> index b700f52b7b67..11fcb1867118 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> @@ -110,8 +110,10 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
> /* Source clock from SoC internal PLL */
> writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> - writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> + if (imx8_phy->drvdata->variant != IMX8MM) {
> + writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> + }
> val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> writel(val | ANA_AUX_RX_TERM_GND_EN,
> imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> --
> 2.44.0
>
>
> --
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy
--
~Vinod
_______________________________________________
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 v3 09/25] media: i2c: imx258: Add support for running on 2 CSI data lanes
From: Luis Garcia @ 2024-04-06 5:25 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2jvUDFnY83drlg@duo.ucw.cz>
On 4/3/24 12:45, Pavel Machek wrote:
> Hi!
>
>> +/*
>> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
>> + * To avoid further computation of clock settings, adopt the same per
>> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
>> + */
>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
>> + { 0x0136, 0x13 },
>> + { 0x0137, 0x33 },
>> + { 0x0301, 0x0A },
>> + { 0x0303, 0x02 },
>> + { 0x0305, 0x03 },
>> + { 0x0306, 0x00 },
>> + { 0x0307, 0xC6 },
>> + { 0x0309, 0x0A },
>> + { 0x030B, 0x01 },
>> + { 0x030D, 0x02 },
>> + { 0x030E, 0x00 },
>> + { 0x030F, 0xD8 },
>> + { 0x0310, 0x00 },
>> +
>> + { 0x0114, 0x01 },
>> + { 0x0820, 0x09 },
>> + { 0x0821, 0xa6 },
>> + { 0x0822, 0x66 },
>> + { 0x0823, 0x66 },
>> +};
>> +
>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
>> { 0x0136, 0x13 },
>> { 0x0137, 0x33 },
>> { 0x0301, 0x05 },
>
> I wish we did not have to copy all the magic values like this.
>
> Best regards,
> Pavel
>
no kidding, magic values everywhere.... it makes it annoying
for me to move things around because they all start to look
similar. Down the line we added in more defined names so its
not as bad but still its bad lol.
_______________________________________________
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 v3 05/25] media: i2c: imx258: Add regulator control
From: Luis Garcia @ 2024-04-06 5:23 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, phone-devel
In-Reply-To: <Zg2jgmmqw0nXDYcF@duo.ucw.cz>
On 4/3/24 12:44, Pavel Machek wrote:
> Hi!
>
>> The device tree bindings define the relevant regulators for the
>> sensor, so update the driver to request the regulators and control
>> them at the appropriate times.
>
>> @@ -995,9 +1007,19 @@ static int imx258_power_on(struct device *dev)
>> struct imx258 *imx258 = to_imx258(sd);
>> int ret;
>>
>> + ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
>> + imx258->supplies);
>> + if (ret) {
>
> Will this make it fail for all current users?
>
> Best regards,
> Pavel
>
It shouldn't affect current users as this was added in by dave for
a completely different sensor and it still works on my ppp. Looking
at the dmesg for imx258 it does reference the regulators that the
ppp doesnt have but it still works.
_______________________________________________
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 v3 21/25] drivers: media: i2c: imx258: Use macros
From: Luis Garcia @ 2024-04-06 5:21 UTC (permalink / raw)
To: Tommaso Merciai
Cc: Sakari Ailus, linux-media, dave.stevenson, jacopo.mondi, mchehab,
robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel, Ondrej Jirman
In-Reply-To: <ZhAGi0X2wVOdsrSe@tom-HP-ZBook-Fury-15-G7-Mobile-Workstation>
On 4/5/24 08:11, Tommaso Merciai wrote:
> Hi Luis,
>
> On Fri, Apr 05, 2024 at 04:33:38AM -0600, Luis Garcia wrote:
>> On 4/4/24 00:46, Sakari Ailus wrote:
>>> On Wed, Apr 03, 2024 at 01:17:26PM -0600, Luigi311 wrote:
>>>> On 4/3/24 10:23, Sakari Ailus wrote:
>>>>> Hi Luis,
>>>>>
>>>>> On Wed, Apr 03, 2024 at 09:03:50AM -0600, git@luigi311.com wrote:
>>>>>> From: Luis Garcia <git@luigi311.com>
>>>>>>
>>>>>> Use understandable macros instead of raw values.
>>>>>>
>>>>>> Signed-off-by: Ondrej Jirman <megi@xff.cz>
>>>>>> Signed-off-by: Luis Garcia <git@luigi311.com>
>>>>>> ---
>>>>>> drivers/media/i2c/imx258.c | 434 ++++++++++++++++++-------------------
>>>>>> 1 file changed, 207 insertions(+), 227 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>>>>>> index e2ecf6109516..30352c33f63c 100644
>>>>>> --- a/drivers/media/i2c/imx258.c
>>>>>> +++ b/drivers/media/i2c/imx258.c
>>>>>> @@ -33,8 +33,6 @@
>>>>>> #define IMX258_VTS_30FPS_VGA 0x034c
>>>>>> #define IMX258_VTS_MAX 65525
>>>>>>
>>>>>> -#define IMX258_REG_VTS 0x0340
>>>>>> -
>>>>>> /* HBLANK control - read only */
>>>>>> #define IMX258_PPL_DEFAULT 5352
>>>>>>
>>>>>> @@ -90,6 +88,53 @@
>>>>>> #define IMX258_PIXEL_ARRAY_WIDTH 4208U
>>>>>> #define IMX258_PIXEL_ARRAY_HEIGHT 3120U
>>>>>>
>>>>>> +/* regs */
>>>>>> +#define IMX258_REG_PLL_MULT_DRIV 0x0310
>>>>>> +#define IMX258_REG_IVTPXCK_DIV 0x0301
>>>>>> +#define IMX258_REG_IVTSYCK_DIV 0x0303
>>>>>> +#define IMX258_REG_PREPLLCK_VT_DIV 0x0305
>>>>>> +#define IMX258_REG_IOPPXCK_DIV 0x0309
>>>>>> +#define IMX258_REG_IOPSYCK_DIV 0x030b
>>>>>> +#define IMX258_REG_PREPLLCK_OP_DIV 0x030d
>>>>>> +#define IMX258_REG_PHASE_PIX_OUTEN 0x3030
>>>>>> +#define IMX258_REG_PDPIX_DATA_RATE 0x3032
>>>>>> +#define IMX258_REG_SCALE_MODE 0x0401
>>>>>> +#define IMX258_REG_SCALE_MODE_EXT 0x3038
>>>>>> +#define IMX258_REG_AF_WINDOW_MODE 0x7bcd
>>>>>> +#define IMX258_REG_FRM_LENGTH_CTL 0x0350
>>>>>> +#define IMX258_REG_CSI_LANE_MODE 0x0114
>>>>>> +#define IMX258_REG_X_EVN_INC 0x0381
>>>>>> +#define IMX258_REG_X_ODD_INC 0x0383
>>>>>> +#define IMX258_REG_Y_EVN_INC 0x0385
>>>>>> +#define IMX258_REG_Y_ODD_INC 0x0387
>>>>>> +#define IMX258_REG_BINNING_MODE 0x0900
>>>>>> +#define IMX258_REG_BINNING_TYPE_V 0x0901
>>>>>> +#define IMX258_REG_FORCE_FD_SUM 0x300d
>>>>>> +#define IMX258_REG_DIG_CROP_X_OFFSET 0x0408
>>>>>> +#define IMX258_REG_DIG_CROP_Y_OFFSET 0x040a
>>>>>> +#define IMX258_REG_DIG_CROP_IMAGE_WIDTH 0x040c
>>>>>> +#define IMX258_REG_DIG_CROP_IMAGE_HEIGHT 0x040e
>>>>>> +#define IMX258_REG_SCALE_M 0x0404
>>>>>> +#define IMX258_REG_X_OUT_SIZE 0x034c
>>>>>> +#define IMX258_REG_Y_OUT_SIZE 0x034e
>>>>>> +#define IMX258_REG_X_ADD_STA 0x0344
>>>>>> +#define IMX258_REG_Y_ADD_STA 0x0346
>>>>>> +#define IMX258_REG_X_ADD_END 0x0348
>>>>>> +#define IMX258_REG_Y_ADD_END 0x034a
>>>>>> +#define IMX258_REG_EXCK_FREQ 0x0136
>>>>>> +#define IMX258_REG_CSI_DT_FMT 0x0112
>>>>>> +#define IMX258_REG_LINE_LENGTH_PCK 0x0342
>>>>>> +#define IMX258_REG_SCALE_M_EXT 0x303a
>>>>>> +#define IMX258_REG_FRM_LENGTH_LINES 0x0340
>>>>>> +#define IMX258_REG_FINE_INTEG_TIME 0x0200
>>>>>> +#define IMX258_REG_PLL_IVT_MPY 0x0306
>>>>>> +#define IMX258_REG_PLL_IOP_MPY 0x030e
>>>>>> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H 0x0820
>>>>>> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L 0x0822
>>>>>> +
>>>>>> +#define REG8(a, v) { a, v }
>>>>>> +#define REG16(a, v) { a, ((v) >> 8) & 0xff }, { (a) + 1, (v) & 0xff }
>>>>>
>>>>> The patch is nice but these macros are better replaced by the V4L2 CCI
>>>>> helper that also offers register access functions. Could you add a patch to
>>>>> convert the driver to use it (maybe after this one)?
>>>>>
>>>>
>>>> Ohh perfect, using something else would be great. Ill go ahead and see
>>>> if I can get that working.
>>>
>>> Thanks. It may be easier to just do it in this one actually. Up to you.
>>>
>>
>> I've made the swap but looks like its not playing nice with my ppp,
>> its causing a crash and showing a call trace as soon as it does its
>> first read to check the identity. I went in and dropped the cci_read
>> and left it with the original implementation and I'm getting a very
>> similar crash with cci_write too so it looks like its not liking
>> how I'm implementing it. Looking at the few other drivers that were
>> swapped over to use that, I don't seem to be missing anything. It's
>> a big change so its not really something I can describe what I've
>> changed but I do have the change on my github here
>> https://github.com/luigi311/linux/commit/840593acb20eee87ce361e6929edf51eefbbe737
>
> I checked your commit to switch to cci helper.
> I think you are missing the right cci regmap initialization.
>
> Please take care to use: devm_cci_regmap_init_i2c
>
> /**
> * devm_cci_regmap_init_i2c() - Create regmap to use with cci_*() register
> * access functions
> *
> * @client: i2c_client to create the regmap for
> * @reg_addr_bits: register address width to use (8 or 16)
> *
> * Note the memory for the created regmap is devm() managed, tied to the client.
> *
> * Return: %0 on success or a negative error code on failure.
> */
>
> Check drivers/media/i2c/imx290.c:1530
> Hope this help :)
>
> Note:
> Somewhere into the github commit you are reading 16bit reg and storing
> that into 64bit val. Take care! :)
>
> Thanks & Regards,
> Tommaso
>
>
Ohh my god your right, looks like i missed that when i was replicating
the cci patch for that same driver lol. Looks like that fixed it and
the driver is now loading so i can continue to debug it and make sure
everything is set correct since i had a few that were messed up since
i couldn't test it.
>> if you can provide some guidance, if not I can skip this change
>> all together and we can do a separate attempt at swapping over to it.
>>
_______________________________________________
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 v1 3/4] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Anand Moon @ 2024-04-06 3:40 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Thinh Nguyen, Greg Kroah-Hartman, Krzysztof Kozlowski,
Alim Akhtar, linux-usb, linux-arm-kernel, linux-samsung-soc,
linux-kernel
In-Reply-To: <ed5153e1-f4de-4268-a9d0-e74b779d2587@wanadoo.fr>
Hi Christophe
On Fri, 5 Apr 2024 at 21:42, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 05/04/2024 à 08:10, Anand Moon a écrit :
> > Hi Christophe, Krzysztof,
> >
> > On Mon, 4 Mar 2024 at 17:16, Anand Moon <linux.amoon@gmail.com> wrote:
> >>
> >> Hi Christophe,
> >>
> >> On Sun, 3 Mar 2024 at 00:07, Christophe JAILLET
> >> <christophe.jaillet@wanadoo.fr> wrote:
> >>>
> >>> Le 02/03/2024 à 17:48, Anand Moon a écrit :
> >>>> Hi Christophe,
> >>>>
> >>>> On Sat, 2 Mar 2024 at 21:20, Christophe JAILLET
> >>>> <christophe.jaillet@wanadoo.fr> wrote:
> >>>>>
> >>>>> Le 01/03/2024 à 20:38, Anand Moon a écrit :
> >>>>>> Use devm_regulator_bulk_get_enable() instead of open coded
> >>>>>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
> >>>>>>
> >>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>>>>> ---
> >>>>>> drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
> >>>>>> 1 file changed, 4 insertions(+), 45 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >>>>>> index 5d365ca51771..7c77f3c69825 100644
> >>>>>> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >>>>>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >>>>>> @@ -32,9 +32,6 @@ struct dwc3_exynos {
> >>>>>> struct clk *clks[DWC3_EXYNOS_MAX_CLOCKS];
> >>>>>> int num_clks;
> >>>>>> int suspend_clk_idx;
> >>>>>> -
> >>>>>> - struct regulator *vdd33;
> >>>>>> - struct regulator *vdd10;
> >>>>>> };
> >>>>>>
> >>>>>> static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> struct device_node *node = dev->of_node;
> >>>>>> const struct dwc3_exynos_driverdata *driver_data;
> >>>>>> int i, ret;
> >>>>>> + static const char * const regulators[] = { "vdd33", "vdd10" };
> >>>>>>
> >>>>>> exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
> >>>>>> if (!exynos)
> >>>>>> @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> if (exynos->suspend_clk_idx >= 0)
> >>>>>> clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
> >>>>>>
> >>>>>> - exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> >>>>>> - if (IS_ERR(exynos->vdd33)) {
> >>>>>> - ret = PTR_ERR(exynos->vdd33);
> >>>>>> - goto vdd33_err;
> >>>>>> - }
> >>>>>> - ret = regulator_enable(exynos->vdd33);
> >>>>>> - if (ret) {
> >>>>>> - dev_err(dev, "Failed to enable VDD33 supply\n");
> >>>>>> - goto vdd33_err;
> >>>>>> - }
> >>>>>> -
> >>>>>> - exynos->vdd10 = devm_regulator_get(dev, "vdd10");
> >>>>>> - if (IS_ERR(exynos->vdd10)) {
> >>>>>> - ret = PTR_ERR(exynos->vdd10);
> >>>>>> - goto vdd10_err;
> >>>>>> - }
> >>>>>> - ret = regulator_enable(exynos->vdd10);
> >>>>>> - if (ret) {
> >>>>>> - dev_err(dev, "Failed to enable VDD10 supply\n");
> >>>>>> - goto vdd10_err;
> >>>>>> - }
> >>>>>> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
> >>>>>> + if (ret)
> >>>>>> + return dev_err_probe(dev, ret, "Failed to enable regulators\n");
> >>>>>>
> >>>>>> if (node) {
> >>>>>> ret = of_platform_populate(node, NULL, NULL, dev);
> >>>>>> @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>>>> return 0;
> >>>>>>
> >>>>>> populate_err:
> >>>>>> - regulator_disable(exynos->vdd10);
> >>>>>> -vdd10_err:
> >>>>>> - regulator_disable(exynos->vdd33);
> >>>>>> -vdd33_err:
> >>>>>> for (i = exynos->num_clks - 1; i >= 0; i--)
> >>>>>> clk_disable_unprepare(exynos->clks[i]);
> >>>>>>
> >>>>>> @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
> >>>>>>
> >>>>>> if (exynos->suspend_clk_idx >= 0)
> >>>>>> clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
> >>>>>> -
> >>>>>> - regulator_disable(exynos->vdd33);
> >>>>>> - regulator_disable(exynos->vdd10);
> >>>>>> }
> >>>>>>
> >>>>>> static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
> >>>>>> @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
> >>>>>> for (i = exynos->num_clks - 1; i >= 0; i--)
> >>>>>> clk_disable_unprepare(exynos->clks[i]);
> >>>>>>
> >>>>>> - regulator_disable(exynos->vdd33);
> >>>>>> - regulator_disable(exynos->vdd10);
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> Same here, I don't think that removing regulator_[en|dis]able from the
> >>>>> suspend and resume function is correct.
> >>>>>
> >>>>> The goal is to stop some hardware when the system is suspended, in order
> >>>>> to save some power.
> >>>> Ok,
> >>>>>
> >>>>> Why did you removed it?
> >>>>
> >>>> As per the description of the function devm_regulator_bulk_get_enable
> >>>>
> >>>> * This helper function allows drivers to get several regulator
> >>>> * consumers in one operation with management, the regulators will
> >>>> * automatically be freed when the device is unbound. If any of the
> >>>> * regulators cannot be acquired then any regulators that were
> >>>> * allocated will be freed before returning to the caller.
> >>>
> >>> The code in suspend/resume is not about freeing some resources. It is
> >>> about enabling/disabling some hardware to save some power.
> >>>
> >>> Think to the probe/remove functions as the software in the kernel that
> >>> knows how to handle some hardawre, and the suspend/resume as the on/off
> >>> button to power-on and off the electrical chips.
> >>>
> >>> When the system is suspended, the software is still around. But some
> >>> hardware can be set in a low consumption mode to save some power.
> >>>
> >>> IMHO, part of the code you removed changed this behaviour and increase
> >>> the power consumption when the system is suspended.
> >>>
> >>
> >> You are correct, I have changed the regulator API from
> >> devm_regulator_get_enable to devm_regulator_bulk_get_enable
> >> which changes this behavior.
> >> I will fix it in the next version.
> >>
> >>> CJ
> >
> > I could not find any example in the kernel to support
> > devm_regulator_bulk_disable
> > but here is my modified file.
> >
> > If you have any suggestions for this plz let me know.
>
> I don't think that your approach is correct, and I don't think that the
> proposed patch does what you expect it to do.
>
> Calling a devm_ function in suspend/resume functions looks really
> strange to me and is likely broken.
>
> Especially here, devm_regulator_bulk_get_enable() in the resume function
> allocates some memory that is not freed in
> devm_regulator_bulk_disable(), because the API is not designed to work
> like that. So this could generate a kind of memory leak.
>
>
> *I think that the code is good enough as-is*, but if you really want to
> change something, maybe:
> - devm_regulator_get()+regulator_enable() in the probe could be
> changed to devm_regulator_get_enable()
> - the resume/suspend function should be left as-is with
> regulator_disable()/regulator_ensable()
> - remove regulator_disable() from the error handling path of the
> probe and from the remove function.
>
> I *think* it would work.
>
Thanks OK, I will test this and update in the next version. .
> CJ
Thanks
-Anand
_______________________________________________
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: imx8mp: Align both CSI2 pixel clock
From: Marek Vasut @ 2024-04-06 2:58 UTC (permalink / raw)
To: Adam Ford, Laurent Pinchart
Cc: linux-arm-kernel, Conor Dooley, Fabio Estevam,
Krzysztof Kozlowski, Paul Elder, Pengutronix Kernel Team,
Rob Herring, Sascha Hauer, Shawn Guo, devicetree, imx
In-Reply-To: <CAHCN7xKX7v4tmhjvoPLirEoUG91jpu-8R2DV9eE=mnWt=3FffA@mail.gmail.com>
On 4/5/24 11:04 PM, Adam Ford wrote:
> On Fri, Apr 5, 2024 at 3:43 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Hi Marek,
>>
>> (CC'ing Adam)
>>
>> Thank you for the patch.
>>
>> On Fri, Apr 05, 2024 at 10:22:26PM +0200, Marek Vasut wrote:
>>> Configure both CSI2 assigned-clock-rates the same way.
>>> There does not seem to be any reason for keeping the
>>> two CSI2 pixel clock set to different frequencies.
>>
>> There's an issue when using two cameras concurrently. This has been
>> discussed some time ago on the linux-media mailing list, see [1]. Adam
>> knows more than I do on this topic.
>>
>> [1] https://lore.kernel.org/linux-media/CAHCN7x+kymRGO2kxvN2=zLiqRjfTc3hdf3VdNVkWjsW3La0bnA@mail.gmail.com/
>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Cc: Paul Elder <paul.elder@ideasonboard.com>
>>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: imx@lists.linux.dev
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> ---
>>> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> index 1bb96e96639f2..2e9ce0c3a9815 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> @@ -1703,7 +1703,7 @@ mipi_csi_1: csi@32e50000 {
>>> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
>>> assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
>>> <&clk IMX8MP_CLK_24M>;
>>> - assigned-clock-rates = <266000000>;
>>> + assigned-clock-rates = <500000000>;
>
> I am traveling, so I don't have the technical documents in front of
> me, but I beleive this is an over-drive speed, and 400MHz would be the
> single clock, standard rate. I created an imx8mm-overdrive and
> imx8mn-overdrive dtsi file to let users who operate in overdrive mode
> to update their clocks in one place.
>
> I also think this goes down if the user is running two cameras instead
> of one. I re-read the old thread, and it's coming back to me, but
> until I can get settled into my hotel in Germany, I won't have time to
> review. I think the original idea was to use the lowest, conservative
> value with the idea that people can tweak their clock settings if
> they're only running one and if they are running in over-drive mode.
MX8MPCEC does indeed read 400 MHz regular, 500 MHz overdrive.
Shall we align both CSI2 ports to 400 MHz ? Currently they are one 500
MHz and the other 266 MHz .
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 00/16] power: sequencing: implement the subsystem and add first users
From: Xilin Wu @ 2024-04-06 3:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov
Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
Bartosz Golaszewski
In-Reply-To: <20240325131624.26023-1-brgl@bgdev.pl>
On 2024/3/25 21:16, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org>
>
> Note: I dropped most of the the review and test tags on purpose, the code
> changed significantly and warrants a new round of reviews and tests.
>
> ===
>
> Problem statement #1: Dynamic bus chicken-and-egg problem.
>
> Certain on-board PCI devices need to be powered up before they are can be
> detected but their PCI drivers won't get bound until the device is
> powered-up so enabling the relevant resources in the PCI device driver
> itself is impossible.
>
> Problem statement #2: Sharing inter-dependent resources between devices.
>
> Certain devices that use separate drivers (often on different busses)
> share resources (regulators, clocks, etc.). Typically these resources
> are reference-counted but in some cases there are additional interactions
> between them to consider, for example specific power-up sequence timings.
>
> ===
>
> The reason for tackling both of these problems in a single series is the
> fact the the platform I'm working on - Qualcomm RB5 - deals with both and
> both need to be addressed in order to enable WLAN and Bluetooth support
> upstream.
>
> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that
> takes inputs from the host and exposes LDO outputs consumed by the BT and
> WLAN modules which can be powered-up and down independently. However
> a delay of 100ms must be respected between enabling the BT- and
> WLAN-enable GPIOs.
>
> A similar design with a discreet PMU is also employed in other models of
> the WCN family of chips although we can often do without the delays. With
> this series we add support for the WCN7850 as well.
>
> ===
>
> We introduce a new subsystem here - the power sequencing framework. The
> qcom-wcn driver that we add is its first user. It implements the power-up
> sequences for QCA6390 and WCN7850 chips. However - we only use it to
> power-up the bluetooth module in the former. We use it to driver the WLAN
> modules in both. The reason for this is that for WCN7850 we have
> comprehensive bindings already upstream together with existing DT users.
> Porting them to using the pwrseq subsystem can be done separately and in
> an incremental manner once the subsystem itself is upstream. We will also
> have to ensure backward DT compatibility. To avoid overcomplicating this
> series, let's leave it out for now.
>
> ===
>
> This series is logically split into several sections. I'll go
> patch-by-patch and explain each step.
>
> Patches 1/16-5/16:
>
> These contain all relevant DT bindings changes. We add new documents for
> the QCA6390 & WCN7850 PMUs and ATH12K devices as well as extend the bindings
> for the Qualcomm Bluetooth and ATH11K modules with regulators used by them
> in QCA6390.
>
> Patches 6/16-8/16:
>
> These contain changes to device-tree sources for the three platforms we
> work with in this series. We model the PMUs of the WLAN/BT chips as
> top-level platform devices on the device tree. In order to limit the scope
> of this series and not introduce an excessive amount of confusion with
> deprecating DT bindings, we leave the Bluetooth nodes on sm8650 and sm8550
> as is (meaning: they continue to consumer the GPIOs and power inputs from
> the host). As the WCN7850 module doesn't require any specific timings, we can
> incrementally change that later.
>
> In both cases we add WLAN nodes that consume the power outputs of the PMU.
> For QCA6390 we also make the Bluetooth node of the RB5 consume the outputs
> of the PMU - we can do it as the bindings for this chip did not define any
> supply handles prior to this series meaning we are able to get this correct
> right away.
>
> Patches 9/16-12/16:
>
> These contain the bulk of the PCI changes for this series. We introduce
> a simple framework for powering up PCI devices before detecting them on
> the bus.
>
> The general approach is as follows: PCI devices that need special
> treatment before they can be powered up, scanned and bound to their PCI
> drivers must be described on the device-tree as child nodes of the PCI
> port node. These devices will be instantiated on the platform bus. They
> will in fact be generic platform devices with the compatible of the form
> used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We
> add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl
> drivers. These drivers are platform drivers that will now be matched
> against the devices instantiated from port children just like any other
> platform pairs.
>
> Both the power control platform device *AND* the associated PCI device
> reuse the same OF node and have access to the same properties. The goal
> of the platform driver is to request and bring up any required resources
> and let the pwrctl framework know that it's now OK to rescan the bus and
> detect the devices. When the device is bound, we are notified about it
> by the PCI bus notifier event and can establish a device link between the
> power control device and the PCI device so that any future extension for
> power-management will already be able to work with the correct hierachy.
>
> The reusing of the OF node is the reason for the small changes to the PCI
> OF core: as the bootloader can possibly leave the relevant regulators on
> before booting linux, the PCI device can be detected before its platform
> abstraction is probed. In this case, we find that device first and mark
> its OF node as reused. The pwrctl framework handles the opposite case
> (when the PCI device is detected only after the platform driver
> successfully enabled it).
>
> Patch 13/16 - 14/16:
>
> These add a relatively simple power sequencing subsystem and the first
> driver using it: the pwrseq module for the PMUs on the WCN family of chips.
>
> I'm proposing to add a subsystem that allows different devices to use a shared
> power sequence split into consumer-specific as well as common "units".
>
> A power sequence provider driver registers a set of units with pwrseq
> core. Each unit can be enabled and disabled and contains an optional list
> of other units which must be enabled before it itself can be. A unit
> represents a discreet chunk of the power sequence.
>
> It also registers a list of targets: a target is an abstraction wrapping
> a unit which allows consumers to tell pwrseq which unit they want to
> reach. Real-life example is the driver we're adding here: there's a set
> of common regulators, two PCIe-specific ones and two enable GPIOs: one
> for Bluetooth and one for WLAN.
>
> The Bluetooth driver requests a descriptor to the power sequencer and
> names the target it wants to reach:
>
> pwrseq = devm_pwrseq_get(dev, "bluetooth");
>
> The pwrseq core then knows that when the driver calls:
>
> pwrseq_power_on(pwrseq);
>
> It must enable the "bluetooth-enable" unit but it depends on the
> "regulators-common" unit so this one is enabled first. The provider
> driver is also in charge of assuring an appropriate delay between
> enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are
> handled by the "wlan-enable" unit and so are not enabled until the WLAN
> driver requests the "wlan" target to be powered on.
>
> Another thing worth discussing is the way we associate the consumer with
> the relevant power sequencer. DT maintainers have expressed a discontent
> with the existing mmc pwrseq bindings and have NAKed an earlier
> initiative to introduce global pwrseq bindings to the kernel[1].
>
> In this approach, we model the existing regulators and GPIOs in DT but
> the pwrseq subsystem requires each provider to provide a .match()
> callback. Whenever a consumer requests a power sequencer handle, we
> iterate over the list of pwrseq drivers and call .match() for each. It's
> up to the driver to verify in a platform-specific way whether it deals
> with its consumer and let the core pwrseq code know.
>
> The advantage of this over reusing the regulator or reset subsystem is
> that it's more generalized and can handle resources of all kinds as well
> as deal with any kind of power-on sequences: for instance, Qualcomm has
> a PCI switch they want a driver for but this switch requires enabling
> some resources first (PCI pwrctl) and then configuring the device over
> I2C (which can be handled by the pwrseq provider).
>
> Patch 15:
>
> This patch makes the Qualcomm Bluetooth driver get and use the power
> sequencer for QCA6390.
>
> Patch 16:
>
> While tiny, this patch is possibly the highlight of the entire series.
> It uses the two abstraction layers we introduced before to create an
> elegant power sequencing PCI power control driver and supports the ath11k
> module on QCA6390 and ath12k on WCN7850.
>
> With this series we can now enable BT and WLAN on several new Qualcomm
> boards upstream.
>
> Tested on RB5, sm8650-qrd and sm8550-qrd.
>
> Changelog:
>
> Since v5:
> - unify the approach to modelling the WCN WLAN/BT chips by always exposing
> the PMU node on the device tree and making the WLAN and BT nodes become
> consumers of its power outputs; this includes a major rework of the DT
> sources, bindings and driver code; there's no more a separate PCI
> pwrctl driver for WCN7850, instead its power-up sequence was moved
> into the pwrseq driver common for all WCN chips
> - don't set load_uA from new regulator consumers
> - fix reported kerneldoc issues
> - drop voltage ranges for PMU outputs from DT
> - many minor tweaks and reworks
>
> v1: Original RFC:
>
> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/
>
> v2: First real patch series (should have been PATCH v2) adding what I
> referred to back then as PCI power sequencing:
>
> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/
>
> v3: RFC for the DT representation of the PMU supplying the WLAN and BT
> modules inside the QCA6391 package (was largely separate from the
> series but probably should have been called PATCH or RFC v3):
>
> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/
>
> v4: Second attempt at the full series with changed scope (introduction of
> the pwrseq subsystem, should have been RFC v4)
>
> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/
>
> v5: Two different ways of handling QCA6390 and WCN7850:
>
> https://lore.kernel.org/lkml/20240216203215.40870-1-brgl@bgdev.pl/
>
> Bartosz Golaszewski (16):
> regulator: dt-bindings: describe the PMU module of the QCA6390 package
> regulator: dt-bindings: describe the PMU module of the WCN7850 package
> dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390
> dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on
> QCA6390
> dt-bindings: net: wireless: describe the ath12k PCI module
> arm64: dts: qcom: sm8550-qrd: add the Wifi node
> arm64: dts: qcom: sm8650-qrd: add the Wifi node
> arm64: dts: qcom: qrb5165-rb5: add the Wifi node
> PCI: hold the rescan mutex when scanning for the first time
> PCI/pwrctl: reuse the OF node for power controlled devices
> PCI/pwrctl: create platform devices for child OF nodes of the port
> node
> PCI/pwrctl: add PCI power control core code
> power: sequencing: implement the pwrseq core
> power: pwrseq: add a driver for the PMU module on the QCom WCN
> chipsets
> Bluetooth: qca: use the power sequencer for QCA6390
> PCI/pwrctl: add a PCI power control driver for power sequenced devices
>
> .../net/bluetooth/qualcomm-bluetooth.yaml | 17 +
> .../net/wireless/qcom,ath11k-pci.yaml | 46 +
> .../bindings/net/wireless/qcom,ath12k.yaml | 100 ++
> .../bindings/regulator/qcom,qca6390-pmu.yaml | 185 +++
> MAINTAINERS | 8 +
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 103 +-
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 +
> arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 ++
> arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 +
> arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 ++
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 +
> drivers/bluetooth/hci_qca.c | 74 +-
> drivers/pci/Kconfig | 1 +
> drivers/pci/Makefile | 1 +
> drivers/pci/bus.c | 9 +-
> drivers/pci/of.c | 14 +-
> drivers/pci/probe.c | 2 +
> drivers/pci/pwrctl/Kconfig | 17 +
> drivers/pci/pwrctl/Makefile | 6 +
> drivers/pci/pwrctl/core.c | 136 +++
> drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++
> drivers/pci/remove.c | 2 +
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/sequencing/Kconfig | 28 +
> drivers/power/sequencing/Makefile | 6 +
> drivers/power/sequencing/core.c | 1065 +++++++++++++++++
> drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 ++++++
> include/linux/pci-pwrctl.h | 51 +
> include/linux/pwrseq/consumer.h | 56 +
> include/linux/pwrseq/provider.h | 75 ++
> 31 files changed, 2614 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
> create mode 100644 drivers/pci/pwrctl/Kconfig
> create mode 100644 drivers/pci/pwrctl/Makefile
> create mode 100644 drivers/pci/pwrctl/core.c
> create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
> create mode 100644 drivers/power/sequencing/Kconfig
> create mode 100644 drivers/power/sequencing/Makefile
> create mode 100644 drivers/power/sequencing/core.c
> create mode 100644 drivers/power/sequencing/pwrseq-qcom-wcn.c
> create mode 100644 include/linux/pci-pwrctl.h
> create mode 100644 include/linux/pwrseq/consumer.h
> create mode 100644 include/linux/pwrseq/provider.h
I tested the patchset on SM8550 and it does give me working WiFi. However I
seethe following warnings during boot.
[ 5.973011] mhi mhi0: Requested to power ON
[ 6.597591] mhi mhi0: Power on setup success
[ 6.597631] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0'
[ 6.597634] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1
[ 6.597635] Hardware name: AYN Odin 2 (DT)
[ 6.597637] Workqueue: async async_run_entry_fn
[ 6.597645] Call trace:
[ 6.597646] dump_backtrace+0xa0/0x128
[ 6.597649] show_stack+0x20/0x38
[ 6.597650] dump_stack_lvl+0x74/0x90
[ 6.597653] dump_stack+0x18/0x28
[ 6.597654] sysfs_warn_dup+0x6c/0x90
[ 6.597658] sysfs_add_bin_file_mode_ns+0xdc/0x100
[ 6.597660] sysfs_create_bin_file+0x7c/0xb8
[ 6.597662] pci_create_attr+0xb4/0x1a8
[ 6.597665] pci_create_resource_files+0x64/0xd0
[ 6.597667] pci_create_sysfs_dev_files+0x24/0x40
[ 6.597669] pci_bus_add_device+0x54/0x138
[ 6.597670] pci_bus_add_devices+0x40/0x98
[ 6.597672] pci_host_probe+0x70/0xf0
[ 6.597673] dw_pcie_host_init+0x248/0x658
[ 6.597676] qcom_pcie_probe+0x234/0x330
[ 6.597677] platform_probe+0x70/0xd8
[ 6.597680] really_probe+0xc8/0x3a0
[ 6.597681] __driver_probe_device+0x84/0x170
[ 6.597682] driver_probe_device+0x44/0x120
[ 6.597683] __device_attach_driver+0xc4/0x168
[ 6.597684] bus_for_each_drv+0x8c/0xf0
[ 6.597686] __device_attach_async_helper+0xb4/0x118
[ 6.597687] async_run_entry_fn+0x40/0x178
[ 6.597689] process_one_work+0x16c/0x410
[ 6.597691] worker_thread+0x284/0x3a0
[ 6.597693] kthread+0x118/0x128
[ 6.597693] ret_from_fork+0x10/0x20
[ 6.597698] ------------[ cut here ]------------
[ 6.597698] proc_dir_entry '0000:00/00.0' already registered
[ 6.597710] WARNING: CPU: 7 PID: 154 at fs/proc/generic.c:375 proc_register+0x138/0x1d0
[ 6.597713] Modules linked in:
[ 6.597714] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1
[ 6.597715] Hardware name: AYN Odin 2 (DT)
[ 6.597716] Workqueue: async async_run_entry_fn
[ 6.597718] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 6.597719] pc : proc_register+0x138/0x1d0
[ 6.597721] lr : proc_register+0x138/0x1d0
[ 6.597723] sp : ffff800081e3b9a0
[ 6.597723] x29: ffff800081e3b9a0 x28: 0000000000000000 x27: ffffddb2a28eabe0
[ 6.597725] x26: ffff3425c9ada5c0 x25: ffffddb2a2d4eef0 x24: ffff3425c9ada540
[ 6.597726] x23: 0000000000000004 x22: ffff3425c7b1822c x21: 0000000000000004
[ 6.597727] x20: ffff3425c7b18180 x19: ffff3425c9adaec8 x18: ffffffffffffffff
[ 6.597729] x17: 3040636f732f6d72 x16: 6f6674616c702f73 x15: ffff800081e3b910
[ 6.597730] x14: 0000000000000000 x13: 0a64657265747369 x12: 6765722079646165
[ 6.597731] x11: fffffffffff00000 x10: ffffddb2a27c4fb0 x9 : ffffddb29f5d7528
[ 6.597733] x8 : 00000000ffff7fff x7 : ffffddb2a27c4fb0 x6 : 80000000ffff8000
[ 6.597734] x5 : 0000000000000358 x4 : 0000000000000000 x3 : 00000000ffffffff
[ 6.597736] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff3425c5ce0000
[ 6.597737] Call trace:
[ 6.597737] proc_register+0x138/0x1d0
[ 6.597739] proc_create_data+0x48/0x78
[ 6.597741] pci_proc_attach_device+0x84/0x118
[ 6.597743] pci_bus_add_device+0x5c/0x138
[ 6.597744] pci_bus_add_devices+0x40/0x98
[ 6.597745] pci_host_probe+0x70/0xf0
[ 6.597746] dw_pcie_host_init+0x248/0x658
[ 6.597748] qcom_pcie_probe+0x234/0x330
[ 6.597749] platform_probe+0x70/0xd8
[ 6.597750] really_probe+0xc8/0x3a0
[ 6.597751] __driver_probe_device+0x84/0x170
[ 6.597752] driver_probe_device+0x44/0x120
[ 6.597753] __device_attach_driver+0xc4/0x168
[ 6.597754] bus_for_each_drv+0x8c/0xf0
[ 6.597756] __device_attach_async_helper+0xb4/0x118
[ 6.597757] async_run_entry_fn+0x40/0x178
[ 6.597759] process_one_work+0x16c/0x410
[ 6.597760] worker_thread+0x284/0x3a0
[ 6.597761] kthread+0x118/0x128
[ 6.597762] ret_from_fork+0x10/0x20
[ 6.597763] ---[ end trace 0000000000000000 ]---
This probably only occurs when the relevant drivers on compiled as built-in.
Similar behavior has been noticed before as well:
https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/#mdeeca9bc8e19458787d53738298abcfff443068a
Thanks,
Xilin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESPIN PATCH] irqchip/gic-v4.1:Fix VSYNC referencing an unmapped VPE
From: Nianyao Tang @ 2024-04-06 2:27 UTC (permalink / raw)
To: maz, tglx, yuzenghui, linux-arm-kernel, linux-kernel
Cc: guoyang2, wangwudi, tangnianyao
As per the GICv4.1 spec (Arm IHI 0069H, 5.3.19):
"A VMAPP with {V, Alloc}=={0, x} is self-synchronizing, This means the
ITS command queue does not show the command as consumed until all of
its effects are completed."
Furthermore, VSYNC is allowed to deliver an SError when referencing a
non existent VPE.
By these definitions, a VMAPP followed by a VSYNC is a bug, as the
later references a VPE that has been unmapped by the former.
Fix it by eliding the VSYNC in this scenario.
Fixes: 64edfaa9a234 ("irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP")
Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
---
drivers/irqchip/irq-gic-v3-its.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fca888b36680..2a537cbfcb07 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -786,6 +786,7 @@ static struct its_vpe *its_build_vmapp_cmd(struct its_node *its,
struct its_cmd_block *cmd,
struct its_cmd_desc *desc)
{
+ struct its_vpe *vpe = valid_vpe(its, desc->its_vmapp_cmd.vpe);
unsigned long vpt_addr, vconf_addr;
u64 target;
bool alloc;
@@ -798,6 +799,11 @@ static struct its_vpe *its_build_vmapp_cmd(struct its_node *its,
if (is_v4_1(its)) {
alloc = !atomic_dec_return(&desc->its_vmapp_cmd.vpe->vmapp_count);
its_encode_alloc(cmd, alloc);
+ /*
+ * Unmapping a VPE is self-synchronizing on GICv4.1,
+ * no need to issue a VSYNC.
+ */
+ vpe = NULL;
}
goto out;
@@ -832,7 +838,7 @@ static struct its_vpe *its_build_vmapp_cmd(struct its_node *its,
out:
its_fixup_cmd(cmd);
- return valid_vpe(its, desc->its_vmapp_cmd.vpe);
+ return vpe;
}
static struct its_vpe *its_build_vmapti_cmd(struct its_node *its,
--
2.30.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2 06/18] PCI: endpoint: test: Implement link_down event operation
From: Manivannan Sadhasivam @ 2024-04-06 2:24 UTC (permalink / raw)
To: Niklas Cassel
Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <Zg//LiMg0Wh7LfT8@x1-carbon>
On Fri, Apr 05, 2024 at 03:39:58PM +0200, Niklas Cassel wrote:
> On Wed, Apr 03, 2024 at 01:18:23PM +0530, Manivannan Sadhasivam wrote:
> > On Sat, Mar 30, 2024 at 01:19:16PM +0900, Damien Le Moal wrote:
> > > Implement the link_down event operation to stop the command execution
> > > delayed work when the endpoint controller notifies a link down event.
> > >
> > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> >
> > This patch is already part of another series I posted [1] and under review. So
> > this can be dropped.
> >
> > - Mani
> >
> > [1] https://lore.kernel.org/linux-pci/20240401-pci-epf-rework-v2-9-970dbe90b99d@linaro.org/
>
> Mani, your patch does not use _sync(),
> so I don't think that we can simply drop this patch.
>
Agree, I was planning to update it in my next version anyway.
- Mani
>
> Kind regards,
> Niklas
>
> >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > drivers/pci/endpoint/functions/pci-epf-test.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index ab40c3182677..e6d4e1747c9f 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > @@ -824,9 +824,19 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
> > > return 0;
> > > }
> > >
> > > +static int pci_epf_test_link_down(struct pci_epf *epf)
> > > +{
> > > + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > > +
> > > + cancel_delayed_work_sync(&epf_test->cmd_handler);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> > > .core_init = pci_epf_test_core_init,
> > > .link_up = pci_epf_test_link_up,
> > > + .link_down = pci_epf_test_link_down,
> > > };
> > >
> > > static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > > --
> > > 2.44.0
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESPIN PATCH] irqchip/gic-v3-its:Fix GICv4.1 needless VSYNC after unmap VPE
From: Tangnianyao @ 2024-04-06 1:55 UTC (permalink / raw)
To: Marc Zyngier; +Cc: tglx, linux-arm-kernel, linux-kernel, guoyang2, wangwudi
In-Reply-To: <86cyr6u58r.wl-maz@kernel.org>
On 4/3/2024 18:09, Marc Zyngier wrote:
> Thanks for respinning this.
>
> A few remarks:
>
> The subject line could be improved. Something like:
>
> "irqchip/gic-v4: Don't issue a VSYNC after VMAPP with V=0"
>
> On Wed, 03 Apr 2024 09:35:56 +0100,
> t00849498 <tangnianyao@huawei.com> wrote:
>> From: Nianyao Tang <tangnianyao@huawei.com>
>>
>> Quote from GIC spec 5.3.19, a VMAPP with {V, Alloc}=={0, x}
>> is self-synchronizing, This means the ITS command queue does not
>> show the command as consumed until all of its effects are completed.
> Since this is a direct quote, make it clear that it is so.
>
>> We don't need VSYNC to guarantee unmap finish. And VSYNC after unmap VPE
>> will reach an invalid vpe table entry, which may trigger exception
>> like SError or RAS. Let's fix it.
> This should be much stronger. It's not that we don't need VSYNC. It is
> that VSYNC is actively wrong. I suggest that you rewrite the commit
> message along these lines:
>
> <msg>
> As per the GICv4.1 spec (Arm IHI 0069H, 5.3.19):
>
> "A VMAPP with {V, Alloc}=={0, x} is self-synchronizing. This means the
> ITS command queue does not show the command as consumed until all of
> its effects are completed."
>
> Furthermore, VSYNC is allowed to deliver an SError when referencing a
> non existent VPE.
>
> By these definitions, a VMAPP followed by a VSYNC is a bug, as the
> later references a VPE that has been unmapped by the former.
>
> Fix it by eliding the VSYNC in this scenario.
> </msg>
Thanks for the above comments, I will resend later.
>
>> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
> Please also add:
>
> Fixes: 64edfaa9a234 ("irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP")
>
> With the above fixed:
>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
>
> Thanks,
>
> M.
>
_______________________________________________
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