lkml.org 
[lkml]   [2021]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v10 3/5] iio: adc: Add Xilinx AMS driver
From


On 11/17/21 21:03, Andy Shevchenko wrote:
> On Wed, Nov 17, 2021 at 04:10:26PM +0000, Anand Ashok Dumbre wrote:
>> The AMS includes an ADC as well as on-chip sensors that can be used to
>> sample external voltages and monitor on-die operating conditions, such
>> as temperature and supply voltage levels. The AMS has two SYSMON blocks.
>> PL-SYSMON block is capable of monitoring off chip voltage and
>> temperature.
>>
>> PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring
>> from an external master. Out of these interfaces currently only DRP is
>> supported. Other block PS-SYSMON is memory mapped to PS.
>>
>> The AMS can use internal channels to monitor voltage and temperature as
>> well as one primary and up to 16 auxiliary channels for measuring
>> external voltages.
>>
>> The voltage and temperature monitoring channels also have event capability
>> which allows to generate an interrupt when their value falls below or
>> raises above a set threshold.
>
> Thanks for an update, my comments below.
>
> ...
>
> Missed bitfields.h as kbuild bot noticed.
>
>> +#include <linux/bits.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/overflow.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/property.h>
>> +#include <linux/slab.h>
>
> ...
>
>> +#define AMS_ALARM_THR_DIRECT_MASK BIT(1)
>
>> +#define AMS_ALARM_THR_MIN ~(BIT(16) - 1)
>
> This is wrong. I already said it.
>
>> +#define AMS_ALARM_THR_MAX (BIT(16) - 1)
>
> ...
>
>> +enum ams_alarm_bit {
>> + AMS_ALARM_BIT_TEMP,
>> + AMS_ALARM_BIT_SUPPLY1,
>> + AMS_ALARM_BIT_SUPPLY2,
>> + AMS_ALARM_BIT_SUPPLY3,
>> + AMS_ALARM_BIT_SUPPLY4,
>> + AMS_ALARM_BIT_SUPPLY5,
>> + AMS_ALARM_BIT_SUPPLY6,
>> + AMS_ALARM_BIT_RESERVED,
>> + AMS_ALARM_BIT_SUPPLY7,
>> + AMS_ALARM_BIT_SUPPLY8,
>> + AMS_ALARM_BIT_SUPPLY9,
>> + AMS_ALARM_BIT_SUPPLY10,
>> + AMS_ALARM_BIT_VCCAMS,
>> + AMS_ALARM_BIT_TEMP_REMOTE
>
> +Comma, same to the rest where the last item is not a terminator one.

And where you are touching this it will be good if you can initiate all
values. That's recommendation we got from Greg some time ago.

Thanks,
Michal

\
 
 \ /
  Last update: 2021-11-18 09:08    [W:0.107 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site