Message-ID: <43aee0f9-3a83-3b90-5e3d-13a935f41b47@quicinc.com>
Date: Thu, 5 Oct 2023 15:26:06 +0530
From: Devi Priya <quic_devipriy@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <andersson@...nel.org>, <agross@...nel.org>, <konrad.dybcio@...aro.org>,
        <mturquette@...libre.com>, <sboyd@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <catalin.marinas@....com>, <will@...nel.org>, <p.zabel@...gutronix.de>,
        <richardcochran@...il.com>, <arnd@...db.de>, <geert+renesas@...der.be>,
        <nfraprado@...labora.com>, <rafal@...ecki.pl>, <peng.fan@....com>,
        <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <netdev@...r.kernel.org>,
        <quic_saahtoma@...cinc.com>
Subject: Re: [PATCH V2 5/7] clk: qcom: Add NSS clock Controller driver for
 IPQ9574



On 10/5/2023 12:49 PM, Dmitry Baryshkov wrote:
> On Thu, 5 Oct 2023 at 09:26, Devi Priya <quic_devipriy@...cinc.com> wrote:
>>
>>
>>
>> On 9/22/2023 5:31 PM, Devi Priya wrote:
>>>
>>>
>>> On 9/20/2023 1:50 PM, Dmitry Baryshkov wrote:
>>>> On Wed, 20 Sept 2023 at 09:39, Devi Priya <quic_devipriy@...cinc.com>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 9/12/2023 7:38 PM, Devi Priya wrote:
>>>>>>
>>>>>>
>>>>>> On 8/25/2023 5:14 PM, Dmitry Baryshkov wrote:
>>>>>>> On Fri, 25 Aug 2023 at 12:15, Devi Priya <quic_devipriy@...cinc.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Add Networking Sub System Clock Controller(NSSCC) driver for ipq9574
>>>>>>>> based
>>>>>>>> devices.
>>>>>>>>
>>>>>>>> Signed-off-by: Devi Priya <quic_devipriy@...cinc.com>
>>>>>>>> ---
>>>>>>>>     Changes in V2:
>>>>>>>>            - Added depends on ARM64 || COMPILE_TEST in Kconfig
>>>>>>>>            - Added module_platform_driver
>>>>>>>>            - Dropped patch [2/6] - clk: qcom: gcc-ipq9574: Mark nssnoc
>>>>>>>> clocks as critical
>>>>>>>>               & added pm_clk for nssnoc clocks
>>>>>>>>            - Updated the uniphy clock names
>>>>>>>>
>>>>>>>>     drivers/clk/qcom/Kconfig         |    7 +
>>>>>>>>     drivers/clk/qcom/Makefile        |    1 +
>>>>>>>>     drivers/clk/qcom/nsscc-ipq9574.c | 3109
>>>>>>>> ++++++++++++++++++++++++++++++
>>>>>>>>     3 files changed, 3117 insertions(+)
>>>>>>>>     create mode 100644 drivers/clk/qcom/nsscc-ipq9574.c
>>>>>>>>
>>>>>>>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>>>>>>>> index bd9bfb11b328..3ecc11e2c8e3 100644
>>>>>>>> --- a/drivers/clk/qcom/Kconfig
>>>>>>>> +++ b/drivers/clk/qcom/Kconfig
>>>>>>>> @@ -203,6 +203,13 @@ config IPQ_GCC_9574
>>>>>>>>              i2c, USB, SD/eMMC, etc. Select this for the root clock
>>>>>>>>              of ipq9574.
>>>>>>>>
>>>>>>>> +config IPQ_NSSCC_9574
>>>>>>>> +       tristate "IPQ9574 NSS Clock Controller"
>>>>>>>> +       depends on ARM64 || COMPILE_TEST
>>>>>>>> +       depends on IPQ_GCC_9574
>>>>>>>> +       help
>>>>>>>> +         Support for NSS clock controller on ipq9574 devices.
>>>>>>>> +
>>>>>>>>     config MSM_GCC_8660
>>>>>>>>            tristate "MSM8660 Global Clock Controller"
>>>>>>>>            depends on ARM || COMPILE_TEST
>>>>>>>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>>>>>>>> index 4790c8cca426..3f084928962e 100644
>>>>>>>> --- a/drivers/clk/qcom/Makefile
>>>>>>>> +++ b/drivers/clk/qcom/Makefile
>>>>>>>> @@ -30,6 +30,7 @@ obj-$(CONFIG_IPQ_GCC_6018) += gcc-ipq6018.o
>>>>>>>>     obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
>>>>>>>>     obj-$(CONFIG_IPQ_GCC_8074) += gcc-ipq8074.o
>>>>>>>>     obj-$(CONFIG_IPQ_GCC_9574) += gcc-ipq9574.o
>>>>>>>> +obj-$(CONFIG_IPQ_NSSCC_9574)   += nsscc-ipq9574.o
>>>>>>>>     obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
>>>>>>>>     obj-$(CONFIG_MDM_GCC_9607) += gcc-mdm9607.o
>>>>>>>>     obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
>>>>>>>> diff --git a/drivers/clk/qcom/nsscc-ipq9574.c
>>>>>>>> b/drivers/clk/qcom/nsscc-ipq9574.c
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000000..65bdb449ae5f
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/drivers/clk/qcom/nsscc-ipq9574.c
>>>>>>>> @@ -0,0 +1,3109 @@
>>>>>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>>>>>> +/*
>>>>>>>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>>>>>>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights
>>>>>>>> reserved.
>>>>>>>> + */
>>>>>>>> +
>>>>>>>> +#include <linux/clk-provider.h>
>>>>>>>> +#include <linux/err.h>
>>>>>>>> +#include <linux/kernel.h>
>>>>>>>> +#include <linux/module.h>
>>>>>>>> +#include <linux/of.h>
>>>>>>>> +#include <linux/of_device.h>
>>>>>>>> +#include <linux/regmap.h>
>>>>>>>> +#include <linux/pm_clock.h>
>>>>>>>> +#include <linux/pm_runtime.h>
>>>>>>>> +
>>>>>>>> +#include <dt-bindings/clock/qcom,ipq9574-nsscc.h>
>>>>>>>> +#include <dt-bindings/reset/qcom,ipq9574-nsscc.h>
>>>>>>>> +
>>>>>>>> +#include "clk-alpha-pll.h"
>>>>>>>> +#include "clk-branch.h"
>>>>>>>> +#include "clk-pll.h"
>>>>>>>> +#include "clk-rcg.h"
>>>>>>>> +#include "clk-regmap.h"
>>>>>>>> +#include "clk-regmap-divider.h"
>>>>>>>> +#include "clk-regmap-mux.h"
>>>>>>>> +#include "common.h"
>>>>>>>> +#include "reset.h"
>>>>>>>> +
>>>>>>>> +/* Need to match the order of clocks in DT binding */
>>>>>>>> +enum {
>>>>>>>> +       DT_NSSNOC_NSSCC_CLK,
>>>>>>>> +       DT_NSSNOC_SNOC_CLK,
>>>>>>>> +       DT_NSSNOC_SNOC_1_CLK,
>>>>>>>
>>>>>>> Not using the index makes it seem that these clocks are not used,
>>>>>>> until one scrolls down to pm_clks.
>>>>>> Okay, got it
>>>>>>>
>>>>>>> BTW: The NSSNOC_SNOC clocks make it look like there is an interconnect
>>>>>>> here (not a simple NIU).
>>>>>>
>>>>>> Hi Dmitry, We are exploring on the ICC driver. In the meantime to
>>>>>> unblock PCIe/NSS changes getting merged, shall we use
>>>>>> regmap_update_bits
>>>>>> and turn on the critical NSSNOC clocks, ANOC & SNOC pcie clocks in the
>>>>>> probe function of the gcc driver itself as like sm8550 driver to get
>>>>>> the
>>>>>> changes merged?
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/qcom/gcc-sm8550.c#n3347
>>>>>
>>>>> Hi Dmitry,
>>>>> Just curious to know if we could send out the next series with the
>>>>> proposed approach if that holds good.
>>>>
>>>> The answer really depends on the structure of your hardware. The issue
>>>> is that once you commit the device bindings,you have to support them
>>>> forever. So, if you commit the NSS clock support without interconnects
>>>> in place, you have to keep this ANOC/SNOC/etc code forever, even after
>>>> you land the interconnect. So I'd suggest landing the icc driver first
>>>> (or at least implementing and sending to the mailing list), so that we
>>>> can see how all these pieces fit together.
>>>
>>> Hi Dmitry,
>>> Unlike MSM chipsets, IPQ chipsets does not have any use case wherein the
>>> NOC clocks have to be scaled. So if these clocks can be enabled in the
>>> probe, there is no need for an interconnect driver at all. The same
>>> applies to both ipq9574 and ipq5332 SoCs.
>>>
>>
>> Hi Dmitry,
>> Just curious to know if we can go ahead with the proposed solution of
>> enabling the NOC clocks in the probe as these clocks need not be scaled
>> in IPQ chipsets & hence there would be no need for an ICC driver in
>> ipq9574 & ipq5332 targets.
> 
> In the probe of which driver?
GCC driver of ipq9574 & ipq5332 targets.

Thanks,
Devi Priya
> 
> 
> 

Powered by blists - more mailing lists