Re: [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers

[Jae Hyun Yoo]

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.
> >
> > "dt-bindings: hwmon: ..." for the subject.
>
> I'll change the subject.
>
> > > Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx>
> > > Reviewed-by: Haiyue Wang <haiyue.wang@xxxxxxxxxxxxxxx>
> > > Reviewed-by: James Feist <james.feist@xxxxxxxxxxxxxxx>
> > > Reviewed-by: Vernon Mauery <vernon.mauery@xxxxxxxxxxxxxxx>
> > > Cc: Alan Cox <alan@xxxxxxxxxxxxxxx>
> > > Cc: Andrew Jeffery <andrew@xxxxxxxx>
> > > Cc: Andrew Lunn <andrew@xxxxxxx>
> > > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > > Cc: Arnd Bergmann <arnd@xxxxxxxx>
> > > Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> > > Cc: Fengguang Wu <fengguang.wu@xxxxxxxxx>
> > > Cc: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> > > Cc: Jason M Biils <jason.m.bills@xxxxxxxxxxxxxxx>
> > > Cc: Jean Delvare <jdelvare@xxxxxxxx>
> > > Cc: Joel Stanley <joel@xxxxxxxxx>
> > > Cc: Julia Cartwright <juliac@xxxxxxxxxxxx>
> > > Cc: Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx>
> > > Cc: Milton Miller II <miltonm@xxxxxxxxxx>
> > > Cc: Pavel Machek <pavel@xxxxxx>
> > > Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> > > Cc: Stef van Os <stef.van.os@xxxxxxxxxxxxxxxxxxxxxxxxx>
> > > Cc: Sumeet R Pawnikar <sumeet.r.pawnikar@xxxxxxxxx>
> > > ---
> > > Â .../devicetree/bindings/hwmon/peci-cputemp.txtÂÂÂÂ | 24 
> > > +++++++++++++++++++++
> > > Â .../devicetree/bindings/hwmon/peci-dimmtemp.txtÂÂÂ | 25 
> > > ++++++++++++++++++++++
> > > Â 2 files changed, 49 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..d5530ef9cfd2
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
> > > @@ -0,0 +1,24 @@
> > > +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.
> > > +ÂÂÂÂÂÂÂÂÂÂ <0x30> .. <0x37> (depends on the PECI_OFFSET_MAX definition)
> >
> > Again, where is PECI_OFFSET_MAX defined? It can't depend on something in
> > the kernel.
>
> I'll remove the unnecessary description.
>
> > > +
> > > +Example:
> > > +ÂÂÂ peci-bus@0 {
> > > +ÂÂÂÂÂÂÂ #address-cells = <1>;
> > > +ÂÂÂÂÂÂÂ #size-cells = <0>;
> > > +ÂÂÂÂÂÂÂ < more properties >
> > > +
> > > +ÂÂÂÂÂÂÂ peci-cputemp@cpu0 {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ compatible = "intel,peci-cputemp";
> > > +ÂÂÂÂÂÂÂÂÂÂÂ reg = <0x30>;
> > > +ÂÂÂÂÂÂÂ };
> > > +
> > > +ÂÂÂÂÂÂÂ peci-cputemp@cpu1 {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ 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..56e5deb61e5c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt
> > > @@ -0,0 +1,25 @@
> > > +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.
> > > +ÂÂÂÂÂÂÂÂÂÂ <0x30> .. <0x37> (depends on the PECI_OFFSET_MAX definition)
> > > +
> > > +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.

> > > +ÂÂÂÂÂÂÂÂÂÂÂ compatible = "intel,peci-dimmtemp";
> > > +ÂÂÂÂÂÂÂÂÂÂÂ reg = <0x30>;
> > > +ÂÂÂÂÂÂÂ };
> > > +
> > > +ÂÂÂÂÂÂÂ peci-dimmtemp@cpu1 {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ compatible = "intel,peci-dimmtemp";
> > > +ÂÂÂÂÂÂÂÂÂÂÂ reg = <0x31>;
> > > +ÂÂÂÂÂÂÂ };
> > > +ÂÂÂ };
> > > --
> > > 2.16.2
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html