| Message ID | 20190824152411.21757-2-sibis@codeaurora.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add PDC Global and AOSS reset support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Sat, 2019-08-24 at 20:54 +0530, Sibi Sankar wrote: > Add SC7180 AOSS reset to the list of possible bindings. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt > index 510c748656ec5..3eb6a22ced4bc 100644 > --- a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt > +++ b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt > @@ -8,8 +8,8 @@ Required properties: > - compatible: > Usage: required > Value type: <string> > - Definition: must be: > - "qcom,sdm845-aoss-cc" > + Definition: must be one of: > + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" Should this emphasize that the common sdm845 compatible always has to be included? + Definition: must be: + "qcom,sdm845-aoss-cc" for SDM845 or + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" for SC7180 or like the qcom,kpss-gcc bindings: + Definition: should be one of the following. The generic compatible + "qcom,sdm845-aoss-cc" should also be included. + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" + "qcom,sdm845-aoss-cc" regards Philipp
Hey Philipp, Thanks for the review! On 2019-08-26 14:06, Philipp Zabel wrote: > On Sat, 2019-08-24 at 20:54 +0530, Sibi Sankar wrote: >> Add SC7180 AOSS reset to the list of possible bindings. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> index 510c748656ec5..3eb6a22ced4bc 100644 >> --- a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> +++ b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> @@ -8,8 +8,8 @@ Required properties: >> - compatible: >> Usage: required >> Value type: <string> >> - Definition: must be: >> - "qcom,sdm845-aoss-cc" >> + Definition: must be one of: >> + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" > > Should this emphasize that the common sdm845 compatible always has to > be > included? > > + Definition: must be: > + "qcom,sdm845-aoss-cc" for SDM845 or can we drop the "or" since we would need to keep adding it in the future. > + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" for SC7180 I prefer ^^ approach for the reasons stated below. > > or like the qcom,kpss-gcc bindings: > > + Definition: should be one of the following. The generic > compatible > + "qcom,sdm845-aoss-cc" should also be > included. > + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" > + It is extremely unlikely that future SoCs would maintain same number of reset lines/offsets due to the constant flux in remote processors being added to the SoCs. So a generic compatible might not make sense here. > "qcom,sdm845-aoss-cc" > > regards > Philipp
Quoting Sibi Sankar (2019-08-24 08:24:10) > Add SC7180 AOSS reset to the list of possible bindings. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt | 4 ++-- Can you convert this binding to YAML/JSON schema? Would help to describe the 'one of' requirement below in a more structured way. > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt > index 510c748656ec5..3eb6a22ced4bc 100644 > --- a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt > +++ b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt > @@ -8,8 +8,8 @@ Required properties: > - compatible: > Usage: required > Value type: <string> > - Definition: must be: > - "qcom,sdm845-aoss-cc" > + Definition: must be one of:
Hey Stephen, Thanks for the review! On 2019-08-30 11:02, Stephen Boyd wrote: > Quoting Sibi Sankar (2019-08-24 08:24:10) >> Add SC7180 AOSS reset to the list of possible bindings. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt | 4 ++-- > > Can you convert this binding to YAML/JSON schema? Would help to > describe > the 'one of' requirement below in a more structured way. yeah converting them shouldn't take time but 'oneof' isn't the requirement here. We want to specify that sc7180 should continue to use the sdm845 compatible since the offset/ num of reset are identical. > >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> index 510c748656ec5..3eb6a22ced4bc 100644 >> --- a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> +++ b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt >> @@ -8,8 +8,8 @@ Required properties: >> - compatible: >> Usage: required >> Value type: <string> >> - Definition: must be: >> - "qcom,sdm845-aoss-cc" >> + Definition: must be one of:
diff --git a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt index 510c748656ec5..3eb6a22ced4bc 100644 --- a/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt +++ b/Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt @@ -8,8 +8,8 @@ Required properties: - compatible: Usage: required Value type: <string> - Definition: must be: - "qcom,sdm845-aoss-cc" + Definition: must be one of: + "qcom,sc7180-aoss-cc", "qcom,sdm845-aoss-cc" - reg: Usage: required
Add SC7180 AOSS reset to the list of possible bindings. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- Documentation/devicetree/bindings/reset/qcom,aoss-reset.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)