Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Lorenzo Bianconi @ 2024-04-03 16:20 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.1712160869.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     | 26 +++++++++++++++++--
 1 file changed, 24 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..cf893d4c74cd 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,25 @@ required:
   - reg
   - '#clock-cells'
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: airoha,en7523-scu
+    then:
+      properties:
+        reg:
+          maxItems: 2
+
+  - if:
+      properties:
+        compatible:
+          const: airoha,en7581-scu
+    then:
+      properties:
+        reg:
+          maxItems: 3
+
 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 2/4] arm64: dts: airoha: Add EN7581 clock node
From: Lorenzo Bianconi @ 2024-04-03 16:20 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.1712160869.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..a1daaaef0de0 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>;
 		};
+
+		scu: system-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 3/4] clk: en7523: make pcie clk_ops accessible through of_device_id struct
From: Lorenzo Bianconi @ 2024-04-03 16:20 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.1712160869.git.lorenzo@kernel.org>

Make pcie clk_ops structure accessible through of_device_id data
pointer in order to define multiple clk_ops for each supported SoC.
This is a preliminary patch to introduce EN7581 clock support.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/clk/clk-en7523.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 7cde328495e2..c7def87b74c6 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -145,11 +145,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 +242,9 @@ 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,
-	};
 	struct clk_init_data init = {
 		.name = "pcie",
-		.ops = &pcie_gate_ops,
+		.ops = of_device_get_match_data(dev),
 	};
 	struct en_clk_gate *cg;
 
@@ -264,7 +254,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 +323,17 @@ static int en7523_clk_probe(struct platform_device *pdev)
 	return r;
 }
 
+static const struct clk_ops en7523_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_pcie_ops },
+	{ /* 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 4/4] clk: en7523: add EN7581 support
From: Lorenzo Bianconi @ 2024-04-03 16:20 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.1712160869.git.lorenzo@kernel.org>

Introduce EN7581 clock support to clk-en7523 driver.

Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/clk/clk-en7523.c | 130 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 125 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index c7def87b74c6..51a6c0cc7f58 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;
@@ -207,14 +225,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 */
@@ -262,6 +280,64 @@ 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);
+
+	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_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);
+
+	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 en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
 				   void __iomem *base, void __iomem *np_base)
 {
@@ -291,6 +367,37 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
 	clk_data->num = EN7523_NUM_CLOCKS;
 }
 
+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 int en7523_clk_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
@@ -306,6 +413,12 @@ static int en7523_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(np_base))
 		return PTR_ERR(np_base);
 
+	if (of_device_is_compatible(node, "airoha,en7581-scu")) {
+		r = en7581_clk_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);
@@ -329,8 +442,15 @@ static const struct clk_ops en7523_pcie_ops = {
 	.unprepare = en7523_pci_unprepare,
 };
 
