mbox series

[v9,0/4] add clock controller of qca8386/qca8084

Message ID 20230923112105.18102-1-quic_luoj@quicinc.com
Headers show
Series add clock controller of qca8386/qca8084 | expand

Message

Luo Jie Sept. 23, 2023, 11:21 a.m. UTC 
qca8xxx is 4 * 2.5GBaseT ports chip, working as switch mode
named by qca8386, or working as PHY mode named by qca8084,
clock hardware reigster is accessed by MDIO bus.

This patch series add the clock controller of qca8363/qca8084,
and add the clock ops clk_branch2_prepare_ops to avoid spin lock
used during the clock operation of qca8k clock controller where
the sleep happens when accessing clock control register by MDIO
bus.

Changes in v2:
	* remove clock flag CLK_ENABLE_MUTEX_LOCK.
	* add clock ops clk_branch2_qca8k_ops.
	* improve yaml file for fixing dtschema warnings.
	* enable clock controller driver in defconfig.

Changes in v3:
	* rename clk_branch2_qca8k_ops to clk_branch2_mdio_ops.
	* fix review comments on yaml file.
	* use dev_err_probe on driver probe error.
	* only use the compatible "qcom,qca8084-nsscc".
	* remove enable clock controller driver patch.

Changes in v4:
	* add _qcom_cc_really_probe function.
	* commonizing the probe function.
	* remove flag CLK_IS_CRITICAL from clocks only needed
	to be enabled in switch device.
	* update device tree property reg to 0x10. 

Changes in v5:
	* commonize qcom_cc_really_probe.
	* add halt_check for the branch clocks.
	* fix the review comments on nsscc-qca8k.c. 

Changes in v6:
	* rename clk_branch2_mdio_ops to clk_branch2_prepare_ops.

Changes in v7:
	* remove the clock flag CLK_IS_CRITICAL.
	* optimize the file nsscc-qca8k.c.
	* identify & fix the comments from Stephen.

Changes in v8:
	* add dependency on ARM in Kconfig.

Changes in v9:
	* take the clk_ops clk_rcg2_mux_closest_ops to remove the
	  redundant freq_tbls.

Luo Jie (4):
  clk: qcom: branch: Add clk_branch2_prepare_ops
  dt-bindings: clock: add qca8386/qca8084 clock and reset definitions
  clk: qcom: common: commonize qcom_cc_really_probe
  clk: qcom: add clock controller driver for qca8386/qca8084

 .../bindings/clock/qcom,qca8k-nsscc.yaml      |   79 +
 drivers/clk/qcom/Kconfig                      |   10 +
 drivers/clk/qcom/Makefile                     |    1 +
 drivers/clk/qcom/apss-ipq6018.c               |    2 +-
 drivers/clk/qcom/camcc-sc7180.c               |    2 +-
 drivers/clk/qcom/camcc-sc7280.c               |    2 +-
 drivers/clk/qcom/camcc-sdm845.c               |    2 +-
 drivers/clk/qcom/camcc-sm6350.c               |    2 +-
 drivers/clk/qcom/camcc-sm8250.c               |    2 +-
 drivers/clk/qcom/camcc-sm8450.c               |    2 +-
 drivers/clk/qcom/clk-branch.c                 |    7 +
 drivers/clk/qcom/clk-branch.h                 |    1 +
 drivers/clk/qcom/common.c                     |    7 +-
 drivers/clk/qcom/common.h                     |    2 +-
 drivers/clk/qcom/dispcc-qcm2290.c             |    2 +-
 drivers/clk/qcom/dispcc-sc7180.c              |    2 +-
 drivers/clk/qcom/dispcc-sc7280.c              |    2 +-
 drivers/clk/qcom/dispcc-sc8280xp.c            |    2 +-
 drivers/clk/qcom/dispcc-sdm845.c              |    2 +-
 drivers/clk/qcom/dispcc-sm6115.c              |    2 +-
 drivers/clk/qcom/dispcc-sm6125.c              |    2 +-
 drivers/clk/qcom/dispcc-sm6350.c              |    2 +-
 drivers/clk/qcom/dispcc-sm6375.c              |    2 +-
 drivers/clk/qcom/dispcc-sm8250.c              |    2 +-
 drivers/clk/qcom/dispcc-sm8450.c              |    2 +-
 drivers/clk/qcom/dispcc-sm8550.c              |    2 +-
 drivers/clk/qcom/gcc-ipq5018.c                |    2 +-
 drivers/clk/qcom/gcc-ipq6018.c                |    2 +-
 drivers/clk/qcom/gcc-ipq8074.c                |    2 +-
 drivers/clk/qcom/gcc-mdm9607.c                |    2 +-
 drivers/clk/qcom/gcc-mdm9615.c                |    2 +-
 drivers/clk/qcom/gcc-msm8917.c                |    2 +-
 drivers/clk/qcom/gcc-msm8939.c                |    2 +-
 drivers/clk/qcom/gcc-msm8953.c                |    2 +-
 drivers/clk/qcom/gcc-msm8976.c                |    2 +-
 drivers/clk/qcom/gcc-msm8996.c                |    2 +-
 drivers/clk/qcom/gcc-msm8998.c                |    2 +-
 drivers/clk/qcom/gcc-qcm2290.c                |    2 +-
 drivers/clk/qcom/gcc-qcs404.c                 |    2 +-
 drivers/clk/qcom/gcc-qdu1000.c                |    2 +-
 drivers/clk/qcom/gcc-sa8775p.c                |    2 +-
 drivers/clk/qcom/gcc-sc7180.c                 |    2 +-
 drivers/clk/qcom/gcc-sc7280.c                 |    2 +-
 drivers/clk/qcom/gcc-sc8180x.c                |    2 +-
 drivers/clk/qcom/gcc-sc8280xp.c               |    2 +-
 drivers/clk/qcom/gcc-sdm660.c                 |    2 +-
 drivers/clk/qcom/gcc-sdm845.c                 |    2 +-
 drivers/clk/qcom/gcc-sdx55.c                  |    2 +-
 drivers/clk/qcom/gcc-sdx65.c                  |    2 +-
 drivers/clk/qcom/gcc-sdx75.c                  |    2 +-
 drivers/clk/qcom/gcc-sm6115.c                 |    2 +-
 drivers/clk/qcom/gcc-sm6125.c                 |    2 +-
 drivers/clk/qcom/gcc-sm6350.c                 |    2 +-
 drivers/clk/qcom/gcc-sm6375.c                 |    2 +-
 drivers/clk/qcom/gcc-sm7150.c                 |    2 +-
 drivers/clk/qcom/gcc-sm8150.c                 |    2 +-
 drivers/clk/qcom/gcc-sm8250.c                 |    2 +-
 drivers/clk/qcom/gcc-sm8350.c                 |    2 +-
 drivers/clk/qcom/gcc-sm8450.c                 |    2 +-
 drivers/clk/qcom/gcc-sm8550.c                 |    2 +-
 drivers/clk/qcom/gpucc-msm8998.c              |    2 +-
 drivers/clk/qcom/gpucc-sa8775p.c              |    2 +-
 drivers/clk/qcom/gpucc-sc7180.c               |    2 +-
 drivers/clk/qcom/gpucc-sc7280.c               |    2 +-
 drivers/clk/qcom/gpucc-sc8280xp.c             |    2 +-
 drivers/clk/qcom/gpucc-sdm660.c               |    2 +-
 drivers/clk/qcom/gpucc-sdm845.c               |    2 +-
 drivers/clk/qcom/gpucc-sm6115.c               |    2 +-
 drivers/clk/qcom/gpucc-sm6125.c               |    2 +-
 drivers/clk/qcom/gpucc-sm6350.c               |    2 +-
 drivers/clk/qcom/gpucc-sm6375.c               |    2 +-
 drivers/clk/qcom/gpucc-sm8150.c               |    2 +-
 drivers/clk/qcom/gpucc-sm8250.c               |    2 +-
 drivers/clk/qcom/gpucc-sm8350.c               |    2 +-
 drivers/clk/qcom/gpucc-sm8450.c               |    2 +-
 drivers/clk/qcom/gpucc-sm8550.c               |    2 +-
 drivers/clk/qcom/lcc-ipq806x.c                |    2 +-
 drivers/clk/qcom/lcc-msm8960.c                |    2 +-
 drivers/clk/qcom/lpassaudiocc-sc7280.c        |    4 +-
 drivers/clk/qcom/lpasscorecc-sc7180.c         |    2 +-
 drivers/clk/qcom/lpasscorecc-sc7280.c         |    2 +-
 drivers/clk/qcom/mmcc-msm8960.c               |    2 +-
 drivers/clk/qcom/mmcc-msm8974.c               |    2 +-
 drivers/clk/qcom/mmcc-msm8994.c               |    2 +-
 drivers/clk/qcom/mmcc-msm8996.c               |    2 +-
 drivers/clk/qcom/mmcc-msm8998.c               |    2 +-
 drivers/clk/qcom/mmcc-sdm660.c                |    2 +-
 drivers/clk/qcom/nsscc-qca8k.c                | 2139 +++++++++++++++++
 drivers/clk/qcom/tcsrcc-sm8550.c              |    2 +-
 drivers/clk/qcom/videocc-sc7180.c             |    2 +-
 drivers/clk/qcom/videocc-sc7280.c             |    2 +-
 drivers/clk/qcom/videocc-sdm845.c             |    2 +-
 drivers/clk/qcom/videocc-sm8150.c             |    2 +-
 drivers/clk/qcom/videocc-sm8250.c             |    2 +-
 drivers/clk/qcom/videocc-sm8350.c             |    2 +-
 drivers/clk/qcom/videocc-sm8450.c             |    2 +-
 drivers/clk/qcom/videocc-sm8550.c             |    2 +-
 include/dt-bindings/clock/qcom,qca8k-nsscc.h  |  101 +
 include/dt-bindings/reset/qcom,qca8k-nsscc.h  |   75 +
 99 files changed, 2507 insertions(+), 95 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,qca8k-nsscc.yaml
 create mode 100644 drivers/clk/qcom/nsscc-qca8k.c
 create mode 100644 include/dt-bindings/clock/qcom,qca8k-nsscc.h
 create mode 100644 include/dt-bindings/reset/qcom,qca8k-nsscc.h


base-commit: 5a1b322cb0b7d0d33a2d13462294dc0f46911172

Comments

Bryan O'Donoghue Oct. 11, 2023, 10:25 a.m. UTC | #1 
On 23/09/2023 12:21, Luo Jie wrote:
> The clock controller driver of qca8386/qca8084 is registered
> as the MDIO device, the hardware register is accessed by MDIO bus
> that is normally used to access general PHY device, which is
> different from the current existed qcom clock controller drivers
> using ioremap to access hardware clock registers.

"nsscc-qca8k is accessed via an MDIO bus"

> MDIO bus is common utilized by both qca8386/qca8084 and other

commonly

> PHY devices, so the mutex lock mdio_bus->mdio_lock should be
> used instead of using the mutex lock of remap.
> 
> To access the hardware clock registers of qca8386/qca8084, there
> is special MDIO frame sequence(three MDIO read/write operations)
> need to be sent to device.

"there is a special MDIO frame sequence"

"which needs to be sent to the device"

the following indentation splat from checkpatch

CHECK: Alignment should match open parenthesis
#2071: FILE: drivers/clk/qcom/nsscc-qca8k.c:2004:
+		ret = __mdiobus_write(bus, switch_phy_id, (reg | 
QCA8K_REG_DATA_UPPER_16_BITS),
+				upper_16_bits(val));

CHECK: Alignment should match open parenthesis
#2131: FILE: drivers/clk/qcom/nsscc-qca8k.c:2064:
+static int qca8k_regmap_update_bits(void *context, unsigned int regaddr,
+		unsigned int mask, unsigned int value)

total: 0 errors, 1 warnings, 2 checks, 2162 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or 
--fix-inplace.

0004-clk-qcom-add-clock-controller-driver-for-qca8386-qca.patch has 
style problems, please review.

