Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit
From: Gilles Talis @ 2024-03-29 22:08 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: devicetree, imx, linux-arm-kernel, conor+dt,
	krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex
In-Reply-To: <147dfc5e-91e5-4ad4-b818-f6717e62df45@lunn.ch>

Hi Andrew,

Thank you for your review.

Le jeu. 28 mars 2024 à 18:11, Andrew Lunn <andrew@lunn.ch> a écrit :
>
> > +&eqos {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pinctrl_eqos>;
> > +     phy-mode = "rgmii-id";
> > +     phy-handle = <&ethphy0>;
> > +     status = "okay";
> > +
> > +     mdio {
> > +             compatible = "snps,dwmac-mdio";
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +
> > +             ethphy0: ethernet-phy@0 {
> > +                     compatible = "ethernet-phy-ieee802.3-c22";
> > +                     reg = <0>;
> > +                     reset-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;
> > +                     reset-assert-us = <1000>;
> > +                     reset-deassert-us = <10000>;
> > +                     qca,disable-smarteee;
> > +                     qca,disable-hibernation-mode;
> > +                     vddio-supply = <&vddio>;
> > +
> > +                     vddio: vddio-regulator {
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +                     };
>
> Please could you explain what this last node is doing.
>
>        Andrew
It seems like this might not be needed. Will check that and fix as appropriate.
thanks
Gilles.

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

^ permalink raw reply

* [PATCH v7 2/7] iommu/arm-smmu-qcom-tbu: Add Qualcomm TBU driver
From: Georgi Djakov @ 2024-03-29 21:06 UTC (permalink / raw)
  To: will, robin.murphy, joro, iommu
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree, andersson,
	konrad.dybcio, robdclark, linux-arm-kernel, linux-kernel,
	linux-arm-msm, quic_cgoldswo, quic_sukadev, quic_pdaly,
	quic_sudaraja, djakov
In-Reply-To: <20240329210638.3647523-1-quic_c_gdjako@quicinc.com>

Operating the TBUs (Translation Buffer Units) from Linux on Qualcomm
platforms can help with debugging context faults. To help with that,
the TBUs can run ATOS (Address Translation Operations) to manually
trigger address translation of IOVA to physical address in hardware
and provide more details when a context fault happens.

The driver will control the resources needed by the TBU to allow
running the debug operations such as ATOS, check for outstanding
transactions, do snapshot capture etc.

Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
---
 drivers/iommu/Kconfig                         |   9 +
 drivers/iommu/arm/arm-smmu/Makefile           |   1 +
 .../iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c    | 372 ++++++++++++++++++
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h    |   2 +
 drivers/iommu/arm/arm-smmu/arm-smmu.h         |   2 +
 5 files changed, 386 insertions(+)
 create mode 100644 drivers/iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 0af39bbbe3a3..b699e88f42c5 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -374,6 +374,15 @@ config ARM_SMMU_QCOM
 	  When running on a Qualcomm platform that has the custom variant
 	  of the ARM SMMU, this needs to be built into the SMMU driver.
 
+config ARM_SMMU_QCOM_TBU
+	bool "Qualcomm TBU driver"
+	depends on ARM_SMMU_QCOM
+	help
+	  The SMMUs on Qualcomm platforms may include Translation Buffer
+	  Units (TBUs) for each master. Enabling support for these units
+	  allows to operate the TBUs and obtain additional information
+	  when debugging memory management issues like context faults.
+
 config ARM_SMMU_QCOM_DEBUG
 	bool "ARM SMMU QCOM implementation defined debug support"
 	depends on ARM_SMMU_QCOM
diff --git a/drivers/iommu/arm/arm-smmu/Makefile b/drivers/iommu/arm/arm-smmu/Makefile
index 2a5a95e8e3f9..c35ff78fcfd5 100644
--- a/drivers/iommu/arm/arm-smmu/Makefile
+++ b/drivers/iommu/arm/arm-smmu/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
 obj-$(CONFIG_ARM_SMMU) += arm_smmu.o
 arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o
 arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o
+arm_smmu-$(CONFIG_ARM_SMMU_QCOM_TBU) += arm-smmu-qcom-tbu.o
 arm_smmu-$(CONFIG_ARM_SMMU_QCOM_DEBUG) += arm-smmu-qcom-debug.o
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c
new file mode 100644
index 000000000000..e3202ed89566
--- /dev/null
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/interconnect.h>
+#include <linux/iopoll.h>
+#include <linux/list.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#include "arm-smmu.h"
+#include "arm-smmu-qcom.h"
+
+#define TBU_DBG_TIMEOUT_US		100
+#define DEBUG_AXUSER_REG		0x30
+#define DEBUG_AXUSER_CDMID		GENMASK_ULL(43, 36)
+#define DEBUG_AXUSER_CDMID_VAL		0xff
+#define DEBUG_PAR_REG			0x28
+#define DEBUG_PAR_FAULT_VAL		BIT(0)
+#define DEBUG_PAR_PA			GENMASK_ULL(47, 12)
+#define DEBUG_SID_HALT_REG		0x0
+#define DEBUG_SID_HALT_VAL		BIT(16)
+#define DEBUG_SID_HALT_SID		GENMASK(9, 0)
+#define DEBUG_SR_HALT_ACK_REG		0x20
+#define DEBUG_SR_HALT_ACK_VAL		BIT(1)
+#define DEBUG_SR_ECATS_RUNNING_VAL	BIT(0)
+#define DEBUG_TXN_AXCACHE		GENMASK(5, 2)
+#define DEBUG_TXN_AXPROT		GENMASK(8, 6)
+#define DEBUG_TXN_AXPROT_PRIV		0x1
+#define DEBUG_TXN_AXPROT_NSEC		0x2
+#define DEBUG_TXN_TRIGG_REG		0x18
+#define DEBUG_TXN_TRIGGER		BIT(0)
+#define DEBUG_VA_ADDR_REG		0x8
+
+static LIST_HEAD(tbu_list);
+static DEFINE_MUTEX(tbu_list_lock);
+static DEFINE_SPINLOCK(atos_lock);
+
+struct qcom_tbu {
+	struct device *dev;
+	struct device_node *smmu_np;
+	u32 sid_range[2];
+	struct list_head list;
+	struct clk *clk;
+	struct icc_path	*path;
+	void __iomem *base;
+	spinlock_t halt_lock; /* multiple halt or resume can't execute concurrently */
+	int halt_count;
+};
+
+static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
+{
+	return container_of(smmu, struct qcom_smmu, smmu);
+}
+
+static struct qcom_tbu *qcom_find_tbu(struct qcom_smmu *qsmmu, u32 sid)
+{
+	struct qcom_tbu *tbu;
+	u32 start, end;
+
+	mutex_lock(&tbu_list_lock);
+
+	if (list_empty(&tbu_list))
+		goto out;
+
+	list_for_each_entry(tbu, &tbu_list, list) {
+		start = tbu->sid_range[0];
+		end = start + tbu->sid_range[1];
+
+		if (qsmmu->smmu.dev->of_node == tbu->smmu_np &&
+		    start <= sid && sid < end) {
+			mutex_unlock(&tbu_list_lock);
+			return tbu;
+		}
+	}
+	dev_err(qsmmu->smmu.dev, "Unable to find TBU for sid 0x%x\n", sid);
+
+out:
+	mutex_unlock(&tbu_list_lock);
+	return NULL;
+}
+
+static int qcom_tbu_halt(struct qcom_tbu *tbu, struct arm_smmu_domain *smmu_domain)
+{
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	int ret = 0, idx = smmu_domain->cfg.cbndx;
+	unsigned long flags;
+	u32 val, fsr, status;
+
+	spin_lock_irqsave(&tbu->halt_lock, flags);
+	if (tbu->halt_count) {
+		tbu->halt_count++;
+		goto out;
+	}
+
+	val = readl_relaxed(tbu->base + DEBUG_SID_HALT_REG);
+	val |= DEBUG_SID_HALT_VAL;
+	writel_relaxed(val, tbu->base + DEBUG_SID_HALT_REG);
+
+	fsr = arm_smmu_cb_read(smmu, idx, ARM_SMMU_CB_FSR);
+	if ((fsr & ARM_SMMU_FSR_FAULT) && (fsr & ARM_SMMU_FSR_SS)) {
+		u32 sctlr_orig, sctlr;
+
+		/*
+		 * We are in a fault. Our request to halt the bus will not
+		 * complete until transactions in front of us (such as the fault
+		 * itself) have completed. Disable iommu faults and terminate
+		 * any existing transactions.
+		 */
+		sctlr_orig = arm_smmu_cb_read(smmu, idx, ARM_SMMU_CB_SCTLR);
+		sctlr = sctlr_orig & ~(ARM_SMMU_SCTLR_CFCFG | ARM_SMMU_SCTLR_CFIE);
+		arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, sctlr);
+		arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_FSR, fsr);
+		arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_RESUME, ARM_SMMU_RESUME_TERMINATE);
+		arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, sctlr_orig);
+	}
+
+	if (readl_poll_timeout_atomic(tbu->base + DEBUG_SR_HALT_ACK_REG, status,
+				      (status & DEBUG_SR_HALT_ACK_VAL),
+				      0, TBU_DBG_TIMEOUT_US)) {
+		dev_err(tbu->dev, "Timeout while trying to halt TBU!\n");
+		ret = -ETIMEDOUT;
+
+		val = readl_relaxed(tbu->base + DEBUG_SID_HALT_REG);
+		val &= ~DEBUG_SID_HALT_VAL;
+		writel_relaxed(val, tbu->base + DEBUG_SID_HALT_REG);
+
+		goto out;
+	}
+
+	tbu->halt_count = 1;
+
+out:
+	spin_unlock_irqrestore(&tbu->halt_lock, flags);
+	return ret;
+}
+
+static void qcom_tbu_resume(struct qcom_tbu *tbu)
+{
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&tbu->halt_lock, flags);
+	if (!tbu->halt_count) {
+		WARN(1, "%s: halt_count is 0", dev_name(tbu->dev));
+		goto out;
+	}
+
+	if (tbu->halt_count > 1) {
+		tbu->halt_count--;
+		goto out;
+	}
+
+	val = readl_relaxed(tbu->base + DEBUG_SID_HALT_REG);
+	val &= ~DEBUG_SID_HALT_VAL;
+	writel_relaxed(val, tbu->base + DEBUG_SID_HALT_REG);
+
+	tbu->halt_count = 0;
+out:
+	spin_unlock_irqrestore(&tbu->halt_lock, flags);
+}
+
+static phys_addr_t qcom_tbu_trigger_atos(struct arm_smmu_domain *smmu_domain,
+					 struct qcom_tbu *tbu, dma_addr_t iova, u32 sid)
+{
+	bool atos_timedout = false;
+	phys_addr_t phys = 0;
+	ktime_t timeout;
+	u64 val;
+
+	/* Set address and stream-id */
+	val = readq_relaxed(tbu->base + DEBUG_SID_HALT_REG);
+	val &= ~DEBUG_SID_HALT_SID;
+	val |= FIELD_PREP(DEBUG_SID_HALT_SID, sid);
+	writeq_relaxed(val, tbu->base + DEBUG_SID_HALT_REG);
+	writeq_relaxed(iova, tbu->base + DEBUG_VA_ADDR_REG);
+	val = FIELD_PREP(DEBUG_AXUSER_CDMID, DEBUG_AXUSER_CDMID_VAL);
+	writeq_relaxed(val, tbu->base + DEBUG_AXUSER_REG);
+
+	/* Write-back read and write-allocate */
+	val = FIELD_PREP(DEBUG_TXN_AXCACHE, 0xf);
+
+	/* Non-secure access */
+	val |= FIELD_PREP(DEBUG_TXN_AXPROT, DEBUG_TXN_AXPROT_NSEC);
+
+	/* Privileged access */
+	val |= FIELD_PREP(DEBUG_TXN_AXPROT, DEBUG_TXN_AXPROT_PRIV);
+
+	val |= DEBUG_TXN_TRIGGER;
+	writeq_relaxed(val, tbu->base + DEBUG_TXN_TRIGG_REG);
+
+	timeout = ktime_add_us(ktime_get(), TBU_DBG_TIMEOUT_US);
+	for (;;) {
+		val = readl_relaxed(tbu->base + DEBUG_SR_HALT_ACK_REG);
+		if (!(val & DEBUG_SR_ECATS_RUNNING_VAL))
+			break;
+		val = readl_relaxed(tbu->base + DEBUG_PAR_REG);
+		if (val & DEBUG_PAR_FAULT_VAL)
+			break;
+		if (ktime_compare(ktime_get(), timeout) > 0) {
+			atos_timedout = true;
+			break;
+		}
+	}
+
+	val = readq_relaxed(tbu->base + DEBUG_PAR_REG);
+	if (val & DEBUG_PAR_FAULT_VAL)
+		dev_err(tbu->dev, "ATOS generated a fault interrupt! PAR = %llx, SID=0x%x\n",
+			val, sid);
+	else if (atos_timedout)
+		dev_err_ratelimited(tbu->dev, "ATOS translation timed out!\n");
+	else
+		phys = FIELD_GET(DEBUG_PAR_PA, val);
+
+	/* Reset hardware */
+	writeq_relaxed(0, tbu->base + DEBUG_TXN_TRIGG_REG);
+	writeq_relaxed(0, tbu->base + DEBUG_VA_ADDR_REG);
+	val = readl_relaxed(tbu->base + DEBUG_SID_HALT_REG);
+	val &= ~DEBUG_SID_HALT_SID;
+	writel_relaxed(val, tbu->base + DEBUG_SID_HALT_REG);
+
+	return phys;
+}
+
+static phys_addr_t qcom_iova_to_phys(struct arm_smmu_domain *smmu_domain,
+				     dma_addr_t iova, u32 sid)
+{
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+	int idx = smmu_domain->cfg.cbndx;
+	struct qcom_tbu *tbu;
+	u32 sctlr_orig, sctlr;
+	phys_addr_t phys = 0;
+	unsigned long flags;
+	int attempt = 0;
+	int ret;
+	u64 fsr;
+
+	tbu = qcom_find_tbu(qsmmu, sid);
+	if (!tbu)
+		return 0;
+
+	ret = icc_set_bw(tbu->path, 0, UINT_MAX);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(tbu->clk);
+	if (ret)
+		goto disable_icc;
+
+	ret = qcom_tbu_halt(tbu, smmu_domain);
+	if (ret)
+		goto disable_clk;
+
+	/*
+	 * ATOS/ECATS can trigger the fault interrupt, so disable it temporarily
+	 * and check for an interrupt manually.
+	 */
+	sctlr_orig = arm_smmu_cb_read(smmu, idx, ARM_SMMU_CB_SCTLR);
+	sctlr = sctlr_orig & ~(ARM_SMMU_SCTLR_CFCFG | ARM_SMMU_SCTLR_CFIE);
+	arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, sctlr);
+
+	fsr = arm_smmu_cb_read(smmu, idx, ARM_SMMU_CB_FSR);
+	if (fsr & ARM_SMMU_FSR_FAULT) {
+		/* Clear pending interrupts */
+		arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_FSR, fsr);
+
+		/*
+		 * TBU halt takes care of resuming any stalled transcation.
+		 * Kept it here for completeness sake.
+		 */
+		if (fsr & ARM_SMMU_FSR_SS)
+			arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_RESUME,
+					  ARM_SMMU_RESUME_TERMINATE);
+	}
+
+	/* Only one concurrent atos operation */
+	spin_lock_irqsave(&atos_lock, flags);
+
+	/*
+	 * If the translation fails, attempt the lookup more time."
+	 */
+	do {
+		phys = qcom_tbu_trigger_atos(smmu_domain, tbu, iova, sid);
+
+		fsr = arm_smmu_cb_read(smmu, idx, ARM_SMMU_CB_FSR);
+		if (fsr & ARM_SMMU_FSR_FAULT) {
+			/* Clear pending interrupts */
+			arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_FSR, fsr);
+
+			if (fsr & ARM_SMMU_FSR_SS)
+				arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_RESUME,
+						  ARM_SMMU_RESUME_TERMINATE);
+		}
+	} while (!phys && attempt++ < 2);
+
+	arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_SCTLR, sctlr_orig);
+	spin_unlock_irqrestore(&atos_lock, flags);
+	qcom_tbu_resume(tbu);
+
+	/* Read to complete prior write transcations */
+	readl_relaxed(tbu->base + DEBUG_SR_HALT_ACK_REG);
+
+disable_clk:
+	clk_disable_unprepare(tbu->clk);
+disable_icc:
+	icc_set_bw(tbu->path, 0, 0);
+
+	return phys;
+}
+
+static int qcom_tbu_probe(struct platform_device *pdev)
+{
+	struct of_phandle_args args = { .args_count = 2 };
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct qcom_tbu *tbu;
+
+	tbu = devm_kzalloc(dev, sizeof(*tbu), GFP_KERNEL);
+	if (!tbu)
+		return -ENOMEM;
+
+	tbu->dev = dev;
+	INIT_LIST_HEAD(&tbu->list);
+	spin_lock_init(&tbu->halt_lock);
+
+	if (of_parse_phandle_with_args(np, "qcom,stream-id-range", "#iommu-cells", 0, &args)) {
+		dev_err(dev, "Cannot parse the 'qcom,stream-id-range' DT property\n");
+		return -EINVAL;
+	}
+
+	tbu->smmu_np =  args.np;
+	tbu->sid_range[0] = args.args[0];
+	tbu->sid_range[1] = args.args[1];
+	of_node_put(args.np);
+
+	tbu->base = devm_of_iomap(dev, np, 0, NULL);
+	if (IS_ERR(tbu->base))
+		return PTR_ERR(tbu->base);
+
+	tbu->clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(tbu->clk))
+		return PTR_ERR(tbu->clk);
+
+	tbu->path = devm_of_icc_get(dev, NULL);
+	if (IS_ERR(tbu->path))
+		return PTR_ERR(tbu->path);
+
+	mutex_lock(&tbu_list_lock);
+	list_add_tail(&tbu->list, &tbu_list);
+	mutex_unlock(&tbu_list_lock);
+
+	return 0;
+}
+
+static const struct of_device_id qcom_tbu_of_match[] = {
+	{ .compatible = "qcom,sc7280-tbu" },
+	{ .compatible = "qcom,sdm845-tbu" },
+	{ }
+};
+
+static struct platform_driver qcom_tbu_driver = {
+	.driver = {
+		.name           = "qcom_tbu",
+		.of_match_table = qcom_tbu_of_match,
+	},
+	.probe = qcom_tbu_probe,
+};
+builtin_platform_driver(qcom_tbu_driver);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
index 593910567b88..9bb3ae7d62da 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
@@ -30,6 +30,8 @@ struct qcom_smmu_match_data {
 	const struct arm_smmu_impl *adreno_impl;
 };
 
