From: Simon Horman <horms@kernel.org>
To: Michal Michalik <michal.michalik@intel.com>
Cc: netdev@vger.kernel.org, vadim.fedorenko@linux.dev,
arkadiusz.kubalewski@intel.com, jonathan.lemon@gmail.com,
pabeni@redhat.com, poros@redhat.com, milena.olech@intel.com,
mschmidt@redhat.com, linux-clk@vger.kernel.org,
bvanassche@acm.org, kuba@kernel.org, davem@davemloft.net,
edumazet@google.com
Subject: Re: [PATCH RFC net-next v4 1/2] netdevsim: implement DPLL for subsystem selftests
Date: Thu, 23 Nov 2023 14:41:27 +0000 [thread overview]
Message-ID: <20231123144127.GJ6339@kernel.org> (raw)
In-Reply-To: <20231123105243.7992-2-michal.michalik@intel.com>
On Thu, Nov 23, 2023 at 05:52:42AM -0500, Michal Michalik wrote:
> DPLL subsystem integration tests require a module which mimics the
> behavior of real driver which supports DPLL hardware. To fully test the
> subsystem the netdevsim is amended with DPLL implementation.
>
> Signed-off-by: Michal Michalik <michal.michalik@intel.com>
...
> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index b4d3b9cde8bd..76da4e8aa9af 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -342,6 +342,17 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
> debugfs_create_file("max_vfs", 0600, nsim_dev->ddir,
> nsim_dev, &nsim_dev_max_vfs_fops);
>
> + debugfs_create_u64("dpll_clock_id", 0600,
> + nsim_dev->ddir, &nsim_dev->dpll.dpll_e_pd.clock_id);
> + debugfs_create_u32("dpll_e_status", 0600, nsim_dev->ddir,
> + &nsim_dev->dpll.dpll_e_pd.status);
> + debugfs_create_u32("dpll_p_status", 0600, nsim_dev->ddir,
> + &nsim_dev->dpll.dpll_p_pd.status);
> + debugfs_create_u32("dpll_e_temp", 0600, nsim_dev->ddir,
> + &nsim_dev->dpll.dpll_e_pd.temperature);
> + debugfs_create_u32("dpll_p_temp", 0600, nsim_dev->ddir,
> + &nsim_dev->dpll.dpll_p_pd.temperature);
> +
> nsim_dev->nodes_ddir = debugfs_create_dir("rate_nodes", nsim_dev->ddir);
> if (IS_ERR(nsim_dev->nodes_ddir)) {
> err = PTR_ERR(nsim_dev->nodes_ddir);
> @@ -1601,14 +1612,21 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
> if (err)
> goto err_psample_exit;
>
> - err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
> + err = nsim_dpll_init_owner(&nsim_dev->dpll, nsim_bus_dev->port_count);
> if (err)
> goto err_hwstats_exit;
>
> + err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
> + if (err)
> + goto err_teardown_dpll;
> +
> nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY;
> devl_unlock(devlink);
> +
> return 0;
>
> +err_teardown_dpll:
> + nsim_dpll_free_owner(&nsim_dev->dpll);
> err_hwstats_exit:
> nsim_dev_hwstats_exit(nsim_dev);
> err_psample_exit:
> @@ -1656,6 +1674,7 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
> }
>
> nsim_dev_port_del_all(nsim_dev);
> + nsim_dpll_free_owner(&nsim_dev->dpll);
> nsim_dev_hwstats_exit(nsim_dev);
> nsim_dev_psample_exit(nsim_dev);
> nsim_dev_health_exit(nsim_dev);
> diff --git a/drivers/net/netdevsim/dpll.c b/drivers/net/netdevsim/dpll.c
> new file mode 100644
> index 000000000000..26a8b0f3be16
> --- /dev/null
> +++ b/drivers/net/netdevsim/dpll.c
> @@ -0,0 +1,489 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023, Intel Corporation.
> + * Author: Michal Michalik <michal.michalik@intel.com>
> + */
> +#include "netdevsim.h"
> +
> +#define EEC_DPLL_DEV 0
> +#define EEC_DPLL_TEMPERATURE 20
> +#define PPS_DPLL_DEV 1
> +#define PPS_DPLL_TEMPERATURE 30
> +
> +#define PIN_GNSS 0
> +#define PIN_GNSS_CAPABILITIES DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE
> +#define PIN_GNSS_PRIORITY 5
> +
> +#define PIN_PPS 1
> +#define PIN_PPS_CAPABILITIES \
> + (DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE | \
> + DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE | \
> + DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE)
> +#define PIN_PPS_PRIORITY 6
> +
> +#define PIN_RCLK 2
> +#define PIN_RCLK_CAPABILITIES \
> + (DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE | \
> + DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE)
> +#define PIN_RCLK_PRIORITY 7
> +
> +#define EEC_PINS_NUMBER 3
> +#define PPS_PINS_NUMBER 2
> +
> +static int nsim_fill_pin_properties(struct dpll_pin_properties *pp,
> + const char *label, enum dpll_pin_type type,
> + unsigned long caps, u32 freq_supp_num,
> + u64 fmin, u64 fmax)
> +{
> + struct dpll_pin_frequency *freq_supp;
> +
> + freq_supp = kzalloc(sizeof(*freq_supp), GFP_KERNEL);
> + if (!freq_supp)
> + goto freq_supp;
> + freq_supp->min = fmin;
> + freq_supp->max = fmax;
> +
> + pp->board_label = kasprintf(GFP_KERNEL, "%s_brd", label);
> + if (!pp->board_label)
> + goto board_label;
> + pp->panel_label = kasprintf(GFP_KERNEL, "%s_pnl", label);
> + if (!pp->panel_label)
> + goto panel_label;
> + pp->package_label = kasprintf(GFP_KERNEL, "%s_pcg", label);
> + if (!pp->package_label)
> + goto package_label;
> + pp->freq_supported_num = freq_supp_num;
> + pp->freq_supported = freq_supp;
> + pp->capabilities = caps;
> + pp->type = type;
> +
> + return 0;
> +
> +package_label:
> + kfree(pp->panel_label);
> +panel_label:
> + kfree(pp->board_label);
> +board_label:
> + kfree(freq_supp);
> +freq_supp:
> + return -ENOMEM;
> +}
> +
> +static void nsim_fill_pin_pd(struct nsim_pin_priv_data *pd, u64 frequency,
> + u32 prio, enum dpll_pin_direction direction)
> +{
> + pd->state_dpll = DPLL_PIN_STATE_DISCONNECTED;
> + pd->state_pin = DPLL_PIN_STATE_DISCONNECTED;
> + pd->frequency = frequency;
> + pd->direction = direction;
> + pd->prio = prio;
> +}
> +
> +static int nsim_dds_ops_mode_get(const struct dpll_device *dpll,
> + void *dpll_priv, enum dpll_mode *mode,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_dpll_priv_data *pd = dpll_priv;
> + *mode = pd->mode;
> + return 0;
> +};
> +
> +static bool nsim_dds_ops_mode_supported(const struct dpll_device *dpll,
> + void *dpll_priv,
> + const enum dpll_mode mode,
> + struct netlink_ext_ack *extack)
> +{
> + return true;
> +};
> +
> +static int nsim_dds_ops_lock_status_get(const struct dpll_device *dpll,
> + void *dpll_priv,
> + enum dpll_lock_status *status,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_dpll_priv_data *pd = dpll_priv;
> +
> + *status = pd->status;
> + return 0;
> +};
> +
> +static int nsim_dds_ops_temp_get(const struct dpll_device *dpll,
> + void *dpll_priv, s32 *temp,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_dpll_priv_data *pd = dpll_priv;
> +
> + *temp = pd->temperature;
> + return 0;
> +};
> +
> +static int nsim_pin_frequency_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv, const u64 frequency,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + pd->frequency = frequency;
> + return 0;
> +};
> +
> +static int nsim_pin_frequency_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv, u64 *frequency,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + *frequency = pd->frequency;
> + return 0;
> +};
> +
> +static int nsim_pin_direction_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv,
> + const enum dpll_pin_direction direction,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + pd->direction = direction;
> + return 0;
> +};
> +
> +static int nsim_pin_direction_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv,
> + enum dpll_pin_direction *direction,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + *direction = pd->direction;
> + return 0;
> +};
> +
> +static int nsim_pin_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_pin *parent_pin,
> + void *parent_priv,
> + enum dpll_pin_state *state,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + *state = pd->state_pin;
> + return 0;
> +};
> +
> +static int nsim_pin_state_on_dpll_get(const struct dpll_pin *pin,
> + void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv,
> + enum dpll_pin_state *state,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + *state = pd->state_dpll;
> + return 0;
> +};
> +
> +static int nsim_pin_state_on_pin_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_pin *parent_pin,
> + void *parent_priv,
> + const enum dpll_pin_state state,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + pd->state_pin = state;
> + return 0;
> +};
> +
> +static int nsim_pin_state_on_dpll_set(const struct dpll_pin *pin,
> + void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv,
> + const enum dpll_pin_state state,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + pd->state_dpll = state;
> + return 0;
> +};
> +
> +static int nsim_pin_prio_get(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + u32 *prio, struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + *prio = pd->prio;
> + return 0;
> +};
> +
> +static int nsim_pin_prio_set(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_device *dpll, void *dpll_priv,
> + const u32 prio, struct netlink_ext_ack *extack)
> +{
> + struct nsim_pin_priv_data *pd = pin_priv;
> +
> + pd->prio = prio;
> + return 0;
> +};
> +
> +static void nsim_free_pin_properties(struct dpll_pin_properties *pp)
> +{
> + kfree(pp->board_label);
> + kfree(pp->panel_label);
> + kfree(pp->package_label);
> + kfree(pp->freq_supported);
> +}
> +
> +static struct dpll_device_ops nsim_dds_ops = {
> + .mode_get = nsim_dds_ops_mode_get,
> + .mode_supported = nsim_dds_ops_mode_supported,
> + .lock_status_get = nsim_dds_ops_lock_status_get,
> + .temp_get = nsim_dds_ops_temp_get,
> +};
> +
> +static struct dpll_pin_ops nsim_pin_ops = {
> + .frequency_set = nsim_pin_frequency_set,
> + .frequency_get = nsim_pin_frequency_get,
> + .direction_set = nsim_pin_direction_set,
> + .direction_get = nsim_pin_direction_get,
> + .state_on_pin_get = nsim_pin_state_on_pin_get,
> + .state_on_dpll_get = nsim_pin_state_on_dpll_get,
> + .state_on_pin_set = nsim_pin_state_on_pin_set,
> + .state_on_dpll_set = nsim_pin_state_on_dpll_set,
> + .prio_get = nsim_pin_prio_get,
> + .prio_set = nsim_pin_prio_set,
> +};
> +
> +int nsim_dpll_init_owner(struct nsim_dpll *dpll, unsigned int ports_count)
> +{
> + u64 clock_id;
> + int err;
> +
> + get_random_bytes(&clock_id, sizeof(clock_id));
> +
> + /* Create EEC DPLL */
> + dpll->dpll_e = dpll_device_get(clock_id, EEC_DPLL_DEV, THIS_MODULE);
> + if (IS_ERR(dpll->dpll_e))
> + return -EFAULT;
> +
> + dpll->dpll_e_pd.temperature = EEC_DPLL_TEMPERATURE;
> + dpll->dpll_e_pd.mode = DPLL_MODE_AUTOMATIC;
> + dpll->dpll_e_pd.clock_id = clock_id;
> + dpll->dpll_e_pd.status = DPLL_LOCK_STATUS_UNLOCKED;
> +
> + err = dpll_device_register(dpll->dpll_e, DPLL_TYPE_EEC, &nsim_dds_ops,
> + &dpll->dpll_e_pd);
> + if (err)
> + goto e_reg;
> +
> + /* Create PPS DPLL */
> + dpll->dpll_p = dpll_device_get(clock_id, PPS_DPLL_DEV, THIS_MODULE);
> + if (IS_ERR(dpll->dpll_p))
> + goto dpll_p;
> +
> + dpll->dpll_p_pd.temperature = PPS_DPLL_TEMPERATURE;
> + dpll->dpll_p_pd.mode = DPLL_MODE_MANUAL;
> + dpll->dpll_p_pd.clock_id = clock_id;
> + dpll->dpll_p_pd.status = DPLL_LOCK_STATUS_UNLOCKED;
> +
> + err = dpll_device_register(dpll->dpll_p, DPLL_TYPE_PPS, &nsim_dds_ops,
> + &dpll->dpll_p_pd);
> + if (err)
> + goto p_reg;
> +
> + /* Create first pin (GNSS) */
> + err = nsim_fill_pin_properties(&dpll->pp_gnss, "GNSS",
> + DPLL_PIN_TYPE_GNSS,
> + PIN_GNSS_CAPABILITIES, 1,
> + DPLL_PIN_FREQUENCY_1_HZ,
> + DPLL_PIN_FREQUENCY_1_HZ);
> + if (err)
> + goto pp_gnss;
> + dpll->p_gnss =
> + dpll_pin_get(clock_id, PIN_GNSS, THIS_MODULE, &dpll->pp_gnss);
> + if (IS_ERR(dpll->p_gnss))
> + goto p_gnss;
Hi Michal,
I think that err needs to be set to something inside the if condition
above.
> + nsim_fill_pin_pd(&dpll->p_gnss_pd, DPLL_PIN_FREQUENCY_1_HZ,
> + PIN_GNSS_PRIORITY, DPLL_PIN_DIRECTION_INPUT);
> + err = dpll_pin_register(dpll->dpll_e, dpll->p_gnss, &nsim_pin_ops,
> + &dpll->p_gnss_pd);
> + if (err)
> + goto e_gnss_reg;
> +
> + /* Create second pin (PPS) */
> + err = nsim_fill_pin_properties(&dpll->pp_pps, "PPS", DPLL_PIN_TYPE_EXT,
> + PIN_PPS_CAPABILITIES, 1,
> + DPLL_PIN_FREQUENCY_1_HZ,
> + DPLL_PIN_FREQUENCY_1_HZ);
> + if (err)
> + goto pp_pps;
> + dpll->p_pps =
> + dpll_pin_get(clock_id, PIN_PPS, THIS_MODULE, &dpll->pp_pps);
> + if (IS_ERR(dpll->p_pps)) {
> + err = -EFAULT;
> + goto p_pps;
> + }
> + nsim_fill_pin_pd(&dpll->p_pps_pd, DPLL_PIN_FREQUENCY_1_HZ,
> + PIN_PPS_PRIORITY, DPLL_PIN_DIRECTION_INPUT);
> + err = dpll_pin_register(dpll->dpll_e, dpll->p_pps, &nsim_pin_ops,
> + &dpll->p_pps_pd);
> + if (err)
> + goto e_pps_reg;
> + err = dpll_pin_register(dpll->dpll_p, dpll->p_pps, &nsim_pin_ops,
> + &dpll->p_pps_pd);
> + if (err)
> + goto p_pps_reg;
> +
> + dpll->pp_rclk =
> + kcalloc(ports_count, sizeof(*dpll->pp_rclk), GFP_KERNEL);
> + dpll->p_rclk = kcalloc(ports_count, sizeof(*dpll->p_rclk), GFP_KERNEL);
> + dpll->p_rclk_pd =
> + kcalloc(ports_count, sizeof(*dpll->p_rclk_pd), GFP_KERNEL);
> +
> + return 0;
> +
> +p_pps_reg:
> + dpll_pin_unregister(dpll->dpll_e, dpll->p_pps, &nsim_pin_ops,
> + &dpll->p_pps_pd);
> +e_pps_reg:
> + dpll_pin_put(dpll->p_pps);
> +p_pps:
> + nsim_free_pin_properties(&dpll->pp_pps);
> +pp_pps:
> + dpll_pin_unregister(dpll->dpll_e, dpll->p_gnss, &nsim_pin_ops,
> + &dpll->p_gnss_pd);
> +e_gnss_reg:
> + dpll_pin_put(dpll->p_gnss);
> +p_gnss:
> + nsim_free_pin_properties(&dpll->pp_gnss);
> +pp_gnss:
> + dpll_device_unregister(dpll->dpll_p, &nsim_dds_ops, &dpll->dpll_p_pd);
> +p_reg:
> + dpll_device_put(dpll->dpll_p);
> +dpll_p:
> + dpll_device_unregister(dpll->dpll_e, &nsim_dds_ops, &dpll->dpll_e_pd);
> +e_reg:
> + dpll_device_put(dpll->dpll_e);
> + return err;
> +}
...
next prev parent reply other threads:[~2023-11-23 14:41 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-23 10:52 [PATCH RFC net-next v4 0/2] selftests/dpll: DPLL subsystem integration tests Michal Michalik
2023-11-23 10:52 ` [PATCH RFC net-next v4 1/2] netdevsim: implement DPLL for subsystem selftests Michal Michalik
2023-11-23 12:24 ` Jiri Pirko
2023-11-30 16:55 ` Michalik, Michal
2023-12-01 7:49 ` Jiri Pirko
2023-11-23 14:41 ` Simon Horman [this message]
2023-11-30 17:22 ` Michalik, Michal
2023-11-23 10:52 ` [PATCH RFC net-next v4 2/2] selftests/dpll: add DPLL system integration selftests Michal Michalik
2023-11-29 17:39 ` Jakub Kicinski
2023-11-30 17:46 ` Michalik, Michal
2023-12-01 6:51 ` Jakub Kicinski
2023-12-01 18:33 ` Michalik, Michal
2023-12-01 19:52 ` Jakub Kicinski
2023-12-04 12:44 ` Michalik, Michal
2023-12-05 3:02 ` Jakub Kicinski
2023-12-01 20:03 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231123144127.GJ6339@kernel.org \
--to=horms@kernel.org \
--cc=arkadiusz.kubalewski@intel.com \
--cc=bvanassche@acm.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jonathan.lemon@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=michal.michalik@intel.com \
--cc=milena.olech@intel.com \
--cc=mschmidt@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=poros@redhat.com \
--cc=vadim.fedorenko@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).