| Message ID | cover.1693996662.git.quic_varada@quicinc.com |
|---|---|
| Headers | show |
| Series | Enable cpufreq for IPQ5332 & IPQ9574 | expand |
On 07/09/2023 07:21, Varadarajan Narayanan wrote: > Document IPQ5332 compatible for Qcom NVMEM CPUFreq driver. > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 7.09.2023 07:21, Varadarajan Narayanan wrote: > The earlier 'l' value of 0x3e is for 1.5GHz. Not all SKUs support > this frequency. Hence set it to 0x2d to get 1.1GHz which is > supported in all SKUs. > > Fixes: c7ef7fbb1ccf ("clk: qcom: apss-ipq-pll: add support for IPQ5332") > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan <quic_varada@quicinc.com> wrote: > > The earlier 'l' value of 0x3e is for 1.5GHz. Not all SKUs support > this frequency. Hence set it to 0x2d to get 1.1GHz which is > supported in all SKUs. > > Fixes: c7ef7fbb1ccf ("clk: qcom: apss-ipq-pll: add support for IPQ5332") > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan <quic_varada@quicinc.com> wrote: > > IPQ53xx have different OPPs available for the CPU based on > SoC variant. This can be determined through use of an eFuse > register present in the silicon. > > Add support to read the eFuse and populate the OPPs based on it. > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++--- > 1 file changed, 31 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > index 82761ae..3ca3f34 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > @@ -91,11 +91,34 @@ > }; > > cpu_opp_table: opp-table-cpu { > - compatible = "operating-points-v2"; > + compatible = "operating-points-v2-kryo-cpu"; > opp-shared; > + nvmem-cells = <&cpu_speed_bin>; > + nvmem-cell-names = "speed_bin"; > + > + /* > + * Listed all supported CPU frequencies and opp-supported-hw > + * values to select CPU frequencies based on the limits fused. > + * ------------------------------------------------------------ > + * Frequency BIT3 BIT2 BIT1 BIT0 opp-supported-hw > + * 1.0GHz 1.2GHz 1.5GHz No Limit > + * ------------------------------------------------------------ > + * 1100000000 1 1 1 1 0xF > + * 1500000000 0 0 1 1 0x3 > + * ----------------------------------------------------------- > + */ This can probably go to the commit message instead. > + > + opp-1100000000 { > + opp-hz = /bits/ 64 <1100000000>; But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz > + opp-microvolt = <850000>; > + opp-supported-hw = <0xF>; > + clock-latency-ns = <200000>; > + }; > > - opp-1488000000 { > - opp-hz = /bits/ 64 <1488000000>; > + opp-1500000000 { > + opp-hz = /bits/ 64 <1500000000>; So, 1.488 GHz or 1.5 GHz? > + opp-microvolt = <950000>; Which regulator is controlled by this microvolt? > + opp-supported-hw = <0x3>; > clock-latency-ns = <200000>; > }; > }; > @@ -150,6 +173,11 @@ > reg = <0x000a4000 0x721>; > #address-cells = <1>; > #size-cells = <1>; > + > + cpu_speed_bin: cpu_speed_bin@1d { > + reg = <0x1d 0x2>; > + bits = <7 2>; > + }; > }; > > rng: rng@e3000 { > -- > 2.7.4 >
On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote: > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan > <quic_varada@quicinc.com> wrote: > > > > IPQ53xx have different OPPs available for the CPU based on > > SoC variant. This can be determined through use of an eFuse > > register present in the silicon. > > > > Add support to read the eFuse and populate the OPPs based on it. > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > --- > > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++--- > > 1 file changed, 31 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > index 82761ae..3ca3f34 100644 > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > @@ -91,11 +91,34 @@ > > }; > > > > cpu_opp_table: opp-table-cpu { > > - compatible = "operating-points-v2"; > > + compatible = "operating-points-v2-kryo-cpu"; > > opp-shared; > > + nvmem-cells = <&cpu_speed_bin>; > > + nvmem-cell-names = "speed_bin"; > > + > > + /* > > + * Listed all supported CPU frequencies and opp-supported-hw > > + * values to select CPU frequencies based on the limits fused. > > + * ------------------------------------------------------------ > > + * Frequency BIT3 BIT2 BIT1 BIT0 opp-supported-hw > > + * 1.0GHz 1.2GHz 1.5GHz No Limit > > + * ------------------------------------------------------------ > > + * 1100000000 1 1 1 1 0xF > > + * 1500000000 0 0 1 1 0x3 > > + * ----------------------------------------------------------- > > + */ > > This can probably go to the commit message instead. Ok > > + > > + opp-1100000000 { > > + opp-hz = /bits/ 64 <1100000000>; > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz Will update it. > > + opp-microvolt = <850000>; > > + opp-supported-hw = <0xF>; > > + clock-latency-ns = <200000>; > > + }; > > > > - opp-1488000000 { > > - opp-hz = /bits/ 64 <1488000000>; > > + opp-1500000000 { > > + opp-hz = /bits/ 64 <1500000000>; > > So, 1.488 GHz or 1.5 GHz? 1.5 GHz > > + opp-microvolt = <950000>; > > Which regulator is controlled by this microvolt? Based on the SKU, the XBL sets up the regulator to provide 950000uV on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux doesn't control it. Thanks Varada > > + opp-supported-hw = <0x3>; > > clock-latency-ns = <200000>; > > }; > > }; > > @@ -150,6 +173,11 @@ > > reg = <0x000a4000 0x721>; > > #address-cells = <1>; > > #size-cells = <1>; > > + > > + cpu_speed_bin: cpu_speed_bin@1d { > > + reg = <0x1d 0x2>; > > + bits = <7 2>; > > + }; > > }; > > > > rng: rng@e3000 { > > -- > > 2.7.4 > > > > > -- > With best wishes > Dmitry
On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan <quic_varada@quicinc.com> wrote: > > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote: > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan > > <quic_varada@quicinc.com> wrote: > > > > > > IPQ53xx have different OPPs available for the CPU based on > > > SoC variant. This can be determined through use of an eFuse > > > register present in the silicon. > > > > > > Add support to read the eFuse and populate the OPPs based on it. > > > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > > --- > > > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++--- > > > 1 file changed, 31 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > index 82761ae..3ca3f34 100644 > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > @@ -91,11 +91,34 @@ > > > }; > > > > > > cpu_opp_table: opp-table-cpu { > > > - compatible = "operating-points-v2"; > > > + compatible = "operating-points-v2-kryo-cpu"; > > > opp-shared; > > > + nvmem-cells = <&cpu_speed_bin>; > > > + nvmem-cell-names = "speed_bin"; > > > + > > > + /* > > > + * Listed all supported CPU frequencies and opp-supported-hw > > > + * values to select CPU frequencies based on the limits fused. > > > + * ------------------------------------------------------------ > > > + * Frequency BIT3 BIT2 BIT1 BIT0 opp-supported-hw > > > + * 1.0GHz 1.2GHz 1.5GHz No Limit > > > + * ------------------------------------------------------------ > > > + * 1100000000 1 1 1 1 0xF > > > + * 1500000000 0 0 1 1 0x3 > > > + * ----------------------------------------------------------- > > > + */ > > > > This can probably go to the commit message instead. > > Ok > > > > + > > > + opp-1100000000 { > > > + opp-hz = /bits/ 64 <1100000000>; > > > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz > > Will update it. > > > > + opp-microvolt = <850000>; > > > + opp-supported-hw = <0xF>; > > > + clock-latency-ns = <200000>; > > > + }; > > > > > > - opp-1488000000 { > > > - opp-hz = /bits/ 64 <1488000000>; > > > + opp-1500000000 { > > > + opp-hz = /bits/ 64 <1500000000>; > > > > So, 1.488 GHz or 1.5 GHz? > > 1.5 GHz > > > > + opp-microvolt = <950000>; > > > > Which regulator is controlled by this microvolt? > > Based on the SKU, the XBL sets up the regulator to provide 950000uV > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux > doesn't control it. Then why do you need this property here in the first place?
On Thu, Oct 05, 2023 at 02:39:43PM +0300, Dmitry Baryshkov wrote: > On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan > <quic_varada@quicinc.com> wrote: > > > > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote: > > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan > > > <quic_varada@quicinc.com> wrote: > > > > > > > > IPQ53xx have different OPPs available for the CPU based on > > > > SoC variant. This can be determined through use of an eFuse > > > > register present in the silicon. > > > > > > > > Add support to read the eFuse and populate the OPPs based on it. > > > > > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > > > --- > > > > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++--- > > > > 1 file changed, 31 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > index 82761ae..3ca3f34 100644 > > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > @@ -91,11 +91,34 @@ > > > > }; > > > > > > > > cpu_opp_table: opp-table-cpu { > > > > - compatible = "operating-points-v2"; > > > > + compatible = "operating-points-v2-kryo-cpu"; > > > > opp-shared; > > > > + nvmem-cells = <&cpu_speed_bin>; > > > > + nvmem-cell-names = "speed_bin"; > > > > + > > > > + /* > > > > + * Listed all supported CPU frequencies and opp-supported-hw > > > > + * values to select CPU frequencies based on the limits fused. > > > > + * ------------------------------------------------------------ > > > > + * Frequency BIT3 BIT2 BIT1 BIT0 opp-supported-hw > > > > + * 1.0GHz 1.2GHz 1.5GHz No Limit > > > > + * ------------------------------------------------------------ > > > > + * 1100000000 1 1 1 1 0xF > > > > + * 1500000000 0 0 1 1 0x3 > > > > + * ----------------------------------------------------------- > > > > + */ > > > > > > This can probably go to the commit message instead. > > > > Ok > > > > > > + > > > > + opp-1100000000 { > > > > + opp-hz = /bits/ 64 <1100000000>; > > > > > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz > > > > Will update it. > > > > > > + opp-microvolt = <850000>; > > > > + opp-supported-hw = <0xF>; > > > > + clock-latency-ns = <200000>; > > > > + }; > > > > > > > > - opp-1488000000 { > > > > - opp-hz = /bits/ 64 <1488000000>; > > > > + opp-1500000000 { > > > > + opp-hz = /bits/ 64 <1500000000>; > > > > > > So, 1.488 GHz or 1.5 GHz? > > > > 1.5 GHz > > > > > > + opp-microvolt = <950000>; > > > > > > Which regulator is controlled by this microvolt? > > > > Based on the SKU, the XBL sets up the regulator to provide 950000uV > > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux > > doesn't control it. > > Then why do you need this property here in the first place? I get these errors without this property [ 1.018065] cpu cpu0: opp_parse_microvolt: opp-microvolt missing although OPP managing regulators [ 1.018074] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22 Thanks Varada
On Thu, 5 Oct 2023 at 17:42, Varadarajan Narayanan <quic_varada@quicinc.com> wrote: > > On Thu, Oct 05, 2023 at 02:39:43PM +0300, Dmitry Baryshkov wrote: > > On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan > > <quic_varada@quicinc.com> wrote: > > > > > > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote: > > > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan > > > > <quic_varada@quicinc.com> wrote: > > > > > > > > > > IPQ53xx have different OPPs available for the CPU based on > > > > > SoC variant. This can be determined through use of an eFuse > > > > > register present in the silicon. > > > > > > > > > > Add support to read the eFuse and populate the OPPs based on it. > > > > > > > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > > > > --- > > > > > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++--- > > > > > 1 file changed, 31 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > > index 82761ae..3ca3f34 100644 > > > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > > @@ -91,11 +91,34 @@ > > > > > }; > > > > > > > > > > cpu_opp_table: opp-table-cpu { > > > > > - compatible = "operating-points-v2"; > > > > > + compatible = "operating-points-v2-kryo-cpu"; > > > > > opp-shared; > > > > > + nvmem-cells = <&cpu_speed_bin>; > > > > > + nvmem-cell-names = "speed_bin"; > > > > > + > > > > > + /* > > > > > + * Listed all supported CPU frequencies and opp-supported-hw > > > > > + * values to select CPU frequencies based on the limits fused. > > > > > + * ------------------------------------------------------------ > > > > > + * Frequency BIT3 BIT2 BIT1 BIT0 opp-supported-hw > > > > > + * 1.0GHz 1.2GHz 1.5GHz No Limit > > > > > + * ------------------------------------------------------------ > > > > > + * 1100000000 1 1 1 1 0xF > > > > > + * 1500000000 0 0 1 1 0x3 > > > > > + * ----------------------------------------------------------- > > > > > + */ > > > > > > > > This can probably go to the commit message instead. > > > > > > Ok > > > > > > > > + > > > > > + opp-1100000000 { > > > > > + opp-hz = /bits/ 64 <1100000000>; > > > > > > > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz > > > > > > Will update it. > > > > > > > > + opp-microvolt = <850000>; > > > > > + opp-supported-hw = <0xF>; > > > > > + clock-latency-ns = <200000>; > > > > > + }; > > > > > > > > > > - opp-1488000000 { > > > > > - opp-hz = /bits/ 64 <1488000000>; > > > > > + opp-1500000000 { > > > > > + opp-hz = /bits/ 64 <1500000000>; > > > > > > > > So, 1.488 GHz or 1.5 GHz? > > > > > > 1.5 GHz > > > > > > > > + opp-microvolt = <950000>; > > > > > > > > Which regulator is controlled by this microvolt? > > > > > > Based on the SKU, the XBL sets up the regulator to provide 950000uV > > > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux > > > doesn't control it. > > > > Then why do you need this property here in the first place? > > I get these errors without this property > > [ 1.018065] cpu cpu0: opp_parse_microvolt: opp-microvolt missing although OPP managing regulators But you have said that "Linux doesn't control it" [the regulator]! > [ 1.018074] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22
On Thu, Oct 05, 2023 at 10:39:55PM +0300, Dmitry Baryshkov wrote: > On Thu, 5 Oct 2023 at 17:42, Varadarajan Narayanan > <quic_varada@quicinc.com> wrote: > > > > On Thu, Oct 05, 2023 at 02:39:43PM +0300, Dmitry Baryshkov wrote: > > > On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan > > > <quic_varada@quicinc.com> wrote: > > > > > > > > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote: > > > > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan > > > > > <quic_varada@quicinc.com> wrote: > > > > > > > > > > > > IPQ53xx have different OPPs available for the CPU based on > > > > > > SoC variant. This can be determined through use of an eFuse > > > > > > register present in the silicon. > > > > > > > > > > > > Add support to read the eFuse and populate the OPPs based on it. > > > > > > > > > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > > > > > --- > > > > > > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++--- > > > > > > 1 file changed, 31 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > > > index 82761ae..3ca3f34 100644 > > > > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > > > > > @@ -91,11 +91,34 @@ > > > > > > }; > > > > > > > > > > > > cpu_opp_table: opp-table-cpu { > > > > > > - compatible = "operating-points-v2"; > > > > > > + compatible = "operating-points-v2-kryo-cpu"; > > > > > > opp-shared; > > > > > > + nvmem-cells = <&cpu_speed_bin>; > > > > > > + nvmem-cell-names = "speed_bin"; > > > > > > + > > > > > > + /* > > > > > > + * Listed all supported CPU frequencies and opp-supported-hw > > > > > > + * values to select CPU frequencies based on the limits fused. > > > > > > + * ------------------------------------------------------------ > > > > > > + * Frequency BIT3 BIT2 BIT1 BIT0 opp-supported-hw > > > > > > + * 1.0GHz 1.2GHz 1.5GHz No Limit > > > > > > + * ------------------------------------------------------------ > > > > > > + * 1100000000 1 1 1 1 0xF > > > > > > + * 1500000000 0 0 1 1 0x3 > > > > > > + * ----------------------------------------------------------- > > > > > > + */ > > > > > > > > > > This can probably go to the commit message instead. > > > > > > > > Ok > > > > > > > > > > + > > > > > > + opp-1100000000 { > > > > > > + opp-hz = /bits/ 64 <1100000000>; > > > > > > > > > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz > > > > > > > > Will update it. > > > > > > > > > > + opp-microvolt = <850000>; > > > > > > + opp-supported-hw = <0xF>; > > > > > > + clock-latency-ns = <200000>; > > > > > > + }; > > > > > > > > > > > > - opp-1488000000 { > > > > > > - opp-hz = /bits/ 64 <1488000000>; > > > > > > + opp-1500000000 { > > > > > > + opp-hz = /bits/ 64 <1500000000>; > > > > > > > > > > So, 1.488 GHz or 1.5 GHz? > > > > > > > > 1.5 GHz > > > > > > > > > > + opp-microvolt = <950000>; > > > > > > > > > > Which regulator is controlled by this microvolt? > > > > > > > > Based on the SKU, the XBL sets up the regulator to provide 950000uV > > > > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux > > > > doesn't control it. > > > > > > Then why do you need this property here in the first place? > > > > I get these errors without this property > > > > [ 1.018065] cpu cpu0: opp_parse_microvolt: opp-microvolt missing although OPP managing regulators > > But you have said that "Linux doesn't control it" [the regulator]! Got confused between the ipq9574 and ipq5332 patches. Have removed and addressed other comments too and posted v2 - https://lore.kernel.org/linux-arm-msm/cover.1697101543.git.quic_varada@quicinc.com/ Please take a look. Thanks Varada > > > [ 1.018074] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22