[PATCH v3 1/2] dt-bindings: input: Add DT bindings for Whiskers switch

[PATCH v3 1/2] dt-bindings: input: Add DT bindings for Whiskers switch

[Ikjoon Jang]

Add the DT binding document for Hammer's TABLET_MODE switch.

Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
---
 .../devicetree/bindings/input/cros-cbas.yaml  | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cros-cbas.yaml

diff --git a/Documentation/devicetree/bindings/input/cros-cbas.yaml b/Documentation/devicetree/bindings/input/cros-cbas.yaml
new file mode 100644
index 000000000000..3bc989c6a295
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cros-cbas.yaml
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cros-cbas.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ChromeOS Hammer's Base Attached Switch
+
+maintainers:
+  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+description:
+  This device is used to signal when a detachable base is attached to a
+  Chrome OS tablet. The node for this device must be under a cros-ec node
+  like google,cros-ec-spi or google,cros-ec-i2c.
+
+properties:
+  compatible:
+    const: google,cros-cbas
+
+required:
+  - compatible
-- 
2.23.0.866.gb869b98d4c-goog

Re: [PATCH v3 1/2] dt-bindings: input: Add DT bindings for Whiskers switch

[Rob Herring]

On Wed, Oct 23, 2019 at 10:43:48AM +0800, Ikjoon Jang wrote:
> Add the DT binding document for Hammer's TABLET_MODE switch.

This doesn't have any properties. Why does it need to be in DT? Just 
have the EC driver instantiate it.

> 
> Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
> ---
>  .../devicetree/bindings/input/cros-cbas.yaml  | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cros-cbas.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/cros-cbas.yaml b/Documentation/devicetree/bindings/input/cros-cbas.yaml
> new file mode 100644
> index 000000000000..3bc989c6a295
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cros-cbas.yaml
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0

(GPL-2.0-only OR BSD-2-Clause) for new bindings please.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cros-cbas.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ChromeOS Hammer's Base Attached Switch
> +
> +maintainers:
> +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> +
> +description:
> +  This device is used to signal when a detachable base is attached to a
> +  Chrome OS tablet. The node for this device must be under a cros-ec node
> +  like google,cros-ec-spi or google,cros-ec-i2c.

This should probably just be part of an EC schema where it can be 
enforced that this is a child node. It could be either embedded into it 
or referenced. I'd lean toward the former given this is only a 
compatible string...

> +
> +properties:
> +  compatible:
> +    const: google,cros-cbas
> +
> +required:
> +  - compatible

Add here:

additionalProperties: false.

> -- 
> 2.23.0.866.gb869b98d4c-goog
> 

Re: [PATCH v3 1/2] dt-bindings: input: Add DT bindings for Whiskers switch

[Dmitry Torokhov]

On Fri, Oct 25, 2019 at 02:41:01PM -0500, Rob Herring wrote:
> On Wed, Oct 23, 2019 at 10:43:48AM +0800, Ikjoon Jang wrote:
> > Add the DT binding document for Hammer's TABLET_MODE switch.
> 
> This doesn't have any properties. Why does it need to be in DT? Just 
> have the EC driver instantiate it.

We need to know whether to instantiate it or not. EC does not allow
querying whether the switch is present.

Thanks.

-- 
Dmitry

Re: [PATCH v3 1/2] dt-bindings: input: Add DT bindings for Whiskers switch

[Ikjoon Jang]

On Sat, Oct 26, 2019 at 3:41 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Oct 23, 2019 at 10:43:48AM +0800, Ikjoon Jang wrote:
> > Add the DT binding document for Hammer's TABLET_MODE switch.
>
> This doesn't have any properties. Why does it need to be in DT? Just
> have the EC driver instantiate it.
>
> >
> > Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
> > ---
> >  .../devicetree/bindings/input/cros-cbas.yaml  | 22 +++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/cros-cbas.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/input/cros-cbas.yaml b/Documentation/devicetree/bindings/input/cros-cbas.yaml
> > new file mode 100644
> > index 000000000000..3bc989c6a295
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/cros-cbas.yaml
> > @@ -0,0 +1,22 @@
> > +# SPDX-License-Identifier: GPL-2.0
>
> (GPL-2.0-only OR BSD-2-Clause) for new bindings please.