+irqreturn_t qcom_smmu_context_fault(int irq, void *dev);
+
 #ifdef CONFIG_ARM_SMMU_QCOM_DEBUG
 void qcom_smmu_tlb_sync_debug(struct arm_smmu_device *smmu);
 #else
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 836ed6799a80..1670e95c4637 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -136,6 +136,7 @@ enum arm_smmu_cbar_type {
 #define ARM_SMMU_CBAR_VMID		GENMASK(7, 0)
 
 #define ARM_SMMU_GR1_CBFRSYNRA(n)	(0x400 + ((n) << 2))
+#define ARM_SMMU_CBFRSYNRA_SID		GENMASK(15, 0)
 
 #define ARM_SMMU_GR1_CBA2R(n)		(0x800 + ((n) << 2))
 #define ARM_SMMU_CBA2R_VMID16		GENMASK(31, 16)
@@ -238,6 +239,7 @@ enum arm_smmu_cbar_type {
 #define ARM_SMMU_CB_ATSR		0x8f0
 #define ARM_SMMU_ATSR_ACTIVE		BIT(0)
 
+#define ARM_SMMU_RESUME_TERMINATE	BIT(0)
 
 /* Maximum number of context banks per SMMU */
 #define ARM_SMMU_MAX_CBS		128

_______________________________________________
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 v7 5/7] arm64: dts: qcom: sdm845: Add DT nodes for the TBUs
From: Georgi Djakov @ 2024-03-29 21:06 UTC (permalink / raw)
  To: will, robin.murphy, joro, iommu
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree, andersson,
	konrad.dybcio, robdclark, linux-arm-kernel, linux-kernel,
	linux-arm-msm, quic_cgoldswo, quic_sukadev, quic_pdaly,
	quic_sudaraja, djakov
In-Reply-To: <20240329210638.3647523-1-quic_c_gdjako@quicinc.com>

Add the device-tree nodes for the TBUs (translation buffer units) that
are present on the sdm845 platforms. The TBUs can be used debug the
kernel and provide additional information when a context faults occur.

Describe the all registers, clocks, interconnects and power-domain
resources that are needed for each of the TBUs.

Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 70 ++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 2f20be99ee7e..381537f03fae 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -15,6 +15,7 @@
 #include <dt-bindings/dma/qcom-gpi.h>
 #include <dt-bindings/firmware/qcom,scm.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interconnect/qcom,icc.h>
 #include <dt-bindings/interconnect/qcom,osm-l3.h>
 #include <dt-bindings/interconnect/qcom,sdm845.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -5085,6 +5086,75 @@ apps_smmu: iommu@15000000 {
 				     <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		anoc_1_tbu: tbu@150c5000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150c5000 0x0 0x1000>;
+			interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_TBU1_GDSC>;
+			qcom,stream-id-range = <&apps_smmu 0x0 0x400>;
+		};
+
+		anoc_2_tbu: tbu@150c9000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150c9000 0x0 0x1000>;
+			interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_TBU2_GDSC>;
+			qcom,stream-id-range = <&apps_smmu 0x400 0x400>;
+		};
+
+		mnoc_hf_0_tbu: tbu@150cd000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150cd000 0x0 0x1000>;
+			interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ACTIVE_ONLY
+					 &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>;
+			qcom,stream-id-range = <&apps_smmu 0x800 0x400>;
+		};
+
+		mnoc_hf_1_tbu: tbu@150d1000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150d1000 0x0 0x1000>;
+			interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ACTIVE_ONLY
+					 &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>;
+			qcom,stream-id-range = <&apps_smmu 0xc00 0x400>;
+		};
+
+		mnoc_sf_0_tbu: tbu@150d5000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150d5000 0x0 0x1000>;
+			interconnects = <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ACTIVE_ONLY
+					 &mmss_noc SLAVE_MNOC_SF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>;
+			qcom,stream-id-range = <&apps_smmu 0x1000 0x400>;
+		};
+
+		compute_dsp_tbu: tbu@150d9000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150d9000 0x0 0x1000>;
+			interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			qcom,stream-id-range = <&apps_smmu 0x1400 0x400>;
+		};
+
+		adsp_tbu: tbu@150dd000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150dd000 0x0 0x1000>;
+			interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_AUDIO_TBU_GDSC>;
+			qcom,stream-id-range = <&apps_smmu 0x1800 0x400>;
+		};
+
+		anoc_1_pcie_tbu: tbu@150e1000 {
+			compatible = "qcom,sdm845-tbu";
+			reg = <0x0 0x150e1000 0x0 0x1000>;
+			clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>;
+			interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>;
+			qcom,stream-id-range = <&apps_smmu 0x1c00 0x400>;
+		};
+
 		lpasscc: clock-controller@17014000 {
 			compatible = "qcom,sdm845-lpasscc";
 			reg = <0 0x17014000 0 0x1f004>, <0 0x17300000 0 0x200>;

_______________________________________________
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 v7 0/7] Add support for Translation Buffer Units
From: Georgi Djakov @ 2024-03-29 21:06 UTC (permalink / raw)
  To: will, robin.murphy, joro, iommu
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree, andersson,
	konrad.dybcio, robdclark, linux-arm-kernel, linux-kernel,
	linux-arm-msm, quic_cgoldswo, quic_sukadev, quic_pdaly,
	quic_sudaraja, djakov

The TCUs (Translation Control Units) and TBUs (Translation Buffer
Units) are key components of the MMU-500. Multiple TBUs are connected
to a single TCU over an interconnect. Each TBU contains a TLB that
caches page tables. The MMU-500 implements a TBU for each connected
master, and the TBU is designed, so that it is local to the master.
A common TBU DT schema is added to describe the TBUs.

The Qualcomm SDM845 and SC7280 platforms have an implementation of the
SMMU-500, that has multiple TBUs. A vendor-specific DT schema is added
to describe the resources for each TBU (register space, power-domains,
interconnects and clocks).

The TBU driver will manage the resources and allow the system to
operate the TBUs during a context fault to obtain details by doing
s1 inv, software + hardware page table walks etc. This is implemented
with ATOS/eCATs as the ATS feature is not supported. Being able to
query the TBUs is useful for debugging various hardware/software
issues on these platforms.

v7:
- Pick Reviewed-by for the DT binding (Rob)
- Don't spam the log if the dts changes are not there (Konrad)

v6: https://lore.kernel.org/r/20240307190525.395291-1-quic_c_gdjako@quicinc.com/
- Use SoC-specific compatibles (Krzysztof)
- Use additionalProperties: false (Krzysztof)
- Wrap description text to 80 cols (Krzysztof)

v5: https://lore.kernel.org/r/20240226172218.69486-1-quic_c_gdjako@quicinc.com
- Drop the common TBU bindings and child nodes. These TBU functionalities
  are only Qualcomm specific and not generic. In the unmodified ARM MMU-500
  implementation there are no TBU-specific resources, so just make them
  standalone DT nodes. (Robin)
- The "qcom,stream-id-range" DT property now takes a phandle to the smmu
  and a stream ID range.

v4: https://lore.kernel.org/r/20240201210529.7728-1-quic_c_gdjako@quicinc.com/
- Create a common TBU schema. Move the vendor-specific properties into
  a separate schema that references the common one. (Rob)
- Drop unused DT labels in example, fix regex. (Rob)
- Properly rebase on latest code.

v3: https://lore.kernel.org/r/20231220060236.18600-1-quic_c_gdjako@quicinc.com
- Having a TBU is not Qualcomm specific, so allow having TBU child
  nodes with no specific constraints on properties. For some of the
  vendor compatibles however, add a schema to describe specific
  properties and allow validation. (Rob)
- Drop the useless reg-names DT property on TBUs. (Rob)
- Make the stream-id-range DT property a common one. (Rob)
- Fix the DT example. (Rob)
- Minor fixes on the TBU driver.
- Add support for SC7280 platforms.

v2: https://lore.kernel.org/r/20231118042730.2799-1-quic_c_gdjako@quicinc.com
- Improve DT binding description, add full example. (Konrad)
- Drop Qcom specific stuff from the generic binding. (Rob)
- Unconditionally try to populate subnodes. (Konrad)
- Improve TBU driver commit text, remove memory barriers. (Bjorn)
- Move TBU stuff into separate file. Make the driver builtin.
- TODO: Evaluate whether to keep TBU support as a separate driver
  or just instantiate things from qcom_smmu_impl_init()

v1: https://lore.kernel.org/r/20231019021923.13939-1-quic_c_gdjako@quicinc.com

Georgi Djakov (7):
  dt-bindings: iommu: Add Qualcomm TBU
  iommu/arm-smmu-qcom-tbu: Add Qualcomm TBU driver
  iommu/arm-smmu: Allow using a threaded handler for context interrupts
  iommu/arm-smmu-qcom: Use a custom context fault handler for sdm845
  arm64: dts: qcom: sdm845: Add DT nodes for the TBUs
  iommu/arm-smmu-qcom: Use the custom fault handler on more platforms
  arm64: dts: qcom: sc7280: Add DT nodes for the TBUs

 .../devicetree/bindings/iommu/qcom,tbu.yaml   |  69 +++
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |  89 +++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |  70 +++
 drivers/iommu/Kconfig                         |   9 +
 drivers/iommu/arm/arm-smmu/Makefile           |   1 +
 .../iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c    | 515 ++++++++++++++++++
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c    |   8 +
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h    |   2 +
 drivers/iommu/arm/arm-smmu/arm-smmu.c         |  12 +-
 drivers/iommu/arm/arm-smmu/arm-smmu.h         |   3 +
 10 files changed, 776 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/qcom,tbu.yaml
 create mode 100644 drivers/iommu/arm/arm-smmu/arm-smmu-qcom-tbu.c


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

^ permalink raw reply

* [PATCH v1 3/4] arm64: dts: amlogic: a1: add new input clock 'sys_pll_div16' to clkc_periphs
From: Dmitry Rokosov @ 2024-03-29 21:04 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov
In-Reply-To: <20240329210453.27530-1-ddrokosov@salutedevices.com>

The input clock 'sys_pll_div16' is a clock with a fixed ratio inherited
from the main system PLL.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 07fd0be828d4..0de809f4d42c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -747,10 +747,12 @@ clkc_periphs: clock-controller@800 {
 					 <&clkc_pll CLKID_FCLK_DIV5>,
 					 <&clkc_pll CLKID_FCLK_DIV7>,
 					 <&clkc_pll CLKID_HIFI_PLL>,
+					 <&clkc_pll CLKID_SYS_PLL_DIV16>,
 					 <&xtal>;
 				clock-names = "fclk_div2", "fclk_div3",
 					      "fclk_div5", "fclk_div7",
-					      "hifi_pll", "xtal";
+					      "hifi_pll", "sys_pll_div16",
+					      "xtal";
 			};
 
 			i2c0: i2c@1400 {
-- 
2.43.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 v1 0/4] arm64: dts: amlogic: a1: Support CPU Power Management
From: Dmitry Rokosov @ 2024-03-29 21:04 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov

The Amlogic A1 SoC family utilizes static operating points and a
PWM-controlled core voltage regulator that is specific to the board. As
the main CPU clock input, the SoC uses CLKID_CPU_CLK from the CPU clock
controller, which can be inherited from the system PLL (syspll) or a
fixed CPU clock.

Currently, the stable operating points at all frequencies are set to
800mV. This value is obtained from the vendor setup of several A1
boards.

The current patch series includes:
    * CPU clock controller declaration
    * syspll setup in the PLL controller
    * operating points
    * CPU special power parameters: voltage-tolerance, clock-latency,
      capacity-dmips-mhz, dynamic-power-coefficient

Please be informed that the AD402 vddcpu PWM regulator does not exist in
this patch series because currently PWM A1 support is under development.
However, it should look like:

```
vddcpu: regulator-vddcpu {
	compatible = "pwm-regulator";
	pinctrl-0 = <&pwm_f_pins4>;
	pinctrl-names = "default";
	regulator-name = "VDDCPU";
	regulator-min-microvolt = <690000>;
	regulator-max-microvolt = <1050000>;
	pwm-supply = <&dc_12v_in>;
	pwms = <&pwm_ef 1 1500 0>; // 667kHz
	voltage-table = <1050000 0>,
			<1040000 3>,
			<1030000 6>,
			<1020000 8>,
			<1010000 11>,
			<1000000 14>,
			<990000 17>,
			<980000 20>,
			<970000 23>,
			<960000 26>,
			<950000 29>,
			<940000 31>,
			<930000 34>,
			<920000 37>,
			<910000 40>,
			<900000 43>,
			<890000 45>,
			<880000 48>,
			<870000 51>,
			<860000 54>,
			<850000 56>,
			<840000 59>,
			<830000 62>,
			<820000 65>,
			<810000 68>,
			<800000 70>,
			<790000 73>,
			<780000 76>,
			<770000 79>,
			<760000 81>,
			<750000 84>,
			<740000 87>,
			<730000 89>,
			<720000 92>,
			<710000 95>,
			<700000 98>,
			<690000 100>;
	regulator-boot-on;
	regulator-always-on;
};
```

This patch series depends on [1].

Links:
    [1] https://lore.kernel.org/all/20240329205904.25002-1-ddrokosov@salutedevices.com/

Dmitry Rokosov (4):
  arm64: dts: amlogic: a1: add new syspll_in input for clkc_pll
    controller
  arm64: dts: amlogic: a1: declare cpu clock controller
  arm64: dts: amlogic: a1: add new input clock 'sys_pll_div16' to
    clkc_periphs
  arm64: dts: amlogic: a1: setup CPU power management

 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 68 ++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 3 deletions(-)

-- 
2.43.0


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

^ permalink raw reply

* [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock controller driver
From: Dmitry Rokosov @ 2024-03-29 20:58 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov
In-Reply-To: <20240329205904.25002-1-ddrokosov@salutedevices.com>

The CPU clock controller plays a general role in the Amlogic A1 SoC
family by generating CPU clocks. As an APB slave module, it offers the
capability to inherit the CPU clock from two sources: the internal fixed
clock known as 'cpu fixed clock' and the external input provided by the
A1 PLL clock controller, referred to as 'syspll'.

It is important for the driver to handle cpu_clk rate switching
effectively by transitioning to the CPU fixed clock to avoid any
potential execution freezes.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/clk/meson/Kconfig  |  10 ++
 drivers/clk/meson/Makefile |   1 +
 drivers/clk/meson/a1-cpu.c | 324 +++++++++++++++++++++++++++++++++++++
 drivers/clk/meson/a1-cpu.h |  16 ++
 4 files changed, 351 insertions(+)
 create mode 100644 drivers/clk/meson/a1-cpu.c
 create mode 100644 drivers/clk/meson/a1-cpu.h

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 80c4a18c83d2..148d4495eee3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -111,6 +111,16 @@ config COMMON_CLK_AXG_AUDIO
 	  Support for the audio clock controller on AmLogic A113D devices,
 	  aka axg, Say Y if you want audio subsystem to work.
 
+config COMMON_CLK_A1_CPU
+	tristate "Amlogic A1 SoC CPU controller support"
+	depends on ARM64
+	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_MESON_CLKC_UTILS
+	help
+	  Support for the CPU clock controller on Amlogic A113L based
+	  device, A1 SoC Family. Say Y if you want A1 CPU clock controller
+	  to work.
+
 config COMMON_CLK_A1_PLL
 	tristate "Amlogic A1 SoC PLL controller support"
 	depends on ARM64
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 4968fc7ad555..2a06eb0303d6 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_AUDIO_RSTC) += meson-audio-rstc.o
 
 obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
 obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