+static const struct clk_ops en7581_pcie_ops = {
+	.is_enabled = en7581_pci_is_enabled,
+	.prepare = en7581_pci_prepare,
+	.unprepare = en7581_pci_unprepare,
+};
+
 static const struct of_device_id of_match_clk_en7523[] = {
 	{ .compatible = "airoha,en7523-scu", .data = &en7523_pcie_ops },
+	{ .compatible = "airoha,en7581-scu", .data = &en7581_pcie_ops },
 	{ /* 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 1/2] PM: EM: Add min/max available performance state limits
From: Lukasz Luba @ 2024-04-03 16:23 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: lukasz.luba, dietmar.eggemann, linux-arm-kernel, sudeep.holla,
	cristian.marussi, linux-samsung-soc, rafael, viresh.kumar,
	quic_sibis
In-Reply-To: <20240403162315.1458337-1-lukasz.luba@arm.com>

On some devices there are HW dependencies for shared frequency and voltage
between devices: CPUs and L3 cache. When the L3 cache frequency is
increased, the affected CPUs might run at higher voltage and frequency.
That higher voltage causes higher CPU power and thus more energy is used
for running the tasks.

Add performance state limits which are applied for the device. This allows
the Energy Model (EM) to better reflect the CPU's currently available
performance states. This information is used by Energy Aware Scheduler
(EAS) during task placement to avoid situation when a simulated energy
cost has error due to using wrong Power Domain (PD) frequency.

The function performs only bare minimum checks (and requires EM as
an argument) to reduce the overhead.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 include/linux/energy_model.h | 22 ++++++++++++++---
 kernel/power/energy_model.c  | 48 ++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index d30d67c2f07cf..feadd0fd6b356 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -55,6 +55,8 @@ struct em_perf_table {
  * struct em_perf_domain - Performance domain
  * @em_table:		Pointer to the runtime modifiable em_perf_table
  * @nr_perf_states:	Number of performance states
+ * @min_ps:		Minimum available performance state index
+ * @max_ps:		Maximum available performance state index
  * @flags:		See "em_perf_domain flags"
  * @cpus:		Cpumask covering the CPUs of the domain. It's here
  *			for performance reasons to avoid potential cache
@@ -70,6 +72,8 @@ struct em_perf_table {
 struct em_perf_domain {
 	struct em_perf_table __rcu *em_table;
 	int nr_perf_states;
+	int min_ps;
+	int max_ps;
 	unsigned long flags;
 	unsigned long cpus[];
 };
@@ -173,6 +177,8 @@ void em_table_free(struct em_perf_table __rcu *table);
 int em_dev_compute_costs(struct device *dev, struct em_perf_state *table,
 			 int nr_states);
 int em_dev_update_chip_binning(struct device *dev);
+int em_update_performance_limits(struct em_perf_domain *pd,
+		unsigned long freq_min_khz, unsigned long freq_max_khz);
 
 /**
  * em_pd_get_efficient_state() - Get an efficient performance state from the EM
@@ -189,12 +195,13 @@ int em_dev_update_chip_binning(struct device *dev);
  */
 static inline int
 em_pd_get_efficient_state(struct em_perf_state *table, int nr_perf_states,
-			  unsigned long max_util, unsigned long pd_flags)
+			  unsigned long max_util, unsigned long pd_flags,
+			  int min_ps, int max_ps)
 {
 	struct em_perf_state *ps;
 	int i;
 
-	for (i = 0; i < nr_perf_states; i++) {
+	for (i = min_ps; i <= max_ps; i++) {
 		ps = &table[i];
 		if (ps->performance >= max_util) {
 			if (pd_flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES &&
@@ -204,7 +211,7 @@ em_pd_get_efficient_state(struct em_perf_state *table, int nr_perf_states,
 		}
 	}
 
-	return nr_perf_states - 1;
+	return max_ps;
 }
 
 /**
@@ -255,7 +262,8 @@ static inline unsigned long em_cpu_energy(struct em_perf_domain *pd,
 	 */
 	em_table = rcu_dereference(pd->em_table);
 	i = em_pd_get_efficient_state(em_table->state, pd->nr_perf_states,
-				      max_util, pd->flags);
+				      max_util, pd->flags, pd->min_ps,
+				      pd->max_ps);
 	ps = &em_table->state[i];
 
 	/*
@@ -392,6 +400,12 @@ static inline int em_dev_update_chip_binning(struct device *dev)
 {
 	return -EINVAL;
 }
+static inline
+int em_update_performance_limits(struct em_perf_domain *pd,
+		unsigned long freq_min_khz, unsigned long freq_max_khz)
+{
+	return -EINVAL;
+}
 #endif
 
 #endif
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 927cc55ba0b3d..1a8b394251cb2 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -628,6 +628,8 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
 		goto unlock;
 
 	dev->em_pd->flags |= flags;
+	dev->em_pd->min_ps = 0;
+	dev->em_pd->max_ps = nr_states - 1;
 
 	em_cpufreq_update_efficiencies(dev, dev->em_pd->em_table->state);
 
@@ -856,3 +858,49 @@ int em_dev_update_chip_binning(struct device *dev)
 	return em_recalc_and_update(dev, pd, em_table);
 }
 EXPORT_SYMBOL_GPL(em_dev_update_chip_binning);
+
+
+/**
+ * em_update_performance_limits() - Update Energy Model with performance
+ *				limits information.
+ * @pd			: Performance Domain with EM that has to be updated.
+ * @freq_min_khz	: New minimum allowed frequency for this device.
+ * @freq_max_khz	: New maximum allowed frequency for this device.
+ *
+ * This function allows to update the EM with information about available
+ * performance levels. It takes the minimum and maximum frequency in kHz
+ * and does internal translation to performance levels.
+ * Returns 0 on success or -EINVAL when failed.
+ */
+int em_update_performance_limits(struct em_perf_domain *pd,
+		unsigned long freq_min_khz, unsigned long freq_max_khz)
+{
+	struct em_perf_state *table;
+	int min_ps = -1;
+	int max_ps = -1;
+	int i;
+
+	if (!pd)
+		return -EINVAL;
+
+	rcu_read_lock();
+	table = em_perf_state_from_pd(pd);
+
+	for (i = 0; i < pd->nr_perf_states; i++) {
+		if (freq_min_khz == table[i].frequency)
+			min_ps = i;
+		if (freq_max_khz == table[i].frequency)
+			max_ps = i;
+	}
+	rcu_read_unlock();
+
+	/* Only update when both are found and sane */
+	if (min_ps < 0 || max_ps < 0 || max_ps < min_ps)
+		return -EINVAL;
+
+	pd->min_ps = min_ps;
+	pd->max_ps = max_ps;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(em_update_performance_limits);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 0/2] Update Energy Model with perfromance limits
From: Lukasz Luba @ 2024-04-03 16:23 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: lukasz.luba, dietmar.eggemann, linux-arm-kernel, sudeep.holla,
	cristian.marussi, linux-samsung-soc, rafael, viresh.kumar,
	quic_sibis

Hi all,

This patch set allows to specify in the EM the range of performance levels that
the device is allowed to operate. It will impact EAS decision, especially for
SoCs where CPUs share the voltage & frequency domain with other CPUs or devices
e.g.
- Mid CPUs + Big CPU
- Little CPU + L3 cache in DSU

The minimum allowed frequency will be taken into account while doing EAS task
placement simulation. When the min frequency is higher for the whole domain
and not driven by the CPUs in that PD utilization, than the energy for
computation in that PD will be higher. This patch helps to reflect that higher
cost.

More explanation can be found in my presentation on OSPM2023 [1].
I have shown experiments with Big CPU running high frequency and increasing
the L3 cache frequency (to reduce the latency), but that impacted Little
CPU which are in the same DVFS domain with L3 cache. It had bad impact for
total energy consumed by small tasks placed on Little CPU. The EAS was not
aware about the min frequency&voltage of the Little CPUs and energy estimation
was wrong.

Depends on:
patch 2/2:
- SCMI cpufreq performance limits notification support (w/ other
   dependency) [2]
patch 1/2:
- EM recent patches for chip binning update - to avoid conflict [3]

Therefore, the patch 1/2 could go first and patch 2/2 can wait longer.

Regards,
Lukasz Luba

[1] https://www.youtube.com/watch?v=2C-5uikSbtM&list=PL0fKordpLTjKsBOUcZqnzlHShri4YBL1H
[2] https://lore.kernel.org/lkml/20240328074131.2839871-1-quic_sibis@quicinc.com/
[3] https://lore.kernel.org/lkml/20240403154907.1420245-1-lukasz.luba@arm.com/

Lukasz Luba (2):
  PM: EM: Add min/max available performance state limits
  cpufreq: scmi: Update Energy Model with allowed performance limits

 drivers/cpufreq/scmi-cpufreq.c | 19 +++++++++++---
 include/linux/energy_model.h   | 22 +++++++++++++---
 kernel/power/energy_model.c    | 48 ++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 7 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/2] cpufreq: scmi: Update Energy Model with allowed performance limits
From: Lukasz Luba @ 2024-04-03 16:23 UTC (permalink / raw)
  To: linux-kernel, linux-pm
  Cc: lukasz.luba, dietmar.eggemann, linux-arm-kernel, sudeep.holla,
	cristian.marussi, linux-samsung-soc, rafael, viresh.kumar,
	quic_sibis
In-Reply-To: <20240403162315.1458337-1-lukasz.luba@arm.com>

The Energy Model (EM) supports performance limits updates. Use the SCMI
notifications to get information from FW about allowed frequency scope for
the CPUs.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/cpufreq/scmi-cpufreq.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
index d946b7a082584..90c8448578cb1 100644
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -185,12 +185,25 @@ static int scmi_limit_notify_cb(struct notifier_block *nb, unsigned long event,
 {
 	struct scmi_data *priv = container_of(nb, struct scmi_data, limit_notify_nb);
 	struct scmi_perf_limits_report *limit_notify = data;
+	unsigned int limit_freq_max_khz, limit_freq_min_khz;
 	struct cpufreq_policy *policy = priv->policy;
-	unsigned int limit_freq_khz;
+	struct em_perf_domain *pd;
+	int ret;
+
+	limit_freq_max_khz = limit_notify->range_max_freq / HZ_PER_KHZ;
+	limit_freq_min_khz = limit_notify->range_min_freq / HZ_PER_KHZ;
 
-	limit_freq_khz = limit_notify->range_max_freq / HZ_PER_KHZ;
+	pd = em_cpu_get(policy->cpu);
+	if (pd) {
+		ret = em_update_performance_limits(pd, limit_freq_min_khz,
+						   limit_freq_max_khz);
+		if (ret)
+			dev_warn(priv->cpu_dev,
+				 "EM perf limits update failed\n");
+	}
 
-	policy->max = clamp(limit_freq_khz, policy->cpuinfo.min_freq, policy->cpuinfo.max_freq);
+	policy->max = clamp(limit_freq_max_khz, policy->cpuinfo.min_freq,
+			    policy->cpuinfo.max_freq);
 
 	cpufreq_update_pressure(policy);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v3 21/25] drivers: media: i2c: imx258: Use macros
From: Sakari Ailus @ 2024-04-03 16:23 UTC (permalink / raw)
  To: git
  Cc: 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: <20240403150355.189229-22-git@luigi311.com>

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)?

> +
>  struct imx258_reg {
>  	u16 address;
>  	u8 val;
> @@ -145,179 +190,139 @@ struct imx258_mode {
>   * 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 },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 10),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x00C6),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 1),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x09A6),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x6666),
>  };
>  
>  static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
> -	{ 0x0136, 0x13 },
> -	{ 0x0137, 0x33 },
> -	{ 0x0301, 0x05 },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x03 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0xC6 },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x03 },
> -	{ 0x0820, 0x13 },
> -	{ 0x0821, 0x4C },
> -	{ 0x0822, 0xCC },
> -	{ 0x0823, 0xCC },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 5),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x00C6),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 3),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x134C),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0xCCCC),
>  };
>  
>  static const struct imx258_reg mipi_1272mbps_24mhz_2l[] = {
> -	{ 0x0136, 0x18 },
> -	{ 0x0137, 0x00 },
> -	{ 0x0301, 0x0a },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x04 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0xD4 },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x01 },
> -	{ 0x0820, 0x13 },
> -	{ 0x0821, 0x4C },
> -	{ 0x0822, 0xCC },
> -	{ 0x0823, 0xCC },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 10),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x00D4),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 1),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x134C),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0xCCCC),
>  };
>  
>  static const struct imx258_reg mipi_1272mbps_24mhz_4l[] = {
> -	{ 0x0136, 0x18 },
> -	{ 0x0137, 0x00 },
> -	{ 0x0301, 0x05 },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x04 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0xD4 },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x03 },
> -	{ 0x0820, 0x13 },
> -	{ 0x0821, 0xE0 },
> -	{ 0x0822, 0x00 },
> -	{ 0x0823, 0x00 },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 5),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x00D4),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 3),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x13E0),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
>  };
>  
>  static const struct imx258_reg mipi_640mbps_19_2mhz_2l[] = {
> -	{ 0x0136, 0x13 },
> -	{ 0x0137, 0x33 },
> -	{ 0x0301, 0x05 },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x03 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0x64 },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x01 },
> -	{ 0x0820, 0x05 },
> -	{ 0x0821, 0x00 },
> -	{ 0x0822, 0x00 },
> -	{ 0x0823, 0x00 },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 5),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x0064),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 1),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0500),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
>  };
>  
>  static const struct imx258_reg mipi_640mbps_19_2mhz_4l[] = {
> -	{ 0x0136, 0x13 },
> -	{ 0x0137, 0x33 },
> -	{ 0x0301, 0x05 },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x03 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0x64 },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x03 },
> -	{ 0x0820, 0x0A },
> -	{ 0x0821, 0x00 },
> -	{ 0x0822, 0x00 },
> -	{ 0x0823, 0x00 },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 5),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x0064),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 3),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
>  };
>  
>  static const struct imx258_reg mipi_642mbps_24mhz_2l[] = {
> -	{ 0x0136, 0x18 },
> -	{ 0x0137, 0x00 },
> -	{ 0x0301, 0x05 },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x04 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0x6B },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x01 },
> -	{ 0x0820, 0x0A },
> -	{ 0x0821, 0x00 },
> -	{ 0x0822, 0x00 },
> -	{ 0x0823, 0x00 },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 5),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x006B),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 1),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
>  };
>  
>  static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
> -	{ 0x0136, 0x18 },
> -	{ 0x0137, 0x00 },
> -	{ 0x0301, 0x05 },
> -	{ 0x0303, 0x02 },
> -	{ 0x0305, 0x04 },
> -	{ 0x0306, 0x00 },
> -	{ 0x0307, 0x6B },
> -	{ 0x0309, 0x0A },
> -	{ 0x030B, 0x01 },
> -	{ 0x030D, 0x02 },
> -	{ 0x030E, 0x00 },
> -	{ 0x030F, 0xD8 },
> -	{ 0x0310, 0x00 },
> -
> -	{ 0x0114, 0x03 },
> -	{ 0x0820, 0x0A },
> -	{ 0x0821, 0x00 },
> -	{ 0x0822, 0x00 },
> -	{ 0x0823, 0x00 },
> +	REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> +	REG8(IMX258_REG_IVTPXCK_DIV, 5),
> +	REG8(IMX258_REG_IVTSYCK_DIV, 2),
> +	REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> +	REG16(IMX258_REG_PLL_IVT_MPY, 0x006B),
> +	REG8(IMX258_REG_IOPPXCK_DIV, 10),
> +	REG8(IMX258_REG_IOPSYCK_DIV, 1),
> +	REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> +	REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> +	REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> +	REG8(IMX258_REG_CSI_LANE_MODE, 3),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
> +	REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
>  };
>  
>  static const struct imx258_reg mode_common_regs[] = {
> @@ -363,45 +368,29 @@ static const struct imx258_reg mode_common_regs[] = {
>  	{ 0x7423, 0xD7 },
>  	{ 0x5F04, 0x00 },
>  	{ 0x5F05, 0xED },
> -	{ 0x0112, 0x0A },
> -	{ 0x0113, 0x0A },
> -	{ 0x0342, 0x14 },
> -	{ 0x0343, 0xE8 },
> -	{ 0x0344, 0x00 },
> -	{ 0x0345, 0x00 },
> -	{ 0x0346, 0x00 },
> -	{ 0x0347, 0x00 },
> -	{ 0x0348, 0x10 },
> -	{ 0x0349, 0x6F },
> -	{ 0x034A, 0x0C },
> -	{ 0x034B, 0x2F },
> -	{ 0x0381, 0x01 },
> -	{ 0x0383, 0x01 },
> -	{ 0x0385, 0x01 },
> -	{ 0x0387, 0x01 },
> -	{ 0x0404, 0x00 },
> -	{ 0x0408, 0x00 },
> -	{ 0x0409, 0x00 },
> -	{ 0x040A, 0x00 },
> -	{ 0x040B, 0x00 },
> -	{ 0x040C, 0x10 },
> -	{ 0x040D, 0x70 },
> -	{ 0x3038, 0x00 },
> -	{ 0x303A, 0x00 },
> -	{ 0x303B, 0x10 },
> -	{ 0x300D, 0x00 },
> -	{ 0x0350, 0x00 },
> -	{ 0x0204, 0x00 },
> -	{ 0x0205, 0x00 },
> -	{ 0x020E, 0x01 },
> -	{ 0x020F, 0x00 },
> -	{ 0x0210, 0x01 },
> -	{ 0x0211, 0x00 },
> -	{ 0x0212, 0x01 },
> -	{ 0x0213, 0x00 },
> -	{ 0x0214, 0x01 },
> -	{ 0x0215, 0x00 },
> -	{ 0x7BCD, 0x00 },
> +	REG16(IMX258_REG_CSI_DT_FMT, 0x0a0a),
> +	REG16(IMX258_REG_LINE_LENGTH_PCK, 5352),
> +	REG16(IMX258_REG_X_ADD_STA, 0),
> +	REG16(IMX258_REG_Y_ADD_STA, 0),
> +	REG16(IMX258_REG_X_ADD_END, 4207),
> +	REG16(IMX258_REG_Y_ADD_END, 3119),
> +	REG8(IMX258_REG_X_EVN_INC, 1),
> +	REG8(IMX258_REG_X_ODD_INC, 1),
> +	REG8(IMX258_REG_Y_EVN_INC, 1),
> +	REG8(IMX258_REG_Y_ODD_INC, 1),
> +	REG16(IMX258_REG_DIG_CROP_X_OFFSET, 0),
> +	REG16(IMX258_REG_DIG_CROP_Y_OFFSET, 0),
> +	REG16(IMX258_REG_DIG_CROP_IMAGE_WIDTH, 4208),
> +	REG8(IMX258_REG_SCALE_MODE_EXT, 0),
> +	REG16(IMX258_REG_SCALE_M_EXT, 16),
> +	REG8(IMX258_REG_FORCE_FD_SUM, 0),
> +	REG8(IMX258_REG_FRM_LENGTH_CTL, 0),
> +	REG16(IMX258_REG_ANALOG_GAIN, 0),
> +	REG16(IMX258_REG_GR_DIGITAL_GAIN, 256),
> +	REG16(IMX258_REG_R_DIGITAL_GAIN, 256),
> +	REG16(IMX258_REG_B_DIGITAL_GAIN, 256),
> +	REG16(IMX258_REG_GB_DIGITAL_GAIN, 256),
> +	REG8(IMX258_REG_AF_WINDOW_MODE, 0),
>  	{ 0x94DC, 0x20 },
>  	{ 0x94DD, 0x20 },
>  	{ 0x94DE, 0x20 },
> @@ -414,48 +403,39 @@ static const struct imx258_reg mode_common_regs[] = {
>  	{ 0x941B, 0x50 },
>  	{ 0x9519, 0x50 },
>  	{ 0x951B, 0x50 },
> -	{ 0x3030, 0x00 },
> -	{ 0x3032, 0x00 },
> -	{ 0x0220, 0x00 },
> +	REG8(IMX258_REG_PHASE_PIX_OUTEN, 0),
> +	REG8(IMX258_REG_PDPIX_DATA_RATE, 0),
> +	REG8(IMX258_REG_HDR, 0),
>  };
>  
>  static const struct imx258_reg mode_4208x3120_regs[] = {
> -	{ 0x0900, 0x00 },
> -	{ 0x0901, 0x11 },
> -	{ 0x0401, 0x00 },
> -	{ 0x0405, 0x10 },
> -	{ 0x040E, 0x0C },
> -	{ 0x040F, 0x30 },
> -	{ 0x034C, 0x10 },
> -	{ 0x034D, 0x70 },
> -	{ 0x034E, 0x0C },
> -	{ 0x034F, 0x30 },
> +	REG8(IMX258_REG_BINNING_MODE, 0),
> +	REG8(IMX258_REG_BINNING_TYPE_V, 0x11),
> +	REG8(IMX258_REG_SCALE_MODE, 0),
> +	REG16(IMX258_REG_SCALE_M, 16),
> +	REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 3120),
> +	REG16(IMX258_REG_X_OUT_SIZE, 4208),
> +	REG16(IMX258_REG_Y_OUT_SIZE, 3120),
>  };
>  
>  static const struct imx258_reg mode_2104_1560_regs[] = {
> -	{ 0x0900, 0x01 },
> -	{ 0x0901, 0x12 },
> -	{ 0x0401, 0x01 },
> -	{ 0x0405, 0x20 },
> -	{ 0x040E, 0x06 },
> -	{ 0x040F, 0x18 },
> -	{ 0x034C, 0x08 },
> -	{ 0x034D, 0x38 },
> -	{ 0x034E, 0x06 },
> -	{ 0x034F, 0x18 },
> +	REG8(IMX258_REG_BINNING_MODE, 1),
> +	REG8(IMX258_REG_BINNING_TYPE_V, 0x12),
> +	REG8(IMX258_REG_SCALE_MODE, 1),
> +	REG16(IMX258_REG_SCALE_M, 32),
> +	REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 1560),
> +	REG16(IMX258_REG_X_OUT_SIZE, 2104),
> +	REG16(IMX258_REG_Y_OUT_SIZE, 1560),
>  };
>  
>  static const struct imx258_reg mode_1048_780_regs[] = {
> -	{ 0x0900, 0x01 },
> -	{ 0x0901, 0x14 },
> -	{ 0x0401, 0x01 },
> -	{ 0x0405, 0x40 },
> -	{ 0x040E, 0x03 },
> -	{ 0x040F, 0x0C },
> -	{ 0x034C, 0x04 },
> -	{ 0x034D, 0x18 },
> -	{ 0x034E, 0x03 },
> -	{ 0x034F, 0x0C },
> +	REG8(IMX258_REG_BINNING_MODE, 1),
> +	REG8(IMX258_REG_BINNING_TYPE_V, 0x14),
> +	REG8(IMX258_REG_SCALE_MODE, 1),
> +	REG16(IMX258_REG_SCALE_M, 64),
> +	REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 780),
> +	REG16(IMX258_REG_X_OUT_SIZE, 1048),
> +	REG16(IMX258_REG_Y_OUT_SIZE, 780),
>  };
>  
>  struct imx258_variant_cfg {
> @@ -923,7 +903,7 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
>  		}
>  		break;
>  	case V4L2_CID_VBLANK:
> -		ret = imx258_write_reg(imx258, IMX258_REG_VTS,
> +		ret = imx258_write_reg(imx258, IMX258_REG_FRM_LENGTH_LINES,
>  				       IMX258_REG_VALUE_16BIT,
>  				       imx258->cur_mode->height + ctrl->val);
>  		break;

-- 
Kind regards,

Sakari Ailus

_______________________________________________
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 23/25] drivers: media: i2c: imx258: Add support for powerdown gpio
From: Sakari Ailus @ 2024-04-03 16:25 UTC (permalink / raw)
  To: git
  Cc: 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: <20240403150355.189229-24-git@luigi311.com>

