* [PATCH v2 0/4] Add Ampere's Altra SMPro hwmon driver
@ 2021-03-29 1:52 Quan Nguyen
2021-03-29 1:52 ` [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers Quan Nguyen
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Quan Nguyen @ 2021-03-29 1:52 UTC (permalink / raw)
To: Joel Stanley, Andrew Jeffery, Jean Delvare, Guenter Roeck,
Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree,
linux-kernel, linux-doc, linux-aspeed, openbmc
Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo
This patch series adds support for Ampere SMpro hwmon driver. This
driver supports accessing various CPU sensors provided by the SMpro
co-processor including temperature, power, voltages, and current found
on Ampere Altra processor family.
v2:
+ Used 'struct of_device_id's .data attribute [Lee Jones]
+ Removed "virtual" sensors [Guenter]
+ Fixed typo "mili" to "milli", "nanoWatt" to "microWatt" [Guenter]
+ Reported SOC_TDP as "Socket TDP" using max attributes [Guenter]
+ Clarified "highest" meaning in documentation [Guenter]
+ Corrected return error code when host is turn off [Guenter]
+ Reported MEM HOT Threshold for all DIMMs as temp*_crit [Guenter]
+ Removed license info as SPDX-License-Identifier existed [Guenter]
+ Added is_visible() support [Guenter]
+ Used HWMON_CHANNEL_INFO() macro and LABEL attributes [Guenter]
+ Made is_valid_id() return boolean [Guenter]
+ Returned -EPROBE_DEFER when smpro reg inaccessible [Guenter]
+ Removed unnecessary error message when dev register fail [Guenter]
+ Removed Socket TDP sensor [Quan]
+ Changed "ampere,ac01-smpro" to "ampere,smpro" [Quan]
+ Included sensor type and channel in labels [Quan]
+ Refactorized code to fix checkpatch.pl --strict complaint [Quan]
Quan Nguyen (4):
dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers
mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support
hwmon: smpro: Add Ampere's Altra smpro-hwmon driver
docs: hwmon: (smpro-hwmon) Add documentation
.../bindings/hwmon/ampere,ac01-hwmon.yaml | 27 +
.../devicetree/bindings/mfd/ampere,smpro.yaml | 82 +++
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/smpro-hwmon.rst | 101 ++++
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/smpro-hwmon.c | 494 ++++++++++++++++++
drivers/mfd/Kconfig | 10 +
drivers/mfd/simple-mfd-i2c.c | 6 +
9 files changed, 730 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/ampere,smpro.yaml
create mode 100644 Documentation/hwmon/smpro-hwmon.rst
create mode 100644 drivers/hwmon/smpro-hwmon.c
--
2.28.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers 2021-03-29 1:52 [PATCH v2 0/4] Add Ampere's Altra SMPro hwmon driver Quan Nguyen @ 2021-03-29 1:52 ` Quan Nguyen 2021-03-30 21:14 ` Rob Herring 2021-03-29 1:52 ` [PATCH v2 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support Quan Nguyen ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Quan Nguyen @ 2021-03-29 1:52 UTC (permalink / raw) To: Joel Stanley, Andrew Jeffery, Jean Delvare, Guenter Roeck, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo Adds device tree bindings for SMPro drivers found on the Mt.Jade hardware reference platform with Ampere's Altra Processor family. Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> --- .../bindings/hwmon/ampere,ac01-hwmon.yaml | 27 ++++++ .../devicetree/bindings/mfd/ampere,smpro.yaml | 82 +++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml create mode 100644 Documentation/devicetree/bindings/mfd/ampere,smpro.yaml diff --git a/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml new file mode 100644 index 000000000000..015130a281f4 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/ampere,ac01-hwmon.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Hardware monitoring driver for the Ampere Altra SMPro + +maintainers: + - Quan Nguyen <quan@os.amperecomputing.com> + +description: | + This module is part of the Ampere Altra SMPro multi-function device. For more + details see ../mfd/ampere,smpro.yaml. + +properties: + compatible: + enum: + - ampere,ac01-hwmon + + reg: + maxItems: 1 + +required: + - compatible + +additionalProperties: false diff --git a/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml new file mode 100644 index 000000000000..bf789c8a3d7d --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml @@ -0,0 +1,82 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/ampere,smpro.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ampere Altra SMPro firmware driver + +maintainers: + - Quan Nguyen <quan@os.amperecomputing.com> + +description: | + Ampere Altra SMPro firmware may contain different blocks like hardware + monitoring, error monitoring and other miscellaneous features. + +properties: + compatible: + const: ampere,smpro + + reg: + description: + I2C device address. + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^hwmon(@[0-9a-f]+)?$": + $ref: ../hwmon/ampere,ac01-hwmon.yaml + + "^misc(@[0-9a-f]+)?$": + type: object + description: Ampere Altra SMPro Misc driver + properties: + compatible: + const: "ampere,ac01-misc" + + "^errmon(@[0-9a-f]+)?$": + type: object + description: Ampere Altra SMPro Error Monitor driver + properties: + compatible: + const: "ampere,ac01-errmon" + +required: + - "#address-cells" + - "#size-cells" + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + smpro@4f { + compatible = "ampere,smpro"; + reg = <0x4f>; + #address-cells = <1>; + #size-cells = <0>; + + hwmon { + compatible = "ampere,ac01-hwmon"; + }; + + misc { + compatible = "ampere,ac01-misc"; + }; + + errmon { + compatible = "ampere,ac01-errmon"; + }; + + }; + }; -- 2.28.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers 2021-03-29 1:52 ` [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers Quan Nguyen @ 2021-03-30 21:14 ` Rob Herring 2021-04-07 9:42 ` Quan Nguyen 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-03-30 21:14 UTC (permalink / raw) To: Quan Nguyen Cc: linux-hwmon, devicetree, Jean Delvare, linux-aspeed, Jonathan Corbet, Andrew Jeffery, openbmc, linux-doc, linux-kernel, Thang Q . Nguyen, Phong Vo, Open Source Submission, Lee Jones, Guenter Roeck On Mon, Mar 29, 2021 at 08:52:35AM +0700, Quan Nguyen wrote: > Adds device tree bindings for SMPro drivers found on the Mt.Jade hardware > reference platform with Ampere's Altra Processor family. > > Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> > --- > .../bindings/hwmon/ampere,ac01-hwmon.yaml | 27 ++++++ > .../devicetree/bindings/mfd/ampere,smpro.yaml | 82 +++++++++++++++++++ > 2 files changed, 109 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > create mode 100644 Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > new file mode 100644 > index 000000000000..015130a281f4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml > @@ -0,0 +1,27 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/ampere,ac01-hwmon.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Hardware monitoring driver for the Ampere Altra SMPro > + > +maintainers: > + - Quan Nguyen <quan@os.amperecomputing.com> > + > +description: | > + This module is part of the Ampere Altra SMPro multi-function device. For more > + details see ../mfd/ampere,smpro.yaml. > + > +properties: > + compatible: > + enum: > + - ampere,ac01-hwmon > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + > +additionalProperties: false > diff --git a/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > new file mode 100644 > index 000000000000..bf789c8a3d7d > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml > @@ -0,0 +1,82 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/ampere,smpro.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Ampere Altra SMPro firmware driver > + > +maintainers: > + - Quan Nguyen <quan@os.amperecomputing.com> > + > +description: | > + Ampere Altra SMPro firmware may contain different blocks like hardware > + monitoring, error monitoring and other miscellaneous features. > + > +properties: > + compatible: > + const: ampere,smpro Only 1 version of SMPro? Needs to be more specific or provide details on how the exact version of firmware/hardware is discovered. > + > + reg: > + description: > + I2C device address. > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > +patternProperties: > + "^hwmon(@[0-9a-f]+)?$": > + $ref: ../hwmon/ampere,ac01-hwmon.yaml > + > + "^misc(@[0-9a-f]+)?$": > + type: object > + description: Ampere Altra SMPro Misc driver Bindings describe h/w, not drivers. > + properties: > + compatible: > + const: "ampere,ac01-misc" > + > + "^errmon(@[0-9a-f]+)?$": > + type: object > + description: Ampere Altra SMPro Error Monitor driver > + properties: > + compatible: > + const: "ampere,ac01-errmon" > + > +required: > + - "#address-cells" > + - "#size-cells" > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + smpro@4f { > + compatible = "ampere,smpro"; > + reg = <0x4f>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + hwmon { > + compatible = "ampere,ac01-hwmon"; > + }; > + > + misc { > + compatible = "ampere,ac01-misc"; > + }; > + > + errmon { > + compatible = "ampere,ac01-errmon"; > + }; None of the child nodes have any resources in DT, so you don't need them in DT. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers 2021-03-30 21:14 ` Rob Herring @ 2021-04-07 9:42 ` Quan Nguyen 0 siblings, 0 replies; 12+ messages in thread From: Quan Nguyen @ 2021-04-07 9:42 UTC (permalink / raw) To: Rob Herring Cc: linux-hwmon, devicetree, Jean Delvare, linux-aspeed, Jonathan Corbet, Andrew Jeffery, openbmc, linux-doc, linux-kernel, Thang Q . Nguyen, Phong Vo, Open Source Submission, Lee Jones, Guenter Roeck Hi Rob, Thank you for reviewing On 31/03/2021 04:14, Rob Herring wrote: > On Mon, Mar 29, 2021 at 08:52:35AM +0700, Quan Nguyen wrote: >> Adds device tree bindings for SMPro drivers found on the Mt.Jade hardware >> reference platform with Ampere's Altra Processor family. >> >> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> >> --- >> .../bindings/hwmon/ampere,ac01-hwmon.yaml | 27 ++++++ >> .../devicetree/bindings/mfd/ampere,smpro.yaml | 82 +++++++++++++++++++ >> 2 files changed, 109 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml >> create mode 100644 Documentation/devicetree/bindings/mfd/ampere,smpro.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml >> new file mode 100644 >> index 000000000000..015130a281f4 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/ampere,ac01-hwmon.yaml >> @@ -0,0 +1,27 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwmon/ampere,ac01-hwmon.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Hardware monitoring driver for the Ampere Altra SMPro >> + >> +maintainers: >> + - Quan Nguyen <quan@os.amperecomputing.com> >> + >> +description: | >> + This module is part of the Ampere Altra SMPro multi-function device. For more >> + details see ../mfd/ampere,smpro.yaml. >> + >> +properties: >> + compatible: >> + enum: >> + - ampere,ac01-hwmon >> + >> + reg: >> + maxItems: 1 >> + >> +required: >> + - compatible >> + >> +additionalProperties: false >> diff --git a/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml >> new file mode 100644 >> index 000000000000..bf789c8a3d7d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/ampere,smpro.yaml >> @@ -0,0 +1,82 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mfd/ampere,smpro.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Ampere Altra SMPro firmware driver >> + >> +maintainers: >> + - Quan Nguyen <quan@os.amperecomputing.com> >> + >> +description: | >> + Ampere Altra SMPro firmware may contain different blocks like hardware >> + monitoring, error monitoring and other miscellaneous features. >> + >> +properties: >> + compatible: >> + const: ampere,smpro > > Only 1 version of SMPro? Needs to be more specific or provide details on > how the exact version of firmware/hardware is discovered. > Will change to enum in next version. enum: - ampere,smpro >> + >> + reg: >> + description: >> + I2C device address. >> + maxItems: 1 >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 >> + >> +patternProperties: >> + "^hwmon(@[0-9a-f]+)?$": >> + $ref: ../hwmon/ampere,ac01-hwmon.yaml >> + >> + "^misc(@[0-9a-f]+)?$": >> + type: object >> + description: Ampere Altra SMPro Misc driver > > Bindings describe h/w, not drivers. > Will fix in next version >> + properties: >> + compatible: >> + const: "ampere,ac01-misc" >> + >> + "^errmon(@[0-9a-f]+)?$": >> + type: object >> + description: Ampere Altra SMPro Error Monitor driver >> + properties: >> + compatible: >> + const: "ampere,ac01-errmon" >> + >> +required: >> + - "#address-cells" >> + - "#size-cells" >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + smpro@4f { >> + compatible = "ampere,smpro"; >> + reg = <0x4f>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + hwmon { >> + compatible = "ampere,ac01-hwmon"; >> + }; >> + >> + misc { >> + compatible = "ampere,ac01-misc"; >> + }; >> + >> + errmon { >> + compatible = "ampere,ac01-errmon"; >> + }; > > None of the child nodes have any resources in DT, so you don't need > them in DT. > > Rob > The intention was to re-use the drivers/mfd/simple-mfd-i2c.c and dont want to add extra codes to this driver just to instantiate these children. So for this case, the child drivers will not be instantiated if there are no child nodes in DT. One solution I have in mind is to introduce resource (reg offset) for each child drivers. -Quan ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support 2021-03-29 1:52 [PATCH v2 0/4] Add Ampere's Altra SMPro hwmon driver Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers Quan Nguyen @ 2021-03-29 1:52 ` Quan Nguyen 2021-04-14 10:03 ` Lee Jones 2021-03-29 1:52 ` [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 4/4] docs: hwmon: (smpro-hwmon) Add documentation Quan Nguyen 3 siblings, 1 reply; 12+ messages in thread From: Quan Nguyen @ 2021-03-29 1:52 UTC (permalink / raw) To: Joel Stanley, Andrew Jeffery, Jean Delvare, Guenter Roeck, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo Adds an MFD driver for SMpro found on the Mt.Jade hardware reference platform with Ampere's Altra processor family. Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> --- drivers/mfd/Kconfig | 10 ++++++++++ drivers/mfd/simple-mfd-i2c.c | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index d07e8cf93286..f7a6460f7aa0 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -77,6 +77,16 @@ config MFD_AS3711 help Support for the AS3711 PMIC from AMS +config MFD_SMPRO + tristate "Ampere Computing MFD SMpro core driver" + select MFD_SIMPLE_MFD_I2C + help + Say yes here to enable SMpro driver support for Ampere's Altra + processor family. + + Ampere's Altra SMpro exposes an I2C regmap interface that can + be accessed by child devices. + config MFD_AS3722 tristate "ams AS3722 Power Management IC" select MFD_CORE diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c index 87f684cff9a1..9a44655f5592 100644 --- a/drivers/mfd/simple-mfd-i2c.c +++ b/drivers/mfd/simple-mfd-i2c.c @@ -21,6 +21,11 @@ static const struct regmap_config simple_regmap_config = { .val_bits = 8, }; +static const struct regmap_config simple_word_regmap_config = { + .reg_bits = 8, + .val_bits = 16, +}; + static int simple_mfd_i2c_probe(struct i2c_client *i2c) { const struct regmap_config *config; @@ -39,6 +44,7 @@ static int simple_mfd_i2c_probe(struct i2c_client *i2c) static const struct of_device_id simple_mfd_i2c_of_match[] = { { .compatible = "kontron,sl28cpld" }, + { .compatible = "ampere,smpro", .data = &simple_word_regmap_config }, {} }; MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match); -- 2.28.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support 2021-03-29 1:52 ` [PATCH v2 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support Quan Nguyen @ 2021-04-14 10:03 ` Lee Jones 0 siblings, 0 replies; 12+ messages in thread From: Lee Jones @ 2021-04-14 10:03 UTC (permalink / raw) To: Quan Nguyen Cc: linux-hwmon, devicetree, Jean Delvare, linux-aspeed, Jonathan Corbet, Andrew Jeffery, openbmc, linux-doc, linux-kernel, Thang Q . Nguyen, Phong Vo, Rob Herring, Open Source Submission, Guenter Roeck On Mon, 29 Mar 2021, Quan Nguyen wrote: > Adds an MFD driver for SMpro found on the Mt.Jade hardware reference > platform with Ampere's Altra processor family. > > Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> > --- > drivers/mfd/Kconfig | 10 ++++++++++ > drivers/mfd/simple-mfd-i2c.c | 6 ++++++ > 2 files changed, 16 insertions(+) For my own reference (apply this as-is to your sign-off block): Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver 2021-03-29 1:52 [PATCH v2 0/4] Add Ampere's Altra SMPro hwmon driver Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support Quan Nguyen @ 2021-03-29 1:52 ` Quan Nguyen 2021-03-30 1:43 ` Guenter Roeck 2021-03-29 1:52 ` [PATCH v2 4/4] docs: hwmon: (smpro-hwmon) Add documentation Quan Nguyen 3 siblings, 1 reply; 12+ messages in thread From: Quan Nguyen @ 2021-03-29 1:52 UTC (permalink / raw) To: Joel Stanley, Andrew Jeffery, Jean Delvare, Guenter Roeck, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo This commit adds support for Ampere SMpro hwmon driver. This driver supports accessing various CPU sensors provided by the SMpro co-processor including temperature, power, voltages, and current. Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> --- drivers/hwmon/Kconfig | 8 + drivers/hwmon/Makefile | 1 + drivers/hwmon/smpro-hwmon.c | 494 ++++++++++++++++++++++++++++++++++++ 3 files changed, 503 insertions(+) create mode 100644 drivers/hwmon/smpro-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 0ddc974b102e..ba4b5a911baf 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -67,6 +67,14 @@ config SENSORS_ABITUGURU3 This driver can also be built as a module. If so, the module will be called abituguru3. +config SENSORS_SMPRO + tristate "Ampere's Altra SMpro hardware monitoring driver" + depends on MFD_SMPRO + help + If you say yes here you get support for the thermal, voltage, + current and power sensors of Ampere's Altra processor family SoC + with SMpro co-processor. + config SENSORS_AD7314 tristate "Analog Devices AD7314 and compatibles" depends on SPI diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 59e78bc212cf..b25391f9c651 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_SHT3x) += sht3x.o obj-$(CONFIG_SENSORS_SHTC1) += shtc1.o obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o obj-$(CONFIG_SENSORS_SMM665) += smm665.o +obj-$(CONFIG_SENSORS_SMPRO) += smpro-hwmon.o obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o diff --git a/drivers/hwmon/smpro-hwmon.c b/drivers/hwmon/smpro-hwmon.c new file mode 100644 index 000000000000..4277736ebc6e --- /dev/null +++ b/drivers/hwmon/smpro-hwmon.c @@ -0,0 +1,494 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Ampere Computing SoC's SMPro Hardware Monitoring Driver + * + * Copyright (c) 2021, Ampere Computing LLC + */ +#include <linux/bitfield.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/kernel.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/regmap.h> + +/* Identification Registers */ +#define MANUFACTURER_ID_REG 0x02 +#define AMPERE_MANUFACTURER_ID 0xCD3A + +/* Logical Power Sensor Registers */ +#define SOC_TEMP_REG 0x10 +#define SOC_VRD_TEMP_REG 0x11 +#define DIMM_VRD_TEMP_REG 0x12 +#define CORE_VRD_TEMP_REG 0x13 +#define CH0_DIMM_TEMP_REG 0x14 +#define CH1_DIMM_TEMP_REG 0x15 +#define CH2_DIMM_TEMP_REG 0x16 +#define CH3_DIMM_TEMP_REG 0x17 +#define CH4_DIMM_TEMP_REG 0x18 +#define CH5_DIMM_TEMP_REG 0x19 +#define CH6_DIMM_TEMP_REG 0x1A +#define CH7_DIMM_TEMP_REG 0x1B +#define RCA_VRD_TEMP_REG 0x1C + +#define CORE_VRD_PWR_REG 0x20 +#define SOC_PWR_REG 0x21 +#define DIMM_VRD1_PWR_REG 0x22 +#define DIMM_VRD2_PWR_REG 0x23 +#define CORE_VRD_PWR_MW_REG 0x26 +#define SOC_PWR_MW_REG 0x27 +#define DIMM_VRD1_PWR_MW_REG 0x28 +#define DIMM_VRD2_PWR_MW_REG 0x29 +#define RCA_VRD_PWR_REG 0x2A +#define RCA_VRD_PWR_MW_REG 0x2B + +#define MEM_HOT_THRESHOLD_REG 0x32 +#define SOC_VR_HOT_THRESHOLD_REG 0x33 +#define CORE_VRD_VOLT_REG 0x34 +#define SOC_VRD_VOLT_REG 0x35 +#define DIMM_VRD1_VOLT_REG 0x36 +#define DIMM_VRD2_VOLT_REG 0x37 +#define RCA_VRD_VOLT_REG 0x38 + +#define CORE_VRD_CURR_REG 0x39 +#define SOC_VRD_CURR_REG 0x3A +#define DIMM_VRD1_CURR_REG 0x3B +#define DIMM_VRD2_CURR_REG 0x3C +#define RCA_VRD_CURR_REG 0x3D + +struct smpro_hwmon { + struct regmap *regmap; +}; + +struct smpro_sensor { + const u8 reg; + const u8 reg_ext; + const char *label; +}; + +static const struct smpro_sensor temperature[] = { + { + .reg = SOC_TEMP_REG, + .label = "temp1 SoC" + }, + { + .reg = SOC_VRD_TEMP_REG, + .label = "temp2 SoC VRD" + }, + { + .reg = DIMM_VRD_TEMP_REG, + .label = "temp3 DIMM VRD" + }, + { + .reg = CORE_VRD_TEMP_REG, + .label = "temp4 CORE VRD" + }, + { + .reg = CH0_DIMM_TEMP_REG, + .label = "temp5 CH0 DIMM" + }, + { + .reg = CH1_DIMM_TEMP_REG, + .label = "temp6 CH1 DIMM" + }, + { + .reg = CH2_DIMM_TEMP_REG, + .label = "temp7 CH2 DIMM" + }, + { + .reg = CH3_DIMM_TEMP_REG, + .label = "temp8 CH3 DIMM" + }, + { + .reg = CH4_DIMM_TEMP_REG, + .label = "temp9 CH4 DIMM" + }, + { + .reg = CH5_DIMM_TEMP_REG, + .label = "temp10 CH5 DIMM" + }, + { + .reg = CH6_DIMM_TEMP_REG, + .label = "temp11 CH6 DIMM" + }, + { + .reg = CH7_DIMM_TEMP_REG, + .label = "temp12 CH7 DIMM" + }, + { + .reg = RCA_VRD_TEMP_REG, + .label = "temp13 RCA VRD" + }, +}; + +static const struct smpro_sensor voltage[] = { + { + .reg = CORE_VRD_VOLT_REG, + .label = "vout0 CORE VRD" + }, + { + .reg = SOC_VRD_VOLT_REG, + .label = "vout1 SoC VRD" + }, + { + .reg = DIMM_VRD1_VOLT_REG, + .label = "vout2 DIMM VRD1" + }, + { + .reg = DIMM_VRD2_VOLT_REG, + .label = "vout3 DIMM VRD2" + }, + { + .reg = RCA_VRD_VOLT_REG, + .label = "vout4 RCA VRD" + }, +}; + +static const struct smpro_sensor curr_sensor[] = { + { + .reg = CORE_VRD_CURR_REG, + .label = "iout1 CORE VRD" + }, + { + .reg = SOC_VRD_CURR_REG, + .label = "iout2 SoC VRD" + }, + { + .reg = DIMM_VRD1_CURR_REG, + .label = "iout3 DIMM VRD1" + }, + { + .reg = DIMM_VRD2_CURR_REG, + .label = "iout4 DIMM VRD2" + }, + { + .reg = RCA_VRD_CURR_REG, + .label = "iout5 RCA VRD" + }, +}; + +static const struct smpro_sensor power[] = { + { + .reg = CORE_VRD_PWR_REG, + .reg_ext = CORE_VRD_PWR_MW_REG, + .label = "power1 CORE VRD" + }, + { + .reg = SOC_PWR_REG, + .reg_ext = SOC_PWR_MW_REG, + .label = "power2 SoC" + }, + { + .reg = DIMM_VRD1_PWR_REG, + .reg_ext = DIMM_VRD1_PWR_MW_REG, + .label = "power3 DIMM VRD1" + }, + { + .reg = DIMM_VRD2_PWR_REG, + .reg_ext = DIMM_VRD2_PWR_MW_REG, + .label = "power4 DIMM VRD2" + }, + { + .reg = RCA_VRD_PWR_REG, + .reg_ext = RCA_VRD_PWR_MW_REG, + .label = "power5 RCA VRD" + }, +}; + +static int smpro_read_temp(struct device *dev, u32 attr, int channel, long *val) +{ + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); + unsigned int value; + int ret; + + switch (attr) { + case hwmon_temp_input: + ret = regmap_read(hwmon->regmap, + temperature[channel].reg, &value); + if (ret) + return ret; + *val = (value & 0x1ff) * 1000; + break; + case hwmon_temp_crit: + if (temperature[channel].reg == SOC_VRD_TEMP_REG) { + ret = regmap_read(hwmon->regmap, SOC_VR_HOT_THRESHOLD_REG, &value); + if (ret) + return ret; + *val = (value & 0x1ff) * 1000; + } else { + /* Report same MEM HOT threshold across DIMM channels */ + ret = regmap_read(hwmon->regmap, MEM_HOT_THRESHOLD_REG, &value); + if (ret) + return ret; + *val = (value & 0x1ff) * 1000; + } + break; + default: + return -EOPNOTSUPP; + } + return 0; +} + +static int smpro_read_in(struct device *dev, u32 attr, int channel, long *val) +{ + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); + unsigned int value; + int ret; + + switch (attr) { + case hwmon_in_input: + ret = regmap_read(hwmon->regmap, voltage[channel].reg, &value); + if (ret < 0) + return ret; + /* Scale reported by the hardware is 1mV */ + *val = value & 0x7fff; + return 0; + default: + return -EOPNOTSUPP; + } +} + +static int smpro_read_curr(struct device *dev, u32 attr, int channel, long *val) +{ + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); + unsigned int value; + int ret; + + switch (attr) { + case hwmon_curr_input: + ret = regmap_read(hwmon->regmap, curr_sensor[channel].reg, &value); + if (ret < 0) + return ret; + /* Scale reported by the hardware is 1mA */ + *val = value & 0x7fff; + return 0; + default: + return -EOPNOTSUPP; + } +} + +static int smpro_read_power(struct device *dev, u32 attr, int channel, long *val_pwr) +{ + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); + unsigned int val = 0, val_mw = 0; + int ret; + + switch (attr) { + case hwmon_power_input: + ret = regmap_read(hwmon->regmap, power[channel].reg, &val); + if (ret) + return ret; + + ret = regmap_read(hwmon->regmap, power[channel].reg_ext, &val_mw); + if (ret) + return ret; + + *val_pwr = val * 1000000 + val_mw * 1000; + return 0; + + default: + return -EOPNOTSUPP; + } +} + +static int smpro_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + switch (type) { + case hwmon_temp: + return smpro_read_temp(dev, attr, channel, val); + case hwmon_in: + return smpro_read_in(dev, attr, channel, val); + case hwmon_power: + return smpro_read_power(dev, attr, channel, val); + case hwmon_curr: + return smpro_read_curr(dev, attr, channel, val); + default: + return -EOPNOTSUPP; + } +} + +static int smpro_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + return -EOPNOTSUPP; +} + +static int smpro_read_string(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, const char **str) +{ + switch (type) { + case hwmon_temp: + switch (attr) { + case hwmon_temp_label: + *str = temperature[channel].label; + return 0; + default: + return -EOPNOTSUPP; + } + break; + + case hwmon_in: + switch (attr) { + case hwmon_in_label: + *str = voltage[channel].label; + return 0; + default: + return -EOPNOTSUPP; + } + break; + + case hwmon_curr: + switch (attr) { + case hwmon_curr_label: + *str = curr_sensor[channel].label; + return 0; + default: + return -EOPNOTSUPP; + } + break; + + case hwmon_power: + switch (attr) { + case hwmon_power_label: + *str = power[channel].label; + return 0; + default: + return -EOPNOTSUPP; + } + break; + default: + return -EOPNOTSUPP; + } + + return -EOPNOTSUPP; +} + +static umode_t smpro_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + const struct smpro_hwmon *hwmon = data; + unsigned int value; + int ret; + + switch (type) { + case hwmon_temp: + switch (attr) { + case hwmon_temp_input: + case hwmon_temp_label: + case hwmon_temp_crit: + ret = regmap_read(hwmon->regmap, temperature[channel].reg, &value); + if (ret || value == 0xFFFF) + return 0; + break; + } + default: + break; + } + + return 0444; +} + +static const struct hwmon_channel_info *smpro_info[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, + HWMON_T_INPUT | HWMON_T_LABEL), + HWMON_CHANNEL_INFO(in, + HWMON_I_INPUT | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LABEL), + HWMON_CHANNEL_INFO(power, + HWMON_P_INPUT | HWMON_P_LABEL, + HWMON_P_INPUT | HWMON_P_LABEL, + HWMON_P_INPUT | HWMON_P_LABEL, + HWMON_P_INPUT | HWMON_P_LABEL, + HWMON_P_INPUT | HWMON_P_LABEL), + HWMON_CHANNEL_INFO(curr, + HWMON_C_INPUT | HWMON_C_LABEL, + HWMON_C_INPUT | HWMON_C_LABEL, + HWMON_C_INPUT | HWMON_C_LABEL, + HWMON_C_INPUT | HWMON_C_LABEL, + HWMON_C_INPUT | HWMON_C_LABEL), + NULL +}; + +static const struct hwmon_ops smpro_hwmon_ops = { + .is_visible = smpro_is_visible, + .read = smpro_read, + .write = smpro_write, + .read_string = smpro_read_string, +}; + +static const struct hwmon_chip_info smpro_chip_info = { + .ops = &smpro_hwmon_ops, + .info = smpro_info, +}; + +static bool is_valid_id(struct regmap *regmap) +{ + unsigned int val; + int ret; + + ret = regmap_read(regmap, MANUFACTURER_ID_REG, &val); + + return (ret || (val != AMPERE_MANUFACTURER_ID)) ? false : true; +} + +static int smpro_hwmon_probe(struct platform_device *pdev) +{ + struct smpro_hwmon *hwmon; + struct device *hwmon_dev; + + hwmon = devm_kzalloc(&pdev->dev, sizeof(struct smpro_hwmon), GFP_KERNEL); + if (!hwmon) + return -ENOMEM; + + hwmon->regmap = dev_get_regmap(pdev->dev.parent, NULL); + if (!hwmon->regmap) + return -ENODEV; + + /* Check for valid ID */ + if (!is_valid_id(hwmon->regmap)) + return -EPROBE_DEFER; + + hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, "smpro_hwmon", + hwmon, &smpro_chip_info, NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct of_device_id smpro_hwmon_of_match[] = { + { .compatible = "ampere,ac01-hwmon" }, + {} +}; +MODULE_DEVICE_TABLE(of, smpro_hwmon_of_match); + +static struct platform_driver smpro_hwmon_driver = { + .probe = smpro_hwmon_probe, + .driver = { + .name = "smpro-hwmon", + .of_match_table = smpro_hwmon_of_match, + }, +}; + +module_platform_driver(smpro_hwmon_driver); + +MODULE_AUTHOR("Thu Nguyen <thu@os.amperecomputing.com>"); +MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>"); +MODULE_DESCRIPTION("Ampere Altra SMPro hwmon driver"); +MODULE_LICENSE("GPL v2"); -- 2.28.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver 2021-03-29 1:52 ` [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver Quan Nguyen @ 2021-03-30 1:43 ` Guenter Roeck 2021-04-07 7:41 ` Quan Nguyen 0 siblings, 1 reply; 12+ messages in thread From: Guenter Roeck @ 2021-03-30 1:43 UTC (permalink / raw) To: Quan Nguyen, Joel Stanley, Andrew Jeffery, Jean Delvare, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo On 3/28/21 6:52 PM, Quan Nguyen wrote: > This commit adds support for Ampere SMpro hwmon driver. This driver > supports accessing various CPU sensors provided by the SMpro co-processor > including temperature, power, voltages, and current. > > Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> > --- > drivers/hwmon/Kconfig | 8 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/smpro-hwmon.c | 494 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 503 insertions(+) > create mode 100644 drivers/hwmon/smpro-hwmon.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 0ddc974b102e..ba4b5a911baf 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -67,6 +67,14 @@ config SENSORS_ABITUGURU3 > This driver can also be built as a module. If so, the module > will be called abituguru3. > > +config SENSORS_SMPRO > + tristate "Ampere's Altra SMpro hardware monitoring driver" > + depends on MFD_SMPRO > + help > + If you say yes here you get support for the thermal, voltage, > + current and power sensors of Ampere's Altra processor family SoC > + with SMpro co-processor. > + > config SENSORS_AD7314 > tristate "Analog Devices AD7314 and compatibles" > depends on SPI > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 59e78bc212cf..b25391f9c651 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_SHT3x) += sht3x.o > obj-$(CONFIG_SENSORS_SHTC1) += shtc1.o > obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o > obj-$(CONFIG_SENSORS_SMM665) += smm665.o > +obj-$(CONFIG_SENSORS_SMPRO) += smpro-hwmon.o > obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o > obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o > obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o > diff --git a/drivers/hwmon/smpro-hwmon.c b/drivers/hwmon/smpro-hwmon.c > new file mode 100644 > index 000000000000..4277736ebc6e > --- /dev/null > +++ b/drivers/hwmon/smpro-hwmon.c > @@ -0,0 +1,494 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Ampere Computing SoC's SMPro Hardware Monitoring Driver > + * > + * Copyright (c) 2021, Ampere Computing LLC > + */ > +#include <linux/bitfield.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/kernel.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > + > +/* Identification Registers */ > +#define MANUFACTURER_ID_REG 0x02 > +#define AMPERE_MANUFACTURER_ID 0xCD3A > + > +/* Logical Power Sensor Registers */ > +#define SOC_TEMP_REG 0x10 > +#define SOC_VRD_TEMP_REG 0x11 > +#define DIMM_VRD_TEMP_REG 0x12 > +#define CORE_VRD_TEMP_REG 0x13 > +#define CH0_DIMM_TEMP_REG 0x14 > +#define CH1_DIMM_TEMP_REG 0x15 > +#define CH2_DIMM_TEMP_REG 0x16 > +#define CH3_DIMM_TEMP_REG 0x17 > +#define CH4_DIMM_TEMP_REG 0x18 > +#define CH5_DIMM_TEMP_REG 0x19 > +#define CH6_DIMM_TEMP_REG 0x1A > +#define CH7_DIMM_TEMP_REG 0x1B > +#define RCA_VRD_TEMP_REG 0x1C > + > +#define CORE_VRD_PWR_REG 0x20 > +#define SOC_PWR_REG 0x21 > +#define DIMM_VRD1_PWR_REG 0x22 > +#define DIMM_VRD2_PWR_REG 0x23 > +#define CORE_VRD_PWR_MW_REG 0x26 > +#define SOC_PWR_MW_REG 0x27 > +#define DIMM_VRD1_PWR_MW_REG 0x28 > +#define DIMM_VRD2_PWR_MW_REG 0x29 > +#define RCA_VRD_PWR_REG 0x2A > +#define RCA_VRD_PWR_MW_REG 0x2B > + > +#define MEM_HOT_THRESHOLD_REG 0x32 > +#define SOC_VR_HOT_THRESHOLD_REG 0x33 > +#define CORE_VRD_VOLT_REG 0x34 > +#define SOC_VRD_VOLT_REG 0x35 > +#define DIMM_VRD1_VOLT_REG 0x36 > +#define DIMM_VRD2_VOLT_REG 0x37 > +#define RCA_VRD_VOLT_REG 0x38 > + > +#define CORE_VRD_CURR_REG 0x39 > +#define SOC_VRD_CURR_REG 0x3A > +#define DIMM_VRD1_CURR_REG 0x3B > +#define DIMM_VRD2_CURR_REG 0x3C > +#define RCA_VRD_CURR_REG 0x3D > + > +struct smpro_hwmon { > + struct regmap *regmap; > +}; > + > +struct smpro_sensor { > + const u8 reg; > + const u8 reg_ext; > + const char *label; > +}; > + > +static const struct smpro_sensor temperature[] = { > + { > + .reg = SOC_TEMP_REG, > + .label = "temp1 SoC" > + }, > + { > + .reg = SOC_VRD_TEMP_REG, > + .label = "temp2 SoC VRD" > + }, > + { > + .reg = DIMM_VRD_TEMP_REG, > + .label = "temp3 DIMM VRD" > + }, > + { > + .reg = CORE_VRD_TEMP_REG, > + .label = "temp4 CORE VRD" > + }, > + { > + .reg = CH0_DIMM_TEMP_REG, > + .label = "temp5 CH0 DIMM" > + }, > + { > + .reg = CH1_DIMM_TEMP_REG, > + .label = "temp6 CH1 DIMM" > + }, > + { > + .reg = CH2_DIMM_TEMP_REG, > + .label = "temp7 CH2 DIMM" > + }, > + { > + .reg = CH3_DIMM_TEMP_REG, > + .label = "temp8 CH3 DIMM" > + }, > + { > + .reg = CH4_DIMM_TEMP_REG, > + .label = "temp9 CH4 DIMM" > + }, > + { > + .reg = CH5_DIMM_TEMP_REG, > + .label = "temp10 CH5 DIMM" > + }, > + { > + .reg = CH6_DIMM_TEMP_REG, > + .label = "temp11 CH6 DIMM" > + }, > + { > + .reg = CH7_DIMM_TEMP_REG, > + .label = "temp12 CH7 DIMM" > + }, > + { > + .reg = RCA_VRD_TEMP_REG, > + .label = "temp13 RCA VRD" > + }, > +}; > + > +static const struct smpro_sensor voltage[] = { > + { > + .reg = CORE_VRD_VOLT_REG, > + .label = "vout0 CORE VRD" > + }, > + { > + .reg = SOC_VRD_VOLT_REG, > + .label = "vout1 SoC VRD" > + }, > + { > + .reg = DIMM_VRD1_VOLT_REG, > + .label = "vout2 DIMM VRD1" > + }, > + { > + .reg = DIMM_VRD2_VOLT_REG, > + .label = "vout3 DIMM VRD2" > + }, > + { > + .reg = RCA_VRD_VOLT_REG, > + .label = "vout4 RCA VRD" > + }, > +}; > + > +static const struct smpro_sensor curr_sensor[] = { > + { > + .reg = CORE_VRD_CURR_REG, > + .label = "iout1 CORE VRD" > + }, > + { > + .reg = SOC_VRD_CURR_REG, > + .label = "iout2 SoC VRD" > + }, > + { > + .reg = DIMM_VRD1_CURR_REG, > + .label = "iout3 DIMM VRD1" > + }, > + { > + .reg = DIMM_VRD2_CURR_REG, > + .label = "iout4 DIMM VRD2" > + }, > + { > + .reg = RCA_VRD_CURR_REG, > + .label = "iout5 RCA VRD" > + }, > +}; > + > +static const struct smpro_sensor power[] = { > + { > + .reg = CORE_VRD_PWR_REG, > + .reg_ext = CORE_VRD_PWR_MW_REG, > + .label = "power1 CORE VRD" > + }, > + { > + .reg = SOC_PWR_REG, > + .reg_ext = SOC_PWR_MW_REG, > + .label = "power2 SoC" > + }, > + { > + .reg = DIMM_VRD1_PWR_REG, > + .reg_ext = DIMM_VRD1_PWR_MW_REG, > + .label = "power3 DIMM VRD1" > + }, > + { > + .reg = DIMM_VRD2_PWR_REG, > + .reg_ext = DIMM_VRD2_PWR_MW_REG, > + .label = "power4 DIMM VRD2" > + }, > + { > + .reg = RCA_VRD_PWR_REG, > + .reg_ext = RCA_VRD_PWR_MW_REG, > + .label = "power5 RCA VRD" > + }, > +}; > + > +static int smpro_read_temp(struct device *dev, u32 attr, int channel, long *val) > +{ > + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); > + unsigned int value; > + int ret; > + > + switch (attr) { > + case hwmon_temp_input: > + ret = regmap_read(hwmon->regmap, > + temperature[channel].reg, &value); > + if (ret) > + return ret; > + *val = (value & 0x1ff) * 1000; > + break; > + case hwmon_temp_crit: > + if (temperature[channel].reg == SOC_VRD_TEMP_REG) { > + ret = regmap_read(hwmon->regmap, SOC_VR_HOT_THRESHOLD_REG, &value); > + if (ret) > + return ret; > + *val = (value & 0x1ff) * 1000; > + } else { > + /* Report same MEM HOT threshold across DIMM channels */ > + ret = regmap_read(hwmon->regmap, MEM_HOT_THRESHOLD_REG, &value); > + if (ret) > + return ret; > + *val = (value & 0x1ff) * 1000; > + } To avoid code duplication: reg = temperature[channel].reg == SOC_VRD_TEMP_REG ? SOC_VR_HOT_THRESHOLD_REG : MEM_HOT_THRESHOLD_REG; ret = regmap_read(hwmon->regmap, reg, &value); if (ret) return ret; But then why don't you just use reg_ext to store SOC_VR_HOT_THRESHOLD_REG or MEM_HOT_THRESHOLD_REG ? It is already available, after all, and with it the code could be simplified to ret = regmap_read(hwmon->regmap, temperature[channel].reg_ext, &value); if (ret) return ret; I don't have a datasheet, but I do wonder what is in bit 9..15. Any idea ? Main question is if there is a sign bit, as theoretic as it may be. > + break; > + default: > + return -EOPNOTSUPP; > + } > + return 0; > +} > + > +static int smpro_read_in(struct device *dev, u32 attr, int channel, long *val) > +{ > + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); > + unsigned int value; > + int ret; > + > + switch (attr) { > + case hwmon_in_input: > + ret = regmap_read(hwmon->regmap, voltage[channel].reg, &value); > + if (ret < 0) > + return ret; > + /* Scale reported by the hardware is 1mV */ > + *val = value & 0x7fff; What is in bit 15 ? > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int smpro_read_curr(struct device *dev, u32 attr, int channel, long *val) > +{ > + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); > + unsigned int value; > + int ret; > + > + switch (attr) { > + case hwmon_curr_input: > + ret = regmap_read(hwmon->regmap, curr_sensor[channel].reg, &value); > + if (ret < 0) > + return ret; > + /* Scale reported by the hardware is 1mA */ > + *val = value & 0x7fff; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int smpro_read_power(struct device *dev, u32 attr, int channel, long *val_pwr) > +{ > + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); > + unsigned int val = 0, val_mw = 0; > + int ret; > + > + switch (attr) { > + case hwmon_power_input: > + ret = regmap_read(hwmon->regmap, power[channel].reg, &val); > + if (ret) > + return ret; > + > + ret = regmap_read(hwmon->regmap, power[channel].reg_ext, &val_mw); > + if (ret) > + return ret; > + > + *val_pwr = val * 1000000 + val_mw * 1000; > + return 0; > + > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int smpro_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + switch (type) { > + case hwmon_temp: > + return smpro_read_temp(dev, attr, channel, val); > + case hwmon_in: > + return smpro_read_in(dev, attr, channel, val); > + case hwmon_power: > + return smpro_read_power(dev, attr, channel, val); > + case hwmon_curr: > + return smpro_read_curr(dev, attr, channel, val); > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int smpro_write(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long val) > +{ > + return -EOPNOTSUPP; > +} There are no writeable attributes, thus the write function is not needed. > + > +static int smpro_read_string(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, const char **str) > +{ > + switch (type) { > + case hwmon_temp: > + switch (attr) { > + case hwmon_temp_label: > + *str = temperature[channel].label; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > + break; > + > + case hwmon_in: > + switch (attr) { > + case hwmon_in_label: > + *str = voltage[channel].label; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > + break; > + > + case hwmon_curr: > + switch (attr) { > + case hwmon_curr_label: > + *str = curr_sensor[channel].label; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > + break; > + > + case hwmon_power: > + switch (attr) { > + case hwmon_power_label: > + *str = power[channel].label; > + return 0; > + default: > + return -EOPNOTSUPP; > + } > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + return -EOPNOTSUPP; If you are returning -ENOPSUPP by default, might as well replace all the same returns above with break; > +} > + > +static umode_t smpro_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + const struct smpro_hwmon *hwmon = data; > + unsigned int value; > + int ret; > + > + switch (type) { > + case hwmon_temp: > + switch (attr) { > + case hwmon_temp_input: > + case hwmon_temp_label: > + case hwmon_temp_crit: > + ret = regmap_read(hwmon->regmap, temperature[channel].reg, &value); > + if (ret || value == 0xFFFF) > + return 0; > + break; > + } > + default: > + break; > + } > + > + return 0444; > +} > + > +static const struct hwmon_channel_info *smpro_info[] = { > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, > + HWMON_T_INPUT | HWMON_T_LABEL), > + HWMON_CHANNEL_INFO(in, > + HWMON_I_INPUT | HWMON_I_LABEL, > + HWMON_I_INPUT | HWMON_I_LABEL, > + HWMON_I_INPUT | HWMON_I_LABEL, > + HWMON_I_INPUT | HWMON_I_LABEL, > + HWMON_I_INPUT | HWMON_I_LABEL), > + HWMON_CHANNEL_INFO(power, > + HWMON_P_INPUT | HWMON_P_LABEL, > + HWMON_P_INPUT | HWMON_P_LABEL, > + HWMON_P_INPUT | HWMON_P_LABEL, > + HWMON_P_INPUT | HWMON_P_LABEL, > + HWMON_P_INPUT | HWMON_P_LABEL), > + HWMON_CHANNEL_INFO(curr, > + HWMON_C_INPUT | HWMON_C_LABEL, > + HWMON_C_INPUT | HWMON_C_LABEL, > + HWMON_C_INPUT | HWMON_C_LABEL, > + HWMON_C_INPUT | HWMON_C_LABEL, > + HWMON_C_INPUT | HWMON_C_LABEL), > + NULL > +}; > + > +static const struct hwmon_ops smpro_hwmon_ops = { > + .is_visible = smpro_is_visible, > + .read = smpro_read, > + .write = smpro_write, > + .read_string = smpro_read_string, > +}; > + > +static const struct hwmon_chip_info smpro_chip_info = { > + .ops = &smpro_hwmon_ops, > + .info = smpro_info, > +}; > + > +static bool is_valid_id(struct regmap *regmap) > +{ > + unsigned int val; > + int ret; > + > + ret = regmap_read(regmap, MANUFACTURER_ID_REG, &val); > + > + return (ret || (val != AMPERE_MANUFACTURER_ID)) ? false : true; I am quite concerned about this: The calling code will translate it to -EPROBE_DEFER even if the manufacturer ID is wrong. It should return -ENODEV in that case. There should be a better means to determine if the controller is not available at all, or not yet. > +} > + > +static int smpro_hwmon_probe(struct platform_device *pdev) > +{ > + struct smpro_hwmon *hwmon; > + struct device *hwmon_dev; > + > + hwmon = devm_kzalloc(&pdev->dev, sizeof(struct smpro_hwmon), GFP_KERNEL); > + if (!hwmon) > + return -ENOMEM; > + > + hwmon->regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!hwmon->regmap) > + return -ENODEV; > + > + /* Check for valid ID */ > + if (!is_valid_id(hwmon->regmap)) > + return -EPROBE_DEFER; > + > + hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, "smpro_hwmon", > + hwmon, &smpro_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct of_device_id smpro_hwmon_of_match[] = { > + { .compatible = "ampere,ac01-hwmon" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, smpro_hwmon_of_match); > + > +static struct platform_driver smpro_hwmon_driver = { > + .probe = smpro_hwmon_probe, > + .driver = { > + .name = "smpro-hwmon", > + .of_match_table = smpro_hwmon_of_match, > + }, > +}; > + > +module_platform_driver(smpro_hwmon_driver); > + > +MODULE_AUTHOR("Thu Nguyen <thu@os.amperecomputing.com>"); > +MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>"); > +MODULE_DESCRIPTION("Ampere Altra SMPro hwmon driver"); > +MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver 2021-03-30 1:43 ` Guenter Roeck @ 2021-04-07 7:41 ` Quan Nguyen 2021-04-07 12:11 ` Guenter Roeck 0 siblings, 1 reply; 12+ messages in thread From: Quan Nguyen @ 2021-04-07 7:41 UTC (permalink / raw) To: Guenter Roeck, Joel Stanley, Andrew Jeffery, Jean Delvare, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo On 30/03/2021 08:43, Guenter Roeck wrote: > On 3/28/21 6:52 PM, Quan Nguyen wrote: >> This commit adds support for Ampere SMpro hwmon driver. This driver >> supports accessing various CPU sensors provided by the SMpro co-processor >> including temperature, power, voltages, and current. >> >> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> >> --- >> drivers/hwmon/Kconfig | 8 + >> drivers/hwmon/Makefile | 1 + >> drivers/hwmon/smpro-hwmon.c | 494 ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 503 insertions(+) >> create mode 100644 drivers/hwmon/smpro-hwmon.c >> >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig >> index 0ddc974b102e..ba4b5a911baf 100644 >> --- a/drivers/hwmon/Kconfig >> +++ b/drivers/hwmon/Kconfig >> @@ -67,6 +67,14 @@ config SENSORS_ABITUGURU3 >> This driver can also be built as a module. If so, the module >> will be called abituguru3. >> >> +config SENSORS_SMPRO >> + tristate "Ampere's Altra SMpro hardware monitoring driver" >> + depends on MFD_SMPRO >> + help >> + If you say yes here you get support for the thermal, voltage, >> + current and power sensors of Ampere's Altra processor family SoC >> + with SMpro co-processor. >> + >> config SENSORS_AD7314 >> tristate "Analog Devices AD7314 and compatibles" >> depends on SPI >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >> index 59e78bc212cf..b25391f9c651 100644 >> --- a/drivers/hwmon/Makefile >> +++ b/drivers/hwmon/Makefile >> @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_SHT3x) += sht3x.o >> obj-$(CONFIG_SENSORS_SHTC1) += shtc1.o >> obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o >> obj-$(CONFIG_SENSORS_SMM665) += smm665.o >> +obj-$(CONFIG_SENSORS_SMPRO) += smpro-hwmon.o >> obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o >> obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o >> obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o >> diff --git a/drivers/hwmon/smpro-hwmon.c b/drivers/hwmon/smpro-hwmon.c >> new file mode 100644 >> index 000000000000..4277736ebc6e >> --- /dev/null >> +++ b/drivers/hwmon/smpro-hwmon.c >> @@ -0,0 +1,494 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Ampere Computing SoC's SMPro Hardware Monitoring Driver >> + * >> + * Copyright (c) 2021, Ampere Computing LLC >> + */ >> +#include <linux/bitfield.h> >> +#include <linux/hwmon.h> >> +#include <linux/hwmon-sysfs.h> >> +#include <linux/kernel.h> >> +#include <linux/mod_devicetable.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/property.h> >> +#include <linux/regmap.h> >> + >> +/* Identification Registers */ >> +#define MANUFACTURER_ID_REG 0x02 >> +#define AMPERE_MANUFACTURER_ID 0xCD3A >> + >> +/* Logical Power Sensor Registers */ >> +#define SOC_TEMP_REG 0x10 >> +#define SOC_VRD_TEMP_REG 0x11 >> +#define DIMM_VRD_TEMP_REG 0x12 >> +#define CORE_VRD_TEMP_REG 0x13 >> +#define CH0_DIMM_TEMP_REG 0x14 >> +#define CH1_DIMM_TEMP_REG 0x15 >> +#define CH2_DIMM_TEMP_REG 0x16 >> +#define CH3_DIMM_TEMP_REG 0x17 >> +#define CH4_DIMM_TEMP_REG 0x18 >> +#define CH5_DIMM_TEMP_REG 0x19 >> +#define CH6_DIMM_TEMP_REG 0x1A >> +#define CH7_DIMM_TEMP_REG 0x1B >> +#define RCA_VRD_TEMP_REG 0x1C >> + >> +#define CORE_VRD_PWR_REG 0x20 >> +#define SOC_PWR_REG 0x21 >> +#define DIMM_VRD1_PWR_REG 0x22 >> +#define DIMM_VRD2_PWR_REG 0x23 >> +#define CORE_VRD_PWR_MW_REG 0x26 >> +#define SOC_PWR_MW_REG 0x27 >> +#define DIMM_VRD1_PWR_MW_REG 0x28 >> +#define DIMM_VRD2_PWR_MW_REG 0x29 >> +#define RCA_VRD_PWR_REG 0x2A >> +#define RCA_VRD_PWR_MW_REG 0x2B >> + >> +#define MEM_HOT_THRESHOLD_REG 0x32 >> +#define SOC_VR_HOT_THRESHOLD_REG 0x33 >> +#define CORE_VRD_VOLT_REG 0x34 >> +#define SOC_VRD_VOLT_REG 0x35 >> +#define DIMM_VRD1_VOLT_REG 0x36 >> +#define DIMM_VRD2_VOLT_REG 0x37 >> +#define RCA_VRD_VOLT_REG 0x38 >> + >> +#define CORE_VRD_CURR_REG 0x39 >> +#define SOC_VRD_CURR_REG 0x3A >> +#define DIMM_VRD1_CURR_REG 0x3B >> +#define DIMM_VRD2_CURR_REG 0x3C >> +#define RCA_VRD_CURR_REG 0x3D >> + >> +struct smpro_hwmon { >> + struct regmap *regmap; >> +}; >> + >> +struct smpro_sensor { >> + const u8 reg; >> + const u8 reg_ext; >> + const char *label; >> +}; >> + >> +static const struct smpro_sensor temperature[] = { >> + { >> + .reg = SOC_TEMP_REG, >> + .label = "temp1 SoC" >> + }, >> + { >> + .reg = SOC_VRD_TEMP_REG, >> + .label = "temp2 SoC VRD" >> + }, >> + { >> + .reg = DIMM_VRD_TEMP_REG, >> + .label = "temp3 DIMM VRD" >> + }, >> + { >> + .reg = CORE_VRD_TEMP_REG, >> + .label = "temp4 CORE VRD" >> + }, >> + { >> + .reg = CH0_DIMM_TEMP_REG, >> + .label = "temp5 CH0 DIMM" >> + }, >> + { >> + .reg = CH1_DIMM_TEMP_REG, >> + .label = "temp6 CH1 DIMM" >> + }, >> + { >> + .reg = CH2_DIMM_TEMP_REG, >> + .label = "temp7 CH2 DIMM" >> + }, >> + { >> + .reg = CH3_DIMM_TEMP_REG, >> + .label = "temp8 CH3 DIMM" >> + }, >> + { >> + .reg = CH4_DIMM_TEMP_REG, >> + .label = "temp9 CH4 DIMM" >> + }, >> + { >> + .reg = CH5_DIMM_TEMP_REG, >> + .label = "temp10 CH5 DIMM" >> + }, >> + { >> + .reg = CH6_DIMM_TEMP_REG, >> + .label = "temp11 CH6 DIMM" >> + }, >> + { >> + .reg = CH7_DIMM_TEMP_REG, >> + .label = "temp12 CH7 DIMM" >> + }, >> + { >> + .reg = RCA_VRD_TEMP_REG, >> + .label = "temp13 RCA VRD" >> + }, >> +}; >> + >> +static const struct smpro_sensor voltage[] = { >> + { >> + .reg = CORE_VRD_VOLT_REG, >> + .label = "vout0 CORE VRD" >> + }, >> + { >> + .reg = SOC_VRD_VOLT_REG, >> + .label = "vout1 SoC VRD" >> + }, >> + { >> + .reg = DIMM_VRD1_VOLT_REG, >> + .label = "vout2 DIMM VRD1" >> + }, >> + { >> + .reg = DIMM_VRD2_VOLT_REG, >> + .label = "vout3 DIMM VRD2" >> + }, >> + { >> + .reg = RCA_VRD_VOLT_REG, >> + .label = "vout4 RCA VRD" >> + }, >> +}; >> + >> +static const struct smpro_sensor curr_sensor[] = { >> + { >> + .reg = CORE_VRD_CURR_REG, >> + .label = "iout1 CORE VRD" >> + }, >> + { >> + .reg = SOC_VRD_CURR_REG, >> + .label = "iout2 SoC VRD" >> + }, >> + { >> + .reg = DIMM_VRD1_CURR_REG, >> + .label = "iout3 DIMM VRD1" >> + }, >> + { >> + .reg = DIMM_VRD2_CURR_REG, >> + .label = "iout4 DIMM VRD2" >> + }, >> + { >> + .reg = RCA_VRD_CURR_REG, >> + .label = "iout5 RCA VRD" >> + }, >> +}; >> + >> +static const struct smpro_sensor power[] = { >> + { >> + .reg = CORE_VRD_PWR_REG, >> + .reg_ext = CORE_VRD_PWR_MW_REG, >> + .label = "power1 CORE VRD" >> + }, >> + { >> + .reg = SOC_PWR_REG, >> + .reg_ext = SOC_PWR_MW_REG, >> + .label = "power2 SoC" >> + }, >> + { >> + .reg = DIMM_VRD1_PWR_REG, >> + .reg_ext = DIMM_VRD1_PWR_MW_REG, >> + .label = "power3 DIMM VRD1" >> + }, >> + { >> + .reg = DIMM_VRD2_PWR_REG, >> + .reg_ext = DIMM_VRD2_PWR_MW_REG, >> + .label = "power4 DIMM VRD2" >> + }, >> + { >> + .reg = RCA_VRD_PWR_REG, >> + .reg_ext = RCA_VRD_PWR_MW_REG, >> + .label = "power5 RCA VRD" >> + }, >> +}; >> + >> +static int smpro_read_temp(struct device *dev, u32 attr, int channel, long *val) >> +{ >> + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); >> + unsigned int value; >> + int ret; >> + >> + switch (attr) { >> + case hwmon_temp_input: >> + ret = regmap_read(hwmon->regmap, >> + temperature[channel].reg, &value); >> + if (ret) >> + return ret; >> + *val = (value & 0x1ff) * 1000; >> + break; >> + case hwmon_temp_crit: >> + if (temperature[channel].reg == SOC_VRD_TEMP_REG) { >> + ret = regmap_read(hwmon->regmap, SOC_VR_HOT_THRESHOLD_REG, &value); >> + if (ret) >> + return ret; >> + *val = (value & 0x1ff) * 1000; >> + } else { >> + /* Report same MEM HOT threshold across DIMM channels */ >> + ret = regmap_read(hwmon->regmap, MEM_HOT_THRESHOLD_REG, &value); >> + if (ret) >> + return ret; >> + *val = (value & 0x1ff) * 1000; >> + } > > To avoid code duplication: > > reg = temperature[channel].reg == SOC_VRD_TEMP_REG ? SOC_VR_HOT_THRESHOLD_REG : MEM_HOT_THRESHOLD_REG; > ret = regmap_read(hwmon->regmap, reg, &value); > if (ret) > return ret; > > But then why don't you just use reg_ext to store SOC_VR_HOT_THRESHOLD_REG > or MEM_HOT_THRESHOLD_REG ? It is already available, after all, and with it > the code could be simplified to > > ret = regmap_read(hwmon->regmap, temperature[channel].reg_ext, &value); > if (ret) > return ret; > Thank you for the comment. Will change code follow this suggestion, will include in next version > I don't have a datasheet, but I do wonder what is in bit 9..15. Any idea ? > Main question is if there is a sign bit, as theoretic as it may be. > The original intention was to use this as 9-bit 2-complement value follow LM75, but the fact is that the operation temperature is 0-125 C degree, so we simply use it as-is. >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + return 0; >> +} >> + >> +static int smpro_read_in(struct device *dev, u32 attr, int channel, long *val) >> +{ >> + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); >> + unsigned int value; >> + int ret; >> + >> + switch (attr) { >> + case hwmon_in_input: >> + ret = regmap_read(hwmon->regmap, voltage[channel].reg, &value); >> + if (ret < 0) >> + return ret; >> + /* Scale reported by the hardware is 1mV */ >> + *val = value & 0x7fff; > > What is in bit 15 ? > This is 15-bit voltage in mV so the bit 15 (0-15) is unused. >> + return 0; >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int smpro_read_curr(struct device *dev, u32 attr, int channel, long *val) >> +{ >> + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); >> + unsigned int value; >> + int ret; >> + >> + switch (attr) { >> + case hwmon_curr_input: >> + ret = regmap_read(hwmon->regmap, curr_sensor[channel].reg, &value); >> + if (ret < 0) >> + return ret; >> + /* Scale reported by the hardware is 1mA */ >> + *val = value & 0x7fff; >> + return 0; >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int smpro_read_power(struct device *dev, u32 attr, int channel, long *val_pwr) >> +{ >> + struct smpro_hwmon *hwmon = dev_get_drvdata(dev); >> + unsigned int val = 0, val_mw = 0; >> + int ret; >> + >> + switch (attr) { >> + case hwmon_power_input: >> + ret = regmap_read(hwmon->regmap, power[channel].reg, &val); >> + if (ret) >> + return ret; >> + >> + ret = regmap_read(hwmon->regmap, power[channel].reg_ext, &val_mw); >> + if (ret) >> + return ret; >> + >> + *val_pwr = val * 1000000 + val_mw * 1000; >> + return 0; >> + >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int smpro_read(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long *val) >> +{ >> + switch (type) { >> + case hwmon_temp: >> + return smpro_read_temp(dev, attr, channel, val); >> + case hwmon_in: >> + return smpro_read_in(dev, attr, channel, val); >> + case hwmon_power: >> + return smpro_read_power(dev, attr, channel, val); >> + case hwmon_curr: >> + return smpro_read_curr(dev, attr, channel, val); >> + default: >> + return -EOPNOTSUPP; >> + } >> +} >> + >> +static int smpro_write(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long val) >> +{ >> + return -EOPNOTSUPP; >> +} > > There are no writeable attributes, thus the write function is not needed. > Agree, will remove in next version >> + >> +static int smpro_read_string(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, const char **str) >> +{ >> + switch (type) { >> + case hwmon_temp: >> + switch (attr) { >> + case hwmon_temp_label: >> + *str = temperature[channel].label; >> + return 0; >> + default: >> + return -EOPNOTSUPP; >> + } >> + break; >> + >> + case hwmon_in: >> + switch (attr) { >> + case hwmon_in_label: >> + *str = voltage[channel].label; >> + return 0; >> + default: >> + return -EOPNOTSUPP; >> + } >> + break; >> + >> + case hwmon_curr: >> + switch (attr) { >> + case hwmon_curr_label: >> + *str = curr_sensor[channel].label; >> + return 0; >> + default: >> + return -EOPNOTSUPP; >> + } >> + break; >> + >> + case hwmon_power: >> + switch (attr) { >> + case hwmon_power_label: >> + *str = power[channel].label; >> + return 0; >> + default: >> + return -EOPNOTSUPP; >> + } >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + return -EOPNOTSUPP; > > If you are returning -ENOPSUPP by default, might as well replace > all the same returns above with break; > Yes, will fix as you suggested. Will include in next version >> +} >> + >> +static umode_t smpro_is_visible(const void *data, enum hwmon_sensor_types type, >> + u32 attr, int channel) >> +{ >> + const struct smpro_hwmon *hwmon = data; >> + unsigned int value; >> + int ret; >> + >> + switch (type) { >> + case hwmon_temp: >> + switch (attr) { >> + case hwmon_temp_input: >> + case hwmon_temp_label: >> + case hwmon_temp_crit: >> + ret = regmap_read(hwmon->regmap, temperature[channel].reg, &value); >> + if (ret || value == 0xFFFF) >> + return 0; >> + break; >> + } >> + default: >> + break; >> + } >> + >> + return 0444; >> +} >> + >> +static const struct hwmon_channel_info *smpro_info[] = { >> + HWMON_CHANNEL_INFO(temp, >> + HWMON_T_INPUT | HWMON_T_LABEL, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL, >> + HWMON_T_INPUT | HWMON_T_LABEL, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT, >> + HWMON_T_INPUT | HWMON_T_LABEL), >> + HWMON_CHANNEL_INFO(in, >> + HWMON_I_INPUT | HWMON_I_LABEL, >> + HWMON_I_INPUT | HWMON_I_LABEL, >> + HWMON_I_INPUT | HWMON_I_LABEL, >> + HWMON_I_INPUT | HWMON_I_LABEL, >> + HWMON_I_INPUT | HWMON_I_LABEL), >> + HWMON_CHANNEL_INFO(power, >> + HWMON_P_INPUT | HWMON_P_LABEL, >> + HWMON_P_INPUT | HWMON_P_LABEL, >> + HWMON_P_INPUT | HWMON_P_LABEL, >> + HWMON_P_INPUT | HWMON_P_LABEL, >> + HWMON_P_INPUT | HWMON_P_LABEL), >> + HWMON_CHANNEL_INFO(curr, >> + HWMON_C_INPUT | HWMON_C_LABEL, >> + HWMON_C_INPUT | HWMON_C_LABEL, >> + HWMON_C_INPUT | HWMON_C_LABEL, >> + HWMON_C_INPUT | HWMON_C_LABEL, >> + HWMON_C_INPUT | HWMON_C_LABEL), >> + NULL >> +}; >> + >> +static const struct hwmon_ops smpro_hwmon_ops = { >> + .is_visible = smpro_is_visible, >> + .read = smpro_read, >> + .write = smpro_write, >> + .read_string = smpro_read_string, >> +}; >> + >> +static const struct hwmon_chip_info smpro_chip_info = { >> + .ops = &smpro_hwmon_ops, >> + .info = smpro_info, >> +}; >> + >> +static bool is_valid_id(struct regmap *regmap) >> +{ >> + unsigned int val; >> + int ret; >> + >> + ret = regmap_read(regmap, MANUFACTURER_ID_REG, &val); >> + >> + return (ret || (val != AMPERE_MANUFACTURER_ID)) ? false : true; > > I am quite concerned about this: The calling code will translate it to > -EPROBE_DEFER even if the manufacturer ID is wrong. It should return > -ENODEV in that case. There should be a better means to determine if the > controller is not available at all, or not yet. > Yes, I agree Will fix in next version: + if the regmap_read return error, return -EPROBE_DEFER + if manufacturer ID is wrong, return -ENODEV >> +} >> + >> +static int smpro_hwmon_probe(struct platform_device *pdev) >> +{ >> + struct smpro_hwmon *hwmon; >> + struct device *hwmon_dev; >> + >> + hwmon = devm_kzalloc(&pdev->dev, sizeof(struct smpro_hwmon), GFP_KERNEL); >> + if (!hwmon) >> + return -ENOMEM; >> + >> + hwmon->regmap = dev_get_regmap(pdev->dev.parent, NULL); >> + if (!hwmon->regmap) >> + return -ENODEV; >> + >> + /* Check for valid ID */ >> + if (!is_valid_id(hwmon->regmap)) >> + return -EPROBE_DEFER; >> + >> + hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, "smpro_hwmon", >> + hwmon, &smpro_chip_info, NULL); >> + >> + return PTR_ERR_OR_ZERO(hwmon_dev); >> +} >> + >> +static const struct of_device_id smpro_hwmon_of_match[] = { >> + { .compatible = "ampere,ac01-hwmon" }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, smpro_hwmon_of_match); >> + >> +static struct platform_driver smpro_hwmon_driver = { >> + .probe = smpro_hwmon_probe, >> + .driver = { >> + .name = "smpro-hwmon", >> + .of_match_table = smpro_hwmon_of_match, >> + }, >> +}; >> + >> +module_platform_driver(smpro_hwmon_driver); >> + >> +MODULE_AUTHOR("Thu Nguyen <thu@os.amperecomputing.com>"); >> +MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>"); >> +MODULE_DESCRIPTION("Ampere Altra SMPro hwmon driver"); >> +MODULE_LICENSE("GPL v2"); >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver 2021-04-07 7:41 ` Quan Nguyen @ 2021-04-07 12:11 ` Guenter Roeck 2021-04-08 12:02 ` Quan Nguyen 0 siblings, 1 reply; 12+ messages in thread From: Guenter Roeck @ 2021-04-07 12:11 UTC (permalink / raw) To: Quan Nguyen, Joel Stanley, Andrew Jeffery, Jean Delvare, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo On 4/7/21 12:41 AM, Quan Nguyen wrote: [ ... ] >> >> But then why don't you just use reg_ext to store SOC_VR_HOT_THRESHOLD_REG >> or MEM_HOT_THRESHOLD_REG ? It is already available, after all, and with it >> the code could be simplified to >> >> ret = regmap_read(hwmon->regmap, temperature[channel].reg_ext, &value); >> if (ret) >> return ret; >> > Thank you for the comment. > > Will change code follow this suggestion, will include in next version > >> I don't have a datasheet, but I do wonder what is in bit 9..15. Any idea ? >> Main question is if there is a sign bit, as theoretic as it may be. >> > The original intention was to use this as 9-bit 2-complement value follow LM75, but the fact is that the operation temperature is 0-125 C degree, so we simply use it as-is. > The operational temperature is not the question here. The question is if the chip _reports_ a sign. If it does, it should be handled, even if it is outside the operational range. The reported range is relevant here, not the operational range. After all, the chip won't really blow apart at -1 degrees C. Thanks, Guenter ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver 2021-04-07 12:11 ` Guenter Roeck @ 2021-04-08 12:02 ` Quan Nguyen 0 siblings, 0 replies; 12+ messages in thread From: Quan Nguyen @ 2021-04-08 12:02 UTC (permalink / raw) To: Guenter Roeck, Joel Stanley, Andrew Jeffery, Jean Delvare, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo On 07/04/2021 19:11, Guenter Roeck wrote: > On 4/7/21 12:41 AM, Quan Nguyen wrote: > [ ... ] >>> >>> But then why don't you just use reg_ext to store SOC_VR_HOT_THRESHOLD_REG >>> or MEM_HOT_THRESHOLD_REG ? It is already available, after all, and with it >>> the code could be simplified to >>> >>> ret = regmap_read(hwmon->regmap, temperature[channel].reg_ext, &value); >>> if (ret) >>> return ret; >>> >> Thank you for the comment. >> >> Will change code follow this suggestion, will include in next version >> >>> I don't have a datasheet, but I do wonder what is in bit 9..15. Any idea ? >>> Main question is if there is a sign bit, as theoretic as it may be. >>> >> The original intention was to use this as 9-bit 2-complement value follow LM75, but the fact is that the operation temperature is 0-125 C degree, so we simply use it as-is. >> > > The operational temperature is not the question here. The question is if the > chip _reports_ a sign. If it does, it should be handled, even if it is outside > the operational range. The reported range is relevant here, not the operational > range. After all, the chip won't really blow apart at -1 degrees C. > I think I've got it, will handle the sign bit in next version. -Quan ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] docs: hwmon: (smpro-hwmon) Add documentation 2021-03-29 1:52 [PATCH v2 0/4] Add Ampere's Altra SMPro hwmon driver Quan Nguyen ` (2 preceding siblings ...) 2021-03-29 1:52 ` [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver Quan Nguyen @ 2021-03-29 1:52 ` Quan Nguyen 3 siblings, 0 replies; 12+ messages in thread From: Quan Nguyen @ 2021-03-29 1:52 UTC (permalink / raw) To: Joel Stanley, Andrew Jeffery, Jean Delvare, Guenter Roeck, Rob Herring, Lee Jones, Jonathan Corbet, linux-hwmon, devicetree, linux-kernel, linux-doc, linux-aspeed, openbmc Cc: Open Source Submission, Thang Q . Nguyen, Phong Vo Add documentation for the Ampere(R)'s Altra(R) SMpro hwmon driver. Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> --- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/smpro-hwmon.rst | 101 ++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 Documentation/hwmon/smpro-hwmon.rst diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index 48bfa7887dd4..3e3631b253b6 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -166,6 +166,7 @@ Hardware Monitoring Kernel Drivers sis5595 sl28cpld smm665 + smpro-hwmon smsc47b397 smsc47m192 smsc47m1 diff --git a/Documentation/hwmon/smpro-hwmon.rst b/Documentation/hwmon/smpro-hwmon.rst new file mode 100644 index 000000000000..f978b1370e16 --- /dev/null +++ b/Documentation/hwmon/smpro-hwmon.rst @@ -0,0 +1,101 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Kernel driver Ampere(R)'s Altra(R) SMpro hwmon +============================================== + +Supported chips: + + * Ampere(R) Altra(R) + + Prefix: 'smpro' + + Reference: Altra SoC BMC Interface Specification + +Author: Thu Nguyen <thu@os.amperecomputing.com> + +Description +----------- +This driver supports hardware monitoring for Ampere(R) Altra(R) SoC's based on the +SMpro co-processor (SMpro). +The following sensor types are supported by the driver: + + * temperature + * voltage + * current + * power + +The SMpro interface provides the registers to query the various sensors and +their values which are then exported to userspace by this driver. + +Usage Notes +----------- + +SMpro hwmon driver creates at least two sysfs files for each sensor. + +* File ``<sensor_type><idx>_label`` reports the sensor label. +* File ``<sensor_type><idx>_input`` returns the sensor value. + +The sysfs files are allocated in the SMpro root fs folder. +There is one root folder for each SMpro instance. + +When the SoC is turned off, the driver will fail to read registers +and return -ENXIO. + +Sysfs entries +------------- + +The following sysfs files are supported: + +* Ampere(R) Altra(R): + +============ ============= ====== =============================================== +Name Unit Perm Description +temp1_input milli Celsius RO SoC temperature +temp2_input milli Celsius RO Max temperature reported among SoC VRDs +temp2_crit milli Celsius RO SoC VRD HOT Threshold temperature +temp3_input milli Celsius RO Max temperature reported among DIMM VRDs +temp4_input milli Celsius RO Max temperature reported among Core VRDs +temp5_input milli Celsius RO Temperature of DIMM0 on CH0 +temp5_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp6_input milli Celsius RO Temperature of DIMM0 on CH1 +temp6_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp7_input milli Celsius RO Temperature of DIMM0 on CH2 +temp7_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp8_input milli Celsius RO Temperature of DIMM0 on CH3 +temp8_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp9_input milli Celsius RO Temperature of DIMM0 on CH4 +temp9_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp10_input milli Celsius RO Temperature of DIMM0 on CH5 +temp10_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp11_input milli Celsius RO Temperature of DIMM0 on CH6 +temp11_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp12_input milli Celsius RO Temperature of DIMM0 on CH7 +temp12_crit milli Celsius RO MEM HOT Threshold for all DIMMs +temp13_input milli Celsius RO Max temperature reported among RCA VRDs +in0_input milli Volts RO Core voltage +in1_input milli Volts RO SoC voltage +in2_input milli Volts RO DIMM VRD1 voltage +in3_input milli Volts RO DIMM VRD2 voltage +in4_input milli Volts RO RCA VRD voltage +cur1_input milli Amperes RO Core VRD current +cur2_input milli Amperes RO SoC VRD current +cur3_input milli Amperes RO DIMM VRD1 current +cur4_input milli Amperes RO DIMM VRD2 current +cur5_input milli Amperes RO RCA VRD current +power1_input micro Watts RO Core VRD power +power2_input micro Watts RO SoC VRD power +power3_input micro Watts RO DIMM VRD1 power +power4_input micro Watts RO DIMM VRD2 power +power5_input micro Watts RO RCA VRD power +============ ============= ====== =============================================== + +Example:: + + # cat in0_input + 830 + # cat temp1_input + 37000 + # cat curr1_input + 9000 + # cat power5_input + 19500000 -- 2.28.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-04-14 10:04 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-29 1:52 [PATCH v2 0/4] Add Ampere's Altra SMPro hwmon driver Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 1/4] dt-bindings: mfd: Add bindings for Ampere Altra SMPro drivers Quan Nguyen 2021-03-30 21:14 ` Rob Herring 2021-04-07 9:42 ` Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 2/4] mfd: simple-mfd-i2c: Adds Ampere's Altra SMpro support Quan Nguyen 2021-04-14 10:03 ` Lee Jones 2021-03-29 1:52 ` [PATCH v2 3/4] hwmon: smpro: Add Ampere's Altra smpro-hwmon driver Quan Nguyen 2021-03-30 1:43 ` Guenter Roeck 2021-04-07 7:41 ` Quan Nguyen 2021-04-07 12:11 ` Guenter Roeck 2021-04-08 12:02 ` Quan Nguyen 2021-03-29 1:52 ` [PATCH v2 4/4] docs: hwmon: (smpro-hwmon) Add documentation Quan Nguyen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).