Re: [PATCH v2 3/4] clk: imx: pll14xx: support spread spectrum clock generation

Peng Fan
2025-02-07T10:19:43Z

Hi Sudeep,

On Thu, Feb 06, 2025 at 04:16:58PM +0000, Sudeep Holla wrote:
>Hi Peng,
>
>I apologise in advance for exploiting this thread to make my point.
>
>On Thu, Feb 06, 2025 at 04:31:46PM +0100, Dario Binacchi wrote:
>>
>> Sorry if I miscounted the lines, but here we are not considering who
>> actually implemented
>> the algorithmic part of the SSC management and all the time spent
>> testing the code on more
>> than one platform/board with each submission of the series for all 9 versions.
>>
>> [1] https://lore.kernel.org/all/20250118124044.157308-18-dario.binacchi@amarulasolutions.com/
>>
>> Your changes, which are unnecessary for the clk-scmi.c changes, only
>> serve to support the
>> DT binding `assigned-clock-sscs`, which, as Krzysztof also reiterated:
>>
>> https://github.com/devicetree-org/dt-schema/pull/154
>>
>> you should have proposed during the review of series [1]. You are the
>> NXP reviewer.
>>
>> >
>> > If you think it is not fair, I could drop this patch in V3 and leave it to you to handle.
>> > I take this patch in the patchset, mainly to ease your work and make
>>
>> Sorry for quoting Krzysztof again, but:
>> "Three months iMX8 patchsets, multiple reviews and no single comment
>> from you till January!"
>>
>> So please, if you really want to ease my work, then remove this patch
>> from this series and resume
>> reviewing series [1].
>>
>
>I had complained once in the past. I am repeating that again. You are not
>new to the kernel development, yet at times I get really surprised with
>the way you manage your patches and create so much confusion. It gets
>extremely difficult to track what is happening if one doesn't follow all
>your patches for a week(week is too lenient IMO, you manage sometime to
>create same amount of confusion in just 2 days).

V2 is actually 2 weeks after V1. So after addressing the comments
from Stephen and Dan, also updated clk-scmi.c to use a non-vendor
changes, I posted out V2.

2 days, this is just after got Cristian's comments. Then I posted V2.
I try to follow your working style on handling scmi patches, but seems you are
not active, so I mainly count on Cristian's comments and update patches.

The i.MX pll patches in V2 is orthogonal to clk scmi, I did not expect
complains. But ...

>
>And as usually you ignore merge window and post a whole set of new series
>on the first day of merge window. Which is fine especially if you are new
>to kernel development(not true in your case though) or even otherwise if
>you don't regularly track upstream cycle so much because of corporate
>commitments(which may be true in your case and I am fine with that). But
>you need to wait at-least a few days after the merge window so you give
>every one a chance to follow your work.

In my view, maintainers have patchwork to maintain patches. patches send
out in merge window will not be reviewed in short time or surely not
picked up, I understand this. patches could just be marked new in patchwork.
If new version is out, old version just marked as not apply.
And I use b4 to manage patchset, and each revision has changelog.

Indeed I not track merge window since I am not maintainer role. I was
not aware this would introduce complain (: I will track the cycle
in following days.

>
>And in this case, I would have avoided scmi changes are you have non-scmi
>specific driver to get the core clock changes review first and then added
>SCMI as it is OEM specific and we need to analyse it without other things
>in flux or under discussion.

the pll changes is orthogonal to clk scmi.

it is just like common changes + several driver changes in a patchset
and this is just an early version (V2). Such as people use devm_x to
simplify various drivers.
1. Introduce devm_x
2. driver1: use devm_x
3. driver2: use devm_x

2 and 3 not impact each other.

My initial idea to introduce pll changes in V2 is to ease Dario's work,
but seems not. Not expect that would introduce confusion to you.

The goal is still to use assigned-clock-sscs and let clk scmi support
spread spectrum.

Thanks,
Peng.

>
>--
>Regards,
>Sudeep