Re: [PATCH 25/26] dt-bindings: reserved-memory: Add Google Kinfo Pixel reserved memory

Krzysztof Kozlowski
2025-11-19T16:02:26Z

On 19/11/2025 16:44, Eugen Hristev wrote:
> Add documentation for Google Kinfo Pixel reserved memory area.

Above and commit msg describe something completely else than binding. In
the binding you described kinfo Linux driver, above you suggest this is
some sort of reserved memory.

>
> Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
> ---
> .../reserved-memory/google,kinfo.yaml | 49 +++++++++++++++++++
> MAINTAINERS | 5 ++
> 2 files changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reserved-memory/google,kinfo.yaml
>
> diff --git a/Documentation/devicetree/bindings/reserved-memory/google,kinfo.yaml b/Documentation/devicetree/bindings/reserved-memory/google,kinfo.yaml
> new file mode 100644
> index 000000000000..12d0b2815c02
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/google,kinfo.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/google,kinfo.yaml#

Filename based on the compatible.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google Pixel Kinfo reserved memory
> +
> +maintainers:
> + - Eugen Hristev <eugen.hristev@linaro.org>
> +
> +description:
> + This binding describes the Google Pixel Kinfo reserved memory, a region

Don't use "This binding", but please describe here hardware.

> + of reserved-memory used to store data for firmware/bootloader on the Pixel
> + platform. The data stored is debugging information on the running kernel.
> +
> +properties:
> + compatible:
> + items:
> + - const: google,kinfo
> +
> + memory-region:
> + maxItems: 1
> + description: Reference to the reserved-memory for the data

This does not match description. Unfortunately it looks like you added a
node just to instantiate Linux driver and this is not allowed.

If this was some special reserved memory region, then it would be part
of reserved memory bindings - see reserved-memory directory.

Compatible suggests that it is purely Linux driver, so another hint.

Looks like this is a SoC specific thing, so maybe this should be folded
in some of the soc drivers.

> +
> +required:
> + - compatible
> + - memory-region
> +
> +additionalProperties: true
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + kinfo_region: smem@fa00000 {
> + reg = <0xfa00000 0x1000>;
> + no-map;
> + };
> + };

Anyway, drop, not relevant.

> +
> + debug-kinfo {
> + compatible = "google,debug-kinfo";

Device node with only one phandle to reserved memory region is a proof
it is not a real device.

Also,
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

Best regards,
Krzysztof