Re: [PATCH v2 2/9] phy: Add configuration interface
Hi Kishon
On Mon, Nov 12, 2018 at 03:32:25PM +0530, Kishon Vijay Abraham I wrote:
> On 06/11/18 8:24 PM, Maxime Ripard wrote:
> > The phy framework is only allowing to configure the power state of the PHY
> > using the init and power_on hooks, and their power_off and exit
> > counterparts.
> >
> > While it works for most, simple, PHYs supported so far, some more advanced
> > PHYs need some configuration depending on runtime parameters. These PHYs
> > have been supported by a number of means already, often by using ad-hoc
> > drivers in their consumer drivers.
> >
> > That doesn't work too well however, when a consumer device needs to deal
> > with multiple PHYs, or when multiple consumers need to deal with the same
> > PHY (a DSI driver and a CSI driver for example).
> >
> > So we'll add a new interface, through two funtions, phy_validate and
> > phy_configure. The first one will allow to check that a current
> > configuration, for a given mode, is applicable. It will also allow the PHY
> > driver to tune the settings given as parameters as it sees fit.
> >
> > phy_configure will actually apply that configuration in the phy itself.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/phy/phy-core.c | 61 ++-
> > include/linux/phy/phy.h | 58 -
> > 2 files changed, 119 insertions(+)
> >
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 35fd38c5a4a1..7bd3ed65c708 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -408,6 +408,67 @@ int phy_calibrate(struct phy *phy)
> > EXPORT_SYMBOL_GPL(phy_calibrate);
> >
> > /**
> > + * phy_configure() - Changes the phy parameters
> > + * @phy: the phy returned by phy_get()
> > + * @opts: New configuration to apply
> > + *
> > + * Used to change the PHY parameters. phy_init() must have been called
> > + * on the phy. The configuration will be applied on the current phy
> > + * mode, that can be changed using phy_set_mode().
> > + *
> > + * Returns: 0 if successful, an negative error code otherwise
> > + */
> > +int phy_configure(struct phy *phy, union phy_configure_opts *opts)
> > +{
> > + int ret;
> > +
> > + if (!phy)
> > + return -EINVAL;
> > +
> > + if (!phy->ops->configure)
> > + return -EOPNOTSUPP;
> > +
> > + mutex_lock(>mutex);
> > + ret = phy->ops->configure(phy, opts);
> > + mutex_unlock(>mutex);
> > +
> > + return ret;
> > +}
>
> EXPORT_SYMBOL_GPL is missing here and below.
Consider it done.
> > +
> > +/**
> > + * phy_validate() - Checks the phy parameters
> > + * @phy: the phy returned by phy_get()
> > + * @mode: phy_mode the configuration is applicable to.
> > + * @opts: Configuration to check
> > + *
> > + * Used to check that the current set of parameters can be handled by
> > + * the phy. Implementations are free to tune the parameters passed as
> > + * arguments if needed by some implementation detail or
> > + * constraints. It will not change any actual configuration of the
> > + * PHY, so calling it as many times as deemed fit will have no side
> > + * effect.
> > + *
> > + * Returns: 0 if successful, an negative error code otherwise
> > + */
> > +int phy_validate(struct phy *phy, enum phy_mode mode,
> > + union phy_configure_opts *opts)
>
> We are planning to switch to mode/submode combination [1], so this might have
> to change.
Yes, I'm aware of that. If needed, it shouldn't be too hard to rework.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/9] phy: Add configuration interface
Hi Maxime,
On 06/11/18 8:24 PM, Maxime Ripard wrote:
> The phy framework is only allowing to configure the power state of the PHY
> using the init and power_on hooks, and their power_off and exit
> counterparts.
>
> While it works for most, simple, PHYs supported so far, some more advanced
> PHYs need some configuration depending on runtime parameters. These PHYs
> have been supported by a number of means already, often by using ad-hoc
> drivers in their consumer drivers.
>
> That doesn't work too well however, when a consumer device needs to deal
> with multiple PHYs, or when multiple consumers need to deal with the same
> PHY (a DSI driver and a CSI driver for example).
>
> So we'll add a new interface, through two funtions, phy_validate and
> phy_configure. The first one will allow to check that a current
> configuration, for a given mode, is applicable. It will also allow the PHY
> driver to tune the settings given as parameters as it sees fit.
>
> phy_configure will actually apply that configuration in the phy itself.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/phy/phy-core.c | 61 ++-
> include/linux/phy/phy.h | 58 -
> 2 files changed, 119 insertions(+)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 35fd38c5a4a1..7bd3ed65c708 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -408,6 +408,67 @@ int phy_calibrate(struct phy *phy)
> EXPORT_SYMBOL_GPL(phy_calibrate);
>
> /**
> + * phy_configure() - Changes the phy parameters
> + * @phy: the phy returned by phy_get()
> + * @opts: New configuration to apply
> + *
> + * Used to change the PHY parameters. phy_init() must have been called
> + * on the phy. The configuration will be applied on the current phy
> + * mode, that can be changed using phy_set_mode().
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> +int phy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> + int ret;
> +
> + if (!phy)
> + return -EINVAL;
> +
> + if (!phy->ops->configure)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(>mutex);
> + ret = phy->ops->configure(phy, opts);
> + mutex_unlock(>mutex);
> +
> + return ret;
> +}
EXPORT_SYMBOL_GPL is missing here and below.
> +
> +/**
> + * phy_validate() - Checks the phy parameters
> + * @phy: the phy returned by phy_get()
> + * @mode: phy_mode the configuration is applicable to.
> + * @opts: Configuration to check
> + *
> + * Used to check that the current set of parameters can be handled by
> + * the phy. Implementations are free to tune the parameters passed as
> + * arguments if needed by some implementation detail or
> + * constraints. It will not change any actual configuration of the
> + * PHY, so calling it as many times as deemed fit will have no side
> + * effect.
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> +int phy_validate(struct phy *phy, enum phy_mode mode,
> + union phy_configure_opts *opts)
We are planning to switch to mode/submode combination [1], so this might have
to change.
Thanks
Kishon
[1] -> https://patchwork.kernel.org/cover/10673295/
> +{
> + int ret;
> +
> + if (!phy)
> + return -EINVAL;
> +
> + if (!phy->ops->validate)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(>mutex);
> + ret = phy->ops->validate(phy, mode, opts);
> + mutex_unlock(>mutex);
> +
> + return ret;
> +}
> +
> +/**
> * _of_phy_get() - lookup and obtain a reference to a phy by phandle
> * @np: device_node for which to get the phy
> * @index: the index of the phy
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index d5c2ac7369f2..89cf8b685546 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -46,6 +46,12 @@ enum phy_mode {
> };
>
> /**
> + * union phy_configure_opts - Opaque generic phy configuration
> + */
> +union phy_configure_opts {
> +};
> +
> +/**
> * struct phy_ops - set of function pointers for performing phy operations
> * @init: operation to be performed for initializing phy
> * @exit: operation to be performed while exiting
> @@ -62,6 +68,37 @@ struct phy_ops {
> int (*power_on)(struct phy *phy);
> int (*power_off)(struct phy *phy);
> int (*set_mode)(struct phy *phy, enum phy_mode mode);
> +
> + /**
> + * @configure:
> + *
> + * Optional.
> + *
> + * Used to change the PHY parameters. phy_init() must have
> + * been called on the phy.
> + *
> + * Returns: 0 if successful, an negative error code otherwise
> + */
> + int (*configure)(struct phy *phy, union phy_configure_opts *opts);
> +
> + /**
> + * @validate:
> + *
> + * Optional.
> + *
> + * Used to check that the current set of parameters can be
> + * handled by the phy.
[PATCH v2 2/9] phy: Add configuration interface
The phy framework is only allowing to configure the power state of the PHY
using the init and power_on hooks, and their power_off and exit
counterparts.
While it works for most, simple, PHYs supported so far, some more advanced
PHYs need some configuration depending on runtime parameters. These PHYs
have been supported by a number of means already, often by using ad-hoc
drivers in their consumer drivers.
That doesn't work too well however, when a consumer device needs to deal
with multiple PHYs, or when multiple consumers need to deal with the same
PHY (a DSI driver and a CSI driver for example).
So we'll add a new interface, through two funtions, phy_validate and
phy_configure. The first one will allow to check that a current
configuration, for a given mode, is applicable. It will also allow the PHY
driver to tune the settings given as parameters as it sees fit.
phy_configure will actually apply that configuration in the phy itself.
Signed-off-by: Maxime Ripard
---
drivers/phy/phy-core.c | 61 ++-
include/linux/phy/phy.h | 58 -
2 files changed, 119 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 35fd38c5a4a1..7bd3ed65c708 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -408,6 +408,67 @@ int phy_calibrate(struct phy *phy)
EXPORT_SYMBOL_GPL(phy_calibrate);
/**
+ * phy_configure() - Changes the phy parameters
+ * @phy: the phy returned by phy_get()
+ * @opts: New configuration to apply
+ *
+ * Used to change the PHY parameters. phy_init() must have been called
+ * on the phy. The configuration will be applied on the current phy
+ * mode, that can be changed using phy_set_mode().
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+int phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+ int ret;
+
+ if (!phy)
+ return -EINVAL;
+
+ if (!phy->ops->configure)
+ return -EOPNOTSUPP;
+
+ mutex_lock(>mutex);
+ ret = phy->ops->configure(phy, opts);
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+
+/**
+ * phy_validate() - Checks the phy parameters
+ * @phy: the phy returned by phy_get()
+ * @mode: phy_mode the configuration is applicable to.
+ * @opts: Configuration to check
+ *
+ * Used to check that the current set of parameters can be handled by
+ * the phy. Implementations are free to tune the parameters passed as
+ * arguments if needed by some implementation detail or
+ * constraints. It will not change any actual configuration of the
+ * PHY, so calling it as many times as deemed fit will have no side
+ * effect.
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
+int phy_validate(struct phy *phy, enum phy_mode mode,
+ union phy_configure_opts *opts)
+{
+ int ret;
+
+ if (!phy)
+ return -EINVAL;
+
+ if (!phy->ops->validate)
+ return -EOPNOTSUPP;
+
+ mutex_lock(>mutex);
+ ret = phy->ops->validate(phy, mode, opts);
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+
+/**
* _of_phy_get() - lookup and obtain a reference to a phy by phandle
* @np: device_node for which to get the phy
* @index: the index of the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index d5c2ac7369f2..89cf8b685546 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -46,6 +46,12 @@ enum phy_mode {
};
/**
+ * union phy_configure_opts - Opaque generic phy configuration
+ */
+union phy_configure_opts {
+};
+
+/**
* struct phy_ops - set of function pointers for performing phy operations
* @init: operation to be performed for initializing phy
* @exit: operation to be performed while exiting
@@ -62,6 +68,37 @@ struct phy_ops {
int (*power_on)(struct phy *phy);
int (*power_off)(struct phy *phy);
int (*set_mode)(struct phy *phy, enum phy_mode mode);
+
+ /**
+* @configure:
+*
+* Optional.
+*
+* Used to change the PHY parameters. phy_init() must have
+* been called on the phy.
+*
+* Returns: 0 if successful, an negative error code otherwise
+*/
+ int (*configure)(struct phy *phy, union phy_configure_opts *opts);
+
+ /**
+* @validate:
+*
+* Optional.
+*
+* Used to check that the current set of parameters can be
+* handled by the phy. Implementations are free to tune the
+* parameters passed as arguments if needed by some
+* implementation detail or constraints. It must not change
+* any actual configuration of the PHY, so calling it as many
+* times as deemed fit by the consumer must have no side
+* effect.
+*
+* Returns: 0 if the configuration can be applied, an negative
+* error code otherwise
+*/
+ int
