[V4,0/4] Add camera clock controller support for SM8550

  Add bindings, driver and devicetree node for camera clock controller on
SM8550.

Jagadeesh Kona (4):
  dt-bindings: clock: qcom: Add SM8550 camera clock controller
  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-sm8550.c               | 3585 +++++++++++++++++
 include/dt-bindings/clock/qcom,sm8550-camcc.h |  187 +
 6 files changed, 3801 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/qcom/camcc-sm8550.c
 create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h
  

On 9.06.2023 13:50, Jagadeesh Kona wrote:
> Add bindings, driver and devicetree node for camera clock controller on
> SM8550.
> 
> Jagadeesh Kona (4):
>   dt-bindings: clock: qcom: Add SM8550 camera clock controller
>   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
What's the final verdict on RINGOSC_L etc.?

Konrad
> 
>  .../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-sm8550.c               | 3585 +++++++++++++++++
>  include/dt-bindings/clock/qcom,sm8550-camcc.h |  187 +
>  6 files changed, 3801 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/clk/qcom/camcc-sm8550.c
>  create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h
>
  

On 09/06/2023 12:50, Jagadeesh Kona wrote:
> Add bindings, driver and devicetree node for camera clock controller on
> SM8550.

This is very confusing.

Your cover letter doesn't detail any changes and your individual patches 
all say "no changes since v3", "no changes since v2"

If this is a RESEND then mark it as a RESEND.

Good practice is to for example add a note that says

"I looked at updating the yaml for the camcc but opted to do this in 
another series" or "opted not to do this at this time" or "it doesn't 
make sense because of X"

https://lore.kernel.org/linux-arm-msm/546876ba-970d-5cd5-648e-723698ca74fd@linaro.org/

Could you perhaps RESEND this V4 with a log that explains what has 
changed from one version to the next.

If nothing has changed then don't bump the version prefix with RESEND..

Second thought even replying to your cover email with the changelog 
would do.....

---
bod
  

On 12/06/2023 04:25, Bryan O'Donoghue wrote:
> On 09/06/2023 12:50, Jagadeesh Kona wrote:
>> Add bindings, driver and devicetree node for camera clock controller on
>> SM8550.
> 
> This is very confusing.
> 
> Your cover letter doesn't detail any changes and your individual patches 
> all say "no changes since v3", "no changes since v2"
> 
> If this is a RESEND then mark it as a RESEND.

That's indeed odd. Three versions without changes...

Best regards,
Krzysztof
  

On 6/13/2023 2:07 PM, Krzysztof Kozlowski wrote:
> On 12/06/2023 04:25, Bryan O'Donoghue wrote:
>> On 09/06/2023 12:50, Jagadeesh Kona wrote:
>>> Add bindings, driver and devicetree node for camera clock controller on
>>> SM8550.
>>
>> This is very confusing.
>>
>> Your cover letter doesn't detail any changes and your individual patches
>> all say "no changes since v3", "no changes since v2"
>>
>> If this is a RESEND then mark it as a RESEND.
> 
> That's indeed odd. Three versions without changes...
> 
> Best regards,
> Krzysztof
> 

This is not a RESEND, actually there were changes from each version to 
next version and change details were updated in respective patches. But 
the patches in which changes were present were dropped in v4 based on 
review comments. Will take care of updating cover letter as well with 
changes across versions if we push the next series.

Please find the summary of changes across versions till v4.

Changes in v4:
  - Dropped the extra patches added in v2, since the review comments on 
v3 recommended an alternate solution.

Changes in v3:
  - Squashed 2 extra patches added in v2 into single patch as per review 
comments
  - Link to v3: 
https://patchwork.kernel.org/project/linux-clk/list/?series=753150

Changes in v2:
  - Took care of review comments from v1
  - Added 2 extra patches updating L configuration value across chipsets 
to include CAL_L and RINGOSC_CAL_L fields and removed setting CAL_L 
field in clk_lucid_evo_pll_configure().
  - Link to v2: 
https://patchwork.kernel.org/project/linux-clk/list/?series=751058

v1:
  - Initial CAMCC changes for SM8550
  - Link to v1: 
https://patchwork.kernel.org/project/linux-clk/list/?series=749294

Thanks,
Jagadeesh
  

On 6/9/2023 6:24 PM, Konrad Dybcio wrote:
> 
> 
> On 9.06.2023 13:50, Jagadeesh Kona wrote:
>> Add bindings, driver and devicetree node for camera clock controller on
>> SM8550.
>>
>> Jagadeesh Kona (4):
>>    dt-bindings: clock: qcom: Add SM8550 camera clock controller
>>    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
> What's the final verdict on RINGOSC_L etc.?
> 
> Konrad

We would like to pass RINGOSC_CAL_L field directly in config->l value 
itself and reuse existing code rather than adding a separate function 
for lucid ole pll configure.

Thanks,
Jagadeesh

>>
>>   .../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-sm8550.c               | 3585 +++++++++++++++++
>>   include/dt-bindings/clock/qcom,sm8550-camcc.h |  187 +
>>   6 files changed, 3801 insertions(+), 2 deletions(-)
>>   create mode 100644 drivers/clk/qcom/camcc-sm8550.c
>>   create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h
>>
  

On Wed, 14 Jun 2023 at 14:58, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
>
>
> On 6/9/2023 6:24 PM, Konrad Dybcio wrote:
> >
> >
> > On 9.06.2023 13:50, Jagadeesh Kona wrote:
> >> Add bindings, driver and devicetree node for camera clock controller on
> >> SM8550.
> >>
> >> Jagadeesh Kona (4):
> >>    dt-bindings: clock: qcom: Add SM8550 camera clock controller
> >>    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
> > What's the final verdict on RINGOSC_L etc.?
> >
> > Konrad
>
> We would like to pass RINGOSC_CAL_L field directly in config->l value
> itself and reuse existing code rather than adding a separate function
> for lucid ole pll configure.

As I wrote in another email, it doesn't sound like a good approach.

>
> Thanks,
> Jagadeesh
>
> >>
> >>   .../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-sm8550.c               | 3585 +++++++++++++++++
> >>   include/dt-bindings/clock/qcom,sm8550-camcc.h |  187 +
> >>   6 files changed, 3801 insertions(+), 2 deletions(-)
> >>   create mode 100644 drivers/clk/qcom/camcc-sm8550.c
> >>   create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h
> >>
  

On 6/14/2023 5:47 PM, Dmitry Baryshkov wrote:
> On Wed, 14 Jun 2023 at 14:58, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 6/9/2023 6:24 PM, Konrad Dybcio wrote:
>>>
>>>
>>> On 9.06.2023 13:50, Jagadeesh Kona wrote:
>>>> Add bindings, driver and devicetree node for camera clock controller on
>>>> SM8550.
>>>>
>>>> Jagadeesh Kona (4):
>>>>     dt-bindings: clock: qcom: Add SM8550 camera clock controller
>>>>     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
>>> What's the final verdict on RINGOSC_L etc.?
>>>
>>> Konrad
>>
>> We would like to pass RINGOSC_CAL_L field directly in config->l value
>> itself and reuse existing code rather than adding a separate function
>> for lucid ole pll configure.
> 
> As I wrote in another email, it doesn't sound like a good approach.
> 

Will avoid this approach and use separate clk_lucid_ole_pll_configure() 
to configure lucid ole PLL's in next series.

Thanks,
Jagadeesh

>>
>> Thanks,
>> Jagadeesh
>>
>>>>
>>>>    .../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-sm8550.c               | 3585 +++++++++++++++++
>>>>    include/dt-bindings/clock/qcom,sm8550-camcc.h |  187 +
>>>>    6 files changed, 3801 insertions(+), 2 deletions(-)
>>>>    create mode 100644 drivers/clk/qcom/camcc-sm8550.c
>>>>    create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h
>>>>
> 
> 
>