On Mon, Sep 21, 2015 at 10:36:04AM +0000, Opensource [Adam Thomson] wrote:
On September 19, 2015 18:10, Mark Brown wrote:
+- 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.
Make them runtime configurable. People can do an at boot runtime configuration if they like.
+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?
The child node is fine for collecting the parameters but the chip interrupt line should be at the chip level.