RE: [PATCH 2/3] ASoC: da7219: Add bindings documentation for DA7219 audio codec
From: Opensource [Adam Thomson]
Date: Mon Sep 21 2015 - 06:36:17 EST
On September 19, 2015 18:10, Mark Brown wrote:
> > +- dlg,io-lvl : Expected voltage level range for digital IO
> > + ["2.5V_3.6V", "1.2V_2.8V"]
>
> If the driver needs to read or set the voltage a supply is at it should
> do that via the regulator API.
This would just be a read for the driver. However it's a fair point, so I'll
look to add passing of the regulator information for VDDIO, so i can set this
based on read voltage.
>
> > +- dlg,cp-mchange : Charge pump voltage tracking mode
> > + ["largest_vol", "dac_vol", "sig_mag"]
> > +- dlg,cp-vol-thresh : Charge pump volume threshold value (6-bit value)
> > + [ 0 - 0x3F ]
>
> Why are these in the device tree rather than runtime parameters?
>
From previous internal discussions, these seemed to be fire and forget
parameters, hence their inclusion in the DT binding, rather than as controls.
Personally didn't see either needing runtime updates.
> > +Child node - 'da7219_aad':
> > +
> > +Required properties:
> > +- interrupt-parent : Specifies the phandle of the interrupt controller to which
> > + the IRQs from DA7219 AAD block are delivered to.
> > +- interrupts : IRQ line info for DA7219 AAD block.
> > + (See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
> > + further information relating to interrupt properties)
>
> Why is this not specified at the device level (the device does not
> appear to support other interrupts)?
Given the way that the driver code was structured, and that the IRQ is only used
for accessory detection, I added it to the child node. The other option would
be to flatten out bindings, and remove the child node. Felt like keeping the
accessory detect items separate though was a sensible approach. What is your
feeling on this?
N§²æ¸yú²X¬¶ÇvØ)Þ{.nÇ·¥{±êX§¶¡Ü}©²ÆzÚj:+v¨¾«êZ+Êzf£¢·h§~Ûÿû®w¥¢¸?¨è&¢)ßfùy§m
á«a¶Úÿ0¶ìå