This will be GPL-2.0-only in next patch.

>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/cros-cbas.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ChromeOS Hammer's Base Attached Switch
> > +
> > +maintainers:
> > +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > +
> > +description:
> > +  This device is used to signal when a detachable base is attached to a
> > +  Chrome OS tablet. The node for this device must be under a cros-ec node
> > +  like google,cros-ec-spi or google,cros-ec-i2c.
>
> This should probably just be part of an EC schema where it can be
> enforced that this is a child node. It could be either embedded into it
> or referenced. I'd lean toward the former given this is only a
> compatible string...

Sorry for basic questions here but I'm a bit confused,
"embedding" means that cros-ec.txt should be converted
into json schema first and embed this child bindings into there?

Many Chrome OS tablets have a switch exposed by 'cros-ec-keyb' device
which is directly controlled by EC. But this 'cros_cbas' switch device is for
other types of tablets which need additional logics on EC and HID.

Currently it doesn't need to have other properties, but maybe it could require
additional properties or device links in the future, plus this device
is not just
a EC subdevice, so I'd prefer this to be a separate binding.

>
> > +
> > +properties:
> > +  compatible:
> > +    const: google,cros-cbas
> > +
> > +required:
> > +  - compatible
>
> Add here:
>
> additionalProperties: false.

Okay, I will add this in a new patch set , thank you!

>
> > --
> > 2.23.0.866.gb869b98d4c-goog
> >

Re: [PATCH v3 1/2] dt-bindings: input: Add DT bindings for Whiskers switch

[Rob Herring]

On Tue, Oct 29, 2019 at 2:23 AM Ikjoon Jang <ikjn@chromium.org> wrote:
>
> On Sat, Oct 26, 2019 at 3:41 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Oct 23, 2019 at 10:43:48AM +0800, Ikjoon Jang wrote:
> > > Add the DT binding document for Hammer's TABLET_MODE switch.
> >
> > This doesn't have any properties. Why does it need to be in DT? Just
> > have the EC driver instantiate it.
> >
> > >
> > > Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
> > > ---
> > >  .../devicetree/bindings/input/cros-cbas.yaml  | 22 +++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/input/cros-cbas.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/cros-cbas.yaml b/Documentation/devicetree/bindings/input/cros-cbas.yaml
> > > new file mode 100644
> > > index 000000000000..3bc989c6a295
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/cros-cbas.yaml
> > > @@ -0,0 +1,22 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> >
> > (GPL-2.0-only OR BSD-2-Clause) for new bindings please.
>
> This will be GPL-2.0-only in next patch.

That's not what I said. Dual licensed please:

# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

>
> >
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/cros-cbas.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ChromeOS Hammer's Base Attached Switch
> > > +
> > > +maintainers:
> > > +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > +
> > > +description:
> > > +  This device is used to signal when a detachable base is attached to a
> > > +  Chrome OS tablet. The node for this device must be under a cros-ec node
> > > +  like google,cros-ec-spi or google,cros-ec-i2c.
> >
> > This should probably just be part of an EC schema where it can be
> > enforced that this is a child node. It could be either embedded into it
> > or referenced. I'd lean toward the former given this is only a
> > compatible string...
>
> Sorry for basic questions here but I'm a bit confused,
> "embedding" means that cros-ec.txt should be converted
> into json schema first and embed this child bindings into there?

Right.

> Many Chrome OS tablets have a switch exposed by 'cros-ec-keyb' device
> which is directly controlled by EC. But this 'cros_cbas' switch device is for
> other types of tablets which need additional logics on EC and HID.
>
> Currently it doesn't need to have other properties, but maybe it could require
> additional properties or device links in the future, plus this device
> is not just
> a EC subdevice, so I'd prefer this to be a separate binding.

But in the description you say it is a child device.

Rob