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��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f