Re: [PATCH RFC 3/6] ASoC: max98504: Add max98504 speaker amplifier driver

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

On 06/13/2016 05:14 PM, Mark Brown wrote:
> On Mon, Jun 13, 2016 at 12:52:37PM +0200, Sylwester Nawrocki wrote:
> 
>> [s.nawrocki: removed unused macro definitions and regulator handling
>>  code, rewrote regmap config code, added support for speaker enable
>>  and global chip enable through DAPM, rewritten as component driver,
>>  redesigned DT bindings]
> 
> Removed the regulator handling code?  Why?

Because it was effectively a dead code (disabled by a DT flag property)
and didn't make any sense on our test board. It was setting regulator's
load and voltage on a shared fixed voltage regulator. Even if calls like
regulator_set_optimum_mode() were added that wouldn't make any difference,
since the corresponding PMIC driver ops were not implemented.
I started more or less with something like this [1].

>> +	default:
>> +		/* Analog input, disable PCM, PDM Rx */
>> +		regmap_write(map, MAX98504_REG_PCM_RX_ENABLE, 0);
>> +		regmap_write(map, MAX98504_REG_PDM_RX_ENABLE, 0);
>> +	}
> 
> Add a break here please, at least for consistency.
> 
>> +		ret = PTR_ERR(max98504->regmap);
>> +		dev_err(&client->dev, "regmap initialization failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = max98504_probe(max98504);
>> +	if (ret < 0)
>> +		return ret;
> 
> Why not just inline this code?

Will do in next iteration. Thanks for your review.

[1]
https://github.com/halaszk/Perseus-halaszk-universal5433/blob/perseus5433-mm/sound/soc/codecs/max98504a.c