2022-11-19 23:37:08

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 00/18] treewide: fix object files shared between several modules

This is a follow-up to the series[0] that adds Kbuild warning if an
object is linked into several modules (including vmlinux) in order
to prevent hidden side effects from appearing.
The original series, as well as this one, was inspired by the recent
issue[1] with the ZSTD modules on a platform which has such sets of
vmlinux cflags and module cflags so that objects built with those
two even refuse to link with each other.
The final goal is to forbid linking one object several times
entirely.

Patches 1-7 and 10-11 was runtime-tested by me. Pathes 8-9 and 12-18
are compile-time tested only (compile, link, modpost), so I
encourage the maintainers to review them carefully. At least the
last one, for cpsw, most likely has issues :D
Masahiro's patches are taken from his WIP tree[2], with the two last
finished by me.

This mostly is a monotonic work, all scores go to Masahiro and
Alexey :P

[0] https://lore.kernel.org/linux-kbuild/[email protected]
[1] https://github.com/torvalds/linux/commit/637a642f5ca5e850186bb64ac75ebb0f124b458d
[2] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/log/?h=tmp4

Alexander Lobakin (9):
EDAC: i10nm, skx: fix mixed module-builtin object
platform/x86: int3472: fix object shared between several modules
mtd: tests: fix object shared between several modules
crypto: octeontx2: fix objects shared between several modules
dsa: ocelot: fix mixed module-builtin object
net: dpaa2: fix mixed module-builtin object
net: hns3: fix mixed module-builtin object
net: octeontx2: fix mixed module-builtin object
net: cpsw: fix mixed module-builtin object

Masahiro Yamada (9):
block/rnbd: fix mixed module-builtin object
drm/bridge: imx: fix mixed module-builtin object
drm/bridge: imx: turn imx8{qm,qxp}-ldb into single-object modules
sound: fix mixed module-builtin object
mfd: rsmu: fix mixed module-builtin object
mfd: rsmu: turn rsmu-{core,i2c,spi} into single-object modules
net: liquidio: fix mixed module-builtin object
net: enetc: fix mixed module-builtin object
net: emac, cpsw: fix mixed module-builtin object (davinci_cpdma)

drivers/block/rnbd/Makefile | 6 +-
drivers/block/rnbd/rnbd-common.c | 23 -
drivers/block/rnbd/rnbd-proto.h | 14 +-
drivers/crypto/marvell/octeontx2/Makefile | 11 +-
drivers/crypto/marvell/octeontx2/cn10k_cpt.c | 9 +-
drivers/crypto/marvell/octeontx2/cn10k_cpt.h | 2 -
.../marvell/octeontx2/otx2_cpt_common.h | 2 -
.../marvell/octeontx2/otx2_cpt_mbox_common.c | 14 +-
drivers/crypto/marvell/octeontx2/otx2_cptlf.c | 11 +
.../marvell/octeontx2/otx2_cptpf_main.c | 2 +
.../marvell/octeontx2/otx2_cptvf_main.c | 2 +
drivers/edac/Kconfig | 11 +-
drivers/edac/Makefile | 7 +-
drivers/edac/i10nm_base.c | 2 +
drivers/edac/skx_base.c | 2 +
drivers/edac/skx_common.c | 21 +-
drivers/edac/skx_common.h | 4 +-
drivers/gpu/drm/bridge/imx/Makefile | 4 -
drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 221 -----
drivers/gpu/drm/bridge/imx/imx-ldb-helper.h | 213 +++-
.../imx/{imx8qm-ldb-drv.c => imx8qm-ldb.c} | 0
.../imx/{imx8qxp-ldb-drv.c => imx8qxp-ldb.c} | 0
drivers/mfd/Kconfig | 8 +-
drivers/mfd/Makefile | 3 +-
drivers/mfd/{rsmu_core.c => rsmu-core.c} | 3 +
drivers/mfd/{rsmu_i2c.c => rsmu-i2c.c} | 0
drivers/mfd/{rsmu_spi.c => rsmu-spi.c} | 0
drivers/mtd/tests/Makefile | 17 +-
drivers/mtd/tests/mtd_test.c | 9 +
drivers/mtd/tests/nandbiterrs.c | 2 +
drivers/mtd/tests/oobtest.c | 2 +
drivers/mtd/tests/pagetest.c | 2 +
drivers/mtd/tests/readtest.c | 2 +
drivers/mtd/tests/speedtest.c | 2 +
drivers/mtd/tests/stresstest.c | 2 +
drivers/mtd/tests/subpagetest.c | 2 +
drivers/mtd/tests/torturetest.c | 2 +
drivers/net/dsa/ocelot/Kconfig | 18 +-
drivers/net/dsa/ocelot/Makefile | 12 +-
drivers/net/dsa/ocelot/felix.c | 6 +
drivers/net/dsa/ocelot/felix_vsc9959.c | 2 +
drivers/net/dsa/ocelot/seville_vsc9953.c | 2 +
drivers/net/ethernet/cavium/Kconfig | 5 +
drivers/net/ethernet/cavium/liquidio/Makefile | 4 +-
.../cavium/liquidio/cn23xx_pf_device.c | 4 +
.../cavium/liquidio/cn23xx_vf_device.c | 3 +
.../ethernet/cavium/liquidio/cn66xx_device.c | 1 +
.../ethernet/cavium/liquidio/cn68xx_device.c | 1 +
.../net/ethernet/cavium/liquidio/lio_core.c | 16 +
.../ethernet/cavium/liquidio/lio_ethtool.c | 1 +
.../ethernet/cavium/liquidio/octeon_device.c | 24 +
.../ethernet/cavium/liquidio/octeon_droq.c | 4 +
.../ethernet/cavium/liquidio/octeon_mem_ops.c | 5 +
.../net/ethernet/cavium/liquidio/octeon_nic.c | 3 +
.../cavium/liquidio/request_manager.c | 14 +
.../cavium/liquidio/response_manager.c | 3 +
drivers/net/ethernet/freescale/dpaa2/Kconfig | 6 +
drivers/net/ethernet/freescale/dpaa2/Makefile | 6 +-
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +
.../net/ethernet/freescale/dpaa2/dpaa2-mac.c | 15 +-
.../ethernet/freescale/dpaa2/dpaa2-switch.c | 2 +
drivers/net/ethernet/freescale/enetc/Kconfig | 5 +
drivers/net/ethernet/freescale/enetc/Makefile | 7 +-
drivers/net/ethernet/freescale/enetc/enetc.c | 21 +
.../net/ethernet/freescale/enetc/enetc_cbdr.c | 7 +
.../ethernet/freescale/enetc/enetc_ethtool.c | 2 +
.../net/ethernet/freescale/enetc/enetc_pf.c | 2 +
.../net/ethernet/freescale/enetc/enetc_vf.c | 2 +
drivers/net/ethernet/hisilicon/Kconfig | 5 +
drivers/net/ethernet/hisilicon/hns3/Makefile | 13 +-
.../hns3/hns3_common/hclge_comm_cmd.c | 27 +-
.../hns3/hns3_common/hclge_comm_cmd.h | 8 -
.../hns3/hns3_common/hclge_comm_rss.c | 17 +
.../hns3/hns3_common/hclge_comm_tqp_stats.c | 5 +
.../hisilicon/hns3/hns3pf/hclge_main.c | 2 +
.../hisilicon/hns3/hns3vf/hclgevf_main.c | 2 +
.../net/ethernet/marvell/octeontx2/Kconfig | 5 +
.../ethernet/marvell/octeontx2/nic/Makefile | 14 +-
.../marvell/octeontx2/nic/otx2_common.h | 1 -
.../marvell/octeontx2/nic/otx2_dcbnl.c | 8 +-
.../marvell/octeontx2/nic/otx2_devlink.c | 2 +
.../ethernet/marvell/octeontx2/nic/otx2_pf.c | 2 +
.../ethernet/marvell/octeontx2/nic/otx2_ptp.c | 2 +-
.../ethernet/marvell/octeontx2/nic/otx2_vf.c | 2 +
drivers/net/ethernet/ti/Kconfig | 13 +
drivers/net/ethernet/ti/Makefile | 16 +-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +
drivers/net/ethernet/ti/cpsw.c | 3 +
drivers/net/ethernet/ti/cpsw_ale.c | 20 +
drivers/net/ethernet/ti/cpsw_ethtool.c | 24 +
drivers/net/ethernet/ti/cpsw_new.c | 3 +
drivers/net/ethernet/ti/cpsw_priv.c | 36 +
drivers/net/ethernet/ti/cpsw_sl.c | 8 +
drivers/net/ethernet/ti/davinci_cpdma.c | 33 +
drivers/net/ethernet/ti/davinci_emac.c | 2 +
drivers/net/ethernet/ti/netcp_core.c | 2 +
drivers/net/ethernet/ti/netcp_ethss.c | 2 +
drivers/platform/x86/intel/int3472/Makefile | 8 +-
drivers/platform/x86/intel/int3472/common.c | 8 +
drivers/platform/x86/intel/int3472/discrete.c | 2 +
drivers/platform/x86/intel/int3472/tps68470.c | 2 +
sound/soc/codecs/Makefile | 6 +-
sound/soc/codecs/wcd-clsh-v2.c | 903 -----------------
sound/soc/codecs/wcd-clsh-v2.h | 917 +++++++++++++++++-
104 files changed, 1679 insertions(+), 1288 deletions(-)
delete mode 100644 drivers/block/rnbd/rnbd-common.c
delete mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
rename drivers/gpu/drm/bridge/imx/{imx8qm-ldb-drv.c => imx8qm-ldb.c} (100%)
rename drivers/gpu/drm/bridge/imx/{imx8qxp-ldb-drv.c => imx8qxp-ldb.c} (100%)
rename drivers/mfd/{rsmu_core.c => rsmu-core.c} (94%)
rename drivers/mfd/{rsmu_i2c.c => rsmu-i2c.c} (100%)
rename drivers/mfd/{rsmu_spi.c => rsmu-spi.c} (100%)
delete mode 100644 sound/soc/codecs/wcd-clsh-v2.c

--
2.38.1




2022-11-19 23:37:29

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 02/18] drm/bridge: imx: fix mixed module-builtin object

From: Masahiro Yamada <[email protected]>

With CONFIG_DRM_IMX8QM_LDB=m and CONFIG_DRM_IMX8QXP_LDB=y (or vice
versa), imx-ldb-helper.o is linked to a module and also to vmlinux
even though the expected CFLAGS are different between builtins and
modules.

This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
Fixing mixed module-builtin objects").

Turn helpers in imx-ldb-helper.c into inline functions.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-and-tested-by: Alexander Lobakin <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/gpu/drm/bridge/imx/Makefile | 4 +-
drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 221 --------------------
drivers/gpu/drm/bridge/imx/imx-ldb-helper.h | 213 +++++++++++++++++--
3 files changed, 197 insertions(+), 241 deletions(-)
delete mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c

diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index aa90ec8d5433..64b93009376a 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -1,7 +1,7 @@
-imx8qm-ldb-objs := imx-ldb-helper.o imx8qm-ldb-drv.o
+imx8qm-ldb-objs := imx8qm-ldb-drv.o
obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o

-imx8qxp-ldb-objs := imx-ldb-helper.o imx8qxp-ldb-drv.o
+imx8qxp-ldb-objs := imx8qxp-ldb-drv.o
obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o

obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
deleted file mode 100644
index 7338b84bc83d..000000000000
--- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
+++ /dev/null
@@ -1,221 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (C) 2012 Sascha Hauer, Pengutronix
- * Copyright 2019,2020,2022 NXP
- */
-
-#include <linux/media-bus-format.h>
-#include <linux/mfd/syscon.h>
-#include <linux/of.h>
-#include <linux/regmap.h>
-
-#include <drm/drm_bridge.h>
-#include <drm/drm_of.h>
-#include <drm/drm_print.h>
-
-#include "imx-ldb-helper.h"
-
-bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
-{
- return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
-}
-
-bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
-{
- return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
- ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
-}
-
-int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
- struct drm_bridge_state *bridge_state,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
-
- ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
- ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
-
- return 0;
-}
-
-void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
- bool is_split = ldb_channel_is_split_link(ldb_ch);
-
- if (is_split)
- ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
-
- switch (ldb_ch->out_bus_format) {
- case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
- break;
- case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
- if (ldb_ch->chno == 0 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
- if (ldb_ch->chno == 1 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
- break;
- case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
- if (ldb_ch->chno == 0 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
- LDB_BIT_MAP_CH0_JEIDA;
- if (ldb_ch->chno == 1 || is_split)
- ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
- LDB_BIT_MAP_CH1_JEIDA;
- break;
- }
-}
-
-void ldb_bridge_enable_helper(struct drm_bridge *bridge)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
-
- /*
- * Platform specific bridge drivers should set ldb_ctrl properly
- * for the enablement, so just write the ctrl_reg here.
- */
- regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
-}
-
-void ldb_bridge_disable_helper(struct drm_bridge *bridge)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
- bool is_split = ldb_channel_is_split_link(ldb_ch);
-
- if (ldb_ch->chno == 0 || is_split)
- ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
- if (ldb_ch->chno == 1 || is_split)
- ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
-
- regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
-}
-
-int ldb_bridge_attach_helper(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- struct ldb_channel *ldb_ch = bridge->driver_private;
- struct ldb *ldb = ldb_ch->ldb;
-
- if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
- DRM_DEV_ERROR(ldb->dev,
- "do not support creating a drm_connector\n");
- return -EINVAL;
- }
-
- if (!bridge->encoder) {
- DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
- return -ENODEV;
- }
-
- return drm_bridge_attach(bridge->encoder,
- ldb_ch->next_bridge, bridge,
- DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-}
-
-int ldb_init_helper(struct ldb *ldb)
-{
- struct device *dev = ldb->dev;
- struct device_node *np = dev->of_node;
- struct device_node *child;
- int ret;
- u32 i;
-
- ldb->regmap = syscon_node_to_regmap(np->parent);
- if (IS_ERR(ldb->regmap)) {
- ret = PTR_ERR(ldb->regmap);
- if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
- return ret;
- }
-
- for_each_available_child_of_node(np, child) {
- struct ldb_channel *ldb_ch;
-
- ret = of_property_read_u32(child, "reg", &i);
- if (ret || i > MAX_LDB_CHAN_NUM - 1) {
- ret = -EINVAL;
- DRM_DEV_ERROR(dev,
- "invalid channel node address: %u\n", i);
- of_node_put(child);
- return ret;
- }
-
- ldb_ch = ldb->channel[i];
- ldb_ch->ldb = ldb;
- ldb_ch->chno = i;
- ldb_ch->is_available = true;
- ldb_ch->np = child;
-
- ldb->available_ch_cnt++;
- }
-
- return 0;
-}
-
-int ldb_find_next_bridge_helper(struct ldb *ldb)
-{
- struct device *dev = ldb->dev;
- struct ldb_channel *ldb_ch;
- int ret, i;
-
- for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
- ldb_ch = ldb->channel[i];
-
- if (!ldb_ch->is_available)
- continue;
-
- ldb_ch->next_bridge = devm_drm_of_get_bridge(dev, ldb_ch->np,
- 1, 0);
- if (IS_ERR(ldb_ch->next_bridge)) {
- ret = PTR_ERR(ldb_ch->next_bridge);
- if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev,
- "failed to get next bridge: %d\n",
- ret);
- return ret;
- }
- }
-
- return 0;
-}
-
-void ldb_add_bridge_helper(struct ldb *ldb,
- const struct drm_bridge_funcs *bridge_funcs)
-{
- struct ldb_channel *ldb_ch;
- int i;
-
- for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
- ldb_ch = ldb->channel[i];
-
- if (!ldb_ch->is_available)
- continue;
-
- ldb_ch->bridge.driver_private = ldb_ch;
- ldb_ch->bridge.funcs = bridge_funcs;
- ldb_ch->bridge.of_node = ldb_ch->np;
-
- drm_bridge_add(&ldb_ch->bridge);
- }
-}
-
-void ldb_remove_bridge_helper(struct ldb *ldb)
-{
- struct ldb_channel *ldb_ch;
- int i;
-
- for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
- ldb_ch = ldb->channel[i];
-
- if (!ldb_ch->is_available)
- continue;
-
- drm_bridge_remove(&ldb_ch->bridge);
- }
-}
diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
index a0a5cde27fbc..42e9b4aa8399 100644
--- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
+++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
@@ -65,32 +65,209 @@ struct ldb {

#define bridge_to_ldb_ch(b) container_of(b, struct ldb_channel, bridge)

-bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch);
-bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch);
+static inline bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
+{
+ return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
+}

-int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
- struct drm_bridge_state *bridge_state,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state);
+static inline bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
+{
+ return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
+ ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
+}

-void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
- const struct drm_display_mode *mode,
- const struct drm_display_mode *adjusted_mode);
+static inline int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;

-void ldb_bridge_enable_helper(struct drm_bridge *bridge);
+ ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
+ ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;

-void ldb_bridge_disable_helper(struct drm_bridge *bridge);
+ return 0;
+}

-int ldb_bridge_attach_helper(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags);
+static inline void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
+ bool is_split = ldb_channel_is_split_link(ldb_ch);

-int ldb_init_helper(struct ldb *ldb);
+ if (is_split)
+ ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;

