Re: [PATCH v6 2/3] clk: cs2600: Add Fractional-N clock driver

[Richard Fitzgerald]

On 07/07/2026 4:58 pm, Brian Masney wrote:
> Hi Richard,
>
> Thanks for the patch.
>
> On Tue, Jun 30, 2026 at 04:55:48PM +0100, Richard Fitzgerald wrote:
> > From: Paul Handrigan <paulha@xxxxxxxxxxxxxxxxxxxxx>
> >
> > Add driver for the Cirrus Logic CS2600 fractional-N clock synthesizer
> > and multiplier.

<SNIP>

> > +
> > +DEFINE_GUARD(cs2600_unfreeze, struct cs2600 *,
> > +	     cs2600_clear_freeze(_T), cs2600_set_freeze(_T))
>
> Can you go through the Sashiko output for this series?

I sent an email responding to those, but it was reply-all to the Sashiko
bot email and I didn't notice that Sashiko had stripped a lot of the
original email recipients.

My responses to Sashiko are here:
https://lore.kernel.org/all/dc5a1c0c-8c6b-4f4b-90f1-e9c83a8b91b8@xxxxxxxxxxxxxxxxxxxxx/


> https://sashiko.dev/#/message/20260630161121.020FB1F000E9%40smtp.kernel.org
>
> The clear/set inverted looks questionable to me. If this is expected,
> then a comment clarifying why should be added.

I'll add a comment in the next version explaining the order.

> [snip]
>
> > +static int cs2600_clk_register(struct cs2600 *cs2600)
> > +{
> > +	static const char *parent_names[2];
>
> I wouldn't worry too about about Sashiko asking about multiple driver
> instances for this section, unless you think it will eventually be used
> in that way.

Trivial to fix though. That 'static' shouldn't be there.

> > +	const struct clk_hw *out_clocks_parent;
> > +	struct clk_init_data init[ARRAY_SIZE(cs2600->hw)] = { };
> > +	int ret, i, n, input_num = 0;
> > +	const char *names[ARRAY_SIZE(cs2600_clks)];
>
> Reverse Christmas tree order.

Will do.

> I can do a more thorough review once you go through the Sashiko results.
>
> Brian