Hi Swapnil, Thank you for the patch.
On Mon, Aug 24, 2020 at 08:28:30PM +0200, Swapnil Jakhade wrote: > Add new PHY attribute max_link_rate to struct phy_attrs. > Add a pair of PHY APIs to get/set all the PHY attributes. > Use phy_get_attrs() to get attribute values and phy_set_attrs() > to set attribute values. > > Signed-off-by: Yuti Amonkar <[email protected]> > Signed-off-by: Swapnil Jakhade <[email protected]> > Acked-by: Kishon Vijay Abraham I <[email protected]> > --- > include/linux/phy/phy.h | 43 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > index bcee8eba62b3..924cd1a3deea 100644 > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -115,10 +115,12 @@ struct phy_ops { > /** > * struct phy_attrs - represents phy attributes > * @bus_width: Data path width implemented by PHY > + * @max_link_rate: Maximum link rate supported by PHY (in Mbps) > * @mode: PHY mode > */ > struct phy_attrs { > u32 bus_width; > + u32 max_link_rate; > enum phy_mode mode; > }; > > @@ -231,6 +233,37 @@ static inline void phy_set_bus_width(struct phy *phy, > int bus_width) > { > phy->attrs.bus_width = bus_width; > } > + > +/** > + * phy_get_attrs() - get the values for PHY attributes. > + * @phy: the phy for which to get the attributes > + * @attrs: current PHY attributes that will be returned > + * > + * Intended to be used by phy consumers to get the current PHY attributes > + * stored in struct phy_attrs. > + */ > +static inline void phy_get_attrs(struct phy *phy, struct phy_attrs *attrs) > +{ > + mutex_lock(&phy->mutex); > + memcpy(attrs, &phy->attrs, sizeof(struct phy_attrs)); > + mutex_unlock(&phy->mutex); Why is the mutex required, what does it guard against ? > +} > + > +/** > + * phy_set_attrs() - set PHY attributes with new values. > + * @phy: the phy for which to set the attributes > + * @attrs: the &struct phy_attrs containing new PHY attributes to be set > + * > + * This can be used by PHY providers or PHY consumers to set the PHY > + * attributes. The locking is used to protect updating attributes when > + * PHY consumer is doing some PHY ops. > + */ > +static inline void phy_set_attrs(struct phy *phy, const struct phy_attrs > *attrs) > +{ > + mutex_lock(&phy->mutex); > + memcpy(&phy->attrs, attrs, sizeof(struct phy_attrs)); > + mutex_unlock(&phy->mutex); > +} > struct phy *phy_get(struct device *dev, const char *string); > struct phy *phy_optional_get(struct device *dev, const char *string); > struct phy *devm_phy_get(struct device *dev, const char *string); > @@ -389,6 +422,16 @@ static inline void phy_set_bus_width(struct phy *phy, > int bus_width) > return; > } > > +static inline void phy_get_attrs(struct phy *phy, struct phy_attrs *attrs) > +{ > + return; You can drop the return statement. > +} > + > +static inline void phy_set_attrs(struct phy *phy, const struct phy_attrs > *attrs) > +{ > + return; Here too. > +} > + > static inline struct phy *phy_get(struct device *dev, const char *string) > { > return ERR_PTR(-ENOSYS); -- Regards, Laurent Pinchart