Hi Luis, Ondrej,

On Wed, Apr 03, 2024 at 09:03:52AM -0600, git@luigi311.com wrote:
> From: Luis Garcia <git@luigi311.com>
> 
> On some boards powerdown signal needs to be deasserted for this
> sensor to be enabled.
> 
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
>  drivers/media/i2c/imx258.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 30352c33f63c..163f04f6f954 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -679,6 +679,8 @@ struct imx258 {
>  	unsigned int lane_mode_idx;
>  	unsigned int csi2_flags;
>  
> +	struct gpio_desc *powerdown_gpio;
> +
>  	/*
>  	 * Mutex for serialized access:
>  	 * Protect sensor module set pad format and start/stop streaming safely.
> @@ -1213,6 +1215,8 @@ static int imx258_power_on(struct device *dev)
>  	struct imx258 *imx258 = to_imx258(sd);
>  	int ret;
>  
> +	gpiod_set_value_cansleep(imx258->powerdown_gpio, 0);

What does the spec say? Should this really happen before switching on the
supplies below?

> +
>  	ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
>  				    imx258->supplies);
>  	if (ret) {
> @@ -1224,6 +1228,7 @@ static int imx258_power_on(struct device *dev)
>  	ret = clk_prepare_enable(imx258->clk);
>  	if (ret) {
>  		dev_err(dev, "failed to enable clock\n");
> +		gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>  		regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>  	}
>  
> @@ -1238,6 +1243,8 @@ static int imx258_power_off(struct device *dev)
>  	clk_disable_unprepare(imx258->clk);
>  	regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>  
> +	gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
> +
>  	return 0;
>  }
>  
> @@ -1541,6 +1548,12 @@ static int imx258_probe(struct i2c_client *client)
>  	if (!imx258->variant_cfg)
>  		imx258->variant_cfg = &imx258_cfg;
>  
> +	/* request optional power down pin */
> +	imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
> +						    GPIOD_OUT_HIGH);
> +	if (IS_ERR(imx258->powerdown_gpio))
> +		return PTR_ERR(imx258->powerdown_gpio);
> +
>  	/* Initialize subdev */
>  	v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>  

-- 
Regards,

Sakari Ailus

_______________________________________________
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 v14 01/18] irqchip/sifive-plic: Convert PLIC driver into a platform driver
From: Samuel Holland @ 2024-04-03 16:28 UTC (permalink / raw)
  To: Lad, Prabhakar, Anup Patel
  Cc: devicetree, Conor Dooley, Geert Uytterhoeven, Marc Zyngier,
	Anup Patel, Atish Patra, linux-kernel, Saravana Kannan,
	Björn Töpel, Rob Herring, Palmer Dabbelt,
	Krzysztof Kozlowski, Paul Walmsley, Thomas Gleixner, Frank Rowand,
	linux-riscv, linux-arm-kernel, Andrew Jones
In-Reply-To: <CA+V-a8ser=hDmst6+XSeOWaEoOd+iY3Ys6bYBWDa5UYPfT+Pug@mail.gmail.com>

Hi Prabhakar,

On 2024-04-03 10:49 AM, Lad, Prabhakar wrote:
> On Wed, Apr 3, 2024 at 3:17 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> On Wed, Apr 3, 2024 at 2:01 PM Lad, Prabhakar
>> <prabhakar.csengg@gmail.com> wrote:
>>>
>>> Hi Anup,
>>>
>>> On Thu, Feb 22, 2024 at 9:41 AM Anup Patel <apatel@ventanamicro.com> wrote:
>>>>
>>>> The PLIC driver does not require very early initialization so convert
>>>> it into a platform driver.
>>>>
>>>> After conversion, the PLIC driver is probed after CPUs are brought-up
>>>> so setup cpuhp state after context handler of all online CPUs are
>>>> initialized otherwise PLIC driver crashes for platforms with multiple
>>>> PLIC instances.
>>>>
>>>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>>> ---
>>>>  drivers/irqchip/irq-sifive-plic.c | 101 ++++++++++++++++++------------
>>>>  1 file changed, 61 insertions(+), 40 deletions(-)
>>>>
>>> This patch seems to have broken things on RZ/Five SoC, after reverting
>>> this patch I get to boot it back again on v6.9-rc2. Looks like there
>>> is some probe order issue after switching to platform driver?
>>
>> Yes, this is most likely related to probe ordering based on your DT.
>>
>> Can you share the failing boot log and DT ?
> 
> non working case, https://paste.debian.net/1312947/

