On 18/01/2025 06:53, Marek Vasut wrote:
> Introduce mdio_init() split off from mdio_alloc(), which is used
> to initialize already allocated struct mii_dev.
> 
> Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org>

Replying here for the whole patch series...

I like where we get to at the end of this series, but I don't like the
intermediate state where the bb_miiphy_buses arrays still exist but are
unused. I also think we should introduce a new ops struct so we're not
duplicating function pointers in each instance of struct bb_miiphy_bus.

I would prefer the following order of changes, let me know if you think
this would be cleaner:

1) Introduce mdio_init().

2) Add mii member to struct bb_miiphy_bus.

3) Modify each driver to use the mii member of the appropriate struct
   bb_miiphy_bus instance, calling mdio_init() instead of mdio_alloc().
   At this point we're still using the instances in the statically
   allocated bb_miiphy_buses array.

4) Use container_of() in bb_miiphy_getbus().

5) Drop global names bb_miiphy_buses and bb_miiphy_buses_num as these
   should no longer be used. The bb_miiphy_buses arrays can be static in
   each driver and at this point the miiphy code should not care whether
   the arrays are statically or dynamically allocated.

6) Drop name member of struct bb_miiphy_bus now that it is unused.

7) arm: mvebu: a38x: Call bb_miiphy init directly in driver probe.

8) Drop bb_miiphy_init() and .init callbacks.

9) Move function pointers to a new ops struct so we're not duplicating
   them in each instance of struct bb_miiphy_bus:

        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 {
                struct bb_miiphy_ops *ops;
                struct mii_dev mii;
                void *priv;
        };

10) Introduce bb_miiphy_alloc()/bb_miiphy_free().

11) Convert each driver from a static array of struct bb_miiphy_bus
    instances to dynamically allocated instances, for each driver
    removing the static array in the same commit that dynamic allocation
    is added.

Thanks,

-- 
Paul Barker

Attachment: OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

Reply via email to