Re: [PATCH v2 2/5] mfd: lochnagar: Add support for the Cirrus Logic Lochnagar

From: Lee Jones
Date: Thu Oct 25 2018 - 03:45:08 EST

Next message: Michal Hocko: "Re: [PATCH V3 3/5] mm/hugetlb: Enable arch specific huge page size support for migration"
Previous message: Federico Vaga: "Re: [PATCH 1/3] i2c:ocores: stop transfer on timeout"
Next in thread: Charles Keepax: "Re: [PATCH v2 2/5] mfd: lochnagar: Add support for the Cirrus Logic Lochnagar"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, 08 Oct 2018, Charles Keepax wrote:

> From: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
>
> Lochnagar is an evaluation and development board for Cirrus
> Logic Smart CODEC and Amp devices. It allows the connection of
> most Cirrus Logic devices on mini-cards, as well as allowing
> connection of various application processor systems to provide a
> full evaluation platform. This driver supports the board
> controller chip on the Lochnagar board. Audio system topology,
> clocking and power can all be controlled through the Lochnagar
> controller chip, allowing the device under test to be used in
> a variety of possible use cases.
>
> As the Lochnagar is a fairly complex device this MFD driver
> allows the drivers for the various features to be bound
> in. Initially clocking, regulator and pinctrl will be added as
> these are necessary to configure the system. But in time at least
> audio and voltage/current monitoring will also be added.
>
> Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
>
> Changes since v1:
> - Update Kconfig to correctly specify dependencies
> - Update commit message a little
>
> Thanks,
> Charles
>
> MAINTAINERS | 13 +
> drivers/mfd/Kconfig | 8 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/lochnagar-i2c.c | 732 ++++++++++++++++++++++++++++++++++++
> include/linux/mfd/lochnagar.h | 48 +++
> include/linux/mfd/lochnagar1_regs.h | 157 ++++++++
> include/linux/mfd/lochnagar2_regs.h | 253 +++++++++++++
> 7 files changed, 1213 insertions(+)
> create mode 100644 drivers/mfd/lochnagar-i2c.c
> create mode 100644 include/linux/mfd/lochnagar.h
> create mode 100644 include/linux/mfd/lochnagar1_regs.h
> create mode 100644 include/linux/mfd/lochnagar2_regs.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d9e6d86488df4..f1f3494ac5cd8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3557,6 +3557,19 @@ L: netdev@xxxxxxxxxxxxxxx
> S: Maintained
> F: drivers/net/ethernet/cirrus/ep93xx_eth.c
>
> +CIRRUS LOGIC LOCHNAGAR DRIVER
> +M: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> +M: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> +L: patches@xxxxxxxxxxxxxxxxxxxxx
> +S: Supported
> +F: drivers/clk/clk-lochnagar.c
> +F: drivers/mfd/lochnagar-i2c.c
> +F: drivers/pinctrl/cirrus/pinctrl-lochnagar.c
> +F: drivers/regulator/lochnagar-regulator.c
> +F: include/dt-bindings/clk/lochnagar.h
> +F: include/dt-bindings/pinctrl/lochnagar.h
> +F: include/linux/mfd/lochnagar*
> +
> CISCO FCOE HBA DRIVER
> M: Satish Kharat <satishkh@xxxxxxxxx>
> M: Sesidhar Baddela <sebaddel@xxxxxxxxx>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index f3a5f8d027906..dc64151373714 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1676,6 +1676,14 @@ config MFD_VX855
> VIA VX855/VX875 south bridge. You will need to enable the vx855_spi
> and/or vx855_gpio drivers for this to do anything useful.
>
> +config MFD_LOCHNAGAR
> + bool "Cirrus Logic Lochnagar Audio Development Board"
> + select MFD_CORE
> + select REGMAP_I2C
> + depends on I2C=y && OF
> + help
> + Support for Cirrus Logic Lochnagar audio development board.
> +
> config MFD_ARIZONA
> select REGMAP
> select REGMAP_IRQ
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 5856a9489cbd8..f16773cb887ff 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -37,6 +37,8 @@ obj-$(CONFIG_MFD_T7L66XB) += t7l66xb.o tmio_core.o
> obj-$(CONFIG_MFD_TC6387XB) += tc6387xb.o tmio_core.o
> obj-$(CONFIG_MFD_TC6393XB) += tc6393xb.o tmio_core.o
>
> +obj-$(CONFIG_MFD_LOCHNAGAR) += lochnagar-i2c.o
> +
> obj-$(CONFIG_MFD_ARIZONA) += arizona-core.o
> obj-$(CONFIG_MFD_ARIZONA) += arizona-irq.o
> obj-$(CONFIG_MFD_ARIZONA_I2C) += arizona-i2c.o
> diff --git a/drivers/mfd/lochnagar-i2c.c b/drivers/mfd/lochnagar-i2c.c
> new file mode 100644
> index 0000000000000..7ac7af9e3130b
> --- /dev/null
> +++ b/drivers/mfd/lochnagar-i2c.c
> @@ -0,0 +1,732 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lochnagar I2C bus interface
> + *
> + * Copyright (c) 2012-2018 Cirrus Logic, Inc. and
> + * Cirrus Logic International Semiconductor Ltd.
> + *
> + * Author: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/mfd/lochnagar.h>
> +
> +#define LOCHNAGAR_BOOT_RETRIES 10
> +#define LOCHNAGAR_BOOT_DELAY_MS 350
> +
> +#define LOCHNAGAR_CONFIG_POLL_US 10000
> +
> +static bool lochnagar1_readable_register(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case LOCHNAGAR_SOFTWARE_RESET:
> + case LOCHNAGAR_FIRMWARE_ID1:
> + case LOCHNAGAR_FIRMWARE_ID2:
> + case LOCHNAGAR1_CDC_AIF1_SEL:
> + case LOCHNAGAR1_CDC_AIF2_SEL:
> + case LOCHNAGAR1_CDC_AIF3_SEL:
> + case LOCHNAGAR1_CDC_MCLK1_SEL:
> + case LOCHNAGAR1_CDC_MCLK2_SEL:
> + case LOCHNAGAR1_CDC_AIF_CTRL1:
> + case LOCHNAGAR1_CDC_AIF_CTRL2:
> + case LOCHNAGAR1_EXT_AIF_CTRL:
> + case LOCHNAGAR1_DSP_AIF1_SEL:
> + case LOCHNAGAR1_DSP_AIF2_SEL:
> + case LOCHNAGAR1_DSP_CLKIN_SEL:
> + case LOCHNAGAR1_DSP_AIF:
> + case LOCHNAGAR1_GF_AIF1:
> + case LOCHNAGAR1_GF_AIF2:
> + case LOCHNAGAR1_PSIA_AIF:
> + case LOCHNAGAR1_PSIA1_SEL:
> + case LOCHNAGAR1_PSIA2_SEL:
> + case LOCHNAGAR1_SPDIF_AIF_SEL:
> + case LOCHNAGAR1_GF_AIF3_SEL:
> + case LOCHNAGAR1_GF_AIF4_SEL:
> + case LOCHNAGAR1_GF_CLKOUT1_SEL:
> + case LOCHNAGAR1_GF_AIF1_SEL:
> + case LOCHNAGAR1_GF_AIF2_SEL:
> + case LOCHNAGAR1_GF_GPIO2:
> + case LOCHNAGAR1_GF_GPIO3:
> + case LOCHNAGAR1_GF_GPIO7:
> + case LOCHNAGAR1_RST:
> + case LOCHNAGAR1_LED1:
> + case LOCHNAGAR1_LED2:
> + case LOCHNAGAR1_I2C_CTRL:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct reg_default lochnagar1_reg_defaults[] = {
> + { LOCHNAGAR1_CDC_AIF1_SEL, 0x00 },
> + { LOCHNAGAR1_CDC_AIF2_SEL, 0x00 },
> + { LOCHNAGAR1_CDC_AIF3_SEL, 0x00 },
> + { LOCHNAGAR1_CDC_MCLK1_SEL, 0x00 },
> + { LOCHNAGAR1_CDC_MCLK2_SEL, 0x00 },
> + { LOCHNAGAR1_CDC_AIF_CTRL1, 0x00 },
> + { LOCHNAGAR1_CDC_AIF_CTRL2, 0x00 },
> + { LOCHNAGAR1_EXT_AIF_CTRL, 0x00 },
> + { LOCHNAGAR1_DSP_AIF1_SEL, 0x00 },
> + { LOCHNAGAR1_DSP_AIF2_SEL, 0x00 },
> + { LOCHNAGAR1_DSP_CLKIN_SEL, 0x01 },
> + { LOCHNAGAR1_DSP_AIF, 0x08 },
> + { LOCHNAGAR1_GF_AIF1, 0x00 },
> + { LOCHNAGAR1_GF_AIF2, 0x00 },
> + { LOCHNAGAR1_PSIA_AIF, 0x00 },
> + { LOCHNAGAR1_PSIA1_SEL, 0x00 },
> + { LOCHNAGAR1_PSIA2_SEL, 0x00 },
> + { LOCHNAGAR1_SPDIF_AIF_SEL, 0x00 },
> + { LOCHNAGAR1_GF_AIF3_SEL, 0x00 },
> + { LOCHNAGAR1_GF_AIF4_SEL, 0x00 },
> + { LOCHNAGAR1_GF_CLKOUT1_SEL, 0x00 },
> + { LOCHNAGAR1_GF_AIF1_SEL, 0x00 },
> + { LOCHNAGAR1_GF_AIF2_SEL, 0x00 },
> + { LOCHNAGAR1_GF_GPIO2, 0x00 },
> + { LOCHNAGAR1_GF_GPIO3, 0x00 },
> + { LOCHNAGAR1_GF_GPIO7, 0x00 },
> + { LOCHNAGAR1_RST, 0x00 },
> + { LOCHNAGAR1_LED1, 0x00 },
> + { LOCHNAGAR1_LED2, 0x00 },
> + { LOCHNAGAR1_I2C_CTRL, 0x01 },
> +};

Why do you need to specify each register value?

> +static const struct regmap_config lochnagar1_i2c_regmap = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .reg_format_endian = REGMAP_ENDIAN_BIG,
> + .val_format_endian = REGMAP_ENDIAN_BIG,
> +
> + .max_register = 0x50,
> + .readable_reg = lochnagar1_readable_register,
> +
> + .use_single_rw = true,
> +
> + .cache_type = REGCACHE_RBTREE,
> +
> + .reg_defaults = lochnagar1_reg_defaults,
> + .num_reg_defaults = ARRAY_SIZE(lochnagar1_reg_defaults),
> +};
> +
> +static const struct reg_sequence lochnagar1_patch[] = {
> + { 0x40, 0x0083 },
> + { 0x46, 0x0001 },
> + { 0x47, 0x0018 },
> + { 0x50, 0x0000 },
> +};

I'm really not a fan of these so call 'patches'.

Can't you set the registers up proper way?

> +static struct mfd_cell lochnagar1_devs[] = {
> + {
> + .name = "lochnagar-pinctrl"
> + },
> + {
> + .name = "lochnagar-clk"
> + },

This should each be on a single line.

> +};
> +
> +static bool lochnagar2_readable_register(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case LOCHNAGAR_SOFTWARE_RESET:
> + case LOCHNAGAR_FIRMWARE_ID1:
> + case LOCHNAGAR_FIRMWARE_ID2:
> + case LOCHNAGAR2_CDC_AIF1_CTRL:
> + case LOCHNAGAR2_CDC_AIF2_CTRL:
> + case LOCHNAGAR2_CDC_AIF3_CTRL:
> + case LOCHNAGAR2_DSP_AIF1_CTRL:
> + case LOCHNAGAR2_DSP_AIF2_CTRL:
> + case LOCHNAGAR2_PSIA1_CTRL:
> + case LOCHNAGAR2_PSIA2_CTRL:
> + case LOCHNAGAR2_GF_AIF3_CTRL:
> + case LOCHNAGAR2_GF_AIF4_CTRL:
> + case LOCHNAGAR2_GF_AIF1_CTRL:
> + case LOCHNAGAR2_GF_AIF2_CTRL:
> + case LOCHNAGAR2_SPDIF_AIF_CTRL:
> + case LOCHNAGAR2_USB_AIF1_CTRL:
> + case LOCHNAGAR2_USB_AIF2_CTRL:
> + case LOCHNAGAR2_ADAT_AIF_CTRL:
> + case LOCHNAGAR2_CDC_MCLK1_CTRL:
> + case LOCHNAGAR2_CDC_MCLK2_CTRL:
> + case LOCHNAGAR2_DSP_CLKIN_CTRL:
> + case LOCHNAGAR2_PSIA1_MCLK_CTRL:
> + case LOCHNAGAR2_PSIA2_MCLK_CTRL:
> + case LOCHNAGAR2_SPDIF_MCLK_CTRL:
> + case LOCHNAGAR2_GF_CLKOUT1_CTRL:
> + case LOCHNAGAR2_GF_CLKOUT2_CTRL:
> + case LOCHNAGAR2_ADAT_MCLK_CTRL:
> + case LOCHNAGAR2_SOUNDCARD_MCLK_CTRL:
> + case LOCHNAGAR2_GPIO_FPGA_GPIO1:
> + case LOCHNAGAR2_GPIO_FPGA_GPIO2:
> + case LOCHNAGAR2_GPIO_FPGA_GPIO3:
> + case LOCHNAGAR2_GPIO_FPGA_GPIO4:
> + case LOCHNAGAR2_GPIO_FPGA_GPIO5:
> + case LOCHNAGAR2_GPIO_FPGA_GPIO6:
> + case LOCHNAGAR2_GPIO_CDC_GPIO1:
> + case LOCHNAGAR2_GPIO_CDC_GPIO2:
> + case LOCHNAGAR2_GPIO_CDC_GPIO3:
> + case LOCHNAGAR2_GPIO_CDC_GPIO4:
> + case LOCHNAGAR2_GPIO_CDC_GPIO5:
> + case LOCHNAGAR2_GPIO_CDC_GPIO6:
> + case LOCHNAGAR2_GPIO_CDC_GPIO7:
> + case LOCHNAGAR2_GPIO_CDC_GPIO8:
> + case LOCHNAGAR2_GPIO_DSP_GPIO1:
> + case LOCHNAGAR2_GPIO_DSP_GPIO2:
> + case LOCHNAGAR2_GPIO_DSP_GPIO3:
> + case LOCHNAGAR2_GPIO_DSP_GPIO4:
> + case LOCHNAGAR2_GPIO_DSP_GPIO5:
> + case LOCHNAGAR2_GPIO_DSP_GPIO6:
> + case LOCHNAGAR2_GPIO_GF_GPIO2:
> + case LOCHNAGAR2_GPIO_GF_GPIO3:
> + case LOCHNAGAR2_GPIO_GF_GPIO7:
> + case LOCHNAGAR2_GPIO_CDC_AIF1_BCLK:
> + case LOCHNAGAR2_GPIO_CDC_AIF1_RXDAT:
> + case LOCHNAGAR2_GPIO_CDC_AIF1_LRCLK:
> + case LOCHNAGAR2_GPIO_CDC_AIF1_TXDAT:
> + case LOCHNAGAR2_GPIO_CDC_AIF2_BCLK:
> + case LOCHNAGAR2_GPIO_CDC_AIF2_RXDAT:
> + case LOCHNAGAR2_GPIO_CDC_AIF2_LRCLK:
> + case LOCHNAGAR2_GPIO_CDC_AIF2_TXDAT:
> + case LOCHNAGAR2_GPIO_CDC_AIF3_BCLK:
> + case LOCHNAGAR2_GPIO_CDC_AIF3_RXDAT:
> + case LOCHNAGAR2_GPIO_CDC_AIF3_LRCLK:
> + case LOCHNAGAR2_GPIO_CDC_AIF3_TXDAT:
> + case LOCHNAGAR2_GPIO_DSP_AIF1_BCLK:
> + case LOCHNAGAR2_GPIO_DSP_AIF1_RXDAT:
> + case LOCHNAGAR2_GPIO_DSP_AIF1_LRCLK:
> + case LOCHNAGAR2_GPIO_DSP_AIF1_TXDAT:
> + case LOCHNAGAR2_GPIO_DSP_AIF2_BCLK:
> + case LOCHNAGAR2_GPIO_DSP_AIF2_RXDAT:
> + case LOCHNAGAR2_GPIO_DSP_AIF2_LRCLK:
> + case LOCHNAGAR2_GPIO_DSP_AIF2_TXDAT:
> + case LOCHNAGAR2_GPIO_PSIA1_BCLK:
> + case LOCHNAGAR2_GPIO_PSIA1_RXDAT:
> + case LOCHNAGAR2_GPIO_PSIA1_LRCLK:
> + case LOCHNAGAR2_GPIO_PSIA1_TXDAT:
> + case LOCHNAGAR2_GPIO_PSIA2_BCLK:
> + case LOCHNAGAR2_GPIO_PSIA2_RXDAT:
> + case LOCHNAGAR2_GPIO_PSIA2_LRCLK:
> + case LOCHNAGAR2_GPIO_PSIA2_TXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF3_BCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF3_RXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF3_LRCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF3_TXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF4_BCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF4_RXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF4_LRCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF4_TXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF1_BCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF1_RXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF1_LRCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF1_TXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF2_BCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF2_RXDAT:
> + case LOCHNAGAR2_GPIO_GF_AIF2_LRCLK:
> + case LOCHNAGAR2_GPIO_GF_AIF2_TXDAT:
> + case LOCHNAGAR2_GPIO_DSP_UART1_RX:
> + case LOCHNAGAR2_GPIO_DSP_UART1_TX:
> + case LOCHNAGAR2_GPIO_DSP_UART2_RX:
> + case LOCHNAGAR2_GPIO_DSP_UART2_TX:
> + case LOCHNAGAR2_GPIO_GF_UART2_RX:
> + case LOCHNAGAR2_GPIO_GF_UART2_TX:
> + case LOCHNAGAR2_GPIO_USB_UART_RX:
> + case LOCHNAGAR2_GPIO_CDC_PDMCLK1:
> + case LOCHNAGAR2_GPIO_CDC_PDMDAT1:
> + case LOCHNAGAR2_GPIO_CDC_PDMCLK2:
> + case LOCHNAGAR2_GPIO_CDC_PDMDAT2:
> + case LOCHNAGAR2_GPIO_CDC_DMICCLK1:
> + case LOCHNAGAR2_GPIO_CDC_DMICDAT1:
> + case LOCHNAGAR2_GPIO_CDC_DMICCLK2:
> + case LOCHNAGAR2_GPIO_CDC_DMICDAT2:
> + case LOCHNAGAR2_GPIO_CDC_DMICCLK3:
> + case LOCHNAGAR2_GPIO_CDC_DMICDAT3:
> + case LOCHNAGAR2_GPIO_CDC_DMICCLK4:
> + case LOCHNAGAR2_GPIO_CDC_DMICDAT4:
> + case LOCHNAGAR2_GPIO_DSP_DMICCLK1:
> + case LOCHNAGAR2_GPIO_DSP_DMICDAT1:
> + case LOCHNAGAR2_GPIO_DSP_DMICCLK2:
> + case LOCHNAGAR2_GPIO_DSP_DMICDAT2:
> + case LOCHNAGAR2_GPIO_I2C2_SCL:
> + case LOCHNAGAR2_GPIO_I2C2_SDA:
> + case LOCHNAGAR2_GPIO_I2C3_SCL:
> + case LOCHNAGAR2_GPIO_I2C3_SDA:
> + case LOCHNAGAR2_GPIO_I2C4_SCL:
> + case LOCHNAGAR2_GPIO_I2C4_SDA:
> + case LOCHNAGAR2_GPIO_DSP_STANDBY:
> + case LOCHNAGAR2_GPIO_CDC_MCLK1:
> + case LOCHNAGAR2_GPIO_CDC_MCLK2:
> + case LOCHNAGAR2_GPIO_DSP_CLKIN:
> + case LOCHNAGAR2_GPIO_PSIA1_MCLK:
> + case LOCHNAGAR2_GPIO_PSIA2_MCLK:
> + case LOCHNAGAR2_GPIO_GF_GPIO1:
> + case LOCHNAGAR2_GPIO_GF_GPIO5:
> + case LOCHNAGAR2_GPIO_DSP_GPIO20:
> + case LOCHNAGAR2_GPIO_CHANNEL1:
> + case LOCHNAGAR2_GPIO_CHANNEL2:
> + case LOCHNAGAR2_GPIO_CHANNEL3:
> + case LOCHNAGAR2_GPIO_CHANNEL4:
> + case LOCHNAGAR2_GPIO_CHANNEL5:
> + case LOCHNAGAR2_GPIO_CHANNEL6:
> + case LOCHNAGAR2_GPIO_CHANNEL7:
> + case LOCHNAGAR2_GPIO_CHANNEL8:
> + case LOCHNAGAR2_GPIO_CHANNEL9:
> + case LOCHNAGAR2_GPIO_CHANNEL10:
> + case LOCHNAGAR2_GPIO_CHANNEL11:
> + case LOCHNAGAR2_GPIO_CHANNEL12:
> + case LOCHNAGAR2_GPIO_CHANNEL13:
> + case LOCHNAGAR2_GPIO_CHANNEL14:
> + case LOCHNAGAR2_GPIO_CHANNEL15:
> + case LOCHNAGAR2_GPIO_CHANNEL16:
> + case LOCHNAGAR2_MINICARD_RESETS:
> + case LOCHNAGAR2_ANALOGUE_PATH_CTRL1:
> + case LOCHNAGAR2_ANALOGUE_PATH_CTRL2:
> + case LOCHNAGAR2_COMMS_CTRL4:
> + case LOCHNAGAR2_SPDIF_CTRL:
> + case LOCHNAGAR2_POWER_CTRL:
> + case LOCHNAGAR2_MICVDD_CTRL1:
> + case LOCHNAGAR2_MICVDD_CTRL2:
> + case LOCHNAGAR2_VDDCORE_CDC_CTRL1:
> + case LOCHNAGAR2_VDDCORE_CDC_CTRL2:
> + case LOCHNAGAR2_SOUNDCARD_AIF_CTRL:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool lochnagar2_volatile_register(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case LOCHNAGAR2_GPIO_CHANNEL1:
> + case LOCHNAGAR2_GPIO_CHANNEL2:
> + case LOCHNAGAR2_GPIO_CHANNEL3:
> + case LOCHNAGAR2_GPIO_CHANNEL4:
> + case LOCHNAGAR2_GPIO_CHANNEL5:
> + case LOCHNAGAR2_GPIO_CHANNEL6:
> + case LOCHNAGAR2_GPIO_CHANNEL7:
> + case LOCHNAGAR2_GPIO_CHANNEL8:
> + case LOCHNAGAR2_GPIO_CHANNEL9:
> + case LOCHNAGAR2_GPIO_CHANNEL10:
> + case LOCHNAGAR2_GPIO_CHANNEL11:
> + case LOCHNAGAR2_GPIO_CHANNEL12:
> + case LOCHNAGAR2_GPIO_CHANNEL13:
> + case LOCHNAGAR2_GPIO_CHANNEL14:
> + case LOCHNAGAR2_GPIO_CHANNEL15:
> + case LOCHNAGAR2_GPIO_CHANNEL16:
> + case LOCHNAGAR2_ANALOGUE_PATH_CTRL1:
> + return true;
> + default:
> + return false;
> + }
> +}

