* [PATCH 02/10] Input: lpc32xx-keys - Convert to devm_platform_ioremap_resource()
From: Yangtao Li @ 2023-07-05 5:23 UTC (permalink / raw)
To: Dmitry Torokhov, Vladimir Zapolskiy
Cc: Yangtao Li, linux-input, linux-arm-kernel, linux-kernel
In-Reply-To: <20230705052346.39337-1-frank.li@vivo.com>
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
drivers/input/keyboard/lpc32xx-keys.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
index 911e1181cd6f..322a87807159 100644
--- a/drivers/input/keyboard/lpc32xx-keys.c
+++ b/drivers/input/keyboard/lpc32xx-keys.c
@@ -160,17 +160,10 @@ static int lpc32xx_kscan_probe(struct platform_device *pdev)
{
struct lpc32xx_kscan_drv *kscandat;
struct input_dev *input;
- struct resource *res;
size_t keymap_size;
int error;
int irq;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "failed to get platform I/O memory\n");
- return -EINVAL;
- }
-
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return -EINVAL;
@@ -221,7 +214,7 @@ static int lpc32xx_kscan_probe(struct platform_device *pdev)
input_set_drvdata(kscandat->input, kscandat);
- kscandat->kscan_base = devm_ioremap_resource(&pdev->dev, res);
+ kscandat->kscan_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(kscandat->kscan_base))
return PTR_ERR(kscandat->kscan_base);
--
2.39.0
^ permalink raw reply related
* [PATCH 01/10] Input: bcm-keypad - Convert to devm_platform_ioremap_resource()
From: Yangtao Li @ 2023-07-05 5:23 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Yangtao Li, linux-input, linux-kernel
Use devm_platform_ioremap_resource() to simplify code.
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
drivers/input/keyboard/bcm-keypad.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
index 56a919ec23b5..92e1fbb092b8 100644
--- a/drivers/input/keyboard/bcm-keypad.c
+++ b/drivers/input/keyboard/bcm-keypad.c
@@ -307,7 +307,6 @@ static int bcm_kp_probe(struct platform_device *pdev)
{
struct bcm_kp *kp;
struct input_dev *input_dev;
- struct resource *res;
int error;
kp = devm_kzalloc(&pdev->dev, sizeof(*kp), GFP_KERNEL);
@@ -353,14 +352,7 @@ static int bcm_kp_probe(struct platform_device *pdev)
return error;
}
- /* Get the KEYPAD base address */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "Missing keypad base address resource\n");
- return -ENODEV;
- }
-
- kp->base = devm_ioremap_resource(&pdev->dev, res);
+ kp->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(kp->base))
return PTR_ERR(kp->base);
--
2.39.0
^ permalink raw reply related
* Re: [PATCH] Input: exc3000 - add ACPI support for EXC80H60
From: kernel test robot @ 2023-07-04 19:00 UTC (permalink / raw)
To: Andreas Helbech Kleist, Dmitry Torokhov
Cc: llvm, oe-kbuild-all, Andreas Helbech Kleist, linux-input,
linux-kernel
In-Reply-To: <20230704105021.898555-1-andreaskleist@gmail.com>
Hi Andreas,
kernel test robot noticed the following build errors:
[auto build test ERROR on v6.4]
[also build test ERROR on linus/master next-20230704]
[cannot apply to dtor-input/next dtor-input/for-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andreas-Helbech-Kleist/Input-exc3000-add-ACPI-support-for-EXC80H60/20230704-185127
base: v6.4
patch link: https://lore.kernel.org/r/20230704105021.898555-1-andreaskleist%40gmail.com
patch subject: [PATCH] Input: exc3000 - add ACPI support for EXC80H60
config: x86_64-randconfig-x001-20230703 (https://download.01.org/0day-ci/archive/20230705/202307050258.WtWKQJdV-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230705/202307050258.WtWKQJdV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307050258.WtWKQJdV-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/input/touchscreen/exc3000.c:460:31: error: incompatible pointer to integer conversion initializing 'kernel_ulong_t' (aka 'unsigned long') with an expression of type 'struct eeti_dev_info *' [-Wint-conversion]
{ "EGA00001", .driver_data = &exc3000_info[EETI_EXC80H60] },
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
vim +460 drivers/input/touchscreen/exc3000.c
457
458 #ifdef CONFIG_ACPI
459 static const struct acpi_device_id exc3000_acpi_match[] = {
> 460 { "EGA00001", .driver_data = &exc3000_info[EETI_EXC80H60] },
461 { }
462 };
463 MODULE_DEVICE_TABLE(acpi, exc3000_acpi_match);
464 #endif
465
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] Input: exc3000 - add ACPI support for EXC80H60
From: kernel test robot @ 2023-07-04 17:27 UTC (permalink / raw)
To: Andreas Helbech Kleist, Dmitry Torokhov
Cc: oe-kbuild-all, Andreas Helbech Kleist, linux-input, linux-kernel
In-Reply-To: <20230704105021.898555-1-andreaskleist@gmail.com>
Hi Andreas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.4]
[also build test WARNING on linus/master next-20230704]
[cannot apply to dtor-input/next dtor-input/for-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andreas-Helbech-Kleist/Input-exc3000-add-ACPI-support-for-EXC80H60/20230704-185127
base: v6.4
patch link: https://lore.kernel.org/r/20230704105021.898555-1-andreaskleist%40gmail.com
patch subject: [PATCH] Input: exc3000 - add ACPI support for EXC80H60
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230705/202307050124.E5Ze5uux-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230705/202307050124.E5Ze5uux-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307050124.E5Ze5uux-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/input/touchscreen/exc3000.c:460:38: warning: initialization of 'long unsigned int' from 'struct eeti_dev_info *' makes integer from pointer without a cast [-Wint-conversion]
460 | { "EGA00001", .driver_data = &exc3000_info[EETI_EXC80H60] },
| ^
drivers/input/touchscreen/exc3000.c:460:38: note: (near initialization for 'exc3000_acpi_match[0].driver_data')
vim +460 drivers/input/touchscreen/exc3000.c
457
458 #ifdef CONFIG_ACPI
459 static const struct acpi_device_id exc3000_acpi_match[] = {
> 460 { "EGA00001", .driver_data = &exc3000_info[EETI_EXC80H60] },
461 { }
462 };
463 MODULE_DEVICE_TABLE(acpi, exc3000_acpi_match);
464 #endif
465
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] Input: exc3000 - add ACPI support for EXC80H60
From: Andreas Helbech Kleist @ 2023-07-04 10:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Andreas Helbech Kleist, linux-input, linux-kernel
EXC80H60 is used in Ambu aBox2 with ACPI _HID "EGA00001".
Snippet of from "apcidump -b; iasl ssdt2.dat" on target:
Device (TPL2)
{
Name (HID2, Zero)
Name (_HID, "EGA00001") // _HID: Hardware ID
Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)
// _CID: Compatible ID
Name (_S0W, 0x04) // _S0W: S0 Device Wake State
Name (SBFB, ResourceTemplate ()
Signed-off-by: Andreas Helbech Kleist <andreaskleist@gmail.com>
---
drivers/input/touchscreen/exc3000.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
index 69eae79e2087..f3a90155fb04 100644
--- a/drivers/input/touchscreen/exc3000.c
+++ b/drivers/input/touchscreen/exc3000.c
@@ -7,6 +7,7 @@
* minimal implementation based on egalax_ts.c and egalax_i2c.c
*/
+#include <linux/acpi.h>
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -454,10 +455,19 @@ static const struct of_device_id exc3000_of_match[] = {
MODULE_DEVICE_TABLE(of, exc3000_of_match);
#endif
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id exc3000_acpi_match[] = {
+ { "EGA00001", .driver_data = &exc3000_info[EETI_EXC80H60] },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, exc3000_acpi_match);
+#endif
+
static struct i2c_driver exc3000_driver = {
.driver = {
.name = "exc3000",
.of_match_table = of_match_ptr(exc3000_of_match),
+ .acpi_match_table = ACPI_PTR(exc3000_acpi_match),
},
.id_table = exc3000_id,
.probe_new = exc3000_probe,
--
2.34.1
^ permalink raw reply related
* [PATCH] HID: amd_sfh: Increase bitwidth to avoid shift-out-of-bounds
From: Kai-Heng Feng @ 2023-07-04 8:17 UTC (permalink / raw)
To: basavaraj.natikar
Cc: mario.limonciello, Kai-Heng Feng, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel
UBSAN complains shift-out-of-bounds on amd_sfh:
[ 7.593412] input: SYNA3105:00 06CB:CEA3 Mouse as /devices/platform/AMDI0010:00/i2c-0/i2c-SYNA3105:00/0018:06CB:CEA3.0001/input/input11
[ 7.593541] input: SYNA3105:00 06CB:CEA3 Touchpad as /devices/platform/AMDI0010:00/i2c-0/i2c-SYNA3105:00/0018:06CB:CEA3.0001/input/input12
[ 7.593625] hid-multitouch 0018:06CB:CEA3.0001: input,hidraw0: I2C HID v1.00 Mouse [SYNA3105:00 06CB:CEA3] on i2c-SYNA3105:00
[ 7.664537] ================================================================================
[ 7.664540] UBSAN: shift-out-of-bounds in drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:149:50
[ 7.664544] shift exponent 103 is too large for 64-bit type 'long unsigned int'
[ 7.664547] CPU: 5 PID: 124 Comm: kworker/5:1 Tainted: G W 6.4.0-next-20230703 #1
[ 7.664549] Hardware name: HP HP EliteBook 835 13 inch G10 Notebook PC/8C10, BIOS V83 Ver. 01.01.09 06/05/2023
[ 7.664551] Workqueue: events amd_sfh_work_buffer [amd_sfh]
[ 7.664562] Call Trace:
[ 7.664564] <TASK>
[ 7.664567] dump_stack_lvl+0x48/0x70
[ 7.664576] dump_stack+0x10/0x20
[ 7.664578] __ubsan_handle_shift_out_of_bounds+0x156/0x310
[ 7.664584] ? sched_clock_noinstr+0x9/0x10
[ 7.664589] ? sched_clock_cpu+0x12/0x1d0
[ 7.664594] ? raw_spin_rq_lock_nested+0x1e/0xa0
[ 7.664597] ? psi_group_change+0x237/0x520
[ 7.664600] float_to_int.cold+0x18/0xcf [amd_sfh]
[ 7.664606] ? __pfx_get_input_rep+0x10/0x10 [amd_sfh]
[ 7.664611] get_input_rep+0x241/0x340 [amd_sfh]
[ 7.664617] amd_sfh_work_buffer+0x91/0x190 [amd_sfh]
[ 7.664622] process_one_work+0x229/0x450
[ 7.664627] worker_thread+0x50/0x3f0
[ 7.664629] ? __pfx_worker_thread+0x10/0x10
[ 7.664632] kthread+0xf4/0x130
[ 7.664635] ? __pfx_kthread+0x10/0x10
[ 7.664638] ret_from_fork+0x29/0x50
[ 7.664644] </TASK>
[ 7.664652] ================================================================================
So increase the variable bitwidth to solve the issue.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 6f0d332ccf51..550639ef8bfe 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -134,7 +134,8 @@ static void get_common_inputs(struct common_input_property *common, int report_i
static int float_to_int(u32 float32)
{
- int fraction, shift, mantissa, sign, exp, zeropre;
+ int fraction, shift, sign, exp, zeropre;
+ s128 mantissa;
mantissa = float32 & GENMASK(22, 0);
sign = (float32 & BIT(31)) ? -1 : 1;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 1/1] HID: Add introduction about HID for non-kernel programmers
From: Peter Hutterer @ 2023-07-04 4:03 UTC (permalink / raw)
To: Marco Morandini
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, Jonathan Corbet,
linux-doc
In-Reply-To: <fcb47559-dac0-963b-8798-dab0d6b0b636@polimi.it>
On Thu, Jun 22, 2023 at 10:37:29PM +0200, Marco Morandini wrote:
>
> >
> > shameless plug! :) I wrote this post a few years ago, feel free to
> > incorporate bits from there into here
> > https://who-t.blogspot.com/2018/12/understanding-hid-report-descriptors.html
>
>
> Damn! This is much better than all the tutorials
> I found while trying to orient
> myself, and I managed to miss it :(
>
> Would it be ok to link to it from the doc?
missed this one sorry - yes, feel free to link to it, extract parts of
it without attribution, etc.
[...]
> > Also: you now already have 2 tools to look at hid and then you use two
> > more later (hidrrd and hid-tools). I'd say it's best to just stick to
> > one tool to reduce the mental load of having to map different tool
> > outputs to what is the same base data.
>
> I'm not sure about this: I think that hidraw needs to be introduced somewhere,
> if for nothing because it's what gets consumed by hid-record .
> Furthermore, looking at samples/hidraw/hid-example.c one can learn something, if not a lot.
> For the time being I'm leaving the paragraph, moving most of it into a footnote;
> of course we can drop it if you have a strong opinion about this.
I don't :) The main question is (and I can't answer this for you)
is how much of an in-depth introduction you want to provide and at what
point prospective readers will have to go to search for more
information. At some point there has to be a cut-off but you get to
decide where that one is.
> >> +in the mouse example, and outputs.
> >> +"Output" means that the information is fed
> >> +from the device to the human; for examples,
> >> +a joystick with force feedback will have
> >> +some output.
> >> +
> >> +
> >> +Report IDs and Evdev events
> >> +===========================
> >> +
> >> +A single device can logically group
> >> +data into different, independent sets.
> >> +It is *as if* the HID presents
> >> +itself as different devices, each exchanging
> >> +its own data. The HID report descriptor is unique,
> >> +but the different reports are identified by means
> >> +of different ``Report ID`` fields. Whenever a ``Report ID``
> >> +is needed it is transmitted as the first byte of any report.
> >
> > This is a bit ambiguous since it's the collections and applications that
> > split the device into different sets, the reports are just different
> > ways of reporting data that belongs to one (or more? not 100% sure) of
> > those sets then.
> >
> > And the example below works because you have different collections on
> > your device here but that's largely orthogonal to the use of reports.
>
> I think this came from my misunderstanding of the role of Report IDs
> and Application Collections (not that I'm sure to have undertsood everything).
> The paragraph has been rewritte, please double check.
> I'm glossing on the fact that the same Collection can have different Report IDs
> inside (btw: is this correct? And if yes, is this something that is done, in practice?).
afaik it is permitted but I'm not sure how common it is. But it's not
hard to imagine that a device out there that sends button events through
one report and motion through another.
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH RESEND] input: cpcap-pwrbutton: remove initial kernel-doc notation
From: Sebastian Reichel @ 2023-07-04 0:08 UTC (permalink / raw)
To: Randy Dunlap; +Cc: linux-kernel, Dmitry Torokhov, linux-input
In-Reply-To: <20230703230005.14877-1-rdunlap@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]
Hi,
On Mon, Jul 03, 2023 at 04:00:05PM -0700, Randy Dunlap wrote:
> Change the beginning "/**" in the file to "/*" since it is not a
> kernel-doc comment. This prevents a kernel-doc warning:
>
> drivers/input/misc/cpcap-pwrbutton.c:2: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
> * CPCAP Power Button Input Driver
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
-- Sebastian
> drivers/input/misc/cpcap-pwrbutton.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -- a/drivers/input/misc/cpcap-pwrbutton.c b/drivers/input/misc/cpcap-pwrbutton.c
> --- a/drivers/input/misc/cpcap-pwrbutton.c
> +++ b/drivers/input/misc/cpcap-pwrbutton.c
> @@ -1,4 +1,4 @@
> -/**
> +/*
> * CPCAP Power Button Input Driver
> *
> * Copyright (C) 2017 Sebastian Reichel <sre@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH RESEND] input: cpcap-pwrbutton: remove initial kernel-doc notation
From: Randy Dunlap @ 2023-07-03 23:00 UTC (permalink / raw)
To: linux-kernel
Cc: Randy Dunlap, Sebastian Reichel, Dmitry Torokhov, linux-input
Change the beginning "/**" in the file to "/*" since it is not a
kernel-doc comment. This prevents a kernel-doc warning:
drivers/input/misc/cpcap-pwrbutton.c:2: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* CPCAP Power Button Input Driver
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
---
drivers/input/misc/cpcap-pwrbutton.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -- a/drivers/input/misc/cpcap-pwrbutton.c b/drivers/input/misc/cpcap-pwrbutton.c
--- a/drivers/input/misc/cpcap-pwrbutton.c
+++ b/drivers/input/misc/cpcap-pwrbutton.c
@@ -1,4 +1,4 @@
-/**
+/*
* CPCAP Power Button Input Driver
*
* Copyright (C) 2017 Sebastian Reichel <sre@kernel.org>
^ permalink raw reply
* [PATCH v1 12/12] HID: cp2112: Use octal permissions
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Octal permissions are preferred as stated in
Documentation/dev-tools/checkpatch.rst. Replace symbolic permissions
with octal permissions when creating the files.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 3c6a3be8fc02..adbe8a47cf67 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -175,7 +175,7 @@ struct cp2112_device {
};
static int gpio_push_pull = CP2112_GPIO_ALL_GPIO_MASK;
-module_param(gpio_push_pull, int, S_IRUGO | S_IWUSR);
+module_param(gpio_push_pull, int, 0644);
MODULE_PARM_DESC(gpio_push_pull, "GPIO push-pull configuration bitmask");
static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -1057,7 +1057,7 @@ static void chmod_sysfs_attrs(struct hid_device *hdev)
}
for (attr = cp2112_attr_group.attrs; *attr; ++attr) {
- umode_t mode = (buf[1] & 1) ? S_IWUSR | S_IRUGO : S_IRUGO;
+ umode_t mode = (buf[1] & 1) ? 0644: 0444;
ret = sysfs_chmod_file(&hdev->dev.kobj, *attr, mode);
if (ret < 0)
hid_err(hdev, "error chmoding sysfs file %s\n",
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 08/12] HID: cp2112: Define all GPIO mask and use it
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Define all GPIO mask and use it in the driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index c8c59d70500e..37ed7fc54770 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -32,6 +32,7 @@
#define CP2112_GPIO_GET_LENGTH 2
#define CP2112_GPIO_SET_LENGTH 3
#define CP2112_GPIO_MAX_GPIO 8
+#define CP2112_GPIO_ALL_GPIO_MASK GENMASK(7, 0)
enum {
CP2112_GPIO_CONFIG = 0x02,
@@ -173,7 +174,7 @@ struct cp2112_device {
u8 gpio_prev_state;
};
-static int gpio_push_pull = 0xFF;
+static int gpio_push_pull = CP2112_GPIO_ALL_GPIO_MASK;
module_param(gpio_push_pull, int, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(gpio_push_pull, "GPIO push-pull configuration bitmask");
@@ -226,7 +227,7 @@ static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
mutex_lock(&dev->lock);
buf[0] = CP2112_GPIO_SET;
- buf[1] = value ? 0xff : 0;
+ buf[1] = value ? CP2112_GPIO_ALL_GPIO_MASK : 0;
buf[2] = 1 << offset;
ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf,
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 11/12] HID: cp2112: Convert to DEVICE_ATTR_RW()
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Instead of custom wrapper, use DEVICE_tATTR_RW() directly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 42 ++++++++++++++++++----------------------
1 file changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 1e45f5407d09..3c6a3be8fc02 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -944,18 +944,10 @@ CP2112_CONFIG_ATTR(release_version, ({
#undef CP2112_CONFIG_ATTR
-struct cp2112_pstring_attribute {
- struct device_attribute attr;
- unsigned char report;
-};
-
-static ssize_t pstr_store(struct device *kdev,
- struct device_attribute *kattr, const char *buf,
- size_t count)
+static ssize_t pstr_store(struct device *kdev, struct device_attribute *kattr,
+ const char *buf, size_t count, int number)
{
struct hid_device *hdev = to_hid_device(kdev);
- struct cp2112_pstring_attribute *attr =
- container_of(kattr, struct cp2112_pstring_attribute, attr);
struct cp2112_string_report report;
int ret;
@@ -963,7 +955,7 @@ static ssize_t pstr_store(struct device *kdev,
ret = utf8s_to_utf16s(buf, count, UTF16_LITTLE_ENDIAN,
report.string, ARRAY_SIZE(report.string));
- report.report = attr->report;
+ report.report = number;
report.length = ret * sizeof(report.string[0]) + 2;
report.type = USB_DT_STRING;
@@ -981,17 +973,15 @@ static ssize_t pstr_store(struct device *kdev,
return count;
}
-static ssize_t pstr_show(struct device *kdev,
- struct device_attribute *kattr, char *buf)
+static ssize_t pstr_show(struct device *kdev, struct device_attribute *kattr,
+ char *buf, int number)
{
struct hid_device *hdev = to_hid_device(kdev);
- struct cp2112_pstring_attribute *attr =
- container_of(kattr, struct cp2112_pstring_attribute, attr);
struct cp2112_string_report report;
u8 length;
int ret;
- ret = cp2112_hid_get(hdev, attr->report, (u8 *)&report.contents,
+ ret = cp2112_hid_get(hdev, number, (u8 *)&report.contents,
sizeof(report.contents), HID_FEATURE_REPORT);
if (ret < 3) {
hid_err(hdev, "error reading %s string: %d\n", kattr->attr.name,
@@ -1016,10 +1006,16 @@ static ssize_t pstr_show(struct device *kdev,
}
#define CP2112_PSTR_ATTR(name, _report) \
-static struct cp2112_pstring_attribute dev_attr_##name = { \
- .attr = __ATTR(name, (S_IWUSR | S_IRUGO), pstr_show, pstr_store), \
- .report = _report, \
-};
+static ssize_t name##_store(struct device *kdev, struct device_attribute *kattr, \
+ const char *buf, size_t count) \
+{ \
+ return pstr_store(kdev, kattr, buf, count, _report); \
+} \
+static ssize_t name##_show(struct device *kdev, struct device_attribute *kattr, char *buf) \
+{ \
+ return pstr_show(kdev, kattr, buf, _report); \
+} \
+static DEVICE_ATTR_RW(name);
CP2112_PSTR_ATTR(manufacturer, CP2112_MANUFACTURER_STRING);
CP2112_PSTR_ATTR(product, CP2112_PRODUCT_STRING);
@@ -1034,9 +1030,9 @@ static const struct attribute_group cp2112_attr_group = {
&dev_attr_max_power.attr,
&dev_attr_power_mode.attr,
&dev_attr_release_version.attr,
- &dev_attr_manufacturer.attr.attr,
- &dev_attr_product.attr.attr,
- &dev_attr_serial.attr.attr,
+ &dev_attr_manufacturer.attr,
+ &dev_attr_product.attr,
+ &dev_attr_serial.attr,
NULL
}
};
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 09/12] HID: cp2112: Use BIT() in GPIO setter and getter
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Use BIT() in GPIO setter and getter for the sake of consistency
with GENMASK() usage elsewhere in the driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 37ed7fc54770..2eebb2b19501 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -197,7 +197,7 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
goto exit;
}
- buf[1] &= ~(1 << offset);
+ buf[1] &= ~BIT(offset);
buf[2] = gpio_push_pull;
ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf,
@@ -228,7 +228,7 @@ static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
buf[0] = CP2112_GPIO_SET;
buf[1] = value ? CP2112_GPIO_ALL_GPIO_MASK : 0;
- buf[2] = 1 << offset;
+ buf[2] = BIT(offset);
ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf,
CP2112_GPIO_SET_LENGTH, HID_FEATURE_REPORT,
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 07/12] HID: cp2112: Define maximum GPIO constant and use it
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Define maximum GPIO constant and use it in the driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 45cd0d2fd3fd..c8c59d70500e 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -31,6 +31,7 @@
#define CP2112_GPIO_CONFIG_LENGTH 5
#define CP2112_GPIO_GET_LENGTH 2
#define CP2112_GPIO_SET_LENGTH 3
+#define CP2112_GPIO_MAX_GPIO 8
enum {
CP2112_GPIO_CONFIG = 0x02,
@@ -1108,7 +1109,7 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
goto exit;
gpio_mask = ret;
- for_each_set_bit(virq, &dev->irq_mask, 8) {
+ for_each_set_bit(virq, &dev->irq_mask, CP2112_GPIO_MAX_GPIO) {
irq = irq_find_mapping(dev->gc.irq.domain, virq);
if (!irq)
continue;
@@ -1295,7 +1296,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
dev->gc.set = cp2112_gpio_set;
dev->gc.get = cp2112_gpio_get;
dev->gc.base = -1;
- dev->gc.ngpio = 8;
+ dev->gc.ngpio = CP2112_GPIO_MAX_GPIO;
dev->gc.can_sleep = 1;
dev->gc.parent = &hdev->dev;
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 06/12] HID: cp2112: Remove dead code
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Remove cp2112_allocate_irq() and counterparts that seems to be
a dead code from day 1. In case somebody needs it, it can be
retrieved from Git index.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 54 ----------------------------------------
1 file changed, 54 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 15b626359281..45cd0d2fd3fd 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -17,8 +17,6 @@
*/
#include <linux/bitops.h>
-#include <linux/gpio/consumer.h>
-#include <linux/gpio/machine.h>
#include <linux/gpio/driver.h>
#include <linux/hid.h>
#include <linux/hidraw.h>
@@ -168,7 +166,6 @@ struct cp2112_device {
u8 *in_out_buffer;
struct mutex lock;
- struct gpio_desc *desc[8];
bool gpio_poll;
struct delayed_work gpio_poll_worker;
unsigned long irq_mask;
@@ -1181,51 +1178,6 @@ static int cp2112_gpio_irq_type(struct irq_data *d, unsigned int type)
return 0;
}
-static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev,
- int pin)
-{
- int ret;
-
- if (dev->desc[pin])
- return -EINVAL;
-
- dev->desc[pin] = gpiochip_request_own_desc(&dev->gc, pin,
- "HID/I2C:Event",
- GPIO_ACTIVE_HIGH,
- GPIOD_IN);
- if (IS_ERR(dev->desc[pin])) {
- dev_err(dev->gc.parent, "Failed to request GPIO\n");
- return PTR_ERR(dev->desc[pin]);
- }
-
- ret = cp2112_gpio_direction_input(&dev->gc, pin);
- if (ret < 0) {
- dev_err(dev->gc.parent, "Failed to set GPIO to input dir\n");
- goto err_desc;
- }
-
- ret = gpiochip_lock_as_irq(&dev->gc, pin);
- if (ret) {
- dev_err(dev->gc.parent, "Failed to lock GPIO as interrupt\n");
- goto err_desc;
- }
-
- ret = gpiod_to_irq(dev->desc[pin]);
- if (ret < 0) {
- dev_err(dev->gc.parent, "Failed to translate GPIO to IRQ\n");
- goto err_lock;
- }
-
- return ret;
-
-err_lock:
- gpiochip_unlock_as_irq(&dev->gc, pin);
-err_desc:
- gpiochip_free_own_desc(dev->desc[pin]);
- dev->desc[pin] = NULL;
- return ret;
-}
-
static const struct irq_chip cp2112_gpio_irqchip = {
.name = "cp2112-gpio",
.irq_startup = cp2112_gpio_irq_startup,
@@ -1390,7 +1342,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
static void cp2112_remove(struct hid_device *hdev)
{
struct cp2112_device *dev = hid_get_drvdata(hdev);
- int i;
sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
i2c_del_adapter(&dev->adap);
@@ -1400,11 +1351,6 @@ static void cp2112_remove(struct hid_device *hdev)
cancel_delayed_work_sync(&dev->gpio_poll_worker);
}
- for (i = 0; i < ARRAY_SIZE(dev->desc); i++) {
- gpiochip_unlock_as_irq(&dev->gc, i);
- gpiochip_free_own_desc(dev->desc[i]);
- }
-
gpiochip_remove(&dev->gc);
/* i2c_del_adapter has finished removing all i2c devices from our
* adapter. Well behaved devices should no longer call our cp2112_xfer
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 10/12] HID: cp2112: Use sysfs_emit() to instead of scnprintf()
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Follow the advice of the Documentation/filesystems/sysfs.rst and show()
should only use sysfs_emit() or sysfs_emit_at() when formatting the
value to be returned to user space.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 2eebb2b19501..1e45f5407d09 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -893,7 +893,7 @@ static ssize_t name##_show(struct device *kdev, \
int ret = cp2112_get_usb_config(hdev, &cfg); \
if (ret) \
return ret; \
- return scnprintf(buf, PAGE_SIZE, format, ##__VA_ARGS__); \
+ return sysfs_emit(buf, format, ##__VA_ARGS__); \
} \
static DEVICE_ATTR_RW(name);
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 03/12] HID: cp2112: Make irq_chip immutable
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Since recently, the kernel is nagging about mutable irq_chips:
"not an immutable chip, please consider fixing it!"
Drop the unneeded copy, flag it as IRQCHIP_IMMUTABLE, add the new
helper functions and call the appropriate gpiolib functions.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 37ccf4714ad1..51399b231d36 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -164,7 +164,6 @@ struct cp2112_device {
atomic_t read_avail;
atomic_t xfer_avail;
struct gpio_chip gc;
- struct irq_chip irq;
u8 *in_out_buffer;
struct mutex lock;
@@ -1079,16 +1078,20 @@ static void cp2112_gpio_irq_mask(struct irq_data *d)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct cp2112_device *dev = gpiochip_get_data(gc);
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
- __clear_bit(d->hwirq, &dev->irq_mask);
+ __clear_bit(hwirq, &dev->irq_mask);
+ gpiochip_disable_irq(gc, hwirq);
}
static void cp2112_gpio_irq_unmask(struct irq_data *d)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct cp2112_device *dev = gpiochip_get_data(gc);
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
- __set_bit(d->hwirq, &dev->irq_mask);
+ gpiochip_enable_irq(gc, hwirq);
+ __set_bit(hwirq, &dev->irq_mask);
}
static void cp2112_gpio_poll_callback(struct work_struct *work)
@@ -1174,6 +1177,7 @@ static void cp2112_gpio_irq_shutdown(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct cp2112_device *dev = gpiochip_get_data(gc);
+ cp2112_gpio_irq_mask(d);
cancel_delayed_work_sync(&dev->gpio_poll_worker);
}
@@ -1227,6 +1231,18 @@ static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev,
return ret;
}
+static const struct irq_chip cp2112_gpio_irqchip = {
+ .name = "cp2112-gpio",
+ .irq_startup = cp2112_gpio_irq_startup,
+ .irq_shutdown = cp2112_gpio_irq_shutdown,
+ .irq_ack = cp2112_gpio_irq_ack,
+ .irq_mask = cp2112_gpio_irq_mask,
+ .irq_unmask = cp2112_gpio_irq_unmask,
+ .irq_set_type = cp2112_gpio_irq_type,
+ .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
+ GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct cp2112_device *dev;
@@ -1336,17 +1352,8 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
dev->gc.can_sleep = 1;
dev->gc.parent = &hdev->dev;
- dev->irq.name = "cp2112-gpio";
- dev->irq.irq_startup = cp2112_gpio_irq_startup;
- dev->irq.irq_shutdown = cp2112_gpio_irq_shutdown;
- dev->irq.irq_ack = cp2112_gpio_irq_ack;
- dev->irq.irq_mask = cp2112_gpio_irq_mask;
- dev->irq.irq_unmask = cp2112_gpio_irq_unmask;
- dev->irq.irq_set_type = cp2112_gpio_irq_type;
- dev->irq.flags = IRQCHIP_MASK_ON_SUSPEND;
-
girq = &dev->gc.irq;
- girq->chip = &dev->irq;
+ gpio_irq_chip_set_chip(girq, &cp2112_gpio_irqchip);
/* The event comes from the outside so no parent handler */
girq->parent_handler = NULL;
girq->num_parents = 0;
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 05/12] HID: cp2112: Don't call ->to_irq() explicitly
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
GPIO library guarantees that ->to_irq() is always exists.
Moreover, it tending to become a nische thingy and has to
not be used in ordinary drivers. Hence, replace that by
irq_find_mapping().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index fb4548feb0c8..15b626359281 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1112,10 +1112,9 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
gpio_mask = ret;
for_each_set_bit(virq, &dev->irq_mask, 8) {
- if (!dev->gc.to_irq)
- break;
-
- irq = dev->gc.to_irq(&dev->gc, virq);
+ irq = irq_find_mapping(dev->gc.irq.domain, virq);
+ if (!irq)
+ continue;
d = irq_get_irq_data(irq);
if (!d)
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 02/12] HID: cp2112: Use str_write_read() and str_read_write()
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Use str_write_read() and str_read_write() from string_choices.h.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 27cadadda7c9..37ccf4714ad1 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -24,6 +24,7 @@
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/nls.h>
+#include <linux/string_choices.h>
#include <linux/usb/ch9.h>
#include "hid-ids.h"
@@ -532,15 +533,13 @@ static int cp2112_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
hid_dbg(hdev, "I2C %d messages\n", num);
if (num == 1) {
+ hid_dbg(hdev, "I2C %s %#04x len %d\n",
+ str_read_write(msgs->flags & I2C_M_RD), msgs->addr, msgs->len);
if (msgs->flags & I2C_M_RD) {
- hid_dbg(hdev, "I2C read %#04x len %d\n",
- msgs->addr, msgs->len);
read_length = msgs->len;
read_buf = msgs->buf;
count = cp2112_read_req(buf, msgs->addr, msgs->len);
} else {
- hid_dbg(hdev, "I2C write %#04x len %d\n",
- msgs->addr, msgs->len);
count = cp2112_i2c_write_req(buf, msgs->addr,
msgs->buf, msgs->len);
}
@@ -648,7 +647,7 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr,
int ret;
hid_dbg(hdev, "%s addr 0x%x flags 0x%x cmd 0x%x size %d\n",
- read_write == I2C_SMBUS_WRITE ? "write" : "read",
+ str_write_read(read_write == I2C_SMBUS_WRITE),
addr, flags, command, size);
switch (size) {
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 01/12] lib/string_choices: Add str_write_read() helper
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
Add an inversed variant of str_read_write(), i.e. str_write_read().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/string_choices.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/string_choices.h b/include/linux/string_choices.h
index 48120222b9b2..3c1091941eb8 100644
--- a/include/linux/string_choices.h
+++ b/include/linux/string_choices.h
@@ -30,6 +30,7 @@ static inline const char *str_read_write(bool v)
{
return v ? "read" : "write";
}
+#define str_write_read(v) str_read_write(!(v))
static inline const char *str_on_off(bool v)
{
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 04/12] HID: cp2112: Switch to for_each_set_bit() to simplify the code
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
In-Reply-To: <20230703185222.50554-1-andriy.shevchenko@linux.intel.com>
It's cleaner to use for_each_set_bit() than open coding it.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-cp2112.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 51399b231d36..fb4548feb0c8 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -16,6 +16,7 @@
* https://www.silabs.com/documents/public/application-notes/an495-cp2112-interface-specification.pdf
*/
+#include <linux/bitops.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio/machine.h>
#include <linux/gpio/driver.h>
@@ -1100,7 +1101,6 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
gpio_poll_worker.work);
struct irq_data *d;
u8 gpio_mask;
- u8 virqs = (u8)dev->irq_mask;
u32 irq_type;
int irq, virq, ret;
@@ -1111,11 +1111,7 @@ static void cp2112_gpio_poll_callback(struct work_struct *work)
goto exit;
gpio_mask = ret;
-
- while (virqs) {
- virq = ffs(virqs) - 1;
- virqs &= ~BIT(virq);
-
+ for_each_set_bit(virq, &dev->irq_mask, 8) {
if (!dev->gc.to_irq)
break;
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 00/12] HID: cp2112: Cleanups and refactorings
From: Andy Shevchenko @ 2023-07-03 18:52 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: Jiri Kosina, Benjamin Tissoires, Andy Shevchenko
After I updated GPIO library for the case Benjamin has with CP2112,
I have a brief look into the CP2112 driver itself.
From GPIO perspective it has two main (maitenance) issues:
- usage of ->to_irq() with IRQ chip present;
- having IRQ chip not immutable.
Besides that there are plenty small cleanups here and there.
Hence this series.
Compile tested only.
Andy Shevchenko (12):
lib/string_choices: Add str_write_read() helper
HID: cp2112: Use str_write_read() and str_read_write()
HID: cp2112: Make irq_chip immutable
HID: cp2112: Switch to for_each_set_bit() to simplify the code
HID: cp2112: Don't call ->to_irq() explicitly
HID: cp2112: Remove dead code
HID: cp2112: Define maximum GPIO constant and use it
HID: cp2112: Define all GPIO mask and use it
HID: cp2112: Use BIT() in GPIO setter and getter
HID: cp2112: Use sysfs_emit() to instead of scnprintf()
HID: cp2112: Convert to DEVICE_ATTR_RW()
HID: cp2112: Use octal permissions
drivers/hid/hid-cp2112.c | 169 +++++++++++----------------------
include/linux/string_choices.h | 1 +
2 files changed, 58 insertions(+), 112 deletions(-)
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply
* Re: [PATCH v5] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-07-03 18:12 UTC (permalink / raw)
To: Benjamin Tissoires, kernel test robot
Cc: linux-input, oe-kbuild-all, linux-kernel, Jiri Kosina,
Benjamin Tissoires
In-Reply-To: <ytk64symj6g2rsoy5jfe4gpojiska7plv5w4lmofm7vifrap5h@4lpu33rqodos>
On Mon, 2023-07-03 at 17:33 +0200, Benjamin Tissoires wrote:
>
> On Jul 03 2023, kernel test robot wrote:
> >
> > Hi Bastien,
> >
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on hid/for-next]
> > [also build test ERROR on linus/master v6.4 next-20230703]
> > [If your patch is applied to the wrong git tree, kindly drop us a
> > note.
> > And when submitting patch, we suggest to use '--base' as documented
> > in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url:
> > https://github.com/intel-lab-lkp/linux/commits/Bastien-Nocera/HID-steelseries-Add-support-for-Arctis-1-XBox/20230703-183124
> > base:
> > https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-
> > next
> > patch link:
> > https://lore.kernel.org/r/20230703102918.9941-1-hadess%40hadess.net
> > patch subject: [PATCH v5] HID: steelseries: Add support for Arctis
> > 1 XBox
> > config: parisc-randconfig-r032-20230703
> > (https://download.01.org/0day-ci/archive/20230703/202307032208.b5br
> > KeCt-lkp@intel.com/config)
> > compiler: hppa-linux-gcc (GCC) 12.3.0
> > reproduce:
> > (https://download.01.org/0day-ci/archive/20230703/202307032208.b5br
> > KeCt-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a
> > new version of
> > the same patch/commit), kindly add following tags
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes:
> > > https://lore.kernel.org/oe-kbuild-all/202307032208.b5brKeCt-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> > hppa-linux-ld: drivers/hid/hid-steelseries.o: in function
> > `.LC10':
> > > > hid-steelseries.c:(.rodata.cst4+0x8): undefined reference to
> > > > `hid_is_usb'
>
> Bastien, you need a depends on USB_HID IIRC in your Kconfig (at
> least CONFIG_USB_HID is not set in that kernel test bot run).
Yeah, I just get to undo the changes from v3 ;)
^ permalink raw reply
* [PATCH v6] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-07-03 18:10 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires
Add support for the Steelseries Arctis 1 XBox headset. This driver
will export the battery information from the headset, as well as the
"wireless_status" property.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
v6:
- Dependency is now on USB_HID :eyeroll: after the changes to v4
v5:
- Move spinlock init as per bentiss review
- Use the already defined response length constant when parsing answers
- Avoid parsing non-battery events (fixes battery showing up as 3%
for a couple of seconds in some rare circumstances)
v4:
- Guard against crash when using uhid
- Print the contents of the raw events for debugging
v3:
- Dependency is on USB not USB_HID
v2:
- Fix missing USB dependency
- Fix config option description
drivers/hid/Kconfig | 6 +-
drivers/hid/hid-steelseries.c | 311 ++++++++++++++++++++++++++++++++--
2 files changed, 300 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce012f83253..1bc99b33329f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1048,9 +1048,11 @@ config STEAM_FF
Deck.
config HID_STEELSERIES
- tristate "Steelseries SRW-S1 steering wheel support"
+ tristate "Steelseries devices support"
+ depends on USB_HID
help
- Support for Steelseries SRW-S1 steering wheel
+ Support for Steelseries SRW-S1 steering wheel, and the Steelseries
+ Arctis 1 Wireless for XBox headset.
config HID_SUNPLUS
tristate "Sunplus wireless desktop"
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index aae3afc4107a..495377686123 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * HID driver for Steelseries SRW-S1
+ * HID driver for Steelseries devices
*
* Copyright (c) 2013 Simon Wood
+ * Copyright (c) 2023 Bastien Nocera
*/
/*
@@ -11,10 +12,28 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/module.h>
+#include <linux/usb.h>
#include <linux/leds.h>
#include "hid-ids.h"
+#define STEELSERIES_SRWS1 BIT(0)
+#define STEELSERIES_ARCTIS_1 BIT(1)
+
+struct steelseries_device {
+ struct hid_device *hdev;
+ unsigned long quirks;
+
+ struct delayed_work battery_work;
+ spinlock_t lock;
+ bool removed;
+
+ struct power_supply_desc battery_desc;
+ struct power_supply *battery;
+ uint8_t battery_capacity;
+ bool headset_connected;
+};
+
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
(IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
#define SRWS1_NUMBER_LEDS 15
@@ -353,9 +372,211 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
}
#endif
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN 8
+const char arctis_1_battery_request[] = { 0x06, 0x12 };
+
+static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
+{
+ u8 *write_buf;
+ int ret;
+
+ /* Request battery information */
+ write_buf = kmemdup(arctis_1_battery_request, sizeof(arctis_1_battery_request), GFP_KERNEL);
+ if (!write_buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0],
+ write_buf, sizeof(arctis_1_battery_request),
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ if (ret < sizeof(arctis_1_battery_request)) {
+ hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+ ret = -ENODATA;
+ }
+ kfree(write_buf);
+ return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ int ret = 0;
+
+ if (sd->quirks & STEELSERIES_ARCTIS_1)
+ ret = steelseries_headset_arctis_1_fetch_battery(hdev);
+
+ if (ret < 0)
+ hid_dbg(hdev,
+ "Battery query failed (err: %d)\n", ret);
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+ struct steelseries_device *sd = container_of(work,
+ struct steelseries_device, battery_work.work);
+ struct hid_device *hdev = sd->hdev;
+
+ steelseries_headset_fetch_battery(hdev);
+}
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct steelseries_device *sd = power_supply_get_drvdata(psy);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = sd->headset_connected ?
+ POWER_SUPPLY_STATUS_DISCHARGING :
+ POWER_SUPPLY_STATUS_UNKNOWN;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = sd->battery_capacity;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+ bool connected)
+{
+ struct usb_interface *intf;
+
+ if (!hid_is_usb(hdev))
+ return;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ usb_set_wireless_status(intf, connected ?
+ USB_WIRELESS_STATUS_CONNECTED :
+ USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+ static atomic_t battery_no = ATOMIC_INIT(0);
+ struct power_supply_config battery_cfg = { .drv_data = sd, };
+ unsigned long n;
+ int ret;
+
+ sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ sd->battery_desc.properties = steelseries_headset_battery_props;
+ sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+ sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+ sd->battery_desc.use_for_apm = 0;
+ n = atomic_inc_return(&battery_no) - 1;
+ sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+ "steelseries_headset_battery_%ld", n);
+ if (!sd->battery_desc.name)
+ return -ENOMEM;
+
+ /* avoid the warning of 0% battery while waiting for the first info */
+ steelseries_headset_set_wireless_status(sd->hdev, false);
+ sd->battery_capacity = 100;
+
+ sd->battery = devm_power_supply_register(&sd->hdev->dev,
+ &sd->battery_desc, &battery_cfg);
+ if (IS_ERR(sd->battery)) {
+ ret = PTR_ERR(sd->battery);
+ hid_err(sd->hdev,
+ "%s:power_supply_register failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+ power_supply_powers(sd->battery, &sd->hdev->dev);
+
+ INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+ steelseries_headset_fetch_battery(sd->hdev);
+
+ return 0;
+}
+
+static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct steelseries_device *sd;
+ int ret;
+
+ sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+ if (!sd)
+ return -ENOMEM;
+ hid_set_drvdata(hdev, sd);
+ sd->hdev = hdev;
+ sd->quirks = id->driver_data;
+
+ if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+ return steelseries_srws1_probe(hdev, id);
+#else
+ return -ENODEV;
+#endif
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return ret;
+
+ spin_lock_init(&sd->lock);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ return ret;
+
+ if (steelseries_headset_battery_register(sd) < 0)
+ hid_err(sd->hdev,
+ "Failed to register battery for headset\n");
+
+ return ret;
+}
+
+static void steelseries_remove(struct hid_device *hdev)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ unsigned long flags;
+
+ if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+ steelseries_srws1_remove(hdev);
+#endif
+ return;
+ }
+
+ spin_lock_irqsave(&sd->lock, flags);
+ sd->removed = true;
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ cancel_delayed_work_sync(&sd->battery_work);
+
+ hid_hw_stop(hdev);
+}
+
static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
+ if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
+ hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
+ return rdesc;
+
if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -365,22 +586,82 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
return rdesc;
}
-static const struct hid_device_id steelseries_srws1_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
+static int steelseries_headset_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *read_buf,
+ int size)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ int capacity = sd->battery_capacity;
+ bool connected = sd->headset_connected;
+ unsigned long flags;
+
+ /* Not a headset */
+ if (sd->quirks & STEELSERIES_SRWS1)
+ return 0;
+
+ if (sd->quirks & STEELSERIES_ARCTIS_1) {
+ hid_dbg(sd->hdev,
+ "Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf);
+ if (size < ARCTIS_1_BATTERY_RESPONSE_LEN ||
+ memcmp (read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request)))
+ return 0;
+ if (read_buf[2] == 0x01) {
+ connected = false;
+ capacity = 100;
+ } else {
+ connected = true;
+ capacity = read_buf[3];
+ }
+ }
+
+ if (connected != sd->headset_connected) {
+ hid_dbg(sd->hdev,
+ "Connected status changed from %sconnected to %sconnected\n",
+ sd->headset_connected ? "" : "not ",
+ connected ? "" : "not ");
+ sd->headset_connected = connected;
+ steelseries_headset_set_wireless_status(hdev, connected);
+ }
+
+ if (capacity != sd->battery_capacity) {
+ hid_dbg(sd->hdev,
+ "Battery capacity changed from %d%% to %d%%\n",
+ sd->battery_capacity, capacity);
+ sd->battery_capacity = capacity;
+ power_supply_changed(sd->battery);
+ }
+
+ spin_lock_irqsave(&sd->lock, flags);
+ if (!sd->removed)
+ schedule_delayed_work(&sd->battery_work,
+ msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ return 0;
+}
+
+static const struct hid_device_id steelseries_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
+ .driver_data = STEELSERIES_SRWS1 },
+
+ { /* SteelSeries Arctis 1 Wireless for XBox */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
+ .driver_data = STEELSERIES_ARCTIS_1 },
+
{ }
};
-MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
-
-static struct hid_driver steelseries_srws1_driver = {
- .name = "steelseries_srws1",
- .id_table = steelseries_srws1_devices,
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
- (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
- .probe = steelseries_srws1_probe,
- .remove = steelseries_srws1_remove,
-#endif
- .report_fixup = steelseries_srws1_report_fixup
+MODULE_DEVICE_TABLE(hid, steelseries_devices);
+
+static struct hid_driver steelseries_driver = {
+ .name = "steelseries",
+ .id_table = steelseries_devices,
+ .probe = steelseries_probe,
+ .remove = steelseries_remove,
+ .report_fixup = steelseries_srws1_report_fixup,
+ .raw_event = steelseries_headset_raw_event,
};
-module_hid_driver(steelseries_srws1_driver);
+module_hid_driver(steelseries_driver);
MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
--
2.41.0
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: Input: exc3000 - Support power supply regulators
From: Krzysztof Kozlowski @ 2023-07-03 16:25 UTC (permalink / raw)
To: Mike Looijmans, devicetree, linux-input
Cc: Conor Dooley, Dmitry Torokhov, Krzysztof Kozlowski, Rob Herring,
linux-kernel
In-Reply-To: <20230703084536.8429-1-mike.looijmans@topic.nl>
On 03/07/2023 10:45, Mike Looijmans wrote:
> Add power supply regulator support to the exc3000 devices.
> This provides the devicetree bindings.
If there is going to be next version/resend, you could skip the last
sentence. It is a bit redundant.
In any case:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox