Messages in this thread | | | Date | Thu, 11 May 2023 11:29:12 +0530 | Subject | Re: [PATCH V6 1/3] dt-bindings: sram: qcom,imem: Add Boot Stat region within IMEM | From | Souradeep Chowdhury <> |
| |
On 5/9/2023 6:35 PM, Dmitry Baryshkov wrote: > On Tue, 9 May 2023 at 15:21, Souradeep Chowdhury > <quic_schowdhu@quicinc.com> wrote: >> >> >> >> On 5/9/2023 5:05 PM, Dmitry Baryshkov wrote: >>> On Tue, 9 May 2023 at 13:53, Souradeep Chowdhury >>> <quic_schowdhu@quicinc.com> wrote: >>>> >>>> All Qualcomm bootloaders log useful timestamp information related >>>> to bootloader stats in the IMEM region. Add the child node within >>>> IMEM for the boot stat region containing register address and >>>> compatible string. >>> >>> I might have a minor vote here. Is there any reason why you have to >>> instantiate the device from DT? >>> It looks like a software interface. Ideally software should not be >>> described in DT (e.g. this can be instantiated from imem >>> driver-to-be). >>> Or we can follow the RPM master-stats approach, where the device is a >>> top-level device, having handle pointers to the sram regions. >> >> This is a dedicated region of IMEM reserved for storing stats related >> information. So it is represented as a child of IMEM, please >> refer to Documentation/devicetree/bindings/sram/sram.yaml which >> follows a similar philosophy. Also since this is a child of IMEM with >> a specific purpose, does it not warrant a dedicated driver? > > I do not question a dedicated driver. I was asking about the DT node. > Even the mentioned bindings file describes the SRAM regions inside the > SRAM, rather than a proper device to be instantiated in the SRAM node. > I'd point to the boot_stats discussions (present on the list in the > last several months). >
Ack. Will instantiate the device from the parent node in the driver and access the stats region to print the boot_stats information.
>> >>> >>>> >>>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> --- >>>> .../devicetree/bindings/sram/qcom,imem.yaml | 22 +++++++++++++++++++ >>>> 1 file changed, 22 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml >>>> index 0548e8e0d30b..bb884c5c8952 100644 >>>> --- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml >>>> +++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml >>>> @@ -50,6 +50,28 @@ patternProperties: >>>> $ref: /schemas/remoteproc/qcom,pil-info.yaml# >>>> description: Peripheral image loader relocation region >>>> >>>> + "^stats@[0-9a-f]+$": >>>> + type: object >>>> + description: >>>> + Imem region dedicated for storing timestamps related >>>> + information regarding bootstats. >>>> + >>>> + additionalProperties: false >>>> + >>>> + properties: >>>> + compatible: >>>> + items: >>>> + - enum: >>>> + - qcom,sm8450-bootstats >>>> + - const: qcom,imem-bootstats >>>> + >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + required: >>>> + - compatible >>>> + - reg >>>> + >>>> required: >>>> - compatible >>>> - reg >>>> -- >>>> 2.17.1 >>>> >>> >>> > > >
| |