* [PATCH v1 0/2] hwmon: Add NPCM7xx PWM driver support
@ 2018-05-29 10:02 Tomer Maimon
2018-05-29 10:02 ` [PATCH v1 1/2] dt-binding: hwmon: Add NPCM7xx PWM documentation Tomer Maimon
2018-05-29 10:02 ` [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver Tomer Maimon
0 siblings, 2 replies; 9+ messages in thread
From: Tomer Maimon @ 2018-05-29 10:02 UTC (permalink / raw)
To: robh+dt, mark.rutland, jdelvare, linux, avifishman70, yuenn,
brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc, Tomer Maimon
This patch set adds Pulse Width Modulation (PWM) support
for the Nuvoton NPCM7xx Baseboard Management Controller (BMC).
The NPCM7xx PWM controller can support 8 PWM output ports.
The hwmon driver provides sysfs entries through which the user can
configure and get the duty cycle between 0 (disable the specific PWM)
to 255 of particular PWM output port.
The NPCM7xx PWM driver tested on NPCM750 evaluation board.
Tomer Maimon (2):
dt-binding: hwmon: Add NPCM7xx PWM documentation
hwmon: npcm-pwm: add NPCM7xx PWM driver
.../devicetree/bindings/hwmon/npcm7xx-pwm.txt | 25 ++
drivers/hwmon/Kconfig | 6 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/npcm7xx-pwm.c | 363 +++++++++++++++++++++
4 files changed, 395 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
create mode 100644 drivers/hwmon/npcm7xx-pwm.c
--
2.14.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] dt-binding: hwmon: Add NPCM7xx PWM documentation
2018-05-29 10:02 [PATCH v1 0/2] hwmon: Add NPCM7xx PWM driver support Tomer Maimon
@ 2018-05-29 10:02 ` Tomer Maimon
2018-05-29 16:57 ` Guenter Roeck
2018-05-29 10:02 ` [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver Tomer Maimon
1 sibling, 1 reply; 9+ messages in thread
From: Tomer Maimon @ 2018-05-29 10:02 UTC (permalink / raw)
To: robh+dt, mark.rutland, jdelvare, linux, avifishman70, yuenn,
brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc, Tomer Maimon
Added device tree binding documentation for Nuvoton BMC
NPCM7xx Pulse Width Modulation (PWM) controller.
Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
.../devicetree/bindings/hwmon/npcm7xx-pwm.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
diff --git a/Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt b/Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
new file mode 100644
index 000000000000..2d8a7ccdc8cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
@@ -0,0 +1,25 @@
+Nuvoton NPCM7xx Pulse-width modulation (PWM) controller device driver
+
+The NPCM7xx has two identical PWM controller modules,
+Each module has four PWM controller outputs.
+NPCM7xx PWM controller module 0 outputs PWM0-3 and NPCM7xx PWM controller
+module 1 outputs PWM4-7.
+
+Required properties:
+- compatible : "nuvoton,npcm750-pwm" for Poleg NPCM7XX.
+- reg : Offset and length of the registers set for the device.
+- clocks : phandle of pwm reference clock.
+- pinctrl-names : a pinctrl state named "default" must be defined.
+- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
+
+pwm:pwm@f0103000 {
+ compatible = "nuvoton,npcm750-pwm";
+ reg = <0xf0103000 0x50
+ 0xf0104000 0x50>;
+ clocks = <&clk NPCM7XX_CLK_APB3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm0_pins &pwm1_pins
+ &pwm2_pins &pwm3_pins
+ &pwm4_pins &pwm5_pins
+ &pwm6_pins &pwm7_pins>;
+};
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
2018-05-29 10:02 [PATCH v1 0/2] hwmon: Add NPCM7xx PWM driver support Tomer Maimon
2018-05-29 10:02 ` [PATCH v1 1/2] dt-binding: hwmon: Add NPCM7xx PWM documentation Tomer Maimon
@ 2018-05-29 10:02 ` Tomer Maimon
2018-05-29 16:56 ` Guenter Roeck
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: Tomer Maimon @ 2018-05-29 10:02 UTC (permalink / raw)
To: robh+dt, mark.rutland, jdelvare, linux, avifishman70, yuenn,
brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc, Tomer Maimon
Add Nuvoton BMC NPCM7xx Pulse Width Modulation (PWM) driver.
The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
each module has four PWM controller outputs.
Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
drivers/hwmon/Kconfig | 6 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/npcm7xx-pwm.c | 363 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 370 insertions(+)
create mode 100644 drivers/hwmon/npcm7xx-pwm.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6ec307c93ece..693ba09cff8e 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1891,6 +1891,12 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
+config SENSORS_NPCM7XX
+ tristate "Nuvoton NPCM7XX PWM driver"
+ help
+ This driver provides support for Nuvoton NPCM7XX PWM
+ controller.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..24aad895a3bb 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
+obj-$(CONFIG_SENSORS_NPCM7XX) += npcm7xx-pwm.o
obj-$(CONFIG_PMBUS) += pmbus/
diff --git a/drivers/hwmon/npcm7xx-pwm.c b/drivers/hwmon/npcm7xx-pwm.c
new file mode 100644
index 000000000000..6122ca82b94d
--- /dev/null
+++ b/drivers/hwmon/npcm7xx-pwm.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2014-2018 Nuvoton Technology corporation.
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/sysfs.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+
+/* NPCM7XX PWM port base address */
+#define NPCM7XX_PWM_REG_PR 0x0
+#define NPCM7XX_PWM_REG_CSR 0x4
+#define NPCM7XX_PWM_REG_CR 0x8
+#define NPCM7XX_PWM_REG_CNRx(PORT) (0xC + (12 * PORT))
+#define NPCM7XX_PWM_REG_CMRx(PORT) (0x10 + (12 * PORT))
+#define NPCM7XX_PWM_REG_PDRx(PORT) (0x14 + (12 * PORT))
+#define NPCM7XX_PWM_REG_PIER 0x3C
+#define NPCM7XX_PWM_REG_PIIR 0x40
+
+#define NPCM7XX_PWM_CTRL_CH0_MODE_BIT BIT(3)
+#define NPCM7XX_PWM_CTRL_CH1_MODE_BIT BIT(11)
+#define NPCM7XX_PWM_CTRL_CH2_MODE_BIT BIT(15)
+#define NPCM7XX_PWM_CTRL_CH3_MODE_BIT BIT(19)
+
+#define NPCM7XX_PWM_CTRL_CH0_INV_BIT BIT(2)
+#define NPCM7XX_PWM_CTRL_CH1_INV_BIT BIT(10)
+#define NPCM7XX_PWM_CTRL_CH2_INV_BIT BIT(14)
+#define NPCM7XX_PWM_CTRL_CH3_INV_BIT BIT(18)
+
+#define NPCM7XX_PWM_CTRL_CH0_EN_BIT BIT(0)
+#define NPCM7XX_PWM_CTRL_CH1_EN_BIT BIT(8)
+#define NPCM7XX_PWM_CTRL_CH2_EN_BIT BIT(12)
+#define NPCM7XX_PWM_CTRL_CH3_EN_BIT BIT(16)
+
+/* Define the maximum PWM channel number */
+#define NPCM7XX_PWM_MAX_CHN_NUM 8
+#define NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE 4
+#define NPCM7XX_PWM_MAX_MODULES 2
+
+/* Define the Counter Register, value = 100 for match 100% */
+#define NPCM7XX_PWM_COUNTER_DEFALUT_NUM 255
+#define NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM 127
+
+#define NPCM7XX_PWM_COMPARATOR_MAX 255
+
+
+/* default all PWM channels PRESCALE2 = 1 */
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 0x4
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 0x40
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 0x400
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3 0x4000
+
+#define PWM_OUTPUT_FREQ_25KHZ 25000
+#define PWN_CNT_DEFAULT 256
+#define MIN_PRESCALE1 2
+#define NPCM7XX_PWM_PRESCALE_SHIFT_CH01 8
+
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT (NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 | \
+ NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 | \
+ NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 | \
+ NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3)
+
+#define NPCM7XX_PWM_CTRL_MODE_DEFALUT (NPCM7XX_PWM_CTRL_CH0_MODE_BIT | \
+ NPCM7XX_PWM_CTRL_CH1_MODE_BIT | \
+ NPCM7XX_PWM_CTRL_CH2_MODE_BIT | \
+ NPCM7XX_PWM_CTRL_CH3_MODE_BIT)
+
+#define NPCM7XX_PWM_CTRL_EN_DEFALUT (NPCM7XX_PWM_CTRL_CH0_EN_BIT | \
+ NPCM7XX_PWM_CTRL_CH1_EN_BIT | \
+ NPCM7XX_PWM_CTRL_CH2_EN_BIT | \
+ NPCM7XX_PWM_CTRL_CH3_EN_BIT)
+
+struct npcm7xx_pwm_data {
+ unsigned long clk_freq;
+ void __iomem *pwm_base[NPCM7XX_PWM_MAX_MODULES];
+ struct mutex npcm7xx_pwm_lock[NPCM7XX_PWM_MAX_CHN_NUM];
+};
+
+static const struct of_device_id pwm_dt_id[];
+
+static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_data *data, int channel,
+ u16 val)
+{
+ u32 PWMChannel = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+ u32 n_module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+ u32 u32TmpBuf = 0, ctrl_en_bit;
+
+ /*
+ * Config PWM Comparator register for setting duty cycle
+ */
+ if (val < 0 || val > NPCM7XX_PWM_COMPARATOR_MAX)
+ return -EINVAL;
+
+ /* write new CMR value */
+ iowrite32(val, data->pwm_base[n_module] +
+ NPCM7XX_PWM_REG_CMRx(PWMChannel));
+
+ u32TmpBuf = ioread32(data->pwm_base[n_module] + NPCM7XX_PWM_REG_CR);
+
+ switch (PWMChannel) {
+ case 0:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH0_EN_BIT;
+ break;
+ case 1:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH1_EN_BIT;
+ break;
+ case 2:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH2_EN_BIT;
+ break;
+ case 3:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH3_EN_BIT;
+ break;
+ default:
+ return -ENODEV;
+ }
+
+ if (val == 0)
+ /* Disable PWM */
+ u32TmpBuf &= ~(ctrl_en_bit);
+ else
+ /* Enable PWM */
+ u32TmpBuf |= ctrl_en_bit;
+
+ mutex_lock(&data->npcm7xx_pwm_lock[n_module]);
+ iowrite32(u32TmpBuf, data->pwm_base[n_module] + NPCM7XX_PWM_REG_CR);
+ mutex_unlock(&data->npcm7xx_pwm_lock[n_module]);
+
+ return 0;
+}
+
+static int npcm7xx_read_pwm(struct device *dev, u32 attr, int channel,
+ long *val)
+{
+ struct npcm7xx_pwm_data *data = dev_get_drvdata(dev);
+ u32 PWMChannel = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+ u32 n_module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ switch (attr) {
+ case hwmon_pwm_input:
+ *val = (long)ioread32(data->pwm_base[n_module] +
+ NPCM7XX_PWM_REG_CMRx(PWMChannel));
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int npcm7xx_write_pwm(struct device *dev, u32 attr, int channel,
+ long val)
+{
+ struct npcm7xx_pwm_data *data = dev_get_drvdata(dev);
+ int err = 0;
+
+ switch (attr) {
+ case hwmon_pwm_input:
+ err = npcm7xx_pwm_config_set(data, channel, (u16)val);
+ break;
+ default:
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ return err;
+}
+
+static umode_t npcm7xx_pwm_is_visible(const void *_data, u32 attr, int channel)
+{
+ switch (attr) {
+ case hwmon_pwm_input:
+ return 0644;
+ default:
+ return 0;
+ }
+}
+
+static int npcm7xx_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ switch (type) {
+ case hwmon_pwm:
+ return npcm7xx_read_pwm(dev, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int npcm7xx_write(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long val)
+{
+ switch (type) {
+ case hwmon_pwm:
+ return npcm7xx_write_pwm(dev, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static umode_t npcm7xx_is_visible(const void *data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_pwm:
+ return npcm7xx_pwm_is_visible(data, attr, channel);
+ default:
+ return 0;
+ }
+}
+
+static const u32 npcm7xx_pwm_config[] = {
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ 0
+};
+
+static const struct hwmon_channel_info npcm7xx_pwm = {
+ .type = hwmon_pwm,
+ .config = npcm7xx_pwm_config,
+};
+
+static const struct hwmon_channel_info *npcm7xx_info[] = {
+ &npcm7xx_pwm,
+ NULL
+};
+
+static const struct hwmon_ops npcm7xx_hwmon_ops = {
+ .is_visible = npcm7xx_is_visible,
+ .read = npcm7xx_read,
+ .write = npcm7xx_write,
+};
+
+static const struct hwmon_chip_info npcm7xx_chip_info = {
+ .ops = &npcm7xx_hwmon_ops,
+ .info = npcm7xx_info,
+};
+
+static int npcm7xx_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct npcm7xx_pwm_data *data;
+ struct resource res[NPCM7XX_PWM_MAX_MODULES];
+ struct device *hwmon;
+ struct clk *clk;
+ int m, ch, res_cnt, ret;
+ u32 Prescale_val, output_freq;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ for (res_cnt = 0; res_cnt < NPCM7XX_PWM_MAX_MODULES ; res_cnt++) {
+ ret = of_address_to_resource(dev->of_node, res_cnt,
+ &res[res_cnt]);
+ if (ret) {
+ pr_err("PWM of_address_to_resource fail ret %d\n",
+ ret);
+ return -EINVAL;
+ }
+
+ data->pwm_base[res_cnt] =
+ devm_ioremap_resource(dev, &(res[res_cnt]));
+ pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
+ res_cnt, (u32)data->pwm_base[res_cnt],
+ res[res_cnt].start, resource_size(&(res[res_cnt])));
+
+ if (!data->pwm_base[res_cnt]) {
+ pr_err("pwm probe failed: can't read pwm base address for resource %d.\n",
+ res_cnt);
+ return -ENOMEM;
+ }
+
+ mutex_init(&data->npcm7xx_pwm_lock[res_cnt]);
+ }
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ return -ENODEV;
+
+ data->clk_freq = clk_get_rate(clk);
+
+ /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
+ output_freq = data->clk_freq / PWN_CNT_DEFAULT;
+ Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
+
+ /* If Prescale_val = 0, then the prescale output clock is stopped */
+ if (Prescale_val < MIN_PRESCALE1)
+ Prescale_val = MIN_PRESCALE1;
+ /*
+ * Prescale_val need to decrement in one because in the PWM Prescale
+ * register the Prescale value increment by one
+ */
+ Prescale_val--;
+
+ /* Setting PWM Prescale Register value register to both modules */
+ Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
+
+ for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
+ iowrite32(Prescale_val,
+ data->pwm_base[m] + NPCM7XX_PWM_REG_PR);
+ iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
+ data->pwm_base[m] + NPCM7XX_PWM_REG_CSR);
+ iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
+ data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
+
+ for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM; ch++) {
+ iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
+ data->pwm_base[m] + NPCM7XX_PWM_REG_CNRx(ch));
+ iowrite32(NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM,
+ data->pwm_base[m] + NPCM7XX_PWM_REG_CMRx(ch));
+ }
+
+ iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT |
+ NPCM7XX_PWM_CTRL_EN_DEFALUT,
+ data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
+ }
+
+ hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm", data,
+ &npcm7xx_chip_info, NULL);
+
+ if (IS_ERR(hwmon)) {
+ pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
+ return PTR_ERR(hwmon);
+ }
+
+ pr_info("NPCM7XX PWM Driver probed, PWM output Freq %dHz\n",
+ output_freq / ((Prescale_val & 0xf) + 1));
+
+ return 0;
+}
+
+static const struct of_device_id of_pwm_match_table[] = {
+ { .compatible = "nuvoton,npcm750-pwm", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_match_table);
+
+static struct platform_driver npcm7xx_pwm_driver = {
+ .probe = npcm7xx_pwm_probe,
+ .driver = {
+ .name = "npcm7xx_pwm",
+ .of_match_table = of_pwm_match_table,
+ },
+};
+
+module_platform_driver(npcm7xx_pwm_driver);
+
+MODULE_DESCRIPTION("Nuvoton NPCM7XX PWM Driver");
+MODULE_AUTHOR("Tomer Maimon <tomer.maimon@nuvoton.com>");
+MODULE_LICENSE("GPL v2");
--
2.14.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
2018-05-29 10:02 ` [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver Tomer Maimon
@ 2018-05-29 16:56 ` Guenter Roeck
[not found] ` <CAP6Zq1h8_n8R=dqmBxC_bK_YstLt1kkE6UCp2OVsNpKp=RK8og@mail.gmail.com>
2018-05-31 7:16 ` kbuild test robot
2018-06-10 11:37 ` kbuild test robot
2 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2018-05-29 16:56 UTC (permalink / raw)
To: Tomer Maimon
Cc: robh+dt, mark.rutland, jdelvare, avifishman70, yuenn,
brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc, linux-pwm, Thierry Reding
On Tue, May 29, 2018 at 01:02:21PM +0300, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM7xx Pulse Width Modulation (PWM) driver.
>
> The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
> each module has four PWM controller outputs.
>
I don't see it guaranteed that all future NPCM7xx devices will be PWM
controllers, much less that they will be compatible to the chip really
supported here. NPCM750, I presume ? I would suggest name the driver
accordingly.
As a generic pwm driver, not specifically tied to a fan controller,
this driver does not belong into hwmon. It should be added to the pwm
subsystem. Copying the maintainer and mailing list.
If the pwm chip is used to control fans, the existing pwm-fan driver can
then be used to create the necessary mapping from pwm controls to fans.
Guenter
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
> drivers/hwmon/Kconfig | 6 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/npcm7xx-pwm.c | 363 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 370 insertions(+)
> create mode 100644 drivers/hwmon/npcm7xx-pwm.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 6ec307c93ece..693ba09cff8e 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1891,6 +1891,12 @@ config SENSORS_XGENE
> If you say yes here you get support for the temperature
> and power sensors for APM X-Gene SoC.
>
> +config SENSORS_NPCM7XX
> + tristate "Nuvoton NPCM7XX PWM driver"
> + help
> + This driver provides support for Nuvoton NPCM7XX PWM
> + controller.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a36e6c4..24aad895a3bb 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
> obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
> obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
> obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
> +obj-$(CONFIG_SENSORS_NPCM7XX) += npcm7xx-pwm.o
>
> obj-$(CONFIG_PMBUS) += pmbus/
>
> diff --git a/drivers/hwmon/npcm7xx-pwm.c b/drivers/hwmon/npcm7xx-pwm.c
> new file mode 100644
> index 000000000000..6122ca82b94d
> --- /dev/null
> +++ b/drivers/hwmon/npcm7xx-pwm.c
> @@ -0,0 +1,363 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2014-2018 Nuvoton Technology corporation.
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/sysfs.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +
> +/* NPCM7XX PWM port base address */
> +#define NPCM7XX_PWM_REG_PR 0x0
> +#define NPCM7XX_PWM_REG_CSR 0x4
> +#define NPCM7XX_PWM_REG_CR 0x8
> +#define NPCM7XX_PWM_REG_CNRx(PORT) (0xC + (12 * PORT))
> +#define NPCM7XX_PWM_REG_CMRx(PORT) (0x10 + (12 * PORT))
> +#define NPCM7XX_PWM_REG_PDRx(PORT) (0x14 + (12 * PORT))
> +#define NPCM7XX_PWM_REG_PIER 0x3C
> +#define NPCM7XX_PWM_REG_PIIR 0x40
> +
> +#define NPCM7XX_PWM_CTRL_CH0_MODE_BIT BIT(3)
> +#define NPCM7XX_PWM_CTRL_CH1_MODE_BIT BIT(11)
> +#define NPCM7XX_PWM_CTRL_CH2_MODE_BIT BIT(15)
> +#define NPCM7XX_PWM_CTRL_CH3_MODE_BIT BIT(19)
> +
> +#define NPCM7XX_PWM_CTRL_CH0_INV_BIT BIT(2)
> +#define NPCM7XX_PWM_CTRL_CH1_INV_BIT BIT(10)
> +#define NPCM7XX_PWM_CTRL_CH2_INV_BIT BIT(14)
> +#define NPCM7XX_PWM_CTRL_CH3_INV_BIT BIT(18)
> +
> +#define NPCM7XX_PWM_CTRL_CH0_EN_BIT BIT(0)
> +#define NPCM7XX_PWM_CTRL_CH1_EN_BIT BIT(8)
> +#define NPCM7XX_PWM_CTRL_CH2_EN_BIT BIT(12)
> +#define NPCM7XX_PWM_CTRL_CH3_EN_BIT BIT(16)
> +
> +/* Define the maximum PWM channel number */
> +#define NPCM7XX_PWM_MAX_CHN_NUM 8
> +#define NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE 4
> +#define NPCM7XX_PWM_MAX_MODULES 2
> +
> +/* Define the Counter Register, value = 100 for match 100% */
> +#define NPCM7XX_PWM_COUNTER_DEFALUT_NUM 255
> +#define NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM 127
> +
> +#define NPCM7XX_PWM_COMPARATOR_MAX 255
> +
> +
> +/* default all PWM channels PRESCALE2 = 1 */
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 0x4
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 0x40
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 0x400
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3 0x4000
> +
> +#define PWM_OUTPUT_FREQ_25KHZ 25000
> +#define PWN_CNT_DEFAULT 256
> +#define MIN_PRESCALE1 2
> +#define NPCM7XX_PWM_PRESCALE_SHIFT_CH01 8
> +
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT (NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3)
> +
> +#define NPCM7XX_PWM_CTRL_MODE_DEFALUT (NPCM7XX_PWM_CTRL_CH0_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH1_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH2_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH3_MODE_BIT)
> +
> +#define NPCM7XX_PWM_CTRL_EN_DEFALUT (NPCM7XX_PWM_CTRL_CH0_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH1_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH2_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH3_EN_BIT)
> +
> +struct npcm7xx_pwm_data {
> + unsigned long clk_freq;
> + void __iomem *pwm_base[NPCM7XX_PWM_MAX_MODULES];
> + struct mutex npcm7xx_pwm_lock[NPCM7XX_PWM_MAX_CHN_NUM];
> +};
> +
> +static const struct of_device_id pwm_dt_id[];
> +
> +static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_data *data, int channel,
> + u16 val)
> +{
> + u32 PWMChannel = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 n_module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 u32TmpBuf = 0, ctrl_en_bit;
> +
> + /*
> + * Config PWM Comparator register for setting duty cycle
> + */
> + if (val < 0 || val > NPCM7XX_PWM_COMPARATOR_MAX)
> + return -EINVAL;
> +
> + /* write new CMR value */
> + iowrite32(val, data->pwm_base[n_module] +
> + NPCM7XX_PWM_REG_CMRx(PWMChannel));
> +
> + u32TmpBuf = ioread32(data->pwm_base[n_module] + NPCM7XX_PWM_REG_CR);
> +
> + switch (PWMChannel) {
> + case 0:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH0_EN_BIT;
> + break;
> + case 1:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH1_EN_BIT;
> + break;
> + case 2:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH2_EN_BIT;
> + break;
> + case 3:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH3_EN_BIT;
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + if (val == 0)
> + /* Disable PWM */
> + u32TmpBuf &= ~(ctrl_en_bit);
> + else
> + /* Enable PWM */
> + u32TmpBuf |= ctrl_en_bit;
> +
> + mutex_lock(&data->npcm7xx_pwm_lock[n_module]);
> + iowrite32(u32TmpBuf, data->pwm_base[n_module] + NPCM7XX_PWM_REG_CR);
> + mutex_unlock(&data->npcm7xx_pwm_lock[n_module]);
> +
> + return 0;
> +}
> +
> +static int npcm7xx_read_pwm(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct npcm7xx_pwm_data *data = dev_get_drvdata(dev);
> + u32 PWMChannel = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 n_module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + *val = (long)ioread32(data->pwm_base[n_module] +
> + NPCM7XX_PWM_REG_CMRx(PWMChannel));
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int npcm7xx_write_pwm(struct device *dev, u32 attr, int channel,
> + long val)
> +{
> + struct npcm7xx_pwm_data *data = dev_get_drvdata(dev);
> + int err = 0;
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + err = npcm7xx_pwm_config_set(data, channel, (u16)val);
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + break;
> + }
> +
> + return err;
> +}
> +
> +static umode_t npcm7xx_pwm_is_visible(const void *_data, u32 attr, int channel)
> +{
> + switch (attr) {
> + case hwmon_pwm_input:
> + return 0644;
> + default:
> + return 0;
> + }
> +}
> +
> +static int npcm7xx_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_read_pwm(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int npcm7xx_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_write_pwm(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t npcm7xx_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_pwm_is_visible(data, attr, channel);
> + default:
> + return 0;
> + }
> +}
> +
> +static const u32 npcm7xx_pwm_config[] = {
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + 0
> +};
> +
> +static const struct hwmon_channel_info npcm7xx_pwm = {
> + .type = hwmon_pwm,
> + .config = npcm7xx_pwm_config,
> +};
> +
> +static const struct hwmon_channel_info *npcm7xx_info[] = {
> + &npcm7xx_pwm,
> + NULL
> +};
> +
> +static const struct hwmon_ops npcm7xx_hwmon_ops = {
> + .is_visible = npcm7xx_is_visible,
> + .read = npcm7xx_read,
> + .write = npcm7xx_write,
> +};
> +
> +static const struct hwmon_chip_info npcm7xx_chip_info = {
> + .ops = &npcm7xx_hwmon_ops,
> + .info = npcm7xx_info,
> +};
> +
> +static int npcm7xx_pwm_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct npcm7xx_pwm_data *data;
> + struct resource res[NPCM7XX_PWM_MAX_MODULES];
> + struct device *hwmon;
> + struct clk *clk;
> + int m, ch, res_cnt, ret;
> + u32 Prescale_val, output_freq;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + for (res_cnt = 0; res_cnt < NPCM7XX_PWM_MAX_MODULES ; res_cnt++) {
> + ret = of_address_to_resource(dev->of_node, res_cnt,
> + &res[res_cnt]);
> + if (ret) {
> + pr_err("PWM of_address_to_resource fail ret %d\n",
> + ret);
> + return -EINVAL;
> + }
> +
> + data->pwm_base[res_cnt] =
> + devm_ioremap_resource(dev, &(res[res_cnt]));
> + pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> + res_cnt, (u32)data->pwm_base[res_cnt],
> + res[res_cnt].start, resource_size(&(res[res_cnt])));
> +
> + if (!data->pwm_base[res_cnt]) {
> + pr_err("pwm probe failed: can't read pwm base address for resource %d.\n",
> + res_cnt);
> + return -ENOMEM;
> + }
> +
> + mutex_init(&data->npcm7xx_pwm_lock[res_cnt]);
> + }
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk))
> + return -ENODEV;
> +
> + data->clk_freq = clk_get_rate(clk);
> +
> + /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
> + output_freq = data->clk_freq / PWN_CNT_DEFAULT;
> + Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
> +
> + /* If Prescale_val = 0, then the prescale output clock is stopped */
> + if (Prescale_val < MIN_PRESCALE1)
> + Prescale_val = MIN_PRESCALE1;
> + /*
> + * Prescale_val need to decrement in one because in the PWM Prescale
> + * register the Prescale value increment by one
> + */
> + Prescale_val--;
> +
> + /* Setting PWM Prescale Register value register to both modules */
> + Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
> +
> + for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
> + iowrite32(Prescale_val,
> + data->pwm_base[m] + NPCM7XX_PWM_REG_PR);
> + iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
> + data->pwm_base[m] + NPCM7XX_PWM_REG_CSR);
> + iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
> + data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
> +
> + for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM; ch++) {
> + iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
> + data->pwm_base[m] + NPCM7XX_PWM_REG_CNRx(ch));
> + iowrite32(NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM,
> + data->pwm_base[m] + NPCM7XX_PWM_REG_CMRx(ch));
> + }
> +
> + iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT |
> + NPCM7XX_PWM_CTRL_EN_DEFALUT,
> + data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
> + }
> +
> + hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm", data,
> + &npcm7xx_chip_info, NULL);
> +
> + if (IS_ERR(hwmon)) {
> + pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
> + return PTR_ERR(hwmon);
> + }
> +
> + pr_info("NPCM7XX PWM Driver probed, PWM output Freq %dHz\n",
> + output_freq / ((Prescale_val & 0xf) + 1));
> +
> + return 0;
> +}
> +
> +static const struct of_device_id of_pwm_match_table[] = {
> + { .compatible = "nuvoton,npcm750-pwm", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_pwm_match_table);
> +
> +static struct platform_driver npcm7xx_pwm_driver = {
> + .probe = npcm7xx_pwm_probe,
> + .driver = {
> + .name = "npcm7xx_pwm",
> + .of_match_table = of_pwm_match_table,
> + },
> +};
> +
> +module_platform_driver(npcm7xx_pwm_driver);
> +
> +MODULE_DESCRIPTION("Nuvoton NPCM7XX PWM Driver");
> +MODULE_AUTHOR("Tomer Maimon <tomer.maimon@nuvoton.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] dt-binding: hwmon: Add NPCM7xx PWM documentation
2018-05-29 10:02 ` [PATCH v1 1/2] dt-binding: hwmon: Add NPCM7xx PWM documentation Tomer Maimon
@ 2018-05-29 16:57 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2018-05-29 16:57 UTC (permalink / raw)
To: Tomer Maimon
Cc: robh+dt, mark.rutland, jdelvare, avifishman70, yuenn,
brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc
On Tue, May 29, 2018 at 01:02:20PM +0300, Tomer Maimon wrote:
> Added device tree binding documentation for Nuvoton BMC
> NPCM7xx Pulse Width Modulation (PWM) controller.
>
As mentioned in the other commit, this should be a pwm driver,
not a hwmon driver.
Guenter
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
> .../devicetree/bindings/hwmon/npcm7xx-pwm.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt b/Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
> new file mode 100644
> index 000000000000..2d8a7ccdc8cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/npcm7xx-pwm.txt
> @@ -0,0 +1,25 @@
> +Nuvoton NPCM7xx Pulse-width modulation (PWM) controller device driver
> +
> +The NPCM7xx has two identical PWM controller modules,
> +Each module has four PWM controller outputs.
> +NPCM7xx PWM controller module 0 outputs PWM0-3 and NPCM7xx PWM controller
> +module 1 outputs PWM4-7.
> +
> +Required properties:
> +- compatible : "nuvoton,npcm750-pwm" for Poleg NPCM7XX.
> +- reg : Offset and length of the registers set for the device.
> +- clocks : phandle of pwm reference clock.
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
> +
> +pwm:pwm@f0103000 {
> + compatible = "nuvoton,npcm750-pwm";
> + reg = <0xf0103000 0x50
> + 0xf0104000 0x50>;
> + clocks = <&clk NPCM7XX_CLK_APB3>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm0_pins &pwm1_pins
> + &pwm2_pins &pwm3_pins
> + &pwm4_pins &pwm5_pins
> + &pwm6_pins &pwm7_pins>;
> +};
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
[not found] ` <CAP6Zq1h8_n8R=dqmBxC_bK_YstLt1kkE6UCp2OVsNpKp=RK8og@mail.gmail.com>
@ 2018-05-30 16:46 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2018-05-30 16:46 UTC (permalink / raw)
To: Tomer Maimon
Cc: Rob Herring, Mark Rutland, jdelvare, Avi Fishman, Nancy Yuen,
Brendan Higgins, Patrick Venture, Joel Stanley, devicetree,
Linux Kernel Mailing List, linux-hwmon, OpenBMC Maillist,
linux-pwm, Thierry Reding
On Wed, May 30, 2018 at 06:44:58PM +0300, Tomer Maimon wrote:
> Hi Guenter,
>
> Thanks for your prompt reply!
>
>
> On 29 May 2018 at 19:56, Guenter Roeck <linux@roeck-us.net> wrote:
>
> > On Tue, May 29, 2018 at 01:02:21PM +0300, Tomer Maimon wrote:
> > > Add Nuvoton BMC NPCM7xx Pulse Width Modulation (PWM) driver.
> > >
> > > The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
> > > each module has four PWM controller outputs.
> > >
> >
> > I don't see it guaranteed that all future NPCM7xx devices will be PWM
> > controllers, much less that they will be compatible to the chip really
> >
>
> Actually all NPCM7xx family have PWM and FAN modules support,
>
>
> > supported here. NPCM750, I presume ? I would suggest name the driver
> > accordingly.
> >
>
> The compatible name can not be a family name like nuvoton,npcm7xx-pwm, only
> a specific chip name. (in our case the NPCM750 is the full modules SOC)
> still you think i should change the driver name? (note: all of our NPCM7xx
> unique modules drivers are named npcm7xx-*.c or *-npcm7xx.c)
>
drivers/watchdog/npcm_wdt.c contradicts that statement.
Please discuss with the pwm maintainer. In hwmon, wildcards in driver file
names are discouraged and will not be accepted. Our recommendation is to pick
one chip from the family and use it as part of the file name.
>
> > As a generic pwm driver, not specifically tied to a fan controller,
> > this driver does not belong into hwmon. It should be added to the pwm
> > subsystem. Copying the maintainer and mailing list.
> >
> > In the NPCM7xx we have PWM and FAN controller modules, usually in the
> aspect of our BMC clients the two module
> are used together to control the fans.
>
> But because in the NPCM7xx the PWM and the FAN controller are separate
> modules we
> thought to do two separate drivers in the hwmon
> is it possible? or you think it is better to do one hwmon driver for the
> PWM and the FAN controller.
>
That depends on how closely the two modules are intertwined. If the pwm module
is generic, it belongs into the pwm subsystem. It only belongs into hwmon
if it can _only_ be used for fan control, eg if the data reported by the fan
module is used by the hardware to control the pwm output based on some
fan speed <-> pw value mapping which is programmed into the chip, and/or
if the pwm output can only connect to a fan and nothing else, and if the
relationship between pwm output and fan input is well defined.
If both end up in hwmon, I don't see the benefit of having two separate
drivers. That means two hwmon devices for the same fan or set of fans,
one being all but useless in respect to output from the 'sensors'
command. You'll have to convince me why that would make sense; I just
don't see the point. In that case, I would also not see the point of
having two separate devicetree nodes; to me, that would be similar
to having two interfaces for a single serial line, one for receive
and one for transmit.
> note that we were going to submit soon also the FAN controller driver under
> hwmon.
>
No problem with that as long as there are no wildcards in the file name.
Guenter
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
2018-05-29 10:02 ` [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver Tomer Maimon
2018-05-29 16:56 ` Guenter Roeck
@ 2018-05-31 7:16 ` kbuild test robot
2018-05-31 13:14 ` Guenter Roeck
2018-06-10 11:37 ` kbuild test robot
2 siblings, 1 reply; 9+ messages in thread
From: kbuild test robot @ 2018-05-31 7:16 UTC (permalink / raw)
To: Tomer Maimon
Cc: kbuild-all, robh+dt, mark.rutland, jdelvare, linux, avifishman70,
yuenn, brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc, Tomer Maimon
Hi Tomer,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.17-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tomer-Maimon/dt-binding-hwmon-Add-NPCM7xx-PWM-documentation/20180531-034040
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: x86_64-allmodconfig
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
make ARCH=x86_64 allmodconfig
make ARCH=x86_64
All warnings (new ones prefixed by >>):
vim +277 drivers/hwmon/npcm7xx-pwm.c
250
251 static int npcm7xx_pwm_probe(struct platform_device *pdev)
252 {
253 struct device *dev = &pdev->dev;
254 struct npcm7xx_pwm_data *data;
255 struct resource res[NPCM7XX_PWM_MAX_MODULES];
256 struct device *hwmon;
257 struct clk *clk;
258 int m, ch, res_cnt, ret;
259 u32 Prescale_val, output_freq;
260
261 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
262 if (!data)
263 return -ENOMEM;
264
265 for (res_cnt = 0; res_cnt < NPCM7XX_PWM_MAX_MODULES ; res_cnt++) {
266 ret = of_address_to_resource(dev->of_node, res_cnt,
267 &res[res_cnt]);
268 if (ret) {
269 pr_err("PWM of_address_to_resource fail ret %d\n",
270 ret);
271 return -EINVAL;
272 }
273
274 data->pwm_base[res_cnt] =
275 devm_ioremap_resource(dev, &(res[res_cnt]));
> 276 pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> 277 res_cnt, (u32)data->pwm_base[res_cnt],
278 res[res_cnt].start, resource_size(&(res[res_cnt])));
279
280 if (!data->pwm_base[res_cnt]) {
281 pr_err("pwm probe failed: can't read pwm base address for resource %d.\n",
282 res_cnt);
283 return -ENOMEM;
284 }
285
286 mutex_init(&data->npcm7xx_pwm_lock[res_cnt]);
287 }
288
289 clk = devm_clk_get(dev, NULL);
290 if (IS_ERR(clk))
291 return -ENODEV;
292
293 data->clk_freq = clk_get_rate(clk);
294
295 /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
296 output_freq = data->clk_freq / PWN_CNT_DEFAULT;
297 Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
298
299 /* If Prescale_val = 0, then the prescale output clock is stopped */
300 if (Prescale_val < MIN_PRESCALE1)
301 Prescale_val = MIN_PRESCALE1;
302 /*
303 * Prescale_val need to decrement in one because in the PWM Prescale
304 * register the Prescale value increment by one
305 */
306 Prescale_val--;
307
308 /* Setting PWM Prescale Register value register to both modules */
309 Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
310
311 for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
312 iowrite32(Prescale_val,
313 data->pwm_base[m] + NPCM7XX_PWM_REG_PR);
314 iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
315 data->pwm_base[m] + NPCM7XX_PWM_REG_CSR);
316 iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
317 data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
318
319 for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM; ch++) {
320 iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
321 data->pwm_base[m] + NPCM7XX_PWM_REG_CNRx(ch));
322 iowrite32(NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM,
323 data->pwm_base[m] + NPCM7XX_PWM_REG_CMRx(ch));
324 }
325
326 iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT |
327 NPCM7XX_PWM_CTRL_EN_DEFALUT,
328 data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
329 }
330
331 hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm", data,
332 &npcm7xx_chip_info, NULL);
333
334 if (IS_ERR(hwmon)) {
335 pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
336 return PTR_ERR(hwmon);
337 }
338
339 pr_info("NPCM7XX PWM Driver probed, PWM output Freq %dHz\n",
340 output_freq / ((Prescale_val & 0xf) + 1));
341
342 return 0;
343 }
344
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
2018-05-31 7:16 ` kbuild test robot
@ 2018-05-31 13:14 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2018-05-31 13:14 UTC (permalink / raw)
To: kbuild test robot, Tomer Maimon
Cc: kbuild-all, robh+dt, mark.rutland, jdelvare, avifishman70, yuenn,
brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc
On 05/31/2018 12:16 AM, kbuild test robot wrote:
> Hi Tomer,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on hwmon/hwmon-next]
> [also build test WARNING on v4.17-rc7]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Tomer-Maimon/dt-binding-hwmon-Add-NPCM7xx-PWM-documentation/20180531-034040
> base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
> config: x86_64-allmodconfig
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> make ARCH=x86_64 allmodconfig
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
>
> vim +277 drivers/hwmon/npcm7xx-pwm.c
>
> 250
> 251 static int npcm7xx_pwm_probe(struct platform_device *pdev)
> 252 {
> 253 struct device *dev = &pdev->dev;
> 254 struct npcm7xx_pwm_data *data;
> 255 struct resource res[NPCM7XX_PWM_MAX_MODULES];
> 256 struct device *hwmon;
> 257 struct clk *clk;
> 258 int m, ch, res_cnt, ret;
> 259 u32 Prescale_val, output_freq;
> 260
> 261 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> 262 if (!data)
> 263 return -ENOMEM;
> 264
> 265 for (res_cnt = 0; res_cnt < NPCM7XX_PWM_MAX_MODULES ; res_cnt++) {
> 266 ret = of_address_to_resource(dev->of_node, res_cnt,
> 267 &res[res_cnt]);
> 268 if (ret) {
> 269 pr_err("PWM of_address_to_resource fail ret %d\n",
> 270 ret);
> 271 return -EINVAL;
> 272 }
> 273
> 274 data->pwm_base[res_cnt] =
> 275 devm_ioremap_resource(dev, &(res[res_cnt]));
> > 276 pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> > 277 res_cnt, (u32)data->pwm_base[res_cnt],
> 278 res[res_cnt].start, resource_size(&(res[res_cnt])));
> 279
No idea what the tool is complaining about, but the messages should use dev_ functions.
> 280 if (!data->pwm_base[res_cnt]) {
> 281 pr_err("pwm probe failed: can't read pwm base address for resource %d.\n",
> 282 res_cnt);
> 283 return -ENOMEM;
> 284 }
> 285
> 286 mutex_init(&data->npcm7xx_pwm_lock[res_cnt]);
> 287 }
> 288
> 289 clk = devm_clk_get(dev, NULL);
> 290 if (IS_ERR(clk))
> 291 return -ENODEV;
> 292
> 293 data->clk_freq = clk_get_rate(clk);
> 294
> 295 /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
> 296 output_freq = data->clk_freq / PWN_CNT_DEFAULT;
> 297 Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
> 298
> 299 /* If Prescale_val = 0, then the prescale output clock is stopped */
> 300 if (Prescale_val < MIN_PRESCALE1)
> 301 Prescale_val = MIN_PRESCALE1;
> 302 /*
> 303 * Prescale_val need to decrement in one because in the PWM Prescale
> 304 * register the Prescale value increment by one
> 305 */
> 306 Prescale_val--;
> 307
> 308 /* Setting PWM Prescale Register value register to both modules */
> 309 Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
> 310
> 311 for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
> 312 iowrite32(Prescale_val,
> 313 data->pwm_base[m] + NPCM7XX_PWM_REG_PR);
> 314 iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
> 315 data->pwm_base[m] + NPCM7XX_PWM_REG_CSR);
> 316 iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
> 317 data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
> 318
> 319 for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM; ch++) {
> 320 iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
> 321 data->pwm_base[m] + NPCM7XX_PWM_REG_CNRx(ch));
> 322 iowrite32(NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM,
> 323 data->pwm_base[m] + NPCM7XX_PWM_REG_CMRx(ch));
> 324 }
> 325
> 326 iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT |
> 327 NPCM7XX_PWM_CTRL_EN_DEFALUT,
> 328 data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
> 329 }
> 330
> 331 hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm", data,
> 332 &npcm7xx_chip_info, NULL);
> 333
> 334 if (IS_ERR(hwmon)) {
> 335 pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
> 336 return PTR_ERR(hwmon);
> 337 }
> 338
> 339 pr_info("NPCM7XX PWM Driver probed, PWM output Freq %dHz\n",
> 340 output_freq / ((Prescale_val & 0xf) + 1));
> 341
> 342 return 0;
> 343 }
> 344
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
2018-05-29 10:02 ` [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver Tomer Maimon
2018-05-29 16:56 ` Guenter Roeck
2018-05-31 7:16 ` kbuild test robot
@ 2018-06-10 11:37 ` kbuild test robot
2 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-06-10 11:37 UTC (permalink / raw)
To: Tomer Maimon
Cc: kbuild-all, robh+dt, mark.rutland, jdelvare, linux, avifishman70,
yuenn, brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc, Tomer Maimon
[-- Attachment #1: Type: text/plain, Size: 6671 bytes --]
Hi Tomer,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tomer-Maimon/dt-binding-hwmon-Add-NPCM7xx-PWM-documentation/20180531-034040
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-x079-06101602 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers//hwmon/npcm7xx-pwm.c:4:
drivers//hwmon/npcm7xx-pwm.c: In function 'npcm7xx_pwm_probe':
>> include/linux/kern_levels.h:5:18: warning: format '%X' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t {aka long long unsigned int}' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
#define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
^~~~~~~~
include/linux/printk.h:339:9: note: in expansion of macro 'KERN_DEBUG'
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~
drivers//hwmon/npcm7xx-pwm.c:276:3: note: in expansion of macro 'pr_debug'
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
drivers//hwmon/npcm7xx-pwm.c:276:50: note: format string is defined here
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
~~~^
%08llX
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers//hwmon/npcm7xx-pwm.c:4:
include/linux/kern_levels.h:5:18: warning: format '%X' expects argument of type 'unsigned int', but argument 5 has type 'resource_size_t {aka long long unsigned int}' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
#define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
^~~~~~~~
include/linux/printk.h:339:9: note: in expansion of macro 'KERN_DEBUG'
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~
drivers//hwmon/npcm7xx-pwm.c:276:3: note: in expansion of macro 'pr_debug'
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
drivers//hwmon/npcm7xx-pwm.c:276:64: note: format string is defined here
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
~~~^
%08llX
--
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/hwmon/npcm7xx-pwm.c:4:
drivers/hwmon/npcm7xx-pwm.c: In function 'npcm7xx_pwm_probe':
>> include/linux/kern_levels.h:5:18: warning: format '%X' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t {aka long long unsigned int}' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
#define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
^~~~~~~~
include/linux/printk.h:339:9: note: in expansion of macro 'KERN_DEBUG'
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~
drivers/hwmon/npcm7xx-pwm.c:276:3: note: in expansion of macro 'pr_debug'
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
drivers/hwmon/npcm7xx-pwm.c:276:50: note: format string is defined here
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
~~~^
%08llX
In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/hwmon/npcm7xx-pwm.c:4:
include/linux/kern_levels.h:5:18: warning: format '%X' expects argument of type 'unsigned int', but argument 5 has type 'resource_size_t {aka long long unsigned int}' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
#define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
^~~~~~~~
include/linux/printk.h:339:9: note: in expansion of macro 'KERN_DEBUG'
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~
drivers/hwmon/npcm7xx-pwm.c:276:3: note: in expansion of macro 'pr_debug'
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
drivers/hwmon/npcm7xx-pwm.c:276:64: note: format string is defined here
pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
~~~^
%08llX
vim +5 include/linux/kern_levels.h
314ba352 Joe Perches 2012-07-30 4
04d2c8c8 Joe Perches 2012-07-30 @5 #define KERN_SOH "\001" /* ASCII Start Of Header */
04d2c8c8 Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
04d2c8c8 Joe Perches 2012-07-30 7
:::::: The code at line 5 was first introduced by commit
:::::: 04d2c8c83d0e3ac5f78aeede51babb3236200112 printk: convert the format for KERN_<LEVEL> to a 2 byte pattern
:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25412 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-10 11:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-29 10:02 [PATCH v1 0/2] hwmon: Add NPCM7xx PWM driver support Tomer Maimon
2018-05-29 10:02 ` [PATCH v1 1/2] dt-binding: hwmon: Add NPCM7xx PWM documentation Tomer Maimon
2018-05-29 16:57 ` Guenter Roeck
2018-05-29 10:02 ` [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver Tomer Maimon
2018-05-29 16:56 ` Guenter Roeck
[not found] ` <CAP6Zq1h8_n8R=dqmBxC_bK_YstLt1kkE6UCp2OVsNpKp=RK8og@mail.gmail.com>
2018-05-30 16:46 ` Guenter Roeck
2018-05-31 7:16 ` kbuild test robot
2018-05-31 13:14 ` Guenter Roeck
2018-06-10 11:37 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).