linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
@ 2023-10-15 16:20 Robert Marko
  2023-10-18 20:29 ` Konrad Dybcio
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Marko @ 2023-10-15 16:20 UTC (permalink / raw)
  To: agross, andersson, konrad.dybcio, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel
  Cc: Robert Marko

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 <robimarko@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-15 16:20 [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock Robert Marko
@ 2023-10-18 20:29 ` Konrad Dybcio
  2023-10-19  6:46   ` Kathiravan Thirumoorthy
  0 siblings, 1 reply; 8+ messages in thread
From: Konrad Dybcio @ 2023-10-18 20:29 UTC (permalink / raw)
  To: Robert Marko, agross, andersson, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel



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 <robimarko@gmail.com>
> ---
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-18 20:29 ` Konrad Dybcio
@ 2023-10-19  6:46   ` Kathiravan Thirumoorthy
  2023-10-19 14:23     ` Robert Marko
  0 siblings, 1 reply; 8+ messages in thread
From: Kathiravan Thirumoorthy @ 2023-10-19  6:46 UTC (permalink / raw)
  To: Konrad Dybcio, Robert Marko, agross, andersson, mturquette,
	sboyd, linux-arm-msm, linux-clk, linux-kernel


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 <robimarko@gmail.com>
>> ---
> 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 <quic_kathirav@quicinc.com>

[1] 
https://lore.kernel.org/linux-arm-msm/2852fc37-284f-6534-f163-45b37b153db1@quicinc.com/


> Konrad

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-19  6:46   ` Kathiravan Thirumoorthy
@ 2023-10-19 14:23     ` Robert Marko
  2023-10-20  6:40       ` Kathiravan Thirumoorthy
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Marko @ 2023-10-19 14:23 UTC (permalink / raw)
  To: Kathiravan Thirumoorthy
  Cc: Konrad Dybcio, agross, andersson, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel

On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
<quic_kathirav@quicinc.com> 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 <robimarko@gmail.com>
> >> ---
> > 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 <quic_kathirav@quicinc.com>

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/2852fc37-284f-6534-f163-45b37b153db1@quicinc.com/
>
>
> > Konrad

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-19 14:23     ` Robert Marko
@ 2023-10-20  6:40       ` Kathiravan Thirumoorthy
  2023-10-21 11:57         ` Robert Marko
  0 siblings, 1 reply; 8+ messages in thread
From: Kathiravan Thirumoorthy @ 2023-10-20  6:40 UTC (permalink / raw)
  To: Robert Marko
  Cc: Konrad Dybcio, agross, andersson, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel


On 10/19/2023 7:53 PM, Robert Marko wrote:
> On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
> <quic_kathirav@quicinc.com> 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 <robimarko@gmail.com>
>>>> ---
>>> 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 <quic_kathirav@quicinc.com>
> 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/2852fc37-284f-6534-f163-45b37b153db1@quicinc.com/
>>
>>
>>> Konrad

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-20  6:40       ` Kathiravan Thirumoorthy
@ 2023-10-21 11:57         ` Robert Marko
  2023-10-25  8:41           ` Kathiravan Thirumoorthy
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Marko @ 2023-10-21 11:57 UTC (permalink / raw)
  To: Kathiravan Thirumoorthy
  Cc: Konrad Dybcio, agross, andersson, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel

On Fri, 20 Oct 2023 at 08:40, Kathiravan Thirumoorthy
<quic_kathirav@quicinc.com> wrote:
>
>
> On 10/19/2023 7:53 PM, Robert Marko wrote:
> > On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
> > <quic_kathirav@quicinc.com> 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 <robimarko@gmail.com>
> >>>> ---
> >>> 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 <quic_kathirav@quicinc.com>
> > 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/2852fc37-284f-6534-f163-45b37b153db1@quicinc.com/
> >>
> >>
> >>> Konrad

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-21 11:57         ` Robert Marko
@ 2023-10-25  8:41           ` Kathiravan Thirumoorthy
  2023-10-25 10:42             ` Robert Marko
  0 siblings, 1 reply; 8+ messages in thread
From: Kathiravan Thirumoorthy @ 2023-10-25  8:41 UTC (permalink / raw)
  To: Robert Marko
  Cc: Konrad Dybcio, agross, andersson, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel



On 10/21/2023 5:27 PM, Robert Marko wrote:
> On Fri, 20 Oct 2023 at 08:40, Kathiravan Thirumoorthy
> <quic_kathirav@quicinc.com> wrote:
>>
>>
>> On 10/19/2023 7:53 PM, Robert Marko wrote:
>>> On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
>>> <quic_kathirav@quicinc.com> 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 <robimarko@gmail.com>
>>>>>> ---
>>>>> 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 <quic_kathirav@quicinc.com>
>>> 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,

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock
  2023-10-25  8:41           ` Kathiravan Thirumoorthy
@ 2023-10-25 10:42             ` Robert Marko
  0 siblings, 0 replies; 8+ messages in thread
From: Robert Marko @ 2023-10-25 10:42 UTC (permalink / raw)
  To: Kathiravan Thirumoorthy
  Cc: Konrad Dybcio, agross, andersson, mturquette, sboyd,
	linux-arm-msm, linux-clk, linux-kernel

On Wed, 25 Oct 2023 at 10:42, Kathiravan Thirumoorthy
<quic_kathirav@quicinc.com> wrote:
>
>
>
> On 10/21/2023 5:27 PM, Robert Marko wrote:
> > On Fri, 20 Oct 2023 at 08:40, Kathiravan Thirumoorthy
> > <quic_kathirav@quicinc.com> wrote:
> >>
> >>
> >> On 10/19/2023 7:53 PM, Robert Marko wrote:
> >>> On Thu, 19 Oct 2023 at 08:46, Kathiravan Thirumoorthy
> >>> <quic_kathirav@quicinc.com> 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 <robimarko@gmail.com>
> >>>>>> ---
> >>>>> 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 <quic_kathirav@quicinc.com>
> >>> 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,

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-10-25 10:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-15 16:20 [PATCH] clk: qcom: gcc-ipq6018: add QUP6 I2C clock Robert Marko
2023-10-18 20:29 ` Konrad Dybcio
2023-10-19  6:46   ` Kathiravan Thirumoorthy
2023-10-19 14:23     ` Robert Marko
2023-10-20  6:40       ` Kathiravan Thirumoorthy
2023-10-21 11:57         ` Robert Marko
2023-10-25  8:41           ` Kathiravan Thirumoorthy
2023-10-25 10:42             ` Robert Marko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).