diff mbox series

[1/2] serial: msm-geni: don't rely on parent misc device

Message ID 20231114-b4-geni-uart-v1-1-48c8ba9dda36@linaro.org
State Accepted
Commit 836b7f447429960fa6ff8b6d25c11e39b8c00b94
Headers show
Series serial: msm-geni: rework oversampling and fix clk API bug | expand

Commit Message

Caleb Connolly Nov. 14, 2023, 12:51 p.m. UTC
commit 1b15483deb3f ("misc: add Qualcomm GENI SE QUP device driver")
introduced support for platform-specific oversampling values, necessary
to configure the UART clocks on all platforms at runtime. However it
relies in probing a parent device. Despite the DM_FLAG_PRE_RELOC flag,
this is not done consistently during boot.

Instead, take another approach by relying on ofnode_ helpers to read the
serial engine base address and do the read directly. This fixes early
UART on boards with a non-default oversampling rate.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/misc/Kconfig             |  7 -------
 drivers/misc/Makefile            |  1 -
 drivers/misc/qcom-geni-se.c      | 41 ----------------------------------------
 drivers/serial/Kconfig           |  2 --
 drivers/serial/serial_msm_geni.c | 29 ++++++++++++++++++----------
 5 files changed, 19 insertions(+), 61 deletions(-)

Comments

Caleb Connolly Nov. 14, 2023, 2:56 p.m. UTC | #1
> diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c
> index 78fd9389c036..3e2e15b6cefe 100644
> --- a/drivers/serial/serial_msm_geni.c
> +++ b/drivers/serial/serial_msm_geni.c
[...]
> @@ -499,16 +499,22 @@ static void geni_set_oversampling(struct udevice *dev)
>  	 * It could happen that GENI SE IP is missing in the board's device
>  	 * tree or GENI UART node is a direct child of SoC device tree node.
>  	 */
> -	if (device_get_uclass_id(parent_dev) != UCLASS_MISC)
> -		return;
> +	if (!ofnode_device_is_compatible(parent_node, "qcom,geni-se-qup")) {
> +		pr_err("%s: UART node must be a child of geniqup node\n",
> +		       __func__);
> +		return -ENODEV;
> +	}
>  
> -	ret = misc_read(parent_dev, QUP_HW_VER_REG,
> -			&geni_se_version, sizeof(geni_se_version));
> -	if (ret != sizeof(geni_se_version))
> -		return;
> +	/* Read the HW_VER register relative to the parents address space */
> +	addr = ofnode_get_addr(parent_node);
> +	geni_se_version = readl(addr + QUP_HW_VER_REG);
> +
> +	printf("geni_se_version: %x\n", geni_se_version);