+obj-$(CONFIG_COMMON_CLK_A1_CPU) += a1-cpu.o
 obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
 obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
 obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
diff --git a/drivers/clk/meson/a1-cpu.c b/drivers/clk/meson/a1-cpu.c
new file mode 100644
index 000000000000..5f5d8ae112e5
--- /dev/null
+++ b/drivers/clk/meson/a1-cpu.c
@@ -0,0 +1,324 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic A1 SoC family CPU Clock Controller driver.
+ *
+ * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
+ * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include "a1-cpu.h"
+#include "clk-regmap.h"
+#include "meson-clkc-utils.h"
+
+#include <dt-bindings/clock/amlogic,a1-cpu-clkc.h>
+
+static u32 cpu_fsource_sel_table[] = { 0, 1, 2 };
+static const struct clk_parent_data cpu_fsource_sel_parents[] = {
+	{ .fw_name = "xtal" },
+	{ .fw_name = "fclk_div2" },
+	{ .fw_name = "fclk_div3" },
+};
+
+static struct clk_regmap cpu_fsource_sel0 = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.mask = 0x3,
+		.shift = 0,
+		.table = cpu_fsource_sel_table,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fsource_sel0",
+		.ops = &clk_regmap_mux_ops,
+		.parent_data = cpu_fsource_sel_parents,
+		.num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_fsource_div0 = {
+	.data = &(struct clk_regmap_div_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.shift = 4,
+		.width = 6,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fsource_div0",
+		.ops = &clk_regmap_divider_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&cpu_fsource_sel0.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_fsel0 = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.mask = 0x1,
+		.shift = 2,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fsel0",
+		.ops = &clk_regmap_mux_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&cpu_fsource_sel0.hw,
+			&cpu_fsource_div0.hw,
+		},
+		.num_parents = 2,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_fsource_sel1 = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.mask = 0x3,
+		.shift = 16,
+		.table = cpu_fsource_sel_table,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fsource_sel1",
+		.ops = &clk_regmap_mux_ops,
+		.parent_data = cpu_fsource_sel_parents,
+		.num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_fsource_div1 = {
+	.data = &(struct clk_regmap_div_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.shift = 20,
+		.width = 6,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fsource_div1",
+		.ops = &clk_regmap_divider_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&cpu_fsource_sel1.hw
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_fsel1 = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.mask = 0x1,
+		.shift = 18,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fsel1",
+		.ops = &clk_regmap_mux_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&cpu_fsource_sel1.hw,
+			&cpu_fsource_div1.hw,
+		},
+		.num_parents = 2,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_fclk = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.mask = 0x1,
+		.shift = 10,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_fclk",
+		.ops = &clk_regmap_mux_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&cpu_fsel0.hw,
+			&cpu_fsel1.hw,
+		},
+		.num_parents = 2,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap cpu_clk = {
+	.data = &(struct clk_regmap_mux_data) {
+		.offset = CPUCTRL_CLK_CTRL0,
+		.mask = 0x1,
+		.shift = 11,
+	},
+	.hw.init = &(struct clk_init_data) {
+		.name = "cpu_clk",
+		.ops = &clk_regmap_mux_ops,
+		.parent_data = (const struct clk_parent_data []) {
+			{ .hw = &cpu_fclk.hw },
+			{ .fw_name = "sys_pll", },
+		},
+		.num_parents = 2,
+		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+	},
+};
+
+/* Array of all clocks registered by this provider */
+static struct clk_hw *a1_cpu_hw_clks[] = {
+	[CLKID_CPU_FSOURCE_SEL0]	= &cpu_fsource_sel0.hw,
+	[CLKID_CPU_FSOURCE_DIV0]	= &cpu_fsource_div0.hw,
+	[CLKID_CPU_FSEL0]		= &cpu_fsel0.hw,
+	[CLKID_CPU_FSOURCE_SEL1]	= &cpu_fsource_sel1.hw,
+	[CLKID_CPU_FSOURCE_DIV1]	= &cpu_fsource_div1.hw,
+	[CLKID_CPU_FSEL1]		= &cpu_fsel1.hw,
+	[CLKID_CPU_FCLK]		= &cpu_fclk.hw,
+	[CLKID_CPU_CLK]			= &cpu_clk.hw,
+};
+
+static struct clk_regmap *const a1_cpu_regmaps[] = {
+	&cpu_fsource_sel0,
+	&cpu_fsource_div0,
+	&cpu_fsel0,
+	&cpu_fsource_sel1,
+	&cpu_fsource_div1,
+	&cpu_fsel1,
+	&cpu_fclk,
+	&cpu_clk,
+};
+
+static struct regmap_config a1_cpu_regmap_cfg = {
+	.reg_bits   = 32,
+	.val_bits   = 32,
+	.reg_stride = 4,
+	.max_register = CPUCTRL_CLK_CTRL1,
+};
+
+static struct meson_clk_hw_data a1_cpu_clks = {
+	.hws = a1_cpu_hw_clks,
+	.num = ARRAY_SIZE(a1_cpu_hw_clks),
+};
+
+struct a1_cpu_clk_nb_data {
+	const struct clk_ops *mux_ops;
+	struct clk_hw *cpu_clk;
+	struct notifier_block nb;
+	u8 parent;
+};
+
+#define MESON_A1_CPU_CLK_GET_PARENT(nbd) \
+	((nbd)->mux_ops->get_parent((nbd)->cpu_clk))
+#define MESON_A1_CPU_CLK_SET_PARENT(nbd, index) \
+	((nbd)->mux_ops->set_parent((nbd)->cpu_clk, index))
+
+static int meson_a1_cpu_clk_notifier_cb(struct notifier_block *nb,
+					unsigned long event, void *data)
+{
+	struct a1_cpu_clk_nb_data *nbd;
+	int ret = 0;
+
+	nbd = container_of(nb, struct a1_cpu_clk_nb_data, nb);
+
+	switch (event) {
+	case PRE_RATE_CHANGE:
+		nbd->parent = MESON_A1_CPU_CLK_GET_PARENT(nbd);
+		/* Fallback to the CPU fixed clock */
+		ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, 0);
+		/* Wait for clock propagation */
+		udelay(100);
+		break;
+
+	case POST_RATE_CHANGE:
+	case ABORT_RATE_CHANGE:
+		/* Back to the original parent clock */
+		ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, nbd->parent);
+		/* Wait for clock propagation */
+		udelay(100);
+		break;
+
+	default:
+		pr_warn("Unknown event %lu for %s notifier block\n",
+			event, clk_hw_get_name(nbd->cpu_clk));
+		break;
+	}
+
+	return notifier_from_errno(ret);
+}
+
+static struct a1_cpu_clk_nb_data a1_cpu_clk_nb_data = {
+	.mux_ops = &clk_regmap_mux_ops,
+	.cpu_clk = &cpu_clk.hw,
+	.nb.notifier_call = meson_a1_cpu_clk_notifier_cb,
+};
+
+static int meson_a1_dvfs_setup(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct clk *notifier_clk;
+	int ret;
+
+	/* Setup clock notifier for cpu_clk */
+	notifier_clk = devm_clk_hw_get_clk(dev, &cpu_clk.hw, "dvfs");
+	if (IS_ERR(notifier_clk))
+		return dev_err_probe(dev, PTR_ERR(notifier_clk),
+				     "can't get cpu_clk as notifier clock\n");
+
+	ret = devm_clk_notifier_register(dev, notifier_clk,
+					 &a1_cpu_clk_nb_data.nb);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "can't register cpu_clk notifier\n");
+
+	return ret;
+}
+
+static int meson_a1_cpu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	void __iomem *base;
+	struct regmap *map;
+	int clkid, i, err;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return dev_err_probe(dev, PTR_ERR(base),
+				     "can't ioremap resource\n");
+
+	map = devm_regmap_init_mmio(dev, base, &a1_cpu_regmap_cfg);
+	if (IS_ERR(map))
+		return dev_err_probe(dev, PTR_ERR(map),
+				     "can't init regmap mmio region\n");
+
+	/* Populate regmap for the regmap backed clocks */
+	for (i = 0; i < ARRAY_SIZE(a1_cpu_regmaps); i++)
+		a1_cpu_regmaps[i]->map = map;
+
+	for (clkid = 0; clkid < a1_cpu_clks.num; clkid++) {
+		err = devm_clk_hw_register(dev, a1_cpu_clks.hws[clkid]);
+		if (err)
+			return dev_err_probe(dev, err,
+					     "clock[%d] registration failed\n",
+					     clkid);
+	}
+
+	err = devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, &a1_cpu_clks);
+	if (err)
+		return dev_err_probe(dev, err, "can't add clk hw provider\n");
+
+	return meson_a1_dvfs_setup(pdev);
+}
+
+static const struct of_device_id a1_cpu_clkc_match_table[] = {
+	{ .compatible = "amlogic,a1-cpu-clkc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, a1_cpu_clkc_match_table);
+
+static struct platform_driver a1_cpu_clkc_driver = {
+	.probe = meson_a1_cpu_probe,
+	.driver = {
+		.name = "a1-cpu-clkc",
+		.of_match_table = a1_cpu_clkc_match_table,
+	},
+};
+
+module_platform_driver(a1_cpu_clkc_driver);
+MODULE_AUTHOR("Dmitry Rokosov <ddrokosov@salutedevices.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/a1-cpu.h b/drivers/clk/meson/a1-cpu.h
new file mode 100644
index 000000000000..e9af4117e26f
--- /dev/null
+++ b/drivers/clk/meson/a1-cpu.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Amlogic A1 CPU Clock Controller internals
+ *
+ * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
+ * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
+ */
+
+#ifndef __A1_CPU_H
+#define __A1_CPU_H
+
+/* cpu clock controller register offset */
+#define CPUCTRL_CLK_CTRL0	0x80
+#define CPUCTRL_CLK_CTRL1	0x84
+
+#endif /* __A1_CPU_H */
-- 
2.43.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 v1 0/6] clk: meson: introduce Amlogic A1 SoC Family CPU clock controller driver
From: Dmitry Rokosov @ 2024-03-29 20:58 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov

The CPU clock controller plays a general role in the Amlogic A1 SoC
family by generating CPU clocks. As an APB slave module, it offers the
capability to inherit the CPU clock from two sources: the internal fixed
clock known as 'cpu fixed clock' and the external input provided by the
A1 PLL clock controller, referred to as 'syspll'.

It is important for the driver to handle the cpu_clk rate switching
effectively by transitioning to the CPU fixed clock to avoid any
potential execution freezes.

Validation:
* to double-check all clk flags, run the below helper script:

```
pushd /sys/kernel/debug/clk
for f in *; do
    if [[ -f "$f/clk_flags" ]]; then
        flags="$(cat $f/clk_flags | awk '{$1=$1};1' | sed ':a;N;$!ba;s/\n/ | /g')"
        echo -e "$f: $flags"
    fi
done
popd
```

* to trace the current clks state, use the
  '/sys/kernel/debug/clk/clk_dump' node with jq post-processing:

```
$ cat /sys/kernel/debug/clk/clk_dump | jq '.' > clk_dump.json
```

* to see the CPU clock hierarchy, use the
'/sys/kernel/debug/clk/clk_summary' node with jq post-processing:

```
$ cat /sys/kernel/debug/clk/clk_summary | jq '.' > clk_dump.json
```

when cpu_clk is inherited from sys_pll, it should be:

```
syspll_in    1  1  0  24000000    0  0  50000  Y  deviceless                 no_connection_id
  sys_pll    2  2  0  1200000000  0  0  50000  Y  deviceless                 no_connection_id
    cpu_clk  1  1  0  1200000000  0  0  50000  Y  cpu0                       no_connection_id
                                                  cpu0                       no_connection_id
                                                  fd000000.clock-controller  dvfs
                                                  deviceless                 no_connection_id
```

and from cpu fixed clock:

```
fclk_div3_div           1  1  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
  fclk_div3             4  4  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
    cpu_fsource_sel0    1  1  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
      cpu_fsource_div0  1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
        cpu_fsel0       1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
          cpu_fclk      1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
            cpu_clk     1  1  0  128000000  0  0  50000  Y  cpu0                       no_connection_id
                                                            cpu0                       no_connection_id
                                                            fd000000.clock-controller  dvfs
                                                            deviceless                 no_connection_id
```

* to debug cpu clk rate propagation and proper parent switching, compile
  kernel with the following definition:
    $ sed -i "s/undef CLOCK_ALLOW_WRITE_DEBUGFS/define CLOCK_ALLOW_WRITE_DEBUGFS/g" drivers/clk/clk.c
  after that, clk_rate debug node for each clock will be available for
  write operation

Dmitry Rokosov (6):
  dt-bindings: clock: meson: a1: pll: introduce new syspll bindings
  clk: meson: a1: pll: support 'syspll' general-purpose PLL for CPU
    clock
  dt-bindings: clock: meson: a1: peripherals: support sys_pll_div16
    input
  clk: meson: a1: peripherals: support 'sys_pll_div16' clock as GEN
    input
  dt-bindings: clock: meson: add A1 CPU clock controller bindings
  clk: meson: a1: add Amlogic A1 CPU clock controller driver

 .../bindings/clock/amlogic,a1-cpu-clkc.yaml   |  64 ++++
 .../clock/amlogic,a1-peripherals-clkc.yaml    |   5 +-
 .../bindings/clock/amlogic,a1-pll-clkc.yaml   |   7 +-
 drivers/clk/meson/Kconfig                     |  10 +
 drivers/clk/meson/Makefile                    |   1 +
 drivers/clk/meson/a1-cpu.c                    | 324 ++++++++++++++++++
 drivers/clk/meson/a1-cpu.h                    |  16 +
 drivers/clk/meson/a1-peripherals.c            |   4 +-
 drivers/clk/meson/a1-pll.c                    |  78 +++++
 drivers/clk/meson/a1-pll.h                    |   6 +
 .../dt-bindings/clock/amlogic,a1-cpu-clkc.h   |  19 +
 .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |   2 +
 12 files changed, 531 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
 create mode 100644 drivers/clk/meson/a1-cpu.c
 create mode 100644 drivers/clk/meson/a1-cpu.h
 create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h

-- 
2.43.0


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

^ permalink raw reply

* [PATCH v1 1/6] dt-bindings: clock: meson: a1: pll: introduce new syspll bindings
From: Dmitry Rokosov @ 2024-03-29 20:58 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov
In-Reply-To: <20240329205904.25002-1-ddrokosov@salutedevices.com>

The 'syspll' PLL is a general-purpose PLL designed specifically for the
CPU clock. It is capable of producing output frequencies within the
range of 768MHz to 1536MHz.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 .../devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml     | 7 +++++--
 include/dt-bindings/clock/amlogic,a1-pll-clkc.h            | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
index a59b188a8bf5..fbba57031278 100644
--- a/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
+++ b/Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
@@ -26,11 +26,13 @@ properties:
     items:
       - description: input fixpll_in
       - description: input hifipll_in
+      - description: input syspll_in
 
   clock-names:
     items:
       - const: fixpll_in
       - const: hifipll_in
+      - const: syspll_in
 
 required:
   - compatible
@@ -53,7 +55,8 @@ examples:
             reg = <0 0x7c80 0 0x18c>;
             #clock-cells = <1>;
             clocks = <&clkc_periphs CLKID_FIXPLL_IN>,
-                     <&clkc_periphs CLKID_HIFIPLL_IN>;
-            clock-names = "fixpll_in", "hifipll_in";
+                     <&clkc_periphs CLKID_HIFIPLL_IN>,
+                     <&clkc_periphs CLKID_SYSPLL_IN>;
+            clock-names = "fixpll_in", "hifipll_in", "syspll_in";
         };
     };
diff --git a/include/dt-bindings/clock/amlogic,a1-pll-clkc.h b/include/dt-bindings/clock/amlogic,a1-pll-clkc.h
index 2b660c0f2c9f..a702d610589c 100644
--- a/include/dt-bindings/clock/amlogic,a1-pll-clkc.h
+++ b/include/dt-bindings/clock/amlogic,a1-pll-clkc.h
@@ -21,5 +21,7 @@
 #define CLKID_FCLK_DIV5		8
 #define CLKID_FCLK_DIV7		9
 #define CLKID_HIFI_PLL		10
+#define CLKID_SYS_PLL		11
+#define CLKID_SYS_PLL_DIV16	12
 
 #endif /* __A1_PLL_CLKC_H */
-- 
2.43.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 v1 4/6] clk: meson: a1: peripherals: support 'sys_pll_div16' clock as GEN input
From: Dmitry Rokosov @ 2024-03-29 20:58 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov
In-Reply-To: <20240329205904.25002-1-ddrokosov@salutedevices.com>

