在 2023/3/4 0:50:54, Emil Renner Berthing 写道:
> On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@xxxxxxxxxxxxxxxx> wrote:
>>
>> dwmac supports multiple modess. When working under rmii and rgmii,
>> you need to set different phy interfaces.
>>
>> According to the dwmac document, when working in rmii, it needs to be
>> set to 0x4, and rgmii needs to be set to 0x1.
>>
>> The phy interface needs to be set in syscon, the format is as follows:
>> starfive,syscon: <&syscon, offset, mask>
>>
>> Signed-off-by: Samin Guo <samin.guo@xxxxxxxxxxxxxxxx>
>> ---
>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 46 +++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> index 566378306f67..40fdd7036127 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>> @@ -7,10 +7,15 @@
>> *
>> */
>>
>> +#include <linux/mfd/syscon.h>
>> #include <linux/of_device.h>
>> +#include <linux/regmap.h>
>>
>> #include "stmmac_platform.h"
>>
>> +#define MACPHYC_PHY_INFT_RMII 0x4
>> +#define MACPHYC_PHY_INFT_RGMII 0x1
>
> Please prefix these with something like STARFIVE_DWMAC_
>
Hi, Emil, These definitions come from the datasheet of dwmac. However, add STARDRIVE_ DWMAC is a good idea. I will fix it,thanks.
>> struct starfive_dwmac {
>> struct device *dev;
>> struct clk *clk_tx;
>> @@ -53,6 +58,46 @@ static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed)
>> dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
>> }
>>
>> +static int starfive_dwmac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> + struct starfive_dwmac *dwmac = plat_dat->bsp_priv;
>> + struct of_phandle_args args;
>> + struct regmap *regmap;
>> + unsigned int reg, mask, mode;
>> + int err;
>> +
>> + switch (plat_dat->interface) {
>> + case PHY_INTERFACE_MODE_RMII:
>> + mode = MACPHYC_PHY_INFT_RMII;
>> + break;
>> +
>> + case PHY_INTERFACE_MODE_RGMII:
>> + case PHY_INTERFACE_MODE_RGMII_ID:
>> + mode = MACPHYC_PHY_INFT_RGMII;
>> + break;
>> +
>> + default:
>> + dev_err(dwmac->dev, "Unsupported interface %d\n",
>> + plat_dat->interface);
>> + }
>> +
>> + err = of_parse_phandle_with_fixed_args(dwmac->dev->of_node,
>> + "starfive,syscon", 2, 0, &args);
>> + if (err) {
>> + dev_dbg(dwmac->dev, "syscon reg not found\n");
>> + return -EINVAL;
>> + }
>> +
>> + reg = args.args[0];
>> + mask = args.args[1];
>> + regmap = syscon_node_to_regmap(args.np);
>> + of_node_put(args.np);
>
> I think the above is basically
> unsigned int args[2];
> syscon_regmap_lookup_by_phandle_args(dwmac->dev_of_node,
> "starfive,syscon", 2, args);
>
> ..but as Andrew points out another solution is to use platform match
> data for this. Eg.
>
> static const struct starfive_dwmac_match_data starfive_dwmac_jh7110_data {
> .phy_interface_offset = 0xc,
> .phy_interface_mask = 0x1c0000,
> };
>
> static const struct of_device_id starfive_dwmac_match[] = {
> { .compatible = "starfive,jh7110-dwmac", .data =
> &starfive_dwmac_jh7110_data },
> { /* sentinel */ }
> };
>
> and in the probe function:
>
Hi Emil, Yes，this is usually a good solution, and I have considered this plan before.
However, gmac0 of jh7110 is different from the reg/mask of gmac1.
You can find it in patch-9:
&gmac0 {
starfive,syscon = <&aon_syscon 0xc 0x1c0000>;
};
&gmac1 {
starfive,syscon = <&sys_syscon 0x90 0x1c>;
};
In this case, using match_data of starfive,jh7110-dwma does not seem to be compatible.
> struct starfive_dwmac_match_data *pdata = device_get_match_data(&pdev->dev);
>
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask));
>> +}
>> +
>> static int starfive_dwmac_probe(struct platform_device *pdev)
>> {
>> struct plat_stmmacenet_data *plat_dat;
>> @@ -93,6 +138,7 @@ static int starfive_dwmac_probe(struct platform_device *pdev)
>> plat_dat->bsp_priv = dwmac;
>> plat_dat->dma_cfg->dche = true;
>>
>> + starfive_dwmac_set_mode(plat_dat);
>
> The function returns errors in an int, but you never check it :(
>
Thank you for pointing out that it will be added in the next version.
>> err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> if (err) {
>> stmmac_remove_config_dt(pdev, plat_dat);
Best regards,
Samin
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@xxxxxxxxxxxxxxxxxxx
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Best regards,
Samin
