diff mbox series

[v3,1/5] mfd: lochnagar: Add initial binding documentation

Message ID 20181019095003.26046-1-ckeepax@opensource.cirrus.com
State Not Applicable, archived
Headers show
Series [v3,1/5] mfd: lochnagar: Add initial binding documentation | expand

Commit Message

Charles Keepax Oct. 19, 2018, 9:49 a.m. UTC
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.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes since v2:
 - Used vendor prefix on aif-master and aif-slave
 - Used -gpios instead of -gpio in example DT
 - Used - rather than _ on node names in example DT
 - Removed some clocks from the clock DT binding doc since they are
   now being registered purely through DT as fixed clocks

Thanks,
Charles

 .../devicetree/bindings/mfd/cirrus,lochnagar.txt   | 230 +++++++++++++++++++++
 include/dt-bindings/clk/lochnagar.h                |  26 +++
 include/dt-bindings/pinctrl/lochnagar.h            | 132 ++++++++++++
 3 files changed, 388 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/cirrus,lochnagar.txt
 create mode 100644 include/dt-bindings/clk/lochnagar.h
 create mode 100644 include/dt-bindings/pinctrl/lochnagar.h

Comments

Mark Brown Oct. 19, 2018, 11:26 a.m. UTC | #1
On Fri, Oct 19, 2018 at 10:50:02AM +0100, Charles Keepax wrote:

Please do not submit new versions of already applied patches, please
submit incremental updates to the existing code.  Modifying existing
commits creates problems for other users building on top of those
commits so it's best practice to only change pubished git commits if
absolutely essential.

> +++ b/drivers/regulator/lochnagar-regulator.c
> @@ -0,0 +1,255 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lochnagar regulator driver

Please don't mix C and C++ comments like this in the same block, just
have it be a C++ block so it looks consistent.
Richard Fitzgerald Oct. 19, 2018, 12:02 p.m. UTC | #2
On 19/10/18 12:26, Mark Brown wrote:
> On Fri, Oct 19, 2018 at 10:50:02AM +0100, Charles Keepax wrote:
> 
> Please do not submit new versions of already applied patches, please
> submit incremental updates to the existing code.  Modifying existing
> commits creates problems for other users building on top of those
> commits so it's best practice to only change pubished git commits if
> absolutely essential.
> 
>> +++ b/drivers/regulator/lochnagar-regulator.c
>> @@ -0,0 +1,255 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Lochnagar regulator driver
> 
> Please don't mix C and C++ comments like this in the same block, just
> have it be a C++ block so it looks consistent.
> 

Most SPDX headers on C files that I've looked at have it this way with a
C++ style comment above a C-style comment, though some don't. license-rules.rst
doesn't define how or if a SPDX comment line should be merged with the following
file header comment. I've had a bunch of patches in different subsystems all
accepted with this mixed format (copied from existing files). Doing the same as
existing files sounds reasonable but often isn't in the Linux kernel. It's a
common problem/barrier to kernel programming that existing code isn't a guide
and there isn't a consistent style across the kernel so one never really knows
what the coding style is until you've pushed a patch and annoyed a maintainer.
And then you adopt that style on your next patch and annoy different maintainer.

Maybe someone should update license-rules.rst to make a definite statement of the
style instead of leaving it to become another style that varies across the kernel
and between files.
Mark Brown Oct. 19, 2018, 12:06 p.m. UTC | #3
On Fri, Oct 19, 2018 at 01:02:45PM +0100, Richard Fitzgerald wrote:

> Most SPDX headers on C files that I've looked at have it this way with a
> C++ style comment above a C-style comment, though some don't. license-rules.rst
> doesn't define how or if a SPDX comment line should be merged with the following
> file header comment. I've had a bunch of patches in different subsystems all
> accepted with this mixed format (copied from existing files). Doing the same as

Right, a lot of the conversions seem to have been done very
mechanically.

> Maybe someone should update license-rules.rst to make a definite statement of the
> style instead of leaving it to become another style that varies across the kernel
> and between files.

Well volunteered!
Charles Keepax Oct. 19, 2018, 12:19 p.m. UTC | #4
On Fri, Oct 19, 2018 at 12:26:22PM +0100, Mark Brown wrote:
> On Fri, Oct 19, 2018 at 10:50:02AM +0100, Charles Keepax wrote:
> 
> Please do not submit new versions of already applied patches, please
> submit incremental updates to the existing code.  Modifying existing
> commits creates problems for other users building on top of those
> commits so it's best practice to only change pubished git commits if
> absolutely essential.
> 

I don't think any of this has been applied anywhere yet.

> > +++ b/drivers/regulator/lochnagar-regulator.c
> > @@ -0,0 +1,255 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Lochnagar regulator driver
> 
> Please don't mix C and C++ comments like this in the same block, just
> have it be a C++ block so it looks consistent.

Can do.

