| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230814201709.655a24e2@kernel.org>
Date: Mon, 14 Aug 2023 20:17:09 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Jiri Pirko <jiri@...nulli.us>, Arkadiusz Kubalewski
<arkadiusz.kubalewski@...el.com>, Jonathan Lemon
<jonathan.lemon@...il.com>, Paolo Abeni <pabeni@...hat.com>, Milena Olech
<milena.olech@...el.com>, Michal Michalik <michal.michalik@...el.com>,
linux-arm-kernel@...ts.infradead.org, poros@...hat.com,
mschmidt@...hat.com, netdev@...r.kernel.org, linux-clk@...r.kernel.org,
Bart Van Assche <bvanassche@....org>, intel-wired-lan@...ts.osuosl.org,
Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH net-next v4 3/9] dpll: core: Add DPLL framework base
functions
On Fri, 11 Aug 2023 21:03:34 +0100 Vadim Fedorenko wrote:
> DPLL framework is used to represent and configure DPLL devices
> in systems. Each device that has DPLL and can configure inputs
> and outputs can use this framework.
>
> Implement core framework functions for further interactions
> with device drivers implementing dpll subsystem, as well as for
> interactions of DPLL netlink framework part with the subsystem
> itself.
> +static struct dpll_device *
> +dpll_device_alloc(const u64 clock_id, u32 device_idx, struct module *module)
> +{
> + struct dpll_device *dpll;
> + int ret;
> +
> + dpll = kzalloc(sizeof(*dpll), GFP_KERNEL);
> + if (!dpll)
> + return ERR_PTR(-ENOMEM);
> + refcount_set(&dpll->refcount, 1);
> + INIT_LIST_HEAD(&dpll->registration_list);
> + dpll->device_idx = device_idx;
> + dpll->clock_id = clock_id;
> + dpll->module = module;
> + ret = xa_alloc(&dpll_device_xa, &dpll->id, dpll, xa_limit_16b,
> + GFP_KERNEL);
Why only 16b and why not _cyclic?
> +/**
> + * dpll_device_register - register the dpll device in the subsystem
> + * @dpll: pointer to a dpll
> + * @type: type of a dpll
> + * @ops: ops for a dpll device
> + * @priv: pointer to private information of owner
> + *
> + * Make dpll device available for user space.
> + *
> + * Context: Acquires a lock (dpll_lock)
> + * Return:
> + * * 0 on success
> + * * negative - error value
> + */
> +int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
> + const struct dpll_device_ops *ops, void *priv)
> +{
> + struct dpll_device_registration *reg;
> + bool first_registration = false;
> +
> + if (WARN_ON(!ops))
> + return -EINVAL;
> + if (WARN_ON(!ops->mode_get))
> + return -EINVAL;
> + if (WARN_ON(!ops->lock_status_get))
> + return -EINVAL;
> + if (WARN_ON(type < DPLL_TYPE_PPS || type > DPLL_TYPE_MAX))
> + return -EINVAL;
> +
> + mutex_lock(&dpll_lock);
> + reg = dpll_device_registration_find(dpll, ops, priv);
> + if (reg) {
> + mutex_unlock(&dpll_lock);
> + return -EEXIST;
> + }
> +
> + reg = kzalloc(sizeof(*reg), GFP_KERNEL);
> + if (!reg) {
> + mutex_unlock(&dpll_lock);
> + return -ENOMEM;
> + }
> + reg->ops = ops;
> + reg->priv = priv;
> + dpll->type = type;
> + first_registration = list_empty(&dpll->registration_list);
> + list_add_tail(®->list, &dpll->registration_list);
> + if (!first_registration) {
> + mutex_unlock(&dpll_lock);
> + return 0;
> + }
> +
> + xa_set_mark(&dpll_device_xa, dpll->id, DPLL_REGISTERED);
> + mutex_unlock(&dpll_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dpll_device_register);
Is the registration flow documented? It's a bit atypical so we should
write some pseudocode somewhere.
> +/**
> + * dpll_device_unregister - unregister dpll device
> + * @dpll: registered dpll pointer
> + * @ops: ops for a dpll device
> + * @priv: pointer to private information of owner
> + *
> + * Unregister device, make it unavailable for userspace.
> + * Note: It does not free the memory
> + * Context: Acquires a lock (dpll_lock)
> + */
> +void dpll_device_unregister(struct dpll_device *dpll,
> + const struct dpll_device_ops *ops, void *priv)
> +{
> + struct dpll_device_registration *reg;
> +
> + mutex_lock(&dpll_lock);
> + ASSERT_DPLL_REGISTERED(dpll);
> + reg = dpll_device_registration_find(dpll, ops, priv);
> + if (WARN_ON(!reg)) {
> + mutex_unlock(&dpll_lock);
> + return;
> + }
> + list_del(®->list);
> + kfree(reg);
> +
> + if (!list_empty(&dpll->registration_list)) {
> + mutex_unlock(&dpll_lock);
> + return;
> + }
> + xa_clear_mark(&dpll_device_xa, dpll->id, DPLL_REGISTERED);
> + mutex_unlock(&dpll_lock);
> +}
> +EXPORT_SYMBOL_GPL(dpll_device_unregister);
> +/**
> + * struct dpll_pin - structure for a dpll pin
> + * @id: unique id number for pin given by dpll subsystem
> + * @pin_idx: index of a pin given by dev driver
> + * @clock_id: clock_id of creator
> + * @module: module of creator
> + * @dpll_refs: hold referencees to dplls pin was registered with
> + * @parent_refs: hold references to parent pins pin was registered with
> + * @prop: pointer to pin properties given by registerer
> + * @rclk_dev_name: holds name of device when pin can recover clock from it
> + * @refcount: refcount
> + **/
> +struct dpll_pin {
> + u32 id;
> + u32 pin_idx;
> + u64 clock_id;
> + struct module *module;
> + struct xarray dpll_refs;
> + struct xarray parent_refs;
> + const struct dpll_pin_properties *prop;
> + char *rclk_dev_name;
Where is rclk_dev_name filled in?
> +struct dpll_pin_ops {
> + int (*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);
> + int (*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);
> + int (*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);
> + int (*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);
> + int (*state_on_pin_get)(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_pin *parent_pin,
> + void *parent_pin_priv,
> + enum dpll_pin_state *state,
> + struct netlink_ext_ack *extack);
> + int (*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);
> + int (*state_on_pin_set)(const struct dpll_pin *pin, void *pin_priv,
> + const struct dpll_pin *parent_pin,
> + void *parent_pin_priv,
> + const enum dpll_pin_state state,
> + struct netlink_ext_ack *extack);
> + int (*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);
> + int (*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);
> + int (*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);
> +};
The ops need a kdoc
> +struct dpll_device
> +*dpll_device_get(u64 clock_id, u32 dev_driver_id, struct module *module);
nit: * is part of the type, it goes on the previous line
Powered by blists - more mailing lists