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?
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel