On Tue, 9 May 2023 at 15:21, Souradeep Chowdhury <quic_schowdhu@xxxxxxxxxxx> wrote: > > > > On 5/9/2023 5:05 PM, Dmitry Baryshkov wrote: > > On Tue, 9 May 2023 at 13:53, Souradeep Chowdhury > > <quic_schowdhu@xxxxxxxxxxx> 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). > > > > >> > >> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@xxxxxxxxxxx> > >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > >> --- > >> .../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 > >> > > > > -- With best wishes Dmitry
