Re: [PATCH 1/3] dt-bindings: soc: samsung: exynos-speedy: Document SPEEDY host controller bindings

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

 



Hi Krzysztof,

On 12/13/24 9:40 AM, Krzysztof Kozlowski wrote:
On 12/12/2024 22:09, Markuss Broks wrote:
Add the schema for the Samsung SPEEDY serial bus host controller.
The bus has 4 bit wide addresses for addressing devices
and 8 bit wide register addressing. Each register is also 8
bit long, so the address can be 0-f (hexadecimal), node name
for child device follows the format: node_name@[0-f].

This wasn't tested so limited review.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Co-developed-by: Maksym Holovach <nergzd@xxxxxxxxxxxxx>
Signed-off-by: Maksym Holovach <nergzd@xxxxxxxxxxxxx>
Signed-off-by: Markuss Broks <markuss.broks@xxxxxxxxx>
---
  .../bindings/soc/samsung/exynos-speedy.yaml        | 78 ++++++++++++++++++++++
Filename must match compatible.

  1 file changed, 78 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..304b322a74ea70f23d8c072b44b6ca86b7cc807f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/samsung/exynos-speedy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos SPEEDY serial bus host controller
Speedy or SPEEDY?
Technically it's an acronym (Serial Protocol in an EffEctive Digital waY), but we could agree on if we use the capitalized or uncapitalised version and use it consistently throughout.

+
+maintainers:
+  - Markuss Broks <markuss.broks@xxxxxxxxx>
+
+description:
+  Samsung SPEEDY is a proprietary Samsung serial 1-wire bus.
1-wire? But not compatible with w1 (onwire)?
Nope, I suppose this requires more clarification, as explained in the previous letter, there are several differences between the protocols, looking at the Samsung patent. [1]

+  It is used on various Samsung Exynos chips. The bus can
+  address at most 4 bit (16) devices. The devices on the bus
+  have 8 bit long register line, and the registers are also
+  8 bit long each. It is typically used for communicating with
+  Samsung PMICs (s2mps17, s2mps18, ...) and other Samsung chips,
+  such as RF parts.
+
+properties:
+  compatible:
+    - items:
+        - enum:
+            - samsung,exynos9810-speedy
+        - const: samsung,exynos-speedy
Drop last compatible and use only SoC specific.
Makes sense, for some reason I didn't realise it doesn't make much sense.

+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    - const: pclk
Drop clock-names, not needed for one entry.

+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
You do not have them in the properties, anyway required goes before
additionalProperties

+
+patternProperties:
+  "^[a-z][a-z0-9]*@[0-9a-f]$":
That's odd regex. Look at other bus bindings.
Okay, I'll look into it.

+    type: object
+    additionalProperties: true
+
+    properties:
+      reg:
+        maxItems: 1
maximum: 15

+
+    required:
+      - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    speedy0: speedy@141c0000 {
Drop unused label.

+      compatible = "samsung,exynos9810-speedy",
+                   "samsung-exynos-speedy";
+      reg = <0x141c0000 0x2000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+
No resources? No clocks? No interrupts?
Will extend the example.



Best regards,
Krzysztof

- Markuss


[1] https://patents.google.com/patent/US9882711B1/en





[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux for Synopsys ARC Processors]    
  • [Linux on Unisoc (RDA Micro) SoCs]     [Linux Actions SoC]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  •   Powered by Linux