On 27/10/2024 16:25, Marek Vasut wrote:
> On 10/24/24 5:24 PM, Paul Barker wrote:
>> Several Renesas SoCs in the RZ/G2L family have two Ethernet interfaces.
>> To support this second interface, we extend the bb_miiphy_buses[] array
>> and keep track of the current bus index in ravb_of_to_plat().
>>
>> Support for an arbitrary number of instances is not implemented - it is
>> expected that bb_miiphy_buses will be replaced with a proper device
>> model/uclass implementation before that is needed.
>>
>> Signed-off-by: Paul Barker <paul.barker...@bp.renesas.com>
>> ---
>> drivers/net/ravb.c | 28 ++++++++++++++++++++++++----
>> 1 file changed, 24 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ravb.c b/drivers/net/ravb.c
>> index f1401d2f6ed2..9b33ce929618 100644
>> --- a/drivers/net/ravb.c
>> +++ b/drivers/net/ravb.c
>> @@ -11,6 +11,7 @@
>> #include <clk.h>
>> #include <cpu_func.h>
>> #include <dm.h>
>> +#include <dm/device_compat.h>
>> #include <errno.h>
>> #include <log.h>
>> #include <miiphy.h>
>> @@ -494,6 +495,7 @@ static int ravb_probe(struct udevice *dev)
>> {
>> struct eth_pdata *pdata = dev_get_plat(dev);
>> struct ravb_priv *eth = dev_get_priv(dev);
>> + struct bb_miiphy_bus *phybus;
>> struct mii_dev *mdiodev;
>> void __iomem *iobase;
>> int ret;
>> @@ -513,7 +515,8 @@ static int ravb_probe(struct udevice *dev)
>>
>> mdiodev->read = bb_miiphy_read;
>> mdiodev->write = bb_miiphy_write;
>> - bb_miiphy_buses[0].priv = eth;
>> + phybus = (struct bb_miiphy_bus *)pdata->priv_pdata;
>> + phybus->priv = eth;
>> snprintf(mdiodev->name, sizeof(mdiodev->name), dev->name);
>>
>> ret = mdio_register(mdiodev);
>> @@ -625,7 +628,17 @@ int ravb_bb_delay(struct bb_miiphy_bus *bus)
>>
>> struct bb_miiphy_bus bb_miiphy_buses[] = {
>> {
>> - .name = "ravb",
>> + .name = "ravb0",
>> + .init = ravb_bb_init,
>> + .mdio_active = ravb_bb_mdio_active,
>> + .mdio_tristate = ravb_bb_mdio_tristate,
>> + .set_mdio = ravb_bb_set_mdio,
>> + .get_mdio = ravb_bb_get_mdio,
>> + .set_mdc = ravb_bb_set_mdc,
>> + .delay = ravb_bb_delay,
>> + },
>> + {
>> + .name = "ravb1",
>> .init = ravb_bb_init,
>> .mdio_active = ravb_bb_mdio_active,
>> .mdio_tristate = ravb_bb_mdio_tristate,
>> @@ -646,10 +659,16 @@ static const struct eth_ops ravb_ops = {
>> .write_hwaddr = ravb_write_hwaddr,
>> };
>>
>> +static int bb_miiphy_index;
>> +
>> int ravb_of_to_plat(struct udevice *dev)
>> {
>> struct eth_pdata *pdata = dev_get_plat(dev);
>> - const fdt32_t *cell;
>> +
>> + if (bb_miiphy_index >= bb_miiphy_buses_num) {
>> + dev_err(dev, "ravb driver supports only 1 or 2 devices!\n");
> Hmmmm, I really do not like this, can we make this dynamic ?
>
> Unless you want to take a look at this yourself, I can add it into my todo ?
I think the real solution here would be to separate the bb_miiphy
operations from the bus instance, so we would have something like:
struct bb_miiphy_bus {
struct bb_miiphy_ops *ops;
void *priv;
};
struct bb_miiphy_ops {
int (*init)(struct bb_miiphy_bus *bus);
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);
};
int bb_miiphy_bus_register(const char *name,
struct bb_miiphy_ops *ops,
void *priv);
Where drivers will call `bb_miiphy_bus_register()` from the probe
function, it will create a `struct bb_miiphy_bus` instance and a
`struct mii_dev` instance then call `mdio_register()`. The driver can
then support an arbitrary number of MDIO busses from a single constant
`struct bb_miiphy_ops` instance.
The bb_miiphy_getbus() function should be dropped from miiphy.c.
Instead, the priv pointer in the `struct mii_dev` instance can point to
the appropriate `struct bb_miiphy_bus` instance.
It looks like all users of CONFIG_BITBANGMII also set
CONFIG_BITBANGMII_MULTI, and there don't seem to be any targets that
define the macros documented in README.bitbangMII (lines 15-22). So, we
can drop the non-BITBANGMII_MULTI code from miiphybb.c and simplify
things a lot.
That's non-trivial but it's not a huge set of changes, maybe something
we could target for v2024.04?
Thanks,
--
Paul Barker
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
