[PATCH 0/7] Add dts for Rpi4 + Cirrus Lochnagar and codecs

[PATCH 0/7] Add dts for Rpi4 + Cirrus Lochnagar and codecs

[Richard Fitzgerald]

This set of patches provides support for using the Cirrus Logic
Lochnagar audio development platform plus Cirrus Logic Madera/Arizona
codecs with the simple-card machine driver and a Raspberry Pi4. The
ultimate aim is to provide the dts file but some updates are needed to
the simple-card machine driver.

Patches are needed to simple-card to enable support for setting the
component sysclks and plls. The codec sysclks and plls cannot be
placed under the clock framwork because they are I2C/SPI-connected
peripherals and access to the registers would cause a nested get
of the I2C/SPI bus clock. The clock framework does not support this
and it would result in a deadlock.

Richard Fitzgerald (7):
  of: base: Add of_count_phandle_with_fixed_args()
  ASoC: simple-card: Add plls and sysclks DT schema
  ASoC: simple-card: Support setting component plls and sysclks
  ASoC: arizona: Allow codecs to be selected from kernel config
  ASoC: madera: Allow codecs to be selected from kernel config
  ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2
  MAINTAINERS: Add dts for Cirrus Logic Lochnagar on RPi4

 .../bindings/sound/simple-card.yaml           |   56 +
 MAINTAINERS                                   |    1 +
 arch/arm/boot/dts/Makefile                    |    1 +
 .../dts/bcm2711-rpi4b-cirrus-lochnagar.dts    | 1296 +++++++++++++++++
 drivers/of/base.c                             |   42 +
 include/linux/of.h                            |    9 +
 include/sound/simple_card_utils.h             |   24 +
 sound/soc/codecs/Kconfig                      |   18 +-
 sound/soc/generic/simple-card-utils.c         |  184 +++
 sound/soc/generic/simple-card.c               |   14 +-
 10 files changed, 1635 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm/boot/dts/bcm2711-rpi4b-cirrus-lochnagar.dts

-- 
2.20.1

[PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args()

[Richard Fitzgerald]

Add an equivalent of of_count_phandle_with_args() for fixed argument
sets, to pair with of_parse_phandle_with_fixed_args().

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 drivers/of/base.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |  9 +++++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ea44fea99813..45d8b0e65345 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
 }
 EXPORT_SYMBOL(of_count_phandle_with_args);
 
+/**
+ * of_count_phandle_with_fixed_args() - Find the number of phandles references in a property
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cell_count: number of argument cells following the phandle
+ *
+ * Returns the number of phandle + argument tuples within a property. It
+ * is a typical pattern to encode a list of phandle and variable
+ * arguments into a single property.
+ */
+int of_count_phandle_with_fixed_args(const struct device_node *np,
+				     const char *list_name,
+				     int cells_count)
+{
+	struct of_phandle_iterator it;
+	int rc, cur_index = 0;
+
+	if (!cells_count) {
+		const __be32 *list;
+		int size;
+
+		list = of_get_property(np, list_name, &size);
+		if (!list)
+			return -ENOENT;
+
+		return size / sizeof(*list);
+	}
+
+	rc = of_phandle_iterator_init(&it, np, list_name, NULL, cells_count);
+	if (rc)
+		return rc;
+
+	while ((rc = of_phandle_iterator_next(&it)) == 0)
+		cur_index += 1;
+
+	if (rc != -ENOENT)
+		return rc;
+
+	return cur_index;
+}
+EXPORT_SYMBOL(of_count_phandle_with_fixed_args);
+
 /**
  * __of_add_property - Add a property to a node without lock operations
  */
diff --git a/include/linux/of.h b/include/linux/of.h
index 5cf7ae0465d1..9f315da4e9da 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -377,6 +377,8 @@ extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	struct of_phandle_args *out_args);
 extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
+extern int of_count_phandle_with_fixed_args(const struct device_node *np,
+	const char *list_name, int cells_count);
 
 /* phandle iterator functions */
 extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
