[PATCH 09/14] net: ravb: Support up to two instances

[Paul Barker]

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.ct at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x27F4B3459F002257.asc
Type: application/pgp-keys
Size: 3520 bytes
Desc: OpenPGP public key
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241028/ca5f9750/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241028/ca5f9750/attachment.sig>