[RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Simple_card does require under some circumstances the ability
to configure certain hw_parameters based on clocks, bits, channels.

This patchset adds a generic way to configure these kind of things
via the device tree easily. This patchset implements this
for simple_card, but other drivers can just as easily make use of
this.

For now we have the following matchers and actions:
* matchers:
  * match_sample_bits
  * match_rate
  * match_channels
* actions:
  * set_fixed_bclk_size

As a note: the available matching rules and action rules right now
are hard-coded, but this could in principle get extended to be more
dynamic via kallsyms_lookup_name that would lookup the requested
symbol and assume it is a struct asoc_generic_hw_params_method,
on which it could apply several sanity-checks before using
the pointers for real.

This would allow other kinds of action methods to get exposed in
codec-drivers, that can then get used to handle some settings
specific to the codec.

I leave this is an exercise for later...

Martin Sperl (2):
  ASoC: hw-params-rules: add generic hw_params-rules
  ASoC: simple_card: add support for hw_params_rules

 .../devicetree/bindings/sound/hw-params-rules.txt  |  86 +++++
 .../devicetree/bindings/sound/simple-card.txt      |   1 +
 sound/soc/generic/Kconfig                          |   6 +
 sound/soc/generic/Makefile                         |   2 +
 sound/soc/generic/hw-params-rules.c                | 402 +++++++++++++++++++++
 sound/soc/generic/hw-params-rules.h                |  49 +++
 sound/soc/generic/simple-card.c                    |  11 +-
 7 files changed, 554 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/hw-params-rules.txt
 create mode 100644 sound/soc/generic/hw-params-rules.c
 create mode 100644 sound/soc/generic/hw-params-rules.h

--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] ASoC: hw-params-rules: add generic hw_params-rules

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Add generic rules that may execute when running hw_params
methods.

These rules are configured via the device-tree.

Right now only a static list of match and action rules exist.
This can get changed in the future to something more dynamic
if security concerns can get handled propperly.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 .../devicetree/bindings/sound/hw-params-rules.txt  |  86 +++++
 sound/soc/generic/Kconfig                          |   6 +
 sound/soc/generic/Makefile                         |   2 +
 sound/soc/generic/hw-params-rules.c                | 402 +++++++++++++++++++++
 sound/soc/generic/hw-params-rules.h                |  49 +++
 5 files changed, 545 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/hw-params-rules.txt
 create mode 100644 sound/soc/generic/hw-params-rules.c
 create mode 100644 sound/soc/generic/hw-params-rules.h

diff --git a/Documentation/devicetree/bindings/sound/hw-params-rules.txt b/Documentation/devicetree/bindings/sound/hw-params-rules.txt
new file mode 100644
index 0000000..8e5f2b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/hw-params-rules.txt
@@ -0,0 +1,86 @@
+HW-params-rules:
+
+Rules that execute actions during hw-params method calls.
+
+The following optional nodes will sit inside the sound card nodes:
+
+hw-params-rule subnodes:
+
+Required properties:
+- priority: the priority of the rule - lowest is applied first.
+
+contains further subnodes:
+
+- match:  list of matches that have to apply
+- action: list of actions to execute
+
+match and action subnodes:
+
+Required properties:
+
+- method: the method name (string) to execute
+
+Possible values for method:
+* asoc_generic_hw_params_match_sample_bits
+* asoc_generic_hw_params_match_rate
+* asoc_generic_hw_params_match_channel
+* asoc_generic_hw_params_set_fixed_bclk_size
+
+
+Required/optional properties depend on the method defined
+typical properies:
+
+- value: u32 value to pass to method.
+- values: u32 value array to pass to method.
+
+Example:
+sound {
+	compatible = "simple-audio-card";
+	...
+	/*
+	 * set bclk_size to 80 when encountering:
+	 * 48kHz or 96kHz with 2 channels with 32bit/channel
+	 */
+	hw-params-rule@0 {
+		priority = <0>;
+		match@0 {
+			method = "asoc_generic_hw_params_match_sample_bits";
+			values = <32>;
+		};
+		match@1 {
+			method = "asoc_generic_hw_params_match_rate";
+			values = <48000>, <96000>;
+		};
+		match@2 {
+			method = "asoc_generic_hw_params_match_channels";
+			values = <2>;
+		};
+		action@0 {
+			method = "asoc_generic_hw_params_set_fixed_bclk_size";
+			value = <80>;
+		};
+	};
+	/*
+	 * set bclk_size to 40 when encountering:
+	 * 48kHz with 2 channels with 16bit/channel
+	 */
+	hw-params-rule@1 {
+		priority = <1>;
+		match@0 {
+			method = "asoc_generic_hw_params_match_sample_bits";
+			values = <16>;
+		};
+		match@1 {
+			method = "asoc_generic_hw_params_match_rate";
+			values = <48000>;
+		};
+		match@2 {
+			method = "asoc_generic_hw_params_match_channels";
+			values = <2>;
+		};
+		action@0 {
+			method = "asoc_generic_hw_params_set_fixed_bclk_size";
+			value = <40>;
+		};
+	};
+};
diff --git a/sound/soc/generic/Kconfig b/sound/soc/generic/Kconfig
index 610f612..2c9f0c1 100644
--- a/sound/soc/generic/Kconfig
+++ b/sound/soc/generic/Kconfig
@@ -1,3 +1,9 @@
+config SND_HW_PARAMS_RULES
+	tristate "ASoC hw_param rules support"
+	depends on OF
+	help
+	  This option enables generic hw_param_rules support
+
 config SND_SIMPLE_CARD
 	tristate "ASoC Simple sound card support"
 	help
