All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Laxman Dewangan
	<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org"
	<sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	"rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org"
	<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	"pawel.moll-5wv7dgnIgG8@public.gmane.org"
	<pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	"mark.rutland-5wv7dgnIgG8@public.gmane.org"
	<mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org"
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	"rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org"
	<rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices
Date: Mon, 23 Sep 2013 14:46:05 -0600	[thread overview]
Message-ID: <5240A88D.8030309@wwwdotorg.org> (raw)
In-Reply-To: <20130919122240.GI22389@lee--X1>

On 09/19/2013 06:22 AM, Lee Jones wrote:
>>>>> Do the sub-nodes have their own properties? If so, it would be worth
>>>>> breaking them up as other OSes could reuse the specifics. If they do,
>>>>> then you need so put them in the binding. If they don't, then you do
>>>>> not require sub-nodes. The MFD core will ensure the sub-devices are
>>>>> probed and there is no requirement for the of_node to be assigned.
>>>> You do see some reusable IP blocks (like the regualtors on the wm831x
>>>> PMICs for example, they're repeated blocks) which can be reused but
>>>> generally they have a register base as part of the binding.  Personally
>>>> if it's just a property or two I'd probably just put them on the root
>>>> node for the device.
>>> Agreed. Besides, there doesn't seem to be *any* sub-device properties
>>> defined in the binding document. So what are you trying to achieve
>>> with the child nodes?
>>
>> I wanted to have the DT like:
>>
>> as3722 {
>>                 compatible = "ams,as3722";
>>                 reg = <0x40>;
>>
>>                 #interrupt-controller;
>>                 .....
>>
>>
>>                 regulators {
>>                             ldo1-in-supply = <..>;
>>                             ....
>>                             sd0 {
>>                                     regulator-name = "vdd-cpu";
>>                                     .....
>>                             };
>>                             sd1 {
>>                                     regulator-name = "vdd-ddr";
>>                                     .....
>>                             };
>>                             ....
>>             };
>> };
>>
>> And regulator driver should get the regulator node by their
>> pdev->dev.of_node.
>> Currently, in most of driver, we are having the code on regulator
>> driver to get "regulators" node from parent node which I want to
>> avoid.
> 
> Ah, I see. Yes, I believe the regulators should have their own node,

The use of a "regulators" node to keep all the regulator configuration
in one place seems fine...

> complete with a compatible string.

... but I see not reason why that node has to have a separate compatible
property, or /has/ to have a separate driver.

I think having a compatible value in this node would only be required if
the HW block that implements those registers is actually expected to be
shared between n different chips, and hence it's likely that you'd get
re-use out of a separate binding, driver, etc.

It's perfectly reasonable for the regulator MFD driver to know that the
binding for the top-level PMIC node has a regulators child node, and go
find it by name, and read whatever properties/nodes it needs directly
out of it. Writing code that way in no ways implies a need for a
compatible value.

> To have each regulator listed
> separately in the parent node seems a little messy. Just out of
> interest, how many regulators are we talking about here?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Lee Jones <lee.jones@linaro.org>
Cc: Laxman Dewangan <ldewangan@nvidia.com>,
	Mark Brown <broonie@kernel.org>,
	"sameo@linux.intel.com" <sameo@linux.intel.com>,
	"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
	"pawel.moll@arm.com" <pawel.moll@arm.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>,
	"rob@landley.net" <rob@landley.net>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices
Date: Mon, 23 Sep 2013 14:46:05 -0600	[thread overview]
Message-ID: <5240A88D.8030309@wwwdotorg.org> (raw)
In-Reply-To: <20130919122240.GI22389@lee--X1>

On 09/19/2013 06:22 AM, Lee Jones wrote:
>>>>> Do the sub-nodes have their own properties? If so, it would be worth
>>>>> breaking them up as other OSes could reuse the specifics. If they do,
>>>>> then you need so put them in the binding. If they don't, then you do
>>>>> not require sub-nodes. The MFD core will ensure the sub-devices are
>>>>> probed and there is no requirement for the of_node to be assigned.
>>>> You do see some reusable IP blocks (like the regualtors on the wm831x
>>>> PMICs for example, they're repeated blocks) which can be reused but
>>>> generally they have a register base as part of the binding.  Personally
>>>> if it's just a property or two I'd probably just put them on the root
>>>> node for the device.
>>> Agreed. Besides, there doesn't seem to be *any* sub-device properties
>>> defined in the binding document. So what are you trying to achieve
>>> with the child nodes?
>>
>> I wanted to have the DT like:
>>
>> as3722 {
>>                 compatible = "ams,as3722";
>>                 reg = <0x40>;
>>
>>                 #interrupt-controller;
>>                 .....
>>
>>
>>                 regulators {
>>                             ldo1-in-supply = <..>;
>>                             ....
>>                             sd0 {
>>                                     regulator-name = "vdd-cpu";
>>                                     .....
>>                             };
>>                             sd1 {
>>                                     regulator-name = "vdd-ddr";
>>                                     .....
>>                             };
>>                             ....
>>             };
>> };
>>
>> And regulator driver should get the regulator node by their
>> pdev->dev.of_node.
>> Currently, in most of driver, we are having the code on regulator
>> driver to get "regulators" node from parent node which I want to
>> avoid.
> 
> Ah, I see. Yes, I believe the regulators should have their own node,

The use of a "regulators" node to keep all the regulator configuration
in one place seems fine...

> complete with a compatible string.

... but I see not reason why that node has to have a separate compatible
property, or /has/ to have a separate driver.

I think having a compatible value in this node would only be required if
the HW block that implements those registers is actually expected to be
shared between n different chips, and hence it's likely that you'd get
re-use out of a separate binding, driver, etc.

It's perfectly reasonable for the regulator MFD driver to know that the
binding for the top-level PMIC node has a regulators child node, and go
find it by name, and read whatever properties/nodes it needs directly
out of it. Writing code that way in no ways implies a need for a
compatible value.

> To have each regulator listed
> separately in the parent node seems a little messy. Just out of
> interest, how many regulators are we talking about here?

  parent reply	other threads:[~2013-09-23 20:46 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-19  8:29 [PATCH] mfd: core: introduce of_node_name for mfd sub devices Laxman Dewangan
2013-09-19  8:29 ` Laxman Dewangan
2013-09-19  8:30 ` Lee Jones
2013-09-19  8:57   ` Laxman Dewangan
2013-09-19  8:57     ` Laxman Dewangan
2013-09-19 11:55   ` Mark Brown
     [not found]     ` <20130919115501.GM21013-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-09-19 12:00       ` Lee Jones
2013-09-19 12:00         ` Lee Jones
2013-09-19 12:28         ` Laxman Dewangan
2013-09-19 12:28           ` Laxman Dewangan
     [not found]           ` <523AEE07.9090405-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-09-19 12:22             ` Lee Jones
2013-09-19 12:22               ` Lee Jones
2013-09-19 12:54               ` Laxman Dewangan
2013-09-23 20:46               ` Stephen Warren [this message]
2013-09-23 20:46                 ` Stephen Warren
2013-09-24 13:55                 ` Lee Jones
     [not found] ` <1379579392-1794-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-09-23 20:50   ` Stephen Warren
2013-09-23 20:50     ` Stephen Warren

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=5240A88D.8030309@wwwdotorg.org \
    --to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org \
    --cc=rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org \
    --cc=sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.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 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.