Once fixed

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Luo Jie Oct. 11, 2023, 11:26 a.m. UTC | #2 
On 10/11/2023 6:25 PM, Bryan O'Donoghue wrote:
> On 23/09/2023 12:21, Luo Jie wrote:
>> The clock controller driver of qca8386/qca8084 is registered
>> as the MDIO device, the hardware register is accessed by MDIO bus
>> that is normally used to access general PHY device, which is
>> different from the current existed qcom clock controller drivers
>> using ioremap to access hardware clock registers.
> 
> "nsscc-qca8k is accessed via an MDIO bus"
> 
>> MDIO bus is common utilized by both qca8386/qca8084 and other
> 
> commonly
> 
>> PHY devices, so the mutex lock mdio_bus->mdio_lock should be
>> used instead of using the mutex lock of remap.
>>
>> To access the hardware clock registers of qca8386/qca8084, there
>> is special MDIO frame sequence(three MDIO read/write operations)
>> need to be sent to device.
> 
> "there is a special MDIO frame sequence"
> 
> "which needs to be sent to the device"

I will update the comments, thanks Bryan.

> 
> the following indentation splat from checkpatch
> 
> CHECK: Alignment should match open parenthesis
> #2071: FILE: drivers/clk/qcom/nsscc-qca8k.c:2004:
> +        ret = __mdiobus_write(bus, switch_phy_id, (reg | 
> QCA8K_REG_DATA_UPPER_16_BITS),
> +                upper_16_bits(val));
> 
> CHECK: Alignment should match open parenthesis
> #2131: FILE: drivers/clk/qcom/nsscc-qca8k.c:2064:
> +static int qca8k_regmap_update_bits(void *context, unsigned int regaddr,
> +        unsigned int mask, unsigned int value)
> 
> total: 0 errors, 1 warnings, 2 checks, 2162 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>        mechanically convert to the typical style using --fix or 
> --fix-inplace.
> 
> 0004-clk-qcom-add-clock-controller-driver-for-qca8386-qca.patch has 
> style problems, please review.

Thanks Bryan for the review. The code line mentioned by CHECK is more 
than 100 columns, so i separate the lines.

> 
> Once fixed
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Konrad Dybcio Oct. 11, 2023, 11:32 a.m. UTC | #3 
On 10/11/23 13:26, Jie Luo wrote:
> 
> 
> On 10/11/2023 6:25 PM, Bryan O'Donoghue wrote:
>> On 23/09/2023 12:21, Luo Jie wrote:
>>> The clock controller driver of qca8386/qca8084 is registered
>>> as the MDIO device, the hardware register is accessed by MDIO bus
>>> that is normally used to access general PHY device, which is
>>> different from the current existed qcom clock controller drivers
>>> using ioremap to access hardware clock registers.
>>
>> "nsscc-qca8k is accessed via an MDIO bus"
>>
>>> MDIO bus is common utilized by both qca8386/qca8084 and other
>>
>> commonly
>>
>>> PHY devices, so the mutex lock mdio_bus->mdio_lock should be
>>> used instead of using the mutex lock of remap.
>>>
>>> To access the hardware clock registers of qca8386/qca8084, there
>>> is special MDIO frame sequence(three MDIO read/write operations)
>>> need to be sent to device.
>>
>> "there is a special MDIO frame sequence"
>>
>> "which needs to be sent to the device"
> 
> I will update the comments, thanks Bryan.
> 
>>
>> the following indentation splat from checkpatch
>>
>> CHECK: Alignment should match open parenthesis
>> #2071: FILE: drivers/clk/qcom/nsscc-qca8k.c:2004:
>> +        ret = __mdiobus_write(bus, switch_phy_id, (reg | 
>> QCA8K_REG_DATA_UPPER_16_BITS),
>> +                upper_16_bits(val));
>>
>> CHECK: Alignment should match open parenthesis
>> #2131: FILE: drivers/clk/qcom/nsscc-qca8k.c:2064:
>> +static int qca8k_regmap_update_bits(void *context, unsigned int regaddr,
>> +        unsigned int mask, unsigned int value)
>>
>> total: 0 errors, 1 warnings, 2 checks, 2162 lines checked
>>
>> NOTE: For some of the reported defects, checkpatch may be able to
>>        mechanically convert to the typical style using --fix or 
>> --fix-inplace.
>>
>> 0004-clk-qcom-add-clock-controller-driver-for-qca8386-qca.patch has 
>> style problems, please review.
> 
> Thanks Bryan for the review. The code line mentioned by CHECK is more 
> than 100 columns, so i separate the lines.
Please read what checkpatch tells you.

It asks you to change

very_long_func_name(arg1, arg2,
	arg3);

to

very_long_func_name(arg1, arg2,
		    arg3);

