From: Rob Herring <robh@kernel.org>
To: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Cc: Alan Cox <alan@linux.intel.com>, Andrew Jeffery <andrew@aj.id.au>,
Andrew Lunn <andrew@lunn.ch>,
Andy Shevchenko <andriy.shevchenko@intel.com>,
Arnd Bergmann <arnd@arndb.de>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Fengguang Wu <fengguang.wu@intel.com>,
Greg KH <gregkh@linuxfoundation.org>,
Guenter Roeck <linux@roeck-us.net>,
Haiyue Wang <haiyue.wang@linux.intel.com>,
James Feist <james.feist@linux.intel.com>,
Jason M Biils <jason.m.bills@linux.intel.com>,
Jean Delvare <jdelvare@suse.com>, Joel Stanley <joel@jms.id.au>,
Julia Cartwright <juliac@eso.teric.us>,
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
Milton Miller II <miltonm@us.ibm.com>,
Pavel Machek <pavel@ucw.cz>, Randy Dunlap <rdunlap@infradead.org>,
Stef van Os <stef.van.os@prodrive-technologies.com>,
Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com>,
Vernon Mauery <vernon.mauery@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
linux-doc@vger.kernel.org, devicetree@vger.kernel.org,
Linux HWMON List <linux-hwmon@vger.kernel.org>,
"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
<linux-arm-kernel@lists.infradead.org>,
OpenBMC Maillist <openbmc@lists.ozlabs.org>
Subject: Re: [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers
Date: Wed, 18 Apr 2018 16:28:41 -0500 [thread overview]
Message-ID: <CAL_JsqLpuNj4kQ8oXB0kxOsS7ww9Jk-oq4tJrroDKkLRTPrjSA@mail.gmail.com> (raw)
In-Reply-To: <1f2a86ff-b902-1d1d-488a-807ac1dd20cc@linux.intel.com>
On Wed, Apr 18, 2018 at 3:28 PM, Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
> On 4/18/2018 7:32 AM, Rob Herring wrote:
>>
>> On Tue, Apr 17, 2018 at 3:40 PM, Jae Hyun Yoo
>> <jae.hyun.yoo@linux.intel.com> wrote:
>>>
>>> On 4/16/2018 4:51 PM, Jae Hyun Yoo wrote:
>>>>
>>>>
>>>> On 4/16/2018 4:22 PM, Jae Hyun Yoo wrote:
>>>>>
>>>>>
>>>>> On 4/16/2018 11:14 AM, Rob Herring wrote:
>>>>>>
>>>>>>
>>>>>> On Tue, Apr 10, 2018 at 11:32:09AM -0700, Jae Hyun Yoo wrote:
>>>>>>>
>>>>>>>
>>>>>>> This commit adds dt-bindings documents for PECI cputemp and dimmtemp
>>>>>>> client
>>>>>>> drivers.
>>>>>>
>>>>>>
>>>>>>
>>>
>>> [...]
>>>
>>>>>>> +Example:
>>>>>>> + peci-bus@0 {
>>>>>>> + #address-cells = <1>;
>>>>>>> + #size-cells = <0>;
>>>>>>> + < more properties >
>>>>>>> +
>>>>>>> + peci-dimmtemp@cpu0 {
>>>>>>
>>>>>>
>>>>>>
>>>>>> unit-address is wrong.
>>>>>>
>>>>>
>>>>> Will fix it using the reg value.
>>>>>
>>>>>> It is a different bus from cputemp? Otherwise, you have conflicting
>>>>>> addresses. If that's the case, probably should make it clear by
>>>>>> showing
>>>>>> different host adapters for each example.
>>>>>>
>>>>>
>>>>> It could be the same bus with cputemp. Also, client address sharing is
>>>>> possible by PECI core if the functionality is different. I mean,
>>>>> cputemp and
>>>>> dimmtemp targeting the same client is possible case like this.
>>>>> peci-cputemp@30
>>>>> peci-dimmtemp@30
>>>>>
>>>>
>>>> Oh, I got your point. Probably, I should change these separate settings
>>>> into one like
>>>>
>>>> peci-client@30 {
>>>> compatible = "intel,peci-client";
>>>> reg = <0x30>;
>>>> };
>>>>
>>>> Then cputemp and dimmtemp drivers could refer the same compatible
>>>> string.
>>>> Will rewrite it.
>>>>
>>>
>>> I've checked it again and realized that it should use function based node
>>> name like:
>>>
>>> peci-cputemp@30
>>> peci-dimmtemp@30
>>>
>>> If it use the same string like 'peci-client@30', the drivers cannot be
>>> selectively enabled. The client address sharing way is well handled in
>>> PECI
>>> core and this way would be better for the future implementations of other
>>> PECI functional drivers such as crash dump driver and so on. So I'm going
>>> change the unit-address only.
>>
>>
>> 2 nodes at the same address is wrong (and soon dtc will warn you on
>> this). You have 2 potential options. The first is you need additional
>> address information in the DT if these are in fact 2 independent
>> devices. This could be something like a function number to use
>> something from PCI addressing. From what I found on PECI, it doesn't
>> seem to have anything like that. The 2nd option is you have a single
>> DT node which registers multiple hwmon devices. DT nodes and drivers
>> don't have to be 1-1. Don't design your DT nodes from how you want to
>> partition drivers in some OS.
>>
>> Rob
>>
>
> Please correct me if I'm wrong but I'm still thinking that it is
> possible. Also, I did compile it but dtc doesn't make a warning. Let me
> show an another use case which is similar to this case:
I did say *soon*. It's in dtc repo, but not the kernel copy yet.
> In arch/arm/boot/dts/aspeed-g5.dtsi
> [...]
> lpc_host: lpc-host@80 {
> compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon";
> reg = <0x80 0x1e0>;
> reg-io-width = <4>;
>
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x80 0x1e0>;
>
> lpc_ctrl: lpc-ctrl@0 {
> compatible = "aspeed,ast2500-lpc-ctrl";
> reg = <0x0 0x80>;
> clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
> status = "disabled";
> };
>
> lpc_snoop: lpc-snoop@0 {
> compatible = "aspeed,ast2500-lpc-snoop";
> reg = <0x0 0x80>;
> interrupts = <8>;
> status = "disabled";
> };
> }
> [...]
>
> This is device tree setting for LPC interface and its child nodes.
> LPC interface can be used as a multi-functional interface such as
> snoop 80, KCS, SIO and so on. In this use case, lpc-ctrl@0 and
> lpc-snoop@0 are sharing their address range from their individual
> driver modules and they can be registered quite well through both
> static dt or dynamic dtoverlay. PECI is also a multi-functional
> interface which is similar to the above case, I think.
This case too is poor design and should be fixed as well. Simply put,
you can have 2 devices on a bus at the same address without some sort
of mux or arbitration device in the middle. If you have a device/block
with multiple functions provided to the OS, then it is the OS's
problem to arbitrate access. It is not a DT problem because OS's can
vary in how they handle that both from OS to OS and over time.
Rob
next prev parent reply other threads:[~2018-04-18 21:28 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-10 18:32 [PATCH v3 00/10] PECI device driver introduction Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 01/10] Documentations: dt-bindings: Add documents of generic PECI bus, adapter and client drivers Jae Hyun Yoo
2018-04-11 11:52 ` Joel Stanley
2018-04-12 2:06 ` Jae Hyun Yoo
2018-04-16 17:59 ` Rob Herring
2018-04-16 23:06 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 02/10] Documentations: ioctl: Add ioctl numbers for PECI subsystem Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core Jae Hyun Yoo
2018-04-19 18:59 ` kbuild test robot
2018-04-23 10:52 ` Greg KH
2018-04-23 17:40 ` Jae Hyun Yoo
2018-04-24 16:01 ` Andy Shevchenko
2018-04-24 16:29 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs Jae Hyun Yoo
2018-04-11 11:52 ` Joel Stanley
2018-04-12 2:11 ` Jae Hyun Yoo
2018-04-16 18:10 ` Rob Herring
2018-04-16 23:12 ` Jae Hyun Yoo
2018-04-17 13:16 ` Rob Herring
2018-04-17 18:16 ` Jae Hyun Yoo
2018-04-17 22:06 ` Jae Hyun Yoo
2018-04-18 13:59 ` Rob Herring
2018-04-18 16:45 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 05/10] ARM: dts: aspeed: peci: Add PECI node Jae Hyun Yoo
2018-04-11 11:52 ` Joel Stanley
2018-04-12 2:20 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 06/10] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx Jae Hyun Yoo
2018-04-11 11:51 ` Joel Stanley
2018-04-12 2:03 ` Jae Hyun Yoo
2018-04-17 13:37 ` Robin Murphy
2018-04-17 18:21 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers Jae Hyun Yoo
2018-04-16 18:14 ` Rob Herring
2018-04-16 23:22 ` Jae Hyun Yoo
2018-04-16 23:51 ` Jae Hyun Yoo
2018-04-17 20:40 ` Jae Hyun Yoo
2018-04-18 14:32 ` Rob Herring
2018-04-18 20:28 ` Jae Hyun Yoo
2018-04-18 21:28 ` Rob Herring [this message]
2018-04-18 21:57 ` Jae Hyun Yoo
2018-04-19 19:48 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 08/10] Documentation: hwmon: " Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 09/10] drivers/hwmon: Add " Jae Hyun Yoo
2018-04-10 22:28 ` Guenter Roeck
2018-04-11 21:59 ` Jae Hyun Yoo
2018-04-12 0:34 ` Guenter Roeck
2018-04-12 2:51 ` Jae Hyun Yoo
2018-04-12 3:40 ` Guenter Roeck
2018-04-12 17:09 ` Jae Hyun Yoo
2018-04-12 17:37 ` Guenter Roeck
2018-04-12 19:51 ` Jae Hyun Yoo
2018-04-24 15:56 ` Andy Shevchenko
2018-04-24 16:26 ` Jae Hyun Yoo
2018-04-10 18:32 ` [PATCH v3 10/10] Add a maintainer for the PECI subsystem Jae Hyun Yoo
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=CAL_JsqLpuNj4kQ8oXB0kxOsS7ww9Jk-oq4tJrroDKkLRTPrjSA@mail.gmail.com \
--to=robh@kernel.org \
--cc=alan@linux.intel.com \
--cc=andrew@aj.id.au \
--cc=andrew@lunn.ch \
--cc=andriy.shevchenko@intel.com \
--cc=arnd@arndb.de \
--cc=benh@kernel.crashing.org \
--cc=devicetree@vger.kernel.org \
--cc=fengguang.wu@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=haiyue.wang@linux.intel.com \
--cc=jae.hyun.yoo@linux.intel.com \
--cc=james.feist@linux.intel.com \
--cc=jason.m.bills@linux.intel.com \
--cc=jdelvare@suse.com \
--cc=joel@jms.id.au \
--cc=juliac@eso.teric.us \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=miltonm@us.ibm.com \
--cc=openbmc@lists.ozlabs.org \
--cc=pavel@ucw.cz \
--cc=rdunlap@infradead.org \
--cc=stef.van.os@prodrive-technologies.com \
--cc=sumeet.r.pawnikar@intel.com \
--cc=vernon.mauery@linux.intel.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).