All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	linux-input@vger.kernel.org, phone-devel@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: input: Add bindings for Immersion ISA1200
Date: Wed, 23 Mar 2022 17:07:39 -0500	[thread overview]
Message-ID: <YjuaK09nOztYOmyn@robh.at.kernel.org> (raw)
In-Reply-To: <CACRpkdbz6Ua+0bTJMf-Qc7tju33CmEEgUsxH5KCS_zW3A_-SYA@mail.gmail.com>

On Wed, Mar 23, 2022 at 03:57:44PM +0100, Linus Walleij wrote:
> On Mon, Mar 21, 2022 at 8:10 PM Rob Herring <robh@kernel.org> wrote:
> 
> > > +properties:
> > > +  compatible:
> > > +    description: One compatible per product using this chip. Each product
> > > +      need deliberate custom values for things such as LRA resonance
> > > +      frequency and these are not stored in the device tree, rather we
> > > +      let the operating system look up the appropriate parameters from a
> > > +      table.
> > > +    enum:
> > > +      - immersion,isa1200-janice
> > > +      - immersion,isa1200-gavini
> >
> > Same device, different boards. I think I would put necessary properties
> > in the DT.
> 
> That will be all of these (from the driver):
> 
> +struct isa1200_config {
> +       u8 ldo_voltage;
> +       bool pwm_in;
> +       bool erm;
> +       u8 clkdiv;
> +       u8 plldiv;
> +       u8 freq;
> +       u8 duty;
> +       u8 period;
> +};

Could be all, but in your 2 cases some of these values are the same.

> 
> Example:
> 
> +/* Configuration for Janice, Samsung Galaxy S Advance GT-I9070 */
> +static const struct isa1200_config isa1200_janice = {
> +       .ldo_voltage = ISA1200_LDO_VOLTAGE_30V,
> +       .pwm_in = false,
> +       .clkdiv = ISA1200_HCTRL0_DIV_256,
> +       .plldiv = 2,
> +       .freq = 0,
> +       .duty = 0x3b,
> +       .period = 0x77,
> +};
> 
> This is derived from the compatible rather than individual properties
> or extra regulator and/or clock abstractions in line with:
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> 
> Which was originally looking like so:
> https://lore.kernel.org/dri-devel/20170813114448.20179-2-linus.walleij@linaro.org/
> 
> To which you replied:
> https://lore.kernel.org/dri-devel/20170817204424.e2wdkmyp4vyx2qj3@rob-hp-laptop/
> 
> "Normally, we the physical panel is described which would imply all these
> settings. Are there lots of panels with this controller that would
> justify all these settings?"

I reserve the right to contradict myself. :)

Seriously, it's always a judgement call.

> 
> In that case there was one (1)
> 
> In this case there are two (2) products that I know of. It does not have the
> relationship between panel and panel controller products though, but...
> it's not very different.
> 
> I don't think this chip was used a lot, I really tried to find other instances.
> But they could exist of course.

Okay, if you want to leave it like this, I'm fine with that.

Rob

  reply	other threads:[~2022-03-23 22:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-15 23:35 [PATCH 1/3] dt-bindings: Add Immersion Corporation prefix Linus Walleij
2022-03-15 23:35 ` [PATCH 2/3] dt-bindings: input: Add bindings for Immersion ISA1200 Linus Walleij
2022-03-21 19:10   ` Rob Herring
2022-03-23 14:57     ` Linus Walleij
2022-03-23 22:07       ` Rob Herring [this message]
2022-03-15 23:35 ` [PATCH 3/3] Input: isa1200 - new driver " Linus Walleij
2022-03-22  0:03   ` Jeff LaBundy
2022-03-21 19:01 ` [PATCH 1/3] dt-bindings: Add Immersion Corporation prefix Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YjuaK09nOztYOmyn@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-input@vger.kernel.org \
    --cc=phone-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.