Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221004003811.4075765-1-jrosenth@chromium.org>
 <Yzvz9hn1G8rU4VaO@kroah.com> <Yzxl2oczTtwEvIqt@chromium.org>
 <Yzxsu2Ms43eTfOYR@kroah.com> <Yzy6KNUHFqxWZb9U@chromium.org>
 <CAODwPW-7Y_CbCch+Y5unH3yJD1T=3epYvqja6w_CB-23C9x9sw@mail.gmail.com> <Yz0jVbfDOITZfE9M@kroah.com>
In-Reply-To: <Yz0jVbfDOITZfE9M@kroah.com>
From:   Julius Werner <jwerner@chromium.org>
Date:   Wed, 5 Oct 2022 16:26:55 -0700
Message-ID: <CAODwPW8R2uXFJ_5V737Dy8z-WJHUwKkLyG4MW_Q50fs-OFm7Sw@mail.gmail.com>
Subject: Re: [PATCH v12] firmware: google: Implement cbmem in sysfs driver
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     Julius Werner <jwerner@chromium.org>,
        Jack Rosenthal <jrosenth@chromium.org>,
        LKML <linux-kernel@vger.kernel.org>,
        chrome-platform@lists.linux.dev,
        Stephen Boyd <swboyd@chromium.org>,
        Tzung-Bi Shih <tzungbi@kernel.org>,
        Guenter Roeck <groeck@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> If the kernel is reporting a value, that value needs to be documented
> somewhere.  If the kernel is acting on that value, it needs to know what
> those values are.
>
> In this specific instance it seems that the kernel knows a subset of the
> values, and some random userspace tool knows all of them?  Think about
> what you would want to see here if you knew nothing about this at all.

The kernel doesn't know any of the values. The kernel is just telling
userspace that spaces with these IDs exist and providing an interface
to access them, it's not supposed to know what any of them mean.

In terms of what you'd want to see in the documentation, I think what
Jack's patch provides is already the best solution? We're referring to
the definitions in the coreboot source tree as the source of truth for
exact details about what each of these IDs mean. Do you want that
documentation to say more explicitly that these are coreboot-internal
data structures exposed for use by coreboot-aware userspace tools and
that their exact meaning and format is only described within coreboot
sources? Or do you want us to put a full link to coreboot's gitiles
page for the file instead of just the file name? Other than that I'm
not sure how we could make this more explicit -- we don't have a big
official documentation page separate from the source code for all
these IDs in coreboot, unfortunately (like I said, some of them a
large and standardized but most of them are small, platform-specific
things for communicating between different firmware stages that don't
need much explanation beyond the source code itself and don't always
have a fixed format).