-int ldb_find_next_bridge_helper(struct ldb *ldb);
+ switch (ldb_ch->out_bus_format) {
+ case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+ if (ldb_ch->chno == 0 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+ if (ldb_ch->chno == 1 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
+ break;
+ case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+ if (ldb_ch->chno == 0 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
+ LDB_BIT_MAP_CH0_JEIDA;
+ if (ldb_ch->chno == 1 || is_split)
+ ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
+ LDB_BIT_MAP_CH1_JEIDA;
+ break;
+ }
+}

-void ldb_add_bridge_helper(struct ldb *ldb,
- const struct drm_bridge_funcs *bridge_funcs);
+static inline void ldb_bridge_enable_helper(struct drm_bridge *bridge)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;

-void ldb_remove_bridge_helper(struct ldb *ldb);
+ /*
+ * Platform specific bridge drivers should set ldb_ctrl properly
+ * for the enablement, so just write the ctrl_reg here.
+ */
+ regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
+}
+
+static inline void ldb_bridge_disable_helper(struct drm_bridge *bridge)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
+ bool is_split = ldb_channel_is_split_link(ldb_ch);
+
+ if (ldb_ch->chno == 0 || is_split)
+ ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
+ if (ldb_ch->chno == 1 || is_split)
+ ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
+
+ regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
+}
+
+static inline int ldb_bridge_attach_helper(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct ldb_channel *ldb_ch = bridge->driver_private;
+ struct ldb *ldb = ldb_ch->ldb;
+
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+ DRM_DEV_ERROR(ldb->dev,
+ "do not support creating a drm_connector\n");
+ return -EINVAL;
+ }
+
+ if (!bridge->encoder) {
+ DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
+ return -ENODEV;
+ }
+
+ return drm_bridge_attach(bridge->encoder,
+ ldb_ch->next_bridge, bridge,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+}
+
+static inline int ldb_init_helper(struct ldb *ldb)
+{
+ struct device *dev = ldb->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *child;
+ int ret;
+ u32 i;
+
+ ldb->regmap = syscon_node_to_regmap(np->parent);
+ if (IS_ERR(ldb->regmap)) {
+ ret = PTR_ERR(ldb->regmap);
+ if (ret != -EPROBE_DEFER)
+ DRM_DEV_ERROR(dev, "failed to get regmap: %d\n", ret);
+ return ret;
+ }
+
+ for_each_available_child_of_node(np, child) {
+ struct ldb_channel *ldb_ch;
+
+ ret = of_property_read_u32(child, "reg", &i);
+ if (ret || i > MAX_LDB_CHAN_NUM - 1) {
+ ret = -EINVAL;
+ DRM_DEV_ERROR(dev,
+ "invalid channel node address: %u\n", i);
+ of_node_put(child);
+ return ret;
+ }
+
+ ldb_ch = ldb->channel[i];
+ ldb_ch->ldb = ldb;
+ ldb_ch->chno = i;
+ ldb_ch->is_available = true;
+ ldb_ch->np = child;
+
+ ldb->available_ch_cnt++;
+ }
+
+ return 0;
+}
+
+static inline int ldb_find_next_bridge_helper(struct ldb *ldb)
+{
+ struct device *dev = ldb->dev;
+ struct ldb_channel *ldb_ch;
+ int ret, i;
+
+ for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
+ ldb_ch = ldb->channel[i];
+
+ if (!ldb_ch->is_available)
+ continue;
+
+ ldb_ch->next_bridge = devm_drm_of_get_bridge(dev, ldb_ch->np,
+ 1, 0);
+ if (IS_ERR(ldb_ch->next_bridge)) {
+ ret = PTR_ERR(ldb_ch->next_bridge);
+ if (ret != -EPROBE_DEFER)
+ DRM_DEV_ERROR(dev,
+ "failed to get next bridge: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static inline void ldb_add_bridge_helper(struct ldb *ldb,
+ const struct drm_bridge_funcs *bridge_funcs)
+{
+ struct ldb_channel *ldb_ch;
+ int i;
+
+ for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
+ ldb_ch = ldb->channel[i];
+
+ if (!ldb_ch->is_available)
+ continue;
+
+ ldb_ch->bridge.driver_private = ldb_ch;
+ ldb_ch->bridge.funcs = bridge_funcs;
+ ldb_ch->bridge.of_node = ldb_ch->np;
+
+ drm_bridge_add(&ldb_ch->bridge);
+ }
+}
+
+static inline void ldb_remove_bridge_helper(struct ldb *ldb)
+{
+ struct ldb_channel *ldb_ch;
+ int i;
+
+ for (i = 0; i < MAX_LDB_CHAN_NUM; i++) {
+ ldb_ch = ldb->channel[i];
+
+ if (!ldb_ch->is_available)
+ continue;
+
+ drm_bridge_remove(&ldb_ch->bridge);
+ }
+}

#endif /* __IMX_LDB_HELPER__ */
--
2.38.1



2022-11-19 23:38:16

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

common.o is linked to both intel_skl_int3472_{discrete,tps68470}:

> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
> common.o is added to multiple modules: intel_skl_int3472_discrete
> intel_skl_int3472_tps68470

Although both drivers share one Kconfig option
(CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
into several modules (and/or vmlinux).
Under certain circumstances, such can lead to the situation fixed by
commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").

Introduce the new module, intel_skl_int3472_common, to provide the
functions from common.o to both discrete and tps68470 drivers. This
adds only 3 exports and doesn't provide any changes to the actual
code.

Fixes: a2f9fbc247ee ("platform/x86: int3472: Split into 2 drivers")
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/platform/x86/intel/int3472/Makefile | 8 +++++---
drivers/platform/x86/intel/int3472/common.c | 8 ++++++++
drivers/platform/x86/intel/int3472/discrete.c | 2 ++
drivers/platform/x86/intel/int3472/tps68470.c | 2 ++
4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/Makefile b/drivers/platform/x86/intel/int3472/Makefile
index cfec7784c5c9..53cc0e7db749 100644
--- a/drivers/platform/x86/intel/int3472/Makefile
+++ b/drivers/platform/x86/intel/int3472/Makefile
@@ -1,4 +1,6 @@
-obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_discrete.o \
+obj-$(CONFIG_INTEL_SKL_INT3472) += intel_skl_int3472_common.o \
+ intel_skl_int3472_discrete.o \
intel_skl_int3472_tps68470.o
-intel_skl_int3472_discrete-y := discrete.o clk_and_regulator.o common.o
-intel_skl_int3472_tps68470-y := tps68470.o tps68470_board_data.o common.o
+intel_skl_int3472_common-y := common.o
+intel_skl_int3472_discrete-y := discrete.o clk_and_regulator.o
+intel_skl_int3472_tps68470-y := tps68470.o tps68470_board_data.o
diff --git a/drivers/platform/x86/intel/int3472/common.c b/drivers/platform/x86/intel/int3472/common.c
index 9db2bb0bbba4..bd573ff46610 100644
--- a/drivers/platform/x86/intel/int3472/common.c
+++ b/drivers/platform/x86/intel/int3472/common.c
@@ -2,6 +2,7 @@
/* Author: Dan Scally <[email protected]> */

#include <linux/acpi.h>
+#include <linux/module.h>
#include <linux/slab.h>

#include "common.h"
@@ -29,6 +30,7 @@ union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev, char *i

return obj;
}
+EXPORT_SYMBOL_NS_GPL(skl_int3472_get_acpi_buffer, INTEL_SKL_INT3472);

int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
{
@@ -52,6 +54,7 @@ int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
kfree(obj);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(skl_int3472_fill_cldb, INTEL_SKL_INT3472);

/* sensor_adev_ret may be NULL, name_ret must not be NULL */
int skl_int3472_get_sensor_adev_and_name(struct device *dev,
@@ -80,3 +83,8 @@ int skl_int3472_get_sensor_adev_and_name(struct device *dev,

return ret;
}
+EXPORT_SYMBOL_NS_GPL(skl_int3472_get_sensor_adev_and_name, INTEL_SKL_INT3472);
+
+MODULE_DESCRIPTION("Intel SkyLake INT3472 Common Module");
+MODULE_AUTHOR("Daniel Scally <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 974a132db651..a1f3b593cea6 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -414,6 +414,8 @@ static struct platform_driver int3472_discrete = {
};
module_platform_driver(int3472_discrete);

+MODULE_IMPORT_NS(INTEL_SKL_INT3472);
+
MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
MODULE_AUTHOR("Daniel Scally <[email protected]>");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index 5b8d1a9620a5..3c983aa7731f 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -255,6 +255,8 @@ static struct i2c_driver int3472_tps68470 = {
};
module_i2c_driver(int3472_tps68470);

+MODULE_IMPORT_NS(INTEL_SKL_INT3472);
+
MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
MODULE_AUTHOR("Daniel Scally <[email protected]>");
MODULE_LICENSE("GPL v2");
--
2.38.1



2022-11-19 23:38:30

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
(or vice versa), felix.o are linked to a module and also to vmlinux
even though the expected CFLAGS are different between builtins and
modules.
This is the same situation as fixed by
commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
There's also no need to duplicate relatively big piece of object
code into two modules.

Introduce the new module, mscc_core, to provide the common functions
to both mscc_felix and mscc_seville.

Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/net/dsa/ocelot/Kconfig | 18 ++++++++++--------
drivers/net/dsa/ocelot/Makefile | 12 +++++-------
drivers/net/dsa/ocelot/felix.c | 6 ++++++
drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++
drivers/net/dsa/ocelot/seville_vsc9953.c | 2 ++
5 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig
index 08db9cf76818..59845274e374 100644
--- a/drivers/net/dsa/ocelot/Kconfig
+++ b/drivers/net/dsa/ocelot/Kconfig
@@ -1,4 +1,12 @@
# SPDX-License-Identifier: GPL-2.0-only
+
+config NET_DSA_MSCC_CORE
+ tristate
+ select MSCC_OCELOT_SWITCH_LIB
+ select NET_DSA_TAG_OCELOT_8021Q
+ select NET_DSA_TAG_OCELOT
+ select PCS_LYNX
+
config NET_DSA_MSCC_FELIX
tristate "Ocelot / Felix Ethernet switch support"
depends on NET_DSA && PCI
@@ -7,11 +15,8 @@ config NET_DSA_MSCC_FELIX
depends on HAS_IOMEM
depends on PTP_1588_CLOCK_OPTIONAL
depends on NET_SCH_TAPRIO || NET_SCH_TAPRIO=n
- select MSCC_OCELOT_SWITCH_LIB
- select NET_DSA_TAG_OCELOT_8021Q
- select NET_DSA_TAG_OCELOT
+ select NET_DSA_MSCC_CORE
select FSL_ENETC_MDIO
- select PCS_LYNX
help
This driver supports the VSC9959 (Felix) switch, which is embedded as
a PCIe function of the NXP LS1028A ENETC RCiEP.
@@ -22,11 +27,8 @@ config NET_DSA_MSCC_SEVILLE
depends on NET_VENDOR_MICROSEMI
depends on HAS_IOMEM
depends on PTP_1588_CLOCK_OPTIONAL
+ select NET_DSA_MSCC_CORE
select MDIO_MSCC_MIIM
- select MSCC_OCELOT_SWITCH_LIB
- select NET_DSA_TAG_OCELOT_8021Q
- select NET_DSA_TAG_OCELOT
- select PCS_LYNX
help
This driver supports the VSC9953 (Seville) switch, which is embedded
as a platform device on the NXP T1040 SoC.
diff --git a/drivers/net/dsa/ocelot/Makefile b/drivers/net/dsa/ocelot/Makefile
index f6dd131e7491..f8c74b59b996 100644
--- a/drivers/net/dsa/ocelot/Makefile
+++ b/drivers/net/dsa/ocelot/Makefile
@@ -1,11 +1,9 @@
# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_NET_DSA_MSCC_CORE) += mscc_core.o
obj-$(CONFIG_NET_DSA_MSCC_FELIX) += mscc_felix.o
obj-$(CONFIG_NET_DSA_MSCC_SEVILLE) += mscc_seville.o

-mscc_felix-objs := \
- felix.o \
- felix_vsc9959.o
-
-mscc_seville-objs := \
- felix.o \
- seville_vsc9953.o
+mscc_core-objs := felix.o
+mscc_felix-objs := felix_vsc9959.o
+mscc_seville-objs := seville_vsc9953.o
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index dd3a18cc89dd..f9d0a24ebc3a 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -2112,6 +2112,7 @@ const struct dsa_switch_ops felix_switch_ops = {
.port_set_host_flood = felix_port_set_host_flood,
.port_change_master = felix_port_change_master,
};
+EXPORT_SYMBOL_NS_GPL(felix_switch_ops, NET_DSA_MSCC_CORE);

struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)
{
@@ -2123,6 +2124,7 @@ struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)

return dsa_to_port(ds, port)->slave;
}
+EXPORT_SYMBOL_NS_GPL(felix_port_to_netdev, NET_DSA_MSCC_CORE);

int felix_netdev_to_port(struct net_device *dev)
{
@@ -2134,3 +2136,7 @@ int felix_netdev_to_port(struct net_device *dev)

return dp->index;
}
+EXPORT_SYMBOL_NS_GPL(felix_netdev_to_port, NET_DSA_MSCC_CORE);
+
+MODULE_DESCRIPTION("MSCC Switch driver core");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 26a35ae322d1..52c8bff79fa3 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2736,5 +2736,7 @@ static struct pci_driver felix_vsc9959_pci_driver = {
};
module_pci_driver(felix_vsc9959_pci_driver);

+MODULE_IMPORT_NS(NET_DSA_MSCC_CORE);
+
MODULE_DESCRIPTION("Felix Switch driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 7af33b2c685d..e9c63c642489 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -1115,5 +1115,7 @@ static struct platform_driver seville_vsc9953_driver = {
};
module_platform_driver(seville_vsc9953_driver);

+MODULE_IMPORT_NS(NET_DSA_MSCC_CORE);
+
MODULE_DESCRIPTION("Seville Switch driver");
MODULE_LICENSE("GPL v2");
--
2.38.1



2022-11-19 23:38:54

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 15/18] net: dpaa2: fix mixed module-builtin object

With CONFIG_FSL_DPAA2_ETH=m and CONFIG_FSL_DPAA2_SWITCH=y (or vice
versa), dpaa2-mac.o and dpmac.o are linked to a module and also to
vmlinux even though the expected CFLAGS are different between
builtins and modules.
This is the same situation as fixed by
commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
There's also no need to duplicate relatively big piece of object
code into two modules.

Introduce the new module, fsl-dpaa2-mac, to provide the common
functions to both fsl-dpaa2-eth and fsl-dpaa2-switch.

Misc: constify and shrink @dpaa2_mac_ethtool_stats while at it.

Fixes: 84cba72956fd ("dpaa2-switch: integrate the MAC endpoint support")
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/net/ethernet/freescale/dpaa2/Kconfig | 6 ++++++
drivers/net/ethernet/freescale/dpaa2/Makefile | 6 ++++--
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 ++
drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 15 ++++++++++++++-
.../net/ethernet/freescale/dpaa2/dpaa2-switch.c | 2 ++
5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/Kconfig b/drivers/net/ethernet/freescale/dpaa2/Kconfig
index d029b69c3f18..54c388f25c43 100644
--- a/drivers/net/ethernet/freescale/dpaa2/Kconfig
+++ b/drivers/net/ethernet/freescale/dpaa2/Kconfig
@@ -1,7 +1,12 @@
# SPDX-License-Identifier: GPL-2.0-only
+
+config FSL_DPAA2_MAC
+ tristate
+
config FSL_DPAA2_ETH
tristate "Freescale DPAA2 Ethernet"
depends on FSL_MC_BUS && FSL_MC_DPIO
+ select FSL_DPAA2_MAC
select PHYLINK
select PCS_LYNX
select FSL_XGMAC_MDIO
@@ -34,6 +39,7 @@ config FSL_DPAA2_SWITCH
tristate "Freescale DPAA2 Ethernet Switch"
depends on BRIDGE || BRIDGE=n
depends on NET_SWITCHDEV
+ select FSL_DPAA2_MAC
help
Driver for Freescale DPAA2 Ethernet Switch. This driver manages
switch objects discovered on the Freeescale MC bus.
diff --git a/drivers/net/ethernet/freescale/dpaa2/Makefile b/drivers/net/ethernet/freescale/dpaa2/Makefile
index 3d9842af7f10..9dbe2273c9a1 100644
--- a/drivers/net/ethernet/freescale/dpaa2/Makefile
+++ b/drivers/net/ethernet/freescale/dpaa2/Makefile
@@ -4,14 +4,16 @@
#

obj-$(CONFIG_FSL_DPAA2_ETH) += fsl-dpaa2-eth.o
+obj-$(CONFIG_FSL_DPAA2_MAC) += fsl-dpaa2-mac.o
obj-$(CONFIG_FSL_DPAA2_PTP_CLOCK) += fsl-dpaa2-ptp.o
obj-$(CONFIG_FSL_DPAA2_SWITCH) += fsl-dpaa2-switch.o

-fsl-dpaa2-eth-objs := dpaa2-eth.o dpaa2-ethtool.o dpni.o dpaa2-mac.o dpmac.o dpaa2-eth-devlink.o
+fsl-dpaa2-eth-objs := dpaa2-eth.o dpaa2-ethtool.o dpni.o dpaa2-eth-devlink.o
fsl-dpaa2-eth-${CONFIG_FSL_DPAA2_ETH_DCB} += dpaa2-eth-dcb.o
fsl-dpaa2-eth-${CONFIG_DEBUG_FS} += dpaa2-eth-debugfs.o
+fsl-dpaa2-mac-objs := dpaa2-mac.o dpmac.o
fsl-dpaa2-ptp-objs := dpaa2-ptp.o dprtc.o
-fsl-dpaa2-switch-objs := dpaa2-switch.o dpaa2-switch-ethtool.o dpsw.o dpaa2-switch-flower.o dpaa2-mac.o dpmac.o
+fsl-dpaa2-switch-objs := dpaa2-switch.o dpaa2-switch-ethtool.o dpsw.o dpaa2-switch-flower.o

# Needed by the tracing framework
CFLAGS_dpaa2-eth.o := -I$(src)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 8d029addddad..876c3ed6e2c5 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -28,6 +28,8 @@
#define CREATE_TRACE_POINTS
#include "dpaa2-eth-trace.h"

+MODULE_IMPORT_NS(FSL_DPAA2_MAC);
+
MODULE_LICENSE("Dual BSD/GPL");
MODULE_AUTHOR("Freescale Semiconductor, Inc");
MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver");
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
index 49ff85633783..dc2c7cde5435 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
@@ -339,12 +339,14 @@ void dpaa2_mac_start(struct dpaa2_mac *mac)
if (mac->serdes_phy)
phy_power_on(mac->serdes_phy);
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_start, FSL_DPAA2_MAC);

void dpaa2_mac_stop(struct dpaa2_mac *mac)
{
if (mac->serdes_phy)
phy_power_off(mac->serdes_phy);
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_stop, FSL_DPAA2_MAC);

int dpaa2_mac_connect(struct dpaa2_mac *mac)
{
@@ -435,6 +437,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)

return err;
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_connect, FSL_DPAA2_MAC);

void dpaa2_mac_disconnect(struct dpaa2_mac *mac)
{
@@ -447,6 +450,7 @@ void dpaa2_mac_disconnect(struct dpaa2_mac *mac)
of_phy_put(mac->serdes_phy);
mac->serdes_phy = NULL;
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_disconnect, FSL_DPAA2_MAC);

int dpaa2_mac_open(struct dpaa2_mac *mac)
{
@@ -495,6 +499,7 @@ int dpaa2_mac_open(struct dpaa2_mac *mac)
dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle);
return err;
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_open, FSL_DPAA2_MAC);

void dpaa2_mac_close(struct dpaa2_mac *mac)
{
@@ -504,8 +509,9 @@ void dpaa2_mac_close(struct dpaa2_mac *mac)
if (mac->fw_node)
fwnode_handle_put(mac->fw_node);
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_close, FSL_DPAA2_MAC);

-static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = {
+static const char * const dpaa2_mac_ethtool_stats[] = {
[DPMAC_CNT_ING_ALL_FRAME] = "[mac] rx all frames",
[DPMAC_CNT_ING_GOOD_FRAME] = "[mac] rx frames ok",
[DPMAC_CNT_ING_ERR_FRAME] = "[mac] rx frame errors",
@@ -542,6 +548,7 @@ int dpaa2_mac_get_sset_count(void)
{
return DPAA2_MAC_NUM_STATS;
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_get_sset_count, FSL_DPAA2_MAC);

void dpaa2_mac_get_strings(u8 *data)
{
@@ -553,6 +560,7 @@ void dpaa2_mac_get_strings(u8 *data)
p += ETH_GSTRING_LEN;
}
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_get_strings, FSL_DPAA2_MAC);

void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
{
@@ -572,3 +580,8 @@ void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data)
*(data + i) = value;
}
}
+EXPORT_SYMBOL_NS_GPL(dpaa2_mac_get_ethtool_stats, FSL_DPAA2_MAC);
+
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("Freescale Semiconductor, Inc");
+MODULE_DESCRIPTION("Freescale DPAA2 MAC Driver");
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 2b5909fa93cf..fccbaf75b512 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -3534,5 +3534,7 @@ static void __exit dpaa2_switch_driver_exit(void)
module_init(dpaa2_switch_driver_init);
module_exit(dpaa2_switch_driver_exit);

+MODULE_IMPORT_NS(FSL_DPAA2_MAC);
+
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("DPAA2 Ethernet Switch Driver");
--
2.38.1



2022-11-19 23:39:26

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 09/18] net: emac, cpsw: fix mixed module-builtin object (davinci_cpdma)

From: Masahiro Yamada <[email protected]>

CONFIG_TI_DAVINCI_EMAC, CONFIG_TI_CPSW and CONFIG_TI_CPSW_SWITCHDEV
are all tristate. This means that davinci_cpdma.o can be linked to
a module and also to vmlinux even though the expected CFLAGS are
different between builtins and modules.

This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
Fixing mixed module-builtin objects").

Introduce the new module, ti_davinci_cpdma, to provide the common
functions to these three modules.

[ alobakin: add exports ]

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Alexander Lobakin <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/net/ethernet/ti/Kconfig | 6 +++++
drivers/net/ethernet/ti/Makefile | 8 +++---
drivers/net/ethernet/ti/cpsw.c | 2 ++
drivers/net/ethernet/ti/cpsw_new.c | 2 ++
drivers/net/ethernet/ti/davinci_cpdma.c | 33 +++++++++++++++++++++++++
drivers/net/ethernet/ti/davinci_emac.c | 2 ++
6 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index fce06663e1e1..2ac0adf0c07d 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -17,9 +17,13 @@ config NET_VENDOR_TI

if NET_VENDOR_TI

+config TI_DAVINCI_CPDMA
+ tristate
+
config TI_DAVINCI_EMAC
tristate "TI DaVinci EMAC Support"
depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 ) || COMPILE_TEST
+ select TI_DAVINCI_CPDMA
select TI_DAVINCI_MDIO
select PHYLIB
select GENERIC_ALLOCATOR
@@ -51,6 +55,7 @@ config TI_CPSW
tristate "TI CPSW Switch Support"
depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
depends on TI_CPTS || !TI_CPTS
+ select TI_DAVINCI_CPDMA
select TI_DAVINCI_MDIO
select MFD_SYSCON
select PAGE_POOL
@@ -68,6 +73,7 @@ config TI_CPSW_SWITCHDEV
depends on NET_SWITCHDEV
depends on TI_CPTS || !TI_CPTS
select PAGE_POOL
+ select TI_DAVINCI_CPDMA
select TI_DAVINCI_MDIO
select MFD_SYSCON
select REGMAP
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index 75f761efbea7..28a741ed0ac8 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -9,15 +9,17 @@ obj-$(CONFIG_TI_CPSW_SWITCHDEV) += cpsw-common.o

obj-$(CONFIG_TLAN) += tlan.o
obj-$(CONFIG_CPMAC) += cpmac.o
+obj-$(CONFIG_TI_DAVINCI_CPDMA) += ti_davinci_cpdma.o
+ti_davinci_cpdma-y := davinci_cpdma.o
obj-$(CONFIG_TI_DAVINCI_EMAC) += ti_davinci_emac.o
-ti_davinci_emac-y := davinci_emac.o davinci_cpdma.o
+ti_davinci_emac-y := davinci_emac.o
obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o
obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o
obj-$(CONFIG_TI_CPTS) += cpts.o
obj-$(CONFIG_TI_CPSW) += ti_cpsw.o
-ti_cpsw-y := cpsw.o davinci_cpdma.o cpsw_ale.o cpsw_priv.o cpsw_sl.o cpsw_ethtool.o
+ti_cpsw-y := cpsw.o cpsw_ale.o cpsw_priv.o cpsw_sl.o cpsw_ethtool.o
obj-$(CONFIG_TI_CPSW_SWITCHDEV) += ti_cpsw_new.o
-ti_cpsw_new-y := cpsw_switchdev.o cpsw_new.o davinci_cpdma.o cpsw_ale.o cpsw_sl.o cpsw_priv.o cpsw_ethtool.o
+ti_cpsw_new-y := cpsw_switchdev.o cpsw_new.o cpsw_ale.o cpsw_sl.o cpsw_priv.o cpsw_ethtool.o

obj-$(CONFIG_TI_KEYSTONE_NETCP) += keystone_netcp.o
keystone_netcp-y := netcp_core.o cpsw_ale.o
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 13c9c2d6b79b..b7ac61329b20 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1796,6 +1796,8 @@ static struct platform_driver cpsw_driver = {

module_platform_driver(cpsw_driver);

+MODULE_IMPORT_NS(TI_DAVINCI_CPDMA);
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Cyril Chemparathy <[email protected]>");
MODULE_AUTHOR("Mugunthan V N <[email protected]>");
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 83596ec0c7cb..9ed398c04c04 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -2116,5 +2116,7 @@ static struct platform_driver cpsw_driver = {

module_platform_driver(cpsw_driver);

+MODULE_IMPORT_NS(TI_DAVINCI_CPDMA);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("TI CPSW switchdev Ethernet driver");
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index d2eab5cd1e0c..32ba94626cda 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -531,6 +531,7 @@ struct cpdma_ctlr *cpdma_ctlr_create(struct cpdma_params *params)
ctlr->num_chan = CPDMA_MAX_CHANNELS;
return ctlr;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctlr_create, TI_DAVINCI_CPDMA);

int cpdma_ctlr_start(struct cpdma_ctlr *ctlr)
{
@@ -591,6 +592,7 @@ int cpdma_ctlr_start(struct cpdma_ctlr *ctlr)
spin_unlock_irqrestore(&ctlr->lock, flags);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctlr_start, TI_DAVINCI_CPDMA);

int cpdma_ctlr_stop(struct cpdma_ctlr *ctlr)
{
@@ -623,6 +625,7 @@ int cpdma_ctlr_stop(struct cpdma_ctlr *ctlr)
spin_unlock_irqrestore(&ctlr->lock, flags);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctlr_stop, TI_DAVINCI_CPDMA);

int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
{
@@ -640,6 +643,7 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
cpdma_desc_pool_destroy(ctlr);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctlr_destroy, TI_DAVINCI_CPDMA);

int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable)
{
@@ -660,21 +664,25 @@ int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable)
spin_unlock_irqrestore(&ctlr->lock, flags);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctlr_int_ctrl, TI_DAVINCI_CPDMA);

void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value)
{
dma_reg_write(ctlr, CPDMA_MACEOIVECTOR, value);
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctlr_eoi, TI_DAVINCI_CPDMA);

u32 cpdma_ctrl_rxchs_state(struct cpdma_ctlr *ctlr)
{
return dma_reg_read(ctlr, CPDMA_RXINTSTATMASKED);
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctrl_rxchs_state, TI_DAVINCI_CPDMA);

u32 cpdma_ctrl_txchs_state(struct cpdma_ctlr *ctlr)
{
return dma_reg_read(ctlr, CPDMA_TXINTSTATMASKED);
}
+EXPORT_SYMBOL_NS_GPL(cpdma_ctrl_txchs_state, TI_DAVINCI_CPDMA);

static void cpdma_chan_set_descs(struct cpdma_ctlr *ctlr,
int rx, int desc_num,
@@ -802,6 +810,7 @@ int cpdma_chan_set_weight(struct cpdma_chan *ch, int weight)
spin_unlock_irqrestore(&ctlr->lock, flags);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_set_weight, TI_DAVINCI_CPDMA);

