mbox series

[V2,0/6] Add camera clock controller support for SM8550

Message ID 20230525172142.9039-1-quic_jkona@quicinc.com
Headers show
Series Add camera clock controller support for SM8550 | expand

Message

Jagadeesh Kona May 25, 2023, 5:21 p.m. UTC
Add bindings, driver and devicetree node for camera clock controller
on SM8550. Update l value in pll configuration for lucid ole and evo
pll's across chipsets and remove explicit CAL_L configuration for EVO
pll's.

Depends on [1] for PLL_TEST_CTL_U2 programming
[1] https://patchwork.kernel.org/project/linux-clk/list/?series=750700

Jagadeesh Kona (6):
  dt-bindings: clock: qcom: Add SM8550 camera clock controller
  clk: qcom: Update l value configuration for lucid ole and evo plls
  clk: qcom: clk-alpha-pll: Remove explicit CAL_L configuration for EVO
    PLL
  clk: qcom: camcc-sm8550: Add camera clock controller driver for SM8550
  clk: qcom: camcc-sm8550: Add support for qdss, sleep and xo clocks
  arm64: dts: qcom: sm8550: Add camera clock controller

 .../bindings/clock/qcom,sm8450-camcc.yaml     |    8 +-
 arch/arm64/boot/dts/qcom/sm8550.dtsi          |   15 +
 drivers/clk/qcom/Kconfig                      |    7 +
 drivers/clk/qcom/Makefile                     |    1 +
 drivers/clk/qcom/camcc-sm8450.c               |   24 +-
 drivers/clk/qcom/camcc-sm8550.c               | 3585 +++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.c              |    6 +-
 drivers/clk/qcom/dispcc-sm8450.c              |   10 +-
 drivers/clk/qcom/dispcc-sm8550.c              |    6 +-
 drivers/clk/qcom/gpucc-sa8775p.c              |    6 +-
 include/dt-bindings/clock/qcom,sm8550-camcc.h |  187 +
 11 files changed, 3832 insertions(+), 23 deletions(-)
 create mode 100644 drivers/clk/qcom/camcc-sm8550.c
 create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h

Comments

Bryan O'Donoghue May 26, 2023, 12:29 p.m. UTC | #1
On 25/05/2023 18:21, Jagadeesh Kona wrote:
> Add device tree bindings for the camera clock controller on
> Qualcomm SM8550 platform.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> ---
> Changes since V1:
>   - Removed new YAML file and reused SM8450 CAMCC YAML file for SM8550
> 
>   .../bindings/clock/qcom,sm8450-camcc.yaml     |   8 +-
>   include/dt-bindings/clock/qcom,sm8550-camcc.h | 187 ++++++++++++++++++
>   2 files changed, 193 insertions(+), 2 deletions(-)
>   create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> index 87ae74166807..8dbc9004202f 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
> @@ -13,11 +13,15 @@ description: |
>     Qualcomm camera clock control module provides the clocks, resets and power
>     domains on SM8450.
>   
> -  See also:: include/dt-bindings/clock/qcom,sm8450-camcc.h
> +  See also::
> +    include/dt-bindings/clock/qcom,sm8450-camcc.h
> +    include/dt-bindings/clock/qcom,sm8550-camcc.h
>   
>   properties:
>     compatible:
> -    const: qcom,sm8450-camcc
> +    enum:
> +      - qcom,sm8450-camcc
> +      - qcom,sm8550-camcc

Hmm,

So looking at Documentation/devicetree/bindings/clock/*camcc*.yaml we 
seem to be repeating yaml over and over again with no substantial 
difference between one description and another.

You've picked at the thread here by adding sm8550 into sm8450.

I think sm8250, sm8450, sm8550, sc7280 and ... probably sm6350 should 
live in the one yaml description 
Documentation/devicetree/bindings/clock/qcom,camcc.yaml

sm6350 looks a bit sparse/incomplete to me so perhaps leave that out. 
The others sc7280, sm8250, sm8450 and sm8550 can/should all be moved 
into the same yaml file with a list of compatibles.

---
bod
Bryan O'Donoghue May 26, 2023, 3:54 p.m. UTC | #2
On 25/05/2023 18:21, Jagadeesh Kona wrote:
> Fixes: 260e36606a03 ("clk: qcom: clk-alpha-pll: add Lucid EVO PLL configuration interfaces")

Is this a "Fixes" without the previous patch to stuff the CAL_L_VAL and 
VAL_L fields ?

[PATCH V2 3/6] clk: qcom: clk-alpha-pll: Remove explicit CAL_L 
configuration for EVO PLL

Surely you need _both_ with this patch depending on the previous, per 
your comment ?

-	.l = 0x3e,
+	/* .l includes CAL_L_VAL, L_VAL fields */
+	.l = 0x0044003e,

---
bod
Bryan O'Donoghue May 26, 2023, 3:57 p.m. UTC | #3
On 26/05/2023 16:54, Bryan O'Donoghue wrote:
> On 25/05/2023 18:21, Jagadeesh Kona wrote:
>> Fixes: 260e36606a03 ("clk: qcom: clk-alpha-pll: add Lucid EVO PLL 
>> configuration interfaces")
> 
> Is this a "Fixes" without the previous patch to stuff the CAL_L_VAL and 
> VAL_L fields ?
> 
> [PATCH V2 3/6] clk: qcom: clk-alpha-pll: Remove explicit CAL_L 
> configuration for EVO PLL
> 
> Surely you need _both_ with this patch depending on the previous, per 
> your comment ?
> 
> -    .l = 0x3e,
> +    /* .l includes CAL_L_VAL, L_VAL fields */
> +    .l = 0x0044003e,
> 
> ---
> bod

i.e. if you pick up this patch on its own you won't populate 
CAL_L_VAL... right ?

It would make more sense to squash the two patches.

---
bod
Jagadeesh Kona June 1, 2023, 2:21 p.m. UTC | #4
Hi Bryan,

Thanks for your review!

On 5/26/2023 9:27 PM, Bryan O'Donoghue wrote:
> On 26/05/2023 16:54, Bryan O'Donoghue wrote:
>> On 25/05/2023 18:21, Jagadeesh Kona wrote:
>>> Fixes: 260e36606a03 ("clk: qcom: clk-alpha-pll: add Lucid EVO PLL 
>>> configuration interfaces")
>>
>> Is this a "Fixes" without the previous patch to stuff the CAL_L_VAL 
>> and VAL_L fields ?
>>
>> [PATCH V2 3/6] clk: qcom: clk-alpha-pll: Remove explicit CAL_L 
>> configuration for EVO PLL
>>
>> Surely you need _both_ with this patch depending on the previous, per 
>> your comment ?
>>
>> -    .l = 0x3e,
>> +    /* .l includes CAL_L_VAL, L_VAL fields */
>> +    .l = 0x0044003e,
>>
>> ---
>> bod
> 
> i.e. if you pick up this patch on its own you won't populate 
> CAL_L_VAL... right ?
> 
> It would make more sense to squash the two patches.
> 
Sure, will squash both the patches in next series.
> ---
> bod

Thanks & Regards,
Jagadeesh