Re: [PATCH 2/2] dt-bindings: mfd: ahc1ec0.yaml: Add Advantech Embedded Controll - AHC1EC0

On Fri, Oct 23, 2020 at 1:30 AM Shihlun.Lin
[off-list ref] wrote:

Hi Rob,

Thank you for taking the time for checking.

quoted

-----Original Message-----
From: Rob Herring [mailto:robh@kernel.org]
Sent: Saturday, October 17, 2020 2:31 AM
To: Shihlun.Lin
Cc: Lee Jones; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
Campion.Kang; AceLan Kao
Subject: Re: [PATCH 2/2] dt-bindings: mfd: ahc1ec0.yaml: Add Advantech
Embedded Controll - AHC1EC0

On Wed, Oct 14, 2020 at 04:29:41PM +0800, Shihlun Lin wrote:

quoted

Add DT binding schema for Advantech embedded controller AHC1EC0.

Where's the driver?

Because the maintainers are different people, I separated the submission
into three groups: maintainer, device tree, and Kernel driver. Should I put
all maintainers into one Email and send all patches in a group at once?

quoted

quoted

Signed-off-by: Shihlun Lin <redacted>
---
 .../devicetree/bindings/mfd/ahc1ec0.yaml      | 76

+++++++++++++++++++

quoted

 1 file changed, 76 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/mfd/ahc1ec0.yaml

quoted

diff --git a/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml

b/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml

quoted

new file mode 100644
index 000000000000..2a3581d2ecab

--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml

@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+# All the top-level keys are standard json-schema keywords except for
+# 'maintainers' and 'select'

Drop this.

I checked other yaml files. Should I drop all content above, or just remove
the last two lines?

quoted

quoted

+
+
+$id: http://devicetree.org/schemas/mfd/ahc1ec0.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Advantech Embedded Controller (AHC1EC0)
+
+maintainers:
+  - Shihlun Lin <shihlun.lin@advantech.com.tw>
+  - Campion Kang <campion.kang@advantech.com.tw>
+
+description: |
+  AHC1EC0 is one of the embedded controllers used by Advantech to

provide several

quoted

+  functions such as watchdog, hwmon, brightness, etc. Advantech

related applications

quoted

+  can control the whole system via these functions.
+
+  # In this case, a 'false' schema will never match.

Drop

I checked other yaml files. I should remove the last line only, right?

quoted

quoted

+
+properties:
+  compatible:
+    const: advantech,ahc1ec0
+
+  advantech,sub-dev-nb:
+    description:
+      The number of sub-devices specified in the platform.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maxItems: 1

Isn't this just the length of the next property?

You are right, exactly. This is just the number of item about the next property.
This property will be used in the driver later.

quoted

quoted

+
+  advantech,sub-dev:
+    description:
+      A list of the sub-devices supported in the platform. Defines for

the

quoted

+      appropriate values can found in dt-bindings/mfd/ahc1ec0.h.
+    $ref: "/schemas/types.yaml#/definitions/uint32-array"
+    minItems: 1
+    maxItems: 6

This is kind of odd. Aren't you going to need DT info for each of the
sub-dev. GPIO is a provider, backlight connection, LED properties, etc.

We will submit 6 sub-devices step-by-step totally. But this time, we only submit
2 sub-devices to upstream (Watchdog and HWMonitor).

Should we remove all information about other sub-devices in the
mfd core Kernel driver, and device tree documentation? Or just keep them?

You should submit a binding for a complete device. At least, as
complete as possible.

What you have here isn't something any other MFD device has done. So
that's not the right way as there's not likely anything special about
this device.

Rob