-
Notifications
You must be signed in to change notification settings - Fork 844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers: iio: adc : Add ad4695 support #2433
Conversation
I'll try to do a proper review in the next few days... One thing that I'll already ask is to rename your driver name. Typically we don't name it with "wildcards" like 'x'. Just pick one of the parts the drivers supports (maybe the one you have) and name the driver against it. |
2f5312b
to
10555f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally remembered about this one 😅
10555f2
to
0761652
Compare
Hi @nunojsa, I made the changes requested by you although there is a chance I missed some. Anyway I will mark this PR as a draft since I tested it with the ZedBoard and appears like the buffer isn't working right, I managed to fix a part of it but still the behavior isn't quit the expected one. |
FYI, we recently upstreamed the driver for this family of chips. https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/iio/adc/ad4695.c?h=testing And @threexc is working on adding more features upstream. |
0761652
to
6f9cf07
Compare
v2 :
|
@RaduSabau1 please make sure to take care about CI complains... |
ce18e56
to
7c8b82f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still some stuff to be done
Adds: - DEFINE_GUARD_COND() / DEFINE_LOCK_GUARD_1_COND() to extend existing guards with conditional lock primitives, eg. mutex_trylock(), mutex_lock_interruptible(). nb. both primitives allow NULL 'locks', which cause the lock to fail (obviously). - extends scoped_guard() to not take the body when the the conditional guard 'fails'. eg. scoped_guard (mutex_intr, &task->signal_cred_guard_mutex) { ... } will only execute the body when the mutex is held. - provides scoped_cond_guard(name, fail, args...); which extends scoped_guard() to do fail when the lock-acquire fails. Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Link: https://lkml.kernel.org/r/20231102110706.460851167%40infradead.org
750381f
to
08d4e23
Compare
@@ -0,0 +1,155 @@ | |||
// SPDX-License-Identifier: GPL-2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to compare notes, here is what I have been using for a .dts
. Feel free to copy anything you think is useful.
A few things to point out:
I noticed that the rx-dmac@44a30000
node has a different clock and adi,destination-bus-width
, so we should probably have a look at the HDL to see which one is right.
IIRC, by default, the COM pin is wired to GND on the eval board. We modified our board so I added a comment about this. But probably best to make the "official" .dts use the eval board defaults with a comment to explain what to change if you resolder the board like we did.
And finally, I know this isn't really how things are done in the ADI tree currently, but I named all of the power supplies after what there name is on the EVAL board schematic instead of which pin on the ADC they are connected to. Since the eval boards tend to have lots of jumpers that can be switched around to rewire things, I feel like doing it this way requires fewer changes to the .dts file if you end up rewiring things. And it makes it more obvious which supply exactly each node represents.
// SPDX-License-Identifier: GPL-2.0
/*
* Analog Devices EVAL-AD4696FMCZ
* https://www.analog.com/en/products/ad4696.html
*
* hdl_project: <ad469x/zed>
* board_revision: <?>
*
* Copyright (C) 2024 Analog Devices Inc.
*/
/dts-v1/;
#include <dt-bindings/dma/axi-dmac.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/iio/adi,ad4695.h>
#include "zynq-zed.dtsi"
#include "zynq-zed-adv7511.dtsi"
/ {
eval_u3: eval-board-u3-regulator {
compatible = "regulator-fixed";
regulator-name = "EVAL ADR4550 5.0V REFERENCE (U3)";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
regulator-always-on;
};
eval_u5: eval-board-u5-regulator {
compatible = "regulator-fixed";
regulator-name = "EVAL 12V/5V,-3.3V/-2.5V DUAL-LDO (U5)";
regulator-always-on;
};
eval_u6: eval-board-u6-regulator {
compatible = "regulator-fixed";
regulator-name = "EVAL 5V/1.8V LDO (U6)";
regulator-always-on;
};
eval_a8: eval-board-a8-voltage-divider {
compatible = "regulator-fixed";
regulator-name = "EVAL VREF/2 DIVIDER CIRCUIT (A8)";
regulator-min-microvolt = <2500000>;
regulator-max-microvolt = <2500000>;
regulator-always-on;
};
signal_gen_neg: signal-generator-negative {
compatible = "regulator-fixed";
regulator-name = "Signal generator negative output";
regulator-min-microvolt = <2500000>;
regulator-max-microvolt = <2500000>;
regulator-always-on;
};
};
&fpga_axi {
adc_trigger: pwm@44b00000 {
compatible = "adi,axi-pwmgen-2.00.a";
reg = <0x44b00000 0x1000>;
#pwm-cells = <2>;
clocks = <&spi_clk>;
};
rx_dma: rx-dmac@44a30000 {
compatible = "adi,axi-dmac-1.00.a";
reg = <0x44a30000 0x1000>;
#dma-cells = <1>;
interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkc 17>;
adi,channels {
#size-cells = <0>;
#address-cells = <1>;
dma-channel@0 {
reg = <0>;
adi,source-bus-width = <32>;
adi,source-bus-type = <AXI_DMAC_BUS_TYPE_AXI_STREAM>;
adi,destination-bus-width = <64>;
adi,destination-bus-type = <AXI_DMAC_BUS_TYPE_AXI_MM>;
};
};
};
spi_clk: clock-controller@44a70000 {
compatible = "adi,axi-clkgen-2.00.a";
reg = <0x44a70000 0x1000>;
#clock-cells = <0>;
clocks = <&clkc 15>;
clock-names = "s_axi_aclk", "clkin1";
clock-output-names = "spi_clk";
/* Set to 2x max SCLK. */
assigned-clocks = <&spi_clk>;
assigned-clock-rates = <160000000>;
};
axi_spi_engine_0: spi@44a00000 {
compatible = "adi,axi-spi-engine-1.00.a";
reg = <0x44a00000 0x1000>;
interrupt-parent = <&intc>;
interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkc 15>, <&spi_clk>;
clock-names = "s_axi_aclk", "spi_clk";
#address-cells = <1>;
#size-cells = <0>;
ad4696: adc@0 {
compatible = "adi,ad4696";
reg = <0>;
spi-max-frequency = <80000000>;
spi-cpha;
spi-cpol;
avdd-supply = <&eval_u5>;
ldo-in-supply = <&eval_u5>;
vio-supply = <&eval_u6>;
ref-supply = <&eval_u3>;
/* Not default: requires changing JP6 to A */
com-supply = <&eval_a8>;
in1-supply = <&signal_gen_neg>;
reset-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
#address-cells = <1>;
#size-cells = <0>;
channel@0 {
reg = <0>;
common-mode-channel = <1>;
bipolar;
};
channel@3 {
reg = <3>;
common-mode-channel = <AD4695_COMMON_MODE_COM>;
bipolar;
};
};
};
};
71543dd
to
e8f4023
Compare
It looks like there are some more dependencies to cherry-pick. These go before "iio: add support for multiple scan types per channel". And we also need to pick up 257b233 which goes right after "regulator: devres: add API for reference voltage supplies". Are you able to compile the kernel yourself to catch these compile errors? |
…tructure Allows use of: iio_device_claim_direct_scoped(return -EBUSY, indio_dev) { } to automatically call iio_device_release_direct_mode() based on scope. Typically seen in combination with local device specific locks which are already have automated cleanup options via guard(mutex)(&st->lock) and scoped_guard(). Using both together allows most error handling to be automated. Reviewed-by: Nuno Sa <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jonathan Cameron <[email protected]>
This gives the channel scan_type a named type so that it can be used to simplify code in later commits. Signed-off-by: David Lechner <[email protected]> Reviewed-by: Nuno Sa <[email protected]> Link: https://lore.kernel.org/r/20240530-iio-add-support-for-multiple-scan-types-v3-1-cbc4acea2cfa@baylibre.com Signed-off-by: Jonathan Cameron <[email protected]>
By using struct iio_scan_type, we can simplify the code by removing lots of duplicate pointer dereferences. This make the code a bit easier to read. This also prepares for a future where channels may have more than one scan_type. Signed-off-by: David Lechner <[email protected]> Reviewed-by: Nuno Sa <[email protected]> Link: https://lore.kernel.org/r/20240530-iio-add-support-for-multiple-scan-types-v3-2-cbc4acea2cfa@baylibre.com Signed-off-by: Jonathan Cameron <[email protected]>
e8f4023
to
a433b4f
Compare
This adds new fields to the iio_channel structure to support multiple scan types per channel. This is useful for devices that support multiple resolution modes or other modes that require different data formats of the raw data. To make use of this, drivers need to implement the new callback get_current_scan_type() to resolve the scan type for a given channel based on the current state of the driver. There is a new scan_type_ext field in the iio_channel structure that should be used to store the scan types for any channel that has more than one. There is also a new flag has_ext_scan_type that acts as a type discriminator for the scan_type/ext_scan_type union. A union is used so that we don't grow the size of the iio_channel structure and also makes it clear that scan_type and ext_scan_type are mutually exclusive. The buffer code is the only code in the IIO core code that is using the scan_type field. This patch updates the buffer code to use the new iio_channel_validate_scan_type() function to ensure it is returning the correct scan type for the current state of the device when reading the sysfs attributes. The buffer validation code is also update to validate any additional scan types that are set in the scan_type_ext field. Part of that code is refactored to a new function to avoid duplication. Some userspace tools may need to be updated to re-read the scan type after writing any other attribute. During testing, we noticed that we had to restart iiod to get it to re-read the scan type after enabling oversampling on the ad7380 driver. Signed-off-by: David Lechner <[email protected]> Reviewed-by: Nuno Sa <[email protected]> Link: https://lore.kernel.org/r/20240530-iio-add-support-for-multiple-scan-types-v3-3-cbc4acea2cfa@baylibre.com Signed-off-by: Jonathan Cameron <[email protected]>
'masklength' is supposed to be an IIO private member. However, drivers (often in trigger handlers) need to access it to iterate over the enabled channels for example (there are other reasons). Hence, a couple of new accessors are being added: * iio_for_each_active_channel() - Iterates over the active channels; * iio_get_masklength() - Get length of the channels mask. The goal of these new accessors is to annotate 'masklength' as private as soon as all drivers accessing it are converted to use the new helpers. Signed-off-by: Nuno Sa <[email protected]> Reviewed-by: Alexandru Ardelean <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jonathan Cameron <[email protected]>
A common use case for regulators is to supply a reference voltage to an analog input or output device. This adds a new devres API to get, enable, and get the voltage in a single call. This allows eliminating boilerplate code in drivers that use reference supplies in this way. Signed-off-by: David Lechner <[email protected]> Link: https://lore.kernel.org/r/20240429-regulator-get-enable-get-votlage-v2-1-b1f11ab766c1@baylibre.com Signed-off-by: Mark Brown <[email protected]>
The devm_regulator_get_enable_read_voltage() function is supposed to return the voltage that the regulator is currently set to. However, it currently returns 0. Fixes: b250c20 ("regulator: devres: add API for reference voltage supplies") Signed-off-by: David Lechner <[email protected]> Link: https://lore.kernel.org/r/20240506-regulator-devm_regulator_get_enable_read_voltage-fixes-v1-1-356cdd152067@baylibre.com Signed-off-by: Mark Brown <[email protected]>
Add device tree bindings for AD4695 and similar ADCs. Reviewed-by: Conor Dooley <[email protected]> Signed-off-by: David Lechner <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jonathan Cameron <[email protected]>
This is a new driver for Analog Devices Inc. AD4695 and similar ADCs. The initial driver supports initializing the chip including configuring all possible LDO and reference voltage sources as well as any possible voltage input channel wiring configuration. Only the 4-wire SPI wiring mode where the CNV pin is tied to the CS pin is supported at this time. And reading sample data from the ADC can only be done in direct mode for now. Co-developed-by: Ramona Gradinariu <[email protected]> Signed-off-by: Ramona Gradinariu <[email protected]> Signed-off-by: David Lechner <[email protected]> Reviewed-by: Nuno Sa <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jonathan Cameron <[email protected]>
The Analog Devices Inc. AD4695 (and similar chips) are complex ADCs that will benefit from a detailed driver documentation. This documents the current features supported by the driver. Signed-off-by: David Lechner <[email protected]> Signed-off-by: Jonathan Cameron <[email protected]>
Imply Ad4695 in Kconfig.adi file in the IIO subsystem. Signed-off-by: RaduSabau1 <[email protected]>
Add devicetree for ad469x_fmcz/zed project Signed-off-by: RaduSabau1 <[email protected]>
a433b4f
to
2391e6e
Compare
No description provided.