* [PATCH v1 1/2] dt-bindings: mfd: Document TI LM3533 MFD
From: Svyatoslav Ryhel @ 2025-02-12 7:58 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Svyatoslav Ryhel,
Andy Shevchenko, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <20250212075845.11338-1-clamor95@gmail.com>
Add bindings for the LM3533 - a complete power source for
backlight, keypad, and indicator LEDs in smartphone handsets.
The high-voltage inductive boost converter provides the
power for two series LED strings display backlight and keypad
functions.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
.../devicetree/bindings/mfd/ti,lm3533.yaml | 221 ++++++++++++++++++
include/dt-bindings/mfd/lm3533.h | 19 ++
2 files changed, 240 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
create mode 100644 include/dt-bindings/mfd/lm3533.h
diff --git a/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
new file mode 100644
index 000000000000..d0307e5894f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
@@ -0,0 +1,221 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ti,lm3533.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3533 Complete Lighting Power Solution
+
+description: |
+ The LM3533 is a complete power source for backlight,
+ keypad, and indicator LEDs in smartphone handsets. The
+ high-voltage inductive boost converter provides the
+ power for two series LED strings display backlight and
+ keypad functions.
+ https://www.ti.com/product/LM3533
+
+maintainers:
+ - Johan Hovold <jhovold@gmail.com>
+
+properties:
+ compatible:
+ const: ti,lm3533
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ enable-gpios:
+ description: GPIO to use to enable/disable the backlight (HWEN pin).
+ maxItems: 1
+
+ ti,boost-ovp:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Boost OVP select (16V, 24V, 32V, 40V)
+ enum: [ 0, 1, 2, 3 ]
+ default: 0
+
+ ti,boost-freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Boost frequency select (500KHz or 1MHz)
+ enum: [ 0, 1 ]
+ default: 0
+
+required:
+ - compatible
+ - reg
+ - '#address-cells'
+ - '#size-cells'
+ - enable-gpios
+
+patternProperties:
+ "^backlight@[01]$":
+ type: object
+ description:
+ Properties for a backlight device.
+
+ properties:
+ compatible:
+ const: lm3533-backlight
+
+ reg:
+ description: |
+ The control bank that is used to program the two current sinks. The
+ LM3533 has two control banks (A and B) and are represented as 0 or 1
+ in this property. The two current sinks can be controlled
+ independently with both banks, or bank A can be configured to control
+ both sinks with the led-sources property.
+ minimum: 0
+ maximum: 1
+
+ max-current-microamp:
+ description:
+ Maximum current in µA with a 800 µA step.
+ enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
+ 10600, 11400, 12200, 13000, 13800, 14600,
+ 15400, 16200, 17000, 17800, 18600, 19400,
+ 20200, 21000, 21800, 22600, 23400, 24200,
+ 25000, 25800, 26600, 27400, 28200, 29000,
+ 29800 ]
+ default: 5000
+
+ default-brightness:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Default brightness level on boot.
+ minimum: 0
+ maximum: 255
+ default: 255
+
+ pwm:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Default pwm level on boot.
+ minimum: 0
+ maximum: 63
+ default: 0
+
+ required:
+ - compatible
+ - reg
+
+ additionalProperties: false
+
+ "^led@[0-3]$":
+ type: object
+ description:
+ Properties for a led device.
+
+ properties:
+ compatible:
+ const: lm3533-leds
+
+ reg:
+ description:
+ 4 led banks
+ minimum: 0
+ maximum: 3
+
+ max-current-microamp:
+ description:
+ Maximum current in µA with a 800 µA step.
+ enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
+ 10600, 11400, 12200, 13000, 13800, 14600,
+ 15400, 16200, 17000, 17800, 18600, 19400,
+ 20200, 21000, 21800, 22600, 23400, 24200,
+ 25000, 25800, 26600, 27400, 28200, 29000,
+ 29800 ]
+ default: 5000
+
+ default-trigger:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ This parameter, if present, is a string defining
+ the trigger assigned to the LED. Check linux,default-trigger.
+
+ pwm:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Default pwm level on boot.
+ minimum: 0
+ maximum: 63
+ default: 0
+
+ required:
+ - compatible
+ - reg
+
+ additionalProperties: false
+
+ "^light-sensor@[0]$":
+ type: object
+ description:
+ Properties for an illumination sensor.
+
+ properties:
+ compatible:
+ const: lm3533-als
+
+ reg:
+ const: 0
+
+ resistor-value:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ PWM resistor value a linear step in currents
+ of 10 µA per code based upon 2V/R_ALS.
+ minimum: 1
+ maximum: 127
+ default: 1
+
+ pwm-mode:
+ type: boolean
+ description: Mode in which ALS is running
+
+ required:
+ - compatible
+ - reg
+
+ additionalProperties: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/mfd/lm3533.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@36 {
+ compatible = "ti,lm3533";
+ reg = <0x36>;
+
+ enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
+
+ ti,boost-ovp = <LM3533_BOOST_OVP_24V>;
+ ti,boost-freq = <LM3533_BOOST_FREQ_500KHZ>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight@0 {
+ compatible = "lm3533-backlight";
+ reg = <0>;
+
+ max-current-microamp = <23400>;
+ default-brightness = <113>;
+ pwm = <0x00>;
+ };
+ };
+ };
+...
diff --git a/include/dt-bindings/mfd/lm3533.h b/include/dt-bindings/mfd/lm3533.h
new file mode 100644
index 000000000000..929988190c52
--- /dev/null
+++ b/include/dt-bindings/mfd/lm3533.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * This header provides macros for TI LM3533 device bindings.
+ */
+
+#ifndef _DT_BINDINGS_MFD_LM3533_H
+#define _DT_BINDINGS_MFD_LM3533_H
+
+/* LM3533 boost freq */
+#define LM3533_BOOST_FREQ_500KHZ 0
+#define LM3533_BOOST_FREQ_1000KHZ 1
+
+/* LM3533 boost ovp */
+#define LM3533_BOOST_OVP_16V 0
+#define LM3533_BOOST_OVP_24V 1
+#define LM3533_BOOST_OVP_32V 2
+#define LM3533_BOOST_OVP_40V 3
+
+#endif
--
2.43.0
^ permalink raw reply related
* [PATCH v1 0/2] mfd: lm3533: convert to use OF
From: Svyatoslav Ryhel @ 2025-02-12 7:58 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Cameron, Lars-Peter Clausen, Pavel Machek,
Daniel Thompson, Jingoo Han, Helge Deller, Svyatoslav Ryhel,
Andy Shevchenko, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-iio, linux-leds, dri-devel,
linux-fbdev
Add schema and add support for lm3533 mfd to use device
tree bindings.
Svyatoslav Ryhel (2):
dt-bindings: mfd: Document TI LM3533 MFD
mfd: lm3533: convert to use OF
.../devicetree/bindings/mfd/ti,lm3533.yaml | 221 ++++++++++++++++++
drivers/iio/light/lm3533-als.c | 58 ++++-
drivers/leds/leds-lm3533.c | 69 +++++-
drivers/mfd/lm3533-core.c | 79 +++++--
drivers/video/backlight/lm3533_bl.c | 72 +++++-
include/dt-bindings/mfd/lm3533.h | 19 ++
include/linux/mfd/lm3533.h | 1 +
7 files changed, 496 insertions(+), 23 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
create mode 100644 include/dt-bindings/mfd/lm3533.h
--
2.43.0
^ permalink raw reply
* [PATCH] staging: sm750fb: fix checkpatch warning architecture specific defines should be avoided
From: Michael Anckaert @ 2025-02-12 7:43 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman,
open list:STAGING - SILICON MOTION SM750 FRAME BUFFER DRIVER,
open list:STAGING SUBSYSTEM, open list
Replace architecture-specific defines with CONFIG_X86 checks to improve
portability and adhere to kernel coding standards.
Fixes checkpatch warning:
- CHECK: architecture specific defines should be avoided.
Changes made:
- Using CONFIG_X86 instead of i386 and x86.
Signed-off-by: Michael Anckaert <michael.anckaert@gmail.com>
---
drivers/staging/sm750fb/ddk750_chip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 02860d3ec365..67a2f60440ca 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -229,7 +229,7 @@ int ddk750_init_hw(struct initchip_param *p_init_param)
reg |= (VGA_CONFIGURATION_PLL | VGA_CONFIGURATION_MODE);
poke32(VGA_CONFIGURATION, reg);
} else {
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef CONFIG_X86
/* set graphic mode via IO method */
outb_p(0x88, 0x3d4);
outb_p(0x06, 0x3d5);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 1/1] fbdev: hyperv_fb: iounmap() the correct memory when removing a device
From: Wei Liu @ 2025-02-12 2:27 UTC (permalink / raw)
To: mhklinux
Cc: haiyangz, wei.liu, decui, deller, weh, dri-devel, linux-fbdev,
linux-kernel, linux-hyperv
In-Reply-To: <20250209235252.2987-1-mhklinux@outlook.com>
On Sun, Feb 09, 2025 at 03:52:52PM -0800, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> When a Hyper-V framebuffer device is removed, or the driver is unbound
> from a device, any allocated and/or mapped memory must be released. In
> particular, MMIO address space that was mapped to the framebuffer must
> be unmapped. Current code unmaps the wrong address, resulting in an
> error like:
>
> [ 4093.980597] iounmap: bad address 00000000c936c05c
>
> followed by a stack dump.
>
> Commit d21987d709e8 ("video: hyperv: hyperv_fb: Support deferred IO for
> Hyper-V frame buffer driver") changed the kind of address stored in
> info->screen_base, and the iounmap() call in hvfb_putmem() was not
> updated accordingly.
>
> Fix this by updating hvfb_putmem() to unmap the correct address.
>
> Fixes: d21987d709e8 ("video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
Applied to hyperv-fixes. Thanks.
^ permalink raw reply
* Re: [PATCH v4 0/9] add support for Software mode on AD7606's iio backend driver
From: Jonathan Cameron @ 2025-02-11 19:57 UTC (permalink / raw)
To: Angelo Dureghello
Cc: Michael Hennerich, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandru Ardelean,
David Lechner, Jonathan Cameron, linux-fbdev, linux-iio,
devicetree, linux-kernel, Guillaume Stols
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
On Mon, 10 Feb 2025 17:10:50 +0100
Angelo Dureghello <adureghello@baylibre.com> wrote:
> The previous series added iio_backend mode, but the configuration for this
> mode was only possible through GPIOs (Hardware mode). Here, we add support
> for configuring the chip using its registers (Software mode).
>
> The bus access is based on Angelo's ad3552 implementation, that is we have
> a particular compatible for the backend (here axi-adc) version supporting
> the ad7606's register writing, and the ad7606 is defined as a child node
> of the backend in the devicetree. Small changes are added to make the code
> a bit more straightforward to understand, and more compact.
>
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
Applied to the togreg branch of iio.git. Briefly pushed that out as testing
to let 0-day take a first look.
Thanks,
Jonathan
^ permalink raw reply
* RE: hyper_bf soft lockup on Azure Gen2 VM when taking kdump or executing kexec
From: Michael Kelley @ 2025-02-11 19:45 UTC (permalink / raw)
To: Maxim Levitsky, thomas.tai@oracle.com, mhkelley58@gmail.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
drawat.floss@gmail.com, javierm@redhat.com, Helge Deller,
daniel@ffwll.ch, airlied@gmail.com, tzimmermann@suse.de
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <3c06cc1bb206f1ff9925dc4c7cf5a23f3f6c3505.camel@redhat.com>
From: Maxim Levitsky <mlevitsk@redhat.com> Sent: Monday, February 10, 2025 3:57 PM
>
> On Mon, 2025-02-10 at 21:35 +0000, Michael Kelley wrote:
> > From: thomas.tai@oracle.com <thomas.tai@oracle.com> Sent: Monday, February 10, 2025 7:08 AM
> > > <snip>
> > >
> > > > > Then the question is why the efifb driver doesn't work in the kdump
> > > > > kernel. Actually, it *does* work in many cases. I built the 6.13.0 kernel
> > > > > on the Oracle Linux 9.4 system, and transferred the kernel image binary
> > > > > and module binaries to an Ubuntu 20.04 VM in Azure. In that VM, the
> > > > > efifb driver is loaded as part of the kdump kernel, and it doesn't cause
> > > > > any problems. But there's an interesting difference. In the Oracle Linux
> > > > > 9.4 VM, the efifb driver finds the framebuffer at 0x40000000, while on
> > > > > the Ubuntu 20.04 VM, it finds the framebuffer at 0x40900000. This
> > > > > difference is due to differences in how the screen_info variable gets
> > > > > setup in the two VMs.
> > > > >
> > > > > When the normal kernel starts in a freshly booted VM, Hyper-V provides
> > > > > the EFI framebuffer at 0x40000000, and it works. But after the Hyper-V
> > > > > FB driver or Hyper-V DRM driver has initialized, Linux has picked a
> > > > > different MMIO address range and told Hyper-V to use the new
> > > > > address range (which often starts at 0x40900000). A kexec does *not*
> > > > > reset Hyper-V's transition to the new range, so when the efifb driver
> > > > > tries to use the framebuffer at 0x40000000, the accesses trap to
> > > > > Hyper-V and probably fail or timeout (I'm not sure of the details). After
> > > > > the guest does some number of these bad references, Hyper-V considers
> > > > > itself to be under attack from an ill-behaving guest, and throttles the
> > > > > guest so that it doesn't run for a few seconds. The throttling repeats,
> > > > > and results in extremely slow running in the kdump kernel.
> > > > >
> > > > > Somehow in the Ubuntu 20.04 VM, the location of the frame buffer
> > > > > as stored in screen_info.lfb_base gets updated to be 0x40900000. I
> > > > > haven't fully debugged how that happens. But with that update, the
> > > > > efifb driver is using the updated framebuffer address and it works. On
> > > > > the Oracle Linux 9.4 system, that update doesn't appear to happen,
> > > > > and the problem occurs.
> > > > >
> > > > > This in an interim update on the problem. I'm still investigating how
> > > > > screen_info.lfb_base is set in the kdump kernel, and why it is different
> > > > > in the Ubuntu 20.04 VM vs. in the Oracle Linux 9.4 VM. Once that is
> > > > > well understood, we can contemplate how to fix the problem. Undoing
> > > > > the revert that is commit 2bebc3cd4870 doesn't seem like the solution
> > > > > since the original code there was reported to cause many other issues.
> > > > > The solution focus will likely be on how to ensure the kdump kernel gets
> > > > > the correct framebuffer address so the efifb driver works, since the
> > > > > framebuffer address changing is a quirk of Hyper-V behavior.
> > > > >
> > > > > If anyone else has insight into what's going on here, please chime in.
> > > > > What I've learned so far is still somewhat tentative.
> > > > >
> > > > Here's what is happening. On Ubuntu 20.04, the kdump image is
> > > > loaded into crash memory using the kexec command. Ubuntu 20.04
> > > > has kexec from the kexec-tools package version 2.0.18-1ubuntu1.1,
> > > > and per the kexec man page, it defaults to using the older kexec_load()
> > > > system call. When using kexec_load(), the contents to be loaded into
> > > > crash memory is constructed in user space by the kexec command.
> > > > The kexec command gets the "screen_info" settings, including the
> > > > physical address of the frame buffer, via the FBIOGET_FSCREENINFO
> > > > ioctl against /dev/fb0. The Hyper-V FB or DRM driver registers itself
> > > > with the fbdev subsystem so that it is /dev/fb0, and the ioctl returns
> > > > the updated framebuffer address. So the efifb driver loads and runs
> > > > correctly.
> > > >
> > > > On Oracle Linux 9.4, the kdump image is also loaded with the
> > > > kexec command, but from kexec-tools package version
> > > > kexec-tools-2.0.28-1.0.10.el9_5.x86_64, which is slightly later than
> > > > the version on Ubuntu 20.04. This newer kexec defaults to using the
> > > > newer kexec_file_load() system call. This system call gets the
> > > > framebuffer address from the screen_info variable in the kernel, which
> > > > has not been updated to reflect the new framebuffer address. Hence
> > > > in the kdump kernel, the efifb driver uses the old framebuffer address,
> > > > and hence the problem.
> > > >
> > > > To further complicate matters, the kexec on Oracle Linux 9.4 seems to
> > > > have a bug when the -c option forces the use of kexec_load() instead
> > > > of kexec_file_load(). As an experiment, I modified the kdumpctl shell
> > > > script to add the "-c" option to kexec, but in that case the value "0x0"
> > > > is passed as the framebuffer address, which is wrong. Furthermore,
> > > > the " screen_info.orig_video_isVGA" value (which I mentioned earlier
> > > > in connection with commit 2bebc3cd4870) is also set to 0, so the
> > > > kdump kernel no longer thinks it has an EFI framebuffer. Hence the
> > > > efifb driver isn't loaded, and the kdump works, though for the wrong
> > > > reasons. If kexec 2.0.18 from Ubuntu is copied onto the Oracle Linux 9.4
> > > > VM, then kdump works as expected, with the efifb driver being loaded
> > > > and using the correct framebuffer address. So something is going wrong
> > > > with kexec 2.0.28 in how it sets up the screen_info when the -c option
> > > > is used. I'll leave the debugging of the kexec bug to someone else.
> > >
> > > Hi Michael,
> > >
> > > Do you think we need to handle Azure Gen2 VM differently in the kexec?
> > >
> > > Or should we change the kexec_file_load() system call to retrieve the correct
> > > framebuffer address?
> >
> > I'm thinking there may be a fix in the Hyper-V FB and Hyper-V DRM drivers.
> > Commit c25a19afb81c may also be a cause of the problem -- see precursor
> > commit 3cb73bc3fa2a, which describes exactly the problem. I still need to
> > do some testing, but without that commit, kdump won't detect that it has
> > an EFI framebuffer, won't load the efifb driver, and so won't encounter the
> > problem. But we probably need to get Thomas Zimmerman to weigh in on
> > the implications of reverting c25a19afb81c.
> >
> > There's one additional variation of the problem. Assume the Hyper-V FB
> > driver is loaded (for example) during boot and moves the framebuffer. Then
> > system runs kexec as part of arming kdump during the boot sequence.
> > The most recent location of the framebuffer (and whether it is an EFI framebuffer)
> > gets picked at the time kexec runs, and is stored in the crash kernel memory area.
> > But what if the framebuffer later moves, perhaps because the Hyper-V FB driver
> > is unbound? The crash kernel memory area doesn’t get updated and kdump
> > could still have the wrong framebuffer address. This anomaly argues for the
> > commit 3cb73bc3fa2a approach of just ensuring that the efifb driver doesn't
> > load. Of course that approach means that the kdump kernel *must* contain
> > either the Hyper-V FB or Hyper-V DRM driver in order to work on a system
> > with only a framebuffer for text output. The efifb driver won't work. But
> > perhaps that's OK.
> >
> > Changing kexec (or the invoking script) to special case Hyper-V Gen 2 VMs and
> > always use kexec_load() instead of kexec_file_load() sounds like a big hack
> > to me. And with that approach, you give up the ability to enforce loading only
> > properly signed kdump images. This is something kexec_file_load() provides
> > that kexec_load() doesn't, and is one of the main reasons that kexec_file_load()
> > was added.
> >
> > Whether the kexec_file_load() system call could be enhanced to get the
> > frame buffer information from the /dev/fb0 device, I'm not sure. That might
> > be a reasonable approach, though it still has the problem that the framebuffer
> > address could change *after* kexec_file_load() runs.
> >
> > Anyway, that's a dump of my current thoughts. I haven't reached a final
> > conclusion or recommendation yet. Comments from others on the
> > thread are welcome.
>
> Hi!
>
> Asking because I also had to do some digging in this area:
>
> Do you think that the kernel can *ask* the hypervisor where the framebuffer is instead
> of relying on bios, the bootloader and/or kexec to somehow provide this information?
>
> If hyperv doesn't provide this API, how hard it would be in your opinion to provide it?
>
> I am asking because, I also had to debug a RHEL downstream issue where a slightly
> botched backport
> ensured that the first stage of the compressed uefi boot image, stopped passing the
> 'screen_info'
> to the second stage (the kernel itself), and as a result of this, the second stage stopped
> loading simplefb, and as a result of *this*, the PCI driver started to try to use the
> framebuffer
> range for its own use which failed and resulted in a cryptic error.
>
> If the kernel was to just issue some form of a hypercall to ask the hypervisor where the
> framebuffer currently is,
> we could avoid a whole class of bugs similar to this.
> What do you think?
>
I'm not aware of a way to ask Hyper-V about the framebuffer location.
I had not previously thought about such a possibility, so it's worth
thinking through. Here's how I see it: The issue is with generic drivers like
efifb (and others) that are hardcoded to read screen_info.lfb_base to
find the framebuffer. So the proposed new hypercall would need to be
made relatively early during boot, and it would update screen_info.lfb_base
to reflect the current location of the framebuffer. Hypercalls can only be
made after the setup in hyperv_init() is done. Fortunately, that's probably
before any framebuffer driver would read screen_info.lfb_base, though I'm
not completely sure.
Another factor is that the Hyper-V framebuffer is provided by the QEMU
equivalent that's embedded in the overall Hyper-V host, and not by
the hypervisor itself. The framebuffer is a VMBus device. So the Hyper-V
people would probably want getting the framebuffer location to be a
VMBus message to the framebuffer device, not a hypercall. And the VMBus
machinery isn't setup up until later -- too late, in fact, to change
screen_info.lfb_base before some generic driver reads it. So that's likely
to be a problem with the idea, though I'm speculating on what the
Hyper-V folks would say.
The last factor is getting Hyper-V to add the feature. Somebody on the
Microsoft side would need to carry that request to the Hyper-V team.
I'm former Microsoft, but retired 1+ years ago, so I'm now just an unpaid
hobbyist contributing to the kernel because I enjoy the challenge. :-) But
I no longer have the Microsoft insider connection to the Hyper-V team.
From my past experience, getting such features added is hard, and takes
a long time (years?) to get implemented and rolled out across the Azure
fleet, unless there's some critical issue that needs to be addressed. This
kdump problem probably doesn't reach that level of criticality.
So I'm not super optimistic about the idea. But maybe your thinking
is different from what I've laid out. I'm happy to hear further discussion.
Michael
^ permalink raw reply
* Re: (subset) [PATCH] backlight: led_bl: Hold led_access lock when calling led_sysfs_disable()
From: Lee Jones @ 2025-02-11 14:12 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Helge Deller,
Tony Lindgren, Pavel Machek, Tomi Valkeinen, Jean-Jacques Hiblot,
Herve Codina
Cc: Daniel Thompson, dri-devel, linux-fbdev, linux-kernel,
Luca Ceresoli, Thomas Petazzoni, stable
In-Reply-To: <20250122091914.309533-1-herve.codina@bootlin.com>
On Wed, 22 Jan 2025 10:19:14 +0100, Herve Codina wrote:
> Lockdep detects the following issue on led-backlight removal:
> [ 142.315935] ------------[ cut here ]------------
> [ 142.315954] WARNING: CPU: 2 PID: 292 at drivers/leds/led-core.c:455 led_sysfs_enable+0x54/0x80
> ...
> [ 142.500725] Call trace:
> [ 142.503176] led_sysfs_enable+0x54/0x80 (P)
> [ 142.507370] led_bl_remove+0x80/0xa8 [led_bl]
> [ 142.511742] platform_remove+0x30/0x58
> [ 142.515501] device_remove+0x54/0x90
> ...
>
> [...]
Applied, thanks!
[1/1] backlight: led_bl: Hold led_access lock when calling led_sysfs_disable()
commit: 276822a00db3c1061382b41e72cafc09d6a0ec30
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 10/13] leds: backlight trigger: Maintain global list of led backlight triggers
From: Lee Jones @ 2025-02-11 14:00 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: pavel, danielt, jingoohan1, deller, simona, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <20250206154033.697495-11-tzimmermann@suse.de>
On Thu, 06 Feb 2025, Thomas Zimmermann wrote:
> Maintain a list of led backlight triggers. This will replace the
> fbdev notifiers that all backlight triggers currently subscribe to.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/leds/trigger/ledtrig-backlight.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
> index 487577d22cfc..c1c1aa60cf07 100644
> --- a/drivers/leds/trigger/ledtrig-backlight.c
> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> @@ -23,8 +23,13 @@ struct bl_trig_notifier {
> int old_status;
> struct notifier_block notifier;
> unsigned invert;
> +
> + struct list_head entry;
You don't appear to be doing anything with the list here.
It would be better if you introduced the list when it's first utilised.
> };
>
> +static struct list_head ledtrig_backlight_list;
> +static struct mutex ledtrig_backlight_list_mutex;
> +
> static int fb_notifier_callback(struct notifier_block *p,
> unsigned long event, void *data)
> {
> @@ -118,6 +123,10 @@ static int bl_trig_activate(struct led_classdev *led)
> if (ret)
> dev_err(led->dev, "unable to register backlight trigger\n");
>
> + mutex_lock(&ledtrig_backlight_list_mutex);
> + list_add(&n->entry, &ledtrig_backlight_list);
> + mutex_unlock(&ledtrig_backlight_list_mutex);
> +
> return 0;
> }
>
> @@ -125,6 +134,10 @@ static void bl_trig_deactivate(struct led_classdev *led)
> {
> struct bl_trig_notifier *n = led_get_trigger_data(led);
>
> + mutex_lock(&ledtrig_backlight_list_mutex);
> + list_del(&n->entry);
> + mutex_unlock(&ledtrig_backlight_list_mutex);
> +
> fb_unregister_client(&n->notifier);
> kfree(n);
> }
> --
> 2.48.1
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call
From: Lee Jones @ 2025-02-11 13:57 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: pavel, danielt, jingoohan1, deller, simona, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <20250206154033.697495-13-tzimmermann@suse.de>
On Thu, 06 Feb 2025, Thomas Zimmermann wrote:
> Remove support for fb events from the led backlight trigger. Provide the
> helper ledtrig_backlight_blank() instead. Call it from fbdev to inform
> the trigger of changes to a display's blank state.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/leds/trigger/ledtrig-backlight.c | 31 +++++-------------------
> drivers/video/fbdev/core/fbmem.c | 21 +++++++++-------
> include/linux/leds.h | 6 +++++
> 3 files changed, 24 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
> index f9317f93483b..e3ae4adc29e3 100644
> --- a/drivers/leds/trigger/ledtrig-backlight.c
> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> @@ -10,7 +10,6 @@
> #include <linux/kernel.h>
> #include <linux/slab.h>
> #include <linux/init.h>
> -#include <linux/fb.h>
> #include <linux/leds.h>
> #include "../leds.h"
>
> @@ -21,7 +20,6 @@ struct bl_trig_notifier {
> struct led_classdev *led;
> int brightness;
> int old_status;
> - struct notifier_block notifier;
> unsigned invert;
>
> struct list_head entry;
> @@ -30,7 +28,7 @@ struct bl_trig_notifier {
> static struct list_head ledtrig_backlight_list;
> static struct mutex ledtrig_backlight_list_mutex;
>
> -static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
> +static void __ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
I'm confused. Didn't you just introduce this?
> {
> struct led_classdev *led = n->led;
> int new_status = !on ? BLANK : UNBLANK;
> @@ -48,23 +46,14 @@ static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
> n->old_status = new_status;
> }
>
> -static int fb_notifier_callback(struct notifier_block *p,
> - unsigned long event, void *data)
> +void ledtrig_backlight_blank(bool on)
> {
> - struct bl_trig_notifier *n = container_of(p,
> - struct bl_trig_notifier, notifier);
> - struct fb_event *fb_event = data;
> - int *blank;
> -
> - /* If we aren't interested in this event, skip it immediately ... */
> - if (event != FB_EVENT_BLANK)
> - return 0;
> -
> - blank = fb_event->data;
> + struct bl_trig_notifier *n;
>
> - ledtrig_backlight_blank(n, !blank[0]);
> + guard(mutex)(&ledtrig_backlight_list_mutex);
>
> - return 0;
> + list_for_each_entry(n, &ledtrig_backlight_list, entry)
> + __ledtrig_backlight_blank(n, on);
> }
>
> static ssize_t bl_trig_invert_show(struct device *dev,
> @@ -110,8 +99,6 @@ ATTRIBUTE_GROUPS(bl_trig);
>
> static int bl_trig_activate(struct led_classdev *led)
> {
> - int ret;
> -
> struct bl_trig_notifier *n;
>
> n = kzalloc(sizeof(struct bl_trig_notifier), GFP_KERNEL);
> @@ -122,11 +109,6 @@ static int bl_trig_activate(struct led_classdev *led)
> n->led = led;
> n->brightness = led->brightness;
> n->old_status = UNBLANK;
> - n->notifier.notifier_call = fb_notifier_callback;
> -
> - ret = fb_register_client(&n->notifier);
> - if (ret)
> - dev_err(led->dev, "unable to register backlight trigger\n");
>
> mutex_lock(&ledtrig_backlight_list_mutex);
> list_add(&n->entry, &ledtrig_backlight_list);
> @@ -143,7 +125,6 @@ static void bl_trig_deactivate(struct led_classdev *led)
> list_del(&n->entry);
> mutex_unlock(&ledtrig_backlight_list_mutex);
>
> - fb_unregister_client(&n->notifier);
> kfree(n);
> }
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index fb7ca143a996..92c3fe2a7aff 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -16,6 +16,7 @@
> #include <linux/fb.h>
> #include <linux/fbcon.h>
> #include <linux/lcd.h>
> +#include <linux/leds.h>
>
> #include <video/nomodeset.h>
>
> @@ -373,11 +374,19 @@ static void fb_lcd_notify_blank(struct fb_info *info)
> #endif
> }
>
> +static void fb_ledtrig_backlight_notify_blank(struct fb_info *info)
> +{
> +#if IS_REACHABLE(CONFIG_LEDS_TRIGGER_BACKLIGHT)
#iferry is generally discouraged in C files.
Does is_defined() work for you?
/
> + if (info->blank == FB_BLANK_UNBLANK)
> + ledtrig_backlight_blank(true);
If !CONFIG_LEDS_TRIGGER_BACKLIGHT(), then ledtrig_backlight_blank() is a
noop anyway, right? So why encompass it in the #if at all?
> + else
> + ledtrig_backlight_blank(false);
> +#endif
> +}
> +
> int fb_blank(struct fb_info *info, int blank)
> {
> int old_blank = info->blank;
> - struct fb_event event;
> - int data[2];
> int ret;
>
> if (!info->fbops->fb_blank)
> @@ -386,11 +395,6 @@ int fb_blank(struct fb_info *info, int blank)
> if (blank > FB_BLANK_POWERDOWN)
> blank = FB_BLANK_POWERDOWN;
>
> - data[0] = blank;
> - data[1] = old_blank;
> - event.info = info;
> - event.data = data;
> -
> info->blank = blank;
>
> ret = info->fbops->fb_blank(blank, info);
> @@ -399,8 +403,7 @@ int fb_blank(struct fb_info *info, int blank)
>
> fb_bl_notify_blank(info, old_blank);
> fb_lcd_notify_blank(info);
> -
> - fb_notifier_call_chain(FB_EVENT_BLANK, &event);
> + fb_ledtrig_backlight_notify_blank(info);
>
> return 0;
>
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 98f9719c924c..8c7c41888f7d 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -640,6 +640,12 @@ static inline void ledtrig_flash_ctrl(bool on) {}
> static inline void ledtrig_torch_ctrl(bool on) {}
> #endif
>
> +#if IS_ENABLED(CONFIG_LEDS_TRIGGER_BACKLIGHT)
> +void ledtrig_backlight_blank(bool on);
> +#else
> +static inline void ledtrig_backlight_blank(bool on) {}
> +#endif
> +
> /*
> * Generic LED platform data for describing LED names and default triggers.
> */
> --
> 2.48.1
>
--
/fb_ledtrig_backlight_notify_blankLee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 00/16] backlight: Do not include <linux/fb.h> in drivers
From: Lee Jones @ 2025-02-11 10:20 UTC (permalink / raw)
To: lee, danielt, jingoohan1, michael.hennerich, support.opensource,
Thomas Zimmermann
Cc: dri-devel, linux-fbdev
In-Reply-To: <20250131140103.278158-1-tzimmermann@suse.de>
On Fri, 31 Jan 2025 14:58:31 +0100, Thomas Zimmermann wrote:
> A number of backlight drivers include <linux/fb.h>. None of them
> requires anything from the header file, so remove the include
> statements.
>
> Thomas Zimmermann (16):
> backlight: 88pm860x_bl: Do not include <linux/fb.h>
> backlight: adp5520_bl: Do not include <linux/fb.h>
> backlight: adp8860_bl: Do not include <linux/fb.h>
> backlight: adp8870_bl: Do not include <linux/fb.h>
> backlight: as3711_bl: Do not include <linux/fb.h>
> backlight: bd6107_bl: Do not include <linux/fb.h>
> backlight: da903x_bl: Do not include <linux/fb.h>
> backlight: da9052_bl: Do not include <linux/fb.h>
> backlight: ep93xx_bl: Do not include <linux/fb.h>
> backlight: hp680_bl: Do not include <linux/fb.h>
> backlight: locomolcd: Do not include <linux/fb.h>
> backlight: lv5207lp: Do not include <linux/fb.h>
> backlight: max8925_bl: Do not include <linux/fb.h>
> backlight: tps65217_bl: Do not include <linux/fb.h>
> backlight: vgg2432a4: Do not include <linux/fb.h>
> backlight: wm831x_bl: Do not include <linux/fb.h>
>
> [...]
Applied, thanks!
[01/16] backlight: 88pm860x_bl: Do not include <linux/fb.h>
commit: 9df4477179f1af4ff7adbacfa243819b57134b9c
[02/16] backlight: adp5520_bl: Do not include <linux/fb.h>
commit: 769562042211e7a194e3dfde9436b42a3734e279
[03/16] backlight: adp8860_bl: Do not include <linux/fb.h>
commit: a84877d7cc5b38dbbaad94ea7f8a784f8b12dbc8
[04/16] backlight: adp8870_bl: Do not include <linux/fb.h>
commit: 1eeab5c83aa3c14790167c4d2b8786b257651bac
[05/16] backlight: as3711_bl: Do not include <linux/fb.h>
commit: 9800ca9c96bc039a5e19391c446d1d69b211756a
[06/16] backlight: bd6107_bl: Do not include <linux/fb.h>
commit: b6c775af0d2f1cf9376261180ce13e997dba583b
[07/16] backlight: da903x_bl: Do not include <linux/fb.h>
commit: 68d112e043a4e9a5078eebc9302d5510d458681e
[08/16] backlight: da9052_bl: Do not include <linux/fb.h>
commit: d670a4da1c60ab1004b79204c09b221eba8af93b
[09/16] backlight: ep93xx_bl: Do not include <linux/fb.h>
commit: fcb0283338d760d37cd2701f6cd2bba0f5e78eb2
[10/16] backlight: hp680_bl: Do not include <linux/fb.h>
commit: df14455987587fb5373eb216511e0f3ab24c5480
[11/16] backlight: locomolcd: Do not include <linux/fb.h>
commit: 5f02729fadee6dedb638cdb8244f9447c8ad4ef9
[12/16] backlight: lv5207lp: Do not include <linux/fb.h>
commit: 8c71b34c636cabb4101098cc6bb619ded9b0905f
[13/16] backlight: max8925_bl: Do not include <linux/fb.h>
commit: aa021f33d2cb5061a2a02e1a3c8faea3a3d2f844
[14/16] backlight: tps65217_bl: Do not include <linux/fb.h>
commit: d520ae4707fd6dafcb55649460059f67f54fc743
[15/16] backlight: vgg2432a4: Do not include <linux/fb.h>
commit: d023cc09d9dbd5c6a4a81e0e3866c3b976d70891
[16/16] backlight: wm831x_bl: Do not include <linux/fb.h>
commit: 373dacfeb55e1ac73dccd91b83437183ca0fbd43
--
Lee Jones [李琼斯]
^ permalink raw reply
* RE: [PATCH 1/1] fbdev: hyperv_fb: iounmap() the correct memory when removing a device
From: Michael Kelley @ 2025-02-11 3:51 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
deller@gmx.de, weh@microsoft.com, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org
In-Reply-To: <20250210165221.GA3465@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Monday, February 10, 2025 8:52 AM
>
> On Mon, Feb 10, 2025 at 06:58:25AM -0800, Saurabh Singh Sengar wrote:
> > On Mon, Feb 10, 2025 at 02:28:35PM +0000, Michael Kelley wrote:
> > > From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Monday, February 10, 2025 4:41 AM
> > > >
> > > > On Sun, Feb 09, 2025 at 03:52:52PM -0800, mhkelley58@gmail.com wrote:
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > >
> > > > > When a Hyper-V framebuffer device is removed, or the driver is unbound
> > > > > from a device, any allocated and/or mapped memory must be released. In
> > > > > particular, MMIO address space that was mapped to the framebuffer must
> > > > > be unmapped. Current code unmaps the wrong address, resulting in an
> > > > > error like:
> > > > >
> > > > > [ 4093.980597] iounmap: bad address 00000000c936c05c
> > > > >
> > > > > followed by a stack dump.
> > > > >
> > > > > Commit d21987d709e8 ("video: hyperv: hyperv_fb: Support deferred IO for
> > > > > Hyper-V frame buffer driver") changed the kind of address stored in
> > > > > info->screen_base, and the iounmap() call in hvfb_putmem() was not
> > > > > updated accordingly.
> > > > >
> > > > > Fix this by updating hvfb_putmem() to unmap the correct address.
> > > > >
> > > > > Fixes: d21987d709e8 ("video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver")
> > > > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > > > ---
> > > > > drivers/video/fbdev/hyperv_fb.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> > > > > index 7fdb5edd7e2e..363e4ccfcdb7 100644
> > > > > --- a/drivers/video/fbdev/hyperv_fb.c
> > > > > +++ b/drivers/video/fbdev/hyperv_fb.c
> > > > > @@ -1080,7 +1080,7 @@ static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
> > > > >
> > > > > if (par->need_docopy) {
> > > > > vfree(par->dio_vp);
> > > > > - iounmap(info->screen_base);
> > > > > + iounmap(par->mmio_vp);
> > > > > vmbus_free_mmio(par->mem->start, screen_fb_size);
> > > > > } else {
> > > > > hvfb_release_phymem(hdev, info->fix.smem_start,
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > > Thanks for fixing this:
> > > > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > >
> > > >
> > > > While we are at it, I want to mention that I also observed below WARN
> > > > while removing the hyperv_fb, but that needs a separate fix.
> > > >
> > > >
> > > > [ 44.111220] WARNING: CPU: 35 PID: 1882 at drivers/video/fbdev/core/fb_info.c:70 framebuffer_release+0x2c/0x40
> > > > < snip >
> > > > [ 44.111289] Call Trace:
> > > > [ 44.111290] <TASK>
> > > > [ 44.111291] ? show_regs+0x6c/0x80
> > > > [ 44.111295] ? __warn+0x8d/0x150
> > > > [ 44.111298] ? framebuffer_release+0x2c/0x40
> > > > [ 44.111300] ? report_bug+0x182/0x1b0
> > > > [ 44.111303] ? handle_bug+0x6e/0xb0
> > > > [ 44.111306] ? exc_invalid_op+0x18/0x80
> > > > [ 44.111308] ? asm_exc_invalid_op+0x1b/0x20
> > > > [ 44.111311] ? framebuffer_release+0x2c/0x40
> > > > [ 44.111313] ? hvfb_remove+0x86/0xa0 [hyperv_fb]
> > > > [ 44.111315] vmbus_remove+0x24/0x40 [hv_vmbus]
> > > > [ 44.111323] device_remove+0x40/0x80
> > > > [ 44.111325] device_release_driver_internal+0x20b/0x270
> > > > [ 44.111327] ? bus_find_device+0xb3/0xf0
> > > >
> > >
> > > Thanks for pointing this out. Interestingly, I'm not seeing this WARN
> > > in my experiments. What base kernel are you testing with? Are you
> > > testing on a local VM or in Azure? What exactly are you doing
> > > to create the problem? I've been doing unbind of the driver,
> > > but maybe you are doing something different.
> > >
> > > FWIW, there is yet another issue where after doing two unbind/bind
> > > cycles of the hyperv_fb driver, there's an error about freeing a
> > > non-existent resource. I know what that problem is, and it's in
> > > vmbus_drv.c. I'll be submitting a patch for that as soon as I figure out
> > > a clean fix.
> > >
> > > Michael
> >
> > This is on local Hyper-V. Kernel: 6.14.0-rc1-next-20250205+
> > I run below command to reproduce the above error:
> > echo "5620e0c7-8062-4dce-aeb7-520c7ef76171" >/sys/bus/vmbus/devices/5620e0c7-8062-4dce-aeb-520c7ef76171/driver/unbind
> >
> > When hvfb_remove is called I can see the refcount for framebuffer is 2 when ,
> > I expect it to be 1. After unregistering this framebuffer there is still 1 refcount
> > remains, which is the reason for this WARN at the time of framebuffer_release.
> >
> > I wonder who is registering/using this extra framebuffer. Its not hyperv_drm or
> > hyperv_fb IIUC.
> >
> > - Saurabh
>
> Here are more details about this WARN:
>
> Xorg opens `/dev/fb0`, which increases the framebuffer's reference
> count, as mentioned above. As a result, when unbinding the driver,
> this WARN is expected, indicating that the framebuffer is still in use.
>
> I am open to suggestion what could be the correct behavior in this case.
> There acan be two possible options:
>
> 1. Check the framebuffer reference count and prevent the driver from
> unbinding/removal.
> OR
>
> 2. Allow the driver to unbind while issuing this WARN. (Current scenario)
OK, that makes sense. I haven't looked at or thought about this issue any
further today, and don't have an opinion yet. Give me a day or two -- I have
one more patch to post related to the FB and DRM driver problems.
Michael
>
> - Saurabh
^ permalink raw reply
* Re: hyper_bf soft lockup on Azure Gen2 VM when taking kdump or executing kexec
From: Maxim Levitsky @ 2025-02-10 23:56 UTC (permalink / raw)
To: Michael Kelley, thomas.tai@oracle.com, mhkelley58@gmail.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
drawat.floss@gmail.com, javierm@redhat.com, Helge Deller,
daniel@ffwll.ch, airlied@gmail.com, tzimmermann@suse.de
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <SN6PR02MB41574DC1576DC20772D9CB05D4F22@SN6PR02MB4157.namprd02.prod.outlook.com>
On Mon, 2025-02-10 at 21:35 +0000, Michael Kelley wrote:
> From: thomas.tai@oracle.com <thomas.tai@oracle.com> Sent: Monday, February 10, 2025 7:08 AM
> > <snip>
> >
> > > > Then the question is why the efifb driver doesn't work in the kdump
> > > > kernel. Actually, it *does* work in many cases. I built the 6.13.0 kernel
> > > > on the Oracle Linux 9.4 system, and transferred the kernel image binary
> > > > and module binaries to an Ubuntu 20.04 VM in Azure. In that VM, the
> > > > efifb driver is loaded as part of the kdump kernel, and it doesn't cause
> > > > any problems. But there's an interesting difference. In the Oracle Linux
> > > > 9.4 VM, the efifb driver finds the framebuffer at 0x40000000, while on
> > > > the Ubuntu 20.04 VM, it finds the framebuffer at 0x40900000. This
> > > > difference is due to differences in how the screen_info variable gets
> > > > setup in the two VMs.
> > > >
> > > > When the normal kernel starts in a freshly booted VM, Hyper-V provides
> > > > the EFI framebuffer at 0x40000000, and it works. But after the Hyper-V
> > > > FB driver or Hyper-V DRM driver has initialized, Linux has picked a
> > > > different MMIO address range and told Hyper-V to use the new
> > > > address range (which often starts at 0x40900000). A kexec does *not*
> > > > reset Hyper-V's transition to the new range, so when the efifb driver
> > > > tries to use the framebuffer at 0x40000000, the accesses trap to
> > > > Hyper-V and probably fail or timeout (I'm not sure of the details). After
> > > > the guest does some number of these bad references, Hyper-V considers
> > > > itself to be under attack from an ill-behaving guest, and throttles the
> > > > guest so that it doesn't run for a few seconds. The throttling repeats,
> > > > and results in extremely slow running in the kdump kernel.
> > > >
> > > > Somehow in the Ubuntu 20.04 VM, the location of the frame buffer
> > > > as stored in screen_info.lfb_base gets updated to be 0x40900000. I
> > > > haven't fully debugged how that happens. But with that update, the
> > > > efifb driver is using the updated framebuffer address and it works. On
> > > > the Oracle Linux 9.4 system, that update doesn't appear to happen,
> > > > and the problem occurs.
> > > >
> > > > This in an interim update on the problem. I'm still investigating how
> > > > screen_info.lfb_base is set in the kdump kernel, and why it is different
> > > > in the Ubuntu 20.04 VM vs. in the Oracle Linux 9.4 VM. Once that is
> > > > well understood, we can contemplate how to fix the problem. Undoing
> > > > the revert that is commit 2bebc3cd4870 doesn't seem like the solution
> > > > since the original code there was reported to cause many other issues.
> > > > The solution focus will likely be on how to ensure the kdump kernel gets
> > > > the correct framebuffer address so the efifb driver works, since the
> > > > framebuffer address changing is a quirk of Hyper-V behavior.
> > > >
> > > > If anyone else has insight into what's going on here, please chime in.
> > > > What I've learned so far is still somewhat tentative.
> > > >
> > > Here's what is happening. On Ubuntu 20.04, the kdump image is
> > > loaded into crash memory using the kexec command. Ubuntu 20.04
> > > has kexec from the kexec-tools package version 2.0.18-1ubuntu1.1,
> > > and per the kexec man page, it defaults to using the older kexec_load()
> > > system call. When using kexec_load(), the contents to be loaded into
> > > crash memory is constructed in user space by the kexec command.
> > > The kexec command gets the "screen_info" settings, including the
> > > physical address of the frame buffer, via the FBIOGET_FSCREENINFO
> > > ioctl against /dev/fb0. The Hyper-V FB or DRM driver registers itself
> > > with the fbdev subsystem so that it is /dev/fb0, and the ioctl returns
> > > the updated framebuffer address. So the efifb driver loads and runs
> > > correctly.
> > >
> > > On Oracle Linux 9.4, the kdump image is also loaded with the
> > > kexec command, but from kexec-tools package version
> > > kexec-tools-2.0.28-1.0.10.el9_5.x86_64, which is slightly later than
> > > the version on Ubuntu 20.04. This newer kexec defaults to using the
> > > newer kexec_file_load() system call. This system call gets the
> > > framebuffer address from the screen_info variable in the kernel, which
> > > has not been updated to reflect the new framebuffer address. Hence
> > > in the kdump kernel, the efifb driver uses the old framebuffer address,
> > > and hence the problem.
> > >
> > > To further complicate matters, the kexec on Oracle Linux 9.4 seems to
> > > have a bug when the -c option forces the use of kexec_load() instead
> > > of kexec_file_load(). As an experiment, I modified the kdumpctl shell
> > > script to add the "-c" option to kexec, but in that case the value "0x0"
> > > is passed as the framebuffer address, which is wrong. Furthermore,
> > > the " screen_info.orig_video_isVGA" value (which I mentioned earlier
> > > in connection with commit 2bebc3cd4870) is also set to 0, so the
> > > kdump kernel no longer thinks it has an EFI framebuffer. Hence the
> > > efifb driver isn't loaded, and the kdump works, though for the wrong
> > > reasons. If kexec 2.0.18 from Ubuntu is copied onto the Oracle Linux 9.4
> > > VM, then kdump works as expected, with the efifb driver being loaded
> > > and using the correct framebuffer address. So something is going wrong
> > > with kexec 2.0.28 in how it sets up the screen_info when the -c option
> > > is used. I'll leave the debugging of the kexec bug to someone else.
> >
> > Hi Michael,
> >
> > Do you think we need to handle Azure Gen2 VM differently in the kexec?
> >
> > Or should we change the kexec_file_load() system call to retrieve the correct
> > framebuffer address?
>
> I'm thinking there may be a fix in the Hyper-V FB and Hyper-V DRM drivers.
> Commit c25a19afb81c may also be a cause of the problem -- see precursor
> commit 3cb73bc3fa2a, which describes exactly the problem. I still need to
> do some testing, but without that commit, kdump won't detect that it has
> an EFI framebuffer, won't load the efifb driver, and so won't encounter the
> problem. But we probably need to get Thomas Zimmerman to weigh in on
> the implications of reverting c25a19afb81c.
>
> There's one additional variation of the problem. Assume the Hyper-V FB
> driver is loaded (for example) during boot and moves the framebuffer. Then
> system runs kexec as part of arming kdump during the boot sequence.
> The most recent location of the framebuffer (and whether it is an EFI framebuffer)
> gets picked at the time kexec runs, and is stored in the crash kernel memory area.
> But what if the framebuffer later moves, perhaps because the Hyper-V FB driver
> is unbound? The crash kernel memory area doesn’t get updated and kdump
> could still have the wrong framebuffer address. This anomaly argues for the
> commit 3cb73bc3fa2a approach of just ensuring that the efifb driver doesn't
> load. Of course that approach means that the kdump kernel *must* contain
> either the Hyper-V FB or Hyper-V DRM driver in order to work on a system
> with only a framebuffer for text output. The efifb driver won't work. But
> perhaps that's OK.
>
> Changing kexec (or the invoking script) to special case Hyper-V Gen 2 VMs and
> always use kexec_load() instead of kexec_file_load() sounds like a big hack
> to me. And with that approach, you give up the ability to enforce loading only
> properly signed kdump images. This is something kexec_file_load() provides
> that kexec_load() doesn't, and is one of the main reasons that kexec_file_load()
> was added.
>
> Whether the kexec_file_load() system call could be enhanced to get the
> frame buffer information from the /dev/fb0 device, I'm not sure. That might
> be a reasonable approach, though it still has the problem that the framebuffer
> address could change *after* kexec_file_load() runs.
>
> Anyway, that's a dump of my current thoughts. I haven't reached a final
> conclusion or recommendation yet. Comments from others on the
> thread are welcome.
Hi!
Asking because I also had to do some digging in this area:
Do you think that the kernel can *ask* the hypervisor where the framebuffer is instead
of relying on bios, the bootloader and/or kexec to somehow provide this information?
If hyperv doesn't provide this API, how hard it would be in your opinion to provide it?
I am asking because, I also had to debug a RHEL downstream issue where a slightly botched backport
ensured that the first stage of the compressed uefi boot image, stopped passing the 'screen_info'
to the second stage (the kernel itself), and as a result of this, the second stage stopped
loading simplefb, and as a result of *this*, the PCI driver started to try to use the framebuffer
range for its own use which failed and resulted in a cryptic error.
If the kernel was to just issue some form of a hypercall to ask the hypervisor where the framebuffer currently is,
we could avoid a whole class of bugs similar to this.
What do you think?
Best regards,
Maxim Levitsky
>
> Michael
>
> > Thank you,
> > Thomas
> >
> > > I'm still thinking about alternatives to fix this mess. Please chime
> > > in if you have suggestions.
> > >
> > > Michael
^ permalink raw reply
* RE: hyper_bf soft lockup on Azure Gen2 VM when taking kdump or executing kexec
From: Michael Kelley @ 2025-02-10 21:35 UTC (permalink / raw)
To: thomas.tai@oracle.com, mhkelley58@gmail.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
drawat.floss@gmail.com, javierm@redhat.com, Helge Deller,
daniel@ffwll.ch, airlied@gmail.com, tzimmermann@suse.de
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <edf0e21a-db9f-42a8-ae0f-76a9d93713fb@oracle.com>
From: thomas.tai@oracle.com <thomas.tai@oracle.com> Sent: Monday, February 10, 2025 7:08 AM
>
> <snip>
>
> >> Then the question is why the efifb driver doesn't work in the kdump
> >> kernel. Actually, it *does* work in many cases. I built the 6.13.0 kernel
> >> on the Oracle Linux 9.4 system, and transferred the kernel image binary
> >> and module binaries to an Ubuntu 20.04 VM in Azure. In that VM, the
> >> efifb driver is loaded as part of the kdump kernel, and it doesn't cause
> >> any problems. But there's an interesting difference. In the Oracle Linux
> >> 9.4 VM, the efifb driver finds the framebuffer at 0x40000000, while on
> >> the Ubuntu 20.04 VM, it finds the framebuffer at 0x40900000. This
> >> difference is due to differences in how the screen_info variable gets
> >> setup in the two VMs.
> >>
> >> When the normal kernel starts in a freshly booted VM, Hyper-V provides
> >> the EFI framebuffer at 0x40000000, and it works. But after the Hyper-V
> >> FB driver or Hyper-V DRM driver has initialized, Linux has picked a
> >> different MMIO address range and told Hyper-V to use the new
> >> address range (which often starts at 0x40900000). A kexec does *not*
> >> reset Hyper-V's transition to the new range, so when the efifb driver
> >> tries to use the framebuffer at 0x40000000, the accesses trap to
> >> Hyper-V and probably fail or timeout (I'm not sure of the details). After
> >> the guest does some number of these bad references, Hyper-V considers
> >> itself to be under attack from an ill-behaving guest, and throttles the
> >> guest so that it doesn't run for a few seconds. The throttling repeats,
> >> and results in extremely slow running in the kdump kernel.
> >>
> >> Somehow in the Ubuntu 20.04 VM, the location of the frame buffer
> >> as stored in screen_info.lfb_base gets updated to be 0x40900000. I
> >> haven't fully debugged how that happens. But with that update, the
> >> efifb driver is using the updated framebuffer address and it works. On
> >> the Oracle Linux 9.4 system, that update doesn't appear to happen,
> >> and the problem occurs.
> >>
> >> This in an interim update on the problem. I'm still investigating how
> >> screen_info.lfb_base is set in the kdump kernel, and why it is different
> >> in the Ubuntu 20.04 VM vs. in the Oracle Linux 9.4 VM. Once that is
> >> well understood, we can contemplate how to fix the problem. Undoing
> >> the revert that is commit 2bebc3cd4870 doesn't seem like the solution
> >> since the original code there was reported to cause many other issues.
> >> The solution focus will likely be on how to ensure the kdump kernel gets
> >> the correct framebuffer address so the efifb driver works, since the
> >> framebuffer address changing is a quirk of Hyper-V behavior.
> >>
> >> If anyone else has insight into what's going on here, please chime in.
> >> What I've learned so far is still somewhat tentative.
> >>
> > Here's what is happening. On Ubuntu 20.04, the kdump image is
> > loaded into crash memory using the kexec command. Ubuntu 20.04
> > has kexec from the kexec-tools package version 2.0.18-1ubuntu1.1,
> > and per the kexec man page, it defaults to using the older kexec_load()
> > system call. When using kexec_load(), the contents to be loaded into
> > crash memory is constructed in user space by the kexec command.
> > The kexec command gets the "screen_info" settings, including the
> > physical address of the frame buffer, via the FBIOGET_FSCREENINFO
> > ioctl against /dev/fb0. The Hyper-V FB or DRM driver registers itself
> > with the fbdev subsystem so that it is /dev/fb0, and the ioctl returns
> > the updated framebuffer address. So the efifb driver loads and runs
> > correctly.
> >
> > On Oracle Linux 9.4, the kdump image is also loaded with the
> > kexec command, but from kexec-tools package version
> > kexec-tools-2.0.28-1.0.10.el9_5.x86_64, which is slightly later than
> > the version on Ubuntu 20.04. This newer kexec defaults to using the
> > newer kexec_file_load() system call. This system call gets the
> > framebuffer address from the screen_info variable in the kernel, which
> > has not been updated to reflect the new framebuffer address. Hence
> > in the kdump kernel, the efifb driver uses the old framebuffer address,
> > and hence the problem.
> >
> > To further complicate matters, the kexec on Oracle Linux 9.4 seems to
> > have a bug when the -c option forces the use of kexec_load() instead
> > of kexec_file_load(). As an experiment, I modified the kdumpctl shell
> > script to add the "-c" option to kexec, but in that case the value "0x0"
> > is passed as the framebuffer address, which is wrong. Furthermore,
> > the " screen_info.orig_video_isVGA" value (which I mentioned earlier
> > in connection with commit 2bebc3cd4870) is also set to 0, so the
> > kdump kernel no longer thinks it has an EFI framebuffer. Hence the
> > efifb driver isn't loaded, and the kdump works, though for the wrong
> > reasons. If kexec 2.0.18 from Ubuntu is copied onto the Oracle Linux 9.4
> > VM, then kdump works as expected, with the efifb driver being loaded
> > and using the correct framebuffer address. So something is going wrong
> > with kexec 2.0.28 in how it sets up the screen_info when the -c option
> > is used. I'll leave the debugging of the kexec bug to someone else.
>
> Hi Michael,
>
> Do you think we need to handle Azure Gen2 VM differently in the kexec?
>
> Or should we change the kexec_file_load() system call to retrieve the correct
> framebuffer address?
I'm thinking there may be a fix in the Hyper-V FB and Hyper-V DRM drivers.
Commit c25a19afb81c may also be a cause of the problem -- see precursor
commit 3cb73bc3fa2a, which describes exactly the problem. I still need to
do some testing, but without that commit, kdump won't detect that it has
an EFI framebuffer, won't load the efifb driver, and so won't encounter the
problem. But we probably need to get Thomas Zimmerman to weigh in on
the implications of reverting c25a19afb81c.
There's one additional variation of the problem. Assume the Hyper-V FB
driver is loaded (for example) during boot and moves the framebuffer. Then
system runs kexec as part of arming kdump during the boot sequence.
The most recent location of the framebuffer (and whether it is an EFI framebuffer)
gets picked at the time kexec runs, and is stored in the crash kernel memory area.
But what if the framebuffer later moves, perhaps because the Hyper-V FB driver
is unbound? The crash kernel memory area doesn’t get updated and kdump
could still have the wrong framebuffer address. This anomaly argues for the
commit 3cb73bc3fa2a approach of just ensuring that the efifb driver doesn't
load. Of course that approach means that the kdump kernel *must* contain
either the Hyper-V FB or Hyper-V DRM driver in order to work on a system
with only a framebuffer for text output. The efifb driver won't work. But
perhaps that's OK.
Changing kexec (or the invoking script) to special case Hyper-V Gen 2 VMs and
always use kexec_load() instead of kexec_file_load() sounds like a big hack
to me. And with that approach, you give up the ability to enforce loading only
properly signed kdump images. This is something kexec_file_load() provides
that kexec_load() doesn't, and is one of the main reasons that kexec_file_load()
was added.
Whether the kexec_file_load() system call could be enhanced to get the
frame buffer information from the /dev/fb0 device, I'm not sure. That might
be a reasonable approach, though it still has the problem that the framebuffer
address could change *after* kexec_file_load() runs.
Anyway, that's a dump of my current thoughts. I haven't reached a final
conclusion or recommendation yet. Comments from others on the
thread are welcome.
Michael
>
> Thank you,
> Thomas
>
> > I'm still thinking about alternatives to fix this mess. Please chime
> > in if you have suggestions.
> >
> > Michael
^ permalink raw reply
* Re: [PATCH v4 1/9] dt-bindings: iio: dac: adi-axi-adc: add ad7606 variant
From: David Lechner @ 2025-02-10 17:46 UTC (permalink / raw)
To: Rob Herring (Arm), Angelo Dureghello
Cc: Michael Hennerich, linux-fbdev, linux-iio, Guillaume Stols,
Krzysztof Kozlowski, Lars-Peter Clausen, Jonathan Cameron,
Alexandru Ardelean, Conor Dooley, Jonathan Cameron, linux-kernel,
devicetree
In-Reply-To: <173920794511.669517.12561205201983200892.robh@kernel.org>
On 2/10/25 11:19 AM, Rob Herring (Arm) wrote:
>
> On Mon, 10 Feb 2025 17:10:51 +0100, Angelo Dureghello wrote:
>> From: Guillaume Stols <gstols@baylibre.com>
>>
>> A new compatible is added to reflect the specialized version of the HDL.
>> We use the parallel interface to write the ADC's registers, and
>> accessing this interface requires to use ADI_AXI_REG_CONFIG_RD,
>> ADI_AXI_REG_CONFIG_WR and ADI_AXI_REG_CONFIG_CTRL in a custom fashion.
>>
>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
>> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
>> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
>> ---
>> .../devicetree/bindings/iio/adc/adi,axi-adc.yaml | 70 +++++++++++++++++++++-
>> 1 file changed, 69 insertions(+), 1 deletion(-)
>>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: adc@0: pwm-names: ['convst1'] is too short
> from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
Caused by missing dependency from v3 that Jonathan already picked up.
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=fixes-togreg&id=02ccd7e5d81af4ae20852fc1ad67e7d943fa5778
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250210-wip-bl-ad7606_add_backend_sw_mode-v4-1-160df18b1da7@baylibre.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
^ permalink raw reply
* Re: [PATCH v4 1/9] dt-bindings: iio: dac: adi-axi-adc: add ad7606 variant
From: Rob Herring (Arm) @ 2025-02-10 17:19 UTC (permalink / raw)
To: Angelo Dureghello
Cc: David Lechner, Michael Hennerich, linux-fbdev, linux-iio,
Guillaume Stols, Krzysztof Kozlowski, Lars-Peter Clausen,
Jonathan Cameron, Alexandru Ardelean, Conor Dooley,
Jonathan Cameron, linux-kernel, devicetree
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-1-160df18b1da7@baylibre.com>
On Mon, 10 Feb 2025 17:10:51 +0100, Angelo Dureghello wrote:
> From: Guillaume Stols <gstols@baylibre.com>
>
> A new compatible is added to reflect the specialized version of the HDL.
> We use the parallel interface to write the ADC's registers, and
> accessing this interface requires to use ADI_AXI_REG_CONFIG_RD,
> ADI_AXI_REG_CONFIG_WR and ADI_AXI_REG_CONFIG_CTRL in a custom fashion.
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> .../devicetree/bindings/iio/adc/adi,axi-adc.yaml | 70 +++++++++++++++++++++-
> 1 file changed, 69 insertions(+), 1 deletion(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.example.dtb: adc@0: pwm-names: ['convst1'] is too short
from schema $id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250210-wip-bl-ad7606_add_backend_sw_mode-v4-1-160df18b1da7@baylibre.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH 1/1] fbdev: hyperv_fb: iounmap() the correct memory when removing a device
From: Saurabh Singh Sengar @ 2025-02-10 16:52 UTC (permalink / raw)
To: Michael Kelley
Cc: haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
deller@gmx.de, weh@microsoft.com, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org
In-Reply-To: <20250210145825.GA12377@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Mon, Feb 10, 2025 at 06:58:25AM -0800, Saurabh Singh Sengar wrote:
> On Mon, Feb 10, 2025 at 02:28:35PM +0000, Michael Kelley wrote:
> > From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Monday, February 10, 2025 4:41 AM
> > >
> > > On Sun, Feb 09, 2025 at 03:52:52PM -0800, mhkelley58@gmail.com wrote:
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > >
> > > > When a Hyper-V framebuffer device is removed, or the driver is unbound
> > > > from a device, any allocated and/or mapped memory must be released. In
> > > > particular, MMIO address space that was mapped to the framebuffer must
> > > > be unmapped. Current code unmaps the wrong address, resulting in an
> > > > error like:
> > > >
> > > > [ 4093.980597] iounmap: bad address 00000000c936c05c
> > > >
> > > > followed by a stack dump.
> > > >
> > > > Commit d21987d709e8 ("video: hyperv: hyperv_fb: Support deferred IO for
> > > > Hyper-V frame buffer driver") changed the kind of address stored in
> > > > info->screen_base, and the iounmap() call in hvfb_putmem() was not
> > > > updated accordingly.
> > > >
> > > > Fix this by updating hvfb_putmem() to unmap the correct address.
> > > >
> > > > Fixes: d21987d709e8 ("video: hyperv: hyperv_fb: Support deferred IO for Hyper-V frame buffer driver")
> > > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > > ---
> > > > drivers/video/fbdev/hyperv_fb.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> > > > index 7fdb5edd7e2e..363e4ccfcdb7 100644
> > > > --- a/drivers/video/fbdev/hyperv_fb.c
> > > > +++ b/drivers/video/fbdev/hyperv_fb.c
> > > > @@ -1080,7 +1080,7 @@ static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info)
> > > >
> > > > if (par->need_docopy) {
> > > > vfree(par->dio_vp);
> > > > - iounmap(info->screen_base);
> > > > + iounmap(par->mmio_vp);
> > > > vmbus_free_mmio(par->mem->start, screen_fb_size);
> > > > } else {
> > > > hvfb_release_phymem(hdev, info->fix.smem_start,
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > Thanks for fixing this:
> > > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > >
> > >
> > > While we are at it, I want to mention that I also observed below WARN
> > > while removing the hyperv_fb, but that needs a separate fix.
> > >
> > >
> > > [ 44.111220] WARNING: CPU: 35 PID: 1882 at drivers/video/fbdev/core/fb_info.c:70 framebuffer_release+0x2c/0x40
> > > < snip >
> > > [ 44.111289] Call Trace:
> > > [ 44.111290] <TASK>
> > > [ 44.111291] ? show_regs+0x6c/0x80
> > > [ 44.111295] ? __warn+0x8d/0x150
> > > [ 44.111298] ? framebuffer_release+0x2c/0x40
> > > [ 44.111300] ? report_bug+0x182/0x1b0
> > > [ 44.111303] ? handle_bug+0x6e/0xb0
> > > [ 44.111306] ? exc_invalid_op+0x18/0x80
> > > [ 44.111308] ? asm_exc_invalid_op+0x1b/0x20
> > > [ 44.111311] ? framebuffer_release+0x2c/0x40
> > > [ 44.111313] ? hvfb_remove+0x86/0xa0 [hyperv_fb]
> > > [ 44.111315] vmbus_remove+0x24/0x40 [hv_vmbus]
> > > [ 44.111323] device_remove+0x40/0x80
> > > [ 44.111325] device_release_driver_internal+0x20b/0x270
> > > [ 44.111327] ? bus_find_device+0xb3/0xf0
> > >
> >
> > Thanks for pointing this out. Interestingly, I'm not seeing this WARN
> > in my experiments. What base kernel are you testing with? Are you
> > testing on a local VM or in Azure? What exactly are you doing
> > to create the problem? I've been doing unbind of the driver,
> > but maybe you are doing something different.
> >
> > FWIW, there is yet another issue where after doing two unbind/bind
> > cycles of the hyperv_fb driver, there's an error about freeing a
> > non-existent resource. I know what that problem is, and it's in
> > vmbus_drv.c. I'll be submitting a patch for that as soon as I figure out
> > a clean fix.
> >
> > Michael
>
> This is on local Hyper-V. Kernel: 6.14.0-rc1-next-20250205+
> I run below command to reproduce the above error:
> echo "5620e0c7-8062-4dce-aeb7-520c7ef76171" > /sys/bus/vmbus/devices/5620e0c7-8062-4dce-aeb7-520c7ef76171/driver/unbind
>
> When hvfb_remove is called I can see the refcount for framebuffer is 2 when ,
> I expect it to be 1. After unregistering this framebuffer there is still 1 refcount
> remains, which is the reason for this WARN at the time of framebuffer_release.
>
> I wonder who is registering/using this extra framebuffer. Its not hyperv_drm or
> hyperv_fb IIUC.
>
> - Saurabh
Here are more details about this WARN:
Xorg opens `/dev/fb0`, which increases the framebuffer's reference
count, as mentioned above. As a result, when unbinding the driver,
this WARN is expected, indicating that the framebuffer is still in use.
I am open to suggestion what could be the correct behavior in this case.
There acan be two possible options:
1. Check the framebuffer reference count and prevent the driver from
unbinding/removal.
OR
2. Allow the driver to unbind while issuing this WARN. (Current scenario)
- Saurabh
^ permalink raw reply
* [PATCH v4 9/9] iio: adc: ad7606: add support for writing registers when using backend
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Guillaume Stols <gstols@baylibre.com>
Add the logic for effectively enabling the software mode for the
iio-backend, i.e. enabling the software mode channel configuration and
implementing the register writing functions.
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/adc/ad7606.h | 15 +++++++++++++
drivers/iio/adc/ad7606_par.c | 52 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index a35b526f3915..71a30525eaab 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -96,6 +96,21 @@
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
0, 0, 16)
+#define AD7606_BI_SW_CHANNEL(num) \
+ AD760X_CHANNEL(num, \
+ /* mask separate */ \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ /* mask type */ \
+ 0, \
+ /* mask all */ \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ /* mask separate available */ \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ /* mask all available */ \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ 16)
+
struct ad7606_state;
typedef int (*ad7606_scale_setup_cb_t)(struct iio_dev *indio_dev,
diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
index 64733b607aa8..335fb481bfde 100644
--- a/drivers/iio/adc/ad7606_par.c
+++ b/drivers/iio/adc/ad7606_par.c
@@ -19,6 +19,7 @@
#include <linux/iio/iio.h>
#include "ad7606.h"
+#include "ad7606_bus_iface.h"
static const struct iio_chan_spec ad7606b_bi_channels[] = {
AD7606_BI_CHANNEL(0),
@@ -31,7 +32,19 @@ static const struct iio_chan_spec ad7606b_bi_channels[] = {
AD7606_BI_CHANNEL(7),
};
-static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
+static const struct iio_chan_spec ad7606b_bi_sw_channels[] = {
+ AD7606_BI_SW_CHANNEL(0),
+ AD7606_BI_SW_CHANNEL(1),
+ AD7606_BI_SW_CHANNEL(2),
+ AD7606_BI_SW_CHANNEL(3),
+ AD7606_BI_SW_CHANNEL(4),
+ AD7606_BI_SW_CHANNEL(5),
+ AD7606_BI_SW_CHANNEL(6),
+ AD7606_BI_SW_CHANNEL(7),
+};
+
+static int ad7606_par_bus_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
{
struct ad7606_state *st = iio_priv(indio_dev);
unsigned int c, ret;
@@ -48,7 +61,8 @@ static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned
return 0;
}
-static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio_dev)
+static int ad7606_par_bus_setup_iio_backend(struct device *dev,
+ struct iio_dev *indio_dev)
{
struct ad7606_state *st = iio_priv(indio_dev);
unsigned int ret, c;
@@ -86,9 +100,39 @@ static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio
return 0;
}
+static int ad7606_par_bus_reg_read(struct ad7606_state *st, unsigned int addr)
+{
+ struct ad7606_platform_data *pdata = st->dev->platform_data;
+ int val, ret;
+
+ ret = pdata->bus_reg_read(st->back, addr, &val);
+ if (ret)
+ return ret;
+
+ return val;
+}
+
+static int ad7606_par_bus_reg_write(struct ad7606_state *st, unsigned int addr,
+ unsigned int val)
+{
+ struct ad7606_platform_data *pdata = st->dev->platform_data;
+
+ return pdata->bus_reg_write(st->back, addr, val);
+}
+
+static int ad7606_par_bus_sw_mode_config(struct iio_dev *indio_dev)
+{
+ indio_dev->channels = ad7606b_bi_sw_channels;
+
+ return 0;
+}
+
static const struct ad7606_bus_ops ad7606_bi_bops = {
- .iio_backend_config = ad7606_bi_setup_iio_backend,
- .update_scan_mode = ad7606_bi_update_scan_mode,
+ .iio_backend_config = ad7606_par_bus_setup_iio_backend,
+ .update_scan_mode = ad7606_par_bus_update_scan_mode,
+ .reg_read = ad7606_par_bus_reg_read,
+ .reg_write = ad7606_par_bus_reg_write,
+ .sw_mode_config = ad7606_par_bus_sw_mode_config,
};
static int ad7606_par16_read_block(struct device *dev,
--
2.47.0
^ permalink raw reply related
* [PATCH v4 8/9] iio: adc: ad7606: change channel macros parameters
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Guillaume Stols <gstols@baylibre.com>
Add the possibility to pass the *_available parameters to the main
macro.
This is a preparation to add the new channels for software mode and
hardware mode in iio backend mode more easily.
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
drivers/iio/adc/ad7606.h | 51 ++++++++++++++++++++++++------------------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index 7a044b499cfe..a35b526f3915 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -40,37 +40,19 @@
#define AD7606_RANGE_CH_ADDR(ch) (0x03 + ((ch) >> 1))
#define AD7606_OS_MODE 0x08
-#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) { \
+#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, \
+ mask_sep_avail, mask_all_avail, bits) { \
.type = IIO_VOLTAGE, \
.indexed = 1, \
.channel = num, \
.address = num, \
.info_mask_separate = mask_sep, \
+ .info_mask_separate_available = \
+ mask_sep_avail, \
.info_mask_shared_by_type = mask_type, \
.info_mask_shared_by_all = mask_all, \
- .scan_index = num, \
- .scan_type = { \
- .sign = 's', \
- .realbits = (bits), \
- .storagebits = (bits) > 16 ? 32 : 16, \
- .endianness = IIO_CPU, \
- }, \
-}
-
-#define AD7606_SW_CHANNEL(num, bits) { \
- .type = IIO_VOLTAGE, \
- .indexed = 1, \
- .channel = num, \
- .address = num, \
- .info_mask_separate = \
- BIT(IIO_CHAN_INFO_RAW) | \
- BIT(IIO_CHAN_INFO_SCALE), \
- .info_mask_separate_available = \
- BIT(IIO_CHAN_INFO_SCALE), \
- .info_mask_shared_by_all = \
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
.info_mask_shared_by_all_available = \
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ mask_all_avail, \
.scan_index = num, \
.scan_type = { \
.sign = 's', \
@@ -80,14 +62,30 @@
}, \
}
+#define AD7606_SW_CHANNEL(num, bits) \
+ AD760X_CHANNEL(num, \
+ /* mask separate */ \
+ BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ /* mask type */ \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ /* mask all */ \
+ 0, \
+ /* mask separate available */ \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ /* mask all available */ \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ bits)
+
#define AD7605_CHANNEL(num) \
AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \
- BIT(IIO_CHAN_INFO_SCALE), 0, 16)
+ BIT(IIO_CHAN_INFO_SCALE), 0, 0, 0, 16)
#define AD7606_CHANNEL(num, bits) \
AD760X_CHANNEL(num, BIT(IIO_CHAN_INFO_RAW), \
BIT(IIO_CHAN_INFO_SCALE), \
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), bits)
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ 0, 0, bits)
#define AD7616_CHANNEL(num) AD7606_SW_CHANNEL(num, 16)
@@ -95,7 +93,8 @@
AD760X_CHANNEL(num, 0, \
BIT(IIO_CHAN_INFO_SCALE), \
BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), 16)
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ 0, 0, 16)
struct ad7606_state;
--
2.47.0
^ permalink raw reply related
* [PATCH v4 7/9] iio: adc: ad7606: protect register access
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Angelo Dureghello <adureghello@baylibre.com>
Protect register (and bus) access from concurrent
read / write. Needed in the backend operating mode.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/adc/ad7606.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 7985570ed152..07656fdd602e 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -862,7 +862,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
}
val = (val * MICRO) + val2;
i = find_closest(val, scale_avail_uv, cs->num_scales);
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
ret = st->write_scale(indio_dev, ch, i + cs->reg_offset);
+ iio_device_release_direct_mode(indio_dev);
if (ret < 0)
return ret;
cs->range = i;
@@ -873,7 +878,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
return -EINVAL;
i = find_closest(val, st->oversampling_avail,
st->num_os_ratios);
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
ret = st->write_os(indio_dev, i);
+ iio_device_release_direct_mode(indio_dev);
if (ret < 0)
return ret;
st->oversampling = st->oversampling_avail[i];
--
2.47.0
^ permalink raw reply related
* [PATCH v4 6/9] iio: adc: adi-axi-adc: add support for AD7606 register writing
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Guillaume Stols <gstols@baylibre.com>
Since we must access the bus parallel bus using a custom procedure,
let's add a specialized compatible, and define specialized callbacks for
writing the registers using the parallel interface.
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/adc/adi-axi-adc.c | 81 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 3e1a997ace76..225ea0497ba9 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -27,6 +27,7 @@
#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
+#include "ad7606_bus_iface.h"
/*
* Register definitions:
* https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map
@@ -73,6 +74,12 @@
#define ADI_AXI_ADC_REG_DELAY(l) (0x0800 + (l) * 0x4)
#define AXI_ADC_DELAY_CTRL_MASK GENMASK(4, 0)
+#define ADI_AXI_REG_CONFIG_WR 0x0080
+#define ADI_AXI_REG_CONFIG_RD 0x0084
+#define ADI_AXI_REG_CONFIG_CTRL 0x008c
+#define ADI_AXI_REG_CONFIG_CTRL_READ 0x03
+#define ADI_AXI_REG_CONFIG_CTRL_WRITE 0x01
+
#define ADI_AXI_ADC_MAX_IO_NUM_LANES 15
#define ADI_AXI_REG_CHAN_CTRL_DEFAULTS \
@@ -80,6 +87,10 @@
ADI_AXI_REG_CHAN_CTRL_FMT_EN | \
ADI_AXI_REG_CHAN_CTRL_ENABLE)
+#define ADI_AXI_REG_READ_BIT 0x8000
+#define ADI_AXI_REG_ADDRESS_MASK 0xff00
+#define ADI_AXI_REG_VALUE_MASK 0x00ff
+
struct axi_adc_info {
unsigned int version;
const struct iio_backend_info *backend_info;
@@ -311,6 +322,75 @@ static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back,
return iio_dmaengine_buffer_setup(st->dev, indio_dev, dma_name);
}
+static int axi_adc_raw_write(struct iio_backend *back, u32 val)
+{
+ struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+ regmap_write(st->regmap, ADI_AXI_REG_CONFIG_WR, val);
+ regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL,
+ ADI_AXI_REG_CONFIG_CTRL_WRITE);
+ fsleep(100);
+ regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, 0x00);
+ fsleep(100);
+
+ return 0;
+}
+
+static int axi_adc_raw_read(struct iio_backend *back, u32 *val)
+{
+ struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+ regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL,
+ ADI_AXI_REG_CONFIG_CTRL_READ);
+ fsleep(100);
+ regmap_read(st->regmap, ADI_AXI_REG_CONFIG_RD, val);
+ regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, 0x00);
+ fsleep(100);
+
+ return 0;
+}
+
+static int ad7606_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val)
+{
+ struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+ int addr;
+
+ guard(mutex)(&st->lock);
+
+ /*
+ * The address is written on the highest weight byte, and the MSB set
+ * at 1 indicates a read operation.
+ */
+ addr = FIELD_PREP(ADI_AXI_REG_ADDRESS_MASK, reg) | ADI_AXI_REG_READ_BIT;
+ axi_adc_raw_write(back, addr);
+ axi_adc_raw_read(back, val);
+
+ /* Write 0x0 on the bus to get back to ADC mode */
+ axi_adc_raw_write(back, 0);
+
+ return 0;
+}
+
+static int ad7606_bus_reg_write(struct iio_backend *back, u32 reg, u32 val)
+{
+ struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+ u32 buf;
+
+ guard(mutex)(&st->lock);
+
+ /* Write any register to switch to register mode */
+ axi_adc_raw_write(back, 0xaf00);
+
+ buf = FIELD_PREP(ADI_AXI_REG_ADDRESS_MASK, reg) |
+ FIELD_PREP(ADI_AXI_REG_VALUE_MASK, val);
+ axi_adc_raw_write(back, buf);
+
+ /* Write 0x0 on the bus to get back to ADC mode */
+ axi_adc_raw_write(back, 0);
+
+ return 0;
+}
+
static void axi_adc_free_buffer(struct iio_backend *back,
struct iio_buffer *buffer)
{
@@ -498,6 +578,7 @@ static const struct axi_adc_info adc_ad7606 = {
/* Match table for of_platform binding */
static const struct of_device_id adi_axi_adc_of_match[] = {
{ .compatible = "adi,axi-adc-10.0.a", .data = &adc_generic },
+ { .compatible = "adi,axi-ad7606x", .data = &adc_ad7606 },
{ /* end of list */ }
};
MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
--
2.47.0
^ permalink raw reply related
* [PATCH v4 5/9] iio: adc: adi-axi-adc: add platform children support
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Angelo Dureghello <adureghello@baylibre.com>
This is a preparation for the next commit adding support for register
read and write functions on AD7606.
Since sometimes a bus will be used, it has been agreed during ad3552's
driver implementation that the device's driver bus is the backend, whose
device node will be a child node.
To provide the special callbacks for setting the register, axi-adc needs
to pass them to the child device's driver through platform data.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/adc/ad7606_bus_iface.h | 16 ++++++++++
drivers/iio/adc/adi-axi-adc.c | 65 ++++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+)
diff --git a/drivers/iio/adc/ad7606_bus_iface.h b/drivers/iio/adc/ad7606_bus_iface.h
new file mode 100644
index 000000000000..f2c979a9b7f3
--- /dev/null
+++ b/drivers/iio/adc/ad7606_bus_iface.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2010-2024 Analog Devices Inc.
+ * Copyright (c) 2025 Baylibre, SAS
+ */
+#ifndef __LINUX_PLATFORM_DATA_AD7606_H__
+#define __LINUX_PLATFORM_DATA_AD7606_H__
+
+struct iio_backend;
+
+struct ad7606_platform_data {
+ int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val);
+ int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val);
+};
+
+#endif /* __LINUX_PLATFORM_DATA_AD7606_H__ */
diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index b38e8a27af94..3e1a997ace76 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -334,6 +334,36 @@ static const struct regmap_config axi_adc_regmap_config = {
.reg_stride = 4,
};
+static void axi_adc_child_remove(void *data)
+{
+ platform_device_unregister(data);
+}
+
+static int axi_adc_create_platform_device(struct adi_axi_adc_state *st,
+ struct fwnode_handle *child)
+{
+ struct platform_device_info pi = {
+ .parent = st->dev,
+ .name = fwnode_get_name(child),
+ .id = PLATFORM_DEVID_AUTO,
+ .fwnode = child,
+ .data = st->info->pdata,
+ .size_data = st->info->pdata_sz,
+ };
+ struct platform_device *pdev;
+ int ret;
+
+ pdev = platform_device_register_full(&pi);
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ ret = devm_add_action_or_reset(st->dev, axi_adc_child_remove, pdev);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static const struct iio_backend_ops adi_axi_adc_ops = {
.enable = axi_adc_enable,
.disable = axi_adc_disable,
@@ -417,6 +447,28 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, ret,
"failed to register iio backend\n");
+ device_for_each_child_node_scoped(&pdev->dev, child) {
+ int val;
+
+ if (!st->info->has_child_nodes)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "invalid fdt axi-dac compatible.");
+
+ /* Processing only reg 0 node */
+ ret = fwnode_property_read_u32(child, "reg", &val);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "invalid reg property.");
+ if (val != 0)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "invalid node address.");
+
+ ret = axi_adc_create_platform_device(st, child);
+ if (ret)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "cannot create device.");
+ }
+
dev_info(&pdev->dev, "AXI ADC IP core (%d.%.2d.%c) probed\n",
ADI_AXI_PCORE_VER_MAJOR(ver),
ADI_AXI_PCORE_VER_MINOR(ver),
@@ -430,6 +482,19 @@ static const struct axi_adc_info adc_generic = {
.backend_info = &adi_axi_adc_generic,
};
+static const struct ad7606_platform_data ad7606_pdata = {
+ .bus_reg_read = ad7606_bus_reg_read,
+ .bus_reg_write = ad7606_bus_reg_write,
+};
+
+static const struct axi_adc_info adc_ad7606 = {
+ .version = ADI_AXI_PCORE_VER(10, 0, 'a'),
+ .backend_info = &adi_axi_adc_generic,
+ .pdata = &ad7606_pdata,
+ .pdata_sz = sizeof(ad7606_pdata),
+ .has_child_nodes = true,
+};
+
/* Match table for of_platform binding */
static const struct of_device_id adi_axi_adc_of_match[] = {
{ .compatible = "adi,axi-adc-10.0.a", .data = &adc_generic },
--
2.47.0
^ permalink raw reply related
* [PATCH v4 4/9] iio: adc: adi-axi-adc: add struct axi_adc_info
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Angelo Dureghello <adureghello@baylibre.com>
Add struct axi_adc_info to allow different axi-adc compatibles that can
be added to this generic implementation.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/adc/adi-axi-adc.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index c7357601f0f8..b38e8a27af94 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -80,7 +80,16 @@
ADI_AXI_REG_CHAN_CTRL_FMT_EN | \
ADI_AXI_REG_CHAN_CTRL_ENABLE)
+struct axi_adc_info {
+ unsigned int version;
+ const struct iio_backend_info *backend_info;
+ bool has_child_nodes;
+ const void *pdata;
+ unsigned int pdata_sz;
+};
+
struct adi_axi_adc_state {
+ const struct axi_adc_info *info;
struct regmap *regmap;
struct device *dev;
/* lock to protect multiple accesses to the device registers */
@@ -348,7 +357,6 @@ static const struct iio_backend_info adi_axi_adc_generic = {
static int adi_axi_adc_probe(struct platform_device *pdev)
{
- const unsigned int *expected_ver;
struct adi_axi_adc_state *st;
void __iomem *base;
unsigned int ver;
@@ -370,8 +378,8 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, PTR_ERR(st->regmap),
"failed to init register map\n");
- expected_ver = device_get_match_data(&pdev->dev);
- if (!expected_ver)
+ st->info = device_get_match_data(&pdev->dev);
+ if (!st->info)
return -ENODEV;
clk = devm_clk_get_enabled(&pdev->dev, NULL);
@@ -391,12 +399,13 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (ADI_AXI_PCORE_VER_MAJOR(ver) != ADI_AXI_PCORE_VER_MAJOR(*expected_ver)) {
+ if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
+ ADI_AXI_PCORE_VER_MAJOR(st->info->version)) {
dev_err(&pdev->dev,
"Major version mismatch. Expected %d.%.2d.%c, Reported %d.%.2d.%c\n",
- ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
- ADI_AXI_PCORE_VER_MINOR(*expected_ver),
- ADI_AXI_PCORE_VER_PATCH(*expected_ver),
+ ADI_AXI_PCORE_VER_MAJOR(st->info->version),
+ ADI_AXI_PCORE_VER_MINOR(st->info->version),
+ ADI_AXI_PCORE_VER_PATCH(st->info->version),
ADI_AXI_PCORE_VER_MAJOR(ver),
ADI_AXI_PCORE_VER_MINOR(ver),
ADI_AXI_PCORE_VER_PATCH(ver));
@@ -416,11 +425,14 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
return 0;
}
-static unsigned int adi_axi_adc_10_0_a_info = ADI_AXI_PCORE_VER(10, 0, 'a');
+static const struct axi_adc_info adc_generic = {
+ .version = ADI_AXI_PCORE_VER(10, 0, 'a'),
+ .backend_info = &adi_axi_adc_generic,
+};
/* Match table for of_platform binding */
static const struct of_device_id adi_axi_adc_of_match[] = {
- { .compatible = "adi,axi-adc-10.0.a", .data = &adi_axi_adc_10_0_a_info },
+ { .compatible = "adi,axi-adc-10.0.a", .data = &adc_generic },
{ /* end of list */ }
};
MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
--
2.47.0
^ permalink raw reply related
* [PATCH v4 3/9] iio: adc: ad7606: move software functions into common file
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Guillaume Stols <gstols@baylibre.com>
Since the register are always the same, whatever bus is used, moving the
software functions into the main file avoids the code to be duplicated
in both SPI and parallel version of the driver.
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/adc/ad7606.c | 133 ++++++++++++++++++++++++++++++++++++++---
drivers/iio/adc/ad7606.h | 37 ++++++++++--
drivers/iio/adc/ad7606_spi.c | 137 +------------------------------------------
3 files changed, 158 insertions(+), 149 deletions(-)
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 376c808df11c..7985570ed152 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -85,6 +85,10 @@ static const unsigned int ad7606_oversampling_avail[7] = {
1, 2, 4, 8, 16, 32, 64,
};
+static const unsigned int ad7606b_oversampling_avail[9] = {
+ 1, 2, 4, 8, 16, 32, 64, 128, 256,
+};
+
static const unsigned int ad7616_oversampling_avail[8] = {
1, 2, 4, 8, 16, 32, 64, 128,
};
@@ -187,6 +191,8 @@ static int ad7608_chan_scale_setup(struct iio_dev *indio_dev,
struct iio_chan_spec *chan, int ch);
static int ad7609_chan_scale_setup(struct iio_dev *indio_dev,
struct iio_chan_spec *chan, int ch);
+static int ad7616_sw_mode_setup(struct iio_dev *indio_dev);
+static int ad7606b_sw_mode_setup(struct iio_dev *indio_dev);
const struct ad7606_chip_info ad7605_4_info = {
.channels = ad7605_channels,
@@ -239,6 +245,7 @@ const struct ad7606_chip_info ad7606b_info = {
.oversampling_avail = ad7606_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
.scale_setup_cb = ad7606_16bit_chan_scale_setup,
+ .sw_setup_cb = ad7606b_sw_mode_setup,
};
EXPORT_SYMBOL_NS_GPL(ad7606b_info, "IIO_AD7606");
@@ -250,6 +257,7 @@ const struct ad7606_chip_info ad7606c_16_info = {
.oversampling_avail = ad7606_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
.scale_setup_cb = ad7606c_16bit_chan_scale_setup,
+ .sw_setup_cb = ad7606b_sw_mode_setup,
};
EXPORT_SYMBOL_NS_GPL(ad7606c_16_info, "IIO_AD7606");
@@ -294,6 +302,7 @@ const struct ad7606_chip_info ad7606c_18_info = {
.oversampling_avail = ad7606_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
.scale_setup_cb = ad7606c_18bit_chan_scale_setup,
+ .sw_setup_cb = ad7606b_sw_mode_setup,
};
EXPORT_SYMBOL_NS_GPL(ad7606c_18_info, "IIO_AD7606");
@@ -307,6 +316,7 @@ const struct ad7606_chip_info ad7616_info = {
.oversampling_num = ARRAY_SIZE(ad7616_oversampling_avail),
.os_req_reset = true,
.scale_setup_cb = ad7606_16bit_chan_scale_setup,
+ .sw_setup_cb = ad7616_sw_mode_setup,
};
EXPORT_SYMBOL_NS_GPL(ad7616_info, "IIO_AD7606");
@@ -1138,16 +1148,118 @@ static const struct iio_trigger_ops ad7606_trigger_ops = {
.validate_device = iio_trigger_validate_own_device,
};
-static int ad7606_sw_mode_setup(struct iio_dev *indio_dev)
+static int ad7606_write_mask(struct ad7606_state *st, unsigned int addr,
+ unsigned long mask, unsigned int val)
+{
+ int readval;
+
+ readval = st->bops->reg_read(st, addr);
+ if (readval < 0)
+ return readval;
+
+ readval &= ~mask;
+ readval |= val;
+
+ return st->bops->reg_write(st, addr, readval);
+}
+
+static int ad7616_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
{
struct ad7606_state *st = iio_priv(indio_dev);
+ unsigned int ch_addr, mode, ch_index;
- st->sw_mode_en = st->bops->sw_mode_config &&
- device_property_present(st->dev, "adi,sw-mode");
- if (!st->sw_mode_en)
- return 0;
+ /*
+ * Ad7616 has 16 channels divided in group A and group B.
+ * The range of channels from A are stored in registers with address 4
+ * while channels from B are stored in register with address 6.
+ * The last bit from channels determines if it is from group A or B
+ * because the order of channels in iio is 0A, 0B, 1A, 1B...
+ */
+ ch_index = ch >> 1;
+
+ ch_addr = AD7616_RANGE_CH_ADDR(ch_index);
+
+ if ((ch & 0x1) == 0) /* channel A */
+ ch_addr += AD7616_RANGE_CH_A_ADDR_OFF;
+ else /* channel B */
+ ch_addr += AD7616_RANGE_CH_B_ADDR_OFF;
+
+ /* 0b01 for 2.5v, 0b10 for 5v and 0b11 for 10v */
+ mode = AD7616_RANGE_CH_MODE(ch_index, ((val + 1) & 0b11));
- indio_dev->info = &ad7606_info_sw_mode;
+ return ad7606_write_mask(st, ch_addr, AD7616_RANGE_CH_MSK(ch_index),
+ mode);
+}
+
+static int ad7616_write_os_sw(struct iio_dev *indio_dev, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return ad7606_write_mask(st, AD7616_CONFIGURATION_REGISTER,
+ AD7616_OS_MASK, val << 2);
+}
+
+static int ad7606_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return ad7606_write_mask(st, AD7606_RANGE_CH_ADDR(ch),
+ AD7606_RANGE_CH_MSK(ch),
+ AD7606_RANGE_CH_MODE(ch, val));
+}
+
+static int ad7606_write_os_sw(struct iio_dev *indio_dev, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return st->bops->reg_write(st, AD7606_OS_MODE, val);
+}
+
+static int ad7616_sw_mode_setup(struct iio_dev *indio_dev)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+ int ret;
+
+ /*
+ * Scale can be configured individually for each channel
+ * in software mode.
+ */
+
+ st->write_scale = ad7616_write_scale_sw;
+ st->write_os = &ad7616_write_os_sw;
+
+ ret = st->bops->sw_mode_config(indio_dev);
+ if (ret)
+ return ret;
+
+ /* Activate Burst mode and SEQEN MODE */
+ return ad7606_write_mask(st, AD7616_CONFIGURATION_REGISTER,
+ AD7616_BURST_MODE | AD7616_SEQEN_MODE,
+ AD7616_BURST_MODE | AD7616_SEQEN_MODE);
+}
+
+static int ad7606b_sw_mode_setup(struct iio_dev *indio_dev)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+ DECLARE_BITMAP(os, 3);
+
+ bitmap_fill(os, 3);
+ /*
+ * Software mode is enabled when all three oversampling
+ * pins are set to high. If oversampling gpios are defined
+ * in the device tree, then they need to be set to high,
+ * otherwise, they must be hardwired to VDD
+ */
+ if (st->gpio_os) {
+ gpiod_set_array_value(st->gpio_os->ndescs, st->gpio_os->desc,
+ st->gpio_os->info, os);
+ }
+ /* OS of 128 and 256 are available only in software mode */
+ st->oversampling_avail = ad7606b_oversampling_avail;
+ st->num_os_ratios = ARRAY_SIZE(ad7606b_oversampling_avail);
+
+ st->write_scale = ad7606_write_scale_sw;
+ st->write_os = &ad7606_write_os_sw;
return st->bops->sw_mode_config(indio_dev);
}
@@ -1326,9 +1438,12 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
st->write_scale = ad7606_write_scale_hw;
st->write_os = ad7606_write_os_hw;
- ret = ad7606_sw_mode_setup(indio_dev);
- if (ret)
- return ret;
+ st->sw_mode_en = st->chip_info->sw_setup_cb &&
+ device_property_present(st->dev, "adi,sw-mode");
+ if (st->sw_mode_en) {
+ indio_dev->info = &ad7606_info_sw_mode;
+ st->chip_info->sw_setup_cb(indio_dev);
+ }
ret = ad7606_chan_scales_setup(indio_dev);
if (ret)
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index 8778ffe515b3..7a044b499cfe 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -10,6 +10,36 @@
#define AD760X_MAX_CHANNELS 16
+#define AD7616_CONFIGURATION_REGISTER 0x02
+#define AD7616_OS_MASK GENMASK(4, 2)
+#define AD7616_BURST_MODE BIT(6)
+#define AD7616_SEQEN_MODE BIT(5)
+#define AD7616_RANGE_CH_A_ADDR_OFF 0x04
+#define AD7616_RANGE_CH_B_ADDR_OFF 0x06
+/*
+ * Range of channels from a group are stored in 2 registers.
+ * 0, 1, 2, 3 in a register followed by 4, 5, 6, 7 in second register.
+ * For channels from second group(8-15) the order is the same, only with
+ * an offset of 2 for register address.
+ */
+#define AD7616_RANGE_CH_ADDR(ch) ((ch) >> 2)
+/* The range of the channel is stored in 2 bits */
+#define AD7616_RANGE_CH_MSK(ch) (0b11 << (((ch) & 0b11) * 2))
+#define AD7616_RANGE_CH_MODE(ch, mode) ((mode) << ((((ch) & 0b11)) * 2))
+
+#define AD7606_CONFIGURATION_REGISTER 0x02
+#define AD7606_SINGLE_DOUT 0x00
+
+/*
+ * Range for AD7606B channels are stored in registers starting with address 0x3.
+ * Each register stores range for 2 channels(4 bits per channel).
+ */
+#define AD7606_RANGE_CH_MSK(ch) (GENMASK(3, 0) << (4 * ((ch) & 0x1)))
+#define AD7606_RANGE_CH_MODE(ch, mode) \
+ ((GENMASK(3, 0) & (mode)) << (4 * ((ch) & 0x1)))
+#define AD7606_RANGE_CH_ADDR(ch) (0x03 + ((ch) >> 1))
+#define AD7606_OS_MODE 0x08
+
#define AD760X_CHANNEL(num, mask_sep, mask_type, mask_all, bits) { \
.type = IIO_VOLTAGE, \
.indexed = 1, \
@@ -71,6 +101,7 @@ struct ad7606_state;
typedef int (*ad7606_scale_setup_cb_t)(struct iio_dev *indio_dev,
struct iio_chan_spec *chan, int ch);
+typedef int (*ad7606_sw_setup_cb_t)(struct iio_dev *indio_dev);
/**
* struct ad7606_chip_info - chip specific information
@@ -80,6 +111,7 @@ typedef int (*ad7606_scale_setup_cb_t)(struct iio_dev *indio_dev,
* @num_channels: number of channels
* @num_adc_channels the number of channels the ADC actually inputs.
* @scale_setup_cb: callback to setup the scales for each channel
+ * @sw_setup_cb: callback to setup the software mode if available.
* @oversampling_avail pointer to the array which stores the available
* oversampling ratios.
* @oversampling_num number of elements stored in oversampling_avail array
@@ -94,6 +126,7 @@ struct ad7606_chip_info {
unsigned int num_adc_channels;
unsigned int num_channels;
ad7606_scale_setup_cb_t scale_setup_cb;
+ ad7606_sw_setup_cb_t sw_setup_cb;
const unsigned int *oversampling_avail;
unsigned int oversampling_num;
bool os_req_reset;
@@ -206,10 +239,6 @@ struct ad7606_bus_ops {
int (*reg_write)(struct ad7606_state *st,
unsigned int addr,
unsigned int val);
- int (*write_mask)(struct ad7606_state *st,
- unsigned int addr,
- unsigned long mask,
- unsigned int val);
int (*update_scan_mode)(struct iio_dev *indio_dev, const unsigned long *scan_mask);
u16 (*rd_wr_cmd)(int addr, char isWriteOp);
};
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index e2c147525706..885bf0b68e77 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -15,36 +15,6 @@
#define MAX_SPI_FREQ_HZ 23500000 /* VDRIVE above 4.75 V */
-#define AD7616_CONFIGURATION_REGISTER 0x02
-#define AD7616_OS_MASK GENMASK(4, 2)
-#define AD7616_BURST_MODE BIT(6)
-#define AD7616_SEQEN_MODE BIT(5)
-#define AD7616_RANGE_CH_A_ADDR_OFF 0x04
-#define AD7616_RANGE_CH_B_ADDR_OFF 0x06
-/*
- * Range of channels from a group are stored in 2 registers.
- * 0, 1, 2, 3 in a register followed by 4, 5, 6, 7 in second register.
- * For channels from second group(8-15) the order is the same, only with
- * an offset of 2 for register address.
- */
-#define AD7616_RANGE_CH_ADDR(ch) ((ch) >> 2)
-/* The range of the channel is stored in 2 bits */
-#define AD7616_RANGE_CH_MSK(ch) (0b11 << (((ch) & 0b11) * 2))
-#define AD7616_RANGE_CH_MODE(ch, mode) ((mode) << ((((ch) & 0b11)) * 2))
-
-#define AD7606_CONFIGURATION_REGISTER 0x02
-#define AD7606_SINGLE_DOUT 0x00
-
-/*
- * Range for AD7606B channels are stored in registers starting with address 0x3.
- * Each register stores range for 2 channels(4 bits per channel).
- */
-#define AD7606_RANGE_CH_MSK(ch) (GENMASK(3, 0) << (4 * ((ch) & 0x1)))
-#define AD7606_RANGE_CH_MODE(ch, mode) \
- ((GENMASK(3, 0) & mode) << (4 * ((ch) & 0x1)))
-#define AD7606_RANGE_CH_ADDR(ch) (0x03 + ((ch) >> 1))
-#define AD7606_OS_MODE 0x08
-
static const struct iio_chan_spec ad7616_sw_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(16),
AD7616_CHANNEL(0),
@@ -89,10 +59,6 @@ static const struct iio_chan_spec ad7606c_18_sw_channels[] = {
AD7606_SW_CHANNEL(7, 18),
};
-static const unsigned int ad7606B_oversampling_avail[9] = {
- 1, 2, 4, 8, 16, 32, 64, 128, 256
-};
-
static u16 ad7616_spi_rd_wr_cmd(int addr, char isWriteOp)
{
/*
@@ -194,118 +160,20 @@ static int ad7606_spi_reg_write(struct ad7606_state *st,
return spi_write(spi, &st->d16[0], sizeof(st->d16[0]));
}
-static int ad7606_spi_write_mask(struct ad7606_state *st,
- unsigned int addr,
- unsigned long mask,
- unsigned int val)
-{
- int readval;
-
- readval = st->bops->reg_read(st, addr);
- if (readval < 0)
- return readval;
-
- readval &= ~mask;
- readval |= val;
-
- return st->bops->reg_write(st, addr, readval);
-}
-
-static int ad7616_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
-{
- struct ad7606_state *st = iio_priv(indio_dev);
- unsigned int ch_addr, mode, ch_index;
-
-
- /*
- * Ad7616 has 16 channels divided in group A and group B.
- * The range of channels from A are stored in registers with address 4
- * while channels from B are stored in register with address 6.
- * The last bit from channels determines if it is from group A or B
- * because the order of channels in iio is 0A, 0B, 1A, 1B...
- */
- ch_index = ch >> 1;
-
- ch_addr = AD7616_RANGE_CH_ADDR(ch_index);
-
- if ((ch & 0x1) == 0) /* channel A */
- ch_addr += AD7616_RANGE_CH_A_ADDR_OFF;
- else /* channel B */
- ch_addr += AD7616_RANGE_CH_B_ADDR_OFF;
-
- /* 0b01 for 2.5v, 0b10 for 5v and 0b11 for 10v */
- mode = AD7616_RANGE_CH_MODE(ch_index, ((val + 1) & 0b11));
- return st->bops->write_mask(st, ch_addr, AD7616_RANGE_CH_MSK(ch_index),
- mode);
-}
-
-static int ad7616_write_os_sw(struct iio_dev *indio_dev, int val)
-{
- struct ad7606_state *st = iio_priv(indio_dev);
-
- return st->bops->write_mask(st, AD7616_CONFIGURATION_REGISTER,
- AD7616_OS_MASK, val << 2);
-}
-
-static int ad7606_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
-{
- struct ad7606_state *st = iio_priv(indio_dev);
-
- return ad7606_spi_write_mask(st,
- AD7606_RANGE_CH_ADDR(ch),
- AD7606_RANGE_CH_MSK(ch),
- AD7606_RANGE_CH_MODE(ch, val));
-}
-
-static int ad7606_write_os_sw(struct iio_dev *indio_dev, int val)
-{
- struct ad7606_state *st = iio_priv(indio_dev);
-
- return ad7606_spi_reg_write(st, AD7606_OS_MODE, val);
-}
-
static int ad7616_sw_mode_config(struct iio_dev *indio_dev)
{
- struct ad7606_state *st = iio_priv(indio_dev);
-
/*
* Scale can be configured individually for each channel
* in software mode.
*/
indio_dev->channels = ad7616_sw_channels;
- st->write_scale = ad7616_write_scale_sw;
- st->write_os = &ad7616_write_os_sw;
-
- /* Activate Burst mode and SEQEN MODE */
- return st->bops->write_mask(st,
- AD7616_CONFIGURATION_REGISTER,
- AD7616_BURST_MODE | AD7616_SEQEN_MODE,
- AD7616_BURST_MODE | AD7616_SEQEN_MODE);
+ return 0;
}
static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
{
struct ad7606_state *st = iio_priv(indio_dev);
- DECLARE_BITMAP(os, 3);
-
- bitmap_fill(os, 3);
- /*
- * Software mode is enabled when all three oversampling
- * pins are set to high. If oversampling gpios are defined
- * in the device tree, then they need to be set to high,
- * otherwise, they must be hardwired to VDD
- */
- if (st->gpio_os) {
- gpiod_set_array_value(st->gpio_os->ndescs,
- st->gpio_os->desc, st->gpio_os->info, os);
- }
- /* OS of 128 and 256 are available only in software mode */
- st->oversampling_avail = ad7606B_oversampling_avail;
- st->num_os_ratios = ARRAY_SIZE(ad7606B_oversampling_avail);
-
- st->write_scale = ad7606_write_scale_sw;
- st->write_os = &ad7606_write_os_sw;
/* Configure device spi to output on a single channel */
st->bops->reg_write(st,
@@ -350,7 +218,6 @@ static const struct ad7606_bus_ops ad7616_spi_bops = {
.read_block = ad7606_spi_read_block,
.reg_read = ad7606_spi_reg_read,
.reg_write = ad7606_spi_reg_write,
- .write_mask = ad7606_spi_write_mask,
.rd_wr_cmd = ad7616_spi_rd_wr_cmd,
.sw_mode_config = ad7616_sw_mode_config,
};
@@ -359,7 +226,6 @@ static const struct ad7606_bus_ops ad7606b_spi_bops = {
.read_block = ad7606_spi_read_block,
.reg_read = ad7606_spi_reg_read,
.reg_write = ad7606_spi_reg_write,
- .write_mask = ad7606_spi_write_mask,
.rd_wr_cmd = ad7606B_spi_rd_wr_cmd,
.sw_mode_config = ad7606B_sw_mode_config,
};
@@ -368,7 +234,6 @@ static const struct ad7606_bus_ops ad7606c_18_spi_bops = {
.read_block = ad7606_spi_read_block18to32,
.reg_read = ad7606_spi_reg_read,
.reg_write = ad7606_spi_reg_write,
- .write_mask = ad7606_spi_write_mask,
.rd_wr_cmd = ad7606B_spi_rd_wr_cmd,
.sw_mode_config = ad7606c_18_sw_mode_config,
};
--
2.47.0
^ permalink raw reply related
* [PATCH v4 2/9] iio: adc: ad7606: move the software mode configuration
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Guillaume Stols <gstols@baylibre.com>
This is a preparation for the intoduction of the sofware functions in
the iio backend version of the driver.
The software mode configuration must be executed once the channels are
configured, and the number of channels is known. This is not the case
before iio-backend's configuration is called, and iio backend version of
the driver does not have a timestamp channel.
Also the sw_mode_config callback is configured during the iio-backend
configuration.
For clarity purpose, I moved the entire block instead of just the
concerned function calls.
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
drivers/iio/adc/ad7606.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index d39354afd539..376c808df11c 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -1246,17 +1246,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
return -ERESTARTSYS;
}
- st->write_scale = ad7606_write_scale_hw;
- st->write_os = ad7606_write_os_hw;
-
- ret = ad7606_sw_mode_setup(indio_dev);
- if (ret)
- return ret;
-
- ret = ad7606_chan_scales_setup(indio_dev);
- if (ret)
- return ret;
-
/* If convst pin is not defined, setup PWM. */
if (!st->gpio_convst) {
st->cnvst_pwm = devm_pwm_get(dev, NULL);
@@ -1334,6 +1323,17 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
return ret;
}
+ st->write_scale = ad7606_write_scale_hw;
+ st->write_os = ad7606_write_os_hw;
+
+ ret = ad7606_sw_mode_setup(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad7606_chan_scales_setup(indio_dev);
+ if (ret)
+ return ret;
+
return devm_iio_device_register(dev, indio_dev);
}
EXPORT_SYMBOL_NS_GPL(ad7606_probe, "IIO_AD7606");
--
2.47.0
^ permalink raw reply related
* [PATCH v4 1/9] dt-bindings: iio: dac: adi-axi-adc: add ad7606 variant
From: Angelo Dureghello @ 2025-02-10 16:10 UTC (permalink / raw)
To: Michael Hennerich, Lars-Peter Clausen, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandru Ardelean, David Lechner
Cc: Jonathan Cameron, linux-fbdev, linux-iio, devicetree,
linux-kernel, Guillaume Stols, Angelo Dureghello
In-Reply-To: <20250210-wip-bl-ad7606_add_backend_sw_mode-v4-0-160df18b1da7@baylibre.com>
From: Guillaume Stols <gstols@baylibre.com>
A new compatible is added to reflect the specialized version of the HDL.
We use the parallel interface to write the ADC's registers, and
accessing this interface requires to use ADI_AXI_REG_CONFIG_RD,
ADI_AXI_REG_CONFIG_WR and ADI_AXI_REG_CONFIG_CTRL in a custom fashion.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Guillaume Stols <gstols@baylibre.com>
Co-developed-by: Angelo Dureghello <adureghello@baylibre.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
.../devicetree/bindings/iio/adc/adi,axi-adc.yaml | 70 +++++++++++++++++++++-
1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
index e1f450b80db2..4fa82dcf6fc9 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,axi-adc.yaml
@@ -17,13 +17,23 @@ description: |
interface for the actual ADC, while this IP core will interface
to the data-lines of the ADC and handle the streaming of data into
memory via DMA.
+ In some cases, the AXI ADC interface is used to perform specialized
+ operation to a particular ADC, e.g access the physical bus through
+ specific registers to write ADC registers.
+ In this case, we use a different compatible which indicates the target
+ IP core's name.
+ The following IP is currently supported:
+ - AXI AD7606x: specialized version of the IP core for all the chips from
+ the ad7606 family.
https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
+ http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
properties:
compatible:
enum:
- adi,axi-adc-10.0.a
+ - adi,axi-ad7606x
reg:
maxItems: 1
@@ -47,17 +57,48 @@ properties:
'#io-backend-cells':
const: 0
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ "^adc@[0-9a-f]+$":
+ type: object
+ properties:
+ reg:
+ maxItems: 1
+ additionalProperties: true
+ required:
+ - compatible
+ - reg
+
required:
- compatible
- dmas
- reg
- clocks
+allOf:
+ - if:
+ properties:
+ compatible:
+ not:
+ contains:
+ const: adi,axi-ad7606x
+ then:
+ properties:
+ '#address-cells': false
+ '#size-cells': false
+ patternProperties:
+ "^adc@[0-9a-f]+$": false
+
additionalProperties: false
examples:
- |
- axi-adc@44a00000 {
+ adc@44a00000 {
compatible = "adi,axi-adc-10.0.a";
reg = <0x44a00000 0x10000>;
dmas = <&rx_dma 0>;
@@ -65,4 +106,31 @@ examples:
clocks = <&axi_clk>;
#io-backend-cells = <0>;
};
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ parallel_bus_controller@44a00000 {
+ compatible = "adi,axi-ad7606x";
+ reg = <0x44a00000 0x10000>;
+ dmas = <&rx_dma 0>;
+ dma-names = "rx";
+ clocks = <&ext_clk>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad7606b";
+ reg = <0>;
+ pwms = <&axi_pwm_gen 0 0>;
+ pwm-names = "convst1";
+ avcc-supply = <&adc_vref>;
+ vdrive-supply = <&vdd_supply>;
+ reset-gpios = <&gpio0 91 GPIO_ACTIVE_HIGH>;
+ standby-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
+ adi,range-gpios = <&gpio0 89 GPIO_ACTIVE_HIGH>;
+ adi,oversampling-ratio-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH
+ &gpio0 87 GPIO_ACTIVE_HIGH
+ &gpio0 86 GPIO_ACTIVE_HIGH>;
+ io-backends = <¶llel_bus_controller>;
+ };
+ };
...
--
2.47.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox