On 14/08/2023 13:25, Sriranjani P wrote: > The FSD SoC contains two instance of the Synopsys DWC ethernet QOS IP core. > The binding that it uses is slightly different from existing ones because > of the integration (clocks, resets). > > For FSD SoC, a mux switch is needed between internal and external clocks. > By default after reset internal clock is used but for receiving packets > properly, external clock is needed. Mux switch to external clock happens > only when the external clock is present. > > Signed-off-by: Chandrasekar R <rcsekar@xxxxxxxxxxx> > Signed-off-by: Suresh Siddha <ssiddha@xxxxxxxxx> > Signed-off-by: Swathi K S <swathi.ks@xxxxxxxxxxx> > Signed-off-by: Sriranjani P <sriranjani.p@xxxxxxxxxxx> > --- > +static int dwc_eqos_setup_rxclock(struct platform_device *pdev, int ins_num) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct regmap *syscon; > + unsigned int reg; > + > + if (np && of_property_read_bool(np, "fsd-rx-clock-skew")) { > + syscon = syscon_regmap_lookup_by_phandle_args(np, > + "fsd-rx-clock-skew", > + 1, ®); > + if (IS_ERR(syscon)) { > + dev_err(&pdev->dev, > + "couldn't get the rx-clock-skew syscon!\n"); > + return PTR_ERR(syscon); > + } > + > + regmap_write(syscon, reg, rx_clock_skew_val[ins_num]); > + } > + > + return 0; > +} > + > +static int fsd_eqos_clk_init(struct fsd_eqos_plat_data *plat, > + struct plat_stmmacenet_data *data) > +{ > + int ret = 0, i; > + > + const struct fsd_eqos_variant *fsd_eqos_v_data = > + plat->fsd_eqos_inst_var; > + > + plat->clks = devm_kcalloc(plat->dev, fsd_eqos_v_data->num_clks, > + sizeof(*plat->clks), GFP_KERNEL); > + if (!plat->clks) > + return -ENOMEM; > + > + for (i = 0; i < fsd_eqos_v_data->num_clks; i++) > + plat->clks[i].id = fsd_eqos_v_data->clk_list[i]; > + > + ret = devm_clk_bulk_get(plat->dev, fsd_eqos_v_data->num_clks, > + plat->clks); Instead of duplicating entire clock management with existing code, you should extend/rework existing one. This code is unfortunately great example how not to stuff vendor code into upstream project. :( > + > + return ret; > +} > + > +static int fsd_clks_endisable(void *priv, bool enabled) > +{ > + int ret, num_clks; > + struct fsd_eqos_plat_data *plat = priv; > + > + num_clks = plat->fsd_eqos_inst_var->num_clks; > + > + if (enabled) { > + ret = clk_bulk_prepare_enable(num_clks, plat->clks); > + if (ret) { > + dev_err(plat->dev, "Clock enable failed, err = %d\n", ret); > + return ret; > + } > + } else { > + clk_bulk_disable_unprepare(num_clks, plat->clks); > + } > + > + return 0; > +} > + > +static int fsd_eqos_probe(struct platform_device *pdev, > + struct plat_stmmacenet_data *data, > + struct stmmac_resources *res) > +{ > + struct fsd_eqos_plat_data *priv_plat; > + struct device_node *np = pdev->dev.of_node; > + int ret = 0; > + > + priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), GFP_KERNEL); > + if (!priv_plat) { > + ret = -ENOMEM; return -ENOMEM > + goto error; > + } > + > + priv_plat->dev = &pdev->dev; > + data->bus_id = of_alias_get_id(np, "eth"); No, you cannot do like this. Aliases are board specific and are based on labeling on the board. > + > + priv_plat->fsd_eqos_inst_var = &fsd_eqos_clk_info[data->bus_id]; > + > + ret = fsd_eqos_clk_init(priv_plat, data); > + > + data->bsp_priv = priv_plat; > + data->clks_config = fsd_clks_endisable; > + data->rxmux_setup = dwc_eqos_rxmux_setup; > + > + ret = fsd_clks_endisable(priv_plat, true); > + if (ret) > + goto error; > + > + ret = dwc_eqos_setup_rxclock(pdev, data->bus_id); > + if (ret) { > + fsd_clks_endisable(priv_plat, false); > + dev_err_probe(&pdev->dev, ret, "Unable to setup rxclock\n"); The syntax is: return dev_err_probe(). > + } > + > +error: > + return ret; > +} .... Best regards, Krzysztof