Re: [alsa-devel] [PATCH 02/16] mfd: madera: Add common support for Cirrus Logic Madera codecs

24 Apr 2017

On Wed, 19 Apr 2017, Richard Fitzgerald wrote:
...
On Wed, 2017-04-12 at 13:54 +0100, Lee Jones wrote:
...
On Wed, 05 Apr 2017, Richard Fitzgerald wrote:
...
This adds the generic core support for Cirrus Logic "Madera" class codecs.
These are complex audio codec SoCs with a variety of digital and analogue
I/O, onboard audio processing and DSPs, and other features.
These codecs are all based off a common set of hardware IP so can be
supported by a core of common code (with a few minor device-to-device
variations).
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
Signed-off-by: Nikesh Oswal Nikesh.Oswal@wolfsonmicro.com
Signed-off-by: Richard Fitzgerald rf@opensource.wolfsonmicro.com

Documentation/devicetree/bindings/mfd/madera.txt |  79 +++
 MAINTAINERS                                      |   3 +
 drivers/mfd/Kconfig                              |  23 +
 drivers/mfd/Makefile                             |   4 +
 drivers/mfd/madera-core.c                        | 689 +++++++++++++++++++++++
 drivers/mfd/madera-i2c.c                         | 130 +++++
 drivers/mfd/madera-spi.c                         | 131 +++++
 drivers/mfd/madera.h                             |  52 ++
 include/linux/mfd/madera/core.h                  | 175 ++++++
 include/linux/mfd/madera/pdata.h                 |  88 +++
 10 files changed, 1374 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/madera.txt
 create mode 100644 drivers/mfd/madera-core.c
 create mode 100644 drivers/mfd/madera-i2c.c
 create mode 100644 drivers/mfd/madera-spi.c
 create mode 100644 drivers/mfd/madera.h
 create mode 100644 include/linux/mfd/madera/core.h
 create mode 100644 include/linux/mfd/madera/pdata.h

diff --git a/Documentation/devicetree/bindings/mfd/madera.txt b/Documentation/devicetree/bindings/mfd/madera.txt
new file mode 100644
index 0000000..a6c3260
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/madera.txt
@@ -0,0 +1,79 @@
+Cirrus Logic Madera class audio codecs multi-function device



+These devices are audio SoCs with extensive digital capabilities and a range
+of analogue I/O.



+See also the child driver bindings in:
+bindings/extcon/extcon-madera.txt
+bindings/gpio/gpio-madera.txt
+bindings/interrupt-controller/cirrus,madera.txt
+bindings/pinctrl/cirrus,madera-pinctrl.txt
+bindings/regulator/madera-ldo1.txt
+bindings/regulator/madera-micsupp.txt
+bindings/sound/madera.txt



+Required properties:



compatible : One of the following chip-specific strings:


   "cirrus,cs47l35"


   "cirrus,cs47l85"


   "cirrus,cs47l90"


   "cirrus,cs47l91"


   "cirrus,wm1840"




reg : I2C slave address when connected using I2C, chip select number when


using SPI.


DCVDD-supply : Power supply for the device as defined in


bindings/regulator/regulator.txt
Mandatory on CS47L35, CS47L90, CS47L91
Optional on CS47L85, WM1840


AVDD-supply, DBVDD1-supply, DBVDD2-supply, CPVDD1-supply, CPVDD2-supply :


Power supplies for the device


DBVDD3-supply, DBVDD4-supply : Power supplies for the device


(CS47L85, CS47L90, CS47L91, WM1840)


SPKVDDL-supply, SPKVDDR-supply : Power supplies for the device


(CS47L85, WM1840)


SPKVDD-supply : Power supply for the device


(CS47L35)


+Optional properties:



MICVDD-supply : Power supply, only need to be specified if


powered externally


reset-gpios : One entry specifying the GPIO controlling /RESET.


As defined in bindings/gpio.txt.
Although optional, it is strongly recommended to use a hardware reset


MICBIASx : Initial data for the MICBIAS regulators, as covered in


Documentation/devicetree/bindings/regulator/regulator.txt.
One for each MICBIAS generator (MICBIAS1, MICBIAS2, ...)
(all codecs)

One for each output pin (MICBIAS1A, MIBCIAS1B, MICBIAS2A, ...)
(all except CS47L85, WM1840)