The clock 'sys_pll_div16' is one of the parents of the GEN clock. It is
generated in the A1 PLL clock controller with a fixed factor.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/clk/meson/a1-peripherals.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/a1-peripherals.c b/drivers/clk/meson/a1-peripherals.c
index 621af1e6e4b2..3c4452f2b146 100644
--- a/drivers/clk/meson/a1-peripherals.c
+++ b/drivers/clk/meson/a1-peripherals.c
@@ -747,13 +747,13 @@ static struct clk_regmap fclk_div2_divn = {
 };
 
 /*
- * the index 2 is sys_pll_div16, it will be implemented in the CPU clock driver,
  * the index 4 is the clock measurement source, it's not supported yet
  */
-static u32 gen_table[] = { 0, 1, 3, 5, 6, 7, 8 };
+static u32 gen_table[] = { 0, 1, 2, 3, 5, 6, 7, 8 };
 static const struct clk_parent_data gen_parent_data[] = {
 	{ .fw_name = "xtal", },
 	{ .hw = &rtc.hw },
+	{ .fw_name = "sys_pll_div16", },
 	{ .fw_name = "hifi_pll", },
 	{ .fw_name = "fclk_div2", },
 	{ .fw_name = "fclk_div3", },
-- 
2.43.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 v1 2/6] clk: meson: a1: pll: support 'syspll' general-purpose PLL for CPU clock
From: Dmitry Rokosov @ 2024-03-29 20:58 UTC (permalink / raw)
  To: neil.armstrong, jbrunet, mturquette, sboyd, robh+dt,
	krzysztof.kozlowski+dt, khilman, martin.blumenstingl
  Cc: kernel, rockosov, linux-amlogic, linux-clk, devicetree,
	linux-kernel, linux-arm-kernel, Dmitry Rokosov
In-Reply-To: <20240329205904.25002-1-ddrokosov@salutedevices.com>

The 'syspll' PLL, also known as the system PLL, is a general and
essential PLL responsible for generating the CPU clock frequency.
With its wide-ranging capabilities, it is designed to accommodate
frequencies within the range of 768MHz to 1536MHz.

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
 drivers/clk/meson/a1-pll.h |  6 +++
 2 files changed, 84 insertions(+)

diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
index 60b2e53e7e51..02fd2d325cc6 100644
--- a/drivers/clk/meson/a1-pll.c
+++ b/drivers/clk/meson/a1-pll.c
@@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
 	},
 };
 
+static const struct pll_mult_range sys_pll_mult_range = {
+	.min = 32,
+	.max = 64,
+};
+
+/*
+ * We assume that the sys_pll_clk has already been set up by the low-level
+ * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
+ * run the initialization sequence.
+ */
+static struct clk_regmap sys_pll = {
+	.data = &(struct meson_clk_pll_data){
+		.en = {
+			.reg_off = ANACTRL_SYSPLL_CTRL0,
+			.shift   = 28,
+			.width   = 1,
+		},
+		.m = {
+			.reg_off = ANACTRL_SYSPLL_CTRL0,
+			.shift   = 0,
+			.width   = 8,
+		},
+		.n = {
+			.reg_off = ANACTRL_SYSPLL_CTRL0,
+			.shift   = 10,
+			.width   = 5,
+		},
+		.frac = {
+			.reg_off = ANACTRL_SYSPLL_CTRL1,
+			.shift   = 0,
+			.width   = 19,
+		},
+		.l = {
+			.reg_off = ANACTRL_SYSPLL_STS,
+			.shift   = 31,
+			.width   = 1,
+		},
+		.current_en = {
+			.reg_off = ANACTRL_SYSPLL_CTRL0,
+			.shift   = 26,
+			.width   = 1,
+		},
+		.l_detect = {
+			.reg_off = ANACTRL_SYSPLL_CTRL2,
+			.shift   = 6,
+			.width   = 1,
+		},
+		.range = &sys_pll_mult_range,
+	},
+	.hw.init = &(struct clk_init_data){
+		.name = "sys_pll",
+		.ops = &meson_clk_pll_ops,
+		.parent_names = (const char *[]){ "syspll_in" },
+		.num_parents = 1,
+		/*
+		 * This clock is used as the main CPU PLL source in low-level
+		 * bootloaders, and it is necessary to mark it as critical.
+		 */
+		.flags = CLK_IS_CRITICAL,
+	},
+};
+
+static struct clk_fixed_factor sys_pll_div16 = {
+	.mult = 1,
+	.div = 16,
+	.hw.init = &(struct clk_init_data){
+		.name = "sys_pll_div16",
+		.ops = &clk_fixed_factor_ops,
+		.parent_hws = (const struct clk_hw *[]) {
+			&sys_pll.hw
+		},
+		.num_parents = 1,
+	},
+};
+
 static struct clk_fixed_factor fclk_div2_div = {
 	.mult = 1,
 	.div = 2,
@@ -283,6 +358,8 @@ static struct clk_hw *a1_pll_hw_clks[] = {
 	[CLKID_FCLK_DIV5]	= &fclk_div5.hw,
 	[CLKID_FCLK_DIV7]	= &fclk_div7.hw,
 	[CLKID_HIFI_PLL]	= &hifi_pll.hw,
+	[CLKID_SYS_PLL]		= &sys_pll.hw,
+	[CLKID_SYS_PLL_DIV16]	= &sys_pll_div16.hw,
 };
 
 static struct clk_regmap *const a1_pll_regmaps[] = {
@@ -293,6 +370,7 @@ static struct clk_regmap *const a1_pll_regmaps[] = {
 	&fclk_div5,
 	&fclk_div7,
 	&hifi_pll,
+	&sys_pll,
 };
 
 static struct regmap_config a1_pll_regmap_cfg = {
diff --git a/drivers/clk/meson/a1-pll.h b/drivers/clk/meson/a1-pll.h
index 4be17b2bf383..666d9b2137e9 100644
--- a/drivers/clk/meson/a1-pll.h
+++ b/drivers/clk/meson/a1-pll.h
@@ -18,6 +18,12 @@
 #define ANACTRL_FIXPLL_CTRL0	0x0
 #define ANACTRL_FIXPLL_CTRL1	0x4
 #define ANACTRL_FIXPLL_STS	0x14
+#define ANACTRL_SYSPLL_CTRL0	0x80
+#define ANACTRL_SYSPLL_CTRL1	0x84
+#define ANACTRL_SYSPLL_CTRL2	0x88
+#define ANACTRL_SYSPLL_CTRL3	0x8c
+#define ANACTRL_SYSPLL_CTRL4	0x90
+#define ANACTRL_SYSPLL_STS	0x94
 #define ANACTRL_HIFIPLL_CTRL0	0xc0
 #define ANACTRL_HIFIPLL_CTRL1	0xc4
 #define ANACTRL_HIFIPLL_CTRL2	0xc8
-- 
2.43.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 v3 2/3] arch: Remove struct fb_info from video helpers
From: Thomas Zimmermann @ 2024-03-29 20:32 UTC (permalink / raw)
  To: arnd, sam, javierm, deller, sui.jingfeng
  Cc: linux-arch, dri-devel, linux-fbdev, sparclinux, linux-sh,
	linuxppc-dev, linux-parisc, linux-mips, linux-m68k, loongarch,
	linux-arm-kernel, linux-snps-arc, linux-kernel, Thomas Zimmermann,
	James E.J. Bottomley, David S. Miller, Andreas Larsson,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
In-Reply-To: <20240329203450.7824-1-tzimmermann@suse.de>

The per-architecture video helpers do not depend on struct fb_info
or anything else from fbdev. Remove it from the interface and replace
fb_is_primary_device() with video_is_primary_device(). The new helper
is similar in functionality, but can operate on non-fbdev devices.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andreas Larsson <andreas@gaisler.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/parisc/include/asm/fb.h     |  8 +++++---
 arch/parisc/video/fbdev.c        |  9 +++++----
 arch/sparc/include/asm/fb.h      |  7 ++++---
 arch/sparc/video/fbdev.c         | 17 ++++++++---------
 arch/x86/include/asm/fb.h        |  8 +++++---
 arch/x86/video/fbdev.c           | 18 +++++++-----------
 drivers/video/fbdev/core/fbcon.c |  2 +-
 include/asm-generic/fb.h         | 11 ++++++-----
 8 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h
index 658a8a7dc5312..ed2a195a3e762 100644
--- a/arch/parisc/include/asm/fb.h
+++ b/arch/parisc/include/asm/fb.h
@@ -2,11 +2,13 @@
 #ifndef _ASM_FB_H_
 #define _ASM_FB_H_
 
-struct fb_info;
+#include <linux/types.h>
+
+struct device;
 
 #if defined(CONFIG_STI_CORE)
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
 #endif
 
 #include <asm-generic/fb.h>
diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index e4f8ac99fc9e0..540fa0c919d59 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -5,12 +5,13 @@
  * Copyright (C) 2001-2002 Thomas Bogendoerfer <tsbogend@alpha.franken.de>
  */
 
-#include <linux/fb.h>
 #include <linux/module.h>
 
 #include <video/sticore.h>
 
-int fb_is_primary_device(struct fb_info *info)
+#include <asm/fb.h>
+
+bool video_is_primary_device(struct device *dev)
 {
 	struct sti_struct *sti;
 
@@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info)
 		return true;
 
 	/* return true if it's the default built-in framebuffer driver */
-	return (sti->dev == info->device);
+	return (sti->dev == dev);
 }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 24440c0fda490..07f0325d6921c 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -3,10 +3,11 @@
 #define _SPARC_FB_H_
 
 #include <linux/io.h>
+#include <linux/types.h>
 
 #include <asm/page.h>
 
-struct fb_info;
+struct device;
 
 #ifdef CONFIG_SPARC32
 static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
@@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
 #define pgprot_framebuffer pgprot_framebuffer
 #endif
 
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
 
 static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n)
 {
diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c
index bff66dd1909a4..e46f0499c2774 100644
--- a/arch/sparc/video/fbdev.c
+++ b/arch/sparc/video/fbdev.c
@@ -1,26 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/console.h>
-#include <linux/fb.h>
+#include <linux/device.h>
 #include <linux/module.h>
 
+#include <asm/fb.h>
 #include <asm/prom.h>
 
-int fb_is_primary_device(struct fb_info *info)
+bool video_is_primary_device(struct device *dev)
 {
-	struct device *dev = info->device;
-	struct device_node *node;
+	struct device_node *node = dev->of_node;
 
 	if (console_set_on_cmdline)
-		return 0;
+		return false;
 
-	node = dev->of_node;
 	if (node && node == of_console_device)
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
 
 MODULE_DESCRIPTION("Sparc fbdev helpers");
 MODULE_LICENSE("GPL");
diff --git a/arch/x86/include/asm/fb.h b/arch/x86/include/asm/fb.h
index c3b9582de7efd..999db33792869 100644
--- a/arch/x86/include/asm/fb.h
+++ b/arch/x86/include/asm/fb.h
@@ -2,17 +2,19 @@
 #ifndef _ASM_X86_FB_H
 #define _ASM_X86_FB_H
 
+#include <linux/types.h>
+
 #include <asm/page.h>
 
-struct fb_info;
+struct device;
 
 pgprot_t pgprot_framebuffer(pgprot_t prot,
 			    unsigned long vm_start, unsigned long vm_end,
 			    unsigned long offset);
 #define pgprot_framebuffer pgprot_framebuffer
 
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
 
 #include <asm-generic/fb.h>
 
diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 1dd6528cc947c..4d87ce8e257fe 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -7,7 +7,6 @@
  *
  */
 
-#include <linux/fb.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/vgaarb.h>
@@ -25,20 +24,17 @@ pgprot_t pgprot_framebuffer(pgprot_t prot,
 }
 EXPORT_SYMBOL(pgprot_framebuffer);
 
-int fb_is_primary_device(struct fb_info *info)
+bool video_is_primary_device(struct device *dev)
 {
-	struct device *device = info->device;
-	struct pci_dev *pci_dev;
+	struct pci_dev *pdev;
 
-	if (!device || !dev_is_pci(device))
-		return 0;
+	if (!dev_is_pci(dev))
+		return false;
 
-	pci_dev = to_pci_dev(device);
+	pdev = to_pci_dev(dev);
 
-	if (pci_dev == vga_default_device())
-		return 1;
-	return 0;
+	return (pdev == vga_default_device());
 }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index fcabc668e9fbe..3f7333dca508c 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2907,7 +2907,7 @@ void fbcon_remap_all(struct fb_info *info)
 static void fbcon_select_primary(struct fb_info *info)
 {
 	if (!map_override && primary_device == -1 &&
-	    fb_is_primary_device(info)) {
+	    video_is_primary_device(info->device)) {
 		int i;
 
 		printk(KERN_INFO "fbcon: %s (fb%i) is primary device\n",
diff --git a/include/asm-generic/fb.h b/include/asm-generic/fb.h
index 6ccabb400aa66..4788c1e1c6bc0 100644
--- a/include/asm-generic/fb.h
+++ b/include/asm-generic/fb.h
@@ -10,8 +10,9 @@
 #include <linux/io.h>
 #include <linux/mm_types.h>
 #include <linux/pgtable.h>
+#include <linux/types.h>
 
-struct fb_info;
+struct device;
 
 #ifndef pgprot_framebuffer
 #define pgprot_framebuffer pgprot_framebuffer
@@ -23,11 +24,11 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
 }
 #endif
 
-#ifndef fb_is_primary_device
-#define fb_is_primary_device fb_is_primary_device
-static inline int fb_is_primary_device(struct fb_info *info)
+#ifndef video_is_primary_device
+#define video_is_primary_device video_is_primary_device
+static inline bool video_is_primary_device(struct device *dev)
 {
-	return 0;
+	return false;
 }
 #endif
 
-- 
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 v3 0/3] arch: Remove fbdev dependency from video helpers
From: Thomas Zimmermann @ 2024-03-29 20:32 UTC (permalink / raw)
  To: arnd, sam, javierm, deller, sui.jingfeng
  Cc: linux-arch, dri-devel, linux-fbdev, sparclinux, linux-sh,
	linuxppc-dev, linux-parisc, linux-mips, linux-m68k, loongarch,
	linux-arm-kernel, linux-snps-arc, linux-kernel, Thomas Zimmermann

Make architecture helpers for display functionality depend on general
video functionality instead of fbdev. This avoids the dependency on
fbdev and makes the functionality available for non-fbdev code.

Patch 1 replaces the variety of Kconfig options that control the
Makefiles with CONFIG_VIDEO. More fine-grained control of the build
can then be done within each video/ directory; see parisc for an
example.

Patch 2 replaces fb_is_primary_device() with video_is_primary_device(),
which has no dependencies on fbdev. The implementation remains identical
on all affected platforms. There's one minor change in fbcon, which is
the only caller of fb_is_primary_device().

Patch 3 renames the source and header files from fbdev to video.

v3:
- arc, arm, arm64, sh, um: generate asm/video.h (Sam, Helge, Arnd)
- fix typos (Sam)
v2:
- improve cover letter
- rebase onto v6.9-rc1

Thomas Zimmermann (3):
  arch: Select fbdev helpers with CONFIG_VIDEO
  arch: Remove struct fb_info from video helpers
  arch: Rename fbdev header and source files

 arch/arc/include/asm/fb.h                    |  8 ------
 arch/arm/include/asm/fb.h                    |  6 -----
 arch/arm64/include/asm/fb.h                  | 10 --------
 arch/loongarch/include/asm/{fb.h => video.h} |  8 +++---
 arch/m68k/include/asm/{fb.h => video.h}      |  8 +++---
 arch/mips/include/asm/{fb.h => video.h}      | 12 ++++-----
 arch/parisc/Makefile                         |  2 +-
 arch/parisc/include/asm/fb.h                 | 14 -----------
 arch/parisc/include/asm/video.h              | 16 ++++++++++++
 arch/parisc/video/Makefile                   |  2 +-
 arch/parisc/video/{fbdev.c => video-sti.c}   |  9 ++++---
 arch/powerpc/include/asm/{fb.h => video.h}   |  8 +++---
 arch/powerpc/kernel/pci-common.c             |  2 +-
 arch/sh/include/asm/fb.h                     |  7 ------
 arch/sparc/Makefile                          |  4 +--
 arch/sparc/include/asm/{fb.h => video.h}     | 15 +++++------
 arch/sparc/video/Makefile                    |  2 +-
 arch/sparc/video/fbdev.c                     | 26 --------------------
 arch/sparc/video/video.c                     | 25 +++++++++++++++++++
 arch/um/include/asm/Kbuild                   |  2 +-
 arch/x86/Makefile                            |  2 +-
 arch/x86/include/asm/fb.h                    | 19 --------------
 arch/x86/include/asm/video.h                 | 21 ++++++++++++++++
 arch/x86/video/Makefile                      |  3 ++-
 arch/x86/video/{fbdev.c => video.c}          | 21 +++++++---------
 drivers/video/fbdev/core/fbcon.c             |  2 +-
 include/asm-generic/Kbuild                   |  2 +-
 include/asm-generic/{fb.h => video.h}        | 17 +++++++------
 include/linux/fb.h                           |  2 +-
 29 files changed, 124 insertions(+), 151 deletions(-)
 delete mode 100644 arch/arc/include/asm/fb.h
 delete mode 100644 arch/arm/include/asm/fb.h
 delete mode 100644 arch/arm64/include/asm/fb.h
 rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
 rename arch/m68k/include/asm/{fb.h => video.h} (86%)
 rename arch/mips/include/asm/{fb.h => video.h} (76%)
 delete mode 100644 arch/parisc/include/asm/fb.h
 create mode 100644 arch/parisc/include/asm/video.h
 rename arch/parisc/video/{fbdev.c => video-sti.c} (78%)
 rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
 delete mode 100644 arch/sh/include/asm/fb.h
 rename arch/sparc/include/asm/{fb.h => video.h} (75%)
 delete mode 100644 arch/sparc/video/fbdev.c
 create mode 100644 arch/sparc/video/video.c
 delete mode 100644 arch/x86/include/asm/fb.h
 create mode 100644 arch/x86/include/asm/video.h
 rename arch/x86/video/{fbdev.c => video.c} (66%)
 rename include/asm-generic/{fb.h => video.h} (89%)

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

* [PATCH v3 1/3] arch: Select fbdev helpers with CONFIG_VIDEO
From: Thomas Zimmermann @ 2024-03-29 20:32 UTC (permalink / raw)
  To: arnd, sam, javierm, deller, sui.jingfeng
  Cc: linux-arch, dri-devel, linux-fbdev, sparclinux, linux-sh,
	linuxppc-dev, linux-parisc, linux-mips, linux-m68k, loongarch,
	linux-arm-kernel, linux-snps-arc, linux-kernel, Thomas Zimmermann,
	James E.J. Bottomley, David S. Miller, Andreas Larsson,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
In-Reply-To: <20240329203450.7824-1-tzimmermann@suse.de>

Various Kconfig options selected the per-architecture helpers for
fbdev. But none of the contained code depends on fbdev. Standardize
on CONFIG_VIDEO, which will allow to add more general helpers for
video functionality.

CONFIG_VIDEO protects each architecture's video/ directory. This
allows for the use of more fine-grained control for each directory's
files, such as the use of CONFIG_STI_CORE on parisc.

v2:
- sparc: rebased onto Makefile changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andreas Larsson <andreas@gaisler.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/parisc/Makefile      | 2 +-
 arch/sparc/Makefile       | 4 ++--
 arch/sparc/video/Makefile | 2 +-
 arch/x86/Makefile         | 2 +-
 arch/x86/video/Makefile   | 3 ++-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
index 316f84f1d15c8..21b8166a68839 100644
--- a/arch/parisc/Makefile
+++ b/arch/parisc/Makefile
@@ -119,7 +119,7 @@ export LIBGCC
 
 libs-y	+= arch/parisc/lib/ $(LIBGCC)
 
-drivers-y += arch/parisc/video/
+drivers-$(CONFIG_VIDEO) += arch/parisc/video/
 
 boot	:= arch/parisc/boot
 
diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile
index 2a03daa68f285..757451c3ea1df 100644
--- a/arch/sparc/Makefile
+++ b/arch/sparc/Makefile
@@ -59,8 +59,8 @@ endif
 libs-y                 += arch/sparc/prom/
 libs-y                 += arch/sparc/lib/
 
-drivers-$(CONFIG_PM) += arch/sparc/power/
-drivers-$(CONFIG_FB_CORE) += arch/sparc/video/
+drivers-$(CONFIG_PM)    += arch/sparc/power/
+drivers-$(CONFIG_VIDEO) += arch/sparc/video/
 
 boot := arch/sparc/boot
 
diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile
index d4d83f1702c61..9dd82880a027a 100644
--- a/arch/sparc/video/Makefile
+++ b/arch/sparc/video/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-obj-$(CONFIG_FB_CORE) += fbdev.o
+obj-y	+= fbdev.o
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 662d9d4033e6b..b80d15c29ecc6 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -260,7 +260,7 @@ drivers-$(CONFIG_PCI)            += arch/x86/pci/
 # suspend and hibernation support
 drivers-$(CONFIG_PM) += arch/x86/power/
 
-drivers-$(CONFIG_FB_CORE) += arch/x86/video/
+drivers-$(CONFIG_VIDEO) += arch/x86/video/
 
 ####
 # boot loader support. Several targets are kept for legacy purposes
diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile
index 5ebe48752ffc4..9dd82880a027a 100644
--- a/arch/x86/video/Makefile
+++ b/arch/x86/video/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_FB_CORE)		+= fbdev.o
+
+obj-y	+= fbdev.o
-- 
2.44.0


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

^ permalink raw reply related

* Re: [PATCH v9 00/13] firmware: qcom: qseecom: convert to using the TZ allocator
From: Maximilian Luz @ 2024-03-29 19:57 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Bartosz Golaszewski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Elliot Berman, Krzysztof Kozlowski, Guru Das Srinagesh,
	Andrew Halaney, Alex Elder, Srini Kandagatla, Arnd Bergmann,
	linux-arm-msm, linux-kernel, linux-arm-kernel, kernel
In-Reply-To: <CACMJSesvM6_PhhR_2sP4JX6bR4ytVVg=MwWBEVrCHf5FNp2JXw@mail.gmail.com>

On 3/29/24 8:46 PM, Bartosz Golaszewski wrote:
> On Fri, 29 Mar 2024 at 20:39, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>> On 3/29/24 8:26 PM, Bartosz Golaszewski wrote:
>>> On Fri, 29 Mar 2024 at 20:22, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>>>
>>>> On 3/29/24 8:07 PM, Bartosz Golaszewski wrote:
>>>>>
>>>>> Both with and without SHM bridge?
>>>>
>>>> With CONFIG_QCOM_TZMEM_MODE_GENERIC=y (and the upcoming fix) everything
>>>> works. With CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y things unfortunately
>>>> still get stuck at boot (regardless of the fix). I think that's
>>>> happening even before anything efivar related should come up.
>>>>
>>>
>>> This is on X13s? I will get one in 3 weeks. Can you get the bootlog
>>> somehow? Does the laptop have any serial console?
>>
>> Surface Pro X (sc8180x), but it should be similar enough to the X13s in
>> that regard. At least from what people with access to the X13s told me,
>> the qseecom stuff seems to behave the same.
>>
>> Unfortunately I don't have a direct serial console. Best I have is
>> USB-serial, but it's not even getting there. I'll have to try and see if
>> I can get some more info on the screen.
>>
> 
> I have access to a sc8180x-primus board, does it make sense to test
> with this one? If so, could you give me instructions on how to do it?

I guess it's worth a shot.

 From what I can tell, there shouldn't be any patches in my tree that
would conflict with it. So I guess it should just be building it with
CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y and booting.

I am currently testing it on top of a patched v6.8 tree though (but that
should just contain patches to get the Pro X running). You can find the
full tree at

     https://github.com/linux-surface/kernel/tree/spx/v6.8

The last commit is the fix I mentioned, so you might want to revert
that, since the shmem issue triggers regardless of that and it prevents
your series from applying cleanly.

Best regards,
Max


_______________________________________________
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 net-next v2] net: axienet: Fix kernel doc warnings
From: patchwork-bot+netdevbpf @ 2024-03-29 19:50 UTC (permalink / raw)
  To: Suraj Gupta
  Cc: radhey.shyam.pandey, davem, edumazet, kuba, pabeni, michal.simek,
	netdev, linux-arm-kernel, linux-kernel, git, harini.katakam
In-Reply-To: <20240328110713.12885-1-suraj.gupta2@amd.com>

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 28 Mar 2024 16:37:13 +0530 you wrote:
> Add description of mdio enable, mdio disable and mdio wait functions.
> Add description of skb pointer in axidma_bd data structure.
> Remove 'phy_node' description in axienet local data structure since
> it is not a valid struct member.
> Correct description of struct axienet_option.
> 
> Fix below kernel-doc warnings in drivers/net/ethernet/xilinx/:
> 1) xilinx_axienet_mdio.c:1: warning: no structured comments found
> 2) xilinx_axienet.h:379: warning: Function parameter or struct member
> 'skb' not described in 'axidma_bd'
> 3) xilinx_axienet.h:538: warning: Excess struct member 'phy_node'
> description in 'axienet_local'
> 4) xilinx_axienet.h:1002: warning: expecting prototype for struct
> axiethernet_option. Prototype was for struct axienet_option instead
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: axienet: Fix kernel doc warnings
    https://git.kernel.org/netdev/net-next/c/06c2a5cd48fe

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

