[PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Ben Zhang]

The rt5677 codec driver looks for ACPI device ID "RT5677CE",
which is specified in coreboot. This patch allows platform
data to be obtained via ACPI

Signed-off-by: Ben Zhang <benzh@chromium.org>
---
 sound/soc/codecs/rt5677.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 5d317c68..384281d 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -20,6 +20,7 @@
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
+#include <linux/acpi.h>
 #include <linux/firmware.h>
 #include <linux/gpio.h>
 #include <sound/core.h>
@@ -4525,6 +4526,43 @@ static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+
+static unsigned long long rt5677_parse_acpi_entry(struct device *dev,
+		acpi_string name)
+{
+	acpi_handle handle = ACPI_HANDLE(dev);
+	unsigned long long val;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(handle, name, NULL, &val);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "Failed to parse ACPI entry %s, default to 0: %d\n",
+				name, status);
+		return 0;
+	}
+	return val;
+}
+
+static void rt5677_parse_acpi(struct rt5677_priv *rt5677, struct device *dev)
+{
+	rt5677->pdata.dmic2_clk_pin = (enum rt5677_dmic2_clk)
+		rt5677_parse_acpi_entry(dev, "DCLK");
+	rt5677->pdata.in1_diff = (bool)rt5677_parse_acpi_entry(dev, "IN1");
+	rt5677->pdata.in2_diff = (bool)rt5677_parse_acpi_entry(dev, "IN2");
+	rt5677->pdata.lout1_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT1");
+	rt5677->pdata.lout2_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT2");
+	rt5677->pdata.lout3_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT3");
+	rt5677->pdata.jd1_gpio = rt5677_parse_acpi_entry(dev, "JD1");
+	rt5677->pdata.jd2_gpio = rt5677_parse_acpi_entry(dev, "JD2");
+	rt5677->pdata.jd3_gpio = rt5677_parse_acpi_entry(dev, "JD3");
+}
+#else
+static void rt5677_parse_acpi(struct rt5677_priv *rt5677, struct device *dev)
+{
+}
+#endif
+
 static struct regmap_irq rt5677_irqs[] = {
 	[RT5677_IRQ_JD1] = {
 		.reg_offset = 0,
@@ -4604,6 +4642,7 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 	if (pdata)
 		rt5677->pdata = *pdata;
 
+	rt5677->pow_ldo2 = -EINVAL;
 	if (i2c->dev.of_node) {
 		ret = rt5677_parse_dt(rt5677, i2c->dev.of_node);
 		if (ret) {
@@ -4611,8 +4650,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 				ret);
 			return ret;
 		}
-	} else {
-		rt5677->pow_ldo2 = -EINVAL;
+	} else if (ACPI_HANDLE(&i2c->dev)) {
+		rt5677_parse_acpi(rt5677, &i2c->dev);
 	}
 
 	if (gpio_is_valid(rt5677->pow_ldo2)) {
@@ -4708,10 +4747,19 @@ static int rt5677_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt5677_acpi_id[] = {
+	{ "RT5677CE", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, rt5677_acpi_id);
+#endif
+
 static struct i2c_driver rt5677_i2c_driver = {
 	.driver = {
 		.name = "rt5677",
 		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(rt5677_acpi_id),
 	},
 	.probe = rt5677_i2c_probe,
 	.remove   = rt5677_i2c_remove,
-- 
2.1.0.rc2.206.gedb03e5

[PATCH 2/2] ASoC: rt5677: add a platform config option for PDM clock divider

[Ben Zhang]

The PDM output clock can use a divider of 1/2/3/4 based on the system clock

Signed-off-by: Ben Zhang <benzh@chromium.org>
---
 Documentation/devicetree/bindings/sound/rt5677.txt | 5 +++++
 include/sound/rt5677.h                             | 8 ++++++++
 sound/soc/codecs/rt5677.c                          | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rt5677.txt b/Documentation/devicetree/bindings/sound/rt5677.txt
index 740ff77..141f3e1 100644
--- a/Documentation/devicetree/bindings/sound/rt5677.txt
+++ b/Documentation/devicetree/bindings/sound/rt5677.txt
@@ -27,6 +27,10 @@ Optional properties:
   Boolean. Indicate MIC1/2 input and LOUT1/2/3 outputs are differential,
   rather than single-ended.
 
+- realtek,pdm_clk_div
+  Select 0/1/2/3 as PDM clock divider 1/2/4/3 respectively.
+  PDM clock = system clock / PDM clock divider
+
 - realtek,gpio-config
   Array of six 8bit elements that configures GPIO.
     0 - floating (reset value)
@@ -71,6 +75,7 @@ rt5677 {
 	realtek,pow-ldo2-gpio =
 		<&gpio TEGRA_GPIO(V, 3) GPIO_ACTIVE_HIGH>;
 	realtek,in1-differential = "true";
+	realtek,pdm_clk_div = <1>  /* PDM clock = system clock / 2 */
 	realtek,gpio-config = /bits/ 8  <0 0 0 0 0 2>;   /* pull up GPIO6 */
 	realtek,jd2-gpio = <3>;  /* Enables Jack detection for GPIO6 */
 };
diff --git a/include/sound/rt5677.h b/include/sound/rt5677.h
index d9eb7d8..368aa33 100644
--- a/include/sound/rt5677.h
+++ b/include/sound/rt5677.h
@@ -17,6 +17,12 @@ enum rt5677_dmic2_clk {
 	RT5677_DMIC_CLK2 = 1,
 };
 
+enum rt5677_pdm_clk_div {
+	RT5677_PDM_CLK_DIV1 = 0,
+	RT5677_PDM_CLK_DIV2 = 1,
+	RT5677_PDM_CLK_DIV4 = 2,
+	RT5677_PDM_CLK_DIV3 = 3,
+};
 
 struct rt5677_platform_data {
 	/* IN1/IN2/LOUT1/LOUT2/LOUT3 can optionally be differential */
@@ -27,6 +33,8 @@ struct rt5677_platform_data {
 	bool lout3_diff;
 	/* DMIC2 clock source selection */
 	enum rt5677_dmic2_clk dmic2_clk_pin;
+	/* System clock to PDM filter divider */
+	enum rt5677_pdm_clk_div pdm_clk_div;
 
 	/* configures GPIO, 0 - floating, 1 - pulldown, 2 - pullup */
 	u8 gpio_config[6];
diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 384281d..383cb61 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -4502,6 +4502,8 @@ static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
 					"realtek,lout2-differential");
 	rt5677->pdata.lout3_diff = of_property_read_bool(np,
 					"realtek,lout3-differential");
+	of_property_read_u32(np, "realtek,pdm_clk_div",
+					&rt5677->pdata.pdm_clk_div);
 
 	rt5677->pow_ldo2 = of_get_named_gpio(np,
 					"realtek,pow-ldo2-gpio", 0);
@@ -4548,6 +4550,8 @@ static void rt5677_parse_acpi(struct rt5677_priv *rt5677, struct device *dev)
 {
 	rt5677->pdata.dmic2_clk_pin = (enum rt5677_dmic2_clk)
 		rt5677_parse_acpi_entry(dev, "DCLK");
+	rt5677->pdata.pdm_clk_div = (enum rt5677_pdm_clk_div)
+		rt5677_parse_acpi_entry(dev, "PCLK");
 	rt5677->pdata.in1_diff = (bool)rt5677_parse_acpi_entry(dev, "IN1");
 	rt5677->pdata.in2_diff = (bool)rt5677_parse_acpi_entry(dev, "IN2");
 	rt5677->pdata.lout1_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT1");
@@ -4730,6 +4734,9 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
 					RT5677_GPIO5_DIR_OUT);
 	}
 
+	regmap_update_bits(rt5677->regmap, RT5677_PDM_DATA_CTRL1,
+			RT5677_PDM_DIV_MASK, rt5677->pdata.pdm_clk_div);
+
 	rt5677_init_gpio(i2c);
 	rt5677_irq_init(i2c);
 
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Fri, Nov 14, 2014 at 10:56:47PM -0800, Ben Zhang wrote:

> The rt5677 codec driver looks for ACPI device ID "RT5677CE",
> which is specified in coreboot. This patch allows platform
> data to be obtained via ACPI

This actually does two things - it adds the ACPI device probing and it
adds the ACPI platform data.  That last bit is a bit fun, I've added
several of the people working on ACPI here since this is another variant
on the whole ACPI platform data thing which probably needs addressing in
the best practice dissemination which ought to be going on now.

> +static unsigned long long rt5677_parse_acpi_entry(struct device *dev,
> +		acpi_string name)
> +{
> +	acpi_handle handle = ACPI_HANDLE(dev);
> +	unsigned long long val;
> +	acpi_status status;
> +
> +	status = acpi_evaluate_integer(handle, name, NULL, &val);
> +	if (ACPI_FAILURE(status)) {

So, here were defining what's essentially an ACPI property read API
which uses something other than _DSD which is the way all the ACPI
platform data specification for devices is apparently supposed to be
going in order to reuse work between DT and ACPI.

> +static void rt5677_parse_acpi(struct rt5677_priv *rt5677, struct device *dev)
> +{
> +	rt5677->pdata.dmic2_clk_pin = (enum rt5677_dmic2_clk)
> +		rt5677_parse_acpi_entry(dev, "DCLK");
> +	rt5677->pdata.in1_diff = (bool)rt5677_parse_acpi_entry(dev, "IN1");
> +	rt5677->pdata.in2_diff = (bool)rt5677_parse_acpi_entry(dev, "IN2");
> +	rt5677->pdata.lout1_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT1");
> +	rt5677->pdata.lout2_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT2");
> +	rt5677->pdata.lout3_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT3");
> +	rt5677->pdata.jd1_gpio = rt5677_parse_acpi_entry(dev, "JD1");
> +	rt5677->pdata.jd2_gpio = rt5677_parse_acpi_entry(dev, "JD2");
> +	rt5677->pdata.jd3_gpio = rt5677_parse_acpi_entry(dev, "JD3");

Here we read a bunch of properties named with a most definitely non-DT
idiom.  The names here don't look great in general, in particular all
the properties for differential outputs are just boolean flags for the
output name which I'd expect to be flags saying if the output is in use
or not rather than saying if it's in differential mode or single ended
mode.  Things like foo_DIFF would seem better.

My understanding is that best practice for ACPI is to use the new
device_property_read_ APIs which idiomatically take DT style property
names.  However if there's BIOSs out there we need to support perhaps we
just have to deal with this but judging from your e-mail address it
seems this may be for a system intended to run Linux natively so perhaps
that's not an issue.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Grant Likely]

On Sat, Nov 15, 2014 at 6:56 AM, Ben Zhang <benzh@chromium.org> wrote:
> The rt5677 codec driver looks for ACPI device ID "RT5677CE",
> which is specified in coreboot. This patch allows platform
> data to be obtained via ACPI
>
> Signed-off-by: Ben Zhang <benzh@chromium.org>

This looks like an ideal time to talk about shared DT and ACPI driver
bindings. This driver /already/ has a firmware binding. It is
documented in the kernel under
Documentation/bindings/sound/rt5677.txt. We now have a standard method
for sharing bindings between DT and ACPI in the _DSD method[1].
Support for DSD is in linux-next and getting merged into v3.19. This
is exactly the case that _DSD should be used for passing additional
data to the driver, and it should use the existing binding.

[1] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

For a long time we've had the rule on DT that new bindings must be
documented before we merge a patch. That rule I think has been a good
one, even if it is a little chaoitc. I think when it comes to ACPI
drivers that we should be requiring the same: Document the binding,
either in the kernel as a DT binding, or point to somewhere else that
has the binding documented.

Also, since this patch is targeted at v3.19 or later, the
device-properties API should be used. Don't create something custom.

g.

> ---
>  sound/soc/codecs/rt5677.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
> index 5d317c68..384281d 100644
> --- a/sound/soc/codecs/rt5677.c
> +++ b/sound/soc/codecs/rt5677.c
> @@ -20,6 +20,7 @@
>  #include <linux/i2c.h>
>  #include <linux/platform_device.h>
>  #include <linux/spi/spi.h>
> +#include <linux/acpi.h>
>  #include <linux/firmware.h>
>  #include <linux/gpio.h>
>  #include <sound/core.h>
> @@ -4525,6 +4526,43 @@ static int rt5677_parse_dt(struct rt5677_priv *rt5677, struct device_node *np)
>         return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +
> +static unsigned long long rt5677_parse_acpi_entry(struct device *dev,
> +               acpi_string name)
> +{
> +       acpi_handle handle = ACPI_HANDLE(dev);
> +       unsigned long long val;
> +       acpi_status status;
> +
> +       status = acpi_evaluate_integer(handle, name, NULL, &val);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(dev, "Failed to parse ACPI entry %s, default to 0: %d\n",
> +                               name, status);
> +               return 0;
> +       }
> +       return val;
> +}
> +
> +static void rt5677_parse_acpi(struct rt5677_priv *rt5677, struct device *dev)
> +{
> +       rt5677->pdata.dmic2_clk_pin = (enum rt5677_dmic2_clk)
> +               rt5677_parse_acpi_entry(dev, "DCLK");
> +       rt5677->pdata.in1_diff = (bool)rt5677_parse_acpi_entry(dev, "IN1");
> +       rt5677->pdata.in2_diff = (bool)rt5677_parse_acpi_entry(dev, "IN2");
> +       rt5677->pdata.lout1_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT1");
> +       rt5677->pdata.lout2_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT2");
> +       rt5677->pdata.lout3_diff = (bool)rt5677_parse_acpi_entry(dev, "OUT3");
> +       rt5677->pdata.jd1_gpio = rt5677_parse_acpi_entry(dev, "JD1");
> +       rt5677->pdata.jd2_gpio = rt5677_parse_acpi_entry(dev, "JD2");
> +       rt5677->pdata.jd3_gpio = rt5677_parse_acpi_entry(dev, "JD3");
> +}
> +#else
> +static void rt5677_parse_acpi(struct rt5677_priv *rt5677, struct device *dev)
> +{
> +}
> +#endif
> +
>  static struct regmap_irq rt5677_irqs[] = {
>         [RT5677_IRQ_JD1] = {
>                 .reg_offset = 0,
> @@ -4604,6 +4642,7 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
>         if (pdata)
>                 rt5677->pdata = *pdata;
>
> +       rt5677->pow_ldo2 = -EINVAL;
>         if (i2c->dev.of_node) {
>                 ret = rt5677_parse_dt(rt5677, i2c->dev.of_node);
>                 if (ret) {
> @@ -4611,8 +4650,8 @@ static int rt5677_i2c_probe(struct i2c_client *i2c,
>                                 ret);
>                         return ret;
>                 }
> -       } else {
> -               rt5677->pow_ldo2 = -EINVAL;
> +       } else if (ACPI_HANDLE(&i2c->dev)) {
> +               rt5677_parse_acpi(rt5677, &i2c->dev);
>         }
>
>         if (gpio_is_valid(rt5677->pow_ldo2)) {
> @@ -4708,10 +4747,19 @@ static int rt5677_i2c_remove(struct i2c_client *i2c)
>         return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id rt5677_acpi_id[] = {
> +       { "RT5677CE", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(acpi, rt5677_acpi_id);
> +#endif
> +
>  static struct i2c_driver rt5677_i2c_driver = {
>         .driver = {
>                 .name = "rt5677",
>                 .owner = THIS_MODULE,
> +               .acpi_match_table = ACPI_PTR(rt5677_acpi_id),
>         },
>         .probe = rt5677_i2c_probe,
>         .remove   = rt5677_i2c_remove,
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [alsa-devel] [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Liam Girdwood]

On Tue, 2014-11-25 at 12:11 +0000, Grant Likely wrote:
> On Sat, Nov 15, 2014 at 6:56 AM, Ben Zhang <benzh@chromium.org> wrote:
> > The rt5677 codec driver looks for ACPI device ID "RT5677CE",
> > which is specified in coreboot. This patch allows platform
> > data to be obtained via ACPI
> >
> > Signed-off-by: Ben Zhang <benzh@chromium.org>
> 
> This looks like an ideal time to talk about shared DT and ACPI driver
> bindings. This driver /already/ has a firmware binding. It is
> documented in the kernel under
> Documentation/bindings/sound/rt5677.txt. We now have a standard method
> for sharing bindings between DT and ACPI in the _DSD method[1].
> Support for DSD is in linux-next and getting merged into v3.19. This
> is exactly the case that _DSD should be used for passing additional
> data to the driver, and it should use the existing binding.
> 
> [1] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> 
> For a long time we've had the rule on DT that new bindings must be
> documented before we merge a patch. That rule I think has been a good
> one, even if it is a little chaoitc. I think when it comes to ACPI
> drivers that we should be requiring the same: Document the binding,
> either in the kernel as a DT binding, or point to somewhere else that
> has the binding documented.
> 
> Also, since this patch is targeted at v3.19 or later, the
> device-properties API should be used. Don't create something custom.
> 

My sentiments exactly, there would be little point having bespoke device
properties for every single device. Btw, we also need to align here with
Windows too !

Liam

Re: [alsa-devel] [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Darren Hart]

On 11/25/14 06:28, Liam Girdwood wrote:
> On Tue, 2014-11-25 at 12:11 +0000, Grant Likely wrote:
>> On Sat, Nov 15, 2014 at 6:56 AM, Ben Zhang <benzh@chromium.org> wrote:
>>> The rt5677 codec driver looks for ACPI device ID "RT5677CE",
>>> which is specified in coreboot. This patch allows platform
>>> data to be obtained via ACPI
>>>
>>> Signed-off-by: Ben Zhang <benzh@chromium.org>
>>
>> This looks like an ideal time to talk about shared DT and ACPI driver
>> bindings. This driver /already/ has a firmware binding. It is
>> documented in the kernel under
>> Documentation/bindings/sound/rt5677.txt. We now have a standard method
>> for sharing bindings between DT and ACPI in the _DSD method[1].
>> Support for DSD is in linux-next and getting merged into v3.19. This
>> is exactly the case that _DSD should be used for passing additional
>> data to the driver, and it should use the existing binding.
>>
>> [1] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>>
>> For a long time we've had the rule on DT that new bindings must be
>> documented before we merge a patch. That rule I think has been a good
>> one, even if it is a little chaoitc. I think when it comes to ACPI
>> drivers that we should be requiring the same: Document the binding,
>> either in the kernel as a DT binding, or point to somewhere else that
>> has the binding documented.
>>
>> Also, since this patch is targeted at v3.19 or later, the
>> device-properties API should be used. Don't create something custom.
>>
> 
> My sentiments exactly, there would be little point having bespoke device
> properties for every single device. Btw, we also need to align here with
> Windows too !
> 

The Windows folks definitely know about _DSD (and helped define it), so
this is a good opportunity to work through that process. Liam, do you
have a good contact to start that discussion?


-- 
Darren Hart
Intel Open Source Technology Center

Re: [alsa-devel] [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Liam Girdwood]

On Tue, 2014-11-25 at 08:01 -0800, Darren Hart wrote:
> 
> On 11/25/14 06:28, Liam Girdwood wrote:
> > On Tue, 2014-11-25 at 12:11 +0000, Grant Likely wrote:
> >> On Sat, Nov 15, 2014 at 6:56 AM, Ben Zhang <benzh@chromium.org> wrote:
> >>> The rt5677 codec driver looks for ACPI device ID "RT5677CE",
> >>> which is specified in coreboot. This patch allows platform
> >>> data to be obtained via ACPI
> >>>
> >>> Signed-off-by: Ben Zhang <benzh@chromium.org>
> >>
> >> This looks like an ideal time to talk about shared DT and ACPI driver
> >> bindings. This driver /already/ has a firmware binding. It is
> >> documented in the kernel under
> >> Documentation/bindings/sound/rt5677.txt. We now have a standard method
> >> for sharing bindings between DT and ACPI in the _DSD method[1].
> >> Support for DSD is in linux-next and getting merged into v3.19. This
> >> is exactly the case that _DSD should be used for passing additional
> >> data to the driver, and it should use the existing binding.
> >>
> >> [1] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> >>
> >> For a long time we've had the rule on DT that new bindings must be
> >> documented before we merge a patch. That rule I think has been a good
> >> one, even if it is a little chaoitc. I think when it comes to ACPI
> >> drivers that we should be requiring the same: Document the binding,
> >> either in the kernel as a DT binding, or point to somewhere else that
> >> has the binding documented.
> >>
> >> Also, since this patch is targeted at v3.19 or later, the
> >> device-properties API should be used. Don't create something custom.
> >>
> > 
> > My sentiments exactly, there would be little point having bespoke device
> > properties for every single device. Btw, we also need to align here with
> > Windows too !
> > 
> 
> The Windows folks definitely know about _DSD (and helped define it), so
> this is a good opportunity to work through that process. Liam, do you
> have a good contact to start that discussion?
> 

I do, I've contacted them off-list to align on this with Realtek.

Liam

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Darren Hart]

On 11/25/14 04:11, Grant Likely wrote:
> On Sat, Nov 15, 2014 at 6:56 AM, Ben Zhang <benzh@chromium.org> wrote:
>> The rt5677 codec driver looks for ACPI device ID "RT5677CE",
>> which is specified in coreboot. This patch allows platform
>> data to be obtained via ACPI
>>
>> Signed-off-by: Ben Zhang <benzh@chromium.org>
> 
> This looks like an ideal time to talk about shared DT and ACPI driver
> bindings. This driver /already/ has a firmware binding. It is
> documented in the kernel under
> Documentation/bindings/sound/rt5677.txt. We now have a standard method
> for sharing bindings between DT and ACPI in the _DSD method[1].
> Support for DSD is in linux-next and getting merged into v3.19. This
> is exactly the case that _DSD should be used for passing additional
> data to the driver, and it should use the existing binding.
> 
> [1] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> 
> For a long time we've had the rule on DT that new bindings must be
> documented before we merge a patch. That rule I think has been a good
> one, even if it is a little chaoitc. I think when it comes to ACPI
> drivers that we should be requiring the same: Document the binding,
> either in the kernel as a DT binding, or point to somewhere else that
> has the binding documented.
> 
> Also, since this patch is targeted at v3.19 or later, the
> device-properties API should be used. Don't create something custom.

Right. The ACPI/UEFI forum is managing the creation of new DSD bindings
and ensuring they are documented online. I believe this is the... 3rd so
far? So we're still optimizing the process. But yes, please, send the
schema itself for review and let's get this documented and migrated over
to _DSD.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 08:00:12AM -0800, Darren Hart wrote:
> On 11/25/14 04:11, Grant Likely wrote:

> > Also, since this patch is targeted at v3.19 or later, the
> > device-properties API should be used. Don't create something custom.

> Right. The ACPI/UEFI forum is managing the creation of new DSD bindings
> and ensuring they are documented online. I believe this is the... 3rd so
> far? So we're still optimizing the process. But yes, please, send the
> schema itself for review and let's get this documented and migrated over
> to _DSD.

To be clear the main reason I'm querying this is that it doesn't appear
to be a _DSD based binding at all (as far as I understand it, the API
it's using is from before the dawn of time or at least the dawn of git).

Given the design of _DSD is to share with DT and we already have device
tree bindings for the device we should be using, it's not clear to me if
we want to grind them all through UEFI and I suspect they'd be unhappy
if we tried but pretty much all audio CODECs are good candidates for use
with ACPI given the new hardware designs Intel have so if we are doing
it I ought to be bouncing everyone to UEFI forum.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Darren Hart]

On 11/25/14 09:21, Mark Brown wrote:
> On Tue, Nov 25, 2014 at 08:00:12AM -0800, Darren Hart wrote:
>> On 11/25/14 04:11, Grant Likely wrote:
> 
>>> Also, since this patch is targeted at v3.19 or later, the 
>>> device-properties API should be used. Don't create something
>>> custom.
> 
>> Right. The ACPI/UEFI forum is managing the creation of new DSD
>> bindings and ensuring they are documented online. I believe this
>> is the... 3rd so far? So we're still optimizing the process. But
>> yes, please, send the schema itself for review and let's get this
>> documented and migrated over to _DSD.
> 
> To be clear the main reason I'm querying this is that it doesn't
> appear to be a _DSD based binding at all (as far as I understand
> it, the API it's using is from before the dawn of time or at least
> the dawn of git).
> 
> Given the design of _DSD is to share with DT and we already have
> device tree bindings for the device we should be using, it's not
> clear to me if we want to grind them all through UEFI and I suspect
> they'd be unhappy if we tried but pretty much all audio CODECs are
> good candidates for use with ACPI given the new hardware designs
> Intel have so if we are doing it I ought to be bouncing everyone to
> UEFI forum.

Right, I realized between sending and driving into the office that my
statement might be construed this way. I meant *new* _DSD bindings
should go through the ACPI/UEFI forum. Where we can reuse DT bindings,
we should absolutely do that, agreed. We should still document this
and link to the DT binding so it can be referenced and used even when
Linux is not the target OS.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 10:33:01AM -0800, Darren Hart wrote:
> On 11/25/14 09:21, Mark Brown wrote:

> > Given the design of _DSD is to share with DT and we already have
> > device tree bindings for the device we should be using, it's not
> > clear to me if we want to grind them all through UEFI and I suspect
> > they'd be unhappy if we tried but pretty much all audio CODECs are
> > good candidates for use with ACPI given the new hardware designs
> > Intel have so if we are doing it I ought to be bouncing everyone to
> > UEFI forum.

> Right, I realized between sending and driving into the office that my
> statement might be construed this way. I meant *new* _DSD bindings
> should go through the ACPI/UEFI forum. Where we can reuse DT bindings,
> we should absolutely do that, agreed. We should still document this
> and link to the DT binding so it can be referenced and used even when
> Linux is not the target OS.

Link from where - do we want to talk to the ACPI/UEFI forum and figure
out some kind of fast track process for them to add an "it's already
covered by DT, see here" entry to their database for example?  We also
ought to work out how to make sure ACPI IDs are registered there as
well, should be possible to have something simple as part of that.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Darren Hart]

On 11/25/14 10:43, Mark Brown wrote:
> On Tue, Nov 25, 2014 at 10:33:01AM -0800, Darren Hart wrote:
>> On 11/25/14 09:21, Mark Brown wrote:
> 
>>> Given the design of _DSD is to share with DT and we already 
>>> have device tree bindings for the device we should be using, 
>>> it's not clear to me if we want to grind them all through UEFI 
>>> and I suspect they'd be unhappy if we tried but pretty much
>>> all audio CODECs are good candidates for use with ACPI given
>>> the new hardware designs Intel have so if we are doing it I
>>> ought to be bouncing everyone to UEFI forum.
> 
>> Right, I realized between sending and driving into the office 
>> that my statement might be construed this way. I meant *new*
>> _DSD bindings should go through the ACPI/UEFI forum. Where we
>> can reuse DT bindings, we should absolutely do that, agreed. We 
>> should still document this and link to the DT binding so it can 
>> be referenced and used even when Linux is not the target OS.
> 
> Link from where - do we want to talk to the ACPI/UEFI forum and 
> figure out some kind of fast track process for them to add an
> "it's already covered by DT, see here" entry to their database for 
> example?  We also ought to work out how to make sure ACPI IDs are 
> registered there as well, should be possible to have something 
> simple as part of that.
> 

This is a current topic with the ACPI working group. We have the
following document:

http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

I don't know if we want to have a list of them here, or if a separate
document is needed. The important point is that it is independent from
the ACPI specification itself so that it can be updated out of band
with the specification, and not be subject to rather plodding pace
that would imply.

Rafael, I've missed several of these meetings unfortunately, and I'm
not sure if we've closed on this point. Do you know?

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 11:07:06AM -0800, Darren Hart wrote:
> On 11/25/14 10:43, Mark Brown wrote:

> > Link from where - do we want to talk to the ACPI/UEFI forum and 
> > figure out some kind of fast track process for them to add an
> > "it's already covered by DT, see here" entry to their database for 
> > example?  We also ought to work out how to make sure ACPI IDs are 
> > registered there as well, should be possible to have something 
> > simple as part of that.

> This is a current topic with the ACPI working group. We have the
> following document:

> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

> I don't know if we want to have a list of them here, or if a separate
> document is needed. The important point is that it is independent from

Seems to me like some indirection is going to be better rather than
having one big document if there's widespread adoption, avoids lots of
document churn.

> the ACPI specification itself so that it can be updated out of band
> with the specification, and not be subject to rather plodding pace
> that would imply.

> Rafael, I've missed several of these meetings unfortunately, and I'm
> not sure if we've closed on this point. Do you know?

OK, I'll see if any of the people at work have been following as well.
For now continuing to just document everything as DT bindings in the way
we are already seems like a sensible interim approach.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Tuesday, November 25, 2014 07:36:23 PM Mark Brown wrote:
> 
> --njUEgrJvs9ryr/H/
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> 
> On Tue, Nov 25, 2014 at 11:07:06AM -0800, Darren Hart wrote:
> > On 11/25/14 10:43, Mark Brown wrote:
> 
> > > Link from where - do we want to talk to the ACPI/UEFI forum and=20
> > > figure out some kind of fast track process for them to add an
> > > "it's already covered by DT, see here" entry to their database for=20
> > > example?  We also ought to work out how to make sure ACPI IDs are=20
> > > registered there as well, should be possible to have something=20
> > > simple as part of that.
> 
> > This is a current topic with the ACPI working group. We have the
> > following document:
> 
> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-=
> UUID.pdf
> 
> > I don't know if we want to have a list of them here, or if a separate
> > document is needed. The important point is that it is independent from
> 
> Seems to me like some indirection is going to be better rather than
> having one big document if there's widespread adoption, avoids lots of
> document churn.
> 
> > the ACPI specification itself so that it can be updated out of band
> > with the specification, and not be subject to rather plodding pace
> > that would imply.
> 
> > Rafael, I've missed several of these meetings unfortunately, and I'm
> > not sure if we've closed on this point. Do you know?
> 
> OK, I'll see if any of the people at work have been following as well.
> For now continuing to just document everything as DT bindings in the way
> we are already seems like a sensible interim approach.

Yes, that currently is the case in my view too.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Tuesday, November 25, 2014 11:07:06 AM Darren Hart wrote:
> 
> On 11/25/14 10:43, Mark Brown wrote:
> > On Tue, Nov 25, 2014 at 10:33:01AM -0800, Darren Hart wrote:
> >> On 11/25/14 09:21, Mark Brown wrote:
> > 
> >>> Given the design of _DSD is to share with DT and we already 
> >>> have device tree bindings for the device we should be using, 
> >>> it's not clear to me if we want to grind them all through UEFI 
> >>> and I suspect they'd be unhappy if we tried but pretty much
> >>> all audio CODECs are good candidates for use with ACPI given
> >>> the new hardware designs Intel have so if we are doing it I
> >>> ought to be bouncing everyone to UEFI forum.
> > 
> >> Right, I realized between sending and driving into the office 
> >> that my statement might be construed this way. I meant *new*
> >> _DSD bindings should go through the ACPI/UEFI forum. Where we
> >> can reuse DT bindings, we should absolutely do that, agreed. We 
> >> should still document this and link to the DT binding so it can 
> >> be referenced and used even when Linux is not the target OS.
> > 
> > Link from where - do we want to talk to the ACPI/UEFI forum and 
> > figure out some kind of fast track process for them to add an
> > "it's already covered by DT, see here" entry to their database for 
> > example?  We also ought to work out how to make sure ACPI IDs are 
> > registered there as well, should be possible to have something 
> > simple as part of that.
> > 
> 
> This is a current topic with the ACPI working group. We have the
> following document:
> 
> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> 
> I don't know if we want to have a list of them here, or if a separate
> document is needed. The important point is that it is independent from
> the ACPI specification itself so that it can be updated out of band
> with the specification, and not be subject to rather plodding pace
> that would imply.
> 
> Rafael, I've missed several of these meetings unfortunately, and I'm
> not sure if we've closed on this point. Do you know?

This hasn't been discussed a lot at the meetings I attended.

The bindings management process is being set up within the UEFI Forum, but I'm
not sure if/how the existing DT bindings documented in the kernel tree are
going to be covered by it ATM.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 09:31:27PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 25, 2014 11:07:06 AM Darren Hart wrote:

> > This is a current topic with the ACPI working group. We have the
> > following document:

> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

> This hasn't been discussed a lot at the meetings I attended.

> The bindings management process is being set up within the UEFI Forum, but I'm
> not sure if/how the existing DT bindings documented in the kernel tree are
> going to be covered by it ATM.

Al Stone (CCed) pointed me at the following two documents:

http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf
http://www.uefi.org/sites/default/files/resources/nic-request-v2.pdf

(the first one being the actual process in so far as it exists).  The
process appears to be to mail requests in a specific format to the ASWG
chairperson (the address is apparently supposed to be awsg@uefi.org).
It looks like all the properties are expected to end up in one or more
PDF files like the second one.

My initial thought would be to require that we send any DT properties
defined for devices with ACPI identifiers registered there and hope the
volume doesn't DoS them.

A more defined format for DT documentation that we can script into the
ASWG format (or vice versa) might be helpful here, and we should add
notes to the DT documentation if this is how we want to proceed.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Tuesday, November 25, 2014 08:27:22 PM Mark Brown wrote:
> 
> --ReaqsoxgOBHFXBhH
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Tue, Nov 25, 2014 at 09:31:27PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, November 25, 2014 11:07:06 AM Darren Hart wrote:
> 
> > > This is a current topic with the ACPI working group. We have the
> > > following document:
> 
> > > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> 
> > This hasn't been discussed a lot at the meetings I attended.
> 
> > The bindings management process is being set up within the UEFI Forum, but I'm
> > not sure if/how the existing DT bindings documented in the kernel tree are
> > going to be covered by it ATM.
> 
> Al Stone (CCed) pointed me at the following two documents:
> 
> http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf
> http://www.uefi.org/sites/default/files/resources/nic-request-v2.pdf
> 
> (the first one being the actual process in so far as it exists).  The
> process appears to be to mail requests in a specific format to the ASWG
> chairperson (the address is apparently supposed to be awsg@uefi.org).
> It looks like all the properties are expected to end up in one or more
> PDF files like the second one.
> 
> My initial thought would be to require that we send any DT properties
> defined for devices with ACPI identifiers registered there and hope the
> volume doesn't DoS them.

We absolutely need to start registering the existing bindings in there, but
that needs to be rate limited somehow, because the process may not be very
efficient to start with.

> A more defined format for DT documentation that we can script into the
> ASWG format (or vice versa) might be helpful here, and we should add
> notes to the DT documentation if this is how we want to proceed.

That's a good point.

Unfortunately, the timing is pretty bad (Thanksgiving) and the closest
ASWG meeting is next Thursday, but that one's likely to be busy for other
reasons.  I presume, then, that the earliest we can seriously get back to
that in the ASWG is mid-December.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 10:40:53PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 25, 2014 08:27:22 PM Mark Brown wrote:

> > My initial thought would be to require that we send any DT properties
> > defined for devices with ACPI identifiers registered there and hope the
> > volume doesn't DoS them.

> We absolutely need to start registering the existing bindings in there, but
> that needs to be rate limited somehow, because the process may not be very
> efficient to start with.

Well, my upstream connection is 20Mbit or something, that's a limit :P

More seriously there's not that many devices and not much use of the
properties API yet so no concrete users; it's three audio CODECs that
I'm aware of which should use it and are in tree now so probably
actually a reasonable thing to throw out there for people to have a look
at.  Liam, do you think some of the Intel audio people could look into
the code side?  I'm tempted to send at least one document myself for
pipe cleaning purposes.

> > A more defined format for DT documentation that we can script into the
> > ASWG format (or vice versa) might be helpful here, and we should add
> > notes to the DT documentation if this is how we want to proceed.

> That's a good point.

> Unfortunately, the timing is pretty bad (Thanksgiving) and the closest
> ASWG meeting is next Thursday, but that one's likely to be busy for other
> reasons.  I presume, then, that the earliest we can seriously get back to
> that in the ASWG is mid-December.

OK, not the most pressing matter I guess.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Ben Zhang]

+Duncan who works on our firmware project.

Please correct me if I'm wrong. Here is the summary of what I understand. Looks
like the recommended practice for passing device platform data is using _DSD and
the new device_property_read_ API from include/linux/property.h

For example, the firmware (coreboot) should specify something like:
Device (CODC)
{
    Name (_HID, "RT5677CE")
    ...
    Name (_DSD, Package () {
        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
        Package () {
            Package (2) {"realtek,in1-differential", 1},
            Package (2) {"realtek,in2-differential", 0},
            Package (2) {"realtek,gpio-config",  Package (6)  { 0, 0,
0, 0, 0, 2 }},
            ...
        }
    })
    ...
}

And the kernel driver should query with something like:
static void rt5677_read_platform_data(struct rt5677_priv *rt5677,
struct device *dev)
{
    u8 val;

    device_property_read_u8_array(dev, "realtek,in1-differential", &val, 1);
    rt5677->pdata.in1_diff = (bool)val;
    device_property_read_u8_array(dev, "realtek,in2-differential", &val, 1);
    rt5677->pdata.in2_diff = (bool)val;

    device_property_read_u8_array(dev, "realtek,gpio-config",
            rt5677->pdata.gpio_config, RT5677_GPIO_NUM);
    ...
}
And the device property API should work for both DT and ACPI.

Also these device property name keys should be registered with the
ACPI working group
following http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf


On Tue, Nov 25, 2014 at 1:40 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, November 25, 2014 08:27:22 PM Mark Brown wrote:
>>
>> --ReaqsoxgOBHFXBhH
>> Content-Type: text/plain; charset=us-ascii
>> Content-Disposition: inline
>>
>> On Tue, Nov 25, 2014 at 09:31:27PM +0100, Rafael J. Wysocki wrote:
>> > On Tuesday, November 25, 2014 11:07:06 AM Darren Hart wrote:
>>
>> > > This is a current topic with the ACPI working group. We have the
>> > > following document:
>>
>> > > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>>
>> > This hasn't been discussed a lot at the meetings I attended.
>>
>> > The bindings management process is being set up within the UEFI Forum, but I'm
>> > not sure if/how the existing DT bindings documented in the kernel tree are
>> > going to be covered by it ATM.
>>
>> Al Stone (CCed) pointed me at the following two documents:
>>
>> http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf
>> http://www.uefi.org/sites/default/files/resources/nic-request-v2.pdf
>>
>> (the first one being the actual process in so far as it exists).  The
>> process appears to be to mail requests in a specific format to the ASWG
>> chairperson (the address is apparently supposed to be awsg@uefi.org).
>> It looks like all the properties are expected to end up in one or more
>> PDF files like the second one.
>>
>> My initial thought would be to require that we send any DT properties
>> defined for devices with ACPI identifiers registered there and hope the
>> volume doesn't DoS them.
>
> We absolutely need to start registering the existing bindings in there, but
> that needs to be rate limited somehow, because the process may not be very
> efficient to start with.
>
>> A more defined format for DT documentation that we can script into the
>> ASWG format (or vice versa) might be helpful here, and we should add
>> notes to the DT documentation if this is how we want to proceed.
>
> That's a good point.
>
> Unfortunately, the timing is pretty bad (Thanksgiving) and the closest
> ASWG meeting is next Thursday, but that one's likely to be busy for other
> reasons.  I presume, then, that the earliest we can seriously get back to
> that in the ASWG is mid-December.
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 02:41:28PM -0800, Ben Zhang wrote:
> +Duncan who works on our firmware project.
> 
> Please correct me if I'm wrong. Here is the summary of what I understand. Looks
> like the recommended practice for passing device platform data is using _DSD and
> the new device_property_read_ API from include/linux/property.h

Your understanding matches mine here.  Doesn't mean I'm not confused
though!

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Grant Likely]

On Tue, 25 Nov 2014 22:40:53 +0100
, "Rafael J. Wysocki" <rjw@rjwysocki.net>
 wrote:
> On Tuesday, November 25, 2014 08:27:22 PM Mark Brown wrote:
> > 
> > --ReaqsoxgOBHFXBhH
> > Content-Type: text/plain; charset=us-ascii
> > Content-Disposition: inline
> > 
> > On Tue, Nov 25, 2014 at 09:31:27PM +0100, Rafael J. Wysocki wrote:
> > > On Tuesday, November 25, 2014 11:07:06 AM Darren Hart wrote:
> > 
> > > > This is a current topic with the ACPI working group. We have the
> > > > following document:
> > 
> > > > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> > 
> > > This hasn't been discussed a lot at the meetings I attended.
> > 
> > > The bindings management process is being set up within the UEFI Forum, but I'm
> > > not sure if/how the existing DT bindings documented in the kernel tree are
> > > going to be covered by it ATM.
> > 
> > Al Stone (CCed) pointed me at the following two documents:
> > 
> > http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf
> > http://www.uefi.org/sites/default/files/resources/nic-request-v2.pdf
> > 
> > (the first one being the actual process in so far as it exists).  The
> > process appears to be to mail requests in a specific format to the ASWG
> > chairperson (the address is apparently supposed to be awsg@uefi.org).
> > It looks like all the properties are expected to end up in one or more
> > PDF files like the second one.
> > 
> > My initial thought would be to require that we send any DT properties
> > defined for devices with ACPI identifiers registered there and hope the
> > volume doesn't DoS them.
> 
> We absolutely need to start registering the existing bindings in there, but
> that needs to be rate limited somehow, because the process may not be very
> efficient to start with.

Beyond having the document point to the existing DT binding
documentation, I think this is a non-starter. It won't be helpful for
anyone to have two separate repositories containing the same bindings.
They will get out of sync and we will have pain as a result.

For existing bindings we need to have a way to share the documentation,
and I don't think we can even talk about whether it makes sense to
migrate the documetation to the UEFI managed repo before the UEFI process is
fully flushed out.

g.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Thu, Dec 04, 2014 at 10:48:19AM +0000, Grant Likely wrote:
> On Tue, 25 Nov 2014 22:40:53 +0100

> > We absolutely need to start registering the existing bindings in there, but
> > that needs to be rate limited somehow, because the process may not be very
> > efficient to start with.

> Beyond having the document point to the existing DT binding
> documentation, I think this is a non-starter. It won't be helpful for
> anyone to have two separate repositories containing the same bindings.
> They will get out of sync and we will have pain as a result.

Right, I think we have to pick one and reference it from the other.
Having the one place be inside the Linux kernel doesn't seem super
awesome.

> For existing bindings we need to have a way to share the documentation,
> and I don't think we can even talk about whether it makes sense to
> migrate the documetation to the UEFI managed repo before the UEFI process is
> fully flushed out.

We definitely don't want to start dumping everything in immediately but
but starting the conversation to both provide urgency and requirements
and and be engaged in the process seems sensible.  Let's try to take
advantage of the recent changes in UEFI governance to help shape the
firmware interfaces we'll be working with.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Thursday, December 04, 2014 10:48:19 AM Grant Likely wrote:
> On Tue, 25 Nov 2014 22:40:53 +0100
> , "Rafael J. Wysocki" <rjw@rjwysocki.net>
>  wrote:
> > On Tuesday, November 25, 2014 08:27:22 PM Mark Brown wrote:
> > > 
> > > --ReaqsoxgOBHFXBhH
> > > Content-Type: text/plain; charset=us-ascii
> > > Content-Disposition: inline
> > > 
> > > On Tue, Nov 25, 2014 at 09:31:27PM +0100, Rafael J. Wysocki wrote:
> > > > On Tuesday, November 25, 2014 11:07:06 AM Darren Hart wrote:
> > > 
> > > > > This is a current topic with the ACPI working group. We have the
> > > > > following document:
> > > 
> > > > > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> > > 
> > > > This hasn't been discussed a lot at the meetings I attended.
> > > 
> > > > The bindings management process is being set up within the UEFI Forum, but I'm
> > > > not sure if/how the existing DT bindings documented in the kernel tree are
> > > > going to be covered by it ATM.
> > > 
> > > Al Stone (CCed) pointed me at the following two documents:
> > > 
> > > http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf
> > > http://www.uefi.org/sites/default/files/resources/nic-request-v2.pdf
> > > 
> > > (the first one being the actual process in so far as it exists).  The
> > > process appears to be to mail requests in a specific format to the ASWG
> > > chairperson (the address is apparently supposed to be awsg@uefi.org).
> > > It looks like all the properties are expected to end up in one or more
> > > PDF files like the second one.
> > > 
> > > My initial thought would be to require that we send any DT properties
> > > defined for devices with ACPI identifiers registered there and hope the
> > > volume doesn't DoS them.
> > 
> > We absolutely need to start registering the existing bindings in there, but
> > that needs to be rate limited somehow, because the process may not be very
> > efficient to start with.
> 
> Beyond having the document point to the existing DT binding
> documentation, I think this is a non-starter. It won't be helpful for
> anyone to have two separate repositories containing the same bindings.

If they *are* the same, it won't.  But at least in some cases (eg. GPIO)
they won't be the same.

Also we need a way to say "this binding has been documented elsewhere, see XYZ
and this is how to translate the format" in the UEFI documentation for the
existing bindings that will be the same.

> They will get out of sync and we will have pain as a result.
> 
> For existing bindings we need to have a way to share the documentation,
> and I don't think we can even talk about whether it makes sense to
> migrate the documetation to the UEFI managed repo before the UEFI process is
> fully flushed out.

Agreed.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Darren Hart]

On 11/25/14 10:43, Mark Brown wrote:
> On Tue, Nov 25, 2014 at 10:33:01AM -0800, Darren Hart wrote:
>> On 11/25/14 09:21, Mark Brown wrote:
> 
>>> Given the design of _DSD is to share with DT and we already
>>> have device tree bindings for the device we should be using,
>>> it's not clear to me if we want to grind them all through UEFI
>>> and I suspect they'd be unhappy if we tried but pretty much all
>>> audio CODECs are good candidates for use with ACPI given the
>>> new hardware designs Intel have so if we are doing it I ought
>>> to be bouncing everyone to UEFI forum.
> 
>> Right, I realized between sending and driving into the office
>> that my statement might be construed this way. I meant *new* _DSD
>> bindings should go through the ACPI/UEFI forum. Where we can
>> reuse DT bindings, we should absolutely do that, agreed. We
>> should still document this and link to the DT binding so it can
>> be referenced and used even when Linux is not the target OS.
> 
> Link from where - do we want to talk to the ACPI/UEFI forum and
> figure out some kind of fast track process for them to add an "it's
> already covered by DT, see here" entry to their database for
> example?  We also ought to work out how to make sure ACPI IDs are
> registered there as well, should be possible to have something
> simple as part of that.
> 

As to registering ACPI IDs, I believe this is the right link:
http://www.uefi.org/PNP_ACPI_Registry

Or did you mean a HID/CID<->DSD mapping?

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Tue, Nov 25, 2014 at 05:48:24PM -0800, Darren Hart wrote:
> On 11/25/14 10:43, Mark Brown wrote:

> > Link from where - do we want to talk to the ACPI/UEFI forum and
> > figure out some kind of fast track process for them to add an "it's
> > already covered by DT, see here" entry to their database for
> > example?  We also ought to work out how to make sure ACPI IDs are
> > registered there as well, should be possible to have something
> > simple as part of that.

> As to registering ACPI IDs, I believe this is the right link:
> http://www.uefi.org/PNP_ACPI_Registry

No, those are vendors as far as I can tell.  I mean identifiers for
specific devices - it appears to be common for for example Intel to
allocate identifiers for devices they don't produce, I'd expect there to
be some effort to keep track of that especially given that _DSD
properties may well end up being specific to the identifier used to
register in cases of parallel evolution.

> Or did you mean a HID/CID<->DSD mapping?

I don't really know what that is, sorry.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Wednesday, November 26, 2014 11:17:16 AM Mark Brown wrote:
> 
> --4mL5lf+nIvB09VHj
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Tue, Nov 25, 2014 at 05:48:24PM -0800, Darren Hart wrote:
> > On 11/25/14 10:43, Mark Brown wrote:
> 
> > > Link from where - do we want to talk to the ACPI/UEFI forum and
> > > figure out some kind of fast track process for them to add an "it's
> > > already covered by DT, see here" entry to their database for
> > > example?  We also ought to work out how to make sure ACPI IDs are
> > > registered there as well, should be possible to have something
> > > simple as part of that.
> 
> > As to registering ACPI IDs, I believe this is the right link:
> > http://www.uefi.org/PNP_ACPI_Registry
> 
> No, those are vendors as far as I can tell.  I mean identifiers for
> specific devices - it appears to be common for for example Intel to
> allocate identifiers for devices they don't produce, I'd expect there to
> be some effort to keep track of that especially given that _DSD
> properties may well end up being specific to the identifier used to
> register in cases of parallel evolution.

The vendor (or more precisely the owner of the initial 3 or 4 letter code)
is supposed to do that.  I'm not aware of any common registry of those IDs
for all vendors.

> > Or did you mean a HID/CID<->DSD mapping?
> 
> I don't really know what that is, sorry.

The device ID is supposed to determine the way all of the ACPI objects for that
device will work, including what is returned by _DSD.  Pretty much in analogy
with PCI device IDs.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Thu, Nov 27, 2014 at 12:09:55AM +0100, Rafael J. Wysocki wrote:
> On Wednesday, November 26, 2014 11:17:16 AM Mark Brown wrote:

> > > As to registering ACPI IDs, I believe this is the right link:
> > > http://www.uefi.org/PNP_ACPI_Registry

> > No, those are vendors as far as I can tell.  I mean identifiers for
> > specific devices - it appears to be common for for example Intel to
> > allocate identifiers for devices they don't produce, I'd expect there to
> > be some effort to keep track of that especially given that _DSD
> > properties may well end up being specific to the identifier used to
> > register in cases of parallel evolution.

> The vendor (or more precisely the owner of the initial 3 or 4 letter code)
> is supposed to do that.  I'm not aware of any common registry of those IDs
> for all vendors.

OK, we probably should have one to aid discoverability since as far as I
can tell what's happening is that people (hi Intel!) are allocating
their own identifiers for devices produced by other vendors that turn up
on their boards.  If people can find the set of IDs in use there's more
chance they'll use the same ones as other people.

> > > Or did you mean a HID/CID<->DSD mapping?

> > I don't really know what that is, sorry.

> The device ID is supposed to determine the way all of the ACPI objects for that
> device will work, including what is returned by _DSD.  Pretty much in analogy
> with PCI device IDs.

So the HID and CID are device IDs then?  Please bear in mind that we're
not all familiar with the acronym soup that tends to go along with ACPI!

If those are device IDs then what you're saying is what I'd expect to
happen and it's part of the reason I'd expect us to be registering IDs
along with registering properties - if people are defining device
specific properties they really ought to be tied to the IDs that are in
use especially if (as seems likely to be the case with the current state
of the world) people are doing things without attempting to coordinate
and we're ending up trying to document the deployed reality.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Friday, November 28, 2014 04:00:36 PM Mark Brown wrote:
> 
> --k9ssVBY1NpawPNl1
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Thu, Nov 27, 2014 at 12:09:55AM +0100, Rafael J. Wysocki wrote:
> > On Wednesday, November 26, 2014 11:17:16 AM Mark Brown wrote:
> 
> > > > As to registering ACPI IDs, I believe this is the right link:
> > > > http://www.uefi.org/PNP_ACPI_Registry
> 
> > > No, those are vendors as far as I can tell.  I mean identifiers for
> > > specific devices - it appears to be common for for example Intel to
> > > allocate identifiers for devices they don't produce, I'd expect there to
> > > be some effort to keep track of that especially given that _DSD
> > > properties may well end up being specific to the identifier used to
> > > register in cases of parallel evolution.
> 
> > The vendor (or more precisely the owner of the initial 3 or 4 letter code)
> > is supposed to do that.  I'm not aware of any common registry of those IDs
> > for all vendors.
> 
> OK, we probably should have one to aid discoverability since as far as I
> can tell what's happening is that people (hi Intel!) are allocating
> their own identifiers for devices produced by other vendors that turn up
> on their boards.  If people can find the set of IDs in use there's more
> chance they'll use the same ones as other people.

There's the PRP0001 ID that can be use to in combination with the
"compatible" property which then works the same way as for DT.

That should be sufficient if the properties are going to be the
same for ACPI (_DSD) and DT.

> > > > Or did you mean a HID/CID<->DSD mapping?
> 
> > > I don't really know what that is, sorry.
> 
> > The device ID is supposed to determine the way all of the ACPI objects for that
> > device will work, including what is returned by _DSD.  Pretty much in analogy
> > with PCI device IDs.
> 
> So the HID and CID are device IDs then?  Please bear in mind that we're
> not all familiar with the acronym soup that tends to go along with ACPI!

Yes, they are supposed to be treated this way.

> If those are device IDs then what you're saying is what I'd expect to
> happen and it's part of the reason I'd expect us to be registering IDs
> along with registering properties - if people are defining device
> specific properties they really ought to be tied to the IDs that are in
> use especially if (as seems likely to be the case with the current state
> of the world) people are doing things without attempting to coordinate
> and we're ending up trying to document the deployed reality.

The rule of thumb (in my view) should be that if the device object's _DSD is
going to return the same set of properties as for DT and no other special
handling determined by the ID is required (ie. nothing along the lines of
"if the device ID is X, the _ABC method works like that" which has to be known
by the driver), "PRP0001" should be returned by _HID and then the value of the
"compatible" property should be the same as for DT.

In other words, "PRP0001" means "a generic device with properties as returned
by _DSD".

Otherwise, a new device ID needs to be allocated for the device and _HID
should return that ID.  Also, if the device is compatible with another
device with an already allocated ID (for _HID), _CID may return the device ID
of the compatible device.  [Of course, it's better if _HID is the same for
identical devices.]


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Sat, Nov 29, 2014 at 12:51:59AM +0100, Rafael J. Wysocki wrote:
> On Friday, November 28, 2014 04:00:36 PM Mark Brown wrote:

> > OK, we probably should have one to aid discoverability since as far as I
> > can tell what's happening is that people (hi Intel!) are allocating
> > their own identifiers for devices produced by other vendors that turn up
> > on their boards.  If people can find the set of IDs in use there's more
> > chance they'll use the same ones as other people.

> There's the PRP0001 ID that can be use to in combination with the
> "compatible" property which then works the same way as for DT.

> That should be sufficient if the properties are going to be the
> same for ACPI (_DSD) and DT.

We've got people making BIOSs right now for systems you can actually buy 
that are intended to run Windows which we need to support; right now the
pressing problem I'm seeing is that we've got BIOS vendors not using
properties at all and we're going to be ending up with configuration
coming from big DMI tables instead which is miserable, Windows is
perfectly happy to have custom drivers installed per machine.  Do we
know if Windows supports PRP0001 as it currently stands?

> > If those are device IDs then what you're saying is what I'd expect to
> > happen and it's part of the reason I'd expect us to be registering IDs
> > along with registering properties - if people are defining device
> > specific properties they really ought to be tied to the IDs that are in
> > use especially if (as seems likely to be the case with the current state
> > of the world) people are doing things without attempting to coordinate
> > and we're ending up trying to document the deployed reality.

> The rule of thumb (in my view) should be that if the device object's _DSD is
> going to return the same set of properties as for DT and no other special
> handling determined by the ID is required (ie. nothing along the lines of
> "if the device ID is X, the _ABC method works like that" which has to be known
> by the driver), "PRP0001" should be returned by _HID and then the value of the
> "compatible" property should be the same as for DT.

This is a reasonable idea but we do need it to be compatible with
Windows and we still need to make sure we have a process in place that
BIOS authors and driver authors focused on Windows can reasonably be
expected to work with.  That's not something we have right now for DT
(our DT process involves contributing to Linux) so the problem remains
effectively the same.

We also need a way of getting the word out to people that they should be
doing this (also a problem no matter if we use PRP0001 or something UEFI
specific).

> Otherwise, a new device ID needs to be allocated for the device and _HID
> should return that ID.  Also, if the device is compatible with another
> device with an already allocated ID (for _HID), _CID may return the device ID
> of the compatible device.  [Of course, it's better if _HID is the same for
> identical devices.]

I honestly don't see BIOS authors as being likely to care about adding
things to the CID if their systems are working (I'm assuming that HID is
the primary device ID and CID are further backup compatible IDs).

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Saturday, November 29, 2014 11:52:09 AM Mark Brown wrote:
> 
> --9GYGtdBumnmR69ER
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> 
> On Sat, Nov 29, 2014 at 12:51:59AM +0100, Rafael J. Wysocki wrote:
> > On Friday, November 28, 2014 04:00:36 PM Mark Brown wrote:
> 
> > > OK, we probably should have one to aid discoverability since as far as I
> > > can tell what's happening is that people (hi Intel!) are allocating
> > > their own identifiers for devices produced by other vendors that turn up
> > > on their boards.  If people can find the set of IDs in use there's more
> > > chance they'll use the same ones as other people.
> 
> > There's the PRP0001 ID that can be use to in combination with the
> > "compatible" property which then works the same way as for DT.
> 
> > That should be sufficient if the properties are going to be the
> > same for ACPI (_DSD) and DT.
> 
> We've got people making BIOSs right now for systems you can actually buy=20
> that are intended to run Windows which we need to support; right now the
> pressing problem I'm seeing is that we've got BIOS vendors not using
> properties at all and we're going to be ending up with configuration
> coming from big DMI tables instead which is miserable, Windows is
> perfectly happy to have custom drivers installed per machine.  Do we
> know if Windows supports PRP0001 as it currently stands?

No, it doesn't.

Separate device IDs are necessary for Windows compatibility AFAICS.

But that also means any device ID registered by us won't be suitable in that
case, because Windows won't use it.

There are two different problems here, though.  The first one is a way to
provide the existing Linux drivers with the information expected by them via
ACPI and that's what _DSD (plus PRP0001 optionally) is.  The second one is
to be able to handle systems with ACPI tables from a random vendor who only
cares about Windows and that's more difficult to address, because our
ecosystem is different from theirs.

There basically are two ways around that.  The first one is to have all
knowledge related to device IDs in drivers (which effectively is what
Windows does and which implies "board files" of sorts) and the second one is
to make it possible to use overlays on top of the existing ACPI tables that
will allow people to provide the properties expected by a more generic driver
(this way, if the vendor didn't care to provide _DSD, for example, in the
original ACPI tables, the system integrator would be able to use an overlay
in an initramfs or boot partition to amend them).

There is a plan to do the latter, but that is going to take some time to
complete.

> > > If those are device IDs then what you're saying is what I'd expect to
> > > happen and it's part of the reason I'd expect us to be registering IDs
> > > along with registering properties - if people are defining device
> > > specific properties they really ought to be tied to the IDs that are in
> > > use especially if (as seems likely to be the case with the current state
> > > of the world) people are doing things without attempting to coordinate
> > > and we're ending up trying to document the deployed reality.
> 
> > The rule of thumb (in my view) should be that if the device object's _DSD=
>  is
> > going to return the same set of properties as for DT and no other special
> > handling determined by the ID is required (ie. nothing along the lines of
> > "if the device ID is X, the _ABC method works like that" which has to be =
> known
> > by the driver), "PRP0001" should be returned by _HID and then the value o=
> f the
> > "compatible" property should be the same as for DT.
> 
> This is a reasonable idea but we do need it to be compatible with
> Windows and we still need to make sure we have a process in place that
> BIOS authors and driver authors focused on Windows can reasonably be
> expected to work with.  That's not something we have right now for DT
> (our DT process involves contributing to Linux) so the problem remains
> effectively the same.

We can't make Windows use our stuff and they have a different model
(a custom driver per device ID vs a common driver per IP block possibly
matching multiple device IDs).

> We also need a way of getting the word out to people that they should be
> doing this (also a problem no matter if we use PRP0001 or something UEFI
> specific).

What do you mean by "something UEFI specific"?

> > Otherwise, a new device ID needs to be allocated for the device and _HID
> > should return that ID.  Also, if the device is compatible with another
> > device with an already allocated ID (for _HID), _CID may return the devic=
> e ID
> > of the compatible device.  [Of course, it's better if _HID is the same for
> > identical devices.]
> 
> I honestly don't see BIOS authors as being likely to care about adding
> things to the CID if their systems are working (I'm assuming that HID is
> the primary device ID and CID are further backup compatible IDs).

They will if they care to be compatible with old (binary) OSes that didn't
know about the new device ID, but shipped drivers that could handle the
device (although possibly in a "crippled" way).


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Sat, Nov 29, 2014 at 11:27:52PM +0100, Rafael J. Wysocki wrote:
> On Saturday, November 29, 2014 11:52:09 AM Mark Brown wrote:

> > > There's the PRP0001 ID that can be use to in combination with the
> > > "compatible" property which then works the same way as for DT.

> > > That should be sufficient if the properties are going to be the
> > > same for ACPI (_DSD) and DT.

> > We've got people making BIOSs right now for systems you can actually buy=20
> > that are intended to run Windows which we need to support; right now the
> > pressing problem I'm seeing is that we've got BIOS vendors not using
> > properties at all and we're going to be ending up with configuration
> > coming from big DMI tables instead which is miserable, Windows is
> > perfectly happy to have custom drivers installed per machine.  Do we
> > know if Windows supports PRP0001 as it currently stands?

> No, it doesn't.

> Separate device IDs are necessary for Windows compatibility AFAICS.

OK, then PRP0001 is a complete non-starter for the applications I'm
looking at.

> But that also means any device ID registered by us won't be suitable in that
> case, because Windows won't use it.

The dream here is that people working on building systems, people
working on Windows drivers and people working on Linux drivers will at
some point be able to collaborate.  If we're going to go off and do our
own thing for Linux without talking to anyone that's not really
addressing the issue.

> There are two different problems here, though.  The first one is a way to
> provide the existing Linux drivers with the information expected by them via
> ACPI and that's what _DSD (plus PRP0001 optionally) is.  The second one is
> to be able to handle systems with ACPI tables from a random vendor who only
> cares about Windows and that's more difficult to address, because our
> ecosystem is different from theirs.

> There basically are two ways around that.  The first one is to have all
> knowledge related to device IDs in drivers (which effectively is what
> Windows does and which implies "board files" of sorts) and the second one is
> to make it possible to use overlays on top of the existing ACPI tables that
> will allow people to provide the properties expected by a more generic driver
> (this way, if the vendor didn't care to provide _DSD, for example, in the
> original ACPI tables, the system integrator would be able to use an overlay
> in an initramfs or boot partition to amend them).

There's also the option that Windows drivers start using _DSD themselves
which is, I understand, the goal towards which the people working on at
least audio are heading.

> There is a plan to do the latter, but that is going to take some time to
> complete.

Like I keep saying we have people trying to ship systems now, you can
buy affected systems already and there's going to be more coming and I'm
not convinced it's going to be fun for system integrators (distros?) to
have to quirk every system that comes out.

> > This is a reasonable idea but we do need it to be compatible with
> > Windows and we still need to make sure we have a process in place that
> > BIOS authors and driver authors focused on Windows can reasonably be
> > expected to work with.  That's not something we have right now for DT
> > (our DT process involves contributing to Linux) so the problem remains
> > effectively the same.

> We can't make Windows use our stuff and they have a different model
> (a custom driver per device ID vs a common driver per IP block possibly
> matching multiple device IDs).

The hardware already doesn't match the Windows model well here, it's
causing them some problems too (and it's making life miserable for the
CODEC vendors who want to ship both Windows and Linux drivers too).  We
should at least try to pull things together so we get something that
works for everyone; in Linux we have quite a bit of experience with this
sort of system which we can hopefully share with the Windows people to
help make everyone's lives better.

One of the things that would help this would be if we were able to give
people a way to write their ACPI tables in a way that is likely to both
work well for us and avoid lots of make work for the Windows people.

Note that all this discussion is pretty much about drivers for single
devices which can be wired into the system in a flexible manner, even in
a Windows world you won't vary the device ID.  At present we're quirking
on DMI.

> > We also need a way of getting the word out to people that they should be
> > doing this (also a problem no matter if we use PRP0001 or something UEFI
> > specific).

> What do you mean by "something UEFI specific"?

Sorry, I mean ACPI specific (UEFI forum).

> > > Otherwise, a new device ID needs to be allocated for the device and _HID
> > > should return that ID.  Also, if the device is compatible with another
> > > device with an already allocated ID (for _HID), _CID may return the devic=
> > e ID
> > > of the compatible device.  [Of course, it's better if _HID is the same for
> > > identical devices.]

> > I honestly don't see BIOS authors as being likely to care about adding
> > things to the CID if their systems are working (I'm assuming that HID is
> > the primary device ID and CID are further backup compatible IDs).

> They will if they care to be compatible with old (binary) OSes that didn't
> know about the new device ID, but shipped drivers that could handle the
> device (although possibly in a "crippled" way).

It's not just the device IDs you need, it's the properties too.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

On Monday, December 01, 2014 05:51:00 PM Mark Brown wrote:
> 
> --MRL9B5bOucwGAkwp
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Sat, Nov 29, 2014 at 11:27:52PM +0100, Rafael J. Wysocki wrote:
> > On Saturday, November 29, 2014 11:52:09 AM Mark Brown wrote:
> 
> > > > There's the PRP0001 ID that can be use to in combination with the
> > > > "compatible" property which then works the same way as for DT.
> 
> > > > That should be sufficient if the properties are going to be the
> > > > same for ACPI (_DSD) and DT.
> 
> > > We've got people making BIOSs right now for systems you can actually buy=20
> > > that are intended to run Windows which we need to support; right now the
> > > pressing problem I'm seeing is that we've got BIOS vendors not using
> > > properties at all and we're going to be ending up with configuration
> > > coming from big DMI tables instead which is miserable, Windows is
> > > perfectly happy to have custom drivers installed per machine.  Do we
> > > know if Windows supports PRP0001 as it currently stands?
> 
> > No, it doesn't.
> 
> > Separate device IDs are necessary for Windows compatibility AFAICS.
> 
> OK, then PRP0001 is a complete non-starter for the applications I'm
> looking at.
> 
> > But that also means any device ID registered by us won't be suitable in that
> > case, because Windows won't use it.
> 
> The dream here is that people working on building systems, people
> working on Windows drivers and people working on Linux drivers will at
> some point be able to collaborate. If we're going to go off and do our
> own thing for Linux without talking to anyone that's not really addressing
> the issue.

Well, that's already going on in the DT land, isn't it?  It has been going on
for quite a while AFAICS.

> > There are two different problems here, though.  The first one is a way to
> > provide the existing Linux drivers with the information expected by them via
> > ACPI and that's what _DSD (plus PRP0001 optionally) is.  The second one is
> > to be able to handle systems with ACPI tables from a random vendor who only
> > cares about Windows and that's more difficult to address, because our
> > ecosystem is different from theirs.
> 
> > There basically are two ways around that.  The first one is to have all
> > knowledge related to device IDs in drivers (which effectively is what
> > Windows does and which implies "board files" of sorts) and the second one is
> > to make it possible to use overlays on top of the existing ACPI tables that
> > will allow people to provide the properties expected by a more generic driver
> > (this way, if the vendor didn't care to provide _DSD, for example, in the
> > original ACPI tables, the system integrator would be able to use an overlay
> > in an initramfs or boot partition to amend them).
> 
> There's also the option that Windows drivers start using _DSD themselves
> which is, I understand, the goal towards which the people working on at
> least audio are heading.

Technically, Windows driver writers can evaluate _DSD and handle the
information the way we do, but I'm not sure if this is really convenient for
them.

We use _DSD, because we want drivers to work with DT as well with ACPI without
adding special DT-specific or ACPI-specific code to them.  The people who work
on Windows drivers don't have this problem, so as I said, if they care about
Linux at all, that may be a good enough motivation for them to look at _DSD,
but if they don't, I honestly don't see why they would do that.

> > There is a plan to do the latter, but that is going to take some time to
> > complete.
> 
> Like I keep saying we have people trying to ship systems now, you can
> buy affected systems already and there's going to be more coming and I'm
> not convinced it's going to be fun for system integrators (distros?) to
> have to quirk every system that comes out.

I'm not sure what we can do to prevent the above from happening, realistically.
Yes, we are late.  No, we can't speed up things too much AFAICS.

> > > This is a reasonable idea but we do need it to be compatible with
> > > Windows and we still need to make sure we have a process in place that
> > > BIOS authors and driver authors focused on Windows can reasonably be
> > > expected to work with.  That's not something we have right now for DT
> > > (our DT process involves contributing to Linux) so the problem remains
> > > effectively the same.
> 
> > We can't make Windows use our stuff and they have a different model
> > (a custom driver per device ID vs a common driver per IP block possibly
> > matching multiple device IDs).
> 
> The hardware already doesn't match the Windows model well here, it's
> causing them some problems too (and it's making life miserable for the
> CODEC vendors who want to ship both Windows and Linux drivers too).  We
> should at least try to pull things together so we get something that
> works for everyone; in Linux we have quite a bit of experience with this
> sort of system which we can hopefully share with the Windows people to
> help make everyone's lives better.

That would be a great outcome, but I'm kind of cautious here ...

> One of the things that would help this would be if we were able to give
> people a way to write their ACPI tables in a way that is likely to both
> work well for us and avoid lots of make work for the Windows people.
> 
> Note that all this discussion is pretty much about drivers for single
> devices which can be wired into the system in a flexible manner, even in
> a Windows world you won't vary the device ID.  At present we're quirking
> on DMI.

So the answer to that in my view is: Use _DSD and allocate your own device IDs
for Windows drivers to bind to.

> > > We also need a way of getting the word out to people that they should be
> > > doing this (also a problem no matter if we use PRP0001 or something UEFI
> > > specific).
> 
> > What do you mean by "something UEFI specific"?
> 
> Sorry, I mean ACPI specific (UEFI forum).

Do you mean a special device ID of some sort, then?

> > > > Otherwise, a new device ID needs to be allocated for the device and _HID
> > > > should return that ID.  Also, if the device is compatible with another
> > > > device with an already allocated ID (for _HID), _CID may return the devic=
> > > e ID
> > > > of the compatible device.  [Of course, it's better if _HID is the same for
> > > > identical devices.]
> 
> > > I honestly don't see BIOS authors as being likely to care about adding
> > > things to the CID if their systems are working (I'm assuming that HID is
> > > the primary device ID and CID are further backup compatible IDs).
> 
> > They will if they care to be compatible with old (binary) OSes that didn't
> > know about the new device ID, but shipped drivers that could handle the
> > device (although possibly in a "crippled" way).
> 
> It's not just the device IDs you need, it's the properties too.

I suppose you have some specific examples in mind that I may not be familiar
with and we may spend an arbitrary amount of time speaking past each other. :-)

I'm quite confident that _DSD can be used to address the problem at hand, but
for that it needs to be adopted by platform firmware writers.  I personally
have no power to make that happen and even then it would not be realistic to
expect that to happen overnight.

Overlays can be used as a workaround here, but as I said it is going to take
some time to make them work reliably enough.

That's where we are today.  Do you have any suggestions on what else we can do?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Mon, Dec 01, 2014 at 11:16:31PM +0100, Rafael J. Wysocki wrote:
> On Monday, December 01, 2014 05:51:00 PM Mark Brown wrote:

> > The dream here is that people working on building systems, people
> > working on Windows drivers and people working on Linux drivers will at
> > some point be able to collaborate. If we're going to go off and do our
> > own thing for Linux without talking to anyone that's not really addressing
> > the issue.

> Well, that's already going on in the DT land, isn't it?  It has been going on
> for quite a while AFAICS.

In theory (and where it's actually relevant in practice to at least some
extent) this stuff is all OS neutral, there's a definite willingness for
it to be so.

> > There's also the option that Windows drivers start using _DSD themselves
> > which is, I understand, the goal towards which the people working on at
> > least audio are heading.

> Technically, Windows driver writers can evaluate _DSD and handle the
> information the way we do, but I'm not sure if this is really convenient for
> them.

I'm not sure how convenient it is, though I'm reasonably sure a helper
library could make it so.

> We use _DSD, because we want drivers to work with DT as well with ACPI without
> adding special DT-specific or ACPI-specific code to them.  The people who work
> on Windows drivers don't have this problem, so as I said, if they care about
> Linux at all, that may be a good enough motivation for them to look at _DSD,
> but if they don't, I honestly don't see why they would do that.

They care about getting properties out, or at least they should, and in
this market many of the device vendors care about Linux at least as much
as they do Windows (sometimes even more than they care about Windows,
there's overlap with the Android market).

> > Note that all this discussion is pretty much about drivers for single
> > devices which can be wired into the system in a flexible manner, even in
> > a Windows world you won't vary the device ID.  At present we're quirking
> > on DMI.

> So the answer to that in my view is: Use _DSD and allocate your own device IDs
> for Windows drivers to bind to.

Right, so we circle back to the original question about documenting
those IDs and _DSD properties.  :)

> > > > We also need a way of getting the word out to people that they should be
> > > > doing this (also a problem no matter if we use PRP0001 or something UEFI
> > > > specific).

> > > What do you mean by "something UEFI specific"?

> > Sorry, I mean ACPI specific (UEFI forum).

> Do you mean a special device ID of some sort, then?

No, just a regular one.

> > It's not just the device IDs you need, it's the properties too.

> I suppose you have some specific examples in mind that I may not be familiar
> with and we may spend an arbitrary amount of time speaking past each other. :-)

Things like Documentation/devicetree/bindings/sound/wm8962.txt (at least
the optional properties), basically "how is this device wired into the
board" properties.

> That's where we are today.  Do you have any suggestions on what else we can do?

I'd like to see a space where people working with a device can publish
what they've done in terms of firmware binding for it in a manner that
might work for them; at present it seems like the UEFI forum is the best
place to start doing that, there's the start of a register and process
for updating it there at least.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Rafael J. Wysocki]

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

On Monday, December 01, 2014 10:19:07 PM Mark Brown wrote:
> On Mon, Dec 01, 2014 at 11:16:31PM +0100, Rafael J. Wysocki wrote:
> > On Monday, December 01, 2014 05:51:00 PM Mark Brown wrote:
> 
> > > The dream here is that people working on building systems, people
> > > working on Windows drivers and people working on Linux drivers will at
> > > some point be able to collaborate. If we're going to go off and do our
> > > own thing for Linux without talking to anyone that's not really addressing
> > > the issue.
> 
> > Well, that's already going on in the DT land, isn't it?  It has been going on
> > for quite a while AFAICS.
> 
> In theory (and where it's actually relevant in practice to at least some
> extent) this stuff is all OS neutral, there's a definite willingness for
> it to be so.
> 
> > > There's also the option that Windows drivers start using _DSD themselves
> > > which is, I understand, the goal towards which the people working on at
> > > least audio are heading.
> 
> > Technically, Windows driver writers can evaluate _DSD and handle the
> > information the way we do, but I'm not sure if this is really convenient for
> > them.
> 
> I'm not sure how convenient it is, though I'm reasonably sure a helper
> library could make it so.

Well, for that we'd need to find a Windows developer willing to write one I suppose ...

> > We use _DSD, because we want drivers to work with DT as well with ACPI without
> > adding special DT-specific or ACPI-specific code to them.  The people who work
> > on Windows drivers don't have this problem, so as I said, if they care about
> > Linux at all, that may be a good enough motivation for them to look at _DSD,
> > but if they don't, I honestly don't see why they would do that.
> 
> They care about getting properties out, or at least they should, and in
> this market many of the device vendors care about Linux at least as much
> as they do Windows (sometimes even more than they care about Windows,
> there's overlap with the Android market).

OK

> > > Note that all this discussion is pretty much about drivers for single
> > > devices which can be wired into the system in a flexible manner, even in
> > > a Windows world you won't vary the device ID.  At present we're quirking
> > > on DMI.
> 
> > So the answer to that in my view is: Use _DSD and allocate your own device IDs
> > for Windows drivers to bind to.
> 
> Right, so we circle back to the original question about documenting
> those IDs and _DSD properties.  :)

IDs are allocated by whoever owns the device description (the starting 3 or 4
code letters need to be registered via the UEFI Forum/ASWG).

Properties can be registered with the UEFI Forum via the ASWG too.

> > > > > We also need a way of getting the word out to people that they should be
> > > > > doing this (also a problem no matter if we use PRP0001 or something UEFI
> > > > > specific).
> 
> > > > What do you mean by "something UEFI specific"?
> 
> > > Sorry, I mean ACPI specific (UEFI forum).
> 
> > Do you mean a special device ID of some sort, then?
> 
> No, just a regular one.
> 
> > > It's not just the device IDs you need, it's the properties too.
> 
> > I suppose you have some specific examples in mind that I may not be familiar
> > with and we may spend an arbitrary amount of time speaking past each other. :-)
> 
> Things like Documentation/devicetree/bindings/sound/wm8962.txt (at least
> the optional properties), basically "how is this device wired into the
> board" properties.
> 
> > That's where we are today.  Do you have any suggestions on what else we can do?
> 
> I'd like to see a space where people working with a device can publish
> what they've done in terms of firmware binding for it in a manner that
> might work for them; at present it seems like the UEFI forum is the best
> place to start doing that, there's the start of a register and process
> for updating it there at least.

That's correct.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Grant Likely]

On Sat, 29 Nov 2014 23:27:52 +0100
, "Rafael J. Wysocki" <rjw@rjwysocki.net>
 wrote:
> On Saturday, November 29, 2014 11:52:09 AM Mark Brown wrote:
> > 
> > --9GYGtdBumnmR69ER
> > Content-Type: text/plain; charset=us-ascii
> > Content-Disposition: inline
> > Content-Transfer-Encoding: quoted-printable
> > 
> > On Sat, Nov 29, 2014 at 12:51:59AM +0100, Rafael J. Wysocki wrote:
> > > On Friday, November 28, 2014 04:00:36 PM Mark Brown wrote:
> > 
> > > > OK, we probably should have one to aid discoverability since as far as I
> > > > can tell what's happening is that people (hi Intel!) are allocating
> > > > their own identifiers for devices produced by other vendors that turn up
> > > > on their boards.  If people can find the set of IDs in use there's more
> > > > chance they'll use the same ones as other people.
> > 
> > > There's the PRP0001 ID that can be use to in combination with the
> > > "compatible" property which then works the same way as for DT.
> > 
> > > That should be sufficient if the properties are going to be the
> > > same for ACPI (_DSD) and DT.
> > 
> > We've got people making BIOSs right now for systems you can actually buy=20
> > that are intended to run Windows which we need to support; right now the
> > pressing problem I'm seeing is that we've got BIOS vendors not using
> > properties at all and we're going to be ending up with configuration
> > coming from big DMI tables instead which is miserable, Windows is
> > perfectly happy to have custom drivers installed per machine.  Do we
> > know if Windows supports PRP0001 as it currently stands?
> 
> No, it doesn't.
> 
> Separate device IDs are necessary for Windows compatibility AFAICS.
> 
> But that also means any device ID registered by us won't be suitable in that
> case, because Windows won't use it.
> 
> There are two different problems here, though.  The first one is a way to
> provide the existing Linux drivers with the information expected by them via
> ACPI and that's what _DSD (plus PRP0001 optionally) is.  The second one is
> to be able to handle systems with ACPI tables from a random vendor who only
> cares about Windows and that's more difficult to address, because our
> ecosystem is different from theirs.

It is a problem if the expected binding method is different between
Linux and Windows. This is a problem we've hit before where the platform
/may/ try to get both right, but in reality only one works, or even
implemented.

I predict that PRP0001 is only going to be practical when the hardware
is only ever going to be driven by Linux or other PRP0001 aware OS. ie.
A Minnowboard Max with a custom expansion board attached. When the
hardware is supported under both Windows and Linux then it will probably
be best for Linux to attempt to bind using the device id before
attempting to use PRP0001.

> There basically are two ways around that.  The first one is to have all
> knowledge related to device IDs in drivers (which effectively is what
> Windows does and which implies "board files" of sorts) and the second one is
> to make it possible to use overlays on top of the existing ACPI tables that
> will allow people to provide the properties expected by a more generic driver
> (this way, if the vendor didn't care to provide _DSD, for example, in the
> original ACPI tables, the system integrator would be able to use an overlay
> in an initramfs or boot partition to amend them).

Either approach amounts to pretty much the same thing. The
kernel/distribution needs to carry around device specific driver data
which is exactly how we've supported x86 hardware. Whether the data is
in-kernel or in-userspace is kind of an implementation detail. :-)


_DSD support is a different situation though. Even if Windows doesn't
care to directly support _DSD, drivers are free to use _DSD properties.
This is the scenario where the shared binding repository will be
important.

g.

Re: [PATCH 1/2] ASoC: rt5677: Add ACPI device probing

[Mark Brown]

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

On Thu, Dec 04, 2014 at 11:12:34AM +0000, Grant Likely wrote:
> On Sat, 29 Nov 2014 23:27:52 +0100

> > There basically are two ways around that.  The first one is to have all
> > knowledge related to device IDs in drivers (which effectively is what
> > Windows does and which implies "board files" of sorts) and the second one is
> > to make it possible to use overlays on top of the existing ACPI tables that
> > will allow people to provide the properties expected by a more generic driver
> > (this way, if the vendor didn't care to provide _DSD, for example, in the
> > original ACPI tables, the system integrator would be able to use an overlay
> > in an initramfs or boot partition to amend them).

> Either approach amounts to pretty much the same thing. The
> kernel/distribution needs to carry around device specific driver data
> which is exactly how we've supported x86 hardware. Whether the data is
> in-kernel or in-userspace is kind of an implementation detail. :-)

This isn't entirely what we've doing for x86 up until now - there's always
been an expectation that the firmware will be setting things up to work
by default (or so we can read the setup back from the hardwaer when we
take over) and we're mostly just carrying quirk data when that doesn't
pan out.

> _DSD support is a different situation though. Even if Windows doesn't
> care to directly support _DSD, drivers are free to use _DSD properties.
> This is the scenario where the shared binding repository will be
> important.

Right, this would be ideal for the devices we're looking at.

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