\documentclass[obeyspaces,spaces,hyphens]{beamer}
\usepackage[utf8]{inputenc}
\usepackage{minted}

\mode<presentation>
\usetheme{Bootlin}

\newcommand{\codehack}[1]{{\usebeamercolor[fg]{code} {\tt #1}}}

\title{Two years of ARM SoC support mainlining: lessons learned}
\authors{Thomas Petazzoni}
\institute{Bootlin}

\usepackage{hyperref}

\newcounter{lessonno}

\newcommand \lesson [1] {
\begin{frame}
  \huge
\begin{center}
  Lesson \#\arabic{lessonno}\\
  \stepcounter{lessonno}
  {\bf #1}
\end{center}
\end{frame}
}

\begin{document}

\begin{frame}{Embedded Linux Conference 2014}
  \begin{columns}[c]
   \column{0.6\textwidth}
     \huge
     Two years of ARM SoC support mainlining: lessons learned\\
     \vspace{2cm}
     \normalsize
     Thomas Petazzoni\\
     {\bf Bootlin}\\
     {\em thomas.petazzoni@bootlin.com}
     \column{0.4\textwidth}
     \includegraphics[width=\textwidth]{armada.jpg}
   \end{columns}
\end{frame}

\begin{frame}{Thomas Petazzoni}
  \begin{itemize}
  \item CTO and Embedded Linux engineer at Bootlin
    \begin{itemize}
    \item Embedded Linux {\bf development}: kernel and driver
      development, system integration, boot time and power
      consumption optimization, consulting, etc.
    \item Embedded Linux {\bf training}, Linux driver development training
      and Android system development training, with materials freely
      available under a Creative Commons license.
    \item \url{http://bootlin.com}
    \end{itemize}
  \item Contributions
    \begin{itemize}
    \item {\bf Kernel support for the Marvell Armada} ARM SoCs from
      Marvell
    \item Major contributor to {\bf Buildroot}, an open-source, simple and
      fast embedded Linux build system
    \end{itemize}
  \item Living in {\bf Toulouse}, south west of France
  \end{itemize}
\end{frame}

\begin{frame}{Context}
  \begin{center}
    \includegraphics[width=\textwidth]{context.pdf}
  \end{center}
\end{frame}

\begin{frame}{Process}
  \begin{center}
    \includegraphics[width=\textwidth]{process.pdf}
  \end{center}
\end{frame}

\begin{frame}{Timeline (1)}
  \begin{center}
    \includegraphics[width=\textwidth]{timeline1.pdf}
  \end{center}
\end{frame}

\begin{frame}{Timeline (2)}
  \begin{center}
    \includegraphics[width=\textwidth]{timeline2.pdf}
  \end{center}
\end{frame}

\lesson{Submit early}

\begin{frame}{Submit early}
  \begin{itemize}
  \item {\em Release early, release often}
  \item Translated into the kernel contributor position: {\bf submit
      early}
  \item You {\bf will} make incorrect choices in your patches
  \item The only solution to know it is to post them and get reviews
    and comments
  \item On several occasions, we had too much internal discussions and
    reviews before posting, and we wasted too much time.
  \end{itemize}
\end{frame}

\lesson{Engage with the community}

\begin{frame}{Community}
  \begin{itemize}
  \item Really {\bf embrace} the power of the community
  \item Test \code{linux-next} and \code{-rc}
    \begin{itemize}
    \item Allows to find regressions early, and make sure your
      platform support is in a minimally working state at all times.
    \item Much better than big jumps every 2/3 years!
    \end{itemize}
  \item Provide boards for the {\bf ARM board farm}
    \begin{itemize}
    \item Talk with Kevin Hilman and Olof Johansson
    \item Gets the latest mainline and linux-next built and booted on
      your platform every day!
    \end{itemize}
  \item Create a {\bf good relationship} with your sub-architecture
    maintainer, and possibly other maintainers.
    \begin{itemize}
    \item They are the key to getting your patches merged.
    \item Attend conferences, and plan a beer/drink budget.
    \end{itemize}
  \end{itemize}
\end{frame}

\lesson{Encourage community contributions}

\begin{frame}{Community contributions}
  \begin{itemize}
  \item While the community may not necessarily work on ``big''
    features, people can {\bf help dramatically} in areas like:
    debugging, bug fixing, performance optimizations, etc.
  \item Provide {\bf boards and datasheets} to a good selection of
    people, and they will solve problems
    \begin{itemize}
    \item Unfortunately Marvell still doesn't provide public
      datasheets for Armada 370/XP
    \end{itemize}
  \item Provide those contributors support/assistance.
  \item {\bf Examples}:
    \begin{itemize}
    \item Willy Tarreau debugged and fixed a major performance issue
      in the Armada 370/XP network driver.
    \item Neil Greatorex, Jason Gunthorpe and Willy Tarreau
      investigated and fixed a number of PCIe related issues.
    \item Help solving communication issues: special PCI terminology
    \item Quick build fixes
    \end{itemize}
  \end{itemize}
\end{frame}

\lesson{Review patches from others}

\begin{frame}{Review patches from others}
  \begin{itemize}
  \item Review patch from other developers. Not the ones in your
    company, people from other companies.
  \item Doing this helps the maintainers
  \item Shows that you care about the community and understand how it
    works
  \item Of course, do it wisely, and don't do stupid reviews!
  \item Ezequiel's evil plan
    \begin{itemize}
    \item {\em Statistically speaking, you can speed-up your own
        reviewing process by reviewing other patches on the same
        queue}
    \end{itemize}
  \end{itemize}
\end{frame}

\lesson{Assign dedicated engineering resources}

\begin{frame}{Engineering resources}
  \begin{itemize}
  \item Mainlining is a {\bf time consuming process}
  \item Have a small team of engineers fully dedicated to
    mainlining.
    \begin{itemize}
    \item Keep the {\bf team small and efficient}, throwing more
      people will not necessarily make things go faster.
    \item Half of the work is technical, half of the work is social.
    \end{itemize}
  \item Make sure this small team has easy access to other engineers
    with deep knowledge of the hardware.
    \begin{itemize}
    \item The datasheet often isn't enough.
    \end{itemize}
  \item The engineers need to be {\bf able to reply quickly} to
    community comments and requests.
    \begin{itemize}
    \item Otherwise the community won't trust that you will fix issue
      and maintain the code moving forward.
    \end{itemize}
  \end{itemize}
\end{frame}

% \begin{frame}{Engineering resources}
%   \begin{center}
%     550 days of work, spread over three engineers.
%     \includegraphics[width=0.8\textwidth]{consumption-graph.png}
%   \end{center}
% \end{frame}

\lesson{Take into account older SOCs}

\begin{frame}{Take into account existing code}
  \begin{itemize}
  \item Yes, you want to push the support for your new SoC / hardware
    {\em right now}
  \item But nothing {\bf upsets more the community than neglecting
      existing hardware support} in the kernel.
  \item In our case:
    \begin{itemize}
    \item We wanted to push the support for Armada 370/XP, sharing a
      lot of HW blocks with previous SOCs
    \item We had to take into account Kirkwood, Discovery, Orion5x and
      Dove when doing changes to core drivers (pinctrl, clock, mbus,
      PCIe, etc.)
    \item Help of the community is key here!
    \end{itemize}
  \item If you care today for older code, you will care tomorrow for
    code you're submitting today.
  \item Also avoids carrying legacy code in your platform.
  \end{itemize}
\end{frame}

\lesson{Code re-use actually work}

\begin{frame}{Code re-use actually work}
  \begin{center}
    \includegraphics[height=0.8\textheight]{kirkwood-armada-comparaison.pdf}
  \end{center}
\end{frame}

\begin{frame}{Code re-use actually work}
  \begin{itemize}
  \item Two successive groups of SOCs:
    \begin{enumerate}
    \item Armada 370/XP
    \item Armada 375/38x
    \end{enumerate}
    \item Many drivers pre-existed from the Kirkwood/Orion era, and we
      could re-use them with just a DT binding addition.
    \item Lot of work done Armada 370/XP: writing (hopefully) good
      Device Tree bindings, proper pinctrl, clock, irqchip drivers,
      etc.
    \item Paid off when Armada 375/38x were introduced: all the
      identical HW blocks were enabled really quickly.
    \item Mainlining is a long term investment, but it pays off,
      especially if you have related SoCs.
  \end{itemize}
\end{frame}

\lesson{Adopt the new code}

\begin{frame}{Adopt the new code}
  \begin{itemize}
  \item The SoC vendor should really adopt the code that has been
    mainlined.
  \item There is a big risk of a split between:
    \begin{itemize}
    \item the SoC vendor custom BSP: ugly code, but lots of QA
    \item mainline: beautiful code, but poor QA
    \end{itemize}
  \item In our case, mainlining effort from 3.6 to 3.12, Marvell
    adopted 3.10 + several backports early January 2014
  \item It was already too late: when they started testing, we
    discovered several issues thanks to their stronger QA effort.
  \item Also, a split means that there is a duplicated debugging
    effort.
  \end{itemize}
\end{frame}

\lesson{Realize there is a culture difference}

\begin{frame}{Culture difference}
  \begin{itemize}
  \item Initially, surprised by the need of a third-party to mainline
    Marvell SOCs: Marvell has highly skilled engineers
  \item However, the {\bf mainlining process is not (only?) about
      making code work}
  \item It's about
    \begin{itemize}
    \item {\bf Making code pretty}: use the appropriate subsystems, don't
      hack core code in a non-generic way
    \item Caring about {\bf other platforms}
    \item Understanding how the {\bf social interactions} with
      maintainers and the community work
    \end{itemize}
  \item As said earlier: {\bf half technical, half social}
  \item Need people having both an understanding of the community, and
    technical expertise. They may not be the deepest technical
    experts, but they know how to create the interface with the
    community.
  \end{itemize}
\end{frame}

\lesson{Know how to schedule patch submission}

\begin{frame}{Patch submission timeline}
  \begin{center}
    \includegraphics[width=\textwidth]{patch-timeline.pdf}
  \end{center}
\end{frame}

\begin{frame}{Patch submission scheduling}
  \begin{itemize}
  \item The maintainers/community has a bounded {\bf absorption rate}
  \item {\bf Don't send too many patches/features} during the same
    cycle
  \item {\bf Complex things posted way before the cycle starts}, and
    be almost in the final stages when the \code{-rc1} of the previous
    cycle is released.
  \item You're {\bf always two releases ahead}: version \code{X} has
    just been released, your stuff for release \code{X+1} should be
    ready since some time, and your active development is for
    \code{X+2}
  \item During a cycle, you typically
    \begin{itemize}
    \item Post the final versions of your patches for \code{X+1}, do
      the remaining polishing and quick iterations to review/comments.
    \item Develop the features for \code{X+2}, post \code{RFC}-level
      patch series.
    \end{itemize}
  \item {\bf Accelerate} patch submission as you approach merging.
  \end{itemize}
\end{frame}

\lesson{Merging special DT bindings is hard}

\begin{frame}{Special DT bindings}
  \begin{itemize}
  \item Bindings for simple, normal devices, are usually relatively
    easy to get merged.
  \item Binding for more complex devices, or buses, require {\bf much
      more} discussion
  \item Armada 370/XP was the first ARM platform to merge a PCI host
    controller driver with a DT binding.
    \begin{itemize}
    \item Initial patch proposed on December 7th, 2012.
    \item 10 iterations until May 16th, 2013.
    \item Finally released as part of 3.11, September 2013.
    \end{itemize}
  \item Similar story for \code{mvebu-mbus}, the driver for the
    flexible MBus Marvell bus.
  \item Be patient, take comments into consideration, explain how your
    hardware works.
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{Special DT bindings}
\fontsize{0.1cm}{0.1em}
\begin{verbatim}
pcie-controller {
        compatible = "marvell,armada-xp-pcie";
        #address-cells = <3>;
        #size-cells = <2>;
        msi-parent = <&mpic>;
        bus-range = <0x00 0xff>;

        ranges =
               <0x82000000 0 0x40000 MBUS_ID(0xf0, 0x01) 0x40000 0 0x00002000   /* Port 0.0 registers */
                0x82000000 0 0x42000 MBUS_ID(0xf0, 0x01) 0x42000 0 0x00002000   /* Port 2.0 registers */
                0x82000000 0 0x44000 MBUS_ID(0xf0, 0x01) 0x44000 0 0x00002000   /* Port 0.1 registers */
                0x82000000 0 0x48000 MBUS_ID(0xf0, 0x01) 0x48000 0 0x00002000   /* Port 0.2 registers */
                0x82000000 0 0x4c000 MBUS_ID(0xf0, 0x01) 0x4c000 0 0x00002000   /* Port 0.3 registers */
                0x82000000 0 0x80000 MBUS_ID(0xf0, 0x01) 0x80000 0 0x00002000   /* Port 1.0 registers */
                0x82000000 0 0x82000 MBUS_ID(0xf0, 0x01) 0x82000 0 0x00002000   /* Port 3.0 registers */
                [...]
                0x82000000 0x1 0     MBUS_ID(0x04, 0xe8) 0 1 0 /* Port 0.0 MEM */
                0x81000000 0x1 0     MBUS_ID(0x04, 0xe0) 0 1 0 /* Port 0.0 IO  */
                0x82000000 0x2 0     MBUS_ID(0x04, 0xd8) 0 1 0 /* Port 0.1 MEM */
                0x81000000 0x2 0     MBUS_ID(0x04, 0xd0) 0 1 0 /* Port 0.1 IO  */
                0x82000000 0x3 0     MBUS_ID(0x04, 0xb8) 0 1 0 /* Port 0.2 MEM */
                0x81000000 0x3 0     MBUS_ID(0x04, 0xb0) 0 1 0 /* Port 0.2 IO  */
                0x82000000 0x4 0     MBUS_ID(0x04, 0x78) 0 1 0 /* Port 0.3 MEM */
                0x81000000 0x4 0     MBUS_ID(0x04, 0x70) 0 1 0 /* Port 0.3 IO  */
                [...]

        pcie@1,0 {
                device_type = "pci";
                assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
                reg = <0x0800 0 0 0 0>;
                #address-cells = <3>;
                #size-cells = <2>;
                #interrupt-cells = <1>;
                ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0
                          0x81000000 0 0 0x81000000 0x1 0 1 0>;
                interrupt-map-mask = <0 0 0 0>;
                interrupt-map = <0 0 0 0 &mpic 58>;
                marvell,pcie-port = <0>;
                marvell,pcie-lane = <0>;
                clocks = <&gateclk 5>;
                status = "disabled";
        };
\end{verbatim}
\end{frame}

\lesson{Keeping DT stability is hard}

\begin{frame}{Keeping DT stability is hard}
  \begin{itemize}
  \item Some hardware blocks have well-defined functions and
    boundaries, generally for devices
  \item But some hardware blocks have less well-defined boundaries,
    generally core components (clocks, power management, etc.)
  \item The need for stable DT bindings pretty much requires a
    complete understanding of how all the hardware works...
  \item ... which goes a bit against the principle of iterative
    development.
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{Keeping DT stability is hard: example (1)}
  \begin{itemize}
  \item For SMP enabling on Armada XP, we needed to fiddle with an
    unit called the PMSU (Power Management Service Unit) and some CPU
    reset registers.
  \item So, we did a DT binding like this:
\begin{verbatim}
armada-370-xp-pmsu@22000 {
        compatible = "marvell,armada-370-xp-pmsu";
        reg = <0x22100 0x400>, <0x20800 0x20>;
};
\end{verbatim}
  \item First register region: PMSU
  \item Second register region: CPU reset registers
  \end{itemize}
\end{frame}

\begin{frame}[fragile]{Keeping DT stability is hard: example (1)}
  A year later, we realized that:
  \begin{itemize}
  \item To implement {\em cpuidle} on Armada XP, we need to access
    PMSU registers between 0x22000 to 0x22100: need to change the base
    address of the first register region.
  \item Armada 375 has CPU reset registers for SMP, but no PMSU: need
    to split in two DT nodes.
  \item And continue support the old DT binding.
  \item Not able to use the new {\em reset} framework
  \item Our experience: be very careful about all these system
    registers, and think twice.
  \end{itemize}
\end{frame}

\lesson{Technical vs. marketing}

\begin{frame}{Technical vs. marketing}
  \begin{itemize}
  \item A new SoC is going to be released and announced.
  \item Technical people: we want the SoC to be supported in mainline
    as soon as it starts shipping.
    \begin{itemize}
    \item Would require to start and post patches early
    \end{itemize}
  \item Marketing people: you're not allowed to disclose any details
    about the SoC before its official release.
    \begin{itemize}
    \item Prevents from posting patches early, and goes against the
      limited {\em absorption rate} problem
    \end{itemize}
  \end{itemize}
  \begin{center}
    \includegraphics[width=\textwidth]{marketing-vs-technical.pdf}
  \end{center}
\end{frame}

\lesson{Making estimates is impossible}

\begin{frame}{Making estimates is impossible}
  \begin{itemize}
  \item Manager: {\em please tell me how much time you need to get
      this merged, and when it will be merged?}
  \item Making precise estimates for kernel mainlining work is very
    difficult, close to impossible.
  \item You can estimate how much work is needed to get to the point
    where you send the {\em first} version.
  \item But then, the review may point out issues, or require
    refactoring of some subsystem: will require time!
    \begin{itemize}
    \item For MSI support on Armada 370/XP, had to touch PowerPC, x86,
      SPARC, Tile, S390!
    \end{itemize}
  \item With the experience, you will progressively get a better
    feeling of how difficult it will be for a given change to be
    merged.
  \end{itemize}
\end{frame}

\begin{frame}
  \begin{center}
  \Huge
  Questions?\\
  \vspace{1.5cm}
  \large
  \code{thomas.petazzoni@bootlin.com}\\
  \vspace{1cm}
  \large
  Thanks to: Tawfik Bayouk, Lior Amsalem, Ezequiel Garcia, Grégory Clement and Marvell.
  \vspace{0.2cm}
  \newline Slides under CC-BY-SA 3.0\\
  \scriptsize
  \url{http://bootlin.com/pub/conferences/2014/elc/petazzoni-soc-mainlining-lessons-learned/}
  \end{center}
\end{frame}

\end{document}