diff --git a/sound/soc/generic/Makefile b/sound/soc/generic/Makefile
index 9c3b246..4eaad07 100644
--- a/sound/soc/generic/Makefile
+++ b/sound/soc/generic/Makefile
@@ -1,3 +1,5 @@
 snd-soc-simple-card-objs	:= simple-card.o
+snd-soc-hw-params-rules-objs	:= hw-params-rules.o
 
 obj-$(CONFIG_SND_SIMPLE_CARD)	+= snd-soc-simple-card.o
+obj-$(CONFIG_SND_HW_PARAMS_RULES)	+= snd-soc-hw-params-rules.o
diff --git a/sound/soc/generic/hw-params-rules.c b/sound/soc/generic/hw-params-rules.c
new file mode 100644
index 0000000..46dbdff
--- /dev/null
+++ b/sound/soc/generic/hw-params-rules.c
@@ -0,0 +1,402 @@
+/*
+ * ASoC generic hw_params_rules support
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/list_sort.h>
+#include <sound/pcm_params.h>
+#include <sound/simple_card.h>
+#include <sound/soc-dai.h>
+#include <sound/soc.h>
+
+struct snd_soc_hw_params_actionmatch {
+	struct list_head list;
+	int (*method)(struct snd_pcm_substream *substream,
+		      struct snd_pcm_hw_params *params,
+		      void *data);
+	void *data;
+};
+
+struct snd_soc_hw_param_rule {
+	struct list_head list;
+	const char *name;
+	u32 priority;
+	struct list_head matches;
+	struct list_head actions;
+};
+
+struct snd_soc_size_u32array {
+	size_t size;
+	u32 data[];
+};
+
+static int asoc_generic_hw_params_read_u32array(
+	struct device *dev, struct device_node *node, void **data)
+{
+	int i, size, ret;
+	struct snd_soc_size_u32array *array;
+
+	size = of_property_count_elems_of_size(node, "values", sizeof(u32));
+	if (size < 0) {
+		dev_err(dev,
+			"%s: Could not read size of property \"values\" - %d\n",
+			of_node_full_name(node), size);
+		return size;
+	}
+
+	array = devm_kzalloc(dev, sizeof(*array) + sizeof(u32) * size,
+			     GFP_KERNEL);
+	if (!array)
+		return -ENOMEM;
+	*data = array;
+
+	array->size = size;
+
+	for (i = 0; i < size; i++) {
+		ret = of_property_read_u32(node, "values", &array->data[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_read_u32(
+	struct device *dev, struct device_node *node, void **data)
+{
+	return of_property_read_u32(node, "value", (u32 *)data);
+}
+
+static int asoc_generic_hw_params_match_sample_bits(
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params,
+	void *data)
+{
+	long int bits =
+		snd_pcm_format_physical_width(params_format(params));
+	struct snd_soc_size_u32array *array = data;
+	int i;
+
+	for (i = 0; i < array->size; i++) {
+		if (bits == array->data[i])
+			return 1;
+	}
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_match_channels(
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params,
+	void *data)
+{
+	int channels = params_channels(params);
+	struct snd_soc_size_u32array *array = data;
+	int i;
+
+	for (i = 0; i < array->size; i++) {
+		if (channels == array->data[i])
+			return 1;
+	}
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_match_rate(
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params,
+	void *data)
+{
+	long int rate = params_rate(params);
+
+	struct snd_soc_size_u32array *array = data;
+	int i;
+
+	for (i = 0; i < array->size; i++) {
+		if (rate == array->data[i])
+			return 1;
+	}
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_set_fixed_bclk_size(
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params,
+	void *data)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+
+	return snd_soc_dai_set_bclk_ratio(cpu_dai, (unsigned int)data);
+}
+
+struct asoc_generic_hw_params_method {
+	const char *name;
+	int (*method)(struct snd_pcm_substream *substream,
+		      struct snd_pcm_hw_params *params,
+		      void *data);
+	int (*parse)(struct device *dev, struct device_node *node,
+		     void **data);
+};
+
+#define HW_PARAMS_METHOD(m, p)		\
+	{.name = #m, .method = m, .parse = p }
+#define HW_PARAMS_METHOD_U32(n) \
+	HW_PARAMS_METHOD(n, asoc_generic_hw_params_read_u32)
+#define HW_PARAMS_METHOD_U32ARRAY(n) \
+	HW_PARAMS_METHOD(n, asoc_generic_hw_params_read_u32array)
+
+static const struct asoc_generic_hw_params_method
+asoc_generic_hw_params_methods[] = {
+	HW_PARAMS_METHOD_U32ARRAY(asoc_generic_hw_params_match_sample_bits),
+	HW_PARAMS_METHOD_U32ARRAY(asoc_generic_hw_params_match_rate),
+	HW_PARAMS_METHOD_U32ARRAY(asoc_generic_hw_params_match_channels),
+	HW_PARAMS_METHOD_U32(asoc_generic_hw_params_set_fixed_bclk_size)
+};
+
+static int asoc_generic_hw_params_lookup_methods(
+	struct device *dev, const char *method,
+	struct device_node *node,
+	struct snd_soc_hw_params_actionmatch *am)
+{
+	const struct asoc_generic_hw_params_method *m;
+	size_t i;
+
+	/*
+	 * hardcoded list of "allowed" methods
+	 * maybe a more dynamic approach using kallsyms could also be taken
+	 */
+	for (i = 0; i < ARRAY_SIZE(asoc_generic_hw_params_methods); i++) {
+		m = &asoc_generic_hw_params_methods[i];
+		if (strcmp(m->name, method) == 0) {
+			am->method = m->method;
+			if (m->parse)
+				return m->parse(dev, node, &am->data);
+			else
+				return 0;
+		}
+	}
+
+	dev_err(dev, "%s: method %s not found\n",
+		of_node_full_name(node), method);
+	return -EINVAL;
+}
+
+static int asoc_generic_hw_params_handle_rule(
+	struct snd_soc_hw_param_rule *rule,
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct device *dev = rtd->dev;
+	struct snd_soc_hw_params_actionmatch *am;
+	int ret;
+
+	dev_dbg(dev, "Trying to apply rule: %s\n", rule->name);
+
+	/* apply match rules */
+	list_for_each_entry(am, &rule->matches, list) {
+		dev_dbg(dev, "\tRunning match %pf(%pK)\n",
+			am->method, am->data);
+		/* match method return 0 on match, 1 otherwise */
+		ret = am->method(substream, params, am->data);
+		if (!ret)
+			return 1;
+	}
+
+	/* so we match, so run all the actions */
+	list_for_each_entry(am, &rule->actions, list) {
+		dev_dbg(dev, "\tRunning action %pf(%pK)\n",
+			am->method, am->data);
+		/* action method returns 0 on success */
+		ret = am->method(substream, params, am->data);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+int asoc_generic_hw_params_process_rules(
+	struct list_head *list_head,
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_hw_param_rule *rule;
+	int ret;
+
+	/* check if the list_head is initialized */
+	if (!list_head->next)
+		return 0;
+
+	/* iterate all rules */
+	list_for_each_entry(rule, list_head, list) {
+		ret = asoc_generic_hw_params_handle_rule(
+			rule, substream, params);
+		if (ret <= 0)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_generic_hw_params_process_rules);
+
+static int asoc_generic_hw_params_actionmatch_parse_of(
+	struct device *dev, struct device_node *node,
+	struct list_head *list_head, const char *nodename)
+{
+	struct snd_soc_hw_params_actionmatch *am;
+	const char *methodname;
+	int ret;
+
+	/* get the method name */
+	ret = of_property_read_string(node, "method", &methodname);
+	if (ret) {
+		dev_err(dev, "%s: missing \"method\" property - %d\n",
+			of_node_full_name(node), ret);
+		return ret;
+	}
+
+	/* alloc the action/match */
+	am = devm_kzalloc(dev, sizeof(*am), GFP_KERNEL);
+	if (!am)
+		return -ENOMEM;
+
+	/* lookup the method */
+	ret = asoc_generic_hw_params_lookup_methods(dev, methodname,
+						    node, am);
+	if (ret)
+		return ret;
+
+	/* append to list */
+	list_add_tail(&am->list, list_head);
+
+	dev_dbg(dev, "\t\tadded %s: %s - %pf(%pK)\n", nodename,
+		of_node_full_name(node), am->method, am->data);
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_actionmatches_parse_of(
+	struct device *dev, struct device_node *node,
+	struct list_head *list_head, const char *nodename)
+{
+	struct device_node *np = NULL;
+	int ret = 0;
+
+	/* init matchers */
+	INIT_LIST_HEAD(list_head);
+
+	/* iterate over all child nodes */
+	for_each_child_of_node(node, np) {
+		if (np->name && (of_node_cmp(np->name, nodename) == 0)) {
+			ret = asoc_generic_hw_params_actionmatch_parse_of(
+				dev, np, list_head, nodename);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_rule_parse_of(
+	struct device *dev, struct device_node *node,
+	struct list_head *list_head)
+{
+	struct snd_soc_hw_param_rule *rule;
+	int ret;
+
+	rule = devm_kzalloc(dev, sizeof(*rule), GFP_KERNEL);
+	if (!rule)
+		return -ENOMEM;
+
+	rule->name = of_node_full_name(node);
+
+	dev_dbg(dev, "\tadding Rule: %s\n", rule->name);
+
+	/* read priority */
+	ret = of_property_read_u32(node, "priority", &rule->priority);
+	if (ret) {
+		dev_err(dev, "%s: can not read \"priority\" - %d\n",
+			rule->name, ret);
+		return ret;
+	}
+
+	/* parse all matches sub-nodes */
+	ret = asoc_generic_hw_params_actionmatches_parse_of(
+		dev, node, &rule->matches, "match");
+	if (ret)
+		return ret;
+
+	/* parse all action sub-nodes */
+	ret = asoc_generic_hw_params_actionmatches_parse_of(
+		dev, node, &rule->actions, "action");
+	if (ret)
+		return ret;
+
+	/* append to list */
+	list_add_tail(&rule->list, list_head);
+
+	return 0;
+}
+
+static int asoc_generic_hw_params_rules_sort(
+	void *data, struct list_head *a, struct list_head *b)
+{
+	struct snd_soc_hw_param_rule *rulea =
+		container_of(a, typeof(*rulea), list);
+	struct snd_soc_hw_param_rule *ruleb =
+		container_of(b, typeof(*ruleb), list);
+
+	if (rulea->priority < ruleb->priority)
+		return -1;
+	if (rulea->priority > ruleb->priority)
+		return 1;
+
+	return 0;
+}
+
+int asoc_generic_hw_params_rules_parse_of(
+	struct device *dev,
+	struct device_node *node,
+	struct list_head *list_head)
+{
+	const char *nodename = "hw-params-rule";
+	struct device_node *np = NULL;
+	int ret = 0;
+
+	/* init matchers */
+	INIT_LIST_HEAD(list_head);
+
+	if (!of_get_child_by_name(node, nodename))
+		return 0;
+
+	for (np = of_find_node_by_name(node, nodename); np;
+	     np = of_find_node_by_name(np, nodename)) {
+		ret = asoc_generic_hw_params_rule_parse_of(
+			dev, np, list_head);
+		if (ret)
+			return ret;
+	}
+
+	/* and sort by name */
+	list_sort(dev, list_head, asoc_generic_hw_params_rules_sort);
+
+	/* iterate the sub-nodes */
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_generic_hw_params_rules_parse_of);
+
+MODULE_AUTHOR("Martin Sperl");
+MODULE_DESCRIPTION("generic hw_params_rules support");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/generic/hw-params-rules.h b/sound/soc/generic/hw-params-rules.h
new file mode 100644
index 0000000..0b054ce
--- /dev/null
+++ b/sound/soc/generic/hw-params-rules.h
@@ -0,0 +1,49 @@
+/*
+ * ASoC generic hw_params_rules support
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __HW_PARAMS_RULES_H
+#define __HW_PARAMS_RULES_H
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/of.h>
+#include <sound/pcm.h>
+
+#if defined(CONFIG_SND_HW_PARAMS_RULES) ||	\
+	defined(CONFIG_SND_HW_PARAMS_RULES_MODULE)
+
+int asoc_generic_hw_params_rules_parse_of(
+	struct device *dev,
+	struct device_node *node,
+	struct list_head *list_head);
+
+int asoc_generic_hw_params_process_rules(
+	struct list_head *list_head,
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params);
+#else
+static int asoc_generic_hw_params_rules_parse_of(
+	struct device *dev,
+	struct device_node *node,
+	struct list_head *list_head)
+{
+	return 0;
+}
+
+static int asoc_generic_hw_params_process_rules(
+	struct list_head *list_head,
+	struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	return 0;
+}
+#endif
+
+#endif
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] ASoC: simple_card: add support for hw_params_rules

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Add support for hw_params_rules to simple_card.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 Documentation/devicetree/bindings/sound/simple-card.txt |  1 +
 sound/soc/generic/simple-card.c                         | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index cf3979e..2cfed39 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -31,6 +31,7 @@ Optional subnodes:
 					  omitted when the card has only one
 					  DAI link. See the examples and the
 					  section bellow.
+- hw-params-rules			: Please refer to hw-params-rules.txt.
 
 Dai-link subnode properties and subnodes:
 
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 2389ab4..6b95ea1 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -8,6 +8,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include "hw-params-rules.h"
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
@@ -33,6 +34,7 @@ struct simple_card_data {
 	int gpio_hp_det_invert;
 	int gpio_mic_det;
 	int gpio_mic_det_invert;
+	struct list_head hw_params_rules;
 	struct snd_soc_dai_link dai_link[];	/* dynamically allocated */
 };
 
@@ -51,7 +53,7 @@ static int asoc_simple_card_startup(struct snd_pcm_substream *substream)
 	ret = clk_prepare_enable(dai_props->cpu_dai.clk);
 	if (ret)
 		return ret;
-	
+
 	ret = clk_prepare_enable(dai_props->codec_dai.clk);
 	if (ret)
 		clk_disable_unprepare(dai_props->cpu_dai.clk);
@@ -99,7 +101,9 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream,
 		if (ret && ret != -ENOTSUPP)
 			goto err;
 	}
-	return 0;
+
+	return asoc_generic_hw_params_process_rules(
+		&priv->hw_params_rules, substream, params);
 err:
 	return ret;
 }
@@ -509,7 +513,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 	if (!priv->snd_card.name)
 		priv->snd_card.name = priv->snd_card.dai_link->name;
 
-	return 0;
+	return asoc_generic_hw_params_rules_parse_of(
+		dev, node, &priv->hw_params_rules);
 }
 
 /* Decrease the reference count of the device nodes */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Lars-Peter Clausen]

On 05/20/2016 02:30 PM, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> Simple_card does require under some circumstances the ability
> to configure certain hw_parameters based on clocks, bits, channels.
> 
> This patchset adds a generic way to configure these kind of things
> via the device tree easily. This patchset implements this
> for simple_card, but other drivers can just as easily make use of
> this.
> 
> For now we have the following matchers and actions:
> * matchers:
>   * match_sample_bits
>   * match_rate
>   * match_channels
> * actions:
>   * set_fixed_bclk_size
> 
> As a note: the available matching rules and action rules right now
> are hard-coded, but this could in principle get extended to be more
> dynamic via kallsyms_lookup_name that would lookup the requested
> symbol and assume it is a struct asoc_generic_hw_params_method,
> on which it could apply several sanity-checks before using
> the pointers for real.

How about allowing to embed eBPF bytecode in the DT that can be installed as
a constraint set? This would allow maximum for flexibility and also make the
implementation a lot easier.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Mark Brown]

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

On Fri, May 20, 2016 at 09:16:09PM +0200, Lars-Peter Clausen wrote:

> How about allowing to embed eBPF bytecode in the DT that can be installed as
> a constraint set? This would allow maximum for flexibility and also make the
> implementation a lot easier.

That seems somewhat Linux specific, I'd like to see some DT maintainer
buy in on that one as well as Takashi's thoughts.

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

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Mark Rutland]

On Mon, May 23, 2016 at 05:59:31PM +0100, Mark Brown wrote:
> On Fri, May 20, 2016 at 09:16:09PM +0200, Lars-Peter Clausen wrote:
> 
> > How about allowing to embed eBPF bytecode in the DT that can be installed as
> > a constraint set? This would allow maximum for flexibility and also make the
> > implementation a lot easier.
> 
> That seems somewhat Linux specific, I'd like to see some DT maintainer
> buy in on that one as well as Takashi's thoughts.

I am very much not in favour of embedding eBPF bytecode into the DT.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Lars-Peter Clausen]

On 05/23/2016 07:15 PM, Mark Rutland wrote:
> On Mon, May 23, 2016 at 05:59:31PM +0100, Mark Brown wrote:
>> On Fri, May 20, 2016 at 09:16:09PM +0200, Lars-Peter Clausen wrote:
>>
>>> How about allowing to embed eBPF bytecode in the DT that can be installed as
>>> a constraint set? This would allow maximum for flexibility and also make the
>>> implementation a lot easier.
>>
>> That seems somewhat Linux specific, I'd like to see some DT maintainer
>> buy in on that one as well as Takashi's thoughts.
> 
> I am very much not in favour of embedding eBPF bytecode into the DT.

This has to be seen in the context of the suggestion to reference kernel
symbols by name from the DT.

