LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Carlos Iglesias <carlos.iglesias@emutex.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"knaack.h@gmx.de" <knaack.h@gmx.de>,
	"lars@metafoo.de" <lars@metafoo.de>,
	"pmeerw@pmeerw.net" <pmeerw@pmeerw.net>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Dan O'Donovan <dan.odonovan@emutex.com>
Subject: Re: [PATCH 1/2] dt-bindings: iio: pressure: Add support for Honeywell HSC SPI sensors
Date: Sat, 3 Nov 2018 10:36:19 +0000	[thread overview]
Message-ID: <20181103103619.7c4addd3@archlinux> (raw)
In-Reply-To: <0dbb2254-8035-b498-6302-26bc3a2219ca@emutex.com>

Rob, just to draw your attention, some discussion around
spi-max-frequency that you might want to look at below.


On Tue, 30 Oct 2018 15:56:58 +0000
Carlos Iglesias <carlos.iglesias@emutex.com> wrote:

> Please see comments inline below.
Hi Carlos,

Please try to use an email client (or configure yours to do it)
that adds indentation to each level of reply.  It's somewhat
fiddly to separate the different parts of the conversation!

> 
> On 10/28/18 5:38 PM, Jonathan Cameron wrote:
> 
> On Fri, 26 Oct 2018 18:14:36 +0000
> Carlos Iglesias <carlos.iglesias@emutex.com><mailto:carlos.iglesias@emutex.com> wrote:
> 
> 
> 
> Add device tree bindings for the HSC pressure sensors.
> 
> Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com><mailto:carlos.iglesias@emutex.com>
> ---
>  .../bindings/iio/pressure/hsc_spi.txt         | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> new file mode 100644
> index 000000000000..2302d6eef7c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> @@ -0,0 +1,85 @@
> +Honeywell HSC Series of Pressure Sensors
> +
> +Pressure sensors from Honeywell with analog, I2C and SPI interfaces
> +
> +Required properties:
> +- compatible: selects the sensor model; must be one of the following
> +       "honeywell,hsc001baa"
> +       "honeywell,hsc001bab"
> +       "honeywell,hsc001bac"
> +       "honeywell,hsc001baf"
> +       "honeywell,hsc1_6baa"
> +       "honeywell,hsc1_6bab"
> +       "honeywell,hsc1_6bac"
> +       "honeywell,hsc1_6baf"
> +       "honeywell,hsc2_5baa"
> +       "honeywell,hsc2_5bab"
> +       "honeywell,hsc2_5bac"
> +       "honeywell,hsc2_5baf"
> +       "honeywell,hsc004baa"
> +       "honeywell,hsc004bab"
> +       "honeywell,hsc004bac"
> +       "honeywell,hsc004baf"
> +       "honeywell,hsc006baa"
> +       "honeywell,hsc006bab"
> +       "honeywell,hsc006bac"
> +       "honeywell,hsc006baf"
> +       "honeywell,hsc010baa"
> +       "honeywell,hsc010bab"
> +       "honeywell,hsc010bac"
> +       "honeywell,hsc010baf"
> +       "honeywell,hsc100kaa"
> +       "honeywell,hsc100kab"
> +       "honeywell,hsc100kac"
> +       "honeywell,hsc100kaf"
> +       "honeywell,hsc160kaa"
> +       "honeywell,hsc160kab"
> +       "honeywell,hsc160kac"
> +       "honeywell,hsc160kaf"
> +       "honeywell,hsc250kaa"
> +       "honeywell,hsc250kab"
> +       "honeywell,hsc250kac"
> +       "honeywell,hsc250kaf"
> +       "honeywell,hsc400kaa"
> +       "honeywell,hsc400kab"
> +       "honeywell,hsc400kac"
> +       "honeywell,hsc400kaf"
> +       "honeywell,hsc600kaa"
> +       "honeywell,hsc600kab"
> +       "honeywell,hsc600kac"
> +       "honeywell,hsc600kaf"
> +       "honeywell,hsc001gaa"
> +       "honeywell,hsc001gab"
> +       "honeywell,hsc001gac"
> +       "honeywell,hsc001gaf"
> +       "honeywell,hsc015paa"
> +       "honeywell,hsc015pab"
> +       "honeywell,hsc015pac"
> +       "honeywell,hsc015paf"
> +       "honeywell,hsc030paa"
> +       "honeywell,hsc030pab"
> +       "honeywell,hsc030pac"
> +       "honeywell,hsc030paf"
> +       "honeywell,hsc060paa"
> +       "honeywell,hsc060pab"
> +       "honeywell,hsc060pac"
> +       "honeywell,hsc060paf"
> +       "honeywell,hsc100paa"
> +       "honeywell,hsc100pab"
> +       "honeywell,hsc100pac"
> +       "honeywell,hsc100paf"
> +       "honeywell,hsc150paa"
> +       "honeywell,hsc150pab"
> +       "honeywell,hsc150pac"
> +       "honeywell,hsc150paf"
> +- reg: the SPI chip select number used by the sensor.
> +- spi-max-frequency: maximum clock frequency (Hz) used for the SPI bus.
> +  The maximum value supported by the sensors is 400000.
> 
> 
> As Rob pointed out in a few reviews this week, the devicetree binding
> for this should only be applying a tighter bound than either the
> device or the bus master. So something introduced by the board
> layout for example, or a level convertor..
> 
> Jonathan
> 
> Checking the devicetree bindings files of other spi sensors I see a frequent approach is to include a reference to Documentation/devicetree/bindings/spi/spi-bus.txt, for example:
> 
> Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt apply. In particular, "reg" and "spi-max-frequency" properties must be given.
The point is that spi-max-frequency doesn't actually need to be given.  If the
fine with which ever is lower of that supported by the device and that supported
by the master then it will use that without spi-max-frequency being provided.

