Re: [PATCH v2 2/2] dt-bindings: clock: spacemit: CLK_SSPA_I2S_BCLK for SSPA

[Yao Zi <ziyao@disroot.org>]

On Tue, Jul 22, 2025 at 05:27:11PM +0800, Troy Mitchell wrote:
> On Tue, Jul 22, 2025 at 05:21:01PM +0800, Yixun Lan wrote:
> > Hi Troy,
> > 
> > On 15:36 Tue 22 Jul     , Troy Mitchell wrote:
> > > This patch adds macro definitions: SSPAx_I2S_BCLK,
> > this is obvious, so no need to repeat, please add something useful
> > 
> > > to introduce a dummy gate for i2s_bclk.
> >                 ~~~~~~ 'virtual'? if it isn't a real gate clock,
> > 	but I'm not sure it's a good approach to introduce such
> > 	virtual clock if underlying hw doesn't comply with this
> I'll leave this question to others.
> 
> > 
> > > 
> > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > > ---
> > >  include/dt-bindings/clock/spacemit,k1-syscon.h | 114 +++++++++++++------------
> > >  1 file changed, 58 insertions(+), 56 deletions(-)
> > dt-binding patch should always go first
> get it
> 
> > 
> > > 
> > > diff --git a/include/dt-bindings/clock/spacemit,k1-syscon.h b/include/dt-bindings/clock/spacemit,k1-syscon.h
> > > index 35968ae98246609c889eb4a7d08b4ff7360de53b..6914ccf5be45a1071d5b6eac354cacb67888e00c 100644
> > > --- a/include/dt-bindings/clock/spacemit,k1-syscon.h
> > > +++ b/include/dt-bindings/clock/spacemit,k1-syscon.h
> > > @@ -123,62 +123,64 @@
> > >  #define CLK_TIMERS2		41
> > >  #define CLK_AIB			42
> > >  #define CLK_ONEWIRE		43
> > > -#define CLK_SSPA0		44
> > > -#define CLK_SSPA1		45
> > > -#define CLK_DRO			46
> > > -#define CLK_IR			47
> > > -#define CLK_TSEN		48
> > > -#define CLK_IPC_AP2AUD		49
> > > -#define CLK_CAN0		50
> > > -#define CLK_CAN0_BUS		51
> > > -#define CLK_UART0_BUS		52
> > > -#define CLK_UART2_BUS		53
> > > -#define CLK_UART3_BUS		54
> > > -#define CLK_UART4_BUS		55
> > > -#define CLK_UART5_BUS		56
> > > -#define CLK_UART6_BUS		57
> > > -#define CLK_UART7_BUS		58
> > > -#define CLK_UART8_BUS		59
> > > -#define CLK_UART9_BUS		60
> > > -#define CLK_GPIO_BUS		61
> > > -#define CLK_PWM0_BUS		62
> > > -#define CLK_PWM1_BUS		63
> > > -#define CLK_PWM2_BUS		64
> > > -#define CLK_PWM3_BUS		65
> > > -#define CLK_PWM4_BUS		66
> > > -#define CLK_PWM5_BUS		67
> > > -#define CLK_PWM6_BUS		68
> > > -#define CLK_PWM7_BUS		69
> > > -#define CLK_PWM8_BUS		70
> > > -#define CLK_PWM9_BUS		71
> > > -#define CLK_PWM10_BUS		72
> > > -#define CLK_PWM11_BUS		73
> > > -#define CLK_PWM12_BUS		74
> > > -#define CLK_PWM13_BUS		75
> > > -#define CLK_PWM14_BUS		76
> > > -#define CLK_PWM15_BUS		77
> > > -#define CLK_PWM16_BUS		78
> > > -#define CLK_PWM17_BUS		79
> > > -#define CLK_PWM18_BUS		80
> > > -#define CLK_PWM19_BUS		81
> > > -#define CLK_SSP3_BUS		82
> > > -#define CLK_RTC_BUS		83
> > > -#define CLK_TWSI0_BUS		84
> > > -#define CLK_TWSI1_BUS		85
> > > -#define CLK_TWSI2_BUS		86
> > > -#define CLK_TWSI4_BUS		87
> > > -#define CLK_TWSI5_BUS		88
> > > -#define CLK_TWSI6_BUS		89
> > > -#define CLK_TWSI7_BUS		90
> > > -#define CLK_TWSI8_BUS		91
> > > -#define CLK_TIMERS1_BUS		92
> > > -#define CLK_TIMERS2_BUS		93
> > > -#define CLK_AIB_BUS		94
> > > -#define CLK_ONEWIRE_BUS		95
> > > -#define CLK_SSPA0_BUS		96
> > > -#define CLK_SSPA1_BUS		97
> > > -#define CLK_TSEN_BUS		98
> > > -#define CLK_IPC_AP2AUD_BUS	99
> > > +#define CLK_SSPA0_I2S_BCLK	44
> > > +#define CLK_SSPA1_I2S_BCLK	45
> > just append the clock at the end, instead of doing massive renaming
> The c file is written in the same order as the h file,
> and I don't want to disrupt this order..
> 
> So should I keep this style or just append at the end?

This isn't only a style issue. This patch breaks devicetree ABI, old
devicetree won't work with the change. Please don't modify existing
definitions but only add new ones.

>                 - Troy
> > 
> > > +#define CLK_SSPA0		46
> > > +#define CLK_SSPA1		47
> > > +#define CLK_DRO			48

Regards,
Yao Zi