Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1226668 > unrolled thread
| Started by | Barry Song <21cnbao@gmail.com> |
|---|---|
| First post | 2015-09-17 07:20 +0200 |
| Last post | 2015-09-17 16:30 +0200 |
| Articles | 7 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v2 1/3] regmap: irq: add support for chips who have separate unmask registers Barry Song <21cnbao@gmail.com> - 2015-09-17 07:20 +0200
[PATCH v2 3/3] mfd: add CSR SiRFSoC on-chip power management module driver Barry Song <21cnbao@gmail.com> - 2015-09-17 07:20 +0200
Re: [PATCH v2 3/3] mfd: add CSR SiRFSoC on-chip power management module driver Lee Jones <lee.jones@linaro.org> - 2015-09-20 06:20 +0200
Re: [PATCH v2 3/3] mfd: add CSR SiRFSoC on-chip power management module driver Barry Song <21cnbao@gmail.com> - 2015-09-21 04:40 +0200
[PATCH v2 2/3] regmap: irq: add ack_invert flag for chips using cleared bits as ack Barry Song <21cnbao@gmail.com> - 2015-09-17 07:20 +0200
Re: [PATCH v2 1/3] regmap: irq: add support for chips who have separate unmask registers Mark Brown <broonie@kernel.org> - 2015-09-17 13:00 +0200
Re: [PATCH v2 1/3] regmap: irq: add support for chips who have separate unmask registers Barry Song <21cnbao@gmail.com> - 2015-09-17 16:30 +0200
| From | Barry Song <21cnbao@gmail.com> |
|---|---|
| Date | 2015-09-17 07:20 +0200 |
| Subject | [PATCH v2 1/3] regmap: irq: add support for chips who have separate unmask registers |
| Message-ID | <q9zLY-1Zr-9@gated-at.bofh.it> |
From: Guo Zeng <Guo.Zeng@csr.com>
Some chips have separate unmask registers from mask registers for
some consideration of concurrency SMP write performance. And this
patch adds a flag for it.
An user will be CSR SiRFSoC ARM chips.
Signed-off-by: Guo Zeng <Guo.Zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
drivers/base/regmap/regmap-irq.c | 28 ++++++++++++++++++++++++++--
include/linux/regmap.h | 3 +++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 38d1f72..1821c3b 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -63,6 +63,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
struct regmap *map = d->map;
int i, ret;
u32 reg;
+ u32 unmask_offset;
if (d->chip->runtime_pm) {
ret = pm_runtime_get_sync(map->dev);
@@ -82,7 +83,22 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
if (d->chip->mask_invert)
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i], ~d->mask_buf[i]);
- else
+ else if (d->chip->unmask_base) {
+ /* set mask with mask_base register */
+ ret = regmap_update_bits(d->map, reg,
+ d->mask_buf_def[i], ~d->mask_buf[i]);
+ if (ret < 0)
+ dev_err(d->map->dev,
+ "Failed to sync unmasks in %x\n",
+ reg);
+ unmask_offset = d->chip->unmask_base -
+ d->chip->mask_base;
+ /* clear mask with unmask_base register */
+ ret = regmap_update_bits(d->map,
+ reg + unmask_offset,
+ d->mask_buf_def[i],
+ d->mask_buf[i]);
+ } else
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i], d->mask_buf[i]);
if (ret != 0)
@@ -339,6 +355,7 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags,
int i;
int ret = -ENOMEM;
u32 reg;
+ u32 unmask_offset;
if (chip->num_regs <= 0)
return -EINVAL;
@@ -420,7 +437,14 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags,
if (chip->mask_invert)
ret = regmap_update_bits(map, reg,
d->mask_buf[i], ~d->mask_buf[i]);
- else
+ else if (d->chip->unmask_base) {
+ unmask_offset = d->chip->unmask_base -
+ d->chip->mask_base;
+ ret = regmap_update_bits(d->map,
+ reg + unmask_offset,
+ d->mask_buf[i],
+ d->mask_buf[i]);
+ } else
ret = regmap_update_bits(map, reg,
d->mask_buf[i], d->mask_buf[i]);
if (ret != 0) {
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 8fc0bfd..f98fe9f 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -800,6 +800,8 @@ struct regmap_irq {
*
* @status_base: Base status register address.
* @mask_base: Base mask register address.
+ * @unmask_base: Base unmask register address. for chips who have
+ * separate mask and unmask registers
* @ack_base: Base ack address. If zero then the chip is clear on read.
* Using zero value is possible with @use_ack bit.
* @wake_base: Base address for wake enables. If zero unsupported.
@@ -820,6 +822,7 @@ struct regmap_irq_chip {
unsigned int status_base;
unsigned int mask_base;
+ unsigned int unmask_base;
unsigned int ack_base;
unsigned int wake_base;
unsigned int irq_reg_stride;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Barry Song <21cnbao@gmail.com> |
|---|---|
| Date | 2015-09-17 07:20 +0200 |
| Subject | [PATCH v2 3/3] mfd: add CSR SiRFSoC on-chip power management module driver |
| Message-ID | <q9zLY-1Zr-17@gated-at.bofh.it> |
| In reply to | #1226668 |
From: Guo Zeng <Guo.Zeng@csr.com>
CSR SiRFSoC Power Control Module includes power on or power
off for sysctl power and gnss, it also includes onkey, RTC
domain clock controllers and interrupt controller for power
events.
Signed-off-by: Guo Zeng <Guo.Zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
.../devicetree/bindings/mfd/sirf-pwrc.txt | 37 ++++
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 2 +
drivers/mfd/sirfsoc_pwrc.c | 238 +++++++++++++++++++++
include/linux/mfd/sirfsoc_pwrc.h | 97 +++++++++
5 files changed, 386 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
create mode 100644 drivers/mfd/sirfsoc_pwrc.c
create mode 100644 include/linux/mfd/sirfsoc_pwrc.h
diff --git a/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt b/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
new file mode 100644
index 0000000..b919cdd
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
@@ -0,0 +1,37 @@
+SiRFSoC Power Controller (PWRC) module
+
+Power Controller is used to control the whole SoC power process.
+The power controller controls the process of Power-On sequence,
+Power-Off sequence, different power modes switching and power
+status configuration. the pwrc is access by io bridge, so the
+node's parent should be io bridge.
+
+Required properties:
+- compatible: "sirf,prima2-pwrc", or "sirf,atlas7-pwrc"
+- reg: Address range of pwrc register set
+- sysctl:mfd cell device of pwrc
+- rtcm-clk:mfd cell device of pwrc
+- onkey:mfd cell device of pwrc
+
+Example:
+
+pwrc@3000 {
+ compatible = "sirf,atlas7-pwrc";
+ reg = <0x3000 0x100>;
+ sysctl {
+ compatible = "sirf,sirf-sysctl";
+ };
+
+ rtcm-clk {
+ compatible = "sirf,atlas7-rtcmclk";
+ };
+
+ onkey {
+ compatible = "sirf,prima2-onkey";
+ };
+};
+
+pwrc@3000 {
+ compatible = "sirf,prima2-pwrc";
+ reg = <0x3000 0x100>;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 99d6367..5b67bee 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1515,5 +1515,17 @@ config MFD_VEXPRESS_SYSREG
System Registers are the platform configuration block
on the ARM Ltd. Versatile Express board.
+config MFD_SIRFSOC_PWRC
+ bool "CSR SiRFSoC on-chip Power Control Module"
+ depends on ARCH_SIRF
+ default y
+ select MFD_CORE
+ select REGMAP_IRQ
+ help
+ CSR SiRFSoC Power Control Module includes power on or power
+ off for sysctl power and gnss, it also includes onkey, RTC
+ domain clock controllers and interrupt controller for power
+ events.
+
endmenu
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a59e3fc..255fb80 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -192,3 +192,5 @@ obj-$(CONFIG_MFD_SKY81452) += sky81452.o
intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
+
+obj-$(CONFIG_MFD_SIRFSOC_PWRC) += sirfsoc_pwrc.o
diff --git a/drivers/mfd/sirfsoc_pwrc.c b/drivers/mfd/sirfsoc_pwrc.c
new file mode 100644
index 0000000..b43f8b4
--- /dev/null
+++ b/drivers/mfd/sirfsoc_pwrc.c
@@ -0,0 +1,238 @@
+/*
+ * power management mfd for CSR SiRFSoC chips
+ *
+ * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+#include <linux/rtc/sirfsoc_rtciobrg.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sirfsoc_pwrc.h>
+
+struct sirfsoc_pwrc_register sirfsoc_a7da_pwrc = {
+ .pwrc_pdn_ctrl_set = 0x0,
+ .pwrc_pdn_ctrl_clr = 0x4,
+ .pwrc_pon_status = 0x8,
+ .pwrc_trigger_en_set = 0xc,
+ .pwrc_trigger_en_clr = 0x10,
+ .pwrc_int_mask_set = 0x14,
+ .pwrc_int_mask_clr = 0x18,
+ .pwrc_int_status = 0x1c,
+ .pwrc_pin_status = 0x20,
+ .pwrc_rtc_pll_ctrl = 0x28,
+ .pwrc_gpio3_debug = 0x34,
+ .pwrc_rtc_noc_pwrctl_set = 0x38,
+ .pwrc_rtc_noc_pwrctl_clr = 0x3c,
+ .pwrc_rtc_can_ctrl = 0x48,
+ .pwrc_rtc_can_status = 0x4c,
+ .pwrc_fsm_m3_ctrl = 0x50,
+ .pwrc_fsm_state = 0x54,
+ .pwrc_rtcldo_reg = 0x58,
+ .pwrc_gnss_ctrl = 0x5c,
+ .pwrc_gnss_status = 0x60,
+ .pwrc_xtal_reg = 0x64,
+ .pwrc_xtal_ldo_mux_sel = 0x68,
+ .pwrc_rtc_sw_rstc_set = 0x6c,
+ .pwrc_rtc_sw_rstc_clr = 0x70,
+ .pwrc_power_sw_ctrl_set = 0x74,
+ .pwrc_power_sw_ctrl_clr = 0x78,
+ .pwrc_rtc_dcog = 0x7c,
+ .pwrc_m3_memories = 0x80,
+ .pwrc_can0_memory = 0x84,
+ .pwrc_rtc_gnss_memory = 0x88,
+ .pwrc_m3_clk_en = 0x8c,
+ .pwrc_can0_clk_en = 0x90,
+ .pwrc_spi0_clk_en = 0x94,
+ .pwrc_rtc_sec_clk_en = 0x98,
+ .pwrc_rtc_noc_clk_en = 0x9c,
+};
+
+struct sirfsoc_pwrc_register sirfsoc_prima2_pwrc = {
+ .pwrc_pdn_ctrl_set = 0x0,
+ .pwrc_pon_status = 0x4,
+ .pwrc_trigger_en_set = 0x8,
+ .pwrc_int_status = 0xc,
+ .pwrc_int_mask_set = 0x10,
+ .pwrc_pin_status = 0x14,
+ .pwrc_scratch_pad1 = 0x18,
+ .pwrc_scratch_pad2 = 0x1c,
+ .pwrc_scratch_pad3 = 0x20,
+ .pwrc_scratch_pad4 = 0x24,
+ .pwrc_scratch_pad5 = 0x28,
+ .pwrc_scratch_pad6 = 0x2c,
+ .pwrc_scratch_pad7 = 0x30,
+ .pwrc_scratch_pad8 = 0x34,
+ .pwrc_scratch_pad9 = 0x38,
+ .pwrc_scratch_pad10 = 0x3c,
+ .pwrc_scratch_pad11 = 0x40,
+ .pwrc_scratch_pad12 = 0x44,
+ .pwrc_gpio3_clk = 0x54,
+ .pwrc_gpio_ds = 0x78,
+};
+
+static const struct regmap_irq pwrc_irqs[] = {
+ /* INT0 */
+ [PWRC_IRQ_ONKEY] = {
+ .mask = BIT(PWRC_IRQ_ONKEY),
+ },
+ [PWRC_IRQ_EXT_ONKEY] = {
+ .mask = BIT(PWRC_IRQ_EXT_ONKEY),
+ },
+};
+
+static struct regmap_irq_chip pwrc_irq_chip = {
+ .name = "pwrc_irq",
+ .irqs = pwrc_irqs,
+ .num_irqs = ARRAY_SIZE(pwrc_irqs),
+ .num_regs = 1,
+ .ack_invert = 1,
+ .init_ack_masked = true,
+};
+
+static const struct of_device_id pwrc_ids[] = {
+ { .compatible = "sirf,prima2-pwrc", .data = &sirfsoc_prima2_pwrc},
+ { .compatible = "sirf,atlas7-pwrc", .data = &sirfsoc_a7da_pwrc},
+ {}
+};
+
+static const struct mfd_cell pwrc_devs[] = {
+ {
+ .name = "rtcmclk",
+ .of_compatible = "sirf,atlas7-rtcmclk",
+ }, {
+ .name = "sirf-sysctl",
+ .of_compatible = "sirf,sirf-sysctl",
+ }, {
+ .name = "gps-power",
+ .of_compatible = "sirf,gps-power",
+ }, {
+ .name = "onkey",
+ .of_compatible = "sirf,prima2-onkey",
+ },
+};
+
+static const struct regmap_config pwrc_regmap_config = {
+ .fast_io = true,
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
+static int sirfsoc_pwrc_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *match;
+ struct sirfsoc_pwrc_info *pwrcinfo;
+ struct regmap_irq_chip *regmap_irq_chip;
+ struct sirfsoc_pwrc_register *pwrc_reg;
+ struct regmap *map;
+ int ret;
+ u32 base;
+
+ if (of_property_read_u32(np, "reg", &base))
+ panic("unable to find base address of pwrc node in dtb\n");
+
+ pwrcinfo = devm_kzalloc(&pdev->dev,
+ sizeof(struct sirfsoc_pwrc_info), GFP_KERNEL);
+ if (!pwrcinfo)
+ return -ENOMEM;
+ pwrcinfo->base = base;
+
+ /*
+ * pwrc behind rtciobrg offset is diff between prima2 and atlas7
+ * here match to each ids data for it.
+ */
+ match = of_match_node(pwrc_ids, np);
+ pwrcinfo->pwrc_reg = (struct sirfsoc_pwrc_register *)match->data;
+
+ if (of_device_is_compatible(np, "sirf,atlas7-pwrc"))
+ pwrcinfo->ver = PWRC_ATLAS7_VER;
+ else if (of_device_is_compatible(np, "sirf,prima2-pwrc"))
+ pwrcinfo->ver = PWRC_PRIMA2_VER;
+ else
+ return -EINVAL;
+
+ of_node_put(np);
+
+ map = (struct regmap *)devm_regmap_init_iobg(&pdev->dev,
+ &pwrc_regmap_config);
+ if (IS_ERR(map)) {
+ ret = PTR_ERR(map);
+ dev_err(&pdev->dev, "Failed to allocate register map: %d\n",
+ ret);
+ goto err;
+ }
+
+ pwrcinfo->regmap = map;
+ pwrcinfo->dev = &pdev->dev;
+ platform_set_drvdata(pdev, pwrcinfo);
+
+ ret = mfd_add_devices(pwrcinfo->dev, 0, pwrc_devs,
+ ARRAY_SIZE(pwrc_devs), NULL, 0, NULL);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to add all pwrc subdev\n");
+ goto err;
+ }
+
+ ret = of_irq_get(pdev->dev.of_node, 0);
+ if (ret <= 0) {
+ dev_info(&pdev->dev,
+ "Unable to find IRQ for pwrc. ret=%d\n", ret);
+ goto err_irq;
+ }
+
+ pwrcinfo->irq = ret;
+ regmap_irq_chip = &pwrc_irq_chip;
+ pwrcinfo->regmap_irq_chip = regmap_irq_chip;
+
+ pwrc_reg = pwrcinfo->pwrc_reg;
+ regmap_irq_chip->mask_base = pwrcinfo->base +
+ pwrc_reg->pwrc_int_mask_set;
+ regmap_irq_chip->unmask_base = pwrcinfo->base +
+ pwrc_reg->pwrc_int_mask_clr;
+ regmap_irq_chip->status_base = pwrcinfo->base +
+ pwrc_reg->pwrc_int_status;
+ regmap_irq_chip->ack_base = pwrcinfo->base +
+ pwrc_reg->pwrc_int_status;
+
+ /* enable onkey trigger interrupt controller */
+ ret = regmap_update_bits(map,
+ pwrcinfo->base +
+ pwrc_reg->pwrc_trigger_en_set,
+ BIT(PWRC_IRQ_ONKEY) | BIT(PWRC_IRQ_EXT_ONKEY),
+ BIT(PWRC_IRQ_ONKEY) | BIT(PWRC_IRQ_EXT_ONKEY));
+ if (ret < 0)
+ goto err_irq;
+
+ /* add irq controller for pwrc */
+ ret = regmap_add_irq_chip(map, pwrcinfo->irq, IRQF_ONESHOT,
+ -1, pwrcinfo->regmap_irq_chip,
+ &pwrcinfo->irq_data);
+
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to add regmap irq controller for pwrc\n");
+ goto err;
+ }
+
+ return 0;
+err_irq:
+ mfd_remove_devices(pwrcinfo->dev);
+err:
+ return ret;
+}
+
+static struct platform_driver sirfsoc_pwrc_driver = {
+ .probe = sirfsoc_pwrc_probe,
+ .driver = {
+ .name = "sirfsoc_pwrc",
+ .of_match_table = pwrc_ids,
+ },
+};
+module_platform_driver(sirfsoc_pwrc_driver);
diff --git a/include/linux/mfd/sirfsoc_pwrc.h b/include/linux/mfd/sirfsoc_pwrc.h
new file mode 100644
index 0000000..ee2b3d5
--- /dev/null
+++ b/include/linux/mfd/sirfsoc_pwrc.h
@@ -0,0 +1,97 @@
+/*
+ * CSR SiRFSoC power control module MFD interface
+ *
+ * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+#ifndef _SIRFSOC_PWRC_H_
+#define _SIRFSOC_PWRC_H_
+#include <linux/interrupt.h>
+#include <linux/regmap.h>
+
+#define PWRC_PDN_CTRL_OFFSET 0
+#define AUDIO_POWER_EN_BIT 14
+
+struct sirfsoc_pwrc_register {
+ /* hardware pwrc specific */
+ u32 pwrc_pdn_ctrl_set;
+ u32 pwrc_pdn_ctrl_clr;
+ u32 pwrc_pon_status;
+ u32 pwrc_trigger_en_set;
+ u32 pwrc_trigger_en_clr;
+ u32 pwrc_int_mask_set;
+ u32 pwrc_int_mask_clr;
+ u32 pwrc_int_status;
+ u32 pwrc_pin_status;
+ u32 pwrc_rtc_pll_ctrl;
+ u32 pwrc_gpio3_debug;
+ u32 pwrc_rtc_noc_pwrctl_set;
+ u32 pwrc_rtc_noc_pwrctl_clr;
+ u32 pwrc_rtc_can_ctrl;
+ u32 pwrc_rtc_can_status;
+ u32 pwrc_fsm_m3_ctrl;
+ u32 pwrc_fsm_state;
+ u32 pwrc_rtcldo_reg;
+ u32 pwrc_gnss_ctrl;
+ u32 pwrc_gnss_status;
+ u32 pwrc_xtal_reg;
+ u32 pwrc_xtal_ldo_mux_sel;
+ u32 pwrc_rtc_sw_rstc_set;
+ u32 pwrc_rtc_sw_rstc_clr;
+ u32 pwrc_power_sw_ctrl_set;
+ u32 pwrc_power_sw_ctrl_clr;
+ u32 pwrc_rtc_dcog;
+ u32 pwrc_m3_memories;
+ u32 pwrc_can0_memory;
+ u32 pwrc_rtc_gnss_memory;
+ u32 pwrc_m3_clk_en;
+ u32 pwrc_can0_clk_en;
+ u32 pwrc_spi0_clk_en;
+ u32 pwrc_rtc_sec_clk_en;
+ u32 pwrc_rtc_noc_clk_en;
+
+ /*only for prima2*/
+ u32 pwrc_scratch_pad1;
+ u32 pwrc_scratch_pad2;
+ u32 pwrc_scratch_pad3;
+ u32 pwrc_scratch_pad4;
+ u32 pwrc_scratch_pad5;
+ u32 pwrc_scratch_pad6;
+ u32 pwrc_scratch_pad7;
+ u32 pwrc_scratch_pad8;
+ u32 pwrc_scratch_pad9;
+ u32 pwrc_scratch_pad10;
+ u32 pwrc_scratch_pad11;
+ u32 pwrc_scratch_pad12;
+ u32 pwrc_gpio3_clk;
+ u32 pwrc_gpio_ds;
+
+};
+
+enum pwrc_version {
+ PWRC_MARCO_VER,
+ PWRC_PRIMA2_VER,
+ PWRC_ATLAS7_VER,
+};
+
+struct sirfsoc_pwrc_info {
+ struct device *dev;
+ struct regmap *regmap;
+ struct sirfsoc_pwrc_register *pwrc_reg;
+ struct regmap_irq_chip *regmap_irq_chip;
+ struct regmap_irq_chip_data *irq_data;
+ u32 ver;
+ u32 base;
+ int irq;
+};
+
+enum {
+ PWRC_IRQ_ONKEY = 0,
+ PWRC_IRQ_EXT_ONKEY,
+ PWRC_MAX_IRQ,
+};
+
+extern struct sirfsoc_pwrc_register sirfsoc_a7da_pwrc;
+extern struct sirfsoc_pwrc_register sirfsoc_prima2_pwrc;
+#endif
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2015-09-20 06:20 +0200 |
| Subject | Re: [PATCH v2 3/3] mfd: add CSR SiRFSoC on-chip power management module driver |
| Message-ID | <qaEgy-5MM-29@gated-at.bofh.it> |
| In reply to | #1226671 |
On Thu, 17 Sep 2015, Barry Song wrote:
> From: Guo Zeng <Guo.Zeng@csr.com>
>
> CSR SiRFSoC Power Control Module includes power on or power
> off for sysctl power and gnss, it also includes onkey, RTC
> domain clock controllers and interrupt controller for power
> events.
There are lots of Three (and four) Letter Abbreviations (TLAs) here,
which mean nothing to the average reader. Please break them out in
the commit log as I have done i.e. "Long Abbreviated Word (LAW)", so
us normies can see what they mean.
> Signed-off-by: Guo Zeng <Guo.Zeng@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
> .../devicetree/bindings/mfd/sirf-pwrc.txt | 37 ++++
This should be in a separate patch.
> drivers/mfd/Kconfig | 12 ++
> drivers/mfd/Makefile | 2 +
> drivers/mfd/sirfsoc_pwrc.c | 238 +++++++++++++++++++++
> include/linux/mfd/sirfsoc_pwrc.h | 97 +++++++++
> 5 files changed, 386 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
> create mode 100644 drivers/mfd/sirfsoc_pwrc.c
> create mode 100644 include/linux/mfd/sirfsoc_pwrc.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt b/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
> new file mode 100644
> index 0000000..b919cdd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
> @@ -0,0 +1,37 @@
> +SiRFSoC Power Controller (PWRC) module
> +
> +Power Controller is used to control the whole SoC power process.
> +The power controller controls the process of Power-On sequence,
s/power controller/Power Controller/
> +Power-Off sequence, different power modes switching and power
> +status configuration. the pwrc is access by io bridge, so the
s/the pwrc/The PWRC/
s/access/accessed/
s/io/IO/
> +node's parent should be io bridge.
s/io/IO/
Not quite sure what an IO bridge is though.
> +Required properties:
> +- compatible: "sirf,prima2-pwrc", or "sirf,atlas7-pwrc"
> +- reg: Address range of pwrc register set
Address start and size.
> +- sysctl:mfd cell device of pwrc
> +- rtcm-clk:mfd cell device of pwrc
> +- onkey:mfd cell device of pwrc
MFD is a Linuxisum. Please find another way to document them.
I always find documentation easier to read then it's tabbed out
nicely. Like:
Required properties:
- compatible : "sirf,prima2-pwrc", or "sirf,atlas7-pwrc"
- reg : Address range of pwrc register set
- sysctl : mfd cell device of pwrc
- rtcm-clk : mfd cell device of pwrc
- onkey : mfd cell device of pwrc
> +Example:
> +
> +pwrc@3000 {
> + compatible = "sirf,atlas7-pwrc";
> + reg = <0x3000 0x100>;
This doesn't look like a real address. It looks like an offset.
Please provide a full example, complete with the parent node (which I
assume has ranges set-up).
> + sysctl {
> + compatible = "sirf,sirf-sysctl";
> + };
> +
> + rtcm-clk {
> + compatible = "sirf,atlas7-rtcmclk";
> + };
> +
> + onkey {
> + compatible = "sirf,prima2-onkey";
> + };
Why do these require their own cells?
Do they have their own properties? If so, where are they documented?
> +};
> +
> +pwrc@3000 {
> + compatible = "sirf,prima2-pwrc";
> + reg = <0x3000 0x100>;
> +};
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 99d6367..5b67bee 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1515,5 +1515,17 @@ config MFD_VEXPRESS_SYSREG
> System Registers are the platform configuration block
> on the ARM Ltd. Versatile Express board.
>
> +config MFD_SIRFSOC_PWRC
> + bool "CSR SiRFSoC on-chip Power Control Module"
> + depends on ARCH_SIRF
> + default y
> + select MFD_CORE
> + select REGMAP_IRQ
> + help
> + CSR SiRFSoC Power Control Module includes power on or power
> + off for sysctl power and gnss, it also includes onkey, RTC
> + domain clock controllers and interrupt controller for power
> + events.
Break out the TLAs.
Your tabbing is all over the place here. Please match existing
entries.
> endmenu
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a59e3fc..255fb80 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -192,3 +192,5 @@ obj-$(CONFIG_MFD_SKY81452) += sky81452.o
> intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
> obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
> +
> +obj-$(CONFIG_MFD_SIRFSOC_PWRC) += sirfsoc_pwrc.o
> diff --git a/drivers/mfd/sirfsoc_pwrc.c b/drivers/mfd/sirfsoc_pwrc.c
> new file mode 100644
> index 0000000..b43f8b4
> --- /dev/null
> +++ b/drivers/mfd/sirfsoc_pwrc.c
> @@ -0,0 +1,238 @@
> +/*
> + * power management mfd for CSR SiRFSoC chips
"Power Management MFD for CSR SiRFSoC chips"
> + * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
This is out of date.
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/io.h>
> +#include <linux/regmap.h>
> +#include <linux/module.h>
This isn't a module.
> +#include <linux/rtc/sirfsoc_rtciobrg.h>
What's this for?
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/sirfsoc_pwrc.h>
Header files should be in alphabetical order.
> +struct sirfsoc_pwrc_register sirfsoc_a7da_pwrc = {
> + .pwrc_pdn_ctrl_set = 0x0,
> + .pwrc_pdn_ctrl_clr = 0x4,
> + .pwrc_pon_status = 0x8,
> + .pwrc_trigger_en_set = 0xc,
> + .pwrc_trigger_en_clr = 0x10,
> + .pwrc_int_mask_set = 0x14,
> + .pwrc_int_mask_clr = 0x18,
> + .pwrc_int_status = 0x1c,
> + .pwrc_pin_status = 0x20,
> + .pwrc_rtc_pll_ctrl = 0x28,
> + .pwrc_gpio3_debug = 0x34,
> + .pwrc_rtc_noc_pwrctl_set = 0x38,
> + .pwrc_rtc_noc_pwrctl_clr = 0x3c,
> + .pwrc_rtc_can_ctrl = 0x48,
> + .pwrc_rtc_can_status = 0x4c,
> + .pwrc_fsm_m3_ctrl = 0x50,
> + .pwrc_fsm_state = 0x54,
> + .pwrc_rtcldo_reg = 0x58,
> + .pwrc_gnss_ctrl = 0x5c,
> + .pwrc_gnss_status = 0x60,
> + .pwrc_xtal_reg = 0x64,
> + .pwrc_xtal_ldo_mux_sel = 0x68,
> + .pwrc_rtc_sw_rstc_set = 0x6c,
> + .pwrc_rtc_sw_rstc_clr = 0x70,
> + .pwrc_power_sw_ctrl_set = 0x74,
> + .pwrc_power_sw_ctrl_clr = 0x78,
> + .pwrc_rtc_dcog = 0x7c,
> + .pwrc_m3_memories = 0x80,
> + .pwrc_can0_memory = 0x84,
> + .pwrc_rtc_gnss_memory = 0x88,
> + .pwrc_m3_clk_en = 0x8c,
> + .pwrc_can0_clk_en = 0x90,
> + .pwrc_spi0_clk_en = 0x94,
> + .pwrc_rtc_sec_clk_en = 0x98,
> + .pwrc_rtc_noc_clk_en = 0x9c,
> +};
> +
> +struct sirfsoc_pwrc_register sirfsoc_prima2_pwrc = {
> + .pwrc_pdn_ctrl_set = 0x0,
> + .pwrc_pon_status = 0x4,
> + .pwrc_trigger_en_set = 0x8,
> + .pwrc_int_status = 0xc,
> + .pwrc_int_mask_set = 0x10,
> + .pwrc_pin_status = 0x14,
> + .pwrc_scratch_pad1 = 0x18,
> + .pwrc_scratch_pad2 = 0x1c,
> + .pwrc_scratch_pad3 = 0x20,
> + .pwrc_scratch_pad4 = 0x24,
> + .pwrc_scratch_pad5 = 0x28,
> + .pwrc_scratch_pad6 = 0x2c,
> + .pwrc_scratch_pad7 = 0x30,
> + .pwrc_scratch_pad8 = 0x34,
> + .pwrc_scratch_pad9 = 0x38,
> + .pwrc_scratch_pad10 = 0x3c,
> + .pwrc_scratch_pad11 = 0x40,
> + .pwrc_scratch_pad12 = 0x44,
> + .pwrc_gpio3_clk = 0x54,
> + .pwrc_gpio_ds = 0x78,
> +};
This is not the way we usually define register addresses.
Please #define them properly.
> +static const struct regmap_irq pwrc_irqs[] = {
> + /* INT0 */
> + [PWRC_IRQ_ONKEY] = {
> + .mask = BIT(PWRC_IRQ_ONKEY),
Better to do the BIT() shifting in the header file.
> + },
> + [PWRC_IRQ_EXT_ONKEY] = {
> + .mask = BIT(PWRC_IRQ_EXT_ONKEY),
> + },
> +};
> +
> +static struct regmap_irq_chip pwrc_irq_chip = {
> + .name = "pwrc_irq",
> + .irqs = pwrc_irqs,
> + .num_irqs = ARRAY_SIZE(pwrc_irqs),
> + .num_regs = 1,
> + .ack_invert = 1,
> + .init_ack_masked = true,
> +};
> +
> +static const struct of_device_id pwrc_ids[] = {
> + { .compatible = "sirf,prima2-pwrc", .data = &sirfsoc_prima2_pwrc},
> + { .compatible = "sirf,atlas7-pwrc", .data = &sirfsoc_a7da_pwrc},
> + {}
> +};
Please put these _just_ before you use them, so just above .probe() in
this case.
> +static const struct mfd_cell pwrc_devs[] = {
> + {
> + .name = "rtcmclk",
> + .of_compatible = "sirf,atlas7-rtcmclk",
> + }, {
> + .name = "sirf-sysctl",
> + .of_compatible = "sirf,sirf-sysctl",
> + }, {
> + .name = "gps-power",
> + .of_compatible = "sirf,gps-power",
> + }, {
> + .name = "onkey",
> + .of_compatible = "sirf,prima2-onkey",
> + },
> +};
> +
> +static const struct regmap_config pwrc_regmap_config = {
> + .fast_io = true,
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> +static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match;
> + struct sirfsoc_pwrc_info *pwrcinfo;
> + struct regmap_irq_chip *regmap_irq_chip;
> + struct sirfsoc_pwrc_register *pwrc_reg;
> + struct regmap *map;
> + int ret;
> + u32 base;
> +
> + if (of_property_read_u32(np, "reg", &base))
> + panic("unable to find base address of pwrc node in dtb\n");
It looks like this driver should depend on OF.
Why are you obtaining the base address manually? Use:
res = platform_get_resource();
devm_ioremap_resource(res);
... instead.
> + pwrcinfo = devm_kzalloc(&pdev->dev,
> + sizeof(struct sirfsoc_pwrc_info), GFP_KERNEL);
sizeof(pwrcinfo)
> + if (!pwrcinfo)
> + return -ENOMEM;
> + pwrcinfo->base = base;
> +
> + /*
> + * pwrc behind rtciobrg offset is diff between prima2 and atlas7
> + * here match to each ids data for it.
> + */
Use uppercase characters for abbreviations.
Besides, I have no idea what you're trying to say.
> + match = of_match_node(pwrc_ids, np);
> + pwrcinfo->pwrc_reg = (struct sirfsoc_pwrc_register *)match->data;
It looks like you're trying to use the same variables two different
device's register sets. That's asking for trouble, please don't do
that.
> + if (of_device_is_compatible(np, "sirf,atlas7-pwrc"))
> + pwrcinfo->ver = PWRC_ATLAS7_VER;
> + else if (of_device_is_compatible(np, "sirf,prima2-pwrc"))
> + pwrcinfo->ver = PWRC_PRIMA2_VER;
> + else
> + return -EINVAL;
These aren't versions, are they? It's different hardware.
Can't you probe for this at runtime?
> + of_node_put(np);
What are you putting here?
> + map = (struct regmap *)devm_regmap_init_iobg(&pdev->dev,
> + &pwrc_regmap_config);
Why are you casting?
> + if (IS_ERR(map)) {
> + ret = PTR_ERR(map);
> + dev_err(&pdev->dev, "Failed to allocate register map: %d\n",
> + ret);
> + goto err;
Please remove the 'err' label and just return from here.
> + }
> +
> + pwrcinfo->regmap = map;
> + pwrcinfo->dev = &pdev->dev;
> + platform_set_drvdata(pdev, pwrcinfo);
> +
> + ret = mfd_add_devices(pwrcinfo->dev, 0, pwrc_devs,
> + ARRAY_SIZE(pwrc_devs), NULL, 0, NULL);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to add all pwrc subdev\n");
Capitalise all abbreviations.
Don't abbreviate things like "sub devices".
> + goto err;
Just return.
> + }
> +
> + ret = of_irq_get(pdev->dev.of_node, 0);
You already put this in to a succinct variable 'np'. Please use it.
> + if (ret <= 0) {
> + dev_info(&pdev->dev,
> + "Unable to find IRQ for pwrc. ret=%d\n", ret);
Just print ret. No need for the ugly "ret=".
> + goto err_irq;
> + }
> +
> + pwrcinfo->irq = ret;
Better change this to 'irq' instead of using 'ret'.
> + regmap_irq_chip = &pwrc_irq_chip;
> + pwrcinfo->regmap_irq_chip = regmap_irq_chip;
> +
> + pwrc_reg = pwrcinfo->pwrc_reg;
> + regmap_irq_chip->mask_base = pwrcinfo->base +
> + pwrc_reg->pwrc_int_mask_set;
> + regmap_irq_chip->unmask_base = pwrcinfo->base +
> + pwrc_reg->pwrc_int_mask_clr;
> + regmap_irq_chip->status_base = pwrcinfo->base +
> + pwrc_reg->pwrc_int_status;
> + regmap_irq_chip->ack_base = pwrcinfo->base +
> + pwrc_reg->pwrc_int_status;
This is ugly.
Better to create 2 regmap_irq_chip structures, one for each device.
> + /* enable onkey trigger interrupt controller */
Capital letters to start a sentence.
> + ret = regmap_update_bits(map,
> + pwrcinfo->base +
> + pwrc_reg->pwrc_trigger_en_set,
> + BIT(PWRC_IRQ_ONKEY) | BIT(PWRC_IRQ_EXT_ONKEY),
> + BIT(PWRC_IRQ_ONKEY) | BIT(PWRC_IRQ_EXT_ONKEY));
> + if (ret < 0)
> + goto err_irq;
> +
> + /* add irq controller for pwrc */
Capital letters to start a sentence and for abbreviations.
> + ret = regmap_add_irq_chip(map, pwrcinfo->irq, IRQF_ONESHOT,
> + -1, pwrcinfo->regmap_irq_chip,
> + &pwrcinfo->irq_data);
> +
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to add regmap irq controller for pwrc\n");
> + goto err;
> + }
> +
> + return 0;
> +err_irq:
> + mfd_remove_devices(pwrcinfo->dev);
> +err:
Remove this label, it's not helpful.
> + return ret;
> +}
> +
> +static struct platform_driver sirfsoc_pwrc_driver = {
> + .probe = sirfsoc_pwrc_probe,
.remove?
> + .driver = {
> + .name = "sirfsoc_pwrc",
> + .of_match_table = pwrc_ids,
of_match_ptr()
> + },
> +};
> +module_platform_driver(sirfsoc_pwrc_driver);
This isn't a module.
> diff --git a/include/linux/mfd/sirfsoc_pwrc.h b/include/linux/mfd/sirfsoc_pwrc.h
> new file mode 100644
> index 0000000..ee2b3d5
> --- /dev/null
> +++ b/include/linux/mfd/sirfsoc_pwrc.h
> @@ -0,0 +1,97 @@
> +/*
> + * CSR SiRFSoC power control module MFD interface
> + *
> + * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
Out of date.
> + * Licensed under GPLv2 or later.
> + */
'\n' here.
> +#ifndef _SIRFSOC_PWRC_H_
> +#define _SIRFSOC_PWRC_H_
'\n' here.
> +#include <linux/interrupt.h>
What's this for?
> +#include <linux/regmap.h>
> +
> +#define PWRC_PDN_CTRL_OFFSET 0
> +#define AUDIO_POWER_EN_BIT 14
> +
> +struct sirfsoc_pwrc_register {
> + /* hardware pwrc specific */
> + u32 pwrc_pdn_ctrl_set;
> + u32 pwrc_pdn_ctrl_clr;
> + u32 pwrc_pon_status;
> + u32 pwrc_trigger_en_set;
> + u32 pwrc_trigger_en_clr;
> + u32 pwrc_int_mask_set;
> + u32 pwrc_int_mask_clr;
> + u32 pwrc_int_status;
> + u32 pwrc_pin_status;
> + u32 pwrc_rtc_pll_ctrl;
> + u32 pwrc_gpio3_debug;
> + u32 pwrc_rtc_noc_pwrctl_set;
> + u32 pwrc_rtc_noc_pwrctl_clr;
> + u32 pwrc_rtc_can_ctrl;
> + u32 pwrc_rtc_can_status;
> + u32 pwrc_fsm_m3_ctrl;
> + u32 pwrc_fsm_state;
> + u32 pwrc_rtcldo_reg;
> + u32 pwrc_gnss_ctrl;
> + u32 pwrc_gnss_status;
> + u32 pwrc_xtal_reg;
> + u32 pwrc_xtal_ldo_mux_sel;
> + u32 pwrc_rtc_sw_rstc_set;
> + u32 pwrc_rtc_sw_rstc_clr;
> + u32 pwrc_power_sw_ctrl_set;
> + u32 pwrc_power_sw_ctrl_clr;
> + u32 pwrc_rtc_dcog;
> + u32 pwrc_m3_memories;
> + u32 pwrc_can0_memory;
> + u32 pwrc_rtc_gnss_memory;
> + u32 pwrc_m3_clk_en;
> + u32 pwrc_can0_clk_en;
> + u32 pwrc_spi0_clk_en;
> + u32 pwrc_rtc_sec_clk_en;
> + u32 pwrc_rtc_noc_clk_en;
> +
> + /*only for prima2*/
> + u32 pwrc_scratch_pad1;
> + u32 pwrc_scratch_pad2;
> + u32 pwrc_scratch_pad3;
> + u32 pwrc_scratch_pad4;
> + u32 pwrc_scratch_pad5;
> + u32 pwrc_scratch_pad6;
> + u32 pwrc_scratch_pad7;
> + u32 pwrc_scratch_pad8;
> + u32 pwrc_scratch_pad9;
> + u32 pwrc_scratch_pad10;
> + u32 pwrc_scratch_pad11;
> + u32 pwrc_scratch_pad12;
> + u32 pwrc_gpio3_clk;
> + u32 pwrc_gpio_ds;
> +
> +};
Please #define these instead.
> +enum pwrc_version {
> + PWRC_MARCO_VER,
> + PWRC_PRIMA2_VER,
> + PWRC_ATLAS7_VER,
> +};
Kerneldoc header?
> +struct sirfsoc_pwrc_info {
> + struct device *dev;
> + struct regmap *regmap;
> + struct sirfsoc_pwrc_register *pwrc_reg;
> + struct regmap_irq_chip *regmap_irq_chip;
> + struct regmap_irq_chip_data *irq_data;
Where are these reused?
> + u32 ver;
What is this used for?
> + u32 base;
Is this a u32 or a memory address?
> + int irq;
> +};
> +
> +enum {
> + PWRC_IRQ_ONKEY = 0,
> + PWRC_IRQ_EXT_ONKEY,
> + PWRC_MAX_IRQ,
> +};
> +
> +extern struct sirfsoc_pwrc_register sirfsoc_a7da_pwrc;
> +extern struct sirfsoc_pwrc_register sirfsoc_prima2_pwrc;
> +#endif
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Barry Song <21cnbao@gmail.com> |
|---|---|
| Date | 2015-09-21 04:40 +0200 |
| Subject | Re: [PATCH v2 3/3] mfd: add CSR SiRFSoC on-chip power management module driver |
| Message-ID | <qaZbk-1MF-3@gated-at.bofh.it> |
| In reply to | #1228824 |
hi Lee,
thanks very much!
2015-09-20 12:15 GMT+08:00 Lee Jones <lee.jones@linaro.org>:
> On Thu, 17 Sep 2015, Barry Song wrote:
>
>> From: Guo Zeng <Guo.Zeng@csr.com>
>>
>> CSR SiRFSoC Power Control Module includes power on or power
>> off for sysctl power and gnss, it also includes onkey, RTC
>> domain clock controllers and interrupt controller for power
>> events.
>
> There are lots of Three (and four) Letter Abbreviations (TLAs) here,
> which mean nothing to the average reader. Please break them out in
> the commit log as I have done i.e. "Long Abbreviated Word (LAW)", so
> us normies can see what they mean.
>
>> Signed-off-by: Guo Zeng <Guo.Zeng@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> .../devicetree/bindings/mfd/sirf-pwrc.txt | 37 ++++
>
> This should be in a separate patch.
>
>> drivers/mfd/Kconfig | 12 ++
>> drivers/mfd/Makefile | 2 +
>> drivers/mfd/sirfsoc_pwrc.c | 238 +++++++++++++++++++++
>> include/linux/mfd/sirfsoc_pwrc.h | 97 +++++++++
>> 5 files changed, 386 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
>> create mode 100644 drivers/mfd/sirfsoc_pwrc.c
>> create mode 100644 include/linux/mfd/sirfsoc_pwrc.h
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt b/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
>> new file mode 100644
>> index 0000000..b919cdd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/sirf-pwrc.txt
>> @@ -0,0 +1,37 @@
>> +SiRFSoC Power Controller (PWRC) module
>> +
>> +Power Controller is used to control the whole SoC power process.
>> +The power controller controls the process of Power-On sequence,
>
> s/power controller/Power Controller/
>
>> +Power-Off sequence, different power modes switching and power
>> +status configuration. the pwrc is access by io bridge, so the
>
> s/the pwrc/The PWRC/
>
> s/access/accessed/
>
> s/io/IO/
>
>> +node's parent should be io bridge.
>
> s/io/IO/
>
> Not quite sure what an IO bridge is though.
>
>> +Required properties:
>> +- compatible: "sirf,prima2-pwrc", or "sirf,atlas7-pwrc"
>> +- reg: Address range of pwrc register set
>
> Address start and size.
>
>> +- sysctl:mfd cell device of pwrc
>> +- rtcm-clk:mfd cell device of pwrc
>> +- onkey:mfd cell device of pwrc
>
> MFD is a Linuxisum. Please find another way to document them.
>
> I always find documentation easier to read then it's tabbed out
> nicely. Like:
>
> Required properties:
> - compatible : "sirf,prima2-pwrc", or "sirf,atlas7-pwrc"
> - reg : Address range of pwrc register set
> - sysctl : mfd cell device of pwrc
> - rtcm-clk : mfd cell device of pwrc
> - onkey : mfd cell device of pwrc
>
>> +Example:
>> +
>> +pwrc@3000 {
>> + compatible = "sirf,atlas7-pwrc";
>> + reg = <0x3000 0x100>;
>
> This doesn't look like a real address. It looks like an offset.
> Please provide a full example, complete with the parent node (which I
> assume has ranges set-up).
>
>> + sysctl {
>> + compatible = "sirf,sirf-sysctl";
>> + };
>> +
>> + rtcm-clk {
>> + compatible = "sirf,atlas7-rtcmclk";
>> + };
>> +
>> + onkey {
>> + compatible = "sirf,prima2-onkey";
>> + };
>
> Why do these require their own cells?
>
> Do they have their own properties? If so, where are they documented?
>
>> +};
>> +
>> +pwrc@3000 {
>> + compatible = "sirf,prima2-pwrc";
>> + reg = <0x3000 0x100>;
>> +};
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 99d6367..5b67bee 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -1515,5 +1515,17 @@ config MFD_VEXPRESS_SYSREG
>> System Registers are the platform configuration block
>> on the ARM Ltd. Versatile Express board.
>>
>> +config MFD_SIRFSOC_PWRC
>> + bool "CSR SiRFSoC on-chip Power Control Module"
>> + depends on ARCH_SIRF
>> + default y
>> + select MFD_CORE
>> + select REGMAP_IRQ
>> + help
>> + CSR SiRFSoC Power Control Module includes power on or power
>> + off for sysctl power and gnss, it also includes onkey, RTC
>> + domain clock controllers and interrupt controller for power
>> + events.
>
> Break out the TLAs.
>
> Your tabbing is all over the place here. Please match existing
> entries.
>
>> endmenu
>> endif
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index a59e3fc..255fb80 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -192,3 +192,5 @@ obj-$(CONFIG_MFD_SKY81452) += sky81452.o
>> intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
>> obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
>> obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
>> +
>> +obj-$(CONFIG_MFD_SIRFSOC_PWRC) += sirfsoc_pwrc.o
>> diff --git a/drivers/mfd/sirfsoc_pwrc.c b/drivers/mfd/sirfsoc_pwrc.c
>> new file mode 100644
>> index 0000000..b43f8b4
>> --- /dev/null
>> +++ b/drivers/mfd/sirfsoc_pwrc.c
>> @@ -0,0 +1,238 @@
>> +/*
>> + * power management mfd for CSR SiRFSoC chips
>
> "Power Management MFD for CSR SiRFSoC chips"
>
>> + * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
>
> This is out of date.
>
>> + * Licensed under GPLv2 or later.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/io.h>
>> +#include <linux/regmap.h>
>> +#include <linux/module.h>
>
> This isn't a module.
>
>> +#include <linux/rtc/sirfsoc_rtciobrg.h>
>
> What's this for?
the registers are behind a rtc io bridge. HW needs to access the
bridge to access registers.
another example similar with pwrc MFD is sysrtc as below:
2019 rtc-iobg@18840000 {
2020 compatible = "sirf,prima2-rtciobg",
2021 "sirf-prima2-rtciobg-bus",
2022 "simple-bus";
2023 #address-cells = <1>;
2024 #size-cells = <1>;
2025 reg = <0x18840000 0x1000>;
2026
2027 sysrtc@2000 {
2028 compatible = "sirf,prima2-sysrtc";
2029 reg = <0x2000 0x100>;
2030 interrupts = <0 52 0>;
2031 };
2032 pwrc@3000 {
2033 compatible = "sirf,atlas7-pwrc";
2034 reg = <0x3000 0x100>;
2035 interrupts = <0 32 0>;
2036 rtcmclk: clock-controller {
its driver is at
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/rtc/rtc-sirfsoc.c
>
>> +#include <linux/mfd/core.h>
>> +#include <linux/mfd/sirfsoc_pwrc.h>
>
> Header files should be in alphabetical order.
>
>> +struct sirfsoc_pwrc_register sirfsoc_a7da_pwrc = {
>> + .pwrc_pdn_ctrl_set = 0x0,
>> + .pwrc_pdn_ctrl_clr = 0x4,
>> + .pwrc_pon_status = 0x8,
>> + .pwrc_trigger_en_set = 0xc,
>> + .pwrc_trigger_en_clr = 0x10,
>> + .pwrc_int_mask_set = 0x14,
>> + .pwrc_int_mask_clr = 0x18,
>> + .pwrc_int_status = 0x1c,
>> + .pwrc_pin_status = 0x20,
>> + .pwrc_rtc_pll_ctrl = 0x28,
>> + .pwrc_gpio3_debug = 0x34,
>> + .pwrc_rtc_noc_pwrctl_set = 0x38,
>> + .pwrc_rtc_noc_pwrctl_clr = 0x3c,
>> + .pwrc_rtc_can_ctrl = 0x48,
>> + .pwrc_rtc_can_status = 0x4c,
>> + .pwrc_fsm_m3_ctrl = 0x50,
>> + .pwrc_fsm_state = 0x54,
>> + .pwrc_rtcldo_reg = 0x58,
>> + .pwrc_gnss_ctrl = 0x5c,
>> + .pwrc_gnss_status = 0x60,
>> + .pwrc_xtal_reg = 0x64,
>> + .pwrc_xtal_ldo_mux_sel = 0x68,
>> + .pwrc_rtc_sw_rstc_set = 0x6c,
>> + .pwrc_rtc_sw_rstc_clr = 0x70,
>> + .pwrc_power_sw_ctrl_set = 0x74,
>> + .pwrc_power_sw_ctrl_clr = 0x78,
>> + .pwrc_rtc_dcog = 0x7c,
>> + .pwrc_m3_memories = 0x80,
>> + .pwrc_can0_memory = 0x84,
>> + .pwrc_rtc_gnss_memory = 0x88,
>> + .pwrc_m3_clk_en = 0x8c,
>> + .pwrc_can0_clk_en = 0x90,
>> + .pwrc_spi0_clk_en = 0x94,
>> + .pwrc_rtc_sec_clk_en = 0x98,
>> + .pwrc_rtc_noc_clk_en = 0x9c,
>> +};
>> +
>> +struct sirfsoc_pwrc_register sirfsoc_prima2_pwrc = {
>> + .pwrc_pdn_ctrl_set = 0x0,
>> + .pwrc_pon_status = 0x4,
>> + .pwrc_trigger_en_set = 0x8,
>> + .pwrc_int_status = 0xc,
>> + .pwrc_int_mask_set = 0x10,
>> + .pwrc_pin_status = 0x14,
>> + .pwrc_scratch_pad1 = 0x18,
>> + .pwrc_scratch_pad2 = 0x1c,
>> + .pwrc_scratch_pad3 = 0x20,
>> + .pwrc_scratch_pad4 = 0x24,
>> + .pwrc_scratch_pad5 = 0x28,
>> + .pwrc_scratch_pad6 = 0x2c,
>> + .pwrc_scratch_pad7 = 0x30,
>> + .pwrc_scratch_pad8 = 0x34,
>> + .pwrc_scratch_pad9 = 0x38,
>> + .pwrc_scratch_pad10 = 0x3c,
>> + .pwrc_scratch_pad11 = 0x40,
>> + .pwrc_scratch_pad12 = 0x44,
>> + .pwrc_gpio3_clk = 0x54,
>> + .pwrc_gpio_ds = 0x78,
>> +};
>
> This is not the way we usually define register addresses.
i agree using MARCO is the general rules. but here moving to struct is
for avoiding things like:
if (prima2...)
...
elif(atlas7)
...
it is making registers offset private data which can be a member of the object.
>
> Please #define them properly.
>
>> +static const struct regmap_irq pwrc_irqs[] = {
>> + /* INT0 */
>> + [PWRC_IRQ_ONKEY] = {
>> + .mask = BIT(PWRC_IRQ_ONKEY),
>
> Better to do the BIT() shifting in the header file.
>
>> + },
>> + [PWRC_IRQ_EXT_ONKEY] = {
>> + .mask = BIT(PWRC_IRQ_EXT_ONKEY),
>> + },
>> +};
>> +
>> +static struct regmap_irq_chip pwrc_irq_chip = {
>> + .name = "pwrc_irq",
>> + .irqs = pwrc_irqs,
>> + .num_irqs = ARRAY_SIZE(pwrc_irqs),
>> + .num_regs = 1,
>> + .ack_invert = 1,
>> + .init_ack_masked = true,
>> +};
>> +
>> +static const struct of_device_id pwrc_ids[] = {
>> + { .compatible = "sirf,prima2-pwrc", .data = &sirfsoc_prima2_pwrc},
>> + { .compatible = "sirf,atlas7-pwrc", .data = &sirfsoc_a7da_pwrc},
>> + {}
>> +};
>
> Please put these _just_ before you use them, so just above .probe() in
> this case.
>
>> +static const struct mfd_cell pwrc_devs[] = {
>> + {
>> + .name = "rtcmclk",
>> + .of_compatible = "sirf,atlas7-rtcmclk",
>> + }, {
>> + .name = "sirf-sysctl",
>> + .of_compatible = "sirf,sirf-sysctl",
>> + }, {
>> + .name = "gps-power",
>> + .of_compatible = "sirf,gps-power",
>> + }, {
>> + .name = "onkey",
>> + .of_compatible = "sirf,prima2-onkey",
>> + },
>> +};
>> +
>> +static const struct regmap_config pwrc_regmap_config = {
>> + .fast_io = true,
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> +};
>> +
>> +static int sirfsoc_pwrc_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + const struct of_device_id *match;
>> + struct sirfsoc_pwrc_info *pwrcinfo;
>> + struct regmap_irq_chip *regmap_irq_chip;
>> + struct sirfsoc_pwrc_register *pwrc_reg;
>> + struct regmap *map;
>> + int ret;
>> + u32 base;
>> +
>> + if (of_property_read_u32(np, "reg", &base))
>> + panic("unable to find base address of pwrc node in dtb\n");
>
> It looks like this driver should depend on OF.
>
> Why are you obtaining the base address manually? Use:
>
> res = platform_get_resource();
> devm_ioremap_resource(res);
>
> ... instead.
this was explained as they are not in memory space, they are behind a
bus bridge.
>
>> + pwrcinfo = devm_kzalloc(&pdev->dev,
>> + sizeof(struct sirfsoc_pwrc_info), GFP_KERNEL);
>
> sizeof(pwrcinfo)
>
>> + if (!pwrcinfo)
>> + return -ENOMEM;
>> + pwrcinfo->base = base;
>> +
>> + /*
>> + * pwrc behind rtciobrg offset is diff between prima2 and atlas7
>> + * here match to each ids data for it.
>> + */
>
> Use uppercase characters for abbreviations.
>
> Besides, I have no idea what you're trying to say.
>
>> + match = of_match_node(pwrc_ids, np);
>> + pwrcinfo->pwrc_reg = (struct sirfsoc_pwrc_register *)match->data;
>
> It looks like you're trying to use the same variables two different
> device's register sets. That's asking for trouble, please don't do
> that.
>
>> + if (of_device_is_compatible(np, "sirf,atlas7-pwrc"))
>> + pwrcinfo->ver = PWRC_ATLAS7_VER;
>> + else if (of_device_is_compatible(np, "sirf,prima2-pwrc"))
>> + pwrcinfo->ver = PWRC_PRIMA2_VER;
>> + else
>> + return -EINVAL;
>
> These aren't versions, are they? It's different hardware.
>
> Can't you probe for this at runtime?
>
>> + of_node_put(np);
>
> What are you putting here?
>
>> + map = (struct regmap *)devm_regmap_init_iobg(&pdev->dev,
>> + &pwrc_regmap_config);
>
> Why are you casting?
>
>> + if (IS_ERR(map)) {
>> + ret = PTR_ERR(map);
>> + dev_err(&pdev->dev, "Failed to allocate register map: %d\n",
>> + ret);
>> + goto err;
>
> Please remove the 'err' label and just return from here.
>
>> + }
>> +
>> + pwrcinfo->regmap = map;
>> + pwrcinfo->dev = &pdev->dev;
>> + platform_set_drvdata(pdev, pwrcinfo);
>> +
>> + ret = mfd_add_devices(pwrcinfo->dev, 0, pwrc_devs,
>> + ARRAY_SIZE(pwrc_devs), NULL, 0, NULL);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to add all pwrc subdev\n");
>
> Capitalise all abbreviations.
>
> Don't abbreviate things like "sub devices".
>
>> + goto err;
>
> Just return.
>
>> + }
>> +
>> + ret = of_irq_get(pdev->dev.of_node, 0);
>
> You already put this in to a succinct variable 'np'. Please use it.
>
>> + if (ret <= 0) {
>> + dev_info(&pdev->dev,
>> + "Unable to find IRQ for pwrc. ret=%d\n", ret);
>
> Just print ret. No need for the ugly "ret=".
>
>> + goto err_irq;
>> + }
>> +
>> + pwrcinfo->irq = ret;
>
> Better change this to 'irq' instead of using 'ret'.
>
>> + regmap_irq_chip = &pwrc_irq_chip;
>> + pwrcinfo->regmap_irq_chip = regmap_irq_chip;
>> +
>> + pwrc_reg = pwrcinfo->pwrc_reg;
>> + regmap_irq_chip->mask_base = pwrcinfo->base +
>> + pwrc_reg->pwrc_int_mask_set;
>> + regmap_irq_chip->unmask_base = pwrcinfo->base +
>> + pwrc_reg->pwrc_int_mask_clr;
>> + regmap_irq_chip->status_base = pwrcinfo->base +
>> + pwrc_reg->pwrc_int_status;
>> + regmap_irq_chip->ack_base = pwrcinfo->base +
>> + pwrc_reg->pwrc_int_status;
>
> This is ugly.
>
> Better to create 2 regmap_irq_chip structures, one for each device.
there is only one device. why two regmap_irq_chip structures?
the driver is compatible with prima2 and atlas7, but any time there is
only one of them,
and the register needs to be adjust from dts and offset table.
>
>> + /* enable onkey trigger interrupt controller */
>
> Capital letters to start a sentence.
>
>> + ret = regmap_update_bits(map,
>> + pwrcinfo->base +
>> + pwrc_reg->pwrc_trigger_en_set,
>> + BIT(PWRC_IRQ_ONKEY) | BIT(PWRC_IRQ_EXT_ONKEY),
>> + BIT(PWRC_IRQ_ONKEY) | BIT(PWRC_IRQ_EXT_ONKEY));
>> + if (ret < 0)
>> + goto err_irq;
>> +
>> + /* add irq controller for pwrc */
>
> Capital letters to start a sentence and for abbreviations.
>
>> + ret = regmap_add_irq_chip(map, pwrcinfo->irq, IRQF_ONESHOT,
>> + -1, pwrcinfo->regmap_irq_chip,
>> + &pwrcinfo->irq_data);
>> +
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to add regmap irq controller for pwrc\n");
>> + goto err;
>> + }
>> +
>> + return 0;
>> +err_irq:
>> + mfd_remove_devices(pwrcinfo->dev);
>> +err:
>
> Remove this label, it's not helpful.
>
>> + return ret;
>> +}
>> +
>> +static struct platform_driver sirfsoc_pwrc_driver = {
>> + .probe = sirfsoc_pwrc_probe,
>
> .remove?
>
>> + .driver = {
>> + .name = "sirfsoc_pwrc",
>> + .of_match_table = pwrc_ids,
>
> of_match_ptr()
>
>> + },
>> +};
>> +module_platform_driver(sirfsoc_pwrc_driver);
>
> This isn't a module.
so do you think it is still a platform, what is the best way to probe them?
>
>> diff --git a/include/linux/mfd/sirfsoc_pwrc.h b/include/linux/mfd/sirfsoc_pwrc.h
>> new file mode 100644
>> index 0000000..ee2b3d5
>> --- /dev/null
>> +++ b/include/linux/mfd/sirfsoc_pwrc.h
>> @@ -0,0 +1,97 @@
>> +/*
>> + * CSR SiRFSoC power control module MFD interface
>> + *
>> + * Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
>
> Out of date.
>
>> + * Licensed under GPLv2 or later.
>> + */
>
> '\n' here.
>
>> +#ifndef _SIRFSOC_PWRC_H_
>> +#define _SIRFSOC_PWRC_H_
>
> '\n' here.
>
>> +#include <linux/interrupt.h>
>
> What's this for?
>
>> +#include <linux/regmap.h>
>> +
>> +#define PWRC_PDN_CTRL_OFFSET 0
>> +#define AUDIO_POWER_EN_BIT 14
>> +
>> +struct sirfsoc_pwrc_register {
>> + /* hardware pwrc specific */
>> + u32 pwrc_pdn_ctrl_set;
>> + u32 pwrc_pdn_ctrl_clr;
>> + u32 pwrc_pon_status;
>> + u32 pwrc_trigger_en_set;
>> + u32 pwrc_trigger_en_clr;
>> + u32 pwrc_int_mask_set;
>> + u32 pwrc_int_mask_clr;
>> + u32 pwrc_int_status;
>> + u32 pwrc_pin_status;
>> + u32 pwrc_rtc_pll_ctrl;
>> + u32 pwrc_gpio3_debug;
>> + u32 pwrc_rtc_noc_pwrctl_set;
>> + u32 pwrc_rtc_noc_pwrctl_clr;
>> + u32 pwrc_rtc_can_ctrl;
>> + u32 pwrc_rtc_can_status;
>> + u32 pwrc_fsm_m3_ctrl;
>> + u32 pwrc_fsm_state;
>> + u32 pwrc_rtcldo_reg;
>> + u32 pwrc_gnss_ctrl;
>> + u32 pwrc_gnss_status;
>> + u32 pwrc_xtal_reg;
>> + u32 pwrc_xtal_ldo_mux_sel;
>> + u32 pwrc_rtc_sw_rstc_set;
>> + u32 pwrc_rtc_sw_rstc_clr;
>> + u32 pwrc_power_sw_ctrl_set;
>> + u32 pwrc_power_sw_ctrl_clr;
>> + u32 pwrc_rtc_dcog;
>> + u32 pwrc_m3_memories;
>> + u32 pwrc_can0_memory;
>> + u32 pwrc_rtc_gnss_memory;
>> + u32 pwrc_m3_clk_en;
>> + u32 pwrc_can0_clk_en;
>> + u32 pwrc_spi0_clk_en;
>> + u32 pwrc_rtc_sec_clk_en;
>> + u32 pwrc_rtc_noc_clk_en;
>> +
>> + /*only for prima2*/
>> + u32 pwrc_scratch_pad1;
>> + u32 pwrc_scratch_pad2;
>> + u32 pwrc_scratch_pad3;
>> + u32 pwrc_scratch_pad4;
>> + u32 pwrc_scratch_pad5;
>> + u32 pwrc_scratch_pad6;
>> + u32 pwrc_scratch_pad7;
>> + u32 pwrc_scratch_pad8;
>> + u32 pwrc_scratch_pad9;
>> + u32 pwrc_scratch_pad10;
>> + u32 pwrc_scratch_pad11;
>> + u32 pwrc_scratch_pad12;
>> + u32 pwrc_gpio3_clk;
>> + u32 pwrc_gpio_ds;
>> +
>> +};
>
> Please #define these instead.
>
>> +enum pwrc_version {
>> + PWRC_MARCO_VER,
>> + PWRC_PRIMA2_VER,
>> + PWRC_ATLAS7_VER,
>> +};
>
> Kerneldoc header?
>
>> +struct sirfsoc_pwrc_info {
>> + struct device *dev;
>> + struct regmap *regmap;
>> + struct sirfsoc_pwrc_register *pwrc_reg;
>> + struct regmap_irq_chip *regmap_irq_chip;
>> + struct regmap_irq_chip_data *irq_data;
>
> Where are these reused?
>
>> + u32 ver;
>
> What is this used for?
>
>> + u32 base;
>
> Is this a u32 or a memory address?
>
>> + int irq;
>> +};
>> +
>> +enum {
>> + PWRC_IRQ_ONKEY = 0,
>> + PWRC_IRQ_EXT_ONKEY,
>> + PWRC_MAX_IRQ,
>> +};
>> +
>> +extern struct sirfsoc_pwrc_register sirfsoc_a7da_pwrc;
>> +extern struct sirfsoc_pwrc_register sirfsoc_prima2_pwrc;
>> +#endif
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Barry Song <21cnbao@gmail.com> |
|---|---|
| Date | 2015-09-17 07:20 +0200 |
| Subject | [PATCH v2 2/3] regmap: irq: add ack_invert flag for chips using cleared bits as ack |
| Message-ID | <q9zLZ-1Zr-19@gated-at.bofh.it> |
| In reply to | #1226668 |
From: Guo Zeng <Guo.Zeng@csr.com>
An user will be CSR SiRFSoC ARM chips.
Signed-off-by: Guo Zeng <Guo.Zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
drivers/base/regmap/regmap-irq.c | 12 ++++++++++--
include/linux/regmap.h | 2 ++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 1821c3b..14af2c6 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -132,7 +132,11 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
if (d->mask_buf[i] && (d->chip->ack_base || d->chip->use_ack)) {
reg = d->chip->ack_base +
(i * map->reg_stride * d->irq_reg_stride);
- ret = regmap_write(map, reg, d->mask_buf[i]);
+ /* some chips ack by write 0 */
+ if (d->chip->ack_invert)
+ ret = regmap_write(map, reg, ~d->mask_buf[i]);
+ else
+ ret = regmap_write(map, reg, d->mask_buf[i]);
if (ret != 0)
dev_err(d->map->dev, "Failed to ack 0x%x: %d\n",
reg, ret);
@@ -469,7 +473,11 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags,
if (d->status_buf[i] && (chip->ack_base || chip->use_ack)) {
reg = chip->ack_base +
(i * map->reg_stride * d->irq_reg_stride);
- ret = regmap_write(map, reg,
+ if (chip->ack_invert)
+ ret = regmap_write(map, reg,
+ ~(d->status_buf[i] & d->mask_buf[i]));
+ else
+ ret = regmap_write(map, reg,
d->status_buf[i] & d->mask_buf[i]);
if (ret != 0) {
dev_err(map->dev, "Failed to ack 0x%x: %d\n",
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index f98fe9f..f36c9f9 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -809,6 +809,7 @@ struct regmap_irq {
* @init_ack_masked: Ack all masked interrupts once during initalization.
* @mask_invert: Inverted mask register: cleared bits are masked out.
* @use_ack: Use @ack register even if it is zero.
+ * @ack_invert: Inverted ack register: cleared bits for ack.
* @wake_invert: Inverted wake register: cleared bits are wake enabled.
* @runtime_pm: Hold a runtime PM lock on the device when accessing it.
*
@@ -829,6 +830,7 @@ struct regmap_irq_chip {
bool init_ack_masked:1;
bool mask_invert:1;
bool use_ack:1;
+ bool ack_invert:1;
bool wake_invert:1;
bool runtime_pm:1;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-09-17 13:00 +0200 |
| Subject | Re: [PATCH v2 1/3] regmap: irq: add support for chips who have separate unmask registers |
| Message-ID | <q9F50-1gS-7@gated-at.bofh.it> |
| In reply to | #1226668 |
[Multipart message — attachments visible in raw view] — view raw
On Thu, Sep 17, 2015 at 05:23:20AM +0000, Barry Song wrote: > From: Guo Zeng <Guo.Zeng@csr.com> > > Some chips have separate unmask registers from mask registers for > some consideration of concurrency SMP write performance. And this > patch adds a flag for it. I've pushed a tag regmap-irq-unmask so these can be pulled into another tree if needed (like for the MFD).
[toc] | [prev] | [next] | [standalone]
| From | Barry Song <21cnbao@gmail.com> |
|---|---|
| Date | 2015-09-17 16:30 +0200 |
| Subject | Re: [PATCH v2 1/3] regmap: irq: add support for chips who have separate unmask registers |
| Message-ID | <q9Imf-6cb-47@gated-at.bofh.it> |
| In reply to | #1226906 |
2015-09-17 18:53 GMT+08:00 Mark Brown <broonie@kernel.org>: > On Thu, Sep 17, 2015 at 05:23:20AM +0000, Barry Song wrote: >> From: Guo Zeng <Guo.Zeng@csr.com> >> >> Some chips have separate unmask registers from mask registers for >> some consideration of concurrency SMP write performance. And this >> patch adds a flag for it. > > I've pushed a tag regmap-irq-unmask so these can be pulled into another > tree if needed (like for the MFD). thanks, Mark, found it in your tree https://git.kernel.org/cgit/linux/kernel/git/broonie/regmap.git/ -barry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web