The following following additional property is supported for the generator
nodes:
 - cirrus,ext-cap : Set to 1 if the MICBIAS has external decoupling


   capacitors attached.




+Example:



+codec: cs47l85@0 {
Node names should be generic.
You can swap these round if you want, so:
cs47l85: codec@0 {
... is valid.
...

compatible = "cirrus,cs47l85";
reg = <0>;

reset-gpios = <&gpio 0>;

MICBIAS1 {
regulator-min-microvolt = <900000>;


regulator-max-microvolt = <3300000>;


cirrus,ext-cap = <1>;


};

+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 02995c9..d28e53f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3266,7 +3266,10 @@ L:	patches@opensource.wolfsonmicro.com
 T:	git https://github.com/CirrusLogic/linux-drivers.git
 W:	https://github.com/CirrusLogic/linux-drivers/wiki
 S:	Supported
+F:	Documentation/devicetree/bindings/mfd/madera.txt
 F:	include/linux/mfd/madera/*
+F:	drivers/mfd/madera*
+F:	drivers/mfd/cs47l*
CLEANCACHE API
 M:	Konrad Rzeszutek Wilk konrad.wilk@oracle.com
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ce3a918..f0f9979 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -203,6 +203,29 @@ config MFD_CROS_EC_SPI
     response time cannot be guaranteed, we support ignoring
     'pre-amble' bytes before the response actually starts.
+config MFD_MADERA

bool
select REGMAP
select MFD_CORE


+config MFD_MADERA_I2C

tristate "Cirrus Logic Madera codecs with I2C"
select MFD_MADERA
select REGMAP_I2C
depends on I2C
help
 Support for the Cirrus Logic Madera platform audio SoC


 core functionality controlled via I2C.




+config MFD_MADERA_SPI

tristate "Cirrus Logic Madera codecs with SPI"
select MFD_MADERA
select REGMAP_SPI
depends on SPI_MASTER
help
 Support for the Cirrus Logic Madera platform audio SoC


 core functionality controlled via SPI.




config MFD_ASIC3
   bool "Compaq ASIC3"
   depends on GPIOLIB && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index fa86dbe..c41f6c9 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -72,6 +72,10 @@ obj-$(CONFIG_MFD_WM8350_I2C)	+= wm8350-i2c.o
 wm8994-objs			:= wm8994-core.o wm8994-irq.o wm8994-regmap.o
 obj-$(CONFIG_MFD_WM8994)	+= wm8994.o
+obj-$(CONFIG_MFD_MADERA)	+= madera-core.o
+obj-$(CONFIG_MFD_MADERA_I2C)	+= madera-i2c.o
+obj-$(CONFIG_MFD_MADERA_SPI)	+= madera-spi.o



obj-$(CONFIG_TPS6105X)		+= tps6105x.o
 obj-$(CONFIG_TPS65010)		+= tps65010.o
 obj-$(CONFIG_TPS6507X)		+= tps6507x.o
diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
new file mode 100644
index 0000000..ab5fe9b
--- /dev/null
+++ b/drivers/mfd/madera-core.c
@@ -0,0 +1,689 @@
+/*


Core MFD support for Cirrus Logic Madera codecs







Copyright 2015-2017 Cirrus Logic







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/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>



+#include <linux/mfd/madera/core.h>
+#include <linux/mfd/madera/registers.h>



+#include "madera.h"



+#define CS47L35_SILICON_ID	0x6360
+#define CS47L85_SILICON_ID	0x6338
+#define CS47L90_SILICON_ID	0x6364



+#define MADERA_32KZ_MCLK2	1



+static const char * const madera_core_supplies[] = {

"AVDD",
"DBVDD1",

+};



+static const struct mfd_cell madera_ldo1_devs[] = {

{ .name = "madera-ldo1", .of_compatible = "cirrus,madera-ldo1" },

+};



+static const struct mfd_cell cs47l35_devs[] = {

{ .name = "madera-pinctrl", .of_compatible = "cirrus,madera-pinctrl" },
{ .name = "madera-irq", },

I believe this should be "interrupt-controller".
I don't think that's the case. I checked other irchip drivers and they
have no particular standard naming. At least one is called
"something-irq", none are called "something-interrupt-controller"
...
irq is ambiguous.
I can't really see what other driver this could be confused with,
especially as it's specified as madera-* so the alternative drivers are
limited. Given the limited set of drivers I think it's clear enough it's
the driver for the IRQ and a longer name doesn't add information.
...
Same goes for the ones below.
Ditto. madera-micsupp will be replaced by the existing arizona-micsupp.
Most gpio drivers are called "something-gpio". Extcon is the name of a
driver subsystem so should be sufficiently clear. Likewise madera-codec.
I meant just the "-irq" entries.
... but I'm not 100% convinced that calling it "-irq" a terrible idea,
just that "irq-controller" or "interrupt-controller" would be better.
Either way, it's not a blocking point.
...
...
...

{ .name = "madera-micsupp", .of_compatible = "cirrus,madera-micsupp" },
{ .name = "madera-gpio",    .of_compatible = "cirrus,madera-gpio" },
{ .name = "madera-extcon",  .of_compatible = "cirrus,madera-extcon" },
{ .name = "cs47l35-codec",  .of_compatible = "cirrus,cs47l35-codec" },
{ .name = "madera-haptics", .of_compatible = "cirrus,madera-haptics" },

+};



+static const struct mfd_cell cs47l85_devs[] = {

{ .name = "madera-pinctrl", .of_compatible = "cirrus,madera-pinctrl" },
{ .name = "madera-irq", },
{ .name = "madera-micsupp", .of_compatible = "cirrus,madera-micsupp" },
{ .name = "madera-gpio",    .of_compatible = "cirrus,madera-gpio" },
{ .name = "madera-extcon",  .of_compatible = "cirrus,madera-extcon" },
{ .name = "cs47l85-codec",  .of_compatible = "cirrus,cs47l85-codec" },
{ .name = "madera-haptics", .of_compatible = "cirrus,madera-haptics" },

+};



+static const struct mfd_cell cs47l90_devs[] = {

{ .name = "madera-pinctrl", .of_compatible = "cirrus,madera-pinctrl" },
{ .name = "madera-irq", },
{ .name = "madera-micsupp", .of_compatible = "cirrus,madera-micsupp" },
{ .name = "madera-gpio",    .of_compatible = "cirrus,madera-gpio" },
{ .name = "madera-extcon",  .of_compatible = "cirrus,madera-extcon" },
{ .name = "cs47l90-codec",  .of_compatible = "cirrus,cs47l90-codec" },
{ .name = "madera-haptics", .of_compatible = "cirrus,madera-haptics" },

+};



+const char *madera_name_from_type(enum madera_type type)
+{

switch (type) {
case CS47L35:
return "CS47L35";


case CS47L85:
return "CS47L85";


case CS47L90:
return "CS47L90";


case CS47L91:
return "CS47L91";


case WM1840:
return "WM1840";


default:
return "Unknown";


}

+}
+EXPORT_SYMBOL_GPL(madera_name_from_type);



+#define MADERA_BOOT_POLL_MAX_INTERVAL_US  5000
+#define MADERA_BOOT_POLL_TIMEOUT_US	 25000



+static int madera_wait_for_boot(struct madera *madera)
+{

unsigned int val;
int ret;

/*
* We can't use an interrupt as we need to runtime resume to do so,


* so we poll the status bit. This won't race with the interrupt


* handler because it will be blocked on runtime resume.


*/


ret = regmap_read_poll_timeout(madera->regmap,
		       MADERA_IRQ1_RAW_STATUS_1,


		       val,


		       (val & MADERA_BOOT_DONE_STS1),


		       MADERA_BOOT_POLL_MAX_INTERVAL_US,


		       MADERA_BOOT_POLL_TIMEOUT_US);


/*
* BOOT_DONE defaults to unmasked on boot so we must ack it.


* Do this unconditionally to avoid interrupt storms


*/


regmap_write(madera->regmap, MADERA_IRQ1_STATUS_1,
     MADERA_BOOT_DONE_EINT1);



if (ret)
dev_err(madera->dev, "Polling BOOT_DONE_STS failed: %d\n", ret);



Why isn't this under the call to regmap_read_poll_timeout()?
It was intended to make it clear that we must ack the BOOT_DONE now no
matter what and to avoid the potential with them the other way around of
someone adding more code in the if (or just a ret) and so accidentally
failing to do the ack. I could swap them but I think I prefer keeping
them this way and changing the comment to say this.
Okay.
I'm going to assume that we are in agreement for all points that you
did not answer (which is good).  I look forward to the next version.
-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

    