/* cpdma_chan_get_min_rate - get minimum allowed rate for channel
* Should be called before cpdma_chan_set_rate.
@@ -816,6 +825,7 @@ u32 cpdma_chan_get_min_rate(struct cpdma_ctlr *ctlr)

return DIV_ROUND_UP(divident, divisor);
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_get_min_rate, TI_DAVINCI_CPDMA);

/* cpdma_chan_set_rate - limits bandwidth for transmit channel.
* The bandwidth * limited channels have to be in order beginning from lowest.
@@ -860,6 +870,7 @@ int cpdma_chan_set_rate(struct cpdma_chan *ch, u32 rate)
spin_unlock_irqrestore(&ctlr->lock, flags);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_set_rate, TI_DAVINCI_CPDMA);

u32 cpdma_chan_get_rate(struct cpdma_chan *ch)
{
@@ -872,6 +883,7 @@ u32 cpdma_chan_get_rate(struct cpdma_chan *ch)

return rate;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_get_rate, TI_DAVINCI_CPDMA);

struct cpdma_chan *cpdma_chan_create(struct cpdma_ctlr *ctlr, int chan_num,
cpdma_handler_fn handler, int rx_type)
@@ -931,6 +943,7 @@ struct cpdma_chan *cpdma_chan_create(struct cpdma_ctlr *ctlr, int chan_num,
spin_unlock_irqrestore(&ctlr->lock, flags);
return chan;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_create, TI_DAVINCI_CPDMA);

int cpdma_chan_get_rx_buf_num(struct cpdma_chan *chan)
{
@@ -943,6 +956,7 @@ int cpdma_chan_get_rx_buf_num(struct cpdma_chan *chan)

return desc_num;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_get_rx_buf_num, TI_DAVINCI_CPDMA);

int cpdma_chan_destroy(struct cpdma_chan *chan)
{
@@ -964,6 +978,7 @@ int cpdma_chan_destroy(struct cpdma_chan *chan)
spin_unlock_irqrestore(&ctlr->lock, flags);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_destroy, TI_DAVINCI_CPDMA);

int cpdma_chan_get_stats(struct cpdma_chan *chan,
struct cpdma_chan_stats *stats)
@@ -976,6 +991,7 @@ int cpdma_chan_get_stats(struct cpdma_chan *chan,
spin_unlock_irqrestore(&chan->lock, flags);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_get_stats, TI_DAVINCI_CPDMA);

static void __cpdma_chan_submit(struct cpdma_chan *chan,
struct cpdma_desc __iomem *desc)
@@ -1100,6 +1116,7 @@ int cpdma_chan_idle_submit(struct cpdma_chan *chan, void *token, void *data,
spin_unlock_irqrestore(&chan->lock, flags);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_idle_submit, TI_DAVINCI_CPDMA);

int cpdma_chan_idle_submit_mapped(struct cpdma_chan *chan, void *token,
dma_addr_t data, int len, int directed)
@@ -1125,6 +1142,7 @@ int cpdma_chan_idle_submit_mapped(struct cpdma_chan *chan, void *token,
spin_unlock_irqrestore(&chan->lock, flags);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_idle_submit_mapped, TI_DAVINCI_CPDMA);

int cpdma_chan_submit(struct cpdma_chan *chan, void *token, void *data,
int len, int directed)
@@ -1150,6 +1168,7 @@ int cpdma_chan_submit(struct cpdma_chan *chan, void *token, void *data,
spin_unlock_irqrestore(&chan->lock, flags);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_submit, TI_DAVINCI_CPDMA);

int cpdma_chan_submit_mapped(struct cpdma_chan *chan, void *token,
dma_addr_t data, int len, int directed)
@@ -1175,6 +1194,7 @@ int cpdma_chan_submit_mapped(struct cpdma_chan *chan, void *token,
spin_unlock_irqrestore(&chan->lock, flags);
return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_submit_mapped, TI_DAVINCI_CPDMA);

bool cpdma_check_free_tx_desc(struct cpdma_chan *chan)
{
@@ -1189,6 +1209,7 @@ bool cpdma_check_free_tx_desc(struct cpdma_chan *chan)
spin_unlock_irqrestore(&chan->lock, flags);
return free_tx_desc;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_check_free_tx_desc, TI_DAVINCI_CPDMA);

static void __cpdma_chan_free(struct cpdma_chan *chan,
struct cpdma_desc __iomem *desc,
@@ -1289,6 +1310,7 @@ int cpdma_chan_process(struct cpdma_chan *chan, int quota)
}
return used;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_process, TI_DAVINCI_CPDMA);

int cpdma_chan_start(struct cpdma_chan *chan)
{
@@ -1308,6 +1330,7 @@ int cpdma_chan_start(struct cpdma_chan *chan)

return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_start, TI_DAVINCI_CPDMA);

int cpdma_chan_stop(struct cpdma_chan *chan)
{
@@ -1370,6 +1393,7 @@ int cpdma_chan_stop(struct cpdma_chan *chan)
spin_unlock_irqrestore(&chan->lock, flags);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_stop, TI_DAVINCI_CPDMA);

int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
{
@@ -1387,6 +1411,7 @@ int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)

return 0;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_chan_int_ctrl, TI_DAVINCI_CPDMA);

int cpdma_control_get(struct cpdma_ctlr *ctlr, int control)
{
@@ -1399,6 +1424,7 @@ int cpdma_control_get(struct cpdma_ctlr *ctlr, int control)

return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_control_get, TI_DAVINCI_CPDMA);

int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
{
@@ -1411,16 +1437,19 @@ int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)

return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_control_set, TI_DAVINCI_CPDMA);

int cpdma_get_num_rx_descs(struct cpdma_ctlr *ctlr)
{
return ctlr->num_rx_desc;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_get_num_rx_descs, TI_DAVINCI_CPDMA);

int cpdma_get_num_tx_descs(struct cpdma_ctlr *ctlr)
{
return ctlr->num_tx_desc;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_get_num_tx_descs, TI_DAVINCI_CPDMA);

int cpdma_set_num_rx_descs(struct cpdma_ctlr *ctlr, int num_rx_desc)
{
@@ -1442,3 +1471,7 @@ int cpdma_set_num_rx_descs(struct cpdma_ctlr *ctlr, int num_rx_desc)

return ret;
}
+EXPORT_SYMBOL_NS_GPL(cpdma_set_num_rx_descs, TI_DAVINCI_CPDMA);
+
+MODULE_DESCRIPTION("TI CPDMA driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 2eb9d5a32588..897def12e6ec 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -2103,6 +2103,8 @@ static void __exit davinci_emac_exit(void)
}
module_exit(davinci_emac_exit);

+MODULE_IMPORT_NS(TI_DAVINCI_CPDMA);
+
MODULE_LICENSE("GPL");
MODULE_AUTHOR("DaVinci EMAC Maintainer: Anant Gole <[email protected]>");
MODULE_AUTHOR("DaVinci EMAC Maintainer: Chaithrika U S <[email protected]>");
--
2.38.1



2022-11-19 23:39:56

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 04/18] sound: fix mixed module-builtin object

From: Masahiro Yamada <[email protected]>

With 'y' and 'm' mixed for CONFIG_SNC_SOC_WCD93{35,4X,8X}, wcd-clsh-v2.o
is linked to module(s) and also to vmlinux even though the expected
CFLAGS are different between builtins and modules.

This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
Fixing mixed module-builtin objects").

Turn helpers in wcd-clsh-v2.o into inline functions.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-and-tested-by: Alexander Lobakin <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
sound/soc/codecs/Makefile | 6 +-
sound/soc/codecs/wcd-clsh-v2.c | 903 --------------------------------
sound/soc/codecs/wcd-clsh-v2.h | 917 ++++++++++++++++++++++++++++++++-
3 files changed, 907 insertions(+), 919 deletions(-)
delete mode 100644 sound/soc/codecs/wcd-clsh-v2.c

diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 9170ee1447dd..90c3c610a72f 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -279,9 +279,9 @@ snd-soc-uda1334-objs := uda1334.o
snd-soc-uda134x-objs := uda134x.o
snd-soc-uda1380-objs := uda1380.o
snd-soc-wcd-mbhc-objs := wcd-mbhc-v2.o
-snd-soc-wcd9335-objs := wcd-clsh-v2.o wcd9335.o
-snd-soc-wcd934x-objs := wcd-clsh-v2.o wcd934x.o
-snd-soc-wcd938x-objs := wcd938x.o wcd-clsh-v2.o
+snd-soc-wcd9335-objs := wcd9335.o
+snd-soc-wcd934x-objs := wcd934x.o
+snd-soc-wcd938x-objs := wcd938x.o
snd-soc-wcd938x-sdw-objs := wcd938x-sdw.o
snd-soc-wl1273-objs := wl1273.o
snd-soc-wm-adsp-objs := wm_adsp.o
diff --git a/sound/soc/codecs/wcd-clsh-v2.c b/sound/soc/codecs/wcd-clsh-v2.c
deleted file mode 100644
index 4c7ebc7fb400..000000000000
--- a/sound/soc/codecs/wcd-clsh-v2.c
+++ /dev/null
@@ -1,903 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
-// Copyright (c) 2017-2018, Linaro Limited
-
-#include <linux/slab.h>
-#include <sound/soc.h>
-#include <linux/kernel.h>
-#include <linux/delay.h>
-#include "wcd9335.h"
-#include "wcd-clsh-v2.h"
-
-struct wcd_clsh_ctrl {
- int state;
- int mode;
- int flyback_users;
- int buck_users;
- int clsh_users;
- int codec_version;
- struct snd_soc_component *comp;
-};
-
-/* Class-H registers for codecs from and above WCD9335 */
-#define WCD9XXX_A_CDC_RX0_RX_PATH_CFG0 WCD9335_REG(0xB, 0x42)
-#define WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK BIT(6)
-#define WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE BIT(6)
-#define WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE 0
-#define WCD9XXX_A_CDC_RX1_RX_PATH_CFG0 WCD9335_REG(0xB, 0x56)
-#define WCD9XXX_A_CDC_RX2_RX_PATH_CFG0 WCD9335_REG(0xB, 0x6A)
-#define WCD9XXX_A_CDC_CLSH_K1_MSB WCD9335_REG(0xC, 0x08)
-#define WCD9XXX_A_CDC_CLSH_K1_MSB_COEF_MASK GENMASK(3, 0)
-#define WCD9XXX_A_CDC_CLSH_K1_LSB WCD9335_REG(0xC, 0x09)
-#define WCD9XXX_A_CDC_CLSH_K1_LSB_COEF_MASK GENMASK(7, 0)
-#define WCD9XXX_A_ANA_RX_SUPPLIES WCD9335_REG(0x6, 0x08)
-#define WCD9XXX_A_ANA_RX_REGULATOR_MODE_MASK BIT(1)
-#define WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_H 0
-#define WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_AB BIT(1)
-#define WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_MASK BIT(2)
-#define WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_UHQA BIT(2)
-#define WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_DEFAULT 0
-#define WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_MASK BIT(3)
-#define WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_UHQA BIT(3)
-#define WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_DEFAULT 0
-#define WCD9XXX_A_ANA_RX_VNEG_EN_MASK BIT(6)
-#define WCD9XXX_A_ANA_RX_VNEG_EN_SHIFT 6
-#define WCD9XXX_A_ANA_RX_VNEG_ENABLE BIT(6)
-#define WCD9XXX_A_ANA_RX_VNEG_DISABLE 0
-#define WCD9XXX_A_ANA_RX_VPOS_EN_MASK BIT(7)
-#define WCD9XXX_A_ANA_RX_VPOS_EN_SHIFT 7
-#define WCD9XXX_A_ANA_RX_VPOS_ENABLE BIT(7)
-#define WCD9XXX_A_ANA_RX_VPOS_DISABLE 0
-#define WCD9XXX_A_ANA_HPH WCD9335_REG(0x6, 0x09)
-#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_MASK GENMASK(3, 2)
-#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_UHQA 0x08
-#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_LP 0x04
-#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_NORMAL 0x0
-#define WCD9XXX_A_CDC_CLSH_CRC WCD9335_REG(0xC, 0x01)
-#define WCD9XXX_A_CDC_CLSH_CRC_CLK_EN_MASK BIT(0)
-#define WCD9XXX_A_CDC_CLSH_CRC_CLK_ENABLE BIT(0)
-#define WCD9XXX_A_CDC_CLSH_CRC_CLK_DISABLE 0
-#define WCD9XXX_FLYBACK_EN WCD9335_REG(0x6, 0xA4)
-#define WCD9XXX_FLYBACK_EN_DELAY_SEL_MASK GENMASK(6, 5)
-#define WCD9XXX_FLYBACK_EN_DELAY_26P25_US 0x40
-#define WCD9XXX_FLYBACK_EN_RESET_BY_EXT_MASK BIT(4)
-#define WCD9XXX_FLYBACK_EN_PWDN_WITHOUT_DELAY BIT(4)
-#define WCD9XXX_FLYBACK_EN_PWDN_WITH_DELAY 0
-#define WCD9XXX_RX_BIAS_FLYB_BUFF WCD9335_REG(0x6, 0xC7)
-#define WCD9XXX_RX_BIAS_FLYB_VNEG_5_UA_MASK GENMASK(7, 4)
-#define WCD9XXX_RX_BIAS_FLYB_VPOS_5_UA_MASK GENMASK(3, 0)
-#define WCD9XXX_HPH_L_EN WCD9335_REG(0x6, 0xD3)
-#define WCD9XXX_HPH_CONST_SEL_L_MASK GENMASK(7, 3)
-#define WCD9XXX_HPH_CONST_SEL_BYPASS 0
-#define WCD9XXX_HPH_CONST_SEL_LP_PATH 0x40
-#define WCD9XXX_HPH_CONST_SEL_HQ_PATH 0x80
-#define WCD9XXX_HPH_R_EN WCD9335_REG(0x6, 0xD6)
-#define WCD9XXX_HPH_REFBUFF_UHQA_CTL WCD9335_REG(0x6, 0xDD)
-#define WCD9XXX_HPH_REFBUFF_UHQA_GAIN_MASK GENMASK(2, 0)
-#define WCD9XXX_CLASSH_CTRL_VCL_2 WCD9335_REG(0x6, 0x9B)
-#define WCD9XXX_CLASSH_CTRL_VCL_2_VREF_FILT_1_MASK GENMASK(5, 4)
-#define WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_50KOHM 0x20
-#define WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_0KOHM 0x0
-#define WCD9XXX_CDC_RX1_RX_PATH_CTL WCD9335_REG(0xB, 0x55)
-#define WCD9XXX_CDC_RX2_RX_PATH_CTL WCD9335_REG(0xB, 0x69)
-#define WCD9XXX_CDC_CLK_RST_CTRL_MCLK_CONTROL WCD9335_REG(0xD, 0x41)
-#define WCD9XXX_CDC_CLK_RST_CTRL_MCLK_EN_MASK BIT(0)
-#define WCD9XXX_CDC_CLK_RST_CTRL_MCLK_11P3_EN_MASK BIT(1)
-#define WCD9XXX_CLASSH_CTRL_CCL_1 WCD9335_REG(0x6, 0x9C)
-#define WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_MASK GENMASK(7, 4)
-#define WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA 0x50
-#define WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_30MA 0x30
-
-#define WCD9XXX_BASE_ADDRESS 0x3000
-#define WCD9XXX_ANA_RX_SUPPLIES (WCD9XXX_BASE_ADDRESS+0x008)
-#define WCD9XXX_ANA_HPH (WCD9XXX_BASE_ADDRESS+0x009)
-#define WCD9XXX_CLASSH_MODE_2 (WCD9XXX_BASE_ADDRESS+0x098)
-#define WCD9XXX_CLASSH_MODE_3 (WCD9XXX_BASE_ADDRESS+0x099)
-#define WCD9XXX_FLYBACK_VNEG_CTRL_1 (WCD9XXX_BASE_ADDRESS+0x0A5)
-#define WCD9XXX_FLYBACK_VNEG_CTRL_4 (WCD9XXX_BASE_ADDRESS+0x0A8)
-#define WCD9XXX_FLYBACK_VNEGDAC_CTRL_2 (WCD9XXX_BASE_ADDRESS+0x0AF)
-#define WCD9XXX_RX_BIAS_HPH_LOWPOWER (WCD9XXX_BASE_ADDRESS+0x0BF)
-#define WCD9XXX_V3_RX_BIAS_FLYB_BUFF (WCD9XXX_BASE_ADDRESS+0x0C7)
-#define WCD9XXX_HPH_PA_CTL1 (WCD9XXX_BASE_ADDRESS+0x0D1)
-#define WCD9XXX_HPH_NEW_INT_PA_MISC2 (WCD9XXX_BASE_ADDRESS+0x138)
-
-#define CLSH_REQ_ENABLE true
-#define CLSH_REQ_DISABLE false
-#define WCD_USLEEP_RANGE 50
-
-enum {
- DAC_GAIN_0DB = 0,
- DAC_GAIN_0P2DB,
- DAC_GAIN_0P4DB,
- DAC_GAIN_0P6DB,
- DAC_GAIN_0P8DB,
- DAC_GAIN_M0P2DB,
- DAC_GAIN_M0P4DB,
- DAC_GAIN_M0P6DB,
-};
-
-static inline void wcd_enable_clsh_block(struct wcd_clsh_ctrl *ctrl,
- bool enable)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if ((enable && ++ctrl->clsh_users == 1) ||
- (!enable && --ctrl->clsh_users == 0))
- snd_soc_component_update_bits(comp, WCD9XXX_A_CDC_CLSH_CRC,
- WCD9XXX_A_CDC_CLSH_CRC_CLK_EN_MASK,
- enable);
- if (ctrl->clsh_users < 0)
- ctrl->clsh_users = 0;
-}
-
-static inline bool wcd_clsh_enable_status(struct snd_soc_component *comp)
-{
- return snd_soc_component_read(comp, WCD9XXX_A_CDC_CLSH_CRC) &
- WCD9XXX_A_CDC_CLSH_CRC_CLK_EN_MASK;
-}
-
-static inline void wcd_clsh_set_buck_mode(struct snd_soc_component *comp,
- int mode)
-{
- /* set to HIFI */
- if (mode == CLS_H_HIFI)
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_MASK,
- WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_UHQA);
- else
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_MASK,
- WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_DEFAULT);
-}
-
-static void wcd_clsh_v3_set_buck_mode(struct snd_soc_component *component,
- int mode)
-{
- if (mode == CLS_H_HIFI || mode == CLS_H_LOHIFI ||
- mode == CLS_AB_HIFI || mode == CLS_AB_LOHIFI)
- snd_soc_component_update_bits(component,
- WCD9XXX_ANA_RX_SUPPLIES,
- 0x08, 0x08); /* set to HIFI */
- else
- snd_soc_component_update_bits(component,
- WCD9XXX_ANA_RX_SUPPLIES,
- 0x08, 0x00); /* set to default */
-}
-
-static inline void wcd_clsh_set_flyback_mode(struct snd_soc_component *comp,
- int mode)
-{
- /* set to HIFI */
- if (mode == CLS_H_HIFI)
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_MASK,
- WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_UHQA);
- else
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_MASK,
- WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_DEFAULT);
-}
-
-static void wcd_clsh_buck_ctrl(struct wcd_clsh_ctrl *ctrl,
- int mode,
- bool enable)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- /* enable/disable buck */
- if ((enable && (++ctrl->buck_users == 1)) ||
- (!enable && (--ctrl->buck_users == 0)))
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_VPOS_EN_MASK,
- enable << WCD9XXX_A_ANA_RX_VPOS_EN_SHIFT);
- /*
- * 500us sleep is required after buck enable/disable
- * as per HW requirement
- */
- usleep_range(500, 500 + WCD_USLEEP_RANGE);
-}
-
-static void wcd_clsh_v3_buck_ctrl(struct snd_soc_component *component,
- struct wcd_clsh_ctrl *ctrl,
- int mode,
- bool enable)
-{
- /* enable/disable buck */
- if ((enable && (++ctrl->buck_users == 1)) ||
- (!enable && (--ctrl->buck_users == 0))) {
- snd_soc_component_update_bits(component,
- WCD9XXX_ANA_RX_SUPPLIES,
- (1 << 7), (enable << 7));
- /*
- * 500us sleep is required after buck enable/disable
- * as per HW requirement
- */
- usleep_range(500, 510);
- if (mode == CLS_H_LOHIFI || mode == CLS_H_ULP ||
- mode == CLS_H_HIFI || mode == CLS_H_LP)
- snd_soc_component_update_bits(component,
- WCD9XXX_CLASSH_MODE_3,
- 0x02, 0x00);
-
- snd_soc_component_update_bits(component,
- WCD9XXX_CLASSH_MODE_2,
- 0xFF, 0x3A);
- /* 500usec delay is needed as per HW requirement */
- usleep_range(500, 500 + WCD_USLEEP_RANGE);
- }
-}
-
-static void wcd_clsh_flyback_ctrl(struct wcd_clsh_ctrl *ctrl,
- int mode,
- bool enable)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- /* enable/disable flyback */
- if ((enable && (++ctrl->flyback_users == 1)) ||
- (!enable && (--ctrl->flyback_users == 0))) {
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_VNEG_EN_MASK,
- enable << WCD9XXX_A_ANA_RX_VNEG_EN_SHIFT);
- /* 100usec delay is needed as per HW requirement */
- usleep_range(100, 110);
- }
- /*
- * 500us sleep is required after flyback enable/disable
- * as per HW requirement
- */
- usleep_range(500, 500 + WCD_USLEEP_RANGE);
-}
-
-static void wcd_clsh_set_gain_path(struct wcd_clsh_ctrl *ctrl, int mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
- int val = 0;
-
- switch (mode) {
- case CLS_H_NORMAL:
- case CLS_AB:
- val = WCD9XXX_HPH_CONST_SEL_BYPASS;
- break;
- case CLS_H_HIFI:
- val = WCD9XXX_HPH_CONST_SEL_HQ_PATH;
- break;
- case CLS_H_LP:
- val = WCD9XXX_HPH_CONST_SEL_LP_PATH;
- break;
- }
-
- snd_soc_component_update_bits(comp, WCD9XXX_HPH_L_EN,
- WCD9XXX_HPH_CONST_SEL_L_MASK,
- val);
-
- snd_soc_component_update_bits(comp, WCD9XXX_HPH_R_EN,
- WCD9XXX_HPH_CONST_SEL_L_MASK,
- val);
-}
-
-static void wcd_clsh_v2_set_hph_mode(struct snd_soc_component *comp, int mode)
-{
- int val = 0, gain = 0, res_val;
- int ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
-
- res_val = WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_0KOHM;
- switch (mode) {
- case CLS_H_NORMAL:
- res_val = WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_50KOHM;
- val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_NORMAL;
- gain = DAC_GAIN_0DB;
- ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
- break;
- case CLS_AB:
- val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_NORMAL;
- gain = DAC_GAIN_0DB;
- ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
- break;
- case CLS_H_HIFI:
- val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_UHQA;
- gain = DAC_GAIN_M0P2DB;
- ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
- break;
- case CLS_H_LP:
- val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_LP;
- ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_30MA;
- break;
- }
-
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_HPH,
- WCD9XXX_A_ANA_HPH_PWR_LEVEL_MASK, val);
- snd_soc_component_update_bits(comp, WCD9XXX_CLASSH_CTRL_VCL_2,
- WCD9XXX_CLASSH_CTRL_VCL_2_VREF_FILT_1_MASK,
- res_val);
- if (mode != CLS_H_LP)
- snd_soc_component_update_bits(comp,
- WCD9XXX_HPH_REFBUFF_UHQA_CTL,
- WCD9XXX_HPH_REFBUFF_UHQA_GAIN_MASK,
- gain);
- snd_soc_component_update_bits(comp, WCD9XXX_CLASSH_CTRL_CCL_1,
- WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_MASK,
- ipeak);
-}
-
-static void wcd_clsh_v3_set_hph_mode(struct snd_soc_component *component,
- int mode)
-{
- u8 val;
-
- switch (mode) {
- case CLS_H_NORMAL:
- val = 0x00;
- break;
- case CLS_AB:
- case CLS_H_ULP:
- val = 0x0C;
- break;
- case CLS_AB_HIFI:
- case CLS_H_HIFI:
- val = 0x08;
- break;
- case CLS_H_LP:
- case CLS_H_LOHIFI:
- case CLS_AB_LP:
- case CLS_AB_LOHIFI:
- val = 0x04;
- break;
- default:
- dev_err(component->dev, "%s:Invalid mode %d\n", __func__, mode);
- return;
- }
-
- snd_soc_component_update_bits(component, WCD9XXX_ANA_HPH, 0x0C, val);
-}
-
-void wcd_clsh_set_hph_mode(struct wcd_clsh_ctrl *ctrl, int mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if (ctrl->codec_version >= WCD937X)
- wcd_clsh_v3_set_hph_mode(comp, mode);
- else
- wcd_clsh_v2_set_hph_mode(comp, mode);
-
-}
-
-static void wcd_clsh_set_flyback_current(struct snd_soc_component *comp,
- int mode)
-{
-
- snd_soc_component_update_bits(comp, WCD9XXX_RX_BIAS_FLYB_BUFF,
- WCD9XXX_RX_BIAS_FLYB_VPOS_5_UA_MASK, 0x0A);
- snd_soc_component_update_bits(comp, WCD9XXX_RX_BIAS_FLYB_BUFF,
- WCD9XXX_RX_BIAS_FLYB_VNEG_5_UA_MASK, 0x0A);
- /* Sleep needed to avoid click and pop as per HW requirement */
- usleep_range(100, 110);
-}
-
-static void wcd_clsh_set_buck_regulator_mode(struct snd_soc_component *comp,
- int mode)
-{
- if (mode == CLS_AB)
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_REGULATOR_MODE_MASK,
- WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_AB);
- else
- snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
- WCD9XXX_A_ANA_RX_REGULATOR_MODE_MASK,
- WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_H);
-}
-
-static void wcd_clsh_v3_set_buck_regulator_mode(struct snd_soc_component *component,
- int mode)
-{
- snd_soc_component_update_bits(component, WCD9XXX_ANA_RX_SUPPLIES,
- 0x02, 0x00);
-}
-
-static void wcd_clsh_v3_set_flyback_mode(struct snd_soc_component *component,
- int mode)
-{
- if (mode == CLS_H_HIFI || mode == CLS_H_LOHIFI ||
- mode == CLS_AB_HIFI || mode == CLS_AB_LOHIFI) {
- snd_soc_component_update_bits(component,
- WCD9XXX_ANA_RX_SUPPLIES,
- 0x04, 0x04);
- snd_soc_component_update_bits(component,
- WCD9XXX_FLYBACK_VNEG_CTRL_4,
- 0xF0, 0x80);
- } else {
- snd_soc_component_update_bits(component,
- WCD9XXX_ANA_RX_SUPPLIES,
- 0x04, 0x00); /* set to Default */
- snd_soc_component_update_bits(component,
- WCD9XXX_FLYBACK_VNEG_CTRL_4,
- 0xF0, 0x70);
- }
-}
-
-static void wcd_clsh_v3_force_iq_ctl(struct snd_soc_component *component,
- int mode, bool enable)
-{
- if (enable) {
- snd_soc_component_update_bits(component,
- WCD9XXX_FLYBACK_VNEGDAC_CTRL_2,
- 0xE0, 0xA0);
- /* 100usec delay is needed as per HW requirement */
- usleep_range(100, 110);
- snd_soc_component_update_bits(component,
- WCD9XXX_CLASSH_MODE_3,
- 0x02, 0x02);
- snd_soc_component_update_bits(component,
- WCD9XXX_CLASSH_MODE_2,
- 0xFF, 0x1C);
- if (mode == CLS_H_LOHIFI || mode == CLS_AB_LOHIFI) {
- snd_soc_component_update_bits(component,
- WCD9XXX_HPH_NEW_INT_PA_MISC2,
- 0x20, 0x20);
- snd_soc_component_update_bits(component,
- WCD9XXX_RX_BIAS_HPH_LOWPOWER,
- 0xF0, 0xC0);
- snd_soc_component_update_bits(component,
- WCD9XXX_HPH_PA_CTL1,
- 0x0E, 0x02);
- }
- } else {
- snd_soc_component_update_bits(component,
- WCD9XXX_HPH_NEW_INT_PA_MISC2,
- 0x20, 0x00);
- snd_soc_component_update_bits(component,
- WCD9XXX_RX_BIAS_HPH_LOWPOWER,
- 0xF0, 0x80);
- snd_soc_component_update_bits(component,
- WCD9XXX_HPH_PA_CTL1,
- 0x0E, 0x06);
- }
-}
-
-static void wcd_clsh_v3_flyback_ctrl(struct snd_soc_component *component,
- struct wcd_clsh_ctrl *ctrl,
- int mode,
- bool enable)
-{
- /* enable/disable flyback */
- if ((enable && (++ctrl->flyback_users == 1)) ||
- (!enable && (--ctrl->flyback_users == 0))) {
- snd_soc_component_update_bits(component,
- WCD9XXX_FLYBACK_VNEG_CTRL_1,
- 0xE0, 0xE0);
- snd_soc_component_update_bits(component,
- WCD9XXX_ANA_RX_SUPPLIES,
- (1 << 6), (enable << 6));
- /*
- * 100us sleep is required after flyback enable/disable
- * as per HW requirement
- */
- usleep_range(100, 110);
- snd_soc_component_update_bits(component,
- WCD9XXX_FLYBACK_VNEGDAC_CTRL_2,
- 0xE0, 0xE0);
- /* 500usec delay is needed as per HW requirement */
- usleep_range(500, 500 + WCD_USLEEP_RANGE);
- }
-}
-
-static void wcd_clsh_v3_set_flyback_current(struct snd_soc_component *component,
- int mode)
-{
- snd_soc_component_update_bits(component, WCD9XXX_V3_RX_BIAS_FLYB_BUFF,
- 0x0F, 0x0A);
- snd_soc_component_update_bits(component, WCD9XXX_V3_RX_BIAS_FLYB_BUFF,
- 0xF0, 0xA0);
- /* Sleep needed to avoid click and pop as per HW requirement */
- usleep_range(100, 110);
-}
-
-static void wcd_clsh_v3_state_aux(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *component = ctrl->comp;
-
- if (is_enable) {
- wcd_clsh_v3_set_buck_mode(component, mode);
- wcd_clsh_v3_set_flyback_mode(component, mode);
- wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_flyback_current(component, mode);
- wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
- } else {
- wcd_clsh_v3_buck_ctrl(component, ctrl, mode, false);
- wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, false);
- wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
- wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_state_lo(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if (mode != CLS_AB) {
- dev_err(comp->dev, "%s: LO cannot be in this mode: %d\n",
- __func__, mode);
- return;
- }
-
- if (is_enable) {
- wcd_clsh_set_buck_regulator_mode(comp, mode);
- wcd_clsh_set_buck_mode(comp, mode);
- wcd_clsh_set_flyback_mode(comp, mode);
- wcd_clsh_flyback_ctrl(ctrl, mode, true);
- wcd_clsh_set_flyback_current(comp, mode);
- wcd_clsh_buck_ctrl(ctrl, mode, true);
- } else {
- wcd_clsh_buck_ctrl(ctrl, mode, false);
- wcd_clsh_flyback_ctrl(ctrl, mode, false);
- wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_regulator_mode(comp, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_v3_state_hph_r(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *component = ctrl->comp;
-
- if (mode == CLS_H_NORMAL) {
- dev_dbg(component->dev, "%s: Normal mode not applicable for hph_r\n",
- __func__);
- return;
- }
-
- if (is_enable) {
- wcd_clsh_v3_set_buck_regulator_mode(component, mode);
- wcd_clsh_v3_set_flyback_mode(component, mode);
- wcd_clsh_v3_force_iq_ctl(component, mode, true);
- wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_flyback_current(component, mode);
- wcd_clsh_v3_set_buck_mode(component, mode);
- wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_hph_mode(component, mode);
- } else {
- wcd_clsh_v3_set_hph_mode(component, CLS_H_NORMAL);
-
- /* buck and flyback set to default mode and disable */
- wcd_clsh_v3_flyback_ctrl(component, ctrl, CLS_H_NORMAL, false);
- wcd_clsh_v3_buck_ctrl(component, ctrl, CLS_H_NORMAL, false);
- wcd_clsh_v3_force_iq_ctl(component, CLS_H_NORMAL, false);
- wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
- wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_state_hph_r(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if (mode == CLS_H_NORMAL) {
- dev_err(comp->dev, "%s: Normal mode not applicable for hph_r\n",
- __func__);
- return;
- }
-
- if (is_enable) {
- if (mode != CLS_AB) {
- wcd_enable_clsh_block(ctrl, true);
- /*
- * These K1 values depend on the Headphone Impedance
- * For now it is assumed to be 16 ohm
- */
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_CLSH_K1_MSB,
- WCD9XXX_A_CDC_CLSH_K1_MSB_COEF_MASK,
- 0x00);
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_CLSH_K1_LSB,
- WCD9XXX_A_CDC_CLSH_K1_LSB_COEF_MASK,
- 0xC0);
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_RX2_RX_PATH_CFG0,
- WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
- WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE);
- }
- wcd_clsh_set_buck_regulator_mode(comp, mode);
- wcd_clsh_set_flyback_mode(comp, mode);
- wcd_clsh_flyback_ctrl(ctrl, mode, true);
- wcd_clsh_set_flyback_current(comp, mode);
- wcd_clsh_set_buck_mode(comp, mode);
- wcd_clsh_buck_ctrl(ctrl, mode, true);
- wcd_clsh_v2_set_hph_mode(comp, mode);
- wcd_clsh_set_gain_path(ctrl, mode);
- } else {
- wcd_clsh_v2_set_hph_mode(comp, CLS_H_NORMAL);
-
- if (mode != CLS_AB) {
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_RX2_RX_PATH_CFG0,
- WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
- WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE);
- wcd_enable_clsh_block(ctrl, false);
- }
- /* buck and flyback set to default mode and disable */
- wcd_clsh_buck_ctrl(ctrl, CLS_H_NORMAL, false);
- wcd_clsh_flyback_ctrl(ctrl, CLS_H_NORMAL, false);
- wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_regulator_mode(comp, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_v3_state_hph_l(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *component = ctrl->comp;
-
- if (mode == CLS_H_NORMAL) {
- dev_dbg(component->dev, "%s: Normal mode not applicable for hph_l\n",
- __func__);
- return;
- }
-
- if (is_enable) {
- wcd_clsh_v3_set_buck_regulator_mode(component, mode);
- wcd_clsh_v3_set_flyback_mode(component, mode);
- wcd_clsh_v3_force_iq_ctl(component, mode, true);
- wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_flyback_current(component, mode);
- wcd_clsh_v3_set_buck_mode(component, mode);
- wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_hph_mode(component, mode);
- } else {
- wcd_clsh_v3_set_hph_mode(component, CLS_H_NORMAL);
-
- /* set buck and flyback to Default Mode */
- wcd_clsh_v3_flyback_ctrl(component, ctrl, CLS_H_NORMAL, false);
- wcd_clsh_v3_buck_ctrl(component, ctrl, CLS_H_NORMAL, false);
- wcd_clsh_v3_force_iq_ctl(component, CLS_H_NORMAL, false);
- wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
- wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_state_hph_l(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if (mode == CLS_H_NORMAL) {
- dev_err(comp->dev, "%s: Normal mode not applicable for hph_l\n",
- __func__);
- return;
- }
-
- if (is_enable) {
- if (mode != CLS_AB) {
- wcd_enable_clsh_block(ctrl, true);
- /*
- * These K1 values depend on the Headphone Impedance
- * For now it is assumed to be 16 ohm
- */
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_CLSH_K1_MSB,
- WCD9XXX_A_CDC_CLSH_K1_MSB_COEF_MASK,
- 0x00);
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_CLSH_K1_LSB,
- WCD9XXX_A_CDC_CLSH_K1_LSB_COEF_MASK,
- 0xC0);
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_RX1_RX_PATH_CFG0,
- WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
- WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE);
- }
- wcd_clsh_set_buck_regulator_mode(comp, mode);
- wcd_clsh_set_flyback_mode(comp, mode);
- wcd_clsh_flyback_ctrl(ctrl, mode, true);
- wcd_clsh_set_flyback_current(comp, mode);
- wcd_clsh_set_buck_mode(comp, mode);
- wcd_clsh_buck_ctrl(ctrl, mode, true);
- wcd_clsh_v2_set_hph_mode(comp, mode);
- wcd_clsh_set_gain_path(ctrl, mode);
- } else {
- wcd_clsh_v2_set_hph_mode(comp, CLS_H_NORMAL);
-
- if (mode != CLS_AB) {
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_RX1_RX_PATH_CFG0,
- WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
- WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE);
- wcd_enable_clsh_block(ctrl, false);
- }
- /* set buck and flyback to Default Mode */
- wcd_clsh_buck_ctrl(ctrl, CLS_H_NORMAL, false);
- wcd_clsh_flyback_ctrl(ctrl, CLS_H_NORMAL, false);
- wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_regulator_mode(comp, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_v3_state_ear(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *component = ctrl->comp;
-
- if (is_enable) {
- wcd_clsh_v3_set_buck_regulator_mode(component, mode);
- wcd_clsh_v3_set_flyback_mode(component, mode);
- wcd_clsh_v3_force_iq_ctl(component, mode, true);
- wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_flyback_current(component, mode);
- wcd_clsh_v3_set_buck_mode(component, mode);
- wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
- wcd_clsh_v3_set_hph_mode(component, mode);
- } else {
- wcd_clsh_v3_set_hph_mode(component, CLS_H_NORMAL);
-
- /* set buck and flyback to Default Mode */
- wcd_clsh_v3_flyback_ctrl(component, ctrl, CLS_H_NORMAL, false);
- wcd_clsh_v3_buck_ctrl(component, ctrl, CLS_H_NORMAL, false);
- wcd_clsh_v3_force_iq_ctl(component, CLS_H_NORMAL, false);
- wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
- wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
- }
-}
-
-static void wcd_clsh_state_ear(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if (mode != CLS_H_NORMAL) {
- dev_err(comp->dev, "%s: mode: %d cannot be used for EAR\n",
- __func__, mode);
- return;
- }
-
- if (is_enable) {
- wcd_enable_clsh_block(ctrl, true);
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_RX0_RX_PATH_CFG0,
- WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
- WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE);
- wcd_clsh_set_buck_mode(comp, mode);
- wcd_clsh_set_flyback_mode(comp, mode);
- wcd_clsh_flyback_ctrl(ctrl, mode, true);
- wcd_clsh_set_flyback_current(comp, mode);
- wcd_clsh_buck_ctrl(ctrl, mode, true);
- } else {
- snd_soc_component_update_bits(comp,
- WCD9XXX_A_CDC_RX0_RX_PATH_CFG0,
- WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
- WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE);
- wcd_enable_clsh_block(ctrl, false);
- wcd_clsh_buck_ctrl(ctrl, mode, false);
- wcd_clsh_flyback_ctrl(ctrl, mode, false);
- wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
- wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
- }
-}
-
-static int _wcd_clsh_ctrl_set_state(struct wcd_clsh_ctrl *ctrl, int req_state,
- bool is_enable, int mode)
-{
- switch (req_state) {
- case WCD_CLSH_STATE_EAR:
- if (ctrl->codec_version >= WCD937X)
- wcd_clsh_v3_state_ear(ctrl, req_state, is_enable, mode);
- else
- wcd_clsh_state_ear(ctrl, req_state, is_enable, mode);
- break;
- case WCD_CLSH_STATE_HPHL:
- if (ctrl->codec_version >= WCD937X)
- wcd_clsh_v3_state_hph_l(ctrl, req_state, is_enable, mode);
- else
- wcd_clsh_state_hph_l(ctrl, req_state, is_enable, mode);
- break;
- case WCD_CLSH_STATE_HPHR:
- if (ctrl->codec_version >= WCD937X)
- wcd_clsh_v3_state_hph_r(ctrl, req_state, is_enable, mode);
- else
- wcd_clsh_state_hph_r(ctrl, req_state, is_enable, mode);
- break;
- case WCD_CLSH_STATE_LO:
- if (ctrl->codec_version < WCD937X)
- wcd_clsh_state_lo(ctrl, req_state, is_enable, mode);
- break;
- case WCD_CLSH_STATE_AUX:
- if (ctrl->codec_version >= WCD937X)
- wcd_clsh_v3_state_aux(ctrl, req_state, is_enable, mode);
- break;
- default:
- break;
- }
-
- return 0;
-}
-
-/*
- * Function: wcd_clsh_is_state_valid
- * Params: state
- * Description:
- * Provides information on valid states of Class H configuration
- */
-static bool wcd_clsh_is_state_valid(int state)
-{
- switch (state) {
- case WCD_CLSH_STATE_IDLE:
- case WCD_CLSH_STATE_EAR:
- case WCD_CLSH_STATE_HPHL:
- case WCD_CLSH_STATE_HPHR:
- case WCD_CLSH_STATE_LO:
- case WCD_CLSH_STATE_AUX:
- return true;
- default:
- return false;
- };
-}
-
-/*
- * Function: wcd_clsh_fsm
- * Params: ctrl, req_state, req_type, clsh_event
- * Description:
- * This function handles PRE DAC and POST DAC conditions of different devices
- * and updates class H configuration of different combination of devices
- * based on validity of their states. ctrl will contain current
- * class h state information
- */
-int wcd_clsh_ctrl_set_state(struct wcd_clsh_ctrl *ctrl,
- enum wcd_clsh_event clsh_event,
- int nstate,
- enum wcd_clsh_mode mode)
-{
- struct snd_soc_component *comp = ctrl->comp;
-
- if (nstate == ctrl->state)
- return 0;
-
- if (!wcd_clsh_is_state_valid(nstate)) {
- dev_err(comp->dev, "Class-H not a valid new state:\n");
- return -EINVAL;
- }
-
- switch (clsh_event) {
- case WCD_CLSH_EVENT_PRE_DAC:
- _wcd_clsh_ctrl_set_state(ctrl, nstate, CLSH_REQ_ENABLE, mode);
- break;
- case WCD_CLSH_EVENT_POST_PA:
- _wcd_clsh_ctrl_set_state(ctrl, nstate, CLSH_REQ_DISABLE, mode);
- break;
- }
-
- ctrl->state = nstate;
- ctrl->mode = mode;
-
- return 0;
-}
-
-int wcd_clsh_ctrl_get_state(struct wcd_clsh_ctrl *ctrl)
-{
- return ctrl->state;
-}
-
-struct wcd_clsh_ctrl *wcd_clsh_ctrl_alloc(struct snd_soc_component *comp,
- int version)
-{
- struct wcd_clsh_ctrl *ctrl;
-
- ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
- if (!ctrl)
- return ERR_PTR(-ENOMEM);
-
- ctrl->state = WCD_CLSH_STATE_IDLE;
- ctrl->comp = comp;
- ctrl->codec_version = version;
-
- return ctrl;
-}
-
-void wcd_clsh_ctrl_free(struct wcd_clsh_ctrl *ctrl)
-{
- kfree(ctrl);
-}
diff --git a/sound/soc/codecs/wcd-clsh-v2.h b/sound/soc/codecs/wcd-clsh-v2.h
index 4e3653058275..3074e9b235d4 100644
--- a/sound/soc/codecs/wcd-clsh-v2.h
+++ b/sound/soc/codecs/wcd-clsh-v2.h
@@ -2,8 +2,14 @@

#ifndef _WCD_CLSH_V2_H_
#define _WCD_CLSH_V2_H_
+
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
#include <sound/soc.h>

+#include "wcd9335.h"
+
enum wcd_clsh_event {
WCD_CLSH_EVENT_PRE_DAC = 1,
WCD_CLSH_EVENT_POST_PA,
@@ -48,18 +54,903 @@ enum wcd_codec_version {
WCD937X = 2,
WCD938X = 3,
};
-struct wcd_clsh_ctrl;
-
-extern struct wcd_clsh_ctrl *wcd_clsh_ctrl_alloc(
- struct snd_soc_component *comp,
- int version);
-extern void wcd_clsh_ctrl_free(struct wcd_clsh_ctrl *ctrl);
-extern int wcd_clsh_ctrl_get_state(struct wcd_clsh_ctrl *ctrl);
-extern int wcd_clsh_ctrl_set_state(struct wcd_clsh_ctrl *ctrl,
- enum wcd_clsh_event clsh_event,
- int nstate,
- enum wcd_clsh_mode mode);
-extern void wcd_clsh_set_hph_mode(struct wcd_clsh_ctrl *ctrl,
- int mode);
+
+struct wcd_clsh_ctrl {
+ int state;
+ int mode;
+ int flyback_users;
+ int buck_users;
+ int clsh_users;
+ int codec_version;
+ struct snd_soc_component *comp;
+};
+
+/* Class-H registers for codecs from and above WCD9335 */
+#define WCD9XXX_A_CDC_RX0_RX_PATH_CFG0 WCD9335_REG(0xB, 0x42)
+#define WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK BIT(6)
+#define WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE BIT(6)
+#define WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE 0
+#define WCD9XXX_A_CDC_RX1_RX_PATH_CFG0 WCD9335_REG(0xB, 0x56)
+#define WCD9XXX_A_CDC_RX2_RX_PATH_CFG0 WCD9335_REG(0xB, 0x6A)
+#define WCD9XXX_A_CDC_CLSH_K1_MSB WCD9335_REG(0xC, 0x08)
+#define WCD9XXX_A_CDC_CLSH_K1_MSB_COEF_MASK GENMASK(3, 0)
+#define WCD9XXX_A_CDC_CLSH_K1_LSB WCD9335_REG(0xC, 0x09)
+#define WCD9XXX_A_CDC_CLSH_K1_LSB_COEF_MASK GENMASK(7, 0)
+#define WCD9XXX_A_ANA_RX_SUPPLIES WCD9335_REG(0x6, 0x08)
+#define WCD9XXX_A_ANA_RX_REGULATOR_MODE_MASK BIT(1)
+#define WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_H 0
+#define WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_AB BIT(1)
+#define WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_MASK BIT(2)
+#define WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_UHQA BIT(2)
+#define WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_DEFAULT 0
+#define WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_MASK BIT(3)
+#define WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_UHQA BIT(3)
+#define WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_DEFAULT 0
+#define WCD9XXX_A_ANA_RX_VNEG_EN_MASK BIT(6)
+#define WCD9XXX_A_ANA_RX_VNEG_EN_SHIFT 6
+#define WCD9XXX_A_ANA_RX_VNEG_ENABLE BIT(6)
+#define WCD9XXX_A_ANA_RX_VNEG_DISABLE 0
+#define WCD9XXX_A_ANA_RX_VPOS_EN_MASK BIT(7)
+#define WCD9XXX_A_ANA_RX_VPOS_EN_SHIFT 7
+#define WCD9XXX_A_ANA_RX_VPOS_ENABLE BIT(7)
+#define WCD9XXX_A_ANA_RX_VPOS_DISABLE 0
+#define WCD9XXX_A_ANA_HPH WCD9335_REG(0x6, 0x09)
+#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_MASK GENMASK(3, 2)
+#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_UHQA 0x08
+#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_LP 0x04
+#define WCD9XXX_A_ANA_HPH_PWR_LEVEL_NORMAL 0x0
+#define WCD9XXX_A_CDC_CLSH_CRC WCD9335_REG(0xC, 0x01)
+#define WCD9XXX_A_CDC_CLSH_CRC_CLK_EN_MASK BIT(0)
+#define WCD9XXX_A_CDC_CLSH_CRC_CLK_ENABLE BIT(0)
+#define WCD9XXX_A_CDC_CLSH_CRC_CLK_DISABLE 0
+#define WCD9XXX_FLYBACK_EN WCD9335_REG(0x6, 0xA4)
+#define WCD9XXX_FLYBACK_EN_DELAY_SEL_MASK GENMASK(6, 5)
+#define WCD9XXX_FLYBACK_EN_DELAY_26P25_US 0x40
+#define WCD9XXX_FLYBACK_EN_RESET_BY_EXT_MASK BIT(4)
+#define WCD9XXX_FLYBACK_EN_PWDN_WITHOUT_DELAY BIT(4)
+#define WCD9XXX_FLYBACK_EN_PWDN_WITH_DELAY 0
+#define WCD9XXX_RX_BIAS_FLYB_BUFF WCD9335_REG(0x6, 0xC7)
+#define WCD9XXX_RX_BIAS_FLYB_VNEG_5_UA_MASK GENMASK(7, 4)
+#define WCD9XXX_RX_BIAS_FLYB_VPOS_5_UA_MASK GENMASK(3, 0)
+#define WCD9XXX_HPH_L_EN WCD9335_REG(0x6, 0xD3)
+#define WCD9XXX_HPH_CONST_SEL_L_MASK GENMASK(7, 3)
+#define WCD9XXX_HPH_CONST_SEL_BYPASS 0
+#define WCD9XXX_HPH_CONST_SEL_LP_PATH 0x40
+#define WCD9XXX_HPH_CONST_SEL_HQ_PATH 0x80
+#define WCD9XXX_HPH_R_EN WCD9335_REG(0x6, 0xD6)
+#define WCD9XXX_HPH_REFBUFF_UHQA_CTL WCD9335_REG(0x6, 0xDD)
+#define WCD9XXX_HPH_REFBUFF_UHQA_GAIN_MASK GENMASK(2, 0)
+#define WCD9XXX_CLASSH_CTRL_VCL_2 WCD9335_REG(0x6, 0x9B)
+#define WCD9XXX_CLASSH_CTRL_VCL_2_VREF_FILT_1_MASK GENMASK(5, 4)
+#define WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_50KOHM 0x20
+#define WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_0KOHM 0x0
+#define WCD9XXX_CDC_RX1_RX_PATH_CTL WCD9335_REG(0xB, 0x55)
+#define WCD9XXX_CDC_RX2_RX_PATH_CTL WCD9335_REG(0xB, 0x69)
+#define WCD9XXX_CDC_CLK_RST_CTRL_MCLK_CONTROL WCD9335_REG(0xD, 0x41)
+#define WCD9XXX_CDC_CLK_RST_CTRL_MCLK_EN_MASK BIT(0)
+#define WCD9XXX_CDC_CLK_RST_CTRL_MCLK_11P3_EN_MASK BIT(1)
+#define WCD9XXX_CLASSH_CTRL_CCL_1 WCD9335_REG(0x6, 0x9C)
+#define WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_MASK GENMASK(7, 4)
+#define WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA 0x50
+#define WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_30MA 0x30
+
+#define WCD9XXX_BASE_ADDRESS 0x3000
+#define WCD9XXX_ANA_RX_SUPPLIES (WCD9XXX_BASE_ADDRESS+0x008)
+#define WCD9XXX_ANA_HPH (WCD9XXX_BASE_ADDRESS+0x009)
+#define WCD9XXX_CLASSH_MODE_2 (WCD9XXX_BASE_ADDRESS+0x098)
+#define WCD9XXX_CLASSH_MODE_3 (WCD9XXX_BASE_ADDRESS+0x099)
+#define WCD9XXX_FLYBACK_VNEG_CTRL_1 (WCD9XXX_BASE_ADDRESS+0x0A5)
+#define WCD9XXX_FLYBACK_VNEG_CTRL_4 (WCD9XXX_BASE_ADDRESS+0x0A8)
+#define WCD9XXX_FLYBACK_VNEGDAC_CTRL_2 (WCD9XXX_BASE_ADDRESS+0x0AF)
+#define WCD9XXX_RX_BIAS_HPH_LOWPOWER (WCD9XXX_BASE_ADDRESS+0x0BF)
+#define WCD9XXX_V3_RX_BIAS_FLYB_BUFF (WCD9XXX_BASE_ADDRESS+0x0C7)
+#define WCD9XXX_HPH_PA_CTL1 (WCD9XXX_BASE_ADDRESS+0x0D1)
+#define WCD9XXX_HPH_NEW_INT_PA_MISC2 (WCD9XXX_BASE_ADDRESS+0x138)
+
+#define CLSH_REQ_ENABLE true
+#define CLSH_REQ_DISABLE false
+#define WCD_USLEEP_RANGE 50
+
+enum {
+ DAC_GAIN_0DB = 0,
+ DAC_GAIN_0P2DB,
+ DAC_GAIN_0P4DB,
+ DAC_GAIN_0P6DB,
+ DAC_GAIN_0P8DB,
+ DAC_GAIN_M0P2DB,
+ DAC_GAIN_M0P4DB,
+ DAC_GAIN_M0P6DB,
+};
+
+static inline void wcd_enable_clsh_block(struct wcd_clsh_ctrl *ctrl,
+ bool enable)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if ((enable && ++ctrl->clsh_users == 1) ||
+ (!enable && --ctrl->clsh_users == 0))
+ snd_soc_component_update_bits(comp, WCD9XXX_A_CDC_CLSH_CRC,
+ WCD9XXX_A_CDC_CLSH_CRC_CLK_EN_MASK,
+ enable);
+ if (ctrl->clsh_users < 0)
+ ctrl->clsh_users = 0;
+}
+
+static inline bool wcd_clsh_enable_status(struct snd_soc_component *comp)
+{
+ return snd_soc_component_read(comp, WCD9XXX_A_CDC_CLSH_CRC) &
+ WCD9XXX_A_CDC_CLSH_CRC_CLK_EN_MASK;
+}
+
+static inline void wcd_clsh_set_buck_mode(struct snd_soc_component *comp,
+ int mode)
+{
+ /* set to HIFI */
+ if (mode == CLS_H_HIFI)
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_MASK,
+ WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_UHQA);
+ else
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_MASK,
+ WCD9XXX_A_ANA_RX_VPOS_PWR_LVL_DEFAULT);
+}
+
+static inline void wcd_clsh_v3_set_buck_mode(struct snd_soc_component *component,
+ int mode)
+{
+ if (mode == CLS_H_HIFI || mode == CLS_H_LOHIFI ||
+ mode == CLS_AB_HIFI || mode == CLS_AB_LOHIFI)
+ snd_soc_component_update_bits(component,
+ WCD9XXX_ANA_RX_SUPPLIES,
+ 0x08, 0x08); /* set to HIFI */
+ else
+ snd_soc_component_update_bits(component,
+ WCD9XXX_ANA_RX_SUPPLIES,
+ 0x08, 0x00); /* set to default */
+}
+
+static inline void wcd_clsh_set_flyback_mode(struct snd_soc_component *comp,
+ int mode)
+{
+ /* set to HIFI */
+ if (mode == CLS_H_HIFI)
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_MASK,
+ WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_UHQA);
+ else
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_MASK,
+ WCD9XXX_A_ANA_RX_VNEG_PWR_LVL_DEFAULT);
+}
+
+static inline void wcd_clsh_buck_ctrl(struct wcd_clsh_ctrl *ctrl,
+ int mode,
+ bool enable)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ /* enable/disable buck */
+ if ((enable && (++ctrl->buck_users == 1)) ||
+ (!enable && (--ctrl->buck_users == 0)))
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_VPOS_EN_MASK,
+ enable << WCD9XXX_A_ANA_RX_VPOS_EN_SHIFT);
+ /*
+ * 500us sleep is required after buck enable/disable
+ * as per HW requirement
+ */
+ usleep_range(500, 500 + WCD_USLEEP_RANGE);
+}
+
+static inline void wcd_clsh_v3_buck_ctrl(struct snd_soc_component *component,
+ struct wcd_clsh_ctrl *ctrl,
+ int mode,
+ bool enable)
+{
+ /* enable/disable buck */
+ if ((enable && (++ctrl->buck_users == 1)) ||
+ (!enable && (--ctrl->buck_users == 0))) {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_ANA_RX_SUPPLIES,
+ (1 << 7), (enable << 7));
+ /*
+ * 500us sleep is required after buck enable/disable
+ * as per HW requirement
+ */
+ usleep_range(500, 510);
+ if (mode == CLS_H_LOHIFI || mode == CLS_H_ULP ||
+ mode == CLS_H_HIFI || mode == CLS_H_LP)
+ snd_soc_component_update_bits(component,
+ WCD9XXX_CLASSH_MODE_3,
+ 0x02, 0x00);
+
+ snd_soc_component_update_bits(component,
+ WCD9XXX_CLASSH_MODE_2,
+ 0xFF, 0x3A);
+ /* 500usec delay is needed as per HW requirement */
+ usleep_range(500, 500 + WCD_USLEEP_RANGE);
+ }
+}
+
+static inline void wcd_clsh_flyback_ctrl(struct wcd_clsh_ctrl *ctrl,
+ int mode,
+ bool enable)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ /* enable/disable flyback */
+ if ((enable && (++ctrl->flyback_users == 1)) ||
+ (!enable && (--ctrl->flyback_users == 0))) {
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_VNEG_EN_MASK,
+ enable << WCD9XXX_A_ANA_RX_VNEG_EN_SHIFT);
+ /* 100usec delay is needed as per HW requirement */
+ usleep_range(100, 110);
+ }
+ /*
+ * 500us sleep is required after flyback enable/disable
+ * as per HW requirement
+ */
+ usleep_range(500, 500 + WCD_USLEEP_RANGE);
+}
+
+static inline void wcd_clsh_set_gain_path(struct wcd_clsh_ctrl *ctrl, int mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+ int val = 0;
+
+ switch (mode) {
+ case CLS_H_NORMAL:
+ case CLS_AB:
+ val = WCD9XXX_HPH_CONST_SEL_BYPASS;
+ break;
+ case CLS_H_HIFI:
+ val = WCD9XXX_HPH_CONST_SEL_HQ_PATH;
+ break;
+ case CLS_H_LP:
+ val = WCD9XXX_HPH_CONST_SEL_LP_PATH;
+ break;
+ }
+
+ snd_soc_component_update_bits(comp, WCD9XXX_HPH_L_EN,
+ WCD9XXX_HPH_CONST_SEL_L_MASK,
+ val);
+
+ snd_soc_component_update_bits(comp, WCD9XXX_HPH_R_EN,
+ WCD9XXX_HPH_CONST_SEL_L_MASK,
+ val);
+}
+
+static inline void wcd_clsh_v2_set_hph_mode(struct snd_soc_component *comp,
+ int mode)
+{
+ int val = 0, gain = 0, res_val;
+ int ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
+
+ res_val = WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_0KOHM;
+ switch (mode) {
+ case CLS_H_NORMAL:
+ res_val = WCD9XXX_CLASSH_CTRL_VCL_VREF_FILT_R_50KOHM;
+ val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_NORMAL;
+ gain = DAC_GAIN_0DB;
+ ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
+ break;
+ case CLS_AB:
+ val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_NORMAL;
+ gain = DAC_GAIN_0DB;
+ ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
+ break;
+ case CLS_H_HIFI:
+ val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_UHQA;
+ gain = DAC_GAIN_M0P2DB;
+ ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_50MA;
+ break;
+ case CLS_H_LP:
+ val = WCD9XXX_A_ANA_HPH_PWR_LEVEL_LP;
+ ipeak = WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_30MA;
+ break;
+ }
+
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_HPH,
+ WCD9XXX_A_ANA_HPH_PWR_LEVEL_MASK, val);
+ snd_soc_component_update_bits(comp, WCD9XXX_CLASSH_CTRL_VCL_2,
+ WCD9XXX_CLASSH_CTRL_VCL_2_VREF_FILT_1_MASK,
+ res_val);
+ if (mode != CLS_H_LP)
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_HPH_REFBUFF_UHQA_CTL,
+ WCD9XXX_HPH_REFBUFF_UHQA_GAIN_MASK,
+ gain);
+ snd_soc_component_update_bits(comp, WCD9XXX_CLASSH_CTRL_CCL_1,
+ WCD9XXX_CLASSH_CTRL_CCL_1_DELTA_IPEAK_MASK,
+ ipeak);
+}
+
+static inline void wcd_clsh_v3_set_hph_mode(struct snd_soc_component *component,
+ int mode)
+{
+ u8 val;
+
+ switch (mode) {
+ case CLS_H_NORMAL:
+ val = 0x00;
+ break;
+ case CLS_AB:
+ case CLS_H_ULP:
+ val = 0x0C;
+ break;
+ case CLS_AB_HIFI:
+ case CLS_H_HIFI:
+ val = 0x08;
+ break;
+ case CLS_H_LP:
+ case CLS_H_LOHIFI:
+ case CLS_AB_LP:
+ case CLS_AB_LOHIFI:
+ val = 0x04;
+ break;
+ default:
+ dev_err(component->dev, "%s:Invalid mode %d\n", __func__, mode);
+ return;
+ }
+
+ snd_soc_component_update_bits(component, WCD9XXX_ANA_HPH, 0x0C, val);
+}
+
+static inline void wcd_clsh_set_hph_mode(struct wcd_clsh_ctrl *ctrl, int mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if (ctrl->codec_version >= WCD937X)
+ wcd_clsh_v3_set_hph_mode(comp, mode);
+ else
+ wcd_clsh_v2_set_hph_mode(comp, mode);
+
+}
+
+static inline void wcd_clsh_set_flyback_current(struct snd_soc_component *comp,
+ int mode)
+{
+
+ snd_soc_component_update_bits(comp, WCD9XXX_RX_BIAS_FLYB_BUFF,
+ WCD9XXX_RX_BIAS_FLYB_VPOS_5_UA_MASK, 0x0A);
+ snd_soc_component_update_bits(comp, WCD9XXX_RX_BIAS_FLYB_BUFF,
+ WCD9XXX_RX_BIAS_FLYB_VNEG_5_UA_MASK, 0x0A);
+ /* Sleep needed to avoid click and pop as per HW requirement */
+ usleep_range(100, 110);
+}
+
+static inline void wcd_clsh_set_buck_regulator_mode(struct snd_soc_component *comp,
+ int mode)
+{
+ if (mode == CLS_AB)
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_REGULATOR_MODE_MASK,
+ WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_AB);
+ else
+ snd_soc_component_update_bits(comp, WCD9XXX_A_ANA_RX_SUPPLIES,
+ WCD9XXX_A_ANA_RX_REGULATOR_MODE_MASK,
+ WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_H);
+}
+
+static inline void wcd_clsh_v3_set_buck_regulator_mode(struct snd_soc_component *component,
+ int mode)
+{
+ snd_soc_component_update_bits(component, WCD9XXX_ANA_RX_SUPPLIES,
+ 0x02, 0x00);
+}
+
+static inline void wcd_clsh_v3_set_flyback_mode(struct snd_soc_component *component,
+ int mode)
+{
+ if (mode == CLS_H_HIFI || mode == CLS_H_LOHIFI ||
+ mode == CLS_AB_HIFI || mode == CLS_AB_LOHIFI) {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_ANA_RX_SUPPLIES,
+ 0x04, 0x04);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_FLYBACK_VNEG_CTRL_4,
+ 0xF0, 0x80);
+ } else {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_ANA_RX_SUPPLIES,
+ 0x04, 0x00); /* set to Default */
+ snd_soc_component_update_bits(component,
+ WCD9XXX_FLYBACK_VNEG_CTRL_4,
+ 0xF0, 0x70);
+ }
+}
+
+static inline void wcd_clsh_v3_force_iq_ctl(struct snd_soc_component *component,
+ int mode, bool enable)
+{
+ if (enable) {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_FLYBACK_VNEGDAC_CTRL_2,
+ 0xE0, 0xA0);
+ /* 100usec delay is needed as per HW requirement */
+ usleep_range(100, 110);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_CLASSH_MODE_3,
+ 0x02, 0x02);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_CLASSH_MODE_2,
+ 0xFF, 0x1C);
+ if (mode == CLS_H_LOHIFI || mode == CLS_AB_LOHIFI) {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_HPH_NEW_INT_PA_MISC2,
+ 0x20, 0x20);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_RX_BIAS_HPH_LOWPOWER,
+ 0xF0, 0xC0);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_HPH_PA_CTL1,
+ 0x0E, 0x02);
+ }
+ } else {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_HPH_NEW_INT_PA_MISC2,
+ 0x20, 0x00);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_RX_BIAS_HPH_LOWPOWER,
+ 0xF0, 0x80);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_HPH_PA_CTL1,
+ 0x0E, 0x06);
+ }
+}
+
+static inline void wcd_clsh_v3_flyback_ctrl(struct snd_soc_component *component,
+ struct wcd_clsh_ctrl *ctrl,
+ int mode,
+ bool enable)
+{
+ /* enable/disable flyback */
+ if ((enable && (++ctrl->flyback_users == 1)) ||
+ (!enable && (--ctrl->flyback_users == 0))) {
+ snd_soc_component_update_bits(component,
+ WCD9XXX_FLYBACK_VNEG_CTRL_1,
+ 0xE0, 0xE0);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_ANA_RX_SUPPLIES,
+ (1 << 6), (enable << 6));
+ /*
+ * 100us sleep is required after flyback enable/disable
+ * as per HW requirement
+ */
+ usleep_range(100, 110);
+ snd_soc_component_update_bits(component,
+ WCD9XXX_FLYBACK_VNEGDAC_CTRL_2,
+ 0xE0, 0xE0);
+ /* 500usec delay is needed as per HW requirement */
+ usleep_range(500, 500 + WCD_USLEEP_RANGE);
+ }
+}
+
+static inline void wcd_clsh_v3_set_flyback_current(struct snd_soc_component *component,
+ int mode)
+{
+ snd_soc_component_update_bits(component, WCD9XXX_V3_RX_BIAS_FLYB_BUFF,
+ 0x0F, 0x0A);
+ snd_soc_component_update_bits(component, WCD9XXX_V3_RX_BIAS_FLYB_BUFF,
+ 0xF0, 0xA0);
+ /* Sleep needed to avoid click and pop as per HW requirement */
+ usleep_range(100, 110);
+}
+
+static inline void wcd_clsh_v3_state_aux(struct wcd_clsh_ctrl *ctrl, int req_state,
+ bool is_enable, int mode)
+{
+ struct snd_soc_component *component = ctrl->comp;
+
+ if (is_enable) {
+ wcd_clsh_v3_set_buck_mode(component, mode);
+ wcd_clsh_v3_set_flyback_mode(component, mode);
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_flyback_current(component, mode);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
+ } else {
+ wcd_clsh_v3_buck_ctrl(component, ctrl, mode, false);
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, false);
+ wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
+ wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_state_lo(struct wcd_clsh_ctrl *ctrl, int req_state,
+ bool is_enable, int mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if (mode != CLS_AB) {
+ dev_err(comp->dev, "%s: LO cannot be in this mode: %d\n",
+ __func__, mode);
+ return;
+ }
+
+ if (is_enable) {
+ wcd_clsh_set_buck_regulator_mode(comp, mode);
+ wcd_clsh_set_buck_mode(comp, mode);
+ wcd_clsh_set_flyback_mode(comp, mode);
+ wcd_clsh_flyback_ctrl(ctrl, mode, true);
+ wcd_clsh_set_flyback_current(comp, mode);
+ wcd_clsh_buck_ctrl(ctrl, mode, true);
+ } else {
+ wcd_clsh_buck_ctrl(ctrl, mode, false);
+ wcd_clsh_flyback_ctrl(ctrl, mode, false);
+ wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_regulator_mode(comp, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_v3_state_hph_r(struct wcd_clsh_ctrl *ctrl,
+ int req_state,
+ bool is_enable, int mode)
+{
+ struct snd_soc_component *component = ctrl->comp;
+
+ if (mode == CLS_H_NORMAL) {
+ dev_dbg(component->dev, "%s: Normal mode not applicable for hph_r\n",
+ __func__);
+ return;
+ }
+
+ if (is_enable) {
+ wcd_clsh_v3_set_buck_regulator_mode(component, mode);
+ wcd_clsh_v3_set_flyback_mode(component, mode);
+ wcd_clsh_v3_force_iq_ctl(component, mode, true);
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_flyback_current(component, mode);
+ wcd_clsh_v3_set_buck_mode(component, mode);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_hph_mode(component, mode);
+ } else {
+ wcd_clsh_v3_set_hph_mode(component, CLS_H_NORMAL);
+
+ /* buck and flyback set to default mode and disable */
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_v3_force_iq_ctl(component, CLS_H_NORMAL, false);
+ wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
+ wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_state_hph_r(struct wcd_clsh_ctrl *ctrl,
+ int req_state, bool is_enable, int mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if (mode == CLS_H_NORMAL) {
+ dev_err(comp->dev, "%s: Normal mode not applicable for hph_r\n",
+ __func__);
+ return;
+ }
+
+ if (is_enable) {
+ if (mode != CLS_AB) {
+ wcd_enable_clsh_block(ctrl, true);
+ /*
+ * These K1 values depend on the Headphone Impedance
+ * For now it is assumed to be 16 ohm
+ */
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_CLSH_K1_MSB,
+ WCD9XXX_A_CDC_CLSH_K1_MSB_COEF_MASK,
+ 0x00);
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_CLSH_K1_LSB,
+ WCD9XXX_A_CDC_CLSH_K1_LSB_COEF_MASK,
+ 0xC0);
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_RX2_RX_PATH_CFG0,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE);
+ }
+ wcd_clsh_set_buck_regulator_mode(comp, mode);
+ wcd_clsh_set_flyback_mode(comp, mode);
+ wcd_clsh_flyback_ctrl(ctrl, mode, true);
+ wcd_clsh_set_flyback_current(comp, mode);
+ wcd_clsh_set_buck_mode(comp, mode);
+ wcd_clsh_buck_ctrl(ctrl, mode, true);
+ wcd_clsh_v2_set_hph_mode(comp, mode);
+ wcd_clsh_set_gain_path(ctrl, mode);
+ } else {
+ wcd_clsh_v2_set_hph_mode(comp, CLS_H_NORMAL);
+
+ if (mode != CLS_AB) {
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_RX2_RX_PATH_CFG0,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE);
+ wcd_enable_clsh_block(ctrl, false);
+ }
+ /* buck and flyback set to default mode and disable */
+ wcd_clsh_buck_ctrl(ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_flyback_ctrl(ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_regulator_mode(comp, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_v3_state_hph_l(struct wcd_clsh_ctrl *ctrl,
+ int req_state, bool is_enable,
+ int mode)
+{
+ struct snd_soc_component *component = ctrl->comp;
+
+ if (mode == CLS_H_NORMAL) {
+ dev_dbg(component->dev, "%s: Normal mode not applicable for hph_l\n",
+ __func__);
+ return;
+ }
+
+ if (is_enable) {
+ wcd_clsh_v3_set_buck_regulator_mode(component, mode);
+ wcd_clsh_v3_set_flyback_mode(component, mode);
+ wcd_clsh_v3_force_iq_ctl(component, mode, true);
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_flyback_current(component, mode);
+ wcd_clsh_v3_set_buck_mode(component, mode);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_hph_mode(component, mode);
+ } else {
+ wcd_clsh_v3_set_hph_mode(component, CLS_H_NORMAL);
+
+ /* set buck and flyback to Default Mode */
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_v3_force_iq_ctl(component, CLS_H_NORMAL, false);
+ wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
+ wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_state_hph_l(struct wcd_clsh_ctrl *ctrl,
+ int req_state, bool is_enable, int mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if (mode == CLS_H_NORMAL) {
+ dev_err(comp->dev, "%s: Normal mode not applicable for hph_l\n",
+ __func__);
+ return;
+ }
+
+ if (is_enable) {
+ if (mode != CLS_AB) {
+ wcd_enable_clsh_block(ctrl, true);
+ /*
+ * These K1 values depend on the Headphone Impedance
+ * For now it is assumed to be 16 ohm
+ */
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_CLSH_K1_MSB,
+ WCD9XXX_A_CDC_CLSH_K1_MSB_COEF_MASK,
+ 0x00);
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_CLSH_K1_LSB,
+ WCD9XXX_A_CDC_CLSH_K1_LSB_COEF_MASK,
+ 0xC0);
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_RX1_RX_PATH_CFG0,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE);
+ }
+ wcd_clsh_set_buck_regulator_mode(comp, mode);
+ wcd_clsh_set_flyback_mode(comp, mode);
+ wcd_clsh_flyback_ctrl(ctrl, mode, true);
+ wcd_clsh_set_flyback_current(comp, mode);
+ wcd_clsh_set_buck_mode(comp, mode);
+ wcd_clsh_buck_ctrl(ctrl, mode, true);
+ wcd_clsh_v2_set_hph_mode(comp, mode);
+ wcd_clsh_set_gain_path(ctrl, mode);
+ } else {
+ wcd_clsh_v2_set_hph_mode(comp, CLS_H_NORMAL);
+
+ if (mode != CLS_AB) {
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_RX1_RX_PATH_CFG0,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE);
+ wcd_enable_clsh_block(ctrl, false);
+ }
+ /* set buck and flyback to Default Mode */
+ wcd_clsh_buck_ctrl(ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_flyback_ctrl(ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_regulator_mode(comp, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_v3_state_ear(struct wcd_clsh_ctrl *ctrl,
+ int req_state, bool is_enable,
+ int mode)
+{
+ struct snd_soc_component *component = ctrl->comp;
+
+ if (is_enable) {
+ wcd_clsh_v3_set_buck_regulator_mode(component, mode);
+ wcd_clsh_v3_set_flyback_mode(component, mode);
+ wcd_clsh_v3_force_iq_ctl(component, mode, true);
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_flyback_current(component, mode);
+ wcd_clsh_v3_set_buck_mode(component, mode);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, mode, true);
+ wcd_clsh_v3_set_hph_mode(component, mode);
+ } else {
+ wcd_clsh_v3_set_hph_mode(component, CLS_H_NORMAL);
+
+ /* set buck and flyback to Default Mode */
+ wcd_clsh_v3_flyback_ctrl(component, ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_v3_buck_ctrl(component, ctrl, CLS_H_NORMAL, false);
+ wcd_clsh_v3_force_iq_ctl(component, CLS_H_NORMAL, false);
+ wcd_clsh_v3_set_flyback_mode(component, CLS_H_NORMAL);
+ wcd_clsh_v3_set_buck_mode(component, CLS_H_NORMAL);
+ }
+}
+
+static inline void wcd_clsh_state_ear(struct wcd_clsh_ctrl *ctrl, int req_state,
+ bool is_enable, int mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if (mode != CLS_H_NORMAL) {
+ dev_err(comp->dev, "%s: mode: %d cannot be used for EAR\n",
+ __func__, mode);
+ return;
+ }
+
+ if (is_enable) {
+ wcd_enable_clsh_block(ctrl, true);
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_RX0_RX_PATH_CFG0,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_ENABLE);
+ wcd_clsh_set_buck_mode(comp, mode);
+ wcd_clsh_set_flyback_mode(comp, mode);
+ wcd_clsh_flyback_ctrl(ctrl, mode, true);
+ wcd_clsh_set_flyback_current(comp, mode);
+ wcd_clsh_buck_ctrl(ctrl, mode, true);
+ } else {
+ snd_soc_component_update_bits(comp,
+ WCD9XXX_A_CDC_RX0_RX_PATH_CFG0,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_EN_MASK,
+ WCD9XXX_A_CDC_RX_PATH_CLSH_DISABLE);
+ wcd_enable_clsh_block(ctrl, false);
+ wcd_clsh_buck_ctrl(ctrl, mode, false);
+ wcd_clsh_flyback_ctrl(ctrl, mode, false);
+ wcd_clsh_set_flyback_mode(comp, CLS_H_NORMAL);
+ wcd_clsh_set_buck_mode(comp, CLS_H_NORMAL);
+ }
+}
+
+static inline int _wcd_clsh_ctrl_set_state(struct wcd_clsh_ctrl *ctrl,
+ int req_state, bool is_enable,
+ int mode)
+{
+ switch (req_state) {
+ case WCD_CLSH_STATE_EAR:
+ if (ctrl->codec_version >= WCD937X)
+ wcd_clsh_v3_state_ear(ctrl, req_state, is_enable, mode);
+ else
+ wcd_clsh_state_ear(ctrl, req_state, is_enable, mode);
+ break;
+ case WCD_CLSH_STATE_HPHL:
+ if (ctrl->codec_version >= WCD937X)
+ wcd_clsh_v3_state_hph_l(ctrl, req_state, is_enable, mode);
+ else
+ wcd_clsh_state_hph_l(ctrl, req_state, is_enable, mode);
+ break;
+ case WCD_CLSH_STATE_HPHR:
+ if (ctrl->codec_version >= WCD937X)
+ wcd_clsh_v3_state_hph_r(ctrl, req_state, is_enable, mode);
+ else
+ wcd_clsh_state_hph_r(ctrl, req_state, is_enable, mode);
+ break;
+ case WCD_CLSH_STATE_LO:
+ if (ctrl->codec_version < WCD937X)
+ wcd_clsh_state_lo(ctrl, req_state, is_enable, mode);
+ break;
+ case WCD_CLSH_STATE_AUX:
+ if (ctrl->codec_version >= WCD937X)
+ wcd_clsh_v3_state_aux(ctrl, req_state, is_enable, mode);
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+/*
+ * Function: wcd_clsh_is_state_valid
+ * Params: state
+ * Description:
+ * Provides information on valid states of Class H configuration
+ */
+static inline bool wcd_clsh_is_state_valid(int state)
+{
+ switch (state) {
+ case WCD_CLSH_STATE_IDLE:
+ case WCD_CLSH_STATE_EAR:
+ case WCD_CLSH_STATE_HPHL:
+ case WCD_CLSH_STATE_HPHR:
+ case WCD_CLSH_STATE_LO:
+ case WCD_CLSH_STATE_AUX:
+ return true;
+ default:
+ return false;
+ };
+}
+
+/*
+ * Function: wcd_clsh_fsm
+ * Params: ctrl, req_state, req_type, clsh_event
+ * Description:
+ * This function handles PRE DAC and POST DAC conditions of different devices
+ * and updates class H configuration of different combination of devices
+ * based on validity of their states. ctrl will contain current
+ * class h state information
+ */
+static inline int wcd_clsh_ctrl_set_state(struct wcd_clsh_ctrl *ctrl,
+ enum wcd_clsh_event clsh_event,
+ int nstate,
+ enum wcd_clsh_mode mode)
+{
+ struct snd_soc_component *comp = ctrl->comp;
+
+ if (nstate == ctrl->state)
+ return 0;
+
+ if (!wcd_clsh_is_state_valid(nstate)) {
+ dev_err(comp->dev, "Class-H not a valid new state:\n");
+ return -EINVAL;
+ }
+
+ switch (clsh_event) {
+ case WCD_CLSH_EVENT_PRE_DAC:
+ _wcd_clsh_ctrl_set_state(ctrl, nstate, CLSH_REQ_ENABLE, mode);
+ break;
+ case WCD_CLSH_EVENT_POST_PA:
+ _wcd_clsh_ctrl_set_state(ctrl, nstate, CLSH_REQ_DISABLE, mode);
+ break;
+ }
+
+ ctrl->state = nstate;
+ ctrl->mode = mode;
+
+ return 0;
+}
+
+static inline int wcd_clsh_ctrl_get_state(struct wcd_clsh_ctrl *ctrl)
+{
+ return ctrl->state;
+}
+
+static inline struct wcd_clsh_ctrl *wcd_clsh_ctrl_alloc(struct snd_soc_component *comp,
+ int version)
+{
+ struct wcd_clsh_ctrl *ctrl;
+
+ ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
+ if (!ctrl)
+ return ERR_PTR(-ENOMEM);
+
+ ctrl->state = WCD_CLSH_STATE_IDLE;
+ ctrl->comp = comp;
+ ctrl->codec_version = version;
+
+ return ctrl;
+}
+
+static inline void wcd_clsh_ctrl_free(struct wcd_clsh_ctrl *ctrl)
+{
+ kfree(ctrl);
+}

#endif /* _WCD_CLSH_V2_H_ */
--
2.38.1



2022-11-20 00:02:36

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH 10/18] EDAC: i10nm, skx: fix mixed module-builtin object

With CONFIG_EDAC_SKX=m and CONFIG_EDAC_I10NM=y (or vice versa),
skx_common.o are linked to a module and also to vmlinux even though
the expected CFLAGS are different between builtins and modules:

> scripts/Makefile.build:252: ./drivers/edac/Makefile: skx_common.o
> is added to multiple modules: i10nm_edac skx_edac

This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
Fixing mixed module-builtin objects").

Introduce the new module, skx_edac_common, to provide the common
functions to skx_edac and i10nm_edac. skx_adxl_{get,put}() loose
their __init/__exit annotations in order to become exportable.

Fixes: d4dc89d069aa ("EDAC, i10nm: Add a driver for Intel 10nm server processors")
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Alexander Lobakin <[email protected]>
---
drivers/edac/Kconfig | 11 +++++++----
drivers/edac/Makefile | 7 +++++--
drivers/edac/i10nm_base.c | 2 ++
drivers/edac/skx_base.c | 2 ++
drivers/edac/skx_common.c | 21 +++++++++++++++++++--
drivers/edac/skx_common.h | 4 ++--
6 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 456602d373b7..c3d96d2a814b 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -236,12 +236,16 @@ config EDAC_SBRIDGE
Support for error detection and correction the Intel
Sandy Bridge, Ivy Bridge and Haswell Integrated Memory Controllers.

+config EDAC_SKX_COMMON
+ tristate
+ select ACPI_ADXL
+ select DMI
+
config EDAC_SKX
tristate "Intel Skylake server Integrated MC"
depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG && ACPI
depends on ACPI_NFIT || !ACPI_NFIT # if ACPI_NFIT=m, EDAC_SKX can't be y
- select DMI
- select ACPI_ADXL
+ select EDAC_SKX_COMMON
help
Support for error detection and correction the Intel
Skylake server Integrated Memory Controllers. If your
@@ -252,8 +256,7 @@ config EDAC_I10NM
tristate "Intel 10nm server Integrated MC"
depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG && ACPI
depends on ACPI_NFIT || !ACPI_NFIT # if ACPI_NFIT=m, EDAC_I10NM can't be y
- select DMI
- select ACPI_ADXL
+ select EDAC_SKX_COMMON
help
Support for error detection and correction the Intel
10nm server Integrated Memory Controllers. If your
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 2d1641a27a28..36e6e07d4048 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -54,10 +54,13 @@ obj-$(CONFIG_EDAC_MPC85XX) += mpc85xx_edac_mod.o
layerscape_edac_mod-y := fsl_ddr_edac.o layerscape_edac.o
obj-$(CONFIG_EDAC_LAYERSCAPE) += layerscape_edac_mod.o

-skx_edac-y := skx_common.o skx_base.o
+skx_edac_common-y := skx_common.o
+obj-$(CONFIG_EDAC_SKX_COMMON) += skx_edac_common.o
+
+skx_edac-y := skx_base.o
obj-$(CONFIG_EDAC_SKX) += skx_edac.o

-i10nm_edac-y := skx_common.o i10nm_base.o
+i10nm_edac-y := i10nm_base.o
obj-$(CONFIG_EDAC_I10NM) += i10nm_edac.o

obj-$(CONFIG_EDAC_CELL) += cell_edac.o
diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c
index a22ea053f8e1..949f665fd94c 100644
--- a/drivers/edac/i10nm_base.c
+++ b/drivers/edac/i10nm_base.c
@@ -900,5 +900,7 @@ MODULE_PARM_DESC(decoding_via_mca, "decoding_via_mca: 0=off(default), 1=enable")
module_param(retry_rd_err_log, int, 0444);
MODULE_PARM_DESC(retry_rd_err_log, "retry_rd_err_log: 0=off(default), 1=bios(Linux doesn't reset any control bits, but just reports values.), 2=linux(Linux tries to take control and resets mode bits, clear valid/UC bits after reading.)");

+MODULE_IMPORT_NS(EDAC_SKX_COMMON);
+
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("MC Driver for Intel 10nm server processors");
diff --git a/drivers/edac/skx_base.c b/drivers/edac/skx_base.c
index 7e2762f62eec..1656cd4cd0ed 100644
--- a/drivers/edac/skx_base.c
+++ b/drivers/edac/skx_base.c
@@ -751,6 +751,8 @@ module_exit(skx_exit);
module_param(edac_op_state, int, 0444);
MODULE_PARM_DESC(edac_op_state, "EDAC Error Reporting state: 0=Poll,1=NMI");

+MODULE_IMPORT_NS(EDAC_SKX_COMMON);
+
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Tony Luck");
MODULE_DESCRIPTION("MC Driver for Intel Skylake server processors");
diff --git a/drivers/edac/skx_common.c b/drivers/edac/skx_common.c
index f0f8e98f6efb..15a3fb1224ae 100644
--- a/drivers/edac/skx_common.c
+++ b/drivers/edac/skx_common.c
@@ -48,7 +48,7 @@ static u64 skx_tolm, skx_tohm;
static LIST_HEAD(dev_edac_list);
static bool skx_mem_cfg_2lm;

-int __init skx_adxl_get(void)
+int skx_adxl_get(void)
{
const char * const *names;
int i, j;
@@ -110,12 +110,14 @@ int __init skx_adxl_get(void)

return -ENODEV;
}
+EXPORT_SYMBOL_NS_GPL(skx_adxl_get, EDAC_SKX_COMMON);

-void __exit skx_adxl_put(void)
+void skx_adxl_put(void)
{
kfree(adxl_values);
kfree(adxl_msg);
}
+EXPORT_SYMBOL_NS_GPL(skx_adxl_put, EDAC_SKX_COMMON);

static bool skx_adxl_decode(struct decoded_addr *res, bool error_in_1st_level_mem)
{
@@ -187,12 +189,14 @@ void skx_set_mem_cfg(bool mem_cfg_2lm)
{
skx_mem_cfg_2lm = mem_cfg_2lm;
}
+EXPORT_SYMBOL_NS_GPL(skx_set_mem_cfg, EDAC_SKX_COMMON);

void skx_set_decode(skx_decode_f decode, skx_show_retry_log_f show_retry_log)
{
driver_decode = decode;
skx_show_retry_rd_err_log = show_retry_log;
}
+EXPORT_SYMBOL_NS_GPL(skx_set_decode, EDAC_SKX_COMMON);

int skx_get_src_id(struct skx_dev *d, int off, u8 *id)
{
@@ -206,6 +210,7 @@ int skx_get_src_id(struct skx_dev *d, int off, u8 *id)
*id = GET_BITFIELD(reg, 12, 14);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(skx_get_src_id, EDAC_SKX_COMMON);

int skx_get_node_id(struct skx_dev *d, u8 *id)
{
@@ -219,6 +224,7 @@ int skx_get_node_id(struct skx_dev *d, u8 *id)
*id = GET_BITFIELD(reg, 0, 2);
return 0;
}
+EXPORT_SYMBOL_NS_GPL(skx_get_node_id, EDAC_SKX_COMMON);

static int get_width(u32 mtr)
{
@@ -284,6 +290,7 @@ int skx_get_all_bus_mappings(struct res_config *cfg, struct list_head **list)
*list = &dev_edac_list;
return ndev;
}
+EXPORT_SYMBOL_NS_GPL(skx_get_all_bus_mappings, EDAC_SKX_COMMON);

int skx_get_hi_lo(unsigned int did, int off[], u64 *tolm, u64 *tohm)
{
@@ -323,6 +330,7 @@ int skx_get_hi_lo(unsigned int did, int off[], u64 *tolm, u64 *tohm)
pci_dev_put(pdev);
return -ENODEV;
}
+EXPORT_SYMBOL_NS_GPL(skx_get_hi_lo, EDAC_SKX_COMMON);

static int skx_get_dimm_attr(u32 reg, int lobit, int hibit, int add,
int minval, int maxval, const char *name)
@@ -394,6 +402,7 @@ int skx_get_dimm_info(u32 mtr, u32 mcmtr, u32 amap, struct dimm_info *dimm,

return 1;
}
+EXPORT_SYMBOL_NS_GPL(skx_get_dimm_info, EDAC_SKX_COMMON);

int skx_get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc,
int chan, int dimmno, const char *mod_str)
@@ -442,6 +451,7 @@ int skx_get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc,

return (size == 0 || size == ~0ull) ? 0 : 1;
}
+EXPORT_SYMBOL_NS_GPL(skx_get_nvdimm_info, EDAC_SKX_COMMON);

int skx_register_mci(struct skx_imc *imc, struct pci_dev *pdev,
const char *ctl_name, const char *mod_str,
@@ -512,6 +522,7 @@ int skx_register_mci(struct skx_imc *imc, struct pci_dev *pdev,
imc->mci = NULL;
return rc;
}
+EXPORT_SYMBOL_NS_GPL(skx_register_mci, EDAC_SKX_COMMON);

static void skx_unregister_mci(struct skx_imc *imc)
{
@@ -694,6 +705,7 @@ int skx_mce_check_error(struct notifier_block *nb, unsigned long val,
mce->kflags |= MCE_HANDLED_EDAC;
return NOTIFY_DONE;
}
+EXPORT_SYMBOL_NS_GPL(skx_mce_check_error, EDAC_SKX_COMMON);

void skx_remove(void)
{
@@ -731,3 +743,8 @@ void skx_remove(void)
kfree(d);
}
}
+EXPORT_SYMBOL_NS_GPL(skx_remove, EDAC_SKX_COMMON);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Tony Luck");
+MODULE_DESCRIPTION("MC Common Library for Intel server processors");
diff --git a/drivers/edac/skx_common.h b/drivers/edac/skx_common.h
index 0cbadd3d2cd3..c0c174c101d2 100644
--- a/drivers/edac/skx_common.h
+++ b/drivers/edac/skx_common.h
@@ -178,8 +178,8 @@ typedef int (*get_dimm_config_f)(struct mem_ctl_info *mci,
typedef bool (*skx_decode_f)(struct decoded_addr *res);
typedef void (*skx_show_retry_log_f)(struct decoded_addr *res, char *msg, int len, bool scrub_err);

-int __init skx_adxl_get(void);
-void __exit skx_adxl_put(void);
+int skx_adxl_get(void);
+void skx_adxl_put(void);
void skx_set_decode(skx_decode_f decode, skx_show_retry_log_f show_retry_log);
void skx_set_mem_cfg(bool mem_cfg_2lm);

--
2.38.1



2022-11-20 12:36:12

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

On Sat, Nov 19, 2022 at 11:03:57PM +0000, Alexander Lobakin wrote:

Your mails appear to be encrypted which isn't helping with
review...


Attachments:
(No filename) (141.00 B)
signature.asc (499.00 B)
Download all attachments

2022-11-20 12:42:25

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

On Sun, Nov 20, 2022 at 11:58:26AM +0000, Mark Brown wrote:
> On Sat, Nov 19, 2022 at 11:03:57PM +0000, Alexander Lobakin wrote:
>
> Your mails appear to be encrypted which isn't helping with
> review...

https://lore.kernel.org/all/[email protected]/

Looks un-encrypted on lore. pm.me looks to be a protonmail domain - I
had issues with protonmail where it picked up Maz's key via Web Key
Directory. "Kernel.org publishes the WKD for all developers who have
kernel.org accounts" & unfortunately proton doesn't (or didn't) offer a
way to disable this. If someone's key is available it gets used & proton
told me, IIRC, that not having a way to disable this is a privacy
feature.

Unfortunately, my workaround was leaving protonmail.


2022-11-20 15:05:09

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
> common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
>
> > scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
> > common.o is added to multiple modules: intel_skl_int3472_discrete
> > intel_skl_int3472_tps68470
>
> Although both drivers share one Kconfig option
> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
> into several modules (and/or vmlinux).
> Under certain circumstances, such can lead to the situation fixed by
> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
>
> Introduce the new module, intel_skl_int3472_common, to provide the
> functions from common.o to both discrete and tps68470 drivers. This
> adds only 3 exports and doesn't provide any changes to the actual
> code.

...

> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> +

Redundant blank line. You may put it to be last MODULE_*() in the file, if you
think it would be more visible.

> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
> MODULE_AUTHOR("Daniel Scally <[email protected]>");
> MODULE_LICENSE("GPL v2");

...

> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> +
> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
> MODULE_AUTHOR("Daniel Scally <[email protected]>");
> MODULE_LICENSE("GPL v2");

Ditto. And the same to all your patches.

--
With Best Regards,
Andy Shevchenko



2022-11-20 21:03:50

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

Hi,

On 11/20/22 14:55, Andy Shevchenko wrote:
> On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
>> common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
>>
>>> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
>>> common.o is added to multiple modules: intel_skl_int3472_discrete
>>> intel_skl_int3472_tps68470
>>
>> Although both drivers share one Kconfig option
>> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
>> into several modules (and/or vmlinux).
>> Under certain circumstances, such can lead to the situation fixed by
>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
>>
>> Introduce the new module, intel_skl_int3472_common, to provide the
>> functions from common.o to both discrete and tps68470 drivers. This
>> adds only 3 exports and doesn't provide any changes to the actual
>> code.

Replying to Andy's reply here since I never saw the original submission
which was not Cc-ed to [email protected] .

As you mention already in the commit msg, the issue from:

commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects")

is not an issue here since both modules sharing the .o file are
behind the same Kconfig option.

So there is not really an issue here and common.o is tiny, so
small chances are it does not ever increase the .ko size
when looking a the .ko size rounded up to a multiple of
the filesystem size.

At the same time adding an extra module does come with significant
costs, it will eat up at least 1 possibly more then 1 fs blocks
(I don't know what the module header size overhead is).

And it needs to be loaded separately and module loading is slow;
and it will grow the /lib/modules/<kver>/modules.* sizes.

So nack from me for this patch, since I really don't see
it adding any value.

Regards,

Hans





>
> ...
>
>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
>> +
>
> Redundant blank line. You may put it to be last MODULE_*() in the file, if you
> think it would be more visible.
>
>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
>> MODULE_LICENSE("GPL v2");
>
> ...
>
>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
>> +
>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
>> MODULE_LICENSE("GPL v2");
>
> Ditto. And the same to all your patches.
>


2022-11-20 23:56:47

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

On Mon, Nov 21, 2022 at 5:55 AM Hans de Goede <[email protected]> wrote:
>
> Hi,
>
> On 11/20/22 14:55, Andy Shevchenko wrote:
> > On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
> >> common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
> >>
> >>> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
> >>> common.o is added to multiple modules: intel_skl_int3472_discrete
> >>> intel_skl_int3472_tps68470
> >>
> >> Although both drivers share one Kconfig option
> >> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
> >> into several modules (and/or vmlinux).
> >> Under certain circumstances, such can lead to the situation fixed by
> >> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> >>
> >> Introduce the new module, intel_skl_int3472_common, to provide the
> >> functions from common.o to both discrete and tps68470 drivers. This
> >> adds only 3 exports and doesn't provide any changes to the actual
> >> code.
>
> Replying to Andy's reply here since I never saw the original submission
> which was not Cc-ed to [email protected] .
>
> As you mention already in the commit msg, the issue from:
>
> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects")
>
> is not an issue here since both modules sharing the .o file are
> behind the same Kconfig option.
>
> So there is not really an issue here and common.o is tiny, so
> small chances are it does not ever increase the .ko size
> when looking a the .ko size rounded up to a multiple of
> the filesystem size.
>
> At the same time adding an extra module does come with significant
> costs, it will eat up at least 1 possibly more then 1 fs blocks
> (I don't know what the module header size overhead is).
>
> And it needs to be loaded separately and module loading is slow;
> and it will grow the /lib/modules/<kver>/modules.* sizes.
>
> So nack from me for this patch, since I really don't see
> it adding any value.




This does have a value.

This clarifies the ownership of the common.o,
in other words, makes KBUILD_MODNAME deterministic.


If an object belongs to a module,
KBUILD_MODNAME is defined as the module name.

If an object is always built-in,
KBUILD_MODNAME is defined as the basename of the object.



Here is a question:
if common.o is shared by two modules intel_skl_int3472_discrete
and intel_skl_int3472_tps68470, what should KBUILD_MODNAME be?


I see some patch submissions relying on the assumption that
KBUILD_MODNAME is unique.
We cannot determine KBUILD_MODNAME correctly if an object is shared
by multiple modules.






BTW, this patch is not the way I suggested.
The Suggested-by should not have been there
(or at least Reported-by)


You argued "common.o is tiny", so I would vote for
making them inline functions, like


https://lore.kernel.org/linux-kbuild/[email protected]/T/#u








> Regards,
>
> Hans
>
>
>
>
>
> >
> > ...
> >
> >> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> >> +
> >
> > Redundant blank line. You may put it to be last MODULE_*() in the file, if you
> > think it would be more visible.
> >
> >> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
> >> MODULE_AUTHOR("Daniel Scally <[email protected]>");
> >> MODULE_LICENSE("GPL v2");
> >
> > ...
> >
> >> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> >> +
> >> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
> >> MODULE_AUTHOR("Daniel Scally <[email protected]>");
> >> MODULE_LICENSE("GPL v2");
> >
> > Ditto. And the same to all your patches.
> >
>


--
Best Regards
Masahiro Yamada

2022-11-21 08:24:51

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

Hi,

On 11/21/22 00:45, Masahiro Yamada wrote:
> On Mon, Nov 21, 2022 at 5:55 AM Hans de Goede <[email protected]> wrote:
>>
>> Hi,
>>
>> On 11/20/22 14:55, Andy Shevchenko wrote:
>>> On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
>>>> common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
>>>>
>>>>> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
>>>>> common.o is added to multiple modules: intel_skl_int3472_discrete
>>>>> intel_skl_int3472_tps68470
>>>>
>>>> Although both drivers share one Kconfig option
>>>> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
>>>> into several modules (and/or vmlinux).
>>>> Under certain circumstances, such can lead to the situation fixed by
>>>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
>>>>
>>>> Introduce the new module, intel_skl_int3472_common, to provide the
>>>> functions from common.o to both discrete and tps68470 drivers. This
>>>> adds only 3 exports and doesn't provide any changes to the actual
>>>> code.
>>
>> Replying to Andy's reply here since I never saw the original submission
>> which was not Cc-ed to [email protected] .
>>
>> As you mention already in the commit msg, the issue from:
>>
>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects")
>>
>> is not an issue here since both modules sharing the .o file are
>> behind the same Kconfig option.
>>
>> So there is not really an issue here and common.o is tiny, so
>> small chances are it does not ever increase the .ko size
>> when looking a the .ko size rounded up to a multiple of
>> the filesystem size.
>>
>> At the same time adding an extra module does come with significant
>> costs, it will eat up at least 1 possibly more then 1 fs blocks
>> (I don't know what the module header size overhead is).
>>
>> And it needs to be loaded separately and module loading is slow;
>> and it will grow the /lib/modules/<kver>/modules.* sizes.
>>
>> So nack from me for this patch, since I really don't see
>> it adding any value.
>
>
>
>
> This does have a value.
>
> This clarifies the ownership of the common.o,
> in other words, makes KBUILD_MODNAME deterministic.
>
>
> If an object belongs to a module,
> KBUILD_MODNAME is defined as the module name.
>
> If an object is always built-in,
> KBUILD_MODNAME is defined as the basename of the object.
>
>
>
> Here is a question:
> if common.o is shared by two modules intel_skl_int3472_discrete
> and intel_skl_int3472_tps68470, what should KBUILD_MODNAME be?
>
>
> I see some patch submissions relying on the assumption that
> KBUILD_MODNAME is unique.
> We cannot determine KBUILD_MODNAME correctly if an object is shared
> by multiple modules.
>
>
>
>
>
>
> BTW, this patch is not the way I suggested.
> The Suggested-by should not have been there
> (or at least Reported-by)
>
>
> You argued "common.o is tiny", so I would vote for
> making them inline functions, like
>
>
> https://lore.kernel.org/linux-kbuild/[email protected]/T/#u

Yes just moving the contents of common.c to static inline helpers in common.h
would be much better.

If someone creates such a patch, please do not forget to Cc
[email protected]

Regards,

Hans



>
>
>
>
>
>
>
>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>
>>>
>>> ...
>>>
>>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
>>>> +
>>>
>>> Redundant blank line. You may put it to be last MODULE_*() in the file, if you
>>> think it would be more visible.
>>>
>>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
>>>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
>>>> MODULE_LICENSE("GPL v2");
>>>
>>> ...
>>>
>>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
>>>> +
>>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
>>>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
>>>> MODULE_LICENSE("GPL v2");
>>>
>>> Ditto. And the same to all your patches.
>>>
>>
>
>


2022-11-21 09:39:02

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

On Mon, Nov 21, 2022 at 5:12 PM Hans de Goede <[email protected]> wrote:
>
> Hi,
>
> On 11/21/22 00:45, Masahiro Yamada wrote:
> > On Mon, Nov 21, 2022 at 5:55 AM Hans de Goede <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> On 11/20/22 14:55, Andy Shevchenko wrote:
> >>> On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
> >>>> common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
> >>>>
> >>>>> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
> >>>>> common.o is added to multiple modules: intel_skl_int3472_discrete
> >>>>> intel_skl_int3472_tps68470
> >>>>
> >>>> Although both drivers share one Kconfig option
> >>>> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
> >>>> into several modules (and/or vmlinux).
> >>>> Under certain circumstances, such can lead to the situation fixed by
> >>>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> >>>>
> >>>> Introduce the new module, intel_skl_int3472_common, to provide the
> >>>> functions from common.o to both discrete and tps68470 drivers. This
> >>>> adds only 3 exports and doesn't provide any changes to the actual
> >>>> code.
> >>
> >> Replying to Andy's reply here since I never saw the original submission
> >> which was not Cc-ed to [email protected] .
> >>
> >> As you mention already in the commit msg, the issue from:
> >>
> >> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects")
> >>
> >> is not an issue here since both modules sharing the .o file are
> >> behind the same Kconfig option.
> >>
> >> So there is not really an issue here and common.o is tiny, so
> >> small chances are it does not ever increase the .ko size
> >> when looking a the .ko size rounded up to a multiple of
> >> the filesystem size.
> >>
> >> At the same time adding an extra module does come with significant
> >> costs, it will eat up at least 1 possibly more then 1 fs blocks
> >> (I don't know what the module header size overhead is).
> >>
> >> And it needs to be loaded separately and module loading is slow;
> >> and it will grow the /lib/modules/<kver>/modules.* sizes.
> >>
> >> So nack from me for this patch, since I really don't see
> >> it adding any value.
> >
> >
> >
> >
> > This does have a value.
> >
> > This clarifies the ownership of the common.o,
> > in other words, makes KBUILD_MODNAME deterministic.
> >
> >
> > If an object belongs to a module,
> > KBUILD_MODNAME is defined as the module name.
> >
> > If an object is always built-in,
> > KBUILD_MODNAME is defined as the basename of the object.
> >
> >
> >
> > Here is a question:
> > if common.o is shared by two modules intel_skl_int3472_discrete
> > and intel_skl_int3472_tps68470, what should KBUILD_MODNAME be?
> >
> >
> > I see some patch submissions relying on the assumption that
> > KBUILD_MODNAME is unique.
> > We cannot determine KBUILD_MODNAME correctly if an object is shared
> > by multiple modules.
> >
> >
> >
> >
> >
> >
> > BTW, this patch is not the way I suggested.
> > The Suggested-by should not have been there
> > (or at least Reported-by)
> >
> >
> > You argued "common.o is tiny", so I would vote for
> > making them inline functions, like
> >
> >
> > https://lore.kernel.org/linux-kbuild/[email protected]/T/#u
>
> Yes just moving the contents of common.c to static inline helpers in common.h
> would be much better.
>
> If someone creates such a patch, please do not forget to Cc
> [email protected]



I think this patch series should be split
and sent to each sub-system instead of kbuild.






> Regards,
>
> Hans
>
>
>
> >
> >
> >
> >
> >
> >
> >
> >
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>
> >>
> >>
> >>>
> >>> ...
> >>>
> >>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> >>>> +
> >>>
> >>> Redundant blank line. You may put it to be last MODULE_*() in the file, if you
> >>> think it would be more visible.
> >>>
> >>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
> >>>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
> >>>> MODULE_LICENSE("GPL v2");
> >>>
> >>> ...
> >>>
> >>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> >>>> +
> >>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
> >>>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
> >>>> MODULE_LICENSE("GPL v2");
> >>>
> >>> Ditto. And the same to all your patches.
> >>>
> >>
> >
> >
>


--
Best Regards
Masahiro Yamada

2022-11-21 09:44:01

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

Hi,

On 11/21/22 10:06, Masahiro Yamada wrote:
> On Mon, Nov 21, 2022 at 5:12 PM Hans de Goede <[email protected]> wrote:
>>
>> Hi,
>>
>> On 11/21/22 00:45, Masahiro Yamada wrote:
>>> On Mon, Nov 21, 2022 at 5:55 AM Hans de Goede <[email protected]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 11/20/22 14:55, Andy Shevchenko wrote:
>>>>> On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
>>>>>> common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
>>>>>>
>>>>>>> scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
>>>>>>> common.o is added to multiple modules: intel_skl_int3472_discrete
>>>>>>> intel_skl_int3472_tps68470
>>>>>>
>>>>>> Although both drivers share one Kconfig option
>>>>>> (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
>>>>>> into several modules (and/or vmlinux).
>>>>>> Under certain circumstances, such can lead to the situation fixed by
>>>>>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
>>>>>>
>>>>>> Introduce the new module, intel_skl_int3472_common, to provide the
>>>>>> functions from common.o to both discrete and tps68470 drivers. This
>>>>>> adds only 3 exports and doesn't provide any changes to the actual
>>>>>> code.
>>>>
>>>> Replying to Andy's reply here since I never saw the original submission
>>>> which was not Cc-ed to [email protected] .
>>>>
>>>> As you mention already in the commit msg, the issue from:
>>>>
>>>> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects")
>>>>
>>>> is not an issue here since both modules sharing the .o file are
>>>> behind the same Kconfig option.
>>>>
>>>> So there is not really an issue here and common.o is tiny, so
>>>> small chances are it does not ever increase the .ko size
>>>> when looking a the .ko size rounded up to a multiple of
>>>> the filesystem size.
>>>>
>>>> At the same time adding an extra module does come with significant
>>>> costs, it will eat up at least 1 possibly more then 1 fs blocks
>>>> (I don't know what the module header size overhead is).
>>>>
>>>> And it needs to be loaded separately and module loading is slow;
>>>> and it will grow the /lib/modules/<kver>/modules.* sizes.
>>>>
>>>> So nack from me for this patch, since I really don't see
>>>> it adding any value.
>>>
>>>
>>>
>>>
>>> This does have a value.
>>>
>>> This clarifies the ownership of the common.o,
>>> in other words, makes KBUILD_MODNAME deterministic.
>>>
>>>
>>> If an object belongs to a module,
>>> KBUILD_MODNAME is defined as the module name.
>>>
>>> If an object is always built-in,
>>> KBUILD_MODNAME is defined as the basename of the object.
>>>
>>>
>>>
>>> Here is a question:
>>> if common.o is shared by two modules intel_skl_int3472_discrete
>>> and intel_skl_int3472_tps68470, what should KBUILD_MODNAME be?
>>>
>>>
>>> I see some patch submissions relying on the assumption that
>>> KBUILD_MODNAME is unique.
>>> We cannot determine KBUILD_MODNAME correctly if an object is shared
>>> by multiple modules.
>>>
>>>
>>>
>>>
>>>
>>>
>>> BTW, this patch is not the way I suggested.
>>> The Suggested-by should not have been there
>>> (or at least Reported-by)
>>>
>>>
>>> You argued "common.o is tiny", so I would vote for
>>> making them inline functions, like
>>>
>>>
>>> https://lore.kernel.org/linux-kbuild/[email protected]/T/#u
>>
>> Yes just moving the contents of common.c to static inline helpers in common.h
>> would be much better.
>>
>> If someone creates such a patch, please do not forget to Cc
>> [email protected]
>
>
>
> I think this patch series should be split
> and sent to each sub-system instead of kbuild.

Yes definitely, the changes are big enough that not merging
this through the subsystem trees is going to cause conflicts.

Regards,

Hans





>>>>> ...
>>>>>
>>>>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
>>>>>> +
>>>>>
>>>>> Redundant blank line. You may put it to be last MODULE_*() in the file, if you
>>>>> think it would be more visible.
>>>>>
>>>>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
>>>>>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
>>>>>> MODULE_LICENSE("GPL v2");
>>>>>
>>>>> ...
>>>>>
>>>>>> +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
>>>>>> +
>>>>>> MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
>>>>>> MODULE_AUTHOR("Daniel Scally <[email protected]>");
>>>>>> MODULE_LICENSE("GPL v2");
>>>>>
>>>>> Ditto. And the same to all your patches.
>>>>>
>>>>
>>>
>>>
>>
>
>


2022-11-21 18:16:40

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
> With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
> (or vice versa), felix.o are linked to a module and also to vmlinux
> even though the expected CFLAGS are different between builtins and
> modules.
> This is the same situation as fixed by
> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> There's also no need to duplicate relatively big piece of object
> code into two modules.
>
> Introduce the new module, mscc_core, to provide the common functions
> to both mscc_felix and mscc_seville.
>
> Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>
> ---

I don't disagree with the patch, but I dislike the name chosen.
How about NET_DSA_OCELOT_LIB and mscc_ocelot_dsa_lib.o? The "core" of
the hardware support is arguably mscc_ocelot_switch_lib.o, I don't think
it would be good to use that word here, since the code you're moving is
no more than a thin glue layer with some DSA specific code.

Adding Colin for a second opinion on the naming. I'm sure things could
have been done better in the first place, just not sure how.

Also, could you please make the commit prefix "net: dsa: ocelot" when
you resend this and the other networking patches to the "net" tree?

> drivers/net/dsa/ocelot/Kconfig | 18 ++++++++++--------
> drivers/net/dsa/ocelot/Makefile | 12 +++++-------
> drivers/net/dsa/ocelot/felix.c | 6 ++++++
> drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++
> drivers/net/dsa/ocelot/seville_vsc9953.c | 2 ++
> 5 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig
> index 08db9cf76818..59845274e374 100644
> --- a/drivers/net/dsa/ocelot/Kconfig
> +++ b/drivers/net/dsa/ocelot/Kconfig
> @@ -1,4 +1,12 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +
> +config NET_DSA_MSCC_CORE
> + tristate
> + select MSCC_OCELOT_SWITCH_LIB
> + select NET_DSA_TAG_OCELOT_8021Q
> + select NET_DSA_TAG_OCELOT
> + select PCS_LYNX

Please keep PCS_LYNX selected by MSCC_FELIX and MSCC_SEVILLE respectively.

> +
> config NET_DSA_MSCC_FELIX
> tristate "Ocelot / Felix Ethernet switch support"
> depends on NET_DSA && PCI
> @@ -7,11 +15,8 @@ config NET_DSA_MSCC_FELIX
> depends on HAS_IOMEM
> depends on PTP_1588_CLOCK_OPTIONAL
> depends on NET_SCH_TAPRIO || NET_SCH_TAPRIO=n
> - select MSCC_OCELOT_SWITCH_LIB
> - select NET_DSA_TAG_OCELOT_8021Q
> - select NET_DSA_TAG_OCELOT
> + select NET_DSA_MSCC_CORE
> select FSL_ENETC_MDIO
> - select PCS_LYNX
> help
> This driver supports the VSC9959 (Felix) switch, which is embedded as
> a PCIe function of the NXP LS1028A ENETC RCiEP.
> @@ -22,11 +27,8 @@ config NET_DSA_MSCC_SEVILLE
> depends on NET_VENDOR_MICROSEMI
> depends on HAS_IOMEM
> depends on PTP_1588_CLOCK_OPTIONAL
> + select NET_DSA_MSCC_CORE
> select MDIO_MSCC_MIIM
> - select MSCC_OCELOT_SWITCH_LIB
> - select NET_DSA_TAG_OCELOT_8021Q
> - select NET_DSA_TAG_OCELOT
> - select PCS_LYNX
> help
> This driver supports the VSC9953 (Seville) switch, which is embedded
> as a platform device on the NXP T1040 SoC.
> diff --git a/drivers/net/dsa/ocelot/Makefile b/drivers/net/dsa/ocelot/Makefile
> index f6dd131e7491..f8c74b59b996 100644
> --- a/drivers/net/dsa/ocelot/Makefile
> +++ b/drivers/net/dsa/ocelot/Makefile
> @@ -1,11 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_NET_DSA_MSCC_CORE) += mscc_core.o
> obj-$(CONFIG_NET_DSA_MSCC_FELIX) += mscc_felix.o
> obj-$(CONFIG_NET_DSA_MSCC_SEVILLE) += mscc_seville.o
>
> -mscc_felix-objs := \
> - felix.o \
> - felix_vsc9959.o
> -
> -mscc_seville-objs := \
> - felix.o \
> - seville_vsc9953.o
> +mscc_core-objs := felix.o
> +mscc_felix-objs := felix_vsc9959.o
> +mscc_seville-objs := seville_vsc9953.o
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index dd3a18cc89dd..f9d0a24ebc3a 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -2112,6 +2112,7 @@ const struct dsa_switch_ops felix_switch_ops = {
> .port_set_host_flood = felix_port_set_host_flood,
> .port_change_master = felix_port_change_master,
> };
> +EXPORT_SYMBOL_NS_GPL(felix_switch_ops, NET_DSA_MSCC_CORE);

What do we gain practically with the symbol namespacing?

>
> struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)
> {
> @@ -2123,6 +2124,7 @@ struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)
>
> return dsa_to_port(ds, port)->slave;
> }
> +EXPORT_SYMBOL_NS_GPL(felix_port_to_netdev, NET_DSA_MSCC_CORE);
>
> int felix_netdev_to_port(struct net_device *dev)
> {
> @@ -2134,3 +2136,7 @@ int felix_netdev_to_port(struct net_device *dev)
>
> return dp->index;
> }
> +EXPORT_SYMBOL_NS_GPL(felix_netdev_to_port, NET_DSA_MSCC_CORE);
> +
> +MODULE_DESCRIPTION("MSCC Switch driver core");

Ocelot switch DSA library

> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 26a35ae322d1..52c8bff79fa3 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -2736,5 +2736,7 @@ static struct pci_driver felix_vsc9959_pci_driver = {
> };
> module_pci_driver(felix_vsc9959_pci_driver);
>
> +MODULE_IMPORT_NS(NET_DSA_MSCC_CORE);
> +
> MODULE_DESCRIPTION("Felix Switch driver");
> MODULE_LICENSE("GPL v2");
> diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
> index 7af33b2c685d..e9c63c642489 100644
> --- a/drivers/net/dsa/ocelot/seville_vsc9953.c
> +++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
> @@ -1115,5 +1115,7 @@ static struct platform_driver seville_vsc9953_driver = {
> };
> module_platform_driver(seville_vsc9953_driver);
>
> +MODULE_IMPORT_NS(NET_DSA_MSCC_CORE);
> +
> MODULE_DESCRIPTION("Seville Switch driver");
> MODULE_LICENSE("GPL v2");
> --
> 2.38.1
>
>

2022-11-21 18:22:12

by Colin Foster

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

On Mon, Nov 21, 2022 at 07:55:04PM +0200, Vladimir Oltean wrote:
> On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
> > With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
> > (or vice versa), felix.o are linked to a module and also to vmlinux
> > even though the expected CFLAGS are different between builtins and
> > modules.
> > This is the same situation as fixed by
> > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> > There's also no need to duplicate relatively big piece of object
> > code into two modules.
> >
> > Introduce the new module, mscc_core, to provide the common functions
> > to both mscc_felix and mscc_seville.
> >
> > Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
> > Suggested-by: Masahiro Yamada <[email protected]>
> > Signed-off-by: Alexander Lobakin <[email protected]>
> > ---
>
> I don't disagree with the patch, but I dislike the name chosen.
> How about NET_DSA_OCELOT_LIB and mscc_ocelot_dsa_lib.o? The "core" of
> the hardware support is arguably mscc_ocelot_switch_lib.o, I don't think
> it would be good to use that word here, since the code you're moving is
> no more than a thin glue layer with some DSA specific code.
>
> Adding Colin for a second opinion on the naming. I'm sure things could
> have been done better in the first place, just not sure how.

Good catch on this patch. "mscc_ocelot_dsa_lib" makes sense. The only
other option that might be considered would be along the lines of
"felix_lib". While I know "Felix" is the chip, in the dsa directory it
seems to represent the DSA lib in general.

Either one seems fine for me. And thanks for the heads up, as I'll need
to make the same changes for ocelot_ext when it is ready.

2022-11-21 20:01:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

On Mon, Nov 21, 2022 at 07:55:04PM +0200, Vladimir Oltean wrote:
> On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:

...

> > +EXPORT_SYMBOL_NS_GPL(felix_switch_ops, NET_DSA_MSCC_CORE);
>
> What do we gain practically with the symbol namespacing?

I guess this wrap-up can possibly answer this:
https://lwn.net/Articles/760045/

--
With Best Regards,
Andy Shevchenko



2022-11-21 20:41:38

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

On Sat, 19 Nov 2022 23:03:57 +0000 Alexander Lobakin wrote:
> Subject: [PATCH 00/18] treewide: fix object files shared between several modules

Could you repost the networking changes separately in v2?
I'm not seeing any reason why this would have to be a treewide
series when merging.

> monotonic

monotonous, unless you mean steady progress ;)

2022-11-21 21:12:40

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

On Mon, Nov 21, 2022 at 10:12:59AM -0800, Colin Foster wrote:
> Good catch on this patch. "mscc_ocelot_dsa_lib" makes sense. The only
> other option that might be considered would be along the lines of
> "felix_lib". While I know "Felix" is the chip, in the dsa directory it
> seems to represent the DSA lib in general.

"mscc_felix_lib" could work too. At least it's more consistent with
felix.c and the function names.

2022-11-22 12:38:02

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

On Sun, Nov 20, 2022 at 12:26:00PM +0000, Conor Dooley wrote:
> On Sun, Nov 20, 2022 at 11:58:26AM +0000, Mark Brown wrote:
> > On Sat, Nov 19, 2022 at 11:03:57PM +0000, Alexander Lobakin wrote:

> > Your mails appear to be encrypted which isn't helping with
> > review...

> https://lore.kernel.org/all/[email protected]/

> Looks un-encrypted on lore. pm.me looks to be a protonmail domain - I
> had issues with protonmail where it picked up Maz's key via Web Key
> Directory. "Kernel.org publishes the WKD for all developers who have
> kernel.org accounts" & unfortunately proton doesn't (or didn't) offer a
> way to disable this. If someone's key is available it gets used & proton
> told me, IIRC, that not having a way to disable this is a privacy
> feature.

It wasn't obviously encrypted to my key either...


Attachments:
(No filename) (857.00 B)
signature.asc (499.00 B)
Download all attachments

2022-11-22 22:08:38

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

From: Mark Brown <[email protected]>
Date: Sun, 20 Nov 2022 11:58:26 +0000

> On Sat, Nov 19, 2022 at 11:03:57PM +0000, Alexander Lobakin wrote:
>
> Your mails appear to be encrypted which isn't helping with
> review...

Oh I'm sorry. I gave ProtonMail one more chance. I had the same
issue with them at spring, they told me then that it's a super-pro
builtin feature that I can't disable :clownface: They promised to
"think on it" tho, so I thought maybe after half a year...
Nope. Ok, whatever. My workaround will be the same as Conor's, just
to change the provider lol.
Should be better now?

Thanks,
Olek

2022-11-23 00:32:45

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

From: Andy Shevchenko <[email protected]>
Date: Sun, 20 Nov 2022 15:55:21 +0200

> On Sat, Nov 19, 2022 at 11:08:17PM +0000, Alexander Lobakin wrote:
> > common.o is linked to both intel_skl_int3472_{discrete,tps68470}:
> >
> > > scripts/Makefile.build:252: ./drivers/platform/x86/intel/int3472/Makefile:
> > > common.o is added to multiple modules: intel_skl_int3472_discrete
> > > intel_skl_int3472_tps68470
> >
> > Although both drivers share one Kconfig option
> > (CONFIG_INTEL_SKL_INT3472), it's better to not link one object file
> > into several modules (and/or vmlinux).
> > Under certain circumstances, such can lead to the situation fixed by
> > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> >
> > Introduce the new module, intel_skl_int3472_common, to provide the
> > functions from common.o to both discrete and tps68470 drivers. This
> > adds only 3 exports and doesn't provide any changes to the actual
> > code.
>
> ...
>
> > +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> > +
>
> Redundant blank line. You may put it to be last MODULE_*() in the file, if you
> think it would be more visible.

My intention was that it's not "standard" module info like license
or description, rather something like exports or initcalls, that's
why I did separate them.
But I haven't been using module namespaces a lot previously, so if
it should be in one block with the rest MODULE_*(), sure, I'll fix.

>
> > MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Discrete Device Driver");
> > MODULE_AUTHOR("Daniel Scally <[email protected]>");
> > MODULE_LICENSE("GPL v2");
>
> ...
>
> > +MODULE_IMPORT_NS(INTEL_SKL_INT3472);
> > +
> > MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI TPS68470 Device Driver");
> > MODULE_AUTHOR("Daniel Scally <[email protected]>");
> > MODULE_LICENSE("GPL v2");
>
> Ditto. And the same to all your patches.
>
> --
> With Best Regards,
> Andy Shevchenko

Thanks,
Olek

2022-11-23 13:28:48

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

On Tue, Nov 22, 2022 at 10:37:54PM +0100, Alexander Lobakin wrote:
> From: Mark Brown <[email protected]>
> > On Sat, Nov 19, 2022 at 11:03:57PM +0000, Alexander Lobakin wrote:

> > Your mails appear to be encrypted which isn't helping with
> > review...

> Oh I'm sorry. I gave ProtonMail one more chance. I had the same
> issue with them at spring, they told me then that it's a super-pro
> builtin feature that I can't disable :clownface: They promised to
> "think on it" tho, so I thought maybe after half a year...
> Nope. Ok, whatever. My workaround will be the same as Conor's, just
> to change the provider lol.
> Should be better now?

Yes, everything's fine now - thanks for looking into it.


Attachments:
(No filename) (719.00 B)
signature.asc (499.00 B)
Download all attachments

2022-11-23 17:01:36

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 09/18] net: emac, cpsw: fix mixed module-builtin object (davinci_cpdma)

On Sun, Nov 20, 2022 at 8:07 AM Alexander Lobakin <[email protected]> wrote:
>
> From: Masahiro Yamada <[email protected]>
>
> CONFIG_TI_DAVINCI_EMAC, CONFIG_TI_CPSW and CONFIG_TI_CPSW_SWITCHDEV
> are all tristate. This means that davinci_cpdma.o can be linked to
> a module and also to vmlinux even though the expected CFLAGS are
> different between builtins and modules.
>
> This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
> Fixing mixed module-builtin objects").
>
> Introduce the new module, ti_davinci_cpdma, to provide the common
> functions to these three modules.
>
> [ alobakin: add exports ]
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Alexander Lobakin <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>
> ---

Please take the authorship for this patch
because I did not finish this patch
(and I am not sure if this is the correct way to fix)

As 18/18 will touch this part again,
perhaps davinci_cpdma.c can go into ti_cpsw_core.ko

Anyway, the maintainer may have a better insight.



--
Best Regards
Masahiro Yamada

2022-11-23 17:21:16

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 10/18] EDAC: i10nm, skx: fix mixed module-builtin object

On Sun, Nov 20, 2022 at 8:08 AM Alexander Lobakin <[email protected]> wrote:
>
> With CONFIG_EDAC_SKX=m and CONFIG_EDAC_I10NM=y (or vice versa),
> skx_common.o are linked to a module and also to vmlinux even though
> the expected CFLAGS are different between builtins and modules:
>
> > scripts/Makefile.build:252: ./drivers/edac/Makefile: skx_common.o
> > is added to multiple modules: i10nm_edac skx_edac
>
> This is the same situation as fixed by commit 637a642f5ca5 ("zstd:
> Fixing mixed module-builtin objects").
>
> Introduce the new module, skx_edac_common, to provide the common
> functions to skx_edac and i10nm_edac. skx_adxl_{get,put}() loose
> their __init/__exit annotations in order to become exportable.
>
> Fixes: d4dc89d069aa ("EDAC, i10nm: Add a driver for Intel 10nm server processors")
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>

Reviewed-by: Masahiro Yamada <[email protected]>





--
Best Regards
Masahiro Yamada

2022-11-23 21:16:49

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 15/18] net: dpaa2: fix mixed module-builtin object

On Sun, Nov 20, 2022 at 8:09 AM Alexander Lobakin <[email protected]> wrote:
>
> With CONFIG_FSL_DPAA2_ETH=m and CONFIG_FSL_DPAA2_SWITCH=y (or vice
> versa), dpaa2-mac.o and dpmac.o are linked to a module and also to
> vmlinux even though the expected CFLAGS are different between
> builtins and modules.
> This is the same situation as fixed by
> commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> There's also no need to duplicate relatively big piece of object
> code into two modules.
>
> Introduce the new module, fsl-dpaa2-mac, to provide the common
> functions to both fsl-dpaa2-eth and fsl-dpaa2-switch.
>
> Misc: constify and shrink @dpaa2_mac_ethtool_stats while at it.
>
> Fixes: 84cba72956fd ("dpaa2-switch: integrate the MAC endpoint support")
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Alexander Lobakin <[email protected]>
> ---

Reviewed-by: Masahiro Yamada <[email protected]>



--
Best Regards
Masahiro Yamada

2022-11-23 21:39:15

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

From: Hans de Goede <[email protected]>
Date: Mon, 21 Nov 2022 09:12:41 +0100

> Hi,
>
> On 11/21/22 00:45, Masahiro Yamada wrote:
> > On Mon, Nov 21, 2022 at 5:55 AM Hans de Goede <[email protected]> wrote:

[...]

> >> So nack from me for this patch, since I really don't see
> >> it adding any value.
> >
> >
> >
> >
> > This does have a value.
> >
> > This clarifies the ownership of the common.o,
> > in other words, makes KBUILD_MODNAME deterministic.
> >
> >
> > If an object belongs to a module,
> > KBUILD_MODNAME is defined as the module name.
> >
> > If an object is always built-in,
> > KBUILD_MODNAME is defined as the basename of the object.
> >
> >
> >
> > Here is a question:
> > if common.o is shared by two modules intel_skl_int3472_discrete
> > and intel_skl_int3472_tps68470, what should KBUILD_MODNAME be?
> >
> >
> > I see some patch submissions relying on the assumption that
> > KBUILD_MODNAME is unique.
> > We cannot determine KBUILD_MODNAME correctly if an object is shared
> > by multiple modules.

+1

> >
> >
> >
> >
> >
> >
> > BTW, this patch is not the way I suggested.
> > The Suggested-by should not have been there
> > (or at least Reported-by)

(to Masahiro)

Ah, you're right, sorry. Will replace all the tags to Reported-by,
that would look more correct.

> >
> >
> > You argued "common.o is tiny", so I would vote for
> > making them inline functions, like
> >
> >
> > https://lore.kernel.org/linux-kbuild/[email protected]/T/#u
>
> Yes just moving the contents of common.c to static inline helpers in common.h
> would be much better.

Sure, why not. There probably are more of shared object files which
would feel better being static inlines in the series, will see.

>
> If someone creates such a patch, please do not forget to Cc
> [email protected]

Got it, sure. get_maintainer.pl dropped a wall on me, so I was
picking stuff manually from it and missed that one.

>
> Regards,
>
> Hans

[...]

> >>> Ditto. And the same to all your patches.

Thanks,
Olek

2022-11-23 21:43:14

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

From: Alexander Lobakin <[email protected]>

From: Colin Foster <[email protected]>
Date: Mon, 21 Nov 2022 10:12:59 -0800

> On Mon, Nov 21, 2022 at 07:55:04PM +0200, Vladimir Oltean wrote:
> > On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
> > > With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
> > > (or vice versa), felix.o are linked to a module and also to vmlinux
> > > even though the expected CFLAGS are different between builtins and
> > > modules.
> > > This is the same situation as fixed by
> > > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> > > There's also no need to duplicate relatively big piece of object
> > > code into two modules.
> > >
> > > Introduce the new module, mscc_core, to provide the common functions
> > > to both mscc_felix and mscc_seville.
> > >
> > > Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
> > > Suggested-by: Masahiro Yamada <[email protected]>
> > > Signed-off-by: Alexander Lobakin <[email protected]>
> > > ---
> >
> > I don't disagree with the patch, but I dislike the name chosen.
> > How about NET_DSA_OCELOT_LIB and mscc_ocelot_dsa_lib.o? The "core" of
> > the hardware support is arguably mscc_ocelot_switch_lib.o, I don't think
> > it would be good to use that word here, since the code you're moving is
> > no more than a thin glue layer with some DSA specific code.

Yes, sure. I'm totally open for renaming stuff -- usually I barely
touch most of the code from the series, so the names can make no
sense at all. _dsa_lib sounds good, I like it.

> >
> > Adding Colin for a second opinion on the naming. I'm sure things could
> > have been done better in the first place, just not sure how.
>
> Good catch on this patch. "mscc_ocelot_dsa_lib" makes sense. The only
> other option that might be considered would be along the lines of
> "felix_lib". While I know "Felix" is the chip, in the dsa directory it
> seems to represent the DSA lib in general.

The thing confused me is that one of the platforms is named Felix
and the other is Seville, but the file they share is also felix,
although Seville has no references to the name "felix" AFAICS :D
I thought maybe Felix is a family and Seville is a chip from this
family, dunno.

>
> Either one seems fine for me. And thanks for the heads up, as I'll need
> to make the same changes for ocelot_ext when it is ready.
>

Ooh, something interesting is coming, nice <.<

Thanks,
Olek

2022-11-23 22:00:42

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

On Sun, Nov 20, 2022 at 8:04 AM Alexander Lobakin <[email protected]> wrote:
>
> This is a follow-up to the series[0] that adds Kbuild warning if an
> object is linked into several modules (including vmlinux) in order
> to prevent hidden side effects from appearing.
> The original series, as well as this one, was inspired by the recent
> issue[1] with the ZSTD modules on a platform which has such sets of
> vmlinux cflags and module cflags so that objects built with those
> two even refuse to link with each other.
> The final goal is to forbid linking one object several times
> entirely.
>
> Patches 1-7 and 10-11 was runtime-tested by me. Pathes 8-9 and 12-18
> are compile-time tested only (compile, link, modpost), so I
> encourage the maintainers to review them carefully. At least the
> last one, for cpsw, most likely has issues :D
> Masahiro's patches are taken from his WIP tree[2], with the two last
> finished by me.
>
> This mostly is a monotonic work, all scores go to Masahiro and
> Alexey :P
>
> [0] https://lore.kernel.org/linux-kbuild/[email protected]
> [1] https://github.com/torvalds/linux/commit/637a642f5ca5e850186bb64ac75ebb0f124b458d
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/log/?h=tmp4
>
> Alexander Lobakin (9):
> EDAC: i10nm, skx: fix mixed module-builtin object
> platform/x86: int3472: fix object shared between several modules
> mtd: tests: fix object shared between several modules
> crypto: octeontx2: fix objects shared between several modules
> dsa: ocelot: fix mixed module-builtin object
> net: dpaa2: fix mixed module-builtin object
> net: hns3: fix mixed module-builtin object
> net: octeontx2: fix mixed module-builtin object
> net: cpsw: fix mixed module-builtin object
>
> Masahiro Yamada (9):
> block/rnbd: fix mixed module-builtin object
> drm/bridge: imx: fix mixed module-builtin object
> drm/bridge: imx: turn imx8{qm,qxp}-ldb into single-object modules
> sound: fix mixed module-builtin object
> mfd: rsmu: fix mixed module-builtin object
> mfd: rsmu: turn rsmu-{core,i2c,spi} into single-object modules
> net: liquidio: fix mixed module-builtin object
> net: enetc: fix mixed module-builtin object
> net: emac, cpsw: fix mixed module-builtin object (davinci_cpdma)




Please split this series and send them to each subsystem.
You can grab authorship for 08, 09
since most of the efforts came from you.


Thanks for helping with this work!





--
Best Regards
Masahiro Yamada

2022-11-23 22:17:46

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 11/18] platform/x86: int3472: fix object shared between several modules

From: Hans de Goede <[email protected]>
Date: Mon, 21 Nov 2022 10:34:11 +0100

> Hi,
>
> On 11/21/22 10:06, Masahiro Yamada wrote:
> > On Mon, Nov 21, 2022 at 5:12 PM Hans de Goede <[email protected]> wrote:

[...]

> > I think this patch series should be split
> > and sent to each sub-system instead of kbuild.
>
> Yes definitely, the changes are big enough that not merging
> this through the subsystem trees is going to cause conflicts.

Makes sense! I'll collect the feedback here and then will be sending
stuff to the corresponding MLs with the reference to the original
commits which introduce Kbuild warnings.

>
> Regards,
>
> Hans

[...]

Thanks!
Olek

2022-11-23 22:26:44

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

From: Colin Foster <[email protected]>,
Date: Mon, 21 Nov 2022 10:12:59 -0800

> On Mon, Nov 21, 2022 at 07:55:04PM +0200, Vladimir Oltean wrote:
> > On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
> > > With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
> > > (or vice versa), felix.o are linked to a module and also to vmlinux
> > > even though the expected CFLAGS are different between builtins and
> > > modules.
> > > This is the same situation as fixed by
> > > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
> > > There's also no need to duplicate relatively big piece of object
> > > code into two modules.
> > >
> > > Introduce the new module, mscc_core, to provide the common functions
> > > to both mscc_felix and mscc_seville.
> > >
> > > Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
> > > Suggested-by: Masahiro Yamada <[email protected]>
> > > Signed-off-by: Alexander Lobakin <[email protected]>
> > > ---
> >
> > I don't disagree with the patch, but I dislike the name chosen.
> > How about NET_DSA_OCELOT_LIB and mscc_ocelot_dsa_lib.o? The "core" of
> > the hardware support is arguably mscc_ocelot_switch_lib.o, I don't think
> > it would be good to use that word here, since the code you're moving is
> > no more than a thin glue layer with some DSA specific code.

Sure. I usually barely work with the code touched by the series, so
often the names can make no sense -- I'm open for better namings
from the real developers.
_dsa_lib sounds good, I like it.

> >
> > Adding Colin for a second opinion on the naming. I'm sure things could
> > have been done better in the first place, just not sure how.
>
> Good catch on this patch. "mscc_ocelot_dsa_lib" makes sense. The only
> other option that might be considered would be along the lines of
> "felix_lib". While I know "Felix" is the chip, in the dsa directory it
> seems to represent the DSA lib in general.

The thing confused me is that one chip is named Felix and the other
one is Seville, but the shared code is named felix as well. So at
first I thought maybe Felix is a family of chips and Seville is a
chip from that family, dunno :D

>
> Either one seems fine for me. And thanks for the heads up, as I'll need
> to make the same changes for ocelot_ext when it is ready.

Something interesting is coming, nice <.<

(re "pls prefix with "net: dsa: ..."" -- roger that)

Thanks,
Olek

2022-11-23 22:31:31

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

From: Jakub Kicinski <[email protected]>
Date: Mon, 21 Nov 2022 11:50:34 -0800

> On Sat, 19 Nov 2022 23:03:57 +0000 Alexander Lobakin wrote:
> > Subject: [PATCH 00/18] treewide: fix object files shared between several modules
>
> Could you repost the networking changes separately in v2?
> I'm not seeing any reason why this would have to be a treewide
> series when merging.

Yes, it will be split on a per-subsys basis. So this mostly is an
RFC, must've prefixed it <.<

>
> > monotonic
>
> monotonous, unless you mean steady progress ;)

Oh thanks, it was "monotonous" actually :D

Olek

2022-11-23 22:41:20

by Colin Foster

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

On Wed, Nov 23, 2022 at 10:47:46PM +0100, Alexander Lobakin wrote:
> From: Colin Foster <[email protected]>,
> Date: Mon, 21 Nov 2022 10:12:59 -0800
> > On Mon, Nov 21, 2022 at 07:55:04PM +0200, Vladimir Oltean wrote:
> > > On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
> > >
> > > Adding Colin for a second opinion on the naming. I'm sure things could
> > > have been done better in the first place, just not sure how.
> >
> > Good catch on this patch. "mscc_ocelot_dsa_lib" makes sense. The only
> > other option that might be considered would be along the lines of
> > "felix_lib". While I know "Felix" is the chip, in the dsa directory it
> > seems to represent the DSA lib in general.
>
> The thing confused me is that one chip is named Felix and the other
> one is Seville, but the shared code is named felix as well. So at
> first I thought maybe Felix is a family of chips and Seville is a
> chip from that family, dunno :D
>

Not important, but in case anyone is curious:

Ocelot is a family of switches. Linux support exists for the internal
MIPS on some of those devices. My understanding is the switching
hardware is licensed out to other chips that can be controlled
externally (e.g. PCIe). Felix was the first chip to do so with full
Linux support. When Seville came along, it utilized a lot of common
code from Felix. Thus, Felix is a "chip" as well as a "library" -
specifically the DSA implementation of Ocelot. At least in my mind.

(Note: I haven't verified this timeline back to the early days of
Felix... I'm mostly speculating)

> >
> > Either one seems fine for me. And thanks for the heads up, as I'll need
> > to make the same changes for ocelot_ext when it is ready.
>
> Something interesting is coming, nice <.<

Interesting to a very select group of people :-) The Ocelot chips can be
controlled externally. 6.1 has basic support for these chips -
essentially an expensive GPIO expander. Adding support for half of the
ports is phase 2, and I need to sit down for another day or two to finish
things up before that can happen. Hopefully very soon, as my calendar is
finally freeing up... Still going for 6.2!

>
> (re "pls prefix with "net: dsa: ..."" -- roger that)
>
> Thanks,
> Olek

2022-11-23 22:54:59

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

On Wed, Nov 23, 2022 at 02:18:02PM -0800, Colin Foster wrote:
> > The thing confused me is that one chip is named Felix and the other
> > one is Seville, but the shared code is named felix as well. So at
> > first I thought maybe Felix is a family of chips and Seville is a
> > chip from that family, dunno :D
>
> Not important, but in case anyone is curious:
>
> Ocelot is a family of switches. Linux support exists for the internal
> MIPS on some of those devices. My understanding is the switching
> hardware is licensed out to other chips that can be controlled
> externally (e.g. PCIe). Felix was the first chip to do so with full
> Linux support. When Seville came along, it utilized a lot of common
> code from Felix. Thus, Felix is a "chip" as well as a "library" -
> specifically the DSA implementation of Ocelot. At least in my mind.
>
> (Note: I haven't verified this timeline back to the early days of
> Felix... I'm mostly speculating)

I'm not sure marketing would agree that Ocelot, Felix, Seville are part
of the same "family". They're all Vitesse switch designs which share the
same core architecture, even if some are sold by other companies.

The Ocelot switchdev driver came first to Linux. The Felix switch was
very similar, except it was DSA and not switchdev. So when it got added,
Ocelot became the name of a library for sharing code between a switchdev
front-end and a DSA front-end, as well as the name of a driver proper.

The Seville hardware is actually much older than both Ocelot and Felix.
It comes from the same family as Serval. It's integrated into old
Freescale PowerPC SoCs. It only got Linux support late in its life,
when it became super easy to do it, basically after Felix paved the way.
When that happened, Felix also got split up into a library (for the DSA
aspects of interfacing with the ocelot library) and a driver proper.

Colin is now working on a switch which marketing really would say that
it's part of the Ocelot family. Except it's DSA, so it has to use the
Felix library.

Anyway, TL;DR: name of common code is given by the first supported
hardware, it's quite a common pattern really.

What's more interesting to me is the strange humour of somebody at
Vitesse (now Microchip) who gave the feline code names for these
switches (Ocelot, Serval, Jaguar). Felix is none other than Felix the Cat.

2023-02-10 17:32:19

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH 00/18] treewide: fix object files shared between several modules

From: Alexander Lobakin <[email protected]>
Date: Sat, 19 Nov 2022 23:03:57 +0000

> This is a follow-up to the series[0] that adds Kbuild warning if an
> object is linked into several modules (including vmlinux) in order
> to prevent hidden side effects from appearing.
> The original series, as well as this one, was inspired by the recent
> issue[1] with the ZSTD modules on a platform which has such sets of
> vmlinux cflags and module cflags so that objects built with those
> two even refuse to link with each other.
> The final goal is to forbid linking one object several times
> entirely.

Oh well,

Sorry for quite abandoning the series. A bit busy RN to work on the kernel
outside work. If someone wants to pick patches related to his driver and
send them separately, just how the Ocelot folks did, feel free to.
I'll get back to this one in approx 2-4 weeks.

>
> Patches 1-7 and 10-11 was runtime-tested by me. Pathes 8-9 and 12-18
> are compile-time tested only (compile, link, modpost), so I
> encourage the maintainers to review them carefully. At least the
> last one, for cpsw, most likely has issues :D
> Masahiro's patches are taken from his WIP tree[2], with the two last
> finished by me.

[...]

> --
> 2.38.1

Thanks,
Olek