From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S932261AbcLLSea (ORCPT <rfc822;w@1wt.eu>);
        Mon, 12 Dec 2016 13:34:30 -0500
Received: from mail-pg0-f54.google.com ([74.125.83.54]:35160 "EHLO
        mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753436AbcLLSe1 (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 12 Dec 2016 13:34:27 -0500
Date: Mon, 12 Dec 2016 10:34:23 -0800
From: Brian Norris <briannorris@chromium.org>
To: Rob Herring <robh@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Jiri Kosina <jikos@kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Doug Anderson <dianders@chromium.org>,
        Caesar Wang <wxt@rock-chips.com>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer +
 regulator support
Message-ID: <20161212183422.GA138477@google.com>
References: <CAD=FV=UMonRe8WJ1feDv3Nh9g8hEGAwwFT5NYLbzXADBqhTf-A@mail.gmail.com>
 <20161208154145.GA30888@mail.corp.redhat.com>
 <CAL_Jsq+fdoFnarXYkPv6FjVue3v31ry-Ddfkq9fQk9Ek+AjETw@mail.gmail.com>
 <E409F66F-1E4B-4A63-9977-69DF44ACAB0A@gmail.com>
 <CAL_JsqKu0yhLVyEjcZs_rn=VqM9O4F_VMhOkfhEEvbYAjvWSTg@mail.gmail.com>
 <alpine.LNX.2.00.1612091532240.16984@cbobk.fhfr.pm>
 <CAL_JsqLpjhHeeKq3PmNToux1Rgkg0M84-dv9HGCOws223ima4w@mail.gmail.com>
 <alpine.LNX.2.00.1612120951130.16984@cbobk.fhfr.pm>
 <20161212100110.GA13907@mail.corp.redhat.com>
 <CAL_JsqJ7t=C4+PgCyNTev66V33-cyPkUHKsTcbnsYgvAaffVfw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAL_JsqJ7t=C4+PgCyNTev66V33-cyPkUHKsTcbnsYgvAaffVfw@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi all,

On Mon, Dec 12, 2016 at 08:47:06AM -0600, Rob Herring wrote:

[Snip Benjamin's proposal; I agree we don't really want a multi-level DT
layout purely for making the driver look a little nicer (I'm not sure
this would really be nicer anyway). And I think, as Rob notes here, our
disagreement is smaller than appears. But I might be wrong.]

> Anyway, we're only debating this:

OK, so I think we might have a consensus of sorts? I'll describe it
here, in case I'm wrong. Otherwise, I'll send another rev.

>          i2c-hid-dev@2c {
>                  compatible = "wacom,w9013", "hid-over-i2c";

I plan to document the above, but not treat "wacom,w9013" specially in
the driver, apart from possibly listing it in the driver of_match_table.
This was mentioned by Dmitry earlier, and I didn't see any objection.

(Note that there are problems with module autoload when using multiple
compatible strings like above. May not be supremely relevant to the
documentation, but it *is* practically important.)

>                  reg = <0x2c>;
>                  hid-descr-addr = <0x0020>;
>                  interrupt-parent = <&gpx3>;
>                  interrupts = <3 2>;
>                  vdd-supply = <sth>;

Document and support 'vdd-supply', optionally.

>                  init-delay-ms = <100>;

Per Rob's mention below, support this as 'post-power-on-delay-ms',
optionally.

We can use either of these properties on any device, with the
intention that if there are future needs for divergent bindings, the
aforementioned compatible property can help us differentiate.

>          };
> 
> vs.
> 
>          i2c-hid-dev@2c {
>                  compatible = "hid-over-i2c";
>                  reg = <0x2c>;
>                  hid-descr-addr = <0x0020>;
>                  interrupt-parent = <&gpx3>;
>                  interrupts = <3 2>;
>                  vdd-supply = <sth>;
>                  init-delay-ms = <100>;
>          };
> 
> My only other nit is use "post-power-on-delay-ms" which is already a
> defined property name rather than "init-delay-ms".

Any objections? Speak now or forever [1] hold your peace.

Brian

[1] Who am I kidding? There's always room for more paint on the
bikeshed.