Messages in this thread | | | Date | Tue, 3 Mar 2020 12:07:10 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v4 3/3] phy: intel: Add driver support for ComboPhy |
| |
On Tue, Mar 03, 2020 at 04:41:17PM +0800, Dilip Kota wrote: > 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,...
Again, this is not obvious from the naming scheme in use. And how do you know the future?
> i can still add the comma at all the places pointed.
Just use a common sense.
> > > +}; > > > +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. > > > > > +};
-- With Best Regards, Andy Shevchenko
| |