| Message ID | 20240225-gcc-ipq5018-register-fixes-v1-2-3c191404d9f0@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | clk: qcom: gcc-ipq5018: fix some register offsets | expand |
On Sun, 25 Feb 2024 at 19:33, Gabor Juhos <j4g8y7@gmail.com> wrote: > > The following table shows the values of the 'halt_reg' and the > 'enable_reg' fields from the pcie clocks defined in the current > driver: > > clock halt_reg enable_reg > > gcc_pcie0_ahb_clk 0x75010 0x75010 > gcc_pcie0_aux_clk 0x75014 0x75014 > gcc_pcie0_axi_m_clk 0x75008 0x75008 > gcc_pcie0_axi_s_bridge_clk 0x75048 0x75048 > gcc_pcie0_axi_s_clk 0x7500c 0x7500c > gcc_pcie0_pipe_clk 0x75018 0x75018 > > gcc_pcie1_ahb_clk 0x76010 0x76010 > gcc_pcie1_aux_clk 0x76014 0x76014 > gcc_pcie1_axi_m_clk 0x76008 0x76008 > gcc_pcie1_axi_s_bridge_clk 0x76048 0x76048 > gcc_pcie1_axi_s_clk 0x7600c 0x7600c > gcc_pcie1_pipe_clk 8* 0x76018 > > Based on the table, it is quite likely that the pcie0 and the pci1 > clocks are using the same register layout, however it seems that > the value of the 'halt_reg' field in the 'gcc_pcie1_pipe_clk' clock > is wrong. > > In the downstream driver [1], the same '0x76018' value is used for > both the 'halt_reg' and for the 'enable_reg' fields of the > 'gcc_pcie1_pipe_clk' clock. > > Update the current driver to use the same value used downstream as > probably that is the correct value. > > 1. https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/blob/NHSS.QSDK.12.4.r4/drivers/clk/qcom/gcc-ipq5018.c?ref_type=heads#L2316 > > Fixes: e3fdbef1bab8 ("clk: qcom: Add Global Clock controller (GCC) driver for IPQ5018") > Signed-off-by: Gabor Juhos <j4g8y7@gmail.com> > --- > drivers/clk/qcom/gcc-ipq5018.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 2/25/2024 11:02 PM, Gabor Juhos wrote: > The following table shows the values of the 'halt_reg' and the > 'enable_reg' fields from the pcie clocks defined in the current > driver: > > clock halt_reg enable_reg > > gcc_pcie0_ahb_clk 0x75010 0x75010 > gcc_pcie0_aux_clk 0x75014 0x75014 > gcc_pcie0_axi_m_clk 0x75008 0x75008 > gcc_pcie0_axi_s_bridge_clk 0x75048 0x75048 > gcc_pcie0_axi_s_clk 0x7500c 0x7500c > gcc_pcie0_pipe_clk 0x75018 0x75018 > > gcc_pcie1_ahb_clk 0x76010 0x76010 > gcc_pcie1_aux_clk 0x76014 0x76014 > gcc_pcie1_axi_m_clk 0x76008 0x76008 > gcc_pcie1_axi_s_bridge_clk 0x76048 0x76048 > gcc_pcie1_axi_s_clk 0x7600c 0x7600c > gcc_pcie1_pipe_clk 8* 0x76018 > > Based on the table, it is quite likely that the pcie0 and the pci1 > clocks are using the same register layout, however it seems that > the value of the 'halt_reg' field in the 'gcc_pcie1_pipe_clk' clock > is wrong. > > In the downstream driver [1], the same '0x76018' value is used for > both the 'halt_reg' and for the 'enable_reg' fields of the > 'gcc_pcie1_pipe_clk' clock. > > Update the current driver to use the same value used downstream as > probably that is the correct value. > > 1. https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/blob/NHSS.QSDK.12.4.r4/drivers/clk/qcom/gcc-ipq5018.c?ref_type=heads#L2316 Reviewed-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> > > Fixes: e3fdbef1bab8 ("clk: qcom: Add Global Clock controller (GCC) driver for IPQ5018") > Signed-off-by: Gabor Juhos <j4g8y7@gmail.com> > --- > drivers/clk/qcom/gcc-ipq5018.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/qcom/gcc-ipq5018.c b/drivers/clk/qcom/gcc-ipq5018.c > index cef9a1e7c9fdb..5e81cfa77293a 100644 > --- a/drivers/clk/qcom/gcc-ipq5018.c > +++ b/drivers/clk/qcom/gcc-ipq5018.c > @@ -2180,7 +2180,7 @@ static struct clk_branch gcc_pcie1_axi_s_clk = { > }; > > static struct clk_branch gcc_pcie1_pipe_clk = { > - .halt_reg = 8, > + .halt_reg = 0x76018, > .halt_check = BRANCH_HALT_DELAY, > .halt_bit = 31, > .clkr = { >
diff --git a/drivers/clk/qcom/gcc-ipq5018.c b/drivers/clk/qcom/gcc-ipq5018.c index cef9a1e7c9fdb..5e81cfa77293a 100644 --- a/drivers/clk/qcom/gcc-ipq5018.c +++ b/drivers/clk/qcom/gcc-ipq5018.c @@ -2180,7 +2180,7 @@ static struct clk_branch gcc_pcie1_axi_s_clk = { }; static struct clk_branch gcc_pcie1_pipe_clk = { - .halt_reg = 8, + .halt_reg = 0x76018, .halt_check = BRANCH_HALT_DELAY, .halt_bit = 31, .clkr = {
The following table shows the values of the 'halt_reg' and the 'enable_reg' fields from the pcie clocks defined in the current driver: clock halt_reg enable_reg gcc_pcie0_ahb_clk 0x75010 0x75010 gcc_pcie0_aux_clk 0x75014 0x75014 gcc_pcie0_axi_m_clk 0x75008 0x75008 gcc_pcie0_axi_s_bridge_clk 0x75048 0x75048 gcc_pcie0_axi_s_clk 0x7500c 0x7500c gcc_pcie0_pipe_clk 0x75018 0x75018 gcc_pcie1_ahb_clk 0x76010 0x76010 gcc_pcie1_aux_clk 0x76014 0x76014 gcc_pcie1_axi_m_clk 0x76008 0x76008 gcc_pcie1_axi_s_bridge_clk 0x76048 0x76048 gcc_pcie1_axi_s_clk 0x7600c 0x7600c gcc_pcie1_pipe_clk 8* 0x76018 Based on the table, it is quite likely that the pcie0 and the pci1 clocks are using the same register layout, however it seems that the value of the 'halt_reg' field in the 'gcc_pcie1_pipe_clk' clock is wrong. In the downstream driver [1], the same '0x76018' value is used for both the 'halt_reg' and for the 'enable_reg' fields of the 'gcc_pcie1_pipe_clk' clock. Update the current driver to use the same value used downstream as probably that is the correct value. 1. https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/blob/NHSS.QSDK.12.4.r4/drivers/clk/qcom/gcc-ipq5018.c?ref_type=heads#L2316 Fixes: e3fdbef1bab8 ("clk: qcom: Add Global Clock controller (GCC) driver for IPQ5018") Signed-off-by: Gabor Juhos <j4g8y7@gmail.com> --- drivers/clk/qcom/gcc-ipq5018.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)