Drop this debugging printf
>  
>  	if (geni_se_version >= QUP_SE_VERSION_2_5)
>  		priv->oversampling /= 2;
> +
> +	return 0;
>  }
>  
>  static inline void geni_serial_init(struct udevice *dev)
> @@ -553,8 +559,11 @@ static inline void geni_serial_init(struct udevice *dev)
>  static int msm_serial_probe(struct udevice *dev)
>  {
>  	struct msm_serial_data *priv = dev_get_priv(dev);
> +	int ret;
>  
> -	geni_set_oversampling(dev);
> +	ret = geni_set_oversampling(dev);
> +	if (ret < 0)
> +		return ret;
>  
>  	/* No need to reinitialize the UART after relocation */
>  	if (gd->flags & GD_FLG_RELOC)
>
diff mbox series

Patch

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 97057de8bf92..423cf22949aa 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -527,13 +527,6 @@  config WINBOND_W83627
 	  legacy UART or other devices in the Winbond Super IO chips
 	  on X86 platforms.
 
-config QCOM_GENI_SE
-	bool "Qualcomm GENI Serial Engine Driver"
-	depends on ARCH_SNAPDRAGON
-	help
-	  The driver manages Generic Interface (GENI) firmware based
-	  Qualcomm Technologies, Inc. Universal Peripheral (QUP) Wrapper.
-
 config QFW
 	bool
 	help
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index b67b82358a6c..2d08181251c7 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -60,7 +60,6 @@  obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o
 obj-$(CONFIG_P2SB) += p2sb-uclass.o
 obj-$(CONFIG_PCA9551_LED) += pca9551_led.o
 obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o
-obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
 ifdef CONFIG_QFW
 obj-y += qfw.o
 obj-$(CONFIG_QFW_PIO) += qfw_pio.o
diff --git a/drivers/misc/qcom-geni-se.c b/drivers/misc/qcom-geni-se.c
deleted file mode 100644
index 281a5ec819a9..000000000000
--- a/drivers/misc/qcom-geni-se.c
+++ /dev/null
@@ -1,41 +0,0 @@ 
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Qualcomm Generic Interface (GENI) Serial Engine (SE) Wrapper
- *
- * Copyright (C) 2023 Linaro Ltd. <vladimir.zapolskiy@linaro.org>
- */
-
-#include <common.h>
-#include <dm.h>
-#include <misc.h>
-#include <asm/io.h>
-
-static int geni_se_qup_read(struct udevice *dev, int offset,
-			    void *buf, int size)
-{
-	fdt_addr_t base = dev_read_addr(dev);
-
-	if (size != sizeof(u32))
-		return -EINVAL;
-
-	*(u32 *)buf = readl(base + offset);
-
-	return size;
-}
-
-static struct misc_ops geni_se_qup_ops = {
-	.read = geni_se_qup_read,
-};
-
-static const struct udevice_id geni_se_qup_ids[] = {
-	{ .compatible = "qcom,geni-se-qup" },
-	{}
-};
-
-U_BOOT_DRIVER(geni_se_qup) = {
-	.name = "geni_se_qup",
-	.id = UCLASS_MISC,
-	.of_match = geni_se_qup_ids,
-	.ops = &geni_se_qup_ops,
-	.flags  = DM_FLAG_PRE_RELOC,
-};
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 9f0f84c9b426..81fdac047824 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -957,8 +957,6 @@  config MSM_SERIAL
 
 config MSM_GENI_SERIAL
 	bool "Qualcomm on-chip GENI UART"
-	select MISC
-	imply QCOM_GENI_SE
 	help
 	  Support UART based on Generic Interface (GENI) Serial Engine (SE),
 	  used on Qualcomm Snapdragon SoCs. Should support all qualcomm SOCs
diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c
index 78fd9389c036..3e2e15b6cefe 100644
--- a/drivers/serial/serial_msm_geni.c
+++ b/drivers/serial/serial_msm_geni.c
@@ -486,12 +486,12 @@  static const struct dm_serial_ops msm_serial_ops = {
 	.setbrg = msm_serial_setbrg,
 };
 
-static void geni_set_oversampling(struct udevice *dev)
+static int geni_set_oversampling(struct udevice *dev)
 {
 	struct msm_serial_data *priv = dev_get_priv(dev);
-	struct udevice *parent_dev = dev_get_parent(dev);
+	ofnode parent_node = ofnode_get_parent(dev_ofnode(dev));
 	u32 geni_se_version;
-	int ret;
+	fdt_addr_t addr;
 
 	priv->oversampling = UART_OVERSAMPLING;
 
@@ -499,16 +499,22 @@  static void geni_set_oversampling(struct udevice *dev)
 	 * It could happen that GENI SE IP is missing in the board's device
 	 * tree or GENI UART node is a direct child of SoC device tree node.
 	 */
-	if (device_get_uclass_id(parent_dev) != UCLASS_MISC)
-		return;
+	if (!ofnode_device_is_compatible(parent_node, "qcom,geni-se-qup")) {
+		pr_err("%s: UART node must be a child of geniqup node\n",
+		       __func__);
+		return -ENODEV;
+	}
 
-	ret = misc_read(parent_dev, QUP_HW_VER_REG,
-			&geni_se_version, sizeof(geni_se_version));
-	if (ret != sizeof(geni_se_version))
-		return;
+	/* Read the HW_VER register relative to the parents address space */
+	addr = ofnode_get_addr(parent_node);
+	geni_se_version = readl(addr + QUP_HW_VER_REG);
+
+	printf("geni_se_version: %x\n", geni_se_version);
 
 	if (geni_se_version >= QUP_SE_VERSION_2_5)
 		priv->oversampling /= 2;
+
+	return 0;
 }
 
 static inline void geni_serial_init(struct udevice *dev)
@@ -553,8 +559,11 @@  static inline void geni_serial_init(struct udevice *dev)
 static int msm_serial_probe(struct udevice *dev)
 {
 	struct msm_serial_data *priv = dev_get_priv(dev);
+	int ret;
 
-	geni_set_oversampling(dev);
+	ret = geni_set_oversampling(dev);
+	if (ret < 0)
+		return ret;
 
 	/* No need to reinitialize the UART after relocation */
 	if (gd->flags & GD_FLG_RELOC)