All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	Jonathan Cameron <jic23@kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
Date: Mon, 8 Feb 2021 11:20:03 +0000	[thread overview]
Message-ID: <20210208112003.000064eb@Huawei.com> (raw)
In-Reply-To: <8967afd32123482482af599b199e347b@hisilicon.com>

On Mon, 8 Feb 2021 08:12:21 +0000
"Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com> wrote:

> > -----Original Message-----
> > From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> > Sent: Monday, February 8, 2021 5:00 AM
> > To: Jonathan Cameron <jic23@kernel.org>; linux-iio@vger.kernel.org
> > Cc: Michael Hennerich <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> > <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> > <jonathan.cameron@huawei.com>; devicetree@vger.kernel.org
> > Subject: Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
> > 
> > On 2/7/21 4:46 PM, Jonathan Cameron wrote:  
> > > +required:
> > > +  - compatible
> > > +  - reg  
> > 
> > Is vdd-supply really optional the way it is implemented in the driver?
> >   
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        cdc@48 {
> > > +            compatible = "adi,ad7150";
> > > +            reg = <0x48>;
> > > +            interrupts = <25 2>, <26 2>;  
> 
> One question, here we have two interrupts, but the driver is reading
> one interrupt only, do we need to call
> of_irq_get(dev, index)
> or
> of_irq_get_byname()?

Driver reads both now (again).  It wasn't the cleanest transition in this series.
I dropped the second irq in patch 5 and brought it back in patch 12.

Whilst we could in theory support only one interrupt, that would make the driver
annoyingly fiddly for something I suspect no one would actually do.
We'd need to support events only on one of the two channels to make it work.

So I vote we take lazy option of saying it's either no interrupts, or both of
them for the two channel devices.  Nothing stops us relaxing that in future
and then using get_by_name.  If we do that and also require names are in order
INT1 INT2, INT1, INT2 but not INT2 INT1 and hence optional except in the case
of only one interrupt provided.

1) New DT, old driver - no interrupts but then there weren't any previously anyway.
2) New Driver old DT - fine, either both are specified or neither.

So there is a clean migration path if we ever find anyone who has wired up only
one of the interrupt lines and really does only want events on one or two channels
but still wants readings on both of them.

Jonathan


> 
> 
> > 
> > I wonder if we should use the symbolic constants for the IRQ type to
> > make the example more clear. E.g.
> > 
> > interrupts = <25 IRQ_TYPE_EDGE_FALLING>, ...
> >   
> > > +            interrupt-parent = <&gpio>;
> > > +        };
> > > +    };
> > > +...  
> >   
> Thanks
> Barry
> 


  reply	other threads:[~2021-02-08 11:24 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
2021-02-07 15:46 ` [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding Jonathan Cameron
2021-02-07 15:46 ` [PATCH 02/24] staging:iio:cdc:ad7150: Remove magnitude adaptive events Jonathan Cameron
2021-02-07 15:46 ` [PATCH 03/24] staging:iio:cdc:ad7150: Refactor event parameter update Jonathan Cameron
2021-02-07 15:46 ` [PATCH 04/24] staging:iio:cdc:ad7150: Timeout register covers both directions so both need updating Jonathan Cameron
2021-02-07 15:46 ` [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support Jonathan Cameron
2021-02-08  8:01   ` Song Bao Hua (Barry Song)
2021-02-08 11:12     ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 06/24] staging:iio:cdc:ad7150: Handle variation in chan_spec across device and irq present or not Jonathan Cameron
2021-02-07 15:46 ` [PATCH 07/24] staging:iio:cdc:ad7150: Simplify event handling by only using rising direction Jonathan Cameron
2021-02-07 15:46 ` [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe Jonathan Cameron
2021-02-08  8:02   ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 09/24] staging:iio:cdc:ad7150: Add sampling_frequency support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 10/24] iio:event: Add timeout event info type Jonathan Cameron
2021-02-07 15:46 ` [PATCH 11/24] staging:iio:cdc:ad7150: Change timeout units to seconds and use core support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 12/24] staging:iio:cdc:ad7150: Rework interrupt handling Jonathan Cameron
2021-02-07 15:46 ` [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming Jonathan Cameron
2021-03-14 17:43   ` Jonathan Cameron
2021-03-16 10:16     ` Alexandru Ardelean
2021-02-07 15:46 ` [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers Jonathan Cameron
2021-02-08  7:54   ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning Jonathan Cameron
2021-02-08  7:54   ` Song Bao Hua (Barry Song)
2021-03-14 17:46     ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments Jonathan Cameron
2021-02-08  7:52   ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 17/24] staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits Jonathan Cameron
2021-02-07 15:46 ` [PATCH 18/24] staging:iio:cdc:ad7150: Add scale and offset to info_mask_shared_by_type Jonathan Cameron
2021-02-07 15:46 ` [PATCH 19/24] staging:iio:cdc:ad7150: Really basic regulator support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
2021-02-08  7:11   ` Alexandru Ardelean
2021-02-11 15:51     ` Jonathan Cameron
2021-02-08  7:50   ` Song Bao Hua (Barry Song)
2021-02-08  8:01     ` Alexandru Ardelean
2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
2021-02-07 16:00   ` Lars-Peter Clausen
2021-02-07 16:18     ` Jonathan Cameron
2021-02-08  8:12     ` Song Bao Hua (Barry Song)
2021-02-08 11:20       ` Jonathan Cameron [this message]
2021-02-21 15:59   ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 22/24] iio:Documentation:ABI Add missing elements as used by the adi,ad7150 Jonathan Cameron
2021-02-07 15:46 ` [PATCH 23/24] staging:iio:cdc:ad7150: Add copyright notice given substantial changes Jonathan Cameron
2021-02-07 15:46 ` [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging Jonathan Cameron
2021-02-07 16:21   ` Jonathan Cameron
2021-02-07 16:12 ` [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
2021-02-08  7:20   ` Alexandru Ardelean

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=20210208112003.000064eb@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=song.bao.hua@hisilicon.com \
    --subject='Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc' \
    /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

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.