[3/8] clk/qcom: move ipq4019 driver from mach-ipq40xx

This driver is just a stub, but it's necessary to support the upcoming
reset driver changes.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 arch/arm/Kconfig                                   |  1 +
 arch/arm/mach-ipq40xx/Makefile                     |  1 -
 drivers/clk/qcom/Kconfig                           |  8 +++++
 drivers/clk/qcom/Makefile                          |  1 +
 .../clk/qcom}/clock-ipq4019.c                      | 41 ++--------------------
 5 files changed, 12 insertions(+), 40 deletions(-)

On Wed, 25 Oct 2023 at 01:54, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> This driver is just a stub, but it's necessary to support the upcoming
> reset driver changes.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  arch/arm/Kconfig                                   |  1 +
>  arch/arm/mach-ipq40xx/Makefile                     |  1 -
>  drivers/clk/qcom/Kconfig                           |  8 +++++
>  drivers/clk/qcom/Makefile                          |  1 +
>  .../clk/qcom}/clock-ipq4019.c                      | 41 ++--------------------
>  5 files changed, 12 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 531b081de996..5aaf7e5e32af 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -766,6 +766,7 @@ config ARCH_IPQ40XX
>         select CLK
>         select SMEM
>         select OF_CONTROL
> +       select CLK_QCOM_IPQ4019
>         imply CMD_DM
>
>  config ARCH_KEYSTONE
> diff --git a/arch/arm/mach-ipq40xx/Makefile b/arch/arm/mach-ipq40xx/Makefile
> index 08a65b8854d3..b36a935c6f9f 100644
> --- a/arch/arm/mach-ipq40xx/Makefile
> +++ b/arch/arm/mach-ipq40xx/Makefile
> @@ -4,6 +4,5 @@
>  #
>  # Author: Robert Marko <robert.marko@sartura.hr>
>
> -obj-y += clock-ipq4019.o
>  obj-y += pinctrl-snapdragon.o
>  obj-y += pinctrl-ipq4019.o
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index a884f077d9b9..0df0d1881a49 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -23,6 +23,14 @@ config CLK_QCOM_APQ8096
>           on the Snapdragon APQ8096 SoC. This driver supports the clocks
>           and resets exposed by the GCC hardware block.
>
> +config CLK_QCOM_IPQ4019
> +       bool "Qualcomm IPQ4019 GCC"
> +       select CLK_QCOM
> +       help
> +         Say Y here to enable support for the Global Clock Controller
> +         on the Snapdragon IPQ4019 SoC. This driver supports the clocks
> +         and resets exposed by the GCC hardware block.
> +
>  config CLK_QCOM_QCS404
>         bool "Qualcomm QCS404 GCC"
>         select CLK_QCOM
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 44d55583596d..25bd2e1e8bb1 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -6,4 +6,5 @@ obj-y += clock-qcom.o
>  obj-$(CONFIG_CLK_QCOM_SDM845) += clock-sdm845.o
>  obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o
>  obj-$(CONFIG_CLK_QCOM_APQ8096) += clock-apq8096.o
> +obj-$(CONFIG_CLK_QCOM_IPQ4019) += clock-ipa4019.o
>  obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o
> diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c
> similarity index 56%
> rename from arch/arm/mach-ipq40xx/clock-ipq4019.c
> rename to drivers/clk/qcom/clock-ipq4019.c
> index c1d5c4ecdd81..04c99964df15 100644
> --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c
> +++ b/drivers/clk/qcom/clock-ipq4019.c
> @@ -12,12 +12,9 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <errno.h>
> -
>  #include <dt-bindings/clock/qcom,ipq4019-gcc.h>
>
> -struct msm_clk_priv {
> -       phys_addr_t base;
> -};
> +#include "clock-qcom.h"
>
>  ulong msm_set_rate(struct clk *clk, ulong rate)
>  {
> @@ -30,23 +27,7 @@ ulong msm_set_rate(struct clk *clk, ulong rate)
>         }
>  }
>
> -static int msm_clk_probe(struct udevice *dev)
> -{
> -       struct msm_clk_priv *priv = dev_get_priv(dev);
> -
> -       priv->base = dev_read_addr(dev);
> -       if (priv->base == FDT_ADDR_T_NONE)
> -               return -EINVAL;
> -
> -       return 0;
> -}
> -
> -static ulong msm_clk_set_rate(struct clk *clk, ulong rate)
> -{
> -       return msm_set_rate(clk, rate);
> -}
> -
> -static int msm_enable(struct clk *clk)
> +int msm_enable(struct clk *clk)
>  {
>         switch (clk->id) {
>         case GCC_BLSP1_QUP1_SPI_APPS_CLK: /*SPI1*/
> @@ -68,21 +49,3 @@ static int msm_enable(struct clk *clk)
>         }
>  }
>
> -static struct clk_ops msm_clk_ops = {
> -       .set_rate = msm_clk_set_rate,
> -       .enable = msm_enable,
> -};
> -
> -static const struct udevice_id msm_clk_ids[] = {
> -       { .compatible = "qcom,gcc-ipq4019" },

This compatible should be moved to "clock-qcom.c".

-Sumit

> -       { }
> -};
> -
> -U_BOOT_DRIVER(clk_msm) = {
> -       .name           = "clk_msm",
> -       .id             = UCLASS_CLK,
> -       .of_match       = msm_clk_ids,
> -       .ops            = &msm_clk_ops,
> -       .priv_auto      = sizeof(struct msm_clk_priv),
> -       .probe          = msm_clk_probe,
> -};
>
> --
> 2.42.0
>

