Re: [PATCH v3 1/7] dt-bindings: reset: Add AOSS reset bindings for SDM845 SoCs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bjorn,
Thanks for the review.

On 03/19/2018 04:14 AM, Bjorn Andersson wrote:
On Wed 14 Mar 02:21 PDT 2018, Sibi S wrote:
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: must specify the base address and size of the register
+	            space.
+
+

Double empty lines.


will remove them

+- #reset-cells:
+	Usage: required
+	Value type: <uint>
+	Definition: must be 1; cell entry represents the reset index.
+
+example:

Please capitalize the initial char.


sure

+
+aoss_reset: qcom,reset-controller@b2e0100 {
+	compatible = "qcom,sdm845-aoss-reset";
+	reg = <0xc2b0000 0x20004>;

0x20004 does seem very even, please verify this size.


even though the reg space after that range is unused, the AOSS reset
driver is supposed to control only those listed reset lines

+	#reset-cells = <1>;
+};
+
+
+Specifying reset lines connected to IP modules

"AOSS reset clients"


yep this heading makes much more sense

Although you could probably get a way with just referring to reset.txt
and the header file.

+==============================================
+
+Device nodes that need access to reset lines should
+specify them as a reset phandle in their corresponding node as
+specified in reset.txt.
+
+For list of all valid reset indicies see
+<dt-bindings/reset/qcom,sdm845-aoss.h>
+
+Example:
+
+modem-pil@4080000 {
+	...
+
+	resets = <&aoss_reset AOSS_CC_MSS_RESTART>;
+	reset-names = "mss_restart";
+
+	...
+};
diff --git a/include/dt-bindings/reset/qcom,sdm845-aoss.h b/include/dt-bindings/reset/qcom,sdm845-aoss.h
new file mode 100644
index 0000000..e9b38fc
--- /dev/null
+++ b/include/dt-bindings/reset/qcom,sdm845-aoss.h
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0

For tooling reasons header files should have their SPDX License tag
wrapped in /* */


Sure will replace it

+/*
+ * Copyright (C) 2018 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_RESET_AOSS_SDM_845_H
+#define _DT_BINDINGS_RESET_AOSS_SDM_845_H
+
+#define AOSS_CC_MSS_RESTART	0
+#define AOSS_CC_CAMSS_RESTART	1
+#define AOSS_CC_VENUS_RESTART	2
+#define AOSS_CC_GPU_RESTART	3
+#define AOSS_CC_DISPSS_RESTART	4
+#define AOSS_CC_WCSS_RESTART	5
+#define AOSS_CC_LPASS_RESTART	6
+
+#endif

Apart from these nits this looks reasonable.

Regards,
Bjorn


--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux