lkml.org 
[lkml]   [2016]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] ASoC: rt5677: Add ACPI support
On Sun, Aug 14, 2016 at 12:18:22PM +0100, John Keeping wrote:

> The Chromebook Pixel 2015 uses this codec with the ACPI ID RT5677CE, so
> add an ACPI match table and support for reading properties from ACPI.

This would be a lot easier to review with a concrete description of what
"support for reading properties from ACPI" means and probably also split
out a bit so that different things were being added separately.

> +/* GPIO indexes defined by ACPI */
> +enum {
> + RT5677_GPIO_PLUG_DET,
> + RT5677_GPIO_MIC_PRESENT_L,
> + RT5677_GPIO_HOTWORD_DET_L,
> + RT5677_GPIO_DSP_INT,
> + RT5677_GPIO_HP_AMP_SHDN_L,
> +};

If these are an ABI you should explicitly assign the values so that they
can't get remapped by future edits. If they're not an ABI I don't
understand the comment.

> + if (ACPI_HANDLE(dev)) {
> + u32 val;
> +
> + if (!device_property_read_u32(dev, "DCLK", &val))
> + rt5677->pdata.dmic2_clk_pin = val;
> +
> + rt5677->pdata.in1_diff = device_property_read_bool(dev, "IN1");
> + rt5677->pdata.in2_diff = device_property_read_bool(dev, "IN2");

What happens if someone makes a machine which uses the DT<->ACPI
mappings (especially given that this is currently undocumented)? That
would not work which defeats the whole purpose of using the device
property APIs. Shouldn't we be accepting either property?
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2016-09-17 09:57    [W:0.121 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site