My opinion is that if the interdependence between the components in your
system is non-trivial you should write a proper driver. Otherwise we'll end
up with DT-script and replicate all the terrible things from the ACPI world
where people ship completely horrendous and broken byte code in the firmware.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Mark Brown]

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

On Mon, May 23, 2016 at 08:26:41PM +0200, Lars-Peter Clausen wrote:

> My opinion is that if the interdependence between the components in your
> system is non-trivial you should write a proper driver. Otherwise we'll end

Yes, I tend to agree - I've not looked at the actual patch yet but
there's a good, solid reason why we have explicit drivers for system
integration.  There is no value in pushing absoutely everything into DT,
at some point you just end up with something at least as complicated as
a real programming language with worse tooling.

> up with DT-script and replicate all the terrible things from the ACPI world
> where people ship completely horrendous and broken byte code in the firmware.

I think that's unfair, there are good solid reasons for having AML and
any interface is open to abuse.

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

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Rob Herring]

On Mon, May 23, 2016 at 06:15:16PM +0100, Mark Rutland wrote:
> On Mon, May 23, 2016 at 05:59:31PM +0100, Mark Brown wrote:
> > On Fri, May 20, 2016 at 09:16:09PM +0200, Lars-Peter Clausen wrote:
> > 
> > > How about allowing to embed eBPF bytecode in the DT that can be installed as
> > > a constraint set? This would allow maximum for flexibility and also make the
> > > implementation a lot easier.
> > 
> > That seems somewhat Linux specific, I'd like to see some DT maintainer
> > buy in on that one as well as Takashi's thoughts.
> 
> I am very much not in favour of embedding eBPF bytecode into the DT.

+1 What's wrong with Forth? ;)

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Takashi Iwai]

On Mon, 23 May 2016 18:59:31 +0200,
Mark Brown wrote:
> 
> On Fri, May 20, 2016 at 09:16:09PM +0200, Lars-Peter Clausen wrote:
> 
> > How about allowing to embed eBPF bytecode in the DT that can be installed as
> > a constraint set? This would allow maximum for flexibility and also make the
> > implementation a lot easier.
> 
> That seems somewhat Linux specific, I'd like to see some DT maintainer
> buy in on that one as well as Takashi's thoughts.

Using eBPF is a good idea for our purposes, but I understand Mark's
concern, too.  That said, I'm in favor of eBPF as long as DT people
don't mind.  We'd need some tools but the rest would be easier (less
patching) once after it's done.


thanks,

Takashi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[Mark Rutland]

On Fri, May 20, 2016 at 12:30:43PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> Simple_card does require under some circumstances the ability
> to configure certain hw_parameters based on clocks, bits, channels.
> 
> This patchset adds a generic way to configure these kind of things
> via the device tree easily. This patchset implements this
> for simple_card, but other drivers can just as easily make use of
> this.

I'm not familiar with the class of hardware here.

What exactly needs to be configured, under which situations?

How varied is this in practice?

Why does this make more sense than having individual drivers?

