Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support

From	Rob Herring <robh@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support
Date	2017-06-15 23:40 +0200
Message-ID	<tSKEF-7OC-7@gated-at.bofh.it> (permalink)
References	(3 earlier) <tMHgt-2mn-7@gated-at.bofh.it> <tQ8Dw-6E7-23@gated-at.bofh.it> <tRH69-kl-1@gated-at.bofh.it> <tSFOG-4JC-21@gated-at.bofh.it> <tSHQv-5ZL-53@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu, Jun 15, 2017 at 1:33 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 15 Jun 09:26 PDT 2017, Sebastian Reichel wrote:
>
>> Hi,
>>
>> On Mon, Jun 12, 2017 at 04:32:03PM -0700, Bjorn Andersson wrote:
> [..]
>> > As such if we split the non-input related handling into another driver
>> > we would need to make the input driver create a subdevice during probe -
>> > or create a new pon-driver with a new compatible that internally spawns
>> > the pwrkey driver. Neither seems desirable to me...
>>
>> The pon-driver would have been the proper solution, but with the
>> binding already being defined that's no longer a nice option :(
>>
>
> We have a binding for the "qcom,pm8941-pwrkey", but as long as we
> maintain the compatibility in the input driver with this we could come
> up with a new binding for the "pon" block.

Yes. My only objection was having the pon node with child devices
purely because that is how Linux splits the driver.

>> > The features of the PON block not yet shown on LKML are status registers
>> > to indicate the reason for powering up the PMIC and a watchdog (which I
>> > don't believe is used or exposed today).
>>
>> So we have a block, which has watchdog, powerdown, reboot, boot-reason,
>> reboot-mode and power key. To me that does not look like it should be
>> one driver.
>>
>
> Unfortunately I do agree with this.

As do I. But that is a separate decision from DT bindings.

> It would make sense to describe the pon in a single DT-node and have a
> pon-driver spawning off individual driver for each functionality. That
> way we get a clean representation in DT and we get clean implementation
> of each component...

My objection here is we should not just create child nodes to align
with current Linux driver needs. That said, child nodes do sometimes
make sense. If, for example, the key function was not fixed and you
needed to configure what key function is used, then probably a child
node makes sense. It's always possible to add child nodes later
without breaking compatibility. It's hard to remove them.

Rob

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-06-08 18:40 +0200
  Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-06-13 01:40 +0200
    Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-06-15 18:30 +0200
      Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-06-15 20:40 +0200
        Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support Rob Herring <robh@kernel.org> - 2017-06-15 23:40 +0200

csiph-web