| Message ID | 1386847117-21240-2-git-send-email-mporter@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Dec 12, 2013 at 06:18:29AM -0500, Matt Porter wrote: > /** > + * struct phy_attrs - represents phy attributes > + * @bus_width: Data path width implemented by PHY > + */ > +struct phy_attrs { > + u32 bus_width; Why u32? > int phy_power_off(struct phy *phy); > +static inline u32 phy_get_bus_width(struct phy *phy) > +{ > + return phy->attrs.bus_width; ... > > +static inline u32 phy_get_bus_width(struct phy *phy) > +{ > + return -ENOSYS; Why u32, especially as you're returning a negative number here. If the bus width is a small integer (I'm assuming you don't have up to 2^30 bus signals) then what's wrong with it being an 'int' ?
On Thu, Dec 12, 2013 at 11:27:15AM +0000, Russell King wrote: > On Thu, Dec 12, 2013 at 06:18:29AM -0500, Matt Porter wrote: > > /** > > + * struct phy_attrs - represents phy attributes > > + * @bus_width: Data path width implemented by PHY > > + */ > > +struct phy_attrs { > > + u32 bus_width; > > Why u32? Kishon suggested it and I changed it on this rev...forgetting about the error path below. > > int phy_power_off(struct phy *phy); > > +static inline u32 phy_get_bus_width(struct phy *phy) > > +{ > > + return phy->attrs.bus_width; > ... > > > > +static inline u32 phy_get_bus_width(struct phy *phy) > > +{ > > + return -ENOSYS; > > Why u32, especially as you're returning a negative number here. > > If the bus width is a small integer (I'm assuming you don't have up to > 2^30 bus signals) then what's wrong with it being an 'int' ? Yes, very correct...it's expected to always be a small positive integer value or zero (when not populated as it's optional). I agree it should go back to 'int' especially due to the negative value when GENERIC_PHY isn't enabled. -Matt
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index 6d72269..a0dcf2d 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -38,6 +38,14 @@ struct phy_ops { }; /** + * struct phy_attrs - represents phy attributes + * @bus_width: Data path width implemented by PHY + */ +struct phy_attrs { + u32 bus_width; +}; + +/** * struct phy - represents the phy device * @dev: phy device * @id: id of the phy device @@ -46,6 +54,7 @@ struct phy_ops { * @mutex: mutex to protect phy_ops * @init_count: used to protect when the PHY is used by multiple consumers * @power_count: used to protect when the PHY is used by multiple consumers + * @phy_attrs: used to specify PHY specific attributes */ struct phy { struct device dev; @@ -55,6 +64,7 @@ struct phy { struct mutex mutex; int init_count; int power_count; + struct phy_attrs attrs; }; /** @@ -127,6 +137,14 @@ int phy_init(struct phy *phy); int phy_exit(struct phy *phy); int phy_power_on(struct phy *phy); int phy_power_off(struct phy *phy); +static inline u32 phy_get_bus_width(struct phy *phy) +{ + return phy->attrs.bus_width; +} +static inline void phy_set_bus_width(struct phy *phy, u32 bus_width) +{ + phy->attrs.bus_width = bus_width; +} struct phy *phy_get(struct device *dev, const char *string); struct phy *devm_phy_get(struct device *dev, const char *string); void phy_put(struct phy *phy); @@ -199,6 +217,16 @@ static inline int phy_power_off(struct phy *phy) return -ENOSYS; } +static inline u32 phy_get_bus_width(struct phy *phy) +{ + return -ENOSYS; +} + +static inline void phy_set_bus_width(struct phy *phy, u32 bus_width) +{ + return; +} + static inline struct phy *phy_get(struct device *dev, const char *string) { return ERR_PTR(-ENOSYS);
This adds a pair of APIs that allows the generic PHY subsystem to provide information on the PHY bus width. The PHY provider driver may use phy_set_bus_width() to set the bus width that the PHY supports. The controller driver may then use phy_get_bus_width() to fetch the PHY bus width in order to properly configure the controller. Signed-off-by: Matt Porter <mporter@linaro.org> --- include/linux/phy/phy.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)