diff mbox series

[3/3] spi: cs42l43: Add bridged cs35l56 amplifiers

Message ID 20240326141108.1079993-4-ckeepax@opensource.cirrus.com (mailing list archive)
State Superseded
Headers show
Series Add bridged amplifiers to cs42l43 | expand

Commit Message

Charles Keepax March 26, 2024, 2:11 p.m. UTC 
From: Maciej Strozek <mstrozek@opensource.cirrus.com>

On some cs42l43 systems a couple of cs35l56 amplifiers are attached
to the cs42l43's SPI and I2S. On Windows the cs42l43 is controlled
by a SDCA class driver and these two amplifiers are controlled by
firmware running on the cs42l43. However, under Linux the decision
was made to interact with the cs42l43 directly, affording the user
greater control over the audio system. However, this has resulted
in an issue where these two bridged cs35l56 amplifiers are not
populated in ACPI and must be added manually.

Check for the presence of the "01fa-cirrus-sidecar-instances" property
in the SDCA extension unit's ACPI properties to confirm the presence
of these two amplifiers and if they exist add them manually onto the
SPI bus.

Signed-off-by: Maciej Strozek <mstrozek@opensource.cirrus.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/spi/spi-cs42l43.c | 152 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 151 insertions(+), 1 deletion(-)

Comments

kernel test robot March 27, 2024, 6:34 p.m. UTC | #1 
Hi Charles,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on brgl/gpio/for-next linus/master v6.9-rc1 next-20240327]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Charles-Keepax/gpio-swnode-Add-ability-to-specify-native-chip-selects-for-SPI/20240326-221422
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20240326141108.1079993-4-ckeepax%40opensource.cirrus.com
patch subject: [PATCH 3/3] spi: cs42l43: Add bridged cs35l56 amplifiers
config: sh-randconfig-r121-20240327 (https://download.01.org/0day-ci/archive/20240328/202403280258.rjWikSAr-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240328/202403280258.rjWikSAr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403280258.rjWikSAr-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/spi/spi-cs42l43.c:48:28: sparse: sparse: symbol 'ampr' was not declared. Should it be static?
>> drivers/spi/spi-cs42l43.c:52:23: sparse: sparse: symbol 'ampl_info' was not declared. Should it be static?
>> drivers/spi/spi-cs42l43.c:61:23: sparse: sparse: symbol 'ampr_info' was not declared. Should it be static?
   drivers/spi/spi-cs42l43.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/slab.h, ...):
   include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false

vim +/ampr +48 drivers/spi/spi-cs42l43.c

    47	
  > 48	const struct software_node ampr = {
    49		.name			= "cs35l56-right",
    50	};
    51	
  > 52	struct spi_board_info ampl_info = {
    53		.modalias		= "cs35l56",
    54		.max_speed_hz		= 2000000,
    55		.chip_select		= 0,
    56		.mode			= SPI_MODE_0,
    57		.swnode			= &ampl,
    58		.use_fwnode_name	= true,
    59	};
    60	
  > 61	struct spi_board_info ampr_info = {
    62		.modalias		= "cs35l56",
    63		.max_speed_hz		= 2000000,
    64		.chip_select		= 1,
    65		.mode			= SPI_MODE_0,
    66		.swnode			= &ampr,
    67		.use_fwnode_name	= true,
    68	};
    69
kernel test robot March 28, 2024, 1:15 p.m. UTC | #2 
Hi Charles,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on brgl/gpio/for-next linus/master v6.9-rc1 next-20240328]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Charles-Keepax/gpio-swnode-Add-ability-to-specify-native-chip-selects-for-SPI/20240326-221422
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20240326141108.1079993-4-ckeepax%40opensource.cirrus.com
patch subject: [PATCH 3/3] spi: cs42l43: Add bridged cs35l56 amplifiers
config: sh-randconfig-r121-20240327 (https://download.01.org/0day-ci/archive/20240328/202403282033.oFvCtRT7-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240328/202403282033.oFvCtRT7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403282033.oFvCtRT7-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/spi/spi-cs42l43.c:48:28: sparse: sparse: symbol 'ampr' was not declared. Should it be static?
>> drivers/spi/spi-cs42l43.c:52:23: sparse: sparse: symbol 'ampl_info' was not declared. Should it be static?
>> drivers/spi/spi-cs42l43.c:61:23: sparse: sparse: symbol 'ampr_info' was not declared. Should it be static?
   drivers/spi/spi-cs42l43.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/slab.h, ...):
   include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false

vim +/ampr +48 drivers/spi/spi-cs42l43.c

    47	
  > 48	const struct software_node ampr = {
    49		.name			= "cs35l56-right",
    50	};
    51	
  > 52	struct spi_board_info ampl_info = {
    53		.modalias		= "cs35l56",
    54		.max_speed_hz		= 2000000,
    55		.chip_select		= 0,
    56		.mode			= SPI_MODE_0,
    57		.swnode			= &ampl,
    58		.use_fwnode_name	= true,
    59	};
    60	
  > 61	struct spi_board_info ampr_info = {
    62		.modalias		= "cs35l56",
    63		.max_speed_hz		= 2000000,
    64		.chip_select		= 1,
    65		.mode			= SPI_MODE_0,
    66		.swnode			= &ampr,
    67		.use_fwnode_name	= true,
    68	};
    69
diff mbox series

Patch

diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
index aabef9fc84bd..3620d4059034 100644
--- a/drivers/spi/spi-cs42l43.c
+++ b/drivers/spi/spi-cs42l43.c
@@ -5,6 +5,8 @@ 
 // Copyright (C) 2022-2023 Cirrus Logic, Inc. and
 //                         Cirrus Logic International Semiconductor Ltd.
 
+#include <dt-bindings/gpio/gpio.h>
+#include <linux/acpi.h>
 #include <linux/bits.h>
 #include <linux/bitfield.h>
 #include <linux/device.h>
@@ -39,6 +41,59 @@  static const unsigned int cs42l43_clock_divs[] = {
 	2, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30
 };
 
+const struct software_node ampl = {
+	.name			= "cs35l56-left",
+};
+
+const struct software_node ampr = {
+	.name			= "cs35l56-right",
+};
+
+struct spi_board_info ampl_info = {
+	.modalias		= "cs35l56",
+	.max_speed_hz		= 2000000,
+	.chip_select		= 0,
+	.mode			= SPI_MODE_0,
+	.swnode			= &ampl,
+	.use_fwnode_name	= true,
+};
+
+struct spi_board_info ampr_info = {
+	.modalias		= "cs35l56",
+	.max_speed_hz		= 2000000,
+	.chip_select		= 1,
+	.mode			= SPI_MODE_0,
+	.swnode			= &ampr,
+	.use_fwnode_name	= true,
+};
+
+static const struct software_node cs42l43_gpiochip_swnode = {
+	.name			= "cs42l43-pinctrl",
+};
+
+static const struct software_node_ref_args cs42l43_cs_refs[] = {
+	{
+		.node		= &cs42l43_gpiochip_swnode,
+		.nargs		= 2,
+		.args		= { 0, GPIO_ACTIVE_LOW },
+	},
+	{
+		.node		= &swnode_gpio_undefined,
+		.nargs		= 0,
+	},
+};
+
+static const struct property_entry cs42l43_cs_props[] = {
+	{
+		.name		= "cs-gpios",
+		.length		= ARRAY_SIZE(cs42l43_cs_refs) *
+				  sizeof(struct software_node_ref_args),
+		.type		= DEV_PROP_REF,
+		.pointer	= cs42l43_cs_refs,
+	},
+	{}
+};
+
 static int cs42l43_spi_tx(struct regmap *regmap, const u8 *buf, unsigned int len)
 {
 	const u8 *end = buf + len;
@@ -203,6 +258,51 @@  static size_t cs42l43_spi_max_length(struct spi_device *spi)
 	return CS42L43_SPI_MAX_LENGTH;
 }
 
