On Wed 24 Jan 2024 at 10:48, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > [[PGP Signed Part:Undecided]] > Hello Jerome, > > On Wed, Jan 24, 2024 at 10:16:17AM +0100, Jerome Brunet wrote: >> On Wed 24 Jan 2024 at 10:02, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: >> > On Fri, Dec 22, 2023 at 12:16:53PM +0100, Jerome Brunet wrote: >> >> @@ -442,6 +439,13 @@ static int meson_pwm_init_channels(struct device *dev) >> >> struct meson_pwm_channel *channel = &meson->channels[i]; >> >> struct clk_parent_data div_parent = {}, gate_parent = {}; >> >> struct clk_init_data init = {}; >> >> + struct clk_divider *div; >> >> + struct clk_gate *gate; >> >> + struct clk_mux *mux; >> >> + >> >> + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL); >> >> + if (!mux) >> >> + return -ENOMEM; >> > >> > I don't like this change. While it doesn't increase the memory used, it >> > fragments the used memory and increases the overhead of memory >> > management and the number of devm allocations. >> > >> > Are these members of meson_pwm_channel in the way for anything later? >> >> Not really. It is just not useful on the SoCs which do use it and not >> used at all starting from s4/a1. > > This remembers me about the old pwm-imx driver. This was essentially a > single file containing two drivers just because both types appeared on > imx machines. Later it was split into imx1 and imx27. > > I didn't look at the relevant differences between the existing driver > and the changes needed for s4, but please don't repeat this issue for > meson. Not sure this fear is justified, just saying ... Noted. Don't worry. s4 is indeed the same PWM block as before, just mux/div/gate migrated from the pwm IP to the main clk controller. That's all ... I know ;) Only the clock registration should change and simplify. > > Best regards > Uwe -- Jerome