Re: [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pierre,

Thanks for looking into this!

On 12/17/19 8:16 PM, Pierre-Louis Bossart wrote:
> is the datasheet public? I thought it was only available under NDA.

It was until recently, but it is now public:


https://www.analog.com/media/en/technical-documentation/user-guides/AD242x_TRM_Rev1.1.pdf

>> +    master->sync_clk = devm_clk_get(dev, "sync");
>> +    if (IS_ERR(master->sync_clk)) {
>> +        ret = PTR_ERR(master->sync_clk);
>> +        if (ret != -EPROBE_DEFER)
>> +            dev_err(dev, "failed to get sync clk: %d\n", ret);
>> +
>> +        return ret;
>> +    }
>> +
>> +    if (of_property_read_u32(dev->of_node, "clock-frequency",
>> +                 &master->sync_clk_rate)) {
>> +        ret = clk_set_rate(master->sync_clk, master->sync_clk_rate);
> 
> shouldn't you check the rate before setting it?
>
>> +        if (ret < 0) {
>> +            dev_err(dev, "Cannot set sync clock rate: %d\n", ret);
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    master->sync_clk_rate = clk_get_rate(master->sync_clk);
>> +    if (master->sync_clk_rate != 44100 && master->sync_clk_rate !=
>> 48000) {
>> +        dev_err(dev, "SYNC clock rate %d is invalid\n",
>> +            master->sync_clk_rate);
>> +        return -EINVAL;
>> +    }
> 
> this is a bit odd, you set the rate in case there is a property but get
> it anyways. the last block could be an else?

The idea is: if 'clock-frequency' is given, we use it to set the clock,
otherwise we rely on the clock having one of the two allowed rates. This
way, we also catch setups where the clock provider cannot generated the
desired frequency, or where the value of 'clock-frequency' is illegal.

>> +    ret = clk_prepare_enable(master->sync_clk);
>> +    if (ret < 0) {
>> +        dev_err(dev, "failed to enable sync clk: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    /* Master node setup */
>> +
>> +    ret = regmap_write(regmap, AD242X_CONTROL,
>> +               AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    ret = ad242x_wait_for_irq(master, &master->run_completion, 10);
> 
> what is 10?

Milliseconds. The parameter needs to get a better name I figure.


Thanks,
Daniel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux