devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Andrew F. Davis" <afd@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Lee Jones <lee.jones@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC
Date: Fri, 23 Oct 2015 19:11:56 -0500	[thread overview]
Message-ID: <562ACCCC.503@ti.com> (raw)
In-Reply-To: <20151023231822.GQ29919@sirena.org.uk>

On 10/23/2015 06:18 PM, Mark Brown wrote:
> On Fri, Oct 23, 2015 at 07:46:39AM -0500, Andrew F. Davis wrote:
>
>> I know just because other drivers do it doesn't mean it's a good idea,
>> but this is not new for MFDs and it is done in other regulators as well
>> (mt6397, tps659038, qcom,spmi, etc..).
>
> mt6397 doesn't do this, it doesn't have a compatible string at all (it's
> doing what I'm recommending that you do).  The SPMI devices are
> standalone devices, their parent device is actually functioning as a bus
> controller here (it's really a microcontroller inside the SoC).  The
> Palmas is part of how we realised this was a problem.
>

mt6397: Documentation/devicetree/bindings/mfd/mt6397.txt
Doing exactly what I'm doing,

pmic {
	compatible = "mediatek,mt6397";

	codec: mt6397codec {
		compatible = "mediatek,mt6397-codec";
	};

	regulators {
		compatible = "mediatek,mt6397-regulator";

		buck_vpca15 {
			....

The Palmas is a great example of why this is a good idea, there are
so many spins on this common base, and look how we can re-use sub-nodes:

tps659038: tps659038@58 {
	compatible = "ti,tps659038";
	reg = <0x58>;
	...

	tps659038_pmic {
		compatible = "ti,tps659038-pmic";
		...
	};

	tps659038_rtc: tps659038_rtc {
		compatible = "ti,palmas-rtc";
		...
	};

	tps659038_pwr_button: tps659038_pwr_button {
		compatible = "ti,palmas-pwrbutton";
		...
	};

	tps659038_gpio: tps659038_gpio {
		compatible = "ti,palmas-gpio";
		...
	};
};

(from am57xx-beagle-x15.dts)

looks like only the "ti,tps659038-pmic" node needed re-made without
re-making the whole driver.

>>> It seems like this is describing how Linux
>>> loads drivers not how the hardware is constructed but DT should describe
>>> the hardware.
>
>> While I agree to a point, if we follow this to its logical conclusion we
>> would end up with one compatible binding per SoC and be basically back to
>> board files. We need some granularity, just finding out where is the issue,
>
> The fact that the SoC DT is not distinct from the board DT is actually
> one of the problems with the way we're using DT at the minute, it means
> that DTBs are much less stable than they should be since we can enhance
> support for SoCs but DTBs need regenerating to take advantage of it.  It
> would be much better if the boards just referenced the SoC they use and
> pulled in a separate definition of the SoC (DT overlays will make it
> much more tractable to implement that if someone has time...).
>

I figured this can already be done by keeping the SoC stuff in dtsi files?
Anyway DT seems to have a lot of use issues with how it is being used, but
I'm probably not a person with enough free time for fixing that.. :|

>> I would say that as these devices belong to different subsystems and are
>> almost completely independent there should be no problem with having their
>> own compatible matched hardware sub-node.
>
> All it's adding is more typing for users.
>

Well I have to match the sub-devices on something, it's ether the node name
or the compatible string, so they might have to get used to typing :)

  reply	other threads:[~2015-10-24  0:11 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-01 20:37 [PATCH v4 0/5] mfd: tps65912: Driver rewrite with DT support Andrew F. Davis
2015-10-01 20:37 ` [PATCH v4 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC Andrew F. Davis
2015-10-01 20:37 ` [PATCH v4 2/5] mfd: tps65912: Remove old driver in preparation for new driver Andrew F. Davis
2015-10-05  9:28   ` Lee Jones
2015-10-05  9:29     ` Lee Jones
2015-10-05 16:01       ` Andrew F. Davis
2015-10-01 20:37 ` [PATCH v4 3/5] mfd: tps65912: Add driver for the TPS65912 PMIC Andrew F. Davis
     [not found]   ` <1443731874-21362-4-git-send-email-afd-l0cyMroinI0@public.gmane.org>
2015-10-01 20:51     ` kbuild test robot
2015-10-01 20:57     ` kbuild test robot
2015-10-01 20:57   ` kbuild test robot
2015-10-01 23:49   ` Andrew F. Davis
2015-10-05  9:24   ` Lee Jones
2015-10-05  9:27     ` Lee Jones
2015-10-12 15:06       ` Andrew F. Davis
     [not found]         ` <561BCC8A.3090402-l0cyMroinI0@public.gmane.org>
2015-10-13  7:34           ` Lee Jones
2015-10-01 20:37 ` [PATCH v4 4/5] regulator: tps65912: Add regulator " Andrew F. Davis
2015-10-02 19:21   ` Grygorii Strashko
2015-10-22 16:47   ` Mark Brown
     [not found]     ` <20151022164724.GZ8232-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-10-23 12:46       ` Andrew F. Davis
2015-10-23 23:18         ` Mark Brown
2015-10-24  0:11           ` Andrew F. Davis [this message]
     [not found]             ` <562ACCCC.503-l0cyMroinI0@public.gmane.org>
2015-10-24 22:14               ` Mark Brown
     [not found]                 ` <20151024221457.GS29919-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-10-25 20:45                   ` Andrew F. Davis
     [not found]                     ` <562D3F77.5040205-l0cyMroinI0@public.gmane.org>
2015-10-26  0:43                       ` Mark Brown
2015-10-26 15:47                         ` Andrew F. Davis
     [not found]                           ` <562E4B1D.4060205-l0cyMroinI0@public.gmane.org>
2015-10-27  0:16                             ` Mark Brown
     [not found]                               ` <20151027001608.GJ28319-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-10-27 14:23                                 ` Andrew F. Davis
2015-11-04 15:35     ` Andrew F. Davis
2015-11-05 10:14       ` Mark Brown
2015-11-05 18:04         ` Andrew F. Davis
2015-11-06 10:43           ` Mark Brown
2015-11-06 18:10             ` Andrew F. Davis
2015-11-06 21:16               ` Mark Brown
     [not found]                 ` <20151106211651.GJ18409-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-11-09 17:41                   ` Andrew F. Davis
2015-11-10  9:57                     ` Mark Brown
2015-11-10 16:47                       ` Andrew F. Davis
2015-11-10 17:04                         ` Mark Brown
2015-11-10 17:52                           ` Andrew F. Davis
2015-11-10 18:44                             ` Mark Brown
2015-11-10 19:40                               ` Andrew F. Davis
     [not found]                                 ` <56424836.7000608-l0cyMroinI0@public.gmane.org>
2015-11-16 18:23                                   ` Mark Brown
2015-10-01 20:37 ` [PATCH v4 5/5] gpio: tps65912: Add GPIO " Andrew F. Davis

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=562ACCCC.503@ti.com \
    --to=afd@ti.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=gnurou@gmail.com \
    --cc=grygorii.strashko@ti.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@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 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).