QUP6 I2C clock is listed in the dt bindings but it was never included in
the GCC driver.
So lets add support for it, its intentionally marked to never be disabled
as its somehow affecting DVFS and if disabled it sometimes crashes the
board.
Signed-off-by: Robert Marko <[email protected]>
---
drivers/clk/qcom/gcc-ipq6018.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/clk/qcom/gcc-ipq6018.c b/drivers/clk/qcom/gcc-ipq6018.c
index 31eef38c5864c..126d87df80c5b 100644
--- a/drivers/clk/qcom/gcc-ipq6018.c
+++ b/drivers/clk/qcom/gcc-ipq6018.c
@@ -2125,6 +2125,22 @@ static struct clk_branch gcc_blsp1_qup5_spi_apps_clk = {
},
};
+static struct clk_branch gcc_blsp1_qup6_i2c_apps_clk = {
+ .halt_reg = 0x07010,
+ .clkr = {
+ .enable_reg = 0x07010,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gcc_blsp1_qup6_i2c_apps_clk",
+ .parent_hws = (const struct clk_hw *[]){
+ &blsp1_qup6_i2c_apps_clk_src.clkr.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
static struct clk_branch gcc_blsp1_qup6_spi_apps_clk = {
.halt_reg = 0x0700c,
.clkr = {
@@ -4281,6 +4297,7 @@ static struct clk_regmap *gcc_ipq6018_clks[] = {
[GCC_BLSP1_QUP4_SPI_APPS_CLK] = &gcc_blsp1_qup4_spi_apps_clk.clkr,
[GCC_BLSP1_QUP5_I2C_APPS_CLK] = &gcc_blsp1_qup5_i2c_apps_clk.clkr,
[GCC_BLSP1_QUP5_SPI_APPS_CLK] = &gcc_blsp1_qup5_spi_apps_clk.clkr,
+ [GCC_BLSP1_QUP6_I2C_APPS_CLK] = &gcc_blsp1_qup6_i2c_apps_clk.clkr,
[GCC_BLSP1_QUP6_SPI_APPS_CLK] = &gcc_blsp1_qup6_spi_apps_clk.clkr,
[GCC_BLSP1_UART1_APPS_CLK] = &gcc_blsp1_uart1_apps_clk.clkr,
[GCC_BLSP1_UART2_APPS_CLK] = &gcc_blsp1_uart2_apps_clk.clkr,
--
2.41.0
On 10/15/23 18:20, Robert Marko wrote:
> QUP6 I2C clock is listed in the dt bindings but it was never included in
> the GCC driver.
> So lets add support for it, its intentionally marked to never be disabled
> as its somehow affecting DVFS and if disabled it sometimes crashes the
> board.
>
> Signed-off-by: Robert Marko <[email protected]>
> ---
Bjorn, would you be able to get an idea of what could be sitting
on that bus?
Or maybe the IPQ folks could know?
Konrad
On 10/19/2023 1:59 AM, Konrad Dybcio wrote:
>
>
> On 10/15/23 18:20, Robert Marko wrote:
>> QUP6 I2C clock is listed in the dt bindings but it was never included in
>> the GCC driver.
>> So lets add support for it, its intentionally marked to never be
>> disabled
>> as its somehow affecting DVFS and if disabled it sometimes crashes the
>> board.
>>
>> Signed-off-by: Robert Marko <[email protected]>
>> ---
> Bjorn, would you be able to get an idea of what could be sitting
> on that bus?
>
> Or maybe the IPQ folks could know?
>
Konrad / Robert,
Similar to IPQ9574, RPM needs this clock to communicate with PMIC over
I2C interface. Discussion happened here[1] is pretty much applicable to
IPQ6018 as well. Based on previous experience, we may need to document
the reason for CLK_IGNORE_UNUSED in driver as well. Nevertheless,
Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
[1]
https://lore.kernel.org/linux-arm-msm/[email protected]/
> Konrad
On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
<[email protected]> wrote:
>
>
> On 10/19/2023 1:59 AM, Konrad Dybcio wrote:
> >
> >
> > On 10/15/23 18:20, Robert Marko wrote:
> >> QUP6 I2C clock is listed in the dt bindings but it was never included in
> >> the GCC driver.
> >> So lets add support for it, its intentionally marked to never be
> >> disabled
> >> as its somehow affecting DVFS and if disabled it sometimes crashes the
> >> board.
> >>
> >> Signed-off-by: Robert Marko <[email protected]>
> >> ---
> > Bjorn, would you be able to get an idea of what could be sitting
> > on that bus?
> >
> > Or maybe the IPQ folks could know?
> >
>
> Konrad / Robert,
>
> Similar to IPQ9574, RPM needs this clock to communicate with PMIC over
> I2C interface. Discussion happened here[1] is pretty much applicable to
> IPQ6018 as well. Based on previous experience, we may need to document
> the reason for CLK_IGNORE_UNUSED in driver as well. Nevertheless,
>
> Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
Thanks for the explanation, it makes sense as I used to control the
MP5496 directly
via I2C.
Is it possible to disable RPM so it doesn't conflict and just use the
full MP5496 driver instead?
I find the RPM quite limiting as there is no way to actually retrieve
the current-voltage.
Regards,
Robert
>
> [1]
> https://lore.kernel.org/linux-arm-msm/[email protected]/
>
>
> > Konrad
On 10/19/2023 7:53 PM, Robert Marko wrote:
> On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
> <[email protected]> wrote:
>>
>> On 10/19/2023 1:59 AM, Konrad Dybcio wrote:
>>>
>>> On 10/15/23 18:20, Robert Marko wrote:
>>>> QUP6 I2C clock is listed in the dt bindings but it was never included in
>>>> the GCC driver.
>>>> So lets add support for it, its intentionally marked to never be
>>>> disabled
>>>> as its somehow affecting DVFS and if disabled it sometimes crashes the
>>>> board.
>>>>
>>>> Signed-off-by: Robert Marko <[email protected]>
>>>> ---
>>> Bjorn, would you be able to get an idea of what could be sitting
>>> on that bus?
>>>
>>> Or maybe the IPQ folks could know?
>>>
>> Konrad / Robert,
>>
>> Similar to IPQ9574, RPM needs this clock to communicate with PMIC over
>> I2C interface. Discussion happened here[1] is pretty much applicable to
>> IPQ6018 as well. Based on previous experience, we may need to document
>> the reason for CLK_IGNORE_UNUSED in driver as well. Nevertheless,
>>
>> Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
> Thanks for the explanation, it makes sense as I used to control the
> MP5496 directly
> via I2C.
>
> Is it possible to disable RPM so it doesn't conflict and just use the
> full MP5496 driver instead?
> I find the RPM quite limiting as there is no way to actually retrieve
> the current-voltage.
Robert, checked with the team and currently we don't have any option to
disable the RPM as such.
>
> Regards,
> Robert
>> [1]
>> https://lore.kernel.org/linux-arm-msm/[email protected]/
>>
>>
>>> Konrad
On Fri, 20 Oct 2023 at 08:40, Kathiravan Thirumoorthy
<[email protected]> wrote:
>
>
> On 10/19/2023 7:53 PM, Robert Marko wrote:
> > On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
> > <[email protected]> wrote:
> >>
> >> On 10/19/2023 1:59 AM, Konrad Dybcio wrote:
> >>>
> >>> On 10/15/23 18:20, Robert Marko wrote:
> >>>> QUP6 I2C clock is listed in the dt bindings but it was never included in
> >>>> the GCC driver.
> >>>> So lets add support for it, its intentionally marked to never be
> >>>> disabled
> >>>> as its somehow affecting DVFS and if disabled it sometimes crashes the
> >>>> board.
> >>>>
> >>>> Signed-off-by: Robert Marko <[email protected]>
> >>>> ---
> >>> Bjorn, would you be able to get an idea of what could be sitting
> >>> on that bus?
> >>>
> >>> Or maybe the IPQ folks could know?
> >>>
> >> Konrad / Robert,
> >>
> >> Similar to IPQ9574, RPM needs this clock to communicate with PMIC over
> >> I2C interface. Discussion happened here[1] is pretty much applicable to
> >> IPQ6018 as well. Based on previous experience, we may need to document
> >> the reason for CLK_IGNORE_UNUSED in driver as well. Nevertheless,
> >>
> >> Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
> > Thanks for the explanation, it makes sense as I used to control the
> > MP5496 directly
> > via I2C.
> >
> > Is it possible to disable RPM so it doesn't conflict and just use the
> > full MP5496 driver instead?
> > I find the RPM quite limiting as there is no way to actually retrieve
> > the current-voltage.
>
>
> Robert, checked with the team and currently we don't have any option to
> disable the RPM as such.
Thanks for checking.
BTW, is there any information you could share on the USB GDSC-s?
I tried converting them to GDSC-s like IPQ8074 has, but it seems that
they have different features.
It would be great to use proper GSDC support instead of manually
setting bits during GCC probe.
Regards,
Robert
>
>
> >
> > Regards,
> > Robert
> >> [1]
> >> https://lore.kernel.org/linux-arm-msm/[email protected]/
> >>
> >>
> >>> Konrad
On 10/21/2023 5:27 PM, Robert Marko wrote:
> On Fri, 20 Oct 2023 at 08:40, Kathiravan Thirumoorthy
> <[email protected]> wrote:
>>
>>
>> On 10/19/2023 7:53 PM, Robert Marko wrote:
>>> On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
>>> <[email protected]> wrote:
>>>>
>>>> On 10/19/2023 1:59 AM, Konrad Dybcio wrote:
>>>>>
>>>>> On 10/15/23 18:20, Robert Marko wrote:
>>>>>> QUP6 I2C clock is listed in the dt bindings but it was never included in
>>>>>> the GCC driver.
>>>>>> So lets add support for it, its intentionally marked to never be
>>>>>> disabled
>>>>>> as its somehow affecting DVFS and if disabled it sometimes crashes the
>>>>>> board.
>>>>>>
>>>>>> Signed-off-by: Robert Marko <[email protected]>
>>>>>> ---
>>>>> Bjorn, would you be able to get an idea of what could be sitting
>>>>> on that bus?
>>>>>
>>>>> Or maybe the IPQ folks could know?
>>>>>
>>>> Konrad / Robert,
>>>>
>>>> Similar to IPQ9574, RPM needs this clock to communicate with PMIC over
>>>> I2C interface. Discussion happened here[1] is pretty much applicable to
>>>> IPQ6018 as well. Based on previous experience, we may need to document
>>>> the reason for CLK_IGNORE_UNUSED in driver as well. Nevertheless,
>>>>
>>>> Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
>>> Thanks for the explanation, it makes sense as I used to control the
>>> MP5496 directly
>>> via I2C.
>>>
>>> Is it possible to disable RPM so it doesn't conflict and just use the
>>> full MP5496 driver instead?
>>> I find the RPM quite limiting as there is no way to actually retrieve
>>> the current-voltage.
>>
>>
>> Robert, checked with the team and currently we don't have any option to
>> disable the RPM as such.
>
> Thanks for checking.
>
> BTW, is there any information you could share on the USB GDSC-s?
> I tried converting them to GDSC-s like IPQ8074 has, but it seems that
> they have different features.
> It would be great to use proper GSDC support instead of manually
> setting bits during GCC probe.
Could you please explain the issue which you are facing? Based on quick
look at the HW documentation, seems there is no difference between
IPQ8074 and IPQ6018 GDSC-s.
Thanks,
On Wed, 25 Oct 2023 at 10:42, Kathiravan Thirumoorthy
<[email protected]> wrote:
>
>
>
> On 10/21/2023 5:27 PM, Robert Marko wrote:
> > On Fri, 20 Oct 2023 at 08:40, Kathiravan Thirumoorthy
> > <[email protected]> wrote:
> >>
> >>
> >> On 10/19/2023 7:53 PM, Robert Marko wrote:
> >>> On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
> >>> <[email protected]> wrote:
> >>>>
> >>>> On 10/19/2023 1:59 AM, Konrad Dybcio wrote:
> >>>>>
> >>>>> On 10/15/23 18:20, Robert Marko wrote:
> >>>>>> QUP6 I2C clock is listed in the dt bindings but it was never included in
> >>>>>> the GCC driver.
> >>>>>> So lets add support for it, its intentionally marked to never be
> >>>>>> disabled
> >>>>>> as its somehow affecting DVFS and if disabled it sometimes crashes the
> >>>>>> board.
> >>>>>>
> >>>>>> Signed-off-by: Robert Marko <[email protected]>
> >>>>>> ---
> >>>>> Bjorn, would you be able to get an idea of what could be sitting
> >>>>> on that bus?
> >>>>>
> >>>>> Or maybe the IPQ folks could know?
> >>>>>
> >>>> Konrad / Robert,
> >>>>
> >>>> Similar to IPQ9574, RPM needs this clock to communicate with PMIC over
> >>>> I2C interface. Discussion happened here[1] is pretty much applicable to
> >>>> IPQ6018 as well. Based on previous experience, we may need to document
> >>>> the reason for CLK_IGNORE_UNUSED in driver as well. Nevertheless,
> >>>>
> >>>> Reviewed-by: Kathiravan Thirumoorthy <[email protected]>
> >>> Thanks for the explanation, it makes sense as I used to control the
> >>> MP5496 directly
> >>> via I2C.
> >>>
> >>> Is it possible to disable RPM so it doesn't conflict and just use the
> >>> full MP5496 driver instead?
> >>> I find the RPM quite limiting as there is no way to actually retrieve
> >>> the current-voltage.
> >>
> >>
> >> Robert, checked with the team and currently we don't have any option to
> >> disable the RPM as such.
> >
> > Thanks for checking.
> >
> > BTW, is there any information you could share on the USB GDSC-s?
> > I tried converting them to GDSC-s like IPQ8074 has, but it seems that
> > they have different features.
> > It would be great to use proper GSDC support instead of manually
> > setting bits during GCC probe.
>
>
> Could you please explain the issue which you are facing? Based on quick
> look at the HW documentation, seems there is no difference between
> IPQ8074 and IPQ6018 GDSC-s.
Thanks for checking, it seems I hit a buggy kernel version last time I tried to
convert the driver to use proper GDSC-s and now it seems to work fine,
I will send patches to add GDSC-s.
Regards,
Robert
>
> Thanks,