Looks like you need to add "keep_bootcon" to your kernel command line to get a
full log here.

> after reverting, https://paste.debian.net/1312948/
> (attached is the DTB)

I don't see anything suspicious between the "riscv-intc" lines and the "Fixed
dependency cycle(s)" lines that looks like it would depend on the PLIC IRQ
domain. Maybe there is some driver that does not handle -EPROBE_DEFER? It's hard
to tell without the full log from the failure case.

Regards,
Samuel


_______________________________________________
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 24/25] drivers: media: i2c: imx258: Add support for reset gpio
From: Sakari Ailus @ 2024-04-03 16:28 UTC (permalink / raw)
  To: git
  Cc: 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: <20240403150355.189229-25-git@luigi311.com>

Hi Luis,

Could you unify the subject prefix for the driver patches, please? E.g.
"media: imx258: " would be fine.

On Wed, Apr 03, 2024 at 09:03:53AM -0600, git@luigi311.com wrote:
> From: Luis Garcia <git@luigi311.com>
> 
> It was documented in DT, but not implemented.
> 
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
>  drivers/media/i2c/imx258.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 163f04f6f954..4c117c4829f1 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -680,6 +680,7 @@ struct imx258 {
>  	unsigned int csi2_flags;
>  
>  	struct gpio_desc *powerdown_gpio;
> +	struct gpio_desc *reset_gpio;
>  
>  	/*
>  	 * Mutex for serialized access:
> @@ -1232,7 +1233,11 @@ static int imx258_power_on(struct device *dev)
>  		regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>  	}
>  
> -	return ret;
> +	gpiod_set_value_cansleep(imx258->reset_gpio, 0);
> +
> +	usleep_range(400, 500);

You could mention this at least in the commit message.

> +
> +	return 0;
>  }
>  
>  static int imx258_power_off(struct device *dev)
> @@ -1243,6 +1248,7 @@ static int imx258_power_off(struct device *dev)
>  	clk_disable_unprepare(imx258->clk);
>  	regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>  
> +	gpiod_set_value_cansleep(imx258->reset_gpio, 1);

Same question than on the other GPIO: does this belong here?

>  	gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>  
>  	return 0;
> @@ -1554,6 +1560,12 @@ static int imx258_probe(struct i2c_client *client)
>  	if (IS_ERR(imx258->powerdown_gpio))
>  		return PTR_ERR(imx258->powerdown_gpio);
>  
> +	/* request optional reset pin */
> +	imx258->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> +						    GPIOD_OUT_HIGH);
> +	if (IS_ERR(imx258->reset_gpio))
> +		return PTR_ERR(imx258->reset_gpio);
> +
>  	/* Initialize subdev */
>  	v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>  

-- 
Regards,

Sakari Ailus

_______________________________________________
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 01/10] cpumask: add cpumask_any_and_but()
From: Yury Norov @ 2024-04-03 16:31 UTC (permalink / raw)
  To: Dawei Li
  Cc: will, mark.rutland, linux, xueshuai, renyu.zj, yangyicong,
	jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
	linux-kernel, linux-arm-msm, Thomas Gleixner, Andrew Morton,
	Peter Zijlstra, Rusty Russell
In-Reply-To: <20240403155950.2068109-2-dawei.li@shingroup.cn>

On Wed, Apr 03, 2024 at 11:59:41PM +0800, Dawei Li wrote:
> From: Mark Rutland <mark.rutland@arm.com>
> 
> In some cases, it's useful to be able to select a random cpu from the
> intersection of two masks, excluding a particular CPU.
> 
> For example, in some systems an uncore PMU is shared by a subset of
> CPUs, and management of this PMU is assigned to some arbitrary CPU in
> this set. Whenever the management CPU is hotplugged out, we wish to
> migrate responsibility to another arbitrary CPU which is both in this
> set and online.
> 
> Today we can use cpumask_any_and() to select an arbitrary CPU in the
> intersection of two masks. We can also use cpumask_any_but() to select
> any arbitrary cpu in a mask excluding, a particular CPU.
> 
> To do both, we either need to use a temporary cpumask, which is
> wasteful, or use some lower-level cpumask helpers, which can be unclear.
> 
> This patch adds a new cpumask_any_and_but() to cater for these cases.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>

Thank you,

Acked-by: Yury Norov <yury.norov@gmail.com>

_______________________________________________
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 17/25] dt-bindings: media: imx258: Rename to include vendor prefix
From: git @ 2024-04-03 15:03 UTC (permalink / raw)
  To: linux-media
  Cc: 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, pavel, phone-devel, Conor Dooley, Luis Garcia
In-Reply-To: <20240403150355.189229-1-git@luigi311.com>

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

imx258.yaml doesn't include the vendor prefix of sony, so
rename to add it.
Update the id entry and MAINTAINERS to match.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Luis Garcia <git@luigi311.com>
---
 .../bindings/media/i2c/{imx258.yaml => sony,imx258.yaml}        | 2 +-
 MAINTAINERS                                                     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (97%)

diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
similarity index 97%
rename from Documentation/devicetree/bindings/media/i2c/imx258.yaml
rename to Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
index 80d24220baa0..bee61a443b23 100644
--- a/Documentation/devicetree/bindings/media/i2c/imx258.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
+$id: http://devicetree.org/schemas/media/i2c/sony,imx258.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
diff --git a/MAINTAINERS b/MAINTAINERS
index 8ceb49f1b630..a8c3a531ad39 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20489,7 +20489,7 @@ M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
-F:	Documentation/devicetree/bindings/media/i2c/imx258.yaml
+F:	Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
 F:	drivers/media/i2c/imx258.c
 
 SONY IMX274 SENSOR DRIVER
-- 
2.42.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 00/10] perf: Avoid placing cpumask var on stack
From: Yury Norov @ 2024-04-03 16:39 UTC (permalink / raw)
  To: Dawei Li
  Cc: Mark Rutland, will, linux, xueshuai, renyu.zj, yangyicong,
	jonathan.cameron, andersson, konrad.dybcio, linux-arm-kernel,
	linux-kernel, linux-arm-msm
In-Reply-To: <5BCB924A8FA6320A+Zg1/xw9C493rZ868@centos8>

On Thu, Apr 04, 2024 at 12:11:51AM +0800, Dawei Li wrote:
> Hi Mark,
> 
> On Wed, Apr 03, 2024 at 03:41:07PM +0100, Mark Rutland wrote:
> > On Wed, Apr 03, 2024 at 08:50:59PM +0800, Dawei Li wrote:
> > > Hi all,
> > 
> > Hi,
> > 
> > > This is v2 of [1] and [2] which basically eliminate cpumask var allocation
> > > on stack for perf subsystem.
> > > 
> > > Change since v1:
> > > - Change from dynamic allocation to a temporary var free helper:
> > >   cpumask_any_and_but().	[Mark]
> > > 
> > > - Some minor coding style improvements, reverse chrismas tree e.g.
> > > 
> > > - For cpumask_any_and_but() itself:
> > >   - Moved to cpumask.h, just like other helpers.
> > >   - Return value converted to unsigned int.
> > >   - Remove EXPORT_SYMBOL, for obvious reason.
> > 
> > Thanks for this!
> > 
> > The logic all looks good; if you can spin a v3 with the updated commit messages
> > I reckon we can queue this up shortly.
> 
> Thanks for review.
> 
> v3 respinned:
> https://lore.kernel.org/lkml/20240403155950.2068109-1-dawei.li@shingroup.cn/
> 
> If it's going through perf tree, do we need Acked-by from bitmap
> maintainers for patch[1]?

There's only one bitmap-related patch, so I agree - the series should
go through Mark's tree. I acked 1st patch in v3. Please go ahead.

Thanks,
Yury

_______________________________________________
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 2/4] kprobes/treewide: Add kprobes_ prefix to insn alloc/free functions
From: Jarkko Sakkinen @ 2024-04-03 16:39 UTC (permalink / raw)
  To: Mark Rutland, linux-kernel
  Cc: anil.s.keshavamurthy, aou, catalin.marinas, davem,
	linux-arm-kernel, mhiramat, naveen.n.rao, palmer, paul.walmsley,
	will
In-Reply-To: <20240403150154.667649-3-mark.rutland@arm.com>

