| Message ID | 20240703-of_property_for_each_u32-v1-0-42c1fc0b82aa@bootlin.com |
|---|---|
| Headers | show |
| Series | Simplify of_property_for_each_u32() | expand |
On 03/07/2024 11:37, Luca Ceresoli wrote: > Simplify code using of_property_for_each_u32_new() as the two additional > parameters in of_property_for_each_u32() are not used here. > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > --- > sound/soc/codecs/arizona.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c > index 7434aeeda292..1a64b9815809 100644 > --- a/sound/soc/codecs/arizona.c > +++ b/sound/soc/codecs/arizona.c > @@ -2786,15 +2786,13 @@ int arizona_of_get_audio_pdata(struct arizona *arizona) > { > struct arizona_pdata *pdata = &arizona->pdata; > struct device_node *np = arizona->dev->of_node; > - struct property *prop; > - const __be32 *cur; > u32 val; > u32 pdm_val[ARIZONA_MAX_PDM_SPK]; > int ret; > int count = 0; > > count = 0; > - of_property_for_each_u32(np, "wlf,inmode", prop, cur, val) { > + of_property_for_each_u32_new(np, "wlf,inmode", val) { > if (count == ARRAY_SIZE(pdata->inmode)) > break; > > @@ -2803,7 +2801,7 @@ int arizona_of_get_audio_pdata(struct arizona *arizona) > } > > count = 0; > - of_property_for_each_u32(np, "wlf,dmic-ref", prop, cur, val) { > + of_property_for_each_u32_new(np, "wlf,dmic-ref", val) { > if (count == ARRAY_SIZE(pdata->dmic_ref)) > break; > > @@ -2812,7 +2810,7 @@ int arizona_of_get_audio_pdata(struct arizona *arizona) > } > > count = 0; > - of_property_for_each_u32(np, "wlf,out-mono", prop, cur, val) { > + of_property_for_each_u32_new(np, "wlf,out-mono", val) { > if (count == ARRAY_SIZE(pdata->out_mono)) > break; > > @@ -2821,7 +2819,7 @@ int arizona_of_get_audio_pdata(struct arizona *arizona) > } > > count = 0; > - of_property_for_each_u32(np, "wlf,max-channels-clocked", prop, cur, val) { > + of_property_for_each_u32_new(np, "wlf,max-channels-clocked", val) { > if (count == ARRAY_SIZE(pdata->max_channels_clocked)) > break; > > @@ -2830,7 +2828,7 @@ int arizona_of_get_audio_pdata(struct arizona *arizona) > } > > count = 0; > - of_property_for_each_u32(np, "wlf,out-volume-limit", prop, cur, val) { > + of_property_for_each_u32_new(np, "wlf,out-volume-limit", val) { > if (count == ARRAY_SIZE(pdata->out_vol_limit)) > break; > > Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>
On Wed, 03 Jul 2024 12:36:49 +0200 Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: > Simplify code using of_property_for_each_u32_new() as the two additional > parameters in of_property_for_each_u32() are not used here. > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > drivers/clk/sunxi/clk-sun8i-bus-gates.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/clk/sunxi/clk-sun8i-bus-gates.c b/drivers/clk/sunxi/clk-sun8i-bus-gates.c > index b87f331f63c9..21f036457a86 100644 > --- a/drivers/clk/sunxi/clk-sun8i-bus-gates.c > +++ b/drivers/clk/sunxi/clk-sun8i-bus-gates.c > @@ -24,11 +24,9 @@ static void __init sun8i_h3_bus_gates_init(struct device_node *node) > const char *parents[PARENT_MAX]; > struct clk_onecell_data *clk_data; > const char *clk_name; > - struct property *prop; > struct resource res; > void __iomem *clk_reg; > void __iomem *reg; > - const __be32 *p; > int number, i; > u8 clk_bit; > int index; > @@ -58,7 +56,7 @@ static void __init sun8i_h3_bus_gates_init(struct device_node *node) > goto err_free_data; > > i = 0; > - of_property_for_each_u32(node, "clock-indices", prop, p, index) { > + of_property_for_each_u32_new(node, "clock-indices", index) { > of_property_read_string_index(node, "clock-output-names", > i, &clk_name); > >
On Wed, Jul 03, 2024 at 12:36:44PM +0200, Luca Ceresoli wrote: > [Note: to reduce the noise I have trimmed the get_maintainers list > manually. Should you want to be removed, or someone else added, to future > versions, just tell me. Sorry for the noise.] > > This series aims at simplifying of_property_for_each_u32() as well as > making it more difficult to misuse it in the future. > > The long-term goal is changing this pattern: > > struct property *prop; > const __be32 *p; > u32 val; > > of_property_for_each_u32(np, "xyz", prop, p, val) { ... } > > to this: > > u32 val; > > of_property_for_each_u32(np, "xyz", val) { ... } > > So, removing the 3rd and 4th arguments which are typically meant to be > internal. Those two parameters used to be unavoidable until the kernel > moved to building with the C11 standard unconditionally. Since then, it is > now possible to get rid of them. However a few users of > of_property_for_each_u32() do actually use those arguments, which > complicates the transition. For this reason this series does the following: > > * Add of_property_for_each_u32_new(), which does not have those two > arguments (patch 1) > * Convert _almost_ every usage to of_property_for_each_u32_new() > * Rename of_property_for_each_u32() to of_property_for_each_u32_old() and > deprecate it, as a incentive to code not (yet) in mainline to upgrade > to the *_new() version (last patch) I don't really see the point of introducing the _old variant. Let's get this done in one step. > > The plan for the next series is to additionally: > > * Convert the few remaining of_property_for_each_u32_old() instantes to > of_property_for_each_u32_new() > * Remove of_property_for_each_u32_old() > * Rename of_property_for_each_u32_new() to of_property_for_each_u32() Honestly, I think there's few enough users we could just convert the whole thing in one patch. It's all got to go thru 1 tree anyways. If there's new cases in -next, then I'd be happy to send it to Linus at the end of the merge window. Rob
Hello Rob, On Wed, 3 Jul 2024 12:07:42 -0600 Rob Herring <robh@kernel.org> wrote: > On Wed, Jul 03, 2024 at 12:36:44PM +0200, Luca Ceresoli wrote: > > [Note: to reduce the noise I have trimmed the get_maintainers list > > manually. Should you want to be removed, or someone else added, to future > > versions, just tell me. Sorry for the noise.] > > > > This series aims at simplifying of_property_for_each_u32() as well as > > making it more difficult to misuse it in the future. > > > > The long-term goal is changing this pattern: > > > > struct property *prop; > > const __be32 *p; > > u32 val; > > > > of_property_for_each_u32(np, "xyz", prop, p, val) { ... } > > > > to this: > > > > u32 val; > > > > of_property_for_each_u32(np, "xyz", val) { ... } > > > > So, removing the 3rd and 4th arguments which are typically meant to be > > internal. Those two parameters used to be unavoidable until the kernel > > moved to building with the C11 standard unconditionally. Since then, it is > > now possible to get rid of them. However a few users of > > of_property_for_each_u32() do actually use those arguments, which > > complicates the transition. For this reason this series does the following: > > > > * Add of_property_for_each_u32_new(), which does not have those two > > arguments (patch 1) > > * Convert _almost_ every usage to of_property_for_each_u32_new() > > * Rename of_property_for_each_u32() to of_property_for_each_u32_old() and > > deprecate it, as a incentive to code not (yet) in mainline to upgrade > > to the *_new() version (last patch) > > I don't really see the point of introducing the _old variant. Let's get > this done in one step. > > > > > The plan for the next series is to additionally: > > > > * Convert the few remaining of_property_for_each_u32_old() instantes to > > of_property_for_each_u32_new() > > * Remove of_property_for_each_u32_old() > > * Rename of_property_for_each_u32_new() to of_property_for_each_u32() > > Honestly, I think there's few enough users we could just convert the > whole thing in one patch. It's all got to go thru 1 tree anyways. If > there's new cases in -next, then I'd be happy to send it to Linus at the > end of the merge window. Sure, make sense. I'll need to convert the few remaining users, then I'm sending a squashed v2. Luca
[Note: to reduce the noise I have trimmed the get_maintainers list manually. Should you want to be removed, or someone else added, to future versions, just tell me. Sorry for the noise.] This series aims at simplifying of_property_for_each_u32() as well as making it more difficult to misuse it in the future. The long-term goal is changing this pattern: struct property *prop; const __be32 *p; u32 val; of_property_for_each_u32(np, "xyz", prop, p, val) { ... } to this: u32 val; of_property_for_each_u32(np, "xyz", val) { ... } So, removing the 3rd and 4th arguments which are typically meant to be internal. Those two parameters used to be unavoidable until the kernel moved to building with the C11 standard unconditionally. Since then, it is now possible to get rid of them. However a few users of of_property_for_each_u32() do actually use those arguments, which complicates the transition. For this reason this series does the following: * Add of_property_for_each_u32_new(), which does not have those two arguments (patch 1) * Convert _almost_ every usage to of_property_for_each_u32_new() * Rename of_property_for_each_u32() to of_property_for_each_u32_old() and deprecate it, as a incentive to code not (yet) in mainline to upgrade to the *_new() version (last patch) The plan for the next series is to additionally: * Convert the few remaining of_property_for_each_u32_old() instantes to of_property_for_each_u32_new() * Remove of_property_for_each_u32_old() * Rename of_property_for_each_u32_new() to of_property_for_each_u32() Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> --- Luca Ceresoli (20): of: add of_property_for_each_u32_new() clk: convert to of_property_for_each_u32_new() clk: qcom: convert to of_property_for_each_u32_new() clk: sunxi: clk-simple-gates: convert to of_property_for_each_u32_new() clk: sunxi: clk-sun8i-bus-gates: convert to of_property_for_each_u32_new() clocksource/drivers/samsung_pwm: convert to of_property_for_each_u32_new() bus: ti-sysc: convert to of_property_for_each_u32_new() lk: clk-conf: convert to of_property_for_each_u32_new() gpio: brcmstb: convert to of_property_for_each_u32_new() pinctrl: s32cc: convert to of_property_for_each_u32_new() irqchip/atmel-aic: convert to of_property_for_each_u32_new() iio: adc: ti_am335x_adc: convert to of_property_for_each_u32_new() pwm: samsung: convert to of_property_for_each_u32_new() tty: sysrq: convert to of_property_for_each_u32_new() usb: usb251xb: convert to of_property_for_each_u32_new() mfd: ti_am335x_tscadc: convert to of_property_for_each_u32_new() ASoC: arizona: convert to of_property_for_each_u32_new() powerpc/xive: convert to of_property_for_each_u32_new() powerpc/xive: convert to of_property_for_each_u32_new() of: deprecate and rename of_property_for_each_u32() .clang-format | 3 ++- arch/powerpc/sysdev/xive/native.c | 4 +--- arch/powerpc/sysdev/xive/spapr.c | 3 +-- drivers/bus/ti-sysc.c | 4 +--- drivers/clk/clk-conf.c | 4 +--- drivers/clk/clk-si5351.c | 4 ++-- drivers/clk/clk.c | 6 ++---- drivers/clk/qcom/common.c | 4 +--- drivers/clk/sunxi/clk-simple-gates.c | 4 +--- drivers/clk/sunxi/clk-sun8i-bus-gates.c | 4 +--- drivers/clocksource/samsung_pwm_timer.c | 4 +--- drivers/gpio/gpio-brcmstb.c | 5 +---- drivers/iio/adc/ti_am335x_adc.c | 4 +--- drivers/irqchip/irq-atmel-aic-common.c | 4 +--- drivers/irqchip/irq-pic32-evic.c | 2 +- drivers/mfd/ti_am335x_tscadc.c | 4 +--- drivers/pinctrl/nxp/pinctrl-s32cc.c | 4 +--- drivers/pinctrl/pinctrl-k210.c | 2 +- drivers/pwm/pwm-samsung.c | 4 +--- drivers/tty/sysrq.c | 4 +--- drivers/usb/misc/usb251xb.c | 4 +--- include/linux/of.h | 14 ++++++++++---- sound/soc/codecs/arizona.c | 12 +++++------- 23 files changed, 39 insertions(+), 68 deletions(-) --- base-commit: e937d48ed96381e9620d9c81fbc1ce666f5b7358 change-id: 20240701-of_property_for_each_u32-460fd02a5d0c Best regards,