(remember tab len is 8 for the linux kernel)

Konrad
Bryan O'Donoghue Oct. 11, 2023, 11:33 a.m. UTC | #4 
On 11/10/2023 12:26, Jie Luo wrote:
>>
>> 0004-clk-qcom-add-clock-controller-driver-for-qca8386-qca.patch has 
>> style problems, please review.
> 
> Thanks Bryan for the review. The code line mentioned by CHECK is more 
> than 100 columns, so i separate the lines.

Yep. Remember to align the indentation as much as possible/reasonable. 
Use your discretion.

---
bod
Luo Jie Oct. 11, 2023, 12:40 p.m. UTC | #5 
On 10/11/2023 7:32 PM, Konrad Dybcio wrote:
> 
> 
> On 10/11/23 13:26, Jie Luo wrote:
>>
>>
>> On 10/11/2023 6:25 PM, Bryan O'Donoghue wrote:
>>> On 23/09/2023 12:21, Luo Jie wrote:
>>>> The clock controller driver of qca8386/qca8084 is registered
>>>> as the MDIO device, the hardware register is accessed by MDIO bus
>>>> that is normally used to access general PHY device, which is
>>>> different from the current existed qcom clock controller drivers
>>>> using ioremap to access hardware clock registers.
>>>
>>> "nsscc-qca8k is accessed via an MDIO bus"
>>>
>>>> MDIO bus is common utilized by both qca8386/qca8084 and other
>>>
>>> commonly
>>>
>>>> PHY devices, so the mutex lock mdio_bus->mdio_lock should be
>>>> used instead of using the mutex lock of remap.
>>>>
>>>> To access the hardware clock registers of qca8386/qca8084, there
>>>> is special MDIO frame sequence(three MDIO read/write operations)
>>>> need to be sent to device.
>>>
>>> "there is a special MDIO frame sequence"
>>>
>>> "which needs to be sent to the device"
>>
>> I will update the comments, thanks Bryan.
>>
>>>
>>> the following indentation splat from checkpatch
>>>
>>> CHECK: Alignment should match open parenthesis
>>> #2071: FILE: drivers/clk/qcom/nsscc-qca8k.c:2004:
>>> +        ret = __mdiobus_write(bus, switch_phy_id, (reg | 
>>> QCA8K_REG_DATA_UPPER_16_BITS),
>>> +                upper_16_bits(val));
>>>
>>> CHECK: Alignment should match open parenthesis
>>> #2131: FILE: drivers/clk/qcom/nsscc-qca8k.c:2064:
>>> +static int qca8k_regmap_update_bits(void *context, unsigned int 
>>> regaddr,
>>> +        unsigned int mask, unsigned int value)
>>>
>>> total: 0 errors, 1 warnings, 2 checks, 2162 lines checked
>>>
>>> NOTE: For some of the reported defects, checkpatch may be able to
>>>        mechanically convert to the typical style using --fix or 
>>> --fix-inplace.
>>>
>>> 0004-clk-qcom-add-clock-controller-driver-for-qca8386-qca.patch has 
>>> style problems, please review.
>>
>> Thanks Bryan for the review. The code line mentioned by CHECK is more 
>> than 100 columns, so i separate the lines.
> Please read what checkpatch tells you.
> 
> It asks you to change
> 
> very_long_func_name(arg1, arg2,
>      arg3);
> 
> to
> 
> very_long_func_name(arg1, arg2,
>              arg3);
> 
> (remember tab len is 8 for the linux kernel)
> 
> Konrad

Got it, thanks Konrad for the reminder.
Luo Jie Oct. 11, 2023, 12:42 p.m. UTC | #6 
On 10/11/2023 7:33 PM, Bryan O'Donoghue wrote:
> On 11/10/2023 12:26, Jie Luo wrote:
>>>
>>> 0004-clk-qcom-add-clock-controller-driver-for-qca8386-qca.patch has 
>>> style problems, please review.
>>
>> Thanks Bryan for the review. The code line mentioned by CHECK is more 
>> than 100 columns, so i separate the lines.
> 
> Yep. Remember to align the indentation as much as possible/reasonable. 
> Use your discretion.
> 
> ---
> bod

Thanks Bod, i will update the patch for fixing this CHECK print.