On Wed Apr 3, 2024 at 6:01 PM EEST, Mark Rutland wrote:
> The alloc_(opt)insn_page() and free_(opt)insn_page() functions are
> specific to KPROBES, but their name makes them sound more generic than
> they are.
>
> Given them a 'kprobes_' prefix to make it clear that they're part of
> kprobes.
>
> This was generated automatically with:
>
>   sed -i 's/alloc_insn_page/kprobes_alloc_insn_page/' $(git grep -l 'alloc_insn_page')
>   sed -i 's/free_insn_page/kprobes_free_insn_page/' $(git grep -l 'free_insn_page')
>   sed -i 's/alloc_optinsn_page/kprobes_alloc_optinsn_page/' $(git grep -l 'alloc_optinsn_page')
>   sed -i 's/free_optinsn_page/kprobes_free_optinsn_page/' $(git grep -l 'free_optinsn_page')
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> ---
>  arch/arm64/kernel/probes/kprobes.c |  2 +-
>  arch/powerpc/kernel/kprobes.c      |  2 +-
>  arch/powerpc/kernel/optprobes.c    |  4 ++--
>  arch/riscv/kernel/probes/kprobes.c |  2 +-
>  arch/s390/kernel/kprobes.c         |  2 +-
>  arch/x86/kernel/kprobes/core.c     |  2 +-
>  include/linux/kprobes.h            |  6 +++---
>  kernel/kprobes.c                   | 20 ++++++++++----------
>  8 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 327855a11df2f..4b6ab7b1fa211 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -129,7 +129,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  	return 0;
>  }
>  
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
>  {
>  	return __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
>  			GFP_KERNEL, PAGE_KERNEL_ROX, VM_FLUSH_RESET_PERMS,
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index bbca90a5e2ec0..0b297718d5de6 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -126,7 +126,7 @@ kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offse
>  	return (kprobe_opcode_t *)(addr + offset);
>  }
>  
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
>  {
>  	void *page;
>  
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 004fae2044a3e..0ddbda217073f 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -27,7 +27,7 @@
>  
>  static bool insn_page_in_use;
>  
> -void *alloc_optinsn_page(void)
> +void *kprobes_alloc_optinsn_page(void)
>  {
>  	if (insn_page_in_use)
>  		return NULL;
> @@ -35,7 +35,7 @@ void *alloc_optinsn_page(void)
>  	return &optinsn_slot;
>  }
>  
> -void free_optinsn_page(void *page)
> +void kprobes_free_optinsn_page(void *page)
>  {
>  	insn_page_in_use = false;
>  }
> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index 2f08c14a933d0..75201ce721057 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -105,7 +105,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  }
>  
>  #ifdef CONFIG_MMU
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
>  {
>  	return  __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
>  				     GFP_KERNEL, PAGE_KERNEL_READ_EXEC,
> diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
> index f0cf20d4b3c58..91ca4d501d4ef 100644
> --- a/arch/s390/kernel/kprobes.c
> +++ b/arch/s390/kernel/kprobes.c
> @@ -34,7 +34,7 @@ struct kretprobe_blackpoint kretprobe_blacklist[] = { };
>  
>  static int insn_page_in_use;
>  
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
>  {
>  	void *page;
>  
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index d0e49bd7c6f3f..7f01bbbfa9e2a 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -491,7 +491,7 @@ static int prepare_singlestep(kprobe_opcode_t *buf, struct kprobe *p,
>  }
>  
>  /* Make page to RO mode when allocate it */
> -void *alloc_insn_page(void)
> +void *kprobes_alloc_insn_page(void)
>  {
>  	void *page;
>  
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 0ff44d6633e33..ad4b561100f9e 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -430,10 +430,10 @@ int enable_kprobe(struct kprobe *kp);
>  
>  void dump_kprobe(struct kprobe *kp);
>  
> -void *alloc_insn_page(void);
> +void *kprobes_alloc_insn_page(void);
>  
> -void *alloc_optinsn_page(void);
> -void free_optinsn_page(void *page);
> +void *kprobes_alloc_optinsn_page(void);
> +void kprobes_free_optinsn_page(void *page);
>  
>  int kprobe_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
>  		       char *sym);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 9d9095e817928..35adf56430c9b 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -110,7 +110,7 @@ enum kprobe_slot_state {
>  	SLOT_USED = 2,
>  };
>  
> -void __weak *alloc_insn_page(void)
> +void __weak *kprobes_alloc_insn_page(void)
>  {
>  	/*
>  	 * Use module_alloc() so this page is within +/- 2GB of where the
> @@ -121,15 +121,15 @@ void __weak *alloc_insn_page(void)
>  	return module_alloc(PAGE_SIZE);
>  }
>  
> -static void free_insn_page(void *page)
> +static void kprobes_free_insn_page(void *page)
>  {
>  	module_memfree(page);
>  }
>  
>  struct kprobe_insn_cache kprobe_insn_slots = {
>  	.mutex = __MUTEX_INITIALIZER(kprobe_insn_slots.mutex),
> -	.alloc = alloc_insn_page,
> -	.free = free_insn_page,
> +	.alloc = kprobes_alloc_insn_page,
> +	.free = kprobes_free_insn_page,
>  	.sym = KPROBE_INSN_PAGE_SYM,
>  	.pages = LIST_HEAD_INIT(kprobe_insn_slots.pages),
>  	.insn_size = MAX_INSN_SIZE,
> @@ -333,21 +333,21 @@ int kprobe_cache_get_kallsym(struct kprobe_insn_cache *c, unsigned int *symnum,
>  }
>  
>  #ifdef CONFIG_OPTPROBES
> -void __weak *alloc_optinsn_page(void)
> +void __weak *kprobes_alloc_optinsn_page(void)
>  {
> -	return alloc_insn_page();
> +	return kprobes_alloc_insn_page();
>  }
>  
> -void __weak free_optinsn_page(void *page)
> +void __weak kprobes_free_optinsn_page(void *page)
>  {
> -	free_insn_page(page);
> +	kprobes_free_insn_page(page);
>  }
>  
>  /* For optimized_kprobe buffer */
>  struct kprobe_insn_cache kprobe_optinsn_slots = {
>  	.mutex = __MUTEX_INITIALIZER(kprobe_optinsn_slots.mutex),
> -	.alloc = alloc_optinsn_page,
> -	.free = free_optinsn_page,
> +	.alloc = kprobes_alloc_optinsn_page,
> +	.free = kprobes_free_optinsn_page,
>  	.sym = KPROBE_OPTINSN_PAGE_SYM,
>  	.pages = LIST_HEAD_INIT(kprobe_optinsn_slots.pages),
>  	/* .insn_size is initialized later */

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

_______________________________________________
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 3/4] kprobes/treewide: Explicitly override alloc/free functions
From: Jarkko Sakkinen @ 2024-04-03 16:40 UTC (permalink / raw)
  To: Mark Rutland, linux-kernel
  Cc: anil.s.keshavamurthy, aou, catalin.marinas, davem,
	linux-arm-kernel, mhiramat, naveen.n.rao, palmer, paul.walmsley,
	will
In-Reply-To: <20240403150154.667649-4-mark.rutland@arm.com>

On Wed Apr 3, 2024 at 6:01 PM EEST, Mark Rutland wrote:
> Currently architectures can override kprobes_alloc_insn_page(), but
> kprobes_free_insn_page() is always implemented using module_memfree(),
> which might not be what an architecture needs, especially as we'd like
> to make it possible to use kprobes without requiring MODULES.
>
> It would be nicer if architectures either:
>
> (a) Used only the generic kprobes_alloc_insn_page() and
>     kprobes_free_insn_page(), implicitly depending on MODULES.
>
> (b) Provided their own implementation of both kprobes_alloc_insn_page()
>     and kprobes_free_insn_page(), handling the relevant dependencies
>     themselves.
>
> This patch applies that split treewide:
>
> (a) Architectures using the generic kprobes_free_insn_page() and
>     kprobes_free_insn_page() are left as-is. The __weak annotation is
>     removed from the generic implementations so that accidental
>     overrides/misuse can be detected easily.
>
> (b) Architectures which provide their own kprobes_free_insn_page() are
>     given a matching implementation of kprobes_free_insn_page(), and
>     select HAVE_KPROBES_ALLOC.
>
>     This new Kconfig symbol will allow subsequent patches to relax the
>     dependency on MODULES to (MODULES || HAVE_KPROBES_ALLOC) once other
>     module dependencies in the core kprobes code are cleaned up.
>
>     Architectures which use module_alloc() are given an implementation
>     using module_memfree() along with an explicit dependency on MODULES.
>
>     Architectures using __vmalloc_node_range() are given an
>     implementation using vfree(). This loses the warning for
>     in_interrupt(), but vfree() can handle this via vfree_atomic(), so
>     the warning isn't necessary.
>
>     On riscv, the allocator depends on !XIP_KERNEL, which is already a
>     dependency for HAVE_KPROBES in arch/riscv/Kconfig.
>
> As of this patch arm64 and riscv have kprobe allocation functions which
> do not explicitly depend on MODULES. The core kprobes code still depends
> on MODULES.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/Kconfig                       | 3 +++
>  arch/arm64/Kconfig                 | 1 +
>  arch/arm64/kernel/probes/kprobes.c | 5 +++++
>  arch/powerpc/Kconfig               | 3 ++-
>  arch/powerpc/kernel/kprobes.c      | 5 +++++
>  arch/riscv/Kconfig                 | 1 +
>  arch/riscv/kernel/probes/kprobes.c | 5 +++++
>  arch/s390/Kconfig                  | 3 ++-
>  arch/s390/kernel/kprobes.c         | 5 +++++
>  arch/x86/Kconfig                   | 3 ++-
>  arch/x86/kernel/kprobes/core.c     | 5 +++++
>  include/linux/kprobes.h            | 1 +
>  kernel/kprobes.c                   | 6 ++++--
>  13 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 9f066785bb71d..85bb59f7b8c07 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -206,6 +206,9 @@ config HAVE_IOREMAP_PROT
>  config HAVE_KPROBES
>  	bool
>  
> +config HAVE_KPROBES_ALLOC
> +	bool
> +
>  config HAVE_KRETPROBES
>  	bool
>  
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 7b11c98b3e84b..bda7913d6c9b8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -233,6 +233,7 @@ config ARM64
>  	select HAVE_STACKPROTECTOR
>  	select HAVE_SYSCALL_TRACEPOINTS
>  	select HAVE_KPROBES
> +	select HAVE_KPROBES_ALLOC
>  	select HAVE_KRETPROBES
>  	select HAVE_GENERIC_VDSO
>  	select HOTPLUG_CORE_SYNC_DEAD if HOTPLUG_CPU
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 4b6ab7b1fa211..69d19a390cd48 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -136,6 +136,11 @@ void *kprobes_alloc_insn_page(void)
>  			NUMA_NO_NODE, __builtin_return_address(0));
>  }
>  
> +void kprobes_free_insn_page(void *page)
> +{
> +	vfree(page);
> +}
> +
>  /* arm kprobe: install breakpoint in text */
>  void __kprobes arch_arm_kprobe(struct kprobe *p)
>  {
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 1c4be33736860..13e0fc51dcdcf 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -254,7 +254,8 @@ config PPC
>  	select HAVE_KERNEL_LZMA			if DEFAULT_UIMAGE
>  	select HAVE_KERNEL_LZO			if DEFAULT_UIMAGE
>  	select HAVE_KERNEL_XZ			if PPC_BOOK3S || 44x
> -	select HAVE_KPROBES
> +	select HAVE_KPROBES			if MODULES
> +	select HAVE_KPROBES_ALLOC
>  	select HAVE_KPROBES_ON_FTRACE
>  	select HAVE_KRETPROBES
>  	select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if HAVE_OBJTOOL_MCOUNT && (!ARCH_USING_PATCHABLE_FUNCTION_ENTRY || (!CC_IS_GCC || GCC_VERSION >= 110100))
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 0b297718d5de6..d0332aaebab09 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -146,6 +146,11 @@ void *kprobes_alloc_insn_page(void)
>  	return NULL;
>  }
>  
> +void kprobes_free_insn_page(void *page)
> +{
> +	module_memfree(page);
> +}
> +
>  int arch_prepare_kprobe(struct kprobe *p)
>  {
>  	int ret = 0;
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index be09c8836d56b..4e22549a522a5 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -139,6 +139,7 @@ config RISCV
>  	select HAVE_GENERIC_VDSO if MMU && 64BIT
>  	select HAVE_IRQ_TIME_ACCOUNTING
>  	select HAVE_KPROBES if !XIP_KERNEL
> +	select HAVE_KPROBES_ALLOC
>  	select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
>  	select HAVE_KRETPROBES if !XIP_KERNEL
>  	# https://github.com/ClangBuiltLinux/linux/issues/1881
> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index 75201ce721057..37fdfa952d999 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -112,6 +112,11 @@ void *kprobes_alloc_insn_page(void)
>  				     VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>  				     __builtin_return_address(0));
>  }
> +
> +void kprobes_free_insn_page(void *page)
> +{
> +	vfree(page);
> +}
>  #endif
>  
>  /* install breakpoint in text */
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 8f01ada6845e3..635eddc3fce80 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -193,7 +193,8 @@ config S390
>  	select HAVE_KERNEL_UNCOMPRESSED
>  	select HAVE_KERNEL_XZ
>  	select HAVE_KERNEL_ZSTD
> -	select HAVE_KPROBES
> +	select HAVE_KPROBES		if MODULES
> +	select HAVE_KPROBES_ALLOC
>  	select HAVE_KPROBES_ON_FTRACE
>  	select HAVE_KRETPROBES
>  	select HAVE_LIVEPATCH
> diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
> index 91ca4d501d4ef..a5b142b8eb0f7 100644
> --- a/arch/s390/kernel/kprobes.c
> +++ b/arch/s390/kernel/kprobes.c
> @@ -45,6 +45,11 @@ void *kprobes_alloc_insn_page(void)
>  	return page;
>  }
>  
> +void kprobes_free_insn_page(void *page)
> +{
> +	module_memfree(page);
> +}
> +
>  static void *alloc_s390_insn_page(void)
>  {
>  	if (xchg(&insn_page_in_use, 1) == 1)
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 4fff6ed46e902..0810cd0bdeca9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -240,7 +240,8 @@ config X86
>  	select HAVE_KERNEL_LZO
>  	select HAVE_KERNEL_XZ
>  	select HAVE_KERNEL_ZSTD
> -	select HAVE_KPROBES
> +	select HAVE_KPROBES			if MODULES
> +	select HAVE_KPROBES_ALLOC
>  	select HAVE_KPROBES_ON_FTRACE
>  	select HAVE_FUNCTION_ERROR_INJECTION
>  	select HAVE_KRETPROBES
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 7f01bbbfa9e2a..5f093b94d9b40 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -508,6 +508,11 @@ void *kprobes_alloc_insn_page(void)
>  	return page;
>  }
>  
> +void kprobes_free_insn_page(void *page)
> +{
> +	module_memfree(page);
> +}
> +
>  /* Kprobe x86 instruction emulation - only regs->ip or IF flag modifiers */
>  
>  static void kprobe_emulate_ifmodifiers(struct kprobe *p, struct pt_regs *regs)
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index ad4b561100f9e..651c807727bea 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -431,6 +431,7 @@ int enable_kprobe(struct kprobe *kp);
>  void dump_kprobe(struct kprobe *kp);
>  
>  void *kprobes_alloc_insn_page(void);
> +void kprobes_free_insn_page(void *page);
>  
>  void *kprobes_alloc_optinsn_page(void);
>  void kprobes_free_optinsn_page(void *page);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 35adf56430c9b..fa2ee4e59eca2 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -110,7 +110,8 @@ enum kprobe_slot_state {
>  	SLOT_USED = 2,
>  };
>  
> -void __weak *kprobes_alloc_insn_page(void)
> +#ifndef CONFIG_HAVE_KPROBES_ALLOC
> +void *kprobes_alloc_insn_page(void)
>  {
>  	/*
>  	 * Use module_alloc() so this page is within +/- 2GB of where the
> @@ -121,10 +122,11 @@ void __weak *kprobes_alloc_insn_page(void)
>  	return module_alloc(PAGE_SIZE);
>  }
>  
> -static void kprobes_free_insn_page(void *page)
> +void kprobes_free_insn_page(void *page)
>  {
>  	module_memfree(page);
>  }
> +#endif
>  
>  struct kprobe_insn_cache kprobe_insn_slots = {
>  	.mutex = __MUTEX_INITIALIZER(kprobe_insn_slots.mutex),

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

_______________________________________________
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/4] kprobes: Remove core dependency on modules
From: Jarkko Sakkinen @ 2024-04-03 16:41 UTC (permalink / raw)
  To: Mark Rutland, linux-kernel
  Cc: anil.s.keshavamurthy, aou, catalin.marinas, davem,
	linux-arm-kernel, mhiramat, naveen.n.rao, palmer, paul.walmsley,
	will
In-Reply-To: <20240403150154.667649-5-mark.rutland@arm.com>

On Wed Apr 3, 2024 at 6:01 PM EEST, Mark Rutland wrote:
> From: Jarkko Sakkinen <jarkko@kernel.org>
>
> Tracing with kprobes while running a monolithic kernel is currently
> impossible because KPROBES depends on MODULES. While this dependency is
> necessary when HAVE_KPROBES_ALLOC=n and the core kprobes code allocates
> memory using module_alloc(), all the other module-specific code only
> exist to handle the case when MODULES=y, and can be hidden behind
> ifdeffery.
>
> Add the necessary ifdeffery, and remove the dependency on MODULES=y when
> HAVE_KPROBES_ALLOC=y.
>
> As of this patch kprobes can be used when MODULES=n on arm64 and
> riscv. All other architectures still depend on MODULES, either by virtue
> of the core dependency on MODULES when HAVE_KPROBES_ALLOC=n, or by
> virtue of an explciit dependency on MODULES in arch code.
>
> Other architectures can enable support by implementing their own
> kprobes_alloc_insn_page() and kprobes_free_insn_page() which do not
> depend on MODULES.
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> Link: https://lore.kernel.org/lkml/20240326134616.7691-1-jarkko@kernel.org/
> [Mark: Remove execmem changes, depend on HAVE_KPROBES_ALLOC]
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/Kconfig                |  2 +-
>  kernel/kprobes.c            | 46 ++++++++++++++++++++++---------------
>  kernel/trace/trace_kprobe.c | 15 ++++++++++--
>  3 files changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 85bb59f7b8c07..0df2c88547b3c 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -52,7 +52,7 @@ config GENERIC_ENTRY
>  
>  config KPROBES
>  	bool "Kprobes"
> -	depends on MODULES
> +	depends on MODULES || HAVE_KPROBES_ALLOC
>  	depends on HAVE_KPROBES
>  	select KALLSYMS
>  	select TASKS_RCU if PREEMPTION
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index fa2ee4e59eca2..ec4493a41b505 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1594,6 +1594,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  			goto out;
>  		}
>  
> +#ifdef CONFIG_MODULES
>  		/*
>  		 * If the module freed '.init.text', we couldn't insert
>  		 * kprobes in there.
> @@ -1604,7 +1605,9 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  			*probed_mod = NULL;
>  			ret = -ENOENT;
>  		}
> +#endif /* CONFIG_MODULES */
>  	}
> +
>  out:
>  	preempt_enable();
>  	jump_label_unlock();
> @@ -2484,24 +2487,6 @@ int kprobe_add_area_blacklist(unsigned long start, unsigned long end)
>  	return 0;
>  }
>  
> -/* Remove all symbols in given area from kprobe blacklist */
> -static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
> -{
> -	struct kprobe_blacklist_entry *ent, *n;
> -
> -	list_for_each_entry_safe(ent, n, &kprobe_blacklist, list) {
> -		if (ent->start_addr < start || ent->start_addr >= end)
> -			continue;
> -		list_del(&ent->list);
> -		kfree(ent);
> -	}
> -}
> -
> -static void kprobe_remove_ksym_blacklist(unsigned long entry)
> -{
> -	kprobe_remove_area_blacklist(entry, entry + 1);
> -}
> -
>  int __weak arch_kprobe_get_kallsym(unsigned int *symnum, unsigned long *value,
>  				   char *type, char *sym)
>  {
> @@ -2566,6 +2551,25 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
>  	return ret ? : arch_populate_kprobe_blacklist();
>  }
>  
> +#ifdef CONFIG_MODULES
> +/* Remove all symbols in given area from kprobe blacklist */
> +static void kprobe_remove_area_blacklist(unsigned long start, unsigned long end)
> +{
> +	struct kprobe_blacklist_entry *ent, *n;
> +
> +	list_for_each_entry_safe(ent, n, &kprobe_blacklist, list) {
> +		if (ent->start_addr < start || ent->start_addr >= end)
> +			continue;
> +		list_del(&ent->list);
> +		kfree(ent);
> +	}
> +}
> +
> +static void kprobe_remove_ksym_blacklist(unsigned long entry)
> +{
> +	kprobe_remove_area_blacklist(entry, entry + 1);
> +}
> +
>  static void add_module_kprobe_blacklist(struct module *mod)
>  {
>  	unsigned long start, end;
> @@ -2662,6 +2666,9 @@ static int kprobes_module_callback(struct notifier_block *nb,
>  	mutex_unlock(&kprobe_mutex);
>  	return NOTIFY_DONE;
>  }
> +#else
> +#define kprobes_module_callback	(NULL)
> +#endif /* CONFIG_MODULES */
>  
>  static struct notifier_block kprobe_module_nb = {
>  	.notifier_call = kprobes_module_callback,
> @@ -2726,7 +2733,8 @@ static int __init init_kprobes(void)
>  	err = arch_init_kprobes();
>  	if (!err)
>  		err = register_die_notifier(&kprobe_exceptions_nb);
> -	if (!err)
> +
> +	if (!err && IS_ENABLED(CONFIG_MODULES))
>  		err = register_module_notifier(&kprobe_module_nb);
>  
>  	kprobes_initialized = (err == 0);
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 14099cc17fc9e..c509ba776e679 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -111,6 +111,7 @@ static nokprobe_inline bool trace_kprobe_within_module(struct trace_kprobe *tk,
>  	return strncmp(module_name(mod), name, len) == 0 && name[len] == ':';
>  }
>  
> +#ifdef CONFIG_MODULES
>  static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
>  {
>  	char *p;
> @@ -129,6 +130,9 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
>  
>  	return ret;
>  }
> +#else
> +#define trace_kprobe_module_exist(tk) false /* aka a module never exists */
> +#endif /* CONFIG_MODULES */
>  
>  static bool trace_kprobe_is_busy(struct dyn_event *ev)
>  {
> @@ -670,6 +674,7 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_MODULES
>  /* Module notifier call back, checking event on the module */
>  static int trace_kprobe_module_callback(struct notifier_block *nb,
>  				       unsigned long val, void *data)
> @@ -699,6 +704,9 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
>  
>  	return NOTIFY_DONE;
>  }
> +#else
> +#define trace_kprobe_module_callback (NULL)
> +#endif /* CONFIG_MODULES */
>  
>  static struct notifier_block trace_kprobe_module_nb = {
>  	.notifier_call = trace_kprobe_module_callback,
> @@ -1933,8 +1941,11 @@ static __init int init_kprobe_trace_early(void)
>  	if (ret)
>  		return ret;
>  
> -	if (register_module_notifier(&trace_kprobe_module_nb))
> -		return -EINVAL;
> +	if (IS_ENABLED(CONFIG_MODULES)) {
> +		ret = register_module_notifier(&trace_kprobe_module_nb);
> +		if (ret)
> +			return -EINVAL;
> +	}
>  
>  	return 0;
>  }

I'll test the patch set asap.

BR, Jarkko

_______________________________________________
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 v14 01/18] irqchip/sifive-plic: Convert PLIC driver into a platform driver
From: Anup Patel @ 2024-04-03 16:42 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Anup Patel, Geert Uytterhoeven, Palmer Dabbelt, Paul Walmsley,
	Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Frank Rowand,
	Conor Dooley, Marc Zyngier, Björn Töpel, Atish Patra,
	Andrew Jones, Sunil V L, Saravana Kannan, linux-riscv,
	linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <CA+V-a8ser=hDmst6+XSeOWaEoOd+iY3Ys6bYBWDa5UYPfT+Pug@mail.gmail.com>

On Wed, Apr 3, 2024 at 9:19 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> On Wed, Apr 3, 2024 at 3:17 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Wed, Apr 3, 2024 at 2:01 PM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > >
> > > Hi Anup,
> > >
> > > On Thu, Feb 22, 2024 at 9:41 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >
> > > > The PLIC driver does not require very early initialization so convert
> > > > it into a platform driver.
> > > >
> > > > After conversion, the PLIC driver is probed after CPUs are brought-up
> > > > so setup cpuhp state after context handler of all online CPUs are
> > > > initialized otherwise PLIC driver crashes for platforms with multiple
> > > > PLIC instances.
> > > >
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > ---
> > > >  drivers/irqchip/irq-sifive-plic.c | 101 ++++++++++++++++++------------
> > > >  1 file changed, 61 insertions(+), 40 deletions(-)
> > > >
> > > This patch seems to have broken things on RZ/Five SoC, after reverting
> > > this patch I get to boot it back again on v6.9-rc2. Looks like there
> > > is some probe order issue after switching to platform driver?
> >
> > Yes, this is most likely related to probe ordering based on your DT.
> >
> > Can you share the failing boot log and DT ?
>
> non working case, https://paste.debian.net/1312947/

> after reverting, https://paste.debian.net/1312948/
> (attached is the DTB)

Can you add "console=ttySC0,115200" to kernel parameters and
share updated boot logs ?

Regards,
Anup

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/4] arm64: patching: always use fixmap
From: Mark Rutland @ 2024-04-03 16:51 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel, Catalin Marinas, Will Deacon, anil.s.keshavamurthy,
	aou, davem, linux-arm-kernel, mhiramat, naveen.n.rao, palmer,
	paul.walmsley
