[PATCH v1 2/5] net: dwc_eth_qos: Add StarFive ethernet driver glue layer

[yanhong wang]

On 2023/3/19 4:20, Simon Glass wrote:
> Hi Yanhong,
> 
> On Thu, 16 Mar 2023 at 19:07, Yanhong Wang
> <yanhong.wang at starfivetech.com> wrote:
>>
>> The StarFive ETHQOS hardware has its own clock and reset,so add a
>> corresponding glue driver to configure them.
>>
>> Signed-off-by: Yanhong Wang <yanhong.wang at starfivetech.com>
>> ---
>>  drivers/net/Kconfig                |   7 +
>>  drivers/net/Makefile               |   1 +
>>  drivers/net/dwc_eth_qos.c          |   6 +
>>  drivers/net/dwc_eth_qos.h          |   2 +
>>  drivers/net/dwc_eth_qos_starfive.c | 306 +++++++++++++++++++++++++++++
>>  5 files changed, 322 insertions(+)
>>  create mode 100644 drivers/net/dwc_eth_qos_starfive.c
> 
>>[..]
> 
>> diff --git a/drivers/net/dwc_eth_qos_starfive.c b/drivers/net/dwc_eth_qos_starfive.c
>> new file mode 100644
>> index 0000000000..eeb45981bd
>> --- /dev/null
>> +++ b/drivers/net/dwc_eth_qos_starfive.c
>> @@ -0,0 +1,306 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
>> + * Author: Yanhong Wang<yanhong.wang at starfivetech.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/cache.h>
>> +#include <asm/gpio.h>
>> +#include <clk.h>
>> +#include <dm.h>
>> +#include <eth_phy.h>
>> +#include <regmap.h>
>> +#include <reset.h>
>> +#include <syscon.h>
>> +
>> +#include "dwc_eth_qos.h"
>> +
>> +#define STARFIVE_DWMAC_PHY_INFT_RGMII  0x1
>> +#define STARFIVE_DWMAC_PHY_INFT_RMII   0x4
>> +#define STARFIVE_DWMAC_PHY_INFT_FIELD  0x7U
>> +
>> +static int eqos_interface_init_jh7110(struct udevice *dev,
>> +                                     phy_interface_t interface_type)
>> +{
>> +       struct regmap *regmap;
>> +       struct ofnode_phandle_args args;
>> +       unsigned int mode;
>> +       int ret;
>> +
>> +       switch (interface_type) {
>> +       case PHY_INTERFACE_MODE_RMII:
>> +               mode = STARFIVE_DWMAC_PHY_INFT_RMII;
>> +               break;
>> +
>> +       case PHY_INTERFACE_MODE_RGMII:
>> +       case PHY_INTERFACE_MODE_RGMII_ID:
>> +               mode = STARFIVE_DWMAC_PHY_INFT_RGMII;
>> +               break;
>> +
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       ret = dev_read_phandle_with_args(dev, "starfive,syscon", NULL,
>> +                                        2, 0, &args);
> 
> You must read the DT in an ofdata_to_plat() method (or probe() if that
> doesn't suit). Put it in a struct.
> 
> You must not read it constantly at runtime throughout your driver. It
> is slow and error-prone.
> [..]
> 

Thanks. I will define a platform data structure for saving device-related configurations in the next version.

>> +
>> +struct eqos_config __maybe_unused eqos_jh7110_config = {
>> +       .reg_access_always_ok = false,
>> +       .mdio_wait = 10,
>> +       .swr_wait = 50,
>> +       .config_mac = EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB,
>> +       .config_mac_mdio = EQOS_MAC_MDIO_ADDRESS_CR_250_300,
>> +       .axi_bus_width = EQOS_AXI_WIDTH_64,
>> +       .interface = dev_read_phy_mode,
>> +       .ops = &eqos_jh7110_ops
>> +};
>> --
>> 2.17.1
>>
> 
> What is that data for? Please add a comment.
> 

The definition of these parameters refers to IMX platform, I will double confirm whether the definition of 
these parameters is accurate, and add corresponding comments in the next version.

> Regards,
> SImom