| Message ID | 20170109215935.30067-2-jaghu@google.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
On Mon, Jan 09, 2017 at 01:59:34PM -0800, Jaghathiswari Rankappagounder Natarajan wrote: > This binding provides interface for adding values related to ASPEED > AST2400/2500 PWM and Fan tach controller support. > The PWM controller can support upto 8 PWM output ports. > The Fan tach controller can support upto 16 tachometer inputs. > PWM clock types M, N and 0 are three types just to have three independent > PWM sources. > > Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com> > --- > .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 153 +++++++++++++++++++++ Perhaps bindings/pwm/... even though this is more than just PWM. > 1 file changed, 153 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt > > diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt > new file mode 100644 > index 000000000000..8f346409ee8c > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt > @@ -0,0 +1,153 @@ > +ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver > + > +The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho > +controller can support upto 16 tachometer inputs. The PWM controller supports > +3 types of frequency mode PWM for fan speed control. PWM clock types M, N and 0 > +are 3 types of frequency mode PWM just to have 3 independent PWM sources. > + > +Required properties for pwm_tacho node: > +- #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 AST2400/AST2500 PWM > + ports. > + > +- compatible : should be "aspeed,aspeed2400-pwm-tacho" for AST2400 or > + "aspeed,aspeed2500-pwm-tacho" for AST2500. > + > +- clocks : a fixed clock providing input clock frequency(PWM > + and Fan Tach clock) > + > +type_values subnode format: Don't use '_' in node or property names. > +=========================== > +Under type_values subnode there can be upto 3 child nodes indicating type M/N/O > +values. Atleast one child node is required. > + > +Required properties for the child node(type M/N/O): > +- pwm_period : indicates type M/N/O PWM period, as per the AST2400/AST2500 > + datasheet. integer value in the range 0 to 255. > + > +- pwm_clock_division_l : indicates type M/N/O PWM clock division L value, > + as per the AST2400/AST2500 datasheet. > + integer value in the range 0 to 15. > + 0 here indicates divide 1, 1 indicates divide 2, > + 2 indicates divide 4, 3 indicates divide 6, and so on > + till 15 indicates divide 30. > + > +- pwm_clock_division_h : indicates type M/N/O PWM clock division H value, > + as per the AST2400/AST2500 datasheet. > + integer value in the range 0 to 15. > + 0 here indicates divide 1, 1 indicates divide 2, > + 2 indicates divide 4, 3 indicates divide 8, and so on > + till 15 indicates divide 32768. Can't you have a single divider value and driver convert to register values? Really, you should specify the PWM period in ns/us and calculate the divider based on the input clock freq. (i.e. use the clock binding). There's already PWM binding to specify the period. I think you should have a node for the fan using the PWM binding and perhaps moving some of these properties to the fan node. > + > +- fan_tach_enable : indicates fan tach enable of type M/N/O as per the > + AST2400/AST2500 datasheet. boolean value. > + > +- fan_tach_clock_division : indicates fan tach clock division as per the > + AST2400/AST2500 datasheet. > + integer value in the range 0 to 7. > + 0 indicates divide 4, 1 indicates divide 16, > + 2 indicates divide 64, 3 indicates divide 256 > + and so on till 7 indicates divide 65536. > + > +- fan_tach_mode_selection : indicates fan tach mode mode selection as per the > + AST2400/AST2500 datasheet. integer value in the > + range 0 to 2. 0 indicates falling edge, 1 indicates > + rising edge and 2 indicates both edges. > + > +- fan_tach_period : indicates fan tach period as per the AST2400/AST2500 > + datasheet. integer value (can be upto 16 bits long). > + > +pwm_port subnode format: > +======================== > +Under pwm_port subnode there can upto 8 child nodes each indicating values > +for one of the 8 PWM output ports. > + > +Required properties for each child node(starting from PWM A through PWM H): > +- enable : enable PWM #X port, X ranges from A through H. boolean value. A connection in the PWM binding would imply this. > + > +- type : indicates type selection value of PWM #X port, X ranges from A > + through H. integer value in the range 0 to 2; > + 0 indicates type M, 1 indicates type N, 2 indicates type O. This is meaningless to me. > +- fan_ctrl : set the PWM fan control initial value. integer value between > + 0(off) and 255(full speed). > + > +fan_tach_channel subnode format: > +================================ > +Under fan_tach_channel subnode there can be upto 16 child nodes each indicating > +values for one of the 16 fan tach channels. > + > +Required properties for each child node(starting from fan tach #0 through > +fan tach #16): > +- fan-ctrl-gpios : should specify the tachometer input pin on the hardware. I don't understand the connection here. You have a tach block with 16 inputs on the SoC, so why the GPIOs? If the GPIOs go to the fan, then a fan node should have the GPIO properties. > + > +- enable : enable fan tach #X, X ranges from 0 through 16. boolean value. 'status' property is the standard way to enable a node or not. Not sure if that makes sense here though. > + > +- pwm_source : indicates PWM source of fan tach #X, X ranges from 0 through 16. > + integer value in the range 0 to 7. 0 indicates PWM port A, > + 1 indicates PWM port B and so on till 7 indicates PWM port H.
On Fri, Jan 13, 2017 at 8:21 AM, Rob Herring <robh@kernel.org> wrote: > On Mon, Jan 09, 2017 at 01:59:34PM -0800, Jaghathiswari Rankappagounder > Natarajan wrote: > > This binding provides interface for adding values related to ASPEED > > AST2400/2500 PWM and Fan tach controller support. > > The PWM controller can support upto 8 PWM output ports. > > The Fan tach controller can support upto 16 tachometer inputs. > > PWM clock types M, N and 0 are three types just to have three independent > > PWM sources. > > > > Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com > > > > --- > > .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 153 > +++++++++++++++++++++ > > Perhaps bindings/pwm/... even though this is more than just PWM. > The corresponding hwmon driver is present in /drivers/hwmon/. So would it make more sense to include the devicetree document for that driver in /devicetree/bindings/hwmon/. In future if there any more hwmon related functionalities are added to this driver then /devicetree/bindings/hwmon/ would be more relevant. > > > 1 file changed, 153 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm- > tacho.txt > > > > diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt > b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt > > new file mode 100644 > > index 000000000000..8f346409ee8c > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt > > @@ -0,0 +1,153 @@ > > +ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver > > + > > +The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED > Fan Tacho > > +controller can support upto 16 tachometer inputs. The PWM controller > supports > > +3 types of frequency mode PWM for fan speed control. PWM clock types M, > N and 0 > > +are 3 types of frequency mode PWM just to have 3 independent PWM > sources. > > + > > +Required properties for pwm_tacho node: > > +- #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 > AST2400/AST2500 PWM > > + ports. > > + > > +- compatible : should be "aspeed,aspeed2400-pwm-tacho" for AST2400 or > > + "aspeed,aspeed2500-pwm-tacho" for AST2500. > > + > > +- clocks : a fixed clock providing input clock frequency(PWM > > + and Fan Tach clock) > > + > > +type_values subnode format: > > Don't use '_' in node or property names. > Done. > > > +=========================== > > +Under type_values subnode there can be upto 3 child nodes indicating > type M/N/O > > +values. Atleast one child node is required. > > + > > +Required properties for the child node(type M/N/O): > > +- pwm_period : indicates type M/N/O PWM period, as per the > AST2400/AST2500 > > + datasheet. integer value in the range 0 to 255. > > > > + > > +- pwm_clock_division_l : indicates type M/N/O PWM clock division L > value, > > + as per the AST2400/AST2500 datasheet. > > + integer value in the range 0 to 15. > > + 0 here indicates divide 1, 1 indicates divide 2, > > + 2 indicates divide 4, 3 indicates divide 6, and > so on > > + till 15 indicates divide 30. > > + > > +- pwm_clock_division_h : indicates type M/N/O PWM clock division H > value, > > + as per the AST2400/AST2500 datasheet. > > + integer value in the range 0 to 15. > > + 0 here indicates divide 1, 1 indicates divide 2, > > + 2 indicates divide 4, 3 indicates divide 8, and > so on > > + till 15 indicates divide 32768. > > Can't you have a single divider value and driver convert to register > values? Really, you should specify the PWM period in ns/us and calculate > the divider based on the input clock freq. (i.e. use the clock binding). > There's already PWM binding to specify the period. > > I think you should have a node for the fan using the PWM binding and > perhaps moving some of these properties to the fan node. > There are three different types M, N, O. These three types are just to have different PWM/Fan Tach settings. Each type can have the following settings: PWM related: 1) PWM period 2) PWM clock division high 3) PWM clock division low Fan Tach related: 1) Fan Tach type enable 2) Fan Tach clock division 3) Fan Tach mode selection 4) Fan Tach period Each PWM port should be assigned a type which can be type M, N or O. Each Fan Tach channel should be assigned a PWM source, which can be PWM port A through H. The Fan Tach channel implicitly gets the type (M, N or O), from the type assigned to its PWM source. If the clock type is type M then : The PWM frequency = 24MHz / (type M clock division L value * type M clock division H value * (type M PWM period value + 1)) Because of this formula, I thought I could use properties to specify type clock division High and Low and type period values so that the desired PWM frequency be achieved. > > + > > +- fan_tach_enable : indicates fan tach enable of type M/N/O as per the > > + AST2400/AST2500 datasheet. boolean value. > > + > > +- fan_tach_clock_division : indicates fan tach clock division as per the > > + AST2400/AST2500 datasheet. > > + integer value in the range 0 to 7. > > + 0 indicates divide 4, 1 indicates divide 16, > > + 2 indicates divide 64, 3 indicates divide 256 > > + and so on till 7 indicates divide 65536. > > + > > +- fan_tach_mode_selection : indicates fan tach mode mode selection as > per the > > + AST2400/AST2500 datasheet. integer value in the > > + range 0 to 2. 0 indicates falling edge, 1 > indicates > > + rising edge and 2 indicates both edges. > > + > > +- fan_tach_period : indicates fan tach period as per the AST2400/AST2500 > > + datasheet. integer value (can be upto 16 bits long). > > + > > +pwm_port subnode format: > > +======================== > > +Under pwm_port subnode there can upto 8 child nodes each indicating > values > > +for one of the 8 PWM output ports. > > + > > +Required properties for each child node(starting from PWM A through PWM > H): > > +- enable : enable PWM #X port, X ranges from A through H. boolean value. > > A connection in the PWM binding would imply this. > Had to change this property to indicate PWM port number. > > > + > > +- type : indicates type selection value of PWM #X port, X ranges from A > > + through H. integer value in the range 0 to 2; > > + 0 indicates type M, 1 indicates type N, 2 indicates type O. > > This is meaningless to me. > Each PWM port should be assigned a type which can be type M, N or O. > > > +- fan_ctrl : set the PWM fan control initial value. integer value > between > > + 0(off) and 255(full speed). > > + > > +fan_tach_channel subnode format: > > +================================ > > +Under fan_tach_channel subnode there can be upto 16 child nodes each > indicating > > +values for one of the 16 fan tach channels. > > + > > +Required properties for each child node(starting from fan tach #0 > through > > +fan tach #16): > > +- fan-ctrl-gpios : should specify the tachometer input pin on the > hardware. > > I don't understand the connection here. You have a tach block with 16 > inputs on the SoC, so why the GPIOs? If the GPIOs go to the fan, then a > fan node should have the GPIO properties. > This property specifies the tachometer input GPIO pin on the hardware. Fan Tachometer function can only work when GPIO is in ”input mode” . > > + > > +- enable : enable fan tach #X, X ranges from 0 through 16. boolean > value. > > 'status' property is the standard way to enable a node or not. Not sure > if that makes sense here though. > Had to change this property to indicate Fan tach channel number. > > > + > > +- pwm_source : indicates PWM source of fan tach #X, X ranges from 0 > through 16. > > + integer value in the range 0 to 7. 0 indicates PWM port A, > > + 1 indicates PWM port B and so on till 7 indicates PWM port > H. >
diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt new file mode 100644 index 000000000000..8f346409ee8c --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt @@ -0,0 +1,153 @@ +ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver + +The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho +controller can support upto 16 tachometer inputs. The PWM controller supports +3 types of frequency mode PWM for fan speed control. PWM clock types M, N and 0 +are 3 types of frequency mode PWM just to have 3 independent PWM sources. + +Required properties for pwm_tacho node: +- #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 AST2400/AST2500 PWM + ports. + +- compatible : should be "aspeed,aspeed2400-pwm-tacho" for AST2400 or + "aspeed,aspeed2500-pwm-tacho" for AST2500. + +- clocks : a fixed clock providing input clock frequency(PWM + and Fan Tach clock) + +type_values subnode format: +=========================== +Under type_values subnode there can be upto 3 child nodes indicating type M/N/O +values. Atleast one child node is required. + +Required properties for the child node(type M/N/O): +- pwm_period : indicates type M/N/O PWM period, as per the AST2400/AST2500 + datasheet. integer value in the range 0 to 255. + +- pwm_clock_division_l : indicates type M/N/O PWM clock division L value, + as per the AST2400/AST2500 datasheet. + integer value in the range 0 to 15. + 0 here indicates divide 1, 1 indicates divide 2, + 2 indicates divide 4, 3 indicates divide 6, and so on + till 15 indicates divide 30. + +- pwm_clock_division_h : indicates type M/N/O PWM clock division H value, + as per the AST2400/AST2500 datasheet. + integer value in the range 0 to 15. + 0 here indicates divide 1, 1 indicates divide 2, + 2 indicates divide 4, 3 indicates divide 8, and so on + till 15 indicates divide 32768. + +- fan_tach_enable : indicates fan tach enable of type M/N/O as per the + AST2400/AST2500 datasheet. boolean value. + +- fan_tach_clock_division : indicates fan tach clock division as per the + AST2400/AST2500 datasheet. + integer value in the range 0 to 7. + 0 indicates divide 4, 1 indicates divide 16, + 2 indicates divide 64, 3 indicates divide 256 + and so on till 7 indicates divide 65536. + +- fan_tach_mode_selection : indicates fan tach mode mode selection as per the + AST2400/AST2500 datasheet. integer value in the + range 0 to 2. 0 indicates falling edge, 1 indicates + rising edge and 2 indicates both edges. + +- fan_tach_period : indicates fan tach period as per the AST2400/AST2500 + datasheet. integer value (can be upto 16 bits long). + +pwm_port subnode format: +======================== +Under pwm_port subnode there can upto 8 child nodes each indicating values +for one of the 8 PWM output ports. + +Required properties for each child node(starting from PWM A through PWM H): +- enable : enable PWM #X port, X ranges from A through H. boolean value. + +- type : indicates type selection value of PWM #X port, X ranges from A + through H. integer value in the range 0 to 2; + 0 indicates type M, 1 indicates type N, 2 indicates type O. + +- fan_ctrl : set the PWM fan control initial value. integer value between + 0(off) and 255(full speed). + +fan_tach_channel subnode format: +================================ +Under fan_tach_channel subnode there can be upto 16 child nodes each indicating +values for one of the 16 fan tach channels. + +Required properties for each child node(starting from fan tach #0 through +fan tach #16): +- fan-ctrl-gpios : should specify the tachometer input pin on the hardware. + +- enable : enable fan tach #X, X ranges from 0 through 16. boolean value. + +- pwm_source : indicates PWM source of fan tach #X, X ranges from 0 through 16. + integer value in the range 0 to 7. 0 indicates PWM port A, + 1 indicates PWM port B and so on till 7 indicates PWM port H. + +Examples: + +pwm_tacho_fixed_clk: fixedclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <24000000>; +} + +pwm_tacho: pwm-tacho-controller@1e786000 { + #address-cells = <1>; + #size-cells = <1>; + reg = <0x1E786000 0x1000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>; + compatible = "aspeed,aspeed2500-pwm-tacho"; + clocks = <&pwm_tacho_fixed_clk>; + type_values { + typem { + pwm_period = /bits/ 8 <0x5F>; + pwm_clock_division_h = /bits/ 8 <0x00>; + pwm_clock_division_l = /bits/ 8 <0x05>; + fan_tach_enable; + fan_tach_clock_division = /bits/ 8 <0x00>; + fan_tach_mode_selection = /bits/ 8 <0x00>; + fan_tach_period = /bits/ 16 <0x1000>; + }; + }; + + pwm_port { + pwm_port0 { + enable; + type = /bits/ 8 <0x00>; + fan_ctrl = /bits/ 8 <0xFF>; + }; + + pwm_port1 { + enable; + type = /bits/ 8 <0x00>; + fan_ctrl = /bits/ 8 <0xFF>; + }; + }; + + fan_tach_channel { + fan_tach0 { + fan-ctrl-gpios = <&gpio ASPEED_GPIO(O, 0) GPIO_ACTIVE_HIGH>; + enable; + pwm_source = /bits/ 8 <0x00>; + }; + + fan_tach1 { + fan-ctrl-gpios = <&gpio ASPEED_GPIO(O, 1) GPIO_ACTIVE_HIGH>; + enable; + pwm_source = /bits/ 8 <0x01>; + }; + + }; +};
This binding provides interface for adding values related to ASPEED AST2400/2500 PWM and Fan tach controller support. The PWM controller can support upto 8 PWM output ports. The Fan tach controller can support upto 16 tachometer inputs. PWM clock types M, N and 0 are three types just to have three independent PWM sources. Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com> --- .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 153 +++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt -- 2.11.0.390.gc69c2f50cf-goog