In-Reply-To: <D0AMI3962WW0.3JKFCSUXVSSVL@kernel.org>

On Wed, Apr 03, 2024 at 07:20:31PM +0300, Jarkko Sakkinen wrote:
> On Wed Apr 3, 2024 at 6:01 PM EEST, Mark Rutland wrote:
> > For historical reasons, patch_map() won't bother to fixmap non-image
> > addresses when CONFIG_STRICT_MODULE_RWX=n, matching the behaviour prior
> > to the introduction of CONFIG_STRICT_MODULE_RWX. However, as arm64
> > doesn't select CONFIG_ARCH_OPTIONAL_KERNEL_RWX, CONFIG_MODULES implies
> > CONFIG_STRICT_MODULE_RWX, so any kernel built with module support will
> > use the fixmap for any non-image address.
> 
> Not familiar with the config flag but I'd guess it is essentially
> w^x enforcement right for the sections?

Essentially, yes.

> > Historically we only used patch_map() for the kernel image and modules,
> > but these days its also used by BPF and KPROBES to write to read-only
> > pages of executable text. Currently these both depend on CONFIG_MODULES,
> > but we'd like to change that in subsequent patches, which will require
> > using the fixmap regardless of CONFIG_STRICT_MODULE_RWX.
> >
> > This patch changes patch_map() to always use the fixmap, and simplifies
> > the logic:
> >
> > * Use is_image_text() directly in the if-else, rather than using a
> >   temporary boolean variable.
> >
> > * Use offset_in_page() to get the offset within the mapping.
> >
> > * Remove uintaddr and cast the address directly when using
> >   is_image_text().
> >
> > For kernels built with CONFIG_MODULES=y, there should be no functional
> > change as a result of this patch.
> >
> > For kernels built with CONFIG_MODULES=n, patch_map() will use the fixmap
> > for non-image addresses, but there are no extant users with non-image
> > addresses when CONFIG_MODULES=n, and hence there should be no functional
> > change as a result of this patch alone.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > ---
> >  arch/arm64/kernel/patching.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > Catalin, Will, this is a prerequisite for the final two patches in the
> > series. Are you happy for this go via the tracing tree?
> >
> > Mark.
> >
> > diff --git a/arch/arm64/kernel/patching.c b/arch/arm64/kernel/patching.c
> > index 2555349303684..f0f3a2a82ca5a 100644
> > --- a/arch/arm64/kernel/patching.c
> > +++ b/arch/arm64/kernel/patching.c
> > @@ -30,20 +30,16 @@ static bool is_image_text(unsigned long addr)
> >  
> >  static void __kprobes *patch_map(void *addr, int fixmap)
> >  {
> > -	unsigned long uintaddr = (uintptr_t) addr;
> > -	bool image = is_image_text(uintaddr);
> >  	struct page *page;
> >  
> > -	if (image)
> > +	if (is_image_text((unsigned long)addr))
> >  		page = phys_to_page(__pa_symbol(addr));
> > -	else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
> > -		page = vmalloc_to_page(addr);
> >  	else
> > -		return addr;
> > +		page = vmalloc_to_page(addr);
> >  
> >  	BUG_ON(!page);
> >  	return (void *)set_fixmap_offset(fixmap, page_to_phys(page) +
> > -			(uintaddr & ~PAGE_MASK));
> > +					 offset_in_page(addr));
> 
> nit: could be a single line but i guess it is up to the taste (and
> subsystem maintainer). I.e. checkpatch will allow it at least.
> 
> I don't mind it too much just mentioning for completeness.

At that point it goes to 93 chars long, and I stuck with the existing line
wrapping at 80 chars. I'd rather have a temporary 'phys_addr_t phys' variable
and do:

	phys = page_to_phys(page) + offset_in_page(addr);
	return (void *)set_fixmap(fixmap, phys);

... but I'll leave this as-is for now.

> >  }
> >  
> >  static void __kprobes patch_unmap(int fixmap)
> 
> If my assumption about the config flag holds this makes sense:
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.rg>

Thanks! I assume that should be "kernel.org", with an 'o' ;)

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 2/4] dt-bindings: arm: fsl: remove reduntant toradex,colibri-imx8x
From: Rob Herring @ 2024-04-03 16:57 UTC (permalink / raw)
  To: Hiago De Franco
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Krzysztof Kozlowski, Conor Dooley, Hiago De Franco, imx,
	linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20240402193512.240417-3-hiagofranco@gmail.com>

On Tue, Apr 02, 2024 at 04:35:10PM -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
> 
> 'toradex,colibri-imx8x' is already present as a constant value for
> 'i.MX8QP Board with Toradex Colibri iMX8X Modules', so there is no need
> to keep it twice as a enum value for 'i.MX8QXP based Boards'.

If the module can operate on its own, then it would be valid to have 
just "toradex,colibri-imx8x". If not, then:

Acked-by: Rob Herring <robh@kernel.org>

> 
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 0027201e19f8..6fdfa10af43c 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -1218,7 +1218,6 @@ properties:
>            - enum:
>                - einfochips,imx8qxp-ai_ml  # i.MX8QXP AI_ML Board
>                - fsl,imx8qxp-mek           # i.MX8QXP MEK Board
> -              - toradex,colibri-imx8x     # Colibri iMX8X Modules
>            - const: fsl,imx8qxp
>  
>        - description: i.MX8DXL based Boards
> -- 
> 2.39.2
> 

_______________________________________________
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 23/25] drivers: media: i2c: imx258: Add support for powerdown gpio
From: Ondřej Jirman @ 2024-04-03 16:57 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: git, 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
In-Reply-To: <Zg2DBasC501hMQSS@kekkonen.localdomain>

Hi Sakari and Luis,