Thanks,
Charles
Mark Brown Oct. 19, 2018, 12:21 p.m. UTC | #5
On Fri, Oct 19, 2018 at 01:19:20PM +0100, Charles Keepax wrote:
> On Fri, Oct 19, 2018 at 12:26:22PM +0100, Mark Brown wrote:

> > Please do not submit new versions of already applied patches, please
> > submit incremental updates to the existing code.  Modifying existing
> > commits creates problems for other users building on top of those
> > commits so it's best practice to only change pubished git commits if
> > absolutely essential.

> I don't think any of this has been applied anywhere yet.

It looks a lot like

bef9391cbec547351c6a13e52f3a26bb2d271ec7 regulator: lochnagar: Add support for the Cirrus Logic Lochnagar

in -next.
Charles Keepax Oct. 19, 2018, 12:34 p.m. UTC | #6
On Fri, Oct 19, 2018 at 01:21:18PM +0100, Mark Brown wrote:
> On Fri, Oct 19, 2018 at 01:19:20PM +0100, Charles Keepax wrote:
> > On Fri, Oct 19, 2018 at 12:26:22PM +0100, Mark Brown wrote:
> 
> > > Please do not submit new versions of already applied patches, please
> > > submit incremental updates to the existing code.  Modifying existing
> > > commits creates problems for other users building on top of those
> > > commits so it's best practice to only change pubished git commits if
> > > absolutely essential.
> 
> > I don't think any of this has been applied anywhere yet.
> 
> It looks a lot like
> 
> bef9391cbec547351c6a13e52f3a26bb2d271ec7 regulator: lochnagar: Add support for the Cirrus Logic Lochnagar
> 
> in -next.

Well it sure does look a lot like my patch :-). Apologies
something must have gone wrong on one of our ends as I don't
seem to have ever received your usual confirmation email it
was merged. I will try to keep a closer eye on people's trees
in future.

No changes in this version, was just updating other patches in the
chain for some feedback so no harm done. Will send an incremental
patch for comment style.

Thanks,
Charles
Mark Brown Oct. 19, 2018, 12:38 p.m. UTC | #7
On Fri, Oct 19, 2018 at 01:34:42PM +0100, Charles Keepax wrote:
> On Fri, Oct 19, 2018 at 01:21:18PM +0100, Mark Brown wrote:

> > It looks a lot like

> > bef9391cbec547351c6a13e52f3a26bb2d271ec7 regulator: lochnagar: Add support for the Cirrus Logic Lochnagar

> > in -next.

> Well it sure does look a lot like my patch :-). Apologies
> something must have gone wrong on one of our ends as I don't
> seem to have ever received your usual confirmation email it
> was merged. I will try to keep a closer eye on people's trees
> in future.

They do sometimes seem to get flagged as spam, I think due to the amount
of boilerplate and possibly even the fact that they include the patches.
Stephen Boyd Oct. 19, 2018, 6:08 p.m. UTC | #8
Quoting Charles Keepax (2018-10-19 02:49:59)
> 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.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>
Linus Walleij Oct. 30, 2018, 1:04 p.m. UTC | #9
On Fri, Oct 19, 2018 at 11:50 AM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:

> 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.
>
> Lochnagar provides many pins which can generally be used for an
> audio function such as an AIF or a PDM interface, but also as
> GPIOs.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>
> Changes since v2:
>  - Updated aif-master/aif-slave to use a vendor prefix
>  - Correctly get and put of_node pointer in probe/remove

Looks better and better! Some comments, probably not the last comments:

> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinconf-generic.h>

#include <linux/bits.h>

