Re: [PATCH V2 1/2] rust: Add clk helpers

[Daniel Almeida <daniel.almeida@collabora.com>]

Hi Viresh,

> On 21 Feb 2025, at 03:33, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> In order to prepare for adding Rust abstractions for the clock APIs,
> this patch adds clock helpers required by the Rust implementation.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> MAINTAINERS                     |  1 +
> rust/bindings/bindings_helper.h |  1 +
> rust/helpers/clk.c              | 57 +++++++++++++++++++++++++++++++++
> rust/helpers/helpers.c          |  1 +
> 4 files changed, 60 insertions(+)
> create mode 100644 rust/helpers/clk.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 25c86f47353d..726110d3c988 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5778,6 +5778,7 @@ F: include/dt-bindings/clock/
> F: include/linux/clk-pr*
> F: include/linux/clk/
> F: include/linux/of_clk.h
> +F: rust/helpers/clk.c
> X: drivers/clk/clkdev.c
> 
> COMMON INTERNET FILE SYSTEM CLIENT (CIFS and SMB3)
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 55354e4dec14..4e4e16c3b479 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -10,6 +10,7 @@
> #include <linux/blk-mq.h>
> #include <linux/blk_types.h>
> #include <linux/blkdev.h>
> +#include <linux/clk.h>
> #include <linux/cred.h>
> #include <linux/errname.h>
> #include <linux/ethtool.h>
> diff --git a/rust/helpers/clk.c b/rust/helpers/clk.c
> new file mode 100644
> index 000000000000..3c63b50ad6fb
> --- /dev/null
> +++ b/rust/helpers/clk.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/clk.h>
> +
> +#ifndef CONFIG_HAVE_CLK

This is a bit confusing. Can you add a comment explaining how
we get inlined stubs if these configs are not set, thus explaining why
we need to define them in helpers.c?

This will let everyone know why we have #ifndef here when the logical thing
would be #ifdef. It will also make it clear why the rust code is not gated
by these configs.

> +struct clk *rust_helper_clk_get(struct device *dev, const char *id)
> +{
> + return clk_get(dev, id);
> +}
> +
> +void rust_helper_clk_put(struct clk *clk)
> +{
> + clk_put(clk);
> +}
> +
> +int rust_helper_clk_enable(struct clk *clk)
> +{
> + return clk_enable(clk);
> +}
> +
> +void rust_helper_clk_disable(struct clk *clk)
> +{
> + clk_disable(clk);
> +}
> +
> +unsigned long rust_helper_clk_get_rate(struct clk *clk)
> +{
> + return clk_get_rate(clk);
> +}
> +
> +int rust_helper_clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> + return clk_set_rate(clk, rate);
> +}
> +#endif
> +
> +#ifndef CONFIG_HAVE_CLK_PREPARE

Same here.

> +int rust_helper_clk_prepare(struct clk *clk)
> +{
> + return clk_prepare(clk);
> +}
> +
> +void rust_helper_clk_unprepare(struct clk *clk)
> +{
> + clk_unprepare(clk);
> +}
> +#endif
> +
> +int rust_helper_clk_prepare_enable(struct clk *clk)
> +{
> + return clk_prepare_enable(clk);
> +}
> +
> +void rust_helper_clk_disable_unprepare(struct clk *clk)
> +{
> + clk_disable_unprepare(clk);
> +}
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0640b7e115be..4700ee7aaf85 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -11,6 +11,7 @@
> #include "bug.c"
> #include "build_assert.c"
> #include "build_bug.c"
> +#include "clk.c"
> #include "cred.c"
> #include "device.c"
> #include "err.c"
> -- 
> 2.31.1.272.g89b43f80a514
> 
> 

With this change,

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>