devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>,
	Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Shreeya Patel <shreeya.patel@collabora.com>,
	Zhigang Shi <Zhigang.Shi@liteon.com>,
	Paul Gazzillo <paul@pgazz.com>,
	Dmitry Osipenko <dmitry.osipenko@collabora.com>,
	Liam Beguin <liambeguin@gmail.com>, Peter Rosin <peda@axentia.se>,
	Randy Dunlap <rdunlap@infradead.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 1/6] dt-bindings: iio: light: Support ROHM BU27034
Date: Thu, 23 Feb 2023 10:26:02 +0100	[thread overview]
Message-ID: <33abc8a3-39f7-af7c-9676-723228624b0f@linaro.org> (raw)
In-Reply-To: <101db5e2-e878-b751-9679-6ea45eb24c26@fi.rohmeurope.com>

On 23/02/2023 07:20, Vaittinen, Matti wrote:
> Hi dee Ho Krzysztof,
> 
> Thanks for the review! It's nice you had the time to take a look on RFC :)
> 
> On 2/22/23 20:57, Krzysztof Kozlowski wrote:
>> On 22/02/2023 17:14, Matti Vaittinen wrote:
>>> ROHM BU27034 is an ambient light sesnor with 3 channels and 3 photo diodes
>>> capable of detecting a very wide range of illuminance. Typical application
>>> is adjusting LCD and backlight power of TVs and mobile phones.
>>>
>>> Add initial dt-bindings.
>>
>> Driver can be "initial", but bindings better to be closer to complete,
>> even if not used by the driver currently.
> 
> Out of the curiosity - why is that? (Please, don't take me wrong, I am 
> not trying to argue against this - just learn the reason behind). I 
> can't immediately see the harm caused by adding new properties later 
> when we learn more of hardware. (and no, I don't expect this simple IC 
> to gain at least many properties).

Linux drivers change, but the hardware does not, thus DTS, which
describes the hardware, can be complete. It should be written based on
the hardware, not based on Linux drivers. If you add incomplete
bindings, this suggests you wrote them to match your driver, not to
match hardware. This in turn (adjusting bindings to driver) makes them
less portable, narrowed to one specific driver implementation and more
ABI-break-prone later.

Imagine you that clock inputs, which you skipped in the binding, were
actually needed but on your board they were enabled by bootloader. The
binding is then used on other systems or by out of tree users. On your
new system the clocks are not enabled by bootloader anymore, thus you
add them to the binding. They are actually required for device to work,
so you make them required. But all these other users cannot be fixed...

What's more, incomplete binding/DTS is then used together with other
pieces - DTS and driver, e.g. via some graphs or other
phandles/supplies/pinctrl. So some other DTS or driver code might rely
on your particular binding. Imagine you had only vdd-supply regulator,
but no reset pins, so the only way to power-cycle device was to turn
off/on regulator supply. Then you figure out that you have reset pins
and it would be useful to add and use it. But already drivers are
written to power cycle via regulator... or even someone wrote new driver
regulator-pwrseq to power cycle your device due to missing reset GPIOs...


Best regards,
Krzysztof


  reply	other threads:[~2023-02-23  9:26 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-22 16:13 [RFC PATCH 0/6] Support ROHM BU27034 ALS sensor Matti Vaittinen
2023-02-22 16:14 ` [RFC PATCH 1/6] dt-bindings: iio: light: Support ROHM BU27034 Matti Vaittinen
2023-02-22 18:57   ` Krzysztof Kozlowski
2023-02-23  6:20     ` Vaittinen, Matti
2023-02-23  9:26       ` Krzysztof Kozlowski [this message]
2023-02-23 10:59         ` Matti Vaittinen
2023-02-26 13:39         ` Jonathan Cameron
2023-02-22 16:14 ` [RFC PATCH 2/6] iio: light: Add gain-time-scale helpers Matti Vaittinen
2023-02-26 16:21   ` Jonathan Cameron
2023-02-28  9:09     ` Vaittinen, Matti
2023-02-28 11:13     ` Vaittinen, Matti
2023-02-22 16:15 ` [RFC PATCH 3/6] iio: test: test " Matti Vaittinen
2023-02-22 16:15 ` [RFC PATCH 4/6] MAINTAINERS: Add IIO " Matti Vaittinen
2023-02-22 16:15 ` [RFC PATCH 5/6] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
2023-02-26 17:13   ` Jonathan Cameron
2023-03-02  7:35     ` Vaittinen, Matti
2023-03-04 19:11       ` Jonathan Cameron
2023-02-22 16:16 ` [RFC PATCH 6/6] MAINTAINERS: Add ROHM BU27034 Matti Vaittinen
2023-02-26 13:42   ` Jonathan Cameron
2023-02-27  5:51     ` Matti Vaittinen
2023-02-26 16:28 ` [RFC PATCH 0/6] Support ROHM BU27034 ALS sensor 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=33abc8a3-39f7-af7c-9676-723228624b0f@linaro.org \
    --to=krzysztof.kozlowski@linaro.org \
    --cc=Matti.Vaittinen@fi.rohmeurope.com \
    --cc=Zhigang.Shi@liteon.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.osipenko@collabora.com \
    --cc=jic23@kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=lars@metafoo.de \
    --cc=liambeguin@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=mazziesaccount@gmail.com \
    --cc=paul@pgazz.com \
    --cc=peda@axentia.se \
    --cc=rdunlap@infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=shreeya.patel@collabora.com \
    /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 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).