+#if IS_ENABLED(CONFIG_ACPI)
+static bool cs42l43_has_sidecar(struct fwnode_handle *fwnode)
+{
+	static const int func_smart_amp = 0x1;
+	struct fwnode_handle *child_fwnode, *ext_fwnode;
+	unsigned long long function;
+	unsigned int val;
+	int ret;
+
+	if (!is_acpi_node(fwnode))
+		return false;
+
+	fwnode_for_each_child_node(fwnode, child_fwnode) {
+		struct acpi_device *adev = to_acpi_device_node(child_fwnode);
+
+		ret = acpi_evaluate_integer(adev->handle, "_ADR", NULL, &function);
+		if (ACPI_FAILURE(ret))
+			continue;
+
+		if (function != func_smart_amp)
+			continue;
+
+		ext_fwnode = fwnode_get_named_child_node(child_fwnode,
+				"mipi-sdca-function-expansion-subproperties");
+		if (!ext_fwnode)
+			continue;
+
+		ret = fwnode_property_read_u32(ext_fwnode,
+					       "01fa-cirrus-sidecar-instances",
+					       &val);
+		if (ret)
+			continue;
+
+		return !!val;
+	}
+
+	return false;
+}
+#else
+static bool cs42l43_has_sidecar(struct fwnode_handle *fwnode)
+{
+	return false;
+}
+#endif
+
 static void cs42l43_release_of_node(void *data)
 {
 	fwnode_handle_put(data);
@@ -213,6 +313,7 @@  static int cs42l43_spi_probe(struct platform_device *pdev)
 	struct cs42l43 *cs42l43 = dev_get_drvdata(pdev->dev.parent);
 	struct cs42l43_spi *priv;
 	struct fwnode_handle *fwnode = dev_fwnode(cs42l43->dev);
+	bool has_sidecar = cs42l43_has_sidecar(fwnode);
 	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -266,16 +367,64 @@  static int cs42l43_spi_probe(struct platform_device *pdev)
 		}
 	}
 
-	device_set_node(&priv->ctlr->dev, fwnode);
+	if (has_sidecar) {
+		ret = software_node_register(&cs42l43_gpiochip_swnode);
+		if (ret) {
+			dev_err(priv->dev, "Failed to register gpio swnode: %d\n", ret);
+			return ret;
+		}
+
+		ret = device_create_managed_software_node(&priv->ctlr->dev, cs42l43_cs_props, NULL);
+		if (ret) {
+			dev_err(priv->dev, "Failed to add swnode: %d\n", ret);
+			goto err;
+		}
+
+	} else {
+		device_set_node(&priv->ctlr->dev, fwnode);
+	}
 
 	ret = devm_spi_register_controller(priv->dev, priv->ctlr);
 	if (ret) {
 		dev_err(priv->dev, "Failed to register SPI controller: %d\n", ret);
+		goto err;
 	}
 
+	if (has_sidecar) {
+		if (!spi_new_device(priv->ctlr, &ampl_info)) {
+			ret = -ENODEV;
+			dev_err(priv->dev, "Failed to create left amp slave\n");
+			goto err;
+		}
+
+		if (!spi_new_device(priv->ctlr, &ampr_info)) {
+			ret = -ENODEV;
+			dev_err(priv->dev, "Failed to create right amp slave\n");
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	if (has_sidecar)
+		software_node_unregister(&cs42l43_gpiochip_swnode);
+
 	return ret;
 }
 
+static int cs42l43_spi_remove(struct platform_device *pdev)
+{
+	struct cs42l43 *cs42l43 = dev_get_drvdata(pdev->dev.parent);
+	struct fwnode_handle *fwnode = dev_fwnode(cs42l43->dev);
+	bool has_sidecar = cs42l43_has_sidecar(fwnode);
+
+	if (has_sidecar)
+		software_node_unregister(&cs42l43_gpiochip_swnode);
+
+	return 0;
+};
+
 static const struct platform_device_id cs42l43_spi_id_table[] = {
 	{ "cs42l43-spi", },
 	{}
@@ -288,6 +437,7 @@  static struct platform_driver cs42l43_spi_driver = {
 	},
 	.probe		= cs42l43_spi_probe,
 	.id_table	= cs42l43_spi_id_table,
+	.remove		= cs42l43_spi_remove,
 };
 module_platform_driver(cs42l43_spi_driver);