Re: [PATCH v2 4/6] ASoC: max98504: Add max98504 speaker amplifier driver

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

On 06/27/2016 06:33 PM, Mark Brown wrote:
> On Thu, Jun 23, 2016 at 12:45:13PM +0200, Sylwester Nawrocki wrote:
> 
>> > +	/* Select analog input by default */
>> > +	regmap_write(map, MAX98504_REG_SPEAKER_SOURCE_SELECT, 0x1);
>
> This should be done by userspace configuration, just leave the chip at
> power on defaults.  This is policy so that we don't have people trying
> to patch their configurations into the kernel.

OK, I will remove this and update the UCM configuration files instead.

>> > +	/* Brownout protection enable */
>> > +	regmap_write(map, MAX98504_REG_PVDD_BROWNOUT_ENABLE, 0x1);
>> > +	/* Threshold 2.9V, 3dB speaker attenuation*/
>> > +	regmap_write(map, MAX98504_REG_PVDD_BROWNOUT_CONFIG(1), 0x33);
>> > +	/* Attack hold time 10 ms */
>> > +	regmap_write(map, MAX98504_REG_PVDD_BROWNOUT_CONFIG(2), 0x0a);
>> > +	/* Brownout hold time 255 ms, brownout release time 255 ms */
>> > +	regmap_write(map, MAX98504_REG_PVDD_BROWNOUT_CONFIG(3), 0xff);
>> > +	regmap_write(map, MAX98504_REG_PVDD_BROWNOUT_CONFIG(4), 0xff);
>
> Should these be DT properties?

I'm not sure, these properties define the speaker gain decrease pattern
during VBAT brownout, it seems a system integration detail and will most
likely differ from board to board.  I'd assume it's acceptable to put
these in DT, looking at other bindings.  I've prepared something like
this:

------8<--------
Optional properties:

- maxim,brownout-threshold - the VBAT brownout threshold, the value must be
  from 0, 1...21 range, corresponding to 2.6V, 2.65V...3.65V voltage range
- maxim,brownout-attenuation - the brownout attenuation to the speaker gain
  applied during the "attack hold" and "timed hold" phase, the value must be
  from 0...6 (dB) range
- maxim,brownout-attack-hold-ms - the brownout attack hold phase time in ms,
  0...255 (VBATBROWN_ATTK_HOLD, register 0x0018)
- maxim,brownout-timed-hold-ms - the brownout timed hold phase time in ms,
  0...255 (VBATBROWN_TIME_HOLD, register 0x0019)
- maxim,brownout-release-rate-ms - the brownout release phase step time in ms,
  0...255 (VBATBROWN_RELEASE, register 0x001A)

The default value when the above properties are not specified is 0,
the maxim,brownout-level property must be specified to actually enable
the VBAT brownout protection.
------8<--------