Re: [PATCH v4 1/2] phy: core: add phy_property_present method

[Andrew Murray <andrew.murray@arm.com>]

On Thu, Nov 21, 2019 at 08:50:07AM +0530, Anvesh Salveru wrote:
> In some platforms, we need information of phy properties in
> the controller drivers. This patch adds a new phy_property_present()
> method which can be used to check if some property exists in PHY
> or not.
> 
> In case of DesignWare PCIe controller, we need to write into controller
> register to specify about ZRX-DC compliance property of the PHY, which
> reduces the power consumption during lower power states.
> 
> Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  drivers/phy/phy-core.c  | 26 ++++++++++++++++++++++++++
>  include/linux/phy/phy.h |  8 ++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index b04f4fe..0a62eca 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -420,6 +420,32 @@ int phy_calibrate(struct phy *phy)
>  EXPORT_SYMBOL_GPL(phy_calibrate);
>  
>  /**
> + * phy_property_present() - checks if the property is present in PHY
> + * @phy: the phy returned by phy_get()
> + * @property: name of the property to check
> + *
> + * Used to check if the given property is present in PHY. PHY drivers
> + * can implement this callback function to expose PHY properties to
> + * controller drivers.
> + *
> + * Returns: true if property exists, false otherwise
> + */
> +bool phy_property_present(struct phy *phy, const char *property)
> +{
> +	bool ret;
> +
> +	if (!phy || !phy->ops->property_present)
> +		return false;
> +
> +	mutex_lock(&phy->mutex);
> +	ret = phy->ops->property_present(phy, property);

I don't understand why it is necessary to require every phy driver to
implement this. Why can't the phy-core driver look up the device node
of the given phy?

Thanks,

Andrew Murray

> +	mutex_unlock(&phy->mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_property_present);
> +
> +/**
>   * phy_configure() - Changes the phy parameters
>   * @phy: the phy returned by phy_get()
>   * @opts: New configuration to apply
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 15032f14..3dd8f3c 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -61,6 +61,7 @@ union phy_configure_opts {
>   * @reset: resetting the phy
>   * @calibrate: calibrate the phy
>   * @release: ops to be performed while the consumer relinquishes the PHY
> + * @property_present: check if some property is present in PHY
>   * @owner: the module owner containing the ops
>   */
>  struct phy_ops {
> @@ -103,6 +104,7 @@ struct phy_ops {
>  	int	(*reset)(struct phy *phy);
>  	int	(*calibrate)(struct phy *phy);
>  	void	(*release)(struct phy *phy);
> +	bool	(*property_present)(struct phy *phy, const char *property);
>  	struct module *owner;
>  };
>  
> @@ -217,6 +219,7 @@ static inline enum phy_mode phy_get_mode(struct phy *phy)
>  }
>  int phy_reset(struct phy *phy);
>  int phy_calibrate(struct phy *phy);
> +bool phy_property_present(struct phy *phy, const char *property);
>  static inline int phy_get_bus_width(struct phy *phy)
>  {
>  	return phy->attrs.bus_width;
> @@ -354,6 +357,11 @@ static inline int phy_calibrate(struct phy *phy)
>  	return -ENOSYS;
>  }
>  
> +static inline bool phy_property_present(struct phy *phy, const char *property)
> +{
> +	return false;
> +}
> +
>  static inline int phy_configure(struct phy *phy,
>  				union phy_configure_opts *opts)
>  {
> -- 
> 2.7.4
>