On Mon, Aug 14, 2023 at 2:24 AM Simon Horman <horms@xxxxxxxxxx> wrote:
>
> On Sat, Aug 12, 2023 at 11:11:34PM +0800, Keguang Zhang wrote:
> > This glue driver is created based on the arch-code
> > implemented earlier with the platform-specific settings.
> >
> > Use syscon for SYSCON register access.
> >
> > Partialy based on the previous work by Serge Semin.
> >
> > Signed-off-by: Keguang Zhang <keguang.zhang@xxxxxxxxx>
>
> ...
>
> Hi Keguang Zhang,
>
> some minor feedback from my side.
>
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson1.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson1.c
>
> ...
>
> > +const struct reg_field ls1b_dwmac_syscon_regfields[] = {
> > + [GMAC1_USE_UART1] = REG_FIELD(LS1X_SYSCON0, 4, 4),
> > + [GMAC1_USE_UART0] = REG_FIELD(LS1X_SYSCON0, 3, 3),
> > + [GMAC1_SHUT] = REG_FIELD(LS1X_SYSCON1, 13, 13),
> > + [GMAC0_SHUT] = REG_FIELD(LS1X_SYSCON1, 12, 12),
> > + [GMAC1_USE_TXCLK] = REG_FIELD(LS1X_SYSCON1, 3, 3),
> > + [GMAC0_USE_TXCLK] = REG_FIELD(LS1X_SYSCON1, 2, 2),
> > + [GMAC1_USE_PWM23] = REG_FIELD(LS1X_SYSCON1, 1, 1),
> > + [GMAC0_USE_PWM01] = REG_FIELD(LS1X_SYSCON1, 0, 0)
> > +};
>
> nit: Perhaps ls1b_dwmac_syscon_regfields should be static.
>
> > +
> > +const struct reg_field ls1c_dwmac_syscon_regfields[] = {
> > + [GMAC_SHUT] = REG_FIELD(LS1X_SYSCON0, 6, 6),
> > + [PHY_INTF_SELI] = REG_FIELD(LS1X_SYSCON1, 28, 30)
> > +};
>
> Likewise, perhaps ls1c_dwmac_syscon_regfields should be static.
>
Will do.
Thanks!
> ...
>
> > +static const struct of_device_id ls1x_dwmac_syscon_match[] = {
> > + { .compatible = "loongson,ls1b-syscon", .data = &ls1b_dwmac_syscon },
> > + { .compatible = "loongson,ls1c-syscon", .data = &ls1c_dwmac_syscon },
> > + { }
> > +};o
>
> I am seeing a warning about ls1x_dwmac_syscon_match being unused.
> I think this is due to CONFIG_OF being unset.
Will fix these warnings.
>
> > +
> > +static int ls1x_dwmac_probe(struct platform_device *pdev)
> > +{
> > + struct plat_stmmacenet_data *plat_dat;
> > + struct stmmac_resources stmmac_res;
> > + struct device_node *syscon_np;
> > + const struct of_device_id *match;
> > + struct regmap *regmap;
> > + struct ls1x_dwmac *dwmac;
> > + const struct ls1x_dwmac_syscon *syscon;
> > + size_t size;
> > + int ret;
> > +
> > + ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> > + if (ret)
> > + return ret;
> > +
> > + /* Probe syscon */
> > + syscon_np = of_parse_phandle(pdev->dev.of_node, "syscon", 0);
> > + if (!syscon_np)
> > + return -ENODEV;
> > +
> > + match = of_match_node(ls1x_dwmac_syscon_match, syscon_np);
> > + if (!match) {
> > + of_node_put(syscon_np);
> > + return -EINVAL;
> > + }
> > + syscon = (const struct ls1x_dwmac_syscon *)match->data;
> > +
> > + regmap = syscon_node_to_regmap(syscon_np);
> > + of_node_put(syscon_np);
> > + if (IS_ERR(regmap)) {
> > + ret = PTR_ERR(regmap);
> > + dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + size = syscon->nr_reg_fields * sizeof(struct regmap_field *);
> > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac) + size, GFP_KERNEL);
> > + if (!dwmac)
> > + return -ENOMEM;
> > +
> > + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > + if (IS_ERR(plat_dat)) {
> > + dev_err(&pdev->dev, "dt configuration failed\n");
> > + return PTR_ERR(plat_dat);
> > + }
> > +
> > + plat_dat->bsp_priv = dwmac;
> > + plat_dat->init = ls1x_dwmac_init;
> > + dwmac->dev = &pdev->dev;
> > + dwmac->plat_dat = plat_dat;
> > + dwmac->syscon = syscon;
> > + dwmac->regmap = regmap;
> > +
> > + ret = stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
> > + if (ret)
> > + goto err_remove_config_dt;
> > +
> > + return 0;
> > +
> > +err_remove_config_dt:
> > + if (pdev->dev.of_node)
> > + stmmac_remove_config_dt(pdev, plat_dat);
> > +
> > + return ret;
> > +}
>
> ...
--
Best regards,
Keguang Zhang
