diff mbox series

docs: driver-api: firmware: add driver firmware guidelines. (v2)

Message ID 20220719065357.2705918-1-airlied@gmail.com
State Superseded
Headers show
Series docs: driver-api: firmware: add driver firmware guidelines. (v2) | expand

Commit Message

Dave Airlie July 19, 2022, 6:53 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

A recent snafu where Intel ignored upstream feedback on a firmware
change, led to a late rc6 fix being required. In order to avoid this
in the future we should document some expectations around
linux-firmware.

I was originally going to write this for drm, but it seems quite generic
advice.

v2: rewritten with suggestions from Thorsten Leemhuis.

Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 Documentation/driver-api/firmware/core.rst    |  1 +
 .../firmware/firmware-usage-guidelines.rst    | 34 +++++++++++++++++++
 2 files changed, 35 insertions(+)
 create mode 100644 Documentation/driver-api/firmware/firmware-usage-guidelines.rst

Comments

Dave Airlie July 21, 2022, 3:43 a.m. UTC | #1
> It is hard to enforce how vendors will version their firmware. On media,
> we have some drivers whose major means different hardware versions. For
> instance, on xc3028, v3.x means low voltage chips, while v2.x means
> "normal" voltage. We end changing the file name on Linux to avoid the risk
> of damaging the hardware, as using v27 firmware on low power chips damage
> them. So, we have:
>
>         drivers/media/tuners/xc2028.h:#define XC2028_DEFAULT_FIRMWARE "xc3028-v27.fw"
>         drivers/media/tuners/xc2028.h:#define XC3028L_DEFAULT_FIRMWARE "xc3028L-v36.fw"
>
> As their main market is not Linux - nor PC - as their main sales are on
> TV sets, and them don't officially support Linux, there's nothing we can
> do to enforce it.
>
> IMO we need a more generic text here to indicate that Linux firmware
> files should be defined in a way that it should be possible to detect
> when there are incompatibilities with past versions.
> So, I would say, instead:
>
>         Firmware files shall be designed in a way that it allows
>         checking for firmware ABI version changes. It is recommended
>         that firmware files to be versioned with at least major/minor
>         version.

This sounds good, will update with this.

>
> > It
> > +  is suggested that the firmware files in linux-firmware be named with
> > +  some device specific name, and just the major version.
>
> > The
> > +  major/minor/patch versions should be stored in a header in the
> > +  firmware file for the driver to detect any non-ABI fixes/issues.
>
> I would also make this more generic. On media, we ended adding the firmware
> version indicated at the file name. For instance, xc4000 driver checks for
> two firmware files:
>
> drivers/media/tuners/xc4000.c:#define XC4000_DEFAULT_FIRMWARE "dvb-fe-xc4000-1.4.fw"
> drivers/media/tuners/xc4000.c:#define XC4000_DEFAULT_FIRMWARE_NEW "dvb-fe-xc4000-1.4.1.fw"

This is probably fine for products where development never produces
much firmwares, but it quickly becomes unmanageable when you end up
with _NEW_NEW_NEW etc.

I'd rather not encourage this sort of thing unless it is totally
outside our control. So I'd like to keep the guidelines for when we
have some control what we'd recommend.

In this case I'd have recommended you put the 1.4.1 in the header of
the fw, and just have it called dvb-fe-xc4000-1.fw and overwrite the
NEW with the OLD, I understand we likely don't have the control here.

> > +  firmware files in linux-firmware should be overwritten with the newest
> > +  compatible major version.
>
> For me "shall" is mandatory, while "should" is optional.
>
> In this specific case, I'm not so sure if overriding it is the best thing
> to do on all subsystems. I mean, even with the same ABI, older firmware
> usually means that some bugs and/or limitations will be present there.

As long as you can detect the minor/patch versions from the firmware
file after loading it you should be able to do sufficient workarounds.
>
> That's specially true on codecs: even having the same ABI, older versions
> won't support decoding newer protocols. We have one case with some
> digital TV decoders that only support some Cable-TV protocols with
> newer firmware versions. We have also one case were remote controller
> decoding is buggy with older firmwares. On both situations, the ABI
> didn't change.

If the only way to figure that out is by the filename or minor
version, then so be it, but where people have some control I'd rather
provide some harder guidelines.

Dave.
diff mbox series

Patch

diff --git a/Documentation/driver-api/firmware/core.rst b/Documentation/driver-api/firmware/core.rst
index 1d1688cbc078..803cd574bbd7 100644
--- a/Documentation/driver-api/firmware/core.rst
+++ b/Documentation/driver-api/firmware/core.rst
@@ -13,4 +13,5 @@  documents these features.
    direct-fs-lookup
    fallback-mechanisms
    lookup-order
+   firmware-usage-guidelines
 
diff --git a/Documentation/driver-api/firmware/firmware-usage-guidelines.rst b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst
new file mode 100644
index 000000000000..34d2412e78c6
--- /dev/null
+++ b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst
@@ -0,0 +1,34 @@ 
+===================
+Firmware Guidelines
+===================
+
+Drivers that use firmware from linux-firmware should attempt to follow
+the rules in this guide.
+
+* Firmware should be versioned with at least a major/minor version. It
+  is suggested that the firmware files in linux-firmware be named with
+  some device specific name, and just the major version. The
+  major/minor/patch versions should be stored in a header in the
+  firmware file for the driver to detect any non-ABI fixes/issues. The
+  firmware files in linux-firmware should be overwritten with the newest
+  compatible major version. Newer major version firmware should remain
+  compatible with all kernels that load that major number.
+
+* Users should *not* have to install newer firmware to use existing
+  hardware when they install a newer kernel.  If the hardware isn't
+  enabled by default or under development, this can be ignored, until
+  the first kernel release that enables that hardware.  This means no
+  major version bumps without the kernel retaining backwards
+  compatibility for the older major versions.  Minor version bumps
+  should not introduce new features that newer kernels depend on
+  non-optionally.
+
+* If a security fix needs lockstep firmware and kernel fixes in order to
+  be successful, then all supported major versions in the linux-firmware
+  repo should be updated with the security fix, and the kernel patches
+  should detect if the firmware is new enough to declare if the security
+  issue is fixed.  All communications around security fixes should point
+  at both the firmware and kernel fixes. If a security fix requires
+  deprecating old major versions, then this should only be done as a
+  last option, and be stated clearly in all communications.
+