| Message ID | 20211114012755.112226-4-konrad.dybcio@somainline.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [01/16] arm64: dts: qcom: sm8350: Move gpio.h inclusion to SoC DTSI | expand |
Quoting Konrad Dybcio (2021-11-13 17:27:43) > Arch timer runs at 19.2 MHz. Specify the rate in the timer node. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> > --- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi > index a30ba3193d84..60866a20a55c 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > @@ -2484,5 +2484,6 @@ timer { > <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; > + clock-frequency = <19200000>; Does the firmware not set the frequency properly?
On 30/11/2021 03:05, Stephen Boyd wrote: > Quoting Konrad Dybcio (2021-11-13 17:27:43) >> Arch timer runs at 19.2 MHz. Specify the rate in the timer node. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> >> --- >> arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi >> index a30ba3193d84..60866a20a55c 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi >> @@ -2484,5 +2484,6 @@ timer { >> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; >> + clock-frequency = <19200000>; > Does the firmware not set the frequency properly? It does on my device on the current firmware version (it wouldn't really boot if it didn't, no?), but who knows if it always will, or if it always has been.. It's present in downstream too, so I reckon it does not hurt to have it here too, even for completeness-of-describing-the-machine-properly sake. Konrad
Quoting Konrad Dybcio (2021-11-30 11:59:03) > > On 30/11/2021 03:05, Stephen Boyd wrote: > > Quoting Konrad Dybcio (2021-11-13 17:27:43) > >> Arch timer runs at 19.2 MHz. Specify the rate in the timer node. > >> > >> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> > >> --- > >> arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi > >> index a30ba3193d84..60866a20a55c 100644 > >> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > >> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > >> @@ -2484,5 +2484,6 @@ timer { > >> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > >> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > >> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; > >> + clock-frequency = <19200000>; > > Does the firmware not set the frequency properly? > > It does on my device on the current firmware version (it wouldn't really > boot if it didn't, no?), > > but who knows if it always will, or if it always has been.. > > > It's present in downstream too, so I reckon it does not hurt to have it > here too, even > > for completeness-of-describing-the-machine-properly sake. > No. We don't want dts files to have this. The property is only there to workaround bad firmware that doesn't set the frequency. Please drop this patch.
On 01.12.2021 21:45, Stephen Boyd wrote: > Quoting Konrad Dybcio (2021-11-30 11:59:03) >> On 30/11/2021 03:05, Stephen Boyd wrote: >>> Quoting Konrad Dybcio (2021-11-13 17:27:43) >>>> Arch timer runs at 19.2 MHz. Specify the rate in the timer node. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> >>>> --- >>>> arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi >>>> index a30ba3193d84..60866a20a55c 100644 >>>> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi >>>> @@ -2484,5 +2484,6 @@ timer { >>>> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >>>> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >>>> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; >>>> + clock-frequency = <19200000>; >>> Does the firmware not set the frequency properly? >> It does on my device on the current firmware version (it wouldn't really >> boot if it didn't, no?), >> >> but who knows if it always will, or if it always has been.. >> >> >> It's present in downstream too, so I reckon it does not hurt to have it >> here too, even >> >> for completeness-of-describing-the-machine-properly sake. >> > No. We don't want dts files to have this. The property is only there to > workaround bad firmware that doesn't set the frequency. Please drop this > patch. After looking at it again, I see I was indeed wrong, and so was this patch. Sorry, and green light for dropping.. Konrad
On Wed 01 Dec 16:00 PST 2021, Konrad Dybcio wrote: > > On 01.12.2021 21:45, Stephen Boyd wrote: > > Quoting Konrad Dybcio (2021-11-30 11:59:03) > >> On 30/11/2021 03:05, Stephen Boyd wrote: > >>> Quoting Konrad Dybcio (2021-11-13 17:27:43) > >>>> Arch timer runs at 19.2 MHz. Specify the rate in the timer node. > >>>> > >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> > >>>> --- > >>>> arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 + > >>>> 1 file changed, 1 insertion(+) > >>>> > >>>> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi > >>>> index a30ba3193d84..60866a20a55c 100644 > >>>> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi > >>>> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi > >>>> @@ -2484,5 +2484,6 @@ timer { > >>>> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > >>>> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > >>>> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; > >>>> + clock-frequency = <19200000>; > >>> Does the firmware not set the frequency properly? > >> It does on my device on the current firmware version (it wouldn't really > >> boot if it didn't, no?), > >> > >> but who knows if it always will, or if it always has been.. > >> > >> > >> It's present in downstream too, so I reckon it does not hurt to have it > >> here too, even > >> > >> for completeness-of-describing-the-machine-properly sake. > >> > > No. We don't want dts files to have this. The property is only there to > > workaround bad firmware that doesn't set the frequency. Please drop this > > patch. > > After looking at it again, I see I was indeed wrong, and so was this patch. > > Sorry, and green light for dropping.. > Can you please send me a patch that reverts the change as I merged it into my -next branch already? Both to simplify for me and to document why it shouldn't be here for others to refer to in the future. Thanks, Bjorn
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi index a30ba3193d84..60866a20a55c 100644 --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi @@ -2484,5 +2484,6 @@ timer { <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; + clock-frequency = <19200000>; }; };
Arch timer runs at 19.2 MHz. Specify the rate in the timer node. Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> --- arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 + 1 file changed, 1 insertion(+)