\documentclass[aspectratio=169,obeyspaces,spaces,hyphens,dvipsnames]{beamer}
\usepackage[utf8]{inputenc}
\usepackage{lmodern}% http://ctan.org/pkg/lm
\usepackage{minted}
\usepackage{hyperref}
\usepackage{xcolor}
\usepackage{pgfplots}
\usepackage{tikz}
\usepackage[normalem]{ulem}

\mode<presentation>
\usetheme{Bootlin}

\def\signed #1{{\leavevmode\unskip\nobreak\hfil\penalty50\hskip2em
  \hbox{}\nobreak\hfil(#1)
  \parfillskip=0pt \finalhyphendemerits=0 \endgraf}}

\newsavebox\mybox
\newenvironment{aquote}[1]
  {\savebox\mybox{#1}\begin{quotation}}
  {\signed{\usebox\mybox}\end{quotation}}

\title{Runtime hotplug on non-discoverable busses with device tree overlays}
\authors{Luca Ceresoli, Hervé Codina}
\email{luca.ceresoli@bootlin.com\\
herve.codina@bootlin.com}
\slidesurl{https://bootlin.com/pub/conferences/}
\institute{Bootlin}
\conference{Linux Plumbers Conference 2024}

\begin{document}

\addtocontents{toc}{\protect\setcounter{tocdepth}{-1}}
\section{Runtime hotplug on non-discoverable busses with device tree overlays}
\addtocontents{toc}{\protect\setcounter{tocdepth}{2}}

\begin{frame}{Luca Ceresoli}
  \begin{itemize}
  \item Embedded Linux engineer at Bootlin
    \begin{itemize}
    \item Embedded Linux {\bf expertise}
    \item {\bf Development}, consulting and training
    \item Strong open-source focus
    \end{itemize}
  \item Linux kernel device driver developer
  \item Bootloaders, Buildroot and Yocto integration
  \item Open-source contributor
  \item Living in {\bf Bergamo}, Italy
  \end{itemize}
  {\small \url{https://bootlin.com/company/staff/luca-ceresoli/}}
\end{frame}

\begin{frame}{Hervé Codina}
  \begin{itemize}
  \item Embedded Linux engineer at Bootlin
    \begin{itemize}
    \item Embedded Linux {\bf expertise}
    \item {\bf Development}, consulting and training
    \item Developer of the {\bf Microchip LAN966x PCI device} support based
      on device tree overlays over PCI
    \item Strong open-source focus
    \end{itemize}
  \item Open-source contributor
  \item Living in {\bf Toulouse}, France
  \end{itemize}
  {\small \url{https://bootlin.com/company/staff/herve-codina/}}
\end{frame}

\subsection{Context}

\begin{frame}{Goal}
  \begin{itemize}
  \item A healthcare product, advanced hardware prototype, to be launched in 2025
  \item Classic ARM64 embedded system, using device tree
    \begin{itemize}
    \item Works standalone with basic features
    \end{itemize}
  \item Has a connector for an add-on to extend features
    \begin{itemize}
    \item Proprietary connector
    \item Hot-pluggable by user at any moment
    \item Connector uses non-discoverable busses (I2C, MIPI DSI,
      interrupts)
    \item Multiple add-on models supported
      \begin{itemize}
      \item Add-ons have an EEPROM with model ID
      \end{itemize}
    \end{itemize}
  \end{itemize}
  \vspace{0.3cm}
  \centering\includegraphics[height=0.38\textheight]{hardware-diagram.pdf}
\end{frame}

\begin{frame}{Overall solution}
  \begin{itemize}
  \item Device tree + Adding hw = calls for {\bf device tree overlays}
  \item Wrote a connector driver
  \item Loads 2 overlays
    \begin{itemize}
    \item 1st: minimum to describe EEPROM with addon model ID (common to
      all add-on models)
    \item 2nd: describes everything else (model-specific)
    \end{itemize}
  \item Works but with several issues still open
  \item v4: \url{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com/}
  \end{itemize}
\end{frame}

\begin{frame}{Somewhat related work}

  {\bf Support device tree overlays for PCI device board}
  \begin{itemize}
  \item by Hervé Codina and Clément Léger (Bootlin)
  \item
    \href{https://lore.kernel.org/all/20240808154658.247873-1-herve.codina@bootlin.com/}
         {\texttt{[PATCH v5 0/8] Add support for the LAN966x PCI device using a DT overlay}}
  \item
    \href{https://lpc.events/event/17/contributions/1421/}
         {Discussed at Linux Plumbers Conference 2023 (Rob Herring)}
  \end{itemize}

  {\bf BeaglePlay MikroBus support}
  \begin{itemize}
  \item by Ayush Singh (beagleboard.org)
  \item
    \href{https://lore.kernel.org/all/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/}
         {\texttt{[PATCH v5 0/7] misc: Add mikroBUS driver}}
  \end{itemize}

  {\bf Generic ``adapter'' overlays for BBB capes, RPi hats, MikroBUS, Grove\dots}
  \begin{itemize}
  \item by Andrew Davis (TI)
  \item
    \href{https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/}
         {\texttt{[PATCH RFC 0/3] Add generic Overlay for Grove Sunlight Sensor}}
  \end{itemize}

  {\bf Prober for non-discoverable 2nd source hardware}
  \begin{itemize}
  \item by Chen-Yu Tsai (chromium.org)
  \item
    \href{https://lore.kernel.org/lkml/20240911072751.365361-1-wenst@chromium.org/}
         {\texttt{[PATCH v7 00/10] platform/chrome: Introduce DT hardware prober}}
  \end{itemize}

\end{frame}

\subsection{Device tree bindings}

\begin{frame}{Device tree bindings}
  \begin{itemize}
  \item The connector is a device on its own
  \item The overlay should add nodes, no properties (deadprops, leaks)
  \item Need to decouple base and addon boards
    \begin{itemize}
    \item Phandles in overlay pointing to base tree are not allowed
    \end{itemize}
  \item v4 approach agreed with Rob (except NVMEM cells, still pending)
  \item See \href{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-1-bc4dfee61be6@bootlin.com/}{patch 1}
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{Device tree bindings --- Main board DTS}
  \begin{block}{}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    / {
        addon-connector {
            compatible = "ge,sunh-addon-connector";
            reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
            plugged-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;

            i2c-dbat {
                i2c-parent = <&i2c2_ch1>;   // <-- Decouple adapter on base from devices on addon
                #address-cells = <1>;
                #size-cells = <0>;
            };
            // more i2c busses...

            dsi {
                ports {
                    #address-cells = <1>;
                    #size-cells = <0>;

                    port@0 {        // <-- the base board side; port@1 is missing for now
                        reg = <0>;

                        hotplug_bridge_sink: endpoint {
                            remote-endpoint = <&dsi_to_hotplug_bridge>;
                        };
                    };
                };
            };
        };
    };
    \end{minted}
  \end{block}
\end{frame}

\begin{frame}[fragile]{Device tree bindings --- 1st overlay DTSO}
  \begin{block}{}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    / {
        fragment@0 {
            target-path = ""; // <-- Driver will map this to the connector node
                              //     ("/addon-connector") at runtime (allows multiple connectors)
            __overlay__ {
                nvmem-cells = <&addon_id>;    // <-- FIXME adding properties to base node
                nvmem-cell-names = "id";

                i2c-dbat {
                    eeprom@51 {
                        compatible = "atmel,24c64";
                        reg = <0x51>;
                        pagesize = <32>;

                        nvmem-layout {
                            compatible = "fixed-layout";
                            #address-cells = <1>;
                            #size-cells = <1>;

                            /* Data cells */
                            addon_id: addon-id@400 { // <-- phandles can reference labels *within the overlay*
                                reg = <0x400 0x1>;
                            };
                        };
                    };
                };
            };
        };
    };
    \end{minted}
  \end{block}
\end{frame}

\begin{frame}[fragile]{Device tree bindings --- 2nd overlay DTSO}
  \begin{block}{}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    / {
        fragment@0 {
            target-path = "";

            __overlay__ {
                dsi {
                    ports {
                        port@1 {
                            reg = <1>;
                            hotplug_bridge_source: endpoint {
                                remote-endpoint = <&sn65dsi84_from_bridge>;
                            };
                        };
                    };
                };

                devices { // <-- ``no bus'' platform devices (normally in the root node)
                    reg_addon_3v3_lcd: regulator-addon-3v3-lcd {
                        compatible = "regulator-fixed";
                        regulator-name = "3V3_LCD_ADDON";
                    };

                    backlight_addon: backlight-addon {
                        compatible = "pwm-backlight";
                        power-supply = <&reg_addon_3v3_lcd>;
                    };
                };
            };
        };
    };
    \end{minted}
  \end{block}
\end{frame}

\begin{frame}[fragile]{NVMEM description}
  \begin{itemize}
  \item In v4, the only remaining addition of properties to nodes in the
    base tree
  \item Different description needed
    \item Idea \#1: move NVMEM cell properties to a subnode (\code{/connector/addon-id})
    \begin{block}{}
      \fontsize{7}{7}\selectfont
      \begin{minted}{dts}
      / {
          fragment@0 {
              target-path = "";

              addon-id {
                  nvmem-cells = <&addon_id>;
                  nvmem-cell-names = "id";
                  // nothing else here
              };
              // *everything* on the add-on stays here
          };
      };
      \end{minted}
    \end{block}
    \begin{itemize}
    \item Implementation: use \kfunc{of_nvmem_device_get} to get it from
      the subnode instead of the simpler \kfunc{nvmem_cell_read_u8}
    \end{itemize}
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{NVMEM description --- other ideas}
  \begin{itemize}
  \item Idea \#2: don't use NVMEM cells, open code in connector driver the
    logic to read from EEPROM
  \item Idea \#3 (under investigation): overlay only adds one node with
    everything inside
    \begin{block}{}
      \fontsize{7}{7}\selectfont
      \begin{minted}{dts}
      / {
          fragment@0 {
              target-path = "";

              addon { // <-- overlay only adds _one_ node (/addon-connector/addon)
                  nvmem-cells = <&addon_id>;    // <-- FIXME adding properties to base node
                  nvmem-cell-names = "id";

                  // *everything* on the add-on is inside this node
              };
          };
      };
      \end{minted}
    \end{block}
    \begin{itemize}
    \item But this would break the current I²C and DSI bindings
    \end{itemize}
  \end{itemize}
\end{frame}

\subsection{Implementation}

\begin{frame}{Topics}
  \begin{itemize}
  \item Connector driver overview
  \item GPIOs and interrupts
  \item Platform devices
  \item I²C devices
  \item DRM devices
  \item Devlink issues
  \end{itemize}
\end{frame}

\begin{frame}{Connector driver}
  \begin{itemize}
  \item Specific to this proprietary connector
    \begin{itemize}
    \item \code{compatible = "ge,sunh-addon-connector"}
    \item Can be generalized to similar use cases
    \end{itemize}
  \item Basic workflow
    \begin{enumerate}
    \item Detect connection (GPIO)
    \item Insert base overlay
      \begin{itemize}
      \item Devices are populated
      \end{itemize}
    \item NVMEM notifier: ID cell is available, add-on model is read
    \item Insert 2nd overlay
      \begin{itemize}
      \item Devices are populated
      \end{itemize}
    \item Profit!
    \item Detect removal (GPIO)
    \item Removes overlays in reverse order
      \begin{itemize}
      \item Devices are depopulated
      \end{itemize}
    \item Goto 1
    \end{enumerate}
  \item See \href{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-8-bc4dfee61be6@bootlin.com/}{patch 8}
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{GPIOs and interrupts: tentative idea, not working}
  \begin{itemize}
  \begin{block}{Base device tree}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    addon-connector {
        devices {
            addon-gpios {
                compatible = "hotplug-connector-gpios"; // <-- gpio-aggregator
                gpios = <&gpio4 1 IRQ_TYPE_EDGE_FALLING>, <&tca6424_1 9 GPIO_ACTIVE_HIGH>;
                interrupt-controller;  #interrupt-cells = <1>;
                gpio-controller;       #gpio-cells = <2>;

    };  };  };
    \end{minted}
  \end{block}
  \begin{block}{Overlay}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    __overlay__ {
        devices {
            addon_gpios: addon-gpios {}; // <-- add a label
        };
        some-chip@123 {
            interrupt-parent = <&addon_gpios>; // <-- phandle to in-overlay label
            interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
    };  };
    \end{minted}
  \end{block}
  \item \kfunc{add_changeset_property} discards \code{phandle} properties
    (would leak anyway)
  \item Removing the check and letting \code{phandle} be applied still does not work
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{GPIOs and interrupts: tentative idea, not tested}
  \begin{itemize}
  \begin{block}{Base device tree}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    addon-connector {
        devices {
            addon-gpios {
                compatible = "hotplug-connector-gpios"; // <-- new ad-hoc driver
                // Define the GPIOs to use from the main board controllers
                gpios = <&gpio4 1 IRQ_TYPE_EDGE_FALLING>, <&tca6424_1 9 GPIO_ACTIVE_HIGH>;
                // This is not the GPIO controller
    };  };  };
    \end{minted}
  \end{block}
  \begin{block}{Overlay}
    \fontsize{7}{7}\selectfont
    \begin{minted}{dts}
    __overlay__ {
        devices {
            addon-gpios {
                addon_gpios: controller { // <-- the node to expose as a controller
                    interrupt-controller;  #interrupt-cells = <1>;
                    gpio-controller;       #gpio-cells = <2>;
        };  };  };
        some-chip@123 {
            interrupt-parent = <&addon_gpios>; // <-- phandle to in-overlay label
            interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
    };  };
    \end{minted}
  \end{block}
  \item Keep the main-board-facing info in the \code{addon-gpios} node
  \item Move the addon-facing info to the \code{controller} subnode
  \end{itemize}
\end{frame}

\begin{frame}{Platform devices}
  \begin{itemize}
  \item Two kinds:
    \begin{itemize}
    \item Devices on a parallel I/O bus (not likely, not in this hardware)
    \item Devices without any bus: GPIO/fixed regulators, backlights,
      panels\dots
    \end{itemize}
  \item In DT, clients are in the \code{devices} node \textrightarrow{} not
    probed automatically
  \item Connector driver calls \kfunc{of_platform_default_populate} after
    overlay is loaded to populate clients
    \begin{itemize}
      \item Passing pointer to \code{<connector>/devices} node
    \end{itemize}
  \item Depopulated automatically by existing code
  \item See \href{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-8-bc4dfee61be6@bootlin.com/}{patch 8}
  \end{itemize}
\end{frame}

\begin{frame}{I²C devices}
  \begin{itemize}
  \item In DT, clients are in the \code{i2c-xyz} node, not in the adapter
    node
  \item Added code to \kfunc{of_i2c_notify} to populate clients upon DT
    node insertion
  \item Depopulated automatically by existing code
  \item Issues with this approach
    \begin{itemize}
    \item Cannot have clients in base \code{addon-connector/i2c-xyz} node (no big deal)
    \item Based on DT notifier: won't populate clients if adapter appears
      after overlay is loaded (unbind/bind, module reload...)
    \item Can be solved by adding info in DT
    \end{itemize}
  \item See \href{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-5-bc4dfee61be6@bootlin.com/}{patch 5}
  \end{itemize}
\end{frame}

\begin{frame}{DRM}
  \begin{itemize}
  \item Discussed on Wednesday
  \item ``Hotplug DRM pipeline components on non-discoverable video busses''
  \item \url{https://lpc.events/event/18/contributions/1750/}
  \end{itemize}
\end{frame}

\begin{frame}{Devlink issues 1/2}
  \begin{itemize}
  \item Firmware-inferred links are relaxed and dropped after boot
  \item[{\bf\textrightarrow{}}] for components in overlays, this can happen before probing
  \item[{\bf\textrightarrow{}}] devlink not available on removal
  \item[{\bf\textrightarrow{}}] some suppliers are removed before consumers
    \begin{itemize}
      \item Details and solution (workaround?) in
        \href{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-7-bc4dfee61be6@bootlin.com/}{patch 7}
      \item Bug exposed by FW device removal
    \end{itemize}
  \item Some devlinks not created
  \item[{\bf\textrightarrow{}}] some suppliers are removed before consumers
    \begin{itemize}
      \item This is a bug even without FW device removal
      \item LED using GPIO:
        \href{https://lore.kernel.org/all/20240220133950.138452-1-herve.codina@bootlin.com/}
             {fix proposed in a separate patch by Hervé}, rejected
      \item Backlight using LED:
        \href{https://lore.kernel.org/all/20240917-hotplug-drm-bridge-v4-6-bc4dfee61be6@bootlin.com/}
             {patch 6}, similar to Hervé's
    \end{itemize}
  \end{itemize}
\end{frame}

\begin{frame}{Devlink issues 2/2}
  \begin{itemize}
  \item DT nodes from overlays default to \ksym{FWNODE_FLAG_NOT_DEVICE}
  \item[{\bf\textrightarrow{}}] some devlinks won't be created for overlays
    \begin{itemize}
      \item Overlay nodes are 2nd class citizens
      \item Bug specific to runtime DT overlays
      \item
        \href{https://lore.kernel.org/linux-devicetree/20240220111044.133776-1-herve.codina@bootlin.com/}
             {\texttt{[PATCH 0/2] devlink: Take care of FWNODE\_FLAG\_NOT\_DEVICE in case of DT overlays}}, rejected
    \end{itemize}
  \item Devlink is known to be broken with overlays
    \begin{itemize}
    \item Some attempts have been made to fix it
      \begin{itemize}
      \item \href{https://lore.kernel.org/lkml/20240411235623.1260061-1-saravanak@google.com/}
                 {\texttt{[PATCH v3 0/2] fw\_devlink overlay fix}} by Saravana
      \end{itemize}
    \end{itemize}
  \end{itemize}
\end{frame}

\questionslide

\end{document}
