zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.62k stars 6.5k forks source link

USB-C subsystem: Spec citations per-file or per-section? #56697

Open alevkoy opened 1 year ago

alevkoy commented 1 year ago

Introduction

The USB-C subsystem is based on several specs that received regular updates. How thoroughly should the implementation cite these specs? Should the citations be per-file or per-spec-section?

Problem description

This is a fork from the conversation in https://github.com/zephyrproject-rtos/zephyr/pull/52993#discussion_r1150872385.

The USB-C subsystem is based primarily on the USB PD specification and the USB Type-C specification. The implementation also references the USB Type-C Port Controller Interface specification and may reference various other related specifications. These specifications reference each other and are mostly consistent with each other, but the USB IF releases them asynchronously, and each has its own versioning scheme.

In particular, the subsystem implements state machines described in the PD and Type-C specs and tries to adhere very closely to the spec in terms of state names, behavior, and transition logic. Each state typically has a numbered subsection in the spec, and the code for each state often cites that subsection. In most cases, the language of the spec is the final word on the correctness of the implementation, so it's valuable to be able to refer to the spec easily when reading the code. Reviewers can use these references to check the correctness of new code, and maintainers can use these references to identify sections of code that need to be updated in response to a spec revision.

Up to this point, citations in the subsystem have mostly been at the file level, referring to a spec name and version. Components within that file have referred to named spec components and sometimes spec sections but rarely spec versions.

The USB IF updates the PD spec approximately quarterly and the Type-C spec approximately annually. This presents several problems:

  1. Spec updates can change the subsection numbering, making a previously valid reference invalid.
  2. Looking at a particular version of a spec, it isn't immediately obvious how the code needs to change to conform to the new spec. Looking at the code, it isn't immediately obvious which parts might need to change to conform to the new spec.
  3. When someone changes part of the code in response to a spec update, it muddies the waters about which spec version the implementation conforms to. The effect may be that there is no version to which the whole implementation conforms. This makes the spec citations more difficult to follow.

Proposed change

Chunks of code corresponding to sections of the specs should contain citations listing the spec name, the release, and the subsection. Where the code directly quotes the specs, it should also list similar citations. Where an entire file is based on a spec, file-level comments may refer to the spec name but should avoid reference to specific releases or sections below the chapter level.

Detailed RFC

Proposed change (Detailed)

  1. In new USB-C code based on specs, code chunks based on spec sections should cite those sections, including the spec release.
  2. In existing USB-C code, file-level references to spec releases should be removed. Function/block-level references with spec releases should be added.
  3. Authors fixing spec conformance bugs or updating the code based on spec changes should update comments for specific sections to a newer spec version, if the code now conforms to that version.

(This is the approach that the Chrome OS TCPM implementers have historically taken most of the time.)

Dependencies

This primarily affects maintainers of the USB-C subsystem.

Concerns and Unresolved Questions

Removing the file-level spec version comments removes the insinuation/expectation that the whole file conforms to the same spec version. This is largely a positive, as it more closely reflects the reality. The specs are large, and keeping an implementation up to date with new spec releases requires sustained effort. However, it's possible that abandoning the idea of a uniformly up-to-date implementation might lead to slower updates. It's also possible that explicitly endorsing incremental updates would lead to faster updates.

Alternatives

alevkoy commented 1 year ago

@sambhurst @keith-zephyr @DianaZig