This is getting silly now. Can't you use ranges?

> +static const struct reg_default lochnagar2_reg_defaults[] = {
> + { LOCHNAGAR2_CDC_AIF1_CTRL, 0x0000 },
> + { LOCHNAGAR2_CDC_AIF2_CTRL, 0x0000 },
> + { LOCHNAGAR2_CDC_AIF3_CTRL, 0x0000 },
> + { LOCHNAGAR2_DSP_AIF1_CTRL, 0x0000 },
> + { LOCHNAGAR2_DSP_AIF2_CTRL, 0x0000 },
> + { LOCHNAGAR2_PSIA1_CTRL, 0x0000 },
> + { LOCHNAGAR2_PSIA2_CTRL, 0x0000 },
> + { LOCHNAGAR2_GF_AIF3_CTRL, 0x0000 },
> + { LOCHNAGAR2_GF_AIF4_CTRL, 0x0000 },
> + { LOCHNAGAR2_GF_AIF1_CTRL, 0x0000 },
> + { LOCHNAGAR2_GF_AIF2_CTRL, 0x0000 },
> + { LOCHNAGAR2_SPDIF_AIF_CTRL, 0x0000 },
> + { LOCHNAGAR2_USB_AIF1_CTRL, 0x0000 },
> + { LOCHNAGAR2_USB_AIF2_CTRL, 0x0000 },
> + { LOCHNAGAR2_ADAT_AIF_CTRL, 0x0000 },
> + { LOCHNAGAR2_CDC_MCLK1_CTRL, 0x0000 },
> + { LOCHNAGAR2_CDC_MCLK2_CTRL, 0x0000 },
> + { LOCHNAGAR2_DSP_CLKIN_CTRL, 0x0000 },
> + { LOCHNAGAR2_PSIA1_MCLK_CTRL, 0x0000 },
> + { LOCHNAGAR2_PSIA2_MCLK_CTRL, 0x0000 },
> + { LOCHNAGAR2_SPDIF_MCLK_CTRL, 0x0000 },
> + { LOCHNAGAR2_GF_CLKOUT1_CTRL, 0x0000 },
> + { LOCHNAGAR2_GF_CLKOUT2_CTRL, 0x0000 },
> + { LOCHNAGAR2_ADAT_MCLK_CTRL, 0x0000 },
> + { LOCHNAGAR2_SOUNDCARD_MCLK_CTRL, 0x0000 },
> + { LOCHNAGAR2_GPIO_FPGA_GPIO1, 0x0000 },
> + { LOCHNAGAR2_GPIO_FPGA_GPIO2, 0x0000 },
> + { LOCHNAGAR2_GPIO_FPGA_GPIO3, 0x0000 },
> + { LOCHNAGAR2_GPIO_FPGA_GPIO4, 0x0000 },
> + { LOCHNAGAR2_GPIO_FPGA_GPIO5, 0x0000 },
> + { LOCHNAGAR2_GPIO_FPGA_GPIO6, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO1, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO2, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO3, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO4, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO5, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO6, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO7, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_GPIO8, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO1, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO2, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO3, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO4, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO5, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO6, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_GPIO2, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_GPIO3, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_GPIO7, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF1_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF1_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF1_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF1_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF2_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF2_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF2_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF2_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF3_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF3_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF3_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_AIF3_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF1_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF1_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF1_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF1_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF2_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF2_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF2_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_AIF2_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA1_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA1_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA1_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA1_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA2_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA2_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA2_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA2_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF3_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF3_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF3_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF3_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF4_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF4_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF4_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF4_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF1_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF1_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF1_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF1_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF2_BCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF2_RXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF2_LRCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_AIF2_TXDAT, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_UART1_RX, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_UART1_TX, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_UART2_RX, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_UART2_TX, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_UART2_RX, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_UART2_TX, 0x0000 },
> + { LOCHNAGAR2_GPIO_USB_UART_RX, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_PDMCLK1, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_PDMDAT1, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_PDMCLK2, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_PDMDAT2, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICCLK1, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICDAT1, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICCLK2, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICDAT2, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICCLK3, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICDAT3, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICCLK4, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_DMICDAT4, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_DMICCLK1, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_DMICDAT1, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_DMICCLK2, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_DMICDAT2, 0x0000 },
> + { LOCHNAGAR2_GPIO_I2C2_SCL, 0x0000 },
> + { LOCHNAGAR2_GPIO_I2C2_SDA, 0x0000 },
> + { LOCHNAGAR2_GPIO_I2C3_SCL, 0x0000 },
> + { LOCHNAGAR2_GPIO_I2C3_SDA, 0x0000 },
> + { LOCHNAGAR2_GPIO_I2C4_SCL, 0x0000 },
> + { LOCHNAGAR2_GPIO_I2C4_SDA, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_STANDBY, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_MCLK1, 0x0000 },
> + { LOCHNAGAR2_GPIO_CDC_MCLK2, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_CLKIN, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA1_MCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_PSIA2_MCLK, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_GPIO1, 0x0000 },
> + { LOCHNAGAR2_GPIO_GF_GPIO5, 0x0000 },
> + { LOCHNAGAR2_GPIO_DSP_GPIO20, 0x0000 },
> + { LOCHNAGAR2_MINICARD_RESETS, 0x0000 },
> + { LOCHNAGAR2_ANALOGUE_PATH_CTRL2, 0x0000 },
> + { LOCHNAGAR2_COMMS_CTRL4, 0x0001 },
> + { LOCHNAGAR2_SPDIF_CTRL, 0x0008 },
> + { LOCHNAGAR2_POWER_CTRL, 0x0001 },
> + { LOCHNAGAR2_SOUNDCARD_AIF_CTRL, 0x0000 },
> +};

