| 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, Jul 3, 2024 at 12:38 PM 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> > --- > drivers/gpio/gpio-brcmstb.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c > index 8dce78ea7139..77557bc596cd 100644 > --- a/drivers/gpio/gpio-brcmstb.c > +++ b/drivers/gpio/gpio-brcmstb.c > @@ -591,8 +591,6 @@ static int brcmstb_gpio_probe(struct platform_device *pdev) > void __iomem *reg_base; > struct brcmstb_gpio_priv *priv; > struct resource *res; > - struct property *prop; > - const __be32 *p; > u32 bank_width; > int num_banks = 0; > int num_gpios = 0; > @@ -636,8 +634,7 @@ static int brcmstb_gpio_probe(struct platform_device *pdev) > flags = BGPIOF_BIG_ENDIAN_BYTE_ORDER; > #endif > > - of_property_for_each_u32(np, "brcm,gpio-bank-widths", prop, p, > - bank_width) { > + of_property_for_each_u32_new(np, "brcm,gpio-bank-widths", bank_width) { > struct brcmstb_gpio_bank *bank; > struct gpio_chip *gc; > > > -- > 2.34.1 > Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Luca Ceresoli <luca.ceresoli@bootlin.com> writes: > Simplify code using of_property_for_each_u32_new() as the two additional > parameters in of_property_for_each_u32() are not used here. > > In this case only the 'prop' variable can be removed and not 'reg', > because 'reg' is used in _previous_ lines of the same function. There > is no side effect because the of_property_for_each_u32() macro being > removed would anyway write 'reg' bwfore reading its value, and 'reg' > is not used in _following_ lines. > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > --- > arch/powerpc/sysdev/xive/spapr.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) cheers
On Wed, Jul 03, 2024 at 12:37:01PM +0200, 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. Acked-by: Mark Brown <broonie@kernel.org>
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, 03 Jul 2024 12:36:48 +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-simple-gates.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c > index 0399627c226a..a30d14937e0b 100644 > --- a/drivers/clk/sunxi/clk-simple-gates.c > +++ b/drivers/clk/sunxi/clk-simple-gates.c > @@ -21,11 +21,9 @@ static void __init sunxi_simple_gates_setup(struct device_node *node, > { > struct clk_onecell_data *clk_data; > const char *clk_parent, *clk_name; > - struct property *prop; > struct resource res; > void __iomem *clk_reg; > void __iomem *reg; > - const __be32 *p; > int number, i = 0, j; > u8 clk_bit; > u32 index; > @@ -47,7 +45,7 @@ static void __init sunxi_simple_gates_setup(struct device_node *node, > if (!clk_data->clks) > goto err_free_data; > > - 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 03/07/2024 at 12:36, 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> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> > --- > drivers/irqchip/irq-atmel-aic-common.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c > index 072bd227b6c6..543ea249df53 100644 > --- a/drivers/irqchip/irq-atmel-aic-common.c > +++ b/drivers/irqchip/irq-atmel-aic-common.c > @@ -111,8 +111,6 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain) > struct device_node *node = irq_domain_get_of_node(domain); > struct irq_chip_generic *gc; > struct aic_chip_data *aic; > - struct property *prop; > - const __be32 *p; > u32 hwirq; > > gc = irq_get_domain_generic_chip(domain, 0); > @@ -120,7 +118,7 @@ static void __init aic_common_ext_irq_of_init(struct irq_domain *domain) > aic = gc->private; > aic->ext_irqs |= 1; > > - of_property_for_each_u32(node, "atmel,external-irqs", prop, p, hwirq) { > + of_property_for_each_u32_new(node, "atmel,external-irqs", hwirq) { > gc = irq_get_domain_generic_chip(domain, hwirq); > if (!gc) { > pr_warn("AIC: external irq %d >= %d skip it\n", > > -- > 2.34.1 >
On Wed, 03 Jul 2024 12:36:56 +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> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
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, On Wed, Jul 03, 2024 at 12:36:57PM +0200, 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> > --- > drivers/pwm/pwm-samsung.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c > index efb60c9f0cb3..fef02a0b023e 100644 > --- a/drivers/pwm/pwm-samsung.c > +++ b/drivers/pwm/pwm-samsung.c > @@ -510,8 +510,6 @@ static int pwm_samsung_parse_dt(struct pwm_chip *chip) > struct samsung_pwm_chip *our_chip = to_samsung_pwm_chip(chip); > struct device_node *np = pwmchip_parent(chip)->of_node; > const struct of_device_id *match; > - struct property *prop; > - const __be32 *cur; > u32 val; > > match = of_match_node(samsung_pwm_matches, np); > @@ -520,7 +518,7 @@ static int pwm_samsung_parse_dt(struct pwm_chip *chip) > > memcpy(&our_chip->variant, match->data, sizeof(our_chip->variant)); > > - of_property_for_each_u32(np, "samsung,pwm-outputs", prop, cur, val) { > + of_property_for_each_u32_new(np, "samsung,pwm-outputs", val) { > if (val >= SAMSUNG_PWM_NUM) { > dev_err(pwmchip_parent(chip), > "%s: invalid channel index in samsung,pwm-outputs property\n", > Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Feel free to merge this together with the change from the first patch. Best regards Uwe
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
Hi Luca, On Wed, Jul 03, 2024 at 12:36:59PM GMT, 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> please feel free to add: Acked-by: Richard Leitner <richard.leitner@linux.dev> > --- > drivers/usb/misc/usb251xb.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > index 7da404f55a6d..490004dbb9d8 100644 > --- a/drivers/usb/misc/usb251xb.c > +++ b/drivers/usb/misc/usb251xb.c > @@ -382,11 +382,9 @@ static void usb251xb_get_ports_field(struct usb251xb *hub, > bool ds_only, u8 *fld) > { > struct device *dev = hub->dev; > - struct property *prop; > - const __be32 *p; > u32 port; > > - of_property_for_each_u32(dev->of_node, prop_name, prop, p, port) { > + of_property_for_each_u32_new(dev->of_node, prop_name, port) { > if ((port >= ds_only ? 1 : 0) && (port <= port_cnt)) > *fld |= BIT(port); > else > > -- > 2.34.1
[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,