| Message ID | 20231222111658.832167-4-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pwm: meson: dt-bindings fixup | expand |
Hello Jerome, On Fri, Dec 22, 2023 at 12:16:51PM +0100, Jerome Brunet wrote: > Meson8 pwm type always has 4 input clocks. Some inputs may be grounded, > like in the AO domain of some SoCs. > > Drop the parent number parameter and make this is constant. > This is also done to make addition of generic meson8 compatible easier. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/pwm/pwm-meson.c | 19 ++++++------------- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c > index 2971bbf3b5e7..ef50c337f444 100644 > --- a/drivers/pwm/pwm-meson.c > +++ b/drivers/pwm/pwm-meson.c > @@ -60,7 +60,7 @@ > #define MISC_A_EN BIT(0) > > #define MESON_NUM_PWMS 2 > -#define MESON_MAX_MUX_PARENTS 4 > +#define MESON_NUM_MUX_PARENTS 4 > > static struct meson_pwm_channel_data { > u8 reg_offset; > @@ -98,7 +98,6 @@ struct meson_pwm_channel { > > struct meson_pwm_data { > const char * const *parent_names; I suggest to make this const char *parent_names[MESON_NUM_MUX_PARENTS]; to make it more explicit that really four entries are needed here. This also makes is unnecessary to add the additional NULL entries to pwm_gxbb_ao_parent_names and the other arrays. Best regards Uwe
On Wed 24 Jan 2024 at 10:08, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > [[PGP Signed Part:Undecided]] > Hello Jerome, > > On Fri, Dec 22, 2023 at 12:16:51PM +0100, Jerome Brunet wrote: >> Meson8 pwm type always has 4 input clocks. Some inputs may be grounded, >> like in the AO domain of some SoCs. >> >> Drop the parent number parameter and make this is constant. >> This is also done to make addition of generic meson8 compatible easier. >> >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> >> --- >> drivers/pwm/pwm-meson.c | 19 ++++++------------- >> 1 file changed, 6 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c >> index 2971bbf3b5e7..ef50c337f444 100644 >> --- a/drivers/pwm/pwm-meson.c >> +++ b/drivers/pwm/pwm-meson.c >> @@ -60,7 +60,7 @@ >> #define MISC_A_EN BIT(0) >> >> #define MESON_NUM_PWMS 2 >> -#define MESON_MAX_MUX_PARENTS 4 >> +#define MESON_NUM_MUX_PARENTS 4 >> >> static struct meson_pwm_channel_data { >> u8 reg_offset; >> @@ -98,7 +98,6 @@ struct meson_pwm_channel { >> >> struct meson_pwm_data { >> const char * const *parent_names; > > I suggest to make this > > const char *parent_names[MESON_NUM_MUX_PARENTS]; Ok. > > to make it more explicit that really four entries are needed here. This > also makes is unnecessary to add the additional NULL entries to > pwm_gxbb_ao_parent_names and the other arrays. I would normally agree but I'd prefer to be explicit. There are some instance where the NULL is in the middle, this can't go away. I think it looks if some inputs are explicitly NULL while the other are implicit. Of course, it is just a preference. I can remove these if that is bothering you. > > Best regards > Uwe
On Wed, Jan 24, 2024 at 10:11:59AM +0100, Jerome Brunet wrote: > On Wed 24 Jan 2024 at 10:08, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > I suggest to make this > > > > const char *parent_names[MESON_NUM_MUX_PARENTS]; > > Ok. > > > > > to make it more explicit that really four entries are needed here. This > > also makes is unnecessary to add the additional NULL entries to > > pwm_gxbb_ao_parent_names and the other arrays. > > I would normally agree but I'd prefer to be explicit. > > There are some instance where the NULL is in the middle, this can't go > away. I think it looks if some inputs are explicitly NULL while the > other are implicit. Adding soem NULLs explicitly is fine for me. Using an array of fixed length still (somewhat) ensures that later no shorter arrays are added which result in surprises. Best reagrds Uwe
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c index 2971bbf3b5e7..ef50c337f444 100644 --- a/drivers/pwm/pwm-meson.c +++ b/drivers/pwm/pwm-meson.c @@ -60,7 +60,7 @@ #define MISC_A_EN BIT(0) #define MESON_NUM_PWMS 2 -#define MESON_MAX_MUX_PARENTS 4 +#define MESON_NUM_MUX_PARENTS 4 static struct meson_pwm_channel_data { u8 reg_offset; @@ -98,7 +98,6 @@ struct meson_pwm_channel { struct meson_pwm_data { const char * const *parent_names; - unsigned int num_parents; }; struct meson_pwm { @@ -343,7 +342,6 @@ static const char * const pwm_meson8b_parent_names[] = { static const struct meson_pwm_data pwm_meson8b_data = { .parent_names = pwm_meson8b_parent_names, - .num_parents = ARRAY_SIZE(pwm_meson8b_parent_names), }; /* @@ -351,12 +349,11 @@ static const struct meson_pwm_data pwm_meson8b_data = { * The last 2 are grounded */ static const char * const pwm_gxbb_ao_parent_names[] = { - "xtal", "clk81" + "xtal", "clk81", NULL, NULL, }; static const struct meson_pwm_data pwm_gxbb_ao_data = { .parent_names = pwm_gxbb_ao_parent_names, - .num_parents = ARRAY_SIZE(pwm_gxbb_ao_parent_names), }; static const char * const pwm_axg_ee_parent_names[] = { @@ -365,7 +362,6 @@ static const char * const pwm_axg_ee_parent_names[] = { static const struct meson_pwm_data pwm_axg_ee_data = { .parent_names = pwm_axg_ee_parent_names, - .num_parents = ARRAY_SIZE(pwm_axg_ee_parent_names), }; static const char * const pwm_axg_ao_parent_names[] = { @@ -374,7 +370,6 @@ static const char * const pwm_axg_ao_parent_names[] = { static const struct meson_pwm_data pwm_axg_ao_data = { .parent_names = pwm_axg_ao_parent_names, - .num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names), }; static const char * const pwm_g12a_ao_ab_parent_names[] = { @@ -383,16 +378,14 @@ static const char * const pwm_g12a_ao_ab_parent_names[] = { static const struct meson_pwm_data pwm_g12a_ao_ab_data = { .parent_names = pwm_g12a_ao_ab_parent_names, - .num_parents = ARRAY_SIZE(pwm_g12a_ao_ab_parent_names), }; static const char * const pwm_g12a_ao_cd_parent_names[] = { - "xtal", "g12a_ao_clk81", + "xtal", "g12a_ao_clk81", NULL, NULL, }; static const struct meson_pwm_data pwm_g12a_ao_cd_data = { .parent_names = pwm_g12a_ao_cd_parent_names, - .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), }; static const struct of_device_id meson_pwm_matches[] = { @@ -434,13 +427,13 @@ MODULE_DEVICE_TABLE(of, meson_pwm_matches); static int meson_pwm_init_channels(struct meson_pwm *meson) { - struct clk_parent_data mux_parent_data[MESON_MAX_MUX_PARENTS] = {}; + struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {}; struct device *dev = meson->chip.dev; unsigned int i; char name[255]; int err; - for (i = 0; i < meson->data->num_parents; i++) { + for (i = 0; i < MESON_NUM_MUX_PARENTS; i++) { mux_parent_data[i].index = -1; mux_parent_data[i].name = meson->data->parent_names[i]; } @@ -456,7 +449,7 @@ static int meson_pwm_init_channels(struct meson_pwm *meson) init.ops = &clk_mux_ops; init.flags = 0; init.parent_data = mux_parent_data; - init.num_parents = meson->data->num_parents; + init.num_parents = MESON_NUM_MUX_PARENTS; channel->mux.reg = meson->base + REG_MISC_AB; channel->mux.shift =
Meson8 pwm type always has 4 input clocks. Some inputs may be grounded, like in the AO domain of some SoCs. Drop the parent number parameter and make this is constant. This is also done to make addition of generic meson8 compatible easier. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/pwm/pwm-meson.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)