^ permalink raw reply

* Re: [PATCH v9 00/13] firmware: qcom: qseecom: convert to using the TZ allocator
From: Bartosz Golaszewski @ 2024-03-29 19:46 UTC (permalink / raw)
  To: Maximilian Luz
  Cc: Bartosz Golaszewski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Elliot Berman, Krzysztof Kozlowski, Guru Das Srinagesh,
	Andrew Halaney, Alex Elder, Srini Kandagatla, Arnd Bergmann,
	linux-arm-msm, linux-kernel, linux-arm-kernel, kernel
In-Reply-To: <9b1e5ea0-bb32-4c42-b2e9-204bde31b905@gmail.com>

On Fri, 29 Mar 2024 at 20:39, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> On 3/29/24 8:26 PM, Bartosz Golaszewski wrote:
> > On Fri, 29 Mar 2024 at 20:22, Maximilian Luz <luzmaximilian@gmail.com> wrote:
> >>
> >> On 3/29/24 8:07 PM, Bartosz Golaszewski wrote:
> >>>
> >>> Both with and without SHM bridge?
> >>
> >> With CONFIG_QCOM_TZMEM_MODE_GENERIC=y (and the upcoming fix) everything
> >> works. With CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y things unfortunately
> >> still get stuck at boot (regardless of the fix). I think that's
> >> happening even before anything efivar related should come up.
> >>
> >
> > This is on X13s? I will get one in 3 weeks. Can you get the bootlog
> > somehow? Does the laptop have any serial console?
>
> Surface Pro X (sc8180x), but it should be similar enough to the X13s in
> that regard. At least from what people with access to the X13s told me,
> the qseecom stuff seems to behave the same.
>
> Unfortunately I don't have a direct serial console. Best I have is
> USB-serial, but it's not even getting there. I'll have to try and see if
> I can get some more info on the screen.
>

I have access to a sc8180x-primus board, does it make sense to test
with this one? If so, could you give me instructions on how to do it?

Bart

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

^ permalink raw reply

* Re: [PATCH 1/3] dt-bindings: reset: Add Amlogic T7 Reset Controller
From: Krzysztof Kozlowski @ 2024-03-29 19:39 UTC (permalink / raw)
  To: kelvin.zhang, Philipp Zabel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: devicetree, linux-arm-kernel, linux-amlogic, linux-kernel,
	Zelong Dong
In-Reply-To: <20240329-t7-reset-v1-1-4c6e2e68359e@amlogic.com>

On 29/03/2024 10:17, Kelvin Zhang via B4 Relay wrote:
> From: Zelong Dong <zelong.dong@amlogic.com>
> 
> Add a new compatible and the related header file
> for Amlogic T7 Reset Controller.
> 
> Signed-off-by: Zelong Dong <zelong.dong@amlogic.com>
> Signed-off-by: Kelvin Zhang <kelvin.zhang@amlogic.com>
> ---
>  .../bindings/reset/amlogic,meson-reset.yaml        |   1 +
>  include/dt-bindings/reset/amlogic,t7-reset.h       | 197 +++++++++++++++++++++
>  2 files changed, 198 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml b/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml
> index f0c6c0df0ce3..fefe343e5afe 100644
> --- a/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml
> @@ -19,6 +19,7 @@ properties:
>        - amlogic,meson-a1-reset # Reset Controller on A1 and compatible SoCs
>        - amlogic,meson-s4-reset # Reset Controller on S4 and compatible SoCs
>        - amlogic,c3-reset # Reset Controller on C3 and compatible SoCs
> +      - amlogic,t7-reset # Reset Controller on T7 and compatible SoCs
>  

If there is going to be any resend, please drop the comment. It's not
really helpful and makes it trickier to read.

>    reg:
>      maxItems: 1
> diff --git a/include/dt-bindings/reset/amlogic,t7-reset.h b/include/dt-bindings/reset/amlogic,t7-reset.h
> new file mode 100644
> index 000000000000..ca4a832eeeec
> --- /dev/null
> +++ b/include/dt-bindings/reset/amlogic,t7-reset.h
> @@ -0,0 +1,197 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_AMLOGIC_T7_RESET_H
> +#define _DT_BINDINGS_AMLOGIC_T7_RESET_H
> +
> +/* RESET0 */
> +/*					0-3	*/

I assume this matches existing drivers which do not use IDs but map the
binding to hardware value? I remember we talked about changing it, so if
something happened about this and it could be changed: please change.

Otherwise, it's fine:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof


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

^ permalink raw reply

* Re: [PATCH v9 00/13] firmware: qcom: qseecom: convert to using the TZ allocator
From: Maximilian Luz @ 2024-03-29 19:38 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Elliot Berman,
	Krzysztof Kozlowski, Guru Das Srinagesh, Andrew Halaney,
	Alex Elder, Srini Kandagatla, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm-kernel, kernel, Bartosz Golaszewski
In-Reply-To: <CAMRc=MdoSPuedbGhy4toDEkw0vSzESDz2bXGpyt_=R4hqXW+Uw@mail.gmail.com>

On 3/29/24 8:26 PM, Bartosz Golaszewski wrote:
> On Fri, 29 Mar 2024 at 20:22, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>> On 3/29/24 8:07 PM, Bartosz Golaszewski wrote:
>>>
>>> Both with and without SHM bridge?
>>
>> With CONFIG_QCOM_TZMEM_MODE_GENERIC=y (and the upcoming fix) everything
>> works. With CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y things unfortunately
>> still get stuck at boot (regardless of the fix). I think that's
>> happening even before anything efivar related should come up.
>>
> 
> This is on X13s? I will get one in 3 weeks. Can you get the bootlog
> somehow? Does the laptop have any serial console?

Surface Pro X (sc8180x), but it should be similar enough to the X13s in
that regard. At least from what people with access to the X13s told me,
the qseecom stuff seems to behave the same.

Unfortunately I don't have a direct serial console. Best I have is
USB-serial, but it's not even getting there. I'll have to try and see if
I can get some more info on the screen.

Best regards,
Max

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

^ permalink raw reply

* Re: [PATCH v9 00/13] firmware: qcom: qseecom: convert to using the TZ allocator
From: Bartosz Golaszewski @ 2024-03-29 19:26 UTC (permalink / raw)
  To: Maximilian Luz
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Elliot Berman,
	Krzysztof Kozlowski, Guru Das Srinagesh, Andrew Halaney,
	Alex Elder, Srini Kandagatla, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm-kernel, kernel, Bartosz Golaszewski
In-Reply-To: <2b1dc031-d645-494c-9103-a2bb422ea60b@gmail.com>

On Fri, 29 Mar 2024 at 20:22, Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> On 3/29/24 8:07 PM, Bartosz Golaszewski wrote:
> >
> > Both with and without SHM bridge?
>
> With CONFIG_QCOM_TZMEM_MODE_GENERIC=y (and the upcoming fix) everything
> works. With CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y things unfortunately
> still get stuck at boot (regardless of the fix). I think that's
> happening even before anything efivar related should come up.
>

This is on X13s? I will get one in 3 weeks. Can you get the bootlog
somehow? Does the laptop have any serial console?

Bart

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

^ permalink raw reply

* Re: [PATCH v9 00/13] firmware: qcom: qseecom: convert to using the TZ allocator
From: Maximilian Luz @ 2024-03-29 19:22 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Elliot Berman,
	Krzysztof Kozlowski, Guru Das Srinagesh, Andrew Halaney,
	Alex Elder, Srini Kandagatla, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm-kernel, kernel, Bartosz Golaszewski
In-Reply-To: <CAMRc=McLJFGcy-A6PZNmjgDXnvx8z0J4k-Dbak-txvWnycHG2A@mail.gmail.com>

On 3/29/24 8:07 PM, Bartosz Golaszewski wrote:
> On Fri, Mar 29, 2024 at 7:56 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>
>>
>>
>> On 3/29/24 7:53 PM, Maximilian Luz wrote:
>>> On 3/29/24 11:22 AM, Bartosz Golaszewski wrote:
>>>> On Thu, Mar 28, 2024 at 7:55 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>>>
>>>>> On Thu, Mar 28, 2024 at 5:50 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>>>>>>
>>>>>> If I understand correctly, it enters an atomic section in
>>>>>> qcom_tzmem_alloc() and then tries to schedule somewhere down the line.
>>>>>> So this shouldn't be qseecom specific.
>>>>>>
>>>>>> I should probably also say that I'm currently testing this on a patched
>>>>>> v6.8 kernel, so there's a chance that it's my fault. However, as far as
>>>>>> I understand, it enters an atomic section in qcom_tzmem_alloc() and then
>>>>>> later tries to expand the pool memory with dma_alloc_coherent(). Which
>>>>>> AFAIK is allowed to sleep with GFP_KERNEL (and I guess that that's the
>>>>>> issue here).
>>>>>>
>>>>>> I've also tried the shmem allocator option, but that seems to get stuck
>>>>>> quite early at boot, before I even have usb-serial access to get any
>>>>>> logs. If I can find some more time, I'll try to see if I can get some
>>>>>> useful output for that.
>>>>>>
>>>>>
>>>>> Ah, I think it happens here:
>>>>>
>>>>> +       guard(spinlock_irqsave)(&pool->lock);
>>>>> +
>>>>> +again:
>>>>> +       vaddr = gen_pool_alloc(pool->genpool, size);
>>>>> +       if (!vaddr) {
>>>>> +               if (qcom_tzmem_try_grow_pool(pool, size, gfp))
>>>>> +                       goto again;
>>>>>
>>>>> We were called with GFP_KERNEL so this is what we pass on to
>>>>> qcom_tzmem_try_grow_pool() but we're now holding the spinlock. I need
>>>>> to revisit it. Thanks for the catch!
>>>>>
>>>>> Bart
>>>>
>>>> Can you try the following tree?
>>>>
>>>>       https://git.codelinaro.org/bartosz_golaszewski/linux.git
>>>> topic/shm-bridge-v10
>>>>
>>>> gen_pool_alloc() and gen_pool_add_virt() can be used without external
>>>> serialization. We only really need to protect the list of areas in the
>>>> pool when adding a new element. We could possibly even use
>>>> list_add_tail_rcu() as it updates the pointers atomically and go
>>>> lockless.
>>>
>>> Thanks! That fixes the allocations for CONFIG_QCOM_TZMEM_MODE_GENERIC=y.
>>> Unfortunately, with the shmbridge mode it still gets stuck at boot (and
>>> I haven't had the time to look into it yet).
>>>
>>> And for more bad news: It looks like the new allocator now fully exposes
>>> a bug that I've been tracking down the last couple of days. In short,
>>> uefisecapp doesn't seem to be happy when we split the allocations for
>>> request and response into two, causing commands to fail. Instead it
>>> wants a single buffer for both. Before, it seemed to be fairly sporadic
>>> (likely because kzalloc in sequence just returned consecutive memory
>>> almost all of the time) but now it's basically every call that fails.
>>>
>>> I have a fix for that almost ready and I'll likely post it in the next
>>> hour. But that means that you'll probably have to rebase this series
>>> on top of it...
>>
>> Forgot to mention: I tested it with the fix and this series, and that
>> works.
>>
> 
> Both with and without SHM bridge?

With CONFIG_QCOM_TZMEM_MODE_GENERIC=y (and the upcoming fix) everything
works. With CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y things unfortunately
still get stuck at boot (regardless of the fix). I think that's
happening even before anything efivar related should come up.

> If so, please Cc me on the fix.

Sure, will do.

Best regards,
Max

_______________________________________________
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 v12 8/8] PCI: endpoint: Remove "core_init_notifier" flag
From: Frank Li @ 2024-03-29 19:17 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Marek Vasut, Yoshihiro Shimoda,
	Thierry Reding, Jonathan Hunter, Vidya Sagar, Vignesh Raghavendra,
	Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Minghuan Lian, Mingkai Hu, Roy Zang, Kunihiko Hayashi,
	Masami Hiramatsu, Kishon Vijay Abraham I, Jesper Nilsson,
	Srikanth Thokala, Shawn Lin, Heiko Stuebner, linux-pci,
	linux-kernel, linux-renesas-soc, linux-arm-msm, linux-tegra,
	linux-omap, linux-arm-kernel, linuxppc-dev, Niklas Cassel,
	linux-arm-kernel, linux-rockchip
In-Reply-To: <20240327-pci-dbi-rework-v12-8-082625472414@linaro.org>

On Wed, Mar 27, 2024 at 02:43:37PM +0530, Manivannan Sadhasivam wrote:
> "core_init_notifier" flag is set by the glue drivers requiring refclk from
> the host to complete the DWC core initialization. Also, those drivers will
> send a notification to the EPF drivers once the initialization is fully
> completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> will start functioning.
> 
> For the rest of the drivers generating refclk locally, EPF drivers will
> start functioning post binding with them. EPF drivers rely on the
> 'core_init_notifier' flag to differentiate between the drivers.
> Unfortunately, this creates two different flows for the EPF drivers.
> 
> So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> a single initialization flow for the EPF drivers. This is done by calling
> the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> send the notification to the EPF drivers once the initialization is fully
> completed.
> 
> Only difference here is that, the drivers requiring refclk from host will
> send the notification once refclk is received, while others will send it
> during probe time itself.
> 
> But this also requires the EPC core driver to deliver the notification
> after EPF driver bind. Because, the glue driver can send the notification
> before the EPF drivers bind() and in those cases the EPF drivers will miss
> the event. To accommodate this, EPC core is now caching the state of the
> EPC initialization in 'init_complete' flag and pci-ep-cfs driver sends the
> notification to EPF drivers based on that after each EPF driver bind.
> 
> Tested-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/controller/cadence/pcie-cadence-ep.c  |  2 ++
>  drivers/pci/controller/dwc/pci-dra7xx.c           |  2 ++
>  drivers/pci/controller/dwc/pci-imx6.c             |  2 ++
>  drivers/pci/controller/dwc/pci-keystone.c         |  2 ++
>  drivers/pci/controller/dwc/pci-layerscape-ep.c    |  2 ++
>  drivers/pci/controller/dwc/pcie-artpec6.c         |  2 ++
>  drivers/pci/controller/dwc/pcie-designware-ep.c   |  1 +
>  drivers/pci/controller/dwc/pcie-designware-plat.c |  2 ++
>  drivers/pci/controller/dwc/pcie-keembay.c         |  2 ++
>  drivers/pci/controller/dwc/pcie-qcom-ep.c         |  1 -
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c       |  2 ++
>  drivers/pci/controller/dwc/pcie-tegra194.c        |  1 -
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c     |  2 ++
>  drivers/pci/controller/pcie-rcar-ep.c             |  2 ++
>  drivers/pci/controller/pcie-rockchip-ep.c         |  2 ++
>  drivers/pci/endpoint/functions/pci-epf-test.c     | 18 +++++-------------
>  drivers/pci/endpoint/pci-ep-cfs.c                 |  9 +++++++++
>  drivers/pci/endpoint/pci-epc-core.c               | 22 ++++++++++++++++++++++
>  include/linux/pci-epc.h                           |  7 ++++---
>  19 files changed, 65 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> index 81c50dc64da9..55c42ca2b777 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> @@ -746,6 +746,8 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
>  
>  	spin_lock_init(&ep->lock);
>  
> +	pci_epc_init_notify(epc);
> +
>  	return 0;
>  
>   free_epc_mem:
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 395042b29ffc..d2d17d37d3e0 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -474,6 +474,8 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
>  		return ret;
>  	}
>  
> +	dw_pcie_ep_init_notify(ep);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 8d28ecc381bc..917c69edee1d 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1131,6 +1131,8 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
>  		return ret;
>  	}
>  
> +	dw_pcie_ep_init_notify(ep);
> +
>  	/* Start LTSSM. */
>  	imx6_pcie_ltssm_enable(dev);
>  
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 81ebac520650..d3a7d14ee685 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1293,6 +1293,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
>  			goto err_ep_init;
>  		}
>  
> +		dw_pcie_ep_init_notify(&pci->ep);
> +
>  		break;
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", mode);
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 9eb2233e3d7f..7dde6d5fa4d8 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -286,6 +286,8 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	dw_pcie_ep_init_notify(&pci->ep);
> +
>  	return ls_pcie_ep_interrupt_init(pcie, pdev);
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
> index a6095561db4a..a4630b92489b 100644
> --- a/drivers/pci/controller/dwc/pcie-artpec6.c
> +++ b/drivers/pci/controller/dwc/pcie-artpec6.c
> @@ -452,6 +452,8 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
>  			return ret;
>  		}
>  
> +		dw_pcie_ep_init_notify(&pci->ep);
> +
>  		break;
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", artpec6_pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2063cf2049e5..47391d7d3a73 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -632,6 +632,7 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
>  	dw_pcie_edma_remove(pci);
> +	ep->epc->init_complete = false;
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup);
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index ca9b22e654cd..8490c5d6ff9f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -154,6 +154,8 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
>  			dw_pcie_ep_deinit(&pci->ep);
>  		}
>  
> +		dw_pcie_ep_init_notify(&pci->ep);
> +
>  		break;
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index b2556dbcffb5..98bbc83182b4 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -442,6 +442,8 @@ static int keembay_pcie_probe(struct platform_device *pdev)
>  			return ret;
>  		}
>  
> +		dw_pcie_ep_init_notify(&pci->ep);
> +
>  		break;
>  	default:
>  		dev_err(dev, "Invalid device type %d\n", pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 3697b4a944cc..2fb8c15e7a91 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -775,7 +775,6 @@ static void qcom_pcie_ep_init_debugfs(struct qcom_pcie_ep *pcie_ep)
>  
>  static const struct pci_epc_features qcom_pcie_epc_features = {
>  	.linkup_notifier = true,
> -	.core_init_notifier = true,
>  	.msi_capable = true,
>  	.msix_capable = false,
>  	.align = SZ_4K,
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index e155a905fb4f..cfeccc2f9ee1 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -437,6 +437,8 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
>  		rcar_gen4_pcie_ep_deinit(rcar);
>  	}
>  
> +	dw_pcie_ep_init_notify(ep);
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index db043f579fbe..ddc23602eca7 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2006,7 +2006,6 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  
>  static const struct pci_epc_features tegra_pcie_epc_features = {
>  	.linkup_notifier = true,
> -	.core_init_notifier = true,
>  	.msi_capable = false,
>  	.msix_capable = false,
>  	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M,
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 0e5e7344de48..a2b844268e28 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -410,6 +410,8 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	dw_pcie_ep_init_notify(&priv->pci.ep);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/pci/controller/pcie-rcar-ep.c b/drivers/pci/controller/pcie-rcar-ep.c
> index 05967c6c0b42..047e2cef5afc 100644
> --- a/drivers/pci/controller/pcie-rcar-ep.c
> +++ b/drivers/pci/controller/pcie-rcar-ep.c
> @@ -542,6 +542,8 @@ static int rcar_pcie_ep_probe(struct platform_device *pdev)
>  		goto err_pm_put;
>  	}
>  
> +	pci_epc_init_notify(epc);
> +
>  	return 0;
>  
>  err_pm_put:
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index c9046e97a1d2..8613df8184df 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -609,6 +609,8 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev)
>  	rockchip_pcie_write(rockchip, PCIE_CLIENT_CONF_ENABLE,
>  			    PCIE_CLIENT_CONFIG);
>  
> +	pci_epc_init_notify(epc);
> +
>  	return 0;
>  err_epc_mem_exit:
>  	pci_epc_mem_exit(epc);
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index cd4ffb39dcdc..212fc303fb63 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -753,6 +753,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
>  	const struct pci_epc_features *epc_features;
>  	struct pci_epc *epc = epf->epc;
>  	struct device *dev = &epf->dev;
> +	bool linkup_notifier = false;
>  	bool msix_capable = false;
>  	bool msi_capable = true;
>  	int ret;
> @@ -795,6 +796,10 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
>  		}
>  	}
>  
> +	linkup_notifier = epc_features->linkup_notifier;
> +	if (!linkup_notifier)
> +		queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
> +
>  	return 0;
>  }
>  
> @@ -890,8 +895,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	const struct pci_epc_features *epc_features;
>  	enum pci_barno test_reg_bar = BAR_0;
>  	struct pci_epc *epc = epf->epc;
> -	bool linkup_notifier = false;
> -	bool core_init_notifier = false;
>  
>  	if (WARN_ON_ONCE(!epc))
>  		return -EINVAL;
> @@ -902,8 +905,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  		return -EOPNOTSUPP;
>  	}
>  
> -	linkup_notifier = epc_features->linkup_notifier;
> -	core_init_notifier = epc_features->core_init_notifier;
>  	test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>  	if (test_reg_bar < 0)
>  		return -EINVAL;
> @@ -916,21 +917,12 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	if (ret)
>  		return ret;
>  
> -	if (!core_init_notifier) {
> -		ret = pci_epf_test_core_init(epf);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	epf_test->dma_supported = true;
>  
>  	ret = pci_epf_test_init_dma_chan(epf_test);
>  	if (ret)
>  		epf_test->dma_supported = false;
>  
> -	if (!linkup_notifier && !core_init_notifier)
> -		queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index 0ea64e24ed61..3b21e28f9b59 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -64,6 +64,9 @@ static int pci_secondary_epc_epf_link(struct config_item *epf_item,
>  		return ret;
>  	}
>  
> +	/* Send any pending EPC initialization complete to the EPF driver */
> +	pci_epc_notify_pending_init(epc, epf);
> +
>  	return 0;
>  }
>  
> @@ -125,6 +128,9 @@ static int pci_primary_epc_epf_link(struct config_item *epf_item,
>  		return ret;
>  	}
>  
> +	/* Send any pending EPC initialization complete to the EPF driver */
> +	pci_epc_notify_pending_init(epc, epf);
> +
>  	return 0;
>  }
>  
> @@ -230,6 +236,9 @@ static int pci_epc_epf_link(struct config_item *epc_item,
>  		return ret;
>  	}
>  
> +	/* Send any pending EPC initialization complete to the EPF driver */
> +	pci_epc_notify_pending_init(epc, epf);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index da3fc0795b0b..47d27ec7439d 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -748,10 +748,32 @@ void pci_epc_init_notify(struct pci_epc *epc)
>  			epf->event_ops->core_init(epf);
>  		mutex_unlock(&epf->lock);
>  	}
> +	epc->init_complete = true;
>  	mutex_unlock(&epc->list_lock);
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_init_notify);
>  
> +/**
> + * pci_epc_notify_pending_init() - Notify the pending EPC device initialization
> + *                                 complete to the EPF device
> + * @epc: the EPC device whose core initialization is pending to be notified
> + * @epf: the EPF device to be notified
> + *
> + * Invoke to notify the pending EPC device initialization complete to the EPF
> + * device. This is used to deliver the notification if the EPC initialization
> + * got completed before the EPF driver bind.
> + */
> +void pci_epc_notify_pending_init(struct pci_epc *epc, struct pci_epf *epf)
> +{
> +	if (epc->init_complete) {
> +		mutex_lock(&epf->lock);
> +		if (epf->event_ops && epf->event_ops->core_init)
> +			epf->event_ops->core_init(epf);
> +		mutex_unlock(&epf->lock);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_notify_pending_init);
> +
>  /**
>   * pci_epc_bme_notify() - Notify the EPF device that the EPC device has received
>   *			  the BME event from the Root complex
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index cc2f70d061c8..acc5f96161fe 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -128,6 +128,8 @@ struct pci_epc_mem {
>   * @group: configfs group representing the PCI EPC device
>   * @lock: mutex to protect pci_epc ops
>   * @function_num_map: bitmap to manage physical function number
> + * @init_complete: flag to indicate whether the EPC initialization is complete
> + *                 or not
>   */
>  struct pci_epc {
>  	struct device			dev;
> @@ -143,6 +145,7 @@ struct pci_epc {
>  	/* mutex to protect against concurrent access of EP controller */
>  	struct mutex			lock;
>  	unsigned long			function_num_map;
> +	bool				init_complete;
>  };
>  
>  /**
> @@ -179,8 +182,6 @@ struct pci_epc_bar_desc {
>  /**
>   * struct pci_epc_features - features supported by a EPC device per function
>   * @linkup_notifier: indicate if the EPC device can notify EPF driver on link up
> - * @core_init_notifier: indicate cores that can notify about their availability
> - *			for initialization
>   * @msi_capable: indicate if the endpoint function has MSI capability
>   * @msix_capable: indicate if the endpoint function has MSI-X capability
>   * @bar: array specifying the hardware description for each BAR
> @@ -188,7 +189,6 @@ struct pci_epc_bar_desc {
>   */
>  struct pci_epc_features {
>  	unsigned int	linkup_notifier : 1;
> -	unsigned int	core_init_notifier : 1;
>  	unsigned int	msi_capable : 1;
>  	unsigned int	msix_capable : 1;
>  	struct	pci_epc_bar_desc bar[PCI_STD_NUM_BARS];
> @@ -225,6 +225,7 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf,
>  void pci_epc_linkup(struct pci_epc *epc);
>  void pci_epc_linkdown(struct pci_epc *epc);
>  void pci_epc_init_notify(struct pci_epc *epc);
> +void pci_epc_notify_pending_init(struct pci_epc *epc, struct pci_epf *epf);
>  void pci_epc_bme_notify(struct pci_epc *epc);
>  void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
>  			enum pci_epc_interface_type type);
> 
> -- 
> 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

* Re: [PATCH v12 7/8] PCI: dwc: ep: Call dw_pcie_ep_init_registers() API directly from all glue drivers
From: Frank Li @ 2024-03-29 19:16 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Marek Vasut, Yoshihiro Shimoda,
	Thierry Reding, Jonathan Hunter, Vidya Sagar, Vignesh Raghavendra,
	Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Minghuan Lian, Mingkai Hu, Roy Zang, Kunihiko Hayashi,
	Masami Hiramatsu, Kishon Vijay Abraham I, Jesper Nilsson,
	Srikanth Thokala, Shawn Lin, Heiko Stuebner, linux-pci,
	linux-kernel, linux-renesas-soc, linux-arm-msm, linux-tegra,
	linux-omap, linux-arm-kernel, linuxppc-dev, Niklas Cassel,
	linux-arm-kernel, linux-rockchip
In-Reply-To: <20240327-pci-dbi-rework-v12-7-082625472414@linaro.org>

On Wed, Mar 27, 2024 at 02:43:36PM +0530, Manivannan Sadhasivam wrote:
> Currently, dw_pcie_ep_init_registers() API is directly called by the glue
> drivers requiring active refclk from host. But for the other drivers, it is
> getting called implicitly by dw_pcie_ep_init(). This is due to the fact
> that this API initializes DWC EP specific registers and that requires an
> active refclk (either from host or generated locally by endpoint itsef).
> 
> But, this causes a discrepancy among the glue drivers. So to avoid this
> confusion, let's call this API directly from all glue drivers irrespective
> of refclk dependency. Only difference here is that the drivers requiring
> refclk from host will call this API only after the refclk is received and
> other drivers without refclk dependency will call this API right after
> dw_pcie_ep_init().
> 
> With this change, the check for 'core_init_notifier' flag can now be
> dropped from dw_pcie_ep_init() API. This will also allow us to remove the
> 'core_init_notifier' flag completely in the later commits.
> 
> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/controller/dwc/pci-dra7xx.c           |  7 +++++++
>  drivers/pci/controller/dwc/pci-imx6.c             |  8 ++++++++
>  drivers/pci/controller/dwc/pci-keystone.c         |  9 +++++++++
>  drivers/pci/controller/dwc/pci-layerscape-ep.c    |  7 +++++++
>  drivers/pci/controller/dwc/pcie-artpec6.c         | 13 ++++++++++++-
>  drivers/pci/controller/dwc/pcie-designware-ep.c   | 22 ----------------------
>  drivers/pci/controller/dwc/pcie-designware-plat.c |  9 +++++++++
>  drivers/pci/controller/dwc/pcie-keembay.c         | 16 +++++++++++++++-
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c       | 12 +++++++++++-
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c     | 13 ++++++++++++-
>  10 files changed, 90 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 0e406677060d..395042b29ffc 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -467,6 +467,13 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
>  		return ret;
>  	}
>  
> +	ret = dw_pcie_ep_init_registers(ep);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +		dw_pcie_ep_deinit(ep);
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 99a60270b26c..8d28ecc381bc 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1123,6 +1123,14 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
>  		dev_err(dev, "failed to initialize endpoint\n");
>  		return ret;
>  	}
> +
> +	ret = dw_pcie_ep_init_registers(ep);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +		dw_pcie_ep_deinit(ep);
> +		return ret;
> +	}
> +
>  	/* Start LTSSM. */
>  	imx6_pcie_ltssm_enable(dev);
>  
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 844de4418724..81ebac520650 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1286,6 +1286,13 @@ static int ks_pcie_probe(struct platform_device *pdev)
>  		ret = dw_pcie_ep_init(&pci->ep);
>  		if (ret < 0)
>  			goto err_get_sync;
> +
> +		ret = dw_pcie_ep_init_registers(&pci->ep);
> +		if (ret) {
> +			dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +			goto err_ep_init;
> +		}
> +
>  		break;
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", mode);
> @@ -1295,6 +1302,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> +err_ep_init:
> +	dw_pcie_ep_deinit(&pci->ep);
>  err_get_sync:
>  	pm_runtime_put(dev);
>  	pm_runtime_disable(dev);
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 1f6ee1460ec2..9eb2233e3d7f 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -279,6 +279,13 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = dw_pcie_ep_init_registers(&pci->ep);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +		dw_pcie_ep_deinit(&pci->ep);
> +		return ret;
> +	}
> +
>  	return ls_pcie_ep_interrupt_init(pcie, pdev);
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
> index 9ed0a9ba7619..a6095561db4a 100644
> --- a/drivers/pci/controller/dwc/pcie-artpec6.c
> +++ b/drivers/pci/controller/dwc/pcie-artpec6.c
> @@ -441,7 +441,18 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
>  
>  		pci->ep.ops = &pcie_ep_ops;
>  
> -		return dw_pcie_ep_init(&pci->ep);
> +		ret = dw_pcie_ep_init(&pci->ep);
> +		if (ret)
> +			return ret;
> +
> +		ret = dw_pcie_ep_init_registers(&pci->ep);
> +		if (ret) {
> +			dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +			dw_pcie_ep_deinit(&pci->ep);
> +			return ret;
> +		}
> +
> +		break;
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", artpec6_pcie->mode);
>  	}
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 761d3012a073..2063cf2049e5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -821,7 +821,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  	struct device *dev = pci->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct device_node *np = dev->of_node;
> -	const struct pci_epc_features *epc_features;
>  
>  	INIT_LIST_HEAD(&ep->func_list);
>  
> @@ -867,29 +866,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  		goto err_exit_epc_mem;
>  	}
>  
> -	if (ep->ops->get_features) {
> -		epc_features = ep->ops->get_features(ep);
> -		if (epc_features->core_init_notifier)
> -			return 0;
> -	}
> -
> -	/*
> -	 * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
> -	 * step as platforms that implement 'core_init_notifier' feature may
> -	 * not have the hardware ready (i.e. core initialized) for access
> -	 * (Ex: tegra194). Any hardware access on such platforms result
> -	 * in system hang.
> -	 */
> -	ret = dw_pcie_ep_init_registers(ep);
> -	if (ret)
> -		goto err_free_epc_mem;
> -
>  	return 0;
>  
> -err_free_epc_mem:
> -	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> -			      epc->mem->window.page_size);
> -
>  err_exit_epc_mem:
>  	pci_epc_mem_exit(epc);
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index 778588b4be70..ca9b22e654cd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -145,6 +145,15 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
>  
>  		pci->ep.ops = &pcie_ep_ops;
>  		ret = dw_pcie_ep_init(&pci->ep);
> +		if (ret)
> +			return ret;
> +
> +		ret = dw_pcie_ep_init_registers(&pci->ep);
> +		if (ret) {
> +			dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +			dw_pcie_ep_deinit(&pci->ep);
> +		}
> +
>  		break;
>  	default:
>  		dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index 5e8e54f597dd..b2556dbcffb5 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -396,6 +396,7 @@ static int keembay_pcie_probe(struct platform_device *pdev)
>  	struct keembay_pcie *pcie;
>  	struct dw_pcie *pci;
>  	enum dw_pcie_device_mode mode;
> +	int ret;
>  
>  	data = device_get_match_data(dev);
>  	if (!data)
> @@ -430,11 +431,24 @@ static int keembay_pcie_probe(struct platform_device *pdev)
>  			return -ENODEV;
>  
>  		pci->ep.ops = &keembay_pcie_ep_ops;
> -		return dw_pcie_ep_init(&pci->ep);
> +		ret = dw_pcie_ep_init(&pci->ep);
> +		if (ret)
> +			return ret;
> +
> +		ret = dw_pcie_ep_init_registers(&pci->ep);
> +		if (ret) {
> +			dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +			dw_pcie_ep_deinit(&pci->ep);
> +			return ret;
> +		}
> +
> +		break;
>  	default:
>  		dev_err(dev, "Invalid device type %d\n", pcie->mode);
>  		return -ENODEV;
>  	}
> +
> +	return 0;
>  }
>  
>  static const struct keembay_pcie_of_data keembay_pcie_rc_of_data = {
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index de4bdfaecab3..e155a905fb4f 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -416,6 +416,7 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
>  static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
>  {
>  	struct dw_pcie_ep *ep = &rcar->dw.ep;
> +	struct device *dev = rcar->dw.dev;
>  	int ret;
>  
>  	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
> @@ -424,8 +425,17 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
>  	ep->ops = &pcie_ep_ops;
>  
>  	ret = dw_pcie_ep_init(ep);
> -	if (ret)
> +	if (ret) {
>  		rcar_gen4_pcie_ep_deinit(rcar);
> +		return ret;
> +	}
> +
> +	ret = dw_pcie_ep_init_registers(ep);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +		dw_pcie_ep_deinit(ep);
> +		rcar_gen4_pcie_ep_deinit(rcar);
> +	}
>  
>  	return ret;
>  }
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 639bc2e12476..0e5e7344de48 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -399,7 +399,18 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	priv->pci.ep.ops = &uniphier_pcie_ep_ops;
> -	return dw_pcie_ep_init(&priv->pci.ep);
> +	ret = dw_pcie_ep_init(&priv->pci.ep);
> +	if (ret)
> +		return ret;
> +
> +	ret = dw_pcie_ep_init_registers(&priv->pci.ep);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> +		dw_pcie_ep_deinit(&priv->pci.ep);
> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
> 
> -- 
> 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

