On 27/02/2020 14:36, Geert Uytterhoeven wrote:

Hi Marc,

Thanks for your patch!

On Wed, Feb 26, 2020 at 4:55 PM Marc Gonzalez [off-list ref] wrote:

quoted

Using the helper produces simpler code, and smaller object size.
E.g. with gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu:

    text           data     bss     dec     hex filename
-   1708             80       0    1788     6fc drivers/clk/clk-devres.o
+   1524             80       0    1604     644 drivers/clk/clk-devres.o

And the size reduction could have been even more ;-)

I'll see what I can do! ;-)

I have another patch with even smaller object code, but it requires
C11 to be well-defined (memcmp the whole struct, which requires zeros
in the padding holes).

quoted

--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c

quoted

@@ -55,25 +51,17 @@ static void devm_clk_bulk_release(struct device *dev, void *res)
 static int __devm_clk_bulk_get(struct device *dev, int num_clks,
                               struct clk_bulk_data *clks, bool optional)
 {
-       struct clk_bulk_devres *devres;
        int ret;

-       devres = devres_alloc(devm_clk_bulk_release,
-                             sizeof(*devres), GFP_KERNEL);
-       if (!devres)
-               return -ENOMEM;
-
        if (optional)
                ret = clk_bulk_get_optional(dev, num_clks, clks);
        else
                ret = clk_bulk_get(dev, num_clks, clks);
-       if (!ret) {
-               devres->clks = clks;
-               devres->num_clks = num_clks;
-               devres_add(dev, devres);
-       } else {
-               devres_free(devres);
-       }
+
+       if (ret)
+               return ret;
+
+       ret = devm_vadd(dev, my_clk_bulk_put, clk_bulk_args, num_clks, clks);

        return ret;

return devm_vadd(...);

If you think that makes it look better, I'll make the change!

quoted

@@ -128,30 +109,22 @@ static int devm_clk_match(struct device *dev, void *res, void *data)

 void devm_clk_put(struct device *dev, struct clk *clk)
 {
-       int ret;
-
-       ret = devres_release(dev, devm_clk_release, devm_clk_match, clk);
-
-       WARN_ON(ret);
+       WARN_ON(devres_release(dev, my_clk_put, devm_clk_match, clk));

Getting rid of "ret" is an unrelated change, which actually increases
kernel size, as the WARN_ON() parameter is stringified for the warning
message.

Weird... Are you sure about that? I built the preprocessed file,
and it didn't appear to be so.

#ifndef WARN_ON
#define WARN_ON(condition) ({						\
	int __ret_warn_on = !!(condition);				\
	if (unlikely(__ret_warn_on))					\
		__WARN();						\
	unlikely(__ret_warn_on);					\
})
#endif

Maybe you were thinking of i915's WARN_ON?

#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")")

Regards.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel