[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZIQlhyXJAtcp1Fjr@nanopsycho>
Date: Sat, 10 Jun 2023 09:25:59 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Cc: kuba@...nel.org, vadfed@...a.com, jonathan.lemon@...il.com,
pabeni@...hat.com, corbet@....net, davem@...emloft.net,
edumazet@...gle.com, vadfed@...com, jesse.brandeburg@...el.com,
anthony.l.nguyen@...el.com, saeedm@...dia.com, leon@...nel.org,
richardcochran@...il.com, sj@...nel.org, javierm@...hat.com,
ricardo.canuelo@...labora.com, mst@...hat.com, tzimmermann@...e.de,
michal.michalik@...el.com, gregkh@...uxfoundation.org,
jacek.lawrynowicz@...ux.intel.com, airlied@...hat.com,
ogabbay@...nel.org, arnd@...db.de, nipun.gupta@....com,
axboe@...nel.dk, linux@...y.sk, masahiroy@...nel.org,
benjamin.tissoires@...hat.com, geert+renesas@...der.be,
milena.olech@...el.com, kuniyu@...zon.com, liuhangbin@...il.com,
hkallweit1@...il.com, andy.ren@...cruise.com, razor@...ckwall.org,
idosch@...dia.com, lucien.xin@...il.com, nicolas.dichtel@...nd.com,
phil@....cc, claudiajkang@...il.com, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
intel-wired-lan@...ts.osuosl.org, linux-rdma@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, poros@...hat.com,
mschmidt@...hat.com, linux-clk@...r.kernel.org,
vadim.fedorenko@...ux.dev
Subject: Re: [RFC PATCH v8 05/10] dpll: api header: Add DPLL framework base
functions
Fri, Jun 09, 2023 at 02:18:48PM CEST, arkadiusz.kubalewski@...el.com wrote:
>From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
>
>DPLL framework is used to represent and configure DPLL devices
>in systems. Each device that has DPLL and can configure sources
>and outputs can use this framework. Netlink interface is used to
>provide configuration data and to receive notification messages
>about changes in the configuration or status of DPLL device.
>Inputs and outputs of the DPLL device are represented as special
>objects which could be dynamically added to and removed from DPLL
>device.
>
>Add kernel api header, make dpll subsystem available to device drivers.
>
>Add/update makefiles/Kconfig to allow compilation of dpll subsystem.
>
>Co-developed-by: Milena Olech <milena.olech@...el.com>
>Signed-off-by: Milena Olech <milena.olech@...el.com>
>Co-developed-by: Michal Michalik <michal.michalik@...el.com>
>Signed-off-by: Michal Michalik <michal.michalik@...el.com>
>Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
>Co-developed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>---
> MAINTAINERS | 8 +++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/dpll/Kconfig | 7 ++
> drivers/dpll/Makefile | 9 +++
> include/linux/dpll.h | 144 ++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 171 insertions(+)
> create mode 100644 drivers/dpll/Kconfig
> create mode 100644 drivers/dpll/Makefile
> create mode 100644 include/linux/dpll.h
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 288d9a5edb9d..0e69429ecc55 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -6306,6 +6306,14 @@ F: Documentation/networking/device_drivers/ethernet/freescale/dpaa2/switch-drive
> F: drivers/net/ethernet/freescale/dpaa2/dpaa2-switch*
> F: drivers/net/ethernet/freescale/dpaa2/dpsw*
>
>+DPLL CLOCK SUBSYSTEM
>+M: Vadim Fedorenko <vadfed@...com>
>+L: netdev@...r.kernel.org
>+S: Maintained
I think status should be rather "Supported":
"Supported: Someone is actually paid to look after this."
Also, I think that it would be good to have Arkadiusz Kubalewski
listed here, as he is the one that knows the subsystem by heart.
Also, if you don't mind, I would be happy as a co-maintainer of the
subsystem to be listed here, as I helped to shape the code and
interfaces and I also know it pretty good.
>+F: drivers/dpll/*
>+F: include/net/dpll.h
>+F: include/uapi/linux/dpll.h
>+
> DRBD DRIVER
> M: Philipp Reisner <philipp.reisner@...bit.com>
> M: Lars Ellenberg <lars.ellenberg@...bit.com>
>diff --git a/drivers/Kconfig b/drivers/Kconfig
>index 514ae6b24cb2..ce5f63918eba 100644
>--- a/drivers/Kconfig
>+++ b/drivers/Kconfig
>@@ -243,4 +243,6 @@ source "drivers/hte/Kconfig"
>
> source "drivers/cdx/Kconfig"
>
>+source "drivers/dpll/Kconfig"
>+
> endmenu
>diff --git a/drivers/Makefile b/drivers/Makefile
>index 7241d80a7b29..6fea42a6dd05 100644
>--- a/drivers/Makefile
>+++ b/drivers/Makefile
>@@ -195,3 +195,4 @@ obj-$(CONFIG_PECI) += peci/
> obj-$(CONFIG_HTE) += hte/
> obj-$(CONFIG_DRM_ACCEL) += accel/
> obj-$(CONFIG_CDX_BUS) += cdx/
>+obj-$(CONFIG_DPLL) += dpll/
>diff --git a/drivers/dpll/Kconfig b/drivers/dpll/Kconfig
>new file mode 100644
>index 000000000000..a4cae73f20d3
>--- /dev/null
>+++ b/drivers/dpll/Kconfig
>@@ -0,0 +1,7 @@
>+# SPDX-License-Identifier: GPL-2.0-only
>+#
>+# Generic DPLL drivers configuration
>+#
>+
>+config DPLL
>+ bool
>diff --git a/drivers/dpll/Makefile b/drivers/dpll/Makefile
>new file mode 100644
>index 000000000000..2e5b27850110
>--- /dev/null
>+++ b/drivers/dpll/Makefile
>@@ -0,0 +1,9 @@
>+# SPDX-License-Identifier: GPL-2.0
>+#
>+# Makefile for DPLL drivers.
>+#
>+
>+obj-$(CONFIG_DPLL) += dpll.o
>+dpll-y += dpll_core.o
>+dpll-y += dpll_netlink.o
>+dpll-y += dpll_nl.o
>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>new file mode 100644
>index 000000000000..a18bcaa13553
>--- /dev/null
>+++ b/include/linux/dpll.h
>@@ -0,0 +1,144 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/*
>+ * Copyright (c) 2023 Meta Platforms, Inc. and affiliates
>+ * Copyright (c) 2023 Intel and affiliates
>+ */
>+
>+#ifndef __DPLL_H__
>+#define __DPLL_H__
>+
>+#include <uapi/linux/dpll.h>
>+#include <linux/device.h>
>+#include <linux/netlink.h>
>+
>+struct dpll_device;
>+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);
>+ int (*mode_set)(const struct dpll_device *dpll, void *dpll_priv,
>+ const 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 (*source_pin_idx_get)(const struct dpll_device *dpll,
>+ void *dpll_priv,
>+ u32 *pin_idx,
>+ 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);
>+ int (*temp_get)(const struct dpll_device *dpll, void *dpll_priv,
>+ s32 *temp, struct netlink_ext_ack *extack);
>+};
>+
>+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);
>+};
>+
>+struct dpll_pin_frequency {
>+ u64 min;
>+ u64 max;
>+};
>+
>+#define DPLL_PIN_FREQUENCY_RANGE(_min, _max) \
>+ { \
>+ .min = _min, \
>+ .max = _max, \
>+ }
>+
>+#define DPLL_PIN_FREQUENCY(_val) DPLL_PIN_FREQUENCY_RANGE(_val, _val)
>+#define DPLL_PIN_FREQUENCY_1PPS \
>+ DPLL_PIN_FREQUENCY(DPLL_PIN_FREQUENCY_1_HZ)
>+#define DPLL_PIN_FREQUENCY_10MHZ \
>+ DPLL_PIN_FREQUENCY(DPLL_PIN_FREQUENCY_10_MHZ)
>+#define DPLL_PIN_FREQUENCY_IRIG_B \
>+ DPLL_PIN_FREQUENCY(DPLL_PIN_FREQUENCY_10_KHZ)
>+#define DPLL_PIN_FREQUENCY_DCF77 \
>+ DPLL_PIN_FREQUENCY(DPLL_PIN_FREQUENCY_77_5_KHZ)
>+
>+struct dpll_pin_properties {
>+ const char *board_label;
>+ const char *panel_label;
>+ const char *package_label;
>+ enum dpll_pin_type type;
>+ unsigned long capabilities;
>+ u32 freq_supported_num;
>+ struct dpll_pin_frequency *freq_supported;
>+};
>+
>+struct dpll_device
>+*dpll_device_get(u64 clock_id, u32 dev_driver_id, struct module *module);
>+
>+void dpll_device_put(struct dpll_device *dpll);
>+
>+int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>+ const struct dpll_device_ops *ops, void *priv);
>+
>+void dpll_device_unregister(struct dpll_device *dpll,
>+ const struct dpll_device_ops *ops, void *priv);
>+
>+struct dpll_pin
>+*dpll_pin_get(u64 clock_id, u32 dev_driver_id, struct module *module,
>+ const struct dpll_pin_properties *prop);
>+
>+int dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
>+ const struct dpll_pin_ops *ops, void *priv);
>+
>+void dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
>+ const struct dpll_pin_ops *ops, void *priv);
>+
>+void dpll_pin_put(struct dpll_pin *pin);
>+
>+int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
>+ const struct dpll_pin_ops *ops, void *priv);
>+
>+void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
>+ const struct dpll_pin_ops *ops, void *priv);
>+
>+int dpll_device_change_ntf(struct dpll_device *dpll);
>+
>+int dpll_pin_change_ntf(struct dpll_pin *pin);
Why exactly did you split this into a separate patch? To me, it does not
make any sense. Please squash this header addition to the
>+
>+#endif
>--
>2.37.3
>
Powered by blists - more mailing lists