> +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); > + } Please add a return -EINVAL; > + > + 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); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + return regmap_update_bits(regmap, reg, mask, mode << __ffs(mask)); This is a poor device tree binding. We generally don't allow bindings which say put value X in register Y. Could you add a table: interface mode, reg, mask? You can then do a simple lookup based on the interface mode? No device tree binding needed at all? Andrew