Support Spi in i2c-multi-instantiate driver
Subsystems (4)
v4 cover Support Spi in i2c-multi-instantiate driver
Add support for SPI bus in the i2c-multi-instantiate driver as upcoming laptops will need to multi instantiate SPI devices from a single device node, which has multiple SpiSerialBus entries at the ACPI table.
With the new SPI support, i2c-multi-instantiate becomes bus-multi-instantiate and is moved to the ACPI folder.
The intention is to support the SPI bus by re-using the current I2C multi instantiate, instead of creating a new SPI multi instantiate, to make it possible for peripherals that can be controlled by I2C or SPI to have the same HID at the ACPI table.
The new driver (Bus multi instantiate, bmi) checks for the hard-coded bus type and returns -ENODEV in case of zero devices found for that bus. In the case of automatic bus detection, the driver will give preference to I2C.
The expectation is for a device node in the ACPI table to have multiple I2cSerialBus only or multiple SpiSerialBus only, not a mix of both; and for the case where there are both entries in one device node, only the I2C ones would be probed.
This new bus multi instantiate will be used in CS35L41 HDA new driver.
Changes since V2:
- Moved bus-multi-instantiate back into platform/x86
Lucas Tanure (4):
platform/x86: i2c-multi-instantiate: Rename it for a generic bus
driver name
platform/x86: bus-multi-instantiate: Reorganize I2C functions
ALSA: hda/realtek: Add support for HP Laptops
ACPI / scan: Create platform device for CS35L41
Stefan Binding (5):
spi: Make spi_alloc_device and spi_add_device public again
spi: Create helper API to lookup ACPI info for spi device
spi: Support selection of the index of the ACPI Spi Resource before
alloc
spi: Add API to count spi acpi resources
platform/x86: bus-multi-instantiate: Add SPI support
MAINTAINERS | 4 +- drivers/acpi/scan.c | 16 +- drivers/platform/x86/Kconfig | 14 +- drivers/platform/x86/Makefile | 2 +- drivers/platform/x86/bus-multi-instantiate.c | 369 +++++++++++++++++++ drivers/platform/x86/i2c-multi-instantiate.c | 174 --------- drivers/spi/spi.c | 142 ++++++- include/linux/spi/spi.h | 32 ++ sound/pci/hda/patch_realtek.c | 43 ++- 9 files changed, 588 insertions(+), 208 deletions(-) create mode 100644 drivers/platform/x86/bus-multi-instantiate.c delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c
Signature · 1 line
2.25.1
PATCH v4 1/9 spi: Make spi_alloc_device and spi_add_device public again
This functions were previously made private since they were not used. However, these functions will be needed again.
Partial revert of commit da21fde0fdb3
("spi: Make several public functions private to spi.c")
--- drivers/spi/spi.c | 6 ++++-- include/linux/spi/spi.h | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-)
drivers/spi/spi.c +4 -2
@@ -532,7 +532,7 @@ static DEFINE_MUTEX(board_lock);* * Return: a pointer to the new device, or NULL. */ -static struct spi_device *spi_alloc_device (struct spi_controller *ctlr) +struct spi_device *spi_alloc_device (struct spi_controller *ctlr) {struct spi_device *spi;
@@ -557,6 +557,7 @@ static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)device_initialize (&spi->dev);return spi; } +EXPORT_SYMBOL_GPL (spi_alloc_device);static void spi_dev_set_name (struct spi_device *spi) {
@@ -652,7 +653,7 @@ static int __spi_add_device(struct spi_device *spi)* * Return: 0 on success; negative errno on failure */ -static int spi_add_device (struct spi_device *spi) +int spi_add_device (struct spi_device *spi) {struct spi_controller *ctlr = spi->controller;struct device *dev = ctlr->dev.parent;
@@ -673,6 +674,7 @@ static int spi_add_device(struct spi_device *spi)mutex_unlock (&ctlr->add_lock);return status; } +EXPORT_SYMBOL_GPL (spi_add_device);static int spi_add_device_locked (struct spi_device *spi) {
include/linux/spi/spi.h +12 -0
@@ -1452,7 +1452,19 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)* use spi_new_device() to describe each device. You can also call * spi_unregister_device() to start making that device vanish, but * normally that would be handled by spi_unregister_controller(). +* +* You can also use spi_alloc_device() and spi_add_device() to use a two +* stage registration sequence for each spi_device. This gives the caller +* some more control over the spi_device structure before it is registered, +* but requires that caller to initialize fields that would otherwise +* be defined using the board info. */ +extern struct spi_device * +spi_alloc_device (struct spi_controller *ctlr); + +extern int +spi_add_device (struct spi_device *spi); +extern struct spi_device *spi_new_device (struct spi_controller *,struct spi_board_info *);
Signature · 1 line
2.25.1
Re: [PATCH v4 1/9] spi: Make spi_alloc_device and spi_add_device public again
Hi,
Quoted 8 lines · Stefan Binding
This functions were previously made private since they were not used. However, these functions will be needed again.
Partial revert of commit
da21fde0fdb3
("spi: Make several public functions private tospi.c")Signed-off-by Stefan Binding <[email protected]>
Thanks, patch looks good to me:
Regards,
Hans
Quoted 68 lines
--- drivers/spi/spi.c | 6 ++++-- include/linux/spi/spi.h | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-)drivers/spi/spi.c +4
-2@@ -532,7 +532,7 @@ static DEFINE_MUTEX(board_lock);* * Return: a pointer to the new device, or NULL. */ -static struct spi_device *spi_alloc_device (struct spi_controller *ctlr) +struct spi_device *spi_alloc_device (struct spi_controller *ctlr) {struct spi_device *spi;@@ -557,6 +557,7 @@ static struct spi_device *spi_alloc_device(struct spi_controller *ctlr)device_initialize (&spi->dev);return spi; } +EXPORT_SYMBOL_GPL (spi_alloc_device);static void spi_dev_set_name (struct spi_device *spi) {@@ -652,7 +653,7 @@ static int __spi_add_device(struct spi_device *spi)* * Return: 0 on success; negative errno on failure */ -static int spi_add_device (struct spi_device *spi) +int spi_add_device (struct spi_device *spi) {struct spi_controller *ctlr = spi->controller;struct device *dev = ctlr->dev.parent;@@ -673,6 +674,7 @@ static int spi_add_device(struct spi_device *spi)mutex_unlock (&ctlr->add_lock);return status; } +EXPORT_SYMBOL_GPL (spi_add_device);static int spi_add_device_locked (struct spi_device *spi) {include/linux/spi/spi.h +12
-0@@ -1452,7 +1452,19 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)* use spi_new_device() to describe each device. You can also call * spi_unregister_device() to start making that device vanish, but * normally that would be handled by spi_unregister_controller(). +* +* You can also use spi_alloc_device() and spi_add_device() to use a two +* stage registration sequence for each spi_device. This gives the caller +* some more control over the spi_device structure before it is registered, +* but requires that caller to initialize fields that would otherwise +* be defined using the board info. */ +extern struct spi_device * +spi_alloc_device (struct spi_controller *ctlr); + +extern int +spi_add_device (struct spi_device *spi); +extern struct spi_device *spi_new_device (struct spi_controller *,struct spi_board_info *);
PATCH v4 2/9 spi: Create helper API to lookup ACPI info for spi device
In reply to Stefan BindingThis can then be used to find a spi resource inside an ACPI node, and allocate a spi device.
--- drivers/spi/spi.c | 46 ++++++++++++++++++++++++++++++++--------- include/linux/spi/spi.h | 12 +++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-)
drivers/spi/spi.c +36 -10
@@ -2410,8 +2410,18 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)return 1 ; } -static acpi_status acpi_register_spi_device (struct spi_controller *ctlr, -struct acpi_device *adev) +/** +* acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information +* @ctlr: controller to which the spi device belongs +* @adev: ACPI Device for the spi device +* +* This should be used to allocate a new spi device from and ACPI Node. +* The caller is responsible for calling spi_add_device to register the spi device. +* +* Return: a pointer to the new device, or ERR_PTR on error. +*/ +struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, +struct acpi_device *adev) {acpi_handle parent_handle =NULL ;struct list_head resource_list;
@@ -2419,10 +2429,6 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,struct spi_device *spi;int ret; -if (acpi_bus_get_status (adev) || !adev->status.present || -acpi_device_enumerated (adev)) -return AE_OK ; - lookup.ctlr = ctlr; lookup.irq =-1 ;
@@ -2433,7 +2439,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,if (ret <0 )/* found SPI in _CRS but it points to another controller */ -return AE_OK ; +return ERR_PTR (-ENODEV ); if (!lookup.max_speed_hz && ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle)) &&
@@ -2443,16 +2449,15 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, }if (!lookup.max_speed_hz) -return AE_OK ; +return ERR_PTR (-ENODEV ); spi =spi_alloc_device (ctlr);if (!spi) {dev_err (&ctlr->dev,"failed to allocate SPI device for %s\n" ,dev_name (&adev->dev)); -return AE_NO_MEMORY ; +return ERR_PTR (-ENOMEM ); } -ACPI_COMPANION_SET (&spi->dev, adev); spi->max_speed_hz = lookup.max_speed_hz; spi->mode |= lookup.mode;
@@ -2460,6 +2465,27 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, spi->bits_per_word = lookup.bits_per_word; spi->chip_select = lookup.chip_select; +return spi; +} +EXPORT_SYMBOL_GPL (acpi_spi_device_alloc); + +static acpi_status acpi_register_spi_device (struct spi_controller *ctlr, +struct acpi_device *adev) +{ +struct spi_device *spi; + +if (acpi_bus_get_status (adev) || !adev->status.present || +acpi_device_enumerated (adev)) +return AE_OK ; + + spi =acpi_spi_device_alloc (ctlr, adev); +if (IS_ERR (spi)) { +if (PTR_ERR (spi) == -ENOMEM ) +return AE_NO_MEMORY ; +else +return AE_OK ; + } +acpi_set_modalias (adev,acpi_device_hid (adev), spi->modalias,sizeof (spi->modalias));
include/linux/spi/spi.h +12 -0
@@ -16,6 +16,7 @@#include <linux/gpio/consumer.h> #include <uapi/linux/spi/spi.h> +#include <linux/acpi.h> struct dma_chan ;struct software_node ;
@@ -759,6 +760,17 @@ extern int devm_spi_register_controller(struct device *dev, struct spi_controller *ctlr);extern void spi_unregister_controller (struct spi_controller *ctlr); +#if IS_ENABLED (CONFIG_ACPI ) +extern struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, +struct acpi_device *adev); +#else +static inline struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, +struct acpi_device *adev); +{ +return ERR_PTR (-EOPNOTSUPP ); +} +#endif +/* * SPI resource management while processing a SPI message */
Signature · 1 line
2.25.1
Re: [PATCH v4 2/9] spi: Create helper API to lookup ACPI info for spi device
Hi Stefan,
First of all thank you for doing the requested refactoring to reuse the existing ACPI SPI enumeration code from drivers/spi/spi.c .
Quoted 118 lines · Stefan Binding
This can then be used to find a spi resource inside an ACPI node, and allocate a spi device.
Signed-off-by Stefan Binding <[email protected]>--- drivers/spi/spi.c | 46 ++++++++++++++++++++++++++++++++--------- include/linux/spi/spi.h | 12 +++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-)drivers/spi/spi.c +36
-10@@ -2410,8 +2410,18 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)return 1 ; } -static acpi_status acpi_register_spi_device (struct spi_controller *ctlr, -struct acpi_device *adev) +/** +* acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information +* @ctlr: controller to which the spi device belongs +* @adev: ACPI Device for the spi device +* +* This should be used to allocate a new spi device from and ACPI Node. +* The caller is responsible for calling spi_add_device to register the spi device. +* +* Return: a pointer to the new device, or ERR_PTR on error. +*/ +struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, +struct acpi_device *adev) {acpi_handle parent_handle =NULL ;struct list_head resource_list;@@ -2419,10 +2429,6 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,struct spi_device *spi;int ret; -if (acpi_bus_get_status (adev) || !adev->status.present || -acpi_device_enumerated (adev)) -return AE_OK ; - lookup.ctlr = ctlr; lookup.irq =-1 ;@@ -2433,7 +2439,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr,if (ret <0 )/* found SPI in _CRS but it points to another controller */ -return AE_OK ; +return ERR_PTR (-ENODEV ); if (!lookup.max_speed_hz && ACPI_SUCCESS(acpi_get_parent(adev->handle, &parent_handle)) &&@@ -2443,16 +2449,15 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, }if (!lookup.max_speed_hz) -return AE_OK ; +return ERR_PTR (-ENODEV ); spi =spi_alloc_device (ctlr);if (!spi) {dev_err (&ctlr->dev,"failed to allocate SPI device for %s\n" ,dev_name (&adev->dev)); -return AE_NO_MEMORY ; +return ERR_PTR (-ENOMEM ); } -ACPI_COMPANION_SET (&spi->dev, adev); spi->max_speed_hz = lookup.max_speed_hz; spi->mode |= lookup.mode;@@ -2460,6 +2465,27 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, spi->bits_per_word = lookup.bits_per_word; spi->chip_select = lookup.chip_select; +return spi; +} +EXPORT_SYMBOL_GPL (acpi_spi_device_alloc); + +static acpi_status acpi_register_spi_device (struct spi_controller *ctlr, +struct acpi_device *adev) +{ +struct spi_device *spi; + +if (acpi_bus_get_status (adev) || !adev->status.present || +acpi_device_enumerated (adev)) +return AE_OK ; + + spi =acpi_spi_device_alloc (ctlr, adev); +if (IS_ERR (spi)) { +if (PTR_ERR (spi) == -ENOMEM ) +return AE_NO_MEMORY ; +else +return AE_OK ; + } +acpi_set_modalias (adev,acpi_device_hid (adev), spi->modalias,sizeof (spi->modalias));include/linux/spi/spi.h +2
-0@@ -16,6 +16,7 @@#include <linux/gpio/consumer.h> #include <uapi/linux/spi/spi.h> +#include <linux/acpi.h> struct dma_chan ;struct software_node ;@@ -759,6 +760,17 @@ extern int devm_spi_register_controller(struct device *dev, struct spi_controller *ctlr);extern void spi_unregister_controller (struct spi_controller *ctlr); +#if IS_ENABLED (CONFIG_ACPI )
include/linux/spi/spi.h extern int devm_spi_register_controller(struct device *dev, · line 773
Quoted 10 lines
include/linux/spi/spi.h +10
-0+extern struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, +struct acpi_device *adev); +#else +static inline struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, +struct acpi_device *adev); +{ +return ERR_PTR (-EOPNOTSUPP ); +} +#endif +
I don't think having a stub here is necessary ? I would expect any callers of this to make sure that both CONFIG_SPI and CONFIG_ACPI are set. Note that not having CONFIG_SPI set is not caught by your stubbing here and there are no stubs for any of the other spi functions.
Regards,
Hans
Quoted 4 lines
include/linux/spi/spi.h +0
-0/* * SPI resource management while processing a SPI message */
kernel test robot
build botRe: [PATCH v4 2/9] spi: Create helper API to lookup ACPI info for spi device
In reply to Stefan BindingThank you for the patch! Yet something to improve:
Hi Stefan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20220120]
[also build test ERROR on v5.16]
[cannot apply to broonie-spi/for-next rafael-pm/linux-next tiwai-sound/for-next linus/master v5.16 v5.16-rc8 v5.16-rc7] [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]
url: https://github.com/0day-ci/linux/commits/Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 base: 7fc5253f5a13271e9df35d6b936ff97b74540a59 config: arc-randconfig-r043-20220120 (https://download.01.org/0day-ci/archive/20220121/202201210121.LFkpILul-lkp(a)intel.com/config) compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/aedf84002ec1568907471591915e28bed40e8f9a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 git checkout aedf84002ec1568907471591915e28bed40e8f9a
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/media/i2c/
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
In file included from include/media/v4l2-common.h:270,
from include/media/v4l2-subdev.h:15,
from include/media/v4l2-device.h:13,
from drivers/media/i2c/tvaudio.c:40:
>> include/linux/spi/spi.h:769:1: error: expected identifier or '(' before '{' token
769 | {
| ^ include/linux/spi/spi.h:767:34: warning: 'acpi_spi_device_alloc' declared 'static' but never defined [-Wunused-function]
767 | static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
| ^~~~~~~~~~~~~~~~~~~~~
vim +769 include/linux/spi/spi.h
762
763 #if IS_ENABLED(CONFIG_ACPI)
764 extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
765 struct acpi_device *adev);
766 #else
767 static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
768 struct acpi_device *adev);
> 769 { 770 return ERR_PTR(-EOPNOTSUPP);
771 }
772 #endif
773
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [PATCH v4 2/9] spi: Create helper API to lookup ACPI info for spi device
In reply to Stefan BindingThank you for the patch! Perhaps something to improve:
Hi Stefan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20220120]
[also build test WARNING on v5.16]
[cannot apply to broonie-spi/for-next rafael-pm/linux-next tiwai-sound/for-next linus/master v5.16 v5.16-rc8 v5.16-rc7] [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]
url: https://github.com/0day-ci/linux/commits/Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 base: 7fc5253f5a13271e9df35d6b936ff97b74540a59 config: hexagon-randconfig-r045-20220120 (https://download.01.org/0day-ci/archive/20220121/202201210053.wHFMeXIN-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f7b7138a62648f4019c55e4671682af1f851f295)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/aedf84002ec1568907471591915e28bed40e8f9a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608
git checkout aedf84002ec1568907471591915e28bed40e8f9a
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/media/platform/mtk-mdp/ drivers/media/platform/s5p-jpeg/ drivers/media/platform/s5p-mfc/ drivers/media/platform/ti-vpe/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
In file included from drivers/media/platform/mtk-mdp/mtk_mdp_core.c:22:
In file included from drivers/media/platform/mtk-mdp/mtk_mdp_core.h:13:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
include/linux/spi/spi.h:769:1: error: expected identifier or '(' {
^
>> drivers/media/platform/mtk-mdp/mtk_mdp_core.c:207:52: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
ret = vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~ 1 warning and 1 error generated.
--
In file included from drivers/media/platform/s5p-jpeg/jpeg-core.c:31:
In file included from drivers/media/platform/s5p-jpeg/jpeg-core.h:14:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
include/linux/spi/spi.h:769:1: error: expected identifier or '(' {
^
>> drivers/media/platform/s5p-jpeg/jpeg-core.c:2913:46: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~ 1 warning and 1 error generated.
--
In file included from drivers/media/platform/s5p-mfc/s5p_mfc.c:24:
In file included from drivers/media/platform/s5p-mfc/s5p_mfc_common.h:18:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
include/linux/spi/spi.h:769:1: error: expected identifier or '(' {
^
>> drivers/media/platform/s5p-mfc/s5p_mfc.c:1169:6: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
DMA_BIT_MASK(32));
^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~
drivers/media/platform/s5p-mfc/s5p_mfc.c:1171:6: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
DMA_BIT_MASK(32));
^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~
drivers/media/platform/s5p-mfc/s5p_mfc.c:1227:39: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~ 3 warnings and 1 error generated.
--
In file included from drivers/media/platform/ti-vpe/cal.c:25:
In file included from include/media/v4l2-common.h:270:
include/linux/spi/spi.h:769:1: error: expected identifier or '(' {
^
>> drivers/media/platform/ti-vpe/cal.c:902:44: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
vb2_dma_contig_set_max_seg_size(cal->dev, DMA_BIT_MASK(32));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~1 warning and 1 error generated.
vim +207 drivers/media/platform/mtk-mdp/mtk_mdp_core.c
86698b9505bbc9 Eizan Miyamoto 2020-05-07 105
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 106 static int mtk_mdp_probe(struct platform_device *pdev)
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 107 {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 108 struct mtk_mdp_dev *mdp;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 109 struct device *dev = &pdev->dev;
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 110 struct device_node *node, *parent;
86698b9505bbc9 Eizan Miyamoto 2020-05-07 111 struct mtk_mdp_comp *comp, *comp_temp;
86698b9505bbc9 Eizan Miyamoto 2020-05-07 112 int ret = 0;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 113
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 114 mdp = devm_kzalloc(dev, sizeof(*mdp), GFP_KERNEL);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 115 if (!mdp)
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 116 return -ENOMEM;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 117
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 118 mdp->id = pdev->id;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 119 mdp->pdev = pdev;
86698b9505bbc9 Eizan Miyamoto 2020-05-07 120 INIT_LIST_HEAD(&mdp->comp_list);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 121 INIT_LIST_HEAD(&mdp->ctx_list);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 122
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 123 mutex_init(&mdp->lock);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 124 mutex_init(&mdp->vpulock);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 125
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 126 /* Old dts had the components as child nodes */
864919ea0380e6 Matthias Brugger 2019-06-21 127 node = of_get_next_child(dev->of_node, NULL);
864919ea0380e6 Matthias Brugger 2019-06-21 128 if (node) {
864919ea0380e6 Matthias Brugger 2019-06-21 129 of_node_put(node);
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 130 parent = dev->of_node;
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 131 dev_warn(dev, "device tree is out of date\n");
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 132 } else {
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 133 parent = dev->of_node->parent;
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 134 }
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 135
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 136 /* Iterate over sibling MDP function blocks */
ba1f1f70c2c08f Daniel Kurtz 2017-05-23 137 for_each_child_of_node(parent, node) {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 138 const struct of_device_id *of_id;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 139 enum mtk_mdp_comp_type comp_type;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 140
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 141 of_id = of_match_node(mtk_mdp_comp_dt_ids, node);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 142 if (!of_id)
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 143 continue;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 144
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 145 if (!of_device_is_available(node)) {
68d9c47b1679ec Rob Herring 2017-07-21 146 dev_err(dev, "Skipping disabled component %pOF\n",
68d9c47b1679ec Rob Herring 2017-07-21 147 node);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 148 continue;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 149 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 150
440aae04f38bf5 Mauro Carvalho Chehab 2021-11-24 151 comp_type = (uintptr_t)of_id->data;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 152
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 153 comp = devm_kzalloc(dev, sizeof(*comp), GFP_KERNEL);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 154 if (!comp) {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 155 ret = -ENOMEM;
22ff7d4ca3f748 Nishka Dasgupta 2019-07-09 156 of_node_put(node);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 157 goto err_comp;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 158 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 159
37e278c801078d Eizan Miyamoto 2020-05-07 160 ret = mtk_mdp_comp_init(dev, node, comp, comp_type);
22ff7d4ca3f748 Nishka Dasgupta 2019-07-09 161 if (ret) {
22ff7d4ca3f748 Nishka Dasgupta 2019-07-09 162 of_node_put(node);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 163 goto err_comp;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 164 }
86698b9505bbc9 Eizan Miyamoto 2020-05-07 165
86698b9505bbc9 Eizan Miyamoto 2020-05-07 166 mtk_mdp_register_component(mdp, comp);
22ff7d4ca3f748 Nishka Dasgupta 2019-07-09 167 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 168
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 169 mdp->job_wq = create_singlethread_workqueue(MTK_MDP_MODULE_NAME);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 170 if (!mdp->job_wq) {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 171 dev_err(&pdev->dev, "unable to alloc job workqueue\n");
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 172 ret = -ENOMEM;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 173 goto err_alloc_job_wq;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 174 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 175
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 176 mdp->wdt_wq = create_singlethread_workqueue("mdp_wdt_wq");
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 177 if (!mdp->wdt_wq) {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 178 dev_err(&pdev->dev, "unable to alloc wdt workqueue\n");
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 179 ret = -ENOMEM;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 180 goto err_alloc_wdt_wq;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 181 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 182 INIT_WORK(&mdp->wdt_work, mtk_mdp_wdt_worker);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 183
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 184 ret = v4l2_device_register(dev, &mdp->v4l2_dev);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 185 if (ret) {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 186 dev_err(&pdev->dev, "Failed to register v4l2 device\n");
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 187 ret = -EINVAL;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 188 goto err_dev_register;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 189 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 190
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 191 ret = mtk_mdp_register_m2m_device(mdp);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 192 if (ret) {
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 193 v4l2_err(&mdp->v4l2_dev, "Failed to init mem2mem device\n");
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 194 goto err_m2m_register;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 195 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 196
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 197 mdp->vpu_dev = vpu_get_plat_device(pdev);
ee18fc7b0b9517 Eizan Miyamoto 2020-05-07 198 ret = vpu_wdt_reg_handler(mdp->vpu_dev, mtk_mdp_reset_handler, mdp,
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 199 VPU_RST_MDP);
ee18fc7b0b9517 Eizan Miyamoto 2020-05-07 200 if (ret) {
ee18fc7b0b9517 Eizan Miyamoto 2020-05-07 201 dev_err(&pdev->dev, "Failed to register reset handler\n");
ee18fc7b0b9517 Eizan Miyamoto 2020-05-07 202 goto err_m2m_register;
ee18fc7b0b9517 Eizan Miyamoto 2020-05-07 203 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 204
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 205 platform_set_drvdata(pdev, mdp);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 206
256a12e1a343e1 Eizan Miyamoto 2020-05-07 @207 ret = vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
256a12e1a343e1 Eizan Miyamoto 2020-05-07 208 if (ret) {
256a12e1a343e1 Eizan Miyamoto 2020-05-07 209 dev_err(&pdev->dev, "Failed to set vb2 dma mag seg size\n");
256a12e1a343e1 Eizan Miyamoto 2020-05-07 210 goto err_m2m_register;
256a12e1a343e1 Eizan Miyamoto 2020-05-07 211 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 212
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 213 pm_runtime_enable(dev);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 214 dev_dbg(dev, "mdp-%d registered successfully\n", mdp->id);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 215
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 216 return 0;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 217
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 218 err_m2m_register:
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 219 v4l2_device_unregister(&mdp->v4l2_dev);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 220
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 221 err_dev_register:
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 222 destroy_workqueue(mdp->wdt_wq);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 223
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 224 err_alloc_wdt_wq:
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 225 destroy_workqueue(mdp->job_wq);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 226
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 227 err_alloc_job_wq:
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 228
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 229 err_comp:
86698b9505bbc9 Eizan Miyamoto 2020-05-07 230 list_for_each_entry_safe(comp, comp_temp, &mdp->comp_list, node) {
86698b9505bbc9 Eizan Miyamoto 2020-05-07 231 mtk_mdp_unregister_component(mdp, comp);
86698b9505bbc9 Eizan Miyamoto 2020-05-07 232 mtk_mdp_comp_deinit(dev, comp);
86698b9505bbc9 Eizan Miyamoto 2020-05-07 233 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 234
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 235 dev_dbg(dev, "err %d\n", ret);
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 236 return ret;
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 237 }
c8eb2d7e8202fd Minghsiu Tsai 2016-09-08 238 ---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
kernel test robot
build botRe: [PATCH v4 2/9] spi: Create helper API to lookup ACPI info for spi device
In reply to Stefan BindingThank you for the patch! Yet something to improve:
Hi Stefan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20220120]
[also build test ERROR on v5.16]
[cannot apply to broonie-spi/for-next rafael-pm/linux-next tiwai-sound/for-next linus/master v5.16 v5.16-rc8 v5.16-rc7] [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]
url: https://github.com/0day-ci/linux/commits/Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 base: 7fc5253f5a13271e9df35d6b936ff97b74540a59 config: riscv-randconfig-r042-20220120 (https://download.01.org/0day-ci/archive/20220121/202201210136.gwZmTll9-lkp(a)intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f7b7138a62648f4019c55e4671682af1f851f295)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/aedf84002ec1568907471591915e28bed40e8f9a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 git checkout aedf84002ec1568907471591915e28bed40e8f9a
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/rtc/
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
In file included from drivers/rtc/rtc-rv3029c2.c:13:
>> include/linux/spi/spi.h:769:1: error: expected identifier or '(' {
^
1 error generated.
vim +769 include/linux/spi/spi.h
762
763 #if IS_ENABLED(CONFIG_ACPI)
764 extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
765 struct acpi_device *adev);
766 #else
767 static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
768 struct acpi_device *adev);
> 769 { 770 return ERR_PTR(-EOPNOTSUPP);
771 }
772 #endif
773
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
PATCH v4 3/9 spi: Support selection of the index of the ACPI Spi Resource before alloc
In reply to Stefan BindingIf a node contains more than one Spi Resources, it may be necessary to use an index to select which one you want to allocate a spi device for.
--- drivers/spi/spi.c | 56 +++++++++++++++++++++++++++++++++++------ include/linux/spi/spi.h | 6 +++-- 2 files changed, 52 insertions(+), 10 deletions(-)
drivers/spi/spi.c +48 -8
@@ -2320,6 +2320,9 @@ struct acpi_spi_lookup {int irq;u8 bits_per_word;u8 chip_select; +int n; +int index; +int irq_index; }; static void acpi_spi_parse_apple_properties(struct acpi_device *dev,
@@ -2351,6 +2354,8 @@ static void acpi_spi_parse_apple_properties(struct acpi_device *dev, lookup->mode |=SPI_CPHA ; } +static struct spi_controller *acpi_spi_find_controller_by_adev (struct acpi_device *adev); +static int acpi_spi_add_resource (struct acpi_resource *ares,void *data) {struct acpi_spi_lookup *lookup = data;
@@ -2364,14 +2369,35 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data) sb = &ares->data.spi_serial_bus;if (sb->type ==ACPI_RESOURCE_SERIAL_TYPE_SPI ) { +if (lookup->index !=-1 && lookup->n++ != lookup->index) +return 1 ; + +if (lookup->index ==-1 && !ctlr) +return -ENODEV ; + status =acpi_get_handle (NULL , sb->resource_source.string_ptr, &parent_handle); -if (ACPI_FAILURE (status) || -ACPI_HANDLE (ctlr->dev.parent) != parent_handle) +if (ACPI_FAILURE (status))return -ENODEV ; +if (ctlr) { +if (ACPI_HANDLE (ctlr->dev.parent) != parent_handle) +return -ENODEV ; + }else { +struct acpi_device *adev; + +if (acpi_bus_get_device (parent_handle, &adev)) +return -ENODEV ; + + ctlr =acpi_spi_find_controller_by_adev (adev); +if (!ctlr) +return -ENODEV ; + + lookup->ctlr = ctlr; + } +/* * ACPI DeviceSelection numbering is handled by the * host controller driver in Windows and can vary
@@ -2402,7 +2428,7 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data) }else if (lookup->irq <0 ) {struct resource r; -if (acpi_dev_resource_interrupt (ares,0 , &r)) +if (acpi_dev_resource_interrupt (ares, lookup->irq_index, &r)) lookup->irq = r.start; }
@@ -2414,14 +2440,22 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)* acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information * @ctlr: controller to which the spi device belongs * @adev: ACPI Device for the spi device +* @index: Index of the spi resource inside the ACPI Node +* @irq_index: Index of the GPIO resource for the IRQ inside the ACPI Node * * This should be used to allocate a new spi device from and ACPI Node. * The caller is responsible for calling spi_add_device to register the spi device. * +* If ctlr is set to NULL, the Controller for the spi device will be looked up +* using the resource. +* If index is set to -1, index is not used. +* Note: If index is -1, ctlr must be set. +* * Return: a pointer to the new device, or ERR_PTR on error. */ struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, -struct acpi_device *adev) +struct acpi_device *adev, +int index,int irq_index) {acpi_handle parent_handle =NULL ;struct list_head resource_list;
@@ -2429,8 +2463,14 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,struct spi_device *spi;int ret; +if (!ctlr && index ==-1 ) +return ERR_PTR (-EINVAL ); + lookup.ctlr = ctlr; lookup.irq =-1 ; + lookup.index = index; + lookup.n =0 ; + lookup.irq_index = irq_index;INIT_LIST_HEAD (&resource_list); ret = acpi_dev_get_resources(adev, &resource_list,
@@ -2443,7 +2483,7 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,if (!lookup.max_speed_hz &&ACPI_SUCCESS (acpi_get_parent (adev->handle, &parent_handle)) && -ACPI_HANDLE (ctlr->dev.parent) == parent_handle) { +ACPI_HANDLE (lookup.ctlr->dev.parent) == parent_handle) {/* Apple does not use _CRS but nested devices for SPI slaves */ acpi_spi_parse_apple_properties (adev, &lookup); }
@@ -2451,9 +2491,9 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,if (!lookup.max_speed_hz)return ERR_PTR (-ENODEV ); - spi =spi_alloc_device (ctlr); + spi =spi_alloc_device (lookup.ctlr);if (!spi) { -dev_err (&ctlr->dev,"failed to allocate SPI device for %s\n" , +dev_err (&lookup.ctlr->dev,"failed to allocate SPI device for %s\n" ,dev_name (&adev->dev));return ERR_PTR (-ENOMEM ); }
@@ -2478,7 +2518,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, acpi_device_enumerated(adev))return AE_OK ; - spi =acpi_spi_device_alloc (ctlr, adev); + spi =acpi_spi_device_alloc (ctlr, adev,-1 ,0 );if (IS_ERR (spi)) {if (PTR_ERR (spi) == -ENOMEM )return AE_NO_MEMORY ;
include/linux/spi/spi.h +4 -2
@@ -762,10 +762,12 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);#if IS_ENABLED (CONFIG_ACPI )extern struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, -struct acpi_device *adev); +struct acpi_device *adev, +int index,int irq_index);#else static inline struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, -struct acpi_device *adev); +struct acpi_device *adev, +int index,int irq_index); {return ERR_PTR (-EOPNOTSUPP ); }
Signature · 1 line
2.25.1
Re: [PATCH v4 3/9] spi: Support selection of the index of the ACPI Spi Resource before alloc
Hi Stefan,
Quoted 60 lines · Stefan Binding
If a node contains more than one Spi Resources, it may be necessary to use an index to select which one you want to allocate a spi device for.
Signed-off-by Stefan Binding <[email protected]>--- drivers/spi/spi.c | 56 +++++++++++++++++++++++++++++++++++------ include/linux/spi/spi.h | 6 +++-- 2 files changed, 52 insertions(+), 10 deletions(-)drivers/spi/spi.c +21
-2@@ -2320,6 +2320,9 @@ struct acpi_spi_lookup {int irq;u8 bits_per_word;u8 chip_select; +int n; +int index; +int irq_index; }; static void acpi_spi_parse_apple_properties(struct acpi_device *dev,@@ -2351,6 +2354,8 @@ static void acpi_spi_parse_apple_properties(struct acpi_device *dev, lookup->mode |=SPI_CPHA ; } +static struct spi_controller *acpi_spi_find_controller_by_adev (struct acpi_device *adev); +static int acpi_spi_add_resource (struct acpi_resource *ares,void *data) {struct acpi_spi_lookup *lookup = data;@@ -2364,14 +2369,35 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data) sb = &ares->data.spi_serial_bus;if (sb->type ==ACPI_RESOURCE_SERIAL_TYPE_SPI ) { +if (lookup->index !=-1 && lookup->n++ != lookup->index) +return 1 ; + +if (lookup->index ==-1 && !ctlr) +return -ENODEV ; + status =acpi_get_handle (NULL , sb->resource_source.string_ptr, &parent_handle); -if (ACPI_FAILURE (status) || -ACPI_HANDLE (ctlr->dev.parent) != parent_handle) +if (ACPI_FAILURE (status))return -ENODEV ; +if (ctlr) { +if (ACPI_HANDLE (ctlr->dev.parent) != parent_handle) +return -ENODEV ; + }else { +struct acpi_device *adev; + +if (acpi_bus_get_device (parent_handle, &adev)) +return -ENODEV ; +
drivers/spi/spi.c static int acpi_spi_add_resource(struct acpi_resource *ares, void *data) · line 2403
Quoted 10 lines
drivers/spi/spi.c +7
-0+ ctlr =acpi_spi_find_controller_by_adev (adev); +if (!ctlr) +return -ENODEV ; + + lookup->ctlr = ctlr; + } +/* * ACPI DeviceSelection numbering is handled by the * host controller driver in Windows and can vary
All of the above looks good to me.
drivers/spi/spi.c static int acpi_spi_add_resource(struct acpi_resource *ares, void *data) · line 2434
Quoted 9 lines
drivers/spi/spi.c +1
-1@@ -2402,7 +2428,7 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data) }else if (lookup->irq <0 ) {struct resource r; -if (acpi_dev_resource_interrupt (ares,0 , &r)) +if (acpi_dev_resource_interrupt (ares, lookup->irq_index, &r)) lookup->irq = r.start; }
I think that this does not do what you want it to do. Quoting drivers/acpi/resource.c:
bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
struct resource *res)
{
struct acpi_resource_irq *irq;
struct acpi_resource_extended_irq *ext_irq;
switch (ares->type) {
case ACPI_RESOURCE_TYPE_IRQ:
/*
* Per spec, only one interrupt per descriptor is allowed in
* _CRS, but some firmware violates this, so parse them all.
*/
irq = &ares->data.irq;
if (index >= irq->interrupt_count) {
irqresource_disabled(res, 0);
return false;
}So now you are actually adding an index into a single:
Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
{
0x00000008,
}ACPI Resource, which typically (as shown here) only has 1 interrupt, I guess what you want is the nth resource of type ACPI_RESOURCE_TYPE_IRQ or ACPI_RESOURCE_TYPE_EXTENDED_IRQ.
So to make this work you should combine this with a separate irq_n for irq_index, increment that on acpi_dev_resource_interrupt() success and then only set lookup->irq = r.start; when irq_n == irq_index.
But since the bus-multi-inst code already has its own irq getting code this is not really necessary, I would just drop the new irq_index field; and then also drop the "if (spi_dev->irq < 0)" condition for calling bmi_get_irq() in "[PATCH v4 7/9] platform/x86: bus-multi-instantiate:
Add SPI support"
Note that for the CS35L41 that if is always true already since that is using a GpioInt resource and acpi_dev_resource_interrupt() does not support those.
TL;DR: please drop the new irq_index fiels and just leave the related bits as is.
Quoted 79 lines
drivers/spi/spi.c +19
-5@@ -2414,14 +2440,22 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)* acpi_spi_device_alloc - Allocate a spi device, and fill it in with ACPI information * @ctlr: controller to which the spi device belongs * @adev: ACPI Device for the spi device +* @index: Index of the spi resource inside the ACPI Node +* @irq_index: Index of the GPIO resource for the IRQ inside the ACPI Node * * This should be used to allocate a new spi device from and ACPI Node. * The caller is responsible for calling spi_add_device to register the spi device. * +* If ctlr is set to NULL, the Controller for the spi device will be looked up +* using the resource. +* If index is set to -1, index is not used. +* Note: If index is -1, ctlr must be set. +* * Return: a pointer to the new device, or ERR_PTR on error. */ struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, -struct acpi_device *adev) +struct acpi_device *adev, +int index,int irq_index) {acpi_handle parent_handle =NULL ;struct list_head resource_list;@@ -2429,8 +2463,14 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,struct spi_device *spi;int ret; +if (!ctlr && index ==-1 ) +return ERR_PTR (-EINVAL ); + lookup.ctlr = ctlr; lookup.irq =-1 ; + lookup.index = index; + lookup.n =0 ; + lookup.irq_index = irq_index;INIT_LIST_HEAD (&resource_list); ret = acpi_dev_get_resources(adev, &resource_list,@@ -2443,7 +2483,7 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,if (!lookup.max_speed_hz &&ACPI_SUCCESS (acpi_get_parent (adev->handle, &parent_handle)) && -ACPI_HANDLE (ctlr->dev.parent) == parent_handle) { +ACPI_HANDLE (lookup.ctlr->dev.parent) == parent_handle) {/* Apple does not use _CRS but nested devices for SPI slaves */ acpi_spi_parse_apple_properties (adev, &lookup); }@@ -2451,9 +2491,9 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,if (!lookup.max_speed_hz)return ERR_PTR (-ENODEV ); - spi =spi_alloc_device (ctlr); + spi =spi_alloc_device (lookup.ctlr);if (!spi) { -dev_err (&ctlr->dev,"failed to allocate SPI device for %s\n" , +dev_err (&lookup.ctlr->dev,"failed to allocate SPI device for %s\n" ,dev_name (&adev->dev));return ERR_PTR (-ENOMEM ); }@@ -2478,7 +2518,7 @@ static acpi_status acpi_register_spi_device(struct spi_controller *ctlr, acpi_device_enumerated(adev))return AE_OK ; - spi =acpi_spi_device_alloc (ctlr, adev); + spi =acpi_spi_device_alloc (ctlr, adev,-1 ,0 );if (IS_ERR (spi)) {if (PTR_ERR (spi) == -ENOMEM )return AE_NO_MEMORY ;include/linux/spi/spi.h +0
-1@@ -762,10 +762,12 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);#if IS_ENABLED (CONFIG_ACPI ) extern struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, -struct acpi_device *adev);
include/linux/spi/spi.h extern void spi_unregister_controller(struct spi_controller *ctlr); · line 773
Quoted 10 lines
include/linux/spi/spi.h +4
-1+struct acpi_device *adev, + int index, int irq_index);#else static inline struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr, -struct acpi_device *adev); +struct acpi_device *adev, +int index,int irq_index); {return ERR_PTR (-EOPNOTSUPP ); }
Like with my review of patch 2/9, I don't think we need stubs here.
Regards,
Hans
PATCH v4 4/9 spi: Add API to count spi acpi resources
In reply to Stefan BindingSome ACPI nodes may have more than one Spi Resource. To be able to handle these case, its necessary to have a way of counting these resources.
--- drivers/spi/spi.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/linux/spi/spi.h | 6 ++++++ 2 files changed, 46 insertions(+)
drivers/spi/spi.c +40 -0
@@ -2325,6 +2325,46 @@ struct acpi_spi_lookup {int irq_index; }; +static int acpi_spi_count (struct acpi_resource *ares,void *data) +{ +struct acpi_resource_spi_serialbus *sb; +int *count = data; + +if (ares->type !=ACPI_RESOURCE_TYPE_SERIAL_BUS ) +return 1 ; + + sb = &ares->data.spi_serial_bus; +if (sb->type !=ACPI_RESOURCE_SERIAL_TYPE_SPI ) +return 1 ; + + *count = *count +1 ; + +return 1 ; +} + +/** +* acpi_spi_count_resources - Count the number of SpiSerialBus resources +* @adev: ACPI device +* +* Returns the number of SpiSerialBus resources in the ACPI-device's +* resource-list; or a negative error code. +*/ +int acpi_spi_count_resources (struct acpi_device *adev) +{ +LIST_HEAD (r); +int count =0 ; +int ret; + + ret =acpi_dev_get_resources (adev, &r, acpi_spi_count, &count); +if (ret <0 ) +return ret; + +acpi_dev_free_resource_list (&r); + +return count; +} +EXPORT_SYMBOL_GPL (acpi_spi_count_resources); +static void acpi_spi_parse_apple_properties (struct acpi_device *dev,struct acpi_spi_lookup *lookup) {
include/linux/spi/spi.h +6 -0
@@ -764,6 +764,7 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);extern struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr,struct acpi_device *adev,int index,int irq_index); +int acpi_spi_count_resources (struct acpi_device *adev);#else static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, struct acpi_device *adev,
@@ -771,6 +772,11 @@ static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ct {return ERR_PTR (-EOPNOTSUPP ); } + +int acpi_spi_count_resources (struct acpi_device *adev) +{ +return -EOPNOTSUPP ; +}#endif /*
Signature · 1 line
2.25.1
Re: [PATCH v4 4/9] spi: Add API to count spi acpi resources
Hi,
Quoted 5 lines · Stefan Binding
Some ACPI nodes may have more than one Spi Resource. To be able to handle these case, its necessary to have a way of counting these resources.
Signed-off-by Stefan Binding <[email protected]>
Thanks, patch looks good to me:
Regards,
Hans
Quoted 81 lines
--- drivers/spi/spi.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/linux/spi/spi.h | 6 ++++++ 2 files changed, 46 insertions(+)drivers/spi/spi.c +40
-0@@ -2325,6 +2325,46 @@ struct acpi_spi_lookup {int irq_index; }; +static int acpi_spi_count (struct acpi_resource *ares,void *data) +{ +struct acpi_resource_spi_serialbus *sb; +int *count = data; + +if (ares->type !=ACPI_RESOURCE_TYPE_SERIAL_BUS ) +return 1 ; + + sb = &ares->data.spi_serial_bus; +if (sb->type !=ACPI_RESOURCE_SERIAL_TYPE_SPI ) +return 1 ; + + *count = *count +1 ; + +return 1 ; +} + +/** +* acpi_spi_count_resources - Count the number of SpiSerialBus resources +* @adev: ACPI device +* +* Returns the number of SpiSerialBus resources in the ACPI-device's +* resource-list; or a negative error code. +*/ +int acpi_spi_count_resources (struct acpi_device *adev) +{ +LIST_HEAD (r); +int count =0 ; +int ret; + + ret =acpi_dev_get_resources (adev, &r, acpi_spi_count, &count); +if (ret <0 ) +return ret; + +acpi_dev_free_resource_list (&r); + +return count; +} +EXPORT_SYMBOL_GPL (acpi_spi_count_resources); +static void acpi_spi_parse_apple_properties (struct acpi_device *dev,struct acpi_spi_lookup *lookup) {include/linux/spi/spi.h +6
-0@@ -764,6 +764,7 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);extern struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr,struct acpi_device *adev,int index,int irq_index); +int acpi_spi_count_resources (struct acpi_device *adev);#else static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, struct acpi_device *adev,@@ -771,6 +772,11 @@ static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ct {return ERR_PTR (-EOPNOTSUPP ); } + +int acpi_spi_count_resources (struct acpi_device *adev) +{ +return -EOPNOTSUPP ; +}#endif /*
Re: [PATCH v4 4/9] spi: Add API to count spi acpi resources
In reply to Stefan BindingHi,
Quoted 75 lines · Stefan Binding
Some ACPI nodes may have more than one Spi Resource. To be able to handle these case, its necessary to have a way of counting these resources.
Signed-off-by Stefan Binding <[email protected]>--- drivers/spi/spi.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/linux/spi/spi.h | 6 ++++++ 2 files changed, 46 insertions(+)drivers/spi/spi.c +40
-0@@ -2325,6 +2325,46 @@ struct acpi_spi_lookup {int irq_index; }; +static int acpi_spi_count (struct acpi_resource *ares,void *data) +{ +struct acpi_resource_spi_serialbus *sb; +int *count = data; + +if (ares->type !=ACPI_RESOURCE_TYPE_SERIAL_BUS ) +return 1 ; + + sb = &ares->data.spi_serial_bus; +if (sb->type !=ACPI_RESOURCE_SERIAL_TYPE_SPI ) +return 1 ; + + *count = *count +1 ; + +return 1 ; +} + +/** +* acpi_spi_count_resources - Count the number of SpiSerialBus resources +* @adev: ACPI device +* +* Returns the number of SpiSerialBus resources in the ACPI-device's +* resource-list; or a negative error code. +*/ +int acpi_spi_count_resources (struct acpi_device *adev) +{ +LIST_HEAD (r); +int count =0 ; +int ret; + + ret =acpi_dev_get_resources (adev, &r, acpi_spi_count, &count); +if (ret <0 ) +return ret; + +acpi_dev_free_resource_list (&r); + +return count; +} +EXPORT_SYMBOL_GPL (acpi_spi_count_resources); +static void acpi_spi_parse_apple_properties (struct acpi_device *dev,struct acpi_spi_lookup *lookup) {include/linux/spi/spi.h +1
-0@@ -764,6 +764,7 @@ extern void spi_unregister_controller(struct spi_controller *ctlr);extern struct spi_device *acpi_spi_device_alloc (struct spi_controller *ctlr,struct acpi_device *adev,int index,int irq_index); +int acpi_spi_count_resources (struct acpi_device *adev);#else static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, struct acpi_device *adev,@@ -771,6 +772,11 @@ static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ct {
include/linux/spi/spi.h static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ct · line 782
Quoted 10 lines
include/linux/spi/spi.h +5
-0return ERR_PTR(-EOPNOTSUPP); } + +int acpi_spi_count_resources (struct acpi_device *adev) +{ +return -EOPNOTSUPP ; +}#endif /*
I just realized I miss this bit, like with my review of patch 2/9, I don't think we need stubs here.
Regards,
Hans
kernel test robot
build botRe: [PATCH v4 4/9] spi: Add API to count spi acpi resources
In reply to Stefan BindingThank you for the patch! Perhaps something to improve:
Hi Stefan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20220120]
[also build test WARNING on v5.16]
[cannot apply to broonie-spi/for-next rafael-pm/linux-next tiwai-sound/for-next linus/master v5.16 v5.16-rc8 v5.16-rc7] [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]
url: https://github.com/0day-ci/linux/commits/Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 base: 7fc5253f5a13271e9df35d6b936ff97b74540a59 config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220121/202201210109.tNnI8P2j-lkp(a)intel.com/config) compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/684295c62acdefb3fab20a0e7633793ba748c6d7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Stefan-Binding/Support-Spi-in-i2c-multi-instantiate-driver/20220120-214608 git checkout 684295c62acdefb3fab20a0e7633793ba748c6d7
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/char/tpm/st33zp24/
If you fix the issue, kindly add following tag as appropriate
All warnings (new ones prefixed by >>):
In file included from drivers/char/tpm/st33zp24/spi.c:8:
include/linux/spi/spi.h:772:1: error: expected identifier or '(' before '{' token
772 | {
| ^
>> include/linux/spi/spi.h:776:5: warning: no previous prototype for 'acpi_spi_count_resources' [-Wmissing-prototypes]
776 | int acpi_spi_count_resources(struct acpi_device *adev)
| ^~~~~~~~~~~~~~~~~~~~~~~~
include/linux/spi/spi.h:769:34: warning: 'acpi_spi_device_alloc' declared 'static' but never defined [-Wunused-function]
769 | static inline struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
| ^~~~~~~~~~~~~~~~~~~~~
drivers/char/tpm/st33zp24/spi.c:408:36: warning: 'st33zp24_spi_acpi_match' defined but not used [-Wunused-const-variable=]
408 | static const struct acpi_device_id st33zp24_spi_acpi_match[] = {
| ^~~~~~~~~~~~~~~~~~~~~~~vim +/acpi_spi_count_resources +776 include/linux/spi/spi.h
775
> 776 int acpi_spi_count_resources(struct acpi_device *adev)
777 {
778 return -EOPNOTSUPP;
779 }
780 #endif
781
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
PATCH v4 5/9 platform/x86: i2c-multi-instantiate: Rename it for a generic bus driver name
In reply to Stefan BindingFrom: Lucas Tanure <[email protected]>
Rename I2C multi instantiate driver to bus-multi-instantiate for upcoming addition of SPI support
---
MAINTAINERS | 4 +-
drivers/acpi/scan.c | 2 +-
drivers/platform/x86/Kconfig | 4 +-
drivers/platform/x86/Makefile | 2 +-
...-instantiate.c => bus-multi-instantiate.c} | 89 +++++++++----------
5 files changed, 50 insertions(+), 51 deletions(-)
rename drivers/platform/x86/{i2c-multi-instantiate.c => bus-multi-instantiate.c} (53%)MAINTAINERS +2 -2
@@ -388,11 +388,11 @@ L: [email protected] (moderated for non-subscribers) S: Maintained F: drivers/acpi/arm64 -ACPI I2C MULTI INSTANTIATE DRIVER +ACPI BUS MULTI INSTANTIATE DRIVER M: Hans de Goede <[email protected]> L: [email protected] S: Maintained -F: drivers/platform/x86/i2c-multi-instantiate.c +F: drivers/platform/x86/bus-multi-instantiate.c ACPI PCC(Platform Communication Channel) MAILBOX DRIVER M: Sudeep Holla <[email protected]>
drivers/acpi/scan.c +1 -1
@@ -1738,7 +1738,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)* must be instantiated for each, each with its own i2c_device_id. * Normally we only instantiate an i2c-client for the first resource, * using the ACPI HID as id. These special cases are handled by the -* drivers/platform/x86/i2c-multi-instantiate.c driver, which knows +* drivers/platform/x86/bus-multi-instantiate.c driver, which knows * which i2c_device_id to use for each resource. */ {"BSG1160", },
drivers/platform/x86/Kconfig +2 -2
@@ -990,7 +990,7 @@ config TOPSTAR_LAPTOP If you have a Topstar laptop, say Y or M here. -config I2C_MULTI_INSTANTIATE +config BUS_MULTI_INSTANTIATE tristate "I2C multi instantiate pseudo device driver" depends on I2C && ACPI help
@@ -999,7 +999,7 @@ config I2C_MULTI_INSTANTIATE for each device in the firmware-node. To compile this driver as a module, choose M here: the module - will be called i2c-multi-instantiate. + will be called bus-multi-instantiate. config MLX_PLATFORM tristate "Mellanox Technologies platform support"
drivers/platform/x86/Makefile +1 -1
@@ -110,7 +110,7 @@ obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o # Platform drivers obj-$(CONFIG_FW_ATTR_CLASS) += firmware_attributes_class.o -obj-$(CONFIG_I2C_MULTI_INSTANTIATE) += i2c-multi-instantiate.o +obj-$(CONFIG_BUS_MULTI_INSTANTIATE) += bus-multi-instantiate.o obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o obj-$(CONFIG_TOUCHSCREEN_DMI) += touchscreen_dmi.o obj-$(CONFIG_WIRELESS_HOTKEY) += wireless-hotkey.o
drivers/platform/x86/bus-multi-instantiate.c renamed from drivers/platform/x86/i2c-multi-instantiate.c +44 -45
194 lines not on this page · raw message
Signature · 1 line
2.25.1
Re: [PATCH v4 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic bus driver name
Hi,
Quoted 7 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Rename I2C multi instantiate driver to bus-multi-instantiate for upcoming addition of SPI support
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>
Thanks, patch looks good to me:
Regards,
Hans
Quoted 283 lines
--- MAINTAINERS | 4 +- drivers/acpi/scan.c | 2 +- drivers/platform/x86/Kconfig | 4 +- drivers/platform/x86/Makefile | 2 +- ...-instantiate.c => bus-multi-instantiate.c} | 89 +++++++++---------- 5 files changed, 50 insertions(+), 51 deletions(-) rename drivers/platform/x86/{i2c-multi-instantiate.c => bus-multi-instantiate.c} (53%)MAINTAINERS +2
-2@@ -388,11 +388,11 @@ L: [email protected] (moderated for non-subscribers) S: Maintained F: drivers/acpi/arm64 -ACPI I2C MULTI INSTANTIATE DRIVER +ACPI BUS MULTI INSTANTIATE DRIVER M: Hans de Goede <[email protected]> L: [email protected] S: Maintained -F: drivers/platform/x86/i2c-multi-instantiate.c +F: drivers/platform/x86/bus-multi-instantiate.c ACPI PCC(Platform Communication Channel) MAILBOX DRIVER M: Sudeep Holla <[email protected]>drivers/acpi/scan.c +1
-1@@ -1738,7 +1738,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)* must be instantiated for each, each with its own i2c_device_id. * Normally we only instantiate an i2c-client for the first resource, * using the ACPI HID as id. These special cases are handled by the -* drivers/platform/x86/i2c-multi-instantiate.c driver, which knows +* drivers/platform/x86/bus-multi-instantiate.c driver, which knows * which i2c_device_id to use for each resource. */ {"BSG1160", },drivers/platform/x86/Kconfig +2
-2@@ -990,7 +990,7 @@ config TOPSTAR_LAPTOP If you have a Topstar laptop, say Y or M here. -config I2C_MULTI_INSTANTIATE +config BUS_MULTI_INSTANTIATE tristate "I2C multi instantiate pseudo device driver" depends on I2C && ACPI help@@ -999,7 +999,7 @@ config I2C_MULTI_INSTANTIATE for each device in the firmware-node. To compile this driver as a module, choose M here: the module - will be called i2c-multi-instantiate. + will be called bus-multi-instantiate. config MLX_PLATFORM tristate "Mellanox Technologies platform support"drivers/platform/x86/Makefile +1
-1@@ -110,7 +110,7 @@ obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o # Platform drivers obj-$(CONFIG_FW_ATTR_CLASS) += firmware_attributes_class.o -obj-$(CONFIG_I2C_MULTI_INSTANTIATE) += i2c-multi-instantiate.o +obj-$(CONFIG_BUS_MULTI_INSTANTIATE) += bus-multi-instantiate.o obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o obj-$(CONFIG_TOUCHSCREEN_DMI) += touchscreen_dmi.o obj-$(CONFIG_WIRELESS_HOTKEY) += wireless-hotkey.odrivers/platform/x86/bus-multi-instantiate.c renamed from drivers/platform/x86/i2c-multi-instantiate.c +44
-45@@ -1,6 +1,6 @@// SPDX-License-Identifier: GPL-2.0+ /* -* I2C multi-instantiate driver, pseudo driver to instantiate multiple +* Bus multi-instantiate driver, pseudo driver to instantiate multiple * i2c-clients from a single fwnode. * * Copyright 2018 Hans de Goede <[email protected]> @@ -21,29 +21,29 @@#define IRQ_RESOURCE_GPIO 1#define IRQ_RESOURCE_APIC 2 -struct i2c_inst_data { +struct bmi_instance {const char *type;unsigned int flags;int irq_idx; }; -struct i2c_multi_inst_data { -int num_clients; -struct i2c_client *clients[]; +struct bmi { +int i2c_num; +struct i2c_client *i2c_devs[]; }; -static int i2c_multi_inst_probe (struct platform_device *pdev) +static int bmi_probe (struct platform_device *pdev) { -struct i2c_multi_inst_data *multi; -const struct i2c_inst_data *inst_data;struct i2c_board_info board_info = {}; +const struct bmi_instance *inst;struct device *dev = &pdev->dev;struct acpi_device *adev; +struct bmi *bmi;char name[32 ];int i, ret; - inst_data =device_get_match_data (dev); -if (!inst_data) { + inst =device_get_match_data (dev); +if (!inst) {dev_err (dev,"Error ACPI match data is missing\n" );return -ENODEV ; }@@ -55,33 +55,32 @@ static int i2c_multi_inst_probe(struct platform_device *pdev)if (ret <0 )return ret; - multi =devm_kmalloc (dev,struct_size (multi, clients, ret),GFP_KERNEL ); -if (!multi) + bmi =devm_kmalloc (dev,struct_size (bmi, i2c_devs, ret),GFP_KERNEL ); +if (!bmi)return -ENOMEM ; - multi->num_clients = ret; + bmi->i2c_num = ret; -for (i =0 ; i < multi->num_clients && inst_data[i].type; i++) { +for (i =0 ; i < bmi->i2c_num && inst[i].type; i++) {memset (&board_info,0 ,sizeof (board_info)); -strlcpy (board_info.type, inst_data[i].type,I2C_NAME_SIZE ); -snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), - inst_data[i].type, i); +strlcpy (board_info.type, inst[i].type,I2C_NAME_SIZE ); +snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst[i].type, i); board_info.dev_name = name; -switch (inst_data[i].flags &IRQ_RESOURCE_TYPE ) { +switch (inst[i].flags &IRQ_RESOURCE_TYPE ) {case IRQ_RESOURCE_GPIO : - ret =acpi_dev_gpio_irq_get (adev, inst_data[i].irq_idx); + ret =acpi_dev_gpio_irq_get (adev, inst[i].irq_idx);if (ret <0 ) {dev_err (dev,"Error requesting irq at index %d: %d\n" , - inst_data[i].irq_idx, ret); + inst[i].irq_idx, ret); goto error; } board_info.irq = ret;break ;case IRQ_RESOURCE_APIC : - ret =platform_get_irq (pdev, inst_data[i].irq_idx); + ret =platform_get_irq (pdev, inst[i].irq_idx);if (ret <0 ) {dev_dbg (dev,"Error requesting irq at index %d: %d\n" , - inst_data[i].irq_idx, ret); + inst[i].irq_idx, ret); goto error; } board_info.irq = ret;@@ -90,48 +89,48 @@ static int i2c_multi_inst_probe(struct platform_device *pdev) board_info.irq =0 ;break ; } - multi->clients[i] =i2c_acpi_new_device (dev, i, &board_info); -if (IS_ERR (multi->clients[i])) { - ret =dev_err_probe (dev,PTR_ERR (multi->clients[i]), + bmi->i2c_devs[i] =i2c_acpi_new_device (dev, i, &board_info); +if (IS_ERR (bmi->i2c_devs[i])) { + ret =dev_err_probe (dev,PTR_ERR (bmi->i2c_devs[i]),"Error creating i2c-client, idx %d\n" , i); goto error; } } -if (i < multi->num_clients) { +if (i < bmi->i2c_num) {dev_err (dev,"Error finding driver, idx %d\n" , i); ret = -ENODEV ; goto error; } -platform_set_drvdata (pdev, multi); +platform_set_drvdata (pdev, bmi);return 0 ; error:while (--i >=0 ) -i2c_unregister_device (multi->clients[i]); +i2c_unregister_device (bmi->i2c_devs[i]);return ret; } -static int i2c_multi_inst_remove (struct platform_device *pdev) +static int bmi_remove (struct platform_device *pdev) { -struct i2c_multi_inst_data *multi =platform_get_drvdata (pdev); +struct bmi *bmi =platform_get_drvdata (pdev);int i; -for (i =0 ; i < multi->num_clients; i++) -i2c_unregister_device (multi->clients[i]); +for (i =0 ; i < bmi->i2c_num; i++) +i2c_unregister_device (bmi->i2c_devs[i]);return 0 ; } -static const struct i2c_inst_data bsg1160_data[] = { +static const struct bmi_instance bsg1160_data[] = { {"bmc150_accel" ,IRQ_RESOURCE_GPIO ,0 }, {"bmc150_magn" }, {"bmg160" }, {} }; -static const struct i2c_inst_data bsg2150_data[] = { +static const struct bmi_instance bsg2150_data[] = { {"bmc150_accel" ,IRQ_RESOURCE_GPIO ,0 }, {"bmc150_magn" },/* The resources describe a 3th client, but it is not really there. */ @@ -139,7 +138,7 @@ static const struct i2c_inst_data bsg2150_data[] = { {} }; -static const struct i2c_inst_data int3515_data[] = { +static const struct bmi_instance int3515_data[] = { {"tps6598x" ,IRQ_RESOURCE_APIC ,0 }, {"tps6598x" ,IRQ_RESOURCE_APIC ,1 }, {"tps6598x" ,IRQ_RESOURCE_APIC ,2 },@@ -148,27 +147,27 @@ static const struct i2c_inst_data int3515_data[] = { };/* -* Note new device-ids must also be added to i2c_multi_instantiate_ids in +* Note new device-ids must also be added to bus_multi_instantiate_ids in * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). */ -static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { +static const struct acpi_device_id bmi_acpi_ids[] = { {"BSG1160" , (unsigned long )bsg1160_data }, {"BSG2150" , (unsigned long )bsg2150_data }, {"INT3515" , (unsigned long )int3515_data }, { } }; -MODULE_DEVICE_TABLE (acpi, i2c_multi_inst_acpi_ids); +MODULE_DEVICE_TABLE (acpi, bmi_acpi_ids); -static struct platform_driver i2c_multi_inst_driver = { +static struct platform_driver bmi_driver = { .driver = { - .name ="I2C multi instantiate pseudo device driver" , - .acpi_match_table = i2c_multi_inst_acpi_ids, + .name ="Bus multi instantiate pseudo device driver" , + .acpi_match_table = bmi_acpi_ids, }, - .probe = i2c_multi_inst_probe, - .remove = i2c_multi_inst_remove, + .probe = bmi_probe, + .remove = bmi_remove, }; -module_platform_driver (i2c_multi_inst_driver); +module_platform_driver (bmi_driver); -MODULE_DESCRIPTION ("I2C multi instantiate pseudo device driver" ); +MODULE_DESCRIPTION ("Bus multi instantiate pseudo device driver" );MODULE_AUTHOR ("Hans de Goede <[email protected]>" );MODULE_LICENSE ("GPL" );
Rafael J. Wysocki
maintainerRe: [PATCH v4 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic bus driver name
In reply to Stefan BindingQuoted 5 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Rename I2C multi instantiate driver to bus-multi-instantiate for upcoming addition of SPI support
TBH, I don't particularly like the new naming.
Namely, the "bus" part seems overly generic (and the "bmi" abbreviation brings something totally different to mind).
AFAICS this can cover serial interfaces only and specifically the ones where serial links can be represented as resources returned by the ACPI _CRS object.
I would call it serial-multi-instance or similar.
Re: [PATCH v4 5/9] platform/x86: i2c-multi-instantiate: Rename it for a generic bus driver name
Hi,
Quoted 10 lines · Rafael J. Wysocki
Quoted 5 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Rename I2C multi instantiate driver to bus-multi-instantiate for upcoming addition of SPI support
TBH, I don't particularly like the new naming.
Quoted 8 lines
Namely, the "bus" part seems overly generic (and the "bmi" abbreviation brings something totally different to mind).
AFAICS this can cover serial interfaces only and specifically the ones where serial links can be represented as resources returned by the ACPI
_CRSobject.I would call it serial-multi-instance or similar.
serial-multi-instantiate works for me. That also nice matches up with the ACPI_RESOURCE_TYPE_SERIAL_BUS which acpi/scan.c uses to detect these kinda resources.
Regards,
Hans
PATCH v4 6/9 platform/x86: bus-multi-instantiate: Reorganize I2C functions
In reply to Stefan BindingFrom: Lucas Tanure <[email protected]>
Reorganize I2C functions to accommodate SPI support Split the probe and factor out parts of the code that will be used in the SPI support
--- drivers/platform/x86/bus-multi-instantiate.c | 150 ++++++++++++------- 1 file changed, 96 insertions(+), 54 deletions(-)
drivers/platform/x86/bus-multi-instantiate.c +96 -54
@@ -29,85 +29,129 @@ struct bmi_instance {struct bmi {int i2c_num; -struct i2c_client *i2c_devs[]; +struct i2c_client **i2c_devs; }; -static int bmi_probe (struct platform_device *pdev) +static int bmi_get_irq (struct platform_device *pdev,struct acpi_device *adev, +const struct bmi_instance *inst) +{ +int ret; + +switch (inst->flags &IRQ_RESOURCE_TYPE ) { +case IRQ_RESOURCE_GPIO : + ret =acpi_dev_gpio_irq_get (adev, inst->irq_idx); +break ; +case IRQ_RESOURCE_APIC : + ret =platform_get_irq (pdev, inst->irq_idx); +break ; +default : + ret =0 ; +break ; + } + +if (ret <0 ) +dev_err_probe (&pdev->dev, ret,"Error requesting irq at index %d: %d\n" , + inst->irq_idx, ret); + +return ret; +} + +static void bmi_devs_unregister (struct bmi *bmi) +{ +while (bmi->i2c_num >0 ) +i2c_unregister_device (bmi->i2c_devs[--bmi->i2c_num]); +} + +/** +* bmi_i2c_probe - Instantiate multiple I2C devices from inst array +* @pdev: Platform device +* @adev: ACPI device +* @bmi: Internal struct for Bus multi instantiate driver +* @inst: Array of instances to probe +* +* Returns the number of I2C devices instantiate, Zero if none is found or a negative error code. +*/ +static int bmi_i2c_probe (struct platform_device *pdev,struct acpi_device *adev,struct bmi *bmi, +const struct bmi_instance *inst_array) {struct i2c_board_info board_info = {}; -const struct bmi_instance *inst;struct device *dev = &pdev->dev; -struct acpi_device *adev; -struct bmi *bmi;char name[32 ]; -int i, ret; +int i, ret =0 , count; - inst =device_get_match_data (dev); -if (!inst) { -dev_err (dev,"Error ACPI match data is missing\n" ); -return -ENODEV ; - } - - adev =ACPI_COMPANION (dev); - -/* Count number of clients to instantiate */ ret =i2c_acpi_client_count (adev); -if (ret <0 ) +if (ret <=0 )return ret; + count = ret; - bmi =devm_kmalloc (dev,struct_size (bmi, i2c_devs, ret),GFP_KERNEL ); -if (!bmi) + bmi->i2c_devs =devm_kcalloc (dev, count,sizeof (*bmi->i2c_devs),GFP_KERNEL ); +if (!bmi->i2c_devs)return -ENOMEM ; - bmi->i2c_num = ret; - -for (i =0 ; i < bmi->i2c_num && inst[i].type; i++) { +for (i =0 ; i < count && inst_array[i].type; i++) {memset (&board_info,0 ,sizeof (board_info)); -strlcpy (board_info.type, inst[i].type,I2C_NAME_SIZE ); -snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst[i].type, i); +strscpy (board_info.type, inst_array[i].type,I2C_NAME_SIZE ); +snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst_array[i].type, i); board_info.dev_name = name; -switch (inst[i].flags &IRQ_RESOURCE_TYPE ) { -case IRQ_RESOURCE_GPIO : - ret =acpi_dev_gpio_irq_get (adev, inst[i].irq_idx); -if (ret <0 ) { -dev_err (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -case IRQ_RESOURCE_APIC : - ret =platform_get_irq (pdev, inst[i].irq_idx); -if (ret <0 ) { -dev_dbg (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -default : - board_info.irq =0 ; -break ; - } + + ret =bmi_get_irq (pdev, adev, &inst_array[i]); +if (ret <0 ) + goto error; + board_info.irq = ret; + bmi->i2c_devs[i] =i2c_acpi_new_device (dev, i, &board_info);if (IS_ERR (bmi->i2c_devs[i])) { ret =dev_err_probe (dev,PTR_ERR (bmi->i2c_devs[i]),"Error creating i2c-client, idx %d\n" , i); goto error; } + bmi->i2c_num++; } -if (i < bmi->i2c_num) { +if (bmi->i2c_num < count) {dev_err (dev,"Error finding driver, idx %d\n" , i); ret = -ENODEV ; goto error; } -platform_set_drvdata (pdev, bmi); -return 0 ; +dev_info (dev,"Instantiate %d I2C devices.\n" , bmi->i2c_num); +return bmi->i2c_num; error: -while (--i >=0 ) -i2c_unregister_device (bmi->i2c_devs[i]); +dev_err_probe (dev, ret,"I2C error %d\n" , ret); +bmi_devs_unregister (bmi); + +return ret; +} + +static int bmi_probe (struct platform_device *pdev) +{ +const struct bmi_instance *inst_array; +struct device *dev = &pdev->dev; +struct acpi_device *adev; +struct bmi *bmi; +int ret; + + inst_array =device_get_match_data (dev); +if (!inst_array) { +dev_err (dev,"Error ACPI match data is missing\n" ); +return -ENODEV ; + } + + adev =ACPI_COMPANION (dev); +if (!adev) +return -ENODEV ; + + bmi =devm_kzalloc (dev,sizeof (*bmi),GFP_KERNEL ); +if (!bmi) +return -ENOMEM ; + +platform_set_drvdata (pdev, bmi); + + ret =bmi_i2c_probe (pdev, adev, bmi, inst_array); +if (ret >0 ) +return 0 ; +if (ret ==0 ) + ret = -ENODEV ;return ret; }
@@ -115,10 +159,8 @@ static int bmi_probe(struct platform_device *pdev)static int bmi_remove (struct platform_device *pdev) {struct bmi *bmi =platform_get_drvdata (pdev); -int i; -for (i =0 ; i < bmi->i2c_num; i++) -i2c_unregister_device (bmi->i2c_devs[i]); +bmi_devs_unregister (bmi);return 0 ; }
Signature · 1 line
2.25.1
Re: [PATCH v4 6/9] platform/x86: bus-multi-instantiate: Reorganize I2C functions
Hi,
Quoted 65 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Reorganize I2C functions to accommodate SPI support Split the probe and factor out parts of the code that will be used in the SPI support
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>--- drivers/platform/x86/bus-multi-instantiate.c | 150 ++++++++++++------- 1 file changed, 96 insertions(+), 54 deletions(-)drivers/platform/x86/bus-multi-instantiate.c +41
-2@@ -29,85 +29,129 @@ struct bmi_instance {struct bmi {int i2c_num; -struct i2c_client *i2c_devs[]; +struct i2c_client **i2c_devs; }; -static int bmi_probe (struct platform_device *pdev) +static int bmi_get_irq (struct platform_device *pdev,struct acpi_device *adev, +const struct bmi_instance *inst) +{ +int ret; + +switch (inst->flags &IRQ_RESOURCE_TYPE ) { +case IRQ_RESOURCE_GPIO : + ret =acpi_dev_gpio_irq_get (adev, inst->irq_idx); +break ; +case IRQ_RESOURCE_APIC : + ret =platform_get_irq (pdev, inst->irq_idx); +break ; +default : + ret =0 ; +break ; + } + +if (ret <0 ) +dev_err_probe (&pdev->dev, ret,"Error requesting irq at index %d: %d\n" , + inst->irq_idx, ret); + +return ret; +} + +static void bmi_devs_unregister (struct bmi *bmi) +{ +while (bmi->i2c_num >0 ) +i2c_unregister_device (bmi->i2c_devs[--bmi->i2c_num]); +} + +/** +* bmi_i2c_probe - Instantiate multiple I2C devices from inst array +* @pdev: Platform device +* @adev: ACPI device +* @bmi: Internal struct for Bus multi instantiate driver +* @inst: Array of instances to probe +* +* Returns the number of I2C devices instantiate, Zero if none is found or a negative error code. +*/ +static int bmi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev, struct bmi *bmi,
drivers/platform/x86/bus-multi-instantiate.c struct bmi_instance { · line 80
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +2
-4+ const struct bmi_instance *inst_array) {struct i2c_board_info board_info = {}; -const struct bmi_instance *inst;struct device *dev = &pdev->dev; -struct acpi_device *adev; -struct bmi *bmi;char name[32 ]; -int i, ret; +int i, ret =0 , count;
Nitpick no need to init ret to 0 here since you re-assing it immediately afterwards.
Quoted 75 lines
drivers/platform/x86/bus-multi-instantiate.c +15
-41- inst =device_get_match_data (dev); -if (!inst) { -dev_err (dev,"Error ACPI match data is missing\n" ); -return -ENODEV ; - } - - adev =ACPI_COMPANION (dev); - -/* Count number of clients to instantiate */ ret =i2c_acpi_client_count (adev); -if (ret <0 ) +if (ret <=0 )return ret; + count = ret; - bmi =devm_kmalloc (dev,struct_size (bmi, i2c_devs, ret),GFP_KERNEL ); -if (!bmi) + bmi->i2c_devs =devm_kcalloc (dev, count,sizeof (*bmi->i2c_devs),GFP_KERNEL ); +if (!bmi->i2c_devs)return -ENOMEM ; - bmi->i2c_num = ret; - -for (i =0 ; i < bmi->i2c_num && inst[i].type; i++) { +for (i =0 ; i < count && inst_array[i].type; i++) {memset (&board_info,0 ,sizeof (board_info)); -strlcpy (board_info.type, inst[i].type,I2C_NAME_SIZE ); -snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst[i].type, i); +strscpy (board_info.type, inst_array[i].type,I2C_NAME_SIZE ); +snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst_array[i].type, i); board_info.dev_name = name; -switch (inst[i].flags &IRQ_RESOURCE_TYPE ) { -case IRQ_RESOURCE_GPIO : - ret =acpi_dev_gpio_irq_get (adev, inst[i].irq_idx); -if (ret <0 ) { -dev_err (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -case IRQ_RESOURCE_APIC : - ret =platform_get_irq (pdev, inst[i].irq_idx); -if (ret <0 ) { -dev_dbg (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -default : - board_info.irq =0 ; -break ; - } + + ret =bmi_get_irq (pdev, adev, &inst_array[i]); +if (ret <0 ) + goto error; + board_info.irq = ret; + bmi->i2c_devs[i] =i2c_acpi_new_device (dev, i, &board_info);if (IS_ERR (bmi->i2c_devs[i])) { ret =dev_err_probe (dev,PTR_ERR (bmi->i2c_devs[i]),"Error creating i2c-client, idx %d\n" , i); goto error; } + bmi->i2c_num++; } -if (i < bmi->i2c_num) { +if (bmi->i2c_num < count) {dev_err (dev,"Error finding driver, idx %d\n" , i); ret = -ENODEV ; goto error; }
drivers/platform/x86/bus-multi-instantiate.c struct bmi_instance { · line 120
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +3
-4-platform_set_drvdata (pdev, bmi); -return 0 ; +dev_info (dev,"Instantiate %d I2C devices.\n" , bmi->i2c_num); +return bmi->i2c_num; error: -while (--i >=0 ) -i2c_unregister_device (bmi->i2c_devs[i]); +dev_err_probe (dev, ret,"I2C error %d\n" , ret);
All the ways to get here already log a better error msg, please drop this.
Quoted 40 lines
drivers/platform/x86/bus-multi-instantiate.c +34
-0+bmi_devs_unregister (bmi); + +return ret; +} + +static int bmi_probe (struct platform_device *pdev) +{ +const struct bmi_instance *inst_array; +struct device *dev = &pdev->dev; +struct acpi_device *adev; +struct bmi *bmi; +int ret; + + inst_array =device_get_match_data (dev); +if (!inst_array) { +dev_err (dev,"Error ACPI match data is missing\n" ); +return -ENODEV ; + } + + adev =ACPI_COMPANION (dev); +if (!adev) +return -ENODEV ; + + bmi =devm_kzalloc (dev,sizeof (*bmi),GFP_KERNEL ); +if (!bmi) +return -ENOMEM ; + +platform_set_drvdata (pdev, bmi); + + ret =bmi_i2c_probe (pdev, adev, bmi, inst_array); +if (ret >0 ) +return 0 ; +if (ret ==0 ) + ret = -ENODEV ;return ret; }@@ -115,10 +159,8 @@ static int bmi_probe(struct platform_device *pdev)static int bmi_remove (struct platform_device *pdev)
drivers/platform/x86/bus-multi-instantiate.c static int bmi_probe(struct platform_device *pdev) · line 166
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +1
-3{struct bmi *bmi =platform_get_drvdata (pdev); -int i; -for (i =0 ; i < bmi->i2c_num; i++) -i2c_unregister_device (bmi->i2c_devs[i]); +bmi_devs_unregister (bmi);return 0 ; }
Otherwise this looks good to me, with my 2 minor
remarsk addressed:
Regards,
Hans
Re: [PATCH v4 6/9] platform/x86: bus-multi-instantiate: Reorganize I2C functions
In reply to Stefan BindingHi,
Quoted 144 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Reorganize I2C functions to accommodate SPI support Split the probe and factor out parts of the code that will be used in the SPI support
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>--- drivers/platform/x86/bus-multi-instantiate.c | 150 ++++++++++++------- 1 file changed, 96 insertions(+), 54 deletions(-)drivers/platform/x86/bus-multi-instantiate.c +57
-46@@ -29,85 +29,129 @@ struct bmi_instance {struct bmi {int i2c_num; -struct i2c_client *i2c_devs[]; +struct i2c_client **i2c_devs; }; -static int bmi_probe (struct platform_device *pdev) +static int bmi_get_irq (struct platform_device *pdev,struct acpi_device *adev, +const struct bmi_instance *inst) +{ +int ret; + +switch (inst->flags &IRQ_RESOURCE_TYPE ) { +case IRQ_RESOURCE_GPIO : + ret =acpi_dev_gpio_irq_get (adev, inst->irq_idx); +break ; +case IRQ_RESOURCE_APIC : + ret =platform_get_irq (pdev, inst->irq_idx); +break ; +default : + ret =0 ; +break ; + } + +if (ret <0 ) +dev_err_probe (&pdev->dev, ret,"Error requesting irq at index %d: %d\n" , + inst->irq_idx, ret); + +return ret; +} + +static void bmi_devs_unregister (struct bmi *bmi) +{ +while (bmi->i2c_num >0 ) +i2c_unregister_device (bmi->i2c_devs[--bmi->i2c_num]); +} + +/** +* bmi_i2c_probe - Instantiate multiple I2C devices from inst array +* @pdev: Platform device +* @adev: ACPI device +* @bmi: Internal struct for Bus multi instantiate driver +* @inst: Array of instances to probe +* +* Returns the number of I2C devices instantiate, Zero if none is found or a negative error code. +*/ +static int bmi_i2c_probe (struct platform_device *pdev,struct acpi_device *adev,struct bmi *bmi, +const struct bmi_instance *inst_array) {struct i2c_board_info board_info = {}; -const struct bmi_instance *inst;struct device *dev = &pdev->dev; -struct acpi_device *adev; -struct bmi *bmi;char name[32 ]; -int i, ret; +int i, ret =0 , count; - inst =device_get_match_data (dev); -if (!inst) { -dev_err (dev,"Error ACPI match data is missing\n" ); -return -ENODEV ; - } - - adev =ACPI_COMPANION (dev); - -/* Count number of clients to instantiate */ ret =i2c_acpi_client_count (adev); -if (ret <0 ) +if (ret <=0 )return ret; + count = ret; - bmi =devm_kmalloc (dev,struct_size (bmi, i2c_devs, ret),GFP_KERNEL ); -if (!bmi) + bmi->i2c_devs =devm_kcalloc (dev, count,sizeof (*bmi->i2c_devs),GFP_KERNEL ); +if (!bmi->i2c_devs)return -ENOMEM ; - bmi->i2c_num = ret; - -for (i =0 ; i < bmi->i2c_num && inst[i].type; i++) { +for (i =0 ; i < count && inst_array[i].type; i++) {memset (&board_info,0 ,sizeof (board_info)); -strlcpy (board_info.type, inst[i].type,I2C_NAME_SIZE ); -snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst[i].type, i); +strscpy (board_info.type, inst_array[i].type,I2C_NAME_SIZE ); +snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst_array[i].type, i); board_info.dev_name = name; -switch (inst[i].flags &IRQ_RESOURCE_TYPE ) { -case IRQ_RESOURCE_GPIO : - ret =acpi_dev_gpio_irq_get (adev, inst[i].irq_idx); -if (ret <0 ) { -dev_err (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -case IRQ_RESOURCE_APIC : - ret =platform_get_irq (pdev, inst[i].irq_idx); -if (ret <0 ) { -dev_dbg (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -default : - board_info.irq =0 ; -break ; - } + + ret =bmi_get_irq (pdev, adev, &inst_array[i]); +if (ret <0 ) + goto error; + board_info.irq = ret; + bmi->i2c_devs[i] =i2c_acpi_new_device (dev, i, &board_info);if (IS_ERR (bmi->i2c_devs[i])) { ret =dev_err_probe (dev,PTR_ERR (bmi->i2c_devs[i]),"Error creating i2c-client, idx %d\n" , i); goto error; } + bmi->i2c_num++; }
drivers/platform/x86/bus-multi-instantiate.c struct bmi_instance { · line 116
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +2
-3-if (i < bmi->i2c_num) { +if (bmi->i2c_num < count) {dev_err (dev,"Error finding driver, idx %d\n" , i); ret = -ENODEV ; goto error; } -platform_set_drvdata (pdev, bmi); -return 0 ; +dev_info (dev,"Instantiate %d I2C devices.\n" , bmi->i2c_num);
Bonus nitpick, please change this to:
dev_info(dev, "Instantiated %d I2C devices.\n", bmi->i2c_num);
IOW do s/Instantiate/Instantiated/
Regards,
Hans
Quoted 56 lines
drivers/platform/x86/bus-multi-instantiate.c +37
-5+return bmi->i2c_num; error: -while (--i >=0 ) -i2c_unregister_device (bmi->i2c_devs[i]); +dev_err_probe (dev, ret,"I2C error %d\n" , ret); +bmi_devs_unregister (bmi); + +return ret; +} + +static int bmi_probe (struct platform_device *pdev) +{ +const struct bmi_instance *inst_array; +struct device *dev = &pdev->dev; +struct acpi_device *adev; +struct bmi *bmi; +int ret; + + inst_array =device_get_match_data (dev); +if (!inst_array) { +dev_err (dev,"Error ACPI match data is missing\n" ); +return -ENODEV ; + } + + adev =ACPI_COMPANION (dev); +if (!adev) +return -ENODEV ; + + bmi =devm_kzalloc (dev,sizeof (*bmi),GFP_KERNEL ); +if (!bmi) +return -ENOMEM ; + +platform_set_drvdata (pdev, bmi); + + ret =bmi_i2c_probe (pdev, adev, bmi, inst_array); +if (ret >0 ) +return 0 ; +if (ret ==0 ) + ret = -ENODEV ;return ret; }@@ -115,10 +159,8 @@ static int bmi_probe(struct platform_device *pdev)static int bmi_remove (struct platform_device *pdev) {struct bmi *bmi =platform_get_drvdata (pdev); -int i; -for (i =0 ; i < bmi->i2c_num; i++) -i2c_unregister_device (bmi->i2c_devs[i]); +bmi_devs_unregister (bmi);return 0 ; }
Re: [PATCH v4 6/9] platform/x86: bus-multi-instantiate: Reorganize I2C functions
In reply to Stefan BindingHi,
Sorry some more remarks after all...
Quoted 79 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Reorganize I2C functions to accommodate SPI support Split the probe and factor out parts of the code that will be used in the SPI support
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>--- drivers/platform/x86/bus-multi-instantiate.c | 150 ++++++++++++------- 1 file changed, 96 insertions(+), 54 deletions(-)drivers/platform/x86/bus-multi-instantiate.c +43
-9@@ -29,85 +29,129 @@ struct bmi_instance {struct bmi {int i2c_num; -struct i2c_client *i2c_devs[]; +struct i2c_client **i2c_devs; }; -static int bmi_probe (struct platform_device *pdev) +static int bmi_get_irq (struct platform_device *pdev,struct acpi_device *adev, +const struct bmi_instance *inst) +{ +int ret; + +switch (inst->flags &IRQ_RESOURCE_TYPE ) { +case IRQ_RESOURCE_GPIO : + ret =acpi_dev_gpio_irq_get (adev, inst->irq_idx); +break ; +case IRQ_RESOURCE_APIC : + ret =platform_get_irq (pdev, inst->irq_idx); +break ; +default : + ret =0 ; +break ; + } + +if (ret <0 ) +dev_err_probe (&pdev->dev, ret,"Error requesting irq at index %d: %d\n" , + inst->irq_idx, ret); + +return ret; +} + +static void bmi_devs_unregister (struct bmi *bmi) +{ +while (bmi->i2c_num >0 ) +i2c_unregister_device (bmi->i2c_devs[--bmi->i2c_num]); +} + +/** +* bmi_i2c_probe - Instantiate multiple I2C devices from inst array +* @pdev: Platform device +* @adev: ACPI device +* @bmi: Internal struct for Bus multi instantiate driver +* @inst: Array of instances to probe +* +* Returns the number of I2C devices instantiate, Zero if none is found or a negative error code. +*/ +static int bmi_i2c_probe (struct platform_device *pdev,struct acpi_device *adev,struct bmi *bmi, +const struct bmi_instance *inst_array) {struct i2c_board_info board_info = {}; -const struct bmi_instance *inst;struct device *dev = &pdev->dev; -struct acpi_device *adev; -struct bmi *bmi;char name[32 ]; -int i, ret; +int i, ret =0 , count; - inst =device_get_match_data (dev); -if (!inst) { -dev_err (dev,"Error ACPI match data is missing\n" );
drivers/platform/x86/bus-multi-instantiate.c struct bmi_instance { · line 84
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +1
-7-return -ENODEV ; - } - - adev =ACPI_COMPANION (dev); - -/* Count number of clients to instantiate */ ret =i2c_acpi_client_count (adev); -if (ret <0 ) +if (ret <=0 )return ret;
Please change this to:
return ret == 0 ? -ENODEV : ret;
This helps making return value handler in the caller cleaner, also see my upcoming review of patch 7/9.
Quoted 57 lines
drivers/platform/x86/bus-multi-instantiate.c +14
-31+ count = ret; - bmi =devm_kmalloc (dev,struct_size (bmi, i2c_devs, ret),GFP_KERNEL ); -if (!bmi) + bmi->i2c_devs =devm_kcalloc (dev, count,sizeof (*bmi->i2c_devs),GFP_KERNEL ); +if (!bmi->i2c_devs)return -ENOMEM ; - bmi->i2c_num = ret; - -for (i =0 ; i < bmi->i2c_num && inst[i].type; i++) { +for (i =0 ; i < count && inst_array[i].type; i++) {memset (&board_info,0 ,sizeof (board_info)); -strlcpy (board_info.type, inst[i].type,I2C_NAME_SIZE ); -snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst[i].type, i); +strscpy (board_info.type, inst_array[i].type,I2C_NAME_SIZE ); +snprintf (name,sizeof (name),"%s-%s.%d" ,dev_name (dev), inst_array[i].type, i); board_info.dev_name = name; -switch (inst[i].flags &IRQ_RESOURCE_TYPE ) { -case IRQ_RESOURCE_GPIO : - ret =acpi_dev_gpio_irq_get (adev, inst[i].irq_idx); -if (ret <0 ) { -dev_err (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -case IRQ_RESOURCE_APIC : - ret =platform_get_irq (pdev, inst[i].irq_idx); -if (ret <0 ) { -dev_dbg (dev,"Error requesting irq at index %d: %d\n" , - inst[i].irq_idx, ret); - goto error; - } - board_info.irq = ret; -break ; -default : - board_info.irq =0 ; -break ; - } + + ret =bmi_get_irq (pdev, adev, &inst_array[i]); +if (ret <0 ) + goto error; + board_info.irq = ret; + bmi->i2c_devs[i] =i2c_acpi_new_device (dev, i, &board_info);if (IS_ERR (bmi->i2c_devs[i])) { ret =dev_err_probe (dev,PTR_ERR (bmi->i2c_devs[i]),"Error creating i2c-client, idx %d\n" , i); goto error; } + bmi->i2c_num++; } -if (i < bmi->i2c_num) { +if (bmi->i2c_num < count) {
drivers/platform/x86/bus-multi-instantiate.c struct bmi_instance { · line 118
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +2
-2dev_err (dev,"Error finding driver, idx %d\n" , i); ret = -ENODEV ; goto error; } -platform_set_drvdata (pdev, bmi); -return 0 ; +dev_info (dev,"Instantiate %d I2C devices.\n" , bmi->i2c_num); +return bmi->i2c_num;
And change this to return 0.
Quoted 30 lines
drivers/platform/x86/bus-multi-instantiate.c +27
-2error: -while (--i >=0 ) -i2c_unregister_device (bmi->i2c_devs[i]); +dev_err_probe (dev, ret,"I2C error %d\n" , ret); +bmi_devs_unregister (bmi); + +return ret; +} + +static int bmi_probe (struct platform_device *pdev) +{ +const struct bmi_instance *inst_array; +struct device *dev = &pdev->dev; +struct acpi_device *adev; +struct bmi *bmi; +int ret; + + inst_array =device_get_match_data (dev); +if (!inst_array) { +dev_err (dev,"Error ACPI match data is missing\n" ); +return -ENODEV ; + } + + adev =ACPI_COMPANION (dev); +if (!adev) +return -ENODEV ; + + bmi =devm_kzalloc (dev,sizeof (*bmi),GFP_KERNEL ); +if (!bmi) +return -ENOMEM ;
drivers/platform/x86/bus-multi-instantiate.c struct bmi_instance { · line 156
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +8
-0+ +platform_set_drvdata (pdev, bmi); + + ret =bmi_i2c_probe (pdev, adev, bmi, inst_array); +if (ret >0 ) +return 0 ; +if (ret ==0 ) + ret = -ENODEV ;return ret;
Then you can simplify the above to just:
return bmi_i2c_probe(pdev, adev, bmi, inst_array);
:)
Regards,
Hans
Quoted 14 lines
drivers/platform/x86/bus-multi-instantiate.c +1
-3}@@ -115,10 +159,8 @@ static int bmi_probe(struct platform_device *pdev)static int bmi_remove (struct platform_device *pdev) {struct bmi *bmi =platform_get_drvdata (pdev); -int i; -for (i =0 ; i < bmi->i2c_num; i++) -i2c_unregister_device (bmi->i2c_devs[i]); +bmi_devs_unregister (bmi);return 0 ; }
PATCH v4 7/9 platform/x86: bus-multi-instantiate: Add SPI support
In reply to Stefan BindingAdd support for spi bus in bus-multi-instantiate driver
Some peripherals can have either a I2C or a SPI connection to the host (but not both) but use the same HID for both types. So it is not possible to use the HID to determine whether it is I2C or SPI. The driver must check the node to see if it contains I2cSerialBus or SpiSerialBus entries.
For backwards-compatibility with the existing nodes I2C is checked first and if such entries are found ONLY I2C devices are created. Since some existing nodes that were already handled by this driver could also contain unrelated SpiSerialBus nodes that were previously ignored, and this preserves that behavior. If there is ever a need to handle a node where both I2C and SPI devices must be instantiated this can be added in future.
--- drivers/acpi/scan.c | 13 +- drivers/platform/x86/Kconfig | 10 +- drivers/platform/x86/bus-multi-instantiate.c | 200 ++++++++++++++++--- 3 files changed, 182 insertions(+), 41 deletions(-)
drivers/acpi/scan.c +7 -6
@@ -1734,12 +1734,13 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)bool is_serial_bus_slave = false;static const struct acpi_device_id ignore_serial_bus_ids[] = {/* -* These devices have multiple I2cSerialBus resources and an i2c-client -* must be instantiated for each, each with its own i2c_device_id. -* Normally we only instantiate an i2c-client for the first resource, -* using the ACPI HID as id. These special cases are handled by the -* drivers/platform/x86/bus-multi-instantiate.c driver, which knows -* which i2c_device_id to use for each resource. +* These devices have multiple I2cSerialBus/SpiSerialBus resources +* and an (i2c/spi)-client must be instantiated for each, each with +* its own i2c_device_id/spi_device_id. +* Normally we only instantiate an (i2c/spi)-client for the first +* resource, using the ACPI HID as id. These special cases are handled +* by the drivers/platform/x86/bus-multi-instantiate.c driver, which +* knows which i2c_device_id or spi_device_id to use for each resource. */ {"BSG1160" , }, {"BSG2150" , },
drivers/platform/x86/Kconfig +5 -5
@@ -991,12 +991,12 @@ config TOPSTAR_LAPTOP If you have a Topstar laptop, say Y or M here. config BUS_MULTI_INSTANTIATE - tristate "I2C multi instantiate pseudo device driver" - depends on I2C && ACPI + tristate "I2C and SPI multi instantiate pseudo device driver" + depends on I2C && SPI && ACPI help - Some ACPI-based systems list multiple i2c-devices in a single ACPI - firmware-node. This driver will instantiate separate i2c-clients - for each device in the firmware-node. + Some ACPI-based systems list multiple i2c/spi devices in a + single ACPI firmware-node. This driver will instantiate separate + i2c-clients or spi-devices for each device in the firmware-node. To compile this driver as a module, choose M here: the module will be called bus-multi-instantiate.
drivers/platform/x86/bus-multi-instantiate.c +170 -30
267 lines not on this page · raw message
Signature · 1 line
2.25.1
Re: [PATCH v4 7/9] platform/x86: bus-multi-instantiate: Add SPI support
Hi,
Quoted 140 lines · Stefan Binding
Add support for spi bus in bus-multi-instantiate driver
Some peripherals can have either a I2C or a SPI connection to the host (but not both) but use the same HID for both types. So it is not possible to use the HID to determine whether it is I2C or SPI. The driver must check the node to see if it contains I2cSerialBus or SpiSerialBus entries.
For backwards-compatibility with the existing nodes I2C is checked first and if such entries are found ONLY I2C devices are created. Since some existing nodes that were already handled by this driver could also contain unrelated SpiSerialBus nodes that were previously ignored, and this preserves that behavior. If there is ever a need to handle a node where both I2C and SPI devices must be instantiated this can be added in future.
Signed-off-by Stefan Binding <[email protected]>--- drivers/acpi/scan.c | 13 +- drivers/platform/x86/Kconfig | 10 +- drivers/platform/x86/bus-multi-instantiate.c | 200 ++++++++++++++++--- 3 files changed, 182 insertions(+), 41 deletions(-)drivers/acpi/scan.c +7
-6@@ -1734,12 +1734,13 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)bool is_serial_bus_slave = false;static const struct acpi_device_id ignore_serial_bus_ids[] = {/* -* These devices have multiple I2cSerialBus resources and an i2c-client -* must be instantiated for each, each with its own i2c_device_id. -* Normally we only instantiate an i2c-client for the first resource, -* using the ACPI HID as id. These special cases are handled by the -* drivers/platform/x86/bus-multi-instantiate.c driver, which knows -* which i2c_device_id to use for each resource. +* These devices have multiple I2cSerialBus/SpiSerialBus resources +* and an (i2c/spi)-client must be instantiated for each, each with +* its own i2c_device_id/spi_device_id. +* Normally we only instantiate an (i2c/spi)-client for the first +* resource, using the ACPI HID as id. These special cases are handled +* by the drivers/platform/x86/bus-multi-instantiate.c driver, which +* knows which i2c_device_id or spi_device_id to use for each resource. */ {"BSG1160" , }, {"BSG2150" , },drivers/platform/x86/Kconfig +5
-5@@ -991,12 +991,12 @@ config TOPSTAR_LAPTOP If you have a Topstar laptop, say Y or M here. config BUS_MULTI_INSTANTIATE - tristate "I2C multi instantiate pseudo device driver" - depends on I2C && ACPI + tristate "I2C and SPI multi instantiate pseudo device driver" + depends on I2C && SPI && ACPI help - Some ACPI-based systems list multiple i2c-devices in a single ACPI - firmware-node. This driver will instantiate separate i2c-clients - for each device in the firmware-node. + Some ACPI-based systems list multiple i2c/spi devices in a + single ACPI firmware-node. This driver will instantiate separate + i2c-clients or spi-devices for each device in the firmware-node. To compile this driver as a module, choose M here: the module will be called bus-multi-instantiate.drivers/platform/x86/bus-multi-instantiate.c +31
-1@@ -1,7 +1,7 @@// SPDX-License-Identifier: GPL-2.0+ /* * Bus multi-instantiate driver, pseudo driver to instantiate multiple -* i2c-clients from a single fwnode. +* i2c-clients or spi-devices from a single fwnode. * * Copyright 2018 Hans de Goede <[email protected]> */ @@ -14,6 +14,7 @@#include <linux/module.h> #include <linux/platform_device.h> #include <linux/property.h> +#include <linux/spi/spi.h> #include <linux/types.h> #define IRQ_RESOURCE_TYPE GENMASK(1, 0)@@ -21,15 +22,28 @@#define IRQ_RESOURCE_GPIO 1#define IRQ_RESOURCE_APIC 2 +enum bmi_bus_type { +BMI_I2C , +BMI_SPI , +BMI_AUTO_DETECT , +}; +struct bmi_instance {const char *type;unsigned int flags;int irq_idx; }; +struct bmi_node { +enum bmi_bus_type bus_type; +struct bmi_instance instances[]; +}; +struct bmi {int i2c_num; +int spi_num;struct i2c_client **i2c_devs; +struct spi_device **spi_devs; }; static int bmi_get_irq(struct platform_device *pdev, struct acpi_device *adev,@@ -60,6 +74,92 @@ static void bmi_devs_unregister(struct bmi *bmi) {while (bmi->i2c_num >0 )i2c_unregister_device (bmi->i2c_devs[--bmi->i2c_num]); + +while (bmi->spi_num >0 ) +spi_unregister_device (bmi->spi_devs[--bmi->spi_num]); +} + +/** +* bmi_spi_probe - Instantiate multiple SPI devices from inst array +* @pdev: Platform device +* @adev: ACPI device +* @bmi: Internal struct for Bus multi instantiate driver +* @inst: Array of instances to probe +* +* Returns the number of SPI devices instantiate, Zero if none is found or a negative error code. +*/ +static int bmi_spi_probe (struct platform_device *pdev,struct acpi_device *adev,struct bmi *bmi, +const struct bmi_instance *inst_array)
drivers/platform/x86/bus-multi-instantiate.c static void bmi_devs_unregister(struct bmi *bmi) · line 102
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +10
-0+{ +struct device *dev = &pdev->dev; +struct spi_controller *ctlr; +struct spi_device *spi_dev; +char name[50 ]; +int i, ret, count; + + ret =acpi_spi_count_resources (adev); +if (ret <=0 ) +return ret;
Please change this to:
return ret == 0 ? -ENODEV : ret;
This helps making return value handler in the caller cleaner.
drivers/platform/x86/bus-multi-instantiate.c static void bmi_devs_unregister(struct bmi *bmi) · line 112
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +10
-0+ count = ret; + + bmi->spi_devs =devm_kcalloc (dev, count,sizeof (*bmi->spi_devs),GFP_KERNEL ); +if (!bmi->spi_devs) +return -ENOMEM ; + +for (i =0 ; i < count && inst_array[i].type; i++) { + + spi_dev =acpi_spi_device_alloc (NULL , adev, i, inst_array[i].irq_idx); +if (IS_ERR (spi_dev)) {
Maybe put a dev_probe_error() here ?
drivers/platform/x86/bus-multi-instantiate.c static void bmi_devs_unregister(struct bmi *bmi) · line 121
Quoted 9 lines
drivers/platform/x86/bus-multi-instantiate.c +9
-0+ ret =PTR_ERR (spi_dev); + goto error; + } + + ctlr = spi_dev->controller; + +strscpy (spi_dev->modalias, inst_array[i].type,sizeof (spi_dev->modalias)); + +if (spi_dev->irq <0 ) {
As already mentioned please drop this if. This will also allow overriding a potentially bad IRQ from the ACPI tables (or one only valid for 1 of the SPI clients) with IRQ_RESOURCE_NONE.
Quoted 23 lines
drivers/platform/x86/bus-multi-instantiate.c +23
-0+ ret =bmi_get_irq (pdev, adev, &inst_array[i]); +if (ret <0 ) { +spi_dev_put (spi_dev); + goto error; + } + spi_dev->irq = ret; + } + +snprintf (name,sizeof (name),"%s-%s-%s.%d" ,dev_name (&ctlr->dev),dev_name (dev), + inst_array[i].type, i); + spi_dev->dev.init_name = name; + + ret =spi_add_device (spi_dev); +if (ret) { +dev_err (&ctlr->dev,"failed to add SPI device %s from ACPI: %d\n" , +dev_name (&adev->dev), ret); +spi_dev_put (spi_dev); + goto error; + } + +dev_dbg (dev,"SPI device %s using chip select %u" , name, spi_dev->chip_select); + + bmi->spi_devs[i] = spi_dev;
drivers/platform/x86/bus-multi-instantiate.c static void bmi_devs_unregister(struct bmi *bmi) · line 154
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +10
-0+ bmi->spi_num++; + } + +if (bmi->spi_num < count) { +dev_err (dev,"Error finding driver, idx %d\n" , i); + ret = -ENODEV ; + goto error; + } + +dev_info (dev,"Instantiate %d SPI devices.\n" , bmi->spi_num);
Nitpick, please change this to:
dev_info(dev, "Instantiated %d SPI devices.\n", bmi->spi_num);
IOW do s/Instantiate/Instantiated/
drivers/platform/x86/bus-multi-instantiate.c static void bmi_devs_unregister(struct bmi *bmi) · line 156
Quoted 2 lines
drivers/platform/x86/bus-multi-instantiate.c +2
-0+ +return bmi->spi_num;
And change this to return 0.
drivers/platform/x86/bus-multi-instantiate.c static void bmi_devs_unregister(struct bmi *bmi) · line 159
Quoted 3 lines
drivers/platform/x86/bus-multi-instantiate.c +3
-0+error: +bmi_devs_unregister (bmi); +dev_err_probe (dev, ret,"SPI error %d\n" , ret);
Please drop the dev_err_probe here, with the dev_err_probe() which I suggested above all the error paths already print a more detailed error.
Quoted 62 lines
drivers/platform/x86/bus-multi-instantiate.c +35
-8+ +return ret; + }/** @@ -125,14 +225,14 @@ static int bmi_i2c_probe(struct platform_device *pdev, struct acpi_device *adev,static int bmi_probe (struct platform_device *pdev) { -const struct bmi_instance *inst_array;struct device *dev = &pdev->dev; +const struct bmi_node *node;struct acpi_device *adev;struct bmi *bmi; -int ret; +int i2c_ret =0 , spi_ret =0 ; - inst_array =device_get_match_data (dev); -if (!inst_array) { + node =device_get_match_data (dev); +if (!node) {dev_err (dev,"Error ACPI match data is missing\n" );return -ENODEV ; }@@ -147,13 +247,44 @@ static int bmi_probe(struct platform_device *pdev)platform_set_drvdata (pdev, bmi); - ret =bmi_i2c_probe (pdev, adev, bmi, inst_array); -if (ret >0 ) +/* Each time this driver probes only one type of bus will be chosen. +* And I2C has preference, which means that if find a I2cSerialBus it assumes +* that all following devices will also be I2C. +* In case there are zero I2C devices, it assumes that all following devices are SPI. +*/ +if (node->bus_type !=BMI_SPI ) { + i2c_ret =bmi_i2c_probe (pdev, adev, bmi, node->instances); +if (i2c_ret >0 ) +return 0 ; +else if (i2c_ret == -EPROBE_DEFER ) +return i2c_ret; +if (node->bus_type ==BMI_I2C ) { +if (i2c_ret ==0 ) +return -ENODEV ; +else +return i2c_ret; + } + } +/* BMI_SPI or BMI_AUTO_DETECT */ + spi_ret =bmi_spi_probe (pdev, adev, bmi, node->instances); +if (spi_ret >0 )return 0 ; -if (ret ==0 ) - ret = -ENODEV ; +else if (spi_ret == -EPROBE_DEFER ) +return -EPROBE_DEFER ; +if (node->bus_type ==BMI_SPI ) { +if (spi_ret ==0 ) +return -ENODEV ; +else +return spi_ret;
drivers/platform/x86/bus-multi-instantiate.c static int bmi_probe(struct platform_device *pdev) · line 287
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +8
-1+ } -return ret; +/* The only way to get here is BMI_AUTO_DETECT and i2c_ret <= 0 and spi_ret <= 0 */ +if (i2c_ret ==0 && spi_ret ==0 ) +return -ENODEV ; +else if (i2c_ret ==0 && spi_ret) +return spi_ret; + +return i2c_ret;
Hmm, the above is rather messy and hard to read. With the changed return values I suggested earlier this can be changed to:
switch (node->bus_type) {
case BMI_I2C:
return bmi_i2c_probe(pdev, adev, bmi, node->instances);
case BMI_SPI:
return bmi_spi_probe(pdev, adev, bmi, node->instances);
case BMI_AUTO_DETECT:
if (i2c_acpi_client_count(adev))
return bmi_i2c_probe(pdev, adev, bmi, node->instances);
else
return bmi_spi_probe(pdev, adev, bmi, node->instances);
}
return 0; /* never reached */
}Which IMHO is much cleaner / easier to follow.
Note this has one functional change compared to your original code, when doing auto-detect and the i2c device instantiating fails with an error other then -EPROBE_DEFER we now fail the entire probe (propagate the error to our caller).
Where as before for auto-detect bmi_i2c_probe() errors other then -EPROBE_DEFER would lead to falling back to SPI probing which does not seem to be what we want.
Quoted 59 lines
drivers/platform/x86/bus-multi-instantiate.c +26
-17static int bmi_remove (struct platform_device *pdev)@@ -165,27 +296,36 @@ static int bmi_remove(struct platform_device *pdev)return 0 ; } -static const struct bmi_instance bsg1160_data[] = { - {"bmc150_accel" ,IRQ_RESOURCE_GPIO ,0 }, - {"bmc150_magn" }, - {"bmg160" }, - {} +static const struct bmi_node bsg1160_data = { + .instances = { + {"bmc150_accel" ,IRQ_RESOURCE_GPIO ,0 }, + {"bmc150_magn" }, + {"bmg160" }, + {} + }, + .bus_type =BMI_I2C , }; -static const struct bmi_instance bsg2150_data[] = { - {"bmc150_accel" ,IRQ_RESOURCE_GPIO ,0 }, - {"bmc150_magn" }, -/* The resources describe a 3th client, but it is not really there. */ - {"bsg2150_dummy_dev" }, - {} +static const struct bmi_node bsg2150_data = { + .instances = { + {"bmc150_accel" ,IRQ_RESOURCE_GPIO ,0 }, + {"bmc150_magn" }, +/* The resources describe a 3th client, but it is not really there. */ + {"bsg2150_dummy_dev" }, + {} + }, + .bus_type =BMI_I2C , }; -static const struct bmi_instance int3515_data[] = { - {"tps6598x" ,IRQ_RESOURCE_APIC ,0 }, - {"tps6598x" ,IRQ_RESOURCE_APIC ,1 }, - {"tps6598x" ,IRQ_RESOURCE_APIC ,2 }, - {"tps6598x" ,IRQ_RESOURCE_APIC ,3 }, - {} +static const struct bmi_node int3515_data = { + .instances = { + {"tps6598x" ,IRQ_RESOURCE_APIC ,0 }, + {"tps6598x" ,IRQ_RESOURCE_APIC ,1 }, + {"tps6598x" ,IRQ_RESOURCE_APIC ,2 }, + {"tps6598x" ,IRQ_RESOURCE_APIC ,3 }, + {} + }, + .bus_type =BMI_I2C , };/* @@ -193,9 +333,9 @@ static const struct bmi_instance int3515_data[] = {* drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). */
drivers/platform/x86/bus-multi-instantiate.c static const struct bmi_instance int3515_data[] = { · line 341
Quoted 10 lines
drivers/platform/x86/bus-multi-instantiate.c +3
-3static const struct acpi_device_id bmi_acpi_ids[] = { - {"BSG1160" , (unsigned long )bsg1160_data }, - {"BSG2150" , (unsigned long )bsg2150_data }, - {"INT3515" , (unsigned long )int3515_data }, + {"BSG1160" , (unsigned long )&bsg1160_data }, + {"BSG2150" , (unsigned long )&bsg2150_data }, + {"INT3515" , (unsigned long )&int3515_data }, { } };MODULE_DEVICE_TABLE (acpi, bmi_acpi_ids);
Regards,
Hans
PATCH v4 8/9 ALSA: hda/realtek: Add support for HP Laptops
In reply to Stefan BindingFrom: Lucas Tanure <[email protected]>
Add support for two and four CS35L41 using the component binding method
--- sound/pci/hda/patch_realtek.c | 43 ++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)
sound/pci/hda/patch_realtek.c +42 -1
@@ -6611,6 +6611,16 @@ static void cs35l41_fixup_i2c_two(struct hda_codec *cdc, const struct hda_fixupcs35l41_generic_fixup (cdc, action,"i2c" ,"CSC3551" ,2 ); } +static void cs35l41_fixup_spi_two (struct hda_codec *codec,const struct hda_fixup *fix,int action) +{ +cs35l41_generic_fixup (codec, action,"spi0" ,"CSC3551" ,2 ); +} + +static void cs35l41_fixup_spi_four (struct hda_codec *codec,const struct hda_fixup *fix,int action) +{ +cs35l41_generic_fixup (codec, action,"spi0" ,"CSC3551" ,4 ); +} +static void alc287_legion_16achg6_playback_hook (struct hda_pcm_stream *hinfo,struct hda_codec *cdc,struct snd_pcm_substream *sub,int action) {
@@ -6948,6 +6958,9 @@ enum {ALC285_FIXUP_LEGION_Y9000X_AUTOMUTE ,ALC287_FIXUP_LEGION_16ACHG6 ,ALC287_FIXUP_CS35L41_I2C_2 , +ALC245_FIXUP_CS35L41_SPI_2 , +ALC245_FIXUP_CS35L41_SPI_4 , +ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED , ALC285_FIXUP_HP_SPEAKERS_MICMUTE_LED, };
@@ -8699,6 +8712,20 @@ static const struct hda_fixup alc269_fixups[] = { .type = HDA_FIXUP_FUNC, .v.func = cs35l41_fixup_i2c_two, }, + [ALC245_FIXUP_CS35L41_SPI_2] = { + .type = HDA_FIXUP_FUNC, + .v.func = cs35l41_fixup_spi_two, + }, + [ALC245_FIXUP_CS35L41_SPI_4] = { + .type = HDA_FIXUP_FUNC, + .v.func = cs35l41_fixup_spi_four, + }, + [ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED] = { + .type = HDA_FIXUP_FUNC, + .v.func = alc245_fixup_hp_gpio_led, + .chained = true, + .chain_id = ALC245_FIXUP_CS35L41_SPI_4, + }, [ALC285_FIXUP_HP_SPEAKERS_MICMUTE_LED] = { .type = HDA_FIXUP_VERBS, .v.verbs = (const struct hda_verb []) {
@@ -8926,8 +8953,22 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {SND_PCI_QUIRK (0x103c ,0x8896 ,"HP EliteBook 855 G8 Notebook PC" ,ALC285_FIXUP_HP_MUTE_LED ),SND_PCI_QUIRK (0x103c ,0x8898 ,"HP EliteBook 845 G8 Notebook PC" ,ALC285_FIXUP_HP_LIMIT_INT_MIC_BOOST ),SND_PCI_QUIRK (0x103c ,0x88d0 ,"HP Pavilion 15-eh1xxx (mainboard 88D0)" ,ALC287_FIXUP_HP_GPIO_LED ), -SND_PCI_QUIRK (0x103c ,0x89c3 ,"HP" ,ALC285_FIXUP_HP_GPIO_LED ),SND_PCI_QUIRK (0x103c ,0x89ca ,"HP" ,ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF ), +SND_PCI_QUIRK (0x103c ,0x896E ,"HP EliteBook x360 830 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8971 ,"HP EliteBook 830 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8972 ,"HP EliteBook 840 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8973 ,"HP EliteBook 860 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8974 ,"HP EliteBook 840 Aero G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8975 ,"HP EliteBook x360 840 Aero G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8981 ,"HP Elite Dragonfly G3" ,ALC245_FIXUP_CS35L41_SPI_4 ), +SND_PCI_QUIRK (0x103c ,0x898E ,"HP EliteBook 835 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x898F ,"HP EliteBook 835 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8991 ,"HP EliteBook 845 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8992 ,"HP EliteBook 845 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8994 ,"HP EliteBook 855 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8995 ,"HP EliteBook 855 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x89C3 ,"Zbook Studio G9" ,ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED ), +SND_PCI_QUIRK (0x103c ,0x89C6 ,"Zbook Fury 17 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ),SND_PCI_QUIRK (0x1043 ,0x103e ,"ASUS X540SA" ,ALC256_FIXUP_ASUS_MIC ),SND_PCI_QUIRK (0x1043 ,0x103f ,"ASUS TX300" ,ALC282_FIXUP_ASUS_TX300 ), SND_PCI_QUIRK(0x1043, 0x106d, "Asus K53BE", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
Signature · 1 line
2.25.1
Re: [PATCH v4 8/9] ALSA: hda/realtek: Add support for HP Laptops
Quoted 8 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Add support for two and four CS35L41 using the component binding method
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>
I guess this patch won't be applicable until ACPI or other tree syncs with the latest Linus tree or sound git tree. Must this patch be merged with other patches?
About the code change:
Quoted 5 lines
sound/pci/hda/patch_realtek.c +5
-0+SND_PCI_QUIRK (0x103c ,0x896E ,"HP EliteBook x360 830 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8971 ,"HP EliteBook 830 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8972 ,"HP EliteBook 840 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8973 ,"HP EliteBook 860 G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), + SND_PCI_QUIRK(0x103c, 0x8974, "HP EliteBook 840 Aero G9", ALC245_FIXUP_CS35L41_SPI_2),
sound/pci/hda/patch_realtek.c static const struct snd_pci_quirk alc269_fixup_tbl[] = { · line 8971
Quoted 10 lines
sound/pci/hda/patch_realtek.c +10
-0+SND_PCI_QUIRK (0x103c ,0x8975 ,"HP EliteBook x360 840 Aero G9" ,ALC245_FIXUP_CS35L41_SPI_2 ), +SND_PCI_QUIRK (0x103c ,0x8981 ,"HP Elite Dragonfly G3" ,ALC245_FIXUP_CS35L41_SPI_4 ), +SND_PCI_QUIRK (0x103c ,0x898E ,"HP EliteBook 835 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x898F ,"HP EliteBook 835 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8991 ,"HP EliteBook 845 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8992 ,"HP EliteBook 845 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8994 ,"HP EliteBook 855 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x8995 ,"HP EliteBook 855 G9" ,ALC287_FIXUP_CS35L41_I2C_2 ), +SND_PCI_QUIRK (0x103c ,0x89C3 ,"Zbook Studio G9" ,ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED ), + SND_PCI_QUIRK(0x103c, 0x89C6, "Zbook Fury 17 G9", ALC245_FIXUP_CS35L41_SPI_2),
Please put the entries at the right places to keep the table in SSID order.
thanks,
Takashi
RE: [PATCH v4 8/9] ALSA: hda/realtek: Add support for HP Laptops
Hi Takashi,
Quoted 6 lines
-----Original Message-----
From: Alsa-devel <[email protected]> On Behalf Of Takashi Iwai
Sent: 20 January 2022 15:27
To: Stefan Binding <[email protected]>
Cc: [email protected]; [email protected];
linux-
Quoted 16 lines
[email protected]; Lucas Tanure <[email protected]>; Rafael J . Wysocki <[email protected]>; [email protected]; Takashi Iwai <[email protected]>; Mark Gross <[email protected]>; Hans de Goede <[email protected]>; Mark Brown <[email protected]>; [email protected]; [email protected]; Len Brown <[email protected]>
Subject: Re: [PATCH v4 8/9] ALSA: hda/realtek: Add support for HP LaptopsQuoted 6 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Add support for two and four CS35L41 using the component binding method
Quoted 6 lines
Quoted 2 lines
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>I guess this patch won't be applicable until ACPI or other tree syncs with the latest Linus tree or sound git tree. Must this patch be merged with other patches?
In order for these laptops to work correctly, it will need all of the patches merged,
including both this change and the ACPI changes - without the ACPI changes, some of the amps will not probe.
Thanks,
Stefan
Re: [PATCH v4 8/9] ALSA: hda/realtek: Add support for HP Laptops
Quoted 29 lines · Stefan Binding
Hi Takashi,
Quoted 6 lines
-----Original Message-----
From: Alsa-devel <[email protected]> On Behalf Of Takashi Iwai
Sent: 20 January 2022 15:27
To: Stefan Binding <[email protected]>
Cc: [email protected]; [email protected];linux-
Quoted 19 lines
[email protected]; Lucas Tanure <[email protected]>; Rafael J . Wysocki <[email protected]>; [email protected]; Takashi Iwai <[email protected]>; Mark Gross <[email protected]>; Hans de Goede <[email protected]>; Mark Brown <[email protected]>; [email protected]; [email protected]; Len Brown <[email protected]>
Subject: Re: [PATCH v4 8/9] ALSA: hda/realtek: Add support for HP LaptopsQuoted 8 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
Add support for two and four CS35L41 using the component binding method
Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>
Quoted 8 lines
Quoted 3 lines
I guess this patch won't be applicable until ACPI or other tree syncs with the latest Linus tree or sound git tree. Must this patch be merged with other patches?
In order for these laptops to work correctly, it will need all of the patches merged,
including both this change and the ACPI changes - without the ACPI changes, some of the amps will not probe.
Well, the question is whether merging it would break other things or not.
Currently the sound on the laptop won't work anyway, and applying this in other tree would need the back-pull of existing HD-audio changes from sound.git tree or Linus treee, which is often cumbersome. The goal is to plumb everything up in 5.17-rc as soon as possible, and it's often faster if each tree concentrates on only the responsible bits.
Takashi
PATCH v4 9/9 ACPI / scan: Create platform device for CS35L41
In reply to Stefan BindingFrom: Lucas Tanure <[email protected]>
The ACPI device with CSC3551 or CLSA0100 are sound cards with multiple instances of CS35L41 connectec by I2C or SPI to the main CPU.
We add an ID to the ignore_serial_bus_ids list to enumerate all I2C or SPI devices correctly.
--- drivers/acpi/scan.c | 3 +++ drivers/platform/x86/bus-multi-instantiate.c | 14 ++++++++++++++ 2 files changed, 17 insertions(+)
drivers/acpi/scan.c +3 -0
@@ -1744,8 +1744,11 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)*/ {"BSG1160", }, {"BSG2150", }, + {"CSC3551", }, {"INT33FE", }, {"INT3515", }, + /* Non-conforming _HID for Cirrus Logic already released */ + {"CLSA0100", },/* * HIDs of device with an UartSerialBusV2 resource for which userspace * expects a regular tty cdev to be created (instead of the in kernel
drivers/platform/x86/bus-multi-instantiate.c +14 -0
@@ -328,6 +328,17 @@ static const struct bmi_node int3515_data = { .bus_type = BMI_I2C, }; +static const struct bmi_node cs35l41_hda = { + .instances = { + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {} + }, + .bus_type =BMI_AUTO_DETECT , +}; +/* * Note new device-ids must also be added to bus_multi_instantiate_ids in * drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
@@ -336,6 +347,9 @@ static const struct acpi_device_id bmi_acpi_ids[] = { { "BSG1160", (unsigned long)&bsg1160_data }, { "BSG2150", (unsigned long)&bsg2150_data }, { "INT3515", (unsigned long)&int3515_data }, + { "CSC3551", (unsigned long)&cs35l41_hda }, +/* Non-conforming _HID for Cirrus Logic already released */ + { "CLSA0100", (unsigned long)&cs35l41_hda }, { } };MODULE_DEVICE_TABLE (acpi, bmi_acpi_ids);
Signature · 1 line
2.25.1
Re: [PATCH v4 9/9] ACPI / scan: Create platform device for CS35L41
Hi,
Quoted 11 lines · Stefan Binding
From: Lucas Tanure <[email protected]>
The ACPI device with CSC3551 or CLSA0100 are sound cards with multiple instances of CS35L41 connectec by I2C or SPI to the main CPU.
We add an ID to the
ignore_serial_bus_idslist to enumerate all I2C or SPI devices correctly.Signed-off-by Lucas Tanure <[email protected]>Signed-off-by Stefan Binding <[email protected]>
Thanks, patch looks good to me:
Regards,
Hans
Quoted 54 lines
--- drivers/acpi/scan.c | 3 +++ drivers/platform/x86/bus-multi-instantiate.c | 14 ++++++++++++++ 2 files changed, 17 insertions(+)drivers/acpi/scan.c +3
-0@@ -1744,8 +1744,11 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)*/ {"BSG1160", }, {"BSG2150", }, + {"CSC3551", }, {"INT33FE", }, {"INT3515", }, + /* Non-conforming _HID for Cirrus Logic already released */ + {"CLSA0100", },/* * HIDs of device with an UartSerialBusV2 resource for which userspace * expects a regular tty cdev to be created (instead of the in kernel drivers/platform/x86/bus-multi-instantiate.c +14
-0@@ -328,6 +328,17 @@ static const struct bmi_node int3515_data = { .bus_type = BMI_I2C, }; +static const struct bmi_node cs35l41_hda = { + .instances = { + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {"cs35l41-hda" ,IRQ_RESOURCE_GPIO ,0 }, + {} + }, + .bus_type =BMI_AUTO_DETECT , +}; +/* * Note new device-ids must also be added to bus_multi_instantiate_ids in * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). @@ -336,6 +347,9 @@ static const struct acpi_device_id bmi_acpi_ids[] = { { "BSG1160", (unsigned long)&bsg1160_data }, { "BSG2150", (unsigned long)&bsg2150_data }, { "INT3515", (unsigned long)&int3515_data }, + { "CSC3551", (unsigned long)&cs35l41_hda }, +/* Non-conforming _HID for Cirrus Logic already released */ + { "CLSA0100", (unsigned long)&cs35l41_hda }, { } };MODULE_DEVICE_TABLE (acpi, bmi_acpi_ids);
Hi Mark,
Quoted 18 lines · Stefan Binding
Add support for SPI bus in the i2c-multi-instantiate driver as upcoming laptops will need to multi instantiate SPI devices from a single device node, which has multiple SpiSerialBus entries at the ACPI table.
With the new SPI support, i2c-multi-instantiate becomes bus-multi-instantiate and is moved to the ACPI folder.
The intention is to support the SPI bus by re-using the current I2C multi instantiate, instead of creating a new SPI multi instantiate, to make it possible for peripherals that can be controlled by I2C or SPI to have the same HID at the ACPI table.
The new driver (Bus multi instantiate, bmi) checks for the hard-coded bus type and returns
-ENODEVin case of zero devices found for that bus. In the case of automatic bus detection, the driver will give preference to I2C.
Quoted 7 lines
The expectation is for a device node in the ACPI table to have multiple I2cSerialBus only or multiple SpiSerialBus only, not a mix of both; and for the case where there are both entries in one device node, only the I2C ones would be probed.
This new bus multi instantiate will be used in CS35L41 HDA new driver.
Mark, since most of my review remarks are small(ish) I expect the next version of this (except patch 8/9) to be ready for merging.
I can either merge all patches on top of 5.17-rc1 once released; and provide an immutable-branch for you to merge for the SPI bits; or you can merge patches 1-4 (the SPI patches) and then send me a pull-req for an immutable-branch with those 4, so that I can merge that and then the rest on top.
Mark, please let me know how you want to proceed with merging this.
Regards,
Hans
Quoted 32 lines
Changes since V2:
- Moved bus-multi-instantiate back into platform/x86Lucas Tanure (4):
platform/x86: i2c-multi-instantiate: Rename it for a generic bus
driver name
platform/x86: bus-multi-instantiate: Reorganize I2C functions
ALSA: hda/realtek: Add support for HP Laptops
ACPI / scan: Create platform device for CS35L41Stefan Binding (5):
spi: Makespi_alloc_deviceandspi_add_devicepublic again
spi: Create helper API to lookup ACPI info for spi device
spi: Support selection of the index of the ACPI Spi Resource before
alloc
spi: Add API to count spi acpi resources
platform/x86: bus-multi-instantiate: Add SPI supportMAINTAINERS | 4 +- drivers/acpi/scan.c | 16 +- drivers/platform/x86/Kconfig | 14 +- drivers/platform/x86/Makefile | 2 +- drivers/platform/x86/bus-multi-instantiate.c | 369 +++++++++++++++++++ drivers/platform/x86/i2c-multi-instantiate.c | 174 --------- drivers/spi/spi.c | 142 ++++++- include/linux/spi/spi.h | 32 ++ sound/pci/hda/patch_realtek.c | 43 ++- 9 files changed, 588 insertions(+), 208 deletions(-) create mode 100644 drivers/platform/x86/bus-multi-instantiate.c delete mode 100644 drivers/platform/x86/i2c-multi-instantiate.c