On 05/22/2013 07:57 PM, Mark Brown wrote:
> On Wed, May 22, 2013 at 07:00:13PM +0200, Lars-Peter Clausen wrote:
>> This patch adds a ASoC CODEC driver for the SSM2516. The SSM2516 is a stereo
>> Class-D audio amplifier with an I2S interface for audio in and a built-in
>> dynamic range control processor.
>
> I'll apply this but
I'll send a v2 with your comments fixed.
>
>> + if (slots == 0) {
>> + return regmap_update_bits(ssm2518->regmap,
>> + SSM2518_REG_SAI_CTRL1, SSM2518_SAI_CTRL1_SAI_MASK,
>> + SSM2518_SAI_CTRL1_SAI_I2S);
>> + }
>
> You've got quite a few single statement if () blocks with { } which
> shouldn't be there.
>
I prefer to only do this for single single-line statements.
[...]
>
>> + switch (freq) {
>> + case 2048000:
>
> Looks like the user can't select 0 for the SYSCLK, I'd expect that to be
> possible for systems that can reprogram the clock so that they can avoid
> having constraints set when they don't need them.
>
Makes sense.
>> + } else if (i2c->dev.of_node) {
>> + ssm2518->enable_gpio = of_get_gpio(i2c->dev.of_node, 0);
>> + if (ssm2518->enable_gpio == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>
> Why are other errors being ignored here?
To make the property optional. But I think it might be better to only allow
-ENOENT and treat everything else as an error in this case.
Thanks,
- Lars
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