@@ -886,6 +888,13 @@ static inline int of_count_phandle_with_args(struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_count_phandle_with_fixed_args(const struct device_node *np,
+						   const char *list_name,
+						   int cells_count)
+{
+	return -ENOSYS;
+}
+
 static inline int of_phandle_iterator_init(struct of_phandle_iterator *it,
 					   const struct device_node *np,
 					   const char *list_name,
-- 
2.20.1

Re: [PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args()

[Rob Herring]

On Wed, Oct 14, 2020 at 9:54 AM Richard Fitzgerald
<rf@opensource.cirrus.com> wrote:
>
> Add an equivalent of of_count_phandle_with_args() for fixed argument
> sets, to pair with of_parse_phandle_with_fixed_args().
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  drivers/of/base.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  9 +++++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ea44fea99813..45d8b0e65345 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
>  }
>  EXPORT_SYMBOL(of_count_phandle_with_args);
>
> +/**
> + * of_count_phandle_with_fixed_args() - Find the number of phandles references in a property
> + * @np:                pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cell_count: number of argument cells following the phandle
> + *
> + * Returns the number of phandle + argument tuples within a property. It
> + * is a typical pattern to encode a list of phandle and variable
> + * arguments into a single property.
> + */
> +int of_count_phandle_with_fixed_args(const struct device_node *np,
> +                                    const char *list_name,
> +                                    int cells_count)
> +{

Looks to me like you can refactor of_count_phandle_with_args to handle
both case and then make this and of_count_phandle_with_args simple
wrapper functions.

> +       struct of_phandle_iterator it;
> +       int rc, cur_index = 0;
> +
> +       if (!cells_count) {
> +               const __be32 *list;
> +               int size;
> +
> +               list = of_get_property(np, list_name, &size);
> +               if (!list)
> +                       return -ENOENT;
> +
> +               return size / sizeof(*list);
> +       }
> +
> +       rc = of_phandle_iterator_init(&it, np, list_name, NULL, cells_count);
> +       if (rc)
> +               return rc;
> +
> +       while ((rc = of_phandle_iterator_next(&it)) == 0)
> +               cur_index += 1;
> +
> +       if (rc != -ENOENT)
> +               return rc;
> +
> +       return cur_index;
> +}
> +EXPORT_SYMBOL(of_count_phandle_with_fixed_args);
> +
>  /**
>   * __of_add_property - Add a property to a node without lock operations
>   */
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 5cf7ae0465d1..9f315da4e9da 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -377,6 +377,8 @@ extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
>         struct of_phandle_args *out_args);
>  extern int of_count_phandle_with_args(const struct device_node *np,
>         const char *list_name, const char *cells_name);
> +extern int of_count_phandle_with_fixed_args(const struct device_node *np,
> +       const char *list_name, int cells_count);
>
>  /* phandle iterator functions */
>  extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
> @@ -886,6 +888,13 @@ static inline int of_count_phandle_with_args(struct device_node *np,
>         return -ENOSYS;
>  }
>
> +static inline int of_count_phandle_with_fixed_args(const struct device_node *np,
> +                                                  const char *list_name,
> +                                                  int cells_count)
> +{
> +       return -ENOSYS;
> +}
> +
>  static inline int of_phandle_iterator_init(struct of_phandle_iterator *it,
>                                            const struct device_node *np,
>                                            const char *list_name,
> --
> 2.20.1
>

Re: [PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args()

[Robin Murphy]

On 2020-10-14 19:39, Rob Herring wrote:
> On Wed, Oct 14, 2020 at 9:54 AM Richard Fitzgerald
> <rf@opensource.cirrus.com> wrote:
>>
>> Add an equivalent of of_count_phandle_with_args() for fixed argument
>> sets, to pair with of_parse_phandle_with_fixed_args().
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> ---
>>   drivers/of/base.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/of.h |  9 +++++++++
>>   2 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index ea44fea99813..45d8b0e65345 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
>>   }
>>   EXPORT_SYMBOL(of_count_phandle_with_args);
>>
>> +/**
>> + * of_count_phandle_with_fixed_args() - Find the number of phandles references in a property
>> + * @np:                pointer to a device tree node containing a list
>> + * @list_name: property name that contains a list
>> + * @cell_count: number of argument cells following the phandle
>> + *
>> + * Returns the number of phandle + argument tuples within a property. It
>> + * is a typical pattern to encode a list of phandle and variable
>> + * arguments into a single property.
>> + */
>> +int of_count_phandle_with_fixed_args(const struct device_node *np,
>> +                                    const char *list_name,
>> +                                    int cells_count)
>> +{
> 
> Looks to me like you can refactor of_count_phandle_with_args to handle
> both case and then make this and of_count_phandle_with_args simple
> wrapper functions.

Although for just counting the number of phandles each with n arguments 
that a property contains, isn't that simply a case of dividing the 
property length by n + 1? The phandles themselves will be validated by 
any subsequent of_parse_phandle*() call anyway, so there doesn't seem 
much point in doing more work then necessary here.

>> +       struct of_phandle_iterator it;
>> +       int rc, cur_index = 0;
>> +
>> +       if (!cells_count) {
>> +               const __be32 *list;
>> +               int size;
>> +
>> +               list = of_get_property(np, list_name, &size);
>> +               if (!list)
>> +                       return -ENOENT;
>> +
>> +               return size / sizeof(*list);

Case in point - if it's OK to do exactly that for n == 0, then clearly 
we're *aren't* fussed about validating anything, so the n > 0 code below 
is nothing more than a massively expensive way to check for a nonzero 
remainder :/

Robin.

>> +       }
>> +
>> +       rc = of_phandle_iterator_init(&it, np, list_name, NULL, cells_count);
>> +       if (rc)
>> +               return rc;
>> +
>> +       while ((rc = of_phandle_iterator_next(&it)) == 0)
>> +               cur_index += 1;
>> +
>> +       if (rc != -ENOENT)
>> +               return rc;
>> +
>> +       return cur_index;
>> +}
>> +EXPORT_SYMBOL(of_count_phandle_with_fixed_args);
>> +
>>   /**
>>    * __of_add_property - Add a property to a node without lock operations
>>    */
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 5cf7ae0465d1..9f315da4e9da 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -377,6 +377,8 @@ extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
>>          struct of_phandle_args *out_args);
>>   extern int of_count_phandle_with_args(const struct device_node *np,
>>          const char *list_name, const char *cells_name);
>> +extern int of_count_phandle_with_fixed_args(const struct device_node *np,
>> +       const char *list_name, int cells_count);
>>
>>   /* phandle iterator functions */
>>   extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
>> @@ -886,6 +888,13 @@ static inline int of_count_phandle_with_args(struct device_node *np,
>>          return -ENOSYS;
>>   }
>>
>> +static inline int of_count_phandle_with_fixed_args(const struct device_node *np,
>> +                                                  const char *list_name,
>> +                                                  int cells_count)
>> +{
>> +       return -ENOSYS;
>> +}
>> +
>>   static inline int of_phandle_iterator_init(struct of_phandle_iterator *it,
>>                                             const struct device_node *np,
>>                                             const char *list_name,
>> --
>> 2.20.1
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args()

[Richard Fitzgerald]

On 15/10/2020 17:52, Robin Murphy wrote:
> On 2020-10-14 19:39, Rob Herring wrote:
>> On Wed, Oct 14, 2020 at 9:54 AM Richard Fitzgerald
>> <rf@opensource.cirrus.com> wrote:
>>>
>>> Add an equivalent of of_count_phandle_with_args() for fixed argument
>>> sets, to pair with of_parse_phandle_with_fixed_args().
>>>
>>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>>> ---
>>>   drivers/of/base.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/of.h |  9 +++++++++
>>>   2 files changed, 51 insertions(+)
>>>
>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>> index ea44fea99813..45d8b0e65345 100644
>>> --- a/drivers/of/base.c
>>> +++ b/drivers/of/base.c
>>> @@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct 
>>> device_node *np, const char *list_na
>>>   }
>>>   EXPORT_SYMBOL(of_count_phandle_with_args);
>>>
>>> +/**
>>> + * of_count_phandle_with_fixed_args() - Find the number of phandles 
>>> references in a property
>>> + * @np:                pointer to a device tree node containing a list
>>> + * @list_name: property name that contains a list
>>> + * @cell_count: number of argument cells following the phandle
>>> + *
>>> + * Returns the number of phandle + argument tuples within a 
>>> property. It
>>> + * is a typical pattern to encode a list of phandle and variable
>>> + * arguments into a single property.
>>> + */
>>> +int of_count_phandle_with_fixed_args(const struct device_node *np,
>>> +                                    const char *list_name,
>>> +                                    int cells_count)
>>> +{
>>
>> Looks to me like you can refactor of_count_phandle_with_args to handle
>> both case and then make this and of_count_phandle_with_args simple
>> wrapper functions.
> 
> Although for just counting the number of phandles each with n arguments 
> that a property contains, isn't that simply a case of dividing the 
> property length by n + 1? The phandles themselves will be validated by 
> any subsequent of_parse_phandle*() call anyway, so there doesn't seem 
> much point in doing more work then necessary here.
> 

As I'm not a DT expert, I'm reluctant to change existing algorithms that
could break everything just for the trivial case of adding a fixed
arguments count. I have a re-worked patch as suggested by Rob that
re-uses the existing counting function for both cases.

>>> +       struct of_phandle_iterator it;
>>> +       int rc, cur_index = 0;
>>> +
>>> +       if (!cells_count) {
>>> +               const __be32 *list;
>>> +               int size;
>>> +
>>> +               list = of_get_property(np, list_name, &size);
>>> +               if (!list)
>>> +                       return -ENOENT;
>>> +
>>> +               return size / sizeof(*list);
> 
> Case in point - if it's OK to do exactly that for n == 0, then clearly 
> we're *aren't* fussed about validating anything, so the n > 0 code below 
> is nothing more than a massively expensive way to check for a nonzero 
> remainder :/
> 
> Robin.
> 
>>> +       }
>>> +
>>> +       rc = of_phandle_iterator_init(&it, np, list_name, NULL, 
>>> cells_count);
>>> +       if (rc)
>>> +               return rc;
>>> +
>>> +       while ((rc = of_phandle_iterator_next(&it)) == 0)
>>> +               cur_index += 1;
>>> +
>>> +       if (rc != -ENOENT)
>>> +               return rc;
>>> +
>>> +       return cur_index;
>>> +}
>>> +EXPORT_SYMBOL(of_count_phandle_with_fixed_args);
>>> +
>>>   /**
>>>    * __of_add_property - Add a property to a node without lock 
>>> operations
>>>    */
>>> diff --git a/include/linux/of.h b/include/linux/of.h
>>> index 5cf7ae0465d1..9f315da4e9da 100644
>>> --- a/include/linux/of.h
>>> +++ b/include/linux/of.h
>>> @@ -377,6 +377,8 @@ extern int of_parse_phandle_with_fixed_args(const 
>>> struct device_node *np,
>>>          struct of_phandle_args *out_args);
>>>   extern int of_count_phandle_with_args(const struct device_node *np,
>>>          const char *list_name, const char *cells_name);
>>> +extern int of_count_phandle_with_fixed_args(const struct device_node 
>>> *np,
>>> +       const char *list_name, int cells_count);
>>>
>>>   /* phandle iterator functions */
>>>   extern int of_phandle_iterator_init(struct of_phandle_iterator *it,
>>> @@ -886,6 +888,13 @@ static inline int 
>>> of_count_phandle_with_args(struct device_node *np,
>>>          return -ENOSYS;
>>>   }
>>>
>>> +static inline int of_count_phandle_with_fixed_args(const struct 
>>> device_node *np,
>>> +                                                  const char 
>>> *list_name,
>>> +                                                  int cells_count)
>>> +{
>>> +       return -ENOSYS;
>>> +}
>>> +
>>>   static inline int of_phandle_iterator_init(struct 
>>> of_phandle_iterator *it,
>>>                                             const struct device_node 
>>> *np,
>>>                                             const char *list_name,
>>> -- 
>>> 2.20.1
>>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>

Re: [PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args()

[Rob Herring]

On Thu, Oct 15, 2020 at 11:52 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-10-14 19:39, Rob Herring wrote:
> > On Wed, Oct 14, 2020 at 9:54 AM Richard Fitzgerald
> > <rf@opensource.cirrus.com> wrote:
> >>
> >> Add an equivalent of of_count_phandle_with_args() for fixed argument
> >> sets, to pair with of_parse_phandle_with_fixed_args().
> >>
> >> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> >> ---
> >>   drivers/of/base.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
> >>   include/linux/of.h |  9 +++++++++
> >>   2 files changed, 51 insertions(+)
> >>
> >> diff --git a/drivers/of/base.c b/drivers/of/base.c
> >> index ea44fea99813..45d8b0e65345 100644
> >> --- a/drivers/of/base.c
> >> +++ b/drivers/of/base.c
> >> @@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
> >>   }
> >>   EXPORT_SYMBOL(of_count_phandle_with_args);
> >>
> >> +/**
> >> + * of_count_phandle_with_fixed_args() - Find the number of phandles references in a property
> >> + * @np:                pointer to a device tree node containing a list
> >> + * @list_name: property name that contains a list
> >> + * @cell_count: number of argument cells following the phandle
> >> + *
> >> + * Returns the number of phandle + argument tuples within a property. It
> >> + * is a typical pattern to encode a list of phandle and variable
> >> + * arguments into a single property.
> >> + */
> >> +int of_count_phandle_with_fixed_args(const struct device_node *np,
> >> +                                    const char *list_name,
> >> +                                    int cells_count)
> >> +{
> >
> > Looks to me like you can refactor of_count_phandle_with_args to handle
> > both case and then make this and of_count_phandle_with_args simple
> > wrapper functions.
>
> Although for just counting the number of phandles each with n arguments
> that a property contains, isn't that simply a case of dividing the
> property length by n + 1? The phandles themselves will be validated by
> any subsequent of_parse_phandle*() call anyway, so there doesn't seem
> much point in doing more work then necessary here.
>
> >> +       struct of_phandle_iterator it;
> >> +       int rc, cur_index = 0;
> >> +
> >> +       if (!cells_count) {
> >> +               const __be32 *list;
> >> +               int size;
> >> +
> >> +               list = of_get_property(np, list_name, &size);
> >> +               if (!list)
> >> +                       return -ENOENT;
> >> +
> >> +               return size / sizeof(*list);
>
> Case in point - if it's OK to do exactly that for n == 0, then clearly
> we're *aren't* fussed about validating anything, so the n > 0 code below
> is nothing more than a massively expensive way to check for a nonzero
> remainder :/

Indeed. We should just generalize this. It can still be refactored to
shared code.

It's probably worthwhile to check for a remainder here IMO.

Rob

Re: [PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args()

[Richard Fitzgerald]

On 16/10/2020 14:31, Rob Herring wrote:
> On Thu, Oct 15, 2020 at 11:52 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2020-10-14 19:39, Rob Herring wrote:
>>> On Wed, Oct 14, 2020 at 9:54 AM Richard Fitzgerald
>>> <rf@opensource.cirrus.com> wrote:
>>>>
>>>> Add an equivalent of of_count_phandle_with_args() for fixed argument
>>>> sets, to pair with of_parse_phandle_with_fixed_args().
>>>>
>>>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>>>> ---
>>>>    drivers/of/base.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>>    include/linux/of.h |  9 +++++++++
>>>>    2 files changed, 51 insertions(+)
>>>>
>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>> index ea44fea99813..45d8b0e65345 100644
>>>> --- a/drivers/of/base.c
>>>> +++ b/drivers/of/base.c
>>>> @@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
>>>>    }
>>>>    EXPORT_SYMBOL(of_count_phandle_with_args);
>>>>
>>>> +/**
>>>> + * of_count_phandle_with_fixed_args() - Find the number of phandles references in a property
>>>> + * @np:                pointer to a device tree node containing a list
>>>> + * @list_name: property name that contains a list
>>>> + * @cell_count: number of argument cells following the phandle
>>>> + *
>>>> + * Returns the number of phandle + argument tuples within a property. It
>>>> + * is a typical pattern to encode a list of phandle and variable
>>>> + * arguments into a single property.
>>>> + */
>>>> +int of_count_phandle_with_fixed_args(const struct device_node *np,
>>>> +                                    const char *list_name,
>>>> +                                    int cells_count)
>>>> +{
>>>
>>> Looks to me like you can refactor of_count_phandle_with_args to handle
>>> both case and then make this and of_count_phandle_with_args simple
>>> wrapper functions.
>>
>> Although for just counting the number of phandles each with n arguments
>> that a property contains, isn't that simply a case of dividing the
>> property length by n + 1? The phandles themselves will be validated by
>> any subsequent of_parse_phandle*() call anyway, so there doesn't seem
>> much point in doing more work then necessary here.
>>
>>>> +       struct of_phandle_iterator it;
>>>> +       int rc, cur_index = 0;
>>>> +
>>>> +       if (!cells_count) {
>>>> +               const __be32 *list;
>>>> +               int size;
>>>> +
>>>> +               list = of_get_property(np, list_name, &size);
>>>> +               if (!list)
>>>> +                       return -ENOENT;
>>>> +
>>>> +               return size / sizeof(*list);
>>
>> Case in point - if it's OK to do exactly that for n == 0, then clearly
>> we're *aren't* fussed about validating anything, so the n > 0 code below
>> is nothing more than a massively expensive way to check for a nonzero
>> remainder :/
> 
> Indeed. We should just generalize this. It can still be refactored to
> shared code.
> 
> It's probably worthwhile to check for a remainder here IMO.
>

Ok, I looked at the implementation of of_phandle_iterator_next() and
it is in fact simply incrementing by 'count' 32-bit words. So as Robin
said the count_phandle_with_x_args()functions could simply divide the
length by count+1.

However, may I suggest that should be done in a separate patch after my
patch to add count_phandle_with_fixed_args()? That way, if replacing the
iteration with the simple length divide causes any unforeseen problems
the patch can just be reverted.

> Rob
> 

[PATCH 2/7] ASoC: simple-card: Add plls and sysclks DT schema

[Richard Fitzgerald]

This adds the two new properties 'plls' and 'sysclks' to the dt bindings
schema document. These add the ability to set values that will be
passed to snd_soc_component_set_sysclk() and snd_soc_component_set_pll().

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 .../bindings/sound/simple-card.yaml           | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
index 35e669020296..48c9a4313d58 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.yaml
+++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
@@ -92,6 +92,25 @@ definitions:
     description: the widget names for which pin switches must be created.
     $ref: /schemas/types.yaml#/definitions/string-array
 
+  plls:
+    description: |
+      A list of component pll settings that will be applied with
+      snd_soc_component_set_pll. Each entry is a phandle to the node of the
+      codec or cpu component, followed by the four arguments id, source,
+      frequency_in, frequency_out. Multiple entries can have the same phandle
+      so that several plls can be set in the same component.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
+  sysclks:
+    description: |
+      A list of component sysclk settings that will be applied with
+      snd_soc_component_set_sysclk. Each entry is a phandle to the node of
+      the codec or cpu component, followed by the four arguments id, source,
+      frequency, direction. Direction is 0 if the clock is an input, 1 if it
+      is an output. Multiple entries can have the same phandle so that several
+      clocks can be set in the same component.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
   format:
     description: audio format.
     items:
@@ -192,6 +211,10 @@ patternProperties:
     $ref: "#/definitions/prefix"
   "^simple-audio-card,pin-switches$":
     $ref: "#/definitions/pin-switches"
+  "^simple-audio-card,plls$":
+    $ref: "#/definitions/plls"
+  "^simple-audio-card,sysclks$":
+    $ref: "#/definitions/sysclks"
   "^simple-audio-card,hp-det-gpio$":
     maxItems: 1
   "^simple-audio-card,mic-det-gpio$":
@@ -489,3 +512,36 @@ examples:
             };
         };
     };
+
+#--------------------
+# Codec with component PLL and SYSCLK settings
+#--------------------
+  - |
+    sound {
+        compatible = "simple-audio-card";
+
+        simple-audio-card,plls = <
+            &cs47l15 1 1 32768 98304000
+        >;
+
+        simple-audio-card,sysclks = <
+            &cs47l15 1 4 98304000 0
+            &cs47l15 8 4 147456000 0
+        >;
+
+        simple-audio-card,format = "i2s";
+        simple-audio-card,bitclock-master = <&cs47l15_codec_dai>;
+        simple-audio-card,frame-master = <&cs47l15_codec_dai>;
+
+        simple-audio-card,cpu {
+            sound-dai = <&axi_i2s0 0>;
+        };
+
+        cs47l15_codec_dai: simple-audio-card,codec {
+            sound-dai = <&cs47l15 0>;
+
+            /* Disable dai_set_sysclk() */
+            system-clock-frequency = <0>;
+            mclk-fs = <0>;
+        };
+    };
-- 
2.20.1

[PATCH 3/7] ASoC: simple-card: Support setting component plls and sysclks

[Richard Fitzgerald]

Some codecs need plls and/or sysclks to be configured using the
snd_soc_component_set_[sysclk|pll] functions. These drivers cannot
necessarily be converted to use the clock framework. If the codec is on
a I2C/SPI bus, a nested clk_get would be needed to enable the bus clock.
But the clock framework does not support nested operations and this would
deadlock. So it isn't possible to implement I2C/SPI connected pll/clock
hardware through the clock framework.

This patch adds new dt properties that list phandles of components with
the pll/sysclk settings to be applied. Multiple settings can be given for
the same phandle to allow for components with multiple clocks and plls.
The plls and sysclks are enabled when the card bia level moves to STANDBY
and disabled when it moved to OFF.

As this is a _simple_ machine driver, the code does not attempt to handle
specifying complex clock ordering interdependencies between components. The
plls and sysclks are applied to a component as it is passed to the card
set_bias_level/set_bias_level_post callbacks. It follows from this that the
order components are configured is the order that they are passed to those
callbacks.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 include/sound/simple_card_utils.h     |  24 ++++
 sound/soc/generic/simple-card-utils.c | 184 ++++++++++++++++++++++++++
 sound/soc/generic/simple-card.c       |  14 +-
 3 files changed, 221 insertions(+), 1 deletion(-)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index 86a1e956991e..67e9034ed807 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -38,6 +38,15 @@ struct asoc_simple_jack {
 	struct snd_soc_jack_gpio gpio;
 };
 
+struct asoc_simple_sysclk_pll {
+	struct device_node *node;
+	int id;
+	int source;
+	unsigned int freq_in;
+	unsigned int freq_out;
+	int dir;
+};
+
 struct asoc_simple_priv {
 	struct snd_soc_card snd_card;
 	struct simple_dai_props {
@@ -56,6 +65,11 @@ struct asoc_simple_priv {
 	struct asoc_simple_dai *dais;
 	struct snd_soc_codec_conf *codec_conf;
 	struct gpio_desc *pa_gpio;
+
+	struct asoc_simple_sysclk_pll *sysclks;
+	int num_sysclks;
+	struct asoc_simple_sysclk_pll *plls;
+	int num_plls;
 };
 #define simple_priv_to_card(priv)	(&(priv)->snd_card)
 #define simple_priv_to_props(priv, i)	((priv)->dai_props + (i))
@@ -94,6 +108,14 @@ void asoc_simple_shutdown(struct snd_pcm_substream *substream);
 int asoc_simple_hw_params(struct snd_pcm_substream *substream,
 			  struct snd_pcm_hw_params *params);
 int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd);
+
+int asoc_simple_set_bias_level(struct snd_soc_card *card,
+			       struct snd_soc_dapm_context *dapm,
+			       enum snd_soc_bias_level level);
+int asoc_simple_set_bias_level_post(struct snd_soc_card *card,
+				    struct snd_soc_dapm_context *dapm,
+				    enum snd_soc_bias_level level);
+
 int asoc_simple_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 				   struct snd_pcm_hw_params *params);
 
@@ -128,6 +150,8 @@ int asoc_simple_parse_widgets(struct snd_soc_card *card,
 				      char *prefix);
 int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
 				   char *prefix);
+int asoc_simple_parse_sysclks(struct asoc_simple_priv *priv, char *prefix);
+int asoc_simple_parse_plls(struct asoc_simple_priv *priv, char *prefix);
 
 int asoc_simple_init_jack(struct snd_soc_card *card,
 			       struct asoc_simple_jack *sjack,
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 6cada4c1e283..6c0675187285 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -399,6 +399,123 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
 }
 EXPORT_SYMBOL_GPL(asoc_simple_dai_init);
 
+static bool asoc_simple_node_is_component(struct snd_soc_component *component,
+					  struct device_node *node)
+{
+	struct device_node *comp_node;
+
+	comp_node = component->dev->of_node;
+	if (!comp_node && component->dev->parent)
+		comp_node = component->dev->parent->of_node;
+
+	return (comp_node == node);
+}
+
+int asoc_simple_set_bias_level(struct snd_soc_card *card,
+			       struct snd_soc_dapm_context *dapm,
+			       enum snd_soc_bias_level level)
+{
+	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_component *component = dapm->component;
+	int i, ret;
+
+	if (!component)
+		return 0;
+
+	switch (level) {
+	case SND_SOC_BIAS_STANDBY:
+		if (dapm->bias_level != SND_SOC_BIAS_OFF)
+			break;
+
+		/* PLLs normally supply SYSCLKs so enable the PLLs first. */
+		for (i = 0; i < priv->num_plls; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->plls[i].node))
+				continue;
+
+			ret = snd_soc_component_set_pll(component,
+							priv->plls[i].id,
+							priv->plls[i].source,
+							priv->plls[i].freq_in,
+							priv->plls[i].freq_out);
+			if (ret) {
+				dev_err(card->dev, "Failed to set pll for %s: %d\n",
+					component->name, ret);
+				return ret;
+			}
+		}
+
+		for (i = 0; i < priv->num_sysclks; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->sysclks[i].node))
+				continue;
+
+			ret = snd_soc_component_set_sysclk(component,
+							   priv->sysclks[i].id,
+							   priv->sysclks[i].source,
+							   priv->sysclks[i].freq_in,
+							   priv->sysclks[i].dir);
+			if (ret) {
+				dev_err(card->dev, "Failed to set sysclk for %s: %d\n",
+					component->name, ret);
+				return ret;
+			}
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_set_bias_level);
+
+int asoc_simple_set_bias_level_post(struct snd_soc_card *card,
+				    struct snd_soc_dapm_context *dapm,
+				    enum snd_soc_bias_level level)
+{
+	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_component *component = dapm->component;
+	int i, ret;
+
+	if (!component)
+		goto out;
+
+	switch (level) {
+	case SND_SOC_BIAS_OFF:
+		for (i = 0; i < priv->num_sysclks; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->sysclks[i].node))
+				continue;
+
+			ret = snd_soc_component_set_sysclk(component,
+							   priv->sysclks[i].id,
+							   0,
+							   0,
+							   priv->sysclks[i].dir);
+			if (ret)
+				dev_warn(card->dev, "Failed to set sysclk for %s: %d\n",
+					 component->name, ret);
+		}
+
+		for (i = 0; i < priv->num_plls; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->plls[i].node))
+				continue;
+
+			ret = snd_soc_component_set_pll(component, priv->plls[i].id, 0, 0, 0);
+			if (ret)
+				dev_warn(card->dev, "Failed to set pll for %s: %d\n",
+					 component->name, ret);
+		}
+		break;
+	default:
+		break;
+	}
+
+out:
+	dapm->bias_level = level;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_set_bias_level_post);
+
 void asoc_simple_canonicalize_platform(struct snd_soc_dai_link *dai_link)
 {
 	/* Assumes platform == cpu */
@@ -433,6 +550,7 @@ EXPORT_SYMBOL_GPL(asoc_simple_canonicalize_cpu);
 
 int asoc_simple_clean_reference(struct snd_soc_card *card)
 {
+	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card);
 	struct snd_soc_dai_link *dai_link;
 	int i;
 
@@ -440,6 +558,13 @@ int asoc_simple_clean_reference(struct snd_soc_card *card)
 		of_node_put(dai_link->cpus->of_node);
 		of_node_put(dai_link->codecs->of_node);
 	}
+
+	for (i = 0; i < priv->num_sysclks; ++i)
+		of_node_put(priv->sysclks[i].node);
+
+	for (i = 0; i < priv->num_plls; ++i)
+		of_node_put(priv->plls[i].node);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(asoc_simple_clean_reference);
@@ -538,6 +663,65 @@ int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(asoc_simple_parse_pin_switches);
 
+static int asoc_simple_parse_sysclks_plls(struct asoc_simple_priv *priv,
+					  char *prefix,
+					  char *prop_root_name,
+					  struct asoc_simple_sysclk_pll **out,
+					  int *out_count)
+{
+	struct device *dev = priv->snd_card.dev;
+	struct device_node *node = dev->of_node;
+	struct of_phandle_args args;
+	int n_elem, i, ret;
+	char prop[128];
+
+	if (!prefix)
+		prefix = "";
+
+	snprintf(prop, sizeof(prop), "%s%s", prefix, prop_root_name);
+	n_elem = of_count_phandle_with_fixed_args(node, prop, 4);
+	if (n_elem == -ENOENT || n_elem == 0) {
+		return 0;
+	} else if (n_elem < 0) {
+		dev_err(dev, "Failed to parse %s: %d\n", prop, n_elem);
+		return n_elem;
+	}
+
+	*out = devm_kcalloc(dev, n_elem, sizeof(**out), GFP_KERNEL);
+	if (!*out)
+		return -ENOMEM;
+	*out_count = n_elem;
+
+	for (i = 0; i < n_elem; ++i) {
+		ret = of_parse_phandle_with_fixed_args(node, prop, 4, i, &args);
+		if (ret < 0)
+			return ret;
+
+		(*out)[i].node		= args.np;
+		(*out)[i].id		= args.args[0];
+		(*out)[i].source	= args.args[1];
+		(*out)[i].freq_in	= args.args[2];
+		(*out)[i].dir		= args.args[3]; /* for sysclks */
+		(*out)[i].freq_out	= args.args[3]; /* for plls */
+	}
+
+	return 0;
+}
+
+int asoc_simple_parse_sysclks(struct asoc_simple_priv *priv, char *prefix)
+{
+	return asoc_simple_parse_sysclks_plls(priv, prefix, "sysclks",
+					      &priv->sysclks, &priv->num_sysclks);
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_sysclks);
+
+int asoc_simple_parse_plls(struct asoc_simple_priv *priv, char *prefix)
+{
+	return asoc_simple_parse_sysclks_plls(priv, prefix, "plls",
+					      &priv->plls, &priv->num_plls);
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_plls);
+
 int asoc_simple_init_jack(struct snd_soc_card *card,
 			  struct asoc_simple_jack *sjack,
 			  int is_hp, char *prefix,
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 75365c7bb393..52c5f737f350 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -475,6 +475,14 @@ static int simple_parse_of(struct asoc_simple_priv *priv)
 
 	ret = snd_soc_of_parse_aux_devs(card, PREFIX "aux-devs");
 
+	ret = asoc_simple_parse_sysclks(priv, PREFIX);
+	if (ret < 0)
+		return ret;
+
+	ret = asoc_simple_parse_plls(priv, PREFIX);
+	if (ret < 0)
+		return ret;
+
 	return ret;
 }
 
@@ -604,6 +612,7 @@ static int asoc_simple_probe(struct platform_device *pdev)
 	card->owner		= THIS_MODULE;
 	card->dev		= dev;
 	card->probe		= simple_soc_probe;
+	snd_soc_card_set_drvdata(card, priv);
 
 	memset(&li, 0, sizeof(li));
 	simple_get_dais_count(priv, &li);
@@ -672,7 +681,10 @@ static int asoc_simple_probe(struct platform_device *pdev)
 					sizeof(*dai_props->codec_dai));
 	}
 
-	snd_soc_card_set_drvdata(card, priv);
+	if (priv->num_sysclks || priv->num_plls) {
+		card->set_bias_level = asoc_simple_set_bias_level;
+		card->set_bias_level_post = asoc_simple_set_bias_level_post;
+	}
 
 	asoc_simple_debug_info(priv);
 
-- 
2.20.1

[PATCH 4/7] ASoC: arizona: Allow codecs to be selected from kernel config

[Richard Fitzgerald]

The codec Kconfig options were hidden and intended to be selected by
the machine driver that requires them. But that means having either a
dedicated machine driver or building all codecs.

This patch makes the Kconfig options visible so that they can be
selected independently of the machine driver, allowing the codec to be
used with simple-card and other machine drivers that are not hardcoded
to use a fixed set of codecs.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/Kconfig | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 34c6dd04b85a..4a8d503bdd38 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -661,7 +661,7 @@ config SND_SOC_CS47L15
 	tristate
 
 config SND_SOC_CS47L24
-	tristate
+	tristate "Cirrus Logic CS47L24 CODEC"
 	depends on MFD_CS47L24
 
 config SND_SOC_CS47L35
@@ -1484,11 +1484,11 @@ config SND_SOC_WM5100
 	depends on I2C
 
 config SND_SOC_WM5102
-	tristate
+	tristate "Cirrus Logic WM5102 CODEC"
 	depends on MFD_WM5102
 
 config SND_SOC_WM5110
-	tristate
+	tristate "Cirrus Logic WM5110 CODEC"
 	depends on MFD_WM5110
 
 config SND_SOC_WM8350
@@ -1657,7 +1657,7 @@ config SND_SOC_WM8997
 	depends on MFD_WM8997
 
 config SND_SOC_WM8998
-	tristate
+	tristate "Cirrus Logic WM8998 CODEC"
 	depends on MFD_WM8998
 
 config SND_SOC_WM9081
-- 
2.20.1

Re: [PATCH 4/7] ASoC: arizona: Allow codecs to be selected from kernel config

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3185 bytes --]

Hi Richard,

I love your patch! Perhaps something to improve:

[auto build test WARNING on asoc/for-next]
[also build test WARNING on robh/for-next sound/for-next v5.9 next-20201013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Richard-Fitzgerald/Add-dts-for-Rpi4-Cirrus-Lochnagar-and-codecs/20201014-225648
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: xtensa-randconfig-s032-20201014 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-rc1-dirty
        # https://github.com/0day-ci/linux/commit/7ddf8ce197a5426e13fe9422a3ed17f0b02a94df
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Richard-Fitzgerald/Add-dts-for-Rpi4-Cirrus-Lochnagar-and-codecs/20201014-225648
        git checkout 7ddf8ce197a5426e13fe9422a3ed17f0b02a94df
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> sound/soc/codecs/wm5102.c:687:35: sparse: sparse: cast to restricted __be16

vim +687 sound/soc/codecs/wm5102.c

cc9e92431ee9c7f Charles Keepax     2014-06-06  677  
cc9e92431ee9c7f Charles Keepax     2014-06-06  678  static int wm5102_out_comp_coeff_put(struct snd_kcontrol *kcontrol,
cc9e92431ee9c7f Charles Keepax     2014-06-06  679  				     struct snd_ctl_elem_value *ucontrol)
cc9e92431ee9c7f Charles Keepax     2014-06-06  680  {
0fe1daa6663ae94 Kuninori Morimoto  2018-02-13  681  	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
0fe1daa6663ae94 Kuninori Morimoto  2018-02-13  682  	struct arizona *arizona = dev_get_drvdata(component->dev->parent);
cc9e92431ee9c7f Charles Keepax     2014-06-06  683  
d74bcaaeb668261 Lars-Peter Clausen 2014-11-09  684  	mutex_lock(&arizona->dac_comp_lock);
cc9e92431ee9c7f Charles Keepax     2014-06-06  685  	memcpy(&arizona->dac_comp_coeff, ucontrol->value.bytes.data,
cc9e92431ee9c7f Charles Keepax     2014-06-06  686  	       sizeof(arizona->dac_comp_coeff));
cc9e92431ee9c7f Charles Keepax     2014-06-06 @687  	arizona->dac_comp_coeff = be16_to_cpu(arizona->dac_comp_coeff);
d74bcaaeb668261 Lars-Peter Clausen 2014-11-09  688  	mutex_unlock(&arizona->dac_comp_lock);
cc9e92431ee9c7f Charles Keepax     2014-06-06  689  
cc9e92431ee9c7f Charles Keepax     2014-06-06  690  	return 0;
cc9e92431ee9c7f Charles Keepax     2014-06-06  691  }
cc9e92431ee9c7f Charles Keepax     2014-06-06  692  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25448 bytes --]

[PATCH 5/7] ASoC: madera: Allow codecs to be selected from kernel config

[Richard Fitzgerald]

The codec Kconfig options were hidden and intended to be selected by
the machine driver that requires them. But that means having either a
dedicated machine driver or building all codecs.

This patch makes the Kconfig options visible so that they can be
selected independently of the machine driver, allowing the codec to be
used with simple-card and other machine drivers that are not hardcoded
to use a fixed set of codecs.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 4a8d503bdd38..e0aed608dbc3 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -658,23 +658,23 @@ config SND_SOC_CS4349
 	depends on I2C
 
 config SND_SOC_CS47L15
-	tristate
+	tristate "Cirrus Logic CS47L15 CODEC"
 
 config SND_SOC_CS47L24
 	tristate "Cirrus Logic CS47L24 CODEC"
 	depends on MFD_CS47L24
 
 config SND_SOC_CS47L35
-	tristate
+	tristate "Cirrus Logic CS47L35 CODEC"
 
 config SND_SOC_CS47L85
-	tristate
+	tristate "Cirrus Logic CS47L85 CODEC"
 
 config SND_SOC_CS47L90
-	tristate
+	tristate "Cirrus Logic CS47L90 CODEC"
 
 config SND_SOC_CS47L92
-	tristate
+	tristate "Cirrus Logic CS47L92 CODEC"
 
 # Cirrus Logic Quad-Channel ADC
 config SND_SOC_CS53L30
-- 
2.20.1

[PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2

[Richard Fitzgerald]

This is based on the default bcm2711-rpi-4-b.dts.

Configurations are provided for Cirrus Logic codecs CS42L92, CS47L15,
CS47L24, CS47L35, CS47L90 and WM8998.

For each codec there is a sound node and a codec device node and both
default to disabled. Enable the pair for the codec in use.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 arch/arm/boot/dts/Makefile                    |    1 +
 .../dts/bcm2711-rpi4b-cirrus-lochnagar.dts    | 1296 +++++++++++++++++
 2 files changed, 1297 insertions(+)
 create mode 100644 arch/arm/boot/dts/bcm2711-rpi4b-cirrus-lochnagar.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 4572db3fa5ae..53db1be9552d 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -92,6 +92,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
 	bcm2837-rpi-3-b-plus.dtb \
 	bcm2837-rpi-cm3-io3.dtb \
 	bcm2711-rpi-4-b.dtb \
+	bcm2711-rpi4b-cirrus-lochnagar.dtb \
 	bcm2835-rpi-zero.dtb \
 	bcm2835-rpi-zero-w.dtb
 dtb-$(CONFIG_ARCH_BCM_5301X) += \
diff --git a/arch/arm/boot/dts/bcm2711-rpi4b-cirrus-lochnagar.dts b/arch/arm/boot/dts/bcm2711-rpi4b-cirrus-lochnagar.dts
new file mode 100644
index 000000000000..80e31ea39af5
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2711-rpi4b-cirrus-lochnagar.dts
@@ -0,0 +1,1296 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+#include "bcm2711.dtsi"
+#include "bcm2835-rpi.dtsi"
+#include "bcm283x-rpi-usb-peripheral.dtsi"
+#include <dt-bindings/clk/lochnagar.h>
+#include <dt-bindings/mfd/arizona.h>
+#include <dt-bindings/pinctrl/bcm2835.h>
+#include <dt-bindings/pinctrl/lochnagar.h>
+#include <dt-bindings/sound/madera.h>
+
+/ {
+	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
+	model = "Raspberry Pi 4 Model B";
+
+	chosen {
+		/* 8250 auxiliary UART instead of pl011 */
+		stdout-path = "serial1:115200n8";
+	};
+
+	/* Will be filled by the bootloader */
+	memory@0 {
+		device_type = "memory";
+		reg = <0 0 0>;
+	};
+
+	aliases {
+		emmc2bus = &emmc2bus;
+		ethernet0 = &genet;
+		pcie0 = &pcie0;
+	};
+
+	leds {
+		act {
+			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
+		};
+
+		pwr {
+			label = "PWR";
+			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
+			default-state = "keep";
+			linux,default-trigger = "default-on";
+		};
+	};
+
+	wifi_pwrseq: wifi-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&expgpio 1 GPIO_ACTIVE_LOW>;
+	};
+
+	sd_io_1v8_reg: sd_io_1v8_reg {
+		compatible = "regulator-gpio";
+		regulator-name = "vdd-sd-io";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+		regulator-settling-time-us = <5000>;
+		gpios = <&expgpio 4 GPIO_ACTIVE_HIGH>;
+		states = <1800000 0x1
+			  3300000 0x0>;
+		status = "okay";
+	};
+
+	sd_vcc_reg: sd_vcc_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc-sd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		enable-active-high;
+		gpio = <&expgpio 6 GPIO_ACTIVE_HIGH>;
+	};
+
+	wallvdd: wallvdd@0 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "WALL_VDD_5V";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	sound-cs47l15 {
+		status = "disabled";
+
+		compatible = "simple-audio-card";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "Cirrus Logic CS47L15";
+
+		simple-audio-card,sysclks = <
+			&cs47l15 1 4 98304000 0
+			&cs47l15 8 4 147456000 0
+		>;
+		simple-audio-card,plls = <
+			&cs47l15 1 0 24576000 98304000
+		>;
+
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack";
+
+		simple-audio-card,routing =
+				"Microphone Jack", "MICBIAS1A",
+				"Microphone Jack", "MICBIAS1B",
+				"IN1BRN", "Microphone Jack",
+				"IN1BRP", "Microphone Jack",
+				"Headphone Jack", "HPOUTL",
+				"Headphone Jack", "HPOUTR";
+
+		/* Host->Codec I2S */
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&cs47l15_codec_dai>;
+			frame-master = <&cs47l15_codec_dai>;
+
+			cpu {
+				sound-dai = <&i2s 0>;
+			};
+
+			cs47l15_codec_dai: codec {
+				sound-dai = <&cs47l15 0>;
+
+				/* Prevent calls to dai_set_sysclk() */
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Debug trace compressed stream */
+		simple-audio-card,dai-link@1 {
+			reg = <1>;
+
+			cpu {
+				sound-dai = <&cs47l15 3>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l15 4>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+	};
+
+	sound-cs47l24 {
+		status = "disabled";
+
+		compatible = "simple-audio-card";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "Cirrus Logic CS47L24";
+
+		simple-audio-card,sysclks = <
+			&cs47l24 1 4 147456000 0
+		>;
+		simple-audio-card,plls = <
+			&cs47l24 1 0 24576000 147456000	/* fll1 */
+			&cs47l24 3 1 32768 0		/* fll1 refclk */
+		>;
+
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack";
+
+		simple-audio-card,routing =
+				"Microphone Jack", "MICBIAS1",
+				"Microphone Jack", "MICBIAS2",
+				"IN1L", "Microphone Jack",
+				"IN1R", "Microphone Jack",
+				"Headphone Jack", "HPOUT1L",
+				"Headphone Jack", "HPOUT1R";
+
+		/* Host->Codec I2S */
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&cs47l24_codec_dai>;
+			frame-master = <&cs47l24_codec_dai>;
+
+			cpu {
+				sound-dai = <&i2s 0>;
+			};
+
+			cs47l24_codec_dai: codec {
+				sound-dai = <&cs47l24 0>;
+
+				/* Prevent calls to dai_set_sysclk() */
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Debug trace compressed stream */
+		simple-audio-card,dai-link@1 {
+			reg = <1>;
+
+			cpu {
+				sound-dai = <&cs47l24 5>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l24 6>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Voice control compressed stream */
+		simple-audio-card,dai-link@2 {
+			reg = <2>;
+
+			cpu {
+				sound-dai = <&cs47l24 3>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l24 4>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+	};
+
+	sound-cs47l35 {
+		status = "disabled";
+
+		compatible = "simple-audio-card";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "Cirrus Logic CS47L35";
+
+		simple-audio-card,sysclks = <
+			&cs47l35 1 4 98304000 0
+			&cs47l35 8 4 147456000 0
+		>;
+		simple-audio-card,plls = <
+			&cs47l35 1 0 24576000 98304000
+		>;
+
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack";
+
+		simple-audio-card,routing =
+				"Microphone Jack", "MICBIAS1A",
+				"Microphone Jack", "MICBIAS2A",
+				"IN1BLN", "Microphone Jack",
+				"IN1BLP", "Microphone Jack",
+				"Headphone Jack", "HPOUTL",
+				"Headphone Jack", "HPOUTR";
+
+		/* Host->Codec I2S */
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&cs47l35_codec_dai>;
+			frame-master = <&cs47l35_codec_dai>;
+
+			cpu {
+				sound-dai = <&i2s 0>;
+			};
+
+			cs47l35_codec_dai: codec {
+				sound-dai = <&cs47l35 0>;
+
+				/* Prevent calls to dai_set_sysclk() */
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Debug trace compressed stream */
+		simple-audio-card,dai-link@1 {
+			reg = <1>;
+
+			cpu {
+				sound-dai = <&cs47l35 7>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l35 8>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Voice control compressed stream */
+		simple-audio-card,dai-link@2 {
+			reg = <2>;
+
+			cpu {
+				sound-dai = <&cs47l35 5>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l35 6>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+	};
+
+	sound-cs47l90 {
+		status = "disabled";
+
+		compatible = "simple-audio-card";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "Cirrus Logic CS47L90";
+
+		simple-audio-card,sysclks = <
+			&cs47l90 1 4 98304000 0
+			&cs47l90 8 4 147456000 0
+		>;
+		simple-audio-card,plls = <
+			&cs47l90 1 0 24576000 98304000
+		>;
+
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack";
+
+		simple-audio-card,routing =
+				"Microphone Jack", "MICBIAS1A",
+				"Microphone Jack", "MICBIAS2A",
+				"IN1BRN", "Microphone Jack",
+				"IN1BRP", "Microphone Jack",
+				"Headphone Jack", "HPOUT1L",
+				"Headphone Jack", "HPOUT1R";
+
+		/* Host->Codec I2S */
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&cs47l90_codec_dai>;
+			frame-master = <&cs47l90_codec_dai>;
+
+			cpu {
+				sound-dai = <&i2s 0>;
+			};
+
+			cs47l90_codec_dai: codec {
+				sound-dai = <&cs47l90 0>;
+
+				/* Prevent calls to dai_set_sysclk() */
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Debug trace compressed stream */
+		simple-audio-card,dai-link@1 {
+			reg = <1>;
+
+			cpu {
+				sound-dai = <&cs47l90 9>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l90 10>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+
+		/* Voice control compressed stream */
+		simple-audio-card,dai-link@2 {
+			reg = <2>;
+
+			cpu {
+				sound-dai = <&cs47l90 7>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+
+			codec {
+				sound-dai = <&cs47l90 8>;
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+	};
+
+	sound-cs42l92 {
+		status = "disabled";
+
+		compatible = "simple-audio-card";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "Cirrus Logic CS42L92";
+
+		simple-audio-card,sysclks = <
+			&cs42l92 1 4 98304000 0
+		>;
+		simple-audio-card,plls = <
+			&cs42l92 1 0 24576000 98304000
+		>;
+
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack";
+
+		simple-audio-card,routing =
+				"Microphone Jack", "MICBIAS1A",
+				"Microphone Jack", "MICBIAS1B",
+				"IN1BLN", "Microphone Jack",
+				"IN1BLP", "Microphone Jack",
+				"Headphone Jack", "HPOUT1L",
+				"Headphone Jack", "HPOUT1R";
+
+		/* Host->Codec I2S */
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&cs42l92_codec_dai>;
+			frame-master = <&cs42l92_codec_dai>;
+
+			cpu {
+				sound-dai = <&i2s 0>;
+			};
+
+			cs42l92_codec_dai: codec {
+				sound-dai = <&cs42l92 0>;
+
+				/* Prevent calls to dai_set_sysclk() */
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+	};
+
+	sound-wm8998 {
+		status = "disabled";
+
+		compatible = "simple-audio-card";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "Cirrus Logic WM8998";
+
+		simple-audio-card,sysclks = <
+			&wm8998 1 4 49152000 0
+		>;
+		simple-audio-card,plls = <
+			&wm8998 1 0 24576000 49152000
+			&wm8998 3 1 32768 0
+		>;
+
+		simple-audio-card,widgets =
+			"Microphone", "Microphone Jack",
+			"Headphone", "Headphone Jack";
+
+		simple-audio-card,routing =
+				"Microphone Jack", "MICBIAS1",
+				"Microphone Jack", "MICBIAS1",
+				"IN2B", "Microphone Jack",
+				"Headphone Jack", "HPOUTL",
+				"Headphone Jack", "HPOUTR";
+
+		/* Host->Codec I2S */
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&wm8998_codec_dai>;
+			frame-master = <&wm8998_codec_dai>;
+
+			cpu {
+				sound-dai = <&i2s 0>;
+			};
+
+			wm8998_codec_dai: codec {
+				sound-dai = <&wm8998 0>;
+
+				/* Prevent calls to dai_set_sysclk() */
+				system-clock-frequency = <0>;
+				mclk-fs = <0>;
+			};
+		};
+	};
+};
+
+&firmware {
+	firmware_clocks: clocks {
+		compatible = "raspberrypi,firmware-clocks";
+		#clock-cells = <1>;
+	};
+
+	expgpio: gpio {
+		compatible = "raspberrypi,firmware-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-line-names = "BT_ON",
+				  "WL_ON",
+				  "PWR_LED_OFF",
+				  "GLOBAL_RESET",
+				  "VDD_SD_IO_SEL",
+				  "CAM_GPIO",
+				  "SD_PWR_ON",
+				  "";
+		status = "okay";
+	};
+};
+
+&gpio {
+	/*
+	 * Parts taken from rpi_SCH_4b_4p0_reduced.pdf and
+	 * the official GPU firmware DT blob.
+	 *
+	 * Legend:
+	 * "FOO" = GPIO line named "FOO" on the schematic
+	 * "FOO_N" = GPIO line named "FOO" on schematic, active low
+	 */
+	gpio-line-names = "ID_SDA",
+			  "ID_SCL",
+			  "SDA1",
+			  "SCL1",
+			  "GPIO_GCLK",
+			  "GPIO5",
+			  "GPIO6",
+			  "SPI_CE1_N",
+			  "SPI_CE0_N",
+			  "SPI_MISO",
+			  "SPI_MOSI",
+			  "SPI_SCLK",
+			  "GPIO12",
+			  "GPIO13",
+			  /* Serial port */
+			  "TXD1",
+			  "RXD1",
+			  "GPIO16",
+			  "GPIO17",
+			  "GPIO18",
+			  "GPIO19",
+			  "GPIO20",
+			  "GPIO21",
+			  "GPIO22",
+			  "GPIO23",
+			  "GPIO24",
+			  "GPIO25",
+			  "GPIO26",
+			  "GPIO27",
+			  "RGMII_MDIO",
+			  "RGMIO_MDC",
+			  /* Used by BT module */
+			  "CTS0",
+			  "RTS0",
+			  "TXD0",
+			  "RXD0",
+			  /* Used by Wifi */
+			  "SD1_CLK",
+			  "SD1_CMD",
+			  "SD1_DATA0",
+			  "SD1_DATA1",
+			  "SD1_DATA2",
+			  "SD1_DATA3",
+			  /* Shared with SPI flash */
+			  "PWM0_MISO",
+			  "PWM1_MOSI",
+			  "STATUS_LED_G_CLK",
+			  "SPIFLASH_CE_N",
+			  "SDA0",
+			  "SCL0",
+			  "RGMII_RXCLK",
+			  "RGMII_RXCTL",
+			  "RGMII_RXD0",
+			  "RGMII_RXD1",
+			  "RGMII_RXD2",
+			  "RGMII_RXD3",
+			  "RGMII_TXCLK",
+			  "RGMII_TXCTL",
+			  "RGMII_TXD0",
+			  "RGMII_TXD1",
+			  "RGMII_TXD2",
+			  "RGMII_TXD3";
+
+	lochnagar_present: lochnagar_present {
+		brcm,pins = <22>;
+		brcm,function = <BCM2835_FSEL_GPIO_OUT>;
+	};
+
+	lochnagar_reset: lochnagar_reset {
+		brcm,pins = <24>;
+		brcm,function = <BCM2835_FSEL_GPIO_OUT>;
+	};
+
+	cdc_irq: cdc_irq {
+		brcm,pins = <27>;
+		brcm,function = <BCM2835_FSEL_GPIO_IN>;
+	};
+
+	spi_pins: spi_pins {
+		brcm,pins = <9 10 11>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+
+	spi_cs: spi_cs {
+		brcm,pins = <7 8>;
+		brcm,function = <BCM2835_FSEL_GPIO_OUT>;
+	};
+
+	i2s_pins: i2s_pins {
+		brcm,pins = <18 19 20 21>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+};
+
+&pwm1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm1_0_gpio40 &pwm1_1_gpio41>;
+	status = "okay";
+};
+
+/* SDHCI is used to control the SDIO for wireless */
+&sdhci {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&emmc_gpio34>;
+	bus-width = <4>;
+	non-removable;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	status = "disabled";
+
+	brcmf: wifi@1 {
+		reg = <1>;
+		compatible = "brcm,bcm4329-fmac";
+	};
+};
+
+/* EMMC2 is used to drive the SD card */
+&emmc2 {
+	vqmmc-supply = <&sd_io_1v8_reg>;
+	vmmc-supply = <&sd_vcc_reg>;
+	broken-cd;
+	status = "okay";
+};
+
+&genet {
+	phy-handle = <&phy1>;
+	phy-mode = "rgmii-rxid";
+	status = "okay";
+};
+
+&genet_mdio {
+	phy1: ethernet-phy@1 {
+		/* No PHY interrupt */
+		reg = <0x1>;
+	};
+};
+
+/* uart0 communicates with the BT module */
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ctsrts_gpio30 &uart0_gpio32>;
+	uart-has-rtscts;
+	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm43438-bt";
+		max-speed = <2000000>;
+		shutdown-gpios = <&expgpio 0 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+/* uart1 is mapped to the pin header */
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_gpio14>;
+	status = "okay";
+};
+
+&vchiq {
+	interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&i2s {
+	status = "okay";
+	#sound-dai-cells = <1>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2s_pins>;
+	dmas = <&dma 2>, <&dma 3>;
+	dma-names = "tx", "rx";
+};
+
+&i2c1 {
+	status = "okay";
+
+	lochnagar: lochnagar@22 {
+		status = "okay";
+
+		compatible = "cirrus,lochnagar2";
+		reg = <0x22>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&lochnagar_present &lochnagar_reset>;
+
+		reset-gpio = <&gpio 24 0>;
+		present-gpio = <&gpio 22 0>;
+
+		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>;
+		};
+
+		clk_pmic: clk_pmic {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <32768>;
+		};
+		clk_24m: clk_24m {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <24576000>;
+		};
+
+		lochnagar_clk: clk {
+			compatible = "cirrus,lochnagar2-clk";
+
+			#clock-cells = <1>;
+
+			clocks = <&clk_pmic>, <&clk_24m>;
+			clock-names = "ln-pmic-32k", "ln-clk-24m";
+
+			assigned-clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+					  <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>,
+					  <&lochnagar_clk LOCHNAGAR_SOUNDCARD_MCLK>;
+			assigned-clock-parents = <&clk_24m>,
+						 <&clk_pmic>,
+						 <&clk_24m>;
+		};
+
+		lochnagar_pin: pin {
+			compatible = "cirrus,lochnagar-pinctrl";
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&lochnagar_pin 0 0 LOCHNAGAR2_PIN_NUM_GPIOS>;
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&pin_settings>;
+
+			pin_settings: pin_settings {
+				rpi_aif {
+					input-enable;
+					groups = "gf-aif1";
+					function = "codec-aif1";
+				};
+				codec_aif1 {
+					output-master;
+					groups = "codec-aif1";
+					function = "gf-aif1";
+				};
+				sc_codec_aif {
+					output-enable;
+					groups = "codec-aif2";
+					function = "soundcard-aif";
+				};
+				sc_lochnagar_aif {
+					input-enable;
+					groups = "soundcard-aif";
+					function = "codec-aif2";
+				};
+			};
+		};
+
+		lochnagar_hwmon: hwmon {
+			compatible = "cirrus,lochnagar2-hwmon";
+		};
+
+		lochnagar_micvdd: MICVDD {
+			compatible = "cirrus,lochnagar2-micvdd";
+
+			SYSVDD-supply = <&wallvdd>;
+
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+		};
+
+		MIC1VDD {
+			compatible = "cirrus,lochnagar2-mic1vdd";
+
+			regulator-always-on;
+			cirrus,micbias-input = <2>;
+		};
+
+		MIC2VDD {
+			compatible = "cirrus,lochnagar2-mic2vdd";
+
+			regulator-always-on;
+			cirrus,micbias-input = <3>;
+		};
+
+		lochnagar_vddcore: VDDCORE {
+			compatible = "cirrus,lochnagar2-vddcore";
+
+			SYSVDD-supply = <&wallvdd>;
+
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+
+		lochnagar_sc: soundcard {
+			compatible = "cirrus,lochnagar2-soundcard";
+
+			#sound-dai-cells = <1>;
+
+			clocks = <&lochnagar_clk LOCHNAGAR_SOUNDCARD_MCLK>;
+			clock-names = "mclk";
+		};
+	};
+
+	cs42l92: cs42l92@1a {
+		status = "disabled";
+
+		compatible = "cirrus,cs47l92";
+		reg = <0x1a>;
+
+		interrupts = <27 8>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		#sound-dai-cells = <1>;
+
+		AVDD-supply = <&lochnagar_vdd1v8>;
+		DBVDD1-supply = <&lochnagar_vdd1v8>;
+		CPVDD1-supply = <&lochnagar_vdd1v8>;
+		CPVDD2-supply = <&lochnagar_vddcore>;
+		DCVDD-supply = <&lochnagar_vddcore>;
+		SPKVDDL-supply = <&wallvdd>;
+		SPKVDDR-supply = <&wallvdd>;
+
+		reset-gpios = <&lochnagar_pin 0 0>;
+
+		cirrus,out-mono = <0 0 1 1 0 0>;
+		cirrus,dmic-ref = <
+			MADERA_DMIC_REF_MICBIAS1
+			MADERA_DMIC_REF_MICBIAS1
+			MADERA_DMIC_REF_MICBIAS1
+			MADERA_DMIC_REF_MICBIAS1
+		>;
+		cirrus,inmode = <
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN1A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN1B */
+			MADERA_INMODE_SE   MADERA_INMODE_SE   /* IN2A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN2B */
+		>;
+
+		clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+			 <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>;
+		clock-names = "mclk1", "mclk2";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cdc_irq &cs42l92_defaults>;
+
+		cs42l92_defaults: cs42l92-gpio-defaults {
+			aif1 {
+				groups = "aif1";
+				function = "aif1";
+				bias-bus-hold;
+			};
+
+			aif2 {
+				groups = "aif2";
+				function = "aif2";
+				bias-bus-hold;
+			};
+
+			aif3 {
+				groups = "aif3";
+				function = "aif3";
+				bias-bus-hold;
+			};
+		};
+
+		micvdd {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+		};
+
+		MICBIAS1 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+		};
+		MICBIAS1A {
+			regulator-active-discharge = <1>;
+		};
+		MICBIAS1B {
+			regulator-active-discharge = <1>;
+		};
+	};
+
+	wm8998: wm8998@1a {
+		status = "disabled";
+
+		compatible = "wlf,wm8998";
+		reg = <0x1a>;
+
+		interrupts = <27 0>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		#sound-dai-cells = <1>;
+
+		LDOVDD-supply = <&lochnagar_vdd1v8>;
+		AVDD-supply = <&lochnagar_vdd1v8>;
+		DBVDD1-supply = <&lochnagar_vdd1v8>;
+		DBVDD2-supply = <&lochnagar_vdd1v8>;
+		DBVDD3-supply = <&lochnagar_vdd1v8>;
+		CPVDD-supply = <&lochnagar_vdd1v8>;
+		SPKVDDL-supply = <&wallvdd>;
+		SPKVDDR-supply = <&wallvdd>;
+
+		wlf,gpio-defaults = <
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+		>;
+
+		wlf,reset = <&lochnagar_pin 0 0>;
+		wlf,ldoena = <&lochnagar_pin 3 0>;
+
+		wlf,micd-configs = <
+			0x0 ARIZONA_DMIC_MICBIAS1 0
+			0x2000 ARIZONA_DMIC_MICBIAS2 1
+		>;
+		wlf,jd-invert;
+		wlf,micd-force-micbias;
+		wlf,gpsw = <ARIZONA_GPSW_CLOSED>;
+		wlf,micd-pol-gpio = <&wm8998 4 GPIO_TRANSITORY>;
+		wlf,micd-rate = <ARIZONA_MICD_TIME_8MS>;
+		wlf,micd-detect-debounce = <500>;
+		wlf,dmic-ref = <ARIZONA_DMIC_MICBIAS2>;
+		wlf,inmode = <ARIZONA_INMODE_DMIC ARIZONA_INMODE_DMIC
+			      ARIZONA_INMODE_DIFF ARIZONA_INMODE_DIFF>;
+
+		clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+			 <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>;
+		clock-names = "mclk1", "mclk2";
+	};
+};
+
+&spi {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi_pins &spi_cs>;
+
+	cs-gpios = <&gpio 8 1>, <&gpio 7 1>;
+
+	spidev1: spidev@1{
+		compatible = "spidev";
+		reg = <1>;
+		status = "disabled";
+	};
+
+	cs47l15: cs47l15@1 {
+		status = "disabled";
+
+		compatible = "cirrus,cs47l15";
+		reg = <0x1>;
+
+		spi-max-frequency = <11000000>;
+
+		interrupts = <27 8>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		#sound-dai-cells = <1>;
+
+		AVDD-supply = <&lochnagar_vdd1v8>;
+		DCVDD-supply = <&lochnagar_vddcore>;
+		DBVDD1-supply = <&lochnagar_vdd1v8>;
+		CPVDD1-supply = <&lochnagar_vdd1v8>;
+		SPKVDD-supply = <&wallvdd>;
+		MICVDD-supply = <&lochnagar_micvdd>;
+
+		reset-gpios = <&lochnagar_pin 0 0>;
+
+		cirrus,dmic-ref = <
+			MADERA_DMIC_REF_MICBIAS1
+			MADERA_DMIC_REF_MICBIAS2
+		>;
+		cirrus,inmode = <
+			MADERA_INMODE_SE   MADERA_INMODE_SE   /* IN1A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN1B */
+			MADERA_INMODE_SE   MADERA_INMODE_SE   /* IN2A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN2B */
+		>;
+
+		clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+			 <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>;
+		clock-names = "mclk1", "mclk2";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cdc_irq &cs47l15_defaults>;
+
+		cs47l15_defaults: cs47l15-gpio-defaults {
+			aif1 {
+				groups = "aif1";
+				function = "aif1";
+				bias-bus-hold;
+			};
+			aif2 {
+				groups = "aif2";
+				function = "aif2";
+				bias-bus-hold;
+			};
+			aif3 {
+				groups = "aif3";
+				function = "aif3";
+				bias-bus-hold;
+			};
+			pdmspk1 {
+				groups = "pdmspk1";
+				function = "pdmspk1";
+			};
+		};
+
+		micvdd {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+		};
+
+		MICBIAS1 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+		};
+		MICBIAS1A {
+			regulator-active-discharge = <1>;
+		};
+		MICBIAS1B {
+			regulator-active-discharge = <1>;
+		};
+		MICBIAS1C {
+			regulator-active-discharge = <1>;
+		};
+	};
+
+	cs47l24: cs47l24@1 {
+		status = "disabled";
+
+		compatible = "cirrus,cs47l24";
+		reg = <0x1>;
+
+		spi-max-frequency = <11000000>;
+
+		interrupts = <27 0>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		#sound-dai-cells = <1>;
+
+		AVDD-supply = <&lochnagar_vdd1v8>;
+		DBVDD1-supply = <&lochnagar_vdd1v8>;
+		DBVDD2-supply = <&lochnagar_vdd1v8>;
+		DBVDD3-supply = <&lochnagar_vdd1v8>;
+		CPVDD-supply = <&lochnagar_vdd1v8>;
+		MICVDD-supply = <&lochnagar_micvdd>;
+		SPKVDD-supply = <&wallvdd>;
+		DCVDD-supply = <&lochnagar_vddcore>;
+
+		wlf,gpio-defaults = <
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+			ARIZONA_GP_DEFAULT
+		>;
+
+		wlf,reset = <&lochnagar_pin 0 0>;
+
+		wlf,dmic-ref = <ARIZONA_DMIC_MICBIAS2 ARIZONA_DMIC_MICBIAS3>;
+		wlf,inmode = <ARIZONA_INMODE_DMIC ARIZONA_INMODE_DMIC>;
+
+		clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+			 <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>;
+		clock-names = "mclk1", "mclk2";
+	};
+
+	cs47l35: cs47l35@1 {
+		status = "disabled";
+
+		compatible = "cirrus,cs47l35";
+		reg = <0x1>;
+
+		spi-max-frequency = <11000000>;
+
+		interrupts = <27 8>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		#sound-dai-cells = <1>;
+
+		AVDD-supply = <&lochnagar_vdd1v8>;
+		DBVDD1-supply = <&lochnagar_vdd1v8>;
+		DBVDD2-supply = <&lochnagar_vdd1v8>;
+		CPVDD1-supply = <&lochnagar_vdd1v8>;
+		CPVDD2-supply = <&lochnagar_vddcore>;
+		DCVDD-supply = <&lochnagar_vddcore>;
+		SPKVDD-supply = <&wallvdd>;
+
+		reset-gpios = <&lochnagar_pin 0 0>;
+
+		cirrus,dmic-ref = <
+			MADERA_DMIC_REF_MICVDD
+			CS47L35_DMIC_REF_MICBIAS2A
+			MADERA_DMIC_REF_MICVDD
+		>;
+		cirrus,inmode = <
+			MADERA_INMODE_SE   MADERA_INMODE_SE   /* IN1A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN1B */
+			MADERA_INMODE_SE   MADERA_INMODE_SE   /* IN2A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN2B */
+		>;
+
+		clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+			 <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>;
+		clock-names = "mclk1", "mclk2";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cdc_irq &cs47l35_defaults>;
+
+		cs47l35_defaults: cs47l35-gpio-defaults {
+			aif1 {
+				groups = "aif1";
+				function = "aif1";
+				bias-bus-hold;
+			};
+
+			aif2 {
+				groups = "aif2";
+				function = "aif2";
+				bias-bus-hold;
+			};
+
+			aif3 {
+				groups = "aif3";
+				function = "aif3";
+				bias-bus-hold;
+			};
+		};
+
+		micvdd {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+		};
+
+		MICBIAS1 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+		};
+		MICBIAS1A {
+			regulator-active-discharge = <1>;
+		};
+
+		MICBIAS2 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+		};
+		MICBIAS2A {
+			regulator-active-discharge = <1>;
+		};
+	};
+
+	cs47l90: cs47l90@1 {
+		status = "disabled";
+
+		compatible = "cirrus,cs47l90";
+		reg = <0x1>;
+
+		spi-max-frequency = <11000000>;
+
+		interrupts = <27 8>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		#sound-dai-cells = <1>;
+
+		AVDD-supply = <&lochnagar_vdd1v8>;
+		DBVDD1-supply = <&lochnagar_vdd1v8>;
+		DBVDD2-supply = <&lochnagar_vdd1v8>;
+		DBVDD3-supply = <&lochnagar_vdd1v8>;
+		DBVDD4-supply = <&lochnagar_vdd1v8>;
+		CPVDD1-supply = <&lochnagar_vdd1v8>;
+		CPVDD2-supply = <&lochnagar_vddcore>;
+		DCVDD-supply = <&lochnagar_vddcore>;
+		SPKVDDL-supply = <&wallvdd>;
+		SPKVDDR-supply = <&wallvdd>;
+
+		reset-gpios = <&lochnagar_pin 0 0>;
+
+		cirrus,dmic-ref = <
+			MADERA_DMIC_REF_MICBIAS1
+			MADERA_DMIC_REF_MICBIAS2
+		>;
+		cirrus,inmode = <
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN1A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN1B */
+			MADERA_INMODE_SE   MADERA_INMODE_SE   /* IN2A */
+			MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN2B */
+		>;
+
+		clocks = <&lochnagar_clk LOCHNAGAR_CDC_MCLK1>,
+			 <&lochnagar_clk LOCHNAGAR_CDC_MCLK2>;
+		clock-names = "mclk1", "mclk2";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cdc_irq &cs47l90_defaults>;
+
+		cs47l90_defaults: cs47l90-gpio-defaults {
+			aif1 {
+				groups = "aif1";
+				function = "aif1";
+				bias-bus-hold;
+			};
+
+			aif2 {
+				groups = "aif2";
+				function = "aif2";
+				bias-bus-hold;
+			};
+
+			aif3 {
+				groups = "aif3";
+				function = "aif3";
+				bias-bus-hold;
+			};
+		};
+
+		micvdd {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+		};
+
+		MICBIAS1 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+		};
+		MICBIAS1A {
+			regulator-active-discharge = <1>;
+		};
+		MICBIAS1B {
+			regulator-active-discharge = <1>;
+		};
+
+		MICBIAS2 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+		};
+		MICBIAS2C {
+			regulator-active-discharge = <1>;
+		};
+	};
+};
-- 
2.20.1

Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

Hi Richard,
your series is very welcome, upstream support for audio codecs on the RPi4 has
always been lackluster.

Could you provide more information on the actual products? Are there custom
made hats for the RPi4 or this wired into a generic development board.

On Wed, 2020-10-14 at 15:54 +0100, Richard Fitzgerald wrote:
> This is based on the default bcm2711-rpi-4-b.dts.

Note that you could've included bcm2711-rpi-4.dts (as if it was a .dtsi).

> Configurations are provided for Cirrus Logic codecs CS42L92, CS47L15,
> CS47L24, CS47L35, CS47L90 and WM8998.
> 
> For each codec there is a sound node and a codec device node and both
> default to disabled. Enable the pair for the codec in use.
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---

Sadly I don't think creating a new device tree is a good solution here. If we
were to do so for every RPi hat/usage it'd become unmanageable very fast. There
is a way to maintain this in the open nonetheless. I suggest you build a DT
overlay and submit it to https://github.com/raspberrypi/linux, see
'arch/arm/boot/dts/overlays.' The Raspberry Pi engineers have a kernel branch
that tracks of the latest kernel release, so once you get the rest of patches
sorted out and they are included in a release it'll make sense to do so.

I can't tell for other distros, but opensuse packages overlays, so the effort
will ultimately be useful to users.

Regards,
Nicolas



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2

[Richard Fitzgerald]

On 15/10/2020 11:25, Nicolas Saenz Julienne wrote:
> Hi Richard,
> your series is very welcome, upstream support for audio codecs on the RPi4 has
> always been lackluster.
> 
> Could you provide more information on the actual products? Are there custom
> made hats for the RPi4 or this wired into a generic development board.
> 
Info on the codecs is available from www.cirrus.com.
The Lochnagar audio development board is not a hat, but it can be wired
over to the RPi GPIO header. It is not specific to the RPi.

> On Wed, 2020-10-14 at 15:54 +0100, Richard Fitzgerald wrote:
>> This is based on the default bcm2711-rpi-4-b.dts.
> 
> Note that you could've included bcm2711-rpi-4.dts (as if it was a .dtsi).
> 
Ok, will change.

>> Configurations are provided for Cirrus Logic codecs CS42L92, CS47L15,
>> CS47L24, CS47L35, CS47L90 and WM8998.
>>
>> For each codec there is a sound node and a codec device node and both
>> default to disabled. Enable the pair for the codec in use.
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> ---
> 
> Sadly I don't think creating a new device tree is a good solution here. If we
> were to do so for every RPi hat/usage it'd become unmanageable very fast. There
> is a way to maintain this in the open nonetheless. I suggest you build a DT
> overlay and submit it to https://github.com/raspberrypi/linux, see
> 'arch/arm/boot/dts/overlays.' The Raspberry Pi engineers have a kernel branch

We want something in mainline so that it can be used by people
developing on mainline and taken as a starting point for configuring
the codecs for other host platforms. The RPi is a convenient platform to
use as the base because it is widely available and low-cost.

> that tracks of the latest kernel release, so once you get the rest of patches
> sorted out and they are included in a release it'll make sense to do so.
> 
> I can't tell for other distros, but opensuse packages overlays, so the effort
> will ultimately be useful to users.
> 
> Regards,
> Nicolas
> 
> 

Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Thu, 2020-10-15 at 12:14 +0100, Richard Fitzgerald wrote:
> > Sadly I don't think creating a new device tree is a good solution here. If we
> > were to do so for every RPi hat/usage it'd become unmanageable very fast. There
> > is a way to maintain this in the open nonetheless. I suggest you build a DT
> > overlay and submit it to https://github.com/raspberrypi/linux, see
> > 'arch/arm/boot/dts/overlays.' The Raspberry Pi engineers have a kernel branch
> 
> We want something in mainline so that it can be used by people
> developing on mainline and taken as a starting point for configuring
> the codecs for other host platforms. The RPi is a convenient platform to
> use as the base because it is widely available and low-cost.

If what you want to convey is the proper way of configuring your specific
device the way to go is writing a devicetree binding. See
Documentation/devicetree. It's even possible to validate a given devicetree
against the bindings (given they are written in yaml format).

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

On Thu, Oct 15, 2020 at 05:12:42PM +0200, Nicolas Saenz Julienne wrote:
> On Thu, 2020-10-15 at 12:14 +0100, Richard Fitzgerald wrote:

> > We want something in mainline so that it can be used by people
> > developing on mainline and taken as a starting point for configuring
> > the codecs for other host platforms. The RPi is a convenient platform to
> > use as the base because it is widely available and low-cost.

> If what you want to convey is the proper way of configuring your specific
> device the way to go is writing a devicetree binding. See
> Documentation/devicetree. It's even possible to validate a given devicetree
> against the bindings (given they are written in yaml format).

These devices already have bindings, that doesn't really help with
describing how a specific board is wired up.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2

[Richard Fitzgerald]

On 15/10/2020 16:12, Nicolas Saenz Julienne wrote:
> On Thu, 2020-10-15 at 12:14 +0100, Richard Fitzgerald wrote:
>>> Sadly I don't think creating a new device tree is a good solution here. If we
>>> were to do so for every RPi hat/usage it'd become unmanageable very fast. There
>>> is a way to maintain this in the open nonetheless. I suggest you build a DT
>>> overlay and submit it to https://github.com/raspberrypi/linux, see
>>> 'arch/arm/boot/dts/overlays.' The Raspberry Pi engineers have a kernel branch
>>
>> We want something in mainline so that it can be used by people
>> developing on mainline and taken as a starting point for configuring
>> the codecs for other host platforms. The RPi is a convenient platform to
>> use as the base because it is widely available and low-cost.
> 
> If what you want to convey is the proper way of configuring your specific
> device the way to go is writing a devicetree binding. See

If we have a working configuration it is unreasonable not to make this
available for people who want to use it or examine it. A working example
can be more helpful than a ton of documentation.

It's also worth noting that setting up a working sound system involves
configuring multiple drivers (for example you also need a properly
configured ASoC machine driver at least) crossing multiple driver
bindings. So a complete working example is valuable to see how
it fits together.

> Documentation/devicetree. It's even possible to validate a given devicetree
> against the bindings (given they are written in yaml format).
> 

Validating only checks syntax and bounds. It doesn't prove that it will
work.

> Regards,
> Nicolas
> 

[PATCH 7/7] MAINTAINERS: Add dts for Cirrus Logic Lochnagar on RPi4

[Richard Fitzgerald]

Update the Cirrus Logic driver maintainers to include the device
tree .dts for using the Lochnagar with a Raspberry Pi 4.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 87ceaac748a4..09bc583a5b2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4199,6 +4199,7 @@ M:	Charles Keepax <ckeepax@opensource.cirrus.com>
 M:	Richard Fitzgerald <rf@opensource.cirrus.com>
 L:	patches@opensource.cirrus.com
 S:	Supported
+F:	arch/arm/boot/dts/bcm2711-rpi4b-cirrus-lochnagar.dts
 F:	Documentation/devicetree/bindings/clock/cirrus,lochnagar.yaml
 F:	Documentation/devicetree/bindings/hwmon/cirrus,lochnagar.yaml
 F:	Documentation/devicetree/bindings/mfd/cirrus,lochnagar.yaml
-- 
2.20.1

Re: [PATCH 0/7] Add dts for Rpi4 + Cirrus Lochnagar and codecs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Wed, Oct 14, 2020 at 03:54:11PM +0100, Richard Fitzgerald wrote:
> This set of patches provides support for using the Cirrus Logic
> Lochnagar audio development platform plus Cirrus Logic Madera/Arizona
> codecs with the simple-card machine driver and a Raspberry Pi4. The
> ultimate aim is to provide the dts file but some updates are needed to
> the simple-card machine driver.

Why extend simple-card and not the more modern and flexible
audio-graph-card?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/7] Add dts for Rpi4 + Cirrus Lochnagar and codecs

[Richard Fitzgerald]

On 14/10/2020 19:56, Mark Brown wrote:
> On Wed, Oct 14, 2020 at 03:54:11PM +0100, Richard Fitzgerald wrote:
>> This set of patches provides support for using the Cirrus Logic
>> Lochnagar audio development platform plus Cirrus Logic Madera/Arizona
>> codecs with the simple-card machine driver and a Raspberry Pi4. The
>> ultimate aim is to provide the dts file but some updates are needed to
>> the simple-card machine driver.
> 
> Why extend simple-card and not the more modern and flexible
> audio-graph-card?
> 

I'm struggling to understand how to use audio-graph-card where there are
multiple alternative codecs. The host I2S endpoint has to point back to
the codec endpoint, like this:

	cpu_i2s_ep_cs47l15: endpoint {
		remote-endpoint = <&cs47l15_aif1>;
	};

But obviously that depends on which codec node was enabled. Listing
multiple endpoints makes the whole port node disabled if any remote
endpoint is in a disabled node. I've tried adding status="disabled"
to endpoints or multiple port definitions with status="disabled" but
I haven't figured out a solution.

Re: [PATCH 0/7] Add dts for Rpi4 + Cirrus Lochnagar and codecs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

On Fri, Oct 16, 2020 at 02:30:08PM +0100, Richard Fitzgerald wrote:
> On 14/10/2020 19:56, Mark Brown wrote:

> > Why extend simple-card and not the more modern and flexible
> > audio-graph-card?

> I'm struggling to understand how to use audio-graph-card where there are
> multiple alternative codecs. The host I2S endpoint has to point back to
> the codec endpoint, like this:

OK, this seems like a more urgent problem to address given that the
graph card is supposed to be able to support things like TDM.
However...

> 	cpu_i2s_ep_cs47l15: endpoint {
> 		remote-endpoint = <&cs47l15_aif1>;
> 	};

> But obviously that depends on which codec node was enabled. Listing
> multiple endpoints makes the whole port node disabled if any remote
> endpoint is in a disabled node. I've tried adding status="disabled"
> to endpoints or multiple port definitions with status="disabled" but
> I haven't figured out a solution.

...it seems like the issue here is that you're essentially trying to
define multiple cards at once in the same overlay.  TBH this feels like
you want two nested levels of overlay, with the extra layer patching the
CODEC compatible.  Or if this is mainly as an example for people you
could just pick one and use that?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/7] Add dts for Rpi4 + Cirrus Lochnagar and codecs

[Rob Herring]

On Fri, Oct 16, 2020 at 02:30:08PM +0100, Richard Fitzgerald wrote:
> On 14/10/2020 19:56, Mark Brown wrote:
> > On Wed, Oct 14, 2020 at 03:54:11PM +0100, Richard Fitzgerald wrote:
> > > This set of patches provides support for using the Cirrus Logic
> > > Lochnagar audio development platform plus Cirrus Logic Madera/Arizona
> > > codecs with the simple-card machine driver and a Raspberry Pi4. The
> > > ultimate aim is to provide the dts file but some updates are needed to
> > > the simple-card machine driver.
> > 
> > Why extend simple-card and not the more modern and flexible
> > audio-graph-card?
> > 
> 
> I'm struggling to understand how to use audio-graph-card where there are
> multiple alternative codecs. The host I2S endpoint has to point back to
> the codec endpoint, like this:
> 
> 	cpu_i2s_ep_cs47l15: endpoint {
> 		remote-endpoint = <&cs47l15_aif1>;
> 	};
>
> But obviously that depends on which codec node was enabled. Listing
> multiple endpoints makes the whole port node disabled if any remote
> endpoint is in a disabled node. I've tried adding status="disabled"
> to endpoints or multiple port definitions with status="disabled" but
> I haven't figured out a solution.

Multiple endpoints is what you should do. And 'status' goes in the 
device nodes (not the graph nodes).

Rob