* [PATCH linux v3 0/3] Support for ASPEED AST2400/AST2500 PWM driver
@ 2016-11-24 9:26 Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support Jaghathiswari Rankappagounder Natarajan
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-11-24 9:26 UTC (permalink / raw)
To: openbmc, joel; +Cc: Jaghathiswari Rankappagounder Natarajan
Support for ASPEED AST2400/AST2500 PWM driver.
The AST2400/AST2500 PWM controller can support 8 PWM output ports.
There can be three different PWM clock types (Type M, N and O); each PWM
clock type can have different settings. Those three types just to have three
independent PWM sources.
Each PWM output port can have one of the three different PWM clock types.
There is a sysfs file through which the user can configure the duty cycle
for the particular PWM output port.
Added devicetree binding documentation for AST2400/AST2500 PWM support.
Added support in Zaius platform for 4 PWM output ports since it has
four fans.
Tested on Zaius board and observed that when duty cycle is lowered
then the fan speed is lowered and when the duty cycle is increased
then the fan speed increases.
Tested on AST2500 EVB board and observed the PWM output pulses come
correctly based on the given settings using Logic Saleae Analyzer.
Jaghathiswari Rankappagounder Natarajan (3):
Documentation: dt-bindings: Document binding for ASPEED AST2400/2500
PWM support
drivers: hwmon: Hwmon driver for ASPEED AST2400/2500 PWM support
arm: dts: Add dt-binding to support ASPEED AST2500 PWM output on zaius
.../devicetree/bindings/hwmon/aspeed-pwm.txt | 104 ++++
Documentation/hwmon/aspeed-pwm | 18 +
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 40 ++
arch/arm/boot/dts/aspeed-g5.dtsi | 6 +
drivers/hwmon/Kconfig | 13 +
drivers/hwmon/Makefile | 3 +-
drivers/hwmon/aspeed-pwm.c | 660 +++++++++++++++++++++
7 files changed, 842 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
create mode 100644 Documentation/hwmon/aspeed-pwm
create mode 100644 drivers/hwmon/aspeed-pwm.c
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support
2016-11-24 9:26 [PATCH linux v3 0/3] Support for ASPEED AST2400/AST2500 PWM driver Jaghathiswari Rankappagounder Natarajan
@ 2016-11-24 9:26 ` Jaghathiswari Rankappagounder Natarajan
2016-11-28 6:13 ` Joel Stanley
2016-11-24 9:26 ` [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver " Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 3/3] arm: dts: Add dt-binding to support ASPEED AST2500 PWM output on zaius Jaghathiswari Rankappagounder Natarajan
2 siblings, 1 reply; 7+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-11-24 9:26 UTC (permalink / raw)
To: openbmc, joel; +Cc: Jaghathiswari Rankappagounder Natarajan
This binding provides interface for adding values related to ASPEED
AST2400/2500 PWM support.
The parent node indicates a PWM controller. The parent node can have
upto 8 child nodes. Each node indicates a PWM output port.
PWM clock types M, N and 0 are three types just to have three independent
PWM sources. Each port can be assigned a different PWM clock type.
Each port can have the duty-cycle set to a different value.
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
v2:
- Made binding documentation update as a separate patch.
- Included AST2500 in the description.
- Changed format to "aspeed,ast2500-pwm".
- Included explanation for types M ,N and O and calculation example to
- determine L, H, and period bits.
v3:
- Included struct clk and phandle reference to clock node.
- Removed clock calculation description from the bindings document.
- Added some whitespace and improved style of binding document.
.../devicetree/bindings/hwmon/aspeed-pwm.txt | 104 +++++++++++++++++++++
1 file changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
new file mode 100644
index 0000000..11a007c
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
@@ -0,0 +1,104 @@
+ASPEED AST2400/AST2500 PWM device driver
+
+The ASPEED PWM controller can support 8 PWM outputs.
+PWM clock types M, N and 0 are three types just to have three
+independent PWM sources. Each could be assigned to the 8 PWM port
+with it's own settings.
+
+Required properties:
+- #address-cells : should be 1.
+
+- #size-cells : should be 1.
+
+- reg : address and length of the register set for the device.
+
+- pinctrl-names : a pinctrl state named "default" must be defined.
+
+- pinctrl-0 : phandle referencing pin configuration of the ASPEED PWM ports.
+
+- compatible : should be "aspeed,ast2400-pwm" for AST2400 or
+ "aspeed,ast2500-pwm" for AST2500.
+
+- clocks : phandle reference to clocks. upto three fixed clocks providing
+ three different input clock frequencies. each clock frequency indicates a
+ desired PWM frequency.
+
+ the first PWM frequency in combination with type M PWM period value
+ (described below) and source clock frequency will be used to calculate the
+ type M PWM clock division high and low values.
+ the second PWM frequency in combination with type N PWM period value and
+ source clock frequency will be used to calculate the type N PWM clock
+ division high and low values.
+ the third PWM frequency in combination with type O PWM period value and
+ source clock frequency will be used to calculate the type O PWM clock
+ division high and low values.
+
+ Since atleast one PWM frequency or one PWM clock type(type M PWM clock type)
+ is required, the first clock is required. the other two are optional.
+
+- pwm_typem_period : indicates type M PWM period. integer value in the range
+ 0 to 255.
+
+Optional properties:
+- pwm_typen_period : indicates type N PWM period. integer value in the range
+ 0 to 255.
+
+- pwm_typeo_period : indicates type O PWM period. integer value in the range
+ 0 to 255.
+
+Each subnode represents a PWM output port.
+
+Subnode Format
+--------------
+Required properties:
+- pwm_port : indicates the PWM port. values 0 through 7 indicate PWM ports
+ A through H respectively.
+
+- pwm_enable : option to enable the PWM port indicated above. 0 is enable
+ and 1 is disable.
+
+- pwm_type : indicates the clock type for the PWM port. value 0 is type M
+ PWM clock. value 1 is type N PWM clock. value 2 is type O PWM clock.
+
+- pwm_fan_ctrl : pulse width modulation fan control. integer value
+ in the range 0 to 255. 255 is max or 100%.
+
+Additional details are available in the detailed datasheet for the device
+AST2400/AST2500.
+
+Examples:
+
+clocks {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pwm_typem_clock: fixedclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <25000>;
+ }
+}
+
+pwm: pwm-controller@1e786000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x1E786000 0x78>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>
+ compatible = "aspeed,ast2500-pwm";
+ clocks = <&pwm_typem_clock>;
+ pwm_typem_period = <95>;
+ pwm_port0 {
+ pwm_port = /bits/ 8 <0x00>;
+ pwm_enable = /bits/ 8 <0x01>;
+ pwm_type = /bits/ 8 <0x00>;
+ pwm_fan_ctrl = /bits/ 8 <0x5A>;
+ };
+ pwm_port1 {
+ pwm_port = /bits/ 8 <0x01>;
+ pwm_enable = /bits/ 8 <0x01>;
+ pwm_type = /bits/ 8 <0x00>;
+ pwm_fan_ctrl = /bits/ 8 <0x5A>;
+ };
+};
+
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver for ASPEED AST2400/2500 PWM support
2016-11-24 9:26 [PATCH linux v3 0/3] Support for ASPEED AST2400/AST2500 PWM driver Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support Jaghathiswari Rankappagounder Natarajan
@ 2016-11-24 9:26 ` Jaghathiswari Rankappagounder Natarajan
2016-11-28 6:13 ` Joel Stanley
2016-11-24 9:26 ` [PATCH linux v3 3/3] arm: dts: Add dt-binding to support ASPEED AST2500 PWM output on zaius Jaghathiswari Rankappagounder Natarajan
2 siblings, 1 reply; 7+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-11-24 9:26 UTC (permalink / raw)
To: openbmc, joel; +Cc: Jaghathiswari Rankappagounder Natarajan
The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
PWM clock types M, N and 0 are three types just to have three independent PWM
sources. Each port can be assigned a different PWM clock type.
The device driver matches on the device tree node. The configuration values
are read from the device tree and written to the respective registers.
The driver provides a sysfs entry "pwm" through which the user can
configure the duty-cycle value (ranging from 0 to 100 percent).
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
v2:
- Merged the drivers for PWM controller and PWM device as one PWM driver.
v3:
- Mentioned that this driver is supported for AST2400 and AST2500.
- Provided explanation of why this is hwmon driver.
- Entered changelog in the correct place.
- Removed name at the top of the file.
- Corrected style of multi-line comments.
- Added GPL version 2 or later.
- Changed prefix as "aspeed".
- Changed val to bool as mentioned in comment.
- Removed switch statements and added a representation that would allow
- to look up the values without requiring a switch statement.
- Changed some variable names.
- Added enum for pwm_port and pwm_clock_type.
- Replaced hwmon name as "aspeed_pwm".
- Removed struct ast_pwm_controller_data allocation in the probe function.
- Used struct clk and related logic.
- Removed remove function.
- Moved header file content to c file.
- Removed some duplicate definitions not needed.
- Used BIT(x).
- Removed guarding () for integers in #define statements. Retained guarding
- for complex calculations; ow getting error when running checkpatch.pl script.
- Added documentation for hwmon driver.
Documentation/hwmon/aspeed-pwm | 18 ++
drivers/hwmon/Kconfig | 13 +
drivers/hwmon/Makefile | 3 +-
drivers/hwmon/aspeed-pwm.c | 660 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 692 insertions(+), 2 deletions(-)
create mode 100644 Documentation/hwmon/aspeed-pwm
create mode 100644 drivers/hwmon/aspeed-pwm.c
diff --git a/Documentation/hwmon/aspeed-pwm b/Documentation/hwmon/aspeed-pwm
new file mode 100644
index 0000000..f1affe0
--- /dev/null
+++ b/Documentation/hwmon/aspeed-pwm
@@ -0,0 +1,18 @@
+Kernel driver aspeed-pwm
+========================
+
+Supported chips:
+ ASPEED AST2400/2500
+
+Authors:
+ <jaghu@google.com>
+
+Description:
+------------
+The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
+PWM clock types M, N and 0 are three types just to have three independent PWM
+sources. Each port can be assigned a different PWM clock type.
+The device driver matches on the device tree node. The configuration values
+are read from the device tree and written to the respective registers.
+The driver provides a sysfs entry "pwm1" through which the user can
+configure the duty-cycle value (ranging from 0 to 255; 255 is 100 percent).
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d4de8d5..7f75b01 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -341,6 +341,19 @@ config SENSORS_ASB100
This driver can also be built as a module. If so, the module
will be called asb100.
+config SENSORS_ASPEED_PWM
+ tristate "ASPEED AST2400/AST2500 PWM driver"
+ help
+ This driver provides support for ASPEED AST2400/AST2500 PWM
+ controller and output ports. The ASPEED PWM controller can support 8
+ PWM outputs. PWM clock types M, N and 0 are three types just to have three
+ independent PWM sources. Each could be assigned to the 8 PWM port
+ with its own settings. The user can configure duty cycle through
+ the exposed hwmon sysfs entry "pwm".
+
+ This driver can also be built as a module. If so, the module
+ will be called aspeed-pwm.
+
config SENSORS_ATXP1
tristate "Attansic ATXP1 VID controller"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4455478..f39c057 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
+obj-$(CONFIG_SENSORS_ASPEED_PWM) += aspeed-pwm.o
obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
@@ -164,8 +165,6 @@ obj-$(CONFIG_SENSORS_W83L785TS) += w83l785ts.o
obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
-
obj-$(CONFIG_PMBUS) += pmbus/
ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
-
diff --git a/drivers/hwmon/aspeed-pwm.c b/drivers/hwmon/aspeed-pwm.c
new file mode 100644
index 0000000..1c96bbe
--- /dev/null
+++ b/drivers/hwmon/aspeed-pwm.c
@@ -0,0 +1,660 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+
+/* ASPEED PWM & FAN Register Definition */
+#define ASPEED_PTCR_CTRL 0x00
+#define ASPEED_PTCR_CLK_CTRL 0x04
+#define ASPEED_PTCR_DUTY0_CTRL 0x08
+#define ASPEED_PTCR_DUTY1_CTRL 0x0c
+#define ASPEED_PTCR_CTRL_EXT 0x40
+#define ASPEED_PTCR_CLK_CTRL_EXT 0x44
+#define ASPEED_PTCR_DUTY2_CTRL 0x48
+#define ASPEED_PTCR_DUTY3_CTRL 0x4c
+
+#define DUTY_CTRL_PWM2_FALL_POINT 24
+#define DUTY_CTRL_PWM2_RISE_POINT 16
+#define DUTY_CTRL_PWM1_FALL_POINT 8
+#define DUTY_CTRL_PWM1_RISE_POINT 0
+
+/* ASPEED_PTCR_CTRL : 0x00 - General Control Register */
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1 15
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2 6
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK (BIT(7) | BIT(15))
+
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1 14
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2 5
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK (BIT(6) | BIT(14))
+
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1 13
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2 4
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK (BIT(5) | BIT(13))
+
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1 12
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2 3
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK (BIT(4) | BIT(12))
+
+#define ASPEED_PTCR_CTRL_PWMD_EN (0x1 << 11)
+#define ASPEED_PTCR_CTRL_PWMC_EN (0x1 << 10)
+#define ASPEED_PTCR_CTRL_PWMB_EN (0x1 << 9)
+#define ASPEED_PTCR_CTRL_PWMA_EN (0x1 << 8)
+
+/*0:24Mhz, 1:MCLK */
+#define ASPEED_PTCR_CTRL_CLK_SRC 0x2
+#define ASPEED_PTCR_CTRL_CLK_EN 0x1
+
+/* ASPEED_PTCR_CLK_CTRL : 0x04 - Clock Control Register */
+/* TYPE N */
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT 24
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT_MASK (0xff << 24)
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_H 20
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_H_MASK (0xf << 20)
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_L 16
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_L_MASK (0xf << 16)
+/* TYPE M */
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT 8
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT_MASK (0xff << 8)
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_H 4
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_H_MASK (0xf << 4)
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_L 0
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_L_MASK 0xf
+
+/* ASPEED_PTCR_CTRL_EXT : 0x40 - General Control Extension #1 Register */
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1 15
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2 6
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK (BIT(7) | BIT(15))
+
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1 14
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2 5
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK (BIT(6) | BIT(14))
+
+
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1 13
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2 4
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK (BIT(5) | BIT(13))
+
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1 12
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2 3
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK (BIT(4) | BIT(12))
+
+#define ASPEED_PTCR_CTRL_PWMH_EN (0x1 << 11)
+#define ASPEED_PTCR_CTRL_PWMG_EN (0x1 << 10)
+#define ASPEED_PTCR_CTRL_PWMF_EN (0x1 << 9)
+#define ASPEED_PTCR_CTRL_PWME_EN (0x1 << 8)
+
+/* ASPEED_PTCR_CLK_EXT_CTRL : 0x44 - Clock Control Extension #1 Register */
+/* TYPE O */
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT 8
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT_MASK (0xff << 8)
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_H 4
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_H_MASK (0xf << 4)
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_L 0
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_L_MASK 0xf
+
+#define PWM_MAX 255
+
+#define MAX_HIGH_LOW_BIT 15
+
+struct aspeed_pwm_port_data {
+ u8 pwm_port;
+ u8 pwm_enable;
+ u8 pwm_type;
+ u8 pwm_fan_ctrl;
+ void __iomem *base;
+};
+
+enum pwm_clock_type { TYPEM, TYPEN, TYPEO };
+
+struct pwm_clock_type_params {
+ u32 l_value;
+ u32 l_mask;
+ u32 h_value;
+ u32 h_mask;
+ u32 unit_value;
+ u32 unit_mask;
+ u32 ctrl_reg_offset;
+};
+
+static const struct pwm_clock_type_params pwm_clock_type_params[] = {
+ [TYPEM] = {
+ .l_value = ASPEED_PTCR_CLK_CTRL_TYPEM_L,
+ .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_L_MASK,
+ .h_value = ASPEED_PTCR_CLK_CTRL_TYPEM_H,
+ .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_H_MASK,
+ .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT,
+ .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT_MASK,
+ .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL,
+ },
+ [TYPEN] = {
+ .l_value = ASPEED_PTCR_CLK_CTRL_TYPEN_L,
+ .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_L_MASK,
+ .h_value = ASPEED_PTCR_CLK_CTRL_TYPEN_H,
+ .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_H_MASK,
+ .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT,
+ .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT_MASK,
+ .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL,
+ },
+ [TYPEO] = {
+ .l_value = ASPEED_PTCR_CLK_CTRL_TYPEO_L,
+ .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_L_MASK,
+ .h_value = ASPEED_PTCR_CLK_CTRL_TYPEO_H,
+ .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_H_MASK,
+ .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT,
+ .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT_MASK,
+ .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL_EXT,
+ }
+};
+
+enum pwm_port { PWMA, PWMB, PWMC, PWMD, PWME, PWMF, PWMG, PWMH };
+
+struct pwm_port_params {
+ u32 pwm_en;
+ u32 ctrl_reg_offset;
+ u32 pwm_type_part1;
+ u32 pwm_type_part2;
+ u32 pwm_type_mask;
+ u32 duty_ctrl_rise_point;
+ u32 duty_ctrl_fall_point;
+ u32 duty_ctrl_reg_offset;
+ u8 duty_ctrl_calc_type;
+};
+
+static const struct pwm_port_params pwm_port_params[] = {
+ [PWMA] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMA_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY0_CTRL,
+ .duty_ctrl_calc_type = 0,
+ },
+ [PWMB] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMB_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY0_CTRL,
+ .duty_ctrl_calc_type = 1,
+ },
+ [PWMC] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMC_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY1_CTRL,
+ .duty_ctrl_calc_type = 0,
+ },
+ [PWMD] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMD_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY1_CTRL,
+ .duty_ctrl_calc_type = 1,
+ },
+ [PWME] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWME_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL_EXT,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY2_CTRL,
+ .duty_ctrl_calc_type = 0,
+ },
+ [PWMF] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMF_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL_EXT,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY2_CTRL,
+ .duty_ctrl_calc_type = 1,
+ },
+ [PWMG] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMG_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL_EXT,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY3_CTRL,
+ .duty_ctrl_calc_type = 0,
+ },
+ [PWMH] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMH_EN,
+ .ctrl_reg_offset = ASPEED_PTCR_CTRL_EXT,
+ .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1,
+ .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2,
+ .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY3_CTRL,
+ .duty_ctrl_calc_type = 1,
+ }
+};
+
+static inline void
+aspeed_pwm_write(void __iomem *base, u32 val, u32 reg)
+{
+ writel(val, base + reg);
+}
+
+static inline u32
+aspeed_pwm_read(void __iomem *base, u32 reg)
+{
+ u32 val = readl(base + reg);
+ return val;
+}
+
+static void
+aspeed_set_pwm_clock_enable(void __iomem *base, bool val)
+{
+ u32 reg_value = aspeed_pwm_read(base, ASPEED_PTCR_CTRL);
+
+ if (val)
+ reg_value |= ASPEED_PTCR_CTRL_CLK_EN;
+ else
+ reg_value &= ~ASPEED_PTCR_CTRL_CLK_EN;
+
+ aspeed_pwm_write(base, reg_value, ASPEED_PTCR_CTRL);
+}
+
+static void
+aspeed_set_pwm_clock_source(void __iomem *base, bool val)
+{
+ u32 reg_value = aspeed_pwm_read(base, ASPEED_PTCR_CTRL);
+
+ if (val)
+ reg_value |= ASPEED_PTCR_CTRL_CLK_SRC;
+ else
+ reg_value &= ~ASPEED_PTCR_CTRL_CLK_SRC;
+
+ aspeed_pwm_write(base, reg_value, ASPEED_PTCR_CTRL);
+}
+
+static void
+aspeed_set_pwm_clock_division_h(void __iomem *base, u8 pwm_clock_type,
+ u8 div_high)
+{
+ u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(base, reg_offset);
+
+ reg_value &= ~pwm_clock_type_params[pwm_clock_type].h_mask;
+ reg_value |= div_high << pwm_clock_type_params[pwm_clock_type].h_value;
+
+ aspeed_pwm_write(base, reg_value, reg_offset);
+}
+
+static void
+aspeed_set_pwm_clock_division_l(void __iomem *base, u8 pwm_clock_type,
+ u8 div_low)
+{
+ u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(base, reg_offset);
+
+ reg_value &= ~pwm_clock_type_params[pwm_clock_type].l_mask;
+ reg_value |= div_low << pwm_clock_type_params[pwm_clock_type].l_value;
+
+ aspeed_pwm_write(base, reg_value, reg_offset);
+}
+
+static void
+aspeed_set_pwm_clock_unit(void __iomem *base, u8 pwm_clock_type,
+ u8 unit)
+{
+ u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(base, reg_offset);
+
+ reg_value &= ~pwm_clock_type_params[pwm_clock_type].unit_mask;
+ reg_value |= unit << pwm_clock_type_params[pwm_clock_type].unit_value;
+
+ aspeed_pwm_write(base, reg_value, reg_offset);
+}
+
+static void
+aspeed_set_pwm_enable(struct aspeed_pwm_port_data *priv, bool enable)
+{
+ u8 pwm_port = priv->pwm_port;
+
+ u32 reg_offset = pwm_port_params[pwm_port].ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(priv->base, reg_offset);
+
+ if (enable)
+ reg_value |= pwm_port_params[pwm_port].pwm_en;
+ else
+ reg_value &= ~pwm_port_params[pwm_port].pwm_en;
+
+ aspeed_pwm_write(priv->base, reg_value, reg_offset);
+}
+
+static void
+aspeed_set_pwm_type(struct aspeed_pwm_port_data *priv, u8 type)
+{
+ u8 pwm_port = priv->pwm_port;
+
+ u32 reg_offset = pwm_port_params[pwm_port].ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(priv->base, reg_offset);
+
+ reg_value &= ~pwm_port_params[pwm_port].pwm_type_mask;
+ reg_value |= (type & 0x1) <<
+ pwm_port_params[pwm_port].pwm_type_part1;
+ reg_value |= (type & 0x2) <<
+ pwm_port_params[pwm_port].pwm_type_part2;
+
+ aspeed_pwm_write(priv->base, reg_value, reg_offset);
+}
+
+static void
+aspeed_set_pwm_duty_rising(struct aspeed_pwm_port_data *priv, u8 rising)
+{
+ u8 pwm_port = priv->pwm_port;
+
+ u32 reg_offset = pwm_port_params[pwm_port].duty_ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(priv->base, reg_offset);
+
+ reg_value &= ~(0xFF << pwm_port_params[pwm_port].duty_ctrl_rise_point);
+
+ if (pwm_port_params[pwm_port].duty_ctrl_calc_type == 0) {
+ reg_value |= rising;
+ } else if (pwm_port_params[pwm_port].duty_ctrl_calc_type == 1) {
+ reg_value |= (rising <<
+ pwm_port_params[pwm_port].duty_ctrl_rise_point);
+ }
+
+ aspeed_pwm_write(priv->base, reg_value, reg_offset);
+}
+
+static void
+aspeed_set_pwm_duty_falling(struct aspeed_pwm_port_data *priv, u8 falling)
+{
+ u8 pwm_port = priv->pwm_port;
+
+ u32 reg_offset = pwm_port_params[pwm_port].duty_ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(priv->base, reg_offset);
+
+ reg_value &= ~(0xFF << pwm_port_params[pwm_port].duty_ctrl_fall_point);
+ reg_value |= (falling <<
+ pwm_port_params[pwm_port].duty_ctrl_fall_point);
+
+ aspeed_pwm_write(priv->base, reg_value, reg_offset);
+}
+
+static u8
+aspeed_get_pwm_clock_unit(struct aspeed_pwm_port_data *priv, u8 pwm_clock_type)
+{
+ u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
+ u32 reg_value = aspeed_pwm_read(priv->base, reg_offset);
+ u8 period;
+
+ reg_value &= pwm_clock_type_params[pwm_clock_type].unit_mask;
+ period = reg_value >> pwm_clock_type_params[pwm_clock_type].unit_value;
+
+ return period;
+}
+
+static void
+aspeed_set_pwm_fan_ctrl(struct aspeed_pwm_port_data *priv, u8 fan_ctrl)
+{
+ u16 period;
+ u16 dc_time_on;
+
+ period = aspeed_get_pwm_clock_unit(priv, priv->pwm_type);
+ period += 1;
+ dc_time_on = (fan_ctrl * period) / PWM_MAX;
+
+ if (dc_time_on == 0) {
+ aspeed_set_pwm_enable(priv, 0);
+ } else {
+ if (dc_time_on == period)
+ dc_time_on = 0;
+
+ aspeed_set_pwm_duty_rising(priv, 0);
+ aspeed_set_pwm_duty_falling(priv, dc_time_on);
+ aspeed_set_pwm_enable(priv, 1);
+ }
+}
+
+static ssize_t
+set_pwm(struct device *dev, struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct aspeed_pwm_port_data *priv = dev_get_drvdata(dev);
+ u8 fan_ctrl;
+ int ret;
+
+ ret = kstrtou8(buf, 10, &fan_ctrl);
+ if (ret)
+ return -EINVAL;
+
+ if (fan_ctrl < 0 || fan_ctrl > PWM_MAX)
+ return -EINVAL;
+
+ if (priv->pwm_fan_ctrl == fan_ctrl)
+ return count;
+
+ priv->pwm_fan_ctrl = fan_ctrl;
+ aspeed_set_pwm_fan_ctrl(priv, fan_ctrl);
+
+ return count;
+}
+
+static ssize_t
+show_pwm(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct aspeed_pwm_port_data *priv = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", priv->pwm_fan_ctrl);
+}
+
+static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 0);
+
+static struct attribute *pwm_dev_attrs[] = {
+ &sensor_dev_attr_pwm1.dev_attr.attr,
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(pwm_dev);
+
+static int
+aspeed_create_pwm_port(struct device *dev, struct device_node *child,
+ void __iomem *base)
+{
+ struct device *hwmon;
+ u8 val;
+ struct aspeed_pwm_port_data *priv;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ hwmon = devm_hwmon_device_register_with_groups(dev, "aspeed_pwm",
+ priv, pwm_dev_groups);
+ if (IS_ERR(hwmon)) {
+ dev_err(dev, "Failed to register hwmon device\n");
+ return PTR_ERR(hwmon);
+ }
+ priv->base = base;
+
+ of_property_read_u8(child, "pwm_port", &val);
+ priv->pwm_port = val;
+
+ of_property_read_u8(child, "pwm_enable", &val);
+ priv->pwm_enable = val;
+ aspeed_set_pwm_enable(priv, val);
+
+ of_property_read_u8(child, "pwm_type", &val);
+ priv->pwm_type = val;
+ aspeed_set_pwm_type(priv, val);
+
+ of_property_read_u8(child, "pwm_fan_ctrl", &val);
+ priv->pwm_fan_ctrl = val;
+ aspeed_set_pwm_fan_ctrl(priv, val);
+
+ return 0;
+}
+
+/*
+ * If the clock type is type M then :
+ * The PWM frequency = 24MHz / (type M clock division L bit *
+ * type M clock division H bit * (type M PWM period bit + 1))
+ * Calculate type M clock division L bit and H bits given the other values
+ */
+static bool
+set_clock_values(u32 pwm_frequency, u32 period, u8 type, void __iomem *base)
+{
+ u32 src_frequency = 24000000;
+ u32 divisor = src_frequency / pwm_frequency;
+ u32 clock_divisor = divisor / (period + 1);
+ u32 tmp_clock_divisor;
+ u8 low;
+ u8 high;
+ u32 calc_high;
+ u32 calc_low;
+
+ for (high = 0; high <= MAX_HIGH_LOW_BIT; high += 1) {
+ for (low = 0; low <= MAX_HIGH_LOW_BIT; low += 1) {
+ calc_high = 0x1 << high;
+ calc_low = (low == 0 ? 1 : (2 * low));
+ tmp_clock_divisor = calc_high * calc_low;
+ if (tmp_clock_divisor >= clock_divisor)
+ goto set_value;
+ }
+ }
+
+ return false;
+
+set_value:
+ aspeed_set_pwm_clock_division_h(base, type, high);
+ aspeed_set_pwm_clock_division_l(base, type, low);
+ aspeed_set_pwm_clock_unit(base, type, period);
+
+ return true;
+}
+
+static int
+aspeed_pwm_probe(struct platform_device *pdev)
+{
+ u32 period;
+ struct device_node *np, *child;
+ struct resource *res;
+ void __iomem *base;
+ bool success;
+ struct clk *typem_clk, *typen_clk, *typeo_clk;
+ u32 pwm_typem_freq, pwm_typen_freq, pwm_typeo_freq;
+ int err;
+
+ np = pdev->dev.of_node;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res == NULL)
+ return -ENOENT;
+
+ base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!base)
+ return -ENOMEM;
+
+ /* Enable PWM clock */
+ aspeed_set_pwm_clock_enable(base, 1);
+ /* Select clock source as 24MHz */
+ aspeed_set_pwm_clock_source(base, 0);
+
+ typem_clk = of_clk_get(np, 0);
+ if (!IS_ERR(typem_clk))
+ pwm_typem_freq = clk_get_rate(typem_clk);
+ else
+ return -EINVAL;
+
+ typen_clk = of_clk_get(np, 1);
+ if (!IS_ERR(typen_clk))
+ pwm_typen_freq = clk_get_rate(typen_clk);
+
+ typeo_clk = of_clk_get(np, 2);
+ if (!IS_ERR(typeo_clk))
+ pwm_typeo_freq = clk_get_rate(typeo_clk);
+
+ err = of_property_read_u32(np, "pwm_typem_period", &period);
+ if (!err) {
+ success = set_clock_values(pwm_typem_freq, period, TYPEM, base);
+ if (!success)
+ return -EINVAL;
+ } else {
+ return -EINVAL;
+ }
+
+ err = of_property_read_u32(np, "pwm_typen_period", &period);
+ if (!err) {
+ success = set_clock_values(pwm_typen_freq, period, TYPEN, base);
+ if (!success)
+ return -EINVAL;
+ }
+
+ err = of_property_read_u32(np, "pwm_typeo_period", &period);
+ if (!err) {
+ success = set_clock_values(pwm_typeo_freq, period, TYPEO, base);
+ if (!success)
+ return -EINVAL;
+ }
+
+ for_each_child_of_node(np, child) {
+ aspeed_create_pwm_port(&pdev->dev,
+ child, base);
+ of_node_put(child);
+ }
+ of_node_put(np);
+
+ return 0;
+}
+
+static const struct of_device_id of_pwm_match_table[] = {
+ { .compatible = "aspeed,ast2400-pwm", },
+ { .compatible = "aspeed,ast2500-pwm", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_match_table);
+
+static struct platform_driver aspeed_pwm_driver = {
+ .probe = aspeed_pwm_probe,
+ .driver = {
+ .name = "aspeed_pwm",
+ .owner = THIS_MODULE,
+ .of_match_table = of_pwm_match_table,
+ },
+};
+
+module_platform_driver(aspeed_pwm_driver);
+
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("ASPEED PWM device driver");
+MODULE_LICENSE("GPL");
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH linux v3 3/3] arm: dts: Add dt-binding to support ASPEED AST2500 PWM output on zaius
2016-11-24 9:26 [PATCH linux v3 0/3] Support for ASPEED AST2400/AST2500 PWM driver Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver " Jaghathiswari Rankappagounder Natarajan
@ 2016-11-24 9:26 ` Jaghathiswari Rankappagounder Natarajan
2 siblings, 0 replies; 7+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-11-24 9:26 UTC (permalink / raw)
To: openbmc, joel; +Cc: Jaghathiswari Rankappagounder Natarajan
The zaius platform has 4 output fans. Add binding data to control the 4 PWM
outputs on ASPEED AST2500 on the zaius platform.
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
v2:
- make the pwmX entries children of the pwm_controller entry.
v3:
- changed/removed some property names to reflect the rules in binding document.
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 40 ++++++++++++++++++++++++++++++
arch/arm/boot/dts/aspeed-g5.dtsi | 6 +++++
2 files changed, 46 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index 8ef4ece..0d5960a 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -43,6 +43,46 @@
gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
};
};
+
+ pwm: pwm-controller@1e786000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x1E786000 0x78>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
+ &pinctrl_pwm2_default &pinctrl_pwm3_default>;
+ compatible = "aspeed,ast2500-pwm";
+ clocks = <&pwm_typem_clk>;
+ pwm_typem_period = <95>;
+
+ pwm_port0 {
+ pwm_port = /bits/ 8 <0x00>;
+ pwm_enable = /bits/ 8 <0x01>;
+ pwm_type = /bits/ 8 <0x00>;
+ pwm_fan_ctrl = /bits/ 8 <0xFF>;
+ };
+
+ pwm_port1 {
+ pwm_port = /bits/ 8 <0x01>;
+ pwm_enable = /bits/ 8 <0x01>;
+ pwm_type = /bits/ 8 <0x00>;
+ pwm_fan_ctrl = /bits/ 8 <0xFF>;
+ };
+
+ pwm_port2 {
+ pwm_port = /bits/ 8 <0x02>;
+ pwm_enable = /bits/ 8 <0x01>;
+ pwm_type = /bits/ 8 <0x00>;
+ pwm_fan_ctrl = /bits/ 8 <0xFF>;
+ };
+
+ pwm_port3 {
+ pwm_port = /bits/ 8 <0x03>;
+ pwm_enable = /bits/ 8 <0x01>;
+ pwm_type = /bits/ 8 <0x00>;
+ pwm_fan_ctrl = /bits/ 8 <0xFF>;
+ };
+ };
};
&fmc {
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 433461b..017cbb7 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -967,6 +967,12 @@
reg = <0x1e6e202c 0x4>;
};
+ pwm_typem_clk: fixed_clock0 {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <25000>;
+ };
+
sram@1e720000 {
compatible = "mmio-sram";
reg = <0x1e720000 0x9000>; // 36K
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver for ASPEED AST2400/2500 PWM support
2016-11-24 9:26 ` [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver " Jaghathiswari Rankappagounder Natarajan
@ 2016-11-28 6:13 ` Joel Stanley
2016-12-02 0:01 ` Jaghathiswari Rankappagounder Natarajan
0 siblings, 1 reply; 7+ messages in thread
From: Joel Stanley @ 2016-11-28 6:13 UTC (permalink / raw)
To: Jaghathiswari Rankappagounder Natarajan; +Cc: OpenBMC Maillist
Hello Jaghu,
This is looking better. I have some more comments below.
On Thu, Nov 24, 2016 at 7:56 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu@google.com> wrote:
> +The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
> +PWM clock types M, N and 0 are three types just to have three independent PWM
> +sources. Each port can be assigned a different PWM clock type.
I suggest you describe the types.
And add some newlines between your paragraphs to make it easier to read.
> +The device driver matches on the device tree node. The configuration values
> +are read from the device tree and written to the respective registers.
> +The driver provides a sysfs entry "pwm1" through which the user can
> +configure the duty-cycle value (ranging from 0 to 255; 255 is 100 percent).
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index d4de8d5..7f75b01 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -341,6 +341,19 @@ config SENSORS_ASB100
> This driver can also be built as a module. If so, the module
> will be called asb100.
>
> +config SENSORS_ASPEED_PWM
> + tristate "ASPEED AST2400/AST2500 PWM driver"
> + help
> + This driver provides support for ASPEED AST2400/AST2500 PWM
> + controller and output ports. The ASPEED PWM controller can support 8
> + PWM outputs. PWM clock types M, N and 0 are three types just to have three
> + independent PWM sources. Each could be assigned to the 8 PWM port
> + with its own settings. The user can configure duty cycle through
> + the exposed hwmon sysfs entry "pwm".
> +
> + This driver can also be built as a module. If so, the module
> + will be called aspeed-pwm.
Make sure you test the code both as a module and built in to the kernel.
> +
> +struct aspeed_pwm_port_data {
> + u8 pwm_port;
> + u8 pwm_enable;
> + u8 pwm_type;
> + u8 pwm_fan_ctrl;
> + void __iomem *base;
> +};
> +
> +enum pwm_clock_type { TYPEM, TYPEN, TYPEO };
> +
> +struct pwm_clock_type_params {
> + u32 l_value;
> + u32 l_mask;
> + u32 h_value;
> + u32 h_mask;
> + u32 unit_value;
> + u32 unit_mask;
> + u32 ctrl_reg_offset;
ctrl_reg or even just ctrl will do.
> +};
> +
I think this is an improvement on the duplicated swtich statements we
had before. Do you agree?
I'm wondering if there are still ways to clean up. The mask and the
value both tell us where in the register each value needs to go;
perhaps we can only include one of them in the pwm_clock_type_params?
> +static const struct pwm_clock_type_params pwm_clock_type_params[] = {
> + [TYPEM] = {
> + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEM_L,
> + .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_L_MASK,
> + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEM_H,
> + .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_H_MASK,
> + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT,
> + .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT_MASK,
> + .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL,
> + },
> + [TYPEN] = {
> + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEN_L,
> + .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_L_MASK,
> + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEN_H,
> + .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_H_MASK,
> + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT,
> + .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT_MASK,
> + .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL,
> + },
> + [TYPEO] = {
> + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEO_L,
> + .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_L_MASK,
> + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEO_H,
> + .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_H_MASK,
> + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT,
> + .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT_MASK,
> + .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL_EXT,
> + }
> +};
> +
> +enum pwm_port { PWMA, PWMB, PWMC, PWMD, PWME, PWMF, PWMG, PWMH };
> +
> +struct pwm_port_params {
> + u32 pwm_en;
> + u32 ctrl_reg_offset;
> + u32 pwm_type_part1;
> + u32 pwm_type_part2;
> + u32 pwm_type_mask;
> + u32 duty_ctrl_rise_point;
> + u32 duty_ctrl_fall_point;
> + u32 duty_ctrl_reg_offset;
> + u8 duty_ctrl_calc_type;
> +};
> +
> +static const struct pwm_port_params pwm_port_params[] = {
> + [PWMA] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMA_EN,
> + .ctrl_reg_offset = ASPEED_PTCR_CTRL,
> + .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1,
> + .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2,
> + .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
> + .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY0_CTRL,
> + .duty_ctrl_calc_type = 0,
> + },
> +
> +static inline void
> +aspeed_pwm_write(void __iomem *base, u32 val, u32 reg)
> +{
> + writel(val, base + reg);
Is there a reason you're using writel and not iowrite32?
> +}
> +
> +static inline u32
> +aspeed_pwm_read(void __iomem *base, u32 reg)
> +{
> + u32 val = readl(base + reg);
> + return val;
You could just do return readl(base + reg).
That said, do you think the helpers for _read and _write are worth it?
> +}
> +
> +static void
> +aspeed_set_pwm_clock_enable(void __iomem *base, bool val)
Most kernel code puts the type and storage class specifiers on the
same line as the function:
static void aspeed_set_pwm_clock_enable(void __iomem *base, bool val)
> +{
> + u32 reg_value = aspeed_pwm_read(base, ASPEED_PTCR_CTRL);
> +
> + if (val)
> + reg_value |= ASPEED_PTCR_CTRL_CLK_EN;
> + else
> + reg_value &= ~ASPEED_PTCR_CTRL_CLK_EN;
> +
> + aspeed_pwm_write(base, reg_value, ASPEED_PTCR_CTRL);
> +}
> +
> +static void
> +aspeed_set_pwm_clock_source(void __iomem *base, bool val)
When I call this function I'm going to write something like this:
aspeed_set_pwm_clock_source(foo, true);
It doesn't make sense to be setting a clock source to "true". I think
you could call it aspeed_pwm_use_pclk() or similar, so the boolean
makes sense.
> +{
> + u32 reg_value = aspeed_pwm_read(base, ASPEED_PTCR_CTRL);
> +
> + if (val)
> + reg_value |= ASPEED_PTCR_CTRL_CLK_SRC;
> + else
> + reg_value &= ~ASPEED_PTCR_CTRL_CLK_SRC;
> +
> + aspeed_pwm_write(base, reg_value, ASPEED_PTCR_CTRL);
> +}
> +
> +static void
> +aspeed_set_pwm_clock_division_h(void __iomem *base, u8 pwm_clock_type,
> + u8 div_high)
> +{
> + u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
> + u32 reg_value = aspeed_pwm_read(base, reg_offset);
> +
> + reg_value &= ~pwm_clock_type_params[pwm_clock_type].h_mask;
> + reg_value |= div_high << pwm_clock_type_params[pwm_clock_type].h_value;
> +
> + aspeed_pwm_write(base, reg_value, reg_offset);
> +}
> +
> +static void
> +aspeed_set_pwm_clock_division_l(void __iomem *base, u8 pwm_clock_type,
> + u8 div_low)
> +{
You could pass around a pointer to the clock type, instead of the index.
Even better would be some kind of struct that includes both base and
the clock type.
> + u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
> + u32 reg_value = aspeed_pwm_read(base, reg_offset);
> +
> + reg_value &= ~pwm_clock_type_params[pwm_clock_type].l_mask;
> + reg_value |= div_low << pwm_clock_type_params[pwm_clock_type].l_value;
> +
> + aspeed_pwm_write(base, reg_value, reg_offset);
> +}
Is there ever a case where you will set the high but not the low bit?
If not you could do this:
aspeed_set_pwm_clock_division(void __iomem *base, u8 pwm_clock_type,
u8 div_low, u8 div_high)
{
struct pwm_clock_type *pwm = &pwm_clock_type_params[pwm_clock_type];
u32 reg = aspeed_pwm_read(base, pwm->ctrl_reg_offset);
reg &= ~(pwm->l_mask | pwm->h_mask);
reg |= (div_low << pwm->l_value) | (div_high << pwm->h_value);
aspeed_pwm_write(base, reg, pwm->ctrl_reg_offset);
}
I suggest attempting cleanups like this to other parts of your code.
> +
> +static void
> +aspeed_set_pwm_clock_unit(void __iomem *base, u8 pwm_clock_type,
> + u8 unit)
> +{
> + u32 reg_offset = pwm_clock_type_params[pwm_clock_type].ctrl_reg_offset;
> + u32 reg_value = aspeed_pwm_read(base, reg_offset);
> +
> + reg_value &= ~pwm_clock_type_params[pwm_clock_type].unit_mask;
> + reg_value |= unit << pwm_clock_type_params[pwm_clock_type].unit_value;
> +
> + aspeed_pwm_write(base, reg_value, reg_offset);
> +}
> +
> +static void
> +aspeed_set_pwm_fan_ctrl(struct aspeed_pwm_port_data *priv, u8 fan_ctrl)
> +{
> + u16 period;
> + u16 dc_time_on;
> +
> + period = aspeed_get_pwm_clock_unit(priv, priv->pwm_type);
> + period += 1;
> + dc_time_on = (fan_ctrl * period) / PWM_MAX;
> +
> + if (dc_time_on == 0) {
> + aspeed_set_pwm_enable(priv, 0);
Doesn't aspeed_set_pwm_enable take a boolean as the second parameter?
> + } else {
> + if (dc_time_on == period)
> + dc_time_on = 0;
> +
> + aspeed_set_pwm_duty_rising(priv, 0);
> + aspeed_set_pwm_duty_falling(priv, dc_time_on);
> + aspeed_set_pwm_enable(priv, 1);
> + }
> +}
> +
> +static ssize_t
> +set_pwm(struct device *dev, struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + struct aspeed_pwm_port_data *priv = dev_get_drvdata(dev);
> + u8 fan_ctrl;
> + int ret;
> +
> + ret = kstrtou8(buf, 10, &fan_ctrl);
If you specify 0 as the format, then users can pass their clock rate
in as hexadecimal (not that it would make much sense...) or decimal
and the sysfs file will do the correct thing.
> + if (ret)
> + return -EINVAL;
> +
> + if (fan_ctrl < 0 || fan_ctrl > PWM_MAX)
> + return -EINVAL;
> +
> + if (priv->pwm_fan_ctrl == fan_ctrl)
> + return count;
> +
> + priv->pwm_fan_ctrl = fan_ctrl;
> + aspeed_set_pwm_fan_ctrl(priv, fan_ctrl);
> +
> + return count;
> +}
> +
> +static ssize_t
> +show_pwm(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_pwm_port_data *priv = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", priv->pwm_fan_ctrl);
> +}
> +
> +static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 0);
> +
> +static struct attribute *pwm_dev_attrs[] = {
> + &sensor_dev_attr_pwm1.dev_attr.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(pwm_dev);
> +
> +static int
> +aspeed_create_pwm_port(struct device *dev, struct device_node *child,
> + void __iomem *base)
> +{
> + struct device *hwmon;
> + u8 val;
> + struct aspeed_pwm_port_data *priv;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + hwmon = devm_hwmon_device_register_with_groups(dev, "aspeed_pwm",
> + priv, pwm_dev_groups);
> + if (IS_ERR(hwmon)) {
> + dev_err(dev, "Failed to register hwmon device\n");
> + return PTR_ERR(hwmon);
> + }
> + priv->base = base;
> +
> + of_property_read_u8(child, "pwm_port", &val);
> + priv->pwm_port = val;
> +
> + of_property_read_u8(child, "pwm_enable", &val);
> + priv->pwm_enable = val;
> + aspeed_set_pwm_enable(priv, val);
> +
> + of_property_read_u8(child, "pwm_type", &val);
> + priv->pwm_type = val;
> + aspeed_set_pwm_type(priv, val);
> +
> + of_property_read_u8(child, "pwm_fan_ctrl", &val);
> + priv->pwm_fan_ctrl = val;
> + aspeed_set_pwm_fan_ctrl(priv, val);
> +
> + return 0;
> +}
> +
> +/*
> + * If the clock type is type M then :
> + * The PWM frequency = 24MHz / (type M clock division L bit *
> + * type M clock division H bit * (type M PWM period bit + 1))
> + * Calculate type M clock division L bit and H bits given the other values
> + */
> +static bool
> +set_clock_values(u32 pwm_frequency, u32 period, u8 type, void __iomem *base)
> +{
> + u32 src_frequency = 24000000;
Where does this come from? It's the pclk I think. This means your
binding should do something like this:
clocks = <&clk_apb>;
Then in your code,
pclk = devm_clk_get(&pdev->dev, NULL);
pclk_rate = clk_get_rate(pclk);
and then use clck_rate where you have 'src_frequency' to calculate the
divisor values. A hwmon driver that does this is g762.c.
> + u32 divisor = src_frequency / pwm_frequency;
> + u32 clock_divisor = divisor / (period + 1);
> + u32 tmp_clock_divisor;
> + u8 low;
> + u8 high;
> + u32 calc_high;
> + u32 calc_low;
> +
> + for (high = 0; high <= MAX_HIGH_LOW_BIT; high += 1) {
> + for (low = 0; low <= MAX_HIGH_LOW_BIT; low += 1) {
> + calc_high = 0x1 << high;
> + calc_low = (low == 0 ? 1 : (2 * low));
> + tmp_clock_divisor = calc_high * calc_low;
> + if (tmp_clock_divisor >= clock_divisor)
> + goto set_value;
> + }
> + }
> +
> + return false;
> +
> +set_value:
> + aspeed_set_pwm_clock_division_h(base, type, high);
> + aspeed_set_pwm_clock_division_l(base, type, low);
> + aspeed_set_pwm_clock_unit(base, type, period);
> +
> + return true;
> +}
> +
> +static int
> +aspeed_pwm_probe(struct platform_device *pdev)
> +{
> + u32 period;
> + struct device_node *np, *child;
> + struct resource *res;
> + void __iomem *base;
> + bool success;
> + struct clk *typem_clk, *typen_clk, *typeo_clk;
> + u32 pwm_typem_freq, pwm_typen_freq, pwm_typeo_freq;
> + int err;
> +
> + np = pdev->dev.of_node;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (res == NULL)
> + return -ENOENT;
> +
> + base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!base)
> + return -ENOMEM;
> +
> + /* Enable PWM clock */
> + aspeed_set_pwm_clock_enable(base, 1);
You're passing a '1' to a function that expects a boolean value.
> + /* Select clock source as 24MHz */
> + aspeed_set_pwm_clock_source(base, 0);
This would be selected by your device tree bindings. You could have it
so if a clock node is populated, it uses that value, and if it's not
it falls back on the 1MHz clock.
> +
> + typem_clk = of_clk_get(np, 0);
This isn't what I meant. See the comment above about referencing the
parent clock in the bindings.
> + if (!IS_ERR(typem_clk))
> + pwm_typem_freq = clk_get_rate(typem_clk);
> + else
> + return -EINVAL;
> +
> + typen_clk = of_clk_get(np, 1);
> + if (!IS_ERR(typen_clk))
> + pwm_typen_freq = clk_get_rate(typen_clk);
> +
> + typeo_clk = of_clk_get(np, 2);
> + if (!IS_ERR(typeo_clk))
> + pwm_typeo_freq = clk_get_rate(typeo_clk);
This doesn't make any sense. Did you test this code?
> +
> + err = of_property_read_u32(np, "pwm_typem_period", &period);
> + if (!err) {
> + success = set_clock_values(pwm_typem_freq, period, TYPEM, base);
> + if (!success)
> + return -EINVAL;
> + } else {
> + return -EINVAL;
> + }
> +
> + err = of_property_read_u32(np, "pwm_typen_period", &period);
> + if (!err) {
> + success = set_clock_values(pwm_typen_freq, period, TYPEN, base);
> + if (!success)
> + return -EINVAL;
> + }
> +
> + err = of_property_read_u32(np, "pwm_typeo_period", &period);
> + if (!err) {
> + success = set_clock_values(pwm_typeo_freq, period, TYPEO, base);
> + if (!success)
> + return -EINVAL;
> + }
> +
> + for_each_child_of_node(np, child) {
> + aspeed_create_pwm_port(&pdev->dev,
> + child, base);
> + of_node_put(child);
> + }
> + of_node_put(np);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support
2016-11-24 9:26 ` [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support Jaghathiswari Rankappagounder Natarajan
@ 2016-11-28 6:13 ` Joel Stanley
0 siblings, 0 replies; 7+ messages in thread
From: Joel Stanley @ 2016-11-28 6:13 UTC (permalink / raw)
To: Jaghathiswari Rankappagounder Natarajan; +Cc: OpenBMC Maillist
Hello Jaghu,
This is the first time I've had a close look at the binding document.
Note that I'm not a device tree expert. I do have some suggestions
below that you might want to look at before we send this upstream.
On Thu, Nov 24, 2016 at 7:56 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu@google.com> wrote:
> This binding provides interface for adding values related to ASPEED
> AST2400/2500 PWM support.
> The parent node indicates a PWM controller. The parent node can have
> upto 8 child nodes. Each node indicates a PWM output port.
> PWM clock types M, N and 0 are three types just to have three independent
> PWM sources. Each port can be assigned a different PWM clock type.
> Each port can have the duty-cycle set to a different value.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> ---
> v2:
> - Made binding documentation update as a separate patch.
> - Included AST2500 in the description.
> - Changed format to "aspeed,ast2500-pwm".
> - Included explanation for types M ,N and O and calculation example to
> - determine L, H, and period bits.
>
> v3:
> - Included struct clk and phandle reference to clock node.
> - Removed clock calculation description from the bindings document.
> - Added some whitespace and improved style of binding document.
>
> .../devicetree/bindings/hwmon/aspeed-pwm.txt | 104 +++++++++++++++++++++
> 1 file changed, 104 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
> new file mode 100644
> index 0000000..11a007c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm.txt
> @@ -0,0 +1,104 @@
> +ASPEED AST2400/AST2500 PWM device driver
> +
> +The ASPEED PWM controller can support 8 PWM outputs.
> +PWM clock types M, N and 0 are three types just to have three
> +independent PWM sources. Each could be assigned to the 8 PWM port
> +with it's own settings.
> +
> +Required properties:
> +- #address-cells : should be 1.
> +
> +- #size-cells : should be 1.
> +
> +- reg : address and length of the register set for the device.
> +
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +
> +- pinctrl-0 : phandle referencing pin configuration of the ASPEED PWM ports.
> +
> +- compatible : should be "aspeed,ast2400-pwm" for AST2400 or
> + "aspeed,ast2500-pwm" for AST2500.
> +
> +- clocks : phandle reference to clocks. upto three fixed clocks providing
> + three different input clock frequencies. each clock frequency indicates a
> + desired PWM frequency.
> +
> + the first PWM frequency in combination with type M PWM period value
> + (described below) and source clock frequency will be used to calculate the
> + type M PWM clock division high and low values.
I can't see where you describe what a "type M PWM period value" is.
Can you teach me?
> + the second PWM frequency in combination with type N PWM period value and
> + source clock frequency will be used to calculate the type N PWM clock
> + division high and low values.
> + the third PWM frequency in combination with type O PWM period value and
> + source clock frequency will be used to calculate the type O PWM clock
> + division high and low values.
> +
> + Since atleast one PWM frequency or one PWM clock type(type M PWM clock type)
> + is required, the first clock is required. the other two are optional.
> +
> +- pwm_typem_period : indicates type M PWM period. integer value in the range
> + 0 to 255.
> +
> +Optional properties:
> +- pwm_typen_period : indicates type N PWM period. integer value in the range
> + 0 to 255.
> +
> +- pwm_typeo_period : indicates type O PWM period. integer value in the range
> + 0 to 255.
> +
> +Each subnode represents a PWM output port.
> +
> +Subnode Format
> +--------------
> +Required properties:
> +- pwm_port : indicates the PWM port. values 0 through 7 indicate PWM ports
> + A through H respectively.
Should we instead use a reg cell to indicate this?
> +
> +- pwm_enable : option to enable the PWM port indicated above. 0 is enable
> + and 1 is disable.
It looks like you're using this as a boolean. I don't think that
upstream will accept this use of the device tree. What does "enable"
mean? Can we use the device tree convention of status =
"disabled"/"okay" to indicate if the driver should bind to this device
and enable the hardware?
> +
> +- pwm_type : indicates the clock type for the PWM port. value 0 is type M
> + PWM clock. value 1 is type N PWM clock. value 2 is type O PWM clock.
Could we instead indicate this type in the compatible string?
> +
> +- pwm_fan_ctrl : pulse width modulation fan control. integer value
> + in the range 0 to 255. 255 is max or 100%.
> +
> +Additional details are available in the detailed datasheet for the device
> +AST2400/AST2500.
I don't think this comment is useful, as anyone reading it will not
necessarily have access to the data sheet.
> +
> +Examples:
> +
> +clocks {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pwm_typem_clock: fixedclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <25000>;
> + }
This is not what I meant about the clocks. See my comments in patch 2.
It does raise a separate point. Does the clock frequency value (25000)
describe a property of how the hardware is configured? If so, we might
want to keep it in the device tree, just differently.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver for ASPEED AST2400/2500 PWM support
2016-11-28 6:13 ` Joel Stanley
@ 2016-12-02 0:01 ` Jaghathiswari Rankappagounder Natarajan
0 siblings, 0 replies; 7+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-02 0:01 UTC (permalink / raw)
To: Joel Stanley; +Cc: OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 18381 bytes --]
On Sun, Nov 27, 2016 at 10:13 PM, Joel Stanley <joel@jms.id.au> wrote:
> Hello Jaghu,
>
> This is looking better. I have some more comments below.
>
> On Thu, Nov 24, 2016 at 7:56 PM, Jaghathiswari Rankappagounder
> Natarajan <jaghu@google.com> wrote:
>
> > +The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
> > +PWM clock types M, N and 0 are three types just to have three
> independent PWM
> > +sources. Each port can be assigned a different PWM clock type.
>
> I suggest you describe the types.
>
> And add some newlines between your paragraphs to make it easier to read.
>
> > +The device driver matches on the device tree node. The configuration
> values
> > +are read from the device tree and written to the respective registers.
> > +The driver provides a sysfs entry "pwm1" through which the user can
> > +configure the duty-cycle value (ranging from 0 to 255; 255 is 100
> percent).
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index d4de8d5..7f75b01 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -341,6 +341,19 @@ config SENSORS_ASB100
> > This driver can also be built as a module. If so, the module
> > will be called asb100.
> >
> > +config SENSORS_ASPEED_PWM
> > + tristate "ASPEED AST2400/AST2500 PWM driver"
> > + help
> > + This driver provides support for ASPEED AST2400/AST2500 PWM
> > + controller and output ports. The ASPEED PWM controller can
> support 8
> > + PWM outputs. PWM clock types M, N and 0 are three types just
> to have three
> > + independent PWM sources. Each could be assigned to the 8 PWM
> port
> > + with its own settings. The user can configure duty cycle
> through
> > + the exposed hwmon sysfs entry "pwm".
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called aspeed-pwm.
>
> Make sure you test the code both as a module and built in to the kernel.
>
> > +
> > +struct aspeed_pwm_port_data {
> > + u8 pwm_port;
> > + u8 pwm_enable;
> > + u8 pwm_type;
> > + u8 pwm_fan_ctrl;
> > + void __iomem *base;
> > +};
> > +
> > +enum pwm_clock_type { TYPEM, TYPEN, TYPEO };
> > +
> > +struct pwm_clock_type_params {
> > + u32 l_value;
> > + u32 l_mask;
> > + u32 h_value;
> > + u32 h_mask;
> > + u32 unit_value;
> > + u32 unit_mask;
> > + u32 ctrl_reg_offset;
>
> ctrl_reg or even just ctrl will do.
>
> > +};
> > +
>
> I think this is an improvement on the duplicated swtich statements we
> had before. Do you agree?
>
> I'm wondering if there are still ways to clean up. The mask and the
> value both tell us where in the register each value needs to go;
> perhaps we can only include one of them in the pwm_clock_type_params?
>
> > +static const struct pwm_clock_type_params pwm_clock_type_params[] = {
> > + [TYPEM] = {
> > + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEM_L,
> > + .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_L_MASK,
> > + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEM_H,
> > + .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_H_MASK,
> > + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT,
> > + .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT_MASK,
> > + .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL,
> > + },
> > + [TYPEN] = {
> > + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEN_L,
> > + .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_L_MASK,
> > + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEN_H,
> > + .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_H_MASK,
> > + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT,
> > + .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT_MASK,
> > + .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL,
> > + },
> > + [TYPEO] = {
> > + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEO_L,
> > + .l_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_L_MASK,
> > + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEO_H,
> > + .h_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_H_MASK,
> > + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT,
> > + .unit_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT_MASK,
> > + .ctrl_reg_offset = ASPEED_PTCR_CLK_CTRL_EXT,
> > + }
> > +};
> > +
> > +enum pwm_port { PWMA, PWMB, PWMC, PWMD, PWME, PWMF, PWMG, PWMH };
> > +
> > +struct pwm_port_params {
> > + u32 pwm_en;
> > + u32 ctrl_reg_offset;
> > + u32 pwm_type_part1;
> > + u32 pwm_type_part2;
> > + u32 pwm_type_mask;
> > + u32 duty_ctrl_rise_point;
> > + u32 duty_ctrl_fall_point;
> > + u32 duty_ctrl_reg_offset;
> > + u8 duty_ctrl_calc_type;
> > +};
> > +
> > +static const struct pwm_port_params pwm_port_params[] = {
> > + [PWMA] = {
> > + .pwm_en = ASPEED_PTCR_CTRL_PWMA_EN,
> > + .ctrl_reg_offset = ASPEED_PTCR_CTRL,
> > + .pwm_type_part1 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1,
> > + .pwm_type_part2 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2,
> > + .pwm_type_mask = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK,
> > + .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
> > + .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
> > + .duty_ctrl_reg_offset = ASPEED_PTCR_DUTY0_CTRL,
> > + .duty_ctrl_calc_type = 0,
> > + },
> > +
> > +static inline void
> > +aspeed_pwm_write(void __iomem *base, u32 val, u32 reg)
> > +{
> > + writel(val, base + reg);
>
> Is there a reason you're using writel and not iowrite32?
>
> > +}
> > +
> > +static inline u32
> > +aspeed_pwm_read(void __iomem *base, u32 reg)
> > +{
> > + u32 val = readl(base + reg);
> > + return val;
>
> You could just do return readl(base + reg).
>
> That said, do you think the helpers for _read and _write are worth it?
>
> > +}
> > +
> > +static void
> > +aspeed_set_pwm_clock_enable(void __iomem *base, bool val)
>
> Most kernel code puts the type and storage class specifiers on the
> same line as the function:
>
> static void aspeed_set_pwm_clock_enable(void __iomem *base, bool val)
>
> > +{
> > + u32 reg_value = aspeed_pwm_read(base, ASPEED_PTCR_CTRL);
> > +
> > + if (val)
> > + reg_value |= ASPEED_PTCR_CTRL_CLK_EN;
> > + else
> > + reg_value &= ~ASPEED_PTCR_CTRL_CLK_EN;
> > +
> > + aspeed_pwm_write(base, reg_value, ASPEED_PTCR_CTRL);
> > +}
> > +
> > +static void
> > +aspeed_set_pwm_clock_source(void __iomem *base, bool val)
>
> When I call this function I'm going to write something like this:
>
> aspeed_set_pwm_clock_source(foo, true);
>
> It doesn't make sense to be setting a clock source to "true". I think
> you could call it aspeed_pwm_use_pclk() or similar, so the boolean
> makes sense.
>
> > +{
> > + u32 reg_value = aspeed_pwm_read(base, ASPEED_PTCR_CTRL);
> > +
> > + if (val)
> > + reg_value |= ASPEED_PTCR_CTRL_CLK_SRC;
> > + else
> > + reg_value &= ~ASPEED_PTCR_CTRL_CLK_SRC;
> > +
> > + aspeed_pwm_write(base, reg_value, ASPEED_PTCR_CTRL);
> > +}
> > +
> > +static void
> > +aspeed_set_pwm_clock_division_h(void __iomem *base, u8 pwm_clock_type,
> > + u8 div_high)
> > +{
> > + u32 reg_offset = pwm_clock_type_params[pwm_
> clock_type].ctrl_reg_offset;
> > + u32 reg_value = aspeed_pwm_read(base, reg_offset);
> > +
> > + reg_value &= ~pwm_clock_type_params[pwm_clock_type].h_mask;
> > + reg_value |= div_high << pwm_clock_type_params[pwm_
> clock_type].h_value;
> > +
> > + aspeed_pwm_write(base, reg_value, reg_offset);
> > +}
> > +
> > +static void
> > +aspeed_set_pwm_clock_division_l(void __iomem *base, u8 pwm_clock_type,
> > + u8 div_low)
> > +{
>
> You could pass around a pointer to the clock type, instead of the index.
>
> Even better would be some kind of struct that includes both base and
> the clock type.
>
> > + u32 reg_offset = pwm_clock_type_params[pwm_
> clock_type].ctrl_reg_offset;
> > + u32 reg_value = aspeed_pwm_read(base, reg_offset);
> > +
> > + reg_value &= ~pwm_clock_type_params[pwm_clock_type].l_mask;
> > + reg_value |= div_low << pwm_clock_type_params[pwm_
> clock_type].l_value;
> > +
> > + aspeed_pwm_write(base, reg_value, reg_offset);
> > +}
>
> Is there ever a case where you will set the high but not the low bit?
> If not you could do this:
>
> aspeed_set_pwm_clock_division(void __iomem *base, u8 pwm_clock_type,
> u8 div_low, u8 div_high)
> {
> struct pwm_clock_type *pwm = &pwm_clock_type_params[pwm_
> clock_type];
> u32 reg = aspeed_pwm_read(base, pwm->ctrl_reg_offset);
>
> reg &= ~(pwm->l_mask | pwm->h_mask);
> reg |= (div_low << pwm->l_value) | (div_high << pwm->h_value);
>
> aspeed_pwm_write(base, reg, pwm->ctrl_reg_offset);
> }
>
> I suggest attempting cleanups like this to other parts of your code.
>
> > +
> > +static void
> > +aspeed_set_pwm_clock_unit(void __iomem *base, u8 pwm_clock_type,
> > + u8 unit)
> > +{
> > + u32 reg_offset = pwm_clock_type_params[pwm_
> clock_type].ctrl_reg_offset;
> > + u32 reg_value = aspeed_pwm_read(base, reg_offset);
> > +
> > + reg_value &= ~pwm_clock_type_params[pwm_clock_type].unit_mask;
> > + reg_value |= unit << pwm_clock_type_params[pwm_
> clock_type].unit_value;
> > +
> > + aspeed_pwm_write(base, reg_value, reg_offset);
> > +}
> > +
>
> > +static void
> > +aspeed_set_pwm_fan_ctrl(struct aspeed_pwm_port_data *priv, u8 fan_ctrl)
> > +{
> > + u16 period;
> > + u16 dc_time_on;
> > +
> > + period = aspeed_get_pwm_clock_unit(priv, priv->pwm_type);
> > + period += 1;
> > + dc_time_on = (fan_ctrl * period) / PWM_MAX;
> > +
> > + if (dc_time_on == 0) {
> > + aspeed_set_pwm_enable(priv, 0);
>
> Doesn't aspeed_set_pwm_enable take a boolean as the second parameter?
>
> > + } else {
> > + if (dc_time_on == period)
> > + dc_time_on = 0;
> > +
> > + aspeed_set_pwm_duty_rising(priv, 0);
> > + aspeed_set_pwm_duty_falling(priv, dc_time_on);
> > + aspeed_set_pwm_enable(priv, 1);
> > + }
> > +}
> > +
> > +static ssize_t
> > +set_pwm(struct device *dev, struct device_attribute *attr, const char
> *buf,
> > + size_t count)
> > +{
> > + struct aspeed_pwm_port_data *priv = dev_get_drvdata(dev);
> > + u8 fan_ctrl;
> > + int ret;
> > +
> > + ret = kstrtou8(buf, 10, &fan_ctrl);
>
> If you specify 0 as the format, then users can pass their clock rate
> in as hexadecimal (not that it would make much sense...) or decimal
> and the sysfs file will do the correct thing.
The user is passing the duty cycle value here (and not the clock rate).
> > + if (ret)
> > + return -EINVAL;
> > +
> > + if (fan_ctrl < 0 || fan_ctrl > PWM_MAX)
> > + return -EINVAL;
> > +
> > + if (priv->pwm_fan_ctrl == fan_ctrl)
> > + return count;
> > +
> > + priv->pwm_fan_ctrl = fan_ctrl;
> > + aspeed_set_pwm_fan_ctrl(priv, fan_ctrl);
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t
> > +show_pwm(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + struct aspeed_pwm_port_data *priv = dev_get_drvdata(dev);
> > +
> > + return sprintf(buf, "%u\n", priv->pwm_fan_ctrl);
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm,
> 0);
> > +
> > +static struct attribute *pwm_dev_attrs[] = {
> > + &sensor_dev_attr_pwm1.dev_attr.attr,
> > + NULL,
> > +};
> > +
> > +ATTRIBUTE_GROUPS(pwm_dev);
> > +
> > +static int
> > +aspeed_create_pwm_port(struct device *dev, struct device_node *child,
> > + void __iomem *base)
> > +{
> > + struct device *hwmon;
> > + u8 val;
> > + struct aspeed_pwm_port_data *priv;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + hwmon = devm_hwmon_device_register_with_groups(dev,
> "aspeed_pwm",
> > + priv,
> pwm_dev_groups);
> > + if (IS_ERR(hwmon)) {
> > + dev_err(dev, "Failed to register hwmon device\n");
> > + return PTR_ERR(hwmon);
> > + }
> > + priv->base = base;
> > +
> > + of_property_read_u8(child, "pwm_port", &val);
> > + priv->pwm_port = val;
> > +
> > + of_property_read_u8(child, "pwm_enable", &val);
> > + priv->pwm_enable = val;
> > + aspeed_set_pwm_enable(priv, val);
> > +
> > + of_property_read_u8(child, "pwm_type", &val);
> > + priv->pwm_type = val;
> > + aspeed_set_pwm_type(priv, val);
> > +
> > + of_property_read_u8(child, "pwm_fan_ctrl", &val);
> > + priv->pwm_fan_ctrl = val;
> > + aspeed_set_pwm_fan_ctrl(priv, val);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * If the clock type is type M then :
> > + * The PWM frequency = 24MHz / (type M clock division L bit *
> > + * type M clock division H bit * (type M PWM period bit + 1))
> > + * Calculate type M clock division L bit and H bits given the other
> values
> > + */
> > +static bool
> > +set_clock_values(u32 pwm_frequency, u32 period, u8 type, void __iomem
> *base)
> > +{
> > + u32 src_frequency = 24000000;
>
> Where does this come from? It's the pclk I think. This means your
> binding should do something like this:
>
> clocks = <&clk_apb>;
>
> Then in your code,
>
> pclk = devm_clk_get(&pdev->dev, NULL);
> pclk_rate = clk_get_rate(pclk);
>
> and then use clck_rate where you have 'src_frequency' to calculate the
> divisor values. A hwmon driver that does this is g762.c.
For the PWM and Fan Tach controller, in the register PTCR00 (AST2500
datasheet page - 671) it is given as :
Clock source selection
0: from 24MHz
1: from MCLK
24MHz seems to be hardcoded. I am not sure which clock producer to use
for 24MHz and for MCLK.
CLKIN seems to have the option of being either 24MHz and 25 MHz.
I am not sure how the pclk is related here.
> > + u32 divisor = src_frequency / pwm_frequency;
> > + u32 clock_divisor = divisor / (period + 1);
> > + u32 tmp_clock_divisor;
> > + u8 low;
> > + u8 high;
> > + u32 calc_high;
> > + u32 calc_low;
> > +
> > + for (high = 0; high <= MAX_HIGH_LOW_BIT; high += 1) {
> > + for (low = 0; low <= MAX_HIGH_LOW_BIT; low += 1) {
> > + calc_high = 0x1 << high;
> > + calc_low = (low == 0 ? 1 : (2 * low));
> > + tmp_clock_divisor = calc_high * calc_low;
> > + if (tmp_clock_divisor >= clock_divisor)
> > + goto set_value;
> > + }
> > + }
> > +
> > + return false;
> > +
> > +set_value:
> > + aspeed_set_pwm_clock_division_h(base, type, high);
> > + aspeed_set_pwm_clock_division_l(base, type, low);
> > + aspeed_set_pwm_clock_unit(base, type, period);
> > +
> > + return true;
> > +}
> > +
> > +static int
> > +aspeed_pwm_probe(struct platform_device *pdev)
> > +{
> > + u32 period;
> > + struct device_node *np, *child;
> > + struct resource *res;
> > + void __iomem *base;
> > + bool success;
> > + struct clk *typem_clk, *typen_clk, *typeo_clk;
> > + u32 pwm_typem_freq, pwm_typen_freq, pwm_typeo_freq;
> > + int err;
> > +
> > + np = pdev->dev.of_node;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (res == NULL)
> > + return -ENOENT;
> > +
> > + base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> > + if (!base)
> > + return -ENOMEM;
> > +
> > + /* Enable PWM clock */
> > + aspeed_set_pwm_clock_enable(base, 1);
>
> You're passing a '1' to a function that expects a boolean value.
>
> > + /* Select clock source as 24MHz */
> > + aspeed_set_pwm_clock_source(base, 0);
>
> This would be selected by your device tree bindings. You could have it
> so if a clock node is populated, it uses that value, and if it's not
> it falls back on the 1MHz clock.
>
> > +
> > + typem_clk = of_clk_get(np, 0);
>
> This isn't what I meant. See the comment above about referencing the
> parent clock in the bindings.
>
> > + if (!IS_ERR(typem_clk))
> > + pwm_typem_freq = clk_get_rate(typem_clk);
> > + else
> > + return -EINVAL;
> > +
> > + typen_clk = of_clk_get(np, 1);
> > + if (!IS_ERR(typen_clk))
> > + pwm_typen_freq = clk_get_rate(typen_clk);
> > +
> > + typeo_clk = of_clk_get(np, 2);
> > + if (!IS_ERR(typeo_clk))
> > + pwm_typeo_freq = clk_get_rate(typeo_clk);
>
> This doesn't make any sense. Did you test this code?
>
> > +
> > + err = of_property_read_u32(np, "pwm_typem_period", &period);
> > + if (!err) {
> > + success = set_clock_values(pwm_typem_freq, period,
> TYPEM, base);
> > + if (!success)
> > + return -EINVAL;
> > + } else {
> > + return -EINVAL;
> > + }
> > +
> > + err = of_property_read_u32(np, "pwm_typen_period", &period);
> > + if (!err) {
> > + success = set_clock_values(pwm_typen_freq, period,
> TYPEN, base);
> > + if (!success)
> > + return -EINVAL;
> > + }
> > +
> > + err = of_property_read_u32(np, "pwm_typeo_period", &period);
> > + if (!err) {
> > + success = set_clock_values(pwm_typeo_freq, period,
> TYPEO, base);
> > + if (!success)
> > + return -EINVAL;
> > + }
> > +
> > + for_each_child_of_node(np, child) {
> > + aspeed_create_pwm_port(&pdev->dev,
> > + child, base);
> > + of_node_put(child);
> > + }
> > + of_node_put(np);
> > +
> > + return 0;
> > +}
>
[-- Attachment #2: Type: text/html, Size: 24479 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-12-02 0:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-24 9:26 [PATCH linux v3 0/3] Support for ASPEED AST2400/AST2500 PWM driver Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 1/3] Documentation: dt-bindings: Document binding for ASPEED AST2400/2500 PWM support Jaghathiswari Rankappagounder Natarajan
2016-11-28 6:13 ` Joel Stanley
2016-11-24 9:26 ` [PATCH linux v3 2/3] drivers: hwmon: Hwmon driver " Jaghathiswari Rankappagounder Natarajan
2016-11-28 6:13 ` Joel Stanley
2016-12-02 0:01 ` Jaghathiswari Rankappagounder Natarajan
2016-11-24 9:26 ` [PATCH linux v3 3/3] arm: dts: Add dt-binding to support ASPEED AST2500 PWM output on zaius Jaghathiswari Rankappagounder Natarajan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.