Re: [PATCH v8 1/2] ASoC: rt5514: Add devicetree bindings for rt5514-spi

[jeffy]

Hi Brian,

On 09/06/2017 01:55 AM, Brian Norris wrote:
> > +This device supports SPI only.
> > >+
> > >+Required properties:
> > >+
> > >+- compatible : "realtek,rt5514".
> You list one string here, but there's 2 in the example. The point of
> this exercise was to document (not just in an Example) the strings
> you're using.
Ok, will do.
> > >+
> > >+- reg : The SPI address of the device.
> > >+
> > >+Optional properties:
> > >+
> > >+- interrupt-parent: Should be the phandle for the interrupt controller
> > >+- interrupts: The interrupt number to the cpu. The interrupt specifier format
> > >+	      depends on the interrupt controller.
> > >+
> > >+Example:
> > >+
> > >+codec: rt5514-spi@0 {
> > >+	compatible = "realtek,rt5514", "realtek,rt5514-spi";
> > >+	reg = <0>;
> > >+
> > >+        /* May run faster once verified. */
> Really? That was barely useful in the rk3399-gru DTSI, but it's
> definitely not worth sticking in an example.
>
> Also, you switch between tabs and spaces for indentation. Please be
> consistent. (And as with C code, I think the right answer is tabs?)
Oops, thanks for noticing.
> > >+        spi-max-frequency = <10000000>;
> > >+
> > >+        pinctrl-names = "default";
> > >+        pinctrl-0 = <&mic_int>;
> > >+
> > >+        interrupt-parent = <&gpio1>;
> > >+        interrupts = <13 IRQ_TYPE_LEVEL_HIGH>;
> > >+        wakeup-source;
> > >+};
> > >diff --git a/Documentation/devicetree/bindings/sound/rt5514.txt b/Documentation/devicetree/bindings/sound/rt5514.txt
> > >index 929ca6756b02..12a8c93a67cc 100644
> > >--- a/Documentation/devicetree/bindings/sound/rt5514.txt
> > >+++ b/Documentation/devicetree/bindings/sound/rt5514.txt
> > >@@ -28,6 +28,6 @@ Pins on the device (for linking into audio routes) for RT5514:
> > >  Example:
> > >
> > >  codec: rt5514@57 {
> > >-	compatible = "realtek,rt5514";
> > >+	compatible = "realtek,rt5514", "realtek,rt5514-i2c";
> Example != Documentation. Please document the string properly.
Ok, will do.
> Brian