Message ID | 20240225-gcc-ipq5018-register-fixes-v1-0-3c191404d9f0@gmail.com |
---|---|
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 value of the 'enable_reg' field in the 'gcc_gmac0_sys_clk' > clock definition seems wrong as it is greater than the > 'max_register' value defined in the regmap configuration. > Additionally, all other gmac specific branch clock definitions > within the driver uses the same value both for the 'enable_reg' > and for the 'halt_reg' fields. > > Due to the lack of documentation the correct value is not known. > Looking into the downstream driver does not help either, as that > uses the same (presumably wrong) value [1]. > > Nevertheless, change the 'enable_reg' field of 'gcc_gmac0_sys_clk' > to use the value from the 'halt_reg' field so it follows the pattern > used in other gmac clock definitions. The change is based on the > assumption that the register layout of this clock is the same > as the other gmac clocks. > > 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#L1889 > > 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> -- With best wishes Dmitry
On Sun, 25 Feb 2024 at 19:34, Gabor Juhos <j4g8y7@gmail.com> wrote: > > The current register offset used for the GCC_UBI0_AXI_ARES reset > seems wrong. Or at least, the downstream driver uses [1] the same > offset which is used for other the GCC_UBI0_*_ARES resets. > > Change the code to use the same offset used in the downstream > driver and also specify the reset bit explicitly to use the > same format as the followup entries. > > 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#L3773 > > 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 value of the 'enable_reg' field in the 'gcc_gmac0_sys_clk' > clock definition seems wrong as it is greater than the > 'max_register' value defined in the regmap configuration. > Additionally, all other gmac specific branch clock definitions > within the driver uses the same value both for the 'enable_reg' > and for the 'halt_reg' fields. > > Due to the lack of documentation the correct value is not known. > Looking into the downstream driver does not help either, as that > uses the same (presumably wrong) value [1]. > > Nevertheless, change the 'enable_reg' field of 'gcc_gmac0_sys_clk' > to use the value from the 'halt_reg' field so it follows the pattern > used in other gmac clock definitions. The change is based on the > assumption that the register layout of this clock is the same > as the other gmac clocks. > > 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#L1889 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 4aba47e8700d2..cef9a1e7c9fdb 100644 > --- a/drivers/clk/qcom/gcc-ipq5018.c > +++ b/drivers/clk/qcom/gcc-ipq5018.c > @@ -1754,7 +1754,7 @@ static struct clk_branch gcc_gmac0_sys_clk = { > .halt_check = BRANCH_HALT_DELAY, > .halt_bit = 31, > .clkr = { > - .enable_reg = 0x683190, > + .enable_reg = 0x68190, > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data) { > .name = "gcc_gmac0_sys_clk", >
On 2/25/2024 11:02 PM, Gabor Juhos wrote: > The current register offset used for the GCC_UBI0_AXI_ARES reset > seems wrong. Or at least, the downstream driver uses [1] the same > offset which is used for other the GCC_UBI0_*_ARES resets. > > Change the code to use the same offset used in the downstream > driver and also specify the reset bit explicitly to use the > same format as the followup entries. > > 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#L3773 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 5e81cfa77293a..e2bd54826a4ce 100644 > --- a/drivers/clk/qcom/gcc-ipq5018.c > +++ b/drivers/clk/qcom/gcc-ipq5018.c > @@ -3632,7 +3632,7 @@ static const struct qcom_reset_map gcc_ipq5018_resets[] = { > [GCC_SYSTEM_NOC_BCR] = { 0x26000, 0 }, > [GCC_TCSR_BCR] = { 0x28000, 0 }, > [GCC_TLMM_BCR] = { 0x34000, 0 }, > - [GCC_UBI0_AXI_ARES] = { 0x680}, > + [GCC_UBI0_AXI_ARES] = { 0x68010, 0 }, > [GCC_UBI0_AHB_ARES] = { 0x68010, 1 }, > [GCC_UBI0_NC_AXI_ARES] = { 0x68010, 2 }, > [GCC_UBI0_DBG_ARES] = { 0x68010, 3 }, >
2024. 02. 25. 22:00 keltezéssel, Dmitry Baryshkov írta: > On Sun, 25 Feb 2024 at 19:33, Gabor Juhos <j4g8y7@gmail.com> wrote: >> >> The value of the 'enable_reg' field in the 'gcc_gmac0_sys_clk' >> clock definition seems wrong as it is greater than the >> 'max_register' value defined in the regmap configuration. >> Additionally, all other gmac specific branch clock definitions >> within the driver uses the same value both for the 'enable_reg' >> and for the 'halt_reg' fields. >> >> Due to the lack of documentation the correct value is not known. >> Looking into the downstream driver does not help either, as that >> uses the same (presumably wrong) value [1]. >> >> Nevertheless, change the 'enable_reg' field of 'gcc_gmac0_sys_clk' >> to use the value from the 'halt_reg' field so it follows the pattern >> used in other gmac clock definitions. The change is based on the >> assumption that the register layout of this clock is the same >> as the other gmac clocks. >> >> 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#L1889 >> >> 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> Thank you for the review! Regards, Gabor
2024. 02. 26. 10:53 keltezéssel, Kathiravan Thirumoorthy írta: > > > On 2/25/2024 11:02 PM, Gabor Juhos wrote: >> The value of the 'enable_reg' field in the 'gcc_gmac0_sys_clk' >> clock definition seems wrong as it is greater than the >> 'max_register' value defined in the regmap configuration. >> Additionally, all other gmac specific branch clock definitions >> within the driver uses the same value both for the 'enable_reg' >> and for the 'halt_reg' fields. >> >> Due to the lack of documentation the correct value is not known. >> Looking into the downstream driver does not help either, as that >> uses the same (presumably wrong) value [1]. >> >> Nevertheless, change the 'enable_reg' field of 'gcc_gmac0_sys_clk' >> to use the value from the 'halt_reg' field so it follows the pattern >> used in other gmac clock definitions. The change is based on the >> assumption that the register layout of this clock is the same >> as the other gmac clocks. >> >> 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#L1889 > > > Reviewed-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> Thank you for the review! Regards, Gabor
On Sun, 25 Feb 2024 18:32:53 +0100, Gabor Juhos wrote: > The purpose of this small series is to fix some, presumably wrong > register offsets in the 'gcc-ipq5018' driver. > > The patches are based on v6.8-rc5. > > Applied, thanks! [1/3] clk: qcom: gcc-ipq5018: fix 'enable_reg' offset of 'gcc_gmac0_sys_clk' commit: f982adcc1b1c02a3114f68ac73c811cbfabe90fa [2/3] clk: qcom: gcc-ipq5018: fix 'halt_reg' offset of 'gcc_pcie1_pipe_clk' commit: 11b752ac5a07cbfd95592fac5237a02f45662926 [3/3] clk: qcom: gcc-ipq5018: fix register offset for GCC_UBI0_AXI_ARES reset commit: 7d474b43087aa356d714d39870c90d77fc6f1186 Best regards,
The purpose of this small series is to fix some, presumably wrong register offsets in the 'gcc-ipq5018' driver. The patches are based on v6.8-rc5. Signed-off-by: Gabor Juhos <j4g8y7@gmail.com> --- Gabor Juhos (3): clk: qcom: gcc-ipq5018: fix 'enable_reg' offset of 'gcc_gmac0_sys_clk' clk: qcom: gcc-ipq5018: fix 'halt_reg' offset of 'gcc_pcie1_pipe_clk' clk: qcom: gcc-ipq5018: fix register offset for GCC_UBI0_AXI_ARES reset drivers/clk/qcom/gcc-ipq5018.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d change-id: 20240224-gcc-ipq5018-register-fixes-394905520fda Best regards,