[...]
>> diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c
>> similarity index 56%
>> rename from arch/arm/mach-ipq40xx/clock-ipq4019.c
>> rename to drivers/clk/qcom/clock-ipq4019.c
>> index c1d5c4ecdd81..04c99964df15 100644
>> --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c
>> +++ b/drivers/clk/qcom/clock-ipq4019.c
>> @@ -12,12 +12,9 @@
[...]
>> -
>> -static const struct udevice_id msm_clk_ids[] = {
>> -       { .compatible = "qcom,gcc-ipq4019" },
> 
> This compatible should be moved to "clock-qcom.c".

Should all of the clock drivers have their udevice_id tables in
clock-qcom.c or just this one?

I'll note that I forgot to adjust the U_BOOT_DRIVER definition below to
be apq4019 specific, it shouldn't say "clk_msm".
> 
> -Sumit
> 
>> -       { }
>> -};
>> -
>> -U_BOOT_DRIVER(clk_msm) = {
>> -       .name           = "clk_msm",
>> -       .id             = UCLASS_CLK,
>> -       .of_match       = msm_clk_ids,
>> -       .ops            = &msm_clk_ops,
>> -       .priv_auto      = sizeof(struct msm_clk_priv),
>> -       .probe          = msm_clk_probe,
>> -};
>>
>> --
>> 2.42.0
>>

On Fri, 27 Oct 2023 at 18:27, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>
> [...]
> >> diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c
> >> similarity index 56%
> >> rename from arch/arm/mach-ipq40xx/clock-ipq4019.c
> >> rename to drivers/clk/qcom/clock-ipq4019.c
> >> index c1d5c4ecdd81..04c99964df15 100644
> >> --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c
> >> +++ b/drivers/clk/qcom/clock-ipq4019.c
> >> @@ -12,12 +12,9 @@
> [...]
> >> -
> >> -static const struct udevice_id msm_clk_ids[] = {
> >> -       { .compatible = "qcom,gcc-ipq4019" },
> >
> > This compatible should be moved to "clock-qcom.c".
>
> Should all of the clock drivers have their udevice_id tables in
> clock-qcom.c or just this one?

Actually this comment was with respect to this patch only being
incomplete in itself. You are trying to reuse compatibles defined in
clock-qcom.c while moving this driver to drivers/clk/qcom/.

I am fine with the later patch to have udevice_id table in
corresponding SoC specific driver. There are lots of moving parts in
this series :).

-Sumit

>
> I'll note that I forgot to adjust the U_BOOT_DRIVER definition below to
> be apq4019 specific, it shouldn't say "clk_msm".
> >
> > -Sumit
> >
> >> -       { }
> >> -};
> >> -
> >> -U_BOOT_DRIVER(clk_msm) = {
> >> -       .name           = "clk_msm",
> >> -       .id             = UCLASS_CLK,
> >> -       .of_match       = msm_clk_ids,
> >> -       .ops            = &msm_clk_ops,
> >> -       .priv_auto      = sizeof(struct msm_clk_priv),
> >> -       .probe          = msm_clk_probe,
> >> -};
> >>
> >> --
> >> 2.42.0
> >>
>
> --
> // Caleb (they/them)

On 27/10/2023 14:03, Sumit Garg wrote:
> On Fri, 27 Oct 2023 at 18:27, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>>
>> [...]
>>>> diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c
>>>> similarity index 56%
>>>> rename from arch/arm/mach-ipq40xx/clock-ipq4019.c
>>>> rename to drivers/clk/qcom/clock-ipq4019.c
>>>> index c1d5c4ecdd81..04c99964df15 100644
>>>> --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c
>>>> +++ b/drivers/clk/qcom/clock-ipq4019.c
>>>> @@ -12,12 +12,9 @@
>> [...]
>>>> -
>>>> -static const struct udevice_id msm_clk_ids[] = {
>>>> -       { .compatible = "qcom,gcc-ipq4019" },
>>>
>>> This compatible should be moved to "clock-qcom.c".
>>
>> Should all of the clock drivers have their udevice_id tables in
>> clock-qcom.c or just this one?
> 
> Actually this comment was with respect to this patch only being
> incomplete in itself. You are trying to reuse compatibles defined in
> clock-qcom.c while moving this driver to drivers/clk/qcom/.

Ah! You're right, thanks for catching that.
> 
> I am fine with the later patch to have udevice_id table in
> corresponding SoC specific driver. There are lots of moving parts in
> this series :).

Yes, it's a lot to keep track of. I appreciate your feedback :)
> 
> -Sumit
> 
>>
>> I'll note that I forgot to adjust the U_BOOT_DRIVER definition below to
>> be apq4019 specific, it shouldn't say "clk_msm".
>>>
>>> -Sumit
>>>
>>>> -       { }
>>>> -};
>>>> -
>>>> -U_BOOT_DRIVER(clk_msm) = {
>>>> -       .name           = "clk_msm",
>>>> -       .id             = UCLASS_CLK,
>>>> -       .of_match       = msm_clk_ids,
>>>> -       .ops            = &msm_clk_ops,
>>>> -       .priv_auto      = sizeof(struct msm_clk_priv),
>>>> -       .probe          = msm_clk_probe,
>>>> -};
>>>>
>>>> --
>>>> 2.42.0
>>>>
>>
>> --
>> // Caleb (they/them)