On Wed, May 10, 2023 at 08:30:25PM +0530, Rohit Agarwal wrote:Will update all the comments in the next patch version.
Add initial Qualcomm SDX75 pinctrl driver to support pin configurationNice, some comment below.
with pinctrl framework for SDX75 SoC.
While at it, reordering the SDX65 entry.
Signed-off-by: Rohit Agarwal <quic_rohiagar@xxxxxxxxxxx>[..]
---
diff --git a/drivers/pinctrl/qcom/pinctrl-sdx75.c b/drivers/pinctrl/qcom/pinctrl-sdx75.cWe typically reference the inner TLMM block and omit this offset... But
new file mode 100644
index 0000000..6f95c0a
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-sdx75.c
@@ -0,0 +1,1595 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include "pinctrl-msm.h"
+
+#define REG_BASE 0x100000
I don't have a strong opinion.
[..]
+enum sdx75_functions {Please sort these alphabetically.
+ msm_mux_gpio,
+ msm_mux_eth0_mdc,[..]
+ msm_mux__,[..]
+};
+
+static const struct pinfunction sdx75_functions[] = {Please sort these alphabetically, and please squash individual pins into
+ MSM_PIN_FUNCTION(gpio),
+ MSM_PIN_FUNCTION(eth0_mdc),
their functional group.
[..]
+ MSM_PIN_FUNCTION(qup_se0_l0),E.g. this forces the DT writer to write individual -pins for each
+ MSM_PIN_FUNCTION(qup_se0_l1),
+ MSM_PIN_FUNCTION(qup_se0_l2),
+ MSM_PIN_FUNCTION(qup_se0_l3),
signal. Better keep it "qup_se0" and the author is free to group the
pins in their states as they need (and as you know you don't need to
specify all pins for a given function).
[..]
+};Please omit this comment.
+
+/* Every pin is maintained as a single group, and missing or non-existing pin
+ * would be maintained as dummy group to synchronize pin group index with
+ * pin descriptor registered with pinctrl core.
+ * Clients would not be able to request these dummy pin groups.
+ */
+static const struct msm_pingroup sdx75_groups[] = {[..]
+ [16] = PINGROUP(16, pri_mi2s_ws, qup_se2_l2, qup_se1_l2_mirb, qdss_cti,Please break the rules and leave these lines unwrapped.
+ qdss_cti, _, _, _, _, _),
+ [17] = PINGROUP(17, pri_mi2s_data0, qup_se2_l3, qup_se1_l3_mirb,[..]
+ qdss_cti, qdss_cti, _, _, _, _, _),
+ [131] = PINGROUP(131, _, _, _, _, _, _, _, _, _, _),Lowercase hex digits please.
+ [132] = PINGROUP(132, _, _, _, _, _, _, _, _, _, _),
+ [133] = SDC_QDSD_PINGROUP(sdc1_rclk, 0x19A000, 16, 0),
+ [134] = SDC_QDSD_PINGROUP(sdc1_clk, 0x19A000, 14, 6),[..]
+ [135] = SDC_QDSD_PINGROUP(sdc1_cmd, 0x19A000, 11, 3),
+ [136] = SDC_QDSD_PINGROUP(sdc1_data, 0x19A000, 9, 0),
+ [137] = SDC_QDSD_PINGROUP(sdc2_clk, 0x19B000, 14, 6),
+ [138] = SDC_QDSD_PINGROUP(sdc2_cmd, 0x19B000, 11, 3),
+ [139] = SDC_QDSD_PINGROUP(sdc2_data, 0x19B000, 9, 0),
+};
+static const struct of_device_id sdx75_pinctrl_of_match[] = {[..]
+ { .compatible = "qcom,sdx75-tlmm", .data = &sdx75_pinctrl },
+ { }
+};
+
+Keep the MODULE_DEVICE_TABLE() just below the sdx75_pinctrl_of_match
+MODULE_DESCRIPTION("QTI sdx75 pinctrl driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, sdx75_pinctrl_of_match);
please, so future readers doesn't need to search for it.
Regards,
Bjorn