* [PATCH] mfd: cpcap: Add minimal support
@ 2016-11-19 1:27 Tony Lindgren
2016-11-21 11:45 ` Lee Jones
0 siblings, 1 reply; 9+ messages in thread
From: Tony Lindgren @ 2016-11-19 1:27 UTC (permalink / raw)
To: Lee Jones, Samuel Ortiz
Cc: linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott, Rob Herring
Many Motorola phones like droid 4 are using a custom PMIC called CPCAP
or 6556002. We can support it's core features quite easily with regmap_spi
and regmap_irq.
The children of cpcap, such as regulators, ADC and USB, can be just regular
device drivers and defined in the dts file. They get probed as we call
of_platform_populate() at the end of our probe, and then the children
can just call dev_get_regmap(dev.parent, NULL) to get the regmap.
Cc: devicetree@vger.kernel.org
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Scott <michael.scott@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Documentation/devicetree/bindings/mfd/cpcap.txt | 36 ++++
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/cpcap.c | 255 ++++++++++++++++++++++++
include/linux/mfd/cpcap.h | 238 ++++++++++++++++++++++
5 files changed, 538 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/cpcap.txt
create mode 100644 drivers/mfd/cpcap.c
create mode 100644 include/linux/mfd/cpcap.h
diff --git a/Documentation/devicetree/bindings/mfd/cpcap.txt b/Documentation/devicetree/bindings/mfd/cpcap.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/cpcap.txt
@@ -0,0 +1,36 @@
+CPCAP PMIC device tree binding
+
+Required properties:
+- compatible : Motorola device "motorola,cpcap", others "st,6556002"
+- reg : Chip select and size
+- interrupt-parent : The parent interrupt controller
+- interrupts : The interrupt line the device is connected to
+- interrupt-controller : Marks the device node as an interrupt controller
+- #interrupt-cells : The number of cells to describe an IRQ, should be 2
+- #address-cells : Child device offset number of cells, typically 1
+- #size-cells : Child device size number of cells, typically 1
+- ranges : Child device register range
+- spi-max-frequency : Typically set to 3000000
+- spi-cs_high : SPI chip select direction
+
+Example:
+
+&mcspi1 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ cpcap: pmic@0 {
+ compatible = "motorola,cpcap", "st,6556002";
+ reg = <0 0>; /* cs0, size 0 */
+ interrupt-parent = <&gpio1>;
+ interrupts = <7 IRQ_TYPE_EDGE_RISING>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0 0x8000>;
+ spi-max-frequency = <3000000>;
+ spi-cs-high;
+ };
+};
+
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -713,6 +713,14 @@ config EZX_PCAP
This enables the PCAP ASIC present on EZX Phones. This is
needed for MMC, TouchScreen, Sound, USB, etc..
+config MFD_CPCAP
+ tristate "Support for CPCAP"
+ depends on SPI && OF
+ help
+ Say yes here if you want to include driver for CPCAP.
+ It is used on many Motorola phones and tablets as a PMIC.
+ At least Motorola Droid 4 is known to use CPCAP.
+
config MFD_VIPERBOARD
tristate "Nano River Technologies Viperboard"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
obj-$(CONFIG_MFD_CORE) += mfd-core.o
obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
+obj-$(CONFIG_MFD_CPCAP) += cpcap.o
obj-$(CONFIG_MCP) += mcp-core.o
obj-$(CONFIG_MCP_SA11X0) += mcp-sa11x0.o
diff --git a/drivers/mfd/cpcap.c b/drivers/mfd/cpcap.c
new file mode 100644
--- /dev/null
+++ b/drivers/mfd/cpcap.c
@@ -0,0 +1,255 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/sysfs.h>
+
+#include <linux/mfd/cpcap.h>
+#include <linux/spi/spi.h>
+
+#define CPCAP_NR_IRQ_BANKS 6
+#define CPCAP_NR_IRQ_DOMAINS 3
+
+struct cpcap_device {
+ struct spi_device *spi;
+ struct device *dev;
+ u16 vendor;
+ u16 revision;
+ const struct cpcap_platform_data *conf;
+ struct regmap_irq *irqs;
+ struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_DOMAINS];
+ const struct regmap_config *regmap_conf;
+ struct regmap *regmap;
+};
+
+static int cpcap_check_revision(struct cpcap_device *cpcap)
+{
+ unsigned int val;
+ int error;
+
+ error = regmap_read(cpcap->regmap, CPCAP_REG_VERSC1, &val);
+ if (error)
+ return error;
+
+ cpcap->vendor = (val >> 6) & 0x0007;
+ cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
+ dev_info(cpcap->dev, "CPCAP vendor: %s rev: %i.%i (%x)\n",
+ cpcap->vendor ? "TI" : "ST", (cpcap->revision >> 4) + 1,
+ cpcap->revision & 0xf, cpcap->revision);
+
+ if (cpcap->revision < CPCAP_REVISION_2_1) {
+ dev_info(cpcap->dev,
+ "Please add old CPCAP revision support as needed\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+/*
+ * First domain is the two private macro interrupt banks, the third
+ * domain is for banks 1 - 4 and is available for drivers to use.
+ */
+static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_DOMAINS] = {
+ {
+ .name = "cpcap-m2",
+ .num_regs = 1,
+ .status_base = CPCAP_REG_MI1,
+ .ack_base = CPCAP_REG_MI1,
+ .mask_base = CPCAP_REG_MIM1,
+ .use_ack = true,
+ },
+ {
+ .name = "cpcap-m2",
+ .num_regs = 1,
+ .status_base = CPCAP_REG_MI2,
+ .ack_base = CPCAP_REG_MI2,
+ .mask_base = CPCAP_REG_MIM2,
+ .use_ack = true,
+ },
+ {
+ .name = "cpcap1-4",
+ .num_regs = 4,
+ .status_base = CPCAP_REG_INT1,
+ .ack_base = CPCAP_REG_INT1,
+ .mask_base = CPCAP_REG_INTM1,
+ .type_base = CPCAP_REG_INTS1,
+ .use_ack = true,
+ },
+};
+
+static int cpcap_init_irq_bank(struct cpcap_device *cpcap, int irq_domain,
+ int irq_start, int nr_irqs)
+{
+ struct regmap_irq_chip *domain = &cpcap_irq_chip[irq_domain];
+ int i, error;
+
+ for (i = irq_start; i < irq_start + nr_irqs; i++) {
+ struct regmap_irq *cpcap_irq = &cpcap->irqs[i];
+
+ cpcap_irq->reg_offset =
+ ((i - irq_start) / cpcap->regmap_conf->val_bits) *
+ cpcap->regmap_conf->reg_stride;
+ cpcap_irq->mask = BIT(i % cpcap->regmap_conf->val_bits);
+ }
+ domain->irqs = &cpcap->irqs[irq_start];
+ domain->num_irqs = nr_irqs;
+ domain->irq_drv_data = cpcap;
+
+ error = devm_regmap_add_irq_chip(cpcap->dev, cpcap->regmap,
+ cpcap->spi->irq,
+ IRQF_TRIGGER_RISING |
+ IRQF_SHARED, -1,
+ domain, &cpcap->irqdata[irq_domain]);
+ if (error) {
+ dev_err(cpcap->dev, "could not add irq domain %i: %i\n",
+ irq_domain, error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int cpcap_init_irq(struct cpcap_device *cpcap)
+{
+ int error;
+
+ cpcap->irqs = devm_kzalloc(cpcap->dev,
+ sizeof(*cpcap->irqs) *
+ CPCAP_NR_IRQ_BANKS *
+ cpcap->regmap_conf->val_bits,
+ GFP_KERNEL);
+ if (!cpcap->irqs)
+ return -ENOMEM;
+
+ error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
+ if (error)
+ return error;
+
+ error = cpcap_init_irq_bank(cpcap, 1, 16, 16);
+ if (error)
+ return error;
+
+ error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
+ if (error)
+ return error;
+
+ enable_irq_wake(cpcap->spi->irq);
+
+ return 0;
+}
+
+static const struct of_device_id cpcap_of_match[] = {
+ {
+ .compatible = "motorola,cpcap",
+ },
+ {
+ .compatible = "st,6556002",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, cpcap_of_match);
+
+static const struct regmap_config cpcap_regmap_config = {
+ .reg_bits = 16,
+ .reg_stride = 4,
+ .pad_bits = 0,
+ .val_bits = 16,
+ .write_flag_mask = 0x8000,
+ .max_register = CPCAP_REG_ST_TEST2,
+ .cache_type = REGCACHE_NONE,
+ .reg_format_endian = REGMAP_ENDIAN_LITTLE,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static const struct of_device_id cpcap_dt_match_table[] = {
+ { .compatible = "simple-bus", },
+ { },
+};
+
+static int cpcap_probe(struct spi_device *spi)
+{
+ const struct of_device_id *match;
+ int error = -EINVAL;
+ struct cpcap_device *cpcap;
+
+ match = of_match_device(of_match_ptr(cpcap_of_match), &spi->dev);
+ if (!match)
+ return -ENODEV;
+
+ cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
+ if (!cpcap)
+ return -ENOMEM;
+
+ cpcap->conf = match->data;
+ cpcap->spi = spi;
+ cpcap->dev = &spi->dev;
+ spi_set_drvdata(spi, cpcap);
+
+ spi->bits_per_word = 16;
+ spi->mode = SPI_MODE_0 | SPI_CS_HIGH;
+ error = spi_setup(spi);
+ if (error < 0)
+ return error;
+
+ cpcap->regmap_conf = &cpcap_regmap_config;
+ cpcap->regmap = devm_regmap_init_spi(spi, &cpcap_regmap_config);
+ if (IS_ERR(cpcap->regmap)) {
+ error = PTR_ERR(cpcap->regmap);
+ dev_err(cpcap->dev, "Failed to initialize regmap: %d\n",
+ error);
+
+ return error;
+ }
+
+ error = cpcap_check_revision(cpcap);
+ if (error)
+ return error;
+
+ error = cpcap_init_irq(cpcap);
+ if (error)
+ return error;
+
+ error = of_platform_populate(spi->dev.of_node,
+ cpcap_dt_match_table,
+ NULL, cpcap->dev);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static int cpcap_remove(struct spi_device *pdev)
+{
+ struct cpcap_device *cpcap = spi_get_drvdata(pdev);
+
+ of_platform_depopulate(cpcap->dev);
+
+ return 0;
+}
+
+static struct spi_driver cpcap_driver = {
+ .driver = {
+ .name = "cpcap-core",
+ .owner = THIS_MODULE,
+ .of_match_table = cpcap_of_match,
+ },
+ .probe = cpcap_probe,
+ .remove = cpcap_remove,
+};
+module_spi_driver(cpcap_driver);
+
+MODULE_ALIAS("platform:cpcap");
+MODULE_DESCRIPTION("CPCAP driver");
+MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/cpcap.h b/include/linux/mfd/cpcap.h
new file mode 100644
--- /dev/null
+++ b/include/linux/mfd/cpcap.h
@@ -0,0 +1,238 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Note that the register defines are based on earlier cpcap.h in
+ * Motorola Linux kernel tree except rewritten for the real register
+ * addresses instead of enumeration so they are usable with regmap.
+ */
+
+#define CPCAP_VENDOR_ST 0
+#define CPCAP_VENDOR_TI 1
+
+#define CPCAP_REVISION_1_0 0x08
+#define CPCAP_REVISION_1_1 0x09
+#define CPCAP_REVISION_2_0 0x10
+#define CPCAP_REVISION_2_1 0x11
+
+/* CPCAP registers */
+#define CPCAP_REG_INT1 0x0000 /* Interrupt 1 */
+#define CPCAP_REG_INT2 0x0004 /* Interrupt 2 */
+#define CPCAP_REG_INT3 0x0008 /* Interrupt 3 */
+#define CPCAP_REG_INT4 0x000c /* Interrupt 4 */
+#define CPCAP_REG_INTM1 0x0010 /* Interrupt Mask 1 */
+#define CPCAP_REG_INTM2 0x0014 /* Interrupt Mask 2 */
+#define CPCAP_REG_INTM3 0x0018 /* Interrupt Mask 3 */
+#define CPCAP_REG_INTM4 0x001c /* Interrupt Mask 4 */
+#define CPCAP_REG_INTS1 0x0020 /* Interrupt Sense 1 */
+#define CPCAP_REG_INTS2 0x0024 /* Interrupt Sense 2 */
+#define CPCAP_REG_INTS3 0x0028 /* Interrupt Sense 3 */
+#define CPCAP_REG_INTS4 0x002c /* Interrupt Sense 4 */
+#define CPCAP_REG_ASSIGN1 0x0030 /* Resource Assignment 1 */
+#define CPCAP_REG_ASSIGN2 0x0034 /* Resource Assignment 2 */
+#define CPCAP_REG_ASSIGN3 0x0038 /* Resource Assignment 3 */
+#define CPCAP_REG_ASSIGN4 0x003c /* Resource Assignment 4 */
+#define CPCAP_REG_ASSIGN5 0x0040 /* Resource Assignment 5 */
+#define CPCAP_REG_ASSIGN6 0x0044 /* Resource Assignment 6 */
+#define CPCAP_REG_VERSC1 0x0048 /* Version Control 1 */
+#define CPCAP_REG_VERSC2 0x004c /* Version Control 2 */
+
+#define CPCAP_REG_MI1 0x0200 /* Macro Interrupt 1 */
+#define CPCAP_REG_MIM1 0x0204 /* Macro Interrupt Mask 1 */
+#define CPCAP_REG_MI2 0x0208 /* Macro Interrupt 2 */
+#define CPCAP_REG_MIM2 0x020c /* Macro Interrupt Mask 2 */
+#define CPCAP_REG_UCC1 0x0210 /* UC Control 1 */
+#define CPCAP_REG_UCC2 0x0214 /* UC Control 2 */
+
+#define CPCAP_REG_PC1 0x021c /* Power Cut 1 */
+#define CPCAP_REG_PC2 0x0220 /* Power Cut 2 */
+#define CPCAP_REG_BPEOL 0x0224 /* BP and EOL */
+#define CPCAP_REG_PGC 0x0228 /* Power Gate and Control */
+#define CPCAP_REG_MT1 0x022c /* Memory Transfer 1 */
+#define CPCAP_REG_MT2 0x0230 /* Memory Transfer 2 */
+#define CPCAP_REG_MT3 0x0234 /* Memory Transfer 3 */
+#define CPCAP_REG_PF 0x0238 /* Print Format */
+
+#define CPCAP_REG_SCC 0x0400 /* System Clock Control */
+#define CPCAP_REG_SW1 0x0404 /* Stop Watch 1 */
+#define CPCAP_REG_SW2 0x0408 /* Stop Watch 2 */
+#define CPCAP_REG_UCTM 0x040c /* UC Turbo Mode */
+#define CPCAP_REG_TOD1 0x0410 /* Time of Day 1 */
+#define CPCAP_REG_TOD2 0x0414 /* Time of Day 2 */
+#define CPCAP_REG_TODA1 0x0418 /* Time of Day Alarm 1 */
+#define CPCAP_REG_TODA2 0x041c /* Time of Day Alarm 2 */
+#define CPCAP_REG_DAY 0x0420 /* Day */
+#define CPCAP_REG_DAYA 0x0424 /* Day Alarm */
+#define CPCAP_REG_VAL1 0x0428 /* Validity 1 */
+#define CPCAP_REG_VAL2 0x042c /* Validity 2 */
+
+#define CPCAP_REG_SDVSPLL 0x0600 /* Switcher DVS and PLL */
+#define CPCAP_REG_SI2CC1 0x0604 /* Switcher I2C Control 1 */
+#define CPCAP_REG_Si2CC2 0x0608 /* Switcher I2C Control 2 */
+#define CPCAP_REG_S1C1 0x060c /* Switcher 1 Control 1 */
+#define CPCAP_REG_S1C2 0x0610 /* Switcher 1 Control 2 */
+#define CPCAP_REG_S2C1 0x0614 /* Switcher 2 Control 1 */
+#define CPCAP_REG_S2C2 0x0618 /* Switcher 2 Control 2 */
+#define CPCAP_REG_S3C 0x061c /* Switcher 3 Control */
+#define CPCAP_REG_S4C1 0x0620 /* Switcher 4 Control 1 */
+#define CPCAP_REG_S4C2 0x0624 /* Switcher 4 Control 2 */
+#define CPCAP_REG_S5C 0x0628 /* Switcher 5 Control */
+#define CPCAP_REG_S6C 0x062c /* Switcher 6 Control */
+#define CPCAP_REG_VCAMC 0x0630 /* VCAM Control */
+#define CPCAP_REG_VCSIC 0x0634 /* VCSI Control */
+#define CPCAP_REG_VDACC 0x0638 /* VDAC Control */
+#define CPCAP_REG_VDIGC 0x063c /* VDIG Control */
+#define CPCAP_REG_VFUSEC 0x0640 /* VFUSE Control */
+#define CPCAP_REG_VHVIOC 0x0644 /* VHVIO Control */
+#define CPCAP_REG_VSDIOC 0x0648 /* VSDIO Control */
+#define CPCAP_REG_VPLLC 0x064c /* VPLL Control */
+#define CPCAP_REG_VRF1C 0x0650 /* VRF1 Control */
+#define CPCAP_REG_VRF2C 0x0654 /* VRF2 Control */
+#define CPCAP_REG_VRFREFC 0x0658 /* VRFREF Control */
+#define CPCAP_REG_VWLAN1C 0x065c /* VWLAN1 Control */
+#define CPCAP_REG_VWLAN2C 0x0660 /* VWLAN2 Control */
+#define CPCAP_REG_VSIMC 0x0664 /* VSIM Control */
+#define CPCAP_REG_VVIBC 0x0668 /* VVIB Control */
+#define CPCAP_REG_VUSBC 0x066c /* VUSB Control */
+#define CPCAP_REG_VUSBINT1C 0x0670 /* VUSBINT1 Control */
+#define CPCAP_REG_VUSBINT2C 0x0674 /* VUSBINT2 Control */
+#define CPCAP_REG_URT 0x0678 /* Useroff Regulator Trigger */
+#define CPCAP_REG_URM1 0x067c /* Useroff Regulator Mask 1 */
+#define CPCAP_REG_URM2 0x0680 /* Useroff Regulator Mask 2 */
+
+#define CPCAP_REG_VAUDIOC 0x0800 /* VAUDIO Control */
+#define CPCAP_REG_CC 0x0804 /* Codec Control */
+#define CPCAP_REG_CDI 0x0808 /* Codec Digital Interface */
+#define CPCAP_REG_SDAC 0x080c /* Stereo DAC */
+#define CPCAP_REG_SDACDI 0x0810 /* Stereo DAC Digital Interface */
+#define CPCAP_REG_TXI 0x0814 /* TX Inputs */
+#define CPCAP_REG_TXMP 0x0818 /* TX MIC PGA's */
+#define CPCAP_REG_RXOA 0x081c /* RX Output Amplifiers */
+#define CPCAP_REG_RXVC 0x0820 /* RX Volume Control */
+#define CPCAP_REG_RXCOA 0x0824 /* RX Codec to Output Amps */
+#define CPCAP_REG_RXSDOA 0x0828 /* RX Stereo DAC to Output Amps */
+#define CPCAP_REG_RXEPOA 0x082c /* RX External PGA to Output Amps */
+#define CPCAP_REG_RXLL 0x0830 /* RX Low Latency */
+#define CPCAP_REG_A2LA 0x0834 /* A2 Loudspeaker Amplifier */
+#define CPCAP_REG_MIPIS1 0x0838 /* MIPI Slimbus 1 */
+#define CPCAP_REG_MIPIS2 0x083c /* MIPI Slimbus 2 */
+#define CPCAP_REG_MIPIS3 0x0840 /* MIPI Slimbus 3. */
+#define CPCAP_REG_LVAB 0x0844 /* LMR Volume and A4 Balanced. */
+
+#define CPCAP_REG_CCC1 0x0a00 /* Coulomb Counter Control 1 */
+#define CPCAP_REG_CRM 0x0a04 /* Charger and Reverse Mode */
+#define CPCAP_REG_CCCC2 0x0a08 /* Coincell and Coulomb Ctr Ctrl 2 */
+#define CPCAP_REG_CCS1 0x0a0c /* Coulomb Counter Sample 1 */
+#define CPCAP_REG_CCS2 0x0a10 /* Coulomb Counter Sample 2 */
+#define CPCAP_REG_CCA1 0x0a14 /* Coulomb Counter Accumulator 1 */
+#define CPCAP_REG_CCA2 0x0a18 /* Coulomb Counter Accumulator 2 */
+#define CPCAP_REG_CCM 0x0a1c /* Coulomb Counter Mode */
+#define CPCAP_REG_CCO 0x0a20 /* Coulomb Counter Offset */
+#define CPCAP_REG_CCI 0x0a24 /* Coulomb Counter Integrator */
+
+#define CPCAP_REG_ADCC1 0x0c00 /* A/D Converter Configuration 1 */
+#define CPCAP_REG_ADCC2 0x0c04 /* A/D Converter Configuration 2 */
+#define CPCAP_REG_ADCD0 0x0c08 /* A/D Converter Data 0 */
+#define CPCAP_REG_ADCD1 0x0c0c /* A/D Converter Data 1 */
+#define CPCAP_REG_ADCD2 0x0c10 /* A/D Converter Data 2 */
+#define CPCAP_REG_ADCD3 0x0c14 /* A/D Converter Data 3 */
+#define CPCAP_REG_ADCD4 0x0c18 /* A/D Converter Data 4 */
+#define CPCAP_REG_ADCD5 0x0c1c /* A/D Converter Data 5 */
+#define CPCAP_REG_ADCD6 0x0c20 /* A/D Converter Data 6 */
+#define CPCAP_REG_ADCD7 0x0c24 /* A/D Converter Data 7 */
+#define CPCAP_REG_ADCAL1 0x0c28 /* A/D Converter Calibration 1 */
+#define CPCAP_REG_ADCAL2 0x0c2c /* A/D Converter Calibration 2 */
+
+#define CPCAP_REG_USBC1 0x0e00 /* USB Control 1 */
+#define CPCAP_REG_USBC2 0x0e04 /* USB Control 2 */
+#define CPCAP_REG_USBC3 0x0e08 /* USB Control 3 */
+#define CPCAP_REG_UVIDL 0x0e0c /* ULPI Vendor ID Low */
+#define CPCAP_REG_UVIDH 0x0e10 /* ULPI Vendor ID High */
+#define CPCAP_REG_UPIDL 0x0e14 /* ULPI Product ID Low */
+#define CPCAP_REG_UPIDH 0x0e18 /* ULPI Product ID High */
+#define CPCAP_REG_UFC1 0x0e1c /* ULPI Function Control 1 */
+#define CPCAP_REG_UFC2 0x0e20 /* ULPI Function Control 2 */
+#define CPCAP_REG_UFC3 0x0e24 /* ULPI Function Control 3 */
+#define CPCAP_REG_UIC1 0x0e28 /* ULPI Interface Control 1 */
+#define CPCAP_REG_UIC2 0x0e2c /* ULPI Interface Control 2 */
+#define CPCAP_REG_UIC3 0x0e30 /* ULPI Interface Control 3 */
+#define CPCAP_REG_USBOTG1 0x0e34 /* USB OTG Control 1 */
+#define CPCAP_REG_USBOTG2 0x0e38 /* USB OTG Control 2 */
+#define CPCAP_REG_USBOTG3 0x0e3c /* USB OTG Control 3 */
+#define CPCAP_REG_UIER1 0x0e40 /* USB Interrupt Enable Rising 1 */
+#define CPCAP_REG_UIER2 0x0e44 /* USB Interrupt Enable Rising 2 */
+#define CPCAP_REG_UIER3 0x0e48 /* USB Interrupt Enable Rising 3 */
+#define CPCAP_REG_UIEF1 0x0e4c /* USB Interrupt Enable Falling 1 */
+#define CPCAP_REG_UIEF2 0x0e50 /* USB Interrupt Enable Falling 1 */
+#define CPCAP_REG_UIEF3 0x0e54 /* USB Interrupt Enable Falling 1 */
+#define CPCAP_REG_UIS 0x0e58 /* USB Interrupt Status */
+#define CPCAP_REG_UIL 0x0e5c /* USB Interrupt Latch */
+#define CPCAP_REG_USBD 0x0e60 /* USB Debug */
+#define CPCAP_REG_SCR1 0x0e64 /* Scratch 1 */
+#define CPCAP_REG_SCR2 0x0e68 /* Scratch 2 */
+#define CPCAP_REG_SCR3 0x0e6c /* Scratch 3 */
+
+#define CPCAP_REG_VMC 0x0eac /* Video Mux Control */
+#define CPCAP_REG_OWDC 0x0eb0 /* One Wire Device Control */
+#define CPCAP_REG_GPIO0 0x0eb4 /* GPIO 0 Control */
+
+#define CPCAP_REG_GPIO1 0x0ebc /* GPIO 1 Control */
+
+#define CPCAP_REG_GPIO2 0x0ec4 /* GPIO 2 Control */
+
+#define CPCAP_REG_GPIO3 0x0ecc /* GPIO 3 Control */
+
+#define CPCAP_REG_GPIO4 0x0ed4 /* GPIO 4 Control */
+
+#define CPCAP_REG_GPIO5 0x0edc /* GPIO 5 Control */
+
+#define CPCAP_REG_GPIO6 0x0ee4 /* GPIO 6 Control */
+
+#define CPCAP_REG_MDLC 0x1000 /* Main Display Lighting Control */
+#define CPCAP_REG_KLC 0x1004 /* Keypad Lighting Control */
+#define CPCAP_REG_ADLC 0x1008 /* Aux Display Lighting Control */
+#define CPCAP_REG_REDC 0x100c /* Red Triode Control */
+#define CPCAP_REG_GREENC 0x1010 /* Green Triode Control */
+#define CPCAP_REG_BLUEC 0x1014 /* Blue Triode Control */
+#define CPCAP_REG_CFC 0x1018 /* Camera Flash Control */
+#define CPCAP_REG_ABC 0x101c /* Adaptive Boost Control */
+#define CPCAP_REG_BLEDC 0x1020 /* Bluetooth LED Control */
+#define CPCAP_REG_CLEDC 0x1024 /* Camera Privacy LED Control */
+
+#define CPCAP_REG_OW1C 0x1200 /* One Wire 1 Command */
+#define CPCAP_REG_OW1D 0x1204 /* One Wire 1 Data */
+#define CPCAP_REG_OW1I 0x1208 /* One Wire 1 Interrupt */
+#define CPCAP_REG_OW1IE 0x120c /* One Wire 1 Interrupt Enable */
+
+#define CPCAP_REG_OW1 0x1214 /* One Wire 1 Control */
+
+#define CPCAP_REG_OW2C 0x1220 /* One Wire 2 Command */
+#define CPCAP_REG_OW2D 0x1224 /* One Wire 2 Data */
+#define CPCAP_REG_OW2I 0x1228 /* One Wire 2 Interrupt */
+#define CPCAP_REG_OW2IE 0x122c /* One Wire 2 Interrupt Enable */
+
+#define CPCAP_REG_OW2 0x1234 /* One Wire 2 Control */
+
+#define CPCAP_REG_OW3C 0x1240 /* One Wire 3 Command */
+#define CPCAP_REG_OW3D 0x1244 /* One Wire 3 Data */
+#define CPCAP_REG_OW3I 0x1248 /* One Wire 3 Interrupt */
+#define CPCAP_REG_OW3IE 0x124c /* One Wire 3 Interrupt Enable */
+
+#define CPCAP_REG_OW3 0x1254 /* One Wire 3 Control */
+#define CPCAP_REG_GCAIC 0x1258 /* GCAI Clock Control */
+#define CPCAP_REG_GCAIM 0x125c /* GCAI GPIO Mode */
+#define CPCAP_REG_LGDIR 0x1260 /* LMR GCAI GPIO Direction */
+#define CPCAP_REG_LGPU 0x1264 /* LMR GCAI GPIO Pull-up */
+#define CPCAP_REG_LGPIN 0x1268 /* LMR GCAI GPIO Pin */
+#define CPCAP_REG_LGMASK 0x126c /* LMR GCAI GPIO Mask */
+#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
+#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
+#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
+#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
+
+#define CPCAP_REG_TEST 0x7c00 /* Test */
+
+#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
+
+#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
--
2.10.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-19 1:27 [PATCH] mfd: cpcap: Add minimal support Tony Lindgren
@ 2016-11-21 11:45 ` Lee Jones
2016-11-21 16:34 ` Rob Herring
2016-11-23 15:26 ` Tony Lindgren
0 siblings, 2 replies; 9+ messages in thread
From: Lee Jones @ 2016-11-21 11:45 UTC (permalink / raw)
To: Tony Lindgren
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott, Rob Herring
On Fri, 18 Nov 2016, Tony Lindgren wrote:
> Many Motorola phones like droid 4 are using a custom PMIC called CPCAP
> or 6556002. We can support it's core features quite easily with regmap_spi
> and regmap_irq.
>
> The children of cpcap, such as regulators, ADC and USB, can be just regular
> device drivers and defined in the dts file. They get probed as we call
> of_platform_populate() at the end of our probe, and then the children
> can just call dev_get_regmap(dev.parent, NULL) to get the regmap.
>
> Cc: devicetree@vger.kernel.org
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Michael Scott <michael.scott@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> Documentation/devicetree/bindings/mfd/cpcap.txt | 36 ++++
> drivers/mfd/Kconfig | 8 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/cpcap.c | 255 ++++++++++++++++++++++++
> include/linux/mfd/cpcap.h | 238 ++++++++++++++++++++++
> 5 files changed, 538 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/cpcap.txt
> create mode 100644 drivers/mfd/cpcap.c
> create mode 100644 include/linux/mfd/cpcap.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/cpcap.txt b/Documentation/devicetree/bindings/mfd/cpcap.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/cpcap.txt
> @@ -0,0 +1,36 @@
> +CPCAP PMIC device tree binding
> +
> +Required properties:
> +- compatible : Motorola device "motorola,cpcap", others "st,6556002"
> +- reg : Chip select and size
> +- interrupt-parent : The parent interrupt controller
> +- interrupts : The interrupt line the device is connected to
> +- interrupt-controller : Marks the device node as an interrupt controller
> +- #interrupt-cells : The number of cells to describe an IRQ, should be 2
> +- #address-cells : Child device offset number of cells, typically 1
> +- #size-cells : Child device size number of cells, typically 1
> +- ranges : Child device register range
> +- spi-max-frequency : Typically set to 3000000
> +- spi-cs_high : SPI chip select direction
> +
> +Example:
> +
> +&mcspi1 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + cpcap: pmic@0 {
> + compatible = "motorola,cpcap", "st,6556002";
> + reg = <0 0>; /* cs0, size 0 */
Is this really correct?
How can ranges have a size of 0x8000 and this 0?
> + interrupt-parent = <&gpio1>;
> + interrupts = <7 IRQ_TYPE_EDGE_RISING>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0x8000>;
> + spi-max-frequency = <3000000>;
> + spi-cs-high;
> + };
> +};
> +
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -713,6 +713,14 @@ config EZX_PCAP
> This enables the PCAP ASIC present on EZX Phones. This is
> needed for MMC, TouchScreen, Sound, USB, etc..
>
> +config MFD_CPCAP
> + tristate "Support for CPCAP"
> + depends on SPI && OF
COMPILE_TEST?
> + help
> + Say yes here if you want to include driver for CPCAP.
> + It is used on many Motorola phones and tablets as a PMIC.
> + At least Motorola Droid 4 is known to use CPCAP.
> +
> config MFD_VIPERBOARD
> tristate "Nano River Technologies Viperboard"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> obj-$(CONFIG_MFD_CORE) += mfd-core.o
>
> obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
Who is the manufacturer?
> obj-$(CONFIG_MCP) += mcp-core.o
> obj-$(CONFIG_MCP_SA11X0) += mcp-sa11x0.o
> diff --git a/drivers/mfd/cpcap.c b/drivers/mfd/cpcap.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/mfd/cpcap.c
> @@ -0,0 +1,255 @@
> +/*
Description?
Author?
Copyright?
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/mfd/cpcap.h>
> +#include <linux/spi/spi.h>
> +
> +#define CPCAP_NR_IRQ_BANKS 6
> +#define CPCAP_NR_IRQ_DOMAINS 3
> +
> +struct cpcap_device {
s/device/ddata/
It's not really the device. Device is 2 lines down.
> + struct spi_device *spi;
> + struct device *dev;
> + u16 vendor;
> + u16 revision;
Why is this in here?
> + const struct cpcap_platform_data *conf;
What's this? Where is it defined?
> + struct regmap_irq *irqs;
Why does this need to be in here?
> + struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_DOMAINS];
And this? Where is it used again?
> + const struct regmap_config *regmap_conf;
> + struct regmap *regmap;
> +};
> +
> +static int cpcap_check_revision(struct cpcap_device *cpcap)
> +{
> + unsigned int val;
> + int error;
> +
> + error = regmap_read(cpcap->regmap, CPCAP_REG_VERSC1, &val);
> + if (error)
> + return error;
> +
> + cpcap->vendor = (val >> 6) & 0x0007;
> + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
Lots of magic numbers here. I suggest you define them.
> + dev_info(cpcap->dev, "CPCAP vendor: %s rev: %i.%i (%x)\n",
> + cpcap->vendor ? "TI" : "ST", (cpcap->revision >> 4) + 1,
> + cpcap->revision & 0xf, cpcap->revision);
> +
> + if (cpcap->revision < CPCAP_REVISION_2_1) {
> + dev_info(cpcap->dev,
> + "Please add old CPCAP revision support as needed\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * First domain is the two private macro interrupt banks, the third
> + * domain is for banks 1 - 4 and is available for drivers to use.
> + */
> +static struct regmap_irq_chip cpcap_irq_chip[CPCAP_NR_IRQ_DOMAINS] = {
> + {
> + .name = "cpcap-m2",
> + .num_regs = 1,
> + .status_base = CPCAP_REG_MI1,
> + .ack_base = CPCAP_REG_MI1,
> + .mask_base = CPCAP_REG_MIM1,
> + .use_ack = true,
> + },
> + {
> + .name = "cpcap-m2",
> + .num_regs = 1,
> + .status_base = CPCAP_REG_MI2,
> + .ack_base = CPCAP_REG_MI2,
> + .mask_base = CPCAP_REG_MIM2,
> + .use_ack = true,
> + },
> + {
> + .name = "cpcap1-4",
> + .num_regs = 4,
> + .status_base = CPCAP_REG_INT1,
> + .ack_base = CPCAP_REG_INT1,
> + .mask_base = CPCAP_REG_INTM1,
> + .type_base = CPCAP_REG_INTS1,
> + .use_ack = true,
> + },
> +};
> +
> +static int cpcap_init_irq_bank(struct cpcap_device *cpcap, int irq_domain,
> + int irq_start, int nr_irqs)
> +{
> + struct regmap_irq_chip *domain = &cpcap_irq_chip[irq_domain];
I suggest the term 'domain' is not correct here.
In Linux terminology these are 'chips'.
If you wish to create an IRQ domain, that requires a different API.
> + int i, error;
> +
> + for (i = irq_start; i < irq_start + nr_irqs; i++) {
> + struct regmap_irq *cpcap_irq = &cpcap->irqs[i];
> +
> + cpcap_irq->reg_offset =
> + ((i - irq_start) / cpcap->regmap_conf->val_bits) *
> + cpcap->regmap_conf->reg_stride;
> + cpcap_irq->mask = BIT(i % cpcap->regmap_conf->val_bits);
> + }
> + domain->irqs = &cpcap->irqs[irq_start];
> + domain->num_irqs = nr_irqs;
> + domain->irq_drv_data = cpcap;
> +
> + error = devm_regmap_add_irq_chip(cpcap->dev, cpcap->regmap,
> + cpcap->spi->irq,
> + IRQF_TRIGGER_RISING |
> + IRQF_SHARED, -1,
> + domain, &cpcap->irqdata[irq_domain]);
> + if (error) {
> + dev_err(cpcap->dev, "could not add irq domain %i: %i\n",
> + irq_domain, error);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int cpcap_init_irq(struct cpcap_device *cpcap)
> +{
> + int error;
> +
> + cpcap->irqs = devm_kzalloc(cpcap->dev,
> + sizeof(*cpcap->irqs) *
> + CPCAP_NR_IRQ_BANKS *
> + cpcap->regmap_conf->val_bits,
> + GFP_KERNEL);
> + if (!cpcap->irqs)
> + return -ENOMEM;
> +
> + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
'ret' is more traditional.
> + if (error)
> + return error;
> +
> + error = cpcap_init_irq_bank(cpcap, 1, 16, 16);
> + if (error)
> + return error;
> +
> + error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
> + if (error)
> + return error;
I don't think I've seen this method of adding bulk IRQ chips before.
Isn't there a cleaner or generic way to do this?
> + enable_irq_wake(cpcap->spi->irq);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id cpcap_of_match[] = {
> + {
> + .compatible = "motorola,cpcap",
> + },
Single line please.
> + {
> + .compatible = "st,6556002",
> + },
Single line please.
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> +
> +static const struct regmap_config cpcap_regmap_config = {
> + .reg_bits = 16,
> + .reg_stride = 4,
> + .pad_bits = 0,
> + .val_bits = 16,
> + .write_flag_mask = 0x8000,
> + .max_register = CPCAP_REG_ST_TEST2,
> + .cache_type = REGCACHE_NONE,
> + .reg_format_endian = REGMAP_ENDIAN_LITTLE,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static const struct of_device_id cpcap_dt_match_table[] = {
> + { .compatible = "simple-bus", },
> + { },
> +};
> +
> +static int cpcap_probe(struct spi_device *spi)
> +{
> + const struct of_device_id *match;
> + int error = -EINVAL;
ret
> + struct cpcap_device *cpcap;
> +
> + match = of_match_device(of_match_ptr(cpcap_of_match), &spi->dev);
> + if (!match)
> + return -ENODEV;
> +
> + cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> + if (!cpcap)
> + return -ENOMEM;
> +
> + cpcap->conf = match->data;
What is contained in the data? Nothing by the looks of it.
> + cpcap->spi = spi;
> + cpcap->dev = &spi->dev;
If you have 'spi' there is no need for 'dev'.
> + spi_set_drvdata(spi, cpcap);
> +
> + spi->bits_per_word = 16;
> + spi->mode = SPI_MODE_0 | SPI_CS_HIGH;
'\n' here.
> + error = spi_setup(spi);
> + if (error < 0)
> + return error;
> +
> + cpcap->regmap_conf = &cpcap_regmap_config;
> + cpcap->regmap = devm_regmap_init_spi(spi, &cpcap_regmap_config);
> + if (IS_ERR(cpcap->regmap)) {
> + error = PTR_ERR(cpcap->regmap);
> + dev_err(cpcap->dev, "Failed to initialize regmap: %d\n",
> + error);
> +
> + return error;
> + }
> +
> + error = cpcap_check_revision(cpcap);
> + if (error)
Are you sure you want to fail silently here?
> + return error;
> +
> + error = cpcap_init_irq(cpcap);
> + if (error)
> + return error;
> +
> + error = of_platform_populate(spi->dev.of_node,
> + cpcap_dt_match_table,
> + NULL, cpcap->dev);
> + if (error)
> + return error;
But don't you just "return of_platform_populate()"?
> + return 0;
> +}
> +
> +static int cpcap_remove(struct spi_device *pdev)
> +{
> + struct cpcap_device *cpcap = spi_get_drvdata(pdev);
> +
> + of_platform_depopulate(cpcap->dev);
> +
> + return 0;
> +}
> +
> +static struct spi_driver cpcap_driver = {
> + .driver = {
> + .name = "cpcap-core",
> + .owner = THIS_MODULE,
No need for this line. It's handled for you.
> + .of_match_table = cpcap_of_match,
> + },
> + .probe = cpcap_probe,
> + .remove = cpcap_remove,
> +};
> +module_spi_driver(cpcap_driver);
> +
> +MODULE_ALIAS("platform:cpcap");
> +MODULE_DESCRIPTION("CPCAP driver");
> +MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/cpcap.h b/include/linux/mfd/cpcap.h
> new file mode 100644
> --- /dev/null
> +++ b/include/linux/mfd/cpcap.h
> @@ -0,0 +1,238 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Note that the register defines are based on earlier cpcap.h in
> + * Motorola Linux kernel tree except rewritten for the real register
> + * addresses instead of enumeration so they are usable with regmap.
> + */
Copyright? Author? Description?
> +#define CPCAP_VENDOR_ST 0
> +#define CPCAP_VENDOR_TI 1
> +
> +#define CPCAP_REVISION_1_0 0x08
> +#define CPCAP_REVISION_1_1 0x09
> +#define CPCAP_REVISION_2_0 0x10
> +#define CPCAP_REVISION_2_1 0x11
> +
> +/* CPCAP registers */
> +#define CPCAP_REG_INT1 0x0000 /* Interrupt 1 */
> +#define CPCAP_REG_INT2 0x0004 /* Interrupt 2 */
> +#define CPCAP_REG_INT3 0x0008 /* Interrupt 3 */
> +#define CPCAP_REG_INT4 0x000c /* Interrupt 4 */
> +#define CPCAP_REG_INTM1 0x0010 /* Interrupt Mask 1 */
> +#define CPCAP_REG_INTM2 0x0014 /* Interrupt Mask 2 */
> +#define CPCAP_REG_INTM3 0x0018 /* Interrupt Mask 3 */
> +#define CPCAP_REG_INTM4 0x001c /* Interrupt Mask 4 */
> +#define CPCAP_REG_INTS1 0x0020 /* Interrupt Sense 1 */
> +#define CPCAP_REG_INTS2 0x0024 /* Interrupt Sense 2 */
> +#define CPCAP_REG_INTS3 0x0028 /* Interrupt Sense 3 */
> +#define CPCAP_REG_INTS4 0x002c /* Interrupt Sense 4 */
> +#define CPCAP_REG_ASSIGN1 0x0030 /* Resource Assignment 1 */
> +#define CPCAP_REG_ASSIGN2 0x0034 /* Resource Assignment 2 */
> +#define CPCAP_REG_ASSIGN3 0x0038 /* Resource Assignment 3 */
> +#define CPCAP_REG_ASSIGN4 0x003c /* Resource Assignment 4 */
> +#define CPCAP_REG_ASSIGN5 0x0040 /* Resource Assignment 5 */
> +#define CPCAP_REG_ASSIGN6 0x0044 /* Resource Assignment 6 */
> +#define CPCAP_REG_VERSC1 0x0048 /* Version Control 1 */
> +#define CPCAP_REG_VERSC2 0x004c /* Version Control 2 */
> +
> +#define CPCAP_REG_MI1 0x0200 /* Macro Interrupt 1 */
> +#define CPCAP_REG_MIM1 0x0204 /* Macro Interrupt Mask 1 */
> +#define CPCAP_REG_MI2 0x0208 /* Macro Interrupt 2 */
> +#define CPCAP_REG_MIM2 0x020c /* Macro Interrupt Mask 2 */
> +#define CPCAP_REG_UCC1 0x0210 /* UC Control 1 */
> +#define CPCAP_REG_UCC2 0x0214 /* UC Control 2 */
> +
> +#define CPCAP_REG_PC1 0x021c /* Power Cut 1 */
> +#define CPCAP_REG_PC2 0x0220 /* Power Cut 2 */
> +#define CPCAP_REG_BPEOL 0x0224 /* BP and EOL */
> +#define CPCAP_REG_PGC 0x0228 /* Power Gate and Control */
> +#define CPCAP_REG_MT1 0x022c /* Memory Transfer 1 */
> +#define CPCAP_REG_MT2 0x0230 /* Memory Transfer 2 */
> +#define CPCAP_REG_MT3 0x0234 /* Memory Transfer 3 */
> +#define CPCAP_REG_PF 0x0238 /* Print Format */
> +
> +#define CPCAP_REG_SCC 0x0400 /* System Clock Control */
> +#define CPCAP_REG_SW1 0x0404 /* Stop Watch 1 */
> +#define CPCAP_REG_SW2 0x0408 /* Stop Watch 2 */
> +#define CPCAP_REG_UCTM 0x040c /* UC Turbo Mode */
> +#define CPCAP_REG_TOD1 0x0410 /* Time of Day 1 */
> +#define CPCAP_REG_TOD2 0x0414 /* Time of Day 2 */
> +#define CPCAP_REG_TODA1 0x0418 /* Time of Day Alarm 1 */
> +#define CPCAP_REG_TODA2 0x041c /* Time of Day Alarm 2 */
> +#define CPCAP_REG_DAY 0x0420 /* Day */
> +#define CPCAP_REG_DAYA 0x0424 /* Day Alarm */
> +#define CPCAP_REG_VAL1 0x0428 /* Validity 1 */
> +#define CPCAP_REG_VAL2 0x042c /* Validity 2 */
> +
> +#define CPCAP_REG_SDVSPLL 0x0600 /* Switcher DVS and PLL */
> +#define CPCAP_REG_SI2CC1 0x0604 /* Switcher I2C Control 1 */
> +#define CPCAP_REG_Si2CC2 0x0608 /* Switcher I2C Control 2 */
> +#define CPCAP_REG_S1C1 0x060c /* Switcher 1 Control 1 */
> +#define CPCAP_REG_S1C2 0x0610 /* Switcher 1 Control 2 */
> +#define CPCAP_REG_S2C1 0x0614 /* Switcher 2 Control 1 */
> +#define CPCAP_REG_S2C2 0x0618 /* Switcher 2 Control 2 */
> +#define CPCAP_REG_S3C 0x061c /* Switcher 3 Control */
> +#define CPCAP_REG_S4C1 0x0620 /* Switcher 4 Control 1 */
> +#define CPCAP_REG_S4C2 0x0624 /* Switcher 4 Control 2 */
> +#define CPCAP_REG_S5C 0x0628 /* Switcher 5 Control */
> +#define CPCAP_REG_S6C 0x062c /* Switcher 6 Control */
> +#define CPCAP_REG_VCAMC 0x0630 /* VCAM Control */
> +#define CPCAP_REG_VCSIC 0x0634 /* VCSI Control */
> +#define CPCAP_REG_VDACC 0x0638 /* VDAC Control */
> +#define CPCAP_REG_VDIGC 0x063c /* VDIG Control */
> +#define CPCAP_REG_VFUSEC 0x0640 /* VFUSE Control */
> +#define CPCAP_REG_VHVIOC 0x0644 /* VHVIO Control */
> +#define CPCAP_REG_VSDIOC 0x0648 /* VSDIO Control */
> +#define CPCAP_REG_VPLLC 0x064c /* VPLL Control */
> +#define CPCAP_REG_VRF1C 0x0650 /* VRF1 Control */
> +#define CPCAP_REG_VRF2C 0x0654 /* VRF2 Control */
> +#define CPCAP_REG_VRFREFC 0x0658 /* VRFREF Control */
> +#define CPCAP_REG_VWLAN1C 0x065c /* VWLAN1 Control */
> +#define CPCAP_REG_VWLAN2C 0x0660 /* VWLAN2 Control */
> +#define CPCAP_REG_VSIMC 0x0664 /* VSIM Control */
> +#define CPCAP_REG_VVIBC 0x0668 /* VVIB Control */
> +#define CPCAP_REG_VUSBC 0x066c /* VUSB Control */
> +#define CPCAP_REG_VUSBINT1C 0x0670 /* VUSBINT1 Control */
> +#define CPCAP_REG_VUSBINT2C 0x0674 /* VUSBINT2 Control */
> +#define CPCAP_REG_URT 0x0678 /* Useroff Regulator Trigger */
> +#define CPCAP_REG_URM1 0x067c /* Useroff Regulator Mask 1 */
> +#define CPCAP_REG_URM2 0x0680 /* Useroff Regulator Mask 2 */
> +
> +#define CPCAP_REG_VAUDIOC 0x0800 /* VAUDIO Control */
> +#define CPCAP_REG_CC 0x0804 /* Codec Control */
> +#define CPCAP_REG_CDI 0x0808 /* Codec Digital Interface */
> +#define CPCAP_REG_SDAC 0x080c /* Stereo DAC */
> +#define CPCAP_REG_SDACDI 0x0810 /* Stereo DAC Digital Interface */
> +#define CPCAP_REG_TXI 0x0814 /* TX Inputs */
> +#define CPCAP_REG_TXMP 0x0818 /* TX MIC PGA's */
> +#define CPCAP_REG_RXOA 0x081c /* RX Output Amplifiers */
> +#define CPCAP_REG_RXVC 0x0820 /* RX Volume Control */
> +#define CPCAP_REG_RXCOA 0x0824 /* RX Codec to Output Amps */
> +#define CPCAP_REG_RXSDOA 0x0828 /* RX Stereo DAC to Output Amps */
> +#define CPCAP_REG_RXEPOA 0x082c /* RX External PGA to Output Amps */
> +#define CPCAP_REG_RXLL 0x0830 /* RX Low Latency */
> +#define CPCAP_REG_A2LA 0x0834 /* A2 Loudspeaker Amplifier */
> +#define CPCAP_REG_MIPIS1 0x0838 /* MIPI Slimbus 1 */
> +#define CPCAP_REG_MIPIS2 0x083c /* MIPI Slimbus 2 */
> +#define CPCAP_REG_MIPIS3 0x0840 /* MIPI Slimbus 3. */
> +#define CPCAP_REG_LVAB 0x0844 /* LMR Volume and A4 Balanced. */
> +
> +#define CPCAP_REG_CCC1 0x0a00 /* Coulomb Counter Control 1 */
> +#define CPCAP_REG_CRM 0x0a04 /* Charger and Reverse Mode */
> +#define CPCAP_REG_CCCC2 0x0a08 /* Coincell and Coulomb Ctr Ctrl 2 */
> +#define CPCAP_REG_CCS1 0x0a0c /* Coulomb Counter Sample 1 */
> +#define CPCAP_REG_CCS2 0x0a10 /* Coulomb Counter Sample 2 */
> +#define CPCAP_REG_CCA1 0x0a14 /* Coulomb Counter Accumulator 1 */
> +#define CPCAP_REG_CCA2 0x0a18 /* Coulomb Counter Accumulator 2 */
> +#define CPCAP_REG_CCM 0x0a1c /* Coulomb Counter Mode */
> +#define CPCAP_REG_CCO 0x0a20 /* Coulomb Counter Offset */
> +#define CPCAP_REG_CCI 0x0a24 /* Coulomb Counter Integrator */
> +
> +#define CPCAP_REG_ADCC1 0x0c00 /* A/D Converter Configuration 1 */
> +#define CPCAP_REG_ADCC2 0x0c04 /* A/D Converter Configuration 2 */
> +#define CPCAP_REG_ADCD0 0x0c08 /* A/D Converter Data 0 */
> +#define CPCAP_REG_ADCD1 0x0c0c /* A/D Converter Data 1 */
> +#define CPCAP_REG_ADCD2 0x0c10 /* A/D Converter Data 2 */
> +#define CPCAP_REG_ADCD3 0x0c14 /* A/D Converter Data 3 */
> +#define CPCAP_REG_ADCD4 0x0c18 /* A/D Converter Data 4 */
> +#define CPCAP_REG_ADCD5 0x0c1c /* A/D Converter Data 5 */
> +#define CPCAP_REG_ADCD6 0x0c20 /* A/D Converter Data 6 */
> +#define CPCAP_REG_ADCD7 0x0c24 /* A/D Converter Data 7 */
> +#define CPCAP_REG_ADCAL1 0x0c28 /* A/D Converter Calibration 1 */
> +#define CPCAP_REG_ADCAL2 0x0c2c /* A/D Converter Calibration 2 */
> +
> +#define CPCAP_REG_USBC1 0x0e00 /* USB Control 1 */
> +#define CPCAP_REG_USBC2 0x0e04 /* USB Control 2 */
> +#define CPCAP_REG_USBC3 0x0e08 /* USB Control 3 */
> +#define CPCAP_REG_UVIDL 0x0e0c /* ULPI Vendor ID Low */
> +#define CPCAP_REG_UVIDH 0x0e10 /* ULPI Vendor ID High */
> +#define CPCAP_REG_UPIDL 0x0e14 /* ULPI Product ID Low */
> +#define CPCAP_REG_UPIDH 0x0e18 /* ULPI Product ID High */
> +#define CPCAP_REG_UFC1 0x0e1c /* ULPI Function Control 1 */
> +#define CPCAP_REG_UFC2 0x0e20 /* ULPI Function Control 2 */
> +#define CPCAP_REG_UFC3 0x0e24 /* ULPI Function Control 3 */
> +#define CPCAP_REG_UIC1 0x0e28 /* ULPI Interface Control 1 */
> +#define CPCAP_REG_UIC2 0x0e2c /* ULPI Interface Control 2 */
> +#define CPCAP_REG_UIC3 0x0e30 /* ULPI Interface Control 3 */
> +#define CPCAP_REG_USBOTG1 0x0e34 /* USB OTG Control 1 */
> +#define CPCAP_REG_USBOTG2 0x0e38 /* USB OTG Control 2 */
> +#define CPCAP_REG_USBOTG3 0x0e3c /* USB OTG Control 3 */
> +#define CPCAP_REG_UIER1 0x0e40 /* USB Interrupt Enable Rising 1 */
> +#define CPCAP_REG_UIER2 0x0e44 /* USB Interrupt Enable Rising 2 */
> +#define CPCAP_REG_UIER3 0x0e48 /* USB Interrupt Enable Rising 3 */
> +#define CPCAP_REG_UIEF1 0x0e4c /* USB Interrupt Enable Falling 1 */
> +#define CPCAP_REG_UIEF2 0x0e50 /* USB Interrupt Enable Falling 1 */
> +#define CPCAP_REG_UIEF3 0x0e54 /* USB Interrupt Enable Falling 1 */
> +#define CPCAP_REG_UIS 0x0e58 /* USB Interrupt Status */
> +#define CPCAP_REG_UIL 0x0e5c /* USB Interrupt Latch */
> +#define CPCAP_REG_USBD 0x0e60 /* USB Debug */
> +#define CPCAP_REG_SCR1 0x0e64 /* Scratch 1 */
> +#define CPCAP_REG_SCR2 0x0e68 /* Scratch 2 */
> +#define CPCAP_REG_SCR3 0x0e6c /* Scratch 3 */
> +
> +#define CPCAP_REG_VMC 0x0eac /* Video Mux Control */
> +#define CPCAP_REG_OWDC 0x0eb0 /* One Wire Device Control */
> +#define CPCAP_REG_GPIO0 0x0eb4 /* GPIO 0 Control */
> +
> +#define CPCAP_REG_GPIO1 0x0ebc /* GPIO 1 Control */
> +
> +#define CPCAP_REG_GPIO2 0x0ec4 /* GPIO 2 Control */
> +
> +#define CPCAP_REG_GPIO3 0x0ecc /* GPIO 3 Control */
> +
> +#define CPCAP_REG_GPIO4 0x0ed4 /* GPIO 4 Control */
> +
> +#define CPCAP_REG_GPIO5 0x0edc /* GPIO 5 Control */
> +
> +#define CPCAP_REG_GPIO6 0x0ee4 /* GPIO 6 Control */
> +
> +#define CPCAP_REG_MDLC 0x1000 /* Main Display Lighting Control */
> +#define CPCAP_REG_KLC 0x1004 /* Keypad Lighting Control */
> +#define CPCAP_REG_ADLC 0x1008 /* Aux Display Lighting Control */
> +#define CPCAP_REG_REDC 0x100c /* Red Triode Control */
> +#define CPCAP_REG_GREENC 0x1010 /* Green Triode Control */
> +#define CPCAP_REG_BLUEC 0x1014 /* Blue Triode Control */
> +#define CPCAP_REG_CFC 0x1018 /* Camera Flash Control */
> +#define CPCAP_REG_ABC 0x101c /* Adaptive Boost Control */
> +#define CPCAP_REG_BLEDC 0x1020 /* Bluetooth LED Control */
> +#define CPCAP_REG_CLEDC 0x1024 /* Camera Privacy LED Control */
> +
> +#define CPCAP_REG_OW1C 0x1200 /* One Wire 1 Command */
> +#define CPCAP_REG_OW1D 0x1204 /* One Wire 1 Data */
> +#define CPCAP_REG_OW1I 0x1208 /* One Wire 1 Interrupt */
> +#define CPCAP_REG_OW1IE 0x120c /* One Wire 1 Interrupt Enable */
> +
> +#define CPCAP_REG_OW1 0x1214 /* One Wire 1 Control */
> +
> +#define CPCAP_REG_OW2C 0x1220 /* One Wire 2 Command */
> +#define CPCAP_REG_OW2D 0x1224 /* One Wire 2 Data */
> +#define CPCAP_REG_OW2I 0x1228 /* One Wire 2 Interrupt */
> +#define CPCAP_REG_OW2IE 0x122c /* One Wire 2 Interrupt Enable */
> +
> +#define CPCAP_REG_OW2 0x1234 /* One Wire 2 Control */
> +
> +#define CPCAP_REG_OW3C 0x1240 /* One Wire 3 Command */
> +#define CPCAP_REG_OW3D 0x1244 /* One Wire 3 Data */
> +#define CPCAP_REG_OW3I 0x1248 /* One Wire 3 Interrupt */
> +#define CPCAP_REG_OW3IE 0x124c /* One Wire 3 Interrupt Enable */
> +
> +#define CPCAP_REG_OW3 0x1254 /* One Wire 3 Control */
> +#define CPCAP_REG_GCAIC 0x1258 /* GCAI Clock Control */
> +#define CPCAP_REG_GCAIM 0x125c /* GCAI GPIO Mode */
> +#define CPCAP_REG_LGDIR 0x1260 /* LMR GCAI GPIO Direction */
> +#define CPCAP_REG_LGPU 0x1264 /* LMR GCAI GPIO Pull-up */
> +#define CPCAP_REG_LGPIN 0x1268 /* LMR GCAI GPIO Pin */
> +#define CPCAP_REG_LGMASK 0x126c /* LMR GCAI GPIO Mask */
> +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> +
> +#define CPCAP_REG_TEST 0x7c00 /* Test */
> +
> +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> +
> +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
It would be nice to line up the entire file. #OCD
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-21 11:45 ` Lee Jones
@ 2016-11-21 16:34 ` Rob Herring
2016-11-23 15:03 ` Tony Lindgren
2016-11-23 15:26 ` Tony Lindgren
1 sibling, 1 reply; 9+ messages in thread
From: Rob Herring @ 2016-11-21 16:34 UTC (permalink / raw)
To: Lee Jones
Cc: Tony Lindgren, Samuel Ortiz, linux-kernel, linux-omap,
devicetree, Marcel Partap, Mark Rutland, Michael Scott
On Mon, Nov 21, 2016 at 11:45:58AM +0000, Lee Jones wrote:
> On Fri, 18 Nov 2016, Tony Lindgren wrote:
>
> > Many Motorola phones like droid 4 are using a custom PMIC called CPCAP
> > or 6556002. We can support it's core features quite easily with regmap_spi
> > and regmap_irq.
> >
> > The children of cpcap, such as regulators, ADC and USB, can be just regular
> > device drivers and defined in the dts file. They get probed as we call
> > of_platform_populate() at the end of our probe, and then the children
> > can just call dev_get_regmap(dev.parent, NULL) to get the regmap.
> >
> > Cc: devicetree@vger.kernel.org
> > Cc: Marcel Partap <mpartap@gmx.net>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Michael Scott <michael.scott@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > ---
> > Documentation/devicetree/bindings/mfd/cpcap.txt | 36 ++++
> > drivers/mfd/Kconfig | 8 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/cpcap.c | 255 ++++++++++++++++++++++++
> > include/linux/mfd/cpcap.h | 238 ++++++++++++++++++++++
> > 5 files changed, 538 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/cpcap.txt
> > create mode 100644 drivers/mfd/cpcap.c
> > create mode 100644 include/linux/mfd/cpcap.h
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/cpcap.txt b/Documentation/devicetree/bindings/mfd/cpcap.txt
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/cpcap.txt
> > @@ -0,0 +1,36 @@
> > +CPCAP PMIC device tree binding
> > +
> > +Required properties:
> > +- compatible : Motorola device "motorola,cpcap", others "st,6556002"
> > +- reg : Chip select and size
> > +- interrupt-parent : The parent interrupt controller
> > +- interrupts : The interrupt line the device is connected to
> > +- interrupt-controller : Marks the device node as an interrupt controller
> > +- #interrupt-cells : The number of cells to describe an IRQ, should be 2
> > +- #address-cells : Child device offset number of cells, typically 1
> > +- #size-cells : Child device size number of cells, typically 1
> > +- ranges : Child device register range
> > +- spi-max-frequency : Typically set to 3000000
> > +- spi-cs_high : SPI chip select direction
> > +
> > +Example:
> > +
> > +&mcspi1 {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
> > + cpcap: pmic@0 {
> > + compatible = "motorola,cpcap", "st,6556002";
> > + reg = <0 0>; /* cs0, size 0 */
>
> Is this really correct?
No, SPI devices are 1 cell and there shouldn't be a ranges prop.
>
> How can ranges have a size of 0x8000 and this 0?
reg here doesn't affect ranges and address translation.
Perhaps this is trying to make address translation work, but if that
does, it is by chance. Children of pmic addresses in the range of
0-0x8000 would get translated to "cpu address" 0-0x8000 as long as the
DT has empty ranges up to the root. If the parent bus (i.e. SoC bus) has
any base address, then that is going to get added which would not be
good.
>
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <7 IRQ_TYPE_EDGE_RISING>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0 0x8000>;
> > + spi-max-frequency = <3000000>;
> > + spi-cs-high;
> > + };
> > +};
> > +
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-21 16:34 ` Rob Herring
@ 2016-11-23 15:03 ` Tony Lindgren
0 siblings, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2016-11-23 15:03 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott
* Rob Herring <robh@kernel.org> [161121 08:34]:
> On Mon, Nov 21, 2016 at 11:45:58AM +0000, Lee Jones wrote:
> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > +Example:
> > > +
> > > +&mcspi1 {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges;
> > > + cpcap: pmic@0 {
> > > + compatible = "motorola,cpcap", "st,6556002";
> > > + reg = <0 0>; /* cs0, size 0 */
> >
> > Is this really correct?
>
> No, SPI devices are 1 cell and there shouldn't be a ranges prop.
>
> >
> > How can ranges have a size of 0x8000 and this 0?
>
> reg here doesn't affect ranges and address translation.
>
> Perhaps this is trying to make address translation work, but if that
> does, it is by chance. Children of pmic addresses in the range of
> 0-0x8000 would get translated to "cpu address" 0-0x8000 as long as the
> DT has empty ranges up to the root. If the parent bus (i.e. SoC bus) has
> any base address, then that is going to get added which would not be
> good.
Yes I was thinking we can just get the register offset from the
cpcap base from dts for children. So a range from 0-0x8000.
Rob, do you have any better suggestions for doing that?
Anyways, the children can just rely on regmap too, so I'll just
drop the ranges here. If we ever come up with suitable ranges for
cases like this we can change it later.
Regards,
Tony
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-21 11:45 ` Lee Jones
2016-11-21 16:34 ` Rob Herring
@ 2016-11-23 15:26 ` Tony Lindgren
2016-11-24 8:59 ` Lee Jones
1 sibling, 1 reply; 9+ messages in thread
From: Tony Lindgren @ 2016-11-23 15:26 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott, Rob Herring
* Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> >
> > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
>
> Who is the manufacturer?
Hmm that I don't know. There seems to be both ST and TI versions
of this chip manufactured for Motorola. So my guess is that it
should be Motorola unless there's some similar catalog part
available from ST used by others. If anybody has more info
on this please let me know :)
> > + cpcap->vendor = (val >> 6) & 0x0007;
> > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
>
> Lots of magic numbers here. I suggest you define them.
I'll check if some earlier code has these defined. Otherwise I'll
just add a comment on the lack of available documentation.
> > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
>
> 'ret' is more traditional.
FYI error seems to be preferred over ret as it's meaning is
clear, git grep "error =" drivers/input for example.
I can of course change it if you prefer ret over error.
> > + error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
> > + if (error)
> > + return error;
>
> I don't think I've seen this method of adding bulk IRQ chips before.
> Isn't there a cleaner or generic way to do this?
I'll check.
...
> > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > +
> > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > +
> > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > +
> > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
>
> It would be nice to line up the entire file. #OCD
Hmm care to clarify what you mean here? I think it's lined up with
tabs to line up. I left empty lines where the registers are not
contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
header files maybe? :)
Anywys thanks for the review, the rest of the comments I will just
fix and repost.
Regards,
Tony
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-23 15:26 ` Tony Lindgren
@ 2016-11-24 8:59 ` Lee Jones
2016-11-24 14:43 ` Tony Lindgren
2016-11-29 15:20 ` Rob Herring
0 siblings, 2 replies; 9+ messages in thread
From: Lee Jones @ 2016-11-24 8:59 UTC (permalink / raw)
To: Tony Lindgren
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott, Rob Herring
On Wed, 23 Nov 2016, Tony Lindgren wrote:
> * Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > --- a/drivers/mfd/Makefile
> > > +++ b/drivers/mfd/Makefile
> > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> > >
> > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
> >
> > Who is the manufacturer?
>
> Hmm that I don't know. There seems to be both ST and TI versions
> of this chip manufactured for Motorola. So my guess is that it
> should be Motorola unless there's some similar catalog part
> available from ST used by others. If anybody has more info
> on this please let me know :)
If this IP is shared amongst vendors, it usually means it was designed
by someone else? Synopsis perhaps?
> > > + cpcap->vendor = (val >> 6) & 0x0007;
> > > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
> >
> > Lots of magic numbers here. I suggest you define them.
>
> I'll check if some earlier code has these defined. Otherwise I'll
> just add a comment on the lack of available documentation.
*sad face*
Does that mean you don't even know what they're for?
> > > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
> >
> > 'ret' is more traditional.
>
> FYI error seems to be preferred over ret as it's meaning is
> clear, git grep "error =" drivers/input for example.
> I can of course change it if you prefer ret over error.
I'd prefer to stick to the conventions of *this* subsystem.
... and the most common convention used kernel wide:
$ git grep "ret =" | wc -l
117976
$ git grep "err =" | wc -l
56708
$ git grep "error =" | wc -l
14427
> > > + error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
> > > + if (error)
> > > + return error;
> >
> > I don't think I've seen this method of adding bulk IRQ chips before.
> > Isn't there a cleaner or generic way to do this?
>
> I'll check.
>
> ...
> > > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > > +
> > > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > > +
> > > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > > +
> > > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
> >
> > It would be nice to line up the entire file. #OCD
>
> Hmm care to clarify what you mean here? I think it's lined up with
I'm missing context now you've <snip>ed.
These look straight, however is the whole file lined up (as much as
*practically* possible)?
> tabs to line up. I left empty lines where the registers are not
> contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
> header files maybe? :)
Yes, that's what it means.
/me likes straight lines. :)
> Anywys thanks for the review, the rest of the comments I will just
> fix and repost.
Welcome.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-24 8:59 ` Lee Jones
@ 2016-11-24 14:43 ` Tony Lindgren
2016-11-29 15:20 ` Rob Herring
1 sibling, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2016-11-24 14:43 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott, Rob Herring
* Lee Jones <lee.jones@linaro.org> [161124 00:56]:
> On Wed, 23 Nov 2016, Tony Lindgren wrote:
>
> > * Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> > > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > > --- a/drivers/mfd/Makefile
> > > > +++ b/drivers/mfd/Makefile
> > > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > > > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> > > >
> > > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
> > >
> > > Who is the manufacturer?
> >
> > Hmm that I don't know. There seems to be both ST and TI versions
> > of this chip manufactured for Motorola. So my guess is that it
> > should be Motorola unless there's some similar catalog part
> > available from ST used by others. If anybody has more info
> > on this please let me know :)
>
> If this IP is shared amongst vendors, it usually means it was designed
> by someone else? Synopsis perhaps?
After searching around, many specs say "ST Ericsson CPCAP". So let's
assume the manufacturer should be ste.
> > > > + cpcap->vendor = (val >> 6) & 0x0007;
> > > > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
> > >
> > > Lots of magic numbers here. I suggest you define them.
> >
> > I'll check if some earlier code has these defined. Otherwise I'll
> > just add a comment on the lack of available documentation.
>
> *sad face*
>
> Does that mean you don't even know what they're for?
Luckily the Motorola driver folks documented all the registers.
Unfortunately the register bits just have names. I need to
check if we have names for these bits.
> > > > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
> > >
> > > 'ret' is more traditional.
> >
> > FYI error seems to be preferred over ret as it's meaning is
> > clear, git grep "error =" drivers/input for example.
> > I can of course change it if you prefer ret over error.
>
> I'd prefer to stick to the conventions of *this* subsystem.
>
> ... and the most common convention used kernel wide:
>
> $ git grep "ret =" | wc -l
> 117976
> $ git grep "err =" | wc -l
> 56708
> $ git grep "error =" | wc -l
> 14427
OK sure will rename.
> > > > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > > > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > > > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > > > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > > > +
> > > > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > > > +
> > > > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > > > +
> > > > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
> > >
> > > It would be nice to line up the entire file. #OCD
> >
> > Hmm care to clarify what you mean here? I think it's lined up with
>
> I'm missing context now you've <snip>ed.
>
> These look straight, however is the whole file lined up (as much as
> *practically* possible)?
Yeah it should be, I'll check.
> > tabs to line up. I left empty lines where the registers are not
> > contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
> > header files maybe? :)
>
> Yes, that's what it means.
>
> /me likes straight lines. :)
Sure nothing wrong with that ;)
Regards,
Tony
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-24 8:59 ` Lee Jones
2016-11-24 14:43 ` Tony Lindgren
@ 2016-11-29 15:20 ` Rob Herring
2016-11-29 15:48 ` Tony Lindgren
1 sibling, 1 reply; 9+ messages in thread
From: Rob Herring @ 2016-11-29 15:20 UTC (permalink / raw)
To: Lee Jones
Cc: Tony Lindgren, Samuel Ortiz, linux-kernel, linux-omap,
devicetree, Marcel Partap, Mark Rutland, Michael Scott
On Thu, Nov 24, 2016 at 2:59 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 23 Nov 2016, Tony Lindgren wrote:
>
>> * Lee Jones <lee.jones@linaro.org> [161121 03:43]:
>> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
>> > > --- a/drivers/mfd/Makefile
>> > > +++ b/drivers/mfd/Makefile
>> > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
>> > > obj-$(CONFIG_MFD_CORE) += mfd-core.o
>> > >
>> > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
>> > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
>> >
>> > Who is the manufacturer?
>>
>> Hmm that I don't know. There seems to be both ST and TI versions
>> of this chip manufactured for Motorola. So my guess is that it
>> should be Motorola unless there's some similar catalog part
>> available from ST used by others. If anybody has more info
>> on this please let me know :)
>
> If this IP is shared amongst vendors, it usually means it was designed
> by someone else? Synopsis perhaps?
xCAP names originated from Motorola cellular group with parts (going
back to analog/2G days) coming from Motorola Semi, TI, and ST it
seems. All individually developed AFAIK.
Rob
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mfd: cpcap: Add minimal support
2016-11-29 15:20 ` Rob Herring
@ 2016-11-29 15:48 ` Tony Lindgren
0 siblings, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2016-11-29 15:48 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Samuel Ortiz, linux-kernel, linux-omap, devicetree,
Marcel Partap, Mark Rutland, Michael Scott
* Rob Herring <robh+dt@kernel.org> [161129 07:20]:
> On Thu, Nov 24, 2016 at 2:59 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Wed, 23 Nov 2016, Tony Lindgren wrote:
> >
> >> * Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> >> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> >> > > --- a/drivers/mfd/Makefile
> >> > > +++ b/drivers/mfd/Makefile
> >> > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> >> > > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> >> > >
> >> > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> >> > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
> >> >
> >> > Who is the manufacturer?
> >>
> >> Hmm that I don't know. There seems to be both ST and TI versions
> >> of this chip manufactured for Motorola. So my guess is that it
> >> should be Motorola unless there's some similar catalog part
> >> available from ST used by others. If anybody has more info
> >> on this please let me know :)
> >
> > If this IP is shared amongst vendors, it usually means it was designed
> > by someone else? Synopsis perhaps?
>
> xCAP names originated from Motorola cellular group with parts (going
> back to analog/2G days) coming from Motorola Semi, TI, and ST it
> seems. All individually developed AFAIK.
OK thanks. Looking at the Motorola Linux kernel source, the child
device drivers do test for both revision and vendor and apply
different workarounds based on that. It also seems that CPCAP is
only used on Motorola devices.
So based on the above, we should call it motorola-cpcap with just
a secondary device tree compatible string for the STE part numbers.
Regards,
Tony
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-11-29 15:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-19 1:27 [PATCH] mfd: cpcap: Add minimal support Tony Lindgren
2016-11-21 11:45 ` Lee Jones
2016-11-21 16:34 ` Rob Herring
2016-11-23 15:03 ` Tony Lindgren
2016-11-23 15:26 ` Tony Lindgren
2016-11-24 8:59 ` Lee Jones
2016-11-24 14:43 ` Tony Lindgren
2016-11-29 15:20 ` Rob Herring
2016-11-29 15:48 ` Tony Lindgren
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).