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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 19/11/2025 17:19, Eugen Hristev wrote:
> 
> 
> On 11/19/25 18:02, Krzysztof Kozlowski wrote:
>> 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.
> 
> I sent this patch for reserved-memory directory, where all the
> reserved-memory bindings reside. Or maybe I do not understand your
> comment ?>

There is no ref to reserved memory here. Please look first how reserved
memory bindings are written,

>> Compatible suggests that it is purely Linux driver, so another hint.
> 
> This reserved memory area is used by both Linux and firmware. Linux
> stores some information into this reserved memory to be used by the
> firmware/bootloader in some specific scenarios (e.g. crash or recovery
> situations)
> As the firmware reserves this memory for this specific purpose, it is
> natural to inform Linux that the memory should not be used by another
> purpose, but by the purpose it was reserved for.

But you did not write bindings for it. You wrote bindings for Linux
device driver, I already explained that last time.

> Which would be the best way to have Linux understand where is this
> memory area so it could be handled?


> 
> 
>>
>> Looks like this is a SoC specific thing, so maybe this should be folded
>> in some of the soc drivers.
>>
> Not really soc specific. Any soc who implements this at firmware level
> can use it. The firmware can reserve some memory for this specific
> purpose and then pass it to Linux, so Linux can fill it up.
> It just happens that the Pixel phone has this implemented right now, but
> it is not constrained to Pixel only.
> 
> Instantiating this driver with a call like platform_device_register_data
> would make the driver unaware of where exactly the firmware looks for
> the data. This is right now passed through the DT node. Do you have a
> better suggestion on how to pass it ?

I do not see how this question is relevant here. I don't care how you
pass it to the driver, because we discuss bindings. You created bindings
for Linux driver and that's a no. If you wanted that, I suggests that it
could be instantiated by some other driver, but sure - we don't have to
go that way, that was just an idea how to solve the problem bindings
like this cannot be accepted.

Best regards,
Krzysztof