Re: [PATCH v9 4/4] clk: qcom: add clock controller driver for qca8386/qca8084

From: Jie Luo
Date: Wed Oct 11 2023 - 08:41:20 EST




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.