[PATCH 1/2] mfd: ahc1ec0: Add Advantech EC include file used by dt-bindings

[PATCH 1/2] mfd: ahc1ec0: Add Advantech EC include file used by dt-bindings

[Shihlun Lin]

This files defines the sud-device types and hwmon profiles support by
Advantech embedded controller.

Signed-off-by: Shihlun Lin <shihlun.lin@advantech.com.tw>
---
 include/dt-bindings/mfd/ahc1ec0.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 include/dt-bindings/mfd/ahc1ec0.h

diff --git a/include/dt-bindings/mfd/ahc1ec0.h b/include/dt-bindings/mfd/ahc1ec0.h
new file mode 100644
index 000000000000..767d29bcb78e
--- /dev/null
+++ b/include/dt-bindings/mfd/ahc1ec0.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Device Tree defines for Advantech Embedded Controller (AHC1EC0)
+ *
+ * Copyright 2020 Advantech Co., Ltd.
+ *
+ * Author: William Sung <william.sung@advantech.com.tw>
+ */
+
+#ifndef _DT_BINDINGS_MFD_AHC1EC0_H
+#define _DT_BINDINGS_MFD_AHC1EC0_H
+
+/* Sub-device Definitions */
+#define AHC1EC0_SUBDEV_BRIGHTNESS 0x0
+#define AHC1EC0_SUBDEV_EEPROM     0x1
+#define AHC1EC0_SUBDEV_GPIO       0x2
+#define AHC1EC0_SUBDEV_HWMON      0x3
+#define AHC1EC0_SUBDEV_LED        0x4
+#define AHC1EC0_SUBDEV_WDT        0x5
+
+/* HWMON Profile Definitions */
+#define AHC1EC0_HWMON_PRO_TEMPLATE 0x0
+#define AHC1EC0_HWMON_PRO_TPC5XXX  0x1
+#define AHC1EC0_HWMON_PRO_PRVR4    0x2
+#define AHC1EC0_HWMON_PRO_UNO2271G 0x3
+#define AHC1EC0_HWMON_PRO_UNO1172A 0x4
+#define AHC1EC0_HWMON_PRO_UNO1372G 0x5
+
+#endif
-- 
2.17.1

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

[Shihlun Lin]

Add DT binding schema for Advantech embedded controller AHC1EC0.

Signed-off-by: Shihlun Lin <shihlun.lin@advantech.com.tw>
---
 .../devicetree/bindings/mfd/ahc1ec0.yaml      | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/ahc1ec0.yaml

diff --git a/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml b/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml
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'
+
+
+$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
+  functions such as watchdog, hwmon, brightness, etc. Advantech related applications
+  can control the whole system via these functions.
+
+  # In this case, a 'false' schema will never match.
+
+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
+
+  advantech,sub-dev:
+    description:
+      A list of the sub-devices supported in the platform. Defines for the
+      appropriate values can found in dt-bindings/mfd/ahc1ec0.h.
+    $ref: "/schemas/types.yaml#/definitions/uint32-array"
+    minItems: 1
+    maxItems: 6
+
+  advantech,hwmon-profile:
+    description:
+      The number of sub-devices specified in the platform. Defines for the
+      hwmon profiles can found in dt-bindings/mfd/ahc1ec0.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maxItems: 1
+
+required:
+  - compatible
+  - advantech,sub-dev-nb
+  - advantech,sub-dev
+
+if:
+  properties:
+    advantech,sub-dev:
+      contains:
+        const: AHC1EC0_SUBDEV_HWMON
+then:
+  required:
+    - advantech,hwmon-profile
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/mfd/ahc1ec0.h>
+    ahc1ec0 {
+        compatible = "advantech,ahc1ec0";
+
+        advantech,sub-dev-nb = <2>;
+        advantech,sub-dev = <AHC1EC0_SUBDEV_HWMON
+                             AHC1EC0_SUBDEV_WDT>;
+
+        advantech,hwmon-profile = <AHC1EC0_HWMON_PRO_UNO2271G>;
+    };
-- 
2.17.1

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

[Rob Herring]

On Wed, Oct 14, 2020 at 04:29:41PM +0800, Shihlun Lin wrote:
> Add DT binding schema for Advantech embedded controller AHC1EC0.

Where's the driver?

> 
> Signed-off-by: Shihlun Lin <shihlun.lin@advantech.com.tw>
> ---
>  .../devicetree/bindings/mfd/ahc1ec0.yaml      | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/ahc1ec0.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml b/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml
> 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. 

> +
> +
> +$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
> +  functions such as watchdog, hwmon, brightness, etc. Advantech related applications
> +  can control the whole system via these functions.
> +
> +  # In this case, a 'false' schema will never match.

Drop

> +
> +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?

> +
> +  advantech,sub-dev:
> +    description:
> +      A list of the sub-devices supported in the platform. Defines for the
> +      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.

> +
> +  advantech,hwmon-profile:
> +    description:
> +      The number of sub-devices specified in the platform. Defines for the
> +      hwmon profiles can found in dt-bindings/mfd/ahc1ec0.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - advantech,sub-dev-nb
> +  - advantech,sub-dev
> +
> +if:
> +  properties:
> +    advantech,sub-dev:
> +      contains:
> +        const: AHC1EC0_SUBDEV_HWMON

defines don't work here.

> +then:
> +  required:
> +    - advantech,hwmon-profile
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/mfd/ahc1ec0.h>
> +    ahc1ec0 {
> +        compatible = "advantech,ahc1ec0";
> +
> +        advantech,sub-dev-nb = <2>;
> +        advantech,sub-dev = <AHC1EC0_SUBDEV_HWMON
> +                             AHC1EC0_SUBDEV_WDT>;
> +
> +        advantech,hwmon-profile = <AHC1EC0_HWMON_PRO_UNO2271G>;
> +    };
> -- 
> 2.17.1
> 

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

[Rob Herring]

On Fri, Oct 23, 2020 at 1:30 AM Shihlun.Lin
<Shihlun.Lin@advantech.com.tw> wrote:
>
> Hi Rob,
>
> Thank you for taking the time for checking.
>
> > -----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:
> > > 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?
>
> > >
> > > Signed-off-by: Shihlun Lin <shihlun.lin@advantech.com.tw>
> > > ---
> > >  .../devicetree/bindings/mfd/ahc1ec0.yaml      | 76
> > +++++++++++++++++++
> > >  1 file changed, 76 insertions(+)
> > >  create mode 100644
> > Documentation/devicetree/bindings/mfd/ahc1ec0.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml
> > b/Documentation/devicetree/bindings/mfd/ahc1ec0.yaml
> > > 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?
>
> > > +
> > > +
> > > +$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
> > > +  functions such as watchdog, hwmon, brightness, etc. Advantech
> > related applications
> > > +  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?
>
> > > +
> > > +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.
>
> > > +
> > > +  advantech,sub-dev:
> > > +    description:
> > > +      A list of the sub-devices supported in the platform. Defines for
> > the
> > > +      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