| Message ID | 20180727152811.15258-1-sibis@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] dt-bindings: reset: Add PDC reset binding for SDM845 SoCs | expand |
Hi Sibi, On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote: > Add SDM845 PDC (Power Domain Controller) reset controller binding > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ > include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ > 2 files changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h > > diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > new file mode 100644 > index 000000000000..85e159962e08 > --- /dev/null > +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > @@ -0,0 +1,52 @@ > +PDC Reset Controller > +====================================== > + > +This binding describes a reset-controller found on PDC-Global(Power Domain > +Controller) block for Qualcomm Technologies Inc SDM845 SoCs. > + > +Required properties: > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be: > + "qcom,sdm845-pdc-global" > + > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: must specify the base address and size of the register > + space. > + > +- #reset-cells: > + Usage: required > + Value type: <uint> > + Definition: must be 1; cell entry represents the reset index. > + > +Example: > + > +pdc_reset: reset-controller@b2e0000 { Is this really just a reset controller? The name makes it sound like a driver binding to this should also provide pm_genpd and the binding should probably call this a power- controller: Documentation/devicetree/bindings/power/power_domain.txt. > + compatible = "qcom,sdm845-pdc-global"; > + reg = <0xb2e0000 0x20000>; This looks like this is the register space of the complete PDC, not just the reset register? > + #reset-cells = <1>; > +}; > + > +PDC reset clients > +====================================== > + > +Device nodes that need access to reset lines should > +specify them as a reset phandle in their corresponding node as > +specified in reset.txt. > + > +For list of all valid reset indicies see > +<dt-bindings/reset/qcom,sdm845-pdc.h> > + > +Example: > + > +modem-pil@4080000 { > + ... > + > + resets = <&pdc_reset PDC_MODEM_SYNC_RESET>; > + reset-names = "pdc_restart"; > + > + ... > +}; > diff --git a/include/dt-bindings/reset/qcom,sdm845-pdc.h b/include/dt-bindings/reset/qcom,sdm845-pdc.h > new file mode 100644 > index 000000000000..53c37f9c319a > --- /dev/null > +++ b/include/dt-bindings/reset/qcom,sdm845-pdc.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2018 The Linux Foundation. All rights reserved. > + */ > + > +#ifndef _DT_BINDINGS_RESET_PDC_SDM_845_H > +#define _DT_BINDINGS_RESET_PDC_SDM_845_H > + > +#define PDC_APPS_SYNC_RESET 0 > +#define PDC_SP_SYNC_RESET 1 > +#define PDC_AUDIO_SYNC_RESET 2 > +#define PDC_SENSORS_SYNC_RESET 3 > +#define PDC_AOP_SYNC_RESET 4 > +#define PDC_DEBUG_SYNC_RESET 5 > +#define PDC_GPU_SYNC_RESET 6 > +#define PDC_DISPLAY_SYNC_RESET 7 > +#define PDC_COMPUTE_SYNC_RESET 8 > +#define PDC_MODEM_SYNC_RESET 9 > + > +#endif regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philipp, Thanks for the review! On 07/31/2018 02:12 PM, Philipp Zabel wrote: > Hi Sibi, > > On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote: >> Add SDM845 PDC (Power Domain Controller) reset controller binding >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ >> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ >> 2 files changed, 72 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt >> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h >> >> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt >> new file mode 100644 >> index 000000000000..85e159962e08 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt >> @@ -0,0 +1,52 @@ >> +PDC Reset Controller >> +====================================== >> + >> +This binding describes a reset-controller found on PDC-Global(Power Domain >> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs. >> + >> +Required properties: >> +- compatible: >> + Usage: required >> + Value type: <string> >> + Definition: must be: >> + "qcom,sdm845-pdc-global" >> + >> +- reg: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: must specify the base address and size of the register >> + space. >> + >> +- #reset-cells: >> + Usage: required >> + Value type: <uint> >> + Definition: must be 1; cell entry represents the reset index. >> + >> +Example: >> + >> +pdc_reset: reset-controller@b2e0000 { > > Is this really just a reset controller? > > The name makes it sound like a driver binding to this should also > provide pm_genpd and the binding should probably call this a power- > controller: Documentation/devicetree/bindings/power/power_domain.txt. > The PDC-global reg space which is a part of PDC-wrapper reg space seems to be only used for the reset lines. Couple of other drivers use other parts of the PDC-wrapper reg space: https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller) https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries to occupy the entire pdc-wrapper reg space) since it couldn't be logically mapped into pdc-interrupt driver, it had to be included as a separate reset driver. >> + compatible = "qcom,sdm845-pdc-global"; >> + reg = <0xb2e0000 0x20000>; > > This looks like this is the register space of the complete PDC, not just > the reset register? > The entire register space was chosen because it is only used for its reset lines (had a good look at the downstream kernel and had a conversation with Lina) and to ensure break backward compatibility for the for the dt entry if the reg-space was used for other purposes in the future. >> + #reset-cells = <1>; >> +}; >> + >> +PDC reset clients >> +====================================== >> + >> +Device nodes that need access to reset lines should >> +specify them as a reset phandle in their corresponding node as >> +specified in reset.txt. >> + >> +For list of all valid reset indicies see >> +<dt-bindings/reset/qcom,sdm845-pdc.h> >> + >> +Example: >> + >> +modem-pil@4080000 { >> + ... >> + >> + resets = <&pdc_reset PDC_MODEM_SYNC_RESET>; >> + reset-names = "pdc_restart"; >> + >> + ... >> +}; >> diff --git a/include/dt-bindings/reset/qcom,sdm845-pdc.h b/include/dt-bindings/reset/qcom,sdm845-pdc.h >> new file mode 100644 >> index 000000000000..53c37f9c319a >> --- /dev/null >> +++ b/include/dt-bindings/reset/qcom,sdm845-pdc.h >> @@ -0,0 +1,20 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Copyright (C) 2018 The Linux Foundation. All rights reserved. >> + */ >> + >> +#ifndef _DT_BINDINGS_RESET_PDC_SDM_845_H >> +#define _DT_BINDINGS_RESET_PDC_SDM_845_H >> + >> +#define PDC_APPS_SYNC_RESET 0 >> +#define PDC_SP_SYNC_RESET 1 >> +#define PDC_AUDIO_SYNC_RESET 2 >> +#define PDC_SENSORS_SYNC_RESET 3 >> +#define PDC_AOP_SYNC_RESET 4 >> +#define PDC_DEBUG_SYNC_RESET 5 >> +#define PDC_GPU_SYNC_RESET 6 >> +#define PDC_DISPLAY_SYNC_RESET 7 >> +#define PDC_COMPUTE_SYNC_RESET 8 >> +#define PDC_MODEM_SYNC_RESET 9 >> + >> +#endif > > regards > Philipp >
On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote: > Hi Philipp, > Thanks for the review! > > On 07/31/2018 02:12 PM, Philipp Zabel wrote: > > Hi Sibi, > > > > On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote: > > > Add SDM845 PDC (Power Domain Controller) reset controller binding > > > > > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > > --- > > > .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ > > > include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ > > > 2 files changed, 72 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > > > create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h > > > > > > diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > > > new file mode 100644 > > > index 000000000000..85e159962e08 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > > > @@ -0,0 +1,52 @@ > > > +PDC Reset Controller > > > +====================================== > > > + > > > +This binding describes a reset-controller found on PDC-Global(Power Domain > > > +Controller) block for Qualcomm Technologies Inc SDM845 SoCs. > > > + > > > +Required properties: > > > +- compatible: > > > + Usage: required > > > + Value type: <string> > > > + Definition: must be: > > > + "qcom,sdm845-pdc-global" > > > + > > > +- reg: > > > + Usage: required > > > + Value type: <prop-encoded-array> > > > + Definition: must specify the base address and size of the register > > > + space. > > > + > > > +- #reset-cells: > > > + Usage: required > > > + Value type: <uint> > > > + Definition: must be 1; cell entry represents the reset index. > > > + > > > +Example: > > > + > > > +pdc_reset: reset-controller@b2e0000 { > > > > Is this really just a reset controller? > > > > The name makes it sound like a driver binding to this should also > > provide pm_genpd and the binding should probably call this a power- > > controller: Documentation/devicetree/bindings/power/power_domain.txt. > > > > The PDC-global reg space which is a part of PDC-wrapper reg space seems > to be only used for the reset lines. > > Couple of other drivers use other parts of the PDC-wrapper reg space: > https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller) > https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries > to occupy the entire pdc-wrapper reg space) > > since it couldn't be logically mapped into pdc-interrupt driver, it had > to be included as a separate reset driver. You can't have overlapping regions in DT (well, you can because we have to work-around existing DTs that do, but you shouldn't). A single node can be multiple providers such as interrupt controller and reset controller. It's an OS problem to split that into multiple drivers. > > > + compatible = "qcom,sdm845-pdc-global"; > > > + reg = <0xb2e0000 0x20000>; > > > > This looks like this is the register space of the complete PDC, not just > > the reset register? > > > > The entire register space was chosen because it is only used for its > reset lines (had a good look at the downstream kernel and had a conversation > with Lina) and to ensure break backward compatibility for > the for the dt entry if the reg-space was used for other purposes in > the future. Why do you want to ensure breaking backwards compatibility? Rob -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, Thanks for the review On 08/07/2018 11:46 PM, Rob Herring wrote: > On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote: >> Hi Philipp, >> Thanks for the review! >> >> On 07/31/2018 02:12 PM, Philipp Zabel wrote: >>> Hi Sibi, >>> >>> On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote: >>>> Add SDM845 PDC (Power Domain Controller) reset controller binding >>>> >>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >>>> --- >>>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ >>>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ >>>> 2 files changed, 72 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt >>>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h >>>> >>>> diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt >>>> new file mode 100644 >>>> index 000000000000..85e159962e08 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt >>>> @@ -0,0 +1,52 @@ >>>> +PDC Reset Controller >>>> +====================================== >>>> + >>>> +This binding describes a reset-controller found on PDC-Global(Power Domain >>>> +Controller) block for Qualcomm Technologies Inc SDM845 SoCs. >>>> + >>>> +Required properties: >>>> +- compatible: >>>> + Usage: required >>>> + Value type: <string> >>>> + Definition: must be: >>>> + "qcom,sdm845-pdc-global" >>>> + >>>> +- reg: >>>> + Usage: required >>>> + Value type: <prop-encoded-array> >>>> + Definition: must specify the base address and size of the register >>>> + space. >>>> + >>>> +- #reset-cells: >>>> + Usage: required >>>> + Value type: <uint> >>>> + Definition: must be 1; cell entry represents the reset index. >>>> + >>>> +Example: >>>> + >>>> +pdc_reset: reset-controller@b2e0000 { >>> >>> Is this really just a reset controller? >>> >>> The name makes it sound like a driver binding to this should also >>> provide pm_genpd and the binding should probably call this a power- >>> controller: Documentation/devicetree/bindings/power/power_domain.txt. >>> >> >> The PDC-global reg space which is a part of PDC-wrapper reg space seems >> to be only used for the reset lines. >> >> Couple of other drivers use other parts of the PDC-wrapper reg space: >> https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller) >> https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries >> to occupy the entire pdc-wrapper reg space) >> >> since it couldn't be logically mapped into pdc-interrupt driver, it had >> to be included as a separate reset driver. > > You can't have overlapping regions in DT (well, you can because we have > to work-around existing DTs that do, but you shouldn't). > > A single node can be multiple providers such as interrupt controller and > reset controller. It's an OS problem to split that into multiple > drivers. > There will be no overlaps. Jordan will be changing the dt binding of gmu_pdc so that there is no overlap I guess. What I meant to say is that pdc-global is a separate reg-space and currently has no other functionality other than exposing the reset lines. >>>> + compatible = "qcom,sdm845-pdc-global"; >>>> + reg = <0xb2e0000 0x20000>; >>> >>> This looks like this is the register space of the complete PDC, not just >>> the reset register? >>> >> >> The entire register space was chosen because it is only used for its >> reset lines (had a good look at the downstream kernel and had a conversation >> with Lina) and to ensure break backward compatibility for >> the for the dt entry if the reg-space was used for other purposes in >> the future. > > Why do you want to ensure breaking backwards compatibility? > Similar to the AOSS reset driver which had a unused clock part, this driver also exposes a reg space of which only reset lines are used. > Rob >
On Wed, Aug 08, 2018 at 09:14:05PM +0530, Sibi Sankar wrote: > Hi Rob, > Thanks for the review > > On 08/07/2018 11:46 PM, Rob Herring wrote: > >On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote: > >>Hi Philipp, > >>Thanks for the review! > >> > >>On 07/31/2018 02:12 PM, Philipp Zabel wrote: > >>>Hi Sibi, > >>> > >>>On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote: > >>>>Add SDM845 PDC (Power Domain Controller) reset controller binding > >>>> > >>>>Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > >>>>--- > >>>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ > >>>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ > >>>> 2 files changed, 72 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > >>>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h > >>>> > >>>>diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > >>>>new file mode 100644 > >>>>index 000000000000..85e159962e08 > >>>>--- /dev/null > >>>>+++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > >>>>@@ -0,0 +1,52 @@ > >>>>+PDC Reset Controller > >>>>+====================================== > >>>>+ > >>>>+This binding describes a reset-controller found on PDC-Global(Power Domain > >>>>+Controller) block for Qualcomm Technologies Inc SDM845 SoCs. > >>>>+ > >>>>+Required properties: > >>>>+- compatible: > >>>>+ Usage: required > >>>>+ Value type: <string> > >>>>+ Definition: must be: > >>>>+ "qcom,sdm845-pdc-global" > >>>>+ > >>>>+- reg: > >>>>+ Usage: required > >>>>+ Value type: <prop-encoded-array> > >>>>+ Definition: must specify the base address and size of the register > >>>>+ space. > >>>>+ > >>>>+- #reset-cells: > >>>>+ Usage: required > >>>>+ Value type: <uint> > >>>>+ Definition: must be 1; cell entry represents the reset index. > >>>>+ > >>>>+Example: > >>>>+ > >>>>+pdc_reset: reset-controller@b2e0000 { > >>> > >>>Is this really just a reset controller? > >>> > >>>The name makes it sound like a driver binding to this should also > >>>provide pm_genpd and the binding should probably call this a power- > >>>controller: Documentation/devicetree/bindings/power/power_domain.txt. > >>> > >> > >>The PDC-global reg space which is a part of PDC-wrapper reg space seems > >>to be only used for the reset lines. > >> > >>Couple of other drivers use other parts of the PDC-wrapper reg space: > >>https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller) > >>https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries > >>to occupy the entire pdc-wrapper reg space) > >> > >>since it couldn't be logically mapped into pdc-interrupt driver, it had > >>to be included as a separate reset driver. > > > >You can't have overlapping regions in DT (well, you can because we have > >to work-around existing DTs that do, but you shouldn't). > > > >A single node can be multiple providers such as interrupt controller and > >reset controller. It's an OS problem to split that into multiple > >drivers. > > There will be no overlaps. Jordan will be changing the dt binding of > gmu_pdc so that there is no overlap I guess. What I meant to say is that > pdc-global is a separate reg-space and currently has no other > functionality other than exposing the reset lines. Correct - the updated GPU DT will be: reg = <0x506a000 0x30000>, <0xb280000 0x10000>, <0xb480000, 0x10000>; reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq"; Jordan
On Wed, Aug 08, 2018 at 03:37:24PM -0600, Jordan Crouse wrote: > On Wed, Aug 08, 2018 at 09:14:05PM +0530, Sibi Sankar wrote: > > Hi Rob, > > Thanks for the review > > > > On 08/07/2018 11:46 PM, Rob Herring wrote: > > >On Tue, Jul 31, 2018 at 06:27:24PM +0530, Sibi S wrote: > > >>Hi Philipp, > > >>Thanks for the review! > > >> > > >>On 07/31/2018 02:12 PM, Philipp Zabel wrote: > > >>>Hi Sibi, > > >>> > > >>>On Fri, 2018-07-27 at 20:58 +0530, Sibi Sankar wrote: > > >>>>Add SDM845 PDC (Power Domain Controller) reset controller binding > > >>>> > > >>>>Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > >>>>--- > > >>>> .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ > > >>>> include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ > > >>>> 2 files changed, 72 insertions(+) > > >>>> create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > > >>>> create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h > > >>>> > > >>>>diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > > >>>>new file mode 100644 > > >>>>index 000000000000..85e159962e08 > > >>>>--- /dev/null > > >>>>+++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > > >>>>@@ -0,0 +1,52 @@ > > >>>>+PDC Reset Controller > > >>>>+====================================== > > >>>>+ > > >>>>+This binding describes a reset-controller found on PDC-Global(Power Domain > > >>>>+Controller) block for Qualcomm Technologies Inc SDM845 SoCs. > > >>>>+ > > >>>>+Required properties: > > >>>>+- compatible: > > >>>>+ Usage: required > > >>>>+ Value type: <string> > > >>>>+ Definition: must be: > > >>>>+ "qcom,sdm845-pdc-global" > > >>>>+ > > >>>>+- reg: > > >>>>+ Usage: required > > >>>>+ Value type: <prop-encoded-array> > > >>>>+ Definition: must specify the base address and size of the register > > >>>>+ space. > > >>>>+ > > >>>>+- #reset-cells: > > >>>>+ Usage: required > > >>>>+ Value type: <uint> > > >>>>+ Definition: must be 1; cell entry represents the reset index. > > >>>>+ > > >>>>+Example: > > >>>>+ > > >>>>+pdc_reset: reset-controller@b2e0000 { > > >>> > > >>>Is this really just a reset controller? > > >>> > > >>>The name makes it sound like a driver binding to this should also > > >>>provide pm_genpd and the binding should probably call this a power- > > >>>controller: Documentation/devicetree/bindings/power/power_domain.txt. > > >>> > > >> > > >>The PDC-global reg space which is a part of PDC-wrapper reg space seems > > >>to be only used for the reset lines. > > >> > > >>Couple of other drivers use other parts of the PDC-wrapper reg space: > > >>https://patchwork.kernel.org/patch/10223701/ (PDC-Interrupt controller) > > >>https://patchwork.kernel.org/patch/10255767/ (GMU-PDC incorrectly tries > > >>to occupy the entire pdc-wrapper reg space) > > >> > > >>since it couldn't be logically mapped into pdc-interrupt driver, it had > > >>to be included as a separate reset driver. > > > > > >You can't have overlapping regions in DT (well, you can because we have > > >to work-around existing DTs that do, but you shouldn't). > > > > > >A single node can be multiple providers such as interrupt controller and > > >reset controller. It's an OS problem to split that into multiple > > >drivers. > > > > There will be no overlaps. Jordan will be changing the dt binding of > > gmu_pdc so that there is no overlap I guess. What I meant to say is that > > pdc-global is a separate reg-space and currently has no other > > functionality other than exposing the reset lines. > > Correct - the updated GPU DT will be: > > reg = <0x506a000 0x30000>, > <0xb280000 0x10000>, > <0xb480000, 0x10000>; > reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq"; > > Jordan Code change: https://patchwork.freedesktop.org/patch/243513/ DT change: https://patchwork.freedesktop.org/patch/243539/ Jordan
On Fri 27 Jul 08:28 PDT 2018, Sibi Sankar wrote: > Add SDM845 PDC (Power Domain Controller) reset controller binding > Even though this is currently describing only a reset controller I think this binding better be talking about the "PDC Global" hardware. > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ > include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ > 2 files changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h > > diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt Rename this qcom,pdc-global to match the compatible, and hardware name. > new file mode 100644 > index 000000000000..85e159962e08 > --- /dev/null > +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt > @@ -0,0 +1,52 @@ > +PDC Reset Controller PDC Global > +====================================== > + > +This binding describes a reset-controller found on PDC-Global(Power Domain > +Controller) block for Qualcomm Technologies Inc SDM845 SoCs. This looks good. > + > +Required properties: > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be: > + "qcom,sdm845-pdc-global" > + > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: must specify the base address and size of the register > + space. > + > +- #reset-cells: > + Usage: required > + Value type: <uint> > + Definition: must be 1; cell entry represents the reset index. > + > +Example: > + > +pdc_reset: reset-controller@b2e0000 { This is perfectly fine, in its current form it is a reset-controller. > + compatible = "qcom,sdm845-pdc-global"; > + reg = <0xb2e0000 0x20000>; > + #reset-cells = <1>; > +}; > + Apart from this, the binding looks good! Regards, Bjorn
diff --git a/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt new file mode 100644 index 000000000000..85e159962e08 --- /dev/null +++ b/Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt @@ -0,0 +1,52 @@ +PDC Reset Controller +====================================== + +This binding describes a reset-controller found on PDC-Global(Power Domain +Controller) block for Qualcomm Technologies Inc SDM845 SoCs. + +Required properties: +- compatible: + Usage: required + Value type: <string> + Definition: must be: + "qcom,sdm845-pdc-global" + +- reg: + Usage: required + Value type: <prop-encoded-array> + Definition: must specify the base address and size of the register + space. + +- #reset-cells: + Usage: required + Value type: <uint> + Definition: must be 1; cell entry represents the reset index. + +Example: + +pdc_reset: reset-controller@b2e0000 { + compatible = "qcom,sdm845-pdc-global"; + reg = <0xb2e0000 0x20000>; + #reset-cells = <1>; +}; + +PDC reset clients +====================================== + +Device nodes that need access to reset lines should +specify them as a reset phandle in their corresponding node as +specified in reset.txt. + +For list of all valid reset indicies see +<dt-bindings/reset/qcom,sdm845-pdc.h> + +Example: + +modem-pil@4080000 { + ... + + resets = <&pdc_reset PDC_MODEM_SYNC_RESET>; + reset-names = "pdc_restart"; + + ... +}; diff --git a/include/dt-bindings/reset/qcom,sdm845-pdc.h b/include/dt-bindings/reset/qcom,sdm845-pdc.h new file mode 100644 index 000000000000..53c37f9c319a --- /dev/null +++ b/include/dt-bindings/reset/qcom,sdm845-pdc.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2018 The Linux Foundation. All rights reserved. + */ + +#ifndef _DT_BINDINGS_RESET_PDC_SDM_845_H +#define _DT_BINDINGS_RESET_PDC_SDM_845_H + +#define PDC_APPS_SYNC_RESET 0 +#define PDC_SP_SYNC_RESET 1 +#define PDC_AUDIO_SYNC_RESET 2 +#define PDC_SENSORS_SYNC_RESET 3 +#define PDC_AOP_SYNC_RESET 4 +#define PDC_DEBUG_SYNC_RESET 5 +#define PDC_GPU_SYNC_RESET 6 +#define PDC_DISPLAY_SYNC_RESET 7 +#define PDC_COMPUTE_SYNC_RESET 8 +#define PDC_MODEM_SYNC_RESET 9 + +#endif
Add SDM845 PDC (Power Domain Controller) reset controller binding Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- .../bindings/reset/qcom,pdc-reset.txt | 52 +++++++++++++++++++ include/dt-bindings/reset/qcom,sdm845-pdc.h | 20 +++++++ 2 files changed, 72 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/qcom,pdc-reset.txt create mode 100644 include/dt-bindings/reset/qcom,sdm845-pdc.h