I agree this wasn't previously my understanding but it does make sense. No
obvious reason why this value would need providing by DT unless there is
a board issue that neither of the 'ends' can see.

We might want to look at cleaning this up in general through the bindings
(and making sure device drivers apply there own maximums).  Rob
is this a worthwhile cleanup?

> 
> Maybe it would be a better option to place such a note instead of the "reg" and "spi-max-frequency" points. What do you think?
> 
> Sorry, I was not aware of Rob Herring's previous comments on this topic, I am not subscribed to the LKML.

Sure, I wasn't aware of them until that day anyway ;)

Thanks,

Jonathan

> 
> 
> 
> 
> +
> +Example:
> +
> +       hsc_spi0: hsc@0 {
> +               compatible = "honeywell,hsc010baa";
> +               reg = <0>;
> +               spi-max-frequency = <400000>;
> +       };
> 
> 
> 
> 


  parent reply	other threads:[~2018-11-03 10:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-26 18:14 [PATCH 0/2] " Carlos Iglesias
2018-10-26 18:14 ` [PATCH 1/2] dt-bindings: " Carlos Iglesias
2018-10-28 17:38   ` Jonathan Cameron
     [not found]     ` <0dbb2254-8035-b498-6302-26bc3a2219ca@emutex.com>
2018-11-03 10:36       ` Jonathan Cameron [this message]
2018-11-06 21:29   ` Rob Herring
2018-10-26 18:14 ` [PATCH 2/2] " Carlos Iglesias
2018-10-28 18:44   ` Jonathan Cameron

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=20181103103619.7c4addd3@archlinux \
    --to=jic23@kernel.org \
    --cc=carlos.iglesias@emutex.com \
    --cc=dan.odonovan@emutex.com \
    --cc=devicetree@vger.kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pmeerw@pmeerw.net \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH 1/2] dt-bindings: iio: pressure: Add support for Honeywell HSC SPI sensors' \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).