[v4 08/11] Documentation: hwmon: Add documents for PECI hwmon client drivers
This commit adds hwmon documents for PECI cputemp and dimmtemp drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Randy Dunlap <rdun...@infradead.org> --- Documentation/hwmon/peci-cputemp | 78 +++ Documentation/hwmon/peci-dimmtemp | 50 2 files changed, 128 insertions(+) create mode 100644 Documentation/hwmon/peci-cputemp create mode 100644 Documentation/hwmon/peci-dimmtemp diff --git a/Documentation/hwmon/peci-cputemp b/Documentation/hwmon/peci-cputemp new file mode 100644 index ..821a9258f2e6 --- /dev/null +++ b/Documentation/hwmon/peci-cputemp @@ -0,0 +1,78 @@ +Kernel driver peci-cputemp +== + +Supported chips: + One of Intel server CPUs listed below which is connected to a PECI bus. + * Intel Xeon E5/E7 v3 server processors + Intel Xeon E5-14xx v3 family + Intel Xeon E5-24xx v3 family + Intel Xeon E5-16xx v3 family + Intel Xeon E5-26xx v3 family + Intel Xeon E5-46xx v3 family + Intel Xeon E7-48xx v3 family + Intel Xeon E7-88xx v3 family + * Intel Xeon E5/E7 v4 server processors + Intel Xeon E5-16xx v4 family + Intel Xeon E5-26xx v4 family + Intel Xeon E5-46xx v4 family + Intel Xeon E7-48xx v4 family + Intel Xeon E7-88xx v4 family + * Intel Xeon Scalable server processors + Intel Xeon Bronze family + Intel Xeon Silver family + Intel Xeon Gold family + Intel Xeon Platinum family + Addresses scanned: PECI client address 0x30 - 0x37 + Datasheet: Available from http://www.intel.com/design/literature.htm + +Author: + Jae Hyun Yoo <jae.hyun@linux.intel.com> + +Description +--- + +This driver implements a generic PECI hwmon feature which provides Digital +Thermal Sensor (DTS) thermal readings of the CPU package and CPU cores that are +accessible using the PECI Client Command Suite via the processor PECI client. + +All temperature values are given in millidegree Celsius and will be measurable +only when the target CPU is powered on. + +sysfs attributes + + +temp1_label"Die" +temp1_inputProvides current die temperature of the CPU package. +temp1_max Provides thermal control temperature of the CPU package + which is also known as Tcontrol. +temp1_crit Provides shutdown temperature of the CPU package which + is also known as the maximum processor junction + temperature, Tjmax or Tprochot. +temp1_crit_hystProvides the hysteresis value from Tcontrol to Tjmax of + the CPU package. + +temp2_label"Tcontrol" +temp2_inputProvides current Tcontrol temperature of the CPU + package which is also known as Fan Temperature target. + Indicates the relative value from thermal monitor trip + temperature at which fans should be engaged. +temp2_crit Provides Tcontrol critical value of the CPU package + which is same to Tjmax. + +temp3_label"Tthrottle" +temp3_inputProvides current Tthrottle temperature of the CPU + package. Used for throttling temperature. If this value + is allowed and lower than Tjmax - the throttle will + occur and reported at lower than Tjmax. + +temp4_label"Tjmax" +temp4_inputProvides the maximum junction temperature, Tjmax of the + CPU package. + +temp[5-*]_labelProvides string "Core X", where X is resolved core + number. +temp[5-*]_inputProvides current temperature of each core. +temp[5-*]_max Provides thermal control temperature of the core. +temp[5-*]_crit Provides shutdown temperature of the core. +temp[5-*]_crit_hystProvides the hysteresis value from Tcontrol to Tjmax of + the core. diff --git a/Documentation/hwmon/peci-dimmtemp b/Documentation/hwmon/peci-dimmtemp new file mode 100644 index ..c54f2526188c --- /dev/null +++ b/Documentation/hwmon/peci-dimmtemp @@ -0,0 +1,50 @@ +Kernel driver peci-dimmtemp +=== +
[v4 02/11] Documentation: ioctl: Add ioctl numbers for PECI subsystem
This commit updates ioctl-number.txt to reflect ioctl numbers used by the PECI subsystem. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Cc: James Feist <james.fe...@linux.intel.com> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Vernon Mauery <vernon.mau...@linux.intel.com> --- Documentation/ioctl/ioctl-number.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt index 480c8609dc58..1670ca4072b2 100644 --- a/Documentation/ioctl/ioctl-number.txt +++ b/Documentation/ioctl/ioctl-number.txt @@ -322,6 +322,8 @@ Code Seq#(hex) Include FileComments 0xB3 00 linux/mmc/ioctl.h 0xB4 00-0F linux/gpio.h<mailto:linux-g...@vger.kernel.org> 0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remotep...@vger.kernel.org> +0xB6 00-0F uapi/linux/peci-ioctl.h PECI subsystem + <mailto:jae.hyun@linux.intel.com> 0xC0 00-0F linux/usb/iowarrior.h 0xCA 00-0F uapi/misc/cxl.h 0xCA 10-2F uapi/misc/ocxl.h -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 00/10] PECI device driver introduction
. * Made client CPU model checking use if available. * Modified adapter heap allocation method to use kobject reference count based. * Added the low-level PECI xfer IOCTL again to support the Redfish requirement. * Added PM domain attach/detach code. * Added logic for device instantiation through sysfs. * Fix a bug of interrupt status checking code in peci-aspeed driver. Changes from v2: * Divided peci-hwmon driver into two drivers, peci-cputemp and peci-dimmtemp. * Added generic dt binding documents for PECI bus, adapter and client. * Removed in_atomic() call from the PECI core driver. * Improved PECI commands masking logic. * Added permission check logic for PECI ioctls. * Removed unnecessary type casts. * Fixed some invalid error return codes. * Added the mark_updated() function to improve update interval checking logic. * Fixed a bug in populated DIMM checking function. * Fixed some typo, grammar and style issues in documents. * Rewrote hwmon drivers to use devm_hwmon_device_register_with_info API. * Made peci_match_id() function as a static. * Replaced a deprecated create_singlethread_workqueue() call with an alloc_ordered_workqueue() call. * Reordered local variable definitions in reversed xmas tree notation. * Listed up client CPUs that can be supported by peci-cputemp and peci-dimmtemp hwmon drivers. * Added CPU generation detection logic which checks CPUID signature through PECI connection. * Improved interrupt handling logic in the Aspeed PECI adapter driver. * Fixed SPDX license identifier style in header files. * Changed some macros in peci.h to static inline functions. * Dropped sleepable context checking code in peci-core. * Adjusted rt_mutex protection scope in peci-core. * Moved adapter->xfer() checking code into peci_register_adapter(). * Improved PECI command retry checking logic. * Changed ioctl base from 'P' to 0xb6 to avoid confiliction and updated ioctl-number.txt to reflect the ioctl number of PECI subsystem. * Added a comment to describe PECI retry action. * Simplified return code handling of peci_ioctl_ping(). * Changed type of peci_ioctl_fn[] to static const. * Fixed range checking code for valid PECI commands. * Fixed the error return code on invalid PECI commands. * Fixed incorrect definitions of PECI ioctl and its handling logic. Changes from v1: * Additionally implemented a core driver to support PECI linux bus driver model. * Modified Aspeed PECI driver to make that to be an adapter driver in PECI bus. * Modified PECI hwmon driver to make that to be a client driver in PECI bus. * Simplified hwmon driver attribute labels and removed redundant strings. * Removed core_nums from device tree setting of hwmon driver and modified core number detection logic to check the resolved_core register in client CPU's local PCI configuration area. * Removed dimm_nums from device tree setting of hwmon driver and added populated DIMM detection logic to support dynamic creation. * Removed indexing gap on core temperature and DIMM temperature attributes. * Improved hwmon registration and dynamic attribute creation logic. * Fixed structure definitions in PECI uapi header to make that use __u8, __u16 and etc. * Modified wait_for_completion_interruptible_timeout error handling logic in Aspeed PECI driver to deliver errors correctly. * Removed low-level xfer command from ioctl and kept only high-level PECI command suite as ioctls. * Fixed I/O timeout logic in Aspeed PECI driver using ktime. * Added a function into hwmon driver to simplify update delay checking. * Added a function into hwmon driver to convert 10.6 to millidegree. * Dropped non-standard attributes in hwmon driver. * Fixed OF table for hwmon to make it indicate as a PECI client of Intel CPU target. * Added a maintainer of PECI subsystem into MAINTAINERS document. Jae Hyun Yoo (11): dt-bindings: Add a document of PECI subsystem Documentation: ioctl: Add ioctl numbers for PECI subsystem drivers/peci: Add support for PECI bus driver core dt-bindings: Add a document of PECI adapter driver for ASPEED AST24xx/25xx SoCs ARM: dts: aspeed: peci: Add PECI node drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx dt-bindings: hwmon: Add documents for PECI hwmon client drivers Documentation: hwmon: Add documents for PECI hwmon client drivers drivers/hwmon: Add PECI cputemp driver drivers/hwmon: Add PECI dimmtemp driver Add maintainers for the PECI subsystem .../bindings/hwmon/peci-cputemp.txt | 23 + .../bindings/hwmon/peci-dimmtemp.txt | 24 + .../devicetree/bindings/peci/peci-aspeed.txt | 57 + .../devicetree/bindings/peci/peci.txt | 59 + Documentation/hwmon/peci-cputemp | 78 + Documentation/hwmon/peci-dimmtemp | 50 + Documentation/ioctl/ioctl-number.txt |2 + MAINTAINERS | 10 + arch/arm/boot/dts/aspeed-g4.dtsi | 26 + arch/arm/boot/
Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core
On 4/24/2018 9:01 AM, Andy Shevchenko wrote: On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote: This commit adds driver implementation for PECI bus core into linux driver framework. All comments you got for patch 6 are applicable here. And perhaps in the rest of the series. The rule of thumb: when you get even single comment in a certain place, re-check _entire_ series for the same / similar patterns! Thanks for your advice. I'll keep that in mind. Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
Hi Andy, Thanks a lot for your review. Please check my inline answers. On 4/24/2018 8:56 AM, Andy Shevchenko wrote: On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote: drivers/hwmon/peci-cputemp.c | 783 ++ drivers/hwmon/peci-dimmtemp.c | 432 +++ Does it make sense one driver per patch? Yes, I'll separate it into two patches. +#define CLIENT_CPU_ID_MASK0xf0ff0 /* Mask for Family / Model info */ +struct cpu_gen_info { + u32 type; + u32 cpu_id; + u32 core_max; +}; +static const struct cpu_gen_info cpu_gen_info_table[] = { + { .type = CPU_GEN_HSX, + .cpu_id = 0x306f0, /* Family code: 6, Model number: 63 (0x3f) */ + .core_max = CORE_MAX_ON_HSX }, + { .type = CPU_GEN_BRX, + .cpu_id = 0x406f0, /* Family code: 6, Model number: 79 (0x4f) */ + .core_max = CORE_MAX_ON_BDX }, + { .type = CPU_GEN_SKX, + .cpu_id = 0x50650, /* Family code: 6, Model number: 85 (0x55) */ + .core_max = CORE_MAX_ON_SKX }, +}; Are we talking about x86 CPU IDs here? If so, why x86 corresponding headers, including intel-family.h are not used? Yes, that would make more sense. I'll include the intel-family.h and will use these defines instead: INTEL_FAM6_HASWELL_X INTEL_FAM6_BROADWELL_X INTEL_FAM6_SKYLAKE_X Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core
On 4/23/2018 3:52 AM, Greg KH wrote: On Tue, Apr 10, 2018 at 11:32:05AM -0700, Jae Hyun Yoo wrote: +static void peci_adapter_dev_release(struct device *dev) +{ + /* do nothing */ +} As per the in-kernel documentation, I am now allowed to make fun of you. You are trying to "out smart" the kernel by getting rid of a warning message that was explicitly put there for you to do something. To think that by just providing an "empty" function you are somehow fulfilling the API requirement is quite bold, don't you think? This has to be fixed. I didn't put that warning in there for no good reason. Please go read the documentation again... greg k-h Hi Greg, Thanks a lot for your review. I think, it should contain actual device resource release code which is being done by peci_del_adapter(), or a coupling logic should be added between peci_adapter_dev_release() and peci_del_adapter(). As you suggested, I'll check it again after reading documentation and understanding core.c code more deeply. Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers
On 4/18/2018 2:57 PM, Jae Hyun Yoo wrote: On 4/18/2018 2:28 PM, Rob Herring wrote: On Wed, Apr 18, 2018 at 3:28 PM, Jae Hyun Yoo <jae.hyun@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@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 = < 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 If I change it to a single DT node which registers 2 hwmon devices using the 2nd option above, then I still have 2 devices on a bus at the same address. Does it also make a problem to the OS then? Jae Additionally, I need to explain that 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.
Re: [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers
On 4/18/2018 7:32 AM, Rob Herring wrote: On Tue, Apr 17, 2018 at 3:40 PM, Jae Hyun Yoo <jae.hyun@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: 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 = < 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. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
On 4/18/2018 6:59 AM, Rob Herring wrote: On Tue, Apr 17, 2018 at 5:06 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: On 4/17/2018 11:16 AM, Jae Hyun Yoo wrote: On 4/17/2018 6:16 AM, Rob Herring wrote: On Mon, Apr 16, 2018 at 6:12 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: On 4/16/2018 11:10 AM, Rob Herring wrote: On Tue, Apr 10, 2018 at 11:32:06AM -0700, Jae Hyun Yoo wrote: This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. [...] +- clocks: Should contain clock source for PECI controller. + Should reference clkin. +- clock_frequency : Should contain the operation frequency of PECI controller + in units of Hz. + 187500 ~ 2400 This is the frequency of the bus or used to derive it? It would be better to specify the bus frequency instead and have the driver calculate its internal freq. And then use "bus-frequency" instead. I agree with you. Actually, it is being used for operation frequency setting of PECI controller module in SoC so it's different from the meaning of "bus-frequency". I'll change it to "operation-frequency". No, now you've gone from a standard property name to something custom. Why do you need to set the frequency in DT if it is not related to the interface frequency? Rob Actually, the interface frequency is affected by the operation frequency but there is no description of its relationship in datasheet. I'll check again about the detail to ASPEED chip vendor and will use 'bus-frequency' if available. I investigated it more deeply. Basically, by the spec, PECI bus speed cannot be set as a fixed speed. A PECI bus can have a wide speed range from 2Kbps to 2Mbps which is dynamically set by a handshaking sequence between an originator and clients called 'timing negotiation' in spec. This timing negotiation behavior happens on every single transaction so the bus speed also can vary on every transactions. So I'm thinking a custom property name for it, 'peci-clk-frequency' if it is acceptable. Okay, seems bus-frequency is not appropriate here. So use 'clock-frequency' (note the '-' not '_' as that is the standard property). Rob Thanks! I'll use 'clock-frequency' for it. Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
On 4/17/2018 11:16 AM, Jae Hyun Yoo wrote: On 4/17/2018 6:16 AM, Rob Herring wrote: On Mon, Apr 16, 2018 at 6:12 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: On 4/16/2018 11:10 AM, Rob Herring wrote: On Tue, Apr 10, 2018 at 11:32:06AM -0700, Jae Hyun Yoo wrote: This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. [...] +- clocks : Should contain clock source for PECI controller. + Should reference clkin. +- clock_frequency : Should contain the operation frequency of PECI controller + in units of Hz. + 187500 ~ 2400 This is the frequency of the bus or used to derive it? It would be better to specify the bus frequency instead and have the driver calculate its internal freq. And then use "bus-frequency" instead. I agree with you. Actually, it is being used for operation frequency setting of PECI controller module in SoC so it's different from the meaning of "bus-frequency". I'll change it to "operation-frequency". No, now you've gone from a standard property name to something custom. Why do you need to set the frequency in DT if it is not related to the interface frequency? Rob Actually, the interface frequency is affected by the operation frequency but there is no description of its relationship in datasheet. I'll check again about the detail to ASPEED chip vendor and will use 'bus-frequency' if available. I investigated it more deeply. Basically, by the spec, PECI bus speed cannot be set as a fixed speed. A PECI bus can have a wide speed range from 2Kbps to 2Mbps which is dynamically set by a handshaking sequence between an originator and clients called 'timing negotiation' in spec. This timing negotiation behavior happens on every single transaction so the bus speed also can vary on every transactions. So I'm thinking a custom property name for it, 'peci-clk-frequency' if it is acceptable. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/10] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx
Hi Robin, On 4/17/2018 6:37 AM, Robin Murphy wrote: Just a drive-by nit: On 10/04/18 19:32, Jae Hyun Yoo wrote: [...] +#define PECI_CTRL_SAMPLING_MASK GENMASK(19, 16) +#define PECI_CTRL_SAMPLING(x) (((x) << 16) & PECI_CTRL_SAMPLING_MASK) +#define PECI_CTRL_SAMPLING_GET(x) (((x) & PECI_CTRL_SAMPLING_MASK) >> 16) FWIW, already provides functionality like this, so it might be worth taking a look at FIELD_{GET,PREP}() to save all these local definitions. Robin. Yes, that looks better. Thanks a lot for your pointing it out. Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
On 4/17/2018 6:16 AM, Rob Herring wrote: On Mon, Apr 16, 2018 at 6:12 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: On 4/16/2018 11:10 AM, Rob Herring wrote: On Tue, Apr 10, 2018 at 11:32:06AM -0700, Jae Hyun Yoo wrote: This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. [...] +- clocks: Should contain clock source for PECI controller. + Should reference clkin. +- clock_frequency : Should contain the operation frequency of PECI controller + in units of Hz. + 187500 ~ 2400 This is the frequency of the bus or used to derive it? It would be better to specify the bus frequency instead and have the driver calculate its internal freq. And then use "bus-frequency" instead. I agree with you. Actually, it is being used for operation frequency setting of PECI controller module in SoC so it's different from the meaning of "bus-frequency". I'll change it to "operation-frequency". No, now you've gone from a standard property name to something custom. Why do you need to set the frequency in DT if it is not related to the interface frequency? Rob Actually, the interface frequency is affected by the operation frequency but there is no description of its relationship in datasheet. I'll check again about the detail to ASPEED chip vendor and will use 'bus-frequency' if available. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers
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@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../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 ..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 ..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
Re: [PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers
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@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../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 ..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 ..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 + 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
On 4/16/2018 11:10 AM, Rob Herring wrote: On Tue, Apr 10, 2018 at 11:32:06AM -0700, Jae Hyun Yoo wrote: This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../devicetree/bindings/peci/peci-aspeed.txt | 60 ++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt new file mode 100644 index ..4598bb8c20fa --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt @@ -0,0 +1,60 @@ +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs. + +Required properties: +- compatible: Should be "aspeed,ast2400-peci" or "aspeed,ast2500-peci" + - aspeed,ast2400-peci: Aspeed AST2400 family PECI +controller + - aspeed,ast2500-peci: Aspeed AST2500 family PECI +controller +- reg : Should contain PECI controller registers location and + length. +- #address-cells: Should be <1>. +- #size-cells : Should be <0>. +- interrupts: Should contain PECI controller interrupt. +- clocks: Should contain clock source for PECI controller. + Should reference clkin. +- clock_frequency : Should contain the operation frequency of PECI controller + in units of Hz. + 187500 ~ 2400 This is the frequency of the bus or used to derive it? It would be better to specify the bus frequency instead and have the driver calculate its internal freq. And then use "bus-frequency" instead. I agree with you. Actually, it is being used for operation frequency setting of PECI controller module in SoC so it's different from the meaning of "bus-frequency". I'll change it to "operation-frequency". + +Optional properties: +- msg-timing-nego : Message timing negotiation period. This value will + determine the period of message timing negotiation to be + issued by PECI controller. The unit of the programmed + value is four times of PECI clock period. + 0 ~ 255 (default: 1) +- addr-timing-nego : Address timing negotiation period. This value will + determine the period of address timing negotiation to be + issued by PECI controller. The unit of the programmed + value is four times of PECI clock period. + 0 ~ 255 (default: 1) +- rd-sampling-point : Read sampling point selection. The whole period of a bit + time will be divided into 16 time frames. This value will + determine the time frame in which the controller will + sample PECI signal for data read back. Usually in the + middle of a bit time is the best. + 0 ~ 15 (default: 8) +- cmd_timeout_ms: Command timeout in units of ms. + 1 ~ 6 (default: 1000) s/_/-/ Will fix it. All these either need vendor prefixes or should be standard properties for PECI adapters. I think probably the latter case. If so, the first 2 should probably be in units of clocks (not 4 clocks). And they should then be documented in the common PECI binding doc. So far I've checked that these are ASPEED PECI controller specific properties so it should be listed in here. + +Example: + peci: peci@1e78b000 { + compatible = "simple-bus"; +
Re: [PATCH v3 01/10] Documentations: dt-bindings: Add documents of generic PECI bus, adapter and client drivers
Hi Rob, Thanks for sharing your time. Please see my answers inline. On 4/16/2018 10:59 AM, Rob Herring wrote: On Tue, Apr 10, 2018 at 11:32:03AM -0700, Jae Hyun Yoo wrote: This commit adds documents of generic PECI bus, adapter and client drivers. "dt-bindings: ..." for the subject prefix please. Sure, I'll change the subject. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../devicetree/bindings/peci/peci-adapter.txt | 23 .../devicetree/bindings/peci/peci-bus.txt | 15 + .../devicetree/bindings/peci/peci-client.txt | 25 ++ This should be all one document. Okay. I'll combine them into one document. 3 files changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-adapter.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-bus.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-client.txt diff --git a/Documentation/devicetree/bindings/peci/peci-adapter.txt b/Documentation/devicetree/bindings/peci/peci-adapter.txt new file mode 100644 index ..9221374f6b11 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-adapter.txt @@ -0,0 +1,23 @@ +Generic device tree configuration for PECI adapters. + +Required properties: +- compatible : Should contain hardware specific definition strings that can + match an adapter driver implementation. +- reg: Should contain PECI controller registers location and length. No need for these 2 here. Will drop these 2. +- #address-cells : Should be <1>. +- #size-cells: Should be <0>. Some details on the addressing for PECI would be good. It is for the PECI client address. Will add details. + +Example: + peci: peci@1000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1000 0x1000>; + This part of the example is not relevant. Just start with the adapter node. Will remove that part. Thanks! + peci0: peci-bus@0 { + compatible = "soc,soc-peci"; + reg = <0x0 0x1000>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; diff --git a/Documentation/devicetree/bindings/peci/peci-bus.txt b/Documentation/devicetree/bindings/peci/peci-bus.txt new file mode 100644 index ..90bcc791ccb0 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-bus.txt @@ -0,0 +1,15 @@ +Generic device tree configuration for PECI buses. + +Required properties: +- compatible : Should be "simple-bus". I don't understand what this has to do with PECI? "simple-bus" already has a defined meaning. Maybe I'm wrong but I intended to show this node is an umbrella node of a PECI bus subsystem. What should I use then? +- #address-cells : Should be <1>. +- #size-cells: Should be <1>. +- ranges : Should contain PECI controller registers ranges. + +Example: + peci: peci@1000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1000 0x1000>; + }; diff --git a/Documentation/devicetree/bindings/peci/peci-client.txt b/Documentation/devicetree/bindings/peci/peci-client.txt new file mode 100644 index ..8e2bfd8532f6 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-client.txt @@ -0,0 +1,25 @@ +Generic device tree configuration for PECI clients. + +Required properties: +- compatible : Should contain target device specific def
Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
On 4/12/2018 10:37 AM, Guenter Roeck wrote: On Thu, Apr 12, 2018 at 10:09:51AM -0700, Jae Hyun Yoo wrote: [ ... ] +static int find_core_index(struct peci_cputemp *priv, int channel) +{ + int core_channel = channel - DEFAULT_CHANNEL_NUMS; + int idx, found = 0; + + for (idx = 0; idx < priv->gen_info->core_max; idx++) { + if (priv->core_mask & BIT(idx)) { + if (core_channel == found) + break; + + found++; + } + } + + return idx; What if nothing is found ? Core temperature group will be registered only when it detects at least one core checked by check_resolved_cores(), so find_core_index() can be called only when priv->core_mask has a non-zero value. The 'nothing is found' case will not happen. That doesn't guarantee a match. If what you are saying is correct there should always be a well defined match of channel -> idx, and the search should be unnecessary. There could be some disabled cores in the resolved core mask bit sequence also it should remove indexing gap in channel numbering so it is the reason why this search function is needed. Well defined match of channel -> idx would not be always satisfied. Are you saying that each call to the function, with the same parameters, can return a different result ? No, the result will be consistent. After reading the priv->core_mask once in check_resolved_cores(), the value will not be changed. I'm saying about this case, for example if core number 2 is unresolved in total 4 cores, then the idx order will be '0, 1, 3' but channel order will be '5, 6, 7' without making any indexing gap. And you yet you claim that this is not well defined ? Or are you concerned about the amount of memory consumed by providing an array for the mapping ? Note that an indexing gap is acceptable and, in many cases, preferred. If the indexing gap is acceptable, the index search function isn't needed anymore. I'll fix all relating code to make that use direct mapping of channel -> idx then. Thanks! [ ... ] + + dev_dbg(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), priv->name); + Why does this message display the device name twice ? For an example, dev_name(hwmon_dev) shows 'hwmon5' and priv->name shows 'peci-cputemp0'. And dev_dbg() shows another device name. So you'll have something like peci-cputemp0: hwmon5: sensor 'peci-cputemp0' Practically it shows like peci-cputemp 0-30:00: hwmon10: sensor 'peci_cputemp.cpu0' where 0-30:00 is assigned by peci core. And what message would you see for cpu1 ? It shows like peci-cputemp 0-31:00: hwmon10: sensor 'peci_cputemp.cpu1' -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
On 4/11/2018 8:40 PM, Guenter Roeck wrote: On 04/11/2018 07:51 PM, Jae Hyun Yoo wrote: On 4/11/2018 5:34 PM, Guenter Roeck wrote: On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote: Hi Guenter, Thanks a lot for sharing your time. Please see my inline answers. On 4/10/2018 3:28 PM, Guenter Roeck wrote: On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote: This commit adds PECI cputemp and dimmtemp hwmon drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- drivers/hwmon/Kconfig | 28 ++ drivers/hwmon/Makefile | 2 + drivers/hwmon/peci-cputemp.c | 783 ++ drivers/hwmon/peci-dimmtemp.c | 432 +++ 4 files changed, 1245 insertions(+) create mode 100644 drivers/hwmon/peci-cputemp.c create mode 100644 drivers/hwmon/peci-dimmtemp.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index f249a4428458..c52f610f81d0 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1259,6 +1259,34 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_CPUTEMP + tristate "PECI CPU temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI + cputemp driver which provides Digital Thermal Sensor (DTS) thermal + readings of the CPU package and CPU cores that are accessible using + the PECI Client Command Suite via the processor PECI client. + Check Documentation/hwmon/peci-cputemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-cputemp. + +config SENSORS_PECI_DIMMTEMP + tristate "PECI DIMM temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI hwmon + driver which provides Digital Thermal Sensor (DTS) thermal readings of + DIMM components that are accessible using the PECI Client Command + Suite via the processor PECI client. + Check Documentation/hwmon/peci-dimmtemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-dimmtemp. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index e7d52a36e6c4..48d9598fcd3a 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_CPUTEMP) += peci-cputemp.o +obj-$(CONFIG_SENSORS_PECI_DIMMTEMP) += peci-dimmtemp.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c new file mode 100644 index ..f0bc92687512 --- /dev/null +++ b/drivers/hwmon/peci-cputemp.c @@ -0,0 +1,783 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include Is this include needed ? No it isn't. Will drop the line. +#include +#include +#include +#include + +#define TEMP_TYPE_PECI 6 /* Sensor type 6: Intel PECI */ + +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ + +#define DEFAULT_CHANNEL
Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
On 4/11/2018 5:34 PM, Guenter Roeck wrote: On 04/11/2018 02:59 PM, Jae Hyun Yoo wrote: Hi Guenter, Thanks a lot for sharing your time. Please see my inline answers. On 4/10/2018 3:28 PM, Guenter Roeck wrote: On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote: This commit adds PECI cputemp and dimmtemp hwmon drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- drivers/hwmon/Kconfig | 28 ++ drivers/hwmon/Makefile | 2 + drivers/hwmon/peci-cputemp.c | 783 ++ drivers/hwmon/peci-dimmtemp.c | 432 +++ 4 files changed, 1245 insertions(+) create mode 100644 drivers/hwmon/peci-cputemp.c create mode 100644 drivers/hwmon/peci-dimmtemp.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index f249a4428458..c52f610f81d0 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1259,6 +1259,34 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_CPUTEMP + tristate "PECI CPU temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI + cputemp driver which provides Digital Thermal Sensor (DTS) thermal + readings of the CPU package and CPU cores that are accessible using + the PECI Client Command Suite via the processor PECI client. + Check Documentation/hwmon/peci-cputemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-cputemp. + +config SENSORS_PECI_DIMMTEMP + tristate "PECI DIMM temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI hwmon + driver which provides Digital Thermal Sensor (DTS) thermal readings of + DIMM components that are accessible using the PECI Client Command + Suite via the processor PECI client. + Check Documentation/hwmon/peci-dimmtemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-dimmtemp. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index e7d52a36e6c4..48d9598fcd3a 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_CPUTEMP) += peci-cputemp.o +obj-$(CONFIG_SENSORS_PECI_DIMMTEMP) += peci-dimmtemp.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c new file mode 100644 index ..f0bc92687512 --- /dev/null +++ b/drivers/hwmon/peci-cputemp.c @@ -0,0 +1,783 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include Is this include needed ? No it isn't. Will drop the line. +#include +#include +#include +#include + +#define TEMP_TYPE_PECI 6 /* Sensor type 6: Intel PECI */ + +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ + +#define DEFAULT_CHANNEL_NUMS 5 +#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX +#define CPUTEMP_CHANNEL_NUMS (DEF
Re: [PATCH v3 05/10] ARM: dts: aspeed: peci: Add PECI node
On 4/11/2018 4:52 AM, Joel Stanley wrote: On 11 April 2018 at 04:02, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit adds PECI bus/adapter node of AST24xx/AST25xx into aspeed-g4 and aspeed-g5. The patches to the device trees get merged by the ASPEED maintainer (me). Once you have the bindings reviewed you can send the patches to me and the linux-aspeed list (I've got a pending patch to maintainers that will ensure get_maintainers.pl does the right thing as far as email addresses go). I'd suggest dropping it from your series and re-sending once the bindings and driver are reviewed. Cheers, Joel Do you mean that bindings and driver of ASPEED peci adapter driver including documents? Thanks, -Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
Hi Joel, On 4/11/2018 4:52 AM, Joel Stanley wrote: On 11 April 2018 at 04:02, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit adds a dt-bindings document of PECI adapter driver for Aspeed We try to capitalise ASPEED. Got it. Will capitalize all Aspeed words. AST24xx/25xx SoCs. --- .../devicetree/bindings/peci/peci-aspeed.txt | 60 ++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt new file mode 100644 index ..4598bb8c20fa --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt @@ -0,0 +1,60 @@ +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs. + +Required properties: +- compatible: Should be "aspeed,ast2400-peci" or "aspeed,ast2500-peci" + - aspeed,ast2400-peci: Aspeed AST2400 family PECI +controller + - aspeed,ast2500-peci: Aspeed AST2500 family PECI +controller +- reg : Should contain PECI controller registers location and + length. +- #address-cells: Should be <1>. +- #size-cells : Should be <0>. +- interrupts: Should contain PECI controller interrupt. +- clocks: Should contain clock source for PECI controller. + Should reference clkin. Are you sure that this is driven by clkin? Most peripherals on the Aspeed are attached to the apb, so should reference that clock. According to the datasheet, PECI controller module is attached to apb but its clock source is the 24MHz external clock. +- clock_frequency : Should contain the operation frequency of PECI controller + in units of Hz. + 187500 ~ 2400 Can you explain why you need both the parent clock and this frequency to be specified? Based on this setting, driver code makes clock divisor value to set operation clock of PECI controller which is adjustable. + +Optional properties: +- msg-timing-nego : Message timing negotiation period. This value will Perhaps msg-timing-period? Or just msg-timing? Will use msg-timing instead. + determine the period of message timing negotiation to be + issued by PECI controller. The unit of the programmed + value is four times of PECI clock period. + 0 ~ 255 (default: 1) +- addr-timing-nego : Address timing negotiation period. This value will + determine the period of address timing negotiation to be + issued by PECI controller. The unit of the programmed + value is four times of PECI clock period. + 0 ~ 255 (default: 1) +- rd-sampling-point : Read sampling point selection. The whole period of a bit + time will be divided into 16 time frames. This value will + determine the time frame in which the controller will + sample PECI signal for data read back. Usually in the + middle of a bit time is the best. + 0 ~ 15 (default: 8) +- cmd_timeout_ms: Command timeout in units of ms. + 1 ~ 6 (default: 1000) + +Example: + 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 = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + }; + }; -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/10] Documentations: dt-bindings: Add documents of generic PECI bus, adapter and client drivers
Hi Joel, On 4/11/2018 4:52 AM, Joel Stanley wrote: Hi Jae, On 11 April 2018 at 04:02, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit adds documents of generic PECI bus, adapter and client drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> That's a hefty cc list. I can't see Rob Herring though, and he's usually the person who you need to convince to get your bindings accepted. I recommend using ./scripts/get_maintainers.pl to build your CC list, and then add others you think are relevant. I'm not sure what the guidelines are for generic bindings, so I'll defer to Rob for this patch. Cheers, Joel Thanks a lot for letting me know that. I'll do as you suggested. -Jae --- .../devicetree/bindings/peci/peci-adapter.txt | 23 .../devicetree/bindings/peci/peci-bus.txt | 15 + .../devicetree/bindings/peci/peci-client.txt | 25 ++ 3 files changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-adapter.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-bus.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-client.txt diff --git a/Documentation/devicetree/bindings/peci/peci-adapter.txt b/Documentation/devicetree/bindings/peci/peci-adapter.txt new file mode 100644 index ..9221374f6b11 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-adapter.txt @@ -0,0 +1,23 @@ +Generic device tree configuration for PECI adapters. + +Required properties: +- compatible : Should contain hardware specific definition strings that can + match an adapter driver implementation. +- reg: Should contain PECI controller registers location and length. +- #address-cells : Should be <1>. +- #size-cells: Should be <0>. + +Example: + peci: peci@1000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1000 0x1000>; + + peci0: peci-bus@0 { + compatible = "soc,soc-peci"; + reg = <0x0 0x1000>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; diff --git a/Documentation/devicetree/bindings/peci/peci-bus.txt b/Documentation/devicetree/bindings/peci/peci-bus.txt new file mode 100644 index ..90bcc791ccb0 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-bus.txt @@ -0,0 +1,15 @@ +Generic device tree configuration for PECI buses. + +Required properties: +- compatible : Should be "simple-bus". +- #address-cells : Should be <1>. +- #size-cells: Should be <1>. +- ranges : Should contain PECI controller registers ranges. + +Example: + peci: peci@1000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1000 0x1000>; + }; diff --git a/Documentation/devicetree/bindings/peci/peci-client.txt b/Documentation/devicetree/bindings/peci/peci-client.txt new file mode 100644 index ..8e2bfd8532f6 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-client.txt @@ -0,0 +1,25 @@ +Generic device tree configuration for PECI clients. + +Required properties: +- compatible : Should contain target device specific definition strings that can + match a client driver implementation. +- reg: Should contain address of a client CPU. Address range of CPU + clients is starting from 0x30 based on PECI specification. + <0x30> .. <0x37>
Re: [PATCH v3 06/10] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx
Hello Joel, Thanks for sharing your time. Please see my answers inline. On 4/11/2018 4:51 AM, Joel Stanley wrote: Hello Jae, On 11 April 2018 at 04:02, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit adds PECI adapter driver implementation for Aspeed AST24xx/AST25xx. The driver is looking good! It looks like you've done some kind of review that we weren't allowed to see, which is a double edged sword - I might be asking about things that you've already spoken about with someone else. I'm only just learning about PECI, but I do have some general comments below. Yes, it took a hidden review process between v2 and v3. I know it's an unusual process but it was requested. Hopefully, change logs in cover letter could roughly provide the details. Thanks for your comments. --- drivers/peci/Kconfig | 28 +++ drivers/peci/Makefile | 3 + drivers/peci/peci-aspeed.c | 504 + 3 files changed, 535 insertions(+) create mode 100644 drivers/peci/peci-aspeed.c diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig index 1fbc13f9e6c2..0e33420365de 100644 --- a/drivers/peci/Kconfig +++ b/drivers/peci/Kconfig @@ -14,4 +14,32 @@ config PECI processors and chipset components to external monitoring or control devices. + If you want PECI support, you should say Y here and also to the + specific driver for your bus adapter(s) below. + +if PECI + +# +# PECI hardware bus configuration +# + +menu "PECI Hardware Bus support" + +config PECI_ASPEED + tristate "Aspeed AST24xx/AST25xx PECI support" I think just saying ASPEED PECI support is enough. That way if the next ASPEED SoC happens to have PECI we don't need to update all of the help text :) Agreed. I'll change the description. + select REGMAP_MMIO + depends on OF + depends on ARCH_ASPEED || COMPILE_TEST + help + Say Y here if you want support for the Platform Environment Control + Interface (PECI) bus adapter driver on the Aspeed AST24XX and AST25XX + SoCs. + + This support is also available as a module. If so, the module + will be called peci-aspeed. + +endmenu + +endif # PECI + endmenu diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile index 9e8615e0d3ff..886285e69765 100644 --- a/drivers/peci/Makefile +++ b/drivers/peci/Makefile @@ -4,3 +4,6 @@ # Core functionality obj-$(CONFIG_PECI) += peci-core.o + +# Hardware specific bus drivers +obj-$(CONFIG_PECI_ASPEED) += peci-aspeed.o diff --git a/drivers/peci/peci-aspeed.c b/drivers/peci/peci-aspeed.c new file mode 100644 index ..be2a1f327eb1 --- /dev/null +++ b/drivers/peci/peci-aspeed.c @@ -0,0 +1,504 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2012-2017 ASPEED Technology Inc. +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DUMP_DEBUG 0 + +/* Aspeed PECI Registers */ +#define AST_PECI_CTRL 0x00 Nit: we use ASPEED instead of AST in the upstream kernel to distingush from the aspeed sdk drivers. If you feel strongly about this then I won't insist you change. Okay then, better change it now than later. Will change all defines. +#define AST_PECI_TIMING 0x04 +#define AST_PECI_CMD 0x08 +#define AST_PECI_CMD_CTRL 0x0c +#define AST_PECI_EXP_FCS 0x10 +#define AST_PECI_CAP_FCS 0x14 +#define AST_PECI_INT_CTRL 0x18 +#define AST_PECI_INT_STS 0x1c +#define AST_PECI_W_DATA0 0x20 +#define AST_PECI_W_DATA1 0x24 +#define AST_PECI_W_DATA2 0x28 +#define AST_PECI_W_DATA3 0x2c +#define AST_PECI_R_DATA0 0x30 +#define AST_PECI_R_DATA1 0x34 +#define AST_PECI_R_DATA2 0x38 +#define AST_PECI_R_DATA3 0x3c +#define AST_PECI_W_DATA4 0x40 +#define AST_PECI_W_DATA5 0x44 +#define AST_PECI_W_DATA6 0x48 +#define AST_PECI_W_DATA7 0x4c +#define AST_PECI_R_DATA4 0x50 +#define AST_PECI_R_DATA5 0x54 +#define AST_PECI_R_DATA6 0x58 +#define AST_PECI_R_DATA7 0x5c + +/* AST_PECI_CTRL - 0x00 : Control Register */ +#define PECI_CTRL_SAMPLING_MASK GENMASK(19, 16) +#define PECI_CTRL_SAMPLING(x) (((x) << 16) & PECI_CTRL_SAMPLING_MASK) +#define PECI_CTRL_SAMPLING_GET(x) (((x) & PECI_CTRL_SAMPLING_MASK) >> 16) +#define PECI_CTRL_READ_MODE_MASKGENMASK(13, 12) +#define PECI_CTRL_READ_MODE(x) (((x) << 12) & PECI_CTRL_READ_MODE_MASK) +#define PECI_CTRL_READ_MODE_GET(x) (((x) & PECI_CTRL_READ_MODE_MASK) >> 12) +#define PECI_CTRL_READ_MODE_COUNT BIT(12) +#define PECI_CTRL_READ_MODE_DBG BIT(13) +#define PECI_CTRL_CLK_SOURCE_MASK BIT(11) +#define PECI_CTRL_CLK_SOURCE(x) (((x) << 11) & PECI_CTRL_CLK_SOURCE_MASK) +#define PECI_CTRL_CLK_SOURCE_GET(x) (((x) & PECI_CTRL_CLK_SOURCE_MASK) >> 11) +#define PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8) +#define PECI_CTRL_CL
Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
Hi Guenter, Thanks a lot for sharing your time. Please see my inline answers. On 4/10/2018 3:28 PM, Guenter Roeck wrote: On Tue, Apr 10, 2018 at 11:32:11AM -0700, Jae Hyun Yoo wrote: This commit adds PECI cputemp and dimmtemp hwmon drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- drivers/hwmon/Kconfig | 28 ++ drivers/hwmon/Makefile| 2 + drivers/hwmon/peci-cputemp.c | 783 ++ drivers/hwmon/peci-dimmtemp.c | 432 +++ 4 files changed, 1245 insertions(+) create mode 100644 drivers/hwmon/peci-cputemp.c create mode 100644 drivers/hwmon/peci-dimmtemp.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index f249a4428458..c52f610f81d0 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1259,6 +1259,34 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_CPUTEMP + tristate "PECI CPU temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI + cputemp driver which provides Digital Thermal Sensor (DTS) thermal + readings of the CPU package and CPU cores that are accessible using + the PECI Client Command Suite via the processor PECI client. + Check Documentation/hwmon/peci-cputemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-cputemp. + +config SENSORS_PECI_DIMMTEMP + tristate "PECI DIMM temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI hwmon + driver which provides Digital Thermal Sensor (DTS) thermal readings of + DIMM components that are accessible using the PECI Client Command + Suite via the processor PECI client. + Check Documentation/hwmon/peci-dimmtemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-dimmtemp. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index e7d52a36e6c4..48d9598fcd3a 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_CPUTEMP) += peci-cputemp.o +obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)+= peci-dimmtemp.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c new file mode 100644 index ..f0bc92687512 --- /dev/null +++ b/drivers/hwmon/peci-cputemp.c @@ -0,0 +1,783 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include Is this include needed ? No it isn't. Will drop the line. +#include +#include +#include +#include + +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ + +#define DEFAULT_CHANNEL_NUMS 5 +#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX +#define CPUTEMP_CHANNEL_NUMS (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNE
[PATCH v3 01/10] Documentations: dt-bindings: Add documents of generic PECI bus, adapter and client drivers
This commit adds documents of generic PECI bus, adapter and client drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../devicetree/bindings/peci/peci-adapter.txt | 23 .../devicetree/bindings/peci/peci-bus.txt | 15 + .../devicetree/bindings/peci/peci-client.txt | 25 ++ 3 files changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-adapter.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-bus.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-client.txt diff --git a/Documentation/devicetree/bindings/peci/peci-adapter.txt b/Documentation/devicetree/bindings/peci/peci-adapter.txt new file mode 100644 index ..9221374f6b11 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-adapter.txt @@ -0,0 +1,23 @@ +Generic device tree configuration for PECI adapters. + +Required properties: +- compatible : Should contain hardware specific definition strings that can + match an adapter driver implementation. +- reg: Should contain PECI controller registers location and length. +- #address-cells : Should be <1>. +- #size-cells: Should be <0>. + +Example: + peci: peci@1000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1000 0x1000>; + + peci0: peci-bus@0 { + compatible = "soc,soc-peci"; + reg = <0x0 0x1000>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; diff --git a/Documentation/devicetree/bindings/peci/peci-bus.txt b/Documentation/devicetree/bindings/peci/peci-bus.txt new file mode 100644 index ..90bcc791ccb0 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-bus.txt @@ -0,0 +1,15 @@ +Generic device tree configuration for PECI buses. + +Required properties: +- compatible : Should be "simple-bus". +- #address-cells : Should be <1>. +- #size-cells: Should be <1>. +- ranges : Should contain PECI controller registers ranges. + +Example: + peci: peci@1000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1000 0x1000>; + }; diff --git a/Documentation/devicetree/bindings/peci/peci-client.txt b/Documentation/devicetree/bindings/peci/peci-client.txt new file mode 100644 index ..8e2bfd8532f6 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-client.txt @@ -0,0 +1,25 @@ +Generic device tree configuration for PECI clients. + +Required properties: +- compatible : Should contain target device specific definition strings that can + match a client driver implementation. +- 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 > + + function@cpu0 { + compatible = "device,function"; + reg = <0x30>; + }; + + function@cpu1 { + compatible = "device,function"; + reg = <0x31>; + }; + }; -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/10] PECI device driver introduction
adapter driver. * Fixed SPDX license identifier style in header files. * Changed some macros in peci.h to static inline functions. * Dropped sleepable context checking code in peci-core. * Adjusted rt_mutex protection scope in peci-core. * Moved adapter->xfer() checking code into peci_register_adapter(). * Improved PECI command retry checking logic. * Changed ioctl base from 'P' to 0xb6 to avoid confiliction and updated ioctl-number.txt to reflect the ioctl number of PECI subsystem. * Added a comment to describe PECI retry action. * Simplified return code handling of peci_ioctl_ping(). * Changed type of peci_ioctl_fn[] to static const. * Fixed range checking code for valid PECI commands. * Fixed the error return code on invalid PECI commands. * Fixed incorrect definitions of PECI ioctl and its handling logic. Changes from v1: * Additionally implemented a core driver to support PECI linux bus driver model. * Modified Aspeed PECI driver to make that to be an adapter driver in PECI bus. * Modified PECI hwmon driver to make that to be a client driver in PECI bus. * Simplified hwmon driver attribute labels and removed redundant strings. * Removed core_nums from device tree setting of hwmon driver and modified core number detection logic to check the resolved_core register in client CPU's local PCI configuration area. * Removed dimm_nums from device tree setting of hwmon driver and added populated DIMM detection logic to support dynamic creation. * Removed indexing gap on core temperature and DIMM temperature attributes. * Improved hwmon registration and dynamic attribute creation logic. * Fixed structure definitions in PECI uapi header to make that use __u8, __u16 and etc. * Modified wait_for_completion_interruptible_timeout error handling logic in Aspeed PECI driver to deliver errors correctly. * Removed low-level xfer command from ioctl and kept only high-level PECI command suite as ioctls. * Fixed I/O timeout logic in Aspeed PECI driver using ktime. * Added a function into hwmon driver to simplify update delay checking. * Added a function into hwmon driver to convert 10.6 to millidegree. * Dropped non-standard attributes in hwmon driver. * Fixed OF table for hwmon to make it indicate as a PECI client of Intel CPU target. * Added a maintainer of PECI subsystem into MAINTAINERS document. Fengguang Wu (1): drivers/peci: Add support for PECI bus driver core Jae Hyun Yoo (10): Documentations: dt-bindings: Add documents of generic PECI bus, adapter and client drivers Documentations: ioctl: Add ioctl numbers for PECI subsystem drivers/peci: Add support for PECI bus driver core Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs ARM: dts: aspeed: peci: Add PECI node drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx Documentation: dt-bindings: Add documents for PECI hwmon client drivers Documentation: hwmon: Add documents for PECI hwmon client drivers drivers/hwmon: Add PECI hwmon client drivers Add a maintainer for the PECI subsystem .../devicetree/bindings/hwmon/peci-cputemp.txt | 24 + .../devicetree/bindings/hwmon/peci-dimmtemp.txt| 25 + .../devicetree/bindings/peci/peci-adapter.txt | 23 + .../devicetree/bindings/peci/peci-aspeed.txt | 60 + .../devicetree/bindings/peci/peci-bus.txt | 15 + .../devicetree/bindings/peci/peci-client.txt | 25 + Documentation/hwmon/peci-cputemp | 88 ++ Documentation/hwmon/peci-dimmtemp | 50 + Documentation/ioctl/ioctl-number.txt |2 + MAINTAINERS| 10 + arch/arm/boot/dts/aspeed-g4.dtsi | 25 + arch/arm/boot/dts/aspeed-g5.dtsi | 25 + drivers/Kconfig|2 + drivers/Makefile |1 + drivers/hwmon/Kconfig | 28 + drivers/hwmon/Makefile |2 + drivers/hwmon/peci-cputemp.c | 783 drivers/hwmon/peci-dimmtemp.c | 432 +++ drivers/peci/Kconfig | 45 + drivers/peci/Makefile |9 + drivers/peci/peci-aspeed.c | 504 drivers/peci/peci-core.c | 1291 include/linux/peci.h | 107 ++ include/uapi/linux/peci-ioctl.h| 200 +++ 24 files changed, 3776 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-cputemp.txt create mode 100644 Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-adapter.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt create mode 100644 Documentation/devicet
[PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core
This commit adds driver implementation for PECI bus core into linux driver framework. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Signed-off-by: Fengguang Wu <fengguang...@intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- drivers/Kconfig |2 + drivers/Makefile|1 + drivers/peci/Kconfig| 17 + drivers/peci/Makefile |6 + drivers/peci/peci-core.c| 1291 +++ include/linux/peci.h| 107 include/uapi/linux/peci-ioctl.h | 200 ++ 7 files changed, 1624 insertions(+) create mode 100644 drivers/peci/Kconfig create mode 100644 drivers/peci/Makefile create mode 100644 drivers/peci/peci-core.c create mode 100644 include/linux/peci.h create mode 100644 include/uapi/linux/peci-ioctl.h diff --git a/drivers/Kconfig b/drivers/Kconfig index 95b9ccc08165..8c44d9738377 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -217,4 +217,6 @@ source "drivers/siox/Kconfig" source "drivers/slimbus/Kconfig" +source "drivers/peci/Kconfig" + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 24cd47014657..250fe3d0fa7e 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -185,3 +185,4 @@ obj-$(CONFIG_TEE) += tee/ obj-$(CONFIG_MULTIPLEXER) += mux/ obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/ obj-$(CONFIG_SIOX) += siox/ +obj-$(CONFIG_PECI) += peci/ diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig new file mode 100644 index ..1fbc13f9e6c2 --- /dev/null +++ b/drivers/peci/Kconfig @@ -0,0 +1,17 @@ +# +# Platform Environment Control Interface (PECI) subsystem configuration +# + +menu "PECI support" + +config PECI + bool "PECI support" + select RT_MUTEXES + select CRC8 + help + The Platform Environment Control Interface (PECI) is a one-wire bus + interface that provides a communication channel between Intel + processors and chipset components to external monitoring or control + devices. + +endmenu diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile new file mode 100644 index ..9e8615e0d3ff --- /dev/null +++ b/drivers/peci/Makefile @@ -0,0 +1,6 @@ +# +# Makefile for the PECI core and bus drivers. +# + +# Core functionality +obj-$(CONFIG_PECI) += peci-core.o diff --git a/drivers/peci/peci-core.c b/drivers/peci/peci-core.c new file mode 100644 index ..9b45869b7c39 --- /dev/null +++ b/drivers/peci/peci-core.c @@ -0,0 +1,1291 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +/* Device Specific Completion Code (CC) Definition */ +#define DEV_PECI_CC_SUCCESS 0x40 +#define DEV_PECI_CC_TIMEOUT 0x80 +#define DEV_PECI_CC_OUT_OF_RESOURCE 0x81 +#define DEV_PECI_CC_UNAVAIL_RESOURCE 0x82 +#define DEV_PECI_CC_INVALID_REQ 0x90 + +/* Completion Code mask to check retry needs */ +#define DEV_PECI_CC_RETRY_CHECK_MASK 0xf0 +#define DEV_PECI_CC_NEED_RETRY 0x80 + +/* Skylake EDS says to retry for 250ms */ +#define DEV_PECI_RETRY_TIME_MS 250 +#define DEV_PECI_RETRY_INTERVAL_MS 10 +#define DEV_PECI_RETRY_BIT 0x01 + +#define GET_TEMP_WR_LEN 1 +#define GET_TEMP_RD_LEN 2 +#define GET_TEMP_PECI_CMD 0x01 + +#define GET_DIB_WR_LEN 1 +#define GET_DIB_RD_LEN 8 +#define GET_DIB_PECI_CMD 0xf7 + +#define RDPKGCFG_WRITE_LEN 5 +#define RDPKGCFG_READ_LEN_BASE 1 +#define RDPKGCFG_PECI_CMD 0xa1 + +#define WRPKGCFG_WRITE_LEN_BASE 6 +#define WRPKGCFG_READ_LEN 1 +#define WRPKGCFG_PECI_CMD 0xa5 + +#define RDIAMSR_WRITE_LEN 5 +#define RDIAMSR_READ_LEN 9 +#define RDIAMSR_PECI_CMD 0xb1 + +#define WRIAMSR_PECI_CMD 0xb5 + +#define RDPCICFG_WRIT
[PATCH v3 02/10] Documentations: ioctl: Add ioctl numbers for PECI subsystem
This commit Updates ioctl-number.txt to reflect ioctl numbers being used by the PECI subsystem. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Haiyue Wang <haiyue.w...@linux.intel.com> Cc: James Feist <james.fe...@linux.intel.com> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> Cc: Vernon Mauery <vernon.mau...@linux.intel.com> --- Documentation/ioctl/ioctl-number.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt index 84bb74dcae12..4bc3a65d7204 100644 --- a/Documentation/ioctl/ioctl-number.txt +++ b/Documentation/ioctl/ioctl-number.txt @@ -323,6 +323,8 @@ Code Seq#(hex) Include FileComments 0xB3 00 linux/mmc/ioctl.h 0xB4 00-0F linux/gpio.h<mailto:linux-g...@vger.kernel.org> 0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remotep...@vger.kernel.org> +0xB6 00-0F uapi/linux/peci-ioctl.h PECI subsystem + <mailto:jae.hyun@linux.intel.com> 0xC0 00-0F linux/usb/iowarrior.h 0xCA 00-0F uapi/misc/cxl.h 0xCA 10-2F uapi/misc/ocxl.h -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/10] ARM: dts: aspeed: peci: Add PECI node
This commit adds PECI bus/adapter node of AST24xx/AST25xx into aspeed-g4 and aspeed-g5. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- arch/arm/boot/dts/aspeed-g4.dtsi | 25 + arch/arm/boot/dts/aspeed-g5.dtsi | 25 + 2 files changed, 50 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi index 518d2bc7c7fc..f7992eee4d1f 100644 --- a/arch/arm/boot/dts/aspeed-g4.dtsi +++ b/arch/arm/boot/dts/aspeed-g4.dtsi @@ -29,6 +29,7 @@ serial3 = serial4 = serial5 = + peci0 = }; cpus { @@ -270,6 +271,13 @@ }; }; + peci: peci@1e78b000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e78b000 0x60>; + }; + uart2: serial@1e78d000 { compatible = "ns16550a"; reg = <0x1e78d000 0x20>; @@ -313,6 +321,23 @@ }; }; + { + peci0: peci-bus@0 { + compatible = "aspeed,ast2400-peci"; + reg = <0x0 0x60>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <15>; + clocks = < ASPEED_CLK_GATE_REFCLK>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + status = "disabled"; + }; +}; + { i2c_ic: interrupt-controller@0 { #interrupt-cells = <1>; diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index f9917717dd08..278791dba8a0 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -29,6 +29,7 @@ serial3 = serial4 = serial5 = + peci0 = }; cpus { @@ -320,6 +321,13 @@ }; }; + peci: peci@1e78b000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e78b000 0x60>; + }; + uart2: serial@1e78d000 { compatible = "ns16550a"; reg = <0x1e78d000 0x20>; @@ -363,6 +371,23 @@ }; }; + { + peci0: peci-bus@0 { + compatible = "aspeed,ast2500-peci"; + reg = <0x0 0x60>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <15>; + clocks = < ASPEED_CLK_GATE_REFCLK>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + status = "disabled"; + }; +}; + { i2c_ic: interrupt-controller@0 { #interrupt-cells = <1>; -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/10] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx
This commit adds PECI adapter driver implementation for Aspeed AST24xx/AST25xx. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- drivers/peci/Kconfig | 28 +++ drivers/peci/Makefile | 3 + drivers/peci/peci-aspeed.c | 504 + 3 files changed, 535 insertions(+) create mode 100644 drivers/peci/peci-aspeed.c diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig index 1fbc13f9e6c2..0e33420365de 100644 --- a/drivers/peci/Kconfig +++ b/drivers/peci/Kconfig @@ -14,4 +14,32 @@ config PECI processors and chipset components to external monitoring or control devices. + If you want PECI support, you should say Y here and also to the + specific driver for your bus adapter(s) below. + +if PECI + +# +# PECI hardware bus configuration +# + +menu "PECI Hardware Bus support" + +config PECI_ASPEED + tristate "Aspeed AST24xx/AST25xx PECI support" + select REGMAP_MMIO + depends on OF + depends on ARCH_ASPEED || COMPILE_TEST + help + Say Y here if you want support for the Platform Environment Control + Interface (PECI) bus adapter driver on the Aspeed AST24XX and AST25XX + SoCs. + + This support is also available as a module. If so, the module + will be called peci-aspeed. + +endmenu + +endif # PECI + endmenu diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile index 9e8615e0d3ff..886285e69765 100644 --- a/drivers/peci/Makefile +++ b/drivers/peci/Makefile @@ -4,3 +4,6 @@ # Core functionality obj-$(CONFIG_PECI) += peci-core.o + +# Hardware specific bus drivers +obj-$(CONFIG_PECI_ASPEED) += peci-aspeed.o diff --git a/drivers/peci/peci-aspeed.c b/drivers/peci/peci-aspeed.c new file mode 100644 index ..be2a1f327eb1 --- /dev/null +++ b/drivers/peci/peci-aspeed.c @@ -0,0 +1,504 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2012-2017 ASPEED Technology Inc. +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DUMP_DEBUG 0 + +/* Aspeed PECI Registers */ +#define AST_PECI_CTRL 0x00 +#define AST_PECI_TIMING 0x04 +#define AST_PECI_CMD 0x08 +#define AST_PECI_CMD_CTRL 0x0c +#define AST_PECI_EXP_FCS 0x10 +#define AST_PECI_CAP_FCS 0x14 +#define AST_PECI_INT_CTRL 0x18 +#define AST_PECI_INT_STS 0x1c +#define AST_PECI_W_DATA0 0x20 +#define AST_PECI_W_DATA1 0x24 +#define AST_PECI_W_DATA2 0x28 +#define AST_PECI_W_DATA3 0x2c +#define AST_PECI_R_DATA0 0x30 +#define AST_PECI_R_DATA1 0x34 +#define AST_PECI_R_DATA2 0x38 +#define AST_PECI_R_DATA3 0x3c +#define AST_PECI_W_DATA4 0x40 +#define AST_PECI_W_DATA5 0x44 +#define AST_PECI_W_DATA6 0x48 +#define AST_PECI_W_DATA7 0x4c +#define AST_PECI_R_DATA4 0x50 +#define AST_PECI_R_DATA5 0x54 +#define AST_PECI_R_DATA6 0x58 +#define AST_PECI_R_DATA7 0x5c + +/* AST_PECI_CTRL - 0x00 : Control Register */ +#define PECI_CTRL_SAMPLING_MASK GENMASK(19, 16) +#define PECI_CTRL_SAMPLING(x) (((x) << 16) & PECI_CTRL_SAMPLING_MASK) +#define PECI_CTRL_SAMPLING_GET(x) (((x) & PECI_CTRL_SAMPLING_MASK) >> 16) +#define PECI_CTRL_READ_MODE_MASKGENMASK(13, 12) +#define PECI_CTRL_READ_MODE(x) (((x) << 12) & PECI_CTRL_READ_MODE_MASK) +#define PECI_CTRL_READ_MODE_GET(x) (((x) & PECI_CTRL_READ_MODE_MASK) >> 12) +#define PECI_CTRL_READ_MODE_COUNT BIT(12) +#define PECI_CTRL_READ_MODE_DBG BIT(13) +#define PECI_CTRL_CLK_SOURCE_MASK BIT(11) +#define PECI_CTRL_CLK_SOURCE(x) (((x) << 11) & PECI_CTRL_CLK_SOURCE_MASK) +#define PECI_CTRL_CLK_SOURCE_GET(x) (((x) & PECI_CTRL_CLK_SOURCE_MASK) >> 11) +#define PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8) +#define PECI_CTRL_CLK_DIV(x
[PATCH v3 08/10] Documentation: hwmon: Add documents for PECI hwmon client drivers
This commit adds hwmon documents for PECI cputemp and dimmtemp drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- Documentation/hwmon/peci-cputemp | 88 +++ Documentation/hwmon/peci-dimmtemp | 50 ++ 2 files changed, 138 insertions(+) create mode 100644 Documentation/hwmon/peci-cputemp create mode 100644 Documentation/hwmon/peci-dimmtemp diff --git a/Documentation/hwmon/peci-cputemp b/Documentation/hwmon/peci-cputemp new file mode 100644 index ..cdd5ea49a4a2 --- /dev/null +++ b/Documentation/hwmon/peci-cputemp @@ -0,0 +1,88 @@ +Kernel driver peci-cputemp +== + +Supported chips: + One of Intel server CPUs listed below which is connected to a PECI bus. + * Intel Xeon E5/E7 v3 server processors + Intel Xeon E5-14xx v3 family + Intel Xeon E5-24xx v3 family + Intel Xeon E5-16xx v3 family + Intel Xeon E5-26xx v3 family + Intel Xeon E5-46xx v3 family + Intel Xeon E7-48xx v3 family + Intel Xeon E7-88xx v3 family + * Intel Xeon E5/E7 v4 server processors + Intel Xeon E5-16xx v4 family + Intel Xeon E5-26xx v4 family + Intel Xeon E5-46xx v4 family + Intel Xeon E7-48xx v4 family + Intel Xeon E7-88xx v4 family + * Intel Xeon Scalable server processors + Intel Xeon Bronze family + Intel Xeon Silver family + Intel Xeon Gold family + Intel Xeon Platinum family + Addresses scanned: PECI client address 0x30 - 0x37 + Datasheet: Available from http://www.intel.com/design/literature.htm + +Author: + Jae Hyun Yoo <jae.hyun@linux.intel.com> + +Description +--- + +This driver implements a generic PECI hwmon feature which provides Digital +Thermal Sensor (DTS) thermal readings of the CPU package and CPU cores that are +accessible using the PECI Client Command Suite via the processor PECI client. + +All temperature values are given in millidegree Celsius and will be measurable +only when the target CPU is powered on. + +sysfs attributes + + +temp1_label"Die" +temp1_inputProvides current die temperature of the CPU package. +temp1_max Provides thermal control temperature of the CPU package + which is also known as Tcontrol. +temp1_crit Provides shutdown temperature of the CPU package which + is also known as the maximum processor junction + temperature, Tjmax or Tprochot. +temp1_crit_hystProvides the hysteresis value from Tcontrol to Tjmax of + the CPU package. + +temp2_label"DTS margin" +temp2_inputProvides current DTS thermal margin to Tcontrol of the + CPU package. Value 0 means it reaches to Tcontrol + temperature. Sub-zero value means the die temperature + goes across Tconrtol to Tjmax. +temp2_min Provides the minimum DTS thermal margin to Tcontrol of + the CPU package. +temp2_lcritProvides the value when the CPU package temperature + reaches to Tjmax. + +temp3_label"Tcontrol" +temp3_inputProvides current Tcontrol temperature of the CPU + package which is also known as Fan Temperature target. + Indicates the relative value from thermal monitor trip +
[PATCH v3 04/10] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../devicetree/bindings/peci/peci-aspeed.txt | 60 ++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt new file mode 100644 index ..4598bb8c20fa --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt @@ -0,0 +1,60 @@ +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs. + +Required properties: +- compatible: Should be "aspeed,ast2400-peci" or "aspeed,ast2500-peci" + - aspeed,ast2400-peci: Aspeed AST2400 family PECI +controller + - aspeed,ast2500-peci: Aspeed AST2500 family PECI +controller +- reg : Should contain PECI controller registers location and + length. +- #address-cells: Should be <1>. +- #size-cells : Should be <0>. +- interrupts: Should contain PECI controller interrupt. +- clocks: Should contain clock source for PECI controller. + Should reference clkin. +- clock_frequency : Should contain the operation frequency of PECI controller + in units of Hz. + 187500 ~ 2400 + +Optional properties: +- msg-timing-nego : Message timing negotiation period. This value will + determine the period of message timing negotiation to be + issued by PECI controller. The unit of the programmed + value is four times of PECI clock period. + 0 ~ 255 (default: 1) +- addr-timing-nego : Address timing negotiation period. This value will + determine the period of address timing negotiation to be + issued by PECI controller. The unit of the programmed + value is four times of PECI clock period. + 0 ~ 255 (default: 1) +- rd-sampling-point : Read sampling point selection. The whole period of a bit + time will be divided into 16 time frames. This value will + determine the time frame in which the controller will + sample PECI signal for data read back. Usually in the + middle of a bit time is the best. + 0 ~ 15 (default: 8) +- cmd_timeout_ms: Command timeout in units of ms. + 1 ~ 6 (default: 1000) + +Example: + 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 = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + }; + }; -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/10] Documentation: dt-bindings: Add documents for PECI hwmon client drivers
This commit adds dt-bindings documents for PECI cputemp and dimmtemp client drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- .../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 ..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) + +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 ..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 { + 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 linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
This commit adds PECI cputemp and dimmtemp hwmon drivers. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- drivers/hwmon/Kconfig | 28 ++ drivers/hwmon/Makefile| 2 + drivers/hwmon/peci-cputemp.c | 783 ++ drivers/hwmon/peci-dimmtemp.c | 432 +++ 4 files changed, 1245 insertions(+) create mode 100644 drivers/hwmon/peci-cputemp.c create mode 100644 drivers/hwmon/peci-dimmtemp.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index f249a4428458..c52f610f81d0 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1259,6 +1259,34 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_CPUTEMP + tristate "PECI CPU temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI + cputemp driver which provides Digital Thermal Sensor (DTS) thermal + readings of the CPU package and CPU cores that are accessible using + the PECI Client Command Suite via the processor PECI client. + Check Documentation/hwmon/peci-cputemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-cputemp. + +config SENSORS_PECI_DIMMTEMP + tristate "PECI DIMM temperature monitoring support" + depends on OF + depends on PECI + help + If you say yes here you get support for the generic Intel PECI hwmon + driver which provides Digital Thermal Sensor (DTS) thermal readings of + DIMM components that are accessible using the PECI Client Command + Suite via the processor PECI client. + Check Documentation/hwmon/peci-dimmtemp for details. + + This driver can also be built as a module. If so, the module + will be called peci-dimmtemp. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index e7d52a36e6c4..48d9598fcd3a 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -136,6 +136,8 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_CPUTEMP) += peci-cputemp.o +obj-$(CONFIG_SENSORS_PECI_DIMMTEMP)+= peci-dimmtemp.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-cputemp.c b/drivers/hwmon/peci-cputemp.c new file mode 100644 index ..f0bc92687512 --- /dev/null +++ b/drivers/hwmon/peci-cputemp.c @@ -0,0 +1,783 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include + +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ + +#define DEFAULT_CHANNEL_NUMS 5 +#define CORETEMP_CHANNEL_NUMS CORE_MAX_ON_SKX +#define CPUTEMP_CHANNEL_NUMS (DEFAULT_CHANNEL_NUMS + CORETEMP_CHANNEL_NUMS) + +#define CLIENT_CPU_ID_MASK0xf0ff0 /* Mask for Family / Model info */ + +#define UPDATE_INTERVAL_MIN HZ + +enum cpu_gens { + CPU_GEN_HSX, /* Haswell Xeon */ + CPU_GEN_BRX, /* Broadwell Xeon */ + CPU_GEN_SKX, /* Skylake Xeon
[PATCH v3 10/10] Add a maintainer for the PECI subsystem
This commit adds a maintainer information for the PECI subsystem. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> Reviewed-by: Haiyue Wang <haiyue.w...@linux.intel.com> Reviewed-by: James Feist <james.fe...@linux.intel.com> Reviewed-by: Vernon Mauery <vernon.mau...@linux.intel.com> Cc: Alan Cox <a...@linux.intel.com> Cc: Andrew Jeffery <and...@aj.id.au> Cc: Andrew Lunn <and...@lunn.ch> Cc: Andy Shevchenko <andriy.shevche...@intel.com> Cc: Arnd Bergmann <a...@arndb.de> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org> Cc: Fengguang Wu <fengguang...@intel.com> Cc: Greg KH <gre...@linuxfoundation.org> Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jason M Biils <jason.m.bi...@linux.intel.com> Cc: Jean Delvare <jdelv...@suse.com> Cc: Joel Stanley <j...@jms.id.au> Cc: Julia Cartwright <jul...@eso.teric.us> Cc: Miguel Ojeda <miguel.ojeda.sando...@gmail.com> Cc: Milton Miller II <milt...@us.ibm.com> Cc: Pavel Machek <pa...@ucw.cz> Cc: Randy Dunlap <rdun...@infradead.org> Cc: Stef van Os <stef.van...@prodrive-technologies.com> Cc: Sumeet R Pawnikar <sumeet.r.pawni...@intel.com> --- MAINTAINERS | 10 ++ 1 file changed, 10 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 5cd5ff0e4428..3e6917e1ad31 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10965,6 +10965,16 @@ L: platform-driver-...@vger.kernel.org S: Maintained F: drivers/platform/x86/peaq-wmi.c +PECI SUBSYSTEM +M: Jae Hyun Yoo <jae.hyun@linux.intel.com> +M: Jason M Biils <jason.m.bi...@linux.intel.com> +S: Maintained +F: Documentation/devicetree/bindings/peci/ +F: drivers/peci/ +F: drivers/hwmon/peci-*.c +F: include/linux/peci.h +F: include/uapi/linux/peci-ioctl.h + PER-CPU MEMORY ALLOCATOR M: Tejun Heo <t...@kernel.org> M: Christoph Lameter <c...@linux.com> -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
Hi Milton, Thanks for sharing your time to review this patch. Please see my answer inline. Jae On 3/9/2018 3:41 PM, Milton Miller II wrote: About 03/07/2018 04:12PM in some time zone, Pavel Machek wrote: Subject: Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs Hi! Are these SoCs x86-based? Yes, these are ARM SoCs. Please see Andrew's answer as well. Understood, thanks. + Read sampling point selection. The whole period of a bit time will be + divided into 16 time frames. This value will determine which time frame + this controller will sample PECI signal for data read back. Usually in + the middle of a bit time is the best. English? "This value will determine when this controller"? Could I change it like below?: "This value will determine in which time frame this controller samples PECI signal for data read back" I guess... I'm not native speaker, I guess this could be improved some more. I agree this wording is still confusing. The problem is that the key subject, the time of the sampling, is in the descriptive clause "in which time frame". "This value will determine the time frame in which the controller will sample" or perhaps phrase it as saving a specific sample from the over-clock, or a phase of the clock. Yes, that looks more better. I'll change the wording as you suggested. Thanks a lot! Jae Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html milton -- Speaking for myself not IBM. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
Hi Julia, Thanks for sharing your time on reviewing it. Please see my inline answers. Jae On 3/6/2018 7:19 PM, Julia Cartwright wrote: On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for PECI bus into linux driver framework. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- [..] +static int peci_locked_xfer(struct peci_adapter *adapter, + struct peci_xfer_msg *msg, + bool do_retry, + bool has_aw_fcs) _locked generally means that this function is invoked with some critical lock held, what lock does the caller need to acquire before invoking this function? I intended to show that this function has a mutex locking inside for serialization of PECI data transactions from multiple callers, but as you commented out below, the mutex protection scope should be adjusted to make that covers the peci_scan_cmd_mask() function too. I'll rewrite the mutex protection scope then this function will be in the locked scope. +{ + ktime_t start, end; + s64 elapsed_ms; + int rc = 0; + + if (!adapter->xfer) { Is this really an optional feature of an adapter? If this is not optional, then this check should be in place when the adapter is registered, not here. (And it should WARN_ON(), because it's a driver developer error). I agree with you. I'll move this code into the peci_register_adapter() function. + dev_dbg(>dev, "PECI level transfers not supported\n"); + return -ENODEV; + } + + if (in_atomic() || irqs_disabled()) { As Andrew mentioned, this is broken. You don't even need a might_sleep(). The locking functions you use here will already include a might_sleep() w/ CONFIG_DEBUG_ATOMIC_SLEEP. Thanks for letting me know that. I'll drop that checking code and might_sleep() too. + rt_mutex_trylock(>bus_lock); + if (!rc) + return -EAGAIN; /* PECI activity is ongoing */ + } else { + rt_mutex_lock(>bus_lock); + } + + if (do_retry) + start = ktime_get(); + + do { + rc = adapter->xfer(adapter, msg); + + if (!do_retry) + break; + + /* Per the PECI spec, need to retry commands that return 0x8x */ + if (!(!rc && ((msg->rx_buf[0] & DEV_PECI_CC_RETRY_ERR_MASK) == + DEV_PECI_CC_TIMEOUT))) + break; This is pretty difficult to parse. Can you split it into two different conditions? Sure. I'll split it out. + + /* Set the retry bit to indicate a retry attempt */ + msg->tx_buf[1] |= DEV_PECI_RETRY_BIT; Are you sure this bit is to be set in the _second_ byte of tx_buf? Yes, I'm pretty sure. The first byte contains a PECI command value and the second byte contains 'HostID[7:1] & Retry[0]' value. + + /* Recalculate the AW FCS if it has one */ + if (has_aw_fcs) + msg->tx_buf[msg->tx_len - 1] = 0x80 ^ + peci_aw_fcs((u8 *)msg, + 2 + msg->tx_len); + + /* Retry for at least 250ms before returning an error */ + end = ktime_get(); + elapsed_ms = ktime_to_ms(ktime_sub(end, start)); + if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) { + dev_dbg(>dev, "Timeout retrying xfer!\n"); + break; + } + } while (true); + + rt_mutex_unlock(>bus_lock); + + return rc; +} + +static int peci_xfer(struct peci_adapter *adapter, struct peci_xfer_msg *msg) +{ + return peci_locked_xfer(adapter, msg, false, false); +} + +static int peci_xfer_with_retries(struct peci_adapter *adapter, + struct peci_xfer_msg *msg, + bool has_aw_fcs) +{ + return peci_locked_xfer(adapter, msg, true, has_aw_fcs); +} + +static int peci_scan_cmd_mask(struct peci_adapter *adapter) +{ + struct peci_xfer_msg msg; + u32 dib; + int rc = 0; + + /* Update command mask just once */ + if (adapter->cmd_mask & BIT(PECI_CMD_PING)) + return 0; + + msg.addr = PECI_BASE_ADDR; + msg.tx_len= GET_DIB_WR_LEN; + msg.rx_len= GET_DIB_RD_LEN; + msg.tx_buf[0] = GET_DIB_PECI_CMD; + + rc = peci_xfer(adapter, ); + if (rc < 0) { + dev_dbg(>dev, "PECI xfer error, rc : %d\n", rc); + return rc; + } + + dib = msg.rx_buf[0] | (msg.rx_buf[1] << 8) | + (msg.rx_buf[2] << 16) | (msg.rx_buf[3] << 24); + +
Re: [PATCH v2 6/8] [PATCH 6/8] Documentation: hwmon: Add a document for PECI hwmon client driver
Hi Randy, On 3/6/2018 12:28 PM, Randy Dunlap wrote: Hi, On 02/21/2018 08:16 AM, Jae Hyun Yoo wrote: +temp_labelProvides DDR DIMM temperature if this label indicates + 'DIMM #'. +temp_inputProvides current temperature of the DDR DIMM. + +Note: + DIMM temperature group will be appeared when the client CPU's BIOS will appear when I'll fix this description as you suggested. Thanks a lot! Jae + completes memory training and testing. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] PECI device driver introduction
Hi Pavel, Please see my answer inline. On 3/6/2018 4:40 AM, Pavel Machek wrote: Hi! Introduction of the Platform Environment Control Interface (PECI) bus device driver. PECI is a one-wire bus interface that provides a communication channel between Intel processor and chipset components to external monitoring or control devices. PECI is designed to support the following sideband functions: * Processor and DRAM thermal management - Processor fan speed control is managed by comparing Digital Thermal Sensor (DTS) thermal readings acquired via PECI against the processor-specific fan speed control reference point, or TCONTROL. Both TCONTROL and DTS thermal readings are accessible via the processor PECI client. These variables are referenced to a common temperature, the TCC activation point, and are both defined as negative offsets from that reference. - PECI based access to the processor package configuration space provides a means for Baseboard Management Controllers (BMC) or other platform management devices to actively manage the processor and memory power and thermal features. * Platform Manageability - Platform manageability functions including thermal, power, and error monitoring. Note that platform 'power' management includes monitoring and control for both the processor and DRAM subsystem to assist with data center power limiting. - PECI allows read access to certain error registers in the processor MSR space and status monitoring registers in the PCI configuration space within the processor and downstream devices. - PECI permits writes to certain registers in the processor PCI configuration space. * Processor Interface Tuning and Diagnostics - Processor interface tuning and diagnostics capabilities (Intel(c) Interconnect BIST). The processors Intel(c) Interconnect Built In Self Test (Intel(c) IBIST) allows for infield diagnostic capabilities in the Intel UPI and memory controller interfaces. PECI provides a port to execute these diagnostics via its PCI Configuration read and write capabilities. * Failure Analysis - Output the state of the processor after a failure for analysis via Crashdump. PECI uses a single wire for self-clocking and data transfer. The bus requires no additional control lines. The physical layer is a self-clocked one-wire bus that begins each bit with a driven, rising edge from an idle level near zero volts. The duration of the signal driven high depends on whether the bit value is a logic '0' or logic '1'. PECI also includes variable data transfer rate established with every message. In this way, it is highly flexible even though underlying logic is simple. The interface design was optimized for interfacing to Intel processor and chipset components in both single processor and multiple processor environments. The single wire interface provides low board routing overhead for the multiple load connections in the congested routing area near the processor and chipset components. Bus speed, error checking, and low protocol overhead provides adequate link bandwidth and reliability to transfer critical device operating conditions and configuration information. This implementation provides the basic framework to add PECI extensions to the Linux bus and device models. A hardware specific 'Adapter' driver can be attached to the PECI bus to provide sideband functions described above. It is also possible to access all devices on an adapter from userspace through the /dev interface. A device specific 'Client' driver also can be attached to the PECI bus so each processor client's features can be supported by the 'Client' driver through an adapter connection in the bus. This patch set includes Aspeed 24xx/25xx PECI driver and a generic PECI hwmon driver as the first implementation for both adapter and client drivers on the PECI bus framework. Ok, how does this interact with ACPI/SMM BIOS/Secure mode code? Does Linux _need_ to control the fan? Or is SMM BIOS capable of doing all the work itself and Linux has just read-only access for monitoring purposes? This driver is not for local CPUs which this driver is running on. Instead, this driver will be running on BMC (Baseboard Management Controller) kernel which is separated from the server machine. In this implementation, it provides just read-only access for monitoring the server's CPU and DIMM temperatures remotely through a PECI connection. The BMC can control fans according to the monitoring data if the BMC has a fan control interface and feature, but it depends on baseboard hardware and software designs. Thanks, Jae Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More
Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
Hi Pavel, Thanks for sharing your time on reviewing it. Please see my answers inline. -Jae On 3/6/2018 4:40 AM, Pavel Machek wrote: Hi! Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- .../devicetree/bindings/peci/peci-aspeed.txt | 73 ++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt new file mode 100644 index ..8a86f346d550 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt @@ -0,0 +1,73 @@ +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs. Are these SoCs x86-based? Yes, these are ARM SoCs. Please see Andrew's answer as well. +Required properties: +- compatible + "aspeed,ast2400-peci" or "aspeed,ast2500-peci" + - aspeed,ast2400-peci: Aspeed AST2400 family PECI controller + - aspeed,ast2500-peci: Aspeed AST2500 family PECI controller + +- reg + Should contain PECI registers location and length. Other dts documents put it on one line, reg: Should contain ... +- clock_frequency + Should contain the operation frequency of PECI hardware module. + 187500 ~ 2400 specify this is Hz? I'll add a description. Thanks! +- rd-sampling-point + Read sampling point selection. The whole period of a bit time will be + divided into 16 time frames. This value will determine which time frame + this controller will sample PECI signal for data read back. Usually in + the middle of a bit time is the best. English? "This value will determine when this controller"? Could I change it like below?: "This value will determine in which time frame this controller samples PECI signal for data read back" + 0 ~ 15 (default: 8) + +- cmd_timeout_ms + Command timeout in units of ms. + 1 ~ 6 (default: 1000) + +Example: + 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 = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + }; + }; \ No newline at end of file -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On 2/23/2018 4:00 PM, Miguel Ojeda wrote: On Thu, Feb 22, 2018 at 2:29 AM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: On 2/21/2018 4:37 PM, Andrew Lunn wrote: But even with this change, it still needs to use delayed creation because BMC side kernel doesn't know how many DIMMs are populated on a remote server before the remote server completes its memory training and testing in BIOS, but it needs to check the remote server's CPU temperature as immediate as possible to make appropriate thermal control based on the remote CPU's temperature to avoid any critical thermal issue. What would be a better solution in this case? You could change this driver so that it supports one DIMM. Move the 'hotplug' part into another driver which creates and destroys instances of the hwmon DIMM device as the DIMMS come and go. Also, do you need to handle CPU hotplug? You could split the CPU temperature part into a separate hwmon driver? And again create and destroy devices as CPUs come and go? Andrew That seems like a possible option. I'll rewrite the hwmon driver again like that. Thanks for the good idea. :) By the way, in the rewrite, please try to avoid the create*workqueue() functions (they are deprecated :). Cheers, Miguel Hi Miguel, Thanks for letting me know that. I'll replace that with alloc_workqueue(). :) Regards, Jae Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] drivers/peci: peci_match_id() can be static
On 2/21/2018 11:01 PM, kbuild test robot wrote: Fixes: 99f5d2b99ecd ("drivers/peci: Add support for PECI bus driver core") Signed-off-by: Fengguang Wu--- peci-core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/peci/peci-core.c b/drivers/peci/peci-core.c index d976c73..4709b8c 100644 --- a/drivers/peci/peci-core.c +++ b/drivers/peci/peci-core.c @@ -770,8 +770,8 @@ peci_of_match_device(const struct of_device_id *matches, } #endif -const struct peci_device_id *peci_match_id(const struct peci_device_id *id, - struct peci_client *client) +static const struct peci_device_id *peci_match_id(const struct peci_device_id *id, + struct peci_client *client) { if (!(id && client)) return NULL; Hi Fengguang, Thanks a lot for the fix. I'll merge your patch in v3 submission. BR, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
On 2/21/2018 10:54 PM, Greg KH wrote: On Wed, Feb 21, 2018 at 12:42:30PM -0800, Jae Hyun Yoo wrote: On 2/21/2018 9:58 AM, Greg KH wrote: On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for PECI bus into linux driver framework. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- Why is there no other Intel developers willing to review and sign off on this patch? Please get their review first before asking us to do their work for them :) thanks, greg k-h Hi Greg, This patch set got our internal review process. Sorry if it's code quality is under your expectation but it's the reason why I'm asking you to review the code. Could you please share your time to review it? Nope. If no other Intel developer thinks it is good enough to put their name on it as part of their review process, why should I? Again, please use the resources you have, to fix the obvious problems in your code, BEFORE asking the community to do that work for you. greg k-h Okay. I'll take our internal review process again on this patch set and collect more credit tags before submitting v3. Thanks for your advice! Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On 2/21/2018 4:37 PM, Andrew Lunn wrote: But even with this change, it still needs to use delayed creation because BMC side kernel doesn't know how many DIMMs are populated on a remote server before the remote server completes its memory training and testing in BIOS, but it needs to check the remote server's CPU temperature as immediate as possible to make appropriate thermal control based on the remote CPU's temperature to avoid any critical thermal issue. What would be a better solution in this case? You could change this driver so that it supports one DIMM. Move the 'hotplug' part into another driver which creates and destroys instances of the hwmon DIMM device as the DIMMS come and go. Also, do you need to handle CPU hotplug? You could split the CPU temperature part into a separate hwmon driver? And again create and destroy devices as CPUs come and go? Andrew That seems like a possible option. I'll rewrite the hwmon driver again like that. Thanks for the good idea. :) Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
On 2/21/2018 1:48 PM, Guenter Roeck wrote: On Wed, Feb 21, 2018 at 01:24:48PM -0800, Jae Hyun Yoo wrote: Hi Guenter, Thanks for sharing your time to review this code. Please check my answers inline. On 2/21/2018 10:26 AM, Guenter Roeck wrote: On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote: This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_HWMON) += peci-hwmon.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg) +{ + return peci_command(priv->client->adapter, cmd, msg); +} + +static int need_update(struct temp_data *temp) +{ + if (temp->valid && + time_be
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
On 2/21/2018 1:51 PM, Andrew Lunn wrote: Is there a real need to do transfers in atomic context, or with interrupts disabled? Actually, no. Generally, this function will be called in sleep-able context so this code is for an exceptional case handling. I'll rewrite this code like below: if (in_atomic() || irqs_disabled()) { dev_dbg(>dev, "xfer in non-sleepable context is not supported\n"); return -EWOULDBLOCK; } I would not even do that. Just add a call to might_sleep(). CONFIG_DEBUG_ATOMIC_SLEEP will then find bad calls. Thanks for the suggestion. I've learned one thing. :) +static int peci_ioctl_get_temp(struct peci_adapter *adapter, void *vmsg) +{ + struct peci_get_temp_msg *umsg = vmsg; + struct peci_xfer_msg msg; + int rc; + Is this getting the temperature? Yes, this is getting the 'die' temperature of a processor package. So the hwmon driver provides this. No need to have both. This this common API in core driver of PECI bus. The hwmon is also uses it through peci_command call. +static long peci_ioctl(struct file *file, unsigned int iocmd, unsigned long arg) +{ + struct peci_adapter *adapter = file->private_data; + void __user *argp = (void __user *)arg; + unsigned int msg_len; + enum peci_cmd cmd; + u8 *msg; + int rc = 0; + + dev_dbg(>dev, "ioctl, cmd=0x%x, arg=0x%lx\n", iocmd, arg); + + switch (iocmd) { + case PECI_IOC_PING: + case PECI_IOC_GET_DIB: + case PECI_IOC_GET_TEMP: + case PECI_IOC_RD_PKG_CFG: + case PECI_IOC_WR_PKG_CFG: + case PECI_IOC_RD_IA_MSR: + case PECI_IOC_RD_PCI_CFG: + case PECI_IOC_RD_PCI_CFG_LOCAL: + case PECI_IOC_WR_PCI_CFG_LOCAL: + cmd = _IOC_TYPE(iocmd) - PECI_IOC_BASE; + msg_len = _IOC_SIZE(iocmd); + break; Adding new ioctl calls is pretty frowned up. Can you export this info via /sysfs? Most of these are not simple IOs so ioctl is better suited, I think. Lets see what other reviewers say, but i think ioctls are wrong. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
Hi Guenter, Thanks for sharing your time to review this code. Please check my answers inline. On 2/21/2018 10:26 AM, Guenter Roeck wrote: On Wed, Feb 21, 2018 at 08:16:05AM -0800, Jae Hyun Yoo wrote: This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_HWMON) += peci-hwmon.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg) +{ + return peci_command(priv->client->adapter, cmd, msg); +} + +static int need_update(struct temp_data *temp) +{ + if (temp->valid && + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN)) + return 0; + + return 1; +}
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
On 2/21/2018 9:58 AM, Greg KH wrote: On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for PECI bus into linux driver framework. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- Why is there no other Intel developers willing to review and sign off on this patch? Please get their review first before asking us to do their work for them :) thanks, greg k-h Hi Greg, This patch set got our internal review process. Sorry if it's code quality is under your expectation but it's the reason why I'm asking you to review the code. Could you please share your time to review it? Thanks a lot, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
On 2/21/2018 9:13 AM, Andrew Lunn wrote: On Wed, Feb 21, 2018 at 08:16:00AM -0800, Jae Hyun Yoo wrote: This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. Hi Jae It would be good to separate this into two. One binding document for a generic adaptor, with a generic PECI bus, and generic client devices. List all the properties you expect at the generic level. Then have an aspeed specific binding for those properties which are specific to the Aspeed adaptor. That makes sense. I'll add generic PECI bus/adapter/client and Aspeed specific documents as separated. Andrew Thanks again for sharing your time to review it. I really appreciate it. BR, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
Hi Andrew, Thanks for sharing your time to review it. Please check my answers inline. On 2/21/2018 9:04 AM, Andrew Lunn wrote: +static int peci_locked_xfer(struct peci_adapter *adapter, + struct peci_xfer_msg *msg, + bool do_retry, + bool has_aw_fcs) +{ + ktime_t start, end; + s64 elapsed_ms; + int rc = 0; + + if (!adapter->xfer) { + dev_dbg(>dev, "PECI level transfers not supported\n"); + return -ENODEV; + } + + if (in_atomic() || irqs_disabled()) { Hi Jae Is there a real need to do transfers in atomic context, or with interrupts disabled? Actually, no. Generally, this function will be called in sleep-able context so this code is for an exceptional case handling. I'll rewrite this code like below: if (in_atomic() || irqs_disabled()) { dev_dbg(>dev, "xfer in non-sleepable context is not supported\n"); return -EWOULDBLOCK; } And then, will add a sleep call into the below loop. I know that in_atomic() call is not recommended in driver code but some driver codes still use it since there is no alternative way at this time, AFAIK. Please tell me if there is a better solution. + rt_mutex_trylock(>bus_lock); + if (!rc) + return -EAGAIN; /* PECI activity is ongoing */ + } else { + rt_mutex_lock(>bus_lock); + } + + if (do_retry) + start = ktime_get(); + + do { + rc = adapter->xfer(adapter, msg); + + if (!do_retry) + break; + + /* Per the PECI spec, need to retry commands that return 0x8x */ + if (!(!rc && ((msg->rx_buf[0] & DEV_PECI_CC_RETRY_ERR_MASK) == + DEV_PECI_CC_TIMEOUT))) + break; + + /* Set the retry bit to indicate a retry attempt */ + msg->tx_buf[1] |= DEV_PECI_RETRY_BIT; + + /* Recalculate the AW FCS if it has one */ + if (has_aw_fcs) + msg->tx_buf[msg->tx_len - 1] = 0x80 ^ + peci_aw_fcs((u8 *)msg, + 2 + msg->tx_len); + + /* Retry for at least 250ms before returning an error */ + end = ktime_get(); + elapsed_ms = ktime_to_ms(ktime_sub(end, start)); + if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) { + dev_dbg(>dev, "Timeout retrying xfer!\n"); + break; + } + } while (true); So you busy loop to 1/4 second? How about putting a sleep in here so other things can be done between each retry. And should it not return -ETIMEDOUT after that 1/4 second? Yes, you are right. I'll rewrite this code like below after adding the above change: /** * Retry for at least 250ms before returning an error. * Retry interval guideline: * No minimum < Retry Interval < No maximum *(recommend 10ms) */ end = ktime_get(); elapsed_ms = ktime_to_ms(ktime_sub(end, start)); if (elapsed_ms >= DEV_PECI_RETRY_TIME_MS) { dev_dbg(>dev, "Timeout retrying xfer!\n"); rc = -ETIMEDOUT; break; } usleep_range(DEV_PECI_RETRY_INTERVAL_MS * 1000, (DEV_PECI_RETRY_INTERVAL_MS * 1000) + 1000); +static int peci_scan_cmd_mask(struct peci_adapter *adapter) +{ + struct peci_xfer_msg msg; + u32 dib; + int rc = 0; + + /* Update command mask just once */ + if (adapter->cmd_mask & BIT(PECI_CMD_PING)) + return 0; + + msg.addr = PECI_BASE_ADDR; + msg.tx_len= GET_DIB_WR_LEN; + msg.rx_len= GET_DIB_RD_LEN; + msg.tx_buf[0] = GET_DIB_PECI_CMD; + + rc = peci_xfer(adapter, ); + if (rc < 0) { + dev_dbg(>dev, "PECI xfer error, rc : %d\n", rc); + return rc; + } + + dib = msg.rx_buf[0] | (msg.rx_buf[1] << 8) | + (msg.rx_buf[2] << 16) | (msg.rx_buf[3] << 24); + + /* Check special case for Get DIB command */ + if (dib == 0x00) { + dev_dbg(>dev, "DIB read as 0x00\n"); + return -1; + } + + if (!rc) { + /** +* setting up the supporting commands based on minor rev# +* see PECI Spec Table 3-1 +*/ + dib = (dib >> 8) & 0xF; + + if (dib >= 0x1) { + adapter->cmd_mask |= BIT(PECI_CMD_RD_PKG_CFG); +
[PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
This commit adds driver implementation for PECI bus into linux driver framework. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/Kconfig |2 + drivers/Makefile|1 + drivers/peci/Kconfig| 20 + drivers/peci/Makefile |6 + drivers/peci/peci-core.c| 1337 +++ include/linux/peci.h| 97 +++ include/uapi/linux/peci-ioctl.h | 207 ++ 7 files changed, 1670 insertions(+) create mode 100644 drivers/peci/Kconfig create mode 100644 drivers/peci/Makefile create mode 100644 drivers/peci/peci-core.c create mode 100644 include/linux/peci.h create mode 100644 include/uapi/linux/peci-ioctl.h diff --git a/drivers/Kconfig b/drivers/Kconfig index 879dc0604cba..031bed5bbe7b 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -219,4 +219,6 @@ source "drivers/siox/Kconfig" source "drivers/slimbus/Kconfig" +source "drivers/peci/Kconfig" + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 24cd47014657..250fe3d0fa7e 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -185,3 +185,4 @@ obj-$(CONFIG_TEE) += tee/ obj-$(CONFIG_MULTIPLEXER) += mux/ obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/ obj-$(CONFIG_SIOX) += siox/ +obj-$(CONFIG_PECI) += peci/ diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig new file mode 100644 index ..1cd2cb4b2298 --- /dev/null +++ b/drivers/peci/Kconfig @@ -0,0 +1,20 @@ +# +# Platform Environment Control Interface (PECI) subsystem configuration +# + +menu "PECI support" + +config PECI + tristate "PECI support" + select RT_MUTEXES + select CRC8 + help + The Platform Environment Control Interface (PECI) is a one-wire bus + interface that provides a communication channel between Intel + processor and chipset components to external monitoring or control + devices. + + This PECI support can also be built as a module. If so, the module + will be called peci-core. + +endmenu diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile new file mode 100644 index ..9e8615e0d3ff --- /dev/null +++ b/drivers/peci/Makefile @@ -0,0 +1,6 @@ +# +# Makefile for the PECI core and bus drivers. +# + +# Core functionality +obj-$(CONFIG_PECI) += peci-core.o diff --git a/drivers/peci/peci-core.c b/drivers/peci/peci-core.c new file mode 100644 index ..d976c7317801 --- /dev/null +++ b/drivers/peci/peci-core.c @@ -0,0 +1,1337 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include + +/* Device Specific Completion Code (CC) Definition */ +#define DEV_PECI_CC_RETRY_ERR_MASK 0xf0 +#define DEV_PECI_CC_SUCCESS 0x40 +#define DEV_PECI_CC_TIMEOUT 0x80 +#define DEV_PECI_CC_OUT_OF_RESOURCE 0x81 +#define DEV_PECI_CC_INVALID_REQ 0x90 + +/* Skylake EDS says to retry for 250ms */ +#define DEV_PECI_RETRY_TIME_MS 250 +#define DEV_PECI_RETRY_BIT 0x01 + +#define GET_TEMP_WR_LEN 1 +#define GET_TEMP_RD_LEN 2 +#define GET_TEMP_PECI_CMD 0x01 + +#define GET_DIB_WR_LEN 1 +#define GET_DIB_RD_LEN 8 +#define GET_DIB_PECI_CMD 0xf7 + +#define RDPKGCFG_WRITE_LEN 5 +#define RDPKGCFG_READ_LEN_BASE 1 +#define RDPKGCFG_PECI_CMD 0xa1 + +#define WRPKGCFG_WRITE_LEN_BASE 6 +#define WRPKGCFG_READ_LEN 1 +#define WRPKGCFG_PECI_CMD 0xa5 + +#define RDIAMSR_WRITE_LEN 5 +#define RDIAMSR_READ_LEN 9 +#define RDIAMSR_PECI_CMD 0xb1 + +#define WRIAMSR_PECI_CMD 0xb5 + +#define RDPCICFG_WRITE_LEN 6 +#define RDPCICFG_READ_LEN 5 +#define RDPCICFG_PECI_CMD 0x61 + +#define WRPCICFG_PECI_CMD 0x65 + +#define RDPCICFGLOCAL_WRITE_LEN 5 +#define RDPCICFGLOCAL_READ_LEN_BASE 1 +#define RDPCICFGLOCAL_PECI_CMD 0xe1 + +#define WRPCICFGLOCAL_WRITE_LEN_BASE 6 +#define WRPCICFGLOCAL_READ_LEN 1 +#define WRPCICFGLOCAL_PECI_CMD 0xe5 + +/* CRC8 table for Assure Write Frame Check */ +#define PECI_CRC8_POLYNOMIAL 0x07 +DECLARE_CRC8_TABLE(peci_crc8_table); + +static struct device_type peci_adapter_type; +static struct device_type peci_client_type; + +#define PECI_CDEV_MAX 16 +static dev_t peci_devt; +static bool is_registered; + +static DEFINE_MUTEX(core_lock); +static DEFINE_IDR(peci_adapter_idr); + +static ssize_t name_show(struct device *dev, +struct device_attribute *attr, +char *buf) +{ + return sprintf(buf, "%s\n", dev->type == _client_type ? + to_peci_client(dev)->name : to_peci_adapter(dev)->name); +} +static DEVICE_ATTR_RO(name); + +static void peci_client_dev_release(struct device *dev) +{ + kfree(to_peci_client(dev)); +} + +static struct attribute *peci_device_attrs[] = { + _attr_name.attr, +
[PATCH v2 2/8] [PATCH 2/8] Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs
This commit adds a dt-bindings document of PECI adapter driver for Aspeed AST24xx/25xx SoCs. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- .../devicetree/bindings/peci/peci-aspeed.txt | 73 ++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt diff --git a/Documentation/devicetree/bindings/peci/peci-aspeed.txt b/Documentation/devicetree/bindings/peci/peci-aspeed.txt new file mode 100644 index ..8a86f346d550 --- /dev/null +++ b/Documentation/devicetree/bindings/peci/peci-aspeed.txt @@ -0,0 +1,73 @@ +Device tree configuration for PECI buses on the AST24XX and AST25XX SoCs. + +Required properties: +- compatible + "aspeed,ast2400-peci" or "aspeed,ast2500-peci" + - aspeed,ast2400-peci: Aspeed AST2400 family PECI controller + - aspeed,ast2500-peci: Aspeed AST2500 family PECI controller + +- reg + Should contain PECI registers location and length. + +- #address-cells + Should be <1>. + +- #size-cells + Should be <0>. + +- interrupts + Should contain PECI interrupt. + +- clocks + Should contain clock source for PECI hardware module. Should reference + clkin clock. + +- clock_frequency + Should contain the operation frequency of PECI hardware module. + 187500 ~ 2400 + +Optional properties: +- msg-timing-nego + Message timing negotiation period. This value will determine the period + of message timing negotiation to be issued by PECI controller. The unit + of the programmed value is four times of PECI clock period. + 0 ~ 255 (default: 1) + +- addr-timing-nego + Address timing negotiation period. This value will determine the period + of address timing negotiation to be issued by PECI controller. The unit + of the programmed value is four times of PECI clock period. + 0 ~ 255 (default: 1) + +- rd-sampling-point + Read sampling point selection. The whole period of a bit time will be + divided into 16 time frames. This value will determine which time frame + this controller will sample PECI signal for data read back. Usually in + the middle of a bit time is the best. + 0 ~ 15 (default: 8) + +- cmd_timeout_ms + Command timeout in units of ms. + 1 ~ 6 (default: 1000) + +Example: + 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 = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + }; + }; \ No newline at end of file -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/8] [PATCH 3/8] ARM: dts: aspeed: peci: Add PECI node
This commit adds PECI bus/adapter node of AST24xx/AST25xx into aspeed-g4 and aspeed-g5. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- arch/arm/boot/dts/aspeed-g4.dtsi | 25 + arch/arm/boot/dts/aspeed-g5.dtsi | 25 + 2 files changed, 50 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi index b0d8431a3700..077b4d6795b8 100644 --- a/arch/arm/boot/dts/aspeed-g4.dtsi +++ b/arch/arm/boot/dts/aspeed-g4.dtsi @@ -29,6 +29,7 @@ serial3 = serial4 = serial5 = + peci0 = }; cpus { @@ -250,6 +251,13 @@ }; }; + peci: peci@1e78b000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e78b000 0x60>; + }; + uart2: serial@1e78d000 { compatible = "ns16550a"; reg = <0x1e78d000 0x20>; @@ -290,6 +298,23 @@ }; }; + { + peci0: peci-bus@0 { + compatible = "aspeed,ast2400-peci"; + reg = <0x0 0x60>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <15>; + clocks = < ASPEED_CLK_GATE_REFCLK>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + status = "disabled"; + }; +}; + { i2c_ic: interrupt-controller@0 { #interrupt-cells = <1>; diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index 40de3b66c33f..5d3b5e177a32 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -29,6 +29,7 @@ serial3 = serial4 = serial5 = + peci0 = }; cpus { @@ -301,6 +302,13 @@ }; }; + peci: peci@1e78b000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e78b000 0x60>; + }; + uart2: serial@1e78d000 { compatible = "ns16550a"; reg = <0x1e78d000 0x20>; @@ -341,6 +349,23 @@ }; }; + { + peci0: peci-bus@0 { + compatible = "aspeed,ast2500-peci"; + reg = <0x0 0x60>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <15>; + clocks = < ASPEED_CLK_GATE_REFCLK>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + status = "disabled"; + }; +}; + { i2c_ic: interrupt-controller@0 { #interrupt-cells = <1>; -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/8] [PATCH 6/8] Documentation: hwmon: Add a document for PECI hwmon client driver
This commit adds a hwmon document for a generic PECI hwmon client driver. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- Documentation/hwmon/peci-hwmon | 73 ++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/hwmon/peci-hwmon diff --git a/Documentation/hwmon/peci-hwmon b/Documentation/hwmon/peci-hwmon new file mode 100644 index ..93e587498536 --- /dev/null +++ b/Documentation/hwmon/peci-hwmon @@ -0,0 +1,73 @@ +Kernel driver peci-hwmon +=== + +Supported chips: + Any recent Intel CPU which is connected through a PECI bus. + Addresses scanned: PECI client address 0x30 - 0x37 + Datasheet: Available from http://www.intel.com/design/literature.htm + +Author: + Jae Hyun Yoo <jae.hyun@linux.intel.com> + +Description +--- + +This driver implements a generic PECI hwmon feature which provides Digital +Thermal Sensor (DTS) thermal readings of the CPU package, CPU cores and DIMM +components that are accessible using the PECI Client Command Suite via the +processor PECI client. + +All temperature values are given in millidegree Celsius and will be measurable +only when the target CPU is powered on. + +sysfs attributes + + +temp1_inputProvides current die temperature of the CPU package. +temp1_max Provides thermal control temperature of the CPU package + which is also known as Tcontrol. +temp1_crit Provides shutdown temperature of the CPU package which + is also known as the maximum processor junction + temperature, Tjmax or Tprochot. +temp1_crit_hystProvides the hysteresis value from Tcontrol to Tjmax of + the CPU package. + +temp2_inputProvides current DTS thermal margin to Tcontrol of the + CPU package. Value 0 means it reaches to Tcontrol + temperature. Sub-zero value means the die temperature + goes across Tconrtol to Tjmax. +temp2_min Provides the minimum DTS thermal margin to Tcontrol of + the CPU package. +temp2_lcritProvides the value when the CPU package temperature + reaches to Tjmax. + +temp3_inputProvides current Tcontrol temperature of the CPU + package which is also known as Fan Temperature target. + Indicates the relative value from thermal monitor trip + temperature at which fans should be engaged. +temp3_crit Provides Tcontrol critical value of the CPU package + which is same to Tjmax. + +temp4_inputProvides current Tthrottle temperature of the CPU + package. Used for throttling temperature. If this value + is allowed and lower than Tjmax - the throttle will + occur and reported at lower than Tjmax. + +temp5_inputProvides the maximum junction temperature, Tjmax of the + CPU package. + +temp_label Provides core temperature if this label indicates + 'Core #'. +temp[n]_input Provides current temperature of each core. +temp[n]_maxProvides thermal control temperature of the core. +temp[n]_crit Provides shutdown temperature of the core. +temp[n]_crit_hyst Provides the hysteresis value from Tcontrol to Tjmax of + the core. + +temp_label Provides DDR DIMM temperature if this label indicates + 'DIMM #'. +temp_input Provides current temperature of the DDR DIMM. + +Note: + DIMM temperature group will be appeared when the client CPU's BIOS + completes memory training and testing. -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
This commit adds a generic PECI hwmon client driver implementation. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 928 + 3 files changed, 939 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ef23553ff5cb..f22e0c31f597 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on PECI + help + If you say yes here you get support for the generic PECI hwmon + driver. + + This driver can also be built as a module. If so, the module + will be called peci-hwmon. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index f814b4ace138..946f54b168e5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_HWMON) += peci-hwmon.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index ..edd27744adcb --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,928 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DIMM_SLOT_NUMS_MAX12 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX 28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI6 /* Sensor type 6: Intel PECI */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN 24 + +#define DEFAULT_ATTR_GRP_NUMS 5 + +#define UPDATE_INTERVAL_MIN HZ +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000) + +enum sign { + POS, + NEG +}; + +struct temp_data { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group { + struct temp_data tjmax; + struct temp_data tcontrol; + struct temp_data tthrottle; + struct temp_data dts_margin; + struct temp_data die; + struct temp_data core[CORE_NUMS_MAX]; + struct temp_data dimm[DIMM_SLOT_NUMS_MAX]; +}; + +struct core_temp_group { + struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS]; + char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct dimm_temp_group { + struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS]; + char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group; +}; + +struct peci_hwmon { + struct peci_client *client; + struct device *dev; + struct device *hwmon_dev; + struct workqueue_struct *work_queue; + struct delayed_work work_handler; + char name[PECI_NAME_SIZE]; + struct temp_group temp; + u8 addr; + uint cpu_no; + u32 core_mask; + u32 dimm_mask; + const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1]; + const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX + 1]; + uint global_idx; + uint core_idx; + uint dimm_idx; +}; + +enum label { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_TJMAX, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die\n", + "DTS margin to Tcontrol\n", + "Tcontrol\n", + "Tthrottle\n", + "Tjmax\n", +}; + +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, void *msg) +{ + return peci_command(priv->client->adapter, cmd, msg); +} + +static int need_update(struct temp_data *temp) +{ + if (temp->valid && + time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN)) + return 0; + + return 1; +} + +static s32 ten_dot_six_to_millidegree(s32 x) +{ + return x) ^ 0x8000) - 0x8000) * 1000 / 64); +} + +static int get_tjmax(struct peci_hwmon *priv) +{ + struct peci_rd_pkg_cfg
[PATCH v2 8/8] [PATCH 8/8] Add a maintainer for the PECI subsystem
This commit adds a maintainer information for the PECI subsystem. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 93a12af4f180..f9c302cbb76b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10830,6 +10830,15 @@ L: platform-driver-...@vger.kernel.org S: Maintained F: drivers/platform/x86/peaq-wmi.c +PECI SUBSYSTEM +M: Jae Hyun Yoo <jae.hyun@linux.intel.com> +S: Maintained +F: Documentation/devicetree/bindings/peci/ +F: drivers/peci/ +F: drivers/hwmon/peci-*.c +F: include/linux/peci.h +F: include/uapi/linux/peci-ioctl.h + PER-CPU MEMORY ALLOCATOR M: Tejun Heo <t...@kernel.org> M: Christoph Lameter <c...@linux.com> -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/8] PECI device driver introduction
ECI driver using ktime. - Added a function into hwmon driver to simplify update delay checking. - Added a function into hwmon driver to convert 10.6 to millidegree. - Dropped non-standard attributes in hwmon driver. - Fixed OF table for hwmon to make it indicate as a PECI client of Intel CPU target. - Added a maintainer of PECI subsystem into MAINTAINERS document. Thanks, -Jae Jae Hyun Yoo (8): drivers/peci: Add support for PECI bus driver core Documentations: dt-bindings: Add a document of PECI adapter driver for Aspeed AST24xx/25xx SoCs ARM: dts: aspeed: peci: Add PECI node drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx Documentation: dt-bindings: Add a document for PECI hwmon client driver Documentation: hwmon: Add a document for a PECI hwmon client driver drivers/hwmon: Add a generic PECI hwmon client driver Add a maintainer for the PECI subsystem .../devicetree/bindings/hwmon/peci-hwmon.txt | 27 + .../devicetree/bindings/peci/peci-aspeed.txt | 73 ++ Documentation/hwmon/peci-hwmon | 73 ++ MAINTAINERS|9 + arch/arm/boot/dts/aspeed-g4.dtsi | 25 + arch/arm/boot/dts/aspeed-g5.dtsi | 25 + drivers/Kconfig|2 + drivers/Makefile |1 + drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile |1 + drivers/hwmon/peci-hwmon.c | 928 ++ drivers/peci/Kconfig | 39 + drivers/peci/Makefile |9 + drivers/peci/peci-aspeed.c | 510 drivers/peci/peci-core.c | 1337 include/linux/peci.h | 97 ++ include/uapi/linux/peci-ioctl.h| 207 +++ 17 files changed, 3373 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-hwmon.txt create mode 100644 Documentation/devicetree/bindings/peci/peci-aspeed.txt create mode 100644 Documentation/hwmon/peci-hwmon create mode 100644 drivers/hwmon/peci-hwmon.c create mode 100644 drivers/peci/Kconfig create mode 100644 drivers/peci/Makefile create mode 100644 drivers/peci/peci-aspeed.c create mode 100644 drivers/peci/peci-core.c create mode 100644 include/linux/peci.h create mode 100644 include/uapi/linux/peci-ioctl.h -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/8] [PATCH 4/8] drivers/peci: Add a PECI adapter driver for Aspeed AST24xx/AST25xx
This commit adds PECI adapter driver implementation for Aspeed AST24xx/AST25xx. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/peci/Kconfig | 19 ++ drivers/peci/Makefile | 3 + drivers/peci/peci-aspeed.c | 510 + 3 files changed, 532 insertions(+) create mode 100644 drivers/peci/peci-aspeed.c diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig index 1cd2cb4b2298..f9875a0d0bc7 100644 --- a/drivers/peci/Kconfig +++ b/drivers/peci/Kconfig @@ -14,7 +14,26 @@ config PECI processor and chipset components to external monitoring or control devices. + If you want PECI support, you should say Y here and also to the + specific driver for your bus adapter(s) below. + This PECI support can also be built as a module. If so, the module will be called peci-core. +if PECI + +config PECI_ASPEED + tristate "Aspeed AST24xx/AST25xx PECI support" + select REGMAP_MMIO + depends on OF && (ARCH_ASPEED || COMPILE_TEST) + help + Say Y here if you want support for the Platform Environment Control + Interface (PECI) bus adapter driver on the Aspeed AST24XX and AST25XX + SoCs. + + This support is also available as a module. If so, the module + will be called peci-aspeed. + +endif # PECI + endmenu diff --git a/drivers/peci/Makefile b/drivers/peci/Makefile index 9e8615e0d3ff..886285e69765 100644 --- a/drivers/peci/Makefile +++ b/drivers/peci/Makefile @@ -4,3 +4,6 @@ # Core functionality obj-$(CONFIG_PECI) += peci-core.o + +# Hardware specific bus drivers +obj-$(CONFIG_PECI_ASPEED) += peci-aspeed.o diff --git a/drivers/peci/peci-aspeed.c b/drivers/peci/peci-aspeed.c new file mode 100644 index ..2b7800e96805 --- /dev/null +++ b/drivers/peci/peci-aspeed.c @@ -0,0 +1,510 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2012-2020 ASPEED Technology Inc. +// Copyright (c) 2018 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DUMP_DEBUG 0 + +/* Aspeed PECI Registers */ +#define AST_PECI_CTRL 0x00 +#define AST_PECI_TIMING 0x04 +#define AST_PECI_CMD 0x08 +#define AST_PECI_CMD_CTRL 0x0c +#define AST_PECI_EXP_FCS 0x10 +#define AST_PECI_CAP_FCS 0x14 +#define AST_PECI_INT_CTRL 0x18 +#define AST_PECI_INT_STS 0x1c +#define AST_PECI_W_DATA0 0x20 +#define AST_PECI_W_DATA1 0x24 +#define AST_PECI_W_DATA2 0x28 +#define AST_PECI_W_DATA3 0x2c +#define AST_PECI_R_DATA0 0x30 +#define AST_PECI_R_DATA1 0x34 +#define AST_PECI_R_DATA2 0x38 +#define AST_PECI_R_DATA3 0x3c +#define AST_PECI_W_DATA4 0x40 +#define AST_PECI_W_DATA5 0x44 +#define AST_PECI_W_DATA6 0x48 +#define AST_PECI_W_DATA7 0x4c +#define AST_PECI_R_DATA4 0x50 +#define AST_PECI_R_DATA5 0x54 +#define AST_PECI_R_DATA6 0x58 +#define AST_PECI_R_DATA7 0x5c + +/* AST_PECI_CTRL - 0x00 : Control Register */ +#define PECI_CTRL_SAMPLING_MASK GENMASK(19, 16) +#define PECI_CTRL_SAMPLING(x) (((x) << 16) & PECI_CTRL_SAMPLING_MASK) +#define PECI_CTRL_SAMPLING_GET(x) (((x) & PECI_CTRL_SAMPLING_MASK) >> 16) +#define PECI_CTRL_READ_MODE_MASKGENMASK(13, 12) +#define PECI_CTRL_READ_MODE(x) (((x) << 12) & PECI_CTRL_READ_MODE_MASK) +#define PECI_CTRL_READ_MODE_GET(x) (((x) & PECI_CTRL_READ_MODE_MASK) >> 12) +#define PECI_CTRL_READ_MODE_COUNT BIT(12) +#define PECI_CTRL_READ_MODE_DBG BIT(13) +#define PECI_CTRL_CLK_SOURCE_MASK BIT(11) +#define PECI_CTRL_CLK_SOURCE(x) (((x) << 11) & PECI_CTRL_CLK_SOURCE_MASK) +#define PECI_CTRL_CLK_SOURCE_GET(x) (((x) & PECI_CTRL_CLK_SOURCE_MASK) >> 11) +#define PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8) +#define PECI_CTRL_CLK_DIV(x)(((x) << 8) & PECI_CTRL_CLK_DIV_MASK) +#define PECI_CTRL_CLK_DIV_GET(x)(((x) & PECI_CTRL_CLK_DIV_MASK) >> 8) +#define PECI_CTRL_INVERT_OUTBIT(7) +#define PECI_CTRL_INVERT_IN BIT(6) +#define PECI_CTRL_BUS_CONTENT_ENBIT(5) +#define PECI_CTRL_PECI_EN BIT(4) +#define PECI_CTRL_PECI_CLK_EN BIT(0) + +/* AST_PECI_TIMING - 0x04 : Timing Negotiation Register */ +#define PECI_TIMING_MESSAGE_MASK GENMASK(15, 8) +#define PECI_TIMING_MESSAGE(x) (((x) << 8) & PECI_TIMING_MESSAGE_MASK) +#define PECI_TIMING_MESSAGE_GET(x) (((x) & PECI_TIMING_MESSAGE_MASK) >> 8) +#define PECI_TIMING_ADDRESS_MASK GENMASK(7, 0) +#define PECI_TIMING_ADDRESS(x) ((x) & PECI_TIMING_ADDRESS_MASK) +#define PECI_TIMING_ADDRESS_GET(x) ((x) & PECI_TIMING_ADDRESS_MASK) + +/* AST_PECI_CMD - 0x08 : Command Register */ +#define PECI_CMD_PIN_MONBIT(31) +#define PECI_CMD_STS_MASK GENMASK(27, 24) +#define PECI_CMD_STS_GET(x) (((x) & PECI_CMD_STS_MASK) >> 24) +#define PECI_CMD_FIRE BIT(0) + +/* AST_PECI_LEN - 0
[PATCH v2 5/8] [PATCH [5/8] Documentation: dt-bindings: Add a document for PECI hwmon client driver
This commit adds a dt-bindings document for a generic PECI hwmon client driver. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- .../devicetree/bindings/hwmon/peci-hwmon.txt | 27 ++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-hwmon.txt diff --git a/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt new file mode 100644 index ..831813158884 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt @@ -0,0 +1,27 @@ +Bindings for Intel PECI (Platform Environment Control Interface) hwmon driver. + +Required properties: +- compatible + Should be "intel,peci-hwmon". + +- 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-hwmon@cpu0 { + compatible = "intel,peci-hwmon"; + reg = <0x30>; + }; + + peci-hwmon@cpu1 { + compatible = "intel,peci-hwmon"; + reg = <0x31>; + }; + }; -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux, dev-4.10, 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon
On 1/11/2018 3:53 PM, Andrew Lunn wrote: On Thu, Jan 11, 2018 at 03:14:37PM -0800, Jae Hyun Yoo wrote: On 1/11/2018 2:18 PM, Andrew Lunn wrote: +static const struct of_device_id peci_of_table[] = { + { .compatible = "peci-hwmon", }, This does not look like a reference to some piece of hardware. This driver provides generic PECI hwmon function to which controller has PECI HW such as Aspeed or Nuvoton BMC chip so it's not dependant on a specific hardware. Should I remove this or any suggestion? PECI seems to be an Intel thing. So at least it should be { .compatible = "intel,peci-hwmon", } assuming it is actually compatible with the Intel specification. Andrew Yes, PECI is an Intel thing but this driver is running on an ARM kernel on Aspeed or Nuvoton chipsets for now. This driver will be monitoring a host server's Intel CPU and DIMM which is running on a separated OS. Hi Jae You need to be careful with the name then. You should not claim the name 'peci' in case somebody actually implements a PECI driver which is compatible with Intel PECI. However, looking at other comments, it seems like this part is going away, if you turn your code into a bus driver. Andrew Hi Andrew, I see. I'll keep that in mind and will keep finding if there is any better way. Thanks for sharing your thought. Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux, dev-4.10, 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon
On 1/11/2018 2:18 PM, Andrew Lunn wrote: +static const struct of_device_id peci_of_table[] = { + { .compatible = "peci-hwmon", }, This does not look like a reference to some piece of hardware. This driver provides generic PECI hwmon function to which controller has PECI HW such as Aspeed or Nuvoton BMC chip so it's not dependant on a specific hardware. Should I remove this or any suggestion? PECI seems to be an Intel thing. So at least it should be { .compatible = "intel,peci-hwmon", } assuming it is actually compatible with the Intel specification. Andrew Yes, PECI is an Intel thing but this driver is running on an ARM kernel on Aspeed or Nuvoton chipsets for now. This driver will be monitoring a host server's Intel CPU and DIMM which is running on a separated OS. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux, dev-4.10, 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon
On 1/11/2018 1:40 PM, Guenter Roeck wrote: On Thu, Jan 11, 2018 at 11:47:01AM -0800, Jae Hyun Yoo wrote: On 1/10/2018 1:47 PM, Guenter Roeck wrote: On Tue, Jan 09, 2018 at 02:31:26PM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for a generic PECI hwmon. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> [ ... ] + + if (priv->temp.tcontrol.valid && + time_before(jiffies, priv->temp.tcontrol.last_updated + +UPDATE_INTERVAL_MIN)) + return 0; + Is the delay necessary ? Otherwise I would suggest to drop it. It adds a lot of complexity to the driver. Also, if the user polls values more often, that is presumably on purpose. I was intended to reduce traffic on PECI bus because it's low speed single wired bus, and temperature values don't change frequently because the value is sampled and averaged in CPU itself. I'll keep this. Then please try to move the common code into a single function. That makes sense. I'll move the common code into a single function. [ ... ] + + rc = of_property_read_u32(np, "cpu-id", >cpu_id); What entity determines cpu-id ? CPU ID numbering is determined by hardware SOCKET_ID strap pins. In this driver implementation, cpu-id is being used as CPU client indexing. Seems to me the necessary information to identify a given CPU should be provided by the PECI core. Also, there are already "cpu" nodes in devicetree which, if I recall correctly, may include information such as CPU Ids. This driver is implemented to support only BMC (Baseboard Management Controllers) chipset which is running on a separated linux kernel from a host server system. Through a PECI connection between them, this driver collects the host server system's CPU and DIMM temperature information which is running on a separated OS. That could be a linux, a Windows OS or any other OSes. I mean, there is no shared devicetree data between them so it is why the 'cpu_id' was added into dt configuration of this driver. Using quite limited hardware connections such as PECI, KSC, eSPI and SMBus, the BMC manages the host server and this hwmon is one of features of BMC. + if (rc || priv->cpu_id >= CPU_ID_MAX) { + dev_err(dev, "Invalid cpu-id configuration\n"); + return rc; + } + + rc = of_property_read_u32(np, "dimm-nums", >dimm_nums); This is an odd devicetree attribute. Normally the number of DIMMs is dynamic. Isn't there a means to get all that information dynamically instead of having to set it through devicetree ? What if someone adds or removes a DIMM ? Who updates the devicetree ? It means the number of DIMM slots each CPU has, doesn't mean the number of currently installed DIMM components. If a DIMM is inserted a slot, CPU reports its actual temperature but on empty slot, CPU reports 0 instead of reporting an error so it is the reason why this driver enumerates all DIMM slots' attribute. And there is no other means to get the number of DIMM slots per CPU ? It just seems to be that this is the wrong location to provide such information. This devicetree attribute will be configured at runtime using dt overlay based on the host server's hardware configuration which will be parsed and managed by a userspace BMC service. [ ... ] + +static const struct of_device_id peci_of_table[] = { + { .compatible = "peci-hwmon", }, This does not look like a reference to some piece of hardware. This driver provides generic PECI hwmon function to which controller has PECI HW such as Aspeed or Nuvoton BMC chip so it's not dependant on a specific hardware. Should I remove this or any suggestion? I don't really know enough about the system to make a recommendation. It seems to me that the PECI core should identify which functionality it supports and instantiate the necessary driver(s). Maybe there should be sub-nodes to the peci node with relevant information. Those sub-nodes should specify the supported functionality in more detail, though - such as indicating the supported CPU and/or DIMM sensors. Guenter As I explained above, BMC and host server are running on separated OSes so this driver cannot be (actually, doesn't need to be) directly associated with other kernel modules in the BMC side OS for identifying the host server system's functionality. My thought is, this driver will use PECI only for identifying the host server's CPU and DIMM information and the userspace BMC service could deliver any additional host server information thru dt overlay if needed before the BMC service initiates this driver at runtime. Thanks a lot, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers
On 1/11/2018 1:06 AM, Benjamin Herrenschmidt wrote: On Tue, 2018-01-09 at 14:31 -0800, Jae Hyun Yoo wrote: +struct peci_rd_ia_msr_msg { + unsigned char target; + unsigned char thread_id; + unsigned short address; + unsigned long value; +}; Those types are representing messages on the wire ? In that case those types aren't suitable. For example "long" will have a different size and alignment for 32 and 64-bit userspace. There are size-explicit userspace types available. Also I didn't see any endianness annotations in there. Is that expected ? IE are those wire format packets ? Cheers, Ben. Only the 'peci_xfer_msg' struct is representing messages on the wire. All userspace messages which is using other struct definitions will be copied into the 'peci_xfer_msg' for each member variable in driver, but anyway, type definitions of each member variable should be fixed as you said. Will fix it. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers
On 1/11/2018 1:02 AM, Benjamin Herrenschmidt wrote: On Wed, 2018-01-10 at 11:18 +0100, Greg KH wrote: On Tue, Jan 09, 2018 at 02:31:23PM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for Aspeed PECI. Also adds generic peci.h and peci_ioctl.h files to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. We don't add code that could be used "sometime in the future". Only include stuff that we use now. Please fix up this series based on that and resubmit. There should not be any need for any uapi file then, right? No Greg, I think you misunderstood (unless I misread myself). What Jae means is that since PECI is a standard and other drivers implementing the same ioctl interface and messages will eventually go upstream, instead of having the ioctl definitions in a driver specific locations, they go in a generic spot, as they define a generic API for all PECI drivers, including the one that's getting merged now. IE. This doesn't add unused stuff, it just puts the API parts of it into a generic location. At least that's my understanding from a, granted cursory, look at the patch. That said, I do have a problem with the structure definitions of the various packet types as they use "long" which has a variable size and unclear alignment. It should be using __u8, __u16 and __u32... Cheers, Ben. Thanks for your clear explanation. That is what I actually intended to. However, the structure definitions you and Greg pointed out need to be corrected. I will fix it. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers
On 1/11/2018 12:56 AM, Benjamin Herrenschmidt wrote: On Thu, 2018-01-11 at 08:30 +0100, Greg KH wrote: 4.13? Why that kernel? It too is obsolete and insecure and unsupported. Haha, it's n-1. come on :-) What keeps you all from just always tracking the latest tree from Linus? What is in your tree that is not upstream that requires you to have a kernel tree at all? There are a couple of ARM based SoC families for which we are in the process of rewriting all the driver in upstreamable form. This takes time. To respond to your other email about the USB CDC, it's mine, I haven't resubmited it yet because it had a dependency on some the aspeed clk driver to function properly (so is unusable without it) and it took 2 kernel versions to get that clk stuff upstream for a number of reasons. So it's all getting upstream and eventually there will be (we hope) no "OpenBMC" kernel, it's just a way for us to get functional code with non-upstream-quality (read: vendor) drivers until we are one rewriting & upstreaming them all. And if you do have out-of-tree code, why not use a process that makes it trivial to update the base kernel version so that you can keep up to date very easily? (hint, just using 'git' is not a good way to do this...) Joel and I both find git perfectly fine for that. I've not touched quilt in eons and frankly don't regret it ;-) That said, Jae should definitely submit a driver against upstream, not against some random OpenBMC tree. Jae, for example when I submitted the original USB stuff back then, I did it from a local upstream based branch (with just a few hacks to work around the lack of the clk stuff). I will rebase it in the next few days to upstream merged with Stephen's clk tree to get the finally merged clk stuff, verify it works, and submit patches against upstream. There should be no mention of dev-4.10 or 4.13 on lkml or other upstream submission lists. Development work should happen upstream *first* and eventually be backported to our older kernels while they exist (hopefully I prefer if we are more aggressive at forward porting the crappy drivers so we can keep our tree more up to date but that's a different discussion). Cheers, Ben. Thanks for your reminding me the upstream process. I'll do like you said afterwards. Thanks, Jae thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux, dev-4.10, 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon
On 1/10/2018 1:47 PM, Guenter Roeck wrote: On Tue, Jan 09, 2018 at 02:31:26PM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for a generic PECI hwmon. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/hwmon/Kconfig | 6 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 953 + 3 files changed, 960 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 9256dd0..3a62c60 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1234,6 +1234,12 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on ASPEED_PECI + help + If you say yes here you get support for the generic PECI hwmon driver. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 98000fc..41d43a5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -131,6 +131,7 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_HWMON) += peci-hwmon.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index 000..2d2a288 --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,953 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2017 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include misc, not linux ? That seems wrong. You are right. I'll fix it. + +#define DEVICE_NAME "peci-hwmon" +#define HWMON_NAME "peci_hwmon" + +#define CPU_ID_MAX 8 /* Max CPU number configured by socket ID */ +#define DIMM_NUMS_MAX16 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX28 /* Max core numbers (max on SKX Platinum) */ I won't insist, but it would be better if this were dynamic, otherwise we'll end up having to increase the defines in the future. Right. As you said, these values should be manually adjusted in the future if CPU architecture has been changed so better implement it as dynamic. I will check again a way of getting these values from client CPU thru PECI connection. +#define TEMP_TYPE_PECI 6 /* Sensor type 6: Intel PECI */ +#define CORE_INDEX_OFFSET100 /* sysfs filename start offset for core temp */ +#define DIMM_INDEX_OFFSET200 /* sysfs filename start offset for DIMM temp */ Did you test with the "sensors" command to ensure that this works, with the large gaps in index values ? Overall, I am not very happy with the indexing. Since each sensor as a label, it might be better to just make it dynamic. Okay, that makes sense. Since all attributes has its own label, indexing gap wouldn't be needed even in case of CPU architecture change happens. I'll remove the indexing gap. +#define TEMP_NAME_HEADER_LEN 4 /* sysfs temp type header length */ +#define OF_DIMM_NUMS_DEFAULT 16 /* default dimm-nums setting */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN24 + +#define UPDATE_INTERVAL_MIN HZ + +enum sign_t { + POS, + NEG +}; + +struct cpuinfo_t { + bool valid; + u32 dib; + u8 cpuid; + u8 platform_id; + u32 microcode; + u8 logical_thread_nums; +}; + +struct temp_data_t { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group_t { + struct temp_data_t tjmax; + struct temp_data_t tcontrol; + struct temp_data_t tthrottle; + struct temp_data_t dts_margin; + struct temp_data_t die; + struct temp_data_t core[CORE_NUMS_MAX]; + struct temp_data_t dimm[DIMM_NUMS_MAX]; +}; + +struct core_temp_attr_group_t { + struct sensor_device_attribute sd_attrs[CORE_NUMS_MAX][CORE_TEMP_ATTRS]; + char attr_name[CORE_NUMS_MAX][CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_NUMS_MAX][CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group[CORE_NUMS_MAX]; +}; + +struct dimm_temp_attr_group_t { + struct sensor_device_attribute sd_attrs[DIMM_NUMS_MAX][DIMM_TEMP_ATTRS]; + char attr_name[DIMM_NUMS_MAX][DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_NUMS_MAX][DIMM_TEMP_ATTRS + 1]; + struct att
Re: [PATCH linux dev-4.10 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon
On 1/10/2018 4:29 AM, Arnd Bergmann wrote: On Tue, Jan 9, 2018 at 11:31 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit adds driver implementation for a generic PECI hwmon. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> +static int xfer_peci_msg(int cmd, void *pmsg) +{ + int rc; + + mutex_lock(_hwmon_lock); + rc = peci_ioctl(NULL, cmd, (unsigned long)pmsg); + mutex_unlock(_hwmon_lock); + + return rc; +} I said earlier that peci_ioctl() looked unused, that was obviously wrong, but what you have here is not a proper way to abstract a bus. Maybe this can be done more like an i2c bus: make the peci controller a bus device and register all known target/index pairs as devices with the peci bus type, and have them probed from DT. The driver can then bind to each of those individually. Not sure if that is getting to granular at that point, I'd have to understand better how it is expected to get used, and what the variances are between implementations. Arnd Thanks for sharing your opinion. In fact, this was also suggested by openbmc community so I should consider of redesigning it. I'm currently thinking about adding a new PECI device class as an abstract layer and any BMC chipset specific driver could be attached to the PECI class driver. Then, each CPU client could be registered as an individual device as you suggested. Will consider your suggestion. Thanks a lot! Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 4/6] Documentation: dt-bindings: Add a generic PECI hwmon
On 1/10/2018 4:20 AM, Arnd Bergmann wrote: On Tue, Jan 9, 2018 at 11:31 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit add a dt-bindings document for a generic PECI hwmon driver. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- .../devicetree/bindings/hwmon/peci-hwmon.txt | 33 ++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-hwmon.txt diff --git a/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt new file mode 100644 index 000..20b86f5 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt @@ -0,0 +1,33 @@ +* Generic PECI (Platform Environment Control Interface) hwmon driver. + +Dependency: +- This driver uses a PECI misc driver as a controller interface so one of PECI + misc drivers which provides compatible ioctls has to be enabled. The binding should only explain how we describe the hardware in an operating-system independent way, but not talk about how an OS is supposed to implement those drivers. Having multiple drivers each provide an exported function is not possible in Linux: it immediately breaks building an 'allyesconfig' kernel, and prevents you from running the same kernel across multiple implementations, so that has to be redesigned anyway. Agreed, I'll consider redesigning of it. +Required properties: +- compatible: "peci-hwmon" +- cpu-id: Should contain CPU socket ID + - 0 ~ 7 + +Optional properties: +- show-core: If this protperty is defined, core tmeperature attrubites will be s/protperty/property/ s/tmeperature/temperature/ s/attrubites/attributes/ Oops! I made this many typos in this single line. Thanks for your pointing it out. Will fix these. +enumerated. +- dimm-nums: Should contain the number of DIMM slots that attached to each CPU +which is indicated by cpu-id. + 0 ~ 16 (default: 16) +In case of 0, DIMM temperature attrubites will not be enumerated. Is this only an initial list that you expect to be extended in the future, or is this a complete list of sensors that can ever be connected to PECI? Should this be PECI version specific? Arnd The maximum supportable number of dimm slots is not PECI version specific but depends on CPU architecture. Currently IA supports up to 16 slot as the maximum but it could vary in the future architecture. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers
On 1/10/2018 3:55 AM, Arnd Bergmann wrote: On Tue, Jan 9, 2018 at 11:31 PM, Jae Hyun Yoo <jae.hyun@linux.intel.com> wrote: This commit adds driver implementation for Aspeed PECI. Also adds generic peci.h and peci_ioctl.h files to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include semaphore.h is not used here and can be dropped. You are right. Will drop it. +static struct aspeed_peci *aspeed_peci_priv; Try to avoid instance variables like this one. You should always be able to find that pointer from whatever structure you were called with. Okay. I will use driver_data instead. + timeout = wait_for_completion_interruptible_timeout( + >xfer_complete, + msecs_to_jiffies(priv->cmd_timeout_ms)); + + dev_dbg(priv->dev, "INT_STS : 0x%08x\n", priv->sts); + if (!regmap_read(priv->regmap, AST_PECI_CMD, _state)) + dev_dbg(priv->dev, "PECI_STATE : 0x%lx\n", + PECI_CMD_STS_GET(peci_state)); + else + dev_dbg(priv->dev, "PECI_STATE : read error\n"); + + if (timeout <= 0 || !(priv->sts & PECI_INT_CMD_DONE)) { + if (timeout <= 0) { + dev_dbg(priv->dev, "Timeout waiting for a response!\n"); + rc = -ETIME; + } else { + dev_dbg(priv->dev, "No valid response!\n"); + rc = -EFAULT; + } + return rc; + } You don't seem to handle -ERESTARTSYS correct here. Either do it right, or drop the _interruptible part above. Will add a handling logic for the -ERESTARTSYS. +typedef int (*ioctl_fn)(struct aspeed_peci *, void *); + +static ioctl_fn peci_ioctl_fn[PECI_CMD_MAX] = { + ioctl_xfer_msg, + ioctl_ping, + ioctl_get_dib, + ioctl_get_temp, + ioctl_rd_pkg_cfg, + ioctl_wr_pkg_cfg, + ioctl_rd_ia_msr, + NULL, /* Reserved */ + ioctl_rd_pci_cfg, + NULL, /* Reserved */ + ioctl_rd_pci_cfg_local, + ioctl_wr_pci_cfg_local, +}; + + +long peci_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +{ + struct aspeed_peci *priv; + long ret = 0; + void __user *argp = (void __user *)arg; + int timeout = PECI_IDLE_CHECK_TIMEOUT; + u8 msg[sizeof(struct peci_xfer_msg)]; + unsigned int peci_cmd, msg_size; + u32 cmd_sts; + + /* +* Treat it as an inter module call when filp is null but only in case +* the private data is initialized. +*/ + if (filp) + priv = container_of(filp->private_data, + struct aspeed_peci, miscdev); + else + priv = aspeed_peci_priv; Drop this. peci_ioctl is being called from peci_hwmon as an inter-module call so it is needed, but as you suggested in the other patch, I'll consider redesign it with adding a peci device class. + if (!priv) + return -ENXIO; + + switch (cmd) { + case PECI_IOC_XFER: + case PECI_IOC_PING: + case PECI_IOC_GET_DIB: + case PECI_IOC_GET_TEMP: + case PECI_IOC_RD_PKG_CFG: + case PECI_IOC_WR_PKG_CFG: + case PECI_IOC_RD_IA_MSR: + case PECI_IOC_RD_PCI_CFG: + case PECI_IOC_RD_PCI_CFG_LOCAL: + case PECI_IOC_WR_PCI_CFG_LOCAL: + peci_cmd = _IOC_TYPE(cmd) - PECI_IOC_BASE; + msg_size = _IOC_SIZE(cmd); + break; Having to keep the switch() statement and the array above seems a little fragile. Can you just do one or the other? Regarding the command set, you have both a low-level PECI_IOC_XFER interface and a high-level interface. Can you explain why? I'd think that generally speaking it's better to have only one of the two. I was intended to provide generic peci command set, also the low level PECI_IOC_XFER to provide flexibility for a case when compose a custom peci command which cannot be covered by the high-level command set. As you said, all other commands can be implemented in the upper layer but the benefit of when this driver has the implementation is, it's easy to manage retry logic since peci is retrial based protocol intends to do not disturb a CPU if the CPU is doing more important task. However, your thought also makes sense. I'll check the spec again whether the high-level command set can cover all cases. If so, I'll remove the low-level command. + /* Check command sts and bus idle state */ + while (!regmap_
Re: [PATCH linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers
On 1/10/2018 12:27 PM, Greg KH wrote: On Wed, Jan 10, 2018 at 11:30:05AM -0800, Jae Hyun Yoo wrote: On 1/10/2018 11:17 AM, Greg KH wrote: On Wed, Jan 10, 2018 at 11:14:34AM -0800, Jae Hyun Yoo wrote: On 1/10/2018 2:17 AM, Greg KH wrote: On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote: From: Jae Hyun Yoo <jae.hyun@intel.com> Hello, This patch set provides support for PECI of AST2400/2500 which can give us PECI functionalities such as temperature monitoring, platform manageability, processor diagnostics and failure analysis. Also provides generic peci.h and peci_ioctl.h headers to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. What is the "dev-4.10" in the subject for? 4.10 is really old and obsolete :( thanks, greg k-h I made this patch set on top of the v4.10 which OpenBmc project is currently using. I'll rebase this patch set onto the current kernel.org mainline. What is "OpenBmc", and why are they using an obsolete and insecure kernel for their project? That seems like a very foolish thing to do... thanks, greg k-h OpenBmc is an open source project to create a highly extensible framework for BMC (Board Management Controller) software for data-center computer systems: https://github.com/openbmc Its current mainline is v4.10 but it is being kept upgrading so it will be upgraded to the latest stable or long-term version soon. Why hasn't it been updated in the year since 4.10 was released? That's a _very_ long time to be running on a totally insecure kernel, and no new development should ever be done on old kernels, that's even crazier (as we can't go back in time and accept patches for new features to old releases...) Thanks for your pointing it out and I totally agree with you. Actually, we are preparing 4.13 update for now and an another update will be followed up. As I answered above, I'll rebase this patch set onto the latest kernel.org mainline. Sorry for my misunderstanding of upstream process. It sounds like the openbmc project needs to learn how to manage their kernels a whole lot better, who do I need to go poke about this? > thanks, greg k-h I've already cc'ed openbmc developers so they are also seeing this thread. Joel Stanley <j...@jms.id.au> is the openbmc kernel maintainer. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers
On 1/10/2018 2:20 AM, Greg KH wrote: On Tue, Jan 09, 2018 at 02:31:23PM -0800, Jae Hyun Yoo wrote: +#pragma pack(push, 1) +struct peci_xfer_msg { + unsigned char client_addr; + unsigned char tx_len; + unsigned char rx_len; + unsigned char tx_buf[MAX_BUFFER_SIZE]; + unsigned char rx_buf[MAX_BUFFER_SIZE]; +}; +#pragma pack(pop) For any structure that crosses the user/kernel boundry, you _HAVE_ to use the "__" variant. So for here you would use __u8 instead of "unsigned char" in order for things to work properly. I'm guessing you didn't test this all out on a mixed 32/64 bit system? Please fix up and test to ensure that it all works properly before resubmitting. thanks, greg k-h Thanks for your pointing it out. I'll fix this. Thanks a lot, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers
On 1/10/2018 2:18 AM, Greg KH wrote: On Tue, Jan 09, 2018 at 02:31:23PM -0800, Jae Hyun Yoo wrote: This commit adds driver implementation for Aspeed PECI. Also adds generic peci.h and peci_ioctl.h files to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. We don't add code that could be used "sometime in the future". Only include stuff that we use now. Please fix up this series based on that and resubmit. There should not be any need for any uapi file then, right? thanks, greg k-h These header files are being used in this patch set as well. I meant, these files also can be used for the future implementation to provide compatibility. I will update the commit message. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers
On 1/10/2018 11:17 AM, Greg KH wrote: On Wed, Jan 10, 2018 at 11:14:34AM -0800, Jae Hyun Yoo wrote: On 1/10/2018 2:17 AM, Greg KH wrote: On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote: From: Jae Hyun Yoo <jae.hyun@intel.com> Hello, This patch set provides support for PECI of AST2400/2500 which can give us PECI functionalities such as temperature monitoring, platform manageability, processor diagnostics and failure analysis. Also provides generic peci.h and peci_ioctl.h headers to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. What is the "dev-4.10" in the subject for? 4.10 is really old and obsolete :( thanks, greg k-h I made this patch set on top of the v4.10 which OpenBmc project is currently using. I'll rebase this patch set onto the current kernel.org mainline. What is "OpenBmc", and why are they using an obsolete and insecure kernel for their project? That seems like a very foolish thing to do... thanks, greg k-h OpenBmc is an open source project to create a highly extensible framework for BMC (Board Management Controller) software for data-center computer systems: https://github.com/openbmc Its current mainline is v4.10 but it is being kept upgrading so it will be upgraded to the latest stable or long-term version soon. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers
On 1/10/2018 2:17 AM, Greg KH wrote: On Tue, Jan 09, 2018 at 02:31:20PM -0800, Jae Hyun Yoo wrote: From: Jae Hyun Yoo <jae.hyun@intel.com> Hello, This patch set provides support for PECI of AST2400/2500 which can give us PECI functionalities such as temperature monitoring, platform manageability, processor diagnostics and failure analysis. Also provides generic peci.h and peci_ioctl.h headers to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. What is the "dev-4.10" in the subject for? 4.10 is really old and obsolete :( thanks, greg k-h I made this patch set on top of the v4.10 which OpenBmc project is currently using. I'll rebase this patch set onto the current kernel.org mainline. Thanks, Jae -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH linux dev-4.10 0/6] Add support PECI and PECI hwmon drivers
From: Jae Hyun Yoo <jae.hyun@intel.com> Hello, This patch set provides support for PECI of AST2400/2500 which can give us PECI functionalities such as temperature monitoring, platform manageability, processor diagnostics and failure analysis. Also provides generic peci.h and peci_ioctl.h headers to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. The misc peci driver can be used as a multi-purpose PECI controller driver which serializes all PECI transactions that coming from user space and from other kernel modules. This misc peci driver could be replaced with other BMC chipsets' implementation if the implementation provide compatible 'peci_ioctl' inter-module call and ioctl scheme defined in peci.h and peci_ioctl.h files. The hwmon peci driver implements a generic PECI hwmon feature which is running with a PECI misc driver supports compatible native PECI command suite for retrieving temperatures of the CPU package, CPU cores and DIMM components. Please review. -Jae Jae Hyun Yoo (6): Documentation: dt-bindings: Add Aspeed PECI ARM: dts: aspeed: peci: Add Aspeed PECI drivers/misc: Add driver for Aspeed PECI and generic PECI headers Documentation: dt-bindings: Add a generic PECI hwmon Documentation: hwmon: Add a generic PECI hwmon drivers/hwmon: Add a driver for a generic PECI hwmon .../devicetree/bindings/hwmon/peci-hwmon.txt | 33 + .../devicetree/bindings/misc/aspeed-peci.txt | 55 + Documentation/hwmon/peci-hwmon | 74 ++ arch/arm/boot/dts/aspeed-g4.dtsi | 14 + arch/arm/boot/dts/aspeed-g5.dtsi | 14 + drivers/hwmon/Kconfig |6 + drivers/hwmon/Makefile |1 + drivers/hwmon/peci-hwmon.c | 953 + drivers/misc/Kconfig |9 + drivers/misc/Makefile |1 + drivers/misc/aspeed-peci.c | 1130 include/misc/peci.h| 11 + include/uapi/linux/Kbuild |1 + include/uapi/linux/peci_ioctl.h| 270 + 14 files changed, 2572 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-hwmon.txt create mode 100644 Documentation/devicetree/bindings/misc/aspeed-peci.txt create mode 100644 Documentation/hwmon/peci-hwmon create mode 100644 drivers/hwmon/peci-hwmon.c create mode 100644 drivers/misc/aspeed-peci.c create mode 100644 include/misc/peci.h create mode 100644 include/uapi/linux/peci_ioctl.h -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers
This commit adds driver implementation for Aspeed PECI. Also adds generic peci.h and peci_ioctl.h files to provide compatibility to peci drivers that can be implemented later e.g. Nuvoton's BMC SoC family. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/misc/Kconfig|9 + drivers/misc/Makefile |1 + drivers/misc/aspeed-peci.c | 1130 +++ include/misc/peci.h | 11 + include/uapi/linux/Kbuild |1 + include/uapi/linux/peci_ioctl.h | 270 ++ 6 files changed, 1422 insertions(+) create mode 100644 drivers/misc/aspeed-peci.c create mode 100644 include/misc/peci.h create mode 100644 include/uapi/linux/peci_ioctl.h diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 02ffdd1..96e1e04 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -782,6 +782,15 @@ config ASPEED_LPC_SNOOP allows the BMC to listen on and save the data written by the host to an arbitrary LPC I/O port. +config ASPEED_PECI + tristate "Aspeed AST2400/AST2500 PECI support" + select CRC8 + select REGMAP_MMIO + depends on ARCH_ASPEED || COMPILE_TEST + help + Provides a driver for Platform Environment Control Interface (PECI) + controller on Aspeed AST2400/AST2500 SoC. + source "drivers/misc/c2port/Kconfig" source "drivers/misc/eeprom/Kconfig" source "drivers/misc/cb710/Kconfig" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index ab8af76..8a22455 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE)+= cxl/ obj-$(CONFIG_PANEL) += panel.o obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o +obj-$(CONFIG_ASPEED_PECI) += aspeed-peci.o lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o diff --git a/drivers/misc/aspeed-peci.c b/drivers/misc/aspeed-peci.c new file mode 100644 index 000..04fb794 --- /dev/null +++ b/drivers/misc/aspeed-peci.c @@ -0,0 +1,1130 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2012-2020 ASPEED Technology Inc. +// Copyright (c) 2017 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SOC_NAME "aspeed" +#define DEVICE_NAME "peci" + +#define DUMP_DEBUG 0 + +/* Aspeed PECI Registers */ +#define AST_PECI_CTRL 0x00 +#define AST_PECI_TIMING 0x04 +#define AST_PECI_CMD 0x08 +#define AST_PECI_CMD_CTRL 0x0c +#define AST_PECI_EXP_FCS 0x10 +#define AST_PECI_CAP_FCS 0x14 +#define AST_PECI_INT_CTRL 0x18 +#define AST_PECI_INT_STS 0x1c +#define AST_PECI_W_DATA0 0x20 +#define AST_PECI_W_DATA1 0x24 +#define AST_PECI_W_DATA2 0x28 +#define AST_PECI_W_DATA3 0x2c +#define AST_PECI_R_DATA0 0x30 +#define AST_PECI_R_DATA1 0x34 +#define AST_PECI_R_DATA2 0x38 +#define AST_PECI_R_DATA3 0x3c +#define AST_PECI_W_DATA4 0x40 +#define AST_PECI_W_DATA5 0x44 +#define AST_PECI_W_DATA6 0x48 +#define AST_PECI_W_DATA7 0x4c +#define AST_PECI_R_DATA4 0x50 +#define AST_PECI_R_DATA5 0x54 +#define AST_PECI_R_DATA6 0x58 +#define AST_PECI_R_DATA7 0x5c + +/* AST_PECI_CTRL - 0x00 : Control Register */ +#define PECI_CTRL_SAMPLING_MASK GENMASK(19, 16) +#define PECI_CTRL_SAMPLING(x) ((x << 16) & PECI_CTRL_SAMPLING_MASK) +#define PECI_CTRL_SAMPLING_GET(x) ((x & PECI_CTRL_SAMPLING_MASK) >> 16) +#define PECI_CTRL_READ_MODE_MASKGENMASK(13, 12) +#define PECI_CTRL_READ_MODE(x) ((x << 12) & PECI_CTRL_READ_MODE_MASK) +#define PECI_CTRL_READ_MODE_GET(x) ((x & PECI_CTRL_READ_MODE_MASK) >> 12) +#define PECI_CTRL_READ_MODE_COUNT BIT(12) +#define PECI_CTRL_READ_MODE_DBG BIT(13) +#define PECI_CTRL_CLK_SOURCE_MASK BIT(11) +#define PECI_CTRL_CLK_SOURCE(x) ((x << 11) & PECI_CTRL_CLK_SOURCE_MASK) +#define PECI_CTRL_CLK_SOURCE_GET(x) ((x & PECI_CTRL_CLK_SOURCE_MASK) >> 11) +#define PECI_CTRL_CLK_DIV_MASK GENMASK(10, 8) +#define PECI_CTRL_CLK_DIV(x)((x << 8) & PECI_CTRL_CLK_DIV_MASK) +#define PECI_CTRL_CLK_DIV_GET(x)((x & PECI_CTRL_CLK_DIV_MASK) >> 8) +#define PECI_CTRL_INVERT_OUTBIT(7) +#define PECI_CTRL_INVERT_IN BIT(6) +#define PECI_CTRL_BUS_CONTENT_ENBIT(5) +#define PECI_CTRL_PECI_EN BIT(4) +#define PECI_CTRL_PECI_CLK_EN BIT(0) + +/* AST_PECI_TIMING - 0x04 : Timing Negotiation Register */ +#define PECI_TIMING_MESSAGE_MASK GENMASK(15, 8) +#define PECI_TIMING_MESSAGE(x) ((x << 8) & PECI_TIMING_MESSAGE_MASK) +#define PECI_TIMING_MESSAGE_GET(x) ((x & PECI_TIMING_MESSAGE_MASK) >> 8) +#define PECI_TIMING_ADDRESS_MASK GENMASK(7, 0) +#d
[PATCH linux dev-4.10 1/6] Documentation: dt-bindings: Add Aspeed PECI
This commit adds a dt-bindings document for Aspeed PECI. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- .../devicetree/bindings/misc/aspeed-peci.txt | 55 ++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/aspeed-peci.txt diff --git a/Documentation/devicetree/bindings/misc/aspeed-peci.txt b/Documentation/devicetree/bindings/misc/aspeed-peci.txt new file mode 100644 index 000..d277c73 --- /dev/null +++ b/Documentation/devicetree/bindings/misc/aspeed-peci.txt @@ -0,0 +1,55 @@ +* ASPEED PECI (Platform Environment Control Interface) misc driver. + +Hardware Interfaces: +- This driver implements support for the ASPEED AST2400/2500 PECI which has the + following features: + - Directly connected to APB bus + - Intel PECI 3.1 compliant (PECI 3.0 for AST2400) + - Maximum packet length is 256 bytes (Baseline transmission unit) + - Support up to 8 CPUs and 2 domains per CPU + - Integrate PECI compliant I/O buffers, can connect to PECI bus directly + - Transmit buffer 32 bytes and receive buffer 32 bytes + +Required properties: +- compatible: "aspeed,ast2400-peci" or "aspeed,ast2500-peci" + - aspeed,ast2400-peci: Aspeed AST2400 family PECI control interface + - aspeed,ast2500-peci: Aspeed AST2500 family PECI control interface +- reg: Should contain PECI registers location and length +- interrupts: Should contain PECI interrupt +- clocks: Should contain clock source. = <_clkin>; +- clock_frequency: Should contain the operation frequency of PECI controller. + 187500 ~ 2400 + +Optional properties: +- msg-timing-nego: Message timing negotiation period. + This value will determine the period of message timing negotiation to be + issued by PECI controller. The unit of the programmed value is four + times of PECI clock period. + 0 ~ 255 (default: 1) +- addr-timing-nego: Address timing negotiation period. + This value will determine the period of address timing negotiation to be + issued by PECI controller. The unit of the programmed value is four + times of PECI clock period. + 0 ~ 255 (default: 1) +- rd-sampling-point: Read sampling point selection. + The whole period of a bit time will be divided into 16 time frames. + This value will determine which time frame this controller will sample + PECI signal for data read back. Usually in the middle of a bit time is + the best. + 0 ~ 15 (default: 8) +- cmd_timeout_ms: Command timeout in units of ms + 1 ~ 6 (default: 1000) + +Example: + peci: peci@1e78b000 { + compatible = "aspeed,ast2500-peci"; + reg = <0x1e78b000 0x60>; + interrupt-controller; + interrupts = <15>; + clocks = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + }; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH linux dev-4.10 5/6] Documentation: hwmon: Add a generic PECI hwmon
From: Jae Hyun Yoo <jae.hyun@intel.com> This commit add a document for a generic PECI hwmon driver. Signed-off-by: Jae Hyun Yoo <jae.hyun@intel.com> --- Documentation/hwmon/peci-hwmon | 74 ++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/hwmon/peci-hwmon diff --git a/Documentation/hwmon/peci-hwmon b/Documentation/hwmon/peci-hwmon new file mode 100644 index 000..e0155b5 --- /dev/null +++ b/Documentation/hwmon/peci-hwmon @@ -0,0 +1,74 @@ +Kernel driver peci-hwmon +=== + +Supported chips: + Generic BMC chips provide PECI controller + +Author: + Jae Hyun Yoo <jae.hyun@linux.intel.com> + + +Hardware Interfaces +--- + +This driver uses a PECI misc driver as a controller interface so one of PECI +misc drivers which provides compatible ioctls has to be enabled. + + +Description +--- + +This driver implements a generic PECI hwmon feature which is running with a PECI +controller driver supports native PECI Client Command Suite for retrieving +temperatures of the CPU package, CPU cores and DIMM components. + +All temperature values are given in millidegree Celsius and will be measurable +only when the target CPU is powered on. + + +sysfs files +--- + +temp1_inputProvides current die temperature of the CPU package. +temp1_max Provides thermal control temperature of the CPU package + which is also known as Tcontrol. +temp1_crit Provides shutdown temperature of the CPU package which + is also known as the maximum processor junction + temperature, Tjmax or Tprochot. +temp1_crit_hystProvides the hysteresis value from Tcontrol to Tjmax of + the CPU package. + +temp2_inputProvides current DTS thermal margin to Tcontrol of the + CPU package. Value 0 means it reaches to Tcontrol + temperature. Sub-zero value means the die temperature + goes across Tconrtol to Tjmax. +temp2_min Provides the minimum DTS thermal margin to Tcontrol of + the CPU package. +temp2_lcritProvides the value when the CPU package temperature + reaches to Tjmax. + +temp3_inputProvides current Tcontrol temperature of the CPU + package which is also known as Fan Temperature target. + Indicates the relative value from thermal monitor trip + temperature at which fans should be engaged. +temp3_crit Provides Tcontrol critical value of the CPU package + which is same to Tjmax. + +temp4_inputProvides current Tthrottle temperature of the CPU + package. Used for throttling temperature. If this value + is allowed and lower than Tjmax - the throttle will + occur and reported at lower than Tjmax. + +temp[100-127]_inputProvides current core temperature. +temp[100-127]_max Provides thermal control temperature of the core. +temp[100-127]_crit Provides shutdown temperature of the core. +temp[100-127]_crit_hystProvides the hysteresis value from Tcontrol to Tjmax of + the core. + +Note: + Core temperature group will be appeared when probing the driver if CPU + is online or when the first reading on other attr happens because it + needs cpu info reading. The number of generated core attrs depends on + the number of cores of the cpu package. + +temp[200-215]_inputProvides current temperature of the DDR DIMM. -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH linux dev-4.10 4/6] Documentation: dt-bindings: Add a generic PECI hwmon
This commit add a dt-bindings document for a generic PECI hwmon driver. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- .../devicetree/bindings/hwmon/peci-hwmon.txt | 33 ++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/peci-hwmon.txt diff --git a/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt new file mode 100644 index 000..20b86f5 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/peci-hwmon.txt @@ -0,0 +1,33 @@ +* Generic PECI (Platform Environment Control Interface) hwmon driver. + +Dependency: +- This driver uses a PECI misc driver as a controller interface so one of PECI + misc drivers which provides compatible ioctls has to be enabled. + +Required properties: +- compatible: "peci-hwmon" +- cpu-id: Should contain CPU socket ID + - 0 ~ 7 + +Optional properties: +- show-core: If this protperty is defined, core tmeperature attrubites will be +enumerated. +- dimm-nums: Should contain the number of DIMM slots that attached to each CPU +which is indicated by cpu-id. + 0 ~ 16 (default: 16) +In case of 0, DIMM temperature attrubites will not be enumerated. + +Example: + peci-hwmon0 { + compatible = "peci-hwmon"; + cpu-id = <0>; + show-core; + dimm-nums = <16>; + }; + + peci-hwmon1 { + compatible = "peci-hwmon"; + cpu-id = <1>; + show-core; + dimm-nums = <16>; + }; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH linux dev-4.10 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon
This commit adds driver implementation for a generic PECI hwmon. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- drivers/hwmon/Kconfig | 6 + drivers/hwmon/Makefile | 1 + drivers/hwmon/peci-hwmon.c | 953 + 3 files changed, 960 insertions(+) create mode 100644 drivers/hwmon/peci-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 9256dd0..3a62c60 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1234,6 +1234,12 @@ config SENSORS_NCT7904 This driver can also be built as a module. If so, the module will be called nct7904. +config SENSORS_PECI_HWMON + tristate "PECI hwmon support" + depends on ASPEED_PECI + help + If you say yes here you get support for the generic PECI hwmon driver. + config SENSORS_NSA320 tristate "ZyXEL NSA320 and compatible fan speed and temperature sensors" depends on GPIOLIB && OF diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 98000fc..41d43a5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -131,6 +131,7 @@ obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o obj-$(CONFIG_SENSORS_NCT7904) += nct7904.o obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o +obj-$(CONFIG_SENSORS_PECI_HWMON) += peci-hwmon.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c new file mode 100644 index 000..2d2a288 --- /dev/null +++ b/drivers/hwmon/peci-hwmon.c @@ -0,0 +1,953 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2017 Intel Corporation + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DEVICE_NAME "peci-hwmon" +#define HWMON_NAME "peci_hwmon" + +#define CPU_ID_MAX 8 /* Max CPU number configured by socket ID */ +#define DIMM_NUMS_MAX16 /* Max DIMM numbers (channel ranks x 2) */ +#define CORE_NUMS_MAX28 /* Max core numbers (max on SKX Platinum) */ +#define TEMP_TYPE_PECI 6 /* Sensor type 6: Intel PECI */ +#define CORE_INDEX_OFFSET100 /* sysfs filename start offset for core temp */ +#define DIMM_INDEX_OFFSET200 /* sysfs filename start offset for DIMM temp */ +#define TEMP_NAME_HEADER_LEN 4 /* sysfs temp type header length */ +#define OF_DIMM_NUMS_DEFAULT 16 /* default dimm-nums setting */ + +#define CORE_TEMP_ATTRS 5 +#define DIMM_TEMP_ATTRS 2 +#define ATTR_NAME_LEN24 + +#define UPDATE_INTERVAL_MIN HZ + +enum sign_t { + POS, + NEG +}; + +struct cpuinfo_t { + bool valid; + u32 dib; + u8 cpuid; + u8 platform_id; + u32 microcode; + u8 logical_thread_nums; +}; + +struct temp_data_t { + bool valid; + s32 value; + unsigned long last_updated; +}; + +struct temp_group_t { + struct temp_data_t tjmax; + struct temp_data_t tcontrol; + struct temp_data_t tthrottle; + struct temp_data_t dts_margin; + struct temp_data_t die; + struct temp_data_t core[CORE_NUMS_MAX]; + struct temp_data_t dimm[DIMM_NUMS_MAX]; +}; + +struct core_temp_attr_group_t { + struct sensor_device_attribute sd_attrs[CORE_NUMS_MAX][CORE_TEMP_ATTRS]; + char attr_name[CORE_NUMS_MAX][CORE_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[CORE_NUMS_MAX][CORE_TEMP_ATTRS + 1]; + struct attribute_group attr_group[CORE_NUMS_MAX]; +}; + +struct dimm_temp_attr_group_t { + struct sensor_device_attribute sd_attrs[DIMM_NUMS_MAX][DIMM_TEMP_ATTRS]; + char attr_name[DIMM_NUMS_MAX][DIMM_TEMP_ATTRS][ATTR_NAME_LEN]; + struct attribute *attrs[DIMM_NUMS_MAX][DIMM_TEMP_ATTRS + 1]; + struct attribute_group attr_group[DIMM_NUMS_MAX]; +}; + +struct peci_hwmon { + struct device *dev; + struct device *hwmon_dev; + char name[NAME_MAX]; + const struct attribute_group **groups; + struct cpuinfo_t cpuinfo; + struct temp_group_t temp; + u32 cpu_id; + bool show_core; + u32 core_nums; + u32 dimm_nums; + atomic_t core_group_created; + struct core_temp_attr_group_t core; + struct dimm_temp_attr_group_t dimm; +}; + +enum label_t { + L_DIE, + L_DTS, + L_TCONTROL, + L_TTHROTTLE, + L_MAX +}; + +static const char *peci_label[L_MAX] = { + "Die temperature\n", + "DTS thermal margin to Tcontrol\n", + "Tcontrol temperature\n", + "Tthrottle temperature\n", +}; + +static DEFINE_MUTEX(peci_hwmon_lock); + +static int create_core_temp_group(struct peci_hwmon *priv, int core_no); + + +static int xfer_peci_msg(int cmd,
[PATCH linux dev-4.10 2/6] ARM: dts: aspeed: peci: Add Aspeed PECI
This commit adds Aspeed PECI node into aspeed-g4 and aspeed-g5. Signed-off-by: Jae Hyun Yoo <jae.hyun@linux.intel.com> --- arch/arm/boot/dts/aspeed-g4.dtsi | 14 ++ arch/arm/boot/dts/aspeed-g5.dtsi | 14 ++ 2 files changed, 28 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi index b82ebef..7ecc7b2 100644 --- a/arch/arm/boot/dts/aspeed-g4.dtsi +++ b/arch/arm/boot/dts/aspeed-g4.dtsi @@ -238,6 +238,20 @@ clocks = <_hpll>; }; + peci: peci@1e78b000 { + compatible = "aspeed,ast2400-peci"; + reg = <0x1e78b000 0x60>; + interrupt-controller; + interrupts = <15>; + clocks = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + status = "disabled"; + }; + sgpio: gpio@0x1e780200 { #gpio-cells = <2>; gpio-controller; diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index ba3607c..b4e8d51 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -289,6 +289,20 @@ clocks = <_hpll>; }; + peci: peci@1e78b000 { + compatible = "aspeed,ast2500-peci"; + reg = <0x1e78b000 0x60>; + interrupt-controller; + interrupts = <15>; + clocks = <_clkin>; + clock-frequency = <2400>; + msg-timing-nego = <1>; + addr-timing-nego = <1>; + rd-sampling-point = <8>; + cmd-timeout-ms = <1000>; + status = "disabled"; + }; + timer: timer@1e782000 { compatible = "aspeed,ast2400-timer"; reg = <0x1e782000 0x90>; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html