OMG! Vile, vile vile!

> +static const struct regmap_config lochnagar2_i2c_regmap = {
> + .reg_bits = 16,
> + .val_bits = 16,
> + .reg_format_endian = REGMAP_ENDIAN_BIG,
> + .val_format_endian = REGMAP_ENDIAN_BIG,
> +
> + .max_register = 0x1F1F,
> + .readable_reg = lochnagar2_readable_register,
> + .volatile_reg = lochnagar2_volatile_register,
> +
> + .cache_type = REGCACHE_RBTREE,
> +
> + .reg_defaults = lochnagar2_reg_defaults,
> + .num_reg_defaults = ARRAY_SIZE(lochnagar2_reg_defaults),
> +};
> +
> +static const struct reg_sequence lochnagar2_patch[] = {
> + { 0x00EE, 0x0000 },
> + { 0x00F0, 0x0001 },
> +};
> +
> +static struct mfd_cell lochnagar2_devs[] = {
> + {
> + .name = "lochnagar-pinctrl"
> + },
> + {
> + .name = "lochnagar-clk"
> + },
> + {
> + .name = "lochnagar-regulator"
> + },
> + {
> + .name = "lochnagar2-sound-card",
> + },
> +};

All the same as above.

> +static struct lochnagar_config {
> + int id;
> + const char * const name;
> + enum lochnagar_type type;
> + const struct regmap_config *regmap;
> + struct mfd_cell *devs;
> + int ndevs;
> + const struct reg_sequence *patch;
> + int npatch;
> +} lochnagar_configs[] = {

Not a fan of this syntax.

Please separate the struct declaration and its use.

> + {
> + .id = 0x50,
> + .name = "lochnagar1",
> + .type = LOCHNAGAR1,
> + .regmap = &lochnagar1_i2c_regmap,
> + .devs = lochnagar1_devs,
> + .ndevs = ARRAY_SIZE(lochnagar1_devs),
> + .patch = lochnagar1_patch,
> + .npatch = ARRAY_SIZE(lochnagar1_patch),
> + },
> + {
> + .id = 0xCB58,
> + .name = "lochnagar2",
> + .type = LOCHNAGAR2,
> + .regmap = &lochnagar2_i2c_regmap,
> + .devs = lochnagar2_devs,
> + .ndevs = ARRAY_SIZE(lochnagar2_devs),
> + .patch = lochnagar2_patch,
> + .npatch = ARRAY_SIZE(lochnagar2_patch),
> + },
> +};
> +
> +static const struct of_device_id lochnagar_of_match[] = {
> + { .compatible = "cirrus,lochnagar1", .data = &lochnagar_configs[0] },
> + { .compatible = "cirrus,lochnagar2", .data = &lochnagar_configs[1] },
> + {},
> +};
> +
> +static int lochnagar_wait_for_boot(struct regmap *regmap, unsigned int *id)
> +{
> + int i, ret;
> +
> + for (i = 0; i < LOCHNAGAR_BOOT_RETRIES; ++i) {
> + msleep(LOCHNAGAR_BOOT_DELAY_MS);
> +
> + ret = regmap_read(regmap, LOCHNAGAR_SOFTWARE_RESET, id);
> + if (!ret)
> + return ret;
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +int lochnagar_update_config(struct lochnagar *lochnagar)
> +{
> + struct regmap *regmap = lochnagar->regmap;
> + unsigned int done = LOCHNAGAR2_ANALOGUE_PATH_UPDATE_STS_MASK;
> + int timeout_ms = LOCHNAGAR_BOOT_DELAY_MS * LOCHNAGAR_BOOT_RETRIES;
> + unsigned int val = 0;
> + int ret;
> +
> + switch (lochnagar->type) {
> + case LOCHNAGAR1:
> + return 0;
> + case LOCHNAGAR2:
> + break;
> + default:
> + return -EINVAL;
> + }

This can be done with a simple if () statement:

if (lochnagar->type != LOCHNAGAR2)
return 0;

> + ret = regmap_write(regmap, LOCHNAGAR2_ANALOGUE_PATH_CTRL1, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, LOCHNAGAR2_ANALOGUE_PATH_CTRL1,
> + LOCHNAGAR2_ANALOGUE_PATH_UPDATE_MASK);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read_poll_timeout(regmap,
> + LOCHNAGAR2_ANALOGUE_PATH_CTRL1, val,
> + (val & done), LOCHNAGAR_CONFIG_POLL_US,
> + timeout_ms * 1000);
> + if (ret < 0)
> + return ret;

What does all this do then? You need a comment.

> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(lochnagar_update_config);
> +
> +static int lochnagar_i2c_probe(struct i2c_client *i2c)
> +{
> + struct device *dev = &i2c->dev;
> + const struct lochnagar_config *config = NULL;
> + const struct of_device_id *of_id;
> + struct lochnagar *lochnagar;
> + unsigned int val;
> + struct gpio_desc *reset, *present;

Put this up higher in the list to keep the simple 'int's together.

> + unsigned int firmwareid;
> + int devid, rev;

Why do these need to be signed?

> + int ret;
> +
> + lochnagar = devm_kzalloc(dev, sizeof(*lochnagar), GFP_KERNEL);
> + if (!lochnagar)
> + return -ENOMEM;
> +
> + of_id = of_match_device(lochnagar_of_match, dev);
> + if (!of_id)
> + return -EINVAL;
> +
> + config = of_id->data;
> +
> + lochnagar->dev = dev;
> + mutex_init(&lochnagar->analogue_config_lock);
> +
> + dev_set_drvdata(dev, lochnagar);
> +
> + reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(reset)) {
> + ret = PTR_ERR(reset);
> + dev_err(dev, "Failed to get reset GPIO: %d\n", ret);
> + return ret;
> + }
> +
> + present = devm_gpiod_get_optional(dev, "present", GPIOD_OUT_HIGH);
> + if (IS_ERR(present)) {
> + ret = PTR_ERR(present);
> + dev_err(dev, "Failed to get present GPIO: %d\n", ret);
> + return ret;
> + }
> +
> + msleep(20);

What's this for?

> + /* Bring Lochnagar out of reset */
> + gpiod_set_value_cansleep(reset, 1);
> +
> + /* Identify Lochnagar */
> + lochnagar->type = config->type;

'\n'

> + lochnagar->regmap = devm_regmap_init_i2c(i2c, config->regmap);
> + if (IS_ERR(lochnagar->regmap)) {
> + ret = PTR_ERR(lochnagar->regmap);
> + dev_err(dev, "Failed to allocate register map: %d\n", ret);
> + return ret;
> + }
> +
> + /* Wait for Lochnagar to boot */
> + ret = lochnagar_wait_for_boot(lochnagar->regmap, &val);
> + if (ret < 0) {
> + dev_err(dev, "Failed to read device ID: %d\n", ret);

Eh?

So you read the LOCHNAGAR_SOFTWARE_RESET register and out pops the
device/revision IDs? That's just random!

> + return ret;
> + }
> +
> + devid = val & LOCHNAGAR_DEVICE_ID_MASK;
> + rev = val & LOCHNAGAR_REV_ID_MASK;
> +
> + if (devid != config->id) {
> + dev_err(dev,
> + "ID does not match %s (expected 0x%x got 0x%x)\n",
> + config->name, config->id, devid);
> + return -ENODEV;
> + }
> +
> + /* Identify firmware */
> + ret = regmap_read(lochnagar->regmap, LOCHNAGAR_FIRMWARE_ID1, &val);
> + if (ret < 0) {
> + dev_err(dev, "Failed to read firmware id 1: %d\n", ret);
> + return ret;
> + }
> +
> + firmwareid = val;
> +
> + ret = regmap_read(lochnagar->regmap, LOCHNAGAR_FIRMWARE_ID2, &val);
> + if (ret < 0) {
> + dev_err(dev, "Failed to read firmware id 2: %d\n", ret);
> + return ret;
> + }
> +
> + firmwareid |= (val << config->regmap->val_bits);
> +
> + dev_info(dev, "Found %s (0x%x) revision %d firmware 0x%.6x\n",
> + config->name, devid, rev + 1, firmwareid);
> +
> + ret = regmap_register_patch(lochnagar->regmap, config->patch,
> + config->npatch);
> + if (ret < 0) {
> + dev_err(dev, "Failed to register patch: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, config->devs,
> + config->ndevs, NULL, 0, NULL);
> + if (ret != 0) {

if (ret)

> + dev_err(dev, "Failed to add subdevices: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_of_platform_populate(dev);
> + if (ret < 0) {
> + dev_err(dev, "Failed to populate child nodes: %d\n", ret);
> + return ret;
> + }

Please do not mix OF and MFD device registration strategies.

Pick one and register all devices through your chosen method.

> + return ret;
> +}
> +
> +static struct i2c_driver lochnagar_i2c_driver = {
> + .driver = {
> + .name = "lochnagar",
> + .of_match_table = of_match_ptr(lochnagar_of_match),
> + .suppress_bind_attrs = true,
> + },
> +

No need for '\n'.

> + .probe_new = lochnagar_i2c_probe,

Hasn't this been replaced yet?

> +};
> +
> +static int __init lochnagar_i2c_init(void)
> +{
> + int ret;
> +
> + ret = i2c_add_driver(&lochnagar_i2c_driver);
> + if (ret != 0)

if (ret)

> + pr_err("Failed to register Lochnagar driver: %d\n", ret);
> +
> + return ret;
> +}
> +subsys_initcall(lochnagar_i2c_init);
> diff --git a/include/linux/mfd/lochnagar.h b/include/linux/mfd/lochnagar.h
> new file mode 100644
> index 0000000000000..be485d6714f7c
> --- /dev/null
> +++ b/include/linux/mfd/lochnagar.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Lochnagar internals
> + *
> + * Copyright (c) 2013-2018 Cirrus Logic, Inc. and
> + * Cirrus Logic International Semiconductor Ltd.
> + *
> + * Author: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#ifndef CIRRUS_LOCHNAGAR_H
> +#define CIRRUS_LOCHNAGAR_H
> +
> +#include "lochnagar1_regs.h"
> +#include "lochnagar2_regs.h"

Why are you including these here?

> +struct device;
> +struct regmap;
> +struct mutex;

Just add the include files.

> +enum lochnagar_type {
> + LOCHNAGAR1,
> + LOCHNAGAR2,
> +};
> +
> +struct lochnagar {
> + enum lochnagar_type type;
> + struct device *dev;
> + struct regmap *regmap;
> +
> + /* Lock to protect updates to the analogue configuration */
> + struct mutex analogue_config_lock;
> +};

Kerneldoc header please.

> +/* Register Addresses */
> +#define LOCHNAGAR_SOFTWARE_RESET 0x00
> +#define LOCHNAGAR_FIRMWARE_ID1 0x01
> +#define LOCHNAGAR_FIRMWARE_ID2 0x02
> +
> +/* (0x0000) Software Reset */
> +#define LOCHNAGAR_DEVICE_ID_MASK 0xFFFC
> +#define LOCHNAGAR_DEVICE_ID_SHIFT 2
> +#define LOCHNAGAR_REV_ID_MASK 0x0003
> +#define LOCHNAGAR_REV_ID_SHIFT 0
> +
> +int lochnagar_update_config(struct lochnagar *lochnagar);
> +
> +#endif

> diff --git a/include/linux/mfd/lochnagar1_regs.h b/include/linux/mfd/lochnagar1_regs.h
> diff --git a/include/linux/mfd/lochnagar2_regs.h b/include/linux/mfd/lochnagar2_regs.h

So Lochnagar 1 and 2 are completely different devices?

What do they do?

--
Lee Jones [æçæ]
Linaro Services Technical Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Next message: Michal Hocko: "Re: [PATCH V3 3/5] mm/hugetlb: Enable arch specific huge page size support for migration"
Previous message: Federico Vaga: "Re: [PATCH 1/3] i2c:ocores: stop transfer on timeout"
Next in thread: Charles Keepax: "Re: [PATCH v2 2/5] mfd: lochnagar: Add support for the Cirrus Logic Lochnagar"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]