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 <redacted>
Signed-off-by: Suresh Siddha <redacted>
Signed-off-by: Swathi K S <redacted>
Signed-off-by: Sriranjani P <redacted>
---

+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, &reg);
+		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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel