Messages in this thread | | | Subject | Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers | From | Jae Hyun Yoo <> | Date | Wed, 23 May 2018 13:03:02 -0700 |
| |
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
| |