* Re: [PATCH] KVM: arm64: Limit stage2_apply_range() batch size to smallest block
From: Krister Johansen @ 2024-03-29 19:15 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, Ali Saidi, David Reaver,
	linux-arm-kernel, kvmarm, linux-kernel
In-Reply-To: <ZgbGtpj5mStTkAkn@linux.dev>

Hi Oliver,
Thanks for the response.

On Fri, Mar 29, 2024 at 06:48:38AM -0700, Oliver Upton wrote:
> On Thu, Mar 28, 2024 at 12:05:08PM -0700, Krister Johansen wrote:
> > stage2_apply_range() for unmap operations can interfere with the
> > performance of IO if the device's interrupts share the CPU where the
> > unmap operation is occurring.  commit 5994bc9e05c2 ("KVM: arm64: Limit
> > stage2_apply_range() batch size to largest block") improved this.  Prior
> > to that commit, workloads that were unfortunate enough to have their IO
> > interrupts pinned to the same CPU as the unmap operation would observe a
> > complete stall.  With the switch to using the largest block size, it is
> > possible for IO to make progress, albeit at a reduced speed.
> 
> Can you describe the workload a bit more? I'm having a hard time
> understanding how you're unmapping that much memory on the fly in
> your workload. Is guest memory getting swapped? Are VMs being torn
> down?

Sorry I wasn't clear here.  Yes, it's the VMs getting torn down that's
causing the problems.  The container VMs don't have long lifetimes, but
some may be up to 256Gb in size, depending on the user.  The workloads
running the VMs aren't especially performance sensitive, but their users
do notice when network connections time-out.  IOW, if the performance is
bad enough to temporarily prevent new TCP connections from being
established or requests / responses being recieved in a timely fashion,
we'll hear about it.  Users deploy their services a lot, so there's a
lot of container vm churn.  (Really it's automation redeploying the
services on behalf of the users in response to new commits to their
repos...)

> Also, it seems a bit odd to steer interrupts *into* the workload you
> care about...

Ah, that was only intentionally done for the purposes of measuring the
impact.  That's not done on purpose in production.

Nevertheless, the example we tend to run into is that a box may have 2
NICs and each NIC has 32 Tx-Rx queues.  This means we've got 64 NIC
interrupts, each assigned to a different CPU.  Our systems have 64 CPUs.
What happens in practice is that a VM will get torn down, and that has a
1-in-64 chance of impacting the performance of the subset of the flows
that are mapped via RSS to the interrupt that happens to be assigned to
the CPU where the VM is being torn down.

Of course, the obvious next question is why not just bind the VMs flows
to the CPUs the VM is running on?  We don't have a 1:1 mapping of
network device to VM, or VM to CPU right now, which frustrates this
approach.

> > Further reducing the stage2_apply_range() batch size has substantial
> > performance improvements for IO that share a CPU performing an unmap
> > operation.  By switching to a 2mb chunk, IO performance regressions were
> > no longer observed in this author's tests.  E.g. it was possible to
> > obtain the advertised device throughput despite an unmap operation
> > occurring on the CPU where the interrupt was running.  There is a
> > tradeoff, however.  No changes were observed in per-operation timings
> > when running the kvm_pagetable_test without an interrupt load.  However,
> > with a 64gb VM, 1 vcpu, and 4k pages and a IO load, map times increased
> > by about 15% and unmap times increased by about 58%.  In essence, this
> > trades slower map/unmap times for improved IO throughput.
> 
> There are other users of the range-based operations, like
> write-protection. Live migration is especially sensitive to the latency
> of page table updates as it can affect the VMM's ability to converge
> with the guest.

To be clear, the reduction in performance was observed when I
concurrently executed both the kvm_pagetable_test and a networking
benchmark where the NIC's interrupts were assigned to the same CPU where
the pagetable test was executing.  I didn't see a slowdown just running
the pagetable test.

> > Cc: <stable@vger.kernel.org> # 5.15.x: 3b5c082bbfa2: KVM: arm64: Work out supported block level at compile time
> > Cc: <stable@vger.kernel.org> # 5.15.x: 5994bc9e05c2: KVM: arm64: Limit stage2_apply_range() batch size to largest block
> > Cc: <stable@vger.kernel.org> # 5.15.x
> 
> This is a performance improvement, *not* a correctness fix. Please don't
> cc stable for it.

Apologies.  I consulted the Stable Rules[1] before applying these tags and
the guidance it gave was just that "It must either fix a real bug that
bothers people."

In our case, the teardown causes TCP throughput to drop from 9.5Gbps to
about 2Gbps during a teardown, which is something that does bother our
users.

> > ---
> >  arch/arm64/include/asm/kvm_pgtable.h | 4 ++++
> >  arch/arm64/kvm/mmu.c                 | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index 19278dfe7978..b0c4651a4d9a 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -19,11 +19,15 @@
> >   *  - 4K (level 1):	1GB
> >   *  - 16K (level 2):	32MB
> >   *  - 64K (level 2):	512MB
> > + *
> > + *  The max block level is the _smallest_ supported block size for KVM.
> 
> This feels like a non sequitur given the old comment is left in place...

I'll fix if we keep this approach.  Is the objection to the name
KVM_PGTABLE_MAX_BLOCK_LEVEL or just the comment?

> >   */
> >  #ifdef CONFIG_ARM64_4K_PAGES
> >  #define KVM_PGTABLE_MIN_BLOCK_LEVEL	1
> > +#define KVM_PGTABLE_MAX_BLOCK_LEVEL	2
> >  #else
> >  #define KVM_PGTABLE_MIN_BLOCK_LEVEL	2
> > +#define KVM_PGTABLE_MAX_BLOCK_LEVEL	KVM_PGTABLE_MIN_BLOCK_LEVEL
> >  #endif
> >  
> >  #define kvm_lpa2_is_enabled()		system_supports_lpa2()
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index dc04bc767865..1e927b306aee 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -41,7 +41,7 @@ static phys_addr_t __stage2_range_addr_end(phys_addr_t addr, phys_addr_t end,
> >  
> >  static phys_addr_t stage2_range_addr_end(phys_addr_t addr, phys_addr_t end)
> >  {
> > -	phys_addr_t size = kvm_granule_size(KVM_PGTABLE_MIN_BLOCK_LEVEL);
> > +	phys_addr_t size = kvm_granule_size(KVM_PGTABLE_MAX_BLOCK_LEVEL);
> >  
> >  	return __stage2_range_addr_end(addr, end, size);
> >  }
> 
> This doesn't feel right to me. A property that we had before is that
> leaf entries are visited at most once, since every mapping size was
> evenly divisible into KVM_PGTABLE_MIN_BLOCK_LEVEL.
> 
> Seems like we could wind up visiting a PUD mapping 512 times, at least
> for 4K pages.

I have an idea, but it seems to go against the current design of the
pagtable walkers.  My sense was that they've been written to
discourage passing mutable state to the function that calls
kvm_pgtable_walk().  If we were willing to permit this, it seems like we
could leverage __kvm_pgtable_visit()'s knowledge about the size of the
mapping it walked to determine whether range_addr_end should be
incremented by our BLOCK_LEVEL constant, or advanced to the end of the
mapping that was already successfully walked.  (If I'm reading right,
anyway)  Does that seem like a reasonable approach?

If we do modify the walk to allow state to be passed back, I have a
second patch I'd like to send you.  Ali found that there was a
performance regression on the kvm_pagetable_test on the map creation
step when a large number of threads operated on a comparatively small
memory range.  (E.g. 64 cpus and 8g of RAM).  We debugged this a bit and
found that there's an unmap in the map creation step if the test ends up
instantiating a readable zero page that needs to be copied and made
writable.  With the deferred TLBI logic, the tlb invalidation happens at
the end of the unmap operation whether a PTE is cleared or not.  With so
many threads, this doesn't always suceeed. The prior approach of just
doing the invalidation in stage2_unmap_put_pte() outperforms the
deferred invalidation, because stage2_unmap_put_pte() only calls
__kvm_tlb_flush_vmid_ipa() if it clears a valid PTE.  If we modify the
walk to keep state on whether any PTEs are successfully cleared, and
condition the deferred invalidation on that state, we obtain performance
that is equivalent to the pre range based deferred invalidation
approach.

Thanks,

-K

_______________________________________________
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 09/23] media: i2c: imx258: Add support for running on 2 CSI data lanes
From: Luigi311 @ 2024-03-29 19:10 UTC (permalink / raw)
  To: Sakari Ailus
  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
In-Reply-To: <493be807-819b-4871-a996-cbe0dd020065@luigi311.com>

On 3/28/24 17:42, Luigi311 wrote:
> On 3/28/24 02:19, Sakari Ailus wrote:
>> Hi Luigi311, Dave,
>>
>> On Wed, Mar 27, 2024 at 05:16:55PM -0600, git@luigi311.com wrote:
>>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>>
>>> Extends the driver to also support 2 data lanes.
>>> Frame rates are obviously more restricted on 2 lanes, but some
>>> hardware simply hasn't wired more up.
>>>
>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>> Signed-off-by: Luigi311 <git@luigi311.com>
>>> ---
>>>  drivers/media/i2c/imx258.c | 214 ++++++++++++++++++++++++++++++++-----
>>>  1 file changed, 190 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
>>> index 6ee7de079454..c65b9aad3b0a 100644
>>> --- a/drivers/media/i2c/imx258.c
>>> +++ b/drivers/media/i2c/imx258.c
>>> @@ -86,12 +86,18 @@ struct imx258_reg_list {
>>>  	const struct imx258_reg *regs;
>>>  };
>>>  
>>> +enum {
>>> +	IMX258_2_LANE_MODE,
>>> +	IMX258_4_LANE_MODE,
>>> +	IMX258_LANE_CONFIGS,
>>> +};
>>> +
>>>  /* Link frequency config */
>>>  struct imx258_link_freq_config {
>>>  	u32 pixels_per_line;
>>>  
>>>  	/* PLL registers for this link frequency */
>>> -	struct imx258_reg_list reg_list;
>>> +	struct imx258_reg_list reg_list[IMX258_LANE_CONFIGS];
>>>  };
>>>  
>>>  /* Mode : resolution and related config&values */
>>> @@ -111,8 +117,34 @@ struct imx258_mode {
>>>  	struct imx258_reg_list reg_list;
>>>  };
>>>  
>>> -/* 4208x3120 needs 1267Mbps/lane, 4 lanes */
>>> -static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
>>> +/*
>>> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
>>> + * To avoid further computation of clock settings, adopt the same per
>>> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
>>> + */
>>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
>>> +	{ 0x0136, 0x13 },
>>> +	{ 0x0137, 0x33 },
>>> +	{ 0x0301, 0x0A },
>>> +	{ 0x0303, 0x02 },
>>> +	{ 0x0305, 0x03 },
>>> +	{ 0x0306, 0x00 },
>>> +	{ 0x0307, 0xC6 },
>>> +	{ 0x0309, 0x0A },
>>> +	{ 0x030B, 0x01 },
>>> +	{ 0x030D, 0x02 },
>>> +	{ 0x030E, 0x00 },
>>> +	{ 0x030F, 0xD8 },
>>> +	{ 0x0310, 0x00 },
>>> +
>>> +	{ 0x0114, 0x01 },
>>> +	{ 0x0820, 0x09 },
>>> +	{ 0x0821, 0xa6 },
>>> +	{ 0x0822, 0x66 },
>>> +	{ 0x0823, 0x66 },
>>> +};
>>> +
>>> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
>>>  	{ 0x0136, 0x13 },
>>>  	{ 0x0137, 0x33 },
>>>  	{ 0x0301, 0x05 },
>>> @@ -126,16 +158,18 @@ static const struct imx258_reg mipi_1267mbps_19_2mhz[] = {
>>>  	{ 0x030E, 0x00 },
>>>  	{ 0x030F, 0xD8 },
>>>  	{ 0x0310, 0x00 },
>>> +
>>> +	{ 0x0114, 0x03 },
>>>  	{ 0x0820, 0x13 },
>>>  	{ 0x0821, 0x4C },
>>>  	{ 0x0822, 0xCC },
>>>  	{ 0x0823, 0xCC },
>>>  };
>>>  
>>> -static const struct imx258_reg mipi_1272mbps_24mhz[] = {
>>> +static const struct imx258_reg mipi_1272mbps_24mhz_2l[] = {
>>>  	{ 0x0136, 0x18 },
>>>  	{ 0x0137, 0x00 },
>>> -	{ 0x0301, 0x05 },
>>> +	{ 0x0301, 0x0a },
>>>  	{ 0x0303, 0x02 },
>>>  	{ 0x0305, 0x04 },
>>>  	{ 0x0306, 0x00 },
>>> @@ -146,13 +180,59 @@ static const struct imx258_reg mipi_1272mbps_24mhz[] = {
>>>  	{ 0x030E, 0x00 },
>>>  	{ 0x030F, 0xD8 },
>>>  	{ 0x0310, 0x00 },
>>> +
>>> +	{ 0x0114, 0x01 },
>>>  	{ 0x0820, 0x13 },
>>>  	{ 0x0821, 0x4C },
>>>  	{ 0x0822, 0xCC },
>>>  	{ 0x0823, 0xCC },
>>>  };
>>>  
>>> -static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
>>> +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 },
>>> +};
>>> +
>>> +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 },
>>> +};
>>> +
>>> +static const struct imx258_reg mipi_640mbps_19_2mhz_4l[] = {
>>>  	{ 0x0136, 0x13 },
>>>  	{ 0x0137, 0x33 },
>>>  	{ 0x0301, 0x05 },
>>> @@ -166,13 +246,37 @@ static const struct imx258_reg mipi_640mbps_19_2mhz[] = {
>>>  	{ 0x030E, 0x00 },
>>>  	{ 0x030F, 0xD8 },
>>>  	{ 0x0310, 0x00 },
>>> +
>>> +	{ 0x0114, 0x03 },
>>> +	{ 0x0820, 0x0A },
>>> +	{ 0x0821, 0x00 },
>>> +	{ 0x0822, 0x00 },
>>> +	{ 0x0823, 0x00 },
>>> +};
>>> +
>>> +static const struct imx258_reg mipi_642mbps_24mhz_2l[] = {
>>> +	{ 0x0136, 0x18 },
>>> +	{ 0x0137, 0x00 },
>>> +	{ 0x0301, 0x0A },
>>> +	{ 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 },
>>>  };
>>>  
>>> -static const struct imx258_reg mipi_642mbps_24mhz[] = {
>>> +static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
>>>  	{ 0x0136, 0x18 },
>>>  	{ 0x0137, 0x00 },
>>>  	{ 0x0301, 0x05 },
>>> @@ -186,6 +290,8 @@ static const struct imx258_reg mipi_642mbps_24mhz[] = {
>>>  	{ 0x030E, 0x00 },
>>>  	{ 0x030F, 0xD8 },
>>>  	{ 0x0310, 0x00 },
>>> +
>>> +	{ 0x0114, 0x03 },
>>>  	{ 0x0820, 0x0A },
>>>  	{ 0x0821, 0x00 },
>>>  	{ 0x0822, 0x00 },
>>> @@ -240,7 +346,6 @@ static const struct imx258_reg mode_common_regs[] = {
>>>  	{ 0x5F05, 0xED },
>>>  	{ 0x0112, 0x0A },
>>>  	{ 0x0113, 0x0A },
>>> -	{ 0x0114, 0x03 },
>>>  	{ 0x0342, 0x14 },
>>>  	{ 0x0343, 0xE8 },
>>>  	{ 0x0344, 0x00 },
>>> @@ -359,11 +464,13 @@ enum {
>>>  
>>>  /*
>>>   * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
>>> - * data rate => double data rate; number of lanes => 4; bits per pixel => 10
>>> + * data rate => double data rate;
>>> + * number of lanes => (configurable 2 or 4);
>>> + * bits per pixel => 10
>>>   */
>>> -static u64 link_freq_to_pixel_rate(u64 f)
>>> +static u64 link_freq_to_pixel_rate(u64 f, unsigned int nlanes)
>>>  {
>>> -	f *= 2 * 4;
>>> +	f *= 2 * nlanes;
>>>  	do_div(f, 10);
>>>  
>>>  	return f;
>>> @@ -393,15 +500,27 @@ static const struct imx258_link_freq_config link_freq_configs_19_2[] = {
>>>  	[IMX258_LINK_FREQ_1267MBPS] = {
>>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>>  		.reg_list = {
>>> -			.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz),
>>> -			.regs = mipi_1267mbps_19_2mhz,
>>> +			[IMX258_2_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz_2l),
>>> +				.regs = mipi_1267mbps_19_2mhz_2l,
>>> +			},
>>> +			[IMX258_4_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_1267mbps_19_2mhz_4l),
>>> +				.regs = mipi_1267mbps_19_2mhz_4l,
>>> +			},
>>>  		}
>>>  	},
>>>  	[IMX258_LINK_FREQ_640MBPS] = {
>>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>>  		.reg_list = {
>>> -			.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz),
>>> -			.regs = mipi_640mbps_19_2mhz,
>>> +			[IMX258_2_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz_2l),
>>> +				.regs = mipi_640mbps_19_2mhz_2l,
>>> +			},
>>> +			[IMX258_4_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_640mbps_19_2mhz_4l),
>>> +				.regs = mipi_640mbps_19_2mhz_4l,
>>> +			},
>>>  		}
>>>  	},
>>>  };
>>> @@ -410,15 +529,27 @@ static const struct imx258_link_freq_config link_freq_configs_24[] = {
>>>  	[IMX258_LINK_FREQ_1267MBPS] = {
>>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>>  		.reg_list = {
>>> -			.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz),
>>> -			.regs = mipi_1272mbps_24mhz,
>>> +			[IMX258_2_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz_2l),
>>> +				.regs = mipi_1272mbps_24mhz_2l,
>>> +			},
>>> +			[IMX258_4_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_1272mbps_24mhz_4l),
>>> +				.regs = mipi_1272mbps_24mhz_4l,
>>> +			},
>>>  		}
>>>  	},
>>>  	[IMX258_LINK_FREQ_640MBPS] = {
>>>  		.pixels_per_line = IMX258_PPL_DEFAULT,
>>>  		.reg_list = {
>>> -			.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz),
>>> -			.regs = mipi_642mbps_24mhz,
>>> +			[IMX258_2_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz_2l),
>>> +				.regs = mipi_642mbps_24mhz_2l,
>>> +			},
>>> +			[IMX258_4_LANE_MODE] = {
>>> +				.num_of_regs = ARRAY_SIZE(mipi_642mbps_24mhz_4l),
>>> +				.regs = mipi_642mbps_24mhz_4l,
>>> +			},
>>>  		}
>>>  	},
>>>  };
>>> @@ -477,6 +608,7 @@ struct imx258 {
>>>  
>>>  	const struct imx258_link_freq_config *link_freq_configs;
>>>  	const s64 *link_freq_menu_items;
>>> +	unsigned int nlanes;
>>>  
>>>  	/*
>>>  	 * Mutex for serialized access:
>>> @@ -782,7 +914,7 @@ static int imx258_set_pad_format(struct v4l2_subdev *sd,
>>>  		__v4l2_ctrl_s_ctrl(imx258->link_freq, mode->link_freq_index);
>>>  
>>>  		link_freq = imx258->link_freq_menu_items[mode->link_freq_index];
>>> -		pixel_rate = link_freq_to_pixel_rate(link_freq);
>>> +		pixel_rate = link_freq_to_pixel_rate(link_freq, imx258->nlanes);
>>>  		__v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate, pixel_rate);
>>>  		/* Update limits and set FPS to default */
>>>  		vblank_def = imx258->cur_mode->vts_def -
>>> @@ -811,11 +943,13 @@ static int imx258_start_streaming(struct imx258 *imx258)
>>>  {
>>>  	struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
>>>  	const struct imx258_reg_list *reg_list;
>>> +	const struct imx258_link_freq_config *link_freq_cfg;
>>>  	int ret, link_freq_index;
>>>  
>>>  	/* Setup PLL */
>>>  	link_freq_index = imx258->cur_mode->link_freq_index;
>>> -	reg_list = &imx258->link_freq_configs[link_freq_index].reg_list;
>>> +	link_freq_cfg = &imx258->link_freq_configs[link_freq_index];
>>> +	reg_list = &link_freq_cfg->reg_list[imx258->nlanes == 2 ? 0 : 1];
>>>  	ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs);
>>>  	if (ret) {
>>>  		dev_err(&client->dev, "%s failed to set plls\n", __func__);
>>> @@ -1033,9 +1167,11 @@ static int imx258_init_controls(struct imx258 *imx258)
>>>  		vflip->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>>  
>>>  	pixel_rate_max =
>>> -		link_freq_to_pixel_rate(imx258->link_freq_menu_items[0]);
>>> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[0],
>>> +					imx258->nlanes);
>>>  	pixel_rate_min =
>>> -		link_freq_to_pixel_rate(imx258->link_freq_menu_items[1]);
>>> +		link_freq_to_pixel_rate(imx258->link_freq_menu_items[1],
>>> +					imx258->nlanes);
>>>  	/* By default, PIXEL_RATE is read only */
>>>  	imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
>>>  				V4L2_CID_PIXEL_RATE,
>>> @@ -1132,6 +1268,10 @@ static int imx258_get_regulators(struct imx258 *imx258,
>>>  static int imx258_probe(struct i2c_client *client)
>>>  {
>>>  	struct imx258 *imx258;
>>> +	struct fwnode_handle *endpoint;
>>> +	struct v4l2_fwnode_endpoint ep = {
>>> +		.bus_type = V4L2_MBUS_CSI2_DPHY
>>> +	};
>>>  	int ret;
>>>  	u32 val = 0;
>>>  
>>> @@ -1172,13 +1312,35 @@ static int imx258_probe(struct i2c_client *client)
>>>  		return -EINVAL;
>>>  	}
>>>  
>>> +	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
>>> +	if (!endpoint) {
>>> +		dev_err(&client->dev, "Endpoint node not found\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep);
>>
>> Here you're obtaining the list of supported link frequencies from the
>> firmware but it is not validated (nor it was validated by the driver
>> previously). I'd regard that a driver bug but fixing it at this point could
>> introduce adverse effects elsewhere.
>>
>> I think what I'd do here is that I'd ignore the issue if there are no
>> frequencies defined for the endpoint but if there are, then enable only
>> those that are listed in the endpoint.
>>
>> Could you add a patch to do this, please? v4l2_link_freq_to_bitmap() has
>> been recently added to facilitate this.
>>
> 
> I can give this a try, it would be similar to this patch that you submitted
> earlier for the imx319 here
> https://github.com/torvalds/linux/commit/726a09c1b6890887b7388745a26c8e93867780ca
> right? 
> 

I believe I got this implemented, added in that v4l2_link_freq_to_bitmap()
and it failed to probe the imx258 device due to missing frequencies so I
checked the device dts and the imx258 had no link-frequencies specified so
added in 321000000 and 636000000 to match the 24mhz and it worked, I then
swapped it over to bogus numbers 311000000 and 626000000 and it complained
about no matching link frequencies found so it failed to probe. Looks
like its working with that new function now. Will include in next revision

>>> +	fwnode_handle_put(endpoint);
>>> +	if (ret) {
>>> +		dev_err(&client->dev, "Parsing endpoint node failed\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	/* Get number of data lanes */
>>> +	imx258->nlanes = ep.bus.mipi_csi2.num_data_lanes;
>>> +	if (imx258->nlanes != 2 && imx258->nlanes != 4) {
>>> +		dev_err(&client->dev, "Invalid data lanes: %u\n",
>>> +			imx258->nlanes);
>>> +		ret = -EINVAL;
>>> +		goto error_endpoint_free;
>>> +	}
>>> +
>>>  	/* Initialize subdev */
>>>  	v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>>>  
>>>  	/* Will be powered off via pm_runtime_idle */
>>>  	ret = imx258_power_on(&client->dev);
>>>  	if (ret)
>>> -		return ret;
>>> +		goto error_endpoint_free;
>>>  
>>>  	/* Check module identity */
>>>  	ret = imx258_identify_module(imx258);
>>> @@ -1211,6 +1373,7 @@ static int imx258_probe(struct i2c_client *client)
>>>  	pm_runtime_set_active(&client->dev);
>>>  	pm_runtime_enable(&client->dev);
>>>  	pm_runtime_idle(&client->dev);
>>> +	v4l2_fwnode_endpoint_free(&ep);
>>>  
>>>  	return 0;
>>>  
>>> @@ -1223,6 +1386,9 @@ static int imx258_probe(struct i2c_client *client)
>>>  error_identify:
>>>  	imx258_power_off(&client->dev);
>>>  
>>> +error_endpoint_free:
>>> +	v4l2_fwnode_endpoint_free(&ep);
>>> +
>>>  	return ret;
>>>  }
>>>  
>>
> 


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

^ permalink raw reply


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