On 09/02/2025 12:01, Marek Vasut wrote:
> Allocate bb_miiphy using bb_miiphy_alloc() and fill in callbacks
> currently listed in bb_miiphy_buses[] array. This is a temporary
> duplication of assignment to avoid breakage, which will be removed
> in follow up patches. At this point, the bb_miiphy callbacks can
> reach these accessors by doing container_of() on struct mii_dev.
> 
> Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org>

[snip]
 
> +     /* Copy the bus accessors, name and private data */
> +     bb_miiphy->mdio_active = ravb_bb_mdio_active;
> +     bb_miiphy->mdio_tristate = ravb_bb_mdio_tristate;
> +     bb_miiphy->set_mdio = ravb_bb_set_mdio;
> +     bb_miiphy->get_mdio = ravb_bb_get_mdio;
> +     bb_miiphy->set_mdc = ravb_bb_set_mdc;
> +     bb_miiphy->delay = ravb_bb_delay;

I would prefer that we have a separate `struct bb_miiphy_ops`, which can
be defined once (static and const) for each driver to reduce duplication
of the pointers in memory.

Something like:

        struct bb_miiphy_ops {
                int (*mdio_active)(struct bb_miiphy_bus *bus);
                int (*mdio_tristate)(struct bb_miiphy_bus *bus);
                int (*set_mdio)(struct bb_miiphy_bus *bus, int v);
                int (*get_mdio)(struct bb_miiphy_bus *bus, int *v);
                int (*set_mdc)(struct bb_miiphy_bus *bus, int v);
                int (*delay)(struct bb_miiphy_bus *bus);
        };

        struct bb_miiphy_bus {
                const struct bb_miiphy_ops *ops;
                struct mii_dev mii;
                void *priv;
        };

We can make this change as a follow-up though as this patch series
otherwise looks good.

Reviewed-by: Paul Barker <paul.barker...@bp.renesas.com>

-- 
Paul Barker

Attachment: OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

Reply via email to