> +struct lochnagar_aif {
> +       const char name[16];
> +
> +       unsigned int pins[4];
> +
> +       unsigned int src_reg;

u32?

> +       unsigned int src_mask;

u32?

> +
> +       unsigned int ctrl_reg;

u32?

> +       unsigned int ena_mask;

u32?

> +       unsigned int master_mask;

u32?

> +struct lochnagar_func {
> +       const char * const name;
> +
> +       enum lochnagar_func_type type;
> +
> +       int op;

unsigned int? u32?

Looks like u8 actually.

> +struct lochnagar_group {
> +       const char * const name;
> +
> +       enum lochnagar_func_type type;
> +
> +       const unsigned int *pins;
> +       int npins;

unsigned int npins?

> +struct lochnagar_func_groups {
> +       const char **groups;
> +       int ngroups;

unsigned int ngroups?

> +struct lochnagar_pin_priv {
> +       struct lochnagar *lochnagar;
> +       struct device *dev;
> +
> +       const struct lochnagar_func *funcs;
> +       int nfuncs;
> +
> +       const struct pinctrl_pin_desc *pins;
> +       int npins;
> +
> +       const struct lochnagar_group *groups;
> +       int ngroups;
> +
> +       struct lochnagar_func_groups func_groups[LN_FTYPE_COUNT];
> +
> +       struct gpio_chip gpio_chip;
> +};

I bet some of these should be unsigned as well.

> +static const struct pinconf_generic_params lochnagar_dt_params[] = {
> +       {"cirrus,aif-master", PIN_CONFIG_AIFMASTER,  0},
> +       {"cirrus,aif-slave",  PIN_CONFIG_AIFSLAVE,   0},
> +};

I guess these are documented in the device tree bindings.

They look suspciously like muxing but I guess it is explained
there.

> +static void lochnagar_gpio_set(struct gpio_chip *chip,
> +                              unsigned int offset, int value)
> +{
> +       struct lochnagar_pin_priv *priv = gpiochip_get_data(chip);
> +       struct lochnagar *lochnagar = priv->lochnagar;
> +       const struct lochnagar_pin *pin = priv->pins[offset].drv_data;
> +       int ret;
> +
> +       value = !!value;

value = value ? BIT(pin->shift) : 0;

> +       dev_dbg(priv->dev, "Set GPIO %s to %s\n",
> +               pin->name, value ? "high" : "low");
> +
> +       switch (pin->type) {
> +       case LN_PTYPE_MUX:
> +               value |= LN2_OP_GPIO;
> +
> +               ret = lochnagar_pin_set_mux(priv, pin, value);
> +               break;
> +       case LN_PTYPE_GPIO:
> +               if (pin->invert)
> +                       value = !value;
> +
> +               ret = regmap_update_bits(lochnagar->regmap, pin->reg,
> +                                        0x1 << pin->shift,

BIT(pin->shift)

> +                                        value << pin->shift);

Just value provided you used the construction above
to construct it.

Yours,
Linus Walleij
Linus Walleij Oct. 30, 2018, 1:10 p.m. UTC | #10
On Fri, Oct 19, 2018 at 11:50 AM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:

> +      gf-aif2-txdat, usb-aif1, usb-aif2, adat-aif, soundcard-aif,
> +
> +  - aif-master : Specifies that an AIF group will be used as a master
> +    interface (either this or aif-slave is required if a group is
> +    being muxed to an AIF)
> +  - aif-slave : Specifies that an AIF group will be used as a slave
> +    interface (either this or aif-master is required if a group is
> +    being muxed to an AIF)

This is not properly described as pin config parameters, but that is how
they are used in the driver. Please describe that this is supposed to
augment the pin config of the pins.

But I think it looks bogus. If the pins are already mixed to groups
like that:

> +       pin-settings: pin-settings {
> +               ap-aif {
> +                       aif-slave;
> +                       groups = "gf-aif1";
> +                       function = "codec-aif3";
> +               };
> +               codec-aif {
> +                       aif-master;
> +                       groups = "codec-aif3";
> +                       function = "gf-aif1";
> +               };
> +       };

OK so what the "special properties" above indicates is really the
direction of the pins, whether out (master) or in (slave".

Just use the standard bool pin config properties:
input-enable;
output-enable;

Reference
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

For this. If you want users to see that this is master and slave,
just /* Write a comment */ in the device tree.

Yours,
Linus Walleij
Charles Keepax Oct. 30, 2018, 1:59 p.m. UTC | #11
On Tue, Oct 30, 2018 at 02:04:20PM +0100, Linus Walleij wrote:
> On Fri, Oct 19, 2018 at 11:50 AM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> 
> > 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.
> >
> > Lochnagar provides many pins which can generally be used for an
> > audio function such as an AIF or a PDM interface, but also as
> > GPIOs.
> >
> > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > ---
> > +static void lochnagar_gpio_set(struct gpio_chip *chip,
> > +                              unsigned int offset, int value)
> > +{
> > +       struct lochnagar_pin_priv *priv = gpiochip_get_data(chip);
> > +       struct lochnagar *lochnagar = priv->lochnagar;
> > +       const struct lochnagar_pin *pin = priv->pins[offset].drv_data;
> > +       int ret;
> > +
> > +       value = !!value;
> 
> value = value ? BIT(pin->shift) : 0;
> 

I think this will end up more complex because...

> > +       dev_dbg(priv->dev, "Set GPIO %s to %s\n",
> > +               pin->name, value ? "high" : "low");
> > +
> > +       switch (pin->type) {
> > +       case LN_PTYPE_MUX:
> > +               value |= LN2_OP_GPIO;

I want the value to be in the lowest bit for this path.

> > +
> > +               ret = lochnagar_pin_set_mux(priv, pin, value);
> > +               break;
> > +       case LN_PTYPE_GPIO:
> > +               if (pin->invert)
> > +                       value = !value;

And then I need to invert the value here.

> > +
> > +               ret = regmap_update_bits(lochnagar->regmap, pin->reg,
> > +                                        0x1 << pin->shift,
> 
> BIT(pin->shift)
> 
> > +                                        value << pin->shift);
> 
> Just value provided you used the construction above
> to construct it.
> 

All the other comments look good will fixup for the next spin.

Thanks,
Charles
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/cirrus,lochnagar.txt b/Documentation/devicetree/bindings/mfd/cirrus,lochnagar.txt
new file mode 100644
index 0000000000000..4bed850aa7101
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/cirrus,lochnagar.txt
@@ -0,0 +1,230 @@ 
+Cirrus Logic Lochnagar Audio Development Board
+
+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.  Audio system topology, clocking and power can all be
+controlled through the Lochnagar, allowing the device under test
+to be used in a variety of possible use cases.
+
+Also see these documents for generic binding information:
+  [1] GPIO : ../gpio/gpio.txt
+  [2] Pinctrl: ../pinctrl/pinctrl-bindings.txt
+  [3] Clock : ../clock/clock-bindings.txt
+  [4] Regulator: ../regulator/regulator.txt
+
+And these for relevant defines:
+  [5] include/dt-bindings/pinctrl/lochnagar.h
+  [6] include/dt-bindings/clock/lochnagar.h
+
+Required properties:
+
+  - compatible : One of the following strings:
+                 "cirrus,lochnagar1"
+                 "cirrus,lochnagar2"
+
+  - reg : I2C slave address
+
+  - gpio-controller : Indicates this device is a GPIO controller.
+  - #gpio-cells : Must be 2. The first cell is the pin number, see
+    [5] for available pins and the second cell is used to specify
+    optional parameters, see [1].
+  - gpio-ranges : Range of pins managed by the GPIO controller, see
+    [1]. Both the GPIO and Pinctrl base should be set to zero and the
+    count to the appropriate of the LOCHNAGARx_PIN_NUM_GPIOS define,
+    see [5].
+
+  - reset-gpios : Reset line to the Lochnagar, see [1].
+
+  - #clock-cells : Must be 1. The first cell indicates the clock
+    number, see [6] for available clocks and [3].
+
+  - pinctrl-names : A pinctrl state named "default" must be defined.
+  - pinctrl-0 : A phandle to the default pinctrl state.
+
+  - SYSVDD-supply: Primary power supply for the Lochnagar.
+
+Optional properties:
+
+  - present-gpios : Host present line, indicating the presence of a
+    host system, see [1]. This can be omitted if the present line is
+    tied in hardware.
+
+  - clocks : Must contain an entry for each clock in clock-names.
+  - clock-names : May contain entries for each of the following
+    clocks:
+     - ln-cdc-clkout : Output clock from CODEC card.
+     - ln-dsp-clkout : Output clock from DSP card.
+     - ln-gf-mclk1,ln-gf-mclk2,ln-gf-mclk3,ln-gf-mclk4 : Optional
+       input audio clocks from host system.
+     - ln-psia1-mclk, ln-psia2-mclk : Optional input audio clocks from
+       external connector.
+     - ln-spdif-clkout : Optional input audio clock from SPDIF.
+     - ln-adat-clkout : Optional input audio clock from ADAT.
+
+  - assigned-clocks : A list of Lochnagar clocks to be reparented, see
+    [6] for available clocks.
+  - assigned-clock-parents : Parents to be assigned to the clocks
+    listed in "assigned-clocks".
+
+  - MICBIAS1-supply, MICBIAS2-supply: Regulator supplies for the
+    MICxVDD outputs, supplying the digital microphones, normally
+    supplied from the attached CODEC.
+
+Required sub-nodes:
+
+The pin configurations are defined as a child of the pinctrl states
+node, see [2]. Each sub-node can have the following properties:
+  - groups : A list of groups to select (either this or "pins" must be
+    specified), available groups:
+      codec-aif1, codec-aif2, codec-aif3, dsp-aif1, dsp-aif2, psia1,
+      psia2, gf-aif1, gf-aif2, gf-aif3, gf-aif4, spdif-aif, usb-aif1,
+      usb-aif2, adat-aif, soundcard-aif
+  - pins : A list of pin names to select (either this or "groups" must
+    be specified), available pins:
+      fgpa-gpio1, fgpa-gpio2, fgpa-gpio3, fgpa-gpio4, fgpa-gpio5,
+      fgpa-gpio6, codec-gpio1, codec-gpio2, codec-gpio3, codec-gpio4,
+      codec-gpio5, codec-gpio6, codec-gpio7, codec-gpio8, dsp-gpio1,
+      dsp-gpio2, dsp-gpio3, dsp-gpio4, dsp-gpio5, dsp-gpio6, gf-gpio2,
+      gf-gpio3, gf-gpio7, codec-aif1-bclk, codec-aif1-rxdat,
+      codec-aif1-lrclk, codec-aif1-txdat, codec-aif2-bclk,
+      codec-aif2-rxdat, codec-aif2-lrclk, codec-aif2-txdat,
+      codec-aif3-bclk, codec-aif3-rxdat, codec-aif3-lrclk,
+      codec-aif3-txdat, dsp-aif1-bclk, dsp-aif1-rxdat, dsp-aif1-lrclk,
+      dsp-aif1-txdat, dsp-aif2-bclk, dsp-aif2-rxdat,
+      dsp-aif2-lrclk, dsp-aif2-txdat, psia1-bclk, psia1-rxdat,
+      psia1-lrclk, psia1-txdat, psia2-bclk, psia2-rxdat, psia2-lrclk,
+      psia2-txdat, gf-aif3-bclk, gf-aif3-rxdat, gf-aif3-lrclk,
+      gf-aif3-txdat, gf-aif4-bclk, gf-aif4-rxdat, gf-aif4-lrclk,
+      gf-aif4-txdat, gf-aif1-bclk, gf-aif1-rxdat, gf-aif1-lrclk,
+      gf-aif1-txdat, gf-aif2-bclk, gf-aif2-rxdat, gf-aif2-lrclk,
+      gf-aif2-txdat, dsp-uart1-rx, dsp-uart1-tx, dsp-uart2-rx,
+      dsp-uart2-tx, gf-uart2-rx, gf-uart2-tx, usb-uart-rx,
+      codec-pdmclk1, codec-pdmdat1, codec-pdmclk2, codec-pdmdat2,
+      codec-dmicclk1, codec-dmicdat1, codec-dmicclk2, codec-dmicdat2,
+      codec-dmicclk3, codec-dmicdat3, codec-dmicclk4, codec-dmicdat4,
+      dsp-dmicclk1, dsp-dmicdat1, dsp-dmicclk2, dsp-dmicdat2, i2c2-scl,
+      i2c2-sda, i2c3-scl, i2c3-sda, i2c4-scl, i2c4-sda, dsp-standby,
+      codec-mclk1, codec-mclk2, dsp-clkin, psia1-mclk, psia2-mclk,
+      gf-gpio1, gf-gpio5, dsp-gpio20, led1, led2
+  - function : The mux function to select, available functions:
+      aif, fgpa-gpio1, fgpa-gpio2, fgpa-gpio3, fgpa-gpio4, fgpa-gpio5,
+      fgpa-gpio6, codec-gpio1, codec-gpio2, codec-gpio3, codec-gpio4,
+      codec-gpio5, codec-gpio6, codec-gpio7, codec-gpio8, dsp-gpio1,
+      dsp-gpio2, dsp-gpio3, dsp-gpio4, dsp-gpio5, dsp-gpio6, gf-gpio2,
+      gf-gpio3, gf-gpio7, gf-gpio1, gf-gpio5, dsp-gpio20, codec-clkout,
+      dsp-clkout, pmic-32k, spdif-clkout, clk-12m288, clk-11m2986,
+      clk-24m576, clk-22m5792, xmos-mclk, gf-clkout1, gf-mclk1,
+      gf-mclk3, gf-mclk2, gf-clkout2, codec-mclk1, codec-mclk2,
+      dsp-clkin, psia1-mclk, psia2-mclk, spdif-mclk, codec-irq,
+      codec-reset, dsp-reset, dsp-irq, dsp-standby, codec-pdmclk1,
+      codec-pdmdat1, codec-pdmclk2, codec-pdmdat2, codec-dmicclk1,
+      codec-dmicdat1, codec-dmicclk2, codec-dmicdat2, codec-dmicclk3,
+      codec-dmicdat3, codec-dmicclk4, codec-dmicdat4, dsp-dmicclk1,
+      dsp-dmicdat1, dsp-dmicclk2, dsp-dmicdat2, dsp-uart1-rx,
+      dsp-uart1-tx, dsp-uart2-rx, dsp-uart2-tx, gf-uart2-rx,
+      gf-uart2-tx, usb-uart-rx, usb-uart-tx, i2c2-scl, i2c2-sda,
+      i2c3-scl, i2c3-sda, i2c4-scl, i2c4-sda, gpio, spdif-aif, psia1,
+      psia1-bclk, psia1-lrclk, psia1-rxdat, psia1-txdat, psia2,
+      psia2-bclk, psia2-lrclk, psia2-rxdat, psia2-txdat, codec-aif1,
+      codec-aif1-bclk, codec-aif1-lrclk, codec-aif1-rxdat,
+      codec-aif1-txdat, codec-aif2, codec-aif2-bclk, codec-aif2-lrclk,
+      codec-aif2-rxdat, codec-aif2-txdat, codec-aif3, codec-aif3-bclk,
+      codec-aif3-lrclk, codec-aif3-rxdat, codec-aif3-txdat, dsp-aif1,
+      dsp-aif1-bclk, dsp-aif1-lrclk, dsp-aif1-rxdat, dsp-aif1-txdat,
+      dsp-aif2, dsp-aif2-bclk, dsp-aif2-lrclk, dsp-aif2-rxdat,
+      dsp-aif2-txdat, gf-aif3, gf-aif3-bclk, gf-aif3-lrclk,
+      gf-aif3-rxdat, gf-aif3-txdat, gf-aif4, gf-aif4-bclk,
+      gf-aif4-lrclk, gf-aif4-rxdat, gf-aif4-txdat, gf-aif1,
+      gf-aif1-bclk, gf-aif1-lrclk, gf-aif1-rxdat, gf-aif1-txdat,
+      gf-aif2, gf-aif2-bclk, gf-aif2-lrclk, gf-aif2-rxdat,
+      gf-aif2-txdat, usb-aif1, usb-aif2, adat-aif, soundcard-aif,
+
+  - aif-master : Specifies that an AIF group will be used as a master
+    interface (either this or aif-slave is required if a group is
+    being muxed to an AIF)
+  - aif-slave : Specifies that an AIF group will be used as a slave
+    interface (either this or aif-master is required if a group is
+    being muxed to an AIF)
+
+Optional sub-nodes:
+
+  - VDDCORE : Initialisation data for the VDDCORE regulator, which
+    supplies the CODECs digital core if it has no build regulator for that
+    purpose.
+
+  - MICVDD : Initialisation data for the MICVDD regulator, which
+    supplies the CODECs MICVDD.
+  - MIC1VDD, MIC2VDD : Initialisation data for the MICxVDD supplies.
+      Optional Properties:
+      - cirrus,micbias-input : A property selecting which of the CODEC
+        minicard micbias outputs should be used, valid values are 1 - 4.
+
+  - VDD1V8 : Recommended fixed regulator for the VDD1V8 regulator, which supplies the
+    CODECs analog and 1.8V digital supplies.
+      Required Properties:
+      - compatible : Should be set to "regulator-fixed"
+      - regulator-min-microvolt : Should be set to 1.8V
+      - regulator-max-microvolt : Should be set to 1.8V
+      - regulator-boot-on
+      - regulator-always-on
+      - vin-supply : Should be set to same supply as SYSVDD
+
+Example:
+lochnagar: lochnagar@22 {
+	compatible = "cirrus,lochnagar2";
+	reg = <0x22>;
+
+	gpio-controller;
+	#gpio-cells = <2>;
+	gpio-ranges = <&lochnagar 0 0 110>;
+
+	reset-gpios = <&gpio0 55 0>;
+	present-gpios = <&gpio0 60 0>;
+
+	#clock-cells = <1>;
+
+	clocks = <&clk-audio>;
+	clock-names = "ln-gf-mclk2";
+
+	assigned-clocks = <&lochnagar LOCHNAGAR_CDC_MCLK1>,
+			  <&lochnagar LOCHNAGAR_CDC_MCLK2>;
+	assigned-clock-parents = <&clk-audio>,
+				 <&lochnagar LOCHNAGAR_PMIC_32K>;
+
+	SYSVDD-supply = <&wallvdd>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pin-settings>;
+
+	pin-settings: pin-settings {
+		ap-aif {
+			aif-slave;
+			groups = "gf-aif1";
+			function = "codec-aif3";
+		};
+		codec-aif {
+			aif-master;
+			groups = "codec-aif3";
+			function = "gf-aif1";
+		};
+	};
+
+	lochnagar-micvdd: MICVDD {
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	lochnagar-vdd1v8: VDD1V8 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VDD1V8";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+
+		vin-supply = <&wallvdd>;
+	};
+};
diff --git a/include/dt-bindings/clk/lochnagar.h b/include/dt-bindings/clk/lochnagar.h
new file mode 100644
index 0000000000000..8fa20551ff171
--- /dev/null
+++ b/include/dt-bindings/clk/lochnagar.h
@@ -0,0 +1,26 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Device Tree defines for Lochnagar clocking
+ *
+ * Copyright (c) 2017-2018 Cirrus Logic, Inc. and
+ *                         Cirrus Logic International Semiconductor Ltd.
+ *
+ * Author: Charles Keepax <ckeepax@opensource.cirrus.com>
+ */
+
+#ifndef DT_BINDINGS_CLK_LOCHNAGAR_H
+#define DT_BINDINGS_CLK_LOCHNAGAR_H
+
+#define LOCHNAGAR_CDC_MCLK1		0
+#define LOCHNAGAR_CDC_MCLK2		1
+#define LOCHNAGAR_DSP_CLKIN		2
+#define LOCHNAGAR_GF_CLKOUT1		3
+#define LOCHNAGAR_GF_CLKOUT2		4
+#define LOCHNAGAR_PSIA1_MCLK		5
+#define LOCHNAGAR_PSIA2_MCLK		6
+#define LOCHNAGAR_SPDIF_MCLK		7
+#define LOCHNAGAR_ADAT_MCLK		8
+#define LOCHNAGAR_SOUNDCARD_MCLK	9
+#define LOCHNAGAR_SPDIF_CLKOUT		10
+
+#endif
diff --git a/include/dt-bindings/pinctrl/lochnagar.h b/include/dt-bindings/pinctrl/lochnagar.h
new file mode 100644
index 0000000000000..644760bf5725b
--- /dev/null
+++ b/include/dt-bindings/pinctrl/lochnagar.h
@@ -0,0 +1,132 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Device Tree defines for Lochnagar pinctrl
+ *
+ * Copyright (c) 2018 Cirrus Logic, Inc. and
+ *                    Cirrus Logic International Semiconductor Ltd.
+ *
+ * Author: Charles Keepax <ckeepax@opensource.cirrus.com>
+ */
+
+#ifndef DT_BINDINGS_PINCTRL_LOCHNAGAR_H
+#define DT_BINDINGS_PINCTRL_LOCHNAGAR_H
+
+#define LOCHNAGAR1_PIN_CDC_RESET		0
+#define LOCHNAGAR1_PIN_DSP_RESET		1
+#define LOCHNAGAR1_PIN_CDC_CIF1MODE		2
+#define LOCHNAGAR1_PIN_NUM_GPIOS		3
+
+#define LOCHNAGAR2_PIN_CDC_RESET		0
+#define LOCHNAGAR2_PIN_DSP_RESET		1
+#define LOCHNAGAR2_PIN_CDC_CIF1MODE		2
+#define LOCHNAGAR2_PIN_CDC_LDOENA		3
+#define LOCHNAGAR2_PIN_SPDIF_HWMODE		4
+#define LOCHNAGAR2_PIN_SPDIF_RESET		5
+#define LOCHNAGAR2_PIN_FPGA_GPIO1		6
+#define LOCHNAGAR2_PIN_FPGA_GPIO2		7
+#define LOCHNAGAR2_PIN_FPGA_GPIO3		8
+#define LOCHNAGAR2_PIN_FPGA_GPIO4		9
+#define LOCHNAGAR2_PIN_FPGA_GPIO5		10
+#define LOCHNAGAR2_PIN_FPGA_GPIO6		11
+#define LOCHNAGAR2_PIN_CDC_GPIO1		12
+#define LOCHNAGAR2_PIN_CDC_GPIO2		13
+#define LOCHNAGAR2_PIN_CDC_GPIO3		14
+#define LOCHNAGAR2_PIN_CDC_GPIO4		15
+#define LOCHNAGAR2_PIN_CDC_GPIO5		16
+#define LOCHNAGAR2_PIN_CDC_GPIO6		17
+#define LOCHNAGAR2_PIN_CDC_GPIO7		18
+#define LOCHNAGAR2_PIN_CDC_GPIO8		19
+#define LOCHNAGAR2_PIN_DSP_GPIO1		20
+#define LOCHNAGAR2_PIN_DSP_GPIO2		21
+#define LOCHNAGAR2_PIN_DSP_GPIO3		22
+#define LOCHNAGAR2_PIN_DSP_GPIO4		23
+#define LOCHNAGAR2_PIN_DSP_GPIO5		24
+#define LOCHNAGAR2_PIN_DSP_GPIO6		25
+#define LOCHNAGAR2_PIN_GF_GPIO2			26
+#define LOCHNAGAR2_PIN_GF_GPIO3			27
+#define LOCHNAGAR2_PIN_GF_GPIO7			28
+#define LOCHNAGAR2_PIN_CDC_AIF1_BCLK		29
+#define LOCHNAGAR2_PIN_CDC_AIF1_RXDAT		30
+#define LOCHNAGAR2_PIN_CDC_AIF1_LRCLK		31
+#define LOCHNAGAR2_PIN_CDC_AIF1_TXDAT		32
+#define LOCHNAGAR2_PIN_CDC_AIF2_BCLK		33
+#define LOCHNAGAR2_PIN_CDC_AIF2_RXDAT		34
+#define LOCHNAGAR2_PIN_CDC_AIF2_LRCLK		35
+#define LOCHNAGAR2_PIN_CDC_AIF2_TXDAT		36
+#define LOCHNAGAR2_PIN_CDC_AIF3_BCLK		37
+#define LOCHNAGAR2_PIN_CDC_AIF3_RXDAT		38
+#define LOCHNAGAR2_PIN_CDC_AIF3_LRCLK		39
+#define LOCHNAGAR2_PIN_CDC_AIF3_TXDAT		40
+#define LOCHNAGAR2_PIN_DSP_AIF1_BCLK		41
+#define LOCHNAGAR2_PIN_DSP_AIF1_RXDAT		42
+#define LOCHNAGAR2_PIN_DSP_AIF1_LRCLK		43
+#define LOCHNAGAR2_PIN_DSP_AIF1_TXDAT		44
+#define LOCHNAGAR2_PIN_DSP_AIF2_BCLK		45
+#define LOCHNAGAR2_PIN_DSP_AIF2_RXDAT		46
+#define LOCHNAGAR2_PIN_DSP_AIF2_LRCLK		47
+#define LOCHNAGAR2_PIN_DSP_AIF2_TXDAT		48
+#define LOCHNAGAR2_PIN_PSIA1_BCLK		49
+#define LOCHNAGAR2_PIN_PSIA1_RXDAT		50
+#define LOCHNAGAR2_PIN_PSIA1_LRCLK		51
+#define LOCHNAGAR2_PIN_PSIA1_TXDAT		52
+#define LOCHNAGAR2_PIN_PSIA2_BCLK		53
+#define LOCHNAGAR2_PIN_PSIA2_RXDAT		54
+#define LOCHNAGAR2_PIN_PSIA2_LRCLK		55
+#define LOCHNAGAR2_PIN_PSIA2_TXDAT		56
+#define LOCHNAGAR2_PIN_GF_AIF3_BCLK		57
+#define LOCHNAGAR2_PIN_GF_AIF3_RXDAT		58
+#define LOCHNAGAR2_PIN_GF_AIF3_LRCLK		59
+#define LOCHNAGAR2_PIN_GF_AIF3_TXDAT		60
+#define LOCHNAGAR2_PIN_GF_AIF4_BCLK		61
+#define LOCHNAGAR2_PIN_GF_AIF4_RXDAT		62
+#define LOCHNAGAR2_PIN_GF_AIF4_LRCLK		63
+#define LOCHNAGAR2_PIN_GF_AIF4_TXDAT		64
+#define LOCHNAGAR2_PIN_GF_AIF1_BCLK		65
+#define LOCHNAGAR2_PIN_GF_AIF1_RXDAT		66
+#define LOCHNAGAR2_PIN_GF_AIF1_LRCLK		67
+#define LOCHNAGAR2_PIN_GF_AIF1_TXDAT		68
+#define LOCHNAGAR2_PIN_GF_AIF2_BCLK		69
+#define LOCHNAGAR2_PIN_GF_AIF2_RXDAT		70
+#define LOCHNAGAR2_PIN_GF_AIF2_LRCLK		71
+#define LOCHNAGAR2_PIN_GF_AIF2_TXDAT		72
+#define LOCHNAGAR2_PIN_DSP_UART1_RX		73
+#define LOCHNAGAR2_PIN_DSP_UART1_TX		74
+#define LOCHNAGAR2_PIN_DSP_UART2_RX		75
+#define LOCHNAGAR2_PIN_DSP_UART2_TX		76
+#define LOCHNAGAR2_PIN_GF_UART2_RX		77
+#define LOCHNAGAR2_PIN_GF_UART2_TX		78
+#define LOCHNAGAR2_PIN_USB_UART_RX		79
+#define LOCHNAGAR2_PIN_CDC_PDMCLK1		80
+#define LOCHNAGAR2_PIN_CDC_PDMDAT1		81
+#define LOCHNAGAR2_PIN_CDC_PDMCLK2		82
+#define LOCHNAGAR2_PIN_CDC_PDMDAT2		83
+#define LOCHNAGAR2_PIN_CDC_DMICCLK1		84
+#define LOCHNAGAR2_PIN_CDC_DMICDAT1		85
+#define LOCHNAGAR2_PIN_CDC_DMICCLK2		86
+#define LOCHNAGAR2_PIN_CDC_DMICDAT2		87
+#define LOCHNAGAR2_PIN_CDC_DMICCLK3		88
+#define LOCHNAGAR2_PIN_CDC_DMICDAT3		89
+#define LOCHNAGAR2_PIN_CDC_DMICCLK4		90
+#define LOCHNAGAR2_PIN_CDC_DMICDAT4		91
+#define LOCHNAGAR2_PIN_DSP_DMICCLK1		92
+#define LOCHNAGAR2_PIN_DSP_DMICDAT1		93
+#define LOCHNAGAR2_PIN_DSP_DMICCLK2		94
+#define LOCHNAGAR2_PIN_DSP_DMICDAT2		95
+#define LOCHNAGAR2_PIN_I2C2_SCL			96
+#define LOCHNAGAR2_PIN_I2C2_SDA			97
+#define LOCHNAGAR2_PIN_I2C3_SCL			98
+#define LOCHNAGAR2_PIN_I2C3_SDA			99
+#define LOCHNAGAR2_PIN_I2C4_SCL			100
+#define LOCHNAGAR2_PIN_I2C4_SDA			101
+#define LOCHNAGAR2_PIN_DSP_STANDBY		102
+#define LOCHNAGAR2_PIN_CDC_MCLK1		103
+#define LOCHNAGAR2_PIN_CDC_MCLK2		104
+#define LOCHNAGAR2_PIN_DSP_CLKIN		105
+#define LOCHNAGAR2_PIN_PSIA1_MCLK		106
+#define LOCHNAGAR2_PIN_PSIA2_MCLK		107
+#define LOCHNAGAR2_PIN_GF_GPIO1			108
+#define LOCHNAGAR2_PIN_GF_GPIO5			109
+#define LOCHNAGAR2_PIN_DSP_GPIO20		110
+#define LOCHNAGAR2_PIN_NUM_GPIOS		111
+
+#endif