Re: [PATCH v4 3/3] phy: intel: Add driver support for ComboPhy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/2/2020 7:19 PM, Andy Shevchenko wrote:
On Mon, Mar 02, 2020 at 04:43:25PM +0800, Dilip Kota wrote:
ComboPhy subsystem provides PHYs for various
controllers like PCIe, SATA and EMAC.
Thanks for an update, my (few minor) comments below.

...

+enum intel_phy_mode {
+	PHY_PCIE_MODE = 0,
+	PHY_XPCS_MODE,
+	PHY_SATA_MODE
 From here it's not visible that above is the only possible values.
Maybe in the future you will have another mode.
So, I suggest to leave comma here...
There won't be no further modes,...
i can still add the comma at all the places pointed.

+};
+enum intel_combo_mode {
+	PCIE0_PCIE1_MODE = 0,
+	PCIE_DL_MODE,
+	RXAUI_MODE,
+	XPCS0_XPCS1_MODE,
+	SATA0_SATA1_MODE
...and here...

+};
+
+enum aggregated_mode {
+	PHY_SL_MODE,
+	PHY_DL_MODE
...and here.

+};
...

+static int intel_cbphy_iphy_cfg(struct intel_cbphy_iphy *iphy,
+				int (*phy_cfg)(struct intel_cbphy_iphy *))
+{
+	struct intel_combo_phy *cbphy = iphy->parent;
+	struct intel_cbphy_iphy *sphy;
+	int ret;
+
+	ret = phy_cfg(iphy);
+	if (ret)
+		return ret;
+
+	if (cbphy->aggr_mode != PHY_DL_MODE)
+		return 0;
+
+	sphy = &cbphy->iphy[PHY_1];
Do you really need temporary variable here?
Can be removed, i will update in the next patch.

+
+	return phy_cfg(sphy);
+}
...

+	if (!cbphy->init_cnt) {
	if (init_cnt)
		return 0;

?

Sure, will do the same.


Thanks,
Dilip

+		clk_disable_unprepare(cbphy->core_clk);
+		intel_cbphy_rst_assert(cbphy);
+	}
+
+	return 0;



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux