* [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers @ 2018-05-21 19:59 Jae Hyun Yoo 2018-05-22 16:42 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Jae Hyun Yoo @ 2018-05-21 19:59 UTC (permalink / raw) To: Rob Herring, Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel Cc: Jae Hyun Yoo, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley This commit adds dt-bindings documents for PECI hwmon client drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> Reviewed-by: James Feist <james.feist@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> Cc: Andrew Jeffery <andrew@aj.id.au> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Jason M Biils <jason.m.bills@linux.intel.com> Cc: Joel Stanley <joel@jms.id.au> --- .../bindings/hwmon/peci-cputemp.txt | 23 ++++++++++++++++++ .../bindings/hwmon/peci-dimmtemp.txt | 24 +++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-cputemp.txt create mode 100644 Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt new file mode 100644 index 000000000000..2f59aee12d9e --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt @@ -0,0 +1,23 @@ +Bindings for Intel PECI (Platform Environment Control Interface) cputemp driver. + +Required properties: +- compatible : Should be "intel,peci-cputemp". +- reg : Should contain address of a client CPU. Address range of CPU + clients is starting from 0x30 based on PECI specification. + +Example: + peci-bus@0 { + #address-cells = <1>; + #size-cells = <0>; + < more properties > + + peci-cputemp@30 { + compatible = "intel,peci-cputemp"; + reg = <0x30>; + }; + + peci-cputemp@31 { + compatible = "intel,peci-cputemp"; + reg = <0x31>; + }; + }; diff --git a/Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt b/Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt new file mode 100644 index 000000000000..4592ce23f620 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt @@ -0,0 +1,24 @@ +Bindings for Intel PECI (Platform Environment Control Interface) dimmtemp +driver. + +Required properties: +- compatible : Should be "intel,peci-dimmtemp". +- reg : Should contain address of a client CPU. Address range of CPU + clients is starting from 0x30 based on PECI specification. + +Example: + peci-bus@0 { + #address-cells = <1>; + #size-cells = <0>; + < more properties > + + peci-dimmtemp@30 { + compatible = "intel,peci-dimmtemp"; + reg = <0x30>; + }; + + peci-dimmtemp@31 { + compatible = "intel,peci-dimmtemp"; + reg = <0x31>; + }; + }; -- 2.17.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-21 19:59 [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers Jae Hyun Yoo @ 2018-05-22 16:42 ` Rob Herring 2018-05-22 17:18 ` Jae Hyun Yoo 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-05-22 16:42 UTC (permalink / raw) To: Jae Hyun Yoo Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: > This commit adds dt-bindings documents for PECI hwmon client drivers. > > Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> > Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> > Reviewed-by: James Feist <james.feist@linux.intel.com> > Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> > Cc: Andrew Jeffery <andrew@aj.id.au> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Jason M Biils <jason.m.bills@linux.intel.com> > Cc: Joel Stanley <joel@jms.id.au> > --- > .../bindings/hwmon/peci-cputemp.txt | 23 ++++++++++++++++++ > .../bindings/hwmon/peci-dimmtemp.txt | 24 +++++++++++++++++++ > 2 files changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/peci-cputemp.txt > create mode 100644 Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt > > diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt > new file mode 100644 > index 000000000000..2f59aee12d9e > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt > @@ -0,0 +1,23 @@ > +Bindings for Intel PECI (Platform Environment Control Interface) cputemp driver. > + > +Required properties: > +- compatible : Should be "intel,peci-cputemp". > +- reg : Should contain address of a client CPU. Address range of CPU > + clients is starting from 0x30 based on PECI specification. > + > +Example: > + peci-bus@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + < more properties > > + > + peci-cputemp@30 { > + compatible = "intel,peci-cputemp"; > + reg = <0x30>; > + }; [...] > + peci-dimmtemp@30 { > + compatible = "intel,peci-dimmtemp"; > + reg = <0x30>; > + }; As I said in the prior version, 2 nodes at the same address is wrong. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-22 16:42 ` Rob Herring @ 2018-05-22 17:18 ` Jae Hyun Yoo 2018-05-23 15:11 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Jae Hyun Yoo @ 2018-05-22 17:18 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On 5/22/2018 9:42 AM, Rob Herring wrote: > On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >> This commit adds dt-bindings documents for PECI hwmon client drivers. >> >> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >> Reviewed-by: James Feist <james.feist@linux.intel.com> >> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >> Cc: Andrew Jeffery <andrew@aj.id.au> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >> Cc: Joel Stanley <joel@jms.id.au> >> --- >> .../bindings/hwmon/peci-cputemp.txt | 23 ++++++++++++++++++ >> .../bindings/hwmon/peci-dimmtemp.txt | 24 +++++++++++++++++++ >> 2 files changed, 47 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >> create mode 100644 Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >> >> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >> new file mode 100644 >> index 000000000000..2f59aee12d9e >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >> @@ -0,0 +1,23 @@ >> +Bindings for Intel PECI (Platform Environment Control Interface) cputemp driver. >> + >> +Required properties: >> +- compatible : Should be "intel,peci-cputemp". >> +- reg : Should contain address of a client CPU. Address range of CPU >> + clients is starting from 0x30 based on PECI specification. >> + >> +Example: >> + peci-bus@0 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + < more properties > >> + >> + peci-cputemp@30 { >> + compatible = "intel,peci-cputemp"; >> + reg = <0x30>; >> + }; > > [...] > >> + peci-dimmtemp@30 { >> + compatible = "intel,peci-dimmtemp"; >> + reg = <0x30>; >> + }; > > As I said in the prior version, 2 nodes at the same address is wrong. > > Rob > In PECI bus, there is one and only bus host (adapter) and multiple clients on a PECI bus, and PECI spec doesn't allow multiple originators so only the host device can originate message. In this implementation, all message transactions on a bus from client driver modules and user space will be serialized well in the PECI core bus driver so bus occupation and traffic arbitration will be managed well in the PECI core bus driver even in case of a bus has 2 client drivers at the same address. I'm sure that this implementation doesn't make that kind of problem to OS. Thanks, -Jae ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-22 17:18 ` Jae Hyun Yoo @ 2018-05-23 15:11 ` Rob Herring 2018-05-23 16:37 ` Jae Hyun Yoo 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-05-23 15:11 UTC (permalink / raw) To: Jae Hyun Yoo Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote: > On 5/22/2018 9:42 AM, Rob Herring wrote: >> >> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>> >>> This commit adds dt-bindings documents for PECI hwmon client drivers. >>> >>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>> Cc: Andrew Jeffery <andrew@aj.id.au> >>> Cc: Arnd Bergmann <arnd@arndb.de> >>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>> Cc: Joel Stanley <joel@jms.id.au> >>> --- >>> .../bindings/hwmon/peci-cputemp.txt | 23 ++++++++++++++++++ >>> .../bindings/hwmon/peci-dimmtemp.txt | 24 +++++++++++++++++++ >>> 2 files changed, 47 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>> create mode 100644 >>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>> new file mode 100644 >>> index 000000000000..2f59aee12d9e >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>> @@ -0,0 +1,23 @@ >>> +Bindings for Intel PECI (Platform Environment Control Interface) cputemp >>> driver. >>> + >>> +Required properties: >>> +- compatible : Should be "intel,peci-cputemp". >>> +- reg : Should contain address of a client CPU. Address range of >>> CPU >>> + clients is starting from 0x30 based on PECI specification. >>> + >>> +Example: >>> + peci-bus@0 { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + < more properties > >>> + >>> + peci-cputemp@30 { >>> + compatible = "intel,peci-cputemp"; >>> + reg = <0x30>; >>> + }; >> >> >> [...] >> >>> + peci-dimmtemp@30 { >>> + compatible = "intel,peci-dimmtemp"; >>> + reg = <0x30>; >>> + }; >> >> >> As I said in the prior version, 2 nodes at the same address is wrong. >> >> Rob >> > > In PECI bus, there is one and only bus host (adapter) and multiple > clients on a PECI bus, and PECI spec doesn't allow multiple originators > so only the host device can originate message. Yes, I get that. A single host still has to address slave devices. > In this implementation, > all message transactions on a bus from client driver modules and user > space will be serialized well in the PECI core bus driver so bus > occupation and traffic arbitration will be managed well in the PECI core > bus driver even in case of a bus has 2 client drivers at the same > address. I'm sure that this implementation doesn't make that kind of > problem to OS. Multiple clients to a single device is common, but that is a software problem and doesn't belong in DT. I don't think there is a single other case in the kernel where multiple drivers can bind to the same device at a given bus address. That is why we have things like MFD. Though in this case, why can't one hwmon driver register multiple hwmon devices (cpu and dimm temps)? Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-23 15:11 ` Rob Herring @ 2018-05-23 16:37 ` Jae Hyun Yoo 2018-05-23 19:33 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Jae Hyun Yoo @ 2018-05-23 16:37 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On 5/23/2018 8:11 AM, Rob Herring wrote: > On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo > <jae.hyun.yoo@linux.intel.com> wrote: >> On 5/22/2018 9:42 AM, Rob Herring wrote: >>> >>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>>> >>>> This commit adds dt-bindings documents for PECI hwmon client drivers. >>>> >>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>>> Cc: Andrew Jeffery <andrew@aj.id.au> >>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>>> Cc: Joel Stanley <joel@jms.id.au> >>>> --- >>>> .../bindings/hwmon/peci-cputemp.txt | 23 ++++++++++++++++++ >>>> .../bindings/hwmon/peci-dimmtemp.txt | 24 +++++++++++++++++++ >>>> 2 files changed, 47 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>> create mode 100644 >>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>> new file mode 100644 >>>> index 000000000000..2f59aee12d9e >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>> @@ -0,0 +1,23 @@ >>>> +Bindings for Intel PECI (Platform Environment Control Interface) cputemp >>>> driver. >>>> + >>>> +Required properties: >>>> +- compatible : Should be "intel,peci-cputemp". >>>> +- reg : Should contain address of a client CPU. Address range of >>>> CPU >>>> + clients is starting from 0x30 based on PECI specification. >>>> + >>>> +Example: >>>> + peci-bus@0 { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + < more properties > >>>> + >>>> + peci-cputemp@30 { >>>> + compatible = "intel,peci-cputemp"; >>>> + reg = <0x30>; >>>> + }; >>> >>> >>> [...] >>> >>>> + peci-dimmtemp@30 { >>>> + compatible = "intel,peci-dimmtemp"; >>>> + reg = <0x30>; >>>> + }; >>> >>> >>> As I said in the prior version, 2 nodes at the same address is wrong. >>> >>> Rob >>> >> >> In PECI bus, there is one and only bus host (adapter) and multiple >> clients on a PECI bus, and PECI spec doesn't allow multiple originators >> so only the host device can originate message. > > Yes, I get that. A single host still has to address slave devices. > >> In this implementation, >> all message transactions on a bus from client driver modules and user >> space will be serialized well in the PECI core bus driver so bus >> occupation and traffic arbitration will be managed well in the PECI core >> bus driver even in case of a bus has 2 client drivers at the same >> address. I'm sure that this implementation doesn't make that kind of >> problem to OS. > > Multiple clients to a single device is common, but that is a software > problem and doesn't belong in DT. > > I don't think there is a single other case in the kernel where > multiple drivers can bind to the same device at a given bus address. > That is why we have things like MFD. Though in this case, why can't > one hwmon driver register multiple hwmon devices (cpu and dimm temps)? > It was implemented as a single driver until v2 but dimm temps need delayed creation unlikely the cpu temps on hwmon subsystem because of memory training behavior of remote x86 cpus. Since hwmon doesn't allow incremental creation, I had to divide it into two, cputemp and dimmtemp, so that cputemp can be registered immediately when the remote x86 cpu turns on and dimmtemp can be registered by delayed creation. It is the reason why I had to make the two hwmon driver modules that sharing a single device address. Additionally, PECI isn't limited for temperature monitoring feature but it can be used for other functions such as platform management, cpu interface tuning and diagnostics and failure analysis, so in case of adding a new driver for the functions, we should add an another DT node which is sharing the same cpu address. Thanks, -Jae ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-23 16:37 ` Jae Hyun Yoo @ 2018-05-23 19:33 ` Rob Herring 2018-05-23 20:03 ` Jae Hyun Yoo 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-05-23 19:33 UTC (permalink / raw) To: Jae Hyun Yoo Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote: > On 5/23/2018 8:11 AM, Rob Herring wrote: >> >> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo >> <jae.hyun.yoo@linux.intel.com> wrote: >>> >>> On 5/22/2018 9:42 AM, Rob Herring wrote: >>>> >>>> >>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>>>> >>>>> >>>>> This commit adds dt-bindings documents for PECI hwmon client drivers. >>>>> >>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>>>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>>>> Cc: Andrew Jeffery <andrew@aj.id.au> >>>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>>>> Cc: Joel Stanley <joel@jms.id.au> >>>>> --- >>>>> .../bindings/hwmon/peci-cputemp.txt | 23 >>>>> ++++++++++++++++++ >>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24 >>>>> +++++++++++++++++++ >>>>> 2 files changed, 47 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>> new file mode 100644 >>>>> index 000000000000..2f59aee12d9e >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>> @@ -0,0 +1,23 @@ >>>>> +Bindings for Intel PECI (Platform Environment Control Interface) >>>>> cputemp >>>>> driver. >>>>> + >>>>> +Required properties: >>>>> +- compatible : Should be "intel,peci-cputemp". >>>>> +- reg : Should contain address of a client CPU. Address range >>>>> of >>>>> CPU >>>>> + clients is starting from 0x30 based on PECI >>>>> specification. >>>>> + >>>>> +Example: >>>>> + peci-bus@0 { >>>>> + #address-cells = <1>; >>>>> + #size-cells = <0>; >>>>> + < more properties > >>>>> + >>>>> + peci-cputemp@30 { >>>>> + compatible = "intel,peci-cputemp"; >>>>> + reg = <0x30>; >>>>> + }; >>>> >>>> >>>> >>>> [...] >>>> >>>>> + peci-dimmtemp@30 { >>>>> + compatible = "intel,peci-dimmtemp"; >>>>> + reg = <0x30>; >>>>> + }; >>>> >>>> >>>> >>>> As I said in the prior version, 2 nodes at the same address is wrong. >>>> >>>> Rob >>>> >>> >>> In PECI bus, there is one and only bus host (adapter) and multiple >>> clients on a PECI bus, and PECI spec doesn't allow multiple originators >>> so only the host device can originate message. >> >> >> Yes, I get that. A single host still has to address slave devices. >> >>> In this implementation, >>> all message transactions on a bus from client driver modules and user >>> space will be serialized well in the PECI core bus driver so bus >>> occupation and traffic arbitration will be managed well in the PECI core >>> bus driver even in case of a bus has 2 client drivers at the same >>> address. I'm sure that this implementation doesn't make that kind of >>> problem to OS. >> >> >> Multiple clients to a single device is common, but that is a software >> problem and doesn't belong in DT. >> >> I don't think there is a single other case in the kernel where >> multiple drivers can bind to the same device at a given bus address. >> That is why we have things like MFD. Though in this case, why can't >> one hwmon driver register multiple hwmon devices (cpu and dimm temps)? >> > > It was implemented as a single driver until v2 but dimm temps need > delayed creation unlikely the cpu temps on hwmon subsystem because of > memory training behavior of remote x86 cpus. Since hwmon doesn't allow > incremental creation, I had to divide it into two, cputemp and dimmtemp, > so that cputemp can be registered immediately when the remote x86 cpu > turns on and dimmtemp can be registered by delayed creation. It is the > reason why I had to make the two hwmon driver modules that sharing a > single device address. That all sounds like kernel problems to me. Stop designing your DT binding around what the kernel can or can't *currently* support. > Additionally, PECI isn't limited for temperature > monitoring feature but it can be used for other functions such as > platform management, cpu interface tuning and diagnostics and failure > analysis, so in case of adding a new driver for the functions, we should > add an another DT node which is sharing the same cpu address. No, the driver should add support for those additional functions. Perhaps you will need to use MFD. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-23 19:33 ` Rob Herring @ 2018-05-23 20:03 ` Jae Hyun Yoo 2018-05-23 21:56 ` Jae Hyun Yoo 0 siblings, 1 reply; 10+ messages in thread From: Jae Hyun Yoo @ 2018-05-23 20:03 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On 5/23/2018 12:33 PM, Rob Herring wrote: > On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo > <jae.hyun.yoo@linux.intel.com> wrote: >> On 5/23/2018 8:11 AM, Rob Herring wrote: >>> >>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo >>> <jae.hyun.yoo@linux.intel.com> wrote: >>>> >>>> On 5/22/2018 9:42 AM, Rob Herring wrote: >>>>> >>>>> >>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>>>>> >>>>>> >>>>>> This commit adds dt-bindings documents for PECI hwmon client drivers. >>>>>> >>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>>>>> Cc: Andrew Jeffery <andrew@aj.id.au> >>>>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>>>>> Cc: Joel Stanley <joel@jms.id.au> >>>>>> --- >>>>>> .../bindings/hwmon/peci-cputemp.txt | 23 >>>>>> ++++++++++++++++++ >>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24 >>>>>> +++++++++++++++++++ >>>>>> 2 files changed, 47 insertions(+) >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>> new file mode 100644 >>>>>> index 000000000000..2f59aee12d9e >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>> @@ -0,0 +1,23 @@ >>>>>> +Bindings for Intel PECI (Platform Environment Control Interface) >>>>>> cputemp >>>>>> driver. >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : Should be "intel,peci-cputemp". >>>>>> +- reg : Should contain address of a client CPU. Address range >>>>>> of >>>>>> CPU >>>>>> + clients is starting from 0x30 based on PECI >>>>>> specification. >>>>>> + >>>>>> +Example: >>>>>> + peci-bus@0 { >>>>>> + #address-cells = <1>; >>>>>> + #size-cells = <0>; >>>>>> + < more properties > >>>>>> + >>>>>> + peci-cputemp@30 { >>>>>> + compatible = "intel,peci-cputemp"; >>>>>> + reg = <0x30>; >>>>>> + }; >>>>> >>>>> >>>>> >>>>> [...] >>>>> >>>>>> + peci-dimmtemp@30 { >>>>>> + compatible = "intel,peci-dimmtemp"; >>>>>> + reg = <0x30>; >>>>>> + }; >>>>> >>>>> >>>>> >>>>> As I said in the prior version, 2 nodes at the same address is wrong. >>>>> >>>>> Rob >>>>> >>>> >>>> In PECI bus, there is one and only bus host (adapter) and multiple >>>> clients on a PECI bus, and PECI spec doesn't allow multiple originators >>>> so only the host device can originate message. >>> >>> >>> Yes, I get that. A single host still has to address slave devices. >>> >>>> In this implementation, >>>> all message transactions on a bus from client driver modules and user >>>> space will be serialized well in the PECI core bus driver so bus >>>> occupation and traffic arbitration will be managed well in the PECI core >>>> bus driver even in case of a bus has 2 client drivers at the same >>>> address. I'm sure that this implementation doesn't make that kind of >>>> problem to OS. >>> >>> >>> Multiple clients to a single device is common, but that is a software >>> problem and doesn't belong in DT. >>> >>> I don't think there is a single other case in the kernel where >>> multiple drivers can bind to the same device at a given bus address. >>> That is why we have things like MFD. Though in this case, why can't >>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)? >>> >> >> It was implemented as a single driver until v2 but dimm temps need >> delayed creation unlikely the cpu temps on hwmon subsystem because of >> memory training behavior of remote x86 cpus. Since hwmon doesn't allow >> incremental creation, I had to divide it into two, cputemp and dimmtemp, >> so that cputemp can be registered immediately when the remote x86 cpu >> turns on and dimmtemp can be registered by delayed creation. It is the >> reason why I had to make the two hwmon driver modules that sharing a >> single device address. > > That all sounds like kernel problems to me. Stop designing your DT > binding around what the kernel can or can't *currently* support. > >> Additionally, PECI isn't limited for temperature >> monitoring feature but it can be used for other functions such as >> platform management, cpu interface tuning and diagnostics and failure >> analysis, so in case of adding a new driver for the functions, we should >> add an another DT node which is sharing the same cpu address. > > No, the driver should add support for those additional functions. > Perhaps you will need to use MFD. > Do you mean that the device address sharing is acceptable if I make these nodes under "simple-mfd"? Thanks, -Jae ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-23 20:03 ` Jae Hyun Yoo @ 2018-05-23 21:56 ` Jae Hyun Yoo 2018-05-24 13:47 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Jae Hyun Yoo @ 2018-05-23 21:56 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On 5/23/2018 1:03 PM, Jae Hyun Yoo wrote: > On 5/23/2018 12:33 PM, Rob Herring wrote: >> On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo >> <jae.hyun.yoo@linux.intel.com> wrote: >>> On 5/23/2018 8:11 AM, Rob Herring wrote: >>>> >>>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo >>>> <jae.hyun.yoo@linux.intel.com> wrote: >>>>> >>>>> On 5/22/2018 9:42 AM, Rob Herring wrote: >>>>>> >>>>>> >>>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>>>>>> >>>>>>> >>>>>>> This commit adds dt-bindings documents for PECI hwmon client >>>>>>> drivers. >>>>>>> >>>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>>>>>> Cc: Andrew Jeffery <andrew@aj.id.au> >>>>>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>>>>>> Cc: Joel Stanley <joel@jms.id.au> >>>>>>> --- >>>>>>> .../bindings/hwmon/peci-cputemp.txt | 23 >>>>>>> ++++++++++++++++++ >>>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24 >>>>>>> +++++++++++++++++++ >>>>>>> 2 files changed, 47 insertions(+) >>>>>>> create mode 100644 >>>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>> create mode 100644 >>>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>> new file mode 100644 >>>>>>> index 000000000000..2f59aee12d9e >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>> @@ -0,0 +1,23 @@ >>>>>>> +Bindings for Intel PECI (Platform Environment Control Interface) >>>>>>> cputemp >>>>>>> driver. >>>>>>> + >>>>>>> +Required properties: >>>>>>> +- compatible : Should be "intel,peci-cputemp". >>>>>>> +- reg : Should contain address of a client CPU. Address >>>>>>> range >>>>>>> of >>>>>>> CPU >>>>>>> + clients is starting from 0x30 based on PECI >>>>>>> specification. >>>>>>> + >>>>>>> +Example: >>>>>>> + peci-bus@0 { >>>>>>> + #address-cells = <1>; >>>>>>> + #size-cells = <0>; >>>>>>> + < more properties > >>>>>>> + >>>>>>> + peci-cputemp@30 { >>>>>>> + compatible = "intel,peci-cputemp"; >>>>>>> + reg = <0x30>; >>>>>>> + }; >>>>>> >>>>>> >>>>>> >>>>>> [...] >>>>>> >>>>>>> + peci-dimmtemp@30 { >>>>>>> + compatible = "intel,peci-dimmtemp"; >>>>>>> + reg = <0x30>; >>>>>>> + }; >>>>>> >>>>>> >>>>>> >>>>>> As I said in the prior version, 2 nodes at the same address is wrong. >>>>>> >>>>>> Rob >>>>>> >>>>> >>>>> In PECI bus, there is one and only bus host (adapter) and multiple >>>>> clients on a PECI bus, and PECI spec doesn't allow multiple >>>>> originators >>>>> so only the host device can originate message. >>>> >>>> >>>> Yes, I get that. A single host still has to address slave devices. >>>> >>>>> In this implementation, >>>>> all message transactions on a bus from client driver modules and user >>>>> space will be serialized well in the PECI core bus driver so bus >>>>> occupation and traffic arbitration will be managed well in the PECI >>>>> core >>>>> bus driver even in case of a bus has 2 client drivers at the same >>>>> address. I'm sure that this implementation doesn't make that kind of >>>>> problem to OS. >>>> >>>> >>>> Multiple clients to a single device is common, but that is a software >>>> problem and doesn't belong in DT. >>>> >>>> I don't think there is a single other case in the kernel where >>>> multiple drivers can bind to the same device at a given bus address. >>>> That is why we have things like MFD. Though in this case, why can't >>>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)? >>>> >>> >>> It was implemented as a single driver until v2 but dimm temps need >>> delayed creation unlikely the cpu temps on hwmon subsystem because of >>> memory training behavior of remote x86 cpus. Since hwmon doesn't allow >>> incremental creation, I had to divide it into two, cputemp and dimmtemp, >>> so that cputemp can be registered immediately when the remote x86 cpu >>> turns on and dimmtemp can be registered by delayed creation. It is the >>> reason why I had to make the two hwmon driver modules that sharing a >>> single device address. >> >> That all sounds like kernel problems to me. Stop designing your DT >> binding around what the kernel can or can't *currently* support. >> >>> Additionally, PECI isn't limited for temperature >>> monitoring feature but it can be used for other functions such as >>> platform management, cpu interface tuning and diagnostics and failure >>> analysis, so in case of adding a new driver for the functions, we should >>> add an another DT node which is sharing the same cpu address. >> >> No, the driver should add support for those additional functions. >> Perhaps you will need to use MFD. >> > > Do you mean that the device address sharing is acceptable if I make > these nodes under "simple-mfd"? > > Thanks, > > -Jae Hi Rob, I'm planning to change the whole PECI node like below: peci: peci@1e78b000 { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges = <0x0 0x1e78b000 0x60>; peci0: peci-bus@0 { compatible = "aspeed,ast2500-peci"; reg = <0x0 0x60>; #address-cells = <1>; #size-cells = <0>; interrupts = <15>; clocks = <&syscon ASPEED_CLK_GATE_REFCLK>; resets = <&syscon ASPEED_RESET_PECI>; clock-frequency = <24000000>; msg-timing = <1>; addr-timing = <1>; rd-sampling-point = <8>; cmd-timeout-ms = <1000>; status = "disabled"; peci-client@30 { compatible = "simple-mfd", "syscon"; reg = <0x30>; cputemp: cputemp { compatible = "intel,peci-cputemp"; }; dimmtemp: dimmtemp { compatible = "intel,peci-dimmtemp"; }; }; peci-client@31 { compatible = "simple-mfd", "syscon"; reg = <0x31>; cputemp: cputemp { compatible = "intel,peci-cputemp"; }; dimmtemp: dimmtemp { compatible = "intel,peci-dimmtemp"; }; }; }; }; Can you please check whether it is acceptable or not? Thanks, -Jae ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-23 21:56 ` Jae Hyun Yoo @ 2018-05-24 13:47 ` Rob Herring 2018-05-24 17:09 ` Jae Hyun Yoo 0 siblings, 1 reply; 10+ messages in thread From: Rob Herring @ 2018-05-24 13:47 UTC (permalink / raw) To: Jae Hyun Yoo Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On Wed, May 23, 2018 at 4:56 PM, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote: > On 5/23/2018 1:03 PM, Jae Hyun Yoo wrote: >> >> On 5/23/2018 12:33 PM, Rob Herring wrote: >>> >>> On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo >>> <jae.hyun.yoo@linux.intel.com> wrote: >>>> >>>> On 5/23/2018 8:11 AM, Rob Herring wrote: >>>>> >>>>> >>>>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo >>>>> <jae.hyun.yoo@linux.intel.com> wrote: >>>>>> >>>>>> >>>>>> On 5/22/2018 9:42 AM, Rob Herring wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> This commit adds dt-bindings documents for PECI hwmon client >>>>>>>> drivers. >>>>>>>> >>>>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>>>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>>>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>>>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>>>>>>> Cc: Andrew Jeffery <andrew@aj.id.au> >>>>>>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>>>>>>> Cc: Joel Stanley <joel@jms.id.au> >>>>>>>> --- >>>>>>>> .../bindings/hwmon/peci-cputemp.txt | 23 >>>>>>>> ++++++++++++++++++ >>>>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24 >>>>>>>> +++++++++++++++++++ >>>>>>>> 2 files changed, 47 insertions(+) >>>>>>>> create mode 100644 >>>>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>> create mode 100644 >>>>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>>>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>> new file mode 100644 >>>>>>>> index 000000000000..2f59aee12d9e >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>> @@ -0,0 +1,23 @@ >>>>>>>> +Bindings for Intel PECI (Platform Environment Control Interface) >>>>>>>> cputemp >>>>>>>> driver. >>>>>>>> + >>>>>>>> +Required properties: >>>>>>>> +- compatible : Should be "intel,peci-cputemp". >>>>>>>> +- reg : Should contain address of a client CPU. Address >>>>>>>> range >>>>>>>> of >>>>>>>> CPU >>>>>>>> + clients is starting from 0x30 based on PECI >>>>>>>> specification. >>>>>>>> + >>>>>>>> +Example: >>>>>>>> + peci-bus@0 { >>>>>>>> + #address-cells = <1>; >>>>>>>> + #size-cells = <0>; >>>>>>>> + < more properties > >>>>>>>> + >>>>>>>> + peci-cputemp@30 { >>>>>>>> + compatible = "intel,peci-cputemp"; >>>>>>>> + reg = <0x30>; >>>>>>>> + }; >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> [...] >>>>>>> >>>>>>>> + peci-dimmtemp@30 { >>>>>>>> + compatible = "intel,peci-dimmtemp"; >>>>>>>> + reg = <0x30>; >>>>>>>> + }; >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> As I said in the prior version, 2 nodes at the same address is wrong. >>>>>>> >>>>>>> Rob >>>>>>> >>>>>> >>>>>> In PECI bus, there is one and only bus host (adapter) and multiple >>>>>> clients on a PECI bus, and PECI spec doesn't allow multiple >>>>>> originators >>>>>> so only the host device can originate message. >>>>> >>>>> >>>>> >>>>> Yes, I get that. A single host still has to address slave devices. >>>>> >>>>>> In this implementation, >>>>>> all message transactions on a bus from client driver modules and user >>>>>> space will be serialized well in the PECI core bus driver so bus >>>>>> occupation and traffic arbitration will be managed well in the PECI >>>>>> core >>>>>> bus driver even in case of a bus has 2 client drivers at the same >>>>>> address. I'm sure that this implementation doesn't make that kind of >>>>>> problem to OS. >>>>> >>>>> >>>>> >>>>> Multiple clients to a single device is common, but that is a software >>>>> problem and doesn't belong in DT. >>>>> >>>>> I don't think there is a single other case in the kernel where >>>>> multiple drivers can bind to the same device at a given bus address. >>>>> That is why we have things like MFD. Though in this case, why can't >>>>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)? >>>>> >>>> >>>> It was implemented as a single driver until v2 but dimm temps need >>>> delayed creation unlikely the cpu temps on hwmon subsystem because of >>>> memory training behavior of remote x86 cpus. Since hwmon doesn't allow >>>> incremental creation, I had to divide it into two, cputemp and dimmtemp, >>>> so that cputemp can be registered immediately when the remote x86 cpu >>>> turns on and dimmtemp can be registered by delayed creation. It is the >>>> reason why I had to make the two hwmon driver modules that sharing a >>>> single device address. >>> >>> >>> That all sounds like kernel problems to me. Stop designing your DT >>> binding around what the kernel can or can't *currently* support. >>> >>>> Additionally, PECI isn't limited for temperature >>>> monitoring feature but it can be used for other functions such as >>>> platform management, cpu interface tuning and diagnostics and failure >>>> analysis, so in case of adding a new driver for the functions, we should >>>> add an another DT node which is sharing the same cpu address. >>> >>> >>> No, the driver should add support for those additional functions. >>> Perhaps you will need to use MFD. >>> >> >> Do you mean that the device address sharing is acceptable if I make >> these nodes under "simple-mfd"? >> >> Thanks, >> >> -Jae > > > Hi Rob, > > I'm planning to change the whole PECI node like below: > > peci: peci@1e78b000 { > compatible = "simple-bus"; > #address-cells = <1>; > #size-cells = <1>; > ranges = <0x0 0x1e78b000 0x60>; > > peci0: peci-bus@0 { > compatible = "aspeed,ast2500-peci"; > reg = <0x0 0x60>; > #address-cells = <1>; > #size-cells = <0>; > interrupts = <15>; > clocks = <&syscon ASPEED_CLK_GATE_REFCLK>; > resets = <&syscon ASPEED_RESET_PECI>; > clock-frequency = <24000000>; > msg-timing = <1>; > addr-timing = <1>; > rd-sampling-point = <8>; > cmd-timeout-ms = <1000>; > status = "disabled"; > > peci-client@30 { > compatible = "simple-mfd", "syscon"; These compatibles alone is not correct. There should be a specific compatible for the device. Also, I don't think "syscon" even makes sense in this case. > reg = <0x30>; > > cputemp: cputemp { > compatible = "intel,peci-cputemp"; > }; There is no point in this node being in DT. It doesn't define any resources. All it does is provide you a convenient way to bind your driver, but that is not the purpose of DT. Put a specific compatible in the parent and its driver can instantiate whatever child devices it wants. Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers 2018-05-24 13:47 ` Rob Herring @ 2018-05-24 17:09 ` Jae Hyun Yoo 0 siblings, 0 replies; 10+ messages in thread From: Jae Hyun Yoo @ 2018-05-24 17:09 UTC (permalink / raw) To: Rob Herring Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree, linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann, Jason M Biils, Joel Stanley On 5/24/2018 6:47 AM, Rob Herring wrote: > On Wed, May 23, 2018 at 4:56 PM, Jae Hyun Yoo > <jae.hyun.yoo@linux.intel.com> wrote: >> On 5/23/2018 1:03 PM, Jae Hyun Yoo wrote: >>> >>> On 5/23/2018 12:33 PM, Rob Herring wrote: >>>> >>>> On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo >>>> <jae.hyun.yoo@linux.intel.com> wrote: >>>>> >>>>> On 5/23/2018 8:11 AM, Rob Herring wrote: >>>>>> >>>>>> >>>>>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo >>>>>> <jae.hyun.yoo@linux.intel.com> wrote: >>>>>>> >>>>>>> >>>>>>> On 5/22/2018 9:42 AM, Rob Herring wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> This commit adds dt-bindings documents for PECI hwmon client >>>>>>>>> drivers. >>>>>>>>> >>>>>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >>>>>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com> >>>>>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com> >>>>>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com> >>>>>>>>> Cc: Andrew Jeffery <andrew@aj.id.au> >>>>>>>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>>>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >>>>>>>>> Cc: Joel Stanley <joel@jms.id.au> >>>>>>>>> --- >>>>>>>>> .../bindings/hwmon/peci-cputemp.txt | 23 >>>>>>>>> ++++++++++++++++++ >>>>>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24 >>>>>>>>> +++++++++++++++++++ >>>>>>>>> 2 files changed, 47 insertions(+) >>>>>>>>> create mode 100644 >>>>>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>>> create mode 100644 >>>>>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt >>>>>>>>> >>>>>>>>> diff --git >>>>>>>>> a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>>> new file mode 100644 >>>>>>>>> index 000000000000..2f59aee12d9e >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt >>>>>>>>> @@ -0,0 +1,23 @@ >>>>>>>>> +Bindings for Intel PECI (Platform Environment Control Interface) >>>>>>>>> cputemp >>>>>>>>> driver. >>>>>>>>> + >>>>>>>>> +Required properties: >>>>>>>>> +- compatible : Should be "intel,peci-cputemp". >>>>>>>>> +- reg : Should contain address of a client CPU. Address >>>>>>>>> range >>>>>>>>> of >>>>>>>>> CPU >>>>>>>>> + clients is starting from 0x30 based on PECI >>>>>>>>> specification. >>>>>>>>> + >>>>>>>>> +Example: >>>>>>>>> + peci-bus@0 { >>>>>>>>> + #address-cells = <1>; >>>>>>>>> + #size-cells = <0>; >>>>>>>>> + < more properties > >>>>>>>>> + >>>>>>>>> + peci-cputemp@30 { >>>>>>>>> + compatible = "intel,peci-cputemp"; >>>>>>>>> + reg = <0x30>; >>>>>>>>> + }; >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> [...] >>>>>>>> >>>>>>>>> + peci-dimmtemp@30 { >>>>>>>>> + compatible = "intel,peci-dimmtemp"; >>>>>>>>> + reg = <0x30>; >>>>>>>>> + }; >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> As I said in the prior version, 2 nodes at the same address is wrong. >>>>>>>> >>>>>>>> Rob >>>>>>>> >>>>>>> >>>>>>> In PECI bus, there is one and only bus host (adapter) and multiple >>>>>>> clients on a PECI bus, and PECI spec doesn't allow multiple >>>>>>> originators >>>>>>> so only the host device can originate message. >>>>>> >>>>>> >>>>>> >>>>>> Yes, I get that. A single host still has to address slave devices. >>>>>> >>>>>>> In this implementation, >>>>>>> all message transactions on a bus from client driver modules and user >>>>>>> space will be serialized well in the PECI core bus driver so bus >>>>>>> occupation and traffic arbitration will be managed well in the PECI >>>>>>> core >>>>>>> bus driver even in case of a bus has 2 client drivers at the same >>>>>>> address. I'm sure that this implementation doesn't make that kind of >>>>>>> problem to OS. >>>>>> >>>>>> >>>>>> >>>>>> Multiple clients to a single device is common, but that is a software >>>>>> problem and doesn't belong in DT. >>>>>> >>>>>> I don't think there is a single other case in the kernel where >>>>>> multiple drivers can bind to the same device at a given bus address. >>>>>> That is why we have things like MFD. Though in this case, why can't >>>>>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)? >>>>>> >>>>> >>>>> It was implemented as a single driver until v2 but dimm temps need >>>>> delayed creation unlikely the cpu temps on hwmon subsystem because of >>>>> memory training behavior of remote x86 cpus. Since hwmon doesn't allow >>>>> incremental creation, I had to divide it into two, cputemp and dimmtemp, >>>>> so that cputemp can be registered immediately when the remote x86 cpu >>>>> turns on and dimmtemp can be registered by delayed creation. It is the >>>>> reason why I had to make the two hwmon driver modules that sharing a >>>>> single device address. >>>> >>>> >>>> That all sounds like kernel problems to me. Stop designing your DT >>>> binding around what the kernel can or can't *currently* support. >>>> >>>>> Additionally, PECI isn't limited for temperature >>>>> monitoring feature but it can be used for other functions such as >>>>> platform management, cpu interface tuning and diagnostics and failure >>>>> analysis, so in case of adding a new driver for the functions, we should >>>>> add an another DT node which is sharing the same cpu address. >>>> >>>> >>>> No, the driver should add support for those additional functions. >>>> Perhaps you will need to use MFD. >>>> >>> >>> Do you mean that the device address sharing is acceptable if I make >>> these nodes under "simple-mfd"? >>> >>> Thanks, >>> >>> -Jae >> >> >> Hi Rob, >> >> I'm planning to change the whole PECI node like below: >> >> peci: peci@1e78b000 { >> compatible = "simple-bus"; >> #address-cells = <1>; >> #size-cells = <1>; >> ranges = <0x0 0x1e78b000 0x60>; >> >> peci0: peci-bus@0 { >> compatible = "aspeed,ast2500-peci"; >> reg = <0x0 0x60>; >> #address-cells = <1>; >> #size-cells = <0>; >> interrupts = <15>; >> clocks = <&syscon ASPEED_CLK_GATE_REFCLK>; >> resets = <&syscon ASPEED_RESET_PECI>; >> clock-frequency = <24000000>; >> msg-timing = <1>; >> addr-timing = <1>; >> rd-sampling-point = <8>; >> cmd-timeout-ms = <1000>; >> status = "disabled"; >> >> peci-client@30 { >> compatible = "simple-mfd", "syscon"; > > These compatibles alone is not correct. There should be a specific > compatible for the device. > > Also, I don't think "syscon" even makes sense in this case. > Got it. I'll change it like: compatible = "intel,peci-client", "simple-mfd"; so that drivers could be instantiated altogether if the drivers use the "intel,peci-client" compatible string. >> reg = <0x30>; >> >> cputemp: cputemp { >> compatible = "intel,peci-cputemp"; >> }; > > There is no point in this node being in DT. It doesn't define any > resources. All it does is provide you a convenient way to bind your > driver, but that is not the purpose of DT. Put a specific compatible > in the parent and its driver can instantiate whatever child devices it > wants. > My intention is making each driver can be selectively instantiated by this node and it could use its parent reg resource which is in simple-mfd node. If the selective instantiation is not needed, drivers could use "intel,peci-client". Please correct me if it is still unacceptable. Thanks, -Jae ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-05-24 17:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-21 19:59 [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers Jae Hyun Yoo 2018-05-22 16:42 ` Rob Herring 2018-05-22 17:18 ` Jae Hyun Yoo 2018-05-23 15:11 ` Rob Herring 2018-05-23 16:37 ` Jae Hyun Yoo 2018-05-23 19:33 ` Rob Herring 2018-05-23 20:03 ` Jae Hyun Yoo 2018-05-23 21:56 ` Jae Hyun Yoo 2018-05-24 13:47 ` Rob Herring 2018-05-24 17:09 ` Jae Hyun Yoo
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).