From: Lee Jones <lee.jones@linaro.org>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: mturquette@baylibre.com, sboyd@kernel.org, broonie@kernel.org,
linus.walleij@linaro.org, robh+dt@kernel.org,
mark.rutland@arm.com, lgirdwood@gmail.com,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
patches@opensource.cirrus.com, linux-clk@vger.kernel.org,
linux-gpio@vger.kernel.org
Subject: Re: [PATCH v2 2/5] mfd: lochnagar: Add support for the Cirrus Logic Lochnagar
Date: Thu, 25 Oct 2018 08:44:59 +0100 [thread overview]
Message-ID: <20181025074459.GF4939@dell> (raw)
In-Reply-To: <20181008132542.19775-2-ckeepax@opensource.cirrus.com>
On Mon, 08 Oct 2018, Charles Keepax wrote:
> From: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>
> 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@opensource.wolfsonmicro.com>
> ---
>
> 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@vger.kernel.org
> S: Maintained
> F: drivers/net/ethernet/cirrus/ep93xx_eth.c
>
> +CIRRUS LOGIC LOCHNAGAR DRIVER
> +M: Charles Keepax <ckeepax@opensource.cirrus.com>
> +M: Richard Fitzgerald <rf@opensource.cirrus.com>
> +L: patches@opensource.cirrus.com
> +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@cisco.com>
> M: Sesidhar Baddela <sebaddel@cisco.com>
> 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@opensource.cirrus.com>
> + */
> +
> +#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@opensource.cirrus.com>
> + */
> +
> +#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 prev parent reply other threads:[~2018-10-25 7:45 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-08 13:25 [PATCH v2 1/5] mfd: lochnagar: Add initial binding documentation Charles Keepax
2018-10-08 13:25 ` Charles Keepax
2018-10-08 13:25 ` [PATCH v2 2/5] mfd: lochnagar: Add support for the Cirrus Logic Lochnagar Charles Keepax
2018-10-08 13:25 ` Charles Keepax
2018-10-25 7:44 ` Lee Jones [this message]
2018-10-25 8:26 ` Charles Keepax
2018-10-25 8:26 ` Charles Keepax
2018-10-25 9:28 ` Richard Fitzgerald
2018-10-25 9:28 ` Richard Fitzgerald
2018-10-25 10:12 ` Mark Brown
2018-10-25 10:56 ` Charles Keepax
2018-10-25 10:56 ` Charles Keepax
2018-10-25 11:42 ` Lee Jones
2018-10-25 12:49 ` Charles Keepax
2018-10-25 12:49 ` Charles Keepax
2018-10-25 13:20 ` Charles Keepax
2018-10-25 13:20 ` Charles Keepax
2018-10-25 13:47 ` Richard Fitzgerald
2018-10-25 13:47 ` Richard Fitzgerald
2018-10-26 15:49 ` Mark Brown
2018-10-26 7:33 ` Lee Jones
2018-10-26 15:47 ` Mark Brown
2018-10-25 13:40 ` Richard Fitzgerald
2018-10-25 13:40 ` Richard Fitzgerald
2018-10-26 8:00 ` Lee Jones
2018-10-26 20:32 ` Mark Brown
2018-10-29 11:04 ` Lee Jones
2018-10-29 11:52 ` Richard Fitzgerald
2018-10-29 11:52 ` Richard Fitzgerald
2018-10-29 12:36 ` Richard Fitzgerald
2018-10-29 12:36 ` Richard Fitzgerald
2018-10-29 12:57 ` Mark Brown
2018-11-01 10:28 ` Charles Keepax
2018-11-01 10:28 ` Charles Keepax
2018-11-01 11:40 ` Richard Fitzgerald
2018-11-01 11:40 ` Richard Fitzgerald
2018-11-01 12:04 ` Mark Brown
2018-11-01 12:01 ` Mark Brown
2018-11-01 14:17 ` Richard Fitzgerald
2018-11-01 14:17 ` Richard Fitzgerald
2018-10-08 13:25 ` [PATCH v2 3/5] clk: " Charles Keepax
2018-10-08 13:25 ` Charles Keepax
2018-10-11 7:00 ` Stephen Boyd
2018-10-11 13:26 ` Charles Keepax
2018-10-11 13:26 ` Charles Keepax
2018-10-12 15:59 ` Stephen Boyd
2018-10-15 10:49 ` Charles Keepax
2018-10-15 10:49 ` Charles Keepax
2018-10-15 16:39 ` Stephen Boyd
2018-10-15 16:55 ` Mark Brown
2018-10-15 21:53 ` Stephen Boyd
2018-10-11 14:54 ` Mark Brown
2018-10-11 19:36 ` Stephen Boyd
2018-10-12 16:52 ` Mark Brown
2018-10-08 13:25 ` [PATCH v2 4/5] regulator: " Charles Keepax
2018-10-08 13:25 ` Charles Keepax
2018-10-08 13:25 ` [PATCH v2 5/5] pinctrl: " Charles Keepax
2018-10-08 13:25 ` Charles Keepax
2018-10-12 22:08 ` [PATCH v2 1/5] mfd: lochnagar: Add initial binding documentation Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181025074459.GF4939@dell \
--to=lee.jones@linaro.org \
--cc=broonie@kernel.org \
--cc=ckeepax@opensource.cirrus.com \
--cc=devicetree@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mturquette@baylibre.com \
--cc=patches@opensource.cirrus.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.