Hello Alexey, On Tue, Jul 25, 2023 at 4:13 PM Alexey Romanov <avromanov@xxxxxxxxxxxxxx> wrote: [...] > static int meson_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait) > { > struct meson_rng_data *data = > container_of(rng, struct meson_rng_data, rng); > + const struct meson_rng_priv *priv = data->priv; > + > + if (priv->check_status_bit) { > + void __iomem *cfg_addr = data->base + priv->cfg_offset; > + int err; Have you considered just creating two separate functions: - meson_rng_read - meson_s4_rng_read Then you don't need all of these if/else. You can even skip the whole offset description in your new struct meson_rng_priv. All of this can make the code easier to understand. > + writel_relaxed(readl_relaxed(cfg_addr) | BIT(SEED_READY_STS_BIT), cfg_addr); Why not just #define SEED_READY_STS as BIT(...) right away? Best regards, Martin