| Message ID | 20231207151204.1007797-1-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4f7aa122bc9219baca0bfface5917062d6c45ee8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] dpll: remove leftover mode_supported() op and use mode_get() instead | expand |
On 07/12/2023 15:12, Jiri Pirko wrote: > From: Jiri Pirko <jiri@nvidia.com> > > Mode supported is currently reported to the user exactly the same, as > the current mode. That's because mode changing is not implemented. > Remove the leftover mode_supported() op and use mode_get() to fill up > the supported mode exposed to user. > > One, if even, mode changing is going to be introduced, this could be > very easily taken back. In the meantime, prevent drivers form > implementing this in wrong way (as for example recent netdevsim > implementation attempt intended to do). > I'm OK to remove from ptp_ocp part because it's really only one mode supported. But I would like to hear something from Arkadiusz about the plans to maybe implement mode change in ice? > Signed-off-by: Jiri Pirko <jiri@nvidia.com> > --- > drivers/dpll/dpll_netlink.c | 16 +++++++----- > drivers/net/ethernet/intel/ice/ice_dpll.c | 26 ------------------- > .../net/ethernet/mellanox/mlx5/core/dpll.c | 9 ------- > drivers/ptp/ptp_ocp.c | 8 ------ > include/linux/dpll.h | 3 --- > 5 files changed, 10 insertions(+), 52 deletions(-) >
Fri, Dec 08, 2023 at 01:06:34PM CET, vadim.fedorenko@linux.dev wrote: >On 07/12/2023 15:12, Jiri Pirko wrote: >> From: Jiri Pirko <jiri@nvidia.com> >> >> Mode supported is currently reported to the user exactly the same, as >> the current mode. That's because mode changing is not implemented. >> Remove the leftover mode_supported() op and use mode_get() to fill up >> the supported mode exposed to user. >> >> One, if even, mode changing is going to be introduced, this could be >> very easily taken back. In the meantime, prevent drivers form >> implementing this in wrong way (as for example recent netdevsim >> implementation attempt intended to do). >> > >I'm OK to remove from ptp_ocp part because it's really only one mode >supported. But I would like to hear something from Arkadiusz about the >plans to maybe implement mode change in ice? As I wrote in the patch description, if ever there is going to be implementation, this could be very easily taken back. Now for sure there was already attempt to misimplement this :) > >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> >> --- >> drivers/dpll/dpll_netlink.c | 16 +++++++----- >> drivers/net/ethernet/intel/ice/ice_dpll.c | 26 ------------------- >> .../net/ethernet/mellanox/mlx5/core/dpll.c | 9 ------- >> drivers/ptp/ptp_ocp.c | 8 ------ >> include/linux/dpll.h | 3 --- >> 5 files changed, 10 insertions(+), 52 deletions(-) >> >
On Sat, Dec 09, 2023 at 11:37:50AM +0100, Jiri Pirko wrote: > Fri, Dec 08, 2023 at 01:06:34PM CET, vadim.fedorenko@linux.dev wrote: > >On 07/12/2023 15:12, Jiri Pirko wrote: > >> From: Jiri Pirko <jiri@nvidia.com> > >> > >> Mode supported is currently reported to the user exactly the same, as > >> the current mode. That's because mode changing is not implemented. > >> Remove the leftover mode_supported() op and use mode_get() to fill up > >> the supported mode exposed to user. > >> > >> One, if even, mode changing is going to be introduced, this could be No need to respin, but I guess this should be "if ever". > >> very easily taken back. In the meantime, prevent drivers form > >> implementing this in wrong way (as for example recent netdevsim > >> implementation attempt intended to do). > >> > > > >I'm OK to remove from ptp_ocp part because it's really only one mode > >supported. But I would like to hear something from Arkadiusz about the > >plans to maybe implement mode change in ice? > > As I wrote in the patch description, if ever there is going > to be implementation, this could be very easily taken back. Now for > sure there was already attempt to misimplement this :) FWIIW, I agree with this reasoning. Let's add the appropriate API when there is a real user of it. Reviewed-by: Simon Horman <horms@kernel.org> ...
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Thu, 7 Dec 2023 16:12:04 +0100 you wrote: > From: Jiri Pirko <jiri@nvidia.com> > > Mode supported is currently reported to the user exactly the same, as > the current mode. That's because mode changing is not implemented. > Remove the leftover mode_supported() op and use mode_get() to fill up > the supported mode exposed to user. > > [...] Here is the summary with links: - [net-next] dpll: remove leftover mode_supported() op and use mode_get() instead https://git.kernel.org/netdev/net-next/c/4f7aa122bc92 You are awesome, thank you!
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c index 442a0ebeb953..e1a4737500f5 100644 --- a/drivers/dpll/dpll_netlink.c +++ b/drivers/dpll/dpll_netlink.c @@ -101,13 +101,17 @@ dpll_msg_add_mode_supported(struct sk_buff *msg, struct dpll_device *dpll, { const struct dpll_device_ops *ops = dpll_device_ops(dpll); enum dpll_mode mode; + int ret; - if (!ops->mode_supported) - return 0; - for (mode = DPLL_MODE_MANUAL; mode <= DPLL_MODE_MAX; mode++) - if (ops->mode_supported(dpll, dpll_priv(dpll), mode, extack)) - if (nla_put_u32(msg, DPLL_A_MODE_SUPPORTED, mode)) - return -EMSGSIZE; + /* No mode change is supported now, so the only supported mode is the + * one obtained by mode_get(). + */ + + ret = ops->mode_get(dpll, dpll_priv(dpll), &mode, extack); + if (ret) + return ret; + if (nla_put_u32(msg, DPLL_A_MODE_SUPPORTED, mode)) + return -EMSGSIZE; return 0; } diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index 86b180cb32a0..b9c5eced6326 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -512,31 +512,6 @@ ice_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv, return 0; } -/** - * ice_dpll_mode_supported - check if dpll's working mode is supported - * @dpll: registered dpll pointer - * @dpll_priv: private data pointer passed on dpll registration - * @mode: mode to be checked for support - * @extack: error reporting - * - * Dpll subsystem callback. Provides information if working mode is supported - * by dpll. - * - * Return: - * * true - mode is supported - * * false - mode is not supported - */ -static bool ice_dpll_mode_supported(const struct dpll_device *dpll, - void *dpll_priv, - enum dpll_mode mode, - struct netlink_ext_ack *extack) -{ - if (mode == DPLL_MODE_AUTOMATIC) - return true; - - return false; -} - /** * ice_dpll_mode_get - get dpll's working mode * @dpll: registered dpll pointer @@ -1197,7 +1172,6 @@ static const struct dpll_pin_ops ice_dpll_output_ops = { static const struct dpll_device_ops ice_dpll_ops = { .lock_status_get = ice_dpll_lock_status_get, - .mode_supported = ice_dpll_mode_supported, .mode_get = ice_dpll_mode_get, }; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c index 2cd81bb32c66..a7ffd61fe248 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c @@ -128,18 +128,9 @@ static int mlx5_dpll_device_mode_get(const struct dpll_device *dpll, return 0; } -static bool mlx5_dpll_device_mode_supported(const struct dpll_device *dpll, - void *priv, - enum dpll_mode mode, - struct netlink_ext_ack *extack) -{ - return mode == DPLL_MODE_MANUAL; -} - static const struct dpll_device_ops mlx5_dpll_device_ops = { .lock_status_get = mlx5_dpll_device_lock_status_get, .mode_get = mlx5_dpll_device_mode_get, - .mode_supported = mlx5_dpll_device_mode_supported, }; static int mlx5_dpll_pin_direction_get(const struct dpll_pin *pin, diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c index 4021d3d325f9..b022af3d20fe 100644 --- a/drivers/ptp/ptp_ocp.c +++ b/drivers/ptp/ptp_ocp.c @@ -4260,13 +4260,6 @@ static int ptp_ocp_dpll_mode_get(const struct dpll_device *dpll, void *priv, return 0; } -static bool ptp_ocp_dpll_mode_supported(const struct dpll_device *dpll, - void *priv, const enum dpll_mode mode, - struct netlink_ext_ack *extack) -{ - return mode == DPLL_MODE_AUTOMATIC; -} - static int ptp_ocp_dpll_direction_get(const struct dpll_pin *pin, void *pin_priv, const struct dpll_device *dpll, @@ -4350,7 +4343,6 @@ static int ptp_ocp_dpll_frequency_get(const struct dpll_pin *pin, static const struct dpll_device_ops dpll_ops = { .lock_status_get = ptp_ocp_dpll_lock_status_get, .mode_get = ptp_ocp_dpll_mode_get, - .mode_supported = ptp_ocp_dpll_mode_supported, }; static const struct dpll_pin_ops dpll_pins_ops = { diff --git a/include/linux/dpll.h b/include/linux/dpll.h index 578fc5fa3750..b1a5f9ca8ee5 100644 --- a/include/linux/dpll.h +++ b/include/linux/dpll.h @@ -17,9 +17,6 @@ struct dpll_pin; struct dpll_device_ops { int (*mode_get)(const struct dpll_device *dpll, void *dpll_priv, enum dpll_mode *mode, struct netlink_ext_ack *extack); - bool (*mode_supported)(const struct dpll_device *dpll, void *dpll_priv, - const enum dpll_mode mode, - struct netlink_ext_ack *extack); int (*lock_status_get)(const struct dpll_device *dpll, void *dpll_priv, enum dpll_lock_status *status, struct netlink_ext_ack *extack);