| Message ID | 20180226175802.20052-3-ilina@codeaurora.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | None | expand |
On Mon, Feb 26, 2018 at 10:58:02AM -0700, Lina Iyer wrote: > From: Mahesh Sivasubramanian <msivasub@codeaurora.org> > > Command DB provides information on shared resources like clocks, > regulators etc., probed at boot by the remote subsytem and made > available in shared memory. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org> > Signed-off-by: Lina Iyer <ilina@codeaurora.org> > --- > > Changes in v4: > - Fix unwanted capitalization > - Add reg property > --- > .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt Reviewed-by: Rob Herring <robh@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 26 Feb 09:58 PST 2018, Lina Iyer wrote: > From: Mahesh Sivasubramanian <msivasub@codeaurora.org> > > Command DB provides information on shared resources like clocks, > regulators etc., probed at boot by the remote subsytem and made > available in shared memory. > > Cc: devicetree@vger.kernel.org > Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org> > Signed-off-by: Lina Iyer <ilina@codeaurora.org> > --- > > Changes in v4: > - Fix unwanted capitalization > - Add reg property > --- > .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt > > diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt [..] > + reserved-memory { > + [...] > + qcom,cmd-db@c3f000c { > + reg = <0x0 0xc3f000c 0x0 0x8>, > + <0x0 0x85fe0000 0x0 0x20000>; I'm still concerned about the use of the redirection mapping here, the relocation at 0xc3f000c is used a convenience thing so that the command db can be relocated, but because of how Linux will consume any non-reserved memory the dts would still need to be manually updated. As such I think you should just describe only the 0x85fe0000 + 0x20000 region here and to support the dynamic aspect of this from a system point of view you can have the boot loader read the information at 0xc3f000c and adjust the reserved memory. (Or just keep the step of manually update the dts without caring about the indirection) Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Lina Iyer (2018-02-26 09:58:02) > diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt > new file mode 100644 > index 000000000000..5737ed2ac6e8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt > @@ -0,0 +1,38 @@ > +Command DB > +--------- > + > +Command DB is a database that provides a mapping between resource key and the s/between/between the/ > +resource address for a system resource managed by a remote processor. The data > +is stored in a shared memory region and is loaded by the remote processor. > + > +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for s/SoC's/SoCs/ > +controlling shared resources. Depending on the board configuration the shared > +resource properties may change. These properties are dynamically probed by the > +remote processor and made available in the shared memory. > + > +The bindings for Command DB is specified in the reserved-memory section in s/bindings/node/ maybe? > +devicetree. The devicetree representation of the command DB driver should be: Maybe drop this last sentence entirely. > + > +Properties: > +- compatible: > + Usage: required > + Value type: <string> > + Definition: Should be "qcom,cmd-db" > + > +- reg: > + Usage: required > + Value type: <prop encoded array> > + Definition: The register address that points to the location of the > + Command DB in memory. Additionally, specify the address > + and size of the actual lacation in memory. s/lacation/location/ > + > +Example: > + > + reserved-memory { > + [...] > + qcom,cmd-db@c3f000c { > + reg = <0x0 0xc3f000c 0x0 0x8>, > + <0x0 0x85fe0000 0x0 0x20000>; > + compatible = "qcom,cmd-db"; > + }; > + }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 05 2018 at 16:15 -0700, Bjorn Andersson wrote: >On Mon 26 Feb 09:58 PST 2018, Lina Iyer wrote: > >> From: Mahesh Sivasubramanian <msivasub@codeaurora.org> >> >> Command DB provides information on shared resources like clocks, >> regulators etc., probed at boot by the remote subsytem and made >> available in shared memory. >> >> Cc: devicetree@vger.kernel.org >> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org> >> Signed-off-by: Lina Iyer <ilina@codeaurora.org> >> --- >> >> Changes in v4: >> - Fix unwanted capitalization >> - Add reg property >> --- >> .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt >> >> diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt >[..] >> + reserved-memory { >> + [...] >> + qcom,cmd-db@c3f000c { >> + reg = <0x0 0xc3f000c 0x0 0x8>, >> + <0x0 0x85fe0000 0x0 0x20000>; > >I'm still concerned about the use of the redirection mapping here, >the relocation at 0xc3f000c is used a convenience thing so that the >command db can be relocated, but because of how Linux will consume any >non-reserved memory the dts would still need to be manually updated. > This location is fixed and it is not expected to change between different boards. OEMs may change the actual address of the command db location, but would not change the dictionary location. >As such I think you should just describe only the 0x85fe0000 + 0x20000 >region here and to support the dynamic aspect of this from a system >point of view you can have the boot loader read the information at >0xc3f000c and adjust the reserved memory. (Or just keep the step of >manually update the dts without caring about the indirection) > It would be incorrect and very board specific to just use the 0x85fe000 as the address. It is not how the SoC defines the location. Upon request earlier, this memory location was added in DT and the location is typical reference platform usage only. Thanks, Lina -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 05 2018 at 17:21 -0700, Stephen Boyd wrote: >Quoting Lina Iyer (2018-02-26 09:58:02) >> diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt >> new file mode 100644 >> index 000000000000..5737ed2ac6e8 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt >> @@ -0,0 +1,38 @@ >> +Command DB >> +--------- >> + >> +Command DB is a database that provides a mapping between resource key and the > >s/between/between the/ > >> +resource address for a system resource managed by a remote processor. The data >> +is stored in a shared memory region and is loaded by the remote processor. >> + >> +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for > >s/SoC's/SoCs/ > >> +controlling shared resources. Depending on the board configuration the shared >> +resource properties may change. These properties are dynamically probed by the >> +remote processor and made available in the shared memory. >> + >> +The bindings for Command DB is specified in the reserved-memory section in > >s/bindings/node/ > >maybe? > >> +devicetree. The devicetree representation of the command DB driver should be: > >Maybe drop this last sentence entirely. > >> + >> +Properties: >> +- compatible: >> + Usage: required >> + Value type: <string> >> + Definition: Should be "qcom,cmd-db" >> + >> +- reg: >> + Usage: required >> + Value type: <prop encoded array> >> + Definition: The register address that points to the location of the >> + Command DB in memory. Additionally, specify the address >> + and size of the actual lacation in memory. > >s/lacation/location/ > Will take care of these in the next spin. >> + >> +Example: >> + >> + reserved-memory { >> + [...] >> + qcom,cmd-db@c3f000c { >> + reg = <0x0 0xc3f000c 0x0 0x8>, >> + <0x0 0x85fe0000 0x0 0x20000>; >> + compatible = "qcom,cmd-db"; >> + }; >> + }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue 06 Mar 07:57 PST 2018, Lina Iyer wrote: > On Mon, Mar 05 2018 at 16:15 -0700, Bjorn Andersson wrote: > > On Mon 26 Feb 09:58 PST 2018, Lina Iyer wrote: > > > > > From: Mahesh Sivasubramanian <msivasub@codeaurora.org> > > > > > > Command DB provides information on shared resources like clocks, > > > regulators etc., probed at boot by the remote subsytem and made > > > available in shared memory. > > > > > > Cc: devicetree@vger.kernel.org > > > Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org> > > > Signed-off-by: Lina Iyer <ilina@codeaurora.org> > > > --- > > > > > > Changes in v4: > > > - Fix unwanted capitalization > > > - Add reg property > > > --- > > > .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++++++++++++++++++++++ > > > 1 file changed, 38 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt > > > > > > diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt > > [..] > > > + reserved-memory { > > > + [...] > > > + qcom,cmd-db@c3f000c { > > > + reg = <0x0 0xc3f000c 0x0 0x8>, > > > + <0x0 0x85fe0000 0x0 0x20000>; > > > > I'm still concerned about the use of the redirection mapping here, > > the relocation at 0xc3f000c is used a convenience thing so that the > > command db can be relocated, but because of how Linux will consume any > > non-reserved memory the dts would still need to be manually updated. > > > This location is fixed and it is not expected to change between > different boards. OEMs may change the actual address of the command db > location, but would not change the dictionary location. > Right, dictionary location is fixed. So to pick another range for command db one would update the numbers that goes into the dictionary and expect all software in the system to use this new range. > > As such I think you should just describe only the 0x85fe0000 + 0x20000 > > region here and to support the dynamic aspect of this from a system > > point of view you can have the boot loader read the information at > > 0xc3f000c and adjust the reserved memory. (Or just keep the step of > > manually update the dts without caring about the indirection) > > > It would be incorrect and very board specific to just use the 0x85fe000 > as the address. It is not how the SoC defines the location. Upon request > earlier, this memory location was added in DT and the location is > typical reference platform usage only. > The problem is that as the db resides in a chunk of memory in the middle of what Linux considers System RAM the DTS must specify this region as reserved. Which means that as you, like described above, update the dictionary something (in your scheme a person) has to update the reserved-memory region as well. That's why I'm proposing that the appropriate implementation for this is to have the boot loader to the dictionary part of this and Linux only care about the actual reserved-memory region. This way you would still implement the dictionary lookup on a system level, but the Linux part no longer depend on a human updating the DTS to match the values of the dictionary. But if we stick with the approach of describing both these and hoping that the values in the first region matches the second (or should we add a sanity check in probe?). The memory reserve defined as 0xc3f000c + 8 looks strange, is this system ram as well and what other things resides in that same page? Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Bjorn Andersson (2018-03-07 11:02:49) > On Tue 06 Mar 07:57 PST 2018, Lina Iyer wrote: > > > On Mon, Mar 05 2018 at 16:15 -0700, Bjorn Andersson wrote: > > > On Mon 26 Feb 09:58 PST 2018, Lina Iyer wrote: > > > > As such I think you should just describe only the 0x85fe0000 + 0x20000 > > > region here and to support the dynamic aspect of this from a system > > > point of view you can have the boot loader read the information at > > > 0xc3f000c and adjust the reserved memory. (Or just keep the step of > > > manually update the dts without caring about the indirection) > > > > > It would be incorrect and very board specific to just use the 0x85fe000 > > as the address. It is not how the SoC defines the location. Upon request > > earlier, this memory location was added in DT and the location is > > typical reference platform usage only. > > > > The problem is that as the db resides in a chunk of memory in the middle > of what Linux considers System RAM the DTS must specify this region as > reserved. Which means that as you, like described above, update the > dictionary something (in your scheme a person) has to update the > reserved-memory region as well. > > That's why I'm proposing that the appropriate implementation for this > is to have the boot loader to the dictionary part of this and Linux only > care about the actual reserved-memory region. This way you would still > implement the dictionary lookup on a system level, but the Linux > part no longer depend on a human updating the DTS to match the values of > the dictionary. Agreed. I thought SMEM had a similar design of a cookie in IMEM to indicate location and size because coordinating changes across all the various software images is a hard problem. But coordinating between linux and the linux bootloader shouldn't be as hard. > > > But if we stick with the approach of describing both these and hoping > that the values in the first region matches the second (or should we add > a sanity check in probe?). The memory reserve defined as 0xc3f000c + 8 > looks strange, is this system ram as well and what other things resides > in that same page? > Doesn't look like it could be RAM, the address is not very close to the other one so I would guess it's something like IMEM. And there are two 32-bit numbers to describe address and size? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri 16 Mar 11:26 PDT 2018, Stephen Boyd wrote: > Quoting Bjorn Andersson (2018-03-07 11:02:49) > > On Tue 06 Mar 07:57 PST 2018, Lina Iyer wrote: > > > > > On Mon, Mar 05 2018 at 16:15 -0700, Bjorn Andersson wrote: > > > > On Mon 26 Feb 09:58 PST 2018, Lina Iyer wrote: > > > > > > As such I think you should just describe only the 0x85fe0000 + 0x20000 > > > > region here and to support the dynamic aspect of this from a system > > > > point of view you can have the boot loader read the information at > > > > 0xc3f000c and adjust the reserved memory. (Or just keep the step of > > > > manually update the dts without caring about the indirection) > > > > > > > It would be incorrect and very board specific to just use the 0x85fe000 > > > as the address. It is not how the SoC defines the location. Upon request > > > earlier, this memory location was added in DT and the location is > > > typical reference platform usage only. > > > > > > > The problem is that as the db resides in a chunk of memory in the middle > > of what Linux considers System RAM the DTS must specify this region as > > reserved. Which means that as you, like described above, update the > > dictionary something (in your scheme a person) has to update the > > reserved-memory region as well. > > > > That's why I'm proposing that the appropriate implementation for this > > is to have the boot loader to the dictionary part of this and Linux only > > care about the actual reserved-memory region. This way you would still > > implement the dictionary lookup on a system level, but the Linux > > part no longer depend on a human updating the DTS to match the values of > > the dictionary. > > Agreed. I thought SMEM had a similar design of a cookie in IMEM to > indicate location and size because coordinating changes across all the > various software images is a hard problem. But coordinating between > linux and the linux bootloader shouldn't be as hard. > Correct, SMEM has exactly the same mechanism. I get that the coordinating of these addresses between all involved images can be a tricky thing, but the only way to make this automatic on the application CPU is to split this between boot and Linux; as any all-Linux solution requires a human to update the reserved-memory node to match the values in the indirection. So for SMEM we agreed that we implement this as "someone needs to tell Linux where SMEM is" and that can be a human or the boot loader. Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt new file mode 100644 index 000000000000..5737ed2ac6e8 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt @@ -0,0 +1,38 @@ +Command DB +--------- + +Command DB is a database that provides a mapping between resource key and the +resource address for a system resource managed by a remote processor. The data +is stored in a shared memory region and is loaded by the remote processor. + +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for +controlling shared resources. Depending on the board configuration the shared +resource properties may change. These properties are dynamically probed by the +remote processor and made available in the shared memory. + +The bindings for Command DB is specified in the reserved-memory section in +devicetree. The devicetree representation of the command DB driver should be: + +Properties: +- compatible: + Usage: required + Value type: <string> + Definition: Should be "qcom,cmd-db" + +- reg: + Usage: required + Value type: <prop encoded array> + Definition: The register address that points to the location of the + Command DB in memory. Additionally, specify the address + and size of the actual lacation in memory. + +Example: + + reserved-memory { + [...] + qcom,cmd-db@c3f000c { + reg = <0x0 0xc3f000c 0x0 0x8>, + <0x0 0x85fe0000 0x0 0x20000>; + compatible = "qcom,cmd-db"; + }; + };