* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver
2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
2021-06-25 10:49 ` kernel test robot
@ 2021-06-29 8:32 ` Dan Carpenter
2021-07-04 18:31 ` Jonathan Cameron
2 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-06-28 21:45 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 6563 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210624182939.12881-3-anand.ashok.dumbre@xilinx.com>
References: <20210624182939.12881-3-anand.ashok.dumbre@xilinx.com>
TO: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
TO: jic23(a)kernel.org
TO: lars(a)metafoo.de
TO: linux-iio(a)vger.kernel.org
TO: git-dev(a)xilinx.com
TO: michal.simek(a)xilinx.com
TO: pmeerw(a)pmeerw.net
TO: devicetree(a)vger.kernel.org
TO: linux-kernel(a)vger.kernel.org
CC: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
CC: Manish Narani <manish.narani@xilinx.com>
Hi Anand,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on iio/togreg]
[also build test WARNING on linus/master v5.13 next-20210628]
[cannot apply to xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
:::::: branch date: 4 days ago
:::::: commit date: 4 days ago
config: microblaze-randconfig-m031-20210628 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/iio/adc/xilinx-ams.c:406 ams_enable_channel_sequence() warn: should '(((1))) << (indio_dev->channels[i]->scan_index)' be a 64 bit type?
drivers/iio/adc/xilinx-ams.c:1126 ams_init_module() error: uninitialized symbol 'chan_node'.
vim +406 drivers/iio/adc/xilinx-ams.c
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 391
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 392 static void ams_enable_channel_sequence(struct iio_dev *indio_dev)
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 393 {
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 394 int i;
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 395 unsigned long long scan_mask;
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 396 struct ams *ams = iio_priv(indio_dev);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 397
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 398 /*
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 399 * Enable channel sequence. First 22 bits of scan_mask represent
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 400 * PS channels, and next remaining bits represent PL channels.
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 401 */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 402
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 403 /* Run calibration of PS & PL as part of the sequence */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 404 scan_mask = 0x1 | BIT(AMS_PS_SEQ_MAX);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 405 for (i = 0; i < indio_dev->num_channels; i++)
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 @406 scan_mask |= BIT(indio_dev->channels[i].scan_index);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 407
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 408 if (ams->ps_base) {
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 409 /* put sysmon in a soft reset to change the sequence */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 410 ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 411 AMS_CONF1_SEQ_DEFAULT);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 412
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 413 /* configure basic channels */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 414 writel(scan_mask & AMS_REG_SEQ0_MASK,
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 415 ams->ps_base + AMS_REG_SEQ_CH0);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 416 writel(AMS_REG_SEQ2_MASK &
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 417 (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT),
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 418 ams->ps_base + AMS_REG_SEQ_CH2);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 419
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 420 /* set continuous sequence mode */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 421 ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 422 AMS_CONF1_SEQ_CONTINUOUS);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 423 }
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 424
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 425 if (ams->pl_base) {
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 426 /* put sysmon in a soft reset to change the sequence */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 427 ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 428 AMS_CONF1_SEQ_DEFAULT);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 429
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 430 /* configure basic channels */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 431 scan_mask = scan_mask >> AMS_PS_SEQ_MAX;
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 432 writel(scan_mask & AMS_REG_SEQ0_MASK,
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 433 ams->pl_base + AMS_REG_SEQ_CH0);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 434 writel(AMS_REG_SEQ2_MASK &
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 435 (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT),
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 436 ams->pl_base + AMS_REG_SEQ_CH2);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 437 writel(AMS_REG_SEQ1_MASK &
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 438 (scan_mask >> AMS_REG_SEQ1_MASK_SHIFT),
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 439 ams->pl_base + AMS_REG_SEQ_CH1);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 440
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 441 /* set continuous sequence mode */
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 442 ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 443 AMS_CONF1_SEQ_CONTINUOUS);
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 444 }
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 445 }
fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 446
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31380 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver @ 2021-06-29 8:32 ` Dan Carpenter 0 siblings, 0 replies; 19+ messages in thread From: Dan Carpenter @ 2021-06-29 8:32 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 5719 bytes --] Hi Anand, url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: microblaze-randconfig-m031-20210628 (attached as .config) compiler: microblaze-linux-gcc (GCC) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/iio/adc/xilinx-ams.c:406 ams_enable_channel_sequence() warn: should '(((1))) << (indio_dev->channels[i]->scan_index)' be a 64 bit type? drivers/iio/adc/xilinx-ams.c:1126 ams_init_module() error: uninitialized symbol 'chan_node'. vim +406 drivers/iio/adc/xilinx-ams.c fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 392 static void ams_enable_channel_sequence(struct iio_dev *indio_dev) fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 393 { fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 394 int i; fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 395 unsigned long long scan_mask; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 396 struct ams *ams = iio_priv(indio_dev); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 397 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 398 /* fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 399 * Enable channel sequence. First 22 bits of scan_mask represent fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 400 * PS channels, and next remaining bits represent PL channels. fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 401 */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 402 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 403 /* Run calibration of PS & PL as part of the sequence */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 404 scan_mask = 0x1 | BIT(AMS_PS_SEQ_MAX); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 405 for (i = 0; i < indio_dev->num_channels; i++) fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 @406 scan_mask |= BIT(indio_dev->channels[i].scan_index); Since scan_mask is ull should we use BIT_ULL() instead of BIT()? (I haven't look at the context outside of this email so I don't know the value of indio_dev->num_channels). fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 407 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 408 if (ams->ps_base) { fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 409 /* put sysmon in a soft reset to change the sequence */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 410 ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 411 AMS_CONF1_SEQ_DEFAULT); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 412 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 413 /* configure basic channels */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 414 writel(scan_mask & AMS_REG_SEQ0_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 415 ams->ps_base + AMS_REG_SEQ_CH0); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 416 writel(AMS_REG_SEQ2_MASK & fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 417 (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT), fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 418 ams->ps_base + AMS_REG_SEQ_CH2); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 419 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 420 /* set continuous sequence mode */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 421 ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 422 AMS_CONF1_SEQ_CONTINUOUS); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 423 } fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 424 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 425 if (ams->pl_base) { fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 426 /* put sysmon in a soft reset to change the sequence */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 427 ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 428 AMS_CONF1_SEQ_DEFAULT); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 429 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 430 /* configure basic channels */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 431 scan_mask = scan_mask >> AMS_PS_SEQ_MAX; fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 432 writel(scan_mask & AMS_REG_SEQ0_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 433 ams->pl_base + AMS_REG_SEQ_CH0); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 434 writel(AMS_REG_SEQ2_MASK & fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 435 (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT), fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 436 ams->pl_base + AMS_REG_SEQ_CH2); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 437 writel(AMS_REG_SEQ1_MASK & fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 438 (scan_mask >> AMS_REG_SEQ1_MASK_SHIFT), fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 439 ams->pl_base + AMS_REG_SEQ_CH1); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 440 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 441 /* set continuous sequence mode */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 442 ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 443 AMS_CONF1_SEQ_CONTINUOUS); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 444 } fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 445 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver @ 2021-06-29 8:32 ` Dan Carpenter 0 siblings, 0 replies; 19+ messages in thread From: Dan Carpenter @ 2021-06-29 8:32 UTC (permalink / raw) To: kbuild, Anand Ashok Dumbre, jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: lkp, kbuild-all, Anand Ashok Dumbre, Manish Narani Hi Anand, url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: microblaze-randconfig-m031-20210628 (attached as .config) compiler: microblaze-linux-gcc (GCC) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/iio/adc/xilinx-ams.c:406 ams_enable_channel_sequence() warn: should '(((1))) << (indio_dev->channels[i]->scan_index)' be a 64 bit type? drivers/iio/adc/xilinx-ams.c:1126 ams_init_module() error: uninitialized symbol 'chan_node'. vim +406 drivers/iio/adc/xilinx-ams.c fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 392 static void ams_enable_channel_sequence(struct iio_dev *indio_dev) fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 393 { fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 394 int i; fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 395 unsigned long long scan_mask; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 396 struct ams *ams = iio_priv(indio_dev); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 397 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 398 /* fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 399 * Enable channel sequence. First 22 bits of scan_mask represent fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 400 * PS channels, and next remaining bits represent PL channels. fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 401 */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 402 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 403 /* Run calibration of PS & PL as part of the sequence */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 404 scan_mask = 0x1 | BIT(AMS_PS_SEQ_MAX); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 405 for (i = 0; i < indio_dev->num_channels; i++) fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 @406 scan_mask |= BIT(indio_dev->channels[i].scan_index); Since scan_mask is ull should we use BIT_ULL() instead of BIT()? (I haven't look at the context outside of this email so I don't know the value of indio_dev->num_channels). fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 407 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 408 if (ams->ps_base) { fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 409 /* put sysmon in a soft reset to change the sequence */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 410 ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 411 AMS_CONF1_SEQ_DEFAULT); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 412 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 413 /* configure basic channels */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 414 writel(scan_mask & AMS_REG_SEQ0_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 415 ams->ps_base + AMS_REG_SEQ_CH0); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 416 writel(AMS_REG_SEQ2_MASK & fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 417 (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT), fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 418 ams->ps_base + AMS_REG_SEQ_CH2); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 419 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 420 /* set continuous sequence mode */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 421 ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 422 AMS_CONF1_SEQ_CONTINUOUS); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 423 } fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 424 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 425 if (ams->pl_base) { fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 426 /* put sysmon in a soft reset to change the sequence */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 427 ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 428 AMS_CONF1_SEQ_DEFAULT); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 429 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 430 /* configure basic channels */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 431 scan_mask = scan_mask >> AMS_PS_SEQ_MAX; fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 432 writel(scan_mask & AMS_REG_SEQ0_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 433 ams->pl_base + AMS_REG_SEQ_CH0); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 434 writel(AMS_REG_SEQ2_MASK & fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 435 (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT), fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 436 ams->pl_base + AMS_REG_SEQ_CH2); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 437 writel(AMS_REG_SEQ1_MASK & fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 438 (scan_mask >> AMS_REG_SEQ1_MASK_SHIFT), fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 439 ams->pl_base + AMS_REG_SEQ_CH1); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 440 fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 441 /* set continuous sequence mode */ fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 442 ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 443 AMS_CONF1_SEQ_CONTINUOUS); fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 444 } fa0ea7aaf7a9bf Anand Ashok Dumbre 2021-06-24 445 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 0/4] Add Xilinx AMS Driver
@ 2021-06-24 18:29 Anand Ashok Dumbre
2021-06-24 18:29 ` [PATCH v6 1/4] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-06-24 18:29 UTC (permalink / raw)
To: jic23, lars, linux-iio, git-dev, michal.simek, pmeerw,
devicetree, linux-kernel
Cc: Anand Ashok Dumbre
Add Xilinx AMS driver which is used for Xilinx's ZynqMP AMS controller.
This AMS driver is used to report various interface voltages and temperatures
across the system.
This driver will be used by iio-hwmon to repport voltages and temperatures
across the system by using various channel interfaces.
This driver handles AMS module including PS-Sysmon & PL-Sysmon. The binding
documentation is added for understanding of AMS, PS, PL Sysmon Channels.
Changes in v2:
- Added documentation for sysfs (Patch-2)
- Addressed code style review comments
- Patch-2 (Now it is Patch-3)
- Arranged the includes in alphabetical order
- Removed the wrapper 'ams_pl_write_reg()' and used writel
instead
- Removed the unnecessary delay of 1ms and used polling of EOC
instead
- Removed spin_lock and used mutex only.
- Used request_irq() instead of devm_request_irq() and handled
respective error conditions
- Moved contents of xilinx-ams.h to inline with xilinx-ams.c
- Patch-1
- Addressed Documentation style comments
Changes in v3:
- Updated bindings document with the suggested modification in v2 review
- Removed documentation for sysfs
- Removed extended names for channels in the Xilinx AMS driver
- Modified dts to use ranges for child nodes
- Reduced address and size cells to 32-bit instead of 64-bit
Changes in v4:
- Updated bindings document with the suggested modification in v3 review
- Changed the Device Tree property 'ranges' for child nodes
- Used Channel Numbers as 'reg' value in DT to avoid confusion
- Removed unused NULL arguments as suggested in v3 patch review
- Addressed comments on Device Tree property naming
Changes in v5:
- Updated bindings document to the YAML format
- Updated bindings document with the suggested modification in v4 review
- Renamed iio_pl_info struct to iio_ams_info in Xilinx AMS driver
- Updated the Xilinx AMS driver to not use iio_priv_to_dev function
- Updated Xilinx AMS node to reflect the changes in bindings document
- Update MAINTAINERS file
Changes in v6:
- Removed all tabs from bindings document.
- Removed the xlnx,ext-channels node from the device tree since
it is not neeeded.
- Fixed unit addresses for ps-ams and pl-ams.
- Removed the names property from bindings.
- Fixed warnings from checkpatch.pl in the driver.
- devm_add_action_or_reset() used for exit/error path.
- devm_request_irq() for managed irq request instead of
request_irq()
Anand Ashok Dumbre (4):
arm64: zynqmp: DT: Add Xilinx AMS node
iio: adc: Add Xilinx AMS driver
dt-bindings: iio: adc: Add Xilinx AMS binding documentation
MAINTAINERS: Add maintainer for xilinx-ams
.../bindings/iio/adc/xlnx,zynqmp-ams.yaml | 228 +++
MAINTAINERS | 7 +
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 26 +-
drivers/iio/adc/Kconfig | 13 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/xilinx-ams.c | 1342 +++++++++++++++++
6 files changed, 1616 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
create mode 100644 drivers/iio/adc/xilinx-ams.c
--
2.17.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v6 1/4] arm64: zynqmp: DT: Add Xilinx AMS node 2021-06-24 18:29 [PATCH v6 0/4] Add Xilinx AMS Driver Anand Ashok Dumbre @ 2021-06-24 18:29 ` Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre ` (2 subsequent siblings) 3 siblings, 0 replies; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-06-24 18:29 UTC (permalink / raw) To: jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: Anand Ashok Dumbre, Manish Narani The Xilinx AMS includes an ADC as well as on-chip sensors that can be used to sample external and monitor on-die operating conditions, such as temperature and supply voltage levels. Signed-off-by: Manish Narani <manish.narani@xilinx.com> Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> --- arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi index 28dccb891a53..cb14fd106fc6 100644 --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0+ +// SPDX-License-Identifier: GPL-2.0 /* * dts file for Xilinx ZynqMP * @@ -849,6 +849,30 @@ timeout-sec = <10>; }; + xilinx_ams: ams@ffa50000 { + compatible = "xlnx,zynqmp-ams"; + status = "disabled"; + interrupt-parent = <&gic>; + interrupts = <0 56 4>; + reg = <0x0 0xffa50000 0x0 0x800>; + #address-cells = <1>; + #size-cells = <1>; + #io-channel-cells = <1>; + ranges = <0 0 0xffa50800 0x800>; + + ams_ps: ams-ps@0,400 { + compatible = "xlnx,zynqmp-ams-ps"; + status = "disabled"; + reg = <0 0x400>; + }; + + ams_pl: ams-pl@400,400 { + compatible = "xlnx,zynqmp-ams-pl"; + status = "disabled"; + reg = <0x400 0x400>; + }; + }; + zynqmp_dpdma: dma-controller@fd4c0000 { compatible = "xlnx,zynqmp-dpdma"; status = "disabled"; -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-06-24 18:29 [PATCH v6 0/4] Add Xilinx AMS Driver Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 1/4] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre @ 2021-06-24 18:29 ` Anand Ashok Dumbre 2021-06-25 10:49 ` kernel test robot ` (2 more replies) 2021-06-24 18:29 ` [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 4/4] MAINTAINERS: Add maintainer for xilinx-ams Anand Ashok Dumbre 3 siblings, 3 replies; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-06-24 18:29 UTC (permalink / raw) To: jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: Anand Ashok Dumbre, Manish Narani 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 external master. Out of these interface 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. Signed-off-by: Manish Narani <manish.narani@xilinx.com> Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> --- drivers/iio/adc/Kconfig | 13 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/xilinx-ams.c | 1342 ++++++++++++++++++++++++++++++++++ 3 files changed, 1356 insertions(+) create mode 100644 drivers/iio/adc/xilinx-ams.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index c7946c439612..c011ab30ec9a 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -1256,4 +1256,17 @@ config XILINX_XADC The driver can also be build as a module. If so, the module will be called xilinx-xadc. +config XILINX_AMS + tristate "Xilinx AMS driver" + depends on ARCH_ZYNQMP || COMPILE_TEST + depends on HAS_IOMEM + help + Say yes here to have support for the Xilinx AMS. + + The driver supports Voltage and Temperature monitoring on Xilinx Ultrascale + devices. + + The driver can also be built as a module. If so, the module will be called + xilinx-ams. + endmenu diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index a226657d19c0..99e031f44479 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -112,4 +112,5 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o +obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c new file mode 100644 index 000000000000..463e3162726c --- /dev/null +++ b/drivers/iio/adc/xilinx-ams.c @@ -0,0 +1,1342 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx AMS driver + * + * Copyright (C) 2021 Xilinx, Inc. + * + * Manish Narani <mnarani@xilinx.com> + * Rajnikant Bhojani <rajnikant.bhojani@xilinx.com> + */ + +#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/of_address.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#include <linux/iio/events.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> + +static const unsigned int AMS_UNMASK_TIMEOUT_MS = 500; + +/* AMS registers definitions */ +#define AMS_ISR_0 0x010 +#define AMS_ISR_1 0x014 +#define AMS_IER_0 0x020 +#define AMS_IER_1 0x024 +#define AMS_IDR_0 0x028 +#define AMS_IDR_1 0x02c +#define AMS_PS_CSTS 0x040 +#define AMS_PL_CSTS 0x044 + +#define AMS_VCC_PSPLL0 0x060 +#define AMS_VCC_PSPLL3 0x06C +#define AMS_VCCINT 0x078 +#define AMS_VCCBRAM 0x07C +#define AMS_VCCAUX 0x080 +#define AMS_PSDDRPLL 0x084 +#define AMS_PSINTFPDDR 0x09C + +#define AMS_VCC_PSPLL0_CH 48 +#define AMS_VCC_PSPLL3_CH 51 +#define AMS_VCCINT_CH 54 +#define AMS_VCCBRAM_CH 55 +#define AMS_VCCAUX_CH 56 +#define AMS_PSDDRPLL_CH 57 +#define AMS_PSINTFPDDR_CH 63 + +#define AMS_REG_CONFIG0 0x100 +#define AMS_REG_CONFIG1 0x104 +#define AMS_REG_CONFIG3 0x10C +#define AMS_REG_SEQ_CH0 0x120 +#define AMS_REG_SEQ_CH1 0x124 +#define AMS_REG_SEQ_CH2 0x118 + +#define AMS_TEMP 0x000 +#define AMS_SUPPLY1 0x004 +#define AMS_SUPPLY2 0x008 +#define AMS_VP_VN 0x00c +#define AMS_VREFP 0x010 +#define AMS_VREFN 0x014 +#define AMS_SUPPLY3 0x018 +#define AMS_SUPPLY4 0x034 +#define AMS_SUPPLY5 0x038 +#define AMS_SUPPLY6 0x03c +#define AMS_SUPPLY7 0x200 +#define AMS_SUPPLY8 0x204 +#define AMS_SUPPLY9 0x208 +#define AMS_SUPPLY10 0x20c +#define AMS_VCCAMS 0x210 +#define AMS_TEMP_REMOTE 0x214 + +#define AMS_REG_VAUX(x) (0x40 + 4 * (x)) + +#define AMS_PS_RESET_VALUE 0xFFFF +#define AMS_PL_RESET_VALUE 0xFFFF + +#define AMS_CONF0_CHANNEL_NUM_MASK GENMASK(6, 0) + +#define AMS_CONF1_SEQ_MASK GENMASK(15, 12) +#define AMS_CONF1_SEQ_DEFAULT (0 << 12) +#define AMS_CONF1_SEQ_CONTINUOUS (2 << 12) +#define AMS_CONF1_SEQ_SINGLE_CHANNEL (3 << 12) + +#define AMS_REG_SEQ0_MASK 0xFFFF +#define AMS_REG_SEQ2_MASK 0x003F +#define AMS_REG_SEQ1_MASK 0xFFFF +#define AMS_REG_SEQ2_MASK_SHIFT 16 +#define AMS_REG_SEQ1_MASK_SHIFT 22 + +#define AMS_REGCFG1_ALARM_MASK 0xF0F +#define AMS_REGCFG3_ALARM_MASK 0x3F + +#define AMS_ALARM_TEMP 0x140 +#define AMS_ALARM_SUPPLY1 0x144 +#define AMS_ALARM_SUPPLY2 0x148 +#define AMS_ALARM_SUPPLY3 0x160 +#define AMS_ALARM_SUPPLY4 0x164 +#define AMS_ALARM_SUPPLY5 0x168 +#define AMS_ALARM_SUPPLY6 0x16c +#define AMS_ALARM_SUPPLY7 0x180 +#define AMS_ALARM_SUPPLY8 0x184 +#define AMS_ALARM_SUPPLY9 0x188 +#define AMS_ALARM_SUPPLY10 0x18c +#define AMS_ALARM_VCCAMS 0x190 +#define AMS_ALARM_TEMP_REMOTE 0x194 +#define AMS_ALARM_THRESHOLD_OFF_10 0x10 +#define AMS_ALARM_THRESHOLD_OFF_20 0x20 + +#define AMS_ALARM_THR_DIRECT_MASK 0x01 +#define AMS_ALARM_THR_MIN 0x0000 +#define AMS_ALARM_THR_MAX 0xffff + +#define AMS_NO_OF_ALARMS 32 +#define AMS_PL_ALARM_START 16 +#define AMS_ISR0_ALARM_MASK 0xFFFFFFFFU +#define AMS_ISR1_ALARM_MASK 0xE000001FU +#define AMS_ISR1_EOC_MASK 0x00000008U +#define AMS_ISR1_INTR_MASK_SHIFT 32 +#define AMS_ISR0_ALARM_2_TO_0_MASK 0x07 +#define AMS_ISR0_ALARM_6_TO_3_MASK 0x78 +#define AMS_ISR0_ALARM_12_TO_7_MASK 0x3F +#define AMS_CONF1_ALARM_2_TO_0_SHIFT 1 +#define AMS_CONF1_ALARM_6_TO_3_SHIFT 5 +#define AMS_CONF3_ALARM_12_TO_7_SHIFT 8 + +#define AMS_PS_CSTS_PS_READY 0x08010000U +#define AMS_PL_CSTS_ACCESS_MASK 0x00000001U + +#define AMS_PL_MAX_FIXED_CHANNEL 10 +#define AMS_PL_MAX_EXT_CHANNEL 20 + +#define AMS_INIT_TIMEOUT_US 10000 + +/* + * Following scale and offset value is derived from + * UG580 (v1.7) December 20, 2016 + */ +#define AMS_SUPPLY_SCALE_1VOLT 1000 +#define AMS_SUPPLY_SCALE_3VOLT 3000 +#define AMS_SUPPLY_SCALE_6VOLT 6000 +#define AMS_SUPPLY_SCALE_DIV_BIT 16 + +#define AMS_TEMP_SCALE 509314 +#define AMS_TEMP_SCALE_DIV_BIT 16 +#define AMS_TEMP_OFFSET -((280230L << 16) / 509314) + +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 +}; + +enum ams_seq { + AMS_SEQ_VCC_PSPLL, + AMS_SEQ_VCC_PSBATT, + AMS_SEQ_VCCINT, + AMS_SEQ_VCCBRAM, + AMS_SEQ_VCCAUX, + AMS_SEQ_PSDDRPLL, + AMS_SEQ_INTDDR +}; + +enum ams_ps_pl_seq { + AMS_SEQ_CALIB, + AMS_SEQ_RSVD_1, + AMS_SEQ_RSVD_2, + AMS_SEQ_TEST, + AMS_SEQ_RSVD_4, + AMS_SEQ_SUPPLY4, + AMS_SEQ_SUPPLY5, + AMS_SEQ_SUPPLY6, + AMS_SEQ_TEMP, + AMS_SEQ_SUPPLY2, + AMS_SEQ_SUPPLY1, + AMS_SEQ_VP_VN, + AMS_SEQ_VREFP, + AMS_SEQ_VREFN, + AMS_SEQ_SUPPLY3, + AMS_SEQ_CURRENT_MON, + AMS_SEQ_SUPPLY7, + AMS_SEQ_SUPPLY8, + AMS_SEQ_SUPPLY9, + AMS_SEQ_SUPPLY10, + AMS_SEQ_VCCAMS, + AMS_SEQ_TEMP_REMOTE, + AMS_SEQ_MAX +}; + +#define AMS_SEQ(x) (AMS_SEQ_MAX + (x)) +#define AMS_VAUX_SEQ(x) (AMS_SEQ_MAX + (x)) + +#define AMS_PS_SEQ_MAX AMS_SEQ_MAX +#define PS_SEQ(x) (x) +#define PL_SEQ(x) (AMS_PS_SEQ_MAX + (x)) + +#define AMS_CHAN_TEMP(_scan_index, _addr) { \ + .type = IIO_TEMP, \ + .indexed = 1, \ + .address = (_addr), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .event_spec = ams_temp_events, \ + .num_event_specs = ARRAY_SIZE(ams_temp_events), \ + .scan_index = (_scan_index), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 12, \ + .storagebits = 16, \ + .shift = 4, \ + .endianness = IIO_CPU, \ + }, \ +} + +#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .address = (_addr), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .event_spec = (_alarm) ? ams_voltage_events : NULL, \ + .num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \ + .scan_index = (_scan_index), \ + .scan_type = { \ + .realbits = 10, \ + .storagebits = 16, \ + .shift = 6, \ + .endianness = IIO_CPU, \ + }, \ +} + +#define AMS_PS_CHAN_TEMP(_scan_index, _addr) \ + AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr) +#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr) \ + AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true) + +#define AMS_PL_CHAN_TEMP(_scan_index, _addr) \ + AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr) +#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm) \ + AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm) +#define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \ + AMS_CHAN_VOLTAGE(PL_SEQ(AMS_VAUX_SEQ(_auxno)), \ + AMS_REG_VAUX(_auxno), false) +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \ + AMS_CHAN_VOLTAGE(PL_SEQ(AMS_VAUX_SEQ(AMS_SEQ(_scan_index))), \ + _addr, false) + +/** + * struct ams - Driver data for xilinx-ams + * @base: physical base address of device + * @ps_base: physical base address of PS device + * @pl_base: physical base address of PL device + * @clk: clocks associated with the device + * @dev: pointer to device struct + * @lock: to handle multiple user interaction + * @region_list: list of the regions of sysmon + * @masked_alarm: currently masked due to alarm + * @alarm_mask: alarm configuration + * @interrupt_mask: interrupt configuration + * @irq: interrupt number of the sysmon + * @ams_unmask_work: re-enables event once the event condition disappears + * + * This structure contains necessary state for Sysmon driver to operate + */ +struct ams { + void __iomem *base; + void __iomem *ps_base; + void __iomem *pl_base; + struct clk *clk; + struct device *dev; + /* check kernel doc above */ + struct mutex lock; + unsigned int alarm_mask; + unsigned int masked_alarm; + u64 intr_mask; + int irq; + struct delayed_work ams_unmask_work; +}; + +static inline void ams_ps_update_reg(struct ams *ams, unsigned int offset, + u32 mask, u32 data) +{ + u32 val; + + val = readl(ams->ps_base + offset); + writel((val & ~mask) | (data & mask), ams->ps_base + offset); +} + +static inline void ams_pl_update_reg(struct ams *ams, unsigned int offset, + u32 mask, u32 data) +{ + u32 val; + + val = readl(ams->pl_base + offset); + writel((val & ~mask) | (data & mask), ams->pl_base + offset); +} + +static void ams_update_intrmask(struct ams *ams, u64 mask, u64 val) +{ + ams->intr_mask &= ~mask; + ams->intr_mask |= (val & mask); + + writel(~(ams->intr_mask | ams->masked_alarm), ams->base + AMS_IER_0); + writel(~(ams->intr_mask >> AMS_ISR1_INTR_MASK_SHIFT), + ams->base + AMS_IER_1); + writel(ams->intr_mask | ams->masked_alarm, ams->base + AMS_IDR_0); + writel(ams->intr_mask >> AMS_ISR1_INTR_MASK_SHIFT, + ams->base + AMS_IDR_1); +} + +static void ams_disable_all_alarms(struct ams *ams) +{ + /* disable PS module alarm */ + if (ams->ps_base) { + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK, + AMS_REGCFG1_ALARM_MASK); + ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK, + AMS_REGCFG3_ALARM_MASK); + } + + /* disable PL module alarm */ + if (ams->pl_base) { + ams_pl_update_reg(ams, AMS_REG_CONFIG1, + AMS_REGCFG1_ALARM_MASK, + AMS_REGCFG1_ALARM_MASK); + ams_pl_update_reg(ams, AMS_REG_CONFIG3, + AMS_REGCFG3_ALARM_MASK, + AMS_REGCFG3_ALARM_MASK); + } +} + +static void ams_update_alarm(struct ams *ams, unsigned long alarm_mask) +{ + u32 cfg; + unsigned long pl_alarm_mask; + + if (ams->ps_base) { + /* Configuring PS alarm enable */ + cfg = ~((alarm_mask & AMS_ISR0_ALARM_2_TO_0_MASK) << + AMS_CONF1_ALARM_2_TO_0_SHIFT); + cfg &= ~((alarm_mask & AMS_ISR0_ALARM_6_TO_3_MASK) << + AMS_CONF1_ALARM_6_TO_3_SHIFT); + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK, + cfg); + + cfg = ~((alarm_mask >> AMS_CONF3_ALARM_12_TO_7_SHIFT) & + AMS_ISR0_ALARM_12_TO_7_MASK); + ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK, + cfg); + } + + if (ams->pl_base) { + pl_alarm_mask = (alarm_mask >> AMS_PL_ALARM_START); + /* Configuring PL alarm enable */ + cfg = ~((pl_alarm_mask & AMS_ISR0_ALARM_2_TO_0_MASK) << + AMS_CONF1_ALARM_2_TO_0_SHIFT); + cfg &= ~((pl_alarm_mask & AMS_ISR0_ALARM_6_TO_3_MASK) << + AMS_CONF1_ALARM_6_TO_3_SHIFT); + ams_pl_update_reg(ams, AMS_REG_CONFIG1, + AMS_REGCFG1_ALARM_MASK, cfg); + + cfg = ~((pl_alarm_mask >> AMS_CONF3_ALARM_12_TO_7_SHIFT) & + AMS_ISR0_ALARM_12_TO_7_MASK); + ams_pl_update_reg(ams, AMS_REG_CONFIG3, + AMS_REGCFG3_ALARM_MASK, cfg); + } + + mutex_lock(&ams->lock); + ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask); + mutex_unlock(&ams->lock); +} + +static void ams_enable_channel_sequence(struct iio_dev *indio_dev) +{ + int i; + unsigned long long scan_mask; + struct ams *ams = iio_priv(indio_dev); + + /* + * Enable channel sequence. First 22 bits of scan_mask represent + * PS channels, and next remaining bits represent PL channels. + */ + + /* Run calibration of PS & PL as part of the sequence */ + scan_mask = 0x1 | BIT(AMS_PS_SEQ_MAX); + for (i = 0; i < indio_dev->num_channels; i++) + scan_mask |= BIT(indio_dev->channels[i].scan_index); + + if (ams->ps_base) { + /* put sysmon in a soft reset to change the sequence */ + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_DEFAULT); + + /* configure basic channels */ + writel(scan_mask & AMS_REG_SEQ0_MASK, + ams->ps_base + AMS_REG_SEQ_CH0); + writel(AMS_REG_SEQ2_MASK & + (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT), + ams->ps_base + AMS_REG_SEQ_CH2); + + /* set continuous sequence mode */ + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_CONTINUOUS); + } + + if (ams->pl_base) { + /* put sysmon in a soft reset to change the sequence */ + ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_DEFAULT); + + /* configure basic channels */ + scan_mask = scan_mask >> AMS_PS_SEQ_MAX; + writel(scan_mask & AMS_REG_SEQ0_MASK, + ams->pl_base + AMS_REG_SEQ_CH0); + writel(AMS_REG_SEQ2_MASK & + (scan_mask >> AMS_REG_SEQ2_MASK_SHIFT), + ams->pl_base + AMS_REG_SEQ_CH2); + writel(AMS_REG_SEQ1_MASK & + (scan_mask >> AMS_REG_SEQ1_MASK_SHIFT), + ams->pl_base + AMS_REG_SEQ_CH1); + + /* set continuous sequence mode */ + ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_CONTINUOUS); + } +} + +static int ams_init_device(struct ams *ams) +{ + u32 reg; + int ret; + + /* reset AMS */ + if (ams->ps_base) { + writel(AMS_PS_RESET_VALUE, ams->ps_base + AMS_VP_VN); + + ret = readl_poll_timeout(ams->base + AMS_PS_CSTS, reg, + (reg & AMS_PS_CSTS_PS_READY) == + AMS_PS_CSTS_PS_READY, 0, + AMS_INIT_TIMEOUT_US); + if (ret) + return ret; + + /* put sysmon in a default state */ + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_DEFAULT); + } + + if (ams->pl_base) { + writel(AMS_PL_RESET_VALUE, ams->pl_base + AMS_VP_VN); + + ret = readl_poll_timeout(ams->base + AMS_PL_CSTS, reg, + (reg & AMS_PL_CSTS_ACCESS_MASK) == + AMS_PL_CSTS_ACCESS_MASK, 0, + AMS_INIT_TIMEOUT_US); + if (ret) + return ret; + + /* put sysmon in a default state */ + ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_DEFAULT); + } + + ams_disable_all_alarms(ams); + + /* Disable interrupt */ + ams_update_intrmask(ams, ~0, ~0); + + /* Clear any pending interrupt */ + writel(AMS_ISR0_ALARM_MASK, ams->base + AMS_ISR_0); + writel(AMS_ISR1_ALARM_MASK, ams->base + AMS_ISR_1); + + return 0; +} + +static int ams_enable_single_channel(struct ams *ams, unsigned int offset) +{ + u8 channel_num = 0; + + switch (offset) { + case AMS_VCC_PSPLL0: + channel_num = AMS_VCC_PSPLL0_CH; + break; + case AMS_VCC_PSPLL3: + channel_num = AMS_VCC_PSPLL3_CH; + break; + case AMS_VCCINT: + channel_num = AMS_VCCINT_CH; + break; + case AMS_VCCBRAM: + channel_num = AMS_VCCBRAM_CH; + break; + case AMS_VCCAUX: + channel_num = AMS_VCCAUX_CH; + break; + case AMS_PSDDRPLL: + channel_num = AMS_PSDDRPLL_CH; + break; + case AMS_PSINTFPDDR: + channel_num = AMS_PSINTFPDDR_CH; + break; + default: + return -EINVAL; + } + + /* set single channel, sequencer off mode */ + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, + AMS_CONF1_SEQ_SINGLE_CHANNEL); + + /* write the channel number */ + ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK, + channel_num); + return 0; +} + +static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data) +{ + u32 reg; + int ret; + + ret = ams_enable_single_channel(ams, offset); + if (ret) + return ret; + + ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg, + (reg & AMS_ISR1_EOC_MASK) == AMS_ISR1_EOC_MASK, + 0, AMS_INIT_TIMEOUT_US); + if (ret) + return ret; + + *data = readl(ams->base + offset); + + return 0; +} + +static int ams_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct ams *ams = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + mutex_lock(&ams->lock); + if (chan->scan_index >= (AMS_PS_SEQ_MAX * 3)) { + ret = ams_read_vcc_reg(ams, chan->address, val); + if (ret) { + mutex_unlock(&ams->lock); + return -EINVAL; + } + ams_enable_channel_sequence(indio_dev); + } else if (chan->scan_index >= AMS_PS_SEQ_MAX) + *val = readl(ams->pl_base + chan->address); + else + *val = readl(ams->ps_base + chan->address); + mutex_unlock(&ams->lock); + + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_VOLTAGE: + switch (chan->address) { + case AMS_SUPPLY1: + case AMS_SUPPLY2: + case AMS_SUPPLY3: + case AMS_SUPPLY4: + *val = AMS_SUPPLY_SCALE_3VOLT; + break; + case AMS_SUPPLY5: + case AMS_SUPPLY6: + if (chan->scan_index < AMS_PS_SEQ_MAX) + *val = AMS_SUPPLY_SCALE_6VOLT; + else + *val = AMS_SUPPLY_SCALE_3VOLT; + break; + case AMS_SUPPLY7: + case AMS_SUPPLY8: + *val = AMS_SUPPLY_SCALE_6VOLT; + break; + case AMS_SUPPLY9: + case AMS_SUPPLY10: + if (chan->scan_index < AMS_PS_SEQ_MAX) + *val = AMS_SUPPLY_SCALE_3VOLT; + else + *val = AMS_SUPPLY_SCALE_6VOLT; + break; + case AMS_VCC_PSPLL0: + case AMS_VCC_PSPLL3: + case AMS_VCCINT: + case AMS_VCCBRAM: + case AMS_VCCAUX: + case AMS_PSDDRPLL: + case AMS_PSINTFPDDR: + *val = AMS_SUPPLY_SCALE_3VOLT; + break; + default: + *val = AMS_SUPPLY_SCALE_1VOLT; + break; + } + *val2 = AMS_SUPPLY_SCALE_DIV_BIT; + return IIO_VAL_FRACTIONAL_LOG2; + case IIO_TEMP: + *val = AMS_TEMP_SCALE; + *val2 = AMS_TEMP_SCALE_DIV_BIT; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } + case IIO_CHAN_INFO_OFFSET: + /* Only the temperature channel has an offset */ + *val = AMS_TEMP_OFFSET; + return IIO_VAL_INT; + } + + return -EINVAL; +} + +static int ams_get_alarm_offset(int scan_index, enum iio_event_direction dir) +{ + int offset = 0; + + if (scan_index >= AMS_PS_SEQ_MAX) + scan_index -= AMS_PS_SEQ_MAX; + + if (dir == IIO_EV_DIR_FALLING) { + if (scan_index < AMS_SEQ_SUPPLY7) + offset = AMS_ALARM_THRESHOLD_OFF_10; + else + offset = AMS_ALARM_THRESHOLD_OFF_20; + } + + switch (scan_index) { + case AMS_SEQ_TEMP: + return AMS_ALARM_TEMP + offset; + case AMS_SEQ_SUPPLY1: + return AMS_ALARM_SUPPLY1 + offset; + case AMS_SEQ_SUPPLY2: + return AMS_ALARM_SUPPLY2 + offset; + case AMS_SEQ_SUPPLY3: + return AMS_ALARM_SUPPLY3 + offset; + case AMS_SEQ_SUPPLY4: + return AMS_ALARM_SUPPLY4 + offset; + case AMS_SEQ_SUPPLY5: + return AMS_ALARM_SUPPLY5 + offset; + case AMS_SEQ_SUPPLY6: + return AMS_ALARM_SUPPLY6 + offset; + case AMS_SEQ_SUPPLY7: + return AMS_ALARM_SUPPLY7 + offset; + case AMS_SEQ_SUPPLY8: + return AMS_ALARM_SUPPLY8 + offset; + case AMS_SEQ_SUPPLY9: + return AMS_ALARM_SUPPLY9 + offset; + case AMS_SEQ_SUPPLY10: + return AMS_ALARM_SUPPLY10 + offset; + case AMS_SEQ_VCCAMS: + return AMS_ALARM_VCCAMS + offset; + case AMS_SEQ_TEMP_REMOTE: + return AMS_ALARM_TEMP_REMOTE + offset; + } + + return 0; +} + +static const struct iio_chan_spec +*ams_event_to_channel(struct iio_dev *indio_dev, u32 event) +{ + int scan_index = 0, i; + + if (event >= AMS_PL_ALARM_START) { + event -= AMS_PL_ALARM_START; + scan_index = AMS_PS_SEQ_MAX; + } + + switch (event) { + case AMS_ALARM_BIT_TEMP: + scan_index += AMS_SEQ_TEMP; + break; + case AMS_ALARM_BIT_SUPPLY1: + scan_index += AMS_SEQ_SUPPLY1; + break; + case AMS_ALARM_BIT_SUPPLY2: + scan_index += AMS_SEQ_SUPPLY2; + break; + case AMS_ALARM_BIT_SUPPLY3: + scan_index += AMS_SEQ_SUPPLY3; + break; + case AMS_ALARM_BIT_SUPPLY4: + scan_index += AMS_SEQ_SUPPLY4; + break; + case AMS_ALARM_BIT_SUPPLY5: + scan_index += AMS_SEQ_SUPPLY5; + break; + case AMS_ALARM_BIT_SUPPLY6: + scan_index += AMS_SEQ_SUPPLY6; + break; + case AMS_ALARM_BIT_SUPPLY7: + scan_index += AMS_SEQ_SUPPLY7; + break; + case AMS_ALARM_BIT_SUPPLY8: + scan_index += AMS_SEQ_SUPPLY8; + break; + case AMS_ALARM_BIT_SUPPLY9: + scan_index += AMS_SEQ_SUPPLY9; + break; + case AMS_ALARM_BIT_SUPPLY10: + scan_index += AMS_SEQ_SUPPLY10; + break; + case AMS_ALARM_BIT_VCCAMS: + scan_index += AMS_SEQ_VCCAMS; + break; + case AMS_ALARM_BIT_TEMP_REMOTE: + scan_index += AMS_SEQ_TEMP_REMOTE; + break; + } + + for (i = 0; i < indio_dev->num_channels; i++) + if (indio_dev->channels[i].scan_index == scan_index) + break; + + return &indio_dev->channels[i]; +} + +static int ams_get_alarm_mask(int scan_index) +{ + int bit = 0; + + if (scan_index >= AMS_PS_SEQ_MAX) { + bit = AMS_PL_ALARM_START; + scan_index -= AMS_PS_SEQ_MAX; + } + + switch (scan_index) { + case AMS_SEQ_TEMP: + return BIT(AMS_ALARM_BIT_TEMP + bit); + case AMS_SEQ_SUPPLY1: + return BIT(AMS_ALARM_BIT_SUPPLY1 + bit); + case AMS_SEQ_SUPPLY2: + return BIT(AMS_ALARM_BIT_SUPPLY2 + bit); + case AMS_SEQ_SUPPLY3: + return BIT(AMS_ALARM_BIT_SUPPLY3 + bit); + case AMS_SEQ_SUPPLY4: + return BIT(AMS_ALARM_BIT_SUPPLY4 + bit); + case AMS_SEQ_SUPPLY5: + return BIT(AMS_ALARM_BIT_SUPPLY5 + bit); + case AMS_SEQ_SUPPLY6: + return BIT(AMS_ALARM_BIT_SUPPLY6 + bit); + case AMS_SEQ_SUPPLY7: + return BIT(AMS_ALARM_BIT_SUPPLY7 + bit); + case AMS_SEQ_SUPPLY8: + return BIT(AMS_ALARM_BIT_SUPPLY8 + bit); + case AMS_SEQ_SUPPLY9: + return BIT(AMS_ALARM_BIT_SUPPLY9 + bit); + case AMS_SEQ_SUPPLY10: + return BIT(AMS_ALARM_BIT_SUPPLY10 + bit); + case AMS_SEQ_VCCAMS: + return BIT(AMS_ALARM_BIT_VCCAMS + bit); + case AMS_SEQ_TEMP_REMOTE: + return BIT(AMS_ALARM_BIT_TEMP_REMOTE + bit); + } + + return 0; +} + +static int ams_read_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir) +{ + struct ams *ams = iio_priv(indio_dev); + + return (ams->alarm_mask & ams_get_alarm_mask(chan->scan_index)) ? 1 : 0; +} + +static int ams_write_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + int state) +{ + struct ams *ams = iio_priv(indio_dev); + unsigned int alarm; + + alarm = ams_get_alarm_mask(chan->scan_index); + + mutex_lock(&ams->lock); + + if (state) + ams->alarm_mask |= alarm; + else + ams->alarm_mask &= ~alarm; + + ams_update_alarm(ams, ams->alarm_mask); + + mutex_unlock(&ams->lock); + + return 0; +} + +static int ams_read_event_value(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, int *val, int *val2) +{ + struct ams *ams = iio_priv(indio_dev); + unsigned int offset = ams_get_alarm_offset(chan->scan_index, dir); + + mutex_lock(&ams->lock); + + if (chan->scan_index >= AMS_PS_SEQ_MAX) + *val = readl(ams->pl_base + offset); + else + *val = readl(ams->ps_base + offset); + + mutex_unlock(&ams->lock); + + return IIO_VAL_INT; +} + +static int ams_write_event_value(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, int val, int val2) +{ + struct ams *ams = iio_priv(indio_dev); + unsigned int offset; + + mutex_lock(&ams->lock); + + /* Set temperature channel threshold to direct threshold */ + if (chan->type == IIO_TEMP) { + offset = ams_get_alarm_offset(chan->scan_index, + IIO_EV_DIR_FALLING); + + if (chan->scan_index >= AMS_PS_SEQ_MAX) + ams_pl_update_reg(ams, offset, + AMS_ALARM_THR_DIRECT_MASK, + AMS_ALARM_THR_DIRECT_MASK); + else + ams_ps_update_reg(ams, offset, + AMS_ALARM_THR_DIRECT_MASK, + AMS_ALARM_THR_DIRECT_MASK); + } + + offset = ams_get_alarm_offset(chan->scan_index, dir); + if (chan->scan_index >= AMS_PS_SEQ_MAX) + writel(val, ams->pl_base + offset); + else + writel(val, ams->ps_base + offset); + + mutex_unlock(&ams->lock); + + return 0; +} + +static void ams_handle_event(struct iio_dev *indio_dev, u32 event) +{ + const struct iio_chan_spec *chan; + + chan = ams_event_to_channel(indio_dev, event); + + if (chan->type == IIO_TEMP) { + /* + * The temperature channel only supports over-temperature + * events + */ + iio_push_event(indio_dev, + IIO_UNMOD_EVENT_CODE(chan->type, chan->channel, + IIO_EV_TYPE_THRESH, + IIO_EV_DIR_RISING), + iio_get_time_ns(indio_dev)); + } else { + /* + * For other channels we don't know whether it is a upper or + * lower threshold event. Userspace will have to check the + * channel value if it wants to know. + */ + iio_push_event(indio_dev, + IIO_UNMOD_EVENT_CODE(chan->type, chan->channel, + IIO_EV_TYPE_THRESH, + IIO_EV_DIR_EITHER), + iio_get_time_ns(indio_dev)); + } +} + +static void ams_handle_events(struct iio_dev *indio_dev, unsigned long events) +{ + unsigned int bit; + + for_each_set_bit(bit, &events, AMS_NO_OF_ALARMS) + ams_handle_event(indio_dev, bit); +} + +/** + * ams_unmask_worker - ams alarm interrupt unmask worker + * @work : work to be done + * + * The ZynqMP threshold interrupts are level sensitive. Since we can't make the + * threshold condition go way from within the interrupt handler, this means as + * soon as a threshold condition is present we would enter the interrupt handler + * again and again. To work around this we mask all active threshold interrupts + * in the interrupt handler and start a timer. In this timer we poll the + * interrupt status and only if the interrupt is inactive we unmask it again. + */ +static void ams_unmask_worker(struct work_struct *work) +{ + struct ams *ams = container_of(work, struct ams, ams_unmask_work.work); + unsigned int status, unmask; + + mutex_lock(&ams->lock); + + status = readl(ams->base + AMS_ISR_0); + + /* Clear those bits which are not active anymore */ + unmask = (ams->masked_alarm ^ status) & ams->masked_alarm; + + /* clear status of disabled alarm */ + unmask |= ams->intr_mask; + + ams->masked_alarm &= status; + + /* Also clear those which are masked out anyway */ + ams->masked_alarm &= ~ams->intr_mask; + + /* Clear the interrupts before we unmask them */ + writel(unmask, ams->base + AMS_ISR_0); + + ams_update_intrmask(ams, 0, 0); + + mutex_unlock(&ams->lock); + + /* if still pending some alarm re-trigger the timer */ + if (ams->masked_alarm) + schedule_delayed_work(&ams->ams_unmask_work, + msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS)); +} + +static irqreturn_t ams_irq(int irq, void *data) +{ + struct iio_dev *indio_dev = data; + struct ams *ams = iio_priv(indio_dev); + u32 isr0; + + isr0 = readl(ams->base + AMS_ISR_0); + + /* only process alarms that are not masked */ + isr0 &= ~((ams->intr_mask & AMS_ISR0_ALARM_MASK) | ams->masked_alarm); + + if (!isr0) + return IRQ_NONE; + + /* clear interrupt */ + writel(isr0, ams->base + AMS_ISR_0); + + /* Mask the alarm interrupts until cleared */ + ams->masked_alarm |= isr0; + ams_update_intrmask(ams, 0, 0); + + ams_handle_events(indio_dev, isr0); + + schedule_delayed_work(&ams->ams_unmask_work, + msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS)); + + return IRQ_HANDLED; +} + +static const struct iio_event_spec ams_temp_events[] = { + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_RISING, + .mask_separate = BIT(IIO_EV_INFO_ENABLE) | + BIT(IIO_EV_INFO_VALUE), + }, +}; + +static const struct iio_event_spec ams_voltage_events[] = { + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_RISING, + .mask_separate = BIT(IIO_EV_INFO_VALUE), + }, + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_FALLING, + .mask_separate = BIT(IIO_EV_INFO_VALUE), + }, + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_EITHER, + .mask_separate = BIT(IIO_EV_INFO_ENABLE), + }, +}; + +static const struct iio_chan_spec ams_ps_channels[] = { + AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP), + AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10), + AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS), +}; + +static const struct iio_chan_spec ams_pl_channels[] = { + AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true), + AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true), + AMS_PL_AUX_CHAN_VOLTAGE(0), + AMS_PL_AUX_CHAN_VOLTAGE(1), + AMS_PL_AUX_CHAN_VOLTAGE(2), + AMS_PL_AUX_CHAN_VOLTAGE(3), + AMS_PL_AUX_CHAN_VOLTAGE(4), + AMS_PL_AUX_CHAN_VOLTAGE(5), + AMS_PL_AUX_CHAN_VOLTAGE(6), + AMS_PL_AUX_CHAN_VOLTAGE(7), + AMS_PL_AUX_CHAN_VOLTAGE(8), + AMS_PL_AUX_CHAN_VOLTAGE(9), + AMS_PL_AUX_CHAN_VOLTAGE(10), + AMS_PL_AUX_CHAN_VOLTAGE(11), + AMS_PL_AUX_CHAN_VOLTAGE(12), + AMS_PL_AUX_CHAN_VOLTAGE(13), + AMS_PL_AUX_CHAN_VOLTAGE(14), + AMS_PL_AUX_CHAN_VOLTAGE(15), +}; + +static const struct iio_chan_spec ams_ctrl_channels[] = { + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0), + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3), + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT), + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM), + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX), + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL), + AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR), +}; + +static int ams_get_ext_chan(struct device_node *chan_node, + struct iio_chan_spec *channels, int num_channels) +{ + struct device_node *child; + unsigned int reg; + int ret; + + for_each_child_of_node(chan_node, child) { + ret = of_property_read_u32(child, "reg", ®); + if (ret || reg > (AMS_PL_MAX_EXT_CHANNEL + 30)) + continue; + + memcpy(&channels[num_channels], &ams_pl_channels[reg + + AMS_PL_MAX_FIXED_CHANNEL - 30], sizeof(*channels)); + + if (of_property_read_bool(child, + "xlnx,bipolar")) + channels[num_channels].scan_type.sign = 's'; + + num_channels++; + } + + return num_channels; +} + +static int ams_init_module(struct iio_dev *indio_dev, struct device_node *np, + struct iio_chan_spec *channels) +{ + struct ams *ams = iio_priv(indio_dev); + struct device_node *chan_node; + int num_channels = 0; + + if (of_device_is_compatible(np, "xlnx,zynqmp-ams-ps")) { + ams->ps_base = of_iomap(np, 0); + if (!ams->ps_base) + return -ENXIO; + + /* add PS channels to iio device channels */ + memcpy(channels + num_channels, ams_ps_channels, + sizeof(ams_ps_channels)); + num_channels += ARRAY_SIZE(ams_ps_channels); + } else if (of_device_is_compatible(np, "xlnx,zynqmp-ams-pl")) { + ams->pl_base = of_iomap(np, 0); + if (!ams->pl_base) + return -ENXIO; + + /* Copy only first 10 fix channels */ + memcpy(channels + num_channels, ams_pl_channels, + AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels)); + num_channels += AMS_PL_MAX_FIXED_CHANNEL; + + num_channels = ams_get_ext_chan(chan_node, channels, + num_channels); + + } else if (of_device_is_compatible(np, "xlnx,zynqmp-ams")) { + /* add AMS channels to iio device channels */ + memcpy(channels + num_channels, ams_ctrl_channels, + sizeof(ams_ctrl_channels)); + num_channels += ARRAY_SIZE(ams_ctrl_channels); + } else { + return -EINVAL; + } + + return num_channels; +} + +static int ams_parse_dt(struct iio_dev *indio_dev, struct platform_device *pdev) +{ + struct ams *ams = iio_priv(indio_dev); + struct iio_chan_spec *ams_channels, *dev_channels; + struct device_node *child_node = NULL, *np = pdev->dev.of_node; + int ret, vol_ch_cnt = 0, temp_ch_cnt = 0, i, rising_off, falling_off; + unsigned int num_channels = 0; + + /* Initialize buffer for channel specification */ + ams_channels = kzalloc(sizeof(ams_ps_channels) + + sizeof(ams_pl_channels) + + sizeof(ams_ctrl_channels), GFP_KERNEL); + if (!ams_channels) + return -ENOMEM; + + if (of_device_is_available(np)) { + ret = ams_init_module(indio_dev, np, ams_channels); + if (ret < 0) + goto err; + + num_channels += ret; + } + + for_each_child_of_node(np, child_node) { + if (of_device_is_available(child_node)) { + ret = ams_init_module(indio_dev, child_node, + ams_channels + num_channels); + if (ret < 0) + goto err; + + num_channels += ret; + } + } + + for (i = 0; i < num_channels; i++) { + if (ams_channels[i].type == IIO_VOLTAGE) + ams_channels[i].channel = vol_ch_cnt++; + else + ams_channels[i].channel = temp_ch_cnt++; + + if (ams_channels[i].scan_index < (AMS_PS_SEQ_MAX * 3)) { + /* set threshold to max and min for each channel */ + falling_off = + ams_get_alarm_offset(ams_channels[i].scan_index, + IIO_EV_DIR_FALLING); + rising_off = + ams_get_alarm_offset(ams_channels[i].scan_index, + IIO_EV_DIR_RISING); + if (ams_channels[i].scan_index >= AMS_PS_SEQ_MAX) { + writel(AMS_ALARM_THR_MIN, + ams->pl_base + falling_off); + writel(AMS_ALARM_THR_MAX, + ams->pl_base + rising_off); + } else { + writel(AMS_ALARM_THR_MIN, + ams->ps_base + falling_off); + writel(AMS_ALARM_THR_MAX, + ams->ps_base + rising_off); + } + } + } + + dev_channels = devm_kzalloc(&pdev->dev, sizeof(*dev_channels) * + num_channels, GFP_KERNEL); + if (!dev_channels) { + ret = -ENOMEM; + goto err; + } + + memcpy(dev_channels, ams_channels, + sizeof(*ams_channels) * num_channels); + indio_dev->channels = dev_channels; + indio_dev->num_channels = num_channels; + + ret = 0; +err: + kfree(ams_channels); + + return ret; +} + +static const struct iio_info iio_ams_info = { + .read_raw = &ams_read_raw, + .read_event_config = &ams_read_event_config, + .write_event_config = &ams_write_event_config, + .read_event_value = &ams_read_event_value, + .write_event_value = &ams_write_event_value, +}; + +static const struct of_device_id ams_of_match_table[] = { + { .compatible = "xlnx,zynqmp-ams" }, + { } +}; +MODULE_DEVICE_TABLE(of, ams_of_match_table); + +static int ams_probe(struct platform_device *pdev) +{ + struct iio_dev *indio_dev; + struct ams *ams; + struct resource *res; + int ret; + + if (!pdev->dev.of_node) + return -ENODEV; + + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ams)); + if (!indio_dev) + return -ENOMEM; + + ams = iio_priv(indio_dev); + mutex_init(&ams->lock); + + indio_dev->name = "xilinx-ams"; + + indio_dev->info = &iio_ams_info; + indio_dev->modes = INDIO_DIRECT_MODE; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + ams->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(ams->base)) + return PTR_ERR(ams->base); + + ams->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(ams->clk)) + return PTR_ERR(ams->clk); + clk_prepare_enable(ams->clk); + devm_add_action_or_reset(&pdev->dev, (void *)clk_disable_unprepare, + ams->clk); + + INIT_DELAYED_WORK(&ams->ams_unmask_work, ams_unmask_worker); + devm_add_action_or_reset(&pdev->dev, (void *)cancel_delayed_work, + &ams->ams_unmask_work); + + ret = ams_init_device(ams); + if (ret) { + dev_err(&pdev->dev, "failed to initialize AMS\n"); + return ret; + } + + ret = ams_parse_dt(indio_dev, pdev); + if (ret) { + dev_err(&pdev->dev, "failure in parsing DT\n"); + return ret; + } + + ams_enable_channel_sequence(indio_dev); + + ams->irq = platform_get_irq(pdev, 0); + ret = devm_request_irq(&pdev->dev, ams->irq, &ams_irq, 0, "ams-irq", + indio_dev); + if (ret < 0) { + dev_err(&pdev->dev, "failed to register interrupt\n"); + return ret; + } + + platform_set_drvdata(pdev, indio_dev); + + return iio_device_register(indio_dev); +} + +static int ams_remove(struct platform_device *pdev) +{ + struct iio_dev *indio_dev = platform_get_drvdata(pdev); + + iio_device_unregister(indio_dev); + + return 0; +} + +static int __maybe_unused ams_suspend(struct device *dev) +{ + struct ams *ams = iio_priv(dev_get_drvdata(dev)); + + clk_disable_unprepare(ams->clk); + + return 0; +} + +static int __maybe_unused ams_resume(struct device *dev) +{ + struct ams *ams = iio_priv(dev_get_drvdata(dev)); + + clk_prepare_enable(ams->clk); + + return 0; +} + +static SIMPLE_DEV_PM_OPS(ams_pm_ops, ams_suspend, ams_resume); + +static struct platform_driver ams_driver = { + .probe = ams_probe, + .remove = ams_remove, + .driver = { + .name = "xilinx-ams", + .pm = &ams_pm_ops, + .of_match_table = ams_of_match_table, + }, +}; +module_platform_driver(ams_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Xilinx, Inc."); -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre @ 2021-06-25 10:49 ` kernel test robot 2021-07-01 8:00 ` kernel test robot 2021-07-04 18:31 ` Jonathan Cameron 2 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2021-06-25 10:49 UTC (permalink / raw) To: Anand Ashok Dumbre, jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: kbuild-all, clang-built-linux, Anand Ashok Dumbre, Manish Narani [-- Attachment #1: Type: text/plain, Size: 3861 bytes --] Hi Anand, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on iio/togreg] [also build test WARNING on linus/master v5.13-rc7 next-20210624] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: x86_64-randconfig-r015-20210625 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9ca0171a9ffdef5fdb1511d197a3fd72490362de) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 git checkout fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/iio/adc/xilinx-ams.c:1126:35: warning: variable 'chan_node' is uninitialized when used here [-Wuninitialized] num_channels = ams_get_ext_chan(chan_node, channels, ^~~~~~~~~ drivers/iio/adc/xilinx-ams.c:1104:31: note: initialize the variable 'chan_node' to silence this warning struct device_node *chan_node; ^ = NULL 1 warning generated. vim +/chan_node +1126 drivers/iio/adc/xilinx-ams.c 1099 1100 static int ams_init_module(struct iio_dev *indio_dev, struct device_node *np, 1101 struct iio_chan_spec *channels) 1102 { 1103 struct ams *ams = iio_priv(indio_dev); 1104 struct device_node *chan_node; 1105 int num_channels = 0; 1106 1107 if (of_device_is_compatible(np, "xlnx,zynqmp-ams-ps")) { 1108 ams->ps_base = of_iomap(np, 0); 1109 if (!ams->ps_base) 1110 return -ENXIO; 1111 1112 /* add PS channels to iio device channels */ 1113 memcpy(channels + num_channels, ams_ps_channels, 1114 sizeof(ams_ps_channels)); 1115 num_channels += ARRAY_SIZE(ams_ps_channels); 1116 } else if (of_device_is_compatible(np, "xlnx,zynqmp-ams-pl")) { 1117 ams->pl_base = of_iomap(np, 0); 1118 if (!ams->pl_base) 1119 return -ENXIO; 1120 1121 /* Copy only first 10 fix channels */ 1122 memcpy(channels + num_channels, ams_pl_channels, 1123 AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels)); 1124 num_channels += AMS_PL_MAX_FIXED_CHANNEL; 1125 > 1126 num_channels = ams_get_ext_chan(chan_node, channels, 1127 num_channels); 1128 1129 } else if (of_device_is_compatible(np, "xlnx,zynqmp-ams")) { 1130 /* add AMS channels to iio device channels */ 1131 memcpy(channels + num_channels, ams_ctrl_channels, 1132 sizeof(ams_ctrl_channels)); 1133 num_channels += ARRAY_SIZE(ams_ctrl_channels); 1134 } else { 1135 return -EINVAL; 1136 } 1137 1138 return num_channels; 1139 } 1140 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 52567 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver @ 2021-06-25 10:49 ` kernel test robot 0 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2021-06-25 10:49 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 3953 bytes --] Hi Anand, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on iio/togreg] [also build test WARNING on linus/master v5.13-rc7 next-20210624] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: x86_64-randconfig-r015-20210625 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9ca0171a9ffdef5fdb1511d197a3fd72490362de) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 git checkout fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/iio/adc/xilinx-ams.c:1126:35: warning: variable 'chan_node' is uninitialized when used here [-Wuninitialized] num_channels = ams_get_ext_chan(chan_node, channels, ^~~~~~~~~ drivers/iio/adc/xilinx-ams.c:1104:31: note: initialize the variable 'chan_node' to silence this warning struct device_node *chan_node; ^ = NULL 1 warning generated. vim +/chan_node +1126 drivers/iio/adc/xilinx-ams.c 1099 1100 static int ams_init_module(struct iio_dev *indio_dev, struct device_node *np, 1101 struct iio_chan_spec *channels) 1102 { 1103 struct ams *ams = iio_priv(indio_dev); 1104 struct device_node *chan_node; 1105 int num_channels = 0; 1106 1107 if (of_device_is_compatible(np, "xlnx,zynqmp-ams-ps")) { 1108 ams->ps_base = of_iomap(np, 0); 1109 if (!ams->ps_base) 1110 return -ENXIO; 1111 1112 /* add PS channels to iio device channels */ 1113 memcpy(channels + num_channels, ams_ps_channels, 1114 sizeof(ams_ps_channels)); 1115 num_channels += ARRAY_SIZE(ams_ps_channels); 1116 } else if (of_device_is_compatible(np, "xlnx,zynqmp-ams-pl")) { 1117 ams->pl_base = of_iomap(np, 0); 1118 if (!ams->pl_base) 1119 return -ENXIO; 1120 1121 /* Copy only first 10 fix channels */ 1122 memcpy(channels + num_channels, ams_pl_channels, 1123 AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels)); 1124 num_channels += AMS_PL_MAX_FIXED_CHANNEL; 1125 > 1126 num_channels = ams_get_ext_chan(chan_node, channels, 1127 num_channels); 1128 1129 } else if (of_device_is_compatible(np, "xlnx,zynqmp-ams")) { 1130 /* add AMS channels to iio device channels */ 1131 memcpy(channels + num_channels, ams_ctrl_channels, 1132 sizeof(ams_ctrl_channels)); 1133 num_channels += ARRAY_SIZE(ams_ctrl_channels); 1134 } else { 1135 return -EINVAL; 1136 } 1137 1138 return num_channels; 1139 } 1140 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 52567 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre @ 2021-07-01 8:00 ` kernel test robot 2021-07-01 8:00 ` kernel test robot 2021-07-04 18:31 ` Jonathan Cameron 2 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2021-07-01 8:00 UTC (permalink / raw) To: Anand Ashok Dumbre, jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: clang-built-linux, kbuild-all, Anand Ashok Dumbre, Manish Narani [-- Attachment #1: Type: text/plain, Size: 5685 bytes --] Hi Anand, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on iio/togreg] [also build test WARNING on linus/master v5.13] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: riscv-randconfig-r022-20210701 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e7e71e9454ed76c1b3d8140170b5333c28bef1be) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/0day-ci/linux/commit/fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 git checkout fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/iio/adc/xilinx-ams.c:631:10: warning: signed shift result (0x446A60000) requires 36 bits to represent, but 'long' only has 32 bits [-Wshift-overflow] *val = AMS_TEMP_OFFSET; ^~~~~~~~~~~~~~~ drivers/iio/adc/xilinx-ams.c:151:38: note: expanded from macro 'AMS_TEMP_OFFSET' #define AMS_TEMP_OFFSET -((280230L << 16) / 509314) ~~~~~~~ ^ ~~ >> drivers/iio/adc/xilinx-ams.c:1126:35: warning: variable 'chan_node' is uninitialized when used here [-Wuninitialized] num_channels = ams_get_ext_chan(chan_node, channels, ^~~~~~~~~ drivers/iio/adc/xilinx-ams.c:1104:31: note: initialize the variable 'chan_node' to silence this warning struct device_node *chan_node; ^ = NULL 2 warnings generated. Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for LOCKDEP Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86) Selected by - PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT - DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT vim +/long +631 drivers/iio/adc/xilinx-ams.c 554 555 static int ams_read_raw(struct iio_dev *indio_dev, 556 struct iio_chan_spec const *chan, 557 int *val, int *val2, long mask) 558 { 559 struct ams *ams = iio_priv(indio_dev); 560 int ret; 561 562 switch (mask) { 563 case IIO_CHAN_INFO_RAW: 564 mutex_lock(&ams->lock); 565 if (chan->scan_index >= (AMS_PS_SEQ_MAX * 3)) { 566 ret = ams_read_vcc_reg(ams, chan->address, val); 567 if (ret) { 568 mutex_unlock(&ams->lock); 569 return -EINVAL; 570 } 571 ams_enable_channel_sequence(indio_dev); 572 } else if (chan->scan_index >= AMS_PS_SEQ_MAX) 573 *val = readl(ams->pl_base + chan->address); 574 else 575 *val = readl(ams->ps_base + chan->address); 576 mutex_unlock(&ams->lock); 577 578 return IIO_VAL_INT; 579 case IIO_CHAN_INFO_SCALE: 580 switch (chan->type) { 581 case IIO_VOLTAGE: 582 switch (chan->address) { 583 case AMS_SUPPLY1: 584 case AMS_SUPPLY2: 585 case AMS_SUPPLY3: 586 case AMS_SUPPLY4: 587 *val = AMS_SUPPLY_SCALE_3VOLT; 588 break; 589 case AMS_SUPPLY5: 590 case AMS_SUPPLY6: 591 if (chan->scan_index < AMS_PS_SEQ_MAX) 592 *val = AMS_SUPPLY_SCALE_6VOLT; 593 else 594 *val = AMS_SUPPLY_SCALE_3VOLT; 595 break; 596 case AMS_SUPPLY7: 597 case AMS_SUPPLY8: 598 *val = AMS_SUPPLY_SCALE_6VOLT; 599 break; 600 case AMS_SUPPLY9: 601 case AMS_SUPPLY10: 602 if (chan->scan_index < AMS_PS_SEQ_MAX) 603 *val = AMS_SUPPLY_SCALE_3VOLT; 604 else 605 *val = AMS_SUPPLY_SCALE_6VOLT; 606 break; 607 case AMS_VCC_PSPLL0: 608 case AMS_VCC_PSPLL3: 609 case AMS_VCCINT: 610 case AMS_VCCBRAM: 611 case AMS_VCCAUX: 612 case AMS_PSDDRPLL: 613 case AMS_PSINTFPDDR: 614 *val = AMS_SUPPLY_SCALE_3VOLT; 615 break; 616 default: 617 *val = AMS_SUPPLY_SCALE_1VOLT; 618 break; 619 } 620 *val2 = AMS_SUPPLY_SCALE_DIV_BIT; 621 return IIO_VAL_FRACTIONAL_LOG2; 622 case IIO_TEMP: 623 *val = AMS_TEMP_SCALE; 624 *val2 = AMS_TEMP_SCALE_DIV_BIT; 625 return IIO_VAL_FRACTIONAL_LOG2; 626 default: 627 return -EINVAL; 628 } 629 case IIO_CHAN_INFO_OFFSET: 630 /* Only the temperature channel has an offset */ > 631 *val = AMS_TEMP_OFFSET; 632 return IIO_VAL_INT; 633 } 634 635 return -EINVAL; 636 } 637 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 35065 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver @ 2021-07-01 8:00 ` kernel test robot 0 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2021-07-01 8:00 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 5832 bytes --] Hi Anand, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on iio/togreg] [also build test WARNING on linus/master v5.13] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: riscv-randconfig-r022-20210701 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e7e71e9454ed76c1b3d8140170b5333c28bef1be) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/0day-ci/linux/commit/fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20210625-023047 git checkout fa0ea7aaf7a9bff3781f19596b07fe33c6ef531d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/iio/adc/xilinx-ams.c:631:10: warning: signed shift result (0x446A60000) requires 36 bits to represent, but 'long' only has 32 bits [-Wshift-overflow] *val = AMS_TEMP_OFFSET; ^~~~~~~~~~~~~~~ drivers/iio/adc/xilinx-ams.c:151:38: note: expanded from macro 'AMS_TEMP_OFFSET' #define AMS_TEMP_OFFSET -((280230L << 16) / 509314) ~~~~~~~ ^ ~~ >> drivers/iio/adc/xilinx-ams.c:1126:35: warning: variable 'chan_node' is uninitialized when used here [-Wuninitialized] num_channels = ams_get_ext_chan(chan_node, channels, ^~~~~~~~~ drivers/iio/adc/xilinx-ams.c:1104:31: note: initialize the variable 'chan_node' to silence this warning struct device_node *chan_node; ^ = NULL 2 warnings generated. Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for LOCKDEP Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86) Selected by - PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT - DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT vim +/long +631 drivers/iio/adc/xilinx-ams.c 554 555 static int ams_read_raw(struct iio_dev *indio_dev, 556 struct iio_chan_spec const *chan, 557 int *val, int *val2, long mask) 558 { 559 struct ams *ams = iio_priv(indio_dev); 560 int ret; 561 562 switch (mask) { 563 case IIO_CHAN_INFO_RAW: 564 mutex_lock(&ams->lock); 565 if (chan->scan_index >= (AMS_PS_SEQ_MAX * 3)) { 566 ret = ams_read_vcc_reg(ams, chan->address, val); 567 if (ret) { 568 mutex_unlock(&ams->lock); 569 return -EINVAL; 570 } 571 ams_enable_channel_sequence(indio_dev); 572 } else if (chan->scan_index >= AMS_PS_SEQ_MAX) 573 *val = readl(ams->pl_base + chan->address); 574 else 575 *val = readl(ams->ps_base + chan->address); 576 mutex_unlock(&ams->lock); 577 578 return IIO_VAL_INT; 579 case IIO_CHAN_INFO_SCALE: 580 switch (chan->type) { 581 case IIO_VOLTAGE: 582 switch (chan->address) { 583 case AMS_SUPPLY1: 584 case AMS_SUPPLY2: 585 case AMS_SUPPLY3: 586 case AMS_SUPPLY4: 587 *val = AMS_SUPPLY_SCALE_3VOLT; 588 break; 589 case AMS_SUPPLY5: 590 case AMS_SUPPLY6: 591 if (chan->scan_index < AMS_PS_SEQ_MAX) 592 *val = AMS_SUPPLY_SCALE_6VOLT; 593 else 594 *val = AMS_SUPPLY_SCALE_3VOLT; 595 break; 596 case AMS_SUPPLY7: 597 case AMS_SUPPLY8: 598 *val = AMS_SUPPLY_SCALE_6VOLT; 599 break; 600 case AMS_SUPPLY9: 601 case AMS_SUPPLY10: 602 if (chan->scan_index < AMS_PS_SEQ_MAX) 603 *val = AMS_SUPPLY_SCALE_3VOLT; 604 else 605 *val = AMS_SUPPLY_SCALE_6VOLT; 606 break; 607 case AMS_VCC_PSPLL0: 608 case AMS_VCC_PSPLL3: 609 case AMS_VCCINT: 610 case AMS_VCCBRAM: 611 case AMS_VCCAUX: 612 case AMS_PSDDRPLL: 613 case AMS_PSINTFPDDR: 614 *val = AMS_SUPPLY_SCALE_3VOLT; 615 break; 616 default: 617 *val = AMS_SUPPLY_SCALE_1VOLT; 618 break; 619 } 620 *val2 = AMS_SUPPLY_SCALE_DIV_BIT; 621 return IIO_VAL_FRACTIONAL_LOG2; 622 case IIO_TEMP: 623 *val = AMS_TEMP_SCALE; 624 *val2 = AMS_TEMP_SCALE_DIV_BIT; 625 return IIO_VAL_FRACTIONAL_LOG2; 626 default: 627 return -EINVAL; 628 } 629 case IIO_CHAN_INFO_OFFSET: 630 /* Only the temperature channel has an offset */ > 631 *val = AMS_TEMP_OFFSET; 632 return IIO_VAL_INT; 633 } 634 635 return -EINVAL; 636 } 637 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 35065 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre 2021-06-25 10:49 ` kernel test robot 2021-07-01 8:00 ` kernel test robot @ 2021-07-04 18:31 ` Jonathan Cameron 2021-07-15 7:48 ` Anand Ashok Dumbre 2 siblings, 1 reply; 19+ messages in thread From: Jonathan Cameron @ 2021-07-04 18:31 UTC (permalink / raw) To: Anand Ashok Dumbre Cc: lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel, Manish Narani On Thu, 24 Jun 2021 19:29:37 +0100 Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> 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 external master. Out of these interface 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. > > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> Hi Anand, A few comments inline from a fresh read. Only significant one is that the use of separate masks and shifts differs from how they are normally done in the kernel these days. FIELD_PREP() and FIELD_GET() allow a single mask definition to be cleanly used for both the shift and masking in a nice clean way. Thanks, Jonathan > --- > drivers/iio/adc/Kconfig | 13 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/xilinx-ams.c | 1342 ++++++++++++++++++++++++++++++++++ > 3 files changed, 1356 insertions(+) > create mode 100644 drivers/iio/adc/xilinx-ams.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index c7946c439612..c011ab30ec9a 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -1256,4 +1256,17 @@ config XILINX_XADC > The driver can also be build as a module. If so, the module will be called > xilinx-xadc. > > +config XILINX_AMS > + tristate "Xilinx AMS driver" > + depends on ARCH_ZYNQMP || COMPILE_TEST > + depends on HAS_IOMEM > + help > + Say yes here to have support for the Xilinx AMS. > + > + The driver supports Voltage and Temperature monitoring on Xilinx Ultrascale > + devices. > + > + The driver can also be built as a module. If so, the module will be called > + xilinx-ams. > + > endmenu > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index a226657d19c0..99e031f44479 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -112,4 +112,5 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o > obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o > xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o > obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o > +obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o > obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c > new file mode 100644 > index 000000000000..463e3162726c > --- /dev/null > +++ b/drivers/iio/adc/xilinx-ams.c > @@ -0,0 +1,1342 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Xilinx AMS driver > + * > + * Copyright (C) 2021 Xilinx, Inc. > + * > + * Manish Narani <mnarani@xilinx.com> > + * Rajnikant Bhojani <rajnikant.bhojani@xilinx.com> > + */ > + > +#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/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +#include <linux/iio/events.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > + > +static const unsigned int AMS_UNMASK_TIMEOUT_MS = 500; > + > +/* AMS registers definitions */ > +#define AMS_ISR_0 0x010 > +#define AMS_ISR_1 0x014 > +#define AMS_IER_0 0x020 > +#define AMS_IER_1 0x024 > +#define AMS_IDR_0 0x028 > +#define AMS_IDR_1 0x02c > +#define AMS_PS_CSTS 0x040 > +#define AMS_PL_CSTS 0x044 > + > +#define AMS_VCC_PSPLL0 0x060 > +#define AMS_VCC_PSPLL3 0x06C > +#define AMS_VCCINT 0x078 > +#define AMS_VCCBRAM 0x07C > +#define AMS_VCCAUX 0x080 > +#define AMS_PSDDRPLL 0x084 > +#define AMS_PSINTFPDDR 0x09C > + > +#define AMS_VCC_PSPLL0_CH 48 > +#define AMS_VCC_PSPLL3_CH 51 > +#define AMS_VCCINT_CH 54 > +#define AMS_VCCBRAM_CH 55 > +#define AMS_VCCAUX_CH 56 > +#define AMS_PSDDRPLL_CH 57 > +#define AMS_PSINTFPDDR_CH 63 > + > +#define AMS_REG_CONFIG0 0x100 > +#define AMS_REG_CONFIG1 0x104 > +#define AMS_REG_CONFIG3 0x10C > +#define AMS_REG_SEQ_CH0 0x120 > +#define AMS_REG_SEQ_CH1 0x124 > +#define AMS_REG_SEQ_CH2 0x118 > + > +#define AMS_TEMP 0x000 > +#define AMS_SUPPLY1 0x004 > +#define AMS_SUPPLY2 0x008 > +#define AMS_VP_VN 0x00c > +#define AMS_VREFP 0x010 > +#define AMS_VREFN 0x014 > +#define AMS_SUPPLY3 0x018 > +#define AMS_SUPPLY4 0x034 > +#define AMS_SUPPLY5 0x038 > +#define AMS_SUPPLY6 0x03c > +#define AMS_SUPPLY7 0x200 > +#define AMS_SUPPLY8 0x204 > +#define AMS_SUPPLY9 0x208 > +#define AMS_SUPPLY10 0x20c > +#define AMS_VCCAMS 0x210 > +#define AMS_TEMP_REMOTE 0x214 > + > +#define AMS_REG_VAUX(x) (0x40 + 4 * (x)) > + > +#define AMS_PS_RESET_VALUE 0xFFFF > +#define AMS_PL_RESET_VALUE 0xFFFF > + > +#define AMS_CONF0_CHANNEL_NUM_MASK GENMASK(6, 0) > + > +#define AMS_CONF1_SEQ_MASK GENMASK(15, 12) > +#define AMS_CONF1_SEQ_DEFAULT (0 << 12) > +#define AMS_CONF1_SEQ_CONTINUOUS (2 << 12) > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL (3 << 12) FIELD_PREP(AMS_CONFG1_SEQ_MASK, 0) or even better, define the values as not shifted and have FIELD_PREP(AMS_CONFIG1_SEQ_MASK, AMS_CONF1_SEQ_DEFAULT) etc in the relevant places inline. > + > +#define AMS_REG_SEQ0_MASK 0xFFFF > +#define AMS_REG_SEQ2_MASK 0x003F > +#define AMS_REG_SEQ1_MASK 0xFFFF > +#define AMS_REG_SEQ2_MASK_SHIFT 16 > +#define AMS_REG_SEQ1_MASK_SHIFT 22 As below, combine mask and shift into a shifted mask then you can use FIELD_PREP() to do the magic for you. > + > +#define AMS_REGCFG1_ALARM_MASK 0xF0F > +#define AMS_REGCFG3_ALARM_MASK 0x3F > + > +#define AMS_ALARM_TEMP 0x140 > +#define AMS_ALARM_SUPPLY1 0x144 > +#define AMS_ALARM_SUPPLY2 0x148 > +#define AMS_ALARM_SUPPLY3 0x160 > +#define AMS_ALARM_SUPPLY4 0x164 > +#define AMS_ALARM_SUPPLY5 0x168 > +#define AMS_ALARM_SUPPLY6 0x16c > +#define AMS_ALARM_SUPPLY7 0x180 > +#define AMS_ALARM_SUPPLY8 0x184 > +#define AMS_ALARM_SUPPLY9 0x188 > +#define AMS_ALARM_SUPPLY10 0x18c > +#define AMS_ALARM_VCCAMS 0x190 > +#define AMS_ALARM_TEMP_REMOTE 0x194 > +#define AMS_ALARM_THRESHOLD_OFF_10 0x10 > +#define AMS_ALARM_THRESHOLD_OFF_20 0x20 > + > +#define AMS_ALARM_THR_DIRECT_MASK 0x01 > +#define AMS_ALARM_THR_MIN 0x0000 > +#define AMS_ALARM_THR_MAX 0xffff > + > +#define AMS_NO_OF_ALARMS 32 > +#define AMS_PL_ALARM_START 16 > +#define AMS_ISR0_ALARM_MASK 0xFFFFFFFFU > +#define AMS_ISR1_ALARM_MASK 0xE000001FU > +#define AMS_ISR1_EOC_MASK 0x00000008U > +#define AMS_ISR1_INTR_MASK_SHIFT 32 > +#define AMS_ISR0_ALARM_2_TO_0_MASK 0x07 > +#define AMS_ISR0_ALARM_6_TO_3_MASK 0x78 > +#define AMS_ISR0_ALARM_12_TO_7_MASK 0x3F > +#define AMS_CONF1_ALARM_2_TO_0_SHIFT 1 Can we handle these via a single mask that includes the shift and FIELD_PREP() etc? That tends to make it easier to review by ensuring we don't have multiple defines to deal with. > +#define AMS_CONF1_ALARM_6_TO_3_SHIFT 5 > +#define AMS_CONF3_ALARM_12_TO_7_SHIFT 8 > + > +#define AMS_PS_CSTS_PS_READY 0x08010000U > +#define AMS_PL_CSTS_ACCESS_MASK 0x00000001U > + > +#define AMS_PL_MAX_FIXED_CHANNEL 10 > +#define AMS_PL_MAX_EXT_CHANNEL 20 > + > +#define AMS_INIT_TIMEOUT_US 10000 > + > +/* > + * Following scale and offset value is derived from > + * UG580 (v1.7) December 20, 2016 > + */ > +#define AMS_SUPPLY_SCALE_1VOLT 1000 > +#define AMS_SUPPLY_SCALE_3VOLT 3000 > +#define AMS_SUPPLY_SCALE_6VOLT 6000 > +#define AMS_SUPPLY_SCALE_DIV_BIT 16 > + > +#define AMS_TEMP_SCALE 509314 > +#define AMS_TEMP_SCALE_DIV_BIT 16 > +#define AMS_TEMP_OFFSET -((280230L << 16) / 509314) > + > +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 > +}; > + > +enum ams_seq { > + AMS_SEQ_VCC_PSPLL, > + AMS_SEQ_VCC_PSBATT, > + AMS_SEQ_VCCINT, > + AMS_SEQ_VCCBRAM, > + AMS_SEQ_VCCAUX, > + AMS_SEQ_PSDDRPLL, > + AMS_SEQ_INTDDR > +}; > + > +enum ams_ps_pl_seq { > + AMS_SEQ_CALIB, > + AMS_SEQ_RSVD_1, > + AMS_SEQ_RSVD_2, > + AMS_SEQ_TEST, > + AMS_SEQ_RSVD_4, > + AMS_SEQ_SUPPLY4, > + AMS_SEQ_SUPPLY5, > + AMS_SEQ_SUPPLY6, > + AMS_SEQ_TEMP, > + AMS_SEQ_SUPPLY2, > + AMS_SEQ_SUPPLY1, > + AMS_SEQ_VP_VN, > + AMS_SEQ_VREFP, > + AMS_SEQ_VREFN, > + AMS_SEQ_SUPPLY3, > + AMS_SEQ_CURRENT_MON, > + AMS_SEQ_SUPPLY7, > + AMS_SEQ_SUPPLY8, > + AMS_SEQ_SUPPLY9, > + AMS_SEQ_SUPPLY10, > + AMS_SEQ_VCCAMS, > + AMS_SEQ_TEMP_REMOTE, > + AMS_SEQ_MAX > +}; > + > +#define AMS_SEQ(x) (AMS_SEQ_MAX + (x)) > +#define AMS_VAUX_SEQ(x) (AMS_SEQ_MAX + (x)) > + > +#define AMS_PS_SEQ_MAX AMS_SEQ_MAX > +#define PS_SEQ(x) (x) > +#define PL_SEQ(x) (AMS_PS_SEQ_MAX + (x)) > + > +#define AMS_CHAN_TEMP(_scan_index, _addr) { \ > + .type = IIO_TEMP, \ > + .indexed = 1, \ > + .address = (_addr), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_OFFSET), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .event_spec = ams_temp_events, \ > + .num_event_specs = ARRAY_SIZE(ams_temp_events), \ > + .scan_index = (_scan_index), \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 12, \ > + .storagebits = 16, \ > + .shift = 4, \ > + .endianness = IIO_CPU, \ Currently these are only documentation i think as no support for using them in this driver (buffered modes). You could drop them until you need them in some future patch. > + }, \ > +} > + ... > +static int ams_enable_single_channel(struct ams *ams, unsigned int offset) > +{ > + u8 channel_num = 0; > + > + switch (offset) { > + case AMS_VCC_PSPLL0: > + channel_num = AMS_VCC_PSPLL0_CH; > + break; > + case AMS_VCC_PSPLL3: > + channel_num = AMS_VCC_PSPLL3_CH; > + break; > + case AMS_VCCINT: > + channel_num = AMS_VCCINT_CH; > + break; > + case AMS_VCCBRAM: > + channel_num = AMS_VCCBRAM_CH; > + break; > + case AMS_VCCAUX: > + channel_num = AMS_VCCAUX_CH; > + break; > + case AMS_PSDDRPLL: > + channel_num = AMS_PSDDRPLL_CH; > + break; > + case AMS_PSINTFPDDR: > + channel_num = AMS_PSINTFPDDR_CH; > + break; > + default: > + return -EINVAL; > + } > + > + /* set single channel, sequencer off mode */ > + ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK, > + AMS_CONF1_SEQ_SINGLE_CHANNEL); > + > + /* write the channel number */ > + ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK, > + channel_num); nitpick: Blank line here. > + return 0; > +} > + ... > +static int ams_get_ext_chan(struct device_node *chan_node, > + struct iio_chan_spec *channels, int num_channels) > +{ > + struct device_node *child; > + unsigned int reg; > + int ret; > + > + for_each_child_of_node(chan_node, child) { > + ret = of_property_read_u32(child, "reg", ®); > + if (ret || reg > (AMS_PL_MAX_EXT_CHANNEL + 30)) > + continue; > + > + memcpy(&channels[num_channels], &ams_pl_channels[reg + > + AMS_PL_MAX_FIXED_CHANNEL - 30], sizeof(*channels)); > + > + if (of_property_read_bool(child, > + "xlnx,bipolar")) Above fits on one line easily. > + channels[num_channels].scan_type.sign = 's'; > + > + num_channels++; > + } > + > + return num_channels; > +} > + ... > + > +static int ams_probe(struct platform_device *pdev) > +{ > + struct iio_dev *indio_dev; > + struct ams *ams; > + struct resource *res; > + int ret; > + > + if (!pdev->dev.of_node) > + return -ENODEV; > + > + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ams)); > + if (!indio_dev) > + return -ENOMEM; > + > + ams = iio_priv(indio_dev); > + mutex_init(&ams->lock); > + > + indio_dev->name = "xilinx-ams"; > + > + indio_dev->info = &iio_ams_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + ams->base = devm_ioremap_resource(&pdev->dev, res); devm_platform_ioremap_resource() Slight reduction in boilerplate. > + if (IS_ERR(ams->base)) > + return PTR_ERR(ams->base); > + > + ams->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(ams->clk)) > + return PTR_ERR(ams->clk); > + clk_prepare_enable(ams->clk); > + devm_add_action_or_reset(&pdev->dev, (void *)clk_disable_unprepare, > + ams->clk); > + > + INIT_DELAYED_WORK(&ams->ams_unmask_work, ams_unmask_worker); > + devm_add_action_or_reset(&pdev->dev, (void *)cancel_delayed_work, I'm not keen on casting away the function pointer type. Normally we'd just wrap it in a local function, to make it clear it was deliberate and avoid potential nasty problems if the signature of the function ever changes. It's 3 lines of boilerplate, but will give me warm fuzzy feelings! Same for the other case above. The fact this isn't done in exising kernel code make this particularly risky. > + &ams->ams_unmask_work); > + > + ret = ams_init_device(ams); > + if (ret) { > + dev_err(&pdev->dev, "failed to initialize AMS\n"); > + return ret; > + } > + > + ret = ams_parse_dt(indio_dev, pdev); > + if (ret) { > + dev_err(&pdev->dev, "failure in parsing DT\n"); > + return ret; > + } > + > + ams_enable_channel_sequence(indio_dev); > + > + ams->irq = platform_get_irq(pdev, 0); platform_get_irq () can return errors, in particular -EPROBE_DEFER so I'd check that and return before you call devm_request_irq() I'm not sure devm_request_irq() will not eat that error code. > + ret = devm_request_irq(&pdev->dev, ams->irq, &ams_irq, 0, "ams-irq", > + indio_dev); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to register interrupt\n"); > + return ret; > + } > + > + platform_set_drvdata(pdev, indio_dev); > + > + return iio_device_register(indio_dev); > +} > + > +static int ams_remove(struct platform_device *pdev) > +{ > + struct iio_dev *indio_dev = platform_get_drvdata(pdev); > + > + iio_device_unregister(indio_dev); If this is all you have in remove, then you can use devm_iio_device_register() in probe() and not need an remove() callback at all. > + > + return 0; > +} > + ... ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-07-04 18:31 ` Jonathan Cameron @ 2021-07-15 7:48 ` Anand Ashok Dumbre 2021-07-15 12:52 ` Jonathan Cameron 0 siblings, 1 reply; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-07-15 7:48 UTC (permalink / raw) To: Jonathan Cameron Cc: lars, linux-iio, git-dev, Michal Simek, pmeerw, devicetree, linux-kernel, Manish Narani > -----Original Message----- > From: Jonathan Cameron <jic23@kernel.org> > Sent: Sunday 4 July 2021 7:31 PM > To: Anand Ashok Dumbre <ANANDASH@xilinx.com> > Cc: lars@metafoo.de; linux-iio@vger.kernel.org; git-dev <git- > dev@xilinx.com>; Michal Simek <michals@xilinx.com>; > pmeerw@pmeerw.net; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Manish Narani <MNARANI@xilinx.com> > Subject: Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver > > On Thu, 24 Jun 2021 19:29:37 +0100 > Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> 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 external master. Out of these interface 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. > > > > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> Hi Jonathan, Thank you for the review. My comments inline. Thanks, Anand > > Hi Anand, > > A few comments inline from a fresh read. > Only significant one is that the use of separate masks and shifts differs from > how they are normally done in the kernel these days. > FIELD_PREP() and FIELD_GET() allow a single mask definition to be cleanly > used for both the shift and masking in a nice clean way. > > Thanks, > > Jonathan > > > --- > > drivers/iio/adc/Kconfig | 13 + > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/xilinx-ams.c | 1342 > > ++++++++++++++++++++++++++++++++++ > > 3 files changed, 1356 insertions(+) > > create mode 100644 drivers/iio/adc/xilinx-ams.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index > > c7946c439612..c011ab30ec9a 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -1256,4 +1256,17 @@ config XILINX_XADC > > The driver can also be build as a module. If so, the module will be > called > > xilinx-xadc. > > > > +config XILINX_AMS > > + tristate "Xilinx AMS driver" > > + depends on ARCH_ZYNQMP || COMPILE_TEST > > + depends on HAS_IOMEM > > + help > > + Say yes here to have support for the Xilinx AMS. > > + > > + The driver supports Voltage and Temperature monitoring on Xilinx > Ultrascale > > + devices. > > + > > + The driver can also be built as a module. If so, the module will be > called > > + xilinx-ams. > > + > > endmenu > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index > > a226657d19c0..99e031f44479 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -112,4 +112,5 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o > > obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o xilinx-xadc-y := > > xilinx-xadc-core.o xilinx-xadc-events.o > > obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o > > +obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o > > obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o diff --git > > a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c new file > > mode 100644 index 000000000000..463e3162726c > > --- /dev/null > > +++ b/drivers/iio/adc/xilinx-ams.c > > @@ -0,0 +1,1342 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Xilinx AMS driver > > + * > > + * Copyright (C) 2021 Xilinx, Inc. > > + * > > + * Manish Narani <mnarani@xilinx.com> > > + * Rajnikant Bhojani <rajnikant.bhojani@xilinx.com> */ > > + > > +#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/of_address.h> > > +#include <linux/platform_device.h> > > +#include <linux/slab.h> > > + > > +#include <linux/iio/events.h> > > +#include <linux/iio/iio.h> > > +#include <linux/iio/sysfs.h> > > + > > +static const unsigned int AMS_UNMASK_TIMEOUT_MS = 500; > > + > > +/* AMS registers definitions */ > > +#define AMS_ISR_0 0x010 > > +#define AMS_ISR_1 0x014 > > +#define AMS_IER_0 0x020 > > +#define AMS_IER_1 0x024 > > +#define AMS_IDR_0 0x028 > > +#define AMS_IDR_1 0x02c > > +#define AMS_PS_CSTS 0x040 > > +#define AMS_PL_CSTS 0x044 > > + > > +#define AMS_VCC_PSPLL0 0x060 > > +#define AMS_VCC_PSPLL3 0x06C > > +#define AMS_VCCINT 0x078 > > +#define AMS_VCCBRAM 0x07C > > +#define AMS_VCCAUX 0x080 > > +#define AMS_PSDDRPLL 0x084 > > +#define AMS_PSINTFPDDR 0x09C > > + > > +#define AMS_VCC_PSPLL0_CH 48 > > +#define AMS_VCC_PSPLL3_CH 51 > > +#define AMS_VCCINT_CH 54 > > +#define AMS_VCCBRAM_CH 55 > > +#define AMS_VCCAUX_CH 56 > > +#define AMS_PSDDRPLL_CH 57 > > +#define AMS_PSINTFPDDR_CH 63 > > + > > +#define AMS_REG_CONFIG0 0x100 > > +#define AMS_REG_CONFIG1 0x104 > > +#define AMS_REG_CONFIG3 0x10C > > +#define AMS_REG_SEQ_CH0 0x120 > > +#define AMS_REG_SEQ_CH1 0x124 > > +#define AMS_REG_SEQ_CH2 0x118 > > + > > +#define AMS_TEMP 0x000 > > +#define AMS_SUPPLY1 0x004 > > +#define AMS_SUPPLY2 0x008 > > +#define AMS_VP_VN 0x00c > > +#define AMS_VREFP 0x010 > > +#define AMS_VREFN 0x014 > > +#define AMS_SUPPLY3 0x018 > > +#define AMS_SUPPLY4 0x034 > > +#define AMS_SUPPLY5 0x038 > > +#define AMS_SUPPLY6 0x03c > > +#define AMS_SUPPLY7 0x200 > > +#define AMS_SUPPLY8 0x204 > > +#define AMS_SUPPLY9 0x208 > > +#define AMS_SUPPLY10 0x20c > > +#define AMS_VCCAMS 0x210 > > +#define AMS_TEMP_REMOTE 0x214 > > + > > +#define AMS_REG_VAUX(x) (0x40 + 4 * (x)) > > + > > +#define AMS_PS_RESET_VALUE 0xFFFF > > +#define AMS_PL_RESET_VALUE 0xFFFF > > + > > +#define AMS_CONF0_CHANNEL_NUM_MASK GENMASK(6, 0) > > + > > +#define AMS_CONF1_SEQ_MASK GENMASK(15, 12) > > +#define AMS_CONF1_SEQ_DEFAULT (0 << 12) > > +#define AMS_CONF1_SEQ_CONTINUOUS (2 << 12) > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL (3 << 12) > > FIELD_PREP(AMS_CONFG1_SEQ_MASK, 0) or even better, define the values > as not shifted and have > > FIELD_PREP(AMS_CONFIG1_SEQ_MASK, AMS_CONF1_SEQ_DEFAULT) etc in > the relevant places inline. > Will fix this in next series. > > > + > > +#define AMS_REG_SEQ0_MASK 0xFFFF > > +#define AMS_REG_SEQ2_MASK 0x003F > > +#define AMS_REG_SEQ1_MASK 0xFFFF > > +#define AMS_REG_SEQ2_MASK_SHIFT 16 > > +#define AMS_REG_SEQ1_MASK_SHIFT 22 > > As below, combine mask and shift into a shifted mask then you can use > FIELD_PREP() to do the magic for you. Will fix this in next series. > > > + > > +#define AMS_REGCFG1_ALARM_MASK 0xF0F > > +#define AMS_REGCFG3_ALARM_MASK 0x3F > > + > > +#define AMS_ALARM_TEMP 0x140 > > +#define AMS_ALARM_SUPPLY1 0x144 > > +#define AMS_ALARM_SUPPLY2 0x148 > > +#define AMS_ALARM_SUPPLY3 0x160 > > +#define AMS_ALARM_SUPPLY4 0x164 > > +#define AMS_ALARM_SUPPLY5 0x168 > > +#define AMS_ALARM_SUPPLY6 0x16c > > +#define AMS_ALARM_SUPPLY7 0x180 > > +#define AMS_ALARM_SUPPLY8 0x184 > > +#define AMS_ALARM_SUPPLY9 0x188 > > +#define AMS_ALARM_SUPPLY10 0x18c > > +#define AMS_ALARM_VCCAMS 0x190 > > +#define AMS_ALARM_TEMP_REMOTE 0x194 > > +#define AMS_ALARM_THRESHOLD_OFF_10 0x10 > > +#define AMS_ALARM_THRESHOLD_OFF_20 0x20 > > + > > +#define AMS_ALARM_THR_DIRECT_MASK 0x01 > > +#define AMS_ALARM_THR_MIN 0x0000 > > +#define AMS_ALARM_THR_MAX 0xffff > > + > > +#define AMS_NO_OF_ALARMS 32 > > +#define AMS_PL_ALARM_START 16 > > +#define AMS_ISR0_ALARM_MASK 0xFFFFFFFFU > > +#define AMS_ISR1_ALARM_MASK 0xE000001FU > > +#define AMS_ISR1_EOC_MASK 0x00000008U > > +#define AMS_ISR1_INTR_MASK_SHIFT 32 > > +#define AMS_ISR0_ALARM_2_TO_0_MASK 0x07 > > +#define AMS_ISR0_ALARM_6_TO_3_MASK 0x78 > > +#define AMS_ISR0_ALARM_12_TO_7_MASK 0x3F > > +#define AMS_CONF1_ALARM_2_TO_0_SHIFT 1 > > Can we handle these via a single mask that includes the shift and > FIELD_PREP() etc? That tends to make it easier to review by ensuring we > don't have multiple defines to deal with. > Will fix this in next series. > > +#define AMS_CONF1_ALARM_6_TO_3_SHIFT 5 > > +#define AMS_CONF3_ALARM_12_TO_7_SHIFT 8 > > > > + > > +#define AMS_PS_CSTS_PS_READY 0x08010000U > > +#define AMS_PL_CSTS_ACCESS_MASK 0x00000001U > > + > > +#define AMS_PL_MAX_FIXED_CHANNEL 10 > > +#define AMS_PL_MAX_EXT_CHANNEL 20 > > + > > +#define AMS_INIT_TIMEOUT_US 10000 > > + > > +/* > > + * Following scale and offset value is derived from > > + * UG580 (v1.7) December 20, 2016 > > + */ > > +#define AMS_SUPPLY_SCALE_1VOLT 1000 > > +#define AMS_SUPPLY_SCALE_3VOLT 3000 > > +#define AMS_SUPPLY_SCALE_6VOLT 6000 > > +#define AMS_SUPPLY_SCALE_DIV_BIT 16 > > + > > +#define AMS_TEMP_SCALE 509314 > > +#define AMS_TEMP_SCALE_DIV_BIT 16 > > +#define AMS_TEMP_OFFSET -((280230L << 16) / > 509314) > > + > > +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 > > +}; > > + > > +enum ams_seq { > > + AMS_SEQ_VCC_PSPLL, > > + AMS_SEQ_VCC_PSBATT, > > + AMS_SEQ_VCCINT, > > + AMS_SEQ_VCCBRAM, > > + AMS_SEQ_VCCAUX, > > + AMS_SEQ_PSDDRPLL, > > + AMS_SEQ_INTDDR > > +}; > > + > > +enum ams_ps_pl_seq { > > + AMS_SEQ_CALIB, > > + AMS_SEQ_RSVD_1, > > + AMS_SEQ_RSVD_2, > > + AMS_SEQ_TEST, > > + AMS_SEQ_RSVD_4, > > + AMS_SEQ_SUPPLY4, > > + AMS_SEQ_SUPPLY5, > > + AMS_SEQ_SUPPLY6, > > + AMS_SEQ_TEMP, > > + AMS_SEQ_SUPPLY2, > > + AMS_SEQ_SUPPLY1, > > + AMS_SEQ_VP_VN, > > + AMS_SEQ_VREFP, > > + AMS_SEQ_VREFN, > > + AMS_SEQ_SUPPLY3, > > + AMS_SEQ_CURRENT_MON, > > + AMS_SEQ_SUPPLY7, > > + AMS_SEQ_SUPPLY8, > > + AMS_SEQ_SUPPLY9, > > + AMS_SEQ_SUPPLY10, > > + AMS_SEQ_VCCAMS, > > + AMS_SEQ_TEMP_REMOTE, > > + AMS_SEQ_MAX > > +}; > > + > > +#define AMS_SEQ(x) (AMS_SEQ_MAX + (x)) > > +#define AMS_VAUX_SEQ(x) (AMS_SEQ_MAX + (x)) > > + > > +#define AMS_PS_SEQ_MAX AMS_SEQ_MAX > > +#define PS_SEQ(x) (x) > > +#define PL_SEQ(x) (AMS_PS_SEQ_MAX + (x)) > > + > > +#define AMS_CHAN_TEMP(_scan_index, _addr) { \ > > + .type = IIO_TEMP, \ > > + .indexed = 1, \ > > + .address = (_addr), \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > > + BIT(IIO_CHAN_INFO_SCALE) | \ > > + BIT(IIO_CHAN_INFO_OFFSET), \ > > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > > + .event_spec = ams_temp_events, \ > > + .num_event_specs = ARRAY_SIZE(ams_temp_events), \ > > + .scan_index = (_scan_index), \ > > + .scan_type = { \ > > + .sign = 'u', \ > > + .realbits = 12, \ > > + .storagebits = 16, \ > > + .shift = 4, \ > > + .endianness = IIO_CPU, \ > > Currently these are only documentation i think as no support for using them > in this driver (buffered modes). You could drop them until you need them in > some future patch. Makes sense, will remove the scan_type part, but will retain the scan index, since it is used in some logic inside the rest of the code. > > > + }, \ > > +} > > + > > ... > > > +static int ams_enable_single_channel(struct ams *ams, unsigned int > > +offset) { > > + u8 channel_num = 0; > > + > > + switch (offset) { > > + case AMS_VCC_PSPLL0: > > + channel_num = AMS_VCC_PSPLL0_CH; > > + break; > > + case AMS_VCC_PSPLL3: > > + channel_num = AMS_VCC_PSPLL3_CH; > > + break; > > + case AMS_VCCINT: > > + channel_num = AMS_VCCINT_CH; > > + break; > > + case AMS_VCCBRAM: > > + channel_num = AMS_VCCBRAM_CH; > > + break; > > + case AMS_VCCAUX: > > + channel_num = AMS_VCCAUX_CH; > > + break; > > + case AMS_PSDDRPLL: > > + channel_num = AMS_PSDDRPLL_CH; > > + break; > > + case AMS_PSINTFPDDR: > > + channel_num = AMS_PSINTFPDDR_CH; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + /* set single channel, sequencer off mode */ > > + ams_ps_update_reg(ams, AMS_REG_CONFIG1, > AMS_CONF1_SEQ_MASK, > > + AMS_CONF1_SEQ_SINGLE_CHANNEL); > > + > > + /* write the channel number */ > > + ams_ps_update_reg(ams, AMS_REG_CONFIG0, > AMS_CONF0_CHANNEL_NUM_MASK, > > + channel_num); > > nitpick: Blank line here. Will fix this in next series. > > > + return 0; > > +} > > + > > ... > > > +static int ams_get_ext_chan(struct device_node *chan_node, > > + struct iio_chan_spec *channels, int num_channels) > { > > + struct device_node *child; > > + unsigned int reg; > > + int ret; > > + > > + for_each_child_of_node(chan_node, child) { > > + ret = of_property_read_u32(child, "reg", ®); > > + if (ret || reg > (AMS_PL_MAX_EXT_CHANNEL + 30)) > > + continue; > > + > > + memcpy(&channels[num_channels], &ams_pl_channels[reg > + > > + AMS_PL_MAX_FIXED_CHANNEL - 30], sizeof(*channels)); > > + > > + if (of_property_read_bool(child, > > + "xlnx,bipolar")) > > Above fits on one line easily. Will fix this in next series. > > > + channels[num_channels].scan_type.sign = 's'; > > + > > + num_channels++; > > + } > > + > > + return num_channels; > > +} > > + > > ... > > > + > > +static int ams_probe(struct platform_device *pdev) { > > + struct iio_dev *indio_dev; > > + struct ams *ams; > > + struct resource *res; > > + int ret; > > + > > + if (!pdev->dev.of_node) > > + return -ENODEV; > > + > > + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ams)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + ams = iio_priv(indio_dev); > > + mutex_init(&ams->lock); > > + > > + indio_dev->name = "xilinx-ams"; > > + > > + indio_dev->info = &iio_ams_info; > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + ams->base = devm_ioremap_resource(&pdev->dev, res); > > devm_platform_ioremap_resource() > Slight reduction in boilerplate. Will fix this in next series. > > > + if (IS_ERR(ams->base)) > > + return PTR_ERR(ams->base); > > + > > + ams->clk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(ams->clk)) > > + return PTR_ERR(ams->clk); > > + clk_prepare_enable(ams->clk); > > + devm_add_action_or_reset(&pdev->dev, (void > *)clk_disable_unprepare, > > + ams->clk); > > + > > + INIT_DELAYED_WORK(&ams->ams_unmask_work, > ams_unmask_worker); > > + devm_add_action_or_reset(&pdev->dev, (void > *)cancel_delayed_work, > > I'm not keen on casting away the function pointer type. Normally we'd just > wrap it in a local function, to make it clear it was deliberate and avoid > potential nasty problems if the signature of the function ever changes. > > It's 3 lines of boilerplate, but will give me warm fuzzy feelings! > Same for the other case above. The fact this isn't done in exising kernel code > make this particularly risky. Makes sense. I will revert the code back to its original and handle the cases using goto and inside remove() > > > + &ams->ams_unmask_work); > > + > > + ret = ams_init_device(ams); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to initialize AMS\n"); > > + return ret; > > + } > > + > > + ret = ams_parse_dt(indio_dev, pdev); > > + if (ret) { > > + dev_err(&pdev->dev, "failure in parsing DT\n"); > > + return ret; > > + } > > + > > + ams_enable_channel_sequence(indio_dev); > > + > > + ams->irq = platform_get_irq(pdev, 0); > > platform_get_irq () can return errors, in particular -EPROBE_DEFER so I'd > check that and return before you call devm_request_irq() I'm not sure > devm_request_irq() will not eat that error code. > Will fix this in next series. > > > + ret = devm_request_irq(&pdev->dev, ams->irq, &ams_irq, 0, "ams- > irq", > > + indio_dev); > > + if (ret < 0) { > > + dev_err(&pdev->dev, "failed to register interrupt\n"); > > + return ret; > > + } > > + > > + platform_set_drvdata(pdev, indio_dev); > > + > > + return iio_device_register(indio_dev); } > > + > > +static int ams_remove(struct platform_device *pdev) { > > + struct iio_dev *indio_dev = platform_get_drvdata(pdev); > > + > > + iio_device_unregister(indio_dev); > > If this is all you have in remove, then you can use devm_iio_device_register() > in probe() and not need an remove() callback at all. I think remove will have more functions since I am getting rid of devm_add_action_or_reset() > > > + > > + return 0; > > +} > > + > > ... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-07-15 7:48 ` Anand Ashok Dumbre @ 2021-07-15 12:52 ` Jonathan Cameron 2021-07-19 7:49 ` Anand Ashok Dumbre 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Cameron @ 2021-07-15 12:52 UTC (permalink / raw) To: Anand Ashok Dumbre Cc: Jonathan Cameron, lars, linux-iio, git-dev, Michal Simek, pmeerw, devicetree, linux-kernel, Manish Narani ... > > > > > + if (IS_ERR(ams->base)) > > > + return PTR_ERR(ams->base); > > > + > > > + ams->clk = devm_clk_get(&pdev->dev, NULL); > > > + if (IS_ERR(ams->clk)) > > > + return PTR_ERR(ams->clk); > > > + clk_prepare_enable(ams->clk); > > > + devm_add_action_or_reset(&pdev->dev, (void > > *)clk_disable_unprepare, > > > + ams->clk); > > > + > > > + INIT_DELAYED_WORK(&ams->ams_unmask_work, > > ams_unmask_worker); > > > + devm_add_action_or_reset(&pdev->dev, (void > > *)cancel_delayed_work, > > > > I'm not keen on casting away the function pointer type. Normally we'd just > > wrap it in a local function, to make it clear it was deliberate and avoid > > potential nasty problems if the signature of the function ever changes. > > > > It's 3 lines of boilerplate, but will give me warm fuzzy feelings! > > Same for the other case above. The fact this isn't done in exising kernel code > > make this particularly risky. > > Makes sense. I will revert the code back to its original and handle the cases using goto > and inside remove() Ah. Not what I meant. I was suggesting you add a little function locally that has the right type and in turn calls cancel_delayed_work(). As that directly exposes the actual function calls, any signature change in future will cause compile breakage (or be picked up any automated tools doing that refactor). > > > > > > + &ams->ams_unmask_work); > > > + > > > + ret = ams_init_device(ams); > > > + if (ret) { > > > + dev_err(&pdev->dev, "failed to initialize AMS\n"); > > > + return ret; > > > + } > > > + > > > + ret = ams_parse_dt(indio_dev, pdev); > > > + if (ret) { > > > + dev_err(&pdev->dev, "failure in parsing DT\n"); > > > + return ret; > > > + } > > > + > > > + ams_enable_channel_sequence(indio_dev); > > > + > > > + ams->irq = platform_get_irq(pdev, 0); > > > > platform_get_irq () can return errors, in particular -EPROBE_DEFER so I'd > > check that and return before you call devm_request_irq() I'm not sure > > devm_request_irq() will not eat that error code. > > > > Will fix this in next series. > > > > > > + ret = devm_request_irq(&pdev->dev, ams->irq, &ams_irq, 0, "ams- > > irq", > > > + indio_dev); > > > + if (ret < 0) { > > > + dev_err(&pdev->dev, "failed to register interrupt\n"); > > > + return ret; > > > + } > > > + > > > + platform_set_drvdata(pdev, indio_dev); > > > + > > > + return iio_device_register(indio_dev); } > > > + > > > +static int ams_remove(struct platform_device *pdev) { > > > + struct iio_dev *indio_dev = platform_get_drvdata(pdev); > > > + > > > + iio_device_unregister(indio_dev); > > > > If this is all you have in remove, then you can use devm_iio_device_register() > > in probe() and not need an remove() callback at all. > > I think remove will have more functions since I am getting rid of devm_add_action_or_reset() See above. J > > > > > > + > > > + return 0; > > > +} > > > + > > > > ... ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver 2021-07-15 12:52 ` Jonathan Cameron @ 2021-07-19 7:49 ` Anand Ashok Dumbre 0 siblings, 0 replies; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-07-19 7:49 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, lars, linux-iio, Michal Simek, pmeerw, devicetree, linux-kernel, Manish Narani, git > -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Sent: Thursday 15 July 2021 1:52 PM > To: Anand Ashok Dumbre <ANANDASH@xilinx.com> > Cc: Jonathan Cameron <jic23@kernel.org>; lars@metafoo.de; linux- > iio@vger.kernel.org; git-dev <git-dev@xilinx.com>; Michal Simek > <michals@xilinx.com>; pmeerw@pmeerw.net; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; Manish Narani <MNARANI@xilinx.com> > Subject: Re: [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver > > ... > > > > > > > > + if (IS_ERR(ams->base)) > > > > + return PTR_ERR(ams->base); > > > > + > > > > + ams->clk = devm_clk_get(&pdev->dev, NULL); > > > > + if (IS_ERR(ams->clk)) > > > > + return PTR_ERR(ams->clk); > > > > + clk_prepare_enable(ams->clk); > > > > + devm_add_action_or_reset(&pdev->dev, (void > > > *)clk_disable_unprepare, > > > > + ams->clk); > > > > + > > > > + INIT_DELAYED_WORK(&ams->ams_unmask_work, > > > ams_unmask_worker); > > > > + devm_add_action_or_reset(&pdev->dev, (void > > > *)cancel_delayed_work, > > > > > > I'm not keen on casting away the function pointer type. Normally > > > we'd just wrap it in a local function, to make it clear it was > > > deliberate and avoid potential nasty problems if the signature of the > function ever changes. > > > > > > It's 3 lines of boilerplate, but will give me warm fuzzy feelings! > > > Same for the other case above. The fact this isn't done in exising > > > kernel code make this particularly risky. > > > > Makes sense. I will revert the code back to its original and handle > > the cases using goto and inside remove() > Ah. Not what I meant. I was suggesting you add a little function locally that > has the right type and in turn calls cancel_delayed_work(). > > As that directly exposes the actual function calls, any signature change in > future will cause compile breakage (or be picked up any automated tools > doing that refactor). Now I understand. Will fix it in the next series. > > > > > > > > > > + &ams->ams_unmask_work); > > > > + > > > > + ret = ams_init_device(ams); > > > > + if (ret) { > > > > + dev_err(&pdev->dev, "failed to initialize AMS\n"); > > > > + return ret; > > > > + } > > > > + > > > > + ret = ams_parse_dt(indio_dev, pdev); > > > > + if (ret) { > > > > + dev_err(&pdev->dev, "failure in parsing DT\n"); > > > > + return ret; > > > > + } > > > > + > > > > + ams_enable_channel_sequence(indio_dev); > > > > + > > > > + ams->irq = platform_get_irq(pdev, 0); > > > > > > platform_get_irq () can return errors, in particular -EPROBE_DEFER > > > so I'd check that and return before you call devm_request_irq() I'm > > > not sure > > > devm_request_irq() will not eat that error code. > > > > > > > Will fix this in next series. > > > > > > > > > + ret = devm_request_irq(&pdev->dev, ams->irq, &ams_irq, 0, "ams- > > > irq", > > > > + indio_dev); > > > > + if (ret < 0) { > > > > + dev_err(&pdev->dev, "failed to register interrupt\n"); > > > > + return ret; > > > > + } > > > > + > > > > + platform_set_drvdata(pdev, indio_dev); > > > > + > > > > + return iio_device_register(indio_dev); } > > > > + > > > > +static int ams_remove(struct platform_device *pdev) { > > > > + struct iio_dev *indio_dev = platform_get_drvdata(pdev); > > > > + > > > > + iio_device_unregister(indio_dev); > > > > > > If this is all you have in remove, then you can use > > > devm_iio_device_register() in probe() and not need an remove() callback > at all. > > > > I think remove will have more functions since I am getting rid of > > devm_add_action_or_reset() > > See above. > > J > > > > > > > > > > + > > > > + return 0; > > > > +} > > > > + > > > > > > ... Thanks, Anand ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation 2021-06-24 18:29 [PATCH v6 0/4] Add Xilinx AMS Driver Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 1/4] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre @ 2021-06-24 18:29 ` Anand Ashok Dumbre 2021-07-04 18:08 ` Jonathan Cameron 2021-07-13 22:31 ` Rob Herring 2021-06-24 18:29 ` [PATCH v6 4/4] MAINTAINERS: Add maintainer for xilinx-ams Anand Ashok Dumbre 3 siblings, 2 replies; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-06-24 18:29 UTC (permalink / raw) To: jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: Anand Ashok Dumbre Xilinx AMS have several ADC channels that can be used for measurement of different voltages and temperatures. Document the same in the bindings. Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> --- .../bindings/iio/adc/xlnx,zynqmp-ams.yaml | 228 ++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml diff --git a/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml new file mode 100644 index 000000000000..a065ddd55d38 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml @@ -0,0 +1,228 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/xlnx,zynqmp-ams.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Xilinx Zynq Ultrascale AMS controller + +maintainers: + - Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> + +description: | + The AMS (Analog Monitoring System) 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 which are PL (Programmable Logic) SYSMON and + PS (Processing System) SYSMON. + All designs should have AMS registers, but PS and PL are optional. The + AMS controller can work with only PS, only PL and both PS and PL + configurations. Please specify registers according to your design. Devicetree + should always have AMS module property. Providing PS & PL module is optional. + + AMS Channel Details + ``````````````````` + Sysmon Block |Channel| Details |Measurement + |Number | |Type + --------------------------------------------------------------------------------------------------------- + AMS CTRL |0 |System PLLs voltage measurement, VCC_PSPLL. |Voltage + |1 |Battery voltage measurement, VCC_PSBATT. |Voltage + |2 |PL Internal voltage measurement, VCCINT. |Voltage + |3 |Block RAM voltage measurement, VCCBRAM. |Voltage + |4 |PL Aux voltage measurement, VCCAUX. |Voltage + |5 |Voltage measurement for six DDR I/O PLLs, VCC_PSDDR_PLL. |Voltage + |6 |VCC_PSINTFP_DDR voltage measurement. |Voltage + --------------------------------------------------------------------------------------------------------- + PS Sysmon |7 |LPD temperature measurement. |Temperature + |8 |FPD temperature measurement (REMOTE). |Temperature + |9 |VCC PS LPD voltage measurement (supply1). |Voltage + |10 |VCC PS FPD voltage measurement (supply2). |Voltage + |11 |PS Aux voltage reference (supply3). |Voltage + |12 |DDR I/O VCC voltage measurement. |Voltage + |13 |PS IO Bank 503 voltage measurement (supply5). |Voltage + |14 |PS IO Bank 500 voltage measurement (supply6). |Voltage + |15 |VCCO_PSIO1 voltage measurement. |Voltage + |16 |VCCO_PSIO2 voltage measurement. |Voltage + |17 |VCC_PS_GTR voltage measurement (VPS_MGTRAVCC). |Voltage + |18 |VTT_PS_GTR voltage measurement (VPS_MGTRAVTT). |Voltage + |19 |VCC_PSADC voltage measurement. |Voltage + --------------------------------------------------------------------------------------------------------- + PL Sysmon |20 |PL temperature measurement. |Temperature + |21 |PL Internal voltage measurement, VCCINT. |Voltage + |22 |PL Auxiliary voltage measurement, VCCAUX. |Voltage + |23 |ADC Reference P+ voltage measurement. |Voltage + |24 |ADC Reference N- voltage measurement. |Voltage + |25 |PL Block RAM voltage measurement, VCCBRAM. |Voltage + |26 |LPD Internal voltage measurement, VCC_PSINTLP (supply4). |Voltage + |27 |FPD Internal voltage measurement, VCC_PSINTFP (supply5). |Voltage + |28 |PS Auxiliary voltage measurement (supply6). |Voltage + |29 |PL VCCADC voltage measurement (vccams). |Voltage + |30 |Differential analog input signal voltage measurment. |Voltage + |31 |VUser0 voltage measurement (supply7). |Voltage + |32 |VUser1 voltage measurement (supply8). |Voltage + |33 |VUser2 voltage measurement (supply9). |Voltage + |34 |VUser3 voltage measurement (supply10). |Voltage + |35 |Auxiliary ch 0 voltage measurement (VAux0). |Voltage + |36 |Auxiliary ch 1 voltage measurement (VAux1). |Voltage + |37 |Auxiliary ch 2 voltage measurement (VAux2). |Voltage + |38 |Auxiliary ch 3 voltage measurement (VAux3). |Voltage + |39 |Auxiliary ch 4 voltage measurement (VAux4). |Voltage + |40 |Auxiliary ch 5 voltage measurement (VAux5). |Voltage + |41 |Auxiliary ch 6 voltage measurement (VAux6). |Voltage + |42 |Auxiliary ch 7 voltage measurement (VAux7). |Voltage + |43 |Auxiliary ch 8 voltage measurement (VAux8). |Voltage + |44 |Auxiliary ch 9 voltage measurement (VAux9). |Voltage + |45 |Auxiliary ch 10 voltage measurement (VAux10). |Voltage + |46 |Auxiliary ch 11 voltage measurement (VAux11). |Voltage + |47 |Auxiliary ch 12 voltage measurement (VAux12). |Voltage + |48 |Auxiliary ch 13 voltage measurement (VAux13). |Voltage + |49 |Auxiliary ch 14 voltage measurement (VAux14). |Voltage + |50 |Auxiliary ch 15 voltage measurement (VAux15). |Voltage + -------------------------------------------------------------------------------------------------------- + +properties: + compatible: + enum: + - xlnx,zynqmp-ams + + interrupts: + maxItems: 1 + + reg: + description: AMS Controller register space + maxItems: 1 + + ranges: + description: + Maps the child address space for PS and/or PL. + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + + '#io-channel-cells': + const: 1 + +patternProperties: + "^ams-ps@0,400$": + type: object + description: | + PS (Processing System) SYSMON is memory mapped to PS. This block has + built-in alarm generation logic that is used to interrupt the processor + based on condition set. + + properties: + compatible: + enum: + - xlnx,zynqmp-ams-ps + + reg: + description: Register Space for PS-SYSMON + maxItems: 1 + + required: + - compatible + - reg + + additionalProperties: false + + "^ams-pl@400,400$": + type: object + description: + PL-SYSMON is capable of monitoring off chip voltage and temperature. + PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring + from external master. Out of this interface currently only DRP is + supported. This block has alarm generation logic that is used to + interrupt the processor based on condition set. + + properties: + compatible: + items: + - enum: + - xlnx,zynqmp-ams-pl + + reg: + description: Register Space for PL-SYSMON. + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + patternProperties: + "^channel@([2-4][0-9]|50)$": + type: object + description: + Describes the external channels connected. + + properties: + reg: + description: + Pair of pins the channel is connected to. This value is + same as Channel Number for a particular channel. + minimum: 20 + maximum: 50 + + xlnx,bipolar: + $ref: /schemas/types.yaml#/definitions/flag + type: boolean + description: + If the set channel is used in bipolar mode. + + required: + - reg + + additionalProperties: false + +required: + - compatible + - reg + - ranges + +additionalProperties: false + +examples: + - | + bus { + #address-cells = <2>; + #size-cells = <2>; + + xilinx_ams: ams@ffa50000 { + compatible = "xlnx,zynqmp-ams"; + interrupt-parent = <&gic>; + interrupts = <0 56 4>; + reg = <0x0 0xffa50000 0x0 0x800>; + #address-cells = <1>; + #size-cells = <1>; + #io-channel-cells = <1>; + ranges = <0 0 0xffa50800 0x800>; + + ams_ps: ams-ps@0,400 { + compatible = "xlnx,zynqmp-ams-ps"; + reg = <0 0x400>; + }; + + ams_pl: ams-pl@400,400 { + compatible = "xlnx,zynqmp-ams-pl"; + reg = <0x400 0x400>; + #address-cells = <1>; + #size-cells = <0>; + channel@30 { + reg = <30>; + xlnx,bipolar; + }; + channel@31 { + reg = <31>; + }; + channel@38 { + reg = <38>; + xlnx,bipolar; + }; + }; + }; + }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation 2021-06-24 18:29 ` [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre @ 2021-07-04 18:08 ` Jonathan Cameron 2021-07-13 22:31 ` Rob Herring 1 sibling, 0 replies; 19+ messages in thread From: Jonathan Cameron @ 2021-07-04 18:08 UTC (permalink / raw) To: Anand Ashok Dumbre Cc: lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel, Rob Herring On Thu, 24 Jun 2021 19:29:38 +0100 Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> wrote: > Xilinx AMS have several ADC channels that can be used for measurement of > different voltages and temperatures. Document the same in the bindings. > > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> Definitely some fiddly bits in here, but it looks fine to me. It's odd enough that I'll be relying on Rob taking a look though :) > --- > .../bindings/iio/adc/xlnx,zynqmp-ams.yaml | 228 ++++++++++++++++++ > 1 file changed, 228 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > new file mode 100644 > index 000000000000..a065ddd55d38 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > @@ -0,0 +1,228 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/xlnx,zynqmp-ams.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Xilinx Zynq Ultrascale AMS controller > + > +maintainers: > + - Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > + > +description: | > + The AMS (Analog Monitoring System) 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 which are PL (Programmable Logic) SYSMON and > + PS (Processing System) SYSMON. > + All designs should have AMS registers, but PS and PL are optional. The > + AMS controller can work with only PS, only PL and both PS and PL > + configurations. Please specify registers according to your design. Devicetree > + should always have AMS module property. Providing PS & PL module is optional. > + > + AMS Channel Details > + ``````````````````` > + Sysmon Block |Channel| Details |Measurement > + |Number | |Type > + --------------------------------------------------------------------------------------------------------- > + AMS CTRL |0 |System PLLs voltage measurement, VCC_PSPLL. |Voltage > + |1 |Battery voltage measurement, VCC_PSBATT. |Voltage > + |2 |PL Internal voltage measurement, VCCINT. |Voltage > + |3 |Block RAM voltage measurement, VCCBRAM. |Voltage > + |4 |PL Aux voltage measurement, VCCAUX. |Voltage > + |5 |Voltage measurement for six DDR I/O PLLs, VCC_PSDDR_PLL. |Voltage > + |6 |VCC_PSINTFP_DDR voltage measurement. |Voltage > + --------------------------------------------------------------------------------------------------------- > + PS Sysmon |7 |LPD temperature measurement. |Temperature > + |8 |FPD temperature measurement (REMOTE). |Temperature > + |9 |VCC PS LPD voltage measurement (supply1). |Voltage > + |10 |VCC PS FPD voltage measurement (supply2). |Voltage > + |11 |PS Aux voltage reference (supply3). |Voltage > + |12 |DDR I/O VCC voltage measurement. |Voltage > + |13 |PS IO Bank 503 voltage measurement (supply5). |Voltage > + |14 |PS IO Bank 500 voltage measurement (supply6). |Voltage > + |15 |VCCO_PSIO1 voltage measurement. |Voltage > + |16 |VCCO_PSIO2 voltage measurement. |Voltage > + |17 |VCC_PS_GTR voltage measurement (VPS_MGTRAVCC). |Voltage > + |18 |VTT_PS_GTR voltage measurement (VPS_MGTRAVTT). |Voltage > + |19 |VCC_PSADC voltage measurement. |Voltage > + --------------------------------------------------------------------------------------------------------- > + PL Sysmon |20 |PL temperature measurement. |Temperature > + |21 |PL Internal voltage measurement, VCCINT. |Voltage > + |22 |PL Auxiliary voltage measurement, VCCAUX. |Voltage > + |23 |ADC Reference P+ voltage measurement. |Voltage > + |24 |ADC Reference N- voltage measurement. |Voltage > + |25 |PL Block RAM voltage measurement, VCCBRAM. |Voltage > + |26 |LPD Internal voltage measurement, VCC_PSINTLP (supply4). |Voltage > + |27 |FPD Internal voltage measurement, VCC_PSINTFP (supply5). |Voltage > + |28 |PS Auxiliary voltage measurement (supply6). |Voltage > + |29 |PL VCCADC voltage measurement (vccams). |Voltage > + |30 |Differential analog input signal voltage measurment. |Voltage > + |31 |VUser0 voltage measurement (supply7). |Voltage > + |32 |VUser1 voltage measurement (supply8). |Voltage > + |33 |VUser2 voltage measurement (supply9). |Voltage > + |34 |VUser3 voltage measurement (supply10). |Voltage > + |35 |Auxiliary ch 0 voltage measurement (VAux0). |Voltage > + |36 |Auxiliary ch 1 voltage measurement (VAux1). |Voltage > + |37 |Auxiliary ch 2 voltage measurement (VAux2). |Voltage > + |38 |Auxiliary ch 3 voltage measurement (VAux3). |Voltage > + |39 |Auxiliary ch 4 voltage measurement (VAux4). |Voltage > + |40 |Auxiliary ch 5 voltage measurement (VAux5). |Voltage > + |41 |Auxiliary ch 6 voltage measurement (VAux6). |Voltage > + |42 |Auxiliary ch 7 voltage measurement (VAux7). |Voltage > + |43 |Auxiliary ch 8 voltage measurement (VAux8). |Voltage > + |44 |Auxiliary ch 9 voltage measurement (VAux9). |Voltage > + |45 |Auxiliary ch 10 voltage measurement (VAux10). |Voltage > + |46 |Auxiliary ch 11 voltage measurement (VAux11). |Voltage > + |47 |Auxiliary ch 12 voltage measurement (VAux12). |Voltage > + |48 |Auxiliary ch 13 voltage measurement (VAux13). |Voltage > + |49 |Auxiliary ch 14 voltage measurement (VAux14). |Voltage > + |50 |Auxiliary ch 15 voltage measurement (VAux15). |Voltage > + -------------------------------------------------------------------------------------------------------- > + > +properties: > + compatible: > + enum: > + - xlnx,zynqmp-ams > + > + interrupts: > + maxItems: 1 > + > + reg: > + description: AMS Controller register space > + maxItems: 1 > + > + ranges: > + description: > + Maps the child address space for PS and/or PL. > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + '#io-channel-cells': > + const: 1 > + > +patternProperties: > + "^ams-ps@0,400$": > + type: object > + description: | > + PS (Processing System) SYSMON is memory mapped to PS. This block has > + built-in alarm generation logic that is used to interrupt the processor > + based on condition set. > + > + properties: > + compatible: > + enum: > + - xlnx,zynqmp-ams-ps > + > + reg: > + description: Register Space for PS-SYSMON > + maxItems: 1 > + > + required: > + - compatible > + - reg > + > + additionalProperties: false > + > + "^ams-pl@400,400$": > + type: object > + description: > + PL-SYSMON is capable of monitoring off chip voltage and temperature. > + PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring > + from external master. Out of this interface currently only DRP is > + supported. This block has alarm generation logic that is used to > + interrupt the processor based on condition set. > + > + properties: > + compatible: > + items: > + - enum: > + - xlnx,zynqmp-ams-pl > + > + reg: > + description: Register Space for PL-SYSMON. > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + patternProperties: > + "^channel@([2-4][0-9]|50)$": > + type: object > + description: > + Describes the external channels connected. > + > + properties: > + reg: > + description: > + Pair of pins the channel is connected to. This value is > + same as Channel Number for a particular channel. > + minimum: 20 > + maximum: 50 > + > + xlnx,bipolar: > + $ref: /schemas/types.yaml#/definitions/flag > + type: boolean > + description: > + If the set channel is used in bipolar mode. > + > + required: > + - reg > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - ranges > + > +additionalProperties: false > + > +examples: > + - | > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + xilinx_ams: ams@ffa50000 { > + compatible = "xlnx,zynqmp-ams"; > + interrupt-parent = <&gic>; > + interrupts = <0 56 4>; > + reg = <0x0 0xffa50000 0x0 0x800>; > + #address-cells = <1>; > + #size-cells = <1>; > + #io-channel-cells = <1>; > + ranges = <0 0 0xffa50800 0x800>; > + > + ams_ps: ams-ps@0,400 { > + compatible = "xlnx,zynqmp-ams-ps"; > + reg = <0 0x400>; > + }; > + > + ams_pl: ams-pl@400,400 { > + compatible = "xlnx,zynqmp-ams-pl"; > + reg = <0x400 0x400>; > + #address-cells = <1>; > + #size-cells = <0>; > + channel@30 { > + reg = <30>; > + xlnx,bipolar; > + }; > + channel@31 { > + reg = <31>; > + }; > + channel@38 { > + reg = <38>; > + xlnx,bipolar; > + }; > + }; > + }; > + }; ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation 2021-06-24 18:29 ` [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre 2021-07-04 18:08 ` Jonathan Cameron @ 2021-07-13 22:31 ` Rob Herring 2021-07-15 6:58 ` Anand Ashok Dumbre 1 sibling, 1 reply; 19+ messages in thread From: Rob Herring @ 2021-07-13 22:31 UTC (permalink / raw) To: Anand Ashok Dumbre Cc: jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel On Thu, Jun 24, 2021 at 07:29:38PM +0100, Anand Ashok Dumbre wrote: > Xilinx AMS have several ADC channels that can be used for measurement of > different voltages and temperatures. Document the same in the bindings. > > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > --- > .../bindings/iio/adc/xlnx,zynqmp-ams.yaml | 228 ++++++++++++++++++ > 1 file changed, 228 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > new file mode 100644 > index 000000000000..a065ddd55d38 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > @@ -0,0 +1,228 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/xlnx,zynqmp-ams.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Xilinx Zynq Ultrascale AMS controller > + > +maintainers: > + - Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > + > +description: | > + The AMS (Analog Monitoring System) 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 which are PL (Programmable Logic) SYSMON and > + PS (Processing System) SYSMON. > + All designs should have AMS registers, but PS and PL are optional. The > + AMS controller can work with only PS, only PL and both PS and PL > + configurations. Please specify registers according to your design. Devicetree > + should always have AMS module property. Providing PS & PL module is optional. > + > + AMS Channel Details > + ``````````````````` > + Sysmon Block |Channel| Details |Measurement > + |Number | |Type > + --------------------------------------------------------------------------------------------------------- > + AMS CTRL |0 |System PLLs voltage measurement, VCC_PSPLL. |Voltage > + |1 |Battery voltage measurement, VCC_PSBATT. |Voltage > + |2 |PL Internal voltage measurement, VCCINT. |Voltage > + |3 |Block RAM voltage measurement, VCCBRAM. |Voltage > + |4 |PL Aux voltage measurement, VCCAUX. |Voltage > + |5 |Voltage measurement for six DDR I/O PLLs, VCC_PSDDR_PLL. |Voltage > + |6 |VCC_PSINTFP_DDR voltage measurement. |Voltage > + --------------------------------------------------------------------------------------------------------- > + PS Sysmon |7 |LPD temperature measurement. |Temperature > + |8 |FPD temperature measurement (REMOTE). |Temperature > + |9 |VCC PS LPD voltage measurement (supply1). |Voltage > + |10 |VCC PS FPD voltage measurement (supply2). |Voltage > + |11 |PS Aux voltage reference (supply3). |Voltage > + |12 |DDR I/O VCC voltage measurement. |Voltage > + |13 |PS IO Bank 503 voltage measurement (supply5). |Voltage > + |14 |PS IO Bank 500 voltage measurement (supply6). |Voltage > + |15 |VCCO_PSIO1 voltage measurement. |Voltage > + |16 |VCCO_PSIO2 voltage measurement. |Voltage > + |17 |VCC_PS_GTR voltage measurement (VPS_MGTRAVCC). |Voltage > + |18 |VTT_PS_GTR voltage measurement (VPS_MGTRAVTT). |Voltage > + |19 |VCC_PSADC voltage measurement. |Voltage > + --------------------------------------------------------------------------------------------------------- > + PL Sysmon |20 |PL temperature measurement. |Temperature > + |21 |PL Internal voltage measurement, VCCINT. |Voltage > + |22 |PL Auxiliary voltage measurement, VCCAUX. |Voltage > + |23 |ADC Reference P+ voltage measurement. |Voltage > + |24 |ADC Reference N- voltage measurement. |Voltage > + |25 |PL Block RAM voltage measurement, VCCBRAM. |Voltage > + |26 |LPD Internal voltage measurement, VCC_PSINTLP (supply4). |Voltage > + |27 |FPD Internal voltage measurement, VCC_PSINTFP (supply5). |Voltage > + |28 |PS Auxiliary voltage measurement (supply6). |Voltage > + |29 |PL VCCADC voltage measurement (vccams). |Voltage > + |30 |Differential analog input signal voltage measurment. |Voltage > + |31 |VUser0 voltage measurement (supply7). |Voltage > + |32 |VUser1 voltage measurement (supply8). |Voltage > + |33 |VUser2 voltage measurement (supply9). |Voltage > + |34 |VUser3 voltage measurement (supply10). |Voltage > + |35 |Auxiliary ch 0 voltage measurement (VAux0). |Voltage > + |36 |Auxiliary ch 1 voltage measurement (VAux1). |Voltage > + |37 |Auxiliary ch 2 voltage measurement (VAux2). |Voltage > + |38 |Auxiliary ch 3 voltage measurement (VAux3). |Voltage > + |39 |Auxiliary ch 4 voltage measurement (VAux4). |Voltage > + |40 |Auxiliary ch 5 voltage measurement (VAux5). |Voltage > + |41 |Auxiliary ch 6 voltage measurement (VAux6). |Voltage > + |42 |Auxiliary ch 7 voltage measurement (VAux7). |Voltage > + |43 |Auxiliary ch 8 voltage measurement (VAux8). |Voltage > + |44 |Auxiliary ch 9 voltage measurement (VAux9). |Voltage > + |45 |Auxiliary ch 10 voltage measurement (VAux10). |Voltage > + |46 |Auxiliary ch 11 voltage measurement (VAux11). |Voltage > + |47 |Auxiliary ch 12 voltage measurement (VAux12). |Voltage > + |48 |Auxiliary ch 13 voltage measurement (VAux13). |Voltage > + |49 |Auxiliary ch 14 voltage measurement (VAux14). |Voltage > + |50 |Auxiliary ch 15 voltage measurement (VAux15). |Voltage > + -------------------------------------------------------------------------------------------------------- > + > +properties: > + compatible: > + enum: > + - xlnx,zynqmp-ams > + > + interrupts: > + maxItems: 1 > + > + reg: > + description: AMS Controller register space > + maxItems: 1 > + > + ranges: > + description: > + Maps the child address space for PS and/or PL. > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + '#io-channel-cells': > + const: 1 > + > +patternProperties: > + "^ams-ps@0,400$": If your going to hardcode the unit-address, then it's a fixed string and should be under 'properties' instead. The unit-address is also wrong. This is memory-mapped ranges, right? If so, this should just be '0' and the next one '400'. > + type: object > + description: | > + PS (Processing System) SYSMON is memory mapped to PS. This block has > + built-in alarm generation logic that is used to interrupt the processor > + based on condition set. > + > + properties: > + compatible: > + enum: > + - xlnx,zynqmp-ams-ps > + > + reg: > + description: Register Space for PS-SYSMON > + maxItems: 1 > + > + required: > + - compatible > + - reg > + > + additionalProperties: false > + > + "^ams-pl@400,400$": > + type: object > + description: > + PL-SYSMON is capable of monitoring off chip voltage and temperature. > + PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring > + from external master. Out of this interface currently only DRP is > + supported. This block has alarm generation logic that is used to > + interrupt the processor based on condition set. > + > + properties: > + compatible: > + items: > + - enum: > + - xlnx,zynqmp-ams-pl > + > + reg: > + description: Register Space for PL-SYSMON. > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + patternProperties: > + "^channel@([2-4][0-9]|50)$": > + type: object > + description: > + Describes the external channels connected. > + > + properties: > + reg: > + description: > + Pair of pins the channel is connected to. This value is > + same as Channel Number for a particular channel. > + minimum: 20 > + maximum: 50 > + > + xlnx,bipolar: Don't we have a common property for this now? > + $ref: /schemas/types.yaml#/definitions/flag > + type: boolean > + description: > + If the set channel is used in bipolar mode. > + > + required: > + - reg > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - ranges > + > +additionalProperties: false > + > +examples: > + - | > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + xilinx_ams: ams@ffa50000 { > + compatible = "xlnx,zynqmp-ams"; > + interrupt-parent = <&gic>; > + interrupts = <0 56 4>; > + reg = <0x0 0xffa50000 0x0 0x800>; > + #address-cells = <1>; > + #size-cells = <1>; > + #io-channel-cells = <1>; > + ranges = <0 0 0xffa50800 0x800>; > + > + ams_ps: ams-ps@0,400 { > + compatible = "xlnx,zynqmp-ams-ps"; > + reg = <0 0x400>; > + }; > + > + ams_pl: ams-pl@400,400 { > + compatible = "xlnx,zynqmp-ams-pl"; > + reg = <0x400 0x400>; > + #address-cells = <1>; > + #size-cells = <0>; > + channel@30 { > + reg = <30>; > + xlnx,bipolar; > + }; > + channel@31 { > + reg = <31>; > + }; > + channel@38 { > + reg = <38>; > + xlnx,bipolar; > + }; > + }; > + }; > + }; > -- > 2.17.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation 2021-07-13 22:31 ` Rob Herring @ 2021-07-15 6:58 ` Anand Ashok Dumbre 0 siblings, 0 replies; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-07-15 6:58 UTC (permalink / raw) To: Rob Herring Cc: jic23, lars, linux-iio, git-dev, Michal Simek, pmeerw, devicetree, linux-kernel Hi Rob, Thank you for reviewing the patch. My responses in line. > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Tuesday 13 July 2021 11:31 PM > To: Anand Ashok Dumbre <ANANDASH@xilinx.com> > Cc: jic23@kernel.org; lars@metafoo.de; linux-iio@vger.kernel.org; git-dev > <git-dev@xilinx.com>; Michal Simek <michals@xilinx.com>; > pmeerw@pmeerw.net; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding > documentation > > On Thu, Jun 24, 2021 at 07:29:38PM +0100, Anand Ashok Dumbre wrote: > > Xilinx AMS have several ADC channels that can be used for measurement > > of different voltages and temperatures. Document the same in the > bindings. > > > > Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > > --- > > .../bindings/iio/adc/xlnx,zynqmp-ams.yaml | 228 ++++++++++++++++++ > > 1 file changed, 228 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > new file mode 100644 > > index 000000000000..a065ddd55d38 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > @@ -0,0 +1,228 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/adc/xlnx,zynqmp-ams.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Xilinx Zynq Ultrascale AMS controller > > + > > +maintainers: > > + - Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> > > + > > +description: | > > + The AMS (Analog Monitoring System) 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 which are PL (Programmable Logic) > > +SYSMON and > > + PS (Processing System) SYSMON. > > + All designs should have AMS registers, but PS and PL are optional. > > +The > > + AMS controller can work with only PS, only PL and both PS and PL > > + configurations. Please specify registers according to your design. > > +Devicetree > > + should always have AMS module property. Providing PS & PL module is > optional. > > + > > + AMS Channel Details > > + ``````````````````` > > + Sysmon Block |Channel| Details > |Measurement > > + |Number | |Type > > + ----------------------------------------------------------------------------------------- > ---------------- > > + AMS CTRL |0 |System PLLs voltage measurement, VCC_PSPLL. > |Voltage > > + |1 |Battery voltage measurement, VCC_PSBATT. > |Voltage > > + |2 |PL Internal voltage measurement, VCCINT. > |Voltage > > + |3 |Block RAM voltage measurement, VCCBRAM. > |Voltage > > + |4 |PL Aux voltage measurement, VCCAUX. > |Voltage > > + |5 |Voltage measurement for six DDR I/O PLLs, > VCC_PSDDR_PLL. |Voltage > > + |6 |VCC_PSINTFP_DDR voltage measurement. > |Voltage > > + ----------------------------------------------------------------------------------------- > ---------------- > > + PS Sysmon |7 |LPD temperature measurement. > |Temperature > > + |8 |FPD temperature measurement (REMOTE). > |Temperature > > + |9 |VCC PS LPD voltage measurement (supply1). > |Voltage > > + |10 |VCC PS FPD voltage measurement (supply2). > |Voltage > > + |11 |PS Aux voltage reference (supply3). |Voltage > > + |12 |DDR I/O VCC voltage measurement. > |Voltage > > + |13 |PS IO Bank 503 voltage measurement (supply5). > |Voltage > > + |14 |PS IO Bank 500 voltage measurement (supply6). > |Voltage > > + |15 |VCCO_PSIO1 voltage measurement. > |Voltage > > + |16 |VCCO_PSIO2 voltage measurement. > |Voltage > > + |17 |VCC_PS_GTR voltage measurement (VPS_MGTRAVCC). > |Voltage > > + |18 |VTT_PS_GTR voltage measurement (VPS_MGTRAVTT). > |Voltage > > + |19 |VCC_PSADC voltage measurement. > |Voltage > > + ----------------------------------------------------------------------------------------- > ---------------- > > + PL Sysmon |20 |PL temperature measurement. > |Temperature > > + |21 |PL Internal voltage measurement, VCCINT. > |Voltage > > + |22 |PL Auxiliary voltage measurement, VCCAUX. > |Voltage > > + |23 |ADC Reference P+ voltage measurement. > |Voltage > > + |24 |ADC Reference N- voltage measurement. > |Voltage > > + |25 |PL Block RAM voltage measurement, VCCBRAM. > |Voltage > > + |26 |LPD Internal voltage measurement, VCC_PSINTLP > (supply4). |Voltage > > + |27 |FPD Internal voltage measurement, VCC_PSINTFP > (supply5). |Voltage > > + |28 |PS Auxiliary voltage measurement (supply6). > |Voltage > > + |29 |PL VCCADC voltage measurement (vccams). > |Voltage > > + |30 |Differential analog input signal voltage measurment. > |Voltage > > + |31 |VUser0 voltage measurement (supply7). > |Voltage > > + |32 |VUser1 voltage measurement (supply8). > |Voltage > > + |33 |VUser2 voltage measurement (supply9). > |Voltage > > + |34 |VUser3 voltage measurement (supply10). > |Voltage > > + |35 |Auxiliary ch 0 voltage measurement (VAux0). > |Voltage > > + |36 |Auxiliary ch 1 voltage measurement (VAux1). > |Voltage > > + |37 |Auxiliary ch 2 voltage measurement (VAux2). > |Voltage > > + |38 |Auxiliary ch 3 voltage measurement (VAux3). > |Voltage > > + |39 |Auxiliary ch 4 voltage measurement (VAux4). > |Voltage > > + |40 |Auxiliary ch 5 voltage measurement (VAux5). > |Voltage > > + |41 |Auxiliary ch 6 voltage measurement (VAux6). > |Voltage > > + |42 |Auxiliary ch 7 voltage measurement (VAux7). > |Voltage > > + |43 |Auxiliary ch 8 voltage measurement (VAux8). > |Voltage > > + |44 |Auxiliary ch 9 voltage measurement (VAux9). > |Voltage > > + |45 |Auxiliary ch 10 voltage measurement (VAux10). > |Voltage > > + |46 |Auxiliary ch 11 voltage measurement (VAux11). > |Voltage > > + |47 |Auxiliary ch 12 voltage measurement (VAux12). > |Voltage > > + |48 |Auxiliary ch 13 voltage measurement (VAux13). > |Voltage > > + |49 |Auxiliary ch 14 voltage measurement (VAux14). > |Voltage > > + |50 |Auxiliary ch 15 voltage measurement (VAux15). > |Voltage > > + > > + -------------------------------------------------------------------- > > + ------------------------------------ > > + > > +properties: > > + compatible: > > + enum: > > + - xlnx,zynqmp-ams > > + > > + interrupts: > > + maxItems: 1 > > + > > + reg: > > + description: AMS Controller register space > > + maxItems: 1 > > + > > + ranges: > > + description: > > + Maps the child address space for PS and/or PL. > > + maxItems: 1 > > + > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 1 > > + > > + '#io-channel-cells': > > + const: 1 > > + > > +patternProperties: > > + "^ams-ps@0,400$": > > If your going to hardcode the unit-address, then it's a fixed string and should > be under 'properties' instead. Can you point me to an example? When I tried putting it under just properties, I was not able to do so and was getting errors when I ran check_dt_bindings. The only way I was not getting the error was by putting it under pattern properties. My node looked something like this, "ams-ps@0": type:object ... ... > > The unit-address is also wrong. This is memory-mapped ranges, right? If so, > this should just be '0' and the next one '400'. I will fix this. > > > + type: object > > + description: | > > + PS (Processing System) SYSMON is memory mapped to PS. This block > has > > + built-in alarm generation logic that is used to interrupt the processor > > + based on condition set. > > + > > + properties: > > + compatible: > > + enum: > > + - xlnx,zynqmp-ams-ps > > + > > + reg: > > + description: Register Space for PS-SYSMON > > + maxItems: 1 > > + > > + required: > > + - compatible > > + - reg > > + > > + additionalProperties: false > > + > > + "^ams-pl@400,400$": > > + type: object > > + description: > > + PL-SYSMON is capable of monitoring off chip voltage and temperature. > > + PL-SYSMON block has DRP, JTAG and I2C interface to enable > monitoring > > + from external master. Out of this interface currently only DRP is > > + supported. This block has alarm generation logic that is used to > > + interrupt the processor based on condition set. > > + > > + properties: > > + compatible: > > + items: > > + - enum: > > + - xlnx,zynqmp-ams-pl > > + > > + reg: > > + description: Register Space for PL-SYSMON. > > + maxItems: 1 > > + > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 0 > > + > > + patternProperties: > > + "^channel@([2-4][0-9]|50)$": > > + type: object > > + description: > > + Describes the external channels connected. > > + > > + properties: > > + reg: > > + description: > > + Pair of pins the channel is connected to. This value is > > + same as Channel Number for a particular channel. > > + minimum: 20 > > + maximum: 50 > > + > > + xlnx,bipolar: > > Don't we have a common property for this now? > > > + $ref: /schemas/types.yaml#/definitions/flag > > + type: boolean > > + description: > > + If the set channel is used in bipolar mode. > > + > > + required: > > + - reg > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - ranges > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + bus { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + xilinx_ams: ams@ffa50000 { > > + compatible = "xlnx,zynqmp-ams"; > > + interrupt-parent = <&gic>; > > + interrupts = <0 56 4>; > > + reg = <0x0 0xffa50000 0x0 0x800>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + #io-channel-cells = <1>; > > + ranges = <0 0 0xffa50800 0x800>; > > + > > + ams_ps: ams-ps@0,400 { > > + compatible = "xlnx,zynqmp-ams-ps"; > > + reg = <0 0x400>; > > + }; > > + > > + ams_pl: ams-pl@400,400 { > > + compatible = "xlnx,zynqmp-ams-pl"; > > + reg = <0x400 0x400>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + channel@30 { > > + reg = <30>; > > + xlnx,bipolar; > > + }; > > + channel@31 { > > + reg = <31>; > > + }; > > + channel@38 { > > + reg = <38>; > > + xlnx,bipolar; > > + }; > > + }; > > + }; > > + }; > > -- > > 2.17.1 > > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 4/4] MAINTAINERS: Add maintainer for xilinx-ams 2021-06-24 18:29 [PATCH v6 0/4] Add Xilinx AMS Driver Anand Ashok Dumbre ` (2 preceding siblings ...) 2021-06-24 18:29 ` [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre @ 2021-06-24 18:29 ` Anand Ashok Dumbre 3 siblings, 0 replies; 19+ messages in thread From: Anand Ashok Dumbre @ 2021-06-24 18:29 UTC (permalink / raw) To: jic23, lars, linux-iio, git-dev, michal.simek, pmeerw, devicetree, linux-kernel Cc: Anand Ashok Dumbre Add maintaner entry for xilinx-ams driver. Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index bd7aff0c120f..2ebb51d828fb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20017,6 +20017,13 @@ M: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> S: Maintained F: drivers/net/ethernet/xilinx/xilinx_axienet* +XILINX AMS DRIVER +M: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> +L: linux-iio@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml +F: drivers/iio/adc/xilinx-ams.c + XILINX CAN DRIVER M: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com> R: Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com> -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-07-19 7:49 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-28 21:45 [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver kernel test robot 2021-06-29 8:32 ` Dan Carpenter 2021-06-29 8:32 ` Dan Carpenter -- strict thread matches above, loose matches on Subject: below -- 2021-06-24 18:29 [PATCH v6 0/4] Add Xilinx AMS Driver Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 1/4] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 2/4] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre 2021-06-25 10:49 ` kernel test robot 2021-06-25 10:49 ` kernel test robot 2021-07-01 8:00 ` kernel test robot 2021-07-01 8:00 ` kernel test robot 2021-07-04 18:31 ` Jonathan Cameron 2021-07-15 7:48 ` Anand Ashok Dumbre 2021-07-15 12:52 ` Jonathan Cameron 2021-07-19 7:49 ` Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 3/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre 2021-07-04 18:08 ` Jonathan Cameron 2021-07-13 22:31 ` Rob Herring 2021-07-15 6:58 ` Anand Ashok Dumbre 2021-06-24 18:29 ` [PATCH v6 4/4] MAINTAINERS: Add maintainer for xilinx-ams Anand Ashok Dumbre
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.