> For now we have the following matchers and actions:
> * matchers:
>   * match_sample_bits
>   * match_rate
>   * match_channels
> * actions:
>   * set_fixed_bclk_size
> 
> As a note: the available matching rules and action rules right now
> are hard-coded, but this could in principle get extended to be more
> dynamic via kallsyms_lookup_name that would lookup the requested
> symbol and assume it is a struct asoc_generic_hw_params_method,
> on which it could apply several sanity-checks before using
> the pointers for real.

I'm hoping that's a joke. ;)

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 0/2] ASoC: simple_card: support for hw-params rules

[kernel-TqfNSX0MhmxHKSADF0wUEw]

> On 23.05.2016, at 19:18, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> 
> On Fri, May 20, 2016 at 12:30:43PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> 
>> Simple_card does require under some circumstances the ability
>> to configure certain hw_parameters based on clocks, bits, channels.
>> 
>> This patchset adds a generic way to configure these kind of things
>> via the device tree easily. This patchset implements this
>> for simple_card, but other drivers can just as easily make use of
>> this.
> 
> I'm not familiar with the class of hardware here.
> 
> What exactly needs to be configured, under which situations?

Well one example - here specifically is to set up bclk rates that
the I2S hw itself is transferring for specific combinations.

Some of these rules depend on:
* the i2s driver hw itself - the i2s bcm2835 can only handle 32 bit
  transfers - so the block size needs to be set as a multiple of 2.
  in this case: sample-bits * channels.
* HW limits - e.g 24 bit/sample need to get transferred as 32 bit
  for the rpi-pcm5102a combination
* Clock selection dependent - the choice of the block size
  may be also 40 or 80 bit so that a integer divider clock
  can get selected as a preference.
  E.g 32 bit/sample * 2 channels * 48kHz gives a divider of 25/4
  using the main oscillator at 19.2MHz, which introduces some
  (possibly) audible  noise due to jitter.
  while 40bit/sample * 2 channels * 48kHz gives an integer divider
  of 20.

So controlling those helps using the hw setup properly.

> 
> How varied is this in practice?
I heard about some other devices besides the HifiBerry DAC that
also would like to control the blockrate the same way.

I remember having seen some other possible things we want to
control on the driver side during hw-params invocation, but
I can not remember what these were on top of my head (GPIO,
external clock selection if I remember correctly)

> 
> Why does this make more sense than having individual drivers?
Because then the driver would have to know a lot more
about the combination of board and codec.

Also the idea proposed by Mark was to use simple-card
instead of a dedicated driver which does not allow for
these kind of controls.

The idea was also to make it generic enough that it could
get even become part of the framework.

> 
>> For now we have the following matchers and actions:
>> * matchers:
>>  * match_sample_bits
>>  * match_rate
>>  * match_channels
>> * actions:
>>  * set_fixed_bclk_size
>> 
>> As a note: the available matching rules and action rules right now
>> are hard-coded, but this could in principle get extended to be more
>> dynamic via kallsyms_lookup_name that would lookup the requested
>> symbol and assume it is a struct asoc_generic_hw_params_method,
>> on which it could apply several sanity-checks before using
>> the pointers for real.
> 
> I'm hoping that's a joke. ;)
Just food for thought, because otherwise there would be dependencies
on compiled codecs (or a long #ifdef list)

As an alternative we could also add a method tables to the codec
structure, so that codecs could expose additional methods, that could
then get used.

Martin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html