On Wed, Apr 03, 2024 at 04:25:41PM GMT, Sakari Ailus wrote:
> Hi Luis, Ondrej,
> 
> On Wed, Apr 03, 2024 at 09:03:52AM -0600, git@luigi311.com wrote:
> > From: Luis Garcia <git@luigi311.com>
> > 
> > On some boards powerdown signal needs to be deasserted for this
> > sensor to be enabled.
> > 
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Luis Garcia <git@luigi311.com>
> > ---
> >  drivers/media/i2c/imx258.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> > index 30352c33f63c..163f04f6f954 100644
> > --- a/drivers/media/i2c/imx258.c
> > +++ b/drivers/media/i2c/imx258.c
> > @@ -679,6 +679,8 @@ struct imx258 {
> >  	unsigned int lane_mode_idx;
> >  	unsigned int csi2_flags;
> >  
> > +	struct gpio_desc *powerdown_gpio;
> > +
> >  	/*
> >  	 * Mutex for serialized access:
> >  	 * Protect sensor module set pad format and start/stop streaming safely.
> > @@ -1213,6 +1215,8 @@ static int imx258_power_on(struct device *dev)
> >  	struct imx258 *imx258 = to_imx258(sd);
> >  	int ret;
> >  
> > +	gpiod_set_value_cansleep(imx258->powerdown_gpio, 0);
> 
> What does the spec say? Should this really happen before switching on the
> supplies below?

There's no powerdown input in the IMX258 manual. The manual only mentions
that XCLR (reset) should be held low during power on.

https://megous.com/dl/tmp/15b0992a720ab82d.png

https://megous.com/dl/tmp/f2cc991046d97641.png

   This sensor doesn’t have a built-in “Power ON Reset” function. The XCLR pin
   is set to “LOW” and the power supplies are brought up. Then the XCLR pin
   should be set to “High” after INCK supplied.

So this input is some feature on camera module itself outside of the
IMX258 chip, which I think is used to gate power to the module. Eg. on Pinephone
Pro, there are two modules with shared power rails, so enabling supply to
one module enables it to the other one, too. So this input becomes the only way
to really enable/disable power to the chip when both are used at once at some
point, because regulator_bulk_enable/disable becomes ineffective at that point.

Luis, maybe you saw some other datasheet that mentions this input? IMO,
it just gates the power rails via some mosfets on the module itself, since
there's not power down input to the chip itself.

kind regards,
	o.

> > +
> >  	ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
> >  				    imx258->supplies);
> >  	if (ret) {
> > @@ -1224,6 +1228,7 @@ static int imx258_power_on(struct device *dev)
> >  	ret = clk_prepare_enable(imx258->clk);
> >  	if (ret) {
> >  		dev_err(dev, "failed to enable clock\n");
> > +		gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
> >  		regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
> >  	}
> >  
> > @@ -1238,6 +1243,8 @@ static int imx258_power_off(struct device *dev)
> >  	clk_disable_unprepare(imx258->clk);
> >  	regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
> >  
> > +	gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -1541,6 +1548,12 @@ static int imx258_probe(struct i2c_client *client)
> >  	if (!imx258->variant_cfg)
> >  		imx258->variant_cfg = &imx258_cfg;
> >  
> > +	/* request optional power down pin */
> > +	imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
> > +						    GPIOD_OUT_HIGH);
> > +	if (IS_ERR(imx258->powerdown_gpio))
> > +		return PTR_ERR(imx258->powerdown_gpio);
> > +
> >  	/* Initialize subdev */
> >  	v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
> >  
> 
> -- 
> Regards,
> 
> Sakari Ailus

_______________________________________________
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] dt-bindings: arm: fsl: Add Colibri iMX8DX
From: Rob Herring @ 2024-04-03 16:58 UTC (permalink / raw)
  To: Hiago De Franco
  Cc: Sascha Hauer, imx, Conor Dooley, Shawn Guo, linux-kernel,
	Fabio Estevam, Pengutronix Kernel Team, devicetree,
	linux-arm-kernel, Krzysztof Kozlowski, Hiago De Franco
In-Reply-To: <20240402193512.240417-4-hiagofranco@gmail.com>


On Tue, 02 Apr 2024 16:35:11 -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
> 
> Add support for Toradex Colibri iMX8DX SoM. As the i.MX8QXP variant is
> already supported, update the description with i.MX8DX and add
> 'fsl,imx8dx' item as well.
> 
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
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/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code
From: Rob Herring @ 2024-04-03 17:02 UTC (permalink / raw)
  To: Arseniy Krasnov
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Kevin Hilman,
	Jerome Brunet, Martin Blumenstingl, linux-mtd, devicetree,
	linux-arm-kernel, linux-amlogic, linux-kernel, oxffffaa, kernel
In-Reply-To: <20240402202705.2355326-2-avkrasnov@salutedevices.com>

On Tue, Apr 02, 2024 at 11:27:04PM +0300, Arseniy Krasnov wrote:
> Boot ROM code on Meson requires that some pages on NAND must be written
> in special mode: "short" ECC mode where each block is 384 bytes and
> scrambling mode is on. Such pages located with the specified interval
> within specified offset. Both interval and offset are located in the
> device tree and used by driver if 'nand-is-boot-medium' is set for
> NAND chip.
> 
> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> ---
>  .../bindings/mtd/amlogic,meson-nand.yaml         | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> index 57b6957c8415..f49819ee76b8 100644
> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> @@ -64,11 +64,27 @@ patternProperties:
>          items:
>            maximum: 0
>  
> +      meson,boot-page-last:

'meson' is not a valid vendor.

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The NFC driver needs this information to select ECC
> +          algorithms supported by the boot ROM.
> +          Only used in combination with 'nand-is-boot-medium'.

No need to state what's captured with constraints.

> +
> +      meson,boot-page-step:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The NFC driver needs this information to select ECC
> +          algorithms supported by the boot ROM.
> +          Only used in combination with 'nand-is-boot-medium'.

step is in blocks/pages/bytes?

> +
>      unevaluatedProperties: false
>  
>      dependencies:
>        nand-ecc-strength: [nand-ecc-step-size]
>        nand-ecc-step-size: [nand-ecc-strength]
> +      meson,boot-page-last: [nand-is-boot-medium]
> +      meson,boot-page-step: [nand-is-boot-medium]

I assume both properties must be present? If so:

meson,boot-page-last: ['meson,boot-page-step']
meson,boot-page-step: ['meson,boot-page-last']

Rob

_______________________________________________
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 24/25] drivers: media: i2c: imx258: Add support for reset gpio
From: Ondřej Jirman @ 2024-04-03 17:03 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: git, 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
In-Reply-To: <Zg2Dy2QBguXQoR3P@kekkonen.localdomain>

Hi,

On Wed, Apr 03, 2024 at 04:28:59PM GMT, Sakari Ailus wrote:
> Hi Luis,
> 
> Could you unify the subject prefix for the driver patches, please? E.g.
> "media: imx258: " would be fine.
> 
> On Wed, Apr 03, 2024 at 09:03:53AM -0600, git@luigi311.com wrote:
> > From: Luis Garcia <git@luigi311.com>
> > 
> > It was documented in DT, but not implemented.
> > 
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > Signed-off-by: Luis Garcia <git@luigi311.com>
> > ---
> >  drivers/media/i2c/imx258.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> > index 163f04f6f954..4c117c4829f1 100644
> > --- a/drivers/media/i2c/imx258.c
> > +++ b/drivers/media/i2c/imx258.c
> > @@ -680,6 +680,7 @@ struct imx258 {
> >  	unsigned int csi2_flags;
> >  
> >  	struct gpio_desc *powerdown_gpio;
> > +	struct gpio_desc *reset_gpio;
> >  
> >  	/*
> >  	 * Mutex for serialized access:
> > @@ -1232,7 +1233,11 @@ static int imx258_power_on(struct device *dev)
> >  		regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
> >  	}
> >  
> > -	return ret;
> > +	gpiod_set_value_cansleep(imx258->reset_gpio, 0);
> > +
> > +	usleep_range(400, 500);
> 
> You could mention this at least in the commit message.

This is T6 in the datasheet: https://megous.com/dl/tmp/92c9223ce877216e.png


> > +
> > +	return 0;
> >  }
> >  
> >  static int imx258_power_off(struct device *dev)
> > @@ -1243,6 +1248,7 @@ static int imx258_power_off(struct device *dev)
> >  	clk_disable_unprepare(imx258->clk);
> >  	regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
> >  
> > +	gpiod_set_value_cansleep(imx258->reset_gpio, 1);
> 
> Same question than on the other GPIO: does this belong here?

No, this should be before the regulator_bulk_disable.

See: https://megous.com/dl/tmp/c96180b23d7ce63a.png

kind regards,
	o.

> >  	gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
> >  
> >  	return 0;
> > @@ -1554,6 +1560,12 @@ static int imx258_probe(struct i2c_client *client)
> >  	if (IS_ERR(imx258->powerdown_gpio))
> >  		return PTR_ERR(imx258->powerdown_gpio);
> >  
> > +	/* request optional reset pin */
> > +	imx258->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> > +						    GPIOD_OUT_HIGH);
> > +	if (IS_ERR(imx258->reset_gpio))
> > +		return PTR_ERR(imx258->reset_gpio);
> > +
> >  	/* Initialize subdev */
> >  	v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
> >  
> 
> -- 
> Regards,
> 
> Sakari Ailus

_______________________________________________
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 10/10] perf/thunderx2: Avoid placing cpumask on the stack
From: Dawei Li @ 2024-04-03 15:59 UTC (permalink / raw)
  To: will, mark.rutland, yury.norov, linux
  Cc: xueshuai, renyu.zj, yangyicong, jonathan.cameron, andersson,
	konrad.dybcio, linux-arm-kernel, linux-kernel, linux-arm-msm,
	Dawei Li
In-Reply-To: <20240403155950.2068109-1-dawei.li@shingroup.cn>

In general it's preferable to avoid placing cpumasks on the stack, as
for large values of NR_CPUS these can consume significant amounts of
stack space and make stack overflows more likely.

Use cpumask_any_and_but() to avoid the need for a temporary cpumask on
the stack.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
---
 drivers/perf/thunderx2_pmu.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
index e16d10c763de..b3377b662ffc 100644
--- a/drivers/perf/thunderx2_pmu.c
+++ b/drivers/perf/thunderx2_pmu.c
@@ -932,9 +932,8 @@ static int tx2_uncore_pmu_online_cpu(unsigned int cpu,
 static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
 		struct hlist_node *hpnode)
 {
-	int new_cpu;
 	struct tx2_uncore_pmu *tx2_pmu;
-	struct cpumask cpu_online_mask_temp;
+	unsigned int new_cpu;
 
 	tx2_pmu = hlist_entry_safe(hpnode,
 			struct tx2_uncore_pmu, hpnode);
@@ -945,11 +944,8 @@ static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
 	if (tx2_pmu->hrtimer_callback)
 		hrtimer_cancel(&tx2_pmu->hrtimer);
 
-	cpumask_copy(&cpu_online_mask_temp, cpu_online_mask);
-	cpumask_clear_cpu(cpu, &cpu_online_mask_temp);
-	new_cpu = cpumask_any_and(
-			cpumask_of_node(tx2_pmu->node),
-			&cpu_online_mask_temp);
+	new_cpu = cpumask_any_and_but(cpumask_of_node(tx2_pmu->node),
+				      cpu_online_mask, cpu);
 
 	tx2_pmu->cpu = new_cpu;
 	if (new_cpu >= nr_cpu_ids)
-- 
2.27.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


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