[v10,0/5] Add Xilinx AMS Driver

Message ID	20211117161028.11775-1-anand.ashok.dumbre@xilinx.com (mailing list archive)
Headers	show Return-Path: <linux-acpi-owner@kernel.org> Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates 149.199.80.198 as permitted sender) receiver=protection.outlook.com; client-ip=149.199.80.198; helo=xir-pvapexch02.xlnx.xilinx.com; Envelope-to: anand.ashok.dumbre@xilinx.com, git@xilinx.com, michal.simek@xilinx.com, linux-kernel@vger.kernel.org, jic23@kernel.org, lars@metafoo.de, linux-iio@vger.kernel.org, gregkh@linuxfoundation.org, rafael@kernel.org, linux-acpi@vger.kernel.org, andriy.shevchenko@linux.intel.com, heikki.krogerus@linux.intel.com From: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> To: <linux-kernel@vger.kernel.org>, <jic23@kernel.org>, <lars@metafoo.de>, <linux-iio@vger.kernel.org>, <git@xilinx.com>, <michal.simek@xilinx.com>, <gregkh@linuxfoundation.org>, <rafael@kernel.org>, <linux-acpi@vger.kernel.org>, <andriy.shevchenko@linux.intel.com>, <heikki.krogerus@linux.intel.com> CC: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com> Subject: [PATCH v10 0/5] Add Xilinx AMS Driver Date: Wed, 17 Nov 2021 16:10:23 +0000 Message-ID: <20211117161028.11775-1-anand.ashok.dumbre@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk
Series	Add Xilinx AMS Driver \| expand [v10,0/5] Add Xilinx AMS Driver [v10,1/5] device property: Add fwnode_iomap() [v10,2/5] arm64: zynqmp: DT: Add Xilinx AMS node [v10,3/5] iio: adc: Add Xilinx AMS driver [v10,4/5] dt-bindings: iio: adc: Add Xilinx AMS binding documentation [v10,5/5] MAINTAINERS: Add maintainer for xilinx-ams

Message ID

20211117161028.11775-1-anand.ashok.dumbre@xilinx.com (mailing list archive)

Headers

Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.80.198 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.80.198; helo=xir-pvapexch02.xlnx.xilinx.com;
Envelope-to: anand.ashok.dumbre@xilinx.com,
 git@xilinx.com,
 michal.simek@xilinx.com,
 linux-kernel@vger.kernel.org,
 jic23@kernel.org,
 lars@metafoo.de,
 linux-iio@vger.kernel.org,
 gregkh@linuxfoundation.org,
 rafael@kernel.org,
 linux-acpi@vger.kernel.org,
 andriy.shevchenko@linux.intel.com,
 heikki.krogerus@linux.intel.com
From: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
To: <linux-kernel@vger.kernel.org>, <jic23@kernel.org>,
        <lars@metafoo.de>, <linux-iio@vger.kernel.org>, <git@xilinx.com>,
        <michal.simek@xilinx.com>, <gregkh@linuxfoundation.org>,
        <rafael@kernel.org>, <linux-acpi@vger.kernel.org>,
        <andriy.shevchenko@linux.intel.com>,
        <heikki.krogerus@linux.intel.com>
CC: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
Subject: [PATCH v10 0/5] Add Xilinx AMS Driver
Date: Wed, 17 Nov 2021 16:10:23 +0000
Message-ID: <20211117161028.11775-1-anand.ashok.dumbre@xilinx.com>
MIME-Version: 1.0
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2021 16:10:38.8229
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 5fe1256b-9a58-4925-ac07-08d9a9e4cc10
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: 
 TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.80.198];Helo=[xir-pvapexch02.xlnx.xilinx.com]
X-MS-Exchange-CrossTenant-AuthSource: 
 SN1NAM02FT0011.eop-nam02.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR02MB7103
Precedence: bulk
List-ID: <linux-acpi.vger.kernel.org>
X-Mailing-List: linux-acpi@vger.kernel.org

Series

Add Xilinx AMS Driver | expand

Message

Anand Ashok Dumbre Nov. 17, 2021, 4:10 p.m. UTC

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()

Changes in v7:
	- Added use of FIELD_PREP and FIELD_GET.
	- Added the spinlocks back the v1 which were removed in v2 for
	  no justifiable reason and replaced with the same mutex. This
	  caused deadlocks.
	- Removed the buffered mode information from channel config.
	- Usage of wrapper functions for devm_add_action_or_reset
	  callbacks to avoid typecasting functions.
	- Usage of devm_platform_iremap_resource().
	- Handled platform_get_irq() return values.
	- Removed the remove() callback.
	- Fixed the dt-bindings.

Changes in v8:
	- Replaced *_of_() APIs with fwnode.
	- Added missing headers.
	- Fixed documentation.
	- Added devm_add_action_or_reset() for iounmap.
	- Restructured read_raw function.
	- Added helper functions.
	- Usage of GENMASK for all masks.
	- Added defaults for most switch cases. Some can't be added
	  since the default will never occur.

Changes in v9:
	- Added a fwnode_iomap().
	- Fixed Kconfig indentation.
	- Added the overflow checks before memory allocation.
	- Usage of fwnode_iomap() instead of iomap().
	- Rename ams_parse_dt() to ams_parse_firmware().

Changes in v10:
	- Fixed licence in zynqmp.dtsi.
	- Changed the macros to use BIT().
	- Realign some code to fit within 100 chars.
	- Modified readl_poll_timeout usage.
	- Usage of array_size() instead of check_mul_overflow().
	- Usage of dev_err_probe() instead of dev_err().
	- Usage of kcalloc instead of kzalloc()


Anand Ashok Dumbre (5):
  device property: Add fwnode_iomap()
  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     |  227 +++
 MAINTAINERS                                   |    7 +
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |   24 +
 drivers/base/property.c                       |   16 +
 drivers/iio/adc/Kconfig                       |   15 +
 drivers/iio/adc/Makefile                      |    1 +
 drivers/iio/adc/xilinx-ams.c                  | 1447 +++++++++++++++++
 include/linux/property.h                      |    2 +
 8 files changed, 1739 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
 create mode 100644 drivers/iio/adc/xilinx-ams.c

Comments

Andy Shevchenko Nov. 17, 2021, 5:13 p.m. UTC | #1

On Wed, Nov 17, 2021 at 6:46 PM Anand Ashok Dumbre
<anand.ashok.dumbre@xilinx.com> wrote:
>
> 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.

Thanks and wait a bit, please. No need to send patches so often. We
haven't settled down yet on v9. Here you effectively ignored my
comment given again patch 5/5. Why?

...

> Changes in v10:
>         - Fixed licence in zynqmp.dtsi.
>         - Changed the macros to use BIT().
>         - Realign some code to fit within 100 chars.
>         - Modified readl_poll_timeout usage.

>         - Usage of array_size() instead of check_mul_overflow().

Why? I have asked you why you haven't used kcalloc there as well.

>         - Usage of dev_err_probe() instead of dev_err().
>         - Usage of kcalloc instead of kzalloc()

Anand Ashok Dumbre Nov. 18, 2021, 3:03 p.m. UTC | #2

Hi Andy,

Thanks for the review.

> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Wednesday 17 November 2021 5:14 PM
> To: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Jonathan
> Cameron <jic23@kernel.org>; Lars-Peter Clausen <lars@metafoo.de>; linux-
> iio <linux-iio@vger.kernel.org>; git <git@xilinx.com>; Michal Simek
> <michals@xilinx.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Rafael J. Wysocki <rafael@kernel.org>; ACPI Devel Maling List <linux-
> acpi@vger.kernel.org>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Krogerus, Heikki
> <heikki.krogerus@linux.intel.com>
> Subject: Re: [PATCH v10 0/5] Add Xilinx AMS Driver
> 
> On Wed, Nov 17, 2021 at 6:46 PM Anand Ashok Dumbre
> <anand.ashok.dumbre@xilinx.com> wrote:
> >
> > 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.
> 
> Thanks and wait a bit, please. No need to send patches so often. We haven't
> settled down yet on v9. Here you effectively ignored my comment given
> again patch 5/5. Why?
> 

Apologies for sending patch too soon. I sent the patch before I saw your response.

> ...
> 
> > Changes in v10:
> >         - Fixed licence in zynqmp.dtsi.
> >         - Changed the macros to use BIT().
> >         - Realign some code to fit within 100 chars.
> >         - Modified readl_poll_timeout usage.
> 
> >         - Usage of array_size() instead of check_mul_overflow().
> 
> Why? I have asked you why you haven't used kcalloc there as well.

Yes, I saw the implementation of kcalloc after reading your comment and
It does check_mul_overflow internally so this is not needed.

I will wait a couple of days before sending new patches.

> 
> >         - Usage of dev_err_probe() instead of dev_err().
> >         - Usage of kcalloc instead of kzalloc()
> 
> --
> With Best Regards,
> Andy Shevchenko

Thanks,
Anand