Re: [PATCH v5-resend 4/7] ASoC: sirf: Add SiRF I2S driver

[RongJun Ying <rjying@gmail.com>]

2014-03-07 14:05 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Wed, Mar 05, 2014 at 04:34:37PM +0800, RongJun Ying wrote:
>
> Mostly OK, a couple of small things below:
>
>> +static int sirf_i2s_set_clkdiv(struct snd_soc_dai *dai, int div_id,
>> +     int src_rate)
>> +{
>> +     struct sirf_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> +
>> +     switch (div_id) {
>> +     case SIRF_I2S_EXT_CLK:
>> +             i2s->ext_clk = 1;
>> +             break;
>> +     case SIRF_I2S_PWM_CLK:
>> +             i2s->ext_clk = 0;
>> +             break;
>> +     default:
>> +             return -EINVAL;
>> +     }
>> +
>> +     i2s->src_clk_rate = src_rate;
>> +     return 0;
>> +}
>
> This should be set_sysclk().

OK

>
>> +     base = devm_ioremap(&pdev->dev, mem_res->start,
>> +             resource_size(mem_res));
>> +     if (base == NULL)
>> +             return -ENOMEM;
>
> As mentioned on the other patch this should be devm_ioremap_resource() -
> if you are sharing the register range use a core device to own the
> register map.

I may use the mfd driver to manage all shring the regsiter range.

Thanks

-- 
------------------------------
Rongjun Ying