linux-kernel - Re: [RFC PATCH] component: Add common helpers for compare/release functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Jan 2022 13:04:07 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Yong Wu <yong.wu@...iatek.com>, dri-devel@...ts.freedesktop.org
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        James Wang <james.qian.wang@....com>,
        Liviu Dudau <liviu.dudau@....com>,
        iommu@...ts.linux-foundation.org,
        Matthias Brugger <matthias.bgg@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        linux-kernel@...r.kernel.org, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>, Arnd Bergmann <arnd@...db.de>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        linux-mediatek@...ts.infradead.org,
        Chun-Kuang Hu <chunkuang.hu@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        Stephen Boyd <sboyd@...nel.org>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Tomasz Figa <tfiga@...omium.org>, srv_heupstream@...iatek.com,
        Rob Clark <robdclark@...il.com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Sebastian Reichel <sre@...nel.org>
Subject: Re: [RFC PATCH] component: Add common helpers for compare/release
 functions

On 2022-01-28 08:11, Yong Wu wrote:
[...]
> diff --git a/include/linux/component.h b/include/linux/component.h
> index 16de18f473d7..5a7468ea827c 100644
> --- a/include/linux/component.h
> +++ b/include/linux/component.h
> @@ -2,6 +2,8 @@
>   #ifndef COMPONENT_H
>   #define COMPONENT_H
>   
> +#include <linux/device.h>
> +#include <linux/of.h>
>   #include <linux/stddef.h>
>   
>   
> @@ -82,6 +84,22 @@ struct component_master_ops {
>   	void (*unbind)(struct device *master);
>   };
>   
> +/* A set common helpers for compare/release functions */
> +static inline int compare_of(struct device *dev, void *data)
> +{
> +	return dev->of_node == data;
> +}

Note that this is effectively just device_match_of_node(), although I 
guess there is an argument that having a nice consistent set of 
component_match API helpers might be worth more than a tiny code saving 
by borrowing one from a different API.

Either way, however, I don't think there's any good argument for 
instantiating separate copies of these functions in every driver that 
uses them. If they're used as callbacks then they can't actually be 
inlined anyway, so they may as well be exported from component.c as 
normal so that the code really is shared (plus then there's nice 
symmetry with the aforementioned device_match API helpers too).

Thanks,
Robin.

> +static inline void release_of(struct device *dev, void *data)
> +{
> +	of_node_put(data);
> +}
> +
> +static inline int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == data;
> +}
> +
>   void component_master_del(struct device *,
>   	const struct component_master_ops *);
>   
> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> index eff200a07d9f..992132cbfb9f 100644
> --- a/sound/soc/codecs/wcd938x.c
> +++ b/sound/soc/codecs/wcd938x.c
> @@ -4417,16 +4417,6 @@ static const struct component_master_ops wcd938x_comp_ops = {
>   	.unbind = wcd938x_unbind,
>   };
>   
> -static int wcd938x_compare_of(struct device *dev, void *data)
> -{
> -	return dev->of_node == data;
> -}
> -
> -static void wcd938x_release_of(struct device *dev, void *data)
> -{
> -	of_node_put(data);
> -}
> -
>   static int wcd938x_add_slave_components(struct wcd938x_priv *wcd938x,
>   					struct device *dev,
>   					struct component_match **matchptr)
> @@ -4442,8 +4432,7 @@ static int wcd938x_add_slave_components(struct wcd938x_priv *wcd938x,
>   	}
>   
>   	of_node_get(wcd938x->rxnode);
> -	component_match_add_release(dev, matchptr, wcd938x_release_of,
> -				    wcd938x_compare_of,	wcd938x->rxnode);
> +	component_match_add_release(dev, matchptr, release_of, compare_of, wcd938x->rxnode);
>   
>   	wcd938x->txnode = of_parse_phandle(np, "qcom,tx-device", 0);
>   	if (!wcd938x->txnode) {
> @@ -4451,8 +4440,7 @@ static int wcd938x_add_slave_components(struct wcd938x_priv *wcd938x,
>   		return -ENODEV;
>   	}
>   	of_node_get(wcd938x->txnode);
> -	component_match_add_release(dev, matchptr, wcd938x_release_of,
> -				    wcd938x_compare_of,	wcd938x->txnode);
> +	component_match_add_release(dev, matchptr, release_of, compare_of, wcd938x->txnode);
>   	return 0;
>   }
>