* Re: [PATCH v3 1/2] usb: xhci-pci: add generic auxiliary device interface
From: Guenter Roeck @ 2026-05-08 13:17 UTC (permalink / raw)
To: Jihong Min, Mathias Nyman, Jihong Min, Greg Kroah-Hartman,
Mathias Nyman
Cc: Jonathan Corbet, Shuah Khan, Mario Limonciello, Basavaraj Natikar,
linux-usb, linux-hwmon, linux-doc, linux-pci, linux-kernel
In-Reply-To: <f47d9cc1-e39b-4199-b031-e91b8e02ab1d@icloud.com>
On 5/8/26 00:04, Jihong Min wrote:
> Hi Mathias,
>
> I tried the xhci-pci-prom21.c approach you suggested, with a PROM21-specific
> PCI glue driver calling xhci_pci_common_probe() and creating the auxiliary
> hwmon child device from that driver.
>
> While doing that I noticed a possible boot-time regression with the module
> case.
>
> If CONFIG_USB_XHCI_PCI=y and CONFIG_USB_XHCI_PCI_PROM21=m, then generic
> xhci-pci sees CONFIG_USB_XHCI_PCI_PROM21 as enabled and refuses the PROM21
> PCI ID:
>
> if (IS_ENABLED(CONFIG_USB_XHCI_PCI_PROM21) &&
> pci_match_id(pci_ids_prom21, dev))
> return -ENODEV;
>
> That means the PROM21 xHCI controller is handled only by
> xhci-pci-prom21.ko. If that module is not present in the initramfs or is not
> loaded early enough, the PROM21 xHCI controller remains unbound during early
> boot. Devices behind that controller, such as a USB keyboard used for early
> boot or disk unlock, would not work even though the generic xhci-pci driver is
> built in and could otherwise operate the controller.
>
> This seems different from the Renesas case, where the separate PCI driver is
> needed for controller-specific firmware handling. For PROM21, the USB/xHCI
> operation itself is still generic; the only extra function is publishing an
> optional hwmon child device.
>
> So I am not sure what the preferred direction should be:
>
> 1. Keep the separate xhci-pci-prom21.c PCI glue driver and make
> USB_XHCI_PCI_PROM21 built-in only, or otherwise constrain the Kconfig so
> the generic xhci-pci handoff cannot break early boot.
>
> 2. Keep PROM21 handled by generic xhci-pci and add only a small
> PROM21-specific auxiliary-device creation hook in xhci-pci after the
> common probe succeeds. In that model, failure to create the optional hwmon
> auxiliary device would not affect USB operation.
>
> 3. Some other split that keeps PROM21-specific sensor code outside
> xhci-pci, but does not prevent generic xhci-pci from binding the
> controller when the optional PROM21 glue is not available early.
>
> Do you still prefer the separate xhci-pci-prom21.c PCI driver for this case,
> or would the minimal xhci-pci auxiliary-device hook be more appropriate given
> the built-in xhci-pci / modular PROM21 glue case?
>
Maybe I am missing something, but it seems to me that CONFIG_USB_XHCI_PCI_PROM21
should be just as built-in as CONFIG_USB_XHCI_PCI.
Thanks,
Guenter
> Sincerely,
> Jihong Min
>
> On 5/7/26 18:31, Mathias Nyman wrote:
>> On 5/7/26 06:31, Jihong Min wrote:
>>> Some xHCI PCI controllers expose controller-specific functionality that is
>>> not part of generic xHCI operation and is better handled by optional child
>>> drivers in other subsystems. Add a small auxiliary device registration path
>>> for selected xHCI PCI controllers.
>>>
>>> The initial PCI ID match table lists AMD Promontory 21 (PROM21) 1022:43fd
>>> controllers. For matching controllers, xhci-pci creates an auxiliary
>>> device and stores it in devres so the remove path destroys it before HCD
>>> teardown.
>>>
>>> Subsystem-specific child drivers can then bind to those devices through
>>> the auxiliary bus and keep their hardware-specific logic outside xhci-pci.
>>>
>>> Assisted-by: Codex:gpt-5.5
>>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
>>> ---
>>> drivers/usb/host/Kconfig | 10 +++++
>>> drivers/usb/host/xhci-pci.c | 83 +++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 93 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>> index 0a277a07cf70..e0c2c7ac5c97 100644
>>> --- a/drivers/usb/host/Kconfig
>>> +++ b/drivers/usb/host/Kconfig
>>> @@ -42,6 +42,16 @@ config USB_XHCI_PCI
>>> depends on USB_PCI
>>> default y
>>> +config USB_XHCI_PCI_AUXDEV
>>> + bool "xHCI PCI auxiliary device support"
>>> + depends on USB_XHCI_PCI
>>> + select AUXILIARY_BUS
>>> + help
>>> + This enables xHCI PCI support for registering auxiliary devices
>>> + for selected controllers. It is used by optional child drivers
>>> + that bind to xHCI PCI controller-specific functionality through
>>> + the auxiliary bus.
>>> +
>>> config USB_XHCI_PCI_RENESAS
>>> tristate "Support for additional Renesas xHCI controller with firmware"
>>> depends on USB_XHCI_PCI
>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>> index 585b2f3117b0..618d6840e108 100644
>>> --- a/drivers/usb/host/xhci-pci.c
>>> +++ b/drivers/usb/host/xhci-pci.c
>>> @@ -8,6 +8,8 @@
>>> * Some code borrowed from the Linux EHCI driver.
>>> */
>>> +#include <linux/auxiliary_bus.h>
>>> +#include <linux/device/devres.h>
>>> #include <linux/pci.h>
>>> #include <linux/slab.h>
>>> #include <linux/module.h>
>>> @@ -80,6 +82,7 @@
>>> #define PCI_DEVICE_ID_AMD_RAVEN_15E1_XHCI 0x15e1
>>> #define PCI_DEVICE_ID_AMD_RAVEN2_XHCI 0x15e5
>>> #define PCI_DEVICE_ID_AMD_RENOIR_XHCI 0x1639
>>> +#define PCI_DEVICE_ID_AMD_PROM21_XHCI 0x43fd
>>> #define PCI_DEVICE_ID_AMD_PROMONTORYA_4 0x43b9
>>> #define PCI_DEVICE_ID_AMD_PROMONTORYA_3 0x43ba
>>> #define PCI_DEVICE_ID_AMD_PROMONTORYA_2 0x43bb
>>> @@ -103,6 +106,80 @@ static int xhci_pci_run(struct usb_hcd *hcd);
>>> static int xhci_pci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
>>> struct usb_tt *tt, gfp_t mem_flags);
>>> +static const struct pci_device_id pci_ids_have_aux[] = {
>>> + { PCI_DEVICE_DATA(AMD, PROM21_XHCI, "prom21_hwmon") },
>>> + { /* end: all zeroes */ }
>>> +};
>>> +
>>> +struct xhci_pci_aux_devres {
>>> + struct auxiliary_device *auxdev;
>>> +};
>>> +
>>> +static const char *xhci_pci_aux_dev_name(struct pci_dev *pdev)
>>> +{
>>> + const struct pci_device_id *id;
>>> +
>>> + id = pci_match_id(pci_ids_have_aux, pdev);
>>> + if (!id)
>>> + return NULL;
>>> +
>>> + return (const char *)id->driver_data;
>>> +}
>>> +
>>> +static void xhci_pci_aux_devres_release(struct device *dev, void *res)
>>> +{
>>> + struct xhci_pci_aux_devres *devres = res;
>>> +
>>> + if (devres->auxdev)
>>> + auxiliary_device_destroy(devres->auxdev);
>>> +}
>>> +
>>> +static void xhci_pci_try_add_aux_device(struct pci_dev *pdev)
>>> +{
>>> + struct xhci_pci_aux_devres *devres;
>>> + struct auxiliary_device *auxdev;
>>> + const char *aux_dev_name;
>>> +
>>> + aux_dev_name = xhci_pci_aux_dev_name(pdev);
>>> + if (!aux_dev_name)
>>> + return;
>>> +
>>> + devres = devres_alloc(xhci_pci_aux_devres_release, sizeof(*devres),
>>> + GFP_KERNEL);
>>> + if (!devres) {
>>> + dev_warn(&pdev->dev,
>>> + "failed to allocate auxiliary device state\n");
>>> + return;
>>> + }
>>> +
>>> + auxdev = auxiliary_device_create(&pdev->dev, KBUILD_MODNAME,
>>> + aux_dev_name, NULL,
>>> + (pci_domain_nr(pdev->bus) << 16) |
>>> + pci_dev_id(pdev));
>>> + if (!auxdev) {
>>> + devres_free(devres);
>>> + dev_warn(&pdev->dev, "failed to add %s auxiliary device\n",
>>> + aux_dev_name);
>>> + return;
>>> + }
>>> +
>>> + devres->auxdev = auxdev;
>>> + devres_add(&pdev->dev, devres);
>>> +}
>>> +
>>> +static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
>>> +{
>>> + struct xhci_pci_aux_devres *devres;
>>> +
>>> + devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release, NULL,
>>> + NULL);
>>> + if (!devres || !devres->auxdev)
>>> + return;
>>> +
>>> + auxiliary_device_destroy(devres->auxdev);
>>> + devres->auxdev = NULL;
>>> +}
>>> +
>>> static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
>>> .reset = xhci_pci_setup,
>>> .start = xhci_pci_run,
>>> @@ -677,6 +754,9 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>> if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
>>> pci_clear_and_set_config_dword(dev, 0xE0, 0, 1 << 22);
>>> + if (IS_ENABLED(CONFIG_USB_XHCI_PCI_AUXDEV))
>>> + xhci_pci_try_add_aux_device(dev);
>>> +
>>> return 0;
>>
>> I think this should be turned around so that the vendor specific code calls the common code.
>> xhci-pci-renesas.c does this nicely.
>>
>> In your case it would be adding something like a xhci-pci-prom21.c pci driver:
>>
>> xhci_pci_prom21_probe(struct pci_dev *dev, const struct pci_device_id *id)
>> {
>> crate_auxiliary_device(dev);
>> return xhci_pci_common_probe(dev, id);
>> }
>>
>> xhci_pci_prom21_remove(struct pci_dev *dev)
>> {
>> destroy_auxiliary_device(dev);
>> xhci_pci_remove(dev);
>> }
>>
>> static const struct pci_device_id pci_ids[] = {
>> { PCI_DEVICE(YOUR_AMD_PCI_VENDOR_ID, YOUR_PROM21_DEVICE_ID) },
>> { /* end: all zeroes */ }
>> };
>> MODULE_DEVICE_TABLE(pci, pci_ids);
>>
>> static struct pci_driver xhci_prom21_pci_driver = {
>> .name = "xhci-pci-prom21",
>> .id_table = pci_ids,
>>
>> .probe = xhci_pci_prom21_probe,
>> .remove = xhci_pci_prom21_remove,
>>
>> .shutdown = usb_hcd_pci_shutdown,
>> .driver = {
>> .pm = pm_ptr(&usb_hcd_pci_pm_ops),
>> },
>> };
>> module_pci_driver(xhci_prom21_pci_driver);
>>
>> MODULE_DESCRIPTION("AMD Promontory 21 xHCI PCI Host Controller Driver");
>> MODULE_IMPORT_NS("xhci");
>> MODULE_LICENSE("GPL v2");
>>
>> -Mathias
>
^ permalink raw reply
* Re: [PATCH 2/3] Documentation/gpu: use === for Intel display section heading underlines
From: Rodrigo Vivi @ 2026-05-08 13:15 UTC (permalink / raw)
To: Jani Nikula
Cc: intel-gfx, intel-xe, dri-devel, linux-doc, Matthew Brost,
Thomas Hellström, joonas.lahtinen, tursulin
In-Reply-To: <f49968792220ca3ff24efde813550850340d092e.1778235406.git.jani.nikula@intel.com>
On Fri, May 08, 2026 at 01:20:48PM +0300, Jani Nikula wrote:
> Prefer to use === instead of --- for top level section heading
> underlines to allow using the latter for sub-headings later.
>
> While at it, fix the underline lenghts where needed.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> Documentation/gpu/intel-display/async-flip.rst | 2 +-
> Documentation/gpu/intel-display/audio.rst | 4 ++--
> Documentation/gpu/intel-display/cdclk.rst | 2 +-
> Documentation/gpu/intel-display/dmc.rst | 6 +++---
> Documentation/gpu/intel-display/dpio.rst | 2 +-
> Documentation/gpu/intel-display/dpll.rst | 2 +-
> Documentation/gpu/intel-display/drrs.rst | 2 +-
> Documentation/gpu/intel-display/dsb.rst | 2 +-
> Documentation/gpu/intel-display/fbc.rst | 2 +-
> Documentation/gpu/intel-display/fifo-underrun.rst | 2 +-
> Documentation/gpu/intel-display/frontbuffer.rst | 2 +-
> Documentation/gpu/intel-display/hotplug.rst | 2 +-
> Documentation/gpu/intel-display/plane.rst | 2 +-
> Documentation/gpu/intel-display/psr.rst | 2 +-
> Documentation/gpu/intel-display/vbt.rst | 2 +-
> 15 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/gpu/intel-display/async-flip.rst b/Documentation/gpu/intel-display/async-flip.rst
> index e4ae4012efc5..40f93e885bb7 100644
> --- a/Documentation/gpu/intel-display/async-flip.rst
> +++ b/Documentation/gpu/intel-display/async-flip.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Asynchronous Page Flip
> -----------------------
> +======================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_display.c
> :doc: asynchronous flip implementation
> diff --git a/Documentation/gpu/intel-display/audio.rst b/Documentation/gpu/intel-display/audio.rst
> index 7d3c1b514b0e..eef95df75f8d 100644
> --- a/Documentation/gpu/intel-display/audio.rst
> +++ b/Documentation/gpu/intel-display/audio.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> High Definition Audio
> ----------------------
> +=====================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_audio.c
> :doc: High Definition Audio over HDMI and Display Port
> @@ -14,7 +14,7 @@ High Definition Audio
> :internal:
>
> Intel HDMI LPE Audio Support
> -----------------------------
> +============================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_lpe_audio.c
> :doc: LPE Audio integration for HDMI or DP playback
> diff --git a/Documentation/gpu/intel-display/cdclk.rst b/Documentation/gpu/intel-display/cdclk.rst
> index 231b22a733e7..a66d623b0ec9 100644
> --- a/Documentation/gpu/intel-display/cdclk.rst
> +++ b/Documentation/gpu/intel-display/cdclk.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Display clocks
> ---------------
> +==============
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_cdclk.c
> :doc: CDCLK / RAWCLK
> diff --git a/Documentation/gpu/intel-display/dmc.rst b/Documentation/gpu/intel-display/dmc.rst
> index 2fcdbd457d79..4368da4c7048 100644
> --- a/Documentation/gpu/intel-display/dmc.rst
> +++ b/Documentation/gpu/intel-display/dmc.rst
> @@ -4,7 +4,7 @@
> .. _drm/intel-display/dmc:
>
> DMC Firmware Support
> ---------------------
> +====================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc.c
> :doc: DMC Firmware Support
> @@ -14,13 +14,13 @@ DMC Firmware Support
>
>
> DMC Flip Queue
> ---------------------
> +==============
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_flipq.c
> :doc: DMC Flip Queue
>
> DMC wakelock support
> ---------------------
> +====================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc_wl.c
> :doc: DMC wakelock support
> diff --git a/Documentation/gpu/intel-display/dpio.rst b/Documentation/gpu/intel-display/dpio.rst
> index 32e6f299f256..84d92ac162f8 100644
> --- a/Documentation/gpu/intel-display/dpio.rst
> +++ b/Documentation/gpu/intel-display/dpio.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> DPIO
> -----
> +====
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpio_phy.c
> :doc: DPIO
> diff --git a/Documentation/gpu/intel-display/dpll.rst b/Documentation/gpu/intel-display/dpll.rst
> index 35e8168ccfb9..c750352e0ae5 100644
> --- a/Documentation/gpu/intel-display/dpll.rst
> +++ b/Documentation/gpu/intel-display/dpll.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Display PLLs
> -------------
> +============
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> :doc: Display PLLs
> diff --git a/Documentation/gpu/intel-display/drrs.rst b/Documentation/gpu/intel-display/drrs.rst
> index adb413f300f1..a5aaba63d6b9 100644
> --- a/Documentation/gpu/intel-display/drrs.rst
> +++ b/Documentation/gpu/intel-display/drrs.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Display Refresh Rate Switching (DRRS)
> --------------------------------------
> +=====================================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_drrs.c
> :doc: Display Refresh Rate Switching (DRRS)
> diff --git a/Documentation/gpu/intel-display/dsb.rst b/Documentation/gpu/intel-display/dsb.rst
> index cbd40b0a4e7b..857aca59995a 100644
> --- a/Documentation/gpu/intel-display/dsb.rst
> +++ b/Documentation/gpu/intel-display/dsb.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Display State Buffer
> ---------------------
> +====================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
> :doc: DSB
> diff --git a/Documentation/gpu/intel-display/fbc.rst b/Documentation/gpu/intel-display/fbc.rst
> index 40f9d16bdebd..de9e19021f50 100644
> --- a/Documentation/gpu/intel-display/fbc.rst
> +++ b/Documentation/gpu/intel-display/fbc.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Frame Buffer Compression (FBC)
> -------------------------------
> +==============================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_fbc.c
> :doc: Frame Buffer Compression (FBC)
> diff --git a/Documentation/gpu/intel-display/fifo-underrun.rst b/Documentation/gpu/intel-display/fifo-underrun.rst
> index 50731f3a1f03..5d8f01921506 100644
> --- a/Documentation/gpu/intel-display/fifo-underrun.rst
> +++ b/Documentation/gpu/intel-display/fifo-underrun.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Display FIFO Underrun Reporting
> --------------------------------
> +===============================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> :doc: fifo underrun handling
> diff --git a/Documentation/gpu/intel-display/frontbuffer.rst b/Documentation/gpu/intel-display/frontbuffer.rst
> index 2a1bc63ba6b4..7ae38e0827bf 100644
> --- a/Documentation/gpu/intel-display/frontbuffer.rst
> +++ b/Documentation/gpu/intel-display/frontbuffer.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Frontbuffer Tracking
> ---------------------
> +====================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.c
> :doc: frontbuffer tracking
> diff --git a/Documentation/gpu/intel-display/hotplug.rst b/Documentation/gpu/intel-display/hotplug.rst
> index 4cd9dd5ac8fc..f33bc0087c27 100644
> --- a/Documentation/gpu/intel-display/hotplug.rst
> +++ b/Documentation/gpu/intel-display/hotplug.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Hotplug
> --------
> +=======
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_hotplug.c
> :doc: Hotplug
> diff --git a/Documentation/gpu/intel-display/plane.rst b/Documentation/gpu/intel-display/plane.rst
> index 41cf6571aab0..59932a82051b 100644
> --- a/Documentation/gpu/intel-display/plane.rst
> +++ b/Documentation/gpu/intel-display/plane.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Atomic Plane Helpers
> ---------------------
> +====================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_plane.c
> :doc: atomic plane helpers
> diff --git a/Documentation/gpu/intel-display/psr.rst b/Documentation/gpu/intel-display/psr.rst
> index 134c905f500e..63e56abcdd56 100644
> --- a/Documentation/gpu/intel-display/psr.rst
> +++ b/Documentation/gpu/intel-display/psr.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Panel Self Refresh PSR (PSR/SRD)
> ---------------------------------
> +================================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_psr.c
> :doc: Panel Self Refresh (PSR/SRD)
> diff --git a/Documentation/gpu/intel-display/vbt.rst b/Documentation/gpu/intel-display/vbt.rst
> index bbc7ee183f1b..be69f7fd7b39 100644
> --- a/Documentation/gpu/intel-display/vbt.rst
> +++ b/Documentation/gpu/intel-display/vbt.rst
> @@ -2,7 +2,7 @@
> .. Copyright © 2026 Intel Corporation
>
> Video BIOS Table (VBT)
> -----------------------
> +======================
>
> .. kernel-doc:: drivers/gpu/drm/i915/display/intel_bios.c
> :doc: Video BIOS Table (VBT)
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH 1/3] Documentation/gpu: add dedicated documentation for Intel display
From: Rodrigo Vivi @ 2026-05-08 13:15 UTC (permalink / raw)
To: Jani Nikula
Cc: intel-gfx, intel-xe, dri-devel, linux-doc, Matthew Brost,
Thomas Hellström, joonas.lahtinen, tursulin
In-Reply-To: <21bfa7777eb0926eadd309d4c6f5c9cf48405cf0.1778235406.git.jani.nikula@intel.com>
On Fri, May 08, 2026 at 01:20:47PM +0300, Jani Nikula wrote:
> Nowadays, the display support for drm/i915 and drm/xe is shared between
> the drivers, even though the code is located under drm/i915/display.
>
> The drm/i915 documentation has everything, including display topics, in
> one huge page, while the drm/xe documentation is well-organized but
> hardly mentions display. It's not great, to put it mildly.
>
> Split out the Intel display documentation to a dedicated directory,
> Documentation/gpu/intel-display. Also directly split the
> functionality/feature documentation to dedicated pages to keep the main
> index page high level and readable. We'll want to organize this further,
> but just sort them alphabetically for starters.
>
> Drop the boilerplate documentation sections that don't actually document
> anything.
>
> Cross-reference drm/i915, drm/xe, and intel-display.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> Tip: 'git show --color-moved' conveniently highlights what gets moved
> and what gets deleted.
> ---
> Documentation/gpu/drivers.rst | 1 +
> Documentation/gpu/drm-kms.rst | 3 +
> Documentation/gpu/i915.rst | 202 +-----------------
> .../gpu/intel-display/async-flip.rst | 8 +
> Documentation/gpu/intel-display/audio.rst | 23 ++
> Documentation/gpu/intel-display/cdclk.rst | 11 +
> Documentation/gpu/intel-display/dmc.rst | 26 +++
> Documentation/gpu/intel-display/dpio.rst | 8 +
> Documentation/gpu/intel-display/dpll.rst | 14 ++
> Documentation/gpu/intel-display/drrs.rst | 11 +
> Documentation/gpu/intel-display/dsb.rst | 11 +
> Documentation/gpu/intel-display/fbc.rst | 11 +
> .../gpu/intel-display/fifo-underrun.rst | 11 +
> .../gpu/intel-display/frontbuffer.rst | 14 ++
> Documentation/gpu/intel-display/hotplug.rst | 11 +
> Documentation/gpu/intel-display/index.rst | 40 ++++
> Documentation/gpu/intel-display/plane.rst | 11 +
> Documentation/gpu/intel-display/psr.rst | 11 +
> Documentation/gpu/intel-display/vbt.rst | 14 ++
> Documentation/gpu/xe/index.rst | 5 +
> 20 files changed, 251 insertions(+), 195 deletions(-)
> create mode 100644 Documentation/gpu/intel-display/async-flip.rst
> create mode 100644 Documentation/gpu/intel-display/audio.rst
> create mode 100644 Documentation/gpu/intel-display/cdclk.rst
> create mode 100644 Documentation/gpu/intel-display/dmc.rst
> create mode 100644 Documentation/gpu/intel-display/dpio.rst
> create mode 100644 Documentation/gpu/intel-display/dpll.rst
> create mode 100644 Documentation/gpu/intel-display/drrs.rst
> create mode 100644 Documentation/gpu/intel-display/dsb.rst
> create mode 100644 Documentation/gpu/intel-display/fbc.rst
> create mode 100644 Documentation/gpu/intel-display/fifo-underrun.rst
> create mode 100644 Documentation/gpu/intel-display/frontbuffer.rst
> create mode 100644 Documentation/gpu/intel-display/hotplug.rst
> create mode 100644 Documentation/gpu/intel-display/index.rst
> create mode 100644 Documentation/gpu/intel-display/plane.rst
> create mode 100644 Documentation/gpu/intel-display/psr.rst
> create mode 100644 Documentation/gpu/intel-display/vbt.rst
>
> diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
> index 2e13e0ad7e88..20d2c454aa1d 100644
> --- a/Documentation/gpu/drivers.rst
> +++ b/Documentation/gpu/drivers.rst
> @@ -8,6 +8,7 @@ GPU Driver Documentation
> amdgpu/index
> i915
> imagination/index
> + intel-display/index
> mcde
> meson
> nouveau
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index fbe0583eb84c..a125052f46bc 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -1,3 +1,6 @@
> +
> +.. _drm-kms:
> +
> =========================
> Kernel Mode Setting (KMS)
> =========================
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index eba09c3ddce4..0c9d68758533 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -1,3 +1,6 @@
> +
> +.. _drm/i915:
> +
> ===========================
> drm/i915 Intel GFX Driver
> ===========================
> @@ -7,6 +10,9 @@ models) integrated GFX chipsets with both Intel display and rendering
> blocks. This excludes a set of SoC platforms with an SGX rendering unit,
> those have basic support through the gma500 drm driver.
>
> +The display, or :ref:`drm-kms`, support for drm/i915 is provided by
> +:ref:`drm/intel-display`, and shared with :ref:`drm/xe <drm/xe>`.
> +
> Core Driver Infrastructure
> ==========================
>
> @@ -64,200 +70,6 @@ Workarounds
> .. kernel-doc:: drivers/gpu/drm/i915/gt/intel_workarounds.c
> :doc: Hardware workarounds
>
> -Display Hardware Handling
> -=========================
> -
> -This section covers everything related to the display hardware including
> -the mode setting infrastructure, plane, sprite and cursor handling and
> -display, output probing and related topics.
> -
> -Mode Setting Infrastructure
> ----------------------------
> -
> -The i915 driver is thus far the only DRM driver which doesn't use the
> -common DRM helper code to implement mode setting sequences. Thus it has
> -its own tailor-made infrastructure for executing a display configuration
> -change.
> -
> -Frontbuffer Tracking
> ---------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.c
> - :doc: frontbuffer tracking
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.h
> - :internal:
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.c
> - :internal:
> -
> -Display FIFO Underrun Reporting
> --------------------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> - :doc: fifo underrun handling
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> - :internal:
> -
> -Plane Configuration
> --------------------
> -
> -This section covers plane configuration and composition with the primary
> -plane, sprites, cursors and overlays. This includes the infrastructure
> -to do atomic vsync'ed updates of all this state and also tightly coupled
> -topics like watermark setup and computation, framebuffer compression and
> -panel self refresh.
> -
> -Atomic Plane Helpers
> ---------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_plane.c
> - :doc: atomic plane helpers
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_plane.c
> - :internal:
> -
> -Asynchronous Page Flip
> -----------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_display.c
> - :doc: asynchronous flip implementation
> -
> -Output Probing
> ---------------
> -
> -This section covers output probing and related infrastructure like the
> -hotplug interrupt storm detection and mitigation code. Note that the
> -i915 driver still uses most of the common DRM helper code for output
> -probing, so those sections fully apply.
> -
> -Hotplug
> --------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_hotplug.c
> - :doc: Hotplug
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_hotplug.c
> - :internal:
> -
> -High Definition Audio
> ----------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_audio.c
> - :doc: High Definition Audio over HDMI and Display Port
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_audio.c
> - :internal:
> -
> -.. kernel-doc:: include/drm/intel/i915_component.h
> - :internal:
> -
> -Intel HDMI LPE Audio Support
> -----------------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_lpe_audio.c
> - :doc: LPE Audio integration for HDMI or DP playback
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_lpe_audio.c
> - :internal:
> -
> -Panel Self Refresh PSR (PSR/SRD)
> ---------------------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_psr.c
> - :doc: Panel Self Refresh (PSR/SRD)
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_psr.c
> - :internal:
> -
> -Frame Buffer Compression (FBC)
> -------------------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fbc.c
> - :doc: Frame Buffer Compression (FBC)
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fbc.c
> - :internal:
> -
> -Display Refresh Rate Switching (DRRS)
> --------------------------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_drrs.c
> - :doc: Display Refresh Rate Switching (DRRS)
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_drrs.c
> - :internal:
> -
> -DPIO
> -----
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpio_phy.c
> - :doc: DPIO
> -
> -DMC Firmware Support
> ---------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc.c
> - :doc: DMC Firmware Support
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc.c
> - :internal:
> -
> -DMC Flip Queue
> ---------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_flipq.c
> - :doc: DMC Flip Queue
> -
> -DMC wakelock support
> ---------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc_wl.c
> - :doc: DMC wakelock support
> -
> -Video BIOS Table (VBT)
> -----------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_bios.c
> - :doc: Video BIOS Table (VBT)
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_bios.c
> - :internal:
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_vbt_defs.h
> - :internal:
> -
> -Display clocks
> ---------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_cdclk.c
> - :doc: CDCLK / RAWCLK
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_cdclk.c
> - :internal:
> -
> -Display PLLs
> -------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> - :doc: Display PLLs
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> - :internal:
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> - :internal:
> -
> -Display State Buffer
> ---------------------
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
> - :doc: DSB
> -
> -.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
> - :internal:
> -
> GT Programming
> ==============
>
> @@ -568,7 +380,7 @@ The HuC FW layout is the same as the GuC one, see `GuC Firmware Layout`_
>
> DMC
> ---
> -See `DMC Firmware Support`_
> +See :ref:`drm/intel-display/dmc`.
>
> Tracing
> =======
> diff --git a/Documentation/gpu/intel-display/async-flip.rst b/Documentation/gpu/intel-display/async-flip.rst
> new file mode 100644
> index 000000000000..e4ae4012efc5
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/async-flip.rst
> @@ -0,0 +1,8 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Asynchronous Page Flip
> +----------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_display.c
> + :doc: asynchronous flip implementation
> diff --git a/Documentation/gpu/intel-display/audio.rst b/Documentation/gpu/intel-display/audio.rst
> new file mode 100644
> index 000000000000..7d3c1b514b0e
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/audio.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +High Definition Audio
> +---------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_audio.c
> + :doc: High Definition Audio over HDMI and Display Port
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_audio.c
> + :internal:
> +
> +.. kernel-doc:: include/drm/intel/i915_component.h
> + :internal:
> +
> +Intel HDMI LPE Audio Support
> +----------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_lpe_audio.c
> + :doc: LPE Audio integration for HDMI or DP playback
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_lpe_audio.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/cdclk.rst b/Documentation/gpu/intel-display/cdclk.rst
> new file mode 100644
> index 000000000000..231b22a733e7
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/cdclk.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Display clocks
> +--------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_cdclk.c
> + :doc: CDCLK / RAWCLK
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_cdclk.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/dmc.rst b/Documentation/gpu/intel-display/dmc.rst
> new file mode 100644
> index 000000000000..2fcdbd457d79
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/dmc.rst
> @@ -0,0 +1,26 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +.. _drm/intel-display/dmc:
> +
> +DMC Firmware Support
> +--------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc.c
> + :doc: DMC Firmware Support
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc.c
> + :internal:
> +
> +
> +DMC Flip Queue
> +--------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_flipq.c
> + :doc: DMC Flip Queue
> +
> +DMC wakelock support
> +--------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dmc_wl.c
> + :doc: DMC wakelock support
> diff --git a/Documentation/gpu/intel-display/dpio.rst b/Documentation/gpu/intel-display/dpio.rst
> new file mode 100644
> index 000000000000..32e6f299f256
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/dpio.rst
> @@ -0,0 +1,8 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +DPIO
> +----
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpio_phy.c
> + :doc: DPIO
> diff --git a/Documentation/gpu/intel-display/dpll.rst b/Documentation/gpu/intel-display/dpll.rst
> new file mode 100644
> index 000000000000..35e8168ccfb9
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/dpll.rst
> @@ -0,0 +1,14 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Display PLLs
> +------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> + :doc: Display PLLs
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> + :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> + :internal:
> diff --git a/Documentation/gpu/intel-display/drrs.rst b/Documentation/gpu/intel-display/drrs.rst
> new file mode 100644
> index 000000000000..adb413f300f1
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/drrs.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Display Refresh Rate Switching (DRRS)
> +-------------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_drrs.c
> + :doc: Display Refresh Rate Switching (DRRS)
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_drrs.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/dsb.rst b/Documentation/gpu/intel-display/dsb.rst
> new file mode 100644
> index 000000000000..cbd40b0a4e7b
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/dsb.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Display State Buffer
> +--------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
> + :doc: DSB
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/fbc.rst b/Documentation/gpu/intel-display/fbc.rst
> new file mode 100644
> index 000000000000..40f9d16bdebd
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/fbc.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Frame Buffer Compression (FBC)
> +------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fbc.c
> + :doc: Frame Buffer Compression (FBC)
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fbc.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/fifo-underrun.rst b/Documentation/gpu/intel-display/fifo-underrun.rst
> new file mode 100644
> index 000000000000..50731f3a1f03
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/fifo-underrun.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Display FIFO Underrun Reporting
> +-------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> + :doc: fifo underrun handling
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/frontbuffer.rst b/Documentation/gpu/intel-display/frontbuffer.rst
> new file mode 100644
> index 000000000000..2a1bc63ba6b4
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/frontbuffer.rst
> @@ -0,0 +1,14 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Frontbuffer Tracking
> +--------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.c
> + :doc: frontbuffer tracking
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.h
> + :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_frontbuffer.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/hotplug.rst b/Documentation/gpu/intel-display/hotplug.rst
> new file mode 100644
> index 000000000000..4cd9dd5ac8fc
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/hotplug.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Hotplug
> +-------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_hotplug.c
> + :doc: Hotplug
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_hotplug.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/index.rst b/Documentation/gpu/intel-display/index.rst
> new file mode 100644
> index 000000000000..8d40363b8f90
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/index.rst
> @@ -0,0 +1,40 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +.. _drm/intel-display:
> +
> +====================
> +Intel Display Driver
> +====================
> +
> +The Intel display driver provides the display, or :ref:`drm-kms`, support for
> +both the :ref:`drm/xe <drm/xe>` and :ref:`drm/i915 <drm/i915>` Intel GPU
> +drivers.
> +
> +The source code currently resides under ``drivers/gpu/drm/i915/display`` due to
> +historical reasons, and it's compiled separately into both drm/xe and drm/i915
> +kernel modules.
> +
> +The drm/xe and drm/i915 drivers are the "core" or "parent" drivers for display,
> +as they initialize and own the drm device, and pass that on to the display
> +driver. The display driver isn't an independent driver in that sense.
> +
> +.. toctree::
> + :maxdepth: 1
> + :caption: Detailed display topics
> +
> + async-flip
> + audio
> + cdclk
> + dmc
> + dpio
> + dpll
> + drrs
> + dsb
> + fbc
> + fifo-underrun
> + frontbuffer
> + hotplug
> + plane
> + psr
> + vbt
> diff --git a/Documentation/gpu/intel-display/plane.rst b/Documentation/gpu/intel-display/plane.rst
> new file mode 100644
> index 000000000000..41cf6571aab0
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/plane.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Atomic Plane Helpers
> +--------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_plane.c
> + :doc: atomic plane helpers
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_plane.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/psr.rst b/Documentation/gpu/intel-display/psr.rst
> new file mode 100644
> index 000000000000..134c905f500e
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/psr.rst
> @@ -0,0 +1,11 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Panel Self Refresh PSR (PSR/SRD)
> +--------------------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_psr.c
> + :doc: Panel Self Refresh (PSR/SRD)
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_psr.c
> + :internal:
> diff --git a/Documentation/gpu/intel-display/vbt.rst b/Documentation/gpu/intel-display/vbt.rst
> new file mode 100644
> index 000000000000..bbc7ee183f1b
> --- /dev/null
> +++ b/Documentation/gpu/intel-display/vbt.rst
> @@ -0,0 +1,14 @@
> +.. SPDX-License-Identifier: MIT
> +.. Copyright © 2026 Intel Corporation
> +
> +Video BIOS Table (VBT)
> +----------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_bios.c
> + :doc: Video BIOS Table (VBT)
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_bios.c
> + :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/i915/display/intel_vbt_defs.h
> + :internal:
> diff --git a/Documentation/gpu/xe/index.rst b/Documentation/gpu/xe/index.rst
> index 874ffcb6da3a..665c0e93601c 100644
> --- a/Documentation/gpu/xe/index.rst
> +++ b/Documentation/gpu/xe/index.rst
> @@ -1,5 +1,7 @@
> .. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
> +.. _drm/xe:
> +
> =======================
> drm/xe Intel GFX Driver
> =======================
> @@ -8,6 +10,9 @@ The drm/xe driver supports some future GFX cards with rendering, display,
> compute and media. Support for currently available platforms like TGL, ADL,
> DG2, etc is provided to prototype the driver.
>
> +The display, or :ref:`drm-kms`, support for drm/xe is provided by
> +:ref:`drm/intel-display`, and shared with :ref:`drm/i915 <drm/i915>`.
Great idea!
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> +
> .. toctree::
> :titlesonly:
>
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH v3 2/2] hwmon: add AMD Promontory 21 xHCI temperature sensor support
From: Guenter Roeck @ 2026-05-08 13:12 UTC (permalink / raw)
To: Jihong Min, Jihong Min
Cc: Greg Kroah-Hartman, Mathias Nyman, Jonathan Corbet, Shuah Khan,
Mario Limonciello, Basavaraj Natikar, linux-usb, linux-hwmon,
linux-doc, linux-pci, linux-kernel
In-Reply-To: <16c4f7e5-e33d-4271-a7af-5d6c7fca0570@icloud.com>
On 5/7/26 22:42, Jihong Min wrote:
> I believe I have addressed the other review comments for v4, including the
> remaining discussion from v2 2/2, your comments, and Mathias's suggestion to
> move the PROM21-specific xHCI PCI handling into a separate glue driver.
>
> I agree that "prom21_hwmon" is not a good name.
>
> I think just "prom21" may also be too broad, because Promontory 21 is a
> chipset/IP block with multiple I/O functions. This driver monitors the
> temperature value exposed through the PROM21 xHCI PCI function's MMIO BAR, so
> the xHCI part should probably be visible in the hwmon naming.
>
> I am considering:
>
> - drivers/hwmon/prom21-xhci.c
> - CONFIG_SENSORS_PROM21_XHCI
> - hwmon name: prom21_xhci
>
> while keeping the USB glue as xhci-pci-prom21.c.
>
> Does that sound reasonable?
>
Yes.
Please note that you keep top-posting. I don't mind that much, but
top-posting is (sometimes strongly) discouraged for linux kernel discussions.
Thanks,
Guenter
> Sincerely,
> Jihong Min
>
> On 5/8/26 00:53, Guenter Roeck wrote:
>> On Thu, May 07, 2026 at 12:31:59PM +0900, Jihong Min wrote:
>>> PROM21 xHCI controllers expose an 8-bit temperature value through a
>>> vendor-specific index/data register pair in the xHCI PCI MMIO BAR
>>> region. Add an auxiliary hwmon driver for PROM21 controllers with PCI
>>> ID 1022:43fd.
>>>
>>> PROM21 is an AMD chipset IP used in single-chip or daisy-chained
>>> configurations to build AMD 6xx/8xx series chipsets.
>>>
>>> The vendor index register is at byte offset 0x3000 from the xHCI MMIO
>>> BAR base and the vendor data register is at byte offset 0x3008. The
>>> driver writes register selector 0x0001e520 to the index register, reads
>>> the raw temperature value from the low 8 bits of the data register, and
>>> restores the previous index before returning. Expose temp1_input and an
>>> xHCI label through hwmon.
>>>
>>> Register the hwmon device under the parent PCI function so userspace
>>> reports it as a PCI adapter, while the auxiliary driver still owns the
>>> hwmon lifetime and unregisters it from the auxiliary remove path.
>>>
>>> No public AMD reference is available for this value. The conversion
>>> formula is derived from observed temperature readings:
>>>
>>> temp[C] = raw * 0.9066 - 78.624
>>>
>>> Testing showed that the temperature register does not return a valid
>>> value while the xHCI PCI function is runtime suspended. By default, the
>>> driver does not wake the parent PCI device from hwmon reads and returns
>>> -EPERM while the device is suspended.
>> Seriously ? Why would this be a permission issue ? Make it -ENODATA.
>>
>>> Document the supported device, register access, conversion formula,
>>> module parameter, sysfs attributes, and sysfs lookup method.
>>>
>>> Assisted-by: Codex:gpt-5.5
>>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
>>> ---
>>> Documentation/hwmon/index.rst | 1 +
>>> Documentation/hwmon/prom21-hwmon.rst | 86 ++++++++
>>> drivers/hwmon/Kconfig | 11 +
>>> drivers/hwmon/Makefile | 1 +
>>> drivers/hwmon/prom21-hwmon.c | 293 +++++++++++++++++++++++++++
>>> 5 files changed, 392 insertions(+)
>>> create mode 100644 Documentation/hwmon/prom21-hwmon.rst
>>> create mode 100644 drivers/hwmon/prom21-hwmon.c
>>>
>>> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
>>> index 8b655e5d6b68..41072977f0ef 100644
>>> --- a/Documentation/hwmon/index.rst
>>> +++ b/Documentation/hwmon/index.rst
>>> @@ -216,6 +216,7 @@ Hardware Monitoring Kernel Drivers
>>> pmbus
>>> powerz
>>> powr1220
>>> + prom21-hwmon
>>> pt5161l
>>> pxe1610
>>> pwm-fan
>>> diff --git a/Documentation/hwmon/prom21-hwmon.rst b/Documentation/hwmon/prom21-hwmon.rst
>>> new file mode 100644
>>> index 000000000000..0ba763e68ae9
>>> --- /dev/null
>>> +++ b/Documentation/hwmon/prom21-hwmon.rst
>>> @@ -0,0 +1,86 @@
>>> +.. SPDX-License-Identifier: GPL-2.0
>>> +
>>> +Kernel driver prom21-hwmon
>>> +==========================
>>> +
>>> +Supported chips:
>>> +
>>> + * AMD Promontory 21 (PROM21) xHCI
>>> +
>>> + Prefix: 'prom21_hwmon'
>> The "hwmon" in this name is redundant. Yes, I know, others like it too,
>> but it is still redundant. I won't comment on it further, though.
>>
>>> +
>>> + PCI ID: 1022:43fd
>>> +
>>> +Author:
>>> +
>>> + - Jihong Min <hurryman2212@gmail.com>
>>> +
>>> +Description
>>> +-----------
>>> +
>>> +This driver exposes the temperature sensor in AMD PROM21 xHCI controllers.
>>> +
>>> +The driver binds to an auxiliary device created by the xHCI PCI driver for
>>> +supported controllers. The sensor value is accessed through a vendor-specific
>>> +index/data register pair in the controller's PCI MMIO BAR.
>>> +
>>> +PROM21 is an AMD chipset IP used in single-chip or daisy-chained configurations
>>> +to build AMD 6xx/8xx series chipsets. Since the xHCI controllers are
>>> +integrated in PROM21, this temperature can also be used as a monitor for a
>>> +temperature close to the AMD chipset temperature.
>>> +
>>> +Register access
>>> +---------------
>>> +
>>> +The temperature value is read through a vendor-specific index/data register
>>> +pair in the xHCI PCI MMIO BAR. The driver uses the following byte offsets from
>>> +the MMIO BAR base:
>>> +
>>> +======================= =====================================================
>>> +0x3000 Vendor index register
>>> +0x3008 Vendor data register
>>> +======================= =====================================================
>>> +
>>> +The driver saves the current vendor index register value, writes the
>>> +temperature selector ``0x0001e520`` to the vendor index register, reads the
>>> +vendor data register, and restores the previous vendor index value before
>>> +returning. The raw temperature value is the low 8 bits of the vendor data
>>> +register value.
>>> +
>>> +No public AMD reference is available for the raw value. The temperature
>>> +conversion formula is derived from observed PROM21 xHCI temperature readings:
>>> +
>>> + temp[C] = raw * 0.9066 - 78.624
>>> +
>>> +Module parameters
>>> +-----------------
>>> +
>>> +pm: bool
>>> + Allow runtime PM state changes for device memory access. This is disabled
>>> + by default. If disabled, the driver does not wake the xHCI PCI device from
>>> + a temperature read. It reads the temperature only when the device is active.
>>> + A read from a suspended device returns ``-EPERM``.
>>> +
>>> +Sysfs entries
>>> +-------------
>>> +
>>> +======================= =====================================================
>>> +temp1_input Temperature in millidegrees Celsius
>>> +temp1_label "xHCI"
>> This is pointless and not the idea behind having a "label" attribute.
>> The driver name itself already associates the sensor with xhci.
>> Please drop.
>>
>>> +======================= =====================================================
>>> +
>>> +The hwmon device name is ``prom21_hwmon``. The sysfs path depends on the hwmon
>>> +device number assigned by the kernel. Userspace can locate the device by
>>> +matching the ``name`` attribute:
>>> +
>>> +.. code-block:: sh
>>> +
>>> + for hwmon in /sys/class/hwmon/hwmon*; do
>>> + [ "$(cat "$hwmon/name")" = "prom21_hwmon" ] || continue
>>> + cat "$hwmon/temp1_label"
>>> + cat "$hwmon/temp1_input"
>>> + done
>>> +
>>> +``temp1_input`` reports millidegrees Celsius, so a value of ``50113`` means
>>> +50.113 degrees Celsius. If the raw register value is invalid, ``temp1_input``
>>> +returns ``-ENODATA``.
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 14e4cea48acc..06d81cc29fec 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -940,6 +940,17 @@ config SENSORS_POWERZ
>>> This driver can also be built as a module. If so, the module
>>> will be called powerz.
>>> +config SENSORS_PROM21
>>> + tristate "AMD Promontory 21 xHCI temperature sensor"
>>> + depends on USB_XHCI_PCI
>>> + select USB_XHCI_PCI_AUXDEV
>>> + help
>>> + If you say yes here you get support for the AMD Promontory 21
>>> + (PROM21) xHCI temperature sensor.
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called prom21-hwmon.
>>> +
>>> config SENSORS_POWR1220
>>> tristate "Lattice POWR1220 Power Monitoring"
>>> depends on I2C
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index 4788996aa137..7693ed3b3f72 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -196,6 +196,7 @@ obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
>>> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
>>> obj-$(CONFIG_SENSORS_POWERZ) += powerz.o
>>> obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
>>> +obj-$(CONFIG_SENSORS_PROM21) += prom21-hwmon.o
>>> obj-$(CONFIG_SENSORS_PT5161L) += pt5161l.o
>>> obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o
>>> obj-$(CONFIG_SENSORS_QNAP_MCU_HWMON) += qnap-mcu-hwmon.o
>>> diff --git a/drivers/hwmon/prom21-hwmon.c b/drivers/hwmon/prom21-hwmon.c
>>> new file mode 100644
>>> index 000000000000..1c137304d65d
>>> --- /dev/null
>>> +++ b/drivers/hwmon/prom21-hwmon.c
>>> @@ -0,0 +1,293 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * AMD PROM21 xHCI Hwmon Implementation
>>> + * (only temperature monitoring is supported)
>>> + *
>>> + * This can be effectively used as the alternative chipset temperature monitor.
>>> + *
>>> + * Copyright (C) 2026 Jihong Min <hurryman2212@gmail.com>
>>> + */
>>> +
>>> +#include <linux/auxiliary_bus.h>
>>> +#include <linux/device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/io.h>
>>> +#include <linux/math.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/usb.h>
>>> +#include <linux/usb/hcd.h>
>>> +
>>> +#define PROM21_INDEX 0x3000
>>> +#define PROM21_DATA 0x3008
>>> +#define PROM21_TEMP_REG 0x0001e520
>>> +
>>> +#define PROM21_HWMON_NAME "prom21_hwmon"
>>> +#define PROM21_TEMP_LABEL "xHCI"
>>> +
>>> +struct prom21_hwmon {
>>> + struct pci_dev *pdev;
>>> + struct device *hwmon_dev;
>>> + void __iomem *regs;
>>> + bool removing;
>>> + struct mutex lock; /* protects removing and the index/data registers */
>> It is difficult to believe that auxiliary device management is so unstable
>> that it needs all that complexity. This will require confirmation from
>> someone who knows how this is supposed to work, and a detailed explanation
>> in the driver explaining why it is necessary.
>>
>>> +};
>>> +
>>> +static bool pm;
>>> +module_param(pm, bool, 0444);
>>> +MODULE_PARM_DESC(pm, "Allow runtime PM state changes for device memory access");
>> No. Either enable it or don't, but please don't add such module parameters.
>> The pm complexity in the driver, as written, makes it all but impossible
>> to determine what is going on.
>>
>>> +
>>> +static void prom21_hwmon_invalidate(struct prom21_hwmon *hwmon)
>>> +{
>>> + mutex_lock(&hwmon->lock);
>>> + hwmon->removing = true;
>>> + mutex_unlock(&hwmon->lock);
>>> +}
>>> +
>>> +static int prom21_hwmon_pm_get(struct prom21_hwmon *hwmon, bool *pm_ref)
>>> +{
>>> + struct device *dev = &hwmon->pdev->dev;
>>> + int ret;
>>> +
>>> + *pm_ref = false;
>>> +
>>> + /*
>>> + * PROM21 temperature register access does not return a valid value while
>>> + * the parent xHCI PCI function is suspended. By default, only read when
>>> + * runtime PM reports the device as active, or when runtime PM is disabled
>>> + * and the device is not marked as suspended. If pm=Y, allow runtime PM
>>> + * state changes while accessing the temperature register.
>>> + */
>>> + if (pm) {
>>> + ret = pm_runtime_resume_and_get(dev);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + *pm_ref = true;
>>> + return 0;
>>> + }
>>> +
>>> + ret = pm_runtime_get_if_active(dev);
>>> + if (ret > 0) {
>>> + *pm_ref = true;
>>> + return 0;
>>> + }
>>> +
>>> + if (ret == -EINVAL && !pm_runtime_status_suspended(dev))
>>> + return 0;
>>> +
>>> + if (!ret || pm_runtime_status_suspended(dev))
>>> + return -EPERM;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/*
>>> + * This is not a pure MMIO read. The PROM21 vendor data register is selected
>>> + * by temporarily writing PROM21_TEMP_REG to the vendor index register. Keep
>>> + * the sequence short and restore the previous index before returning.
>>> + */
>>> +static int prom21_hwmon_read_temp_raw_restore_index(struct prom21_hwmon *hwmon,
>>> + u8 *raw)
>>> +{
>>> + struct device *dev = &hwmon->pdev->dev;
>>> + bool pm_ref;
>>> + u32 index;
>>> + u32 data;
>>> + int ret;
>>> +
>>> + /*
>>> + * The xHCI PCI remove path destroys the auxiliary device before HCD
>>> + * teardown. Keep runtime PM and MMIO inside the critical section so a
>>> + * sysfs read cannot use the vendor register pair after remove starts.
>>> + */
>>> + mutex_lock(&hwmon->lock);
>>> + if (hwmon->removing) {
>>> + mutex_unlock(&hwmon->lock);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + ret = prom21_hwmon_pm_get(hwmon, &pm_ref);
>>> + if (ret) {
>>> + mutex_unlock(&hwmon->lock);
>>> + return ret;
>>> + }
>>> +
>>> + index = readl(hwmon->regs + PROM21_INDEX);
>>> + /* Select the PROM21 temperature register through the vendor index. */
>>> + writel(PROM21_TEMP_REG, hwmon->regs + PROM21_INDEX);
>>> + data = readl(hwmon->regs + PROM21_DATA);
>>> + /* Restore the previous vendor index register value. */
>>> + writel(index, hwmon->regs + PROM21_INDEX);
>>> + readl(hwmon->regs + PROM21_INDEX);
>>> +
>>> + if (pm_ref) {
>>> + /*
>>> + * Use autosuspend so repeated sysfs reads do not suspend the
>>> + * controller immediately after each successful register access.
>>> + */
>>> + pm_runtime_mark_last_busy(dev);
>>> + pm_runtime_put_autosuspend(dev);
>>> + }
>>> + mutex_unlock(&hwmon->lock);
>>> +
>>> + *raw = data & 0xff;
>>> + if (!*raw || *raw == 0xff)
>>> + return -ENODATA;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static long prom21_hwmon_raw_to_millicelsius(u8 raw)
>>> +{
>>> + /*
>>> + * No public AMD reference is available for this value.
>>> + * The scale was derived from observed PROM21 xHCI temperature readings:
>>> + * temp[C] = raw * 0.9066 - 78.624
>>> + */
>>> + return DIV_ROUND_CLOSEST(raw * 9066, 10) - 78624;
>>> +}
>>> +
>>> +static umode_t prom21_hwmon_is_visible(const void *drvdata,
>>> + enum hwmon_sensor_types type, u32 attr,
>>> + int channel)
>>> +{
>>> + if (type != hwmon_temp || channel)
>>> + return 0;
>>> +
>>> + switch (attr) {
>>> + case hwmon_temp_input:
>>> + case hwmon_temp_label:
>>> + return 0444;
>>> + default:
>>> + return 0;
>>> + }
>>> +}
>>> +
>>> +static int prom21_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>> + u32 attr, int channel, long *val)
>>> +{
>>> + struct prom21_hwmon *hwmon = dev_get_drvdata(dev);
>>> + u8 raw;
>>> + int ret;
>>> +
>>> + if (type != hwmon_temp || attr != hwmon_temp_input || channel)
>>> + return -EOPNOTSUPP;
>>> +
>>> + ret = prom21_hwmon_read_temp_raw_restore_index(hwmon, &raw);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + *val = prom21_hwmon_raw_to_millicelsius(raw);
>>> + return 0;
>>> +}
>>> +
>>> +static int prom21_hwmon_read_string(struct device *dev,
>>> + enum hwmon_sensor_types type, u32 attr,
>>> + int channel, const char **str)
>>> +{
>>> + if (type != hwmon_temp || attr != hwmon_temp_label || channel)
>>> + return -EOPNOTSUPP;
>>> +
>>> + *str = PROM21_TEMP_LABEL;
>>> + return 0;
>>> +}
>>> +
>>> +static const struct hwmon_ops prom21_hwmon_ops = {
>>> + .is_visible = prom21_hwmon_is_visible,
>>> + .read = prom21_hwmon_read,
>>> + .read_string = prom21_hwmon_read_string,
>>> +};
>>> +
>>> +static const struct hwmon_channel_info *const prom21_hwmon_info[] = {
>>> + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
>>> + NULL,
>>> +};
>>> +
>>> +static const struct hwmon_chip_info prom21_hwmon_chip_info = {
>>> + .ops = &prom21_hwmon_ops,
>>> + .info = prom21_hwmon_info,
>>> +};
>>> +
>>> +static int prom21_hwmon_probe(struct auxiliary_device *auxdev,
>>> + const struct auxiliary_device_id *id)
>>> +{
>>> + struct device *dev = &auxdev->dev;
>>> + struct device *parent = dev->parent;
>>> + struct prom21_hwmon *hwmon;
>>> + struct pci_dev *pdev;
>>> + struct usb_hcd *hcd;
>>> + int ret;
>>> +
>>> + if (!parent || !dev_is_pci(parent))
>>> + return -ENODEV;
>>> +
>>> + pdev = to_pci_dev(parent);
>>> + hcd = pci_get_drvdata(pdev);
>>> + if (!hcd)
>>> + return dev_err_probe(dev, -ENODEV,
>>> + "xHCI HCD data unavailable\n");
>>> +
>>> + if (!hcd->regs || hcd->rsrc_len < PROM21_DATA + sizeof(u32))
>>> + return dev_err_probe(dev, -ENODEV, "invalid MMIO resource\n");
>>> +
>>> + hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
>>> + if (!hwmon)
>>> + return -ENOMEM;
>>> +
>>> + ret = devm_mutex_init(dev, &hwmon->lock);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + hwmon->pdev = pdev;
>>> + hwmon->regs = hcd->regs;
>>> + auxiliary_set_drvdata(auxdev, hwmon);
>>> +
>>> + /*
>>> + * Use the PCI function as the hwmon parent so user space reports it as
>>> + * a PCI adapter. Lifetime is still owned by this auxiliary driver;
>>> + * remove() unregisters the hwmon device before xhci-pci tears down the
>>> + * HCD.
>>> + */
>>> + hwmon->hwmon_dev =
>>> + hwmon_device_register_with_info(&pdev->dev, PROM21_HWMON_NAME,
>>> + hwmon, &prom21_hwmon_chip_info,
>>> + NULL);
>>> + if (IS_ERR(hwmon->hwmon_dev))
>>> + return PTR_ERR(hwmon->hwmon_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void prom21_hwmon_remove(struct auxiliary_device *auxdev)
>>> +{
>>> + struct prom21_hwmon *hwmon = auxiliary_get_drvdata(auxdev);
>>> +
>>> + if (hwmon) {
>>> + prom21_hwmon_invalidate(hwmon);
>>> + hwmon_device_unregister(hwmon->hwmon_dev);
>>> + }
>>> +}
>>> +
>>> +static const struct auxiliary_device_id prom21_hwmon_id_table[] = {
>>> + { .name = "xhci_pci." PROM21_HWMON_NAME },
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(auxiliary, prom21_hwmon_id_table);
>>> +
>>> +static struct auxiliary_driver prom21_hwmon_driver = {
>>> + .name = "prom21-hwmon",
>>> + .probe = prom21_hwmon_probe,
>>> + .remove = prom21_hwmon_remove,
>>> + .id_table = prom21_hwmon_id_table,
>>> +};
>>> +module_auxiliary_driver(prom21_hwmon_driver);
>>> +
>>> +MODULE_AUTHOR("Jihong Min <hurryman2212@gmail.com>");
>>> +MODULE_DESCRIPTION("AMD PROM21 xHCI hwmon driver");
>>> +MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-05-08 13:08 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
rodrigo.alencar
In-Reply-To: <20260508134843.7646c4f5@jic23-huawei>
On Fri, 8 May 2026 13:48:43 +0100
Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 8 May 2026 13:55:47 +0200
> Janani Sunil <janani.sunil@analog.com> wrote:
>
> > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > buffered voltage output digital-to-analog converter (DAC) with an
> > integrated precision reference.
> >
> > Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> > ---
> > .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 96 ++++++++++++++++++++++
> > MAINTAINERS | 7 ++
> > 2 files changed, 103 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> > new file mode 100644
> > index 000000000000..f531b4865b01
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> > @@ -0,0 +1,96 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
>
> How is one device bother 12 and 16-bit? That sometimes happens for
> ADCs where it is really reflecting oversampling or for device with hardware
> FIFOs where storage space is saved by using lower bit rate. I'm not sure either
> applies here.
Having read the driver I now understand. This is supporting two parts and
doing device ID based detection. In an unusual step for Analog they have
the same base part number with a post fix. Whilst this approach works today
it fundamentally breaks fallback dt-compatibles being used in future (the
driver fails for any non match of WHOAMI value as it needs them to look
up device specific data) As such I think you need to have separate
compatibles for the 12 and 16 bit versions.
^ permalink raw reply
* Re: [PATCH v2 3/3] Documentation: iio: Add AD5529R Documentation
From: Jonathan Cameron @ 2026-05-08 13:00 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil
In-Reply-To: <20260508-ad5529r-driver-v2-3-e315441685d7@analog.com>
On Fri, 8 May 2026 13:55:49 +0200
Janani Sunil <janani.sunil@analog.com> wrote:
> Add documentation for AD5529R high voltage, 16-channel 12/16 bit DAC
Whilst it is good to have documentation for devices - I've made some
comments below on not providing documentation of standard things (too much
duplication) and being careful to work out who the document is for.
These tend to be for users and board integrators etc so we don't tend
to have much about the internals of the driver. For that see driver!
Jonathan
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> Documentation/iio/ad5529r.rst | 216 ++++++++++++++++++++++++++++++++++++++++++
> Documentation/iio/index.rst | 1 +
> MAINTAINERS | 1 +
> 3 files changed, 218 insertions(+)
>
> diff --git a/Documentation/iio/ad5529r.rst b/Documentation/iio/ad5529r.rst
> new file mode 100644
> index 000000000000..41fea1521790
> --- /dev/null
> +++ b/Documentation/iio/ad5529r.rst
> @@ -0,0 +1,216 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +==============
> +AD5529R driver
> +==============
> +
> +Device driver for Analog Devices Inc. AD5529R 16-Channel 12/16-bit High Voltage DAC.
> +The module name is ``ad5529r``.
> +
> +Supported devices
> +=================
> +
> +* `AD5529R <https://www.analog.com/en/products/ad5529r.html>`_
> +
> +Description
> +===========
> +
> +The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
Long line. Wrap to 80 chars consistently
> +digital-to-analog converter (DAC) with an integrated precision reference.
> +The device operates from unipolar and bipolar supplies and is guaranteed
> +monotonic. It has built-in rail-to-rail output buffers that can source or
> +sink up to 25mA.
> +
> +Hardware Features
> +=================
> +
> +* 16 independent 12-bit or 16-bit DAC channels
> +* Independently programmable output ranges:
> +
> + - 0V to 5V (current driver default)
With such wide ranges we have in some previous drivers made it a device
tree constraint. It rarely makes sense to switch between them at runtime
as these are really about what circuit is downstream of the DAC.
That needs to be there from initial driver otherwise we have a
backwards compatibility problem. A 'default' of smallest range should
be safe though (gets messier for some device where 'smallest' could be
unipolar or bipolar)
> + - 0V to 10V
> + - 0V to 20V
> + - 0V to 40V
> + - ±5V
> + - ±10V
> + - ±15V
> + - ±20V
> +
> +* 4.096V precision reference (12ppm/°C maximum)
> +* Built-in function generation capabilities (hardware support)
I'd drop the "(hardware support)" Given you are talking about functions
of the hardware, bit odd if the hardware didn't support them!
> +* Output voltage and current monitoring (hardware support)
> +* Temperature monitoring with 8 on-chip sensors (hardware support)
> +* Over-temperature protection
> +* SPI interface with CRC error detection support
> +
> +Current Driver Implementation
> +=============================
> +
> +The current driver provides basic DAC functionality with the following features:
> +
> +* Basic DAC output control for all 16 channels
> +* Scale attributes for voltage conversion (0-5V default range)
> +* SPI communication with regmap support
> +* Reset control framework support
> +* Automatic hardware variant detection (16-bit vs 12-bit) based on product ID
> +* Debugfs register access for development
> +
> +SPI Configuration:
> +
> +* **Mode**: Supports SPI mode 0 and mode 3 (default: mode 0)
In what sense is it the default? I'd drop that as just depends on the DT.
> +* **Frequency**: Up to 50 MHz (typically tested at lower frequencies)
> +* **Word Size**: 16-bit transactions
> +
> +.. note::
> + The device default configuration uses address decrement mode (ADDR_ASCENSION=0)
> + for multi-byte SPI transactions. Therefore, all 16-bit register addresses are
> + incremented by 1 in the driver to access the last byte first, allowing the
> + hardware to decrement and access the complete multi-byte register correctly.
This doc is a slightly odd mix of stuff for users (most of it) and
driver details like this. I'd move this to a comment in the code and
keep the doc for users.
> +
> +IIO Attributes (Currently Implemented)
> +======================================
> +
> +Basic DAC Control
> +-----------------
> +
> +For each of the 16 channels (0-15):
> +
> +**out_voltageY_raw**
> + Raw DAC code (12-bit: 0-4095, 16-bit: 0-65535)
> +
> + * Read: Returns the current DAC register value
> + * Write: Sets the DAC output code
> +
> +**out_voltageY_scale**
> + Scale factor for voltage conversion (millivolts per LSB)
> +
> + Based on the formula: VOUTn = A × D/2^N + B, where A=5V, B=0V, N=resolution
> +
> + * 16-bit: 0.076294 mV/LSB (5V ÷ 2^16 = 5V ÷ 65536 = 0.076294mV)
> + * 12-bit: 1.220703 mV/LSB (5V ÷ 2^12 = 5V ÷ 4096 = 1.220703mV)
> + * Read-only attribute
> +
> +Debug Interface
> +===============
> +
> +**Register Access**
> +
> +The driver provides debugfs register access for debugging and development:
> +
> +``/sys/kernel/debug/iio/iio:deviceX/direct_reg_access``
> + Direct register read/write access. Format:
> +
> + * Read: ``echo <register_address> > direct_reg_access; cat direct_reg_access``
> + * Write: ``echo <register_address> <value> > direct_reg_access``
> +
> +Usage examples
> +==============
> +
> +Basic DAC Output Control
> +------------------------
> +
> +.. code-block:: bash
> +
> + # Set channel 0 to mid-scale (approximately 2.5V with 0V to 5V range)
> + echo "32768" > /sys/bus/iio/devices/iio:device0/out_voltage0_raw
> +
> + # Set channel 15 to full scale
> + echo "65535" > /sys/bus/iio/devices/iio:device0/out_voltage15_raw
> +
> + # Read current value from channel 5
> + cat /sys/bus/iio/devices/iio:device0/out_voltage5_raw
> +
> +Scale Attributes
> +----------------
> +
> +.. code-block:: bash
> +
> + # Read scale factor (millivolts per LSB)
> + cat /sys/bus/iio/devices/iio:device0/out_voltage0_scale
> + # Output: 0.076294 (for 16-bit) or 1.220703 (for 12-bit)
> +
> + # Convert raw to voltage: voltage_mv = raw * scale
> + # Formula: VOUTn = A × D/2^N + B where A=5V, B=0V
This stuff is very standard. Consider if it is worth documenting
for this specific part. To me it isn't..
> +
> +Register Access for Development
> +-------------------------------
Likewise this section is very standard so I'd not expect per device
docs for it.
> +
> +.. code-block:: bash
> +
> + # Navigate to debugfs directory
> + cd /sys/kernel/debug/iio/iio:device0/
> +
> + # Read device product ID (register 0x04)
> + echo 4 > direct_reg_access
> + cat direct_reg_access
> +
> + # Write to a 16-bit configuration register (example: LDAC_HW_SW register 0x19)
> + echo "0x019 0xAA11" > direct_reg_access
> + cat direct_reg_access
> + # Output: 0xAA11
> +
> + # Write to DAC channel registers (16-bit values)
> + echo "0x149 32768" > direct_reg_access # DAC channel 0 mid-scale
> + echo "0x14B 65535" > direct_reg_access # DAC channel 1 full-scale
> +
> + # Read back DAC register values
> + echo 0x149 > direct_reg_access && cat direct_reg_access # Read channel 0
> + echo 0x14B > direct_reg_access && cat direct_reg_access # Read channel 1
> +
> +.. note::
> + For 16-bit registers, use hexadecimal format for addresses (0x019, 0x149, etc.).
Are there non 16-bit registers where we shouldn't use hexadecimal? Otherwise this
note seems odd.
> + Values can be decimal (32768) or hexadecimal (0xAA11). Register addresses shown
> + include the +1 offset required for decrement mode operation.
Ah. This is worth noting as users need to be aware of it so keep this bit.
> +
> +Device Tree Configuration
> +=========================
> +
> +Basic configuration example:
> +
> +.. code-block:: devicetree
> +
> + &spi0 {
> + status = "okay";
> +
> + ad5529r@0 {
> + compatible = "adi,ad5529r";
> + reg = <0>;
> + spi-max-frequency = <25000000>;
> +
> + vdd-supply = <&vdd_regulator>;
> + avdd-supply = <&avdd_regulator>;
> + hvdd-supply = <&hvdd_regulator>;
> + hvss-supply = <&hvss_regulator>;
> +
> + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> +For complete device tree binding documentation, see:
> +``Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml``
Hard no to replicating the device tree example - that is just
noise. The cross reference is enough.
> +
> +Driver Architecture
> +===================
> +
> +The driver is structured as follows:
> +
> +* **Core**: Basic SPI communication and device initialization
> +* **IIO Interface**: Standard IIO DAC channel interface with scale attributes
> +* **Dual Regmap**: Uses standard regmap-spi for both 8-bit and 16-bit register access
> +* **Reset Framework**: Reset control support
Not seeing value in anything except the dual regmap and again this
is mixing user stuff with driver internal details. That stuff probably
belongs in the driver.
> +
> +Development Notes
> +=================
> +
> +* The driver uses standard regmap-spi for both 8-bit and 16-bit register access
> +* SPI mode 0 (CPOL=0, CPHA=0) is typically used
Drop that. We don't care as long as both the options the dt-binding allows work fine.
> +* Reset control framework support for device initialization
> +* Register addresses are incremented by 1 for 16-bit registers due to decrement mode addressing
> +* Scale attributes provide voltage conversion for 0-5V range
> +* Automatic regmap selection based on register address (≤0x13: 8-bit, >0x13: 16-bit)
All this is driver internal stuff so I'd drop it.
> +
> +References
> +==========
> +
> +* AD5529R Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
> +* Linux IIO Subsystem: https://www.kernel.org/doc/html/latest/driver-api/iio/index.html
I'd skip that IIO generic docs reference. Not seeing it as adding much in this file.
> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index 007e0a1fcc5a..27f2ab41f05e 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -25,6 +25,7 @@ Industrial I/O Kernel Drivers
> ad4062
> ad4691
> ad4695
> + ad5529r
> ad7191
> ad7380
> ad7606
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 143714e27d51..41f42eb1adf2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1513,6 +1513,7 @@ L: linux-iio@vger.kernel.org
> S: Supported
> W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> +F: Documentation/iio/ad5529r.rst
> F: drivers/iio/dac/ad5529r.c
>
> ANALOG DEVICES INC AD5706R DRIVER
>
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-05-08 12:48 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
rodrigo.alencar
In-Reply-To: <20260508-ad5529r-driver-v2-1-e315441685d7@analog.com>
On Fri, 8 May 2026 13:55:47 +0200
Janani Sunil <janani.sunil@analog.com> wrote:
> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> buffered voltage output digital-to-analog converter (DAC) with an
> integrated precision reference.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 96 ++++++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 103 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> new file mode 100644
> index 000000000000..f531b4865b01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
How is one device bother 12 and 16-bit? That sometimes happens for
ADCs where it is really reflecting oversampling or for device with hardware
FIFOs where storage space is saved by using lower bit rate. I'm not sure either
applies here.
> +
> +maintainers:
> + - Janani Sunil <janani.sunil@analog.com>
> +
> +description: |
> + The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
> + digital-to-analog converter (DAC) with an integrated precision reference.
> + The device operates from unipolar and bipolar supplies. It is guaranteed
> + monotonic and has built-in rail-to-rail output buffers that can source or
> + sink up to 25mA.
> +
> + Specifications:
> + * 16 independent 12-bit or 16-bit DAC channels
> + * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
> + 0V to 40V, ±5V, ±10V, ±15V, and ±20V
> + * The device supports SPI communication with Mode 0 and Mode 3.
> + * 4.096V precision reference, 12ppm/°C maximum
> + * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
Interesting - so this is a DDS, be it a simple one. +CC Rodrigo who has been
wrestling with one of those recently. Rodrigo, can you take a look at this
driver and see if it fits in the ABI etc you've been hammering out? Thanks!
> + * Multiplexer for output voltage, load current sense and die temperature
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
> +
> +properties:
> + compatible:
> + const: adi,ad5529r
> +
> + reg:
> + maxItems: 1
> +
> + spi-max-frequency:
> + maximum: 50000000
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + GPIO connected to the RESET pin. Active low. When asserted low,
> + performs a power-on reset and initializes the device to its default state.
> +
> + vdd-supply:
> + description: Digital power supply (typically 3.3V)
> +
> + avdd-supply:
> + description: Analog power supply (typically 5V)
> +
> + hvdd-supply:
> + description: High voltage positive supply (up to 40V for output range)
> +
> + hvss-supply:
> + description: High voltage negative supply (ground or negative voltage)
I don't mind doing it this way but in some similar cases where 0 is something that
can be considered the 'default' we've made the supply optional. What was
your reasoning for requiring it in this case?
dt-bindings should be as complete as we can make them - with that in mind...
There are some more interesting corners on this device the binding doesn't
currently cover such as mux_out pin. We'd normally do that by making the
driver potentially a client of an ADC
Easier though is !alarm which smells like an interrupt.
!clear probably a gpio. TG0-3 also GPIOs.
> +
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> + - avdd-supply
> + - hvdd-supply
> + - hvss-supply
^ permalink raw reply
* Re: [PATCH v2 0/3] iio: dac: Add support for AD5529R DAC
From: Jonathan Cameron @ 2026-05-08 12:36 UTC (permalink / raw)
To: Janani Sunil
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil
In-Reply-To: <20260508-ad5529r-driver-v2-0-e315441685d7@analog.com>
On Fri, 8 May 2026 13:55:46 +0200
Janani Sunil <janani.sunil@analog.com> wrote:
> This patch series adds support for Analog Devices AD5529R, a 16 channel
> 16 and 12 bit voltage Digital-to-Analog Converter (DAC) with integrated
> precision reference. The AD5529R operates from both unipolar and
> bipolar supplies. The device communicates via SPI interface.
Hi Janani
Welcome to IIO. Please slow down. For a new driver convention is to
wait at least a week between versions so there is time for more reviews
to come in and hopefully reduce the total number of versions needed!
If you got feedback from Sashiko or other AI tools after posting then
feel free to address them in replies to your own series - they aren't
a reason to move faster with a new version.
Key for why we don't want too many versions is that those few of us
who review a lot of code very rarely manage to hold in our memories
what we checked on previous versions so tend to end up doing each version
from more or less scratch. That means each version takes a non trivial
amount of review time.
Anyhow, you've sent this now so I'll review it but for v3 if needed
slower please! If you have time and want to increase the chances
reviewers get to your patch, use it to review those of others. That is
is typically where our bottlenecks lie.
I've not been griping about this too much for the many cleanups on the
list at the moment because the overhead of those is smaller and also
I know some are students trying to get patches upstream in a finite
window.
Thanks
Jonathan
>
> **Device Overview:**
> The AD5529R features 16 independent DAC channels, with 16 or 12 bit
> resolution, allowing independently programmable output ranges. The
> internal 4.096V precision reference sets the accuracy of the output
> voltage.
>
> **Features Implemented:**
> - Automatic detection of 12/16 bit variant with product ID read.
> - Reset support via GPIO.
> - Dual regmap configuration to handle 8 and 16 bit registers.
>
> **Patch Summary:**
> 1. **dt-bindings**: Binding documentation with channel configuration.
> 2. **driver**: Implement IIO DAC Driver with regmap support.
> 3. **documentation**: Add driver documentation with usage examples.
>
> **Testing:**
> The driver was compiled and tested on the EVAL-AD5529R-ARDZ using a
> coraZ7 with a mainline v7.0 kernel.
>
> **Driver Rationale:**
> AD5529R introduces:
> 1. A unique register layout
> 2. Mixed 8-bit and 16-bit register accesses
> 3. Product ID based generic identification
> 4. Hardware specific features like function generators, multi-die
> hotpath registers etc.
>
> The device warrants its own drivers due to these fundamental
> architectural differences, that would require substantial changes to
> existing drivers without providing reusable benefits. The standalone
> driver also allows future extensions for related devices in the same
> family.
>
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
> ---
> Changes in v2:
> - Fix IIO scale to use millivolts per ABI requirement
> - Fix documentation voltage calculations (2.5V not 2.048V)
> - Fix bipolar ranges in documentation (±5V, ±10V, ±15V, ±20V)
> - Fix alphabetical ordering in documentation index
> - Add missing newline to documentation file
> - Fix scale units description (millivolts not microvolts)
> - Include a section for driver rationale in the cover letter
> - Reword contents in cover letter 12/16 bit generic->variant
> - Add dependency array for spi-cpha and spi-cpol properties
> - Link to v1: https://lore.kernel.org/r/20260507-ad5529r-driver-v1-0-b4460f3cb44f@analog.com
>
> ---
> Janani Sunil (3):
> dt-bindings: iio: dac: Add AD5529R
> iio: dac: Add AD5529R DAC driver support
> Documentation: iio: Add AD5529R Documentation
>
> .../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 96 ++++
> Documentation/iio/ad5529r.rst | 216 ++++++++
> Documentation/iio/index.rst | 1 +
> MAINTAINERS | 9 +
> drivers/iio/dac/Kconfig | 17 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ad5529r.c | 564 +++++++++++++++++++++
> 7 files changed, 904 insertions(+)
> ---
> base-commit: 93df88612859e8e19dec93c69d563b4b73e9bd4b
> change-id: 20260507-ad5529r-driver-866bbdd864de
>
> Best regards,
^ permalink raw reply
* Re: [PATCH v1 0/4] driver core, iio: suppress driver_override
From: Jonathan Cameron @ 2026-05-08 12:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Danilo Krummrich, Mark Brown, driver-core, linux-doc,
linux-kernel, linux-iio, linux-spi, Greg Kroah-Hartman,
Rafael J. Wysocki, Jonathan Corbet, Shuah Khan,
Jean-Baptiste Maneyrol, David Lechner, Nuno Sá,
Andy Shevchenko
In-Reply-To: <20260508095224.1275645-1-andriy.shevchenko@linux.intel.com>
On Fri, 8 May 2026 11:42:38 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Recently Sashiko started reporting the missed NULL check of
> spi_get_device_match_data() or device_get_match_data() in SPI drivers
> in IIO subsystem. It appears that the way to crash can be made by use
> of driver_override sysfs attribute. However, many drivers, that may be
> instantiate via ACPI, OF, or, in some cases, user space won't work
> without necessary driver data. These are, e.g., most of the drivers
> in IIO subsystem. Trying to override the driver for the device that
> has no matching entry makes no sense in such cases and might lead to
> a crash, when the driver is not prepared for that. Instead of adding
> a NULL check for driver data pointer in each of that drivers, effectively
> meaning a dead code for normal functionality, introduce a special
> attribute in the struct device_driver to allow drivers just to hide
> the attribute for good.
>
> The last two patches are the examples of use and code simplification
> at the same time.
>
> I consider getting an Ack from Mark for SPI, and from Jonathan for IIO
> and route this via driver core, while providing an immutable branch/tag
> for the above mentioned stakeholders.
Works for me.
>
> Note, this doesn't change the state of affairs for some busses that
> do not have driver_override flag set while using custom approach.
> On a brief look it's the s390 crypto case which may not ever need
> the above and hence left untouched.
Excellent! To me this makes a lot of sense as removing an attribute
that never works for particular drivers is neat and tidy. I suppose
there is a very small risk that some really strange scripts rely on
that attribute existing but not on it doing anything useful.
If that turns out to be the case we can just make it fail in the callbacks
but definitely not as neat.
One possible concern though based on a typical driver evolution:
1) Driver written - supports only one part, so to match_data involved
and driver_override is fine.
2) Driver gains support for second device. match_data added and now
driver_override is not ok so we set the new flag. Interface
disappears and perhaps someone was using it. Obviously that case
is broken anyway.
I guess I can make an IIO wide rule that we always set the flag. That
leaves us with a gap though as we have a lot of drivers currently at
step 1 of above that might progress to step 2 over time. I guess we deal
with any reported regressions on a case by case basis.
Ah well - still a step in the right direction even with risks of
regressions.
For all patches
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Thanks for proposing this Andy
Jonathan
>
> Andy Shevchenko (4):
> driver core: allow certain drivers prohibit override via sysfs
> spi: Support suppress_override_attrs flag
> iio: imu: inv_mpu6050: Suppress driver_override sysfs attribute
> iio: imu: inv_icm42600: Suppress driver_override sysfs attribute
>
> Documentation/driver-api/driver-model/binding.rst | 4 ++++
> drivers/base/bus.c | 4 ++--
> drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c | 8 ++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 3 +--
> drivers/spi/spi.c | 11 +++++++++++
> include/linux/device/driver.h | 2 ++
> 6 files changed, 22 insertions(+), 10 deletions(-)
>
^ permalink raw reply
* [PATCH v2 2/3] iio: dac: Add AD5529R DAC driver support
From: Janani Sunil @ 2026-05-08 11:55 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
In-Reply-To: <20260508-ad5529r-driver-v2-0-e315441685d7@analog.com>
Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/dac/Kconfig | 17 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ad5529r.c | 564 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 583 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 320e84765ce6..143714e27d51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1513,6 +1513,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+F: drivers/iio/dac/ad5529r.c
ANALOG DEVICES INC AD5706R DRIVER
M: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..bb1d59889a2a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -134,6 +134,23 @@ config AD5449
To compile this driver as a module, choose M here: the
module will be called ad5449.
+config AD5529R
+ tristate "Analog Devices AD5529R High Voltage DAC driver"
+ depends on SPI_MASTER
+ select REGMAP_SPI
+ help
+ Say yes here to build support for Analog Devices AD5529R
+ 16-Channel, 12-Bit/16-Bit, 40V High Voltage Precision Digital to Analog
+ Converter.
+
+ The device features multiple output voltage ranges from -20V to +20V,
+ built-in 4.096V voltage reference, and digital functions including
+ toggle, dither, and ramp modes. Supports both 12-bit and 16-bit
+ resolution variants.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad5529r.
+
config AD5592R_BASE
tristate
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 003431798498..f35e060b3643 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_AD5446) += ad5446.o
obj-$(CONFIG_AD5446_SPI) += ad5446-spi.o
obj-$(CONFIG_AD5446_I2C) += ad5446-i2c.o
obj-$(CONFIG_AD5449) += ad5449.o
+obj-$(CONFIG_AD5529R) += ad5529r.o
obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
obj-$(CONFIG_AD5592R) += ad5592r.o
obj-$(CONFIG_AD5593R) += ad5593r.o
diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
new file mode 100644
index 000000000000..3676956f6eff
--- /dev/null
+++ b/drivers/iio/dac/ad5529r.c
@@ -0,0 +1,564 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD5529R Digital-to-Analog Converter Driver
+ * 16-Channel, 12/16-Bit, 40V High Voltage Precision DAC
+ *
+ * Copyright 2026 Analog Devices Inc.
+ * Author: Janani Sunil <janani.sunil@analog.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/spi/spi.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
+
+/* Register Map */
+#define AD5529R_REG_INTERFACE_CONFIG_A 0x00
+#define AD5529R_REG_INTERFACE_CONFIG_B 0x01
+#define AD5529R_REG_DEVICE_CONFIG 0x02
+#define AD5529R_REG_CHIP_TYPE 0x03
+#define AD5529R_REG_PRODUCT_ID_L 0x04
+#define AD5529R_REG_PRODUCT_ID_H 0x05
+#define AD5529R_REG_CHIP_GRADE 0x06
+#define AD5529R_REG_SCRATCH_PAD 0x0A
+#define AD5529R_REG_SPI_REVISION 0x0B
+#define AD5529R_REG_VENDOR_L 0x0C
+#define AD5529R_REG_VENDOR_H 0x0D
+#define AD5529R_REG_STREAM_MODE 0x0E
+#define AD5529R_REG_TRANSFER_CONFIG 0x0F
+#define AD5529R_REG_INTERFACE_CONFIG_C 0x10
+#define AD5529R_REG_INTERFACE_STATUS_A 0x11
+
+/* Configuration registers */
+#define AD5529R_REG_MULTI_DAC_CH_SEL (0x14 + 1)
+#define AD5529R_REG_LDAC_SYNC_ASYNC (0x16 + 1)
+#define AD5529R_REG_LDAC_HW_SW (0x18 + 1)
+
+/* Hardware LDAC source and edge select registers (per channel, 16-bit) */
+#define AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE (0x1A + 1)
+#define AD5529R_REG_LDAC_HW_SRC_EDGE_SEL(ch) \
+ (AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE + (ch) * 2)
+
+/* Output configuration */
+#define AD5529R_REG_OUT_OPERATING_MODE (0x3A + 1)
+#define AD5529R_REG_OUT_RANGE_BASE (0x3C + 1)
+#define AD5529R_REG_OUT_RANGE(ch) (AD5529R_REG_OUT_RANGE_BASE + (ch) * 2)
+
+/* Calibration registers */
+#define AD5529R_REG_CAL_GAIN_BASE (0x5C + 1)
+#define AD5529R_REG_CAL_GAIN(ch) (AD5529R_REG_CAL_GAIN_BASE + (ch) * 2)
+
+#define AD5529R_REG_CAL_OFFSET_BASE (0x7C + 1)
+#define AD5529R_REG_CAL_OFFSET(ch) (AD5529R_REG_CAL_OFFSET_BASE + (ch) * 2)
+
+/* Function generator registers */
+#define AD5529R_REG_FUNC_EN (0x9C + 1)
+#define AD5529R_REG_FUNC_MODE_SEL_BASE (0x9E + 1)
+#define AD5529R_REG_FUNC_MODE_SEL(ch) \
+ (AD5529R_REG_FUNC_MODE_SEL_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_DAC_INPUT_B_BASE (0xBE + 1)
+#define AD5529R_REG_FUNC_DAC_INPUT_B(ch) \
+ (AD5529R_REG_FUNC_DAC_INPUT_B_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_DITHER_PERIOD_BASE (0xDE + 1)
+#define AD5529R_REG_FUNC_DITHER_PERIOD(ch) \
+ (AD5529R_REG_FUNC_DITHER_PERIOD_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_DITHER_PHASE_BASE (0xFE + 1)
+#define AD5529R_REG_FUNC_DITHER_PHASE(ch) \
+ (AD5529R_REG_FUNC_DITHER_PHASE_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_RAMP_STEP_BASE (0x11E + 1)
+#define AD5529R_REG_FUNC_RAMP_STEP(ch) \
+ (AD5529R_REG_FUNC_RAMP_STEP_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_INT_EN (0x13E + 1)
+
+/* Multiplexer and main DAC registers */
+#define AD5529R_REG_MUX_OUT_SEL (0x140 + 1)
+#define AD5529R_REG_MULTI_DAC_SW_LDAC (0x142 + 1)
+#define AD5529R_REG_MULTI_DAC_INPUT_A (0x144 + 1)
+#define AD5529R_REG_DAC_SW_LDAC (0x146 + 1)
+
+#define AD5529R_REG_DAC_INPUT_A_BASE (0x148 + 1)
+#define AD5529R_REG_DAC_INPUT_A(ch) (AD5529R_REG_DAC_INPUT_A_BASE + (ch) * 2)
+
+/* Status and readback registers */
+#define AD5529R_REG_FUNC_INT_STAT (0x168 + 1)
+#define AD5529R_REG_DAC_DATA_READBACK_BASE (0x16A + 1)
+#define AD5529R_REG_DAC_DATA_READBACK(ch) \
+ (AD5529R_REG_DAC_DATA_READBACK_BASE + (ch) * 2)
+
+/* Temperature sensor registers */
+#define AD5529R_REG_TSENS_EN (0x18A + 1)
+#define AD5529R_REG_TSENS_ALERT_FLAG (0x18C + 1)
+#define AD5529R_REG_TSENS_SHTD_FLAG (0x18E + 1)
+#define AD5529R_REG_TSENS_ALERT_STAT (0x190 + 1)
+#define AD5529R_REG_TSENS_SHTD_STAT (0x192 + 1)
+#define AD5529R_REG_ALARMB_TSENS_EN (0x194 + 1)
+#define AD5529R_REG_ALARMB_TSENS_SEL (0x196 + 1)
+#define AD5529R_REG_TSENS_SHTD_EN_CH (0x198 + 1)
+#define AD5529R_REG_DAC_DIS_DEGLITCH_CH (0x19A + 1)
+#define AD5529R_REG_DAC_INT_EN (0x19C + 1)
+#define AD5529R_REG_ALL_FUNC_INT_STAT (0x19E + 1)
+#define AD5529R_REG_FUNC_BUSY (0x1A0 + 1)
+#define AD5529R_REG_REF_SRC_SEL (0x1A2 + 1)
+#define AD5529R_REG_INIT_CRC_ERR_STAT (0x1A4 + 1)
+
+/* Hotpath registers for multi-device support */
+#define AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC (0x1A8 + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_0 (0x1AA + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_1 (0x1AC + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_2 (0x1AE + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_3 (0x1B0 + 1)
+#define AD5529R_REG_DAC_HOTPATH_SW_LDAC (0x1B2 + 1)
+
+/* Hotpath per-channel DAC input registers for each die */
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE (0x1B4 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0(ch) \
+ (AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE + (ch) * 2)
+
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE (0x1D4 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1(ch) \
+ (AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE + (ch) * 2)
+
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE (0x1F4 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2(ch) \
+ (AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE + (ch) * 2)
+
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE (0x214 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3(ch) \
+ (AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE + (ch) * 2)
+
+#define AD5529R_INTERFACE_CONFIG_A_SW_RESET (BIT(7) | BIT(0))
+#define AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION BIT(5)
+#define AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE BIT(4)
+#define AD5529R_INTERFACE_CONFIG_A_DEFAULT 0x10
+#define AD5529R_NUM_CHANNELS 16
+#define AD5529R_MAX_CHANNEL_INDEX (AD5529R_NUM_CHANNELS - 1)
+#define AD5529R_MAX_REGISTER (0x232 + 1)
+#define AD5529R_8BIT_REG_MAX 0x13
+#define AD5529R_ADDR(reg_addr) ((reg_addr) & 0xFFF)
+#define AD5529R_RESET_PULSE_US 1000
+#define AD5529R_RESET_DELAY_US 10000
+#define AD5529R_SPI_BUF_SIZE 4
+#define AD5529R_NUM_SUPPLIES 4
+#define AD5529R_SPI_READ_FLAG 0x80
+
+/* Device identification values */
+#define AD5529R_PRODUCT_ID_16BIT 0x4A
+#define AD5529R_PRODUCT_ID_12BIT 0x49
+
+struct ad5529r_model_data {
+ const char *model_name;
+ unsigned int resolution;
+ const struct iio_chan_spec *channels;
+};
+
+#define AD5529R_DAC_CHANNEL(chan, bits) { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .output = 1, \
+ .channel = (chan), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (bits), \
+ .storagebits = 16, \
+ }, \
+}
+
+static const char * const ad5529r_supply_names[AD5529R_NUM_SUPPLIES] = {
+ "vdd",
+ "avdd",
+ "hvdd",
+ "hvss",
+};
+
+static const struct iio_chan_spec ad5529r_channels_16bit[] = {
+ AD5529R_DAC_CHANNEL(0, 16),
+ AD5529R_DAC_CHANNEL(1, 16),
+ AD5529R_DAC_CHANNEL(2, 16),
+ AD5529R_DAC_CHANNEL(3, 16),
+ AD5529R_DAC_CHANNEL(4, 16),
+ AD5529R_DAC_CHANNEL(5, 16),
+ AD5529R_DAC_CHANNEL(6, 16),
+ AD5529R_DAC_CHANNEL(7, 16),
+ AD5529R_DAC_CHANNEL(8, 16),
+ AD5529R_DAC_CHANNEL(9, 16),
+ AD5529R_DAC_CHANNEL(10, 16),
+ AD5529R_DAC_CHANNEL(11, 16),
+ AD5529R_DAC_CHANNEL(12, 16),
+ AD5529R_DAC_CHANNEL(13, 16),
+ AD5529R_DAC_CHANNEL(14, 16),
+ AD5529R_DAC_CHANNEL(15, 16),
+};
+
+static const struct iio_chan_spec ad5529r_channels_12bit[] = {
+ AD5529R_DAC_CHANNEL(0, 12),
+ AD5529R_DAC_CHANNEL(1, 12),
+ AD5529R_DAC_CHANNEL(2, 12),
+ AD5529R_DAC_CHANNEL(3, 12),
+ AD5529R_DAC_CHANNEL(4, 12),
+ AD5529R_DAC_CHANNEL(5, 12),
+ AD5529R_DAC_CHANNEL(6, 12),
+ AD5529R_DAC_CHANNEL(7, 12),
+ AD5529R_DAC_CHANNEL(8, 12),
+ AD5529R_DAC_CHANNEL(9, 12),
+ AD5529R_DAC_CHANNEL(10, 12),
+ AD5529R_DAC_CHANNEL(11, 12),
+ AD5529R_DAC_CHANNEL(12, 12),
+ AD5529R_DAC_CHANNEL(13, 12),
+ AD5529R_DAC_CHANNEL(14, 12),
+ AD5529R_DAC_CHANNEL(15, 12),
+};
+
+static const struct ad5529r_model_data ad5529r_16bit_model_data = {
+ .model_name = "ad5529r-16",
+ .resolution = 16,
+ .channels = ad5529r_channels_16bit,
+};
+
+static const struct ad5529r_model_data ad5529r_12bit_model_data = {
+ .model_name = "ad5529r-12",
+ .resolution = 12,
+ .channels = ad5529r_channels_12bit,
+};
+
+struct ad5529r_state {
+ struct spi_device *spi;
+ const struct ad5529r_model_data *model_data;
+ struct regmap *regmap_8bit;
+ struct regmap *regmap_16bit;
+};
+
+static const struct regmap_range ad5529r_8bit_readable_ranges[] = {
+ regmap_reg_range(AD5529R_REG_INTERFACE_CONFIG_A, AD5529R_REG_CHIP_GRADE),
+ regmap_reg_range(AD5529R_REG_SCRATCH_PAD, AD5529R_REG_VENDOR_H),
+ regmap_reg_range(AD5529R_REG_STREAM_MODE, AD5529R_REG_INTERFACE_STATUS_A),
+};
+
+static const struct regmap_range ad5529r_16bit_readable_ranges[] = {
+ regmap_reg_range(AD5529R_REG_MULTI_DAC_CH_SEL, AD5529R_REG_LDAC_HW_SW),
+ regmap_reg_range(AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE,
+ AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_OUT_OPERATING_MODE, AD5529R_REG_OUT_OPERATING_MODE),
+ regmap_reg_range(AD5529R_REG_OUT_RANGE_BASE,
+ AD5529R_REG_OUT_RANGE_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_CAL_GAIN_BASE,
+ AD5529R_REG_CAL_GAIN_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_CAL_OFFSET_BASE,
+ AD5529R_REG_CAL_OFFSET_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_EN, AD5529R_REG_FUNC_EN),
+ regmap_reg_range(AD5529R_REG_FUNC_MODE_SEL_BASE,
+ AD5529R_REG_FUNC_MODE_SEL_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_DAC_INPUT_B_BASE,
+ AD5529R_REG_FUNC_DAC_INPUT_B_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_DITHER_PERIOD_BASE,
+ AD5529R_REG_FUNC_DITHER_PERIOD_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_DITHER_PHASE_BASE,
+ AD5529R_REG_FUNC_DITHER_PHASE_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_RAMP_STEP_BASE,
+ AD5529R_REG_FUNC_RAMP_STEP_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_INT_EN, AD5529R_REG_DAC_SW_LDAC),
+ regmap_reg_range(AD5529R_REG_DAC_INPUT_A_BASE,
+ AD5529R_REG_DAC_INPUT_A_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_FUNC_INT_STAT, AD5529R_REG_FUNC_INT_STAT),
+ regmap_reg_range(AD5529R_REG_DAC_DATA_READBACK_BASE,
+ AD5529R_REG_DAC_DATA_READBACK_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_TSENS_EN, AD5529R_REG_INIT_CRC_ERR_STAT),
+ regmap_reg_range(AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC, AD5529R_REG_DAC_HOTPATH_SW_LDAC),
+ regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE,
+ AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE +
+ AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE,
+ AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE +
+ AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE,
+ AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE +
+ AD5529R_MAX_CHANNEL_INDEX * 2),
+ regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE,
+ AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE +
+ AD5529R_MAX_CHANNEL_INDEX * 2),
+};
+
+static const struct regmap_access_table ad5529r_8bit_readable_table = {
+ .yes_ranges = ad5529r_8bit_readable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(ad5529r_8bit_readable_ranges),
+};
+
+static const struct regmap_access_table ad5529r_16bit_readable_table = {
+ .yes_ranges = ad5529r_16bit_readable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(ad5529r_16bit_readable_ranges),
+};
+
+static const struct regmap_range ad5529r_8bit_read_only_ranges[] = {
+ regmap_reg_range(AD5529R_REG_CHIP_TYPE, AD5529R_REG_CHIP_GRADE),
+ regmap_reg_range(AD5529R_REG_SPI_REVISION, AD5529R_REG_VENDOR_H),
+ regmap_reg_range(AD5529R_REG_DEVICE_CONFIG, AD5529R_REG_DEVICE_CONFIG),
+};
+
+static const struct regmap_range ad5529r_16bit_read_only_ranges[] = {
+ regmap_reg_range(AD5529R_REG_TSENS_ALERT_FLAG, AD5529R_REG_TSENS_SHTD_STAT),
+ regmap_reg_range(AD5529R_REG_ALL_FUNC_INT_STAT, AD5529R_REG_FUNC_BUSY),
+ regmap_reg_range(AD5529R_REG_INIT_CRC_ERR_STAT, AD5529R_REG_INIT_CRC_ERR_STAT),
+ regmap_reg_range(AD5529R_REG_DAC_DATA_READBACK_BASE,
+ AD5529R_REG_DAC_DATA_READBACK_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+};
+
+static const struct regmap_access_table ad5529r_8bit_writeable_table = {
+ .no_ranges = ad5529r_8bit_read_only_ranges,
+ .n_no_ranges = ARRAY_SIZE(ad5529r_8bit_read_only_ranges),
+};
+
+static const struct regmap_access_table ad5529r_16bit_writeable_table = {
+ .no_ranges = ad5529r_16bit_read_only_ranges,
+ .n_no_ranges = ARRAY_SIZE(ad5529r_16bit_read_only_ranges),
+};
+
+static const struct regmap_config ad5529r_regmap_8bit_config = {
+ .name = "ad5529r-8bit",
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = AD5529R_8BIT_REG_MAX,
+ .read_flag_mask = AD5529R_SPI_READ_FLAG,
+ .rd_table = &ad5529r_8bit_readable_table,
+ .wr_table = &ad5529r_8bit_writeable_table,
+};
+
+static const struct regmap_config ad5529r_regmap_16bit_config = {
+ .name = "ad5529r-16bit",
+ .reg_bits = 16,
+ .val_bits = 16,
+ .max_register = AD5529R_MAX_REGISTER,
+ .read_flag_mask = AD5529R_SPI_READ_FLAG,
+ .rd_table = &ad5529r_16bit_readable_table,
+ .wr_table = &ad5529r_16bit_writeable_table,
+};
+
+static struct regmap *ad5529r_get_regmap(struct ad5529r_state *st, unsigned int reg)
+{
+ if (reg <= AD5529R_8BIT_REG_MAX)
+ return st->regmap_8bit;
+
+ return st->regmap_16bit;
+}
+
+static int ad5529r_debugfs_reg_read(struct ad5529r_state *st, unsigned int reg,
+ unsigned int *val)
+{
+ return regmap_read(ad5529r_get_regmap(st, reg), reg, val);
+}
+
+static int ad5529r_debugfs_reg_write(struct ad5529r_state *st, unsigned int reg,
+ unsigned int val)
+{
+ return regmap_write(ad5529r_get_regmap(st, reg), reg, val);
+}
+
+static int ad5529r_detect_device(struct ad5529r_state *st)
+{
+ unsigned int product_id;
+ int ret;
+
+ ret = regmap_read(st->regmap_8bit, AD5529R_REG_PRODUCT_ID_L, &product_id);
+ if (ret)
+ return ret;
+
+ switch (product_id) {
+ case AD5529R_PRODUCT_ID_16BIT:
+ st->model_data = &ad5529r_16bit_model_data;
+ break;
+ case AD5529R_PRODUCT_ID_12BIT:
+ st->model_data = &ad5529r_12bit_model_data;
+ break;
+ default:
+ dev_err(&st->spi->dev, "Unknown product ID: 0x%02X\n", product_id);
+ return -ENODEV;
+ }
+
+ dev_dbg(&st->spi->dev, "Detected %s variant (Product ID: 0x%02X)\n",
+ st->model_data->model_name, product_id);
+
+ return 0;
+}
+
+static int ad5529r_reset(struct ad5529r_state *st)
+{
+ struct reset_control *rst;
+ int ret;
+
+ rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
+ if (IS_ERR(rst))
+ return PTR_ERR(rst);
+
+ if (rst) {
+ ret = reset_control_deassert(rst);
+ if (ret)
+ return ret;
+ } else {
+ ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
+ AD5529R_INTERFACE_CONFIG_A_SW_RESET);
+ if (ret)
+ return ret;
+ }
+
+ fsleep(AD5529R_RESET_DELAY_US);
+
+ return regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
+ AD5529R_INTERFACE_CONFIG_A_DEFAULT);
+}
+
+static int ad5529r_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct ad5529r_state *st = iio_priv(indio_dev);
+ unsigned int reg_addr;
+ unsigned int reg_val_h;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ reg_addr = AD5529R_REG_DAC_INPUT_A(chan->channel);
+ ret = regmap_read(st->regmap_16bit, reg_addr, ®_val_h);
+ if (ret)
+ return ret;
+
+ *val = reg_val_h;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ /*
+ * Using default 0-5V range: VOUTn = A × D/2^N + B
+ * where A = 5V, B = 0V, D = digital code, N = resolution
+ * Scale = 5000mV / 2^resolution
+ */
+ *val = 5000;
+ *val2 = st->model_data->resolution;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5529r_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct ad5529r_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val < 0 || val > GENMASK(st->model_data->resolution - 1, 0))
+ return -EINVAL;
+
+ return regmap_write(st->regmap_16bit, AD5529R_REG_DAC_INPUT_A(chan->channel), val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5529r_reg_access(struct iio_dev *indio_dev,
+ unsigned int reg,
+ unsigned int writeval,
+ unsigned int *readval)
+{
+ struct ad5529r_state *st = iio_priv(indio_dev);
+
+ if (!readval)
+ return ad5529r_debugfs_reg_write(st, reg, writeval);
+
+ return ad5529r_debugfs_reg_read(st, reg, readval);
+}
+
+static const struct iio_info ad5529r_info = {
+ .read_raw = ad5529r_read_raw,
+ .write_raw = ad5529r_write_raw,
+ .debugfs_reg_access = ad5529r_reg_access,
+};
+
+static int ad5529r_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct ad5529r_state *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+
+ st->spi = spi;
+
+ ret = devm_regulator_bulk_get_enable(dev, AD5529R_NUM_SUPPLIES,
+ ad5529r_supply_names);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get and enable regulators\n");
+
+ st->regmap_8bit = devm_regmap_init_spi(spi, &ad5529r_regmap_8bit_config);
+ if (IS_ERR(st->regmap_8bit))
+ return dev_err_probe(dev, PTR_ERR(st->regmap_8bit),
+ "Failed to initialize 8-bit regmap\n");
+
+ st->regmap_16bit = devm_regmap_init_spi(spi, &ad5529r_regmap_16bit_config);
+ if (IS_ERR(st->regmap_16bit))
+ return dev_err_probe(dev, PTR_ERR(st->regmap_16bit),
+ "Failed to initialize 16-bit regmap\n");
+
+ ret = ad5529r_reset(st);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to reset device\n");
+
+ ret = ad5529r_detect_device(st);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to detect device variant\n");
+
+ indio_dev->name = st->model_data->model_name;
+ indio_dev->info = &ad5529r_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = st->model_data->channels;
+ indio_dev->num_channels = AD5529R_NUM_CHANNELS;
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ad5529r_of_match[] = {
+ { .compatible = "adi,ad5529r" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad5529r_of_match);
+
+static const struct spi_device_id ad5529r_id[] = {
+ { "ad5529r" },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad5529r_id);
+
+static struct spi_driver ad5529r_driver = {
+ .driver = {
+ .name = "ad5529r",
+ .of_match_table = ad5529r_of_match,
+ },
+ .probe = ad5529r_probe,
+ .id_table = ad5529r_id,
+};
+module_spi_driver(ad5529r_driver);
+
+MODULE_AUTHOR("Janani Sunil <janani.sunil@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5529R 12/16-bit DAC driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: iio: dac: Add AD5529R
From: Janani Sunil @ 2026-05-08 11:55 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
In-Reply-To: <20260508-ad5529r-driver-v2-0-e315441685d7@analog.com>
Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
buffered voltage output digital-to-analog converter (DAC) with an
integrated precision reference.
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
.../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 96 ++++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 103 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
new file mode 100644
index 000000000000..f531b4865b01
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
+
+maintainers:
+ - Janani Sunil <janani.sunil@analog.com>
+
+description: |
+ The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
+ digital-to-analog converter (DAC) with an integrated precision reference.
+ The device operates from unipolar and bipolar supplies. It is guaranteed
+ monotonic and has built-in rail-to-rail output buffers that can source or
+ sink up to 25mA.
+
+ Specifications:
+ * 16 independent 12-bit or 16-bit DAC channels
+ * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
+ 0V to 40V, ±5V, ±10V, ±15V, and ±20V
+ * The device supports SPI communication with Mode 0 and Mode 3.
+ * 4.096V precision reference, 12ppm/°C maximum
+ * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
+ * Multiplexer for output voltage, load current sense and die temperature
+
+ Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
+
+properties:
+ compatible:
+ const: adi,ad5529r
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 50000000
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the RESET pin. Active low. When asserted low,
+ performs a power-on reset and initializes the device to its default state.
+
+ vdd-supply:
+ description: Digital power supply (typically 3.3V)
+
+ avdd-supply:
+ description: Analog power supply (typically 5V)
+
+ hvdd-supply:
+ description: High voltage positive supply (up to 40V for output range)
+
+ hvss-supply:
+ description: High voltage negative supply (ground or negative voltage)
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - avdd-supply
+ - hvdd-supply
+ - hvss-supply
+
+dependencies:
+ spi-cpha: [ spi-cpol ]
+ spi-cpol: [ spi-cpha ]
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dac@0 {
+ compatible = "adi,ad5529r";
+ reg = <0>;
+ spi-max-frequency = <25000000>;
+
+ vdd-supply = <&vdd_regulator>;
+ avdd-supply = <&avdd_regulator>;
+ hvdd-supply = <&hvdd_regulator>;
+ hvss-supply = <&hvss_regulator>;
+
+ reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d6c3c7d22403..320e84765ce6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1507,6 +1507,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/adc/adi,ad4851.yaml
F: drivers/iio/adc/ad4851.c
+ANALOG DEVICES INC AD5529R DRIVER
+M: Janani Sunil <janani.sunil@analog.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+
ANALOG DEVICES INC AD5706R DRIVER
M: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/3] Documentation: iio: Add AD5529R Documentation
From: Janani Sunil @ 2026-05-08 11:55 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
In-Reply-To: <20260508-ad5529r-driver-v2-0-e315441685d7@analog.com>
Add documentation for AD5529R high voltage, 16-channel 12/16 bit DAC
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
Documentation/iio/ad5529r.rst | 216 ++++++++++++++++++++++++++++++++++++++++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 1 +
3 files changed, 218 insertions(+)
diff --git a/Documentation/iio/ad5529r.rst b/Documentation/iio/ad5529r.rst
new file mode 100644
index 000000000000..41fea1521790
--- /dev/null
+++ b/Documentation/iio/ad5529r.rst
@@ -0,0 +1,216 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==============
+AD5529R driver
+==============
+
+Device driver for Analog Devices Inc. AD5529R 16-Channel 12/16-bit High Voltage DAC.
+The module name is ``ad5529r``.
+
+Supported devices
+=================
+
+* `AD5529R <https://www.analog.com/en/products/ad5529r.html>`_
+
+Description
+===========
+
+The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
+digital-to-analog converter (DAC) with an integrated precision reference.
+The device operates from unipolar and bipolar supplies and is guaranteed
+monotonic. It has built-in rail-to-rail output buffers that can source or
+sink up to 25mA.
+
+Hardware Features
+=================
+
+* 16 independent 12-bit or 16-bit DAC channels
+* Independently programmable output ranges:
+
+ - 0V to 5V (current driver default)
+ - 0V to 10V
+ - 0V to 20V
+ - 0V to 40V
+ - ±5V
+ - ±10V
+ - ±15V
+ - ±20V
+
+* 4.096V precision reference (12ppm/°C maximum)
+* Built-in function generation capabilities (hardware support)
+* Output voltage and current monitoring (hardware support)
+* Temperature monitoring with 8 on-chip sensors (hardware support)
+* Over-temperature protection
+* SPI interface with CRC error detection support
+
+Current Driver Implementation
+=============================
+
+The current driver provides basic DAC functionality with the following features:
+
+* Basic DAC output control for all 16 channels
+* Scale attributes for voltage conversion (0-5V default range)
+* SPI communication with regmap support
+* Reset control framework support
+* Automatic hardware variant detection (16-bit vs 12-bit) based on product ID
+* Debugfs register access for development
+
+SPI Configuration:
+
+* **Mode**: Supports SPI mode 0 and mode 3 (default: mode 0)
+* **Frequency**: Up to 50 MHz (typically tested at lower frequencies)
+* **Word Size**: 16-bit transactions
+
+.. note::
+ The device default configuration uses address decrement mode (ADDR_ASCENSION=0)
+ for multi-byte SPI transactions. Therefore, all 16-bit register addresses are
+ incremented by 1 in the driver to access the last byte first, allowing the
+ hardware to decrement and access the complete multi-byte register correctly.
+
+IIO Attributes (Currently Implemented)
+======================================
+
+Basic DAC Control
+-----------------
+
+For each of the 16 channels (0-15):
+
+**out_voltageY_raw**
+ Raw DAC code (12-bit: 0-4095, 16-bit: 0-65535)
+
+ * Read: Returns the current DAC register value
+ * Write: Sets the DAC output code
+
+**out_voltageY_scale**
+ Scale factor for voltage conversion (millivolts per LSB)
+
+ Based on the formula: VOUTn = A × D/2^N + B, where A=5V, B=0V, N=resolution
+
+ * 16-bit: 0.076294 mV/LSB (5V ÷ 2^16 = 5V ÷ 65536 = 0.076294mV)
+ * 12-bit: 1.220703 mV/LSB (5V ÷ 2^12 = 5V ÷ 4096 = 1.220703mV)
+ * Read-only attribute
+
+Debug Interface
+===============
+
+**Register Access**
+
+The driver provides debugfs register access for debugging and development:
+
+``/sys/kernel/debug/iio/iio:deviceX/direct_reg_access``
+ Direct register read/write access. Format:
+
+ * Read: ``echo <register_address> > direct_reg_access; cat direct_reg_access``
+ * Write: ``echo <register_address> <value> > direct_reg_access``
+
+Usage examples
+==============
+
+Basic DAC Output Control
+------------------------
+
+.. code-block:: bash
+
+ # Set channel 0 to mid-scale (approximately 2.5V with 0V to 5V range)
+ echo "32768" > /sys/bus/iio/devices/iio:device0/out_voltage0_raw
+
+ # Set channel 15 to full scale
+ echo "65535" > /sys/bus/iio/devices/iio:device0/out_voltage15_raw
+
+ # Read current value from channel 5
+ cat /sys/bus/iio/devices/iio:device0/out_voltage5_raw
+
+Scale Attributes
+----------------
+
+.. code-block:: bash
+
+ # Read scale factor (millivolts per LSB)
+ cat /sys/bus/iio/devices/iio:device0/out_voltage0_scale
+ # Output: 0.076294 (for 16-bit) or 1.220703 (for 12-bit)
+
+ # Convert raw to voltage: voltage_mv = raw * scale
+ # Formula: VOUTn = A × D/2^N + B where A=5V, B=0V
+
+Register Access for Development
+-------------------------------
+
+.. code-block:: bash
+
+ # Navigate to debugfs directory
+ cd /sys/kernel/debug/iio/iio:device0/
+
+ # Read device product ID (register 0x04)
+ echo 4 > direct_reg_access
+ cat direct_reg_access
+
+ # Write to a 16-bit configuration register (example: LDAC_HW_SW register 0x19)
+ echo "0x019 0xAA11" > direct_reg_access
+ cat direct_reg_access
+ # Output: 0xAA11
+
+ # Write to DAC channel registers (16-bit values)
+ echo "0x149 32768" > direct_reg_access # DAC channel 0 mid-scale
+ echo "0x14B 65535" > direct_reg_access # DAC channel 1 full-scale
+
+ # Read back DAC register values
+ echo 0x149 > direct_reg_access && cat direct_reg_access # Read channel 0
+ echo 0x14B > direct_reg_access && cat direct_reg_access # Read channel 1
+
+.. note::
+ For 16-bit registers, use hexadecimal format for addresses (0x019, 0x149, etc.).
+ Values can be decimal (32768) or hexadecimal (0xAA11). Register addresses shown
+ include the +1 offset required for decrement mode operation.
+
+Device Tree Configuration
+=========================
+
+Basic configuration example:
+
+.. code-block:: devicetree
+
+ &spi0 {
+ status = "okay";
+
+ ad5529r@0 {
+ compatible = "adi,ad5529r";
+ reg = <0>;
+ spi-max-frequency = <25000000>;
+
+ vdd-supply = <&vdd_regulator>;
+ avdd-supply = <&avdd_regulator>;
+ hvdd-supply = <&hvdd_regulator>;
+ hvss-supply = <&hvss_regulator>;
+
+ reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+For complete device tree binding documentation, see:
+``Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml``
+
+Driver Architecture
+===================
+
+The driver is structured as follows:
+
+* **Core**: Basic SPI communication and device initialization
+* **IIO Interface**: Standard IIO DAC channel interface with scale attributes
+* **Dual Regmap**: Uses standard regmap-spi for both 8-bit and 16-bit register access
+* **Reset Framework**: Reset control support
+
+Development Notes
+=================
+
+* The driver uses standard regmap-spi for both 8-bit and 16-bit register access
+* SPI mode 0 (CPOL=0, CPHA=0) is typically used
+* Reset control framework support for device initialization
+* Register addresses are incremented by 1 for 16-bit registers due to decrement mode addressing
+* Scale attributes provide voltage conversion for 0-5V range
+* Automatic regmap selection based on register address (≤0x13: 8-bit, >0x13: 16-bit)
+
+References
+==========
+
+* AD5529R Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
+* Linux IIO Subsystem: https://www.kernel.org/doc/html/latest/driver-api/iio/index.html
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 007e0a1fcc5a..27f2ab41f05e 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -25,6 +25,7 @@ Industrial I/O Kernel Drivers
ad4062
ad4691
ad4695
+ ad5529r
ad7191
ad7380
ad7606
diff --git a/MAINTAINERS b/MAINTAINERS
index 143714e27d51..41f42eb1adf2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1513,6 +1513,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+F: Documentation/iio/ad5529r.rst
F: drivers/iio/dac/ad5529r.c
ANALOG DEVICES INC AD5706R DRIVER
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/3] iio: dac: Add support for AD5529R DAC
From: Janani Sunil @ 2026-05-08 11:55 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
Janani Sunil
This patch series adds support for Analog Devices AD5529R, a 16 channel
16 and 12 bit voltage Digital-to-Analog Converter (DAC) with integrated
precision reference. The AD5529R operates from both unipolar and
bipolar supplies. The device communicates via SPI interface.
**Device Overview:**
The AD5529R features 16 independent DAC channels, with 16 or 12 bit
resolution, allowing independently programmable output ranges. The
internal 4.096V precision reference sets the accuracy of the output
voltage.
**Features Implemented:**
- Automatic detection of 12/16 bit variant with product ID read.
- Reset support via GPIO.
- Dual regmap configuration to handle 8 and 16 bit registers.
**Patch Summary:**
1. **dt-bindings**: Binding documentation with channel configuration.
2. **driver**: Implement IIO DAC Driver with regmap support.
3. **documentation**: Add driver documentation with usage examples.
**Testing:**
The driver was compiled and tested on the EVAL-AD5529R-ARDZ using a
coraZ7 with a mainline v7.0 kernel.
**Driver Rationale:**
AD5529R introduces:
1. A unique register layout
2. Mixed 8-bit and 16-bit register accesses
3. Product ID based generic identification
4. Hardware specific features like function generators, multi-die
hotpath registers etc.
The device warrants its own drivers due to these fundamental
architectural differences, that would require substantial changes to
existing drivers without providing reusable benefits. The standalone
driver also allows future extensions for related devices in the same
family.
Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
Changes in v2:
- Fix IIO scale to use millivolts per ABI requirement
- Fix documentation voltage calculations (2.5V not 2.048V)
- Fix bipolar ranges in documentation (±5V, ±10V, ±15V, ±20V)
- Fix alphabetical ordering in documentation index
- Add missing newline to documentation file
- Fix scale units description (millivolts not microvolts)
- Include a section for driver rationale in the cover letter
- Reword contents in cover letter 12/16 bit generic->variant
- Add dependency array for spi-cpha and spi-cpol properties
- Link to v1: https://lore.kernel.org/r/20260507-ad5529r-driver-v1-0-b4460f3cb44f@analog.com
---
Janani Sunil (3):
dt-bindings: iio: dac: Add AD5529R
iio: dac: Add AD5529R DAC driver support
Documentation: iio: Add AD5529R Documentation
.../devicetree/bindings/iio/dac/adi,ad5529r.yaml | 96 ++++
Documentation/iio/ad5529r.rst | 216 ++++++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 9 +
drivers/iio/dac/Kconfig | 17 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ad5529r.c | 564 +++++++++++++++++++++
7 files changed, 904 insertions(+)
---
base-commit: 93df88612859e8e19dec93c69d563b4b73e9bd4b
change-id: 20260507-ad5529r-driver-866bbdd864de
Best regards,
--
Janani Sunil <janani.sunil@analog.com>
^ permalink raw reply
* Re: [PATCH v10 8/9] platform/chrome: Protect cros_ec_device lifecycle with revocable
From: Jason Gunthorpe @ 2026-05-08 11:53 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Arnd Bergmann, Greg Kroah-Hartman, Bartosz Golaszewski,
Linus Walleij, Benson Leung, linux-kernel, chrome-platform,
driver-core, linux-doc, linux-gpio, Rafael J. Wysocki,
Danilo Krummrich, Jonathan Corbet, Shuah Khan, Laurent Pinchart,
Wolfram Sang, Johan Hovold, Paul E . McKenney
In-Reply-To: <20260508105448.31799-9-tzungbi@kernel.org>
On Fri, May 08, 2026 at 06:54:47PM +0800, Tzung-Bi Shih wrote:
> struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
> @@ -47,6 +49,15 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
> if (!ec_dev)
> return NULL;
>
> + ec_dev->its_rev = revocable_alloc(ec_dev);
> + if (!ec_dev->its_rev)
> + return NULL;
> + /*
> + * Drop the extra reference for the caller as the caller is the
> + * resource provider.
> + */
> + revocable_put(ec_dev->its_rev);
> +
> ec_dev->din_size = sizeof(struct ec_host_response) +
> sizeof(struct ec_response_get_protocol_info) +
> EC_MAX_RESPONSE_OVERHEAD;
FWIW I am still very much against seeing any revokable concept used
*between two drivers*. That will turn the kernel's lifetime model into
spaghetti code.
Your other series where you only have to change
drivers/platform/chrome/cros_ec_chardev.c just confirms how wrong this
approach is.
Given you say this is such a bug I think you really should be sending
a series that is patches 5 through 7 from the other series and a
simple rwsem instead of misc_deregister_sync() to deal with this bug
ASAP. No need to complicate a simple bug fix in a driver with all
these core changes.
Once the bug is fixed you can continue to try to propose more general
solutions.
Jason
^ permalink raw reply
* RE: [PATCH iwl-next v11] ice: add support for unmanaged DPLL on E830 NIC
From: Kubalewski, Arkadiusz @ 2026-05-08 11:24 UTC (permalink / raw)
To: Keller, Jacob E, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Nguyen, Anthony L, Kitszel, Przemyslaw,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
pmenzel@molgen.mpg.de, Loktionov, Aleksandr, horms@kernel.org,
Nitka, Grzegorz, Grinberg, Vitaly, Fodor, Zoltan
In-Reply-To: <ca256097-1538-4c08-ba01-777bf646fc33@intel.com>
>From: Keller, Jacob E <jacob.e.keller@intel.com>
>Sent: Tuesday, May 5, 2026 12:31 AM
>
>On 2/17/2026 7:58 AM, Arkadiusz Kubalewski wrote:
>> Hardware variants of E830 may support an unmanaged DPLL where the
>> configuration is hardcoded within the hardware and firmware, meaning
>> users cannot modify settings. However, users are able to check the DPLL
>> lock status and obtain configuration information through the Linux DPLL
>> and devlink health subsystem.
>>
>> Availability of 'loss of lock' health status code determines if such
>> support is available, if true, register single DPLL device with 1 input
>> and 1 output and provide hardcoded/read only properties of a pin and
>> DPLL device. User is only allowed to check DPLL device status and
>> receive
>> notifications on DPLL lock status change.
>>
>> When present, the DPLL device locks to an external signal provided
>> through the PCIe/OCP pin. The expected input signal is 1PPS
>> (1 Pulse Per Second) embedded on a 10MHz reference clock.
>> The DPLL produces output:
>> - for MAC (Media Access Control) & PHY (Physical Layer) clocks,
>> - 1PPS for synchronization of onboard PHC (Precision Hardware Clock)
>> timer.
>>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
>> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>> ---
>> v11:
>> - rebase and fix conflicts
>
>
>Jakub rightfully pointed out the mistaken inclusion of HAVE_DPLL_ESYNC.
>
True, fixed in v12.
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>> b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> index 73a4e28ed75f..64518202dfff 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> @@ -2588,6 +2601,21 @@ static const struct dpll_pin_ops
>> ice_dpll_output_ops = {
>> .esync_get = ice_dpll_output_esync_get,
>> };
>>
>> +static const struct dpll_pin_ops ice_dpll_input_unmanaged_ops = {
>> + .frequency_get = ice_dpll_input_frequency_get,
>> + .direction_get = ice_dpll_input_direction,
>> + .state_on_dpll_get = ice_dpll_input_state_get,
>> +#if defined(HAVE_DPLL_ESYNC)
>> + .esync_get = ice_dpll_input_esync_get,
>> +#endif /* HAVE_DPLL_ESYNC */
>> +};
>> +
>Please fix this, and remove any other remaining kcompat cruft as well.
>
>Also could you please review the following Sashiko reports and make sure
>there is no issues?
>
Sure thing.
>The netdev sashiko.dev bot says this:
>
>> commit 7aae4a54d3ef18d8694bc535b6e5959839c9adfd
>> Author: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>> ice: add support for unmanaged DPLL on E830 NIC
>> Hardware variants of E830 may support an unmanaged DPLL where the
>> configuration is hardcoded. This patch allows users to check the DPLL
>> lock status and obtain configuration information through the Linux DPLL
>> and devlink health subsystem.
>>> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c
>>>b/drivers/net/ethernet/intel/ice/devlink/health.c
>>> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
>>> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c
>> [ ... ]
>>> @@ -242,6 +244,8 @@ void ice_process_health_status_event(struct ice_pf
>>>*pf, struct ice_rq_event_info
>>> pf->health_reporters.fw_status = *health_info;
>>> devlink_health_report(pf->health_reporters.fw,
>>> "FW syndrome reported", NULL);
>>> + if (status_code ==
>>> ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK)
>>> + ice_dpll_lock_state_set_unmanaged(pf,
>> health_info, true);
>> Is it possible for this health event to arrive when the DPLL subsystem
>> is not
>> fully initialized or already torn down?
>> Looking at ice_dpll_lock_state_set_unmanaged(), it unconditionally
>> acquires
>> pf->dplls.lock:
>> void ice_dpll_lock_state_set_unmanaged(...)
>> {
>> ...
>> mutex_lock(&pf->dplls.lock);
>> ...
>> }
>> If the hardware doesn't support DPLL, ice_dpll_init() is never called,
>> leaving
>> the mutex uninitialized. Additionally, if ice_dpll_init_unmanaged()
>> fails,
>> it destroys the mutex and exits.
>> During driver unload, ice_dpll_deinit() destroys the mutex and frees the
>> DPLL structures, but the service task processing these admin queue
>> events
>> might not be stopped until later in the teardown sequence.
>> Could this lead to a kernel panic from locking an uninitialized or
>> destroyed
>> mutex, or a use-after-free when passing the d->dpll pointer to
>> dpll_device_change_ntf()?
>> Should there be a check ensuring DPLL is active (e.g. by checking
>> test_bit(ICE_FLAG_DPLL, pf->flags)) before handling the event?
>
>
>My own execution using claude had this to say:
>
Sure, fix in v12.
Except last finding.
>> Now I have everything. Let me compose the report.
>> commit d9b481c1d5d8be9dbbca36f40be64db91eb12caa
>> Author: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>> ice: add support for unmanaged DPLL on E830 NIC
>> This patch adds support for an unmanaged DPLL on E830 NIC variants,
>> where the DPLL configuration is hardcoded in hardware/firmware. It
>> registers a single PPS DPLL device with one input and one output,
>> reporting lock status via health status events and the DPLL subsystem.
>> A few things stood out during review.
>>> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c
>>> b/drivers/net/ethernet/intel/ice/devlink/health.c
>>> index 8e9a8a8178d4..31e6c5107c97 100644
>>> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
>>> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c
>> [ ... ]
>>> @@ -242,6 +244,8 @@ void ice_process_health_status_event(struct ice_pf
>>> *pf, struct ice_rq_event_info
>>> pf->health_reporters.fw_status = *health_info;
>>> devlink_health_report(pf->health_reporters.fw,
>>> "FW syndrome reported", NULL);
>>>+ if (status_code ==
>>>ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK)
>>>+ ice_dpll_lock_state_set_unmanaged(pf,
>>>health_info, true);
>>> break;
>> This calls ice_dpll_lock_state_set_unmanaged() for every PF that
>> receives a 0x601 health event, with no check for pf->dplls.unmanaged
>> or test_bit(ICE_FLAG_DPLL, pf->flags).
>> The only guard inside ice_dpll_lock_state_set_unmanaged() is
>> ice_pf_src_tmr_owned(), which checks PTP timer ownership -- not
>> whether the DPLL subsystem was initialized.
>> Consider a device where the firmware supports health code 0x601 but
>> the ZL80640 CGU is not in the netlist. In ice_init_features():
>>>+ err = ice_is_health_status_code_supported(&pf->hw, code,
>>>+ &pf->dplls.unmanaged);
>>>+ if (err || !ice_is_unmanaged_cgu_in_netlist(&pf->hw))
>>>+ pf->dplls.unmanaged = false;
>> This sets pf->dplls.unmanaged = false, so ice_dpll_init() is never
>> called and pf->dplls.lock is never initialized via mutex_init().
>> But the firmware can still send the health event, and then:
>>>+void ice_dpll_lock_state_set_unmanaged(struct ice_pf *pf,
>>>+ const struct ice_aqc_health_status_elem
>>>*buff,
>>>+ bool notify)
>>>+{
>> [ ... ]
>>>+ mutex_lock(&pf->dplls.lock);
>> Can this mutex_lock() on an uninitialized mutex cause undefined
>> behavior? With CONFIG_DEBUG_MUTEXES this would trigger a BUG().
>> Also, on managed DPLL devices (e.g. E810 with CGU), the DPLL worker
>> thread maintains pf->dplls.pps.dpll_state via ice_dpll_update_state().
>> Would this unconditional call overwrite the managed state machine?
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>>> index 3eea83b0cdc9..00704dfeb3b2 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> [ ... ]
>>>+static const struct dpll_pin_ops ice_dpll_input_unmanaged_ops = {
>>>+ .frequency_get = ice_dpll_input_frequency_get,
>>>+ .direction_get = ice_dpll_input_direction,
>>>+ .state_on_dpll_get = ice_dpll_input_state_get,
>>>+#if defined(HAVE_DPLL_ESYNC)
>>>+ .esync_get = ice_dpll_input_esync_get,
>>>+#endif /* HAVE_DPLL_ESYNC */
>>>+};
>> HAVE_DPLL_ESYNC does not appear to be defined anywhere in the upstream
>> kernel tree -- it looks like an out-of-tree compatibility macro. In
>> upstream builds this means .esync_get is always NULL for unmanaged
>> input pins.
>> The managed ice_dpll_input_ops sets .esync_get unconditionally, and
>> ice_dpll_input_esync_get() already has explicit unmanaged-specific
>> logic added by this patch. The init code also unconditionally sets
>> ICE_AQC_GET_CGU_IN_CFG_STATUS_ESYNC_CAP and ICE_DPLL_IN_ESYNC_ENABLED
>> on unmanaged input pins.
>> The documentation example at the end of the patch shows
>> esync-frequency and esync-pulse in the pin dump, but those fields
>> will never appear in an upstream build since the callback is compiled
>> out. Should the ifdef be removed so this matches ice_dpll_input_ops?
>> [ ... ]
>>> @@ -3153,12 +3181,15 @@ ice_dpll_init_direct_pins(struct ice_pf *pf,
>>> bool cgu,
>>> int ret;
>>>
>>> ret = ice_dpll_get_pins(pf, pins, start_idx, count, pf-
>>> dplls.clock_id);
>>>- if (ret)
>>>+ if (!cgu || ret)
>>> return ret;
>>>- if (cgu) {
>>>+
>>>+ if (first) {
>>> ret = ice_dpll_register_pins(first, pins, ops, count);
>>> if (ret)
>>> goto release_pins;
>>>+ }
>>>+ if (second) {
>>> ret = ice_dpll_register_pins(second, pins, ops, count);
>>> if (ret)
>>> goto unregister_first;
>> ice_dpll_init_direct_pins() was updated with NULL guards for first
>> and second. However, ice_dpll_deinit_direct_pins() was not:
>> ice_dpll_deinit_direct_pins() {
>> if (cgu) {
>> ice_dpll_unregister_pins(first, pins, ops, count);
>> ice_dpll_unregister_pins(second, pins, ops, count);
>> }
>> ...
>> }
>> For the unmanaged case, ice_dpll_init_unmanaged() only creates the
>> PPS dpll -- eec.dpll stays NULL. If output pin init fails inside
>> ice_dpll_init_pins(pf, true), the error path falls through to
>> deinit_inputs which calls:
>> ice_dpll_deinit_direct_pins(pf, cgu=true, ...,
>> pf->dplls.pps.dpll,
>> pf->dplls.eec.dpll);
>> ^^^^
>> NULL
>> This passes NULL as second to ice_dpll_unregister_pins(), which
>> calls dpll_pin_unregister(NULL, ...), dereferencing the NULL dpll
>> pointer. Would it make sense to add the same NULL guards to
>> ice_dpll_deinit_direct_pins() that were added to the init path?
>> [ ... ]
>>> @@ -3923,6 +4001,15 @@ ice_dpll_init_info_direct_pins(struct ice_pf
>>> *pf,
>>> pins[i].prop.board_label = ice_cgu_get_pin_name(hw, i, input);
>>> pins[i].prop.type = ice_cgu_get_pin_type(hw, i, input);
>>> if (input) {
>>>+ if (pf->dplls.unmanaged) {
>>>+ pins[i].freq = ice_dpll_pin_freq_info(hw, i,
>>>+ input);
>>>+ pins[i].state[0] = DPLL_PIN_STATE_CONNECTED;
>>>+ pins[i].status =
>>>+ ICE_AQC_GET_CGU_IN_CFG_STATUS_ESYNC_CAP;
>>>+ pins[i].flags[0] = ICE_DPLL_IN_ESYNC_ENABLED;
>>>+ continue;
>>>+ }
>> [ ... ]
>>> } else {
>>>+ if (pf->dplls.unmanaged) {
>>>+ pins[i].freq = ice_dpll_pin_freq_info(hw, i,
>>>+ input);
>>>+ pins[i].state[0] = DPLL_PIN_STATE_CONNECTED;
>>>+ continue;
>>>+ }
>> [ ... ]
>>> pins[i].prop.freq_supported =
>>> ice_cgu_get_pin_freq_supp(hw, i, input, &freq_supp_num);
>>> pins[i].prop.freq_supported_num = freq_supp_num;
>>> pins[i].pf = pf;
>>> }
>> Both unmanaged branches use continue, which skips the assignments of
>> pins[i].prop.freq_supported, pins[i].prop.freq_supported_num, and
>> pins[i].pf at the bottom of the loop.
>> The missing freq_supported / freq_supported_num means the DPLL netlink
>> code cannot report supported frequency ranges to userspace for
>> unmanaged pins, even though the data is available from
>> ice_cgu_get_pin_freq_supp().
>> The missing pins[i].pf = pf assignment leaves the pointer NULL (from
>> kcalloc zeroing). Current unmanaged callbacks use d->pf from
>> dpll_priv so there is no immediate crash, but if any callback that
>> dereferences p->pf were added to the unmanaged ops in the future it
>> would hit a NULL pointer dereference. Was the continue intended to
>> skip all of these assignments?
>
The last one is intended behavior, added a comment in v12.
Thank you!
Arkadiusz
^ permalink raw reply
* [PATCH iwl-next v12] ice: add support for unmanaged DPLL on E830 NIC
From: Arkadiusz Kubalewski @ 2026-05-08 11:08 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, anthony.l.nguyen, przemyslaw.kitszel, linux-doc,
linux-kernel, pmenzel, aleksandr.loktionov, horms, grzegorz.nitka,
vgrinber, zoltan.fodor, Arkadiusz Kubalewski
Hardware variants of E830 may support an unmanaged DPLL where the
configuration is hardcoded within the hardware and firmware, meaning
users cannot modify settings. However, users are able to check the DPLL
lock status and obtain configuration information through the Linux DPLL
and devlink health subsystem.
Availability of 'loss of lock' health status code determines if such
support is available, if true, register single DPLL device with 1 input
and 1 output and provide hardcoded/read only properties of a pin and
DPLL device. User is only allowed to check DPLL device status and receive
notifications on DPLL lock status change.
When present, the DPLL device locks to an external signal provided
through the PCIe/OCP pin. The expected input signal is 1PPS
(1 Pulse Per Second) embedded on a 10MHz reference clock.
The DPLL produces output:
- for MAC (Media Access Control) & PHY (Physical Layer) clocks,
- 1PPS for synchronization of onboard PHC (Precision Hardware Clock) timer.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
v12:
- remove HAVE_DPLL_ESYNC ifdef
- guard ice_dpll_lock_state_set_unmanaged() call in health event handler
with test_bit(ICE_FLAG_DPLL, pf->flags) and pf->dplls.unmanaged
- add NULL guards for first/second dpll in ice_dpll_deinit_direct_pins()
- add comments explaining intentional continue in
ice_dpll_init_info_direct_pins() for unmanaged pins
v11:
- rebase and fix conflicts
---
.../device_drivers/ethernet/intel/ice.rst | 83 +++++
.../net/ethernet/intel/ice/devlink/health.c | 6 +
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 12 +
drivers/net/ethernet/intel/ice/ice_common.c | 136 ++++++++
drivers/net/ethernet/intel/ice/ice_common.h | 8 +
drivers/net/ethernet/intel/ice/ice_dpll.c | 314 ++++++++++++++++--
drivers/net/ethernet/intel/ice/ice_dpll.h | 10 +
drivers/net/ethernet/intel/ice/ice_main.c | 11 +-
drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 46 +++
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 1 +
10 files changed, 604 insertions(+), 23 deletions(-)
diff --git a/Documentation/networking/device_drivers/ethernet/intel/ice.rst b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
index 0bca293cf9cb..09877066b031 100644
--- a/Documentation/networking/device_drivers/ethernet/intel/ice.rst
+++ b/Documentation/networking/device_drivers/ethernet/intel/ice.rst
@@ -941,6 +941,89 @@ To see input signal on those PTP pins, you need to configure DPLL properly.
Output signal is only visible on DPLL and to send it to the board SMA/U.FL pins,
DPLL output pins have to be manually configured.
+Unmanaged DPLL Support
+----------------------
+Hardware variants of E830 may support an unmanaged DPLL:
+
+- Intel(R) Ethernet Network Adapter E830-XXVDA8F for OCP 3.0,
+
+- Intel(R) Ethernet Network Adapter E830-XXVDA4F.
+
+In the case of the unmanaged DPLL, the configuration is hardcoded within the
+hardware and firmware, meaning users cannot modify settings. However,
+users can check the DPLL lock status and obtain configuration information
+through the Linux DPLL subsystem.
+
+When present, the DPLL device locks to an external signal provided through the
+PCIe/OCP pin. The expected input signal is 1PPS (1 Pulse Per Second) embedded
+on a 10MHz reference clock.
+The DPLL produces output:
+
+- for MAC (Media Access Control) & PHY (Physical Layer) clocks,
+
+- 1PPS for synchronization of onboard PHC (Precision Hardware Clock) timer.
+
+Requirements: The Linux kernel must have support for both the DPLL Subsystem
+and the Embedded Sync patch series.
+
+Example output of querying the Linux DPLL subsystem can be found below.
+
+.. code-block:: console
+ :caption: Dumping the DPLL pins
+
+ $ <ynl> --spec Documentation/netlink/specs/dpll.yaml --dump pin-get
+ [{'board-label': '1588-TIME_SYNC',
+ 'capabilities': set(),
+ 'clock-id': 282574471561216,
+ 'esync-frequency': 1,
+ 'esync-frequency-supported': [{'frequency-max': 1, 'frequency-min': 1}],
+ 'esync-pulse': 25,
+ 'frequency': 10000000,
+ 'id': 13,
+ 'module-name': 'ice',
+ 'parent-device': [{'direction': 'input',
+ 'parent-id': 6,
+ 'state': 'connected'}],
+ 'phase-adjust-max': 0,
+ 'phase-adjust-min': 0,
+ 'type': 'ext'},
+ {'board-label': 'MAC-PHY-CLK',
+ 'capabilities': set(),
+ 'clock-id': 282574471561216,
+ 'frequency': 156250000,
+ 'id': 14,
+ 'module-name': 'ice',
+ 'parent-device': [{'direction': 'output',
+ 'parent-id': 6,
+ 'state': 'connected'}],
+ 'phase-adjust-max': 0,
+ 'phase-adjust-min': 0,
+ 'type': 'synce-eth-port'},
+ {'board-label': '1588-TIME_REF',
+ 'capabilities': set(),
+ 'clock-id': 282574471561216,
+ 'frequency': 1,
+ 'id': 15,
+ 'module-name': 'ice',
+ 'parent-device': [{'direction': 'output',
+ 'parent-id': 6,
+ 'state': 'connected'}],
+ 'phase-adjust-max': 0,
+ 'phase-adjust-min': 0,
+ 'type': 'int-oscillator'}]
+
+.. code-block:: console
+ :caption: Dumping the DPLL devices
+
+ $ <ynl> --spec Documentation/netlink/specs/dpll.yaml --dump device-get
+ [{'clock-id': 282574471561216,
+ 'id': 6,
+ 'lock-status': 'locked',
+ 'mode': 'manual',
+ 'mode-supported': ['manual'],
+ 'module-name': 'ice',
+ 'type': 'pps'}]
+
GNSS module
-----------
Requires kernel compiled with CONFIG_GNSS=y or CONFIG_GNSS=m.
diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
index 8e9a8a8178d4..a83eb9f104c8 100644
--- a/drivers/net/ethernet/intel/ice/devlink/health.c
+++ b/drivers/net/ethernet/intel/ice/devlink/health.c
@@ -101,6 +101,8 @@ static const struct ice_health_status ice_health_status_lookup[] = {
"Supplied MIB file is invalid. DCB reverted to default configuration.",
"Disable FW-LLDP and check DCBx system configuration.",
{ice_port_number_label, "MIB ID"}},
+ {ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK, "Local DPLL lock status",
+ NULL,},
};
static int ice_health_status_lookup_compare(const void *a, const void *b)
@@ -242,6 +244,10 @@ void ice_process_health_status_event(struct ice_pf *pf, struct ice_rq_event_info
pf->health_reporters.fw_status = *health_info;
devlink_health_report(pf->health_reporters.fw,
"FW syndrome reported", NULL);
+ if (status_code == ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK &&
+ test_bit(ICE_FLAG_DPLL, pf->flags) &&
+ pf->dplls.unmanaged)
+ ice_dpll_lock_state_set_unmanaged(pf, health_info, true);
break;
case ICE_AQC_HEALTH_STATUS_PF:
case ICE_AQC_HEALTH_STATUS_PORT:
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index eeffbcf9480d..07fc72da347c 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1498,6 +1498,7 @@ struct ice_aqc_get_link_topo {
#define ICE_AQC_GET_LINK_TOPO_NODE_NR_PCA9575 0x21
#define ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032 0x24
#define ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384 0x25
+#define ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL80640 0x27
#define ICE_AQC_GET_LINK_TOPO_NODE_NR_E822_PHY 0x30
#define ICE_AQC_GET_LINK_TOPO_NODE_NR_C827 0x31
#define ICE_AQC_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX 0x47
@@ -2481,11 +2482,14 @@ enum ice_aqc_health_status {
ICE_AQC_HEALTH_STATUS_ERR_BMC_RESET = 0x50B,
ICE_AQC_HEALTH_STATUS_ERR_LAST_MNG_FAIL = 0x50C,
ICE_AQC_HEALTH_STATUS_ERR_RESOURCE_ALLOC_FAIL = 0x50D,
+ ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK = 0x601,
ICE_AQC_HEALTH_STATUS_ERR_FW_LOOP = 0x1000,
ICE_AQC_HEALTH_STATUS_ERR_FW_PFR_FAIL = 0x1001,
ICE_AQC_HEALTH_STATUS_ERR_LAST_FAIL_AQ = 0x1002,
};
+#define ICE_AQC_HEALTH_STATUS_CODE_NUM 64
+
/* Get Health Status (indirect 0xFF22) */
struct ice_aqc_get_health_status {
__le16 health_status_count;
@@ -2512,6 +2516,13 @@ struct ice_aqc_health_status_elem {
__le32 internal_data2;
};
+/* Get Health Status response buffer entry, (0xFF21)
+ * repeated per reported health status
+ */
+struct ice_aqc_health_status_supp_elem {
+ __le16 health_status_code;
+};
+
/* Admin Queue command opcodes */
enum ice_adminq_opc {
/* AQ commands */
@@ -2675,6 +2686,7 @@ enum ice_adminq_opc {
/* System Diagnostic commands */
ice_aqc_opc_set_health_status_cfg = 0xFF20,
+ ice_aqc_opc_get_supported_health_status_codes = 0xFF21,
ice_aqc_opc_get_health_status = 0xFF22,
/* FW Logging Commands */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index b617a6bff891..ef856d686f0a 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -3048,6 +3048,29 @@ bool ice_is_cgu_in_netlist(struct ice_hw *hw)
return false;
}
+/**
+ * ice_is_unmanaged_cgu_in_netlist - check for unmanaged CGU presence
+ * @hw: pointer to the hw struct
+ *
+ * Check if the unmanaged Clock Generation Unit (CGU) device is present in the netlist.
+ * Save the CGU part number in the hw structure for later use.
+ * Return:
+ * * true - unmanaged cgu is present
+ * * false - unmanaged cgu is not present
+ */
+bool ice_is_unmanaged_cgu_in_netlist(struct ice_hw *hw)
+{
+ if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
+ ICE_AQC_LINK_TOPO_NODE_CTX_GLOBAL,
+ ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL80640,
+ NULL)) {
+ hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL80640;
+ return true;
+ }
+
+ return false;
+}
+
/**
* ice_is_gps_in_netlist
* @hw: pointer to the hw struct
@@ -6310,6 +6333,119 @@ bool ice_is_fw_health_report_supported(struct ice_hw *hw)
ICE_FW_API_HEALTH_REPORT_PATCH);
}
+/**
+ * ice_aq_get_health_status_supported - get supported health status codes
+ * @hw: pointer to the HW struct
+ * @buff: pointer to buffer where health status elements will be stored
+ * @num: number of health status elements buffer can hold
+ *
+ * Return:
+ * * 0 - success,
+ * * negative - AQ error code.
+ */
+static int
+ice_aq_get_health_status_supported(struct ice_hw *hw,
+ struct ice_aqc_health_status_supp_elem *buff,
+ int num)
+{
+ u16 code = ice_aqc_opc_get_supported_health_status_codes;
+ struct libie_aq_desc desc;
+
+ ice_fill_dflt_direct_cmd_desc(&desc, code);
+
+ return ice_aq_send_cmd(hw, &desc, buff, num * sizeof(*buff), NULL);
+}
+
+/**
+ * ice_aq_get_health_status - get current health status array from the firmware
+ * @hw: pointer to the HW struct
+ * @buff: pointer to buffer where health status elements will be stored
+ * @num: number of health status elements buffer can hold
+ *
+ * Return:
+ * * 0 - success,
+ * * negative - AQ error code.
+ */
+int ice_aq_get_health_status(struct ice_hw *hw,
+ struct ice_aqc_health_status_elem *buff, int num)
+{
+ struct libie_aq_desc desc;
+
+ ice_fill_dflt_direct_cmd_desc(&desc,
+ ice_aqc_opc_get_health_status);
+
+ return ice_aq_send_cmd(hw, &desc, buff, num * sizeof(*buff), NULL);
+}
+
+/**
+ * ice_is_health_status_code_supported - check if health status code is supported
+ * @hw: pointer to the hardware structure
+ * @code: health status code to check
+ * @supported: pointer to boolean result
+ *
+ * Return: 0 on success, negative error code otherwise
+ */
+int ice_is_health_status_code_supported(struct ice_hw *hw, u16 code,
+ bool *supported)
+{
+ const int BUFF_SIZE = ICE_AQC_HEALTH_STATUS_CODE_NUM;
+ struct ice_aqc_health_status_supp_elem *buff;
+ int ret;
+
+ *supported = false;
+ buff = kzalloc_objs(*buff, BUFF_SIZE);
+ if (!buff)
+ return -ENOMEM;
+ ret = ice_aq_get_health_status_supported(hw, buff, BUFF_SIZE);
+ if (ret)
+ goto free_buff;
+ for (int i = 0; i < BUFF_SIZE && buff[i].health_status_code; i++)
+ if (le16_to_cpu(buff[i].health_status_code) == code) {
+ *supported = true;
+ break;
+ }
+
+free_buff:
+ kfree(buff);
+ return ret;
+}
+
+/**
+ * ice_get_last_health_status_code - get last health status for given code
+ * @hw: pointer to the hardware structure
+ * @out: pointer to the health status struct to be filled
+ * @code: health status code to check
+ *
+ * Return: 0 on success, negative error code otherwise
+ */
+int ice_get_last_health_status_code(struct ice_hw *hw,
+ struct ice_aqc_health_status_elem *out,
+ u16 code)
+{
+ const int BUFF_SIZE = ICE_AQC_HEALTH_STATUS_CODE_NUM;
+ struct ice_aqc_health_status_elem *buff;
+ int ret, last_status = -1;
+
+ buff = kzalloc_objs(*buff, BUFF_SIZE);
+ if (!buff)
+ return -ENOMEM;
+ ret = ice_aq_get_health_status(hw, buff, BUFF_SIZE);
+ if (ret)
+ goto free_buff;
+ for (int i = 0; i < BUFF_SIZE && buff[i].health_status_code; i++)
+ if (le16_to_cpu(buff[i].health_status_code) == code)
+ last_status = i;
+
+ if (last_status >= 0)
+ memcpy(out, &buff[last_status], sizeof(*out));
+ else
+ memset(out, 0, sizeof(*out));
+
+free_buff:
+ kfree(buff);
+ return ret;
+}
+
/**
* ice_aq_set_health_status_cfg - Configure FW health events
* @hw: pointer to the HW struct
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index ff6393e9be0c..ebced9edd5e3 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -162,6 +162,7 @@ ice_aq_get_phy_caps(struct ice_port_info *pi, bool qual_mods, u8 report_mode,
bool ice_is_phy_rclk_in_netlist(struct ice_hw *hw);
bool ice_is_clock_mux_in_netlist(struct ice_hw *hw);
bool ice_is_cgu_in_netlist(struct ice_hw *hw);
+bool ice_is_unmanaged_cgu_in_netlist(struct ice_hw *hw);
bool ice_is_gps_in_netlist(struct ice_hw *hw);
int
ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo *cmd,
@@ -188,6 +189,13 @@ ice_get_link_default_override(struct ice_link_default_override_tlv *ldo,
struct ice_port_info *pi);
bool ice_is_phy_caps_an_enabled(struct ice_aqc_get_phy_caps_data *caps);
bool ice_is_fw_health_report_supported(struct ice_hw *hw);
+int ice_aq_get_health_status(struct ice_hw *hw,
+ struct ice_aqc_health_status_elem *buff, int num);
+int ice_is_health_status_code_supported(struct ice_hw *hw, u16 code,
+ bool *supported);
+int ice_get_last_health_status_code(struct ice_hw *hw,
+ struct ice_aqc_health_status_elem *out,
+ u16 code);
int ice_aq_set_health_status_cfg(struct ice_hw *hw, u8 event_source);
int ice_aq_get_phy_equalization(struct ice_hw *hw, u16 data_in, u16 op_code,
u8 serdes_num, int *output);
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index b9c7df50123d..afe8a1480014 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -18,6 +18,8 @@
#define ICE_DPLL_SW_PIN_INPUT_BASE_SFP 4
#define ICE_DPLL_SW_PIN_INPUT_BASE_QSFP 6
#define ICE_DPLL_SW_PIN_OUTPUT_BASE 0
+#define ICE_DPLL_HEALTH_STATUS_LOCKED 1
+#define ICE_DPLL_HEALTH_STATUS_UNLOCKED 0
#define ICE_DPLL_PIN_SW_INPUT_ABS(in_idx) \
(ICE_DPLL_SW_PIN_INPUT_BASE_SFP + (in_idx))
@@ -80,6 +82,10 @@ static const struct dpll_pin_frequency ice_esync_range[] = {
DPLL_PIN_FREQUENCY_RANGE(0, DPLL_PIN_FREQUENCY_1_HZ),
};
+static const struct dpll_pin_frequency ice_esync_range_unmanaged[] = {
+ DPLL_PIN_FREQUENCY_1PPS,
+};
+
/**
* ice_dpll_is_sw_pin - check if given pin shall be controlled by SW
* @pf: private board structure
@@ -1089,9 +1095,11 @@ ice_dpll_pin_state_get(const struct dpll_pin *pin, void *pin_priv,
return -EBUSY;
mutex_lock(&pf->dplls.lock);
- ret = ice_dpll_pin_state_update(pf, p, pin_type, extack);
- if (ret)
- goto unlock;
+ if (!pf->dplls.unmanaged) {
+ ret = ice_dpll_pin_state_update(pf, p, pin_type, extack);
+ if (ret)
+ goto unlock;
+ }
if (pin_type == ICE_DPLL_PIN_TYPE_INPUT ||
pin_type == ICE_DPLL_PIN_TYPE_OUTPUT)
*state = p->state[d->dpll_idx];
@@ -2234,9 +2242,14 @@ ice_dpll_input_esync_get(const struct dpll_pin *pin, void *pin_priv,
mutex_unlock(&pf->dplls.lock);
return -EOPNOTSUPP;
}
- esync->range = ice_esync_range;
- esync->range_num = ARRAY_SIZE(ice_esync_range);
- if (p->flags[0] & ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN) {
+ if (pf->dplls.unmanaged) {
+ esync->range = ice_esync_range_unmanaged;
+ esync->range_num = ARRAY_SIZE(ice_esync_range_unmanaged);
+ } else {
+ esync->range = ice_esync_range;
+ esync->range_num = ARRAY_SIZE(ice_esync_range);
+ }
+ if (p->flags[0] & ICE_DPLL_IN_ESYNC_ENABLED) {
esync->freq = DPLL_PIN_FREQUENCY_1_HZ;
esync->pulse = ICE_DPLL_PIN_ESYNC_PULSE_HIGH_PERCENT;
} else {
@@ -2671,6 +2684,19 @@ static const struct dpll_pin_ops ice_dpll_output_ops = {
.esync_get = ice_dpll_output_esync_get,
};
+static const struct dpll_pin_ops ice_dpll_input_unmanaged_ops = {
+ .frequency_get = ice_dpll_input_frequency_get,
+ .direction_get = ice_dpll_input_direction,
+ .state_on_dpll_get = ice_dpll_input_state_get,
+ .esync_get = ice_dpll_input_esync_get,
+};
+
+static const struct dpll_pin_ops ice_dpll_output_unmanaged_ops = {
+ .frequency_get = ice_dpll_output_frequency_get,
+ .direction_get = ice_dpll_output_direction,
+ .state_on_dpll_get = ice_dpll_output_state_get,
+};
+
static const struct dpll_device_ops ice_dpll_ops = {
.lock_status_get = ice_dpll_lock_status_get,
.mode_get = ice_dpll_mode_get,
@@ -3225,8 +3251,10 @@ ice_dpll_deinit_direct_pins(struct ice_pf *pf, bool cgu,
struct dpll_device *second)
{
if (cgu) {
- ice_dpll_unregister_pins(first, pins, ops, count);
- ice_dpll_unregister_pins(second, pins, ops, count);
+ if (first)
+ ice_dpll_unregister_pins(first, pins, ops, count);
+ if (second)
+ ice_dpll_unregister_pins(second, pins, ops, count);
}
ice_dpll_release_pins(pins, count);
}
@@ -3258,12 +3286,15 @@ ice_dpll_init_direct_pins(struct ice_pf *pf, bool cgu,
int ret;
ret = ice_dpll_get_pins(pf, pins, start_idx, count, pf->dplls.clock_id);
- if (ret)
+ if (!cgu || ret)
return ret;
- if (cgu) {
+
+ if (first) {
ret = ice_dpll_register_pins(first, pins, ops, count);
if (ret)
goto release_pins;
+ }
+ if (second) {
ret = ice_dpll_register_pins(second, pins, ops, count);
if (ret)
goto unregister_first;
@@ -3272,7 +3303,8 @@ ice_dpll_init_direct_pins(struct ice_pf *pf, bool cgu,
return 0;
unregister_first:
- ice_dpll_unregister_pins(first, pins, ops, count);
+ if (first)
+ ice_dpll_unregister_pins(first, pins, ops, count);
release_pins:
ice_dpll_release_pins(pins, count);
return ret;
@@ -3529,6 +3561,18 @@ static void ice_dpll_deinit_pins(struct ice_pf *pf, bool cgu)
struct ice_dpll *de = &d->eec;
struct ice_dpll *dp = &d->pps;
+ if (d->unmanaged) {
+ ice_dpll_unregister_pins(dp->dpll, inputs,
+ &ice_dpll_input_unmanaged_ops,
+ num_inputs);
+ ice_dpll_unregister_pins(dp->dpll, outputs,
+ &ice_dpll_output_unmanaged_ops,
+ num_outputs);
+ ice_dpll_release_pins(inputs, num_inputs);
+ ice_dpll_release_pins(outputs, num_outputs);
+ return;
+ }
+
ice_dpll_deinit_rclk_pin(pf);
if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825)
ice_dpll_deinit_fwnode_pins(pf, pf->dplls.inputs, 0);
@@ -3713,23 +3757,29 @@ static int ice_dpll_init_pins(struct ice_pf *pf, bool cgu)
const struct dpll_pin_ops *input_ops;
int ret, count;
- input_ops = &ice_dpll_input_ops;
- output_ops = &ice_dpll_output_ops;
+ if (!pf->dplls.unmanaged) {
+ input_ops = &ice_dpll_input_ops;
+ output_ops = &ice_dpll_output_ops;
+ } else {
+ input_ops = &ice_dpll_input_unmanaged_ops;
+ output_ops = &ice_dpll_output_unmanaged_ops;
+ }
ret = ice_dpll_init_direct_pins(pf, cgu, pf->dplls.inputs, 0,
pf->dplls.num_inputs, input_ops,
- pf->dplls.eec.dpll,
- pf->dplls.pps.dpll);
+ pf->dplls.eec.dpll, pf->dplls.pps.dpll);
if (ret)
return ret;
count = pf->dplls.num_inputs;
- if (cgu) {
+ if (cgu || pf->dplls.unmanaged) {
ret = ice_dpll_init_direct_pins(pf, cgu, pf->dplls.outputs,
count, pf->dplls.num_outputs,
output_ops, pf->dplls.eec.dpll,
pf->dplls.pps.dpll);
if (ret)
goto deinit_inputs;
+ if (pf->dplls.unmanaged)
+ return 0;
count += pf->dplls.num_outputs;
if (!pf->dplls.generic) {
ret = ice_dpll_init_direct_pins(pf, cgu, pf->dplls.sma,
@@ -3842,7 +3892,8 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
if (type == DPLL_TYPE_PPS && ice_dpll_is_pps_phase_monitor(pf))
ops = &ice_dpll_pom_ops;
- ice_dpll_update_state(pf, d, true);
+ if (!pf->dplls.unmanaged)
+ ice_dpll_update_state(pf, d, true);
ret = dpll_device_register(d->dpll, type, ops, d);
if (ret) {
dpll_device_put(d->dpll, &d->tracker);
@@ -3869,6 +3920,33 @@ static void ice_dpll_deinit_worker(struct ice_pf *pf)
kthread_destroy_worker(d->kworker);
}
+/**
+ * ice_dpll_pin_freq_info - find pin frequency from supported ones
+ * @hw: pointer to the hardware structure
+ * @pin_idx: pin index
+ * @input: if input pin
+ *
+ * This function searches through the array of supported frequencies for a
+ * DPLL pin and returns single frequency pin is capable, if pin support only
+ * one frequency. Shall be used only for dpll with driver hardcoded frequency.
+ *
+ * Return:
+ * * 0 - failure, pin uses multiple frequencies,
+ * * frequency - success.
+ */
+static u64 ice_dpll_pin_freq_info(struct ice_hw *hw, u8 pin_idx, bool input)
+{
+ struct dpll_pin_frequency *freqs;
+ u8 freq_num;
+
+ /* Get supported frequencies for this pin */
+ freqs = ice_cgu_get_pin_freq_supp(hw, pin_idx, input, &freq_num);
+ if (!freqs || freq_num != 1 || freqs[0].min != freqs[0].max)
+ return 0;
+
+ return freqs[0].min;
+}
+
/**
* ice_dpll_init_worker - Initialize DPLLs periodic worker
* @pf: board private structure
@@ -4028,6 +4106,19 @@ ice_dpll_init_info_direct_pins(struct ice_pf *pf,
pins[i].prop.board_label = ice_cgu_get_pin_name(hw, i, input);
pins[i].prop.type = ice_cgu_get_pin_type(hw, i, input);
if (input) {
+ if (pf->dplls.unmanaged) {
+ pins[i].freq = ice_dpll_pin_freq_info(hw, i,
+ input);
+ pins[i].state[0] = DPLL_PIN_STATE_CONNECTED;
+ pins[i].status =
+ ICE_AQC_GET_CGU_IN_CFG_STATUS_ESYNC_CAP;
+ pins[i].flags[0] = ICE_DPLL_IN_ESYNC_ENABLED;
+ /* skip priority, capabilities, phase range,
+ * pin state AQ query and freq_supported -
+ * not available for unmanaged DPLL
+ */
+ continue;
+ }
ret = ice_aq_get_cgu_ref_prio(hw, de->dpll_idx, i,
&de->input_prio[i]);
if (ret)
@@ -4041,6 +4132,16 @@ ice_dpll_init_info_direct_pins(struct ice_pf *pf,
if (ice_dpll_is_sw_pin(pf, i, true))
pins[i].hidden = true;
} else {
+ if (pf->dplls.unmanaged) {
+ pins[i].freq = ice_dpll_pin_freq_info(hw, i,
+ input);
+ pins[i].state[0] = DPLL_PIN_STATE_CONNECTED;
+ /* skip output state caps, phase range,
+ * pin state AQ query and freq_supported -
+ * not available for unmanaged DPLL
+ */
+ continue;
+ }
ret = ice_cgu_get_output_pin_state_caps(hw, i, &caps);
if (ret)
return ret;
@@ -4058,10 +4159,13 @@ ice_dpll_init_info_direct_pins(struct ice_pf *pf,
pins[i].prop.freq_supported_num = freq_supp_num;
pins[i].pf = pf;
}
- if (input)
+ if (input && !pf->dplls.unmanaged) {
ret = ice_dpll_init_ref_sync_inputs(pf);
+ if (ret)
+ return ret;
+ }
- return ret;
+ return 0;
}
/**
@@ -4271,7 +4375,6 @@ static int ice_dpll_init_info_e825c(struct ice_pf *pf)
d->clock_id = ice_generate_clock_id(pf);
d->num_inputs = ICE_SYNCE_CLK_NUM;
-
d->inputs = kzalloc_objs(*d->inputs, d->num_inputs);
if (!d->inputs)
return -ENOMEM;
@@ -4296,6 +4399,82 @@ static int ice_dpll_init_info_e825c(struct ice_pf *pf)
return ret;
}
+/**
+ * ice_dpll_lock_state_init_unmanaged - initialize lock state for unmanaged dpll
+ * @pf: board private structure
+ *
+ * Initialize the lock state for unmanaged DPLL by checking health status.
+ * For unmanaged DPLL, we rely on hardware autonomous operation.
+ *
+ * Return:
+ * * 0 - success
+ * * negative - init failure reason
+ */
+static int ice_dpll_lock_state_init_unmanaged(struct ice_pf *pf)
+{
+ u16 code = ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK;
+ struct ice_aqc_health_status_elem buff;
+ int ret;
+
+ ret = ice_get_last_health_status_code(&pf->hw, &buff, code);
+ if (ret)
+ return ret;
+ ice_dpll_lock_state_set_unmanaged(pf, &buff, false);
+
+ return ret;
+}
+
+/**
+ * ice_dpll_init_info_unmanaged - init dpll information for unmanaged dpll
+ * @pf: board private structure
+ *
+ * Acquire (from HW) and set basic dpll information (on pf->dplls struct).
+ * For unmanaged dpll mode.
+ *
+ * Return:
+ * * 0 - success
+ * * negative - init failure reason
+ */
+static int ice_dpll_init_info_unmanaged(struct ice_pf *pf)
+{
+ struct ice_dplls *d = &pf->dplls;
+ int ret;
+
+ d->clock_id = ice_generate_clock_id(pf);
+ d->num_inputs = ice_cgu_get_pin_num(&pf->hw, true);
+ d->num_outputs = ice_cgu_get_pin_num(&pf->hw, false);
+ ret = ice_dpll_lock_state_init_unmanaged(pf);
+ if (ret)
+ return ret;
+ d->inputs = kzalloc_objs(*d->inputs, d->num_inputs);
+ if (!d->inputs)
+ return -ENOMEM;
+
+ ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_INPUT);
+ if (ret)
+ goto deinit_info;
+
+ d->outputs = kzalloc_objs(*d->outputs, d->num_outputs);
+ if (!d->outputs) {
+ ret = -ENOMEM;
+ goto deinit_info;
+ }
+
+ ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_OUTPUT);
+ if (ret)
+ goto deinit_info;
+
+ d->pps.mode = DPLL_MODE_MANUAL;
+ dev_dbg(ice_pf_to_dev(pf), "%s - success, inputs:%u, outputs:%u\n",
+ __func__, d->num_inputs, d->num_outputs);
+ return 0;
+deinit_info:
+ dev_err(ice_pf_to_dev(pf), "%s - fail: d->inputs:%p, d->outputs:%p\n",
+ __func__, d->inputs, d->outputs);
+ ice_dpll_deinit_info(pf);
+ return ret;
+}
+
/**
* ice_dpll_init_info - prepare pf's dpll information structure
* @pf: board private structure
@@ -4395,6 +4574,42 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
return ret;
}
+/**
+ * ice_dpll_lock_state_set_unmanaged - determine lock state from health status
+ * @pf: board private structure
+ * @buff: health status buffer
+ * @notify: if true, notify dpll device
+ *
+ * Set unmanaged dpll lock state based on health status code and internal data.
+ * Context: Acquires and releases pf->dplls.lock (must release before notify
+ * if called).
+ */
+void ice_dpll_lock_state_set_unmanaged(struct ice_pf *pf,
+ const struct ice_aqc_health_status_elem *buff,
+ bool notify)
+{
+ u32 internal_data = le32_to_cpu(buff->internal_data1);
+ struct ice_dpll *d = &pf->dplls.pps;
+
+ if (!ice_pf_src_tmr_owned(pf))
+ return;
+
+ mutex_lock(&pf->dplls.lock);
+ if (buff->health_status_code == 0 ||
+ internal_data == ICE_DPLL_HEALTH_STATUS_LOCKED)
+ d->dpll_state = DPLL_LOCK_STATUS_LOCKED;
+ else
+ d->dpll_state = DPLL_LOCK_STATUS_UNLOCKED;
+
+ if (d->prev_dpll_state == d->dpll_state)
+ notify = false;
+ else
+ d->prev_dpll_state = d->dpll_state;
+ mutex_unlock(&pf->dplls.lock);
+ if (notify && d->dpll)
+ dpll_device_change_ntf(d->dpll);
+}
+
/**
* ice_dpll_deinit - Disable the driver/HW support for dpll subsystem
* the dpll device.
@@ -4414,15 +4629,55 @@ void ice_dpll_deinit(struct ice_pf *pf)
if (cgu)
ice_dpll_deinit_worker(pf);
- ice_dpll_deinit_pins(pf, cgu);
+ ice_dpll_deinit_pins(pf, cgu || pf->dplls.unmanaged);
if (!IS_ERR_OR_NULL(pf->dplls.pps.dpll))
- ice_dpll_deinit_dpll(pf, &pf->dplls.pps, cgu);
+ ice_dpll_deinit_dpll(pf, &pf->dplls.pps,
+ cgu || pf->dplls.unmanaged);
if (!IS_ERR_OR_NULL(pf->dplls.eec.dpll))
ice_dpll_deinit_dpll(pf, &pf->dplls.eec, cgu);
ice_dpll_deinit_info(pf);
mutex_destroy(&pf->dplls.lock);
}
+/**
+ * ice_dpll_init_unmanaged - initialize support for unmanaged dpll subsystem
+ * @pf: board private structure
+ *
+ * Set up the device dplls for unmanaged mode, register them and pins connected
+ * within Linux dpll subsystem. Allow userspace to obtain state of DPLL.
+ *
+ * Context: Initializes pf->dplls.lock mutex.
+ */
+static void ice_dpll_init_unmanaged(struct ice_pf *pf)
+{
+ struct ice_dplls *d = &pf->dplls;
+ int err;
+
+ if (!ice_pf_src_tmr_owned(pf))
+ return;
+ mutex_init(&d->lock);
+ err = ice_dpll_init_info_unmanaged(pf);
+ if (err)
+ goto err_exit;
+ err = ice_dpll_init_dpll(pf, &pf->dplls.pps, true, DPLL_TYPE_PPS);
+ if (err)
+ goto deinit_info;
+ err = ice_dpll_init_pins(pf, true);
+ if (err)
+ goto deinit_pps;
+ set_bit(ICE_FLAG_DPLL, pf->flags);
+
+ return;
+
+deinit_pps:
+ ice_dpll_deinit_dpll(pf, &pf->dplls.pps, true);
+deinit_info:
+ ice_dpll_deinit_info(pf);
+err_exit:
+ mutex_destroy(&d->lock);
+ dev_warn(ice_pf_to_dev(pf), "DPLLs init failure err:%d\n", err);
+}
+
/**
* ice_dpll_init_e825 - initialize support for dpll subsystem
* @pf: board private structure
@@ -4510,8 +4765,23 @@ static void ice_dpll_init_e810(struct ice_pf *pf)
dev_warn(ice_pf_to_dev(pf), "DPLLs init failure err:%d\n", err);
}
+/**
+ * ice_dpll_init - initialize support for dpll subsystem
+ * @pf: board private structure
+ *
+ * Set up the device dplls, register them and pins connected within Linux dpll
+ * subsystem. Allow userspace to obtain state of DPLL and handling of DPLL
+ * configuration requests.
+ *
+ * Context: Initializes pf->dplls.lock mutex.
+ */
void ice_dpll_init(struct ice_pf *pf)
{
+ if (pf->dplls.unmanaged) {
+ ice_dpll_init_unmanaged(pf);
+ return;
+ }
+
switch (pf->hw.mac_type) {
case ICE_MAC_GENERIC_3K_E825:
ice_dpll_init_e825(pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index 8678575359b9..bb70c4333789 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -23,6 +23,8 @@
#define ICE_CGU_R11_SYNCE_S_BYP_CLK GENMASK(6, 1)
#define ICE_CGU_BYPASS_MUX_OFFSET_E825C 3
+#define ICE_DPLL_UNMANAGED_PIN_NUM 4
+#define ICE_DPLL_IN_ESYNC_ENABLED ICE_AQC_GET_CGU_IN_CFG_FLG2_ESYNC_EN
/**
* enum ice_dpll_pin_sw - enumerate ice software pin indices:
@@ -162,14 +164,22 @@ struct ice_dplls {
s32 output_phase_adj_max;
u32 periodic_counter;
bool generic;
+ bool unmanaged;
};
#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
void ice_dpll_init(struct ice_pf *pf);
void ice_dpll_deinit(struct ice_pf *pf);
+void ice_dpll_lock_state_set_unmanaged(struct ice_pf *pf,
+ const struct ice_aqc_health_status_elem *buff,
+ bool notify);
#else
static inline void ice_dpll_init(struct ice_pf *pf) { }
static inline void ice_dpll_deinit(struct ice_pf *pf) { }
+static inline void
+ice_dpll_lock_state_set_unmanaged(struct ice_pf *pf,
+ const struct ice_aqc_health_status_elem *buff,
+ bool notify) { }
#endif
#endif
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index b9a421773e91..1748e6e1ed01 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4716,7 +4716,9 @@ void ice_deinit_dev(struct ice_pf *pf)
static void ice_init_features(struct ice_pf *pf)
{
+ u16 code = ICE_AQC_HEALTH_STATUS_INFO_LOSS_OF_LOCK;
struct device *dev = ice_pf_to_dev(pf);
+ int err;
if (ice_is_safe_mode(pf))
return;
@@ -4728,8 +4730,15 @@ static void ice_init_features(struct ice_pf *pf)
if (ice_is_feature_supported(pf, ICE_F_GNSS))
ice_gnss_init(pf);
+ /* Initialize unmanaged DPLL detection */
+ err = ice_is_health_status_code_supported(&pf->hw, code,
+ &pf->dplls.unmanaged);
+ if (err || !ice_is_unmanaged_cgu_in_netlist(&pf->hw))
+ pf->dplls.unmanaged = false;
+
if (ice_is_feature_supported(pf, ICE_F_CGU) ||
- ice_is_feature_supported(pf, ICE_F_PHY_RCLK))
+ ice_is_feature_supported(pf, ICE_F_PHY_RCLK) ||
+ pf->dplls.unmanaged)
ice_dpll_init(pf);
/* Note: Flow director init failure is non-fatal to load */
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 24fb7a3e14d6..d8d20b1ef209 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -20,6 +20,10 @@ static struct dpll_pin_frequency ice_cgu_pin_freq_10_mhz[] = {
DPLL_PIN_FREQUENCY_10MHZ,
};
+static struct dpll_pin_frequency ice_cgu_pin_freq_156_25mhz[] = {
+ DPLL_PIN_FREQUENCY_RANGE(156250000, 156250000),
+};
+
static const struct ice_cgu_pin_desc ice_e810t_sfp_cgu_inputs[] = {
{ "CVL-SDP22", ZL_REF0P, DPLL_PIN_TYPE_INT_OSCILLATOR,
ARRAY_SIZE(ice_cgu_pin_freq_common), ice_cgu_pin_freq_common },
@@ -131,6 +135,18 @@ static const struct ice_cgu_pin_desc ice_e823_zl_cgu_outputs[] = {
{ "NONE", ZL_OUT5, 0, 0 },
};
+static const struct ice_cgu_pin_desc ice_e830_unmanaged_inputs[] = {
+ { "1588-TIME_SYNC", 0, DPLL_PIN_TYPE_EXT,
+ ARRAY_SIZE(ice_cgu_pin_freq_10_mhz), ice_cgu_pin_freq_10_mhz },
+};
+
+static const struct ice_cgu_pin_desc ice_e830_unmanaged_outputs[] = {
+ { "MAC-PHY-CLK", 0, DPLL_PIN_TYPE_SYNCE_ETH_PORT,
+ ARRAY_SIZE(ice_cgu_pin_freq_156_25mhz), ice_cgu_pin_freq_156_25mhz },
+ { "1588-TIME_REF", 1, DPLL_PIN_TYPE_INT_OSCILLATOR,
+ ARRAY_SIZE(ice_cgu_pin_freq_1_hz), ice_cgu_pin_freq_1_hz},
+};
+
/* Low level functions for interacting with and managing the device clock used
* for the Precision Time Protocol.
*
@@ -5923,6 +5939,24 @@ ice_cgu_get_pin_desc(struct ice_hw *hw, bool input, int *size)
case ICE_DEV_ID_E823C_SGMII:
t = ice_cgu_get_pin_desc_e823(hw, input, size);
break;
+ case ICE_DEV_ID_E830CC_BACKPLANE:
+ case ICE_DEV_ID_E830CC_QSFP56:
+ case ICE_DEV_ID_E830CC_SFP:
+ case ICE_DEV_ID_E830CC_SFP_DD:
+ case ICE_DEV_ID_E830C_BACKPLANE:
+ case ICE_DEV_ID_E830C_QSFP:
+ case ICE_DEV_ID_E830C_SFP:
+ case ICE_DEV_ID_E830_XXV_BACKPLANE:
+ case ICE_DEV_ID_E830_XXV_QSFP:
+ case ICE_DEV_ID_E830_XXV_SFP:
+ if (input) {
+ t = ice_e830_unmanaged_inputs;
+ *size = ARRAY_SIZE(ice_e830_unmanaged_inputs);
+ } else {
+ t = ice_e830_unmanaged_outputs;
+ *size = ARRAY_SIZE(ice_e830_unmanaged_outputs);
+ }
+ break;
default:
break;
}
@@ -5949,6 +5983,18 @@ int ice_cgu_get_num_pins(struct ice_hw *hw, bool input)
return 0;
}
+/**
+ * ice_cgu_get_pin_num - get pin description array size
+ * @hw: pointer to the hw struct
+ * @input: if request is done against input or output pins
+ *
+ * Return: size of pin description array for given hw.
+ */
+int ice_cgu_get_pin_num(struct ice_hw *hw, bool input)
+{
+ return ice_cgu_get_num_pins(hw, input);
+}
+
/**
* ice_cgu_get_pin_type - get pin's type
* @hw: pointer to the hw struct
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 1c9e77dbc770..98bca7cae88d 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -357,6 +357,7 @@ int ice_read_sma_ctrl(struct ice_hw *hw, u8 *data);
int ice_write_sma_ctrl(struct ice_hw *hw, u8 data);
int ice_ptp_read_sdp_ac(struct ice_hw *hw, __le16 *entries, uint *num_entries);
int ice_cgu_get_num_pins(struct ice_hw *hw, bool input);
+int ice_cgu_get_pin_num(struct ice_hw *hw, bool input);
enum dpll_pin_type ice_cgu_get_pin_type(struct ice_hw *hw, u8 pin, bool input);
struct dpll_pin_frequency *
ice_cgu_get_pin_freq_supp(struct ice_hw *hw, u8 pin, bool input, u8 *num);
base-commit: 1a5abe9a93c8f8f0c9d10fc313aff320e4487268
--
2.47.0
^ permalink raw reply related
* [PATCH 3/3] Documentation/gpu/rfc: fix toctree
From: Jani Nikula @ 2026-05-08 11:12 UTC (permalink / raw)
To: dri-devel, linux-doc; +Cc: jani.nikula
In-Reply-To: <cover.1778238671.git.jani.nikula@intel.com>
Just one toctree is enough. The .rst suffixes are superfluous in the
toctree. Fix indent. Add missing newline at the end of the file.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
Documentation/gpu/rfc/index.rst | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index ef19b0ba2a3e..26a7ebe6fb44 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -18,23 +18,9 @@ host such documentation:
.. toctree::
- gpusvm.rst
-
-.. toctree::
-
- i915_gem_lmem.rst
-
-.. toctree::
-
- i915_scheduler.rst
-
-.. toctree::
-
- i915_small_bar.rst
-
-.. toctree::
-
- i915_vm_bind.rst
-
-.. toctree::
- color_pipeline.rst
\ No newline at end of file
+ gpusvm
+ i915_gem_lmem
+ i915_scheduler
+ i915_small_bar
+ i915_vm_bind
+ color_pipeline
--
2.47.3
^ permalink raw reply related
* [PATCH 2/3] Documentation/gpu: add some tables of contents to large documents
From: Jani Nikula @ 2026-05-08 11:12 UTC (permalink / raw)
To: dri-devel, linux-doc; +Cc: jani.nikula
In-Reply-To: <cover.1778238671.git.jani.nikula@intel.com>
Some of the GPU documentation pages are quite long, with various levels
of details. Add document internal tables of contents to the larger
documents to make them easier to navigate.
The index.rst in the sub-directories have toctrees, which provide
similar overviews.
Fix one missing newline at the end of drm-uapi.rst while at it,
primarily because rst should have it, and secondarily because my editor
rst mode refuses to save the file without it.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
Documentation/gpu/driver-uapi.rst | 2 ++
Documentation/gpu/drm-internals.rst | 2 ++
Documentation/gpu/drm-kms-helpers.rst | 2 ++
Documentation/gpu/drm-kms.rst | 2 ++
Documentation/gpu/drm-mm.rst | 2 ++
Documentation/gpu/drm-ras.rst | 2 ++
Documentation/gpu/drm-uapi.rst | 4 +++-
Documentation/gpu/drm-usage-stats.rst | 2 ++
Documentation/gpu/introduction.rst | 2 ++
9 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
index 1f15a8ca1265..627fc68c7a21 100644
--- a/Documentation/gpu/driver-uapi.rst
+++ b/Documentation/gpu/driver-uapi.rst
@@ -2,6 +2,8 @@
DRM Driver uAPI
===============
+.. contents::
+
drm/i915 uAPI
=============
diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index 94f93fd3b8a0..a3ce25a36f1d 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -18,6 +18,8 @@ event handling, memory management, output management, framebuffer
management, command submission & fencing, suspend/resume support, and
DMA services.
+.. contents::
+
Driver Initialization
=====================
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index b4a9e5ae81f6..80453dda33b8 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -33,6 +33,8 @@ There are a few areas these helpers can grouped into:
pipeline: Planes, handling rectangles for visibility checking and scissoring,
flip queues and assorted bits.
+.. contents::
+
Modeset Helper Reference for Common Vtables
===========================================
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index fbe0583eb84c..d22817fdf9aa 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -15,6 +15,8 @@ be setup by initializing the following fields.
- struct drm_mode_config_funcs \*funcs;
Mode setting functions.
+.. contents::
+
Overview
========
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 32fb506db05b..2dea94f77d52 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -25,6 +25,8 @@ share it. GEM has simpler initialization and execution requirements than
TTM, but has no video RAM management capabilities and is thus limited to
UMA devices.
+.. contents::
+
The Translation Table Manager (TTM)
===================================
diff --git a/Documentation/gpu/drm-ras.rst b/Documentation/gpu/drm-ras.rst
index 4636e68f5678..83c21853b74b 100644
--- a/Documentation/gpu/drm-ras.rst
+++ b/Documentation/gpu/drm-ras.rst
@@ -24,6 +24,8 @@ Key Goals:
nodes for different IP blocks, sub-blocks, or other logical subdivisions
as applicable.
+.. contents::
+
Nodes
=====
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 32206ce62931..2c2f939322fb 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -16,6 +16,8 @@ management, and output management.
Cover generic ioctls and sysfs layout here. We only need high-level
info, since man pages should cover the rest.
+.. contents::
+
libdrm Device Lookup
====================
@@ -765,4 +767,4 @@ Stable uAPI events
From ``drivers/gpu/drm/scheduler/gpu_scheduler_trace.h``
.. kernel-doc:: drivers/gpu/drm/scheduler/gpu_scheduler_trace.h
- :doc: uAPI trace events
\ No newline at end of file
+ :doc: uAPI trace events
diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
index 24d3012ca7a6..70b7cfcc194f 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -16,6 +16,8 @@ output is split between common and driver specific parts. Having said that,
wherever possible effort should still be made to standardise as much as
possible.
+.. contents::
+
File format specification
=========================
diff --git a/Documentation/gpu/introduction.rst b/Documentation/gpu/introduction.rst
index d8f519693fc2..64074ac22d9b 100644
--- a/Documentation/gpu/introduction.rst
+++ b/Documentation/gpu/introduction.rst
@@ -16,6 +16,8 @@ found in current kernels.
[Insert diagram of typical DRM stack here]
+.. contents::
+
Style Guidelines
================
--
2.47.3
^ permalink raw reply related
* RE: [PATCH v9 4/6] iio: adc: ad4691: add SPI offload support
From: Sabau, Radu bogdan @ 2026-05-08 11:11 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
Bartosz Golaszewski, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <20260507161111.555bba75@jic23-huawei>
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Thursday, May 7, 2026 6:11 PM
> To: Sabau, Radu bogdan <Radu.Sabau@analog.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; David Lechner <dlechner@baylibre.com>;
> Sa, Nuno <Nuno.Sa@analog.com>; Andy Shevchenko <andy@kernel.org>;
> Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Uwe Kleine-König
> <ukleinek@kernel.org>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; Linus Walleij <linusw@kernel.org>; Bartosz
> Golaszewski <brgl@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Shuah Khan
> <skhan@linuxfoundation.org>; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> pwm@vger.kernel.org; linux-gpio@vger.kernel.org; linux-doc@vger.kernel.org
> Subject: Re: [PATCH v9 4/6] iio: adc: ad4691: add SPI offload support
>
>
> > > + /* TX: address phase, CS stays asserted into data phase */
> > > + st->scan_xfers[2 * k].tx_buf = offload->tx_cmd[k];
> > > + st->scan_xfers[2 * k].len = sizeof(offload->tx_cmd[k]);
> > > + st->scan_xfers[2 * k].bits_per_word = bpw;
> >
> > "When bits_per_word is greater than 8 (like bpw = 16 here), the SPI
> framework
> > treats tx_buf as an array of native 16-bit words.
> > On little-endian architectures, the controller will byte-swap the data before
> > transmitting it. Will using a u8 array and put_unaligned_be16() result in the
> > command bytes being reversed on the wire?"
> >
> > Switched to cpu_to_be16() assigned directly into __be16 scan_tx[],
> > matching the non-offload path. This makes the intended wire format
> > self-evident and sidesteps the byte-ordering question entirely.
>
> This confuses me a bit because the SPI controller should work with
> native endian and from that generate the expected big endian on the wire.
>
> So on a little endian host byte order in address space is LH but it will
> write top bit of H first thus the ADC channel address needs to be in the
> second byte.
> On a big endian host despite the ordering in memory being HL, the top
> bit of H is still written first thus in needs to be in the first byte.
>
>
> If you using cpu_to_be16() to assign a 16 bit value swapping only on little
> endian
> and start with the cmd in L on little endian you'll end up with LH swapped to
> HL and on big endian HL but the little endian SPI controller should then swap
> it again sending what it thinks is the high byte first (L) whereas the big endian
> system will send H.
>
> Upshot. I think the field should be native endian. If a byte swap is needed
> it should be unconditional and not rely on endianness of the host.
>
Correct. With bits_per_word=16 the SPI controller reads tx_buf as a native
16-bit word. cpu_to_be16() stores BE bytes, but on an LE host those bytes are
read back as a different native integer, sending the wrong byte first.
scan_tx is declared as u16. Both offload paths store the exact wire value as a
plain native u16 — no endianness macro. Native storage is self-consistent:
store X, read back X, SPI shifts X out MSB-first → correct wire bytes on any
host. Trace for AVG_IN(0) = 0x8201, expected wire [0x82, 0x01]:
cpu_to_be16(0x8201), LE host → native 0x0182 → wire [0x01, 0x82] (incorrect)
(u16)0x8201, LE host → native 0x8201 → wire [0x82, 0x01] (correct)
(u16)0x8201, BE host → native 0x8201 → wire [0x82, 0x01] (correct)
Manual offload: TX and RX are full-duplex (same clock cycles, shared xfer):
st->scan_tx[k] = (u16)(AD4691_ADC_CHAN(bit) << 8);
/* e.g. ch=0: 0x8000 → wire [0x80, 0x00] */
CNV burst offload: TX and RX are separate xfers, both use bits_per_word=bpw:
st->scan_tx[k] = 0x8000 | AD4691_AVG_IN(bit);
/* e.g. AVG_IN(0): 0x8201 → wire [0x82, 0x01] */
The state-reset transfer uses a u8 buffer with bits_per_word=8 (default);
put_unaligned_be16() writes bytes in memory order, which SPI sends as-is.
^ permalink raw reply
* [PATCH 1/3] Documentation/gpu: limit main toctree depth to 2
From: Jani Nikula @ 2026-05-08 11:12 UTC (permalink / raw)
To: dri-devel, linux-doc; +Cc: jani.nikula
In-Reply-To: <cover.1778238671.git.jani.nikula@intel.com>
The main GPU documentation toctree has no limit to the toctree depth,
which means the main GPU index page recursively includes all the
headings in all of GPU documentation in the single table of
contents. This makes getting any kind of overview of the documentation
really difficult.
Limit the main toctree depth to 2 i.e. show at most two levels of
headings.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
Documentation/gpu/index.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
index 5d708a106b3f..65bf3b26e4f4 100644
--- a/Documentation/gpu/index.rst
+++ b/Documentation/gpu/index.rst
@@ -3,6 +3,7 @@ GPU Driver Developer's Guide
============================
.. toctree::
+ :maxdepth: 2
introduction
drm-internals
--
2.47.3
^ permalink raw reply related
* Re: [PATCH 7.2 v16 03/13] mm/khugepaged: rework max_ptes_* handling with helper functions
From: Lance Yang @ 2026-05-08 11:11 UTC (permalink / raw)
To: npache
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, david, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
lance.yang, Liam.Howlett, ljs, mathieu.desnoyers, matthew.brost,
mhiramat, mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <20260419185750.260784-4-npache@redhat.com>
On Sun, Apr 19, 2026 at 12:57:40PM -0600, Nico Pache wrote:
>The following cleanup reworks all the max_ptes_* handling into helper
>functions. This increases the code readability and will later be used to
>implement the mTHP handling of these variables.
>
>With these changes we abstract all the madvise_collapse() special casing
>(dont respect the sysctls) away from the functions that utilize them. And
>will later in this series to cleanly restrict mTHP collapses behaviors.
>
>Suggested-by: David Hildenbrand <david@kernel.org>
>Signed-off-by: Nico Pache <npache@redhat.com>
>---
Nice. It should all be an equivalence-preserving refactor.
With David's suggested changes:
Reviewed-by: Lance Yang <lance.yang@linux.dev>
^ permalink raw reply
* [PATCH 0/3] Documentation/gpu: tables of contents cleanups and fixes
From: Jani Nikula @ 2026-05-08 11:12 UTC (permalink / raw)
To: dri-devel, linux-doc; +Cc: jani.nikula
Make the GPU documentation slightly easier to navigate.
Jani Nikula (3):
Documentation/gpu: limit main toctree depth to 2
Documentation/gpu: add some tables of contents to large documents
Documentation/gpu/rfc: fix toctree
Documentation/gpu/driver-uapi.rst | 2 ++
Documentation/gpu/drm-internals.rst | 2 ++
Documentation/gpu/drm-kms-helpers.rst | 2 ++
Documentation/gpu/drm-kms.rst | 2 ++
Documentation/gpu/drm-mm.rst | 2 ++
Documentation/gpu/drm-ras.rst | 2 ++
Documentation/gpu/drm-uapi.rst | 4 +++-
Documentation/gpu/drm-usage-stats.rst | 2 ++
Documentation/gpu/index.rst | 1 +
Documentation/gpu/introduction.rst | 2 ++
Documentation/gpu/rfc/index.rst | 26 ++++++--------------------
11 files changed, 26 insertions(+), 21 deletions(-)
--
2.47.3
^ permalink raw reply
* RE: [PATCH v9 3/6] iio: adc: ad4691: add triggered buffer support
From: Sabau, Radu bogdan @ 2026-05-08 11:08 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
Bartosz Golaszewski, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <20260507152532.09b385eb@jic23-huawei>
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Thursday, May 7, 2026 5:26 PM
...
> > > +static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
> > > +{
> > > + struct ad4691_state *st = iio_priv(indio_dev);
> > > + unsigned int prev_i, k, i;
> > > + bool first;
> > > + int ret;
> > > +
> > > + memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
> > > + memset(st->scan_tx, 0, sizeof(st->scan_tx));
> > > +
> > > + spi_message_init(&st->scan_msg);
> > > +
> > > + first = true;
> > > + prev_i = 0;
> > > + k = 0;
> > > + iio_for_each_active_channel(indio_dev, i) {
> > > + st->scan_tx[k] = cpu_to_be16(AD4691_ADC_CHAN(i));
> > > + st->scan_xfers[k].tx_buf = &st->scan_tx[k];
> > > + /*
> > > + * The pipeline means xfer[0] receives the residual from the
> > > + * previous sequence, not a valid sample for channel i. Point
> > > + * it at vals[i] anyway; xfer[1] (or the NOOP when only one
> > > + * channel is active) will overwrite that slot with the real
> > > + * result, so no separate dummy buffer is needed.
> > > + */
> > > + if (first) {
> > > + st->scan_xfers[k].rx_buf = &st->vals[i];
> > > + first = false;
> > > + } else {
> > > + st->scan_xfers[k].rx_buf = &st->vals[prev_i];
> > > + }
> >
> >
> > "The IIO subsystem expects data pushed to the buffer to be densely packed
> > according to the active channels in the scan mask.
> > If only a subset of channels are enabled, does assigning the rx_buf pointer
> > directly to absolute array indices at &st->vals[i] leave holes in the buffer?
> > When iio_push_to_buffers_with_ts() is called, this might cause it to read
> > uninitialized memory instead of the expected samples."
> >
> > I would say there is no change needed. Writing to &st->vals[scan_index] and
> > passing the full array to iio_push_to_buffers_with_ts() is the standard IIO
> kfifo
> > pattern: the core demultiplexes by reading data[scan_index * storagebits/8]
> > for each active channel; holes at inactive indices are silently ignored.
> > The same pattern is used in ad4695, ad_sigma_delta, and others. The
> > pipeline residual in the first manual-mode transfer is overwritten by the
> > subsequent transfer before the scan is pushed, as the comment explains.
>
> This looks wrong to me.
>
> What holes? If available_scan_masks is set we will do a bunch of
> demux work - but then this code would see the mask picked from that
> list. If it's not then typically we won't (subject to multiple consumers
> forcing it - but that still won't close up holes here).
>
> If the active_scan_mask == the one requested, there is no demux at all
> and I think that's the case here - the code pushes the data passed in
> directly to the kfifo.
>
> Perhaps given an illustration of what the layout of resulting data
> is if only even numbered channels are enabled.
>
Correct. scan_bytes is a dense count (channels 0, 2, 4 active → 3 × 2 = 6
bytes). With the old sparse layout (rx_buf = &vals[scan_index]):
vals[0] = ch0 result (correct)
vals[1] = hole (incorrect) <- userspace reads as ch2
vals[2] = ch2 result (correct) <-userspace reads as ch4
Fixed by using the slot counter k as the rx_buf index rather than the channel
index i, giving a densely packed vals[]. In manual mode xfer[k] delivers the
previous channel's result (pipelined), so subsequent transfers write into
vals[k-1]; vals[0] serves as a throwaway slot for the first transfer's garbage.
^ permalink raw reply
* [PATCH v10 9/9] platform/chrome: cros_ec_chardev: Consume cros_ec_device via revocable
From: Tzung-Bi Shih @ 2026-05-08 10:54 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Bartosz Golaszewski,
Linus Walleij
Cc: Benson Leung, tzungbi, linux-kernel, chrome-platform, driver-core,
linux-doc, linux-gpio, Rafael J. Wysocki, Danilo Krummrich,
Jonathan Corbet, Shuah Khan, Laurent Pinchart, Wolfram Sang,
Jason Gunthorpe, Johan Hovold, Paul E . McKenney
In-Reply-To: <20260508105448.31799-1-tzungbi@kernel.org>
The cros_ec_chardev driver provides a character device interface to the
ChromeOS EC. A file handle to this device can remain open in userspace
even if the underlying EC device is removed.
This creates a classic use-after-free vulnerability. Any file operation
(ioctl, release, etc.) on the open handle after the EC device has gone
would access a stale pointer, leading to a system crash.
To prevent this, leverage the revocable and convert cros_ec_chardev to a
resource consumer of cros_ec_device.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v10:
- No changes.
v9: https://lore.kernel.org/all/20260427135841.96266-10-tzungbi@kernel.org
- New to the series.
- Change revocable API usages accordingly.
v4 - v8:
- Doesn't exist.
v3: https://lore.kernel.org/all/20250912081718.3827390-6-tzungbi@kernel.org
- Use specific labels for different cleanup in cros_ec_chardev_open().
v2: https://lore.kernel.org/all/20250820081645.847919-6-tzungbi@kernel.org
- Rename "ref_proxy" -> "revocable".
- Fix a sparse warning by removing the redundant __rcu annotation.
v1: https://lore.kernel.org/all/20250814091020.1302888-4-tzungbi@kernel.org
---
drivers/platform/chrome/cros_ec_chardev.c | 80 +++++++++++++++++------
1 file changed, 61 insertions(+), 19 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 002be3352100..c597dc92d519 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -22,6 +22,7 @@
#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/poll.h>
+#include <linux/revocable.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/uaccess.h>
@@ -32,7 +33,7 @@
#define CROS_MAX_EVENT_LEN PAGE_SIZE
struct chardev_priv {
- struct cros_ec_device *ec_dev;
+ struct revocable *rev;
struct notifier_block notifier;
wait_queue_head_t wait_event;
unsigned long event_mask;
@@ -55,6 +56,7 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
};
struct ec_response_get_version *resp;
struct cros_ec_command *msg;
+ struct cros_ec_device *ec_dev;
int ret;
msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
@@ -64,12 +66,19 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
msg->command = EC_CMD_GET_VERSION + priv->cmd_offset;
msg->insize = sizeof(*resp);
- ret = cros_ec_cmd_xfer_status(priv->ec_dev, msg);
- if (ret < 0) {
- snprintf(str, maxlen,
- "Unknown EC version, returned error: %d\n",
- msg->result);
- goto exit;
+ revocable_try_access_with_scoped(priv->rev, ec_dev) {
+ if (!ec_dev) {
+ ret = -ENODEV;
+ goto exit;
+ }
+
+ ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+ if (ret < 0) {
+ snprintf(str, maxlen,
+ "Unknown EC version, returned error: %d\n",
+ msg->result);
+ goto exit;
+ }
}
resp = (struct ec_response_get_version *)msg->data;
@@ -92,10 +101,15 @@ static int cros_ec_chardev_mkbp_event(struct notifier_block *nb,
{
struct chardev_priv *priv = container_of(nb, struct chardev_priv,
notifier);
- struct cros_ec_device *ec_dev = priv->ec_dev;
+ struct cros_ec_device *ec_dev;
struct ec_event *event;
- unsigned long event_bit = 1 << ec_dev->event_data.event_type;
- int total_size = sizeof(*event) + ec_dev->event_size;
+ unsigned long event_bit;
+ int total_size;
+
+ revocable_try_access_or_return_err(priv->rev, ec_dev, NOTIFY_DONE);
+
+ event_bit = 1 << ec_dev->event_data.event_type;
+ total_size = sizeof(*event) + ec_dev->event_size;
if (!(event_bit & priv->event_mask) ||
(priv->event_len + total_size) > CROS_MAX_EVENT_LEN)
@@ -166,7 +180,8 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
if (!priv)
return -ENOMEM;
- priv->ec_dev = ec_dev;
+ priv->rev = ec_dev->its_rev;
+ revocable_get(priv->rev);
priv->cmd_offset = ec->cmd_offset;
filp->private_data = priv;
INIT_LIST_HEAD(&priv->events);
@@ -178,6 +193,7 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
&priv->notifier);
if (ret) {
dev_err(ec_dev->dev, "failed to register event notifier\n");
+ revocable_put(priv->rev);
kfree(priv);
}
@@ -251,11 +267,13 @@ static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer,
static int cros_ec_chardev_release(struct inode *inode, struct file *filp)
{
struct chardev_priv *priv = filp->private_data;
- struct cros_ec_device *ec_dev = priv->ec_dev;
+ struct cros_ec_device *ec_dev;
struct ec_event *event, *e;
- blocking_notifier_chain_unregister(&ec_dev->event_notifier,
- &priv->notifier);
+ revocable_try_access_or_skip_scoped(priv->rev, ec_dev)
+ blocking_notifier_chain_unregister(&ec_dev->event_notifier,
+ &priv->notifier);
+ revocable_put(priv->rev);
list_for_each_entry_safe(event, e, &priv->events, node) {
list_del(&event->node);
@@ -273,6 +291,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
{
struct cros_ec_command *s_cmd;
struct cros_ec_command u_cmd;
+ struct cros_ec_device *ec_dev;
long ret;
if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
@@ -299,10 +318,17 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
}
s_cmd->command += priv->cmd_offset;
- ret = cros_ec_cmd_xfer(priv->ec_dev, s_cmd);
- /* Only copy data to userland if data was received. */
- if (ret < 0)
- goto exit;
+ revocable_try_access_with_scoped(priv->rev, ec_dev) {
+ if (!ec_dev) {
+ ret = -ENODEV;
+ goto exit;
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, s_cmd);
+ /* Only copy data to userland if data was received. */
+ if (ret < 0)
+ goto exit;
+ }
if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + s_cmd->insize))
ret = -EFAULT;
@@ -313,10 +339,12 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
static long cros_ec_chardev_ioctl_readmem(struct chardev_priv *priv, void __user *arg)
{
- struct cros_ec_device *ec_dev = priv->ec_dev;
+ struct cros_ec_device *ec_dev;
struct cros_ec_readmem s_mem = { };
long num;
+ revocable_try_access_or_return(priv->rev, ec_dev);
+
/* Not every platform supports direct reads */
if (!ec_dev->cmd_readmem)
return -ENOTTY;
@@ -370,11 +398,25 @@ static const struct file_operations chardev_fops = {
#endif
};
+static void cros_ec_chardev_free(void *data)
+{
+ struct revocable *rev = data;
+
+ revocable_put(rev);
+}
+
static int cros_ec_chardev_probe(struct platform_device *pdev)
{
struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
+ struct revocable *rev = ec->ec_dev->its_rev;
struct miscdevice *misc;
+ int ret;
+
+ revocable_get(rev);
+ ret = devm_add_action_or_reset(&pdev->dev, cros_ec_chardev_free, rev);
+ if (ret)
+ return ret;
/* Create a char device: we want to create it anew */
misc = devm_kzalloc(&pdev->dev, sizeof(*misc), GFP_KERNEL);
--
2.51.0
^ permalink raw reply related
* [PATCH v10 8/9] platform/chrome: Protect cros_ec_device lifecycle with revocable
From: Tzung-Bi Shih @ 2026-05-08 10:54 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Bartosz Golaszewski,
Linus Walleij
Cc: Benson Leung, tzungbi, linux-kernel, chrome-platform, driver-core,
linux-doc, linux-gpio, Rafael J. Wysocki, Danilo Krummrich,
Jonathan Corbet, Shuah Khan, Laurent Pinchart, Wolfram Sang,
Jason Gunthorpe, Johan Hovold, Paul E . McKenney
In-Reply-To: <20260508105448.31799-1-tzungbi@kernel.org>
The cros_ec_device can be unregistered when the underlying device is
removed. Other kernel drivers that interact with the EC may hold a
pointer to the cros_ec_device, creating a risk of a use-after-free
error if the EC device is removed while still being referenced.
To prevent this, leverage the revocable and convert the underlying
device drivers to resource providers of cros_ec_device.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v10:
- No changes.
v9: https://lore.kernel.org/all/20260427135841.96266-9-tzungbi@kernel.org
- New to the series.
- Change revocable API usages accordingly.
- Rename "revocable_provider" -> "its_rev".
v5 - v8:
- Doesn't exist.
v4: https://lore.kernel.org/all/20250923075302.591026-5-tzungbi@kernel.org
- No changes.
v3: https://lore.kernel.org/all/20250912081718.3827390-5-tzungbi@kernel.org
- Initialize the revocable provider in cros_ec_device_alloc() instead of
spreading in protocol device drivers.
v2: https://lore.kernel.org/all/20250820081645.847919-5-tzungbi@kernel.org
- Rename "ref_proxy" -> "revocable".
v1: https://lore.kernel.org/all/20250814091020.1302888-3-tzungbi@kernel.org
---
drivers/platform/chrome/cros_ec.c | 11 +++++++++++
include/linux/platform_data/cros_ec_proto.h | 3 +++
2 files changed, 14 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 1da79e3d215b..2702a1bbfeb5 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -16,6 +16,7 @@
#include <linux/platform_device.h>
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/revocable.h>
#include <linux/slab.h>
#include <linux/suspend.h>
@@ -37,6 +38,7 @@ static void cros_ec_device_free(void *data)
mutex_destroy(&ec_dev->lock);
lockdep_unregister_key(&ec_dev->lockdep_key);
+ revocable_revoke(ec_dev->its_rev);
}
struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
@@ -47,6 +49,15 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
if (!ec_dev)
return NULL;
+ ec_dev->its_rev = revocable_alloc(ec_dev);
+ if (!ec_dev->its_rev)
+ return NULL;
+ /*
+ * Drop the extra reference for the caller as the caller is the
+ * resource provider.
+ */
+ revocable_put(ec_dev->its_rev);
+
ec_dev->din_size = sizeof(struct ec_host_response) +
sizeof(struct ec_response_get_protocol_info) +
EC_MAX_RESPONSE_OVERHEAD;
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index de14923720a5..e8c3bd03403c 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -12,6 +12,7 @@
#include <linux/lockdep_types.h>
#include <linux/mutex.h>
#include <linux/notifier.h>
+#include <linux/revocable.h>
#include <linux/platform_data/cros_ec_commands.h>
@@ -165,6 +166,7 @@ struct cros_ec_command {
* @pd: The platform_device used by the mfd driver to interface with the
* PD behind an EC.
* @panic_notifier: EC panic notifier.
+ * @its_rev: The revocable_provider to this device.
*/
struct cros_ec_device {
/* These are used by other drivers that want to talk to the EC */
@@ -211,6 +213,7 @@ struct cros_ec_device {
struct platform_device *pd;
struct blocking_notifier_head panic_notifier;
+ struct revocable *its_rev;
};
/**
--
2.51.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