| Message ID | 20191023090219.15603-12-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add device tree support for sc7180 | expand |
Hi Rajendra/Maulik, On Wed, Oct 23, 2019 at 02:32:19PM +0530, Rajendra Nayak wrote: > From: Maulik Shah <mkshah@codeaurora.org> > > Add pdc interrupt controller for sc7180 > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > v3: > Used the qcom,sdm845-pdc compatible for pdc node > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index f2981ada578f..07ea393c2b5f 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -184,6 +184,16 @@ > #power-domain-cells = <1>; > }; > > + pdc: interrupt-controller@b220000 { Aren't the nodes supposed to be ordered by address as for SDM845? If so this node should be added after 'qupv3_id_1: geniqup@ac0000', not before.
Quoting Rajendra Nayak (2019-10-23 02:02:19) > From: Maulik Shah <mkshah@codeaurora.org> > > Add pdc interrupt controller for sc7180 > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- > v3: > Used the qcom,sdm845-pdc compatible for pdc node Everything else isn't doing the weird old compatible thing. Why not just add the new compatible and update the driver? I guess I'll have to go read the history.
On Tue, Oct 29, 2019 at 09:50:40AM -0700, Stephen Boyd wrote: > Quoting Rajendra Nayak (2019-10-23 02:02:19) > > From: Maulik Shah <mkshah@codeaurora.org> > > > > Add pdc interrupt controller for sc7180 > > > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > > --- > > v3: > > Used the qcom,sdm845-pdc compatible for pdc node > > Everything else isn't doing the weird old compatible thing. Why not just > add the new compatible and update the driver? I guess I'll have to go > read the history. Marc Zyngier complained on v2 about the churn from adding compatible strings for identical components, and I kinda see his point. I agree that using the 'sdm845' compatible string for sc7180 is odd too. Maybe we should introduce SoC independent compatible strings for IP blocks that are shared across multiple SoCs? If differentiation is needed SoC specific strings can be added.
On 10/26/2019 1:17 AM, Matthias Kaehlcke wrote: > Hi Rajendra/Maulik, > > On Wed, Oct 23, 2019 at 02:32:19PM +0530, Rajendra Nayak wrote: >> From: Maulik Shah <mkshah@codeaurora.org> >> >> Add pdc interrupt controller for sc7180 >> >> Signed-off-by: Maulik Shah <mkshah@codeaurora.org> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> --- >> v3: >> Used the qcom,sdm845-pdc compatible for pdc node >> >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> index f2981ada578f..07ea393c2b5f 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> @@ -184,6 +184,16 @@ >> #power-domain-cells = <1>; >> }; >> >> + pdc: interrupt-controller@b220000 { > > Aren't the nodes supposed to be ordered by address as for SDM845? > If so this node should be added after 'qupv3_id_1: geniqup@ac0000', > not before. yes, indeed. my sorting seems to have gone wrong. Will fix and repost. thanks >
On 10/31/2019 1:20 AM, Matthias Kaehlcke wrote: > On Tue, Oct 29, 2019 at 09:50:40AM -0700, Stephen Boyd wrote: >> Quoting Rajendra Nayak (2019-10-23 02:02:19) >>> From: Maulik Shah <mkshah@codeaurora.org> >>> >>> Add pdc interrupt controller for sc7180 >>> >>> Signed-off-by: Maulik Shah <mkshah@codeaurora.org> >>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >>> --- >>> v3: >>> Used the qcom,sdm845-pdc compatible for pdc node >> >> Everything else isn't doing the weird old compatible thing. Why not just >> add the new compatible and update the driver? I guess I'll have to go >> read the history. > > Marc Zyngier complained on v2 about the churn from adding compatible > strings for identical components, and I kinda see his point. > > I agree that using the 'sdm845' compatible string for sc7180 is odd too. > Maybe we should introduce SoC independent compatible strings for IP blocks > that are shared across multiple SoCs? If differentiation is needed SoC > specific strings can be added. Sure, I will perhaps add a qcom,pdc SoC independent compatible to avoid confusion.
On Sun 03 Nov 22:17 PST 2019, Rajendra Nayak wrote: > > > On 10/31/2019 1:20 AM, Matthias Kaehlcke wrote: > > On Tue, Oct 29, 2019 at 09:50:40AM -0700, Stephen Boyd wrote: > > > Quoting Rajendra Nayak (2019-10-23 02:02:19) > > > > From: Maulik Shah <mkshah@codeaurora.org> > > > > > > > > Add pdc interrupt controller for sc7180 > > > > > > > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > > > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > > > > --- > > > > v3: > > > > Used the qcom,sdm845-pdc compatible for pdc node > > > > > > Everything else isn't doing the weird old compatible thing. Why not just > > > add the new compatible and update the driver? I guess I'll have to go > > > read the history. > > > > Marc Zyngier complained on v2 about the churn from adding compatible > > strings for identical components, and I kinda see his point. > > > > I agree that using the 'sdm845' compatible string for sc7180 is odd too. > > Maybe we should introduce SoC independent compatible strings for IP blocks > > that are shared across multiple SoCs? If differentiation is needed SoC > > specific strings can be added. > > Sure, I will perhaps add a qcom,pdc SoC independent compatible to avoid > confusion. > I agree, compatible = "qcom,sc7180-pdc", "qcom,pdc"; is the way to go. Reusing qcom,sdm845-pdc would prevent us from tackling any unforeseen issues/variations/erratas with one or the other platform in the future. Regards, Bjorn > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation
On 11/4/2019 12:03 PM, Bjorn Andersson wrote: > On Sun 03 Nov 22:17 PST 2019, Rajendra Nayak wrote: > >> >> >> On 10/31/2019 1:20 AM, Matthias Kaehlcke wrote: >>> On Tue, Oct 29, 2019 at 09:50:40AM -0700, Stephen Boyd wrote: >>>> Quoting Rajendra Nayak (2019-10-23 02:02:19) >>>>> From: Maulik Shah <mkshah@codeaurora.org> >>>>> >>>>> Add pdc interrupt controller for sc7180 >>>>> >>>>> Signed-off-by: Maulik Shah <mkshah@codeaurora.org> >>>>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >>>>> --- >>>>> v3: >>>>> Used the qcom,sdm845-pdc compatible for pdc node >>>> >>>> Everything else isn't doing the weird old compatible thing. Why not just >>>> add the new compatible and update the driver? I guess I'll have to go >>>> read the history. >>> >>> Marc Zyngier complained on v2 about the churn from adding compatible >>> strings for identical components, and I kinda see his point. >>> >>> I agree that using the 'sdm845' compatible string for sc7180 is odd too. >>> Maybe we should introduce SoC independent compatible strings for IP blocks >>> that are shared across multiple SoCs? If differentiation is needed SoC >>> specific strings can be added. >> >> Sure, I will perhaps add a qcom,pdc SoC independent compatible to avoid >> confusion. >> > > I agree, > > compatible = "qcom,sc7180-pdc", "qcom,pdc"; > > is the way to go. I wasn't planning on adding a qcom,sc7180-pdc, but instead just use the qcom,pdc one for sc7180. > > Reusing qcom,sdm845-pdc would prevent us from tackling any unforeseen > issues/variations/erratas with one or the other platform in the future. That was the intention of adding qcom,sc7180-pdc in the first place, but Marc Zyngier was not happy with the churn, given there aren't really any variations or erratas that we know of. > > Regards, > Bjorn > >> >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member >> of Code Aurora Forum, hosted by The Linux Foundation
On Sun 03 Nov 22:56 PST 2019, Rajendra Nayak wrote: > > > On 11/4/2019 12:03 PM, Bjorn Andersson wrote: > > On Sun 03 Nov 22:17 PST 2019, Rajendra Nayak wrote: > > > > > > > > > > > On 10/31/2019 1:20 AM, Matthias Kaehlcke wrote: > > > > On Tue, Oct 29, 2019 at 09:50:40AM -0700, Stephen Boyd wrote: > > > > > Quoting Rajendra Nayak (2019-10-23 02:02:19) > > > > > > From: Maulik Shah <mkshah@codeaurora.org> > > > > > > > > > > > > Add pdc interrupt controller for sc7180 > > > > > > > > > > > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > > > > > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > > > > > > --- > > > > > > v3: > > > > > > Used the qcom,sdm845-pdc compatible for pdc node > > > > > > > > > > Everything else isn't doing the weird old compatible thing. Why not just > > > > > add the new compatible and update the driver? I guess I'll have to go > > > > > read the history. > > > > > > > > Marc Zyngier complained on v2 about the churn from adding compatible > > > > strings for identical components, and I kinda see his point. > > > > > > > > I agree that using the 'sdm845' compatible string for sc7180 is odd too. > > > > Maybe we should introduce SoC independent compatible strings for IP blocks > > > > that are shared across multiple SoCs? If differentiation is needed SoC > > > > specific strings can be added. > > > > > > Sure, I will perhaps add a qcom,pdc SoC independent compatible to avoid > > > confusion. > > > > > > > I agree, > > > > compatible = "qcom,sc7180-pdc", "qcom,pdc"; > > > > is the way to go. > > I wasn't planning on adding a qcom,sc7180-pdc, but instead just use the > qcom,pdc one for sc7180. > > > > > Reusing qcom,sdm845-pdc would prevent us from tackling any unforeseen > > issues/variations/erratas with one or the other platform in the future. > > That was the intention of adding qcom,sc7180-pdc in the first place, > but Marc Zyngier was not happy with the churn, given there aren't really > any variations or erratas that we know of. > Right, but by putting both compatibles in the dts and the generic one in the driver we avoid the driver churn and we're future compatible. And given that we haven't yet added the qcom,sdm845-pdc node to the sdm845.dtsi we don't need to maintain the qcom,sdm845-pdc in the driver. So switch qcom,sdm845-pdc to qcom,pdc in qcom-pdc.c. Regards, Bjorn > > > > Regards, > > Bjorn > > > > > > > > -- > > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > > > of Code Aurora Forum, hosted by The Linux Foundation > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation
Quoting Bjorn Andersson (2019-11-03 23:10:27) > > Right, but by putting both compatibles in the dts and the generic one in > the driver we avoid the driver churn and we're future compatible. > > And given that we haven't yet added the qcom,sdm845-pdc node to the > sdm845.dtsi we don't need to maintain the qcom,sdm845-pdc in the driver. > So switch qcom,sdm845-pdc to qcom,pdc in qcom-pdc.c. > I like this plan!
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index f2981ada578f..07ea393c2b5f 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -184,6 +184,16 @@ #power-domain-cells = <1>; }; + pdc: interrupt-controller@b220000 { + compatible = "qcom,sdm845-pdc"; + reg = <0 0xb220000 0 0x30000>; + qcom,pdc-ranges = <0 480 15>, <17 497 98>, + <119 634 4>, <124 639 1>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + qupv3_id_1: geniqup@ac0000 { compatible = "qcom,geni-se-qup"; reg = <0 0x00ac0000 0 0x6000>;