Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v6 01/15] platform/x86/amd/pmf: Add PMF TEE interface
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>

AMD PMF driver loads the PMF TA (Trusted Application) into the AMD
ASP's (AMD Security Processor) TEE (Trusted Execution Environment).

PMF Trusted Application is a secured firmware placed under
/lib/firmware/amdtee gets loaded only when the TEE environment is
initialized. Add the initial code path to build these pipes.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/Kconfig  |   1 +
 drivers/platform/x86/amd/pmf/Makefile |   3 +-
 drivers/platform/x86/amd/pmf/core.c   |  10 ++-
 drivers/platform/x86/amd/pmf/pmf.h    |  16 ++++
 drivers/platform/x86/amd/pmf/tee-if.c | 105 ++++++++++++++++++++++++++
 5 files changed, 130 insertions(+), 5 deletions(-)
 create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c

diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 3064bc8ea167..32a029e8db80 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -9,6 +9,7 @@ config AMD_PMF
 	depends on POWER_SUPPLY
 	depends on AMD_NB
 	select ACPI_PLATFORM_PROFILE
+	depends on TEE
 	help
 	  This driver provides support for the AMD Platform Management Framework.
 	  The goal is to enhance end user experience by making AMD PCs smarter,
diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index fdededf54392..d2746ee7369f 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -6,4 +6,5 @@
 
 obj-$(CONFIG_AMD_PMF) += amd-pmf.o
 amd-pmf-objs := core.o acpi.o sps.o \
-		auto-mode.o cnqf.o
+		auto-mode.o cnqf.o \
+		tee-if.o
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 78ed3ee22555..ec92d1cc0dac 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -309,13 +309,13 @@ static void amd_pmf_init_features(struct amd_pmf_dev *dev)
 		dev_dbg(dev->dev, "SPS enabled and Platform Profiles registered\n");
 	}
 
-	/* Enable Auto Mode */
-	if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+	if (!amd_pmf_init_smart_pc(dev)) {
+		dev_dbg(dev->dev, "Smart PC Solution Enabled\n");
+	} else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
 		amd_pmf_init_auto_mode(dev);
 		dev_dbg(dev->dev, "Auto Mode Init done\n");
 	} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
 			  is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
-		/* Enable Cool n Quiet Framework (CnQF) */
 		ret = amd_pmf_init_cnqf(dev);
 		if (ret)
 			dev_warn(dev->dev, "CnQF Init failed\n");
@@ -330,7 +330,9 @@ static void amd_pmf_deinit_features(struct amd_pmf_dev *dev)
 		amd_pmf_deinit_sps(dev);
 	}
 
-	if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+	if (!dev->smart_pc_enabled) {
+		amd_pmf_deinit_smart_pc(dev);
+	} else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
 		amd_pmf_deinit_auto_mode(dev);
 	} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
 			  is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index deba88e6e4c8..bd40458937ba 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -179,6 +179,12 @@ struct amd_pmf_dev {
 	bool cnqf_enabled;
 	bool cnqf_supported;
 	struct notifier_block pwr_src_notifier;
+	/* Smart PC solution builder */
+	struct tee_context *tee_ctx;
+	struct tee_shm *fw_shm_pool;
+	u32 session_id;
+	void *shbuf;
+	bool smart_pc_enabled;
 };
 
 struct apmf_sps_prop_granular {
@@ -389,6 +395,13 @@ struct apmf_dyn_slider_output {
 	struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
 } __packed;
 
+struct ta_pmf_shared_memory {
+	int command_id;
+	int resp_id;
+	u32 pmf_result;
+	u32 if_version;
+};
+
 /* Core Layer */
 int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
 void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
@@ -433,4 +446,7 @@ void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
 int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
 extern const struct attribute_group cnqf_feature_attribute_group;
 
+/* Smart PC builder Layer */
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
 #endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
new file mode 100644
index 000000000000..6ec8c3726624
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver - TEE Interface
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include "pmf.h"
+
+#define MAX_TEE_PARAM	4
+static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
+						0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+
+static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+	return ver->impl_id == TEE_IMPL_ID_AMDTEE;
+}
+
+static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id)
+{
+	struct tee_ioctl_open_session_arg sess_arg = {};
+	int rc;
+
+	export_uuid(sess_arg.uuid, &amd_pmf_ta_uuid);
+	sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+	sess_arg.num_params = 0;
+
+	rc = tee_client_open_session(ctx, &sess_arg, NULL);
+	if (rc < 0 || sess_arg.ret != 0) {
+		pr_err("Failed to open TEE session err:%#x, rc:%d\n", sess_arg.ret, rc);
+		return rc;
+	}
+
+	*id = sess_arg.session;
+
+	return rc;
+}
+
+static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
+{
+	u32 size;
+	int ret;
+
+	dev->tee_ctx = tee_client_open_context(NULL, amd_pmf_amdtee_ta_match, NULL, NULL);
+	if (IS_ERR(dev->tee_ctx)) {
+		dev_err(dev->dev, "Failed to open TEE context\n");
+		return PTR_ERR(dev->tee_ctx);
+	}
+
+	ret = amd_pmf_ta_open_session(dev->tee_ctx, &dev->session_id);
+	if (ret) {
+		dev_err(dev->dev, "Failed to open TA session (%d)\n", ret);
+		ret = -EINVAL;
+		goto out_ctx;
+	}
+
+	size = sizeof(struct ta_pmf_shared_memory);
+	dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
+	if (IS_ERR(dev->fw_shm_pool)) {
+		dev_err(dev->dev, "Failed to alloc TEE shared memory\n");
+		ret = PTR_ERR(dev->fw_shm_pool);
+		goto out_sess;
+	}
+
+	dev->shbuf = tee_shm_get_va(dev->fw_shm_pool, 0);
+	if (IS_ERR(dev->shbuf)) {
+		dev_err(dev->dev, "Failed to get TEE virtual address\n");
+		ret = PTR_ERR(dev->shbuf);
+		goto out_shm;
+	}
+	dev_dbg(dev->dev, "TEE init done\n");
+
+	return 0;
+
+out_shm:
+	tee_shm_free(dev->fw_shm_pool);
+out_sess:
+	tee_client_close_session(dev->tee_ctx, dev->session_id);
+out_ctx:
+	tee_client_close_context(dev->tee_ctx);
+
+	return ret;
+}
+
+static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
+{
+	tee_shm_free(dev->fw_shm_pool);
+	tee_client_close_session(dev->tee_ctx, dev->session_id);
+	tee_client_close_context(dev->tee_ctx);
+}
+
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
+{
+	return amd_pmf_tee_init(dev);
+}
+
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
+{
+	amd_pmf_tee_deinit(dev);
+}
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 00/15] Introduce PMF Smart PC Solution Builder Feature
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K

Smart PC Solutions Builder allows for OEM to define a large number of
custom system states to dynamically switch to. The system states are
referred to as policies, and multiple policies can be loaded onto the
system at any given time, however only one policy can be active at a
given time.

Policy is a combination of PMF input and output capabilities. The inputs
are the incoming information from the other kernel subsystems like LID
state, Sensor info, GPU info etc and the actions are the updating the 
power limits of SMU etc.

The policy binary is signed and encrypted by a special key from AMD. This
policy binary shall have the inputs and outputs which the OEMs can build
for the platform customization that can enhance the user experience and
system behavior.

This series adds the initial support for Smart PC solution to PMF driver.

Note that, on platforms where CnQF and Smart PC is advertised, Smart PC
shall have higher precedence and same applies for Auto Mode.

v5->v6:
---------
 - Add Ilpo's and Mario's Reviewed-by tags
 - Drop 13/17 and 14/17 patches from this series which are GPU centric
 - Drop separate checks for battery handling.
 - Handle SFH failure cases

v4->v5:
---------
 - Remove PMF-GPU interface from amdgpu driver and add DRM/backlight
   changes within PMF
 - Add module_softdep for AMDGPU
 - remove error checks for debugfs_create_file()
 - Add "Reviewed-by:" tags
 - Add kerneldoc for kernel-wide headers
 - Add checks for acpi_backlight_native
 - Add early return for SFH path
 - other cosmetic changes
 
v3->v4:
---------
- Split v3 9/16 into 2 patches, that addresses using generic fn names
- Add softdep [Ilpo] instead of request_module()
- return proper ACPI status [Mario]
- Update comments in code [Mario]
- Remove missed double _ remarks
- handle battery status branches [Ilpo]
- Address KASAN problems 

v2->v3:
---------
- Remove pci_get_device() for getting gpu handle
- add .suspend handler for pmf driver
- remove unwanted type caste
- Align comments, spaces etc.
- add wrapper for print_hex_dump_debug()
- Remove lkp tags in commit-msg
- Add macros for magic numbers
- use right format specifiers for printing
- propagate error codes back to the caller
- remove unwanted comments


v1->v2:
---------
- Remove __func__ macros
- Remove manual function names inside prints
- Handle tee_shm_get_va() failure
- Remove double _
- Add meaningful prints
- pass amd_pmf_set_dram_addr() failure errors
- Add more information to commit messages
- use right format specifiers
- use devm_ioremap() instead of ioremap()
- address unsigned long vs u32 problems
- Fix lkp reported issues
- Add amd_pmf_remove_pb() to remove the debugfs files created(if any).
- Make amd_pmf_open_pb() as static.
- Add cooling device APIs for controlling amdgpu backlight
- handle amd_pmf_apply_policies() failures
- Split v1 14/15 into 2 patches further
- use linux/units.h for better handling
- add "depends on" AMD_SFH_HID for interaction with SFH
- other cosmetic remarks

Basavaraj Natikar (3):
  HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
  platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
  platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS

Shyam Sundar S K (12):
  platform/x86/amd/pmf: Add PMF TEE interface
  platform/x86/amd/pmf: Add support for PMF-TA interaction
  platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
  platform/x86/amd/pmf: Add support for PMF Policy Binary
  platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
  platform/x86/amd/pmf: Add support to get inputs from other subsystems
  platform/x86/amd/pmf: Add support update p3t limit
  platform/x86/amd/pmf: Add support to update system state
  platform/x86/amd/pmf: Make source_as_str() as non-static
  platform/x86/amd/pmf: Add facility to dump TA inputs
  platform/x86/amd/pmf: Add capability to sideload of policy binary
  platform/x86/amd/pmf: dump policy binary data

 Documentation/admin-guide/index.rst           |   1 +
 Documentation/admin-guide/pmf.rst             |  24 +
 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |   6 +
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c |  28 +-
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  20 +
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    |  52 ++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |   2 +
 drivers/platform/x86/amd/pmf/Kconfig          |   2 +
 drivers/platform/x86/amd/pmf/Makefile         |   3 +-
 drivers/platform/x86/amd/pmf/acpi.c           |  37 ++
 drivers/platform/x86/amd/pmf/core.c           |  63 ++-
 drivers/platform/x86/amd/pmf/pmf.h            | 203 ++++++++
 drivers/platform/x86/amd/pmf/spc.c            | 187 ++++++++
 drivers/platform/x86/amd/pmf/sps.c            |   5 +-
 drivers/platform/x86/amd/pmf/tee-if.c         | 452 ++++++++++++++++++
 include/linux/amd-pmf-io.h                    |  50 ++
 16 files changed, 1105 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/admin-guide/pmf.rst
 create mode 100644 drivers/platform/x86/amd/pmf/spc.c
 create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
 create mode 100644 include/linux/amd-pmf-io.h

-- 
2.25.1


^ permalink raw reply

* Re: [PATCH v2 1/6] x86/vmware: Move common macros to vmware.h
From: Borislav Petkov @ 2023-12-04 10:32 UTC (permalink / raw)
  To: Alexey Makhalov
  Cc: linux-kernel, virtualization, hpa, dave.hansen, bp, mingo, tglx,
	x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
	linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
	jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
	maarten.lankhorst, horms
In-Reply-To: <20231201232452.220355-2-amakhalov@vmware.com>

On Fri, Dec 01, 2023 at 03:24:47PM -0800, Alexey Makhalov wrote:
> Move VMware hypercall macros to vmware.h as a preparation step
> for the next commit. No functional changes besides exporting

"next commit" in git is ambiguous. Get rid of such formulations.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* Re: [PATCH v2 6/6] x86/vmware: Add TDX hypercall support
From: Borislav Petkov @ 2023-12-04 10:31 UTC (permalink / raw)
  To: Alexey Makhalov
  Cc: linux-kernel, virtualization, hpa, dave.hansen, bp, mingo, tglx,
	x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
	linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
	jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
	maarten.lankhorst, horms
In-Reply-To: <20231201232452.220355-7-amakhalov@vmware.com>

On Fri, Dec 01, 2023 at 03:24:52PM -0800, Alexey Makhalov wrote:
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +/* __tdx_hypercall() is not exported. So, export the wrapper */
> +void vmware_tdx_hypercall_args(struct tdx_module_args *args)
> +{
> +	__tdx_hypercall(args);
> +}
> +EXPORT_SYMBOL_GPL(vmware_tdx_hypercall_args);

Uuuh, lovely. I'd like to see what the TDX folks think about this
export first.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* Re: supporting binary (near-far) proximity sensors over gpio
From: Jonathan Cameron @ 2023-12-04 10:09 UTC (permalink / raw)
  To: Philipp Jungkamp
  Cc: Jeff LaBundy, David Lechner, Sicelo, linux-iio, maemo-leste,
	Ivaylo Dimitrov, linux-input
In-Reply-To: <94eafe4dfe176d0ca01ddbd0ec599e3c5fb2e0a3.camel@gmx.net>

On Sun, 26 Nov 2023 11:59:05 +0100
Philipp Jungkamp <p.jungkamp@gmx.net> wrote:

> Hi,
> 
> would it make sense to move the HID human presence driver at
> drivers/iio/light/hid-sensor-prox.c to the input system then? This
> driver only checks for the "Biometric" (0x2004b0) and "Biometric: Human
> Presence" (0x2004b1) HID usages. The former has a vendor defined value
> range, the latter is defined as a boolean switch. See the HID Usage
> Tables, section 21.1 and 21.6.

These boundaries always get messy.  Given there is a value range, it
might get used for something else.
> 
> I take from this discussion that the input subsystem would make more
> sense for the "Biometric: Human Presence" usage.

Potentially yes
> 
> I just wanted to chime in as there seems to be some older precedent for
> a binary sensor in the iio subsystem. I tried to get that sensor
> working for the proximity sensor on my laptop last year.

I guess this one landed because it looked much like the hid light sensors
and no one raised the question at the time (that I recall).

Probably not worth moving it, but we may well have suggested putting
it in input if we'd noticed at the time!

Jonathan

> 
> Regards,
> Philipp Jungkamp
> 
> On Sat, 2023-11-25 at 22:33 -0600, Jeff LaBundy wrote:
> > Hi Sicelo and David,
> > 
> > On Sat, Nov 18, 2023 at 06:09:18PM -0600, David Lechner wrote:  
> > > On Fri, Nov 17, 2023 at 12:22 PM Sicelo <absicsz@gmail.com> wrote:  
> > > > 
> > > > Hi
> > > > 
> > > > Some phones have 1-bit proximity sensors, which simply toggle a
> > > > GPIO
> > > > line to indicate that an object is near or far. Thresholds are
> > > > set at
> > > > hardware level. One such sensor is OSRAM SFH 7741 [1], which is
> > > > used on
> > > > the Nokia N900.
> > > > 
> > > > It is currently exported over evdev, emitting the
> > > > SW_FRONT_PROXIMITY key
> > > > code [2].
> > > > 
> > > > So the question is: should a new, general purpose iio-gpio driver
> > > > be
> > > > written, that would switch such a proximity sensor to the iio
> > > > framework?
> > > > Or evdev is really the best place to support it?
> > > > 
> > > > There are a couple of people who are willing to write such an iio
> > > > driver, if iio is the way to go.
> > > > 
> > > > Regards,
> > > > Sicelo
> > > > 
> > > > [1]
> > > > https://media.digikey.com/pdf/Data%20Sheets/Osram%20PDFs/SFH_7741.pdf
> > > > [2]
> > > > https://elixir.bootlin.com/linux/v6.6.1/source/arch/arm/boot/dts/ti/omap/omap3-n900.dts#L111
> > > >   
> > > Since this is really a proximity switch (it is either on or off)
> > > rather than measuring a proximity value over a continuous range, it
> > > doesn't seem like a good fit for the iio subsystem. If the sensor
> > > is
> > > on a phone, then it is likely to detect human presence so the input
> > > subsystem does seem like the right one for that application.
> > > 
> > > More at
> > > https://www.kernel.org/doc/html/latest/driver-api/iio/intro.html
> > >   
> > 
> > I tend to agree; if there are only two discrete states as is the case
> > for a
> > GPIO, then this is technically a switch and not a sensor. Therefore,
> > input
> > seems like a better fit; that is just my $.02.
> > 
> > FWIW, a similar discussion came up a few years ago in [1] and again
> > the key
> > differentiator was whether the output is discrete or continuous.
> > 
> > Kind regards,
> > Jeff LaBundy
> > 
> > [1]
> > https://lore.kernel.org/linux-iio/9f9b0ff6-3bf1-63c4-eb36-901cecd7c4d9@redhat.com/
> >   
> 


^ permalink raw reply

* Re: [RFC PATCH v3 2/5] i2c: of: Introduce component probe function
From: Chen-Yu Tsai @ 2023-12-04  9:52 UTC (permalink / raw)
  To: Doug Anderson, Wolfram Sang
  Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
	Tzung-Bi Shih, chrome-platform, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Johan Hovold, Hsin-Yi Wang,
	Dmitry Torokhov, andriy.shevchenko, Jiri Kosina, linus.walleij,
	broonie, gregkh, hdegoede, james.clark, james, keescook, rafael,
	tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=U_+iQJtV0Wii89DQT1V_fJCeS9wcqA8EJAs-hmmmLLLg@mail.gmail.com>

On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -217,4 +217,114 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
> >  struct notifier_block i2c_of_notifier = {
> >         .notifier_call = of_i2c_notify,
> >  };
> > +
> > +/*
> > + * Some devices, such as Google Hana Chromebooks, are produced by multiple
> > + * vendors each using their preferred components. Such components are all
> > + * in the device tree. Instead of having all of them enabled and having each
> > + * driver separately try and probe its device while fighting over shared
> > + * resources, they can be marked as "fail-needs-probe" and have a prober
> > + * figure out which one is actually used beforehand.
> > + *
> > + * This prober assumes such drop-in parts are on the same I2C bus, have
> > + * non-conflicting addresses, and can be directly probed by seeing which
> > + * address responds.
> > + *
> > + * TODO:
> > + * - Support handling common regulators and GPIOs.
>
> IMO you should prototype how you're going to handle regulators and
> GPIOs before finalizing the design. I was going to write that you
> should just document that it was up to the caller to power things up
> before calling this function, but then I realized that the caller
> would have to duplicate much of this function in order to do so. In
> the very least they'd have to find the nodes of the relevant devices
> so that they could grab regulators and/or GPIOs. In order to avoid
> this duplication, would the design need to change? Perhaps this would
> be as simple as adding a callback function here that's called with all
> of the nodes before probing? If that's right, it would be nice to have
> that callback from the beginning so we don't need two variants of the
> function...

So I think I can prototype designs with one GPIO and multiple regulators,
assuming each node has the same number of both? At least they should if
they're on the same connector.

More than one GPIO probably means there are some ordering and timing
constraints, and won't be as generic.

> > + * - Support I2C muxes
> > + */
> > +
> > +/**
> > + * i2c_of_probe_component() - probe for devices of "type" on the same i2c bus
> > + * @dev: &struct device of the caller, only used for dev_* printk messages
> > + * @type: a string to match the device node name prefix to probe for
> > + *
> > + * Probe for possible I2C components of the same "type" on the same I2C bus
> > + * that have their status marked as "fail".
>
> Should document these current limitations with the code:
>
> * Assumes that across the entire device tree the only instances of
> nodes named "type" are ones we're trying to handle second sourcing
> for. In other words if we're searching for "touchscreen" then all
> nodes named "touchscreen" are ones that need to be probed.
>
> * Assumes that there is exactly one group of each "type". In other
> words, if we're searching for "touchscreen" then exactly one
> touchscreen will be enabled across the whole tree.
>
> Obviously those could be relaxed with more code, but that's the
> current limitation and it makes the code easier to understand with
> that context.

Done.

> > + */
> > +int i2c_of_probe_component(struct device *dev, const char *type)
> > +{
> > +       struct device_node *node, *i2c_node;
> > +       struct i2c_adapter *i2c;
> > +       struct of_changeset *ocs = NULL;
> > +       int ret;
> > +
> > +       node = of_find_node_by_name(NULL, type);
> > +       if (!node)
> > +               return dev_err_probe(dev, -ENODEV, "Could not find %s device node\n", type);
> > +
> > +       i2c_node = of_get_next_parent(node);
> > +       if (!of_node_name_eq(i2c_node, "i2c")) {
> > +               of_node_put(i2c_node);
> > +               return dev_err_probe(dev, -EINVAL, "%s device isn't on I2C bus\n", type);
> > +       }
>
> Personally I'd skip checking for the "i2c" node name. Just rely on
> of_get_i2c_adapter_by_node() returning an error.
>
> Oh, I guess you have this because you need to tell the difference
> between -EINVAL and -EPROBE_DEFER? It feels like instead you could use
> the firmware node to lookup a device more generically. If a device
> isn't associated with the node at all then you return -EPROBE_DEFER.
> Otherwise if it's associated but not an i2c device then you return
> -EINVAL. I guess maybe it doesn't make a huge difference, but it
> somehow feels less fragile than relying on the node name being "i2c".
> Maybe I just haven't had enough DT Kool-Aid.

The current way it's written is to do the device tree structure checks
first before doing any driver model access. Also it needs to tell
(as you mentioned later) if the i2c adapter is "disabled" or just not
probed yet.

> One thing this made me wonder is if of_get_i2c_adapter_by_node() is
> race free anyway. Can't that return you a device that hasn't finished
> probing yet? I see:
>
> - i2c_register_adapter()
> -- device_register()
> --- device_add()
> ---- bus_add_device()
> ---- bus_probe_device()
>
> As soon as bus_add_device() is called then it will be in
> "klist_devices" and I believe i2c_find_device_by_fwnode() will be able
> to find it. However, it hasn't necessarily been probed yet. I think
> that means calling i2c_smbus_xfer() on it might not work yet...

It does look like there's a small window between the device_register()
and when the i2c adapter is ready, i.e. can't bail out on an error.

I guess it needs either a flag or some reordering of the code.

It looks like Wolfram will be at OSS JP. I'll try and have a chat about
the whole thing.

> One last thing is that you should check to make sure that the i2c
> adapter is not marked "disabled". ...otherwise I think you'd end up
> constantly trying again and again...

Yeah. I added a check for that as well.

> > +       for_each_child_of_node(i2c_node, node) {
> > +               if (!of_node_name_prefix(node, type))
> > +                       continue;
> > +               if (of_device_is_available(node)) {
> > +                       /*
> > +                        * Device tree has component already enabled. Either the
> > +                        * device tree isn't supported or we already probed once.
>
> I guess the "already probed once" is somewhat expected if more than
> one type of second source component is defined and we end up deferring
> the second one? We don't undo the resolution of the first one and
> probably don't keep track of the fact that it succeeded?

That's right.

> Probably should be added to the function comments that this is an
> expected/normal case?

Added to the Return: section of the kernel-doc.

> > +                        */
> > +                       of_node_put(node);
> > +                       of_node_put(i2c_node);
> > +                       return 0;
> > +               }
> > +       }
> > +
> > +       i2c = of_get_i2c_adapter_by_node(i2c_node);
> > +       if (!i2c) {
> > +               of_node_put(i2c_node);
> > +               return dev_err_probe(dev, -EPROBE_DEFER, "Couldn't get I2C adapter\n");
> > +       }
> > +
> > +       ret = 0;
>
> Why init ret to 0?

It was requested by Andy. I believe the reason was having the initial
value used for bailout closer was better.

> > +       for_each_child_of_node(i2c_node, node) {
> > +               union i2c_smbus_data data;
> > +               u32 addr;
> > +
> > +               if (!of_node_name_prefix(node, type))
> > +                       continue;
> > +               if (of_property_read_u32(node, "reg", &addr))
> > +                       continue;
> > +               if (i2c_smbus_xfer(i2c, addr, 0, I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data) < 0)
>
> I'd be tempted to say that the caller should be able to pass in a
> function pointer here so they could use an alternative method to probe
> instead of i2c_smbus_xfer(), though you'd want to make it easy to
> default to i2c_smbus_xfer(). I could imagine someone might need a
> different way to probe. For instance if you had two touchscreens both
> at the same "reg" but that had different "hid-descr-addr" then this
> could be important.

I'd say the only specific probable type is hid-i2c. And that could be
generic enough that we could incorporate it here if we wanted. However
I think we want to keep the initial version a bit simpler.

> > +                       continue;
> > +
>
> Put the "break" right here. You've found the device and that was the
> point of the loop.

In its place we'd have an if (node) { <enable node> } block. I guess it
makes it easier to read still?

> Once you do that then the error handling makes a little more sense. It
> was weird that the error handling was jumped through from inside the
> loop...
>
>
> > +               dev_info(dev, "Enabling %pOF\n", node);
> > +
> > +               ocs = kzalloc(sizeof(*ocs), GFP_KERNEL);
> > +               if (!ocs) {
> > +                       ret = -ENOMEM;
> > +                       goto err_put_node;
>
> I think this error path (and some others) miss "i2c_put_adapter()" and
> "of_node_put(i2c_node)"

Right. I'll check.


Thanks
ChenYu

^ permalink raw reply

* [PATCH] HID: nintendo: add support for nso controllers
From: Ryan McClelland @ 2023-12-04  8:17 UTC (permalink / raw)
  To: linux-input; +Cc: djogorchock, benjamin.tissoires, jikos, Ryan McClelland

This adds support for the nintendo switch online controllers which
include the SNES, Genesis, and N64 Controllers.

As each nso controller only implements a subset of what a pro
controller can do. Each of these 'features' were broken up in to
seperate functions which include right stick, left stick, imu, and
dpad and depending on the controller type that it is, it will call
the supported functions appropriately.

Each controller now has a struct which maps the bit within the hid
in report to a button.

The name given to the device now comes directly from the hid
device name rather than looking up a predefined string.

Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
 drivers/hid/Kconfig        |  13 +-
 drivers/hid/hid-ids.h      |   5 +-
 drivers/hid/hid-nintendo.c | 897 ++++++++++++++++++++++++++-----------
 3 files changed, 649 insertions(+), 266 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce74af79657..347c284fb27e 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -761,14 +761,15 @@ config HID_MULTITOUCH
 	  module will be called hid-multitouch.
 
 config HID_NINTENDO
-	tristate "Nintendo Joy-Con and Pro Controller support"
+	tristate "Nintendo Joy-Con, NSO, and Pro Controller support"
 	depends on NEW_LEDS
 	depends on LEDS_CLASS
 	select POWER_SUPPLY
 	help
-	Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
+	Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller.
 	All controllers support bluetooth, and the Pro Controller also supports
-	its USB mode.
+	its USB mode. This also includes support for the Nintendo Switch Online
+	Controllers which include the Genesis, SNES, and N64 controllers.
 
 	To compile this driver as a module, choose M here: the
 	module will be called hid-nintendo.
@@ -779,9 +780,9 @@ config NINTENDO_FF
 	select INPUT_FF_MEMLESS
 	help
 	Say Y here if you have a Nintendo Switch controller and want to enable
-	force feedback support for it. This works for both joy-cons and the pro
-	controller. For the pro controller, both rumble motors can be controlled
-	individually.
+	force feedback support for it. This works for both joy-cons, the pro
+	controller, and the NSO N64 controller. For the pro controller, both
+	rumble motors can be controlled individually.
 
 config HID_NTI
 	tristate "NTI keyboard adapters"
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c6e4e0d1f214..a90aa3c31dd0 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -986,7 +986,10 @@
 #define USB_DEVICE_ID_NINTENDO_JOYCONL	0x2006
 #define USB_DEVICE_ID_NINTENDO_JOYCONR	0x2007
 #define USB_DEVICE_ID_NINTENDO_PROCON	0x2009
-#define USB_DEVICE_ID_NINTENDO_CHRGGRIP	0x200E
+#define USB_DEVICE_ID_NINTENDO_CHRGGRIP	0x200e
+#define USB_DEVICE_ID_NINTENDO_SNESCON	0x2017
+#define USB_DEVICE_ID_NINTENDO_GENCON	0x201e
+#define USB_DEVICE_ID_NINTENDO_N64CON	0x2019
 
 #define USB_VENDOR_ID_NOVATEK		0x0603
 #define USB_DEVICE_ID_NOVATEK_PCT	0x0600
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 138f154fecef..47af111ef3a2 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -3,6 +3,9 @@
  * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
  *
  * Copyright (c) 2019-2021 Daniel J. Ogorchock <djogorchock@gmail.com>
+ * Portions Copyright (c) 2020 Nadia Holmquist Pedersen <nadia@nhp.sh>
+ * Copyright (c) 2022 Emily Strickland <linux@emily.st>
+ * Copyright (c) 2023 Ryan McClelland <rymcclel@gmail.com>
  *
  * The following resources/projects were referenced for this driver:
  *   https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
@@ -17,6 +20,9 @@
  * This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
  * Pro Controllers can either be used over USB or Bluetooth.
  *
+ * This driver also incorporates support for Nintendo Switch Online controllers
+ * for the NES, SNES, Sega Genesis, and N64.
+ *
  * The driver will retrieve the factory calibration info from the controllers,
  * so little to no user calibration should be required.
  *
@@ -305,9 +311,14 @@ enum joycon_ctlr_state {
 
 /* Controller type received as part of device info */
 enum joycon_ctlr_type {
-	JOYCON_CTLR_TYPE_JCL = 0x01,
-	JOYCON_CTLR_TYPE_JCR = 0x02,
-	JOYCON_CTLR_TYPE_PRO = 0x03,
+	JOYCON_CTLR_TYPE_JCL  = 0x01,
+	JOYCON_CTLR_TYPE_JCR  = 0x02,
+	JOYCON_CTLR_TYPE_PRO  = 0x03,
+	JOYCON_CTLR_TYPE_NESL = 0x09,
+	JOYCON_CTLR_TYPE_NESR = 0x0A,
+	JOYCON_CTLR_TYPE_SNES = 0x0B,
+	JOYCON_CTLR_TYPE_GEN  = 0x0D,
+	JOYCON_CTLR_TYPE_N64  = 0x0C,
 };
 
 struct joycon_stick_cal {
@@ -348,6 +359,137 @@ static const u32 JC_BTN_SL_L	= BIT(21);
 static const u32 JC_BTN_L	= BIT(22);
 static const u32 JC_BTN_ZL	= BIT(23);
 
+struct joycon_ctlr_button_mapping {
+	u32 code;
+	u32 bit;
+};
+
+/*
+ * D-pad is configured as buttons for the left Joy-Con only!
+ */
+static const struct joycon_ctlr_button_mapping left_joycon_button_mappings[] = {
+	{ BTN_TL,		JC_BTN_L,	},
+	{ BTN_TL2,		JC_BTN_ZL,	},
+	{ BTN_SELECT,		JC_BTN_MINUS,	},
+	{ BTN_THUMBL,		JC_BTN_LSTICK,	},
+	{ BTN_DPAD_UP,		JC_BTN_UP,	},
+	{ BTN_DPAD_DOWN,	JC_BTN_DOWN,	},
+	{ BTN_DPAD_LEFT,	JC_BTN_LEFT,	},
+	{ BTN_DPAD_RIGHT,	JC_BTN_RIGHT,	},
+	{ BTN_Z,		JC_BTN_CAP,	},
+	{ /* sentinel */ },
+};
+
+/*
+ * The unused *right*-side triggers become the SL/SR triggers for the *left*
+ * Joy-Con, if and only if we're not using a charging grip.
+ */
+static const struct joycon_ctlr_button_mapping left_joycon_s_button_mappings[] = {
+	{ BTN_TR,	JC_BTN_SL_L,	},
+	{ BTN_TR2,	JC_BTN_SR_L,	},
+	{ /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping right_joycon_button_mappings[] = {
+	{ BTN_EAST,	JC_BTN_A,	},
+	{ BTN_SOUTH,	JC_BTN_B,	},
+	{ BTN_NORTH,	JC_BTN_X,	},
+	{ BTN_WEST,	JC_BTN_Y,	},
+	{ BTN_TR,	JC_BTN_R,	},
+	{ BTN_TR2,	JC_BTN_ZR,	},
+	{ BTN_START,	JC_BTN_PLUS,	},
+	{ BTN_THUMBR,	JC_BTN_RSTICK,	},
+	{ BTN_MODE,	JC_BTN_HOME,	},
+	{ /* sentinel */ },
+};
+
+/*
+ * The unused *left*-side triggers become the SL/SR triggers for the *right*
+ * Joy-Con, if and only if we're not using a charging grip.
+ */
+static const struct joycon_ctlr_button_mapping right_joycon_s_button_mappings[] = {
+	{ BTN_TL,	JC_BTN_SL_R,	},
+	{ BTN_TL2,	JC_BTN_SR_R,	},
+	{ /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping procon_button_mappings[] = {
+	{ BTN_EAST,	JC_BTN_A,	},
+	{ BTN_SOUTH,	JC_BTN_B,	},
+	{ BTN_NORTH,	JC_BTN_X,	},
+	{ BTN_WEST,	JC_BTN_Y,	},
+	{ BTN_TL,	JC_BTN_L,	},
+	{ BTN_TR,	JC_BTN_R,	},
+	{ BTN_TL2,	JC_BTN_ZL,	},
+	{ BTN_TR2,	JC_BTN_ZR,	},
+	{ BTN_SELECT,	JC_BTN_MINUS,	},
+	{ BTN_START,	JC_BTN_PLUS,	},
+	{ BTN_THUMBL,	JC_BTN_LSTICK,	},
+	{ BTN_THUMBR,	JC_BTN_RSTICK,	},
+	{ BTN_MODE,	JC_BTN_HOME,	},
+	{ BTN_Z,	JC_BTN_CAP,	},
+	{ /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping nescon_button_mappings[] = {
+	{ BTN_SOUTH,	JC_BTN_A,	},
+	{ BTN_EAST,	JC_BTN_B,	},
+	{ BTN_TL,	JC_BTN_L,	},
+	{ BTN_TR,	JC_BTN_R,	},
+	{ BTN_SELECT,	JC_BTN_MINUS,	},
+	{ BTN_START,	JC_BTN_PLUS,	},
+	{ /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping snescon_button_mappings[] = {
+	{ BTN_EAST,	JC_BTN_A,	},
+	{ BTN_SOUTH,	JC_BTN_B,	},
+	{ BTN_NORTH,	JC_BTN_X,	},
+	{ BTN_WEST,	JC_BTN_Y,	},
+	{ BTN_TL,	JC_BTN_L,	},
+	{ BTN_TR,	JC_BTN_R,	},
+	{ BTN_TL2,	JC_BTN_ZL,	},
+	{ BTN_TR2,	JC_BTN_ZR,	},
+	{ BTN_SELECT,	JC_BTN_MINUS,	},
+	{ BTN_START,	JC_BTN_PLUS,	},
+	{ /* sentinel */ },
+};
+
+/*
+ * "A", "B", and "C" are mapped positionally, rather than by label (e.g., "A"
+ * gets assigned to BTN_EAST instead of BTN_A).
+ */
+static const struct joycon_ctlr_button_mapping gencon_button_mappings[] = {
+	{ BTN_SOUTH,	JC_BTN_A,	},
+	{ BTN_EAST,	JC_BTN_B,	},
+	{ BTN_WEST,	JC_BTN_R,	},
+	{ BTN_SELECT,	JC_BTN_ZR,	},
+	{ BTN_START,	JC_BTN_PLUS,	},
+	{ BTN_MODE,	JC_BTN_HOME,	},
+	{ BTN_Z,	JC_BTN_CAP,	},
+	{ /* sentinel */ },
+};
+
+/*
+ * N64's C buttons get assigned to d-pad directions and registered as buttons.
+ */
+static const struct joycon_ctlr_button_mapping n64con_button_mappings[] = {
+	{ BTN_A,		JC_BTN_A,	},
+	{ BTN_B,		JC_BTN_B,	},
+	{ BTN_TL2,		JC_BTN_ZL,	}, /* Z */
+	{ BTN_TL,		JC_BTN_L,	},
+	{ BTN_TR,		JC_BTN_R,	},
+	{ BTN_TR2,		JC_BTN_LSTICK,	}, /* ZR */
+	{ BTN_START,		JC_BTN_PLUS,	},
+	{ BTN_FORWARD,		JC_BTN_Y,	}, /* C UP */
+	{ BTN_BACK,		JC_BTN_ZR,	}, /* C DOWN */
+	{ BTN_LEFT,		JC_BTN_X,	}, /* C LEFT */
+	{ BTN_RIGHT,		JC_BTN_MINUS,	}, /* C RIGHT */
+	{ BTN_MODE,		JC_BTN_HOME,	},
+	{ BTN_Z,		JC_BTN_CAP,	},
+	{ /* sentinel */ },
+};
+
 enum joycon_msg_type {
 	JOYCON_MSG_TYPE_NONE,
 	JOYCON_MSG_TYPE_USB,
@@ -506,13 +648,182 @@ struct joycon_ctlr {
 /* Does this controller have inputs associated with left joycon? */
 #define jc_type_has_left(ctlr) \
 	(ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCL || \
-	 ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO)
+	 ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO || \
+	 ctlr->ctlr_type == JOYCON_CTLR_TYPE_N64)
 
 /* Does this controller have inputs associated with right joycon? */
 #define jc_type_has_right(ctlr) \
 	(ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCR || \
 	 ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO)
 
+
+/*
+ * Controller device helpers
+ *
+ * These look at the device ID known to the HID subsystem to identify a device,
+ * but take caution: some NSO devices lie about themselves (NES Joy-Cons and
+ * Sega Genesis controller). See type helpers below.
+ *
+ * These helpers are most useful early during the HID probe or in conjunction
+ * with the capability helpers below.
+ */
+static inline bool joycon_device_is_left_joycon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL;
+}
+
+static inline bool joycon_device_is_right_joycon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR;
+}
+
+static inline bool joycon_device_is_procon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_PROCON;
+}
+
+static inline bool joycon_device_is_chrggrip(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_CHRGGRIP;
+}
+
+static inline bool joycon_device_is_snescon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_SNESCON;
+}
+
+static inline bool joycon_device_is_gencon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_GENCON;
+}
+
+static inline bool joycon_device_is_n64con(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_N64CON;
+}
+
+static inline bool joycon_device_has_usb(struct joycon_ctlr *ctlr)
+{
+	return joycon_device_is_procon(ctlr) ||
+	       joycon_device_is_chrggrip(ctlr) ||
+	       joycon_device_is_snescon(ctlr) ||
+	       joycon_device_is_gencon(ctlr) ||
+	       joycon_device_is_n64con(ctlr);
+}
+
+/*
+ * Controller type helpers
+ *
+ * These are slightly different than the device-ID-based helpers above. They are
+ * generally more reliable, since they can distinguish between, e.g., Genesis
+ * versus SNES, or NES Joy-Cons versus regular Switch Joy-Cons. They're most
+ * useful for reporting available inputs. For other kinds of distinctions, see
+ * the capability helpers below.
+ *
+ * They have two major drawbacks: (1) they're not available until after we set
+ * the reporting method and then request the device info; (2) they can't
+ * distinguish all controllers (like the Charging Grip from the Pro controller.)
+ */
+static inline bool joycon_type_is_left_joycon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCL;
+}
+
+static inline bool joycon_type_is_right_joycon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCR;
+}
+
+static inline bool joycon_type_is_procon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO;
+}
+
+static inline bool joycon_type_is_snescon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_SNES;
+}
+
+static inline bool joycon_type_is_gencon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_GEN;
+}
+
+static inline bool joycon_type_is_n64con(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_N64;
+}
+
+static inline bool joycon_type_is_left_nescon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_NESL;
+}
+
+static inline bool joycon_type_is_right_nescon(struct joycon_ctlr *ctlr)
+{
+	return ctlr->ctlr_type == JOYCON_CTLR_TYPE_NESR;
+}
+
+static inline bool joycon_type_has_left_controls(struct joycon_ctlr *ctlr)
+{
+	return joycon_type_is_left_joycon(ctlr) ||
+	       joycon_type_is_procon(ctlr);
+}
+
+static inline bool joycon_type_has_right_controls(struct joycon_ctlr *ctlr)
+{
+	return joycon_type_is_right_joycon(ctlr) ||
+	       joycon_type_is_procon(ctlr);
+}
+
+static inline bool joycon_type_is_any_joycon(struct joycon_ctlr *ctlr)
+{
+	return joycon_type_is_left_joycon(ctlr) ||
+	       joycon_type_is_right_joycon(ctlr) ||
+	       joycon_device_is_chrggrip(ctlr);
+}
+
+static inline bool joycon_type_is_any_nescon(struct joycon_ctlr *ctlr)
+{
+	return joycon_type_is_left_nescon(ctlr) ||
+	       joycon_type_is_right_nescon(ctlr);
+}
+
+/*
+ * Controller capability helpers
+ *
+ * These helpers combine the use of the helpers above to detect certain
+ * capabilities during initialization. They are always accurate but (since they
+ * use type helpers) cannot be used early in the HID probe.
+ */
+static inline bool joycon_has_imu(struct joycon_ctlr *ctlr)
+{
+	return joycon_device_is_chrggrip(ctlr) ||
+	       joycon_type_is_any_joycon(ctlr) ||
+	       joycon_type_is_procon(ctlr);
+}
+
+static inline bool joycon_has_joysticks(struct joycon_ctlr *ctlr)
+{
+	return joycon_device_is_chrggrip(ctlr) ||
+	       joycon_type_is_any_joycon(ctlr) ||
+	       joycon_type_is_procon(ctlr) ||
+	       joycon_type_is_n64con(ctlr);
+}
+
+static inline bool joycon_has_rumble(struct joycon_ctlr *ctlr)
+{
+	return joycon_device_is_chrggrip(ctlr) ||
+	       joycon_type_is_any_joycon(ctlr) ||
+	       joycon_type_is_procon(ctlr) ||
+	       joycon_type_is_n64con(ctlr);
+}
+
+static inline bool joycon_using_usb(struct joycon_ctlr *ctlr)
+{
+	return ctlr->hdev->bus == BUS_USB;
+}
+
 static int __joycon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
 {
 	u8 *buf;
@@ -1283,15 +1594,10 @@ static void joycon_parse_imu_report(struct joycon_ctlr *ctlr,
 	}
 }
 
-static void joycon_parse_report(struct joycon_ctlr *ctlr,
-				struct joycon_input_report *rep)
+static void joycon_handle_rumble_report(struct joycon_ctlr *ctlr, struct joycon_input_report *rep)
 {
-	struct input_dev *dev = ctlr->input;
 	unsigned long flags;
-	u8 tmp;
-	u32 btns;
 	unsigned long msecs = jiffies_to_msecs(jiffies);
-	unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 	if (IS_ENABLED(CONFIG_NINTENDO_FF) && rep->vibrator_report &&
@@ -1310,11 +1616,21 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
 		queue_work(ctlr->rumble_queue, &ctlr->rumble_worker);
 	}
 
-	/* Parse the battery status */
+	spin_unlock_irqrestore(&ctlr->lock, flags);
+}
+
+static void joycon_parse_battery_status(struct joycon_ctlr *ctlr, struct joycon_input_report *rep)
+{
+	u8 tmp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctlr->lock, flags);
+
 	tmp = rep->bat_con;
 	ctlr->host_powered = tmp & BIT(0);
 	ctlr->battery_charging = tmp & BIT(4);
 	tmp = tmp >> 5;
+
 	switch (tmp) {
 	case 0: /* empty */
 		ctlr->battery_capacity = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
@@ -1336,102 +1652,121 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
 		hid_warn(ctlr->hdev, "Invalid battery status\n");
 		break;
 	}
+
 	spin_unlock_irqrestore(&ctlr->lock, flags);
+}
 
-	/* Parse the buttons and sticks */
-	btns = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
-
-	if (jc_type_has_left(ctlr)) {
-		u16 raw_x;
-		u16 raw_y;
-		s32 x;
-		s32 y;
-
-		/* get raw stick values */
-		raw_x = hid_field_extract(ctlr->hdev, rep->left_stick, 0, 12);
-		raw_y = hid_field_extract(ctlr->hdev,
-					  rep->left_stick + 1, 4, 12);
-		/* map the stick values */
-		x = joycon_map_stick_val(&ctlr->left_stick_cal_x, raw_x);
-		y = -joycon_map_stick_val(&ctlr->left_stick_cal_y, raw_y);
-		/* report sticks */
-		input_report_abs(dev, ABS_X, x);
-		input_report_abs(dev, ABS_Y, y);
-
-		/* report buttons */
-		input_report_key(dev, BTN_TL, btns & JC_BTN_L);
-		input_report_key(dev, BTN_TL2, btns & JC_BTN_ZL);
-		input_report_key(dev, BTN_SELECT, btns & JC_BTN_MINUS);
-		input_report_key(dev, BTN_THUMBL, btns & JC_BTN_LSTICK);
-		input_report_key(dev, BTN_Z, btns & JC_BTN_CAP);
-
-		if (jc_type_is_joycon(ctlr)) {
-			/* Report the S buttons as the non-existent triggers */
-			input_report_key(dev, BTN_TR, btns & JC_BTN_SL_L);
-			input_report_key(dev, BTN_TR2, btns & JC_BTN_SR_L);
-
-			/* Report d-pad as digital buttons for the joy-cons */
-			input_report_key(dev, BTN_DPAD_DOWN,
-					 btns & JC_BTN_DOWN);
-			input_report_key(dev, BTN_DPAD_UP, btns & JC_BTN_UP);
-			input_report_key(dev, BTN_DPAD_RIGHT,
-					 btns & JC_BTN_RIGHT);
-			input_report_key(dev, BTN_DPAD_LEFT,
-					 btns & JC_BTN_LEFT);
-		} else {
-			int hatx = 0;
-			int haty = 0;
-
-			/* d-pad x */
-			if (btns & JC_BTN_LEFT)
-				hatx = -1;
-			else if (btns & JC_BTN_RIGHT)
-				hatx = 1;
-			input_report_abs(dev, ABS_HAT0X, hatx);
-
-			/* d-pad y */
-			if (btns & JC_BTN_UP)
-				haty = -1;
-			else if (btns & JC_BTN_DOWN)
-				haty = 1;
-			input_report_abs(dev, ABS_HAT0Y, haty);
-		}
-	}
-	if (jc_type_has_right(ctlr)) {
-		u16 raw_x;
-		u16 raw_y;
-		s32 x;
-		s32 y;
-
-		/* get raw stick values */
-		raw_x = hid_field_extract(ctlr->hdev, rep->right_stick, 0, 12);
-		raw_y = hid_field_extract(ctlr->hdev,
-					  rep->right_stick + 1, 4, 12);
-		/* map stick values */
-		x = joycon_map_stick_val(&ctlr->right_stick_cal_x, raw_x);
-		y = -joycon_map_stick_val(&ctlr->right_stick_cal_y, raw_y);
-		/* report sticks */
-		input_report_abs(dev, ABS_RX, x);
-		input_report_abs(dev, ABS_RY, y);
-
-		/* report buttons */
-		input_report_key(dev, BTN_TR, btns & JC_BTN_R);
-		input_report_key(dev, BTN_TR2, btns & JC_BTN_ZR);
-		if (jc_type_is_joycon(ctlr)) {
-			/* Report the S buttons as the non-existent triggers */
-			input_report_key(dev, BTN_TL, btns & JC_BTN_SL_R);
-			input_report_key(dev, BTN_TL2, btns & JC_BTN_SR_R);
-		}
-		input_report_key(dev, BTN_START, btns & JC_BTN_PLUS);
-		input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK);
-		input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME);
-		input_report_key(dev, BTN_WEST, btns & JC_BTN_Y);
-		input_report_key(dev, BTN_NORTH, btns & JC_BTN_X);
-		input_report_key(dev, BTN_EAST, btns & JC_BTN_A);
-		input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B);
+static void joycon_report_left_stick(struct joycon_ctlr *ctlr,
+				     struct joycon_input_report *rep)
+{
+	u16 raw_x;
+	u16 raw_y;
+	s32 x;
+	s32 y;
+
+	raw_x = hid_field_extract(ctlr->hdev, rep->left_stick, 0, 12);
+	raw_y = hid_field_extract(ctlr->hdev, rep->left_stick + 1, 4, 12);
+
+	x = joycon_map_stick_val(&ctlr->left_stick_cal_x, raw_x);
+	y = -joycon_map_stick_val(&ctlr->left_stick_cal_y, raw_y);
+
+	input_report_abs(ctlr->input, ABS_X, x);
+	input_report_abs(ctlr->input, ABS_Y, y);
+}
+
+static void joycon_report_right_stick(struct joycon_ctlr *ctlr,
+				      struct joycon_input_report *rep)
+{
+	u16 raw_x;
+	u16 raw_y;
+	s32 x;
+	s32 y;
+
+	raw_x = hid_field_extract(ctlr->hdev, rep->right_stick, 0, 12);
+	raw_y = hid_field_extract(ctlr->hdev, rep->right_stick + 1, 4, 12);
+
+	x = joycon_map_stick_val(&ctlr->right_stick_cal_x, raw_x);
+	y = -joycon_map_stick_val(&ctlr->right_stick_cal_y, raw_y);
+
+	input_report_abs(ctlr->input, ABS_RX, x);
+	input_report_abs(ctlr->input, ABS_RY, y);
+}
+
+static void joycon_report_dpad(struct joycon_ctlr *ctlr,
+			       struct joycon_input_report *rep)
+{
+	int hatx = 0;
+	int haty = 0;
+	u32 btns = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
+
+	if (btns & JC_BTN_LEFT)
+		hatx = -1;
+	else if (btns & JC_BTN_RIGHT)
+		hatx = 1;
+
+	if (btns & JC_BTN_UP)
+		haty = -1;
+	else if (btns & JC_BTN_DOWN)
+		haty = 1;
+
+	input_report_abs(ctlr->input, ABS_HAT0X, hatx);
+	input_report_abs(ctlr->input, ABS_HAT0Y, haty);
+}
+
+static void joycon_report_buttons(struct joycon_ctlr *ctlr,
+				  struct joycon_input_report *rep,
+				  const struct joycon_ctlr_button_mapping button_mappings[])
+{
+	const struct joycon_ctlr_button_mapping *button;
+	u32 status = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
+
+	for (button = button_mappings; button->code; button++)
+		input_report_key(ctlr->input, button->code, status & button->bit);
+}
+
+static void joycon_parse_report(struct joycon_ctlr *ctlr,
+				struct joycon_input_report *rep)
+{
+	unsigned long flags;
+	unsigned long msecs = jiffies_to_msecs(jiffies);
+	unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
+
+	if (joycon_has_rumble(ctlr))
+		joycon_handle_rumble_report(ctlr, rep);
+
+	joycon_parse_battery_status(ctlr, rep);
+
+	if (joycon_type_is_left_joycon(ctlr)) {
+		joycon_report_left_stick(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, left_joycon_button_mappings);
+		if (!joycon_device_is_chrggrip(ctlr))
+			joycon_report_buttons(ctlr, rep, left_joycon_s_button_mappings);
+	} else if (joycon_type_is_right_joycon(ctlr)) {
+		joycon_report_right_stick(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, right_joycon_button_mappings);
+		if (!joycon_device_is_chrggrip(ctlr))
+			joycon_report_buttons(ctlr, rep, right_joycon_s_button_mappings);
+	} else if (joycon_type_is_procon(ctlr)) {
+		joycon_report_left_stick(ctlr, rep);
+		joycon_report_right_stick(ctlr, rep);
+		joycon_report_dpad(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, procon_button_mappings);
+	} else if (joycon_type_is_any_nescon(ctlr)) {
+		joycon_report_dpad(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, nescon_button_mappings);
+	} else if (joycon_type_is_snescon(ctlr)) {
+		joycon_report_dpad(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, snescon_button_mappings);
+	} else if (joycon_type_is_gencon(ctlr)) {
+		joycon_report_dpad(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, gencon_button_mappings);
+	} else if (joycon_type_is_n64con(ctlr)) {
+		joycon_report_left_stick(ctlr, rep);
+		joycon_report_dpad(ctlr, rep);
+		joycon_report_buttons(ctlr, rep, n64con_button_mappings);
 	}
 
-	input_sync(dev);
+	input_sync(ctlr->input);
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 	ctlr->last_input_report_msecs = msecs;
@@ -1471,7 +1806,7 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
 	}
 
 	/* parse IMU data if present */
-	if (rep->id == JC_INPUT_IMU_DATA)
+	if ((rep->id == JC_INPUT_IMU_DATA) && joycon_has_imu(ctlr))
 		joycon_parse_imu_report(ctlr, rep);
 }
 
@@ -1684,123 +2019,65 @@ static int joycon_play_effect(struct input_dev *dev, void *data,
 }
 #endif /* IS_ENABLED(CONFIG_NINTENDO_FF) */
 
-static const unsigned int joycon_button_inputs_l[] = {
-	BTN_SELECT, BTN_Z, BTN_THUMBL,
-	BTN_TL, BTN_TL2,
-	0 /* 0 signals end of array */
-};
-
-static const unsigned int joycon_button_inputs_r[] = {
-	BTN_START, BTN_MODE, BTN_THUMBR,
-	BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
-	BTN_TR, BTN_TR2,
-	0 /* 0 signals end of array */
-};
-
-/* We report joy-con d-pad inputs as buttons and pro controller as a hat. */
-static const unsigned int joycon_dpad_inputs_jc[] = {
-	BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
-	0 /* 0 signals end of array */
-};
-
-static int joycon_input_create(struct joycon_ctlr *ctlr)
+static void joycon_config_left_stick(struct input_dev *idev)
 {
-	struct hid_device *hdev;
-	const char *name;
-	const char *imu_name;
-	int ret;
-	int i;
-
-	hdev = ctlr->hdev;
+	input_set_abs_params(idev,
+			     ABS_X,
+			     -JC_MAX_STICK_MAG,
+			     JC_MAX_STICK_MAG,
+			     JC_STICK_FUZZ,
+			     JC_STICK_FLAT);
+	input_set_abs_params(idev,
+			     ABS_Y,
+			     -JC_MAX_STICK_MAG,
+			     JC_MAX_STICK_MAG,
+			     JC_STICK_FUZZ,
+			     JC_STICK_FLAT);
+}
 
-	switch (hdev->product) {
-	case USB_DEVICE_ID_NINTENDO_PROCON:
-		name = "Nintendo Switch Pro Controller";
-		imu_name = "Nintendo Switch Pro Controller IMU";
-		break;
-	case USB_DEVICE_ID_NINTENDO_CHRGGRIP:
-		if (jc_type_has_left(ctlr)) {
-			name = "Nintendo Switch Left Joy-Con (Grip)";
-			imu_name = "Nintendo Switch Left Joy-Con IMU (Grip)";
-		} else {
-			name = "Nintendo Switch Right Joy-Con (Grip)";
-			imu_name = "Nintendo Switch Right Joy-Con IMU (Grip)";
-		}
-		break;
-	case USB_DEVICE_ID_NINTENDO_JOYCONL:
-		name = "Nintendo Switch Left Joy-Con";
-		imu_name = "Nintendo Switch Left Joy-Con IMU";
-		break;
-	case USB_DEVICE_ID_NINTENDO_JOYCONR:
-		name = "Nintendo Switch Right Joy-Con";
-		imu_name = "Nintendo Switch Right Joy-Con IMU";
-		break;
-	default: /* Should be impossible */
-		hid_err(hdev, "Invalid hid product\n");
-		return -EINVAL;
-	}
+static void joycon_config_right_stick(struct input_dev *idev)
+{
+	input_set_abs_params(idev,
+			     ABS_RX,
+			     -JC_MAX_STICK_MAG,
+			     JC_MAX_STICK_MAG,
+			     JC_STICK_FUZZ,
+			     JC_STICK_FLAT);
+	input_set_abs_params(idev,
+			     ABS_RY,
+			     -JC_MAX_STICK_MAG,
+			     JC_MAX_STICK_MAG,
+			     JC_STICK_FUZZ,
+			     JC_STICK_FLAT);
+}
 
-	ctlr->input = devm_input_allocate_device(&hdev->dev);
-	if (!ctlr->input)
-		return -ENOMEM;
-	ctlr->input->id.bustype = hdev->bus;
-	ctlr->input->id.vendor = hdev->vendor;
-	ctlr->input->id.product = hdev->product;
-	ctlr->input->id.version = hdev->version;
-	ctlr->input->uniq = ctlr->mac_addr_str;
-	ctlr->input->name = name;
-	ctlr->input->phys = hdev->phys;
-	input_set_drvdata(ctlr->input, ctlr);
+static void joycon_config_dpad(struct input_dev *idev)
+{
+	input_set_abs_params(idev,
+			     ABS_HAT0X,
+			     -JC_MAX_DPAD_MAG,
+			     JC_MAX_DPAD_MAG,
+			     JC_DPAD_FUZZ,
+			     JC_DPAD_FLAT);
+	input_set_abs_params(idev,
+			     ABS_HAT0Y,
+			     -JC_MAX_DPAD_MAG,
+			     JC_MAX_DPAD_MAG,
+			     JC_DPAD_FUZZ,
+			     JC_DPAD_FLAT);
+}
 
-	/* set up sticks and buttons */
-	if (jc_type_has_left(ctlr)) {
-		input_set_abs_params(ctlr->input, ABS_X,
-				     -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
-				     JC_STICK_FUZZ, JC_STICK_FLAT);
-		input_set_abs_params(ctlr->input, ABS_Y,
-				     -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
-				     JC_STICK_FUZZ, JC_STICK_FLAT);
-
-		for (i = 0; joycon_button_inputs_l[i] > 0; i++)
-			input_set_capability(ctlr->input, EV_KEY,
-					     joycon_button_inputs_l[i]);
-
-		/* configure d-pad differently for joy-con vs pro controller */
-		if (hdev->product != USB_DEVICE_ID_NINTENDO_PROCON) {
-			for (i = 0; joycon_dpad_inputs_jc[i] > 0; i++)
-				input_set_capability(ctlr->input, EV_KEY,
-						     joycon_dpad_inputs_jc[i]);
-		} else {
-			input_set_abs_params(ctlr->input, ABS_HAT0X,
-					     -JC_MAX_DPAD_MAG, JC_MAX_DPAD_MAG,
-					     JC_DPAD_FUZZ, JC_DPAD_FLAT);
-			input_set_abs_params(ctlr->input, ABS_HAT0Y,
-					     -JC_MAX_DPAD_MAG, JC_MAX_DPAD_MAG,
-					     JC_DPAD_FUZZ, JC_DPAD_FLAT);
-		}
-	}
-	if (jc_type_has_right(ctlr)) {
-		input_set_abs_params(ctlr->input, ABS_RX,
-				     -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
-				     JC_STICK_FUZZ, JC_STICK_FLAT);
-		input_set_abs_params(ctlr->input, ABS_RY,
-				     -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
-				     JC_STICK_FUZZ, JC_STICK_FLAT);
-
-		for (i = 0; joycon_button_inputs_r[i] > 0; i++)
-			input_set_capability(ctlr->input, EV_KEY,
-					     joycon_button_inputs_r[i]);
-	}
+static void joycon_config_buttons(struct input_dev *idev,
+		 const struct joycon_ctlr_button_mapping button_mappings[])
+{
+	const struct joycon_ctlr_button_mapping *button;
 
-	/* Let's report joy-con S triggers separately */
-	if (hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL) {
-		input_set_capability(ctlr->input, EV_KEY, BTN_TR);
-		input_set_capability(ctlr->input, EV_KEY, BTN_TR2);
-	} else if (hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR) {
-		input_set_capability(ctlr->input, EV_KEY, BTN_TL);
-		input_set_capability(ctlr->input, EV_KEY, BTN_TL2);
-	}
+	for (button = button_mappings; button->code; button++)
+		input_set_capability(idev, EV_KEY, button->code);
+}
 
+static void joycon_config_rumble(struct joycon_ctlr *ctlr)
+{
 #if IS_ENABLED(CONFIG_NINTENDO_FF)
 	/* set up rumble */
 	input_set_capability(ctlr->input, EV_FF, FF_RUMBLE);
@@ -1813,10 +2090,15 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
 	joycon_set_rumble(ctlr, 0, 0, false);
 	ctlr->rumble_msecs = jiffies_to_msecs(jiffies);
 #endif
+}
 
-	ret = input_register_device(ctlr->input);
-	if (ret)
-		return ret;
+static int joycon_imu_input_create(struct joycon_ctlr *ctlr)
+{
+	struct hid_device *hdev;
+	const char *imu_name;
+	int ret;
+
+	hdev = ctlr->hdev;
 
 	/* configure the imu input device */
 	ctlr->imu_input = devm_input_allocate_device(&hdev->dev);
@@ -1828,8 +2110,14 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
 	ctlr->imu_input->id.product = hdev->product;
 	ctlr->imu_input->id.version = hdev->version;
 	ctlr->imu_input->uniq = ctlr->mac_addr_str;
-	ctlr->imu_input->name = imu_name;
 	ctlr->imu_input->phys = hdev->phys;
+
+	imu_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s (IMU)", ctlr->input->name);
+	if (!imu_name)
+		return -ENOMEM;
+
+	ctlr->imu_input->name = imu_name;
+
 	input_set_drvdata(ctlr->imu_input, ctlr);
 
 	/* configure imu axes */
@@ -1871,6 +2159,71 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
 	return 0;
 }
 
+static int joycon_input_create(struct joycon_ctlr *ctlr)
+{
+	struct hid_device *hdev;
+	int ret;
+
+	hdev = ctlr->hdev;
+
+	ctlr->input = devm_input_allocate_device(&hdev->dev);
+	if (!ctlr->input)
+		return -ENOMEM;
+	ctlr->input->id.bustype = hdev->bus;
+	ctlr->input->id.vendor = hdev->vendor;
+	ctlr->input->id.product = hdev->product;
+	ctlr->input->id.version = hdev->version;
+	ctlr->input->uniq = ctlr->mac_addr_str;
+	ctlr->input->name = hdev->name;
+	ctlr->input->phys = hdev->phys;
+	input_set_drvdata(ctlr->input, ctlr);
+
+	ret = input_register_device(ctlr->input);
+	if (ret)
+		return ret;
+
+	if (joycon_type_is_right_joycon(ctlr)) {
+		joycon_config_right_stick(ctlr->input);
+		joycon_config_buttons(ctlr->input, right_joycon_button_mappings);
+		if (!joycon_device_is_chrggrip(ctlr))
+			joycon_config_buttons(ctlr->input, right_joycon_s_button_mappings);
+	} else if (joycon_type_is_left_joycon(ctlr)) {
+		joycon_config_left_stick(ctlr->input);
+		joycon_config_buttons(ctlr->input, left_joycon_button_mappings);
+		if (!joycon_device_is_chrggrip(ctlr))
+			joycon_config_buttons(ctlr->input, left_joycon_s_button_mappings);
+	} else if (joycon_type_is_procon(ctlr)) {
+		joycon_config_left_stick(ctlr->input);
+		joycon_config_right_stick(ctlr->input);
+		joycon_config_dpad(ctlr->input);
+		joycon_config_buttons(ctlr->input, procon_button_mappings);
+	} else if (joycon_type_is_any_nescon(ctlr)) {
+		joycon_config_dpad(ctlr->input);
+		joycon_config_buttons(ctlr->input, nescon_button_mappings);
+	} else if (joycon_type_is_snescon(ctlr)) {
+		joycon_config_dpad(ctlr->input);
+		joycon_config_buttons(ctlr->input, snescon_button_mappings);
+	} else if (joycon_type_is_gencon(ctlr)) {
+		joycon_config_dpad(ctlr->input);
+		joycon_config_buttons(ctlr->input, gencon_button_mappings);
+	} else if (joycon_type_is_n64con(ctlr)) {
+		joycon_config_dpad(ctlr->input);
+		joycon_config_left_stick(ctlr->input);
+		joycon_config_buttons(ctlr->input, n64con_button_mappings);
+	}
+
+	if (joycon_has_imu(ctlr)) {
+		ret = joycon_imu_input_create(ctlr);
+		if (ret)
+			return ret;
+	}
+
+	if (joycon_has_rumble(ctlr))
+		joycon_config_rumble(ctlr);
+
+	return 0;
+}
+
 /* Because the subcommand sets all the leds at once, the brightness argument is ignored */
 static int joycon_player_led_brightness_set(struct led_classdev *led,
 					    enum led_brightness brightness)
@@ -2107,9 +2460,7 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
 	struct joycon_input_report *report;
 
 	req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
-	mutex_lock(&ctlr->output_mutex);
 	ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
-	mutex_unlock(&ctlr->output_mutex);
 	if (ret) {
 		hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
 		return ret;
@@ -2132,8 +2483,17 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
 		return -ENOMEM;
 	hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
 
-	/* Retrieve the type so we can distinguish for charging grip */
+	/*
+	 * Retrieve the type so we can distinguish the controller type
+	 * Unfortantly the hdev->product can't always be used due to a ?bug?
+	 * with the NSO Genesis controller. Over USB, it will report the
+	 * PID as 0x201E, but over bluetooth it will report the PID as 0x2017
+	 * which is the same as the NSO SNES controller. This is different from
+	 * the rest of the controllers which will report the same PID over USB
+	 * and bluetooth.
+	 */
 	ctlr->ctlr_type = report->subcmd_reply.data[2];
+	hid_dbg(ctlr->hdev, "controller type = 0x%02X\n", ctlr->ctlr_type);
 
 	return 0;
 }
@@ -2145,8 +2505,7 @@ static int joycon_init(struct hid_device *hdev)
 
 	mutex_lock(&ctlr->output_mutex);
 	/* if handshake command fails, assume ble pro controller */
-	if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
-	    !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
+	if (joycon_using_usb(ctlr) && !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
 		hid_dbg(hdev, "detected USB controller\n");
 		/* set baudrate for improved latency */
 		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
@@ -2171,24 +2530,43 @@ static int joycon_init(struct hid_device *hdev)
 		goto out_unlock;
 	}
 
-	/* get controller calibration data, and parse it */
-	ret = joycon_request_calibration(ctlr);
+	/* needed to retrieve the controller type */
+	ret = joycon_read_info(ctlr);
 	if (ret) {
-		/*
-		 * We can function with default calibration, but it may be
-		 * inaccurate. Provide a warning, and continue on.
-		 */
-		hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+		hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
+			ret);
+		goto out_unlock;
 	}
 
-	/* get IMU calibration data, and parse it */
-	ret = joycon_request_imu_calibration(ctlr);
-	if (ret) {
-		/*
-		 * We can function with default calibration, but it may be
-		 * inaccurate. Provide a warning, and continue on.
-		 */
-		hid_warn(hdev, "Unable to read IMU calibration data\n");
+	if (joycon_has_joysticks(ctlr)) {
+		/* get controller calibration data, and parse it */
+		ret = joycon_request_calibration(ctlr);
+		if (ret) {
+			/*
+			 * We can function with default calibration, but it may be
+			 * inaccurate. Provide a warning, and continue on.
+			 */
+			hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+		}
+	}
+
+	if (joycon_has_imu(ctlr)) {
+		/* get IMU calibration data, and parse it */
+		ret = joycon_request_imu_calibration(ctlr);
+		if (ret) {
+			/*
+			 * We can function with default calibration, but it may be
+			 * inaccurate. Provide a warning, and continue on.
+			 */
+			hid_warn(hdev, "Unable to read IMU calibration data\n");
+		}
+
+		/* Enable the IMU */
+		ret = joycon_enable_imu(ctlr);
+		if (ret) {
+			hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
+			goto out_unlock;
+		}
 	}
 
 	/* Set the reporting mode to 0x30, which is the full report mode */
@@ -2198,18 +2576,13 @@ static int joycon_init(struct hid_device *hdev)
 		goto out_unlock;
 	}
 
-	/* Enable rumble */
-	ret = joycon_enable_rumble(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
-		goto out_unlock;
-	}
-
-	/* Enable the IMU */
-	ret = joycon_enable_imu(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
-		goto out_unlock;
+	if (joycon_has_rumble(ctlr)) {
+		/* Enable rumble */
+		ret = joycon_enable_rumble(ctlr);
+		if (ret) {
+			hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
+			goto out_unlock;
+		}
 	}
 
 out_unlock:
@@ -2354,13 +2727,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
 		goto err_close;
 	}
 
-	ret = joycon_read_info(ctlr);
-	if (ret) {
-		hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
-			ret);
-		goto err_close;
-	}
-
 	/* Initialize the leds */
 	ret = joycon_leds_create(ctlr);
 	if (ret) {
@@ -2432,6 +2798,12 @@ static int nintendo_hid_resume(struct hid_device *hdev)
 static const struct hid_device_id nintendo_hid_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
 			 USB_DEVICE_ID_NINTENDO_PROCON) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_SNESCON) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_GENCON) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_N64CON) },
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
 			 USB_DEVICE_ID_NINTENDO_PROCON) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
@@ -2440,6 +2812,12 @@ static const struct hid_device_id nintendo_hid_devices[] = {
 			 USB_DEVICE_ID_NINTENDO_JOYCONL) },
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
 			 USB_DEVICE_ID_NINTENDO_JOYCONR) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_SNESCON) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_GENCON) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+			 USB_DEVICE_ID_NINTENDO_N64CON) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
@@ -2458,6 +2836,7 @@ static struct hid_driver nintendo_hid_driver = {
 module_hid_driver(nintendo_hid_driver);
 
 MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Ryan McClelland <rymcclel@gmail.com>");
+MODULE_AUTHOR("Emily Strickland <linux@emily.st>");
 MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
 MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
-
-- 
2.34.1


^ permalink raw reply related

* Re: [RFC PATCH v3 3/5] platform/chrome: Introduce device tree hardware prober
From: Chen-Yu Tsai @ 2023-12-04  7:53 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
	Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
	Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
	linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
	keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAGXv+5Hz3wfjCRa2AiOQgOv7zo8bzAmtG=a=jWJhO2MZNrFtpw@mail.gmail.com>

On Mon, Dec 4, 2023 at 3:24 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > >
> > > @@ -61,6 +61,17 @@ config CHROMEOS_TBMC
> > >           To compile this driver as a module, choose M here: the
> > >           module will be called chromeos_tbmc.
> > >
> > > +config CHROMEOS_OF_HW_PROBER
> > > +       bool "ChromeOS Device Tree Hardware Prober"
> >
> > Any reason that it can't be a module?
>
> No technical one. However if it's a module, the user has to manually load
> it. So I think it's more of a usability thing.

We could probably manually add module aliases. I thought about aliases
against the machine compatibles, but there doesn't seem to be a device
for it to trigger. Or target something common to ChromeOS devices like
the EC? It's really hacky though.

ChenYu

> OOTH I think this needs to be a module if I2C is built as a module.
> Somehow I had thought of it at one point but then it slipped my mind.
>
> > > +       depends on OF
> > > +       depends on I2C
> > > +       select OF_DYNAMIC
> > > +       default OF
> >
> > You probably don't want "default OF". This means that everyone will
> > automatically get this new driver enabled which is unlikely to be
> > right.
>
> I thought this whole section was guarded behind KCONFIG_CHROME_PLATFORMS.
> So if the user has CHROME_PLATFORMS enabled and has OF enabled, they
> likely need the prober.
>
> > > +static int chromeos_of_hw_prober_probe(struct platform_device *pdev)
> > > +{
> > > +       for (size_t i = 0; i < ARRAY_SIZE(hw_prober_platforms); i++)
> > > +               if (of_machine_is_compatible(hw_prober_platforms[i].compatible)) {
> > > +                       int ret;
> > > +
> > > +                       ret = hw_prober_platforms[i].prober(&pdev->dev,
> > > +                                                           hw_prober_platforms[i].data);
> > > +                       if (ret)
> >
> > Should it only check for -EPROBE_DEFER here? ...and then maybe warn
> > for other cases and go through the loop? If there's some error
> > enabling the touchscreen I'd still want the trackpad to probe...
>
> Makes sense. However there's no extra information to give in the
> warning though.
>
> > > +                               return ret;
> > > +               }
> > > +
> > > +       return 0;
> >
> > Random thought: once we get here, the driver is useless / just wasting
> > memory. Any way to have it freed? ;-)
>
> I don't think there is a good way to do that, except maybe marking all
> the functions as __init? But that likely doesn't work in combination
> with deferred probing (say the i2c driver is a module).
>
> ChenYu

^ permalink raw reply

* Re: [RFC PATCH v3 3/5] platform/chrome: Introduce device tree hardware prober
From: Chen-Yu Tsai @ 2023-12-04  7:24 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
	Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
	Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
	linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
	keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=XV0+G=uFBE_n6WFGVW2szGcKToZgCNTdSrNf3LVk9MOQ@mail.gmail.com>

On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -61,6 +61,17 @@ config CHROMEOS_TBMC
> >           To compile this driver as a module, choose M here: the
> >           module will be called chromeos_tbmc.
> >
> > +config CHROMEOS_OF_HW_PROBER
> > +       bool "ChromeOS Device Tree Hardware Prober"
>
> Any reason that it can't be a module?

No technical one. However if it's a module, the user has to manually load
it. So I think it's more of a usability thing.

OOTH I think this needs to be a module if I2C is built as a module.
Somehow I had thought of it at one point but then it slipped my mind.

> > +       depends on OF
> > +       depends on I2C
> > +       select OF_DYNAMIC
> > +       default OF
>
> You probably don't want "default OF". This means that everyone will
> automatically get this new driver enabled which is unlikely to be
> right.

I thought this whole section was guarded behind KCONFIG_CHROME_PLATFORMS.
So if the user has CHROME_PLATFORMS enabled and has OF enabled, they
likely need the prober.

> > +static int chromeos_of_hw_prober_probe(struct platform_device *pdev)
> > +{
> > +       for (size_t i = 0; i < ARRAY_SIZE(hw_prober_platforms); i++)
> > +               if (of_machine_is_compatible(hw_prober_platforms[i].compatible)) {
> > +                       int ret;
> > +
> > +                       ret = hw_prober_platforms[i].prober(&pdev->dev,
> > +                                                           hw_prober_platforms[i].data);
> > +                       if (ret)
>
> Should it only check for -EPROBE_DEFER here? ...and then maybe warn
> for other cases and go through the loop? If there's some error
> enabling the touchscreen I'd still want the trackpad to probe...

Makes sense. However there's no extra information to give in the
warning though.

> > +                               return ret;
> > +               }
> > +
> > +       return 0;
>
> Random thought: once we get here, the driver is useless / just wasting
> memory. Any way to have it freed? ;-)

I don't think there is a good way to do that, except maybe marking all
the functions as __init? But that likely doesn't work in combination
with deferred probing (say the i2c driver is a module).

ChenYu

^ permalink raw reply

* Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail
From: Chen-Yu Tsai @ 2023-12-04  6:59 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
	Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
	Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
	linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
	keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=W01gfxV6RN2o6CVS7jjf8qgKP-jUy9Bp94d2hWzVC48A@mail.gmail.com>

On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> >                 reg = <0x2c>;
> >                 hid-descr-addr = <0x0020>;
> >                 wakeup-source;
> > +               status = "fail-needs-probe";
>
> While doing this, you could also remove the hack where the trackpad
> IRQ pinctrl is listed under i2c4.

Sure. I do think we can do away with it though. According to at least one
schematic, the interrupt line has pull-ups on both sides of the voltage
shifter.

BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
both sides of the voltage shifter as well.

ChenYu

^ permalink raw reply

* Re: [RFC PATCH v3 1/5] of: dynamic: Add of_changeset_update_prop_string
From: Chen-Yu Tsai @ 2023-12-04  6:28 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
	Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
	Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
	linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
	keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=V_v11eZ6+3gUwOvdWGNM9owG7zCK5EiezTY7RJ3eaEMw@mail.gmail.com>

On Sat, Dec 2, 2023 at 9:01 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -1039,3 +1039,50 @@ int of_changeset_add_prop_u32_array(struct of_changeset *ocs,
> >         return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(of_changeset_add_prop_u32_array);
> > +
> > +static int of_changeset_update_prop_helper(struct of_changeset *ocs,
> > +                                          struct device_node *np,
> > +                                          const struct property *pp)
> > +{
> > +       struct property *new_pp;
> > +       int ret;
> > +
> > +       new_pp = __of_prop_dup(pp, GFP_KERNEL);
> > +       if (!new_pp)
> > +               return -ENOMEM;
> > +
> > +       ret = of_changeset_update_property(ocs, np, new_pp);
> > +       if (ret) {
> > +               kfree(new_pp->name);
> > +               kfree(new_pp->value);
> > +               kfree(new_pp);
>
> Given that this is the 3rd copy of the freeing logic, does it make
> sense to make __of_prop_free() that's documented to free what was
> returned by __of_prop_dupe()?

Makes sense.  There's also one in property_list_free(). I'll add a patch
for it.

ChenYu

^ permalink raw reply

* RE: [PATCH] input/vmmouse: Fix device name copies
From: David Laight @ 2023-12-03 21:14 UTC (permalink / raw)
  To: 'Arnd Bergmann', Dmitry Torokhov, Zack Rusin
  Cc: linux-kernel@vger.kernel.org, VMware Graphics Reviewers,
	Robert Jarzmik, Raul Rangel, linux-input@vger.kernel.org,
	stable@vger.kernel.org
In-Reply-To: <d180f06b-64b0-4885-9794-5127c297a0f0@app.fastmail.com>

From: Arnd Bergmann
> Sent: 03 December 2023 20:51
> On Sun, Dec 3, 2023, at 19:41, Dmitry Torokhov wrote:
> > On Mon, Nov 27, 2023 at 03:42:06PM -0500, Zack Rusin wrote:
> >> From: Zack Rusin <zackr@vmware.com>
> >>
> >> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
> >> plus an extra string, extend it to 64.
> >>
> >> Fixes gcc13 warnings:
> >> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
> >> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing
> 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
> >>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> >>       |                                                     ^~~~~~~
> >> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a
> destination of size 32
> >>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> >>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>   456 |                  psmouse->ps2dev.serio->phys);
> >>       |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This simply wastes 32 bytes. It is perfectly fine to truncate phys
> > (which does not happen in real life).
> >
> > -Wformat-truncation is disabled in normal builds, folks should stop
> > using it with W=1 as well.
> 
> It does find real bugs, and we are fairly close to being able
> to enable it by default once the remaining warnings are all
> fixed.
> 
> It also doesn't waste any memory in this specific case since
> vmmouse_data is currently at 168 bytes, which gets rounded
> up to either 192 or 256 bytes anyway. I'd suggest using
> the minimum size that is large enough though, in this case
> 39 bytes for the string I guess.

That rather depends on whether any of the earlier char[] lengths
have been rounded up to a 'nice' value.

I'd also have thought that dangerous overflows would come from
unbounded %s formats, not fixed size strings or integers that are
always small.

There really ought to be a sane method of telling gcc not to bleat
about snprintf() potentially overflowing the target.

I've tried a few thing but none of them work.
IIRC using the result (in some ways) is enough, but neither
(void)snprintf(...); or if (snprintf(...)); is enough
(but they 'fix' 'warn unused result').

	David

> 
>      Arnd

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH] input/vmmouse: Fix device name copies
From: Arnd Bergmann @ 2023-12-03 20:50 UTC (permalink / raw)
  To: Dmitry Torokhov, Zack Rusin
  Cc: linux-kernel, VMware Graphics Reviewers, Robert Jarzmik,
	Raul Rangel, linux-input, stable
In-Reply-To: <ZWzLvctpo1nNTMOo@google.com>

On Sun, Dec 3, 2023, at 19:41, Dmitry Torokhov wrote:
> On Mon, Nov 27, 2023 at 03:42:06PM -0500, Zack Rusin wrote:
>> From: Zack Rusin <zackr@vmware.com>
>> 
>> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
>> plus an extra string, extend it to 64.
>> 
>> Fixes gcc13 warnings:
>> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
>> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
>>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>>       |                                                     ^~~~~~~
>> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
>>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   456 |                  psmouse->ps2dev.serio->phys);
>>       |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This simply wastes 32 bytes. It is perfectly fine to truncate phys
> (which does not happen in real life).
>
> -Wformat-truncation is disabled in normal builds, folks should stop
> using it with W=1 as well.

It does find real bugs, and we are fairly close to being able
to enable it by default once the remaining warnings are all
fixed.

It also doesn't waste any memory in this specific case since
vmmouse_data is currently at 168 bytes, which gets rounded
up to either 192 or 256 bytes anyway. I'd suggest using
the minimum size that is large enough though, in this case
39 bytes for the string I guess.

     Arnd

^ permalink raw reply

* Re: [PATCH] ipaq-micro-keys: Add error handling for devm_kmemdup
From: Dmitry Torokhov @ 2023-12-03 19:06 UTC (permalink / raw)
  To: Haoran Liu; +Cc: linux-input, linux-kernel
In-Reply-To: <20231203164653.38983-1-liuhaoran14@163.com>

On Sun, Dec 03, 2023 at 08:46:53AM -0800, Haoran Liu wrote:
> Check the return value of i2c_add_adapter. Static analysis revealed that
> the function did not properly handle potential failures of
> i2c_add_adapter, which could lead to partial initialization of the I2C
> adapter and unstable operation.
> 
> Signed-off-by: Haoran Liu <liuhaoran14@163.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input/vmmouse: Fix device name copies
From: Dmitry Torokhov @ 2023-12-03 18:41 UTC (permalink / raw)
  To: Zack Rusin
  Cc: linux-kernel, VMware Graphics Reviewers, Arnd Bergmann,
	Robert Jarzmik, Raul Rangel, linux-input, stable
In-Reply-To: <20231127204206.3593559-1-zack@kde.org>

Zack,

On Mon, Nov 27, 2023 at 03:42:06PM -0500, Zack Rusin wrote:
> From: Zack Rusin <zackr@vmware.com>
> 
> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
> plus an extra string, extend it to 64.
> 
> Fixes gcc13 warnings:
> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>       |                                                     ^~~~~~~
> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   456 |                  psmouse->ps2dev.serio->phys);
>       |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Zack Rusin <zackr@vmware.com>
> Fixes: 8b8be51b4fd3 ("Input: add vmmouse driver")
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: VMware Graphics Reviewers <linux-graphics-maintainer@vmware.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Raul Rangel <rrangel@chromium.org>
> Cc: linux-input@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v4.1+
> ---
>  drivers/input/mouse/vmmouse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> index ea9eff7c8099..7248cada4c8c 100644
> --- a/drivers/input/mouse/vmmouse.c
> +++ b/drivers/input/mouse/vmmouse.c
> @@ -72,7 +72,7 @@
>   */
>  struct vmmouse_data {
>  	struct input_dev *abs_dev;
> -	char phys[32];
> +	char phys[64];

This simply wastes 32 bytes. It is perfectly fine to truncate phys
(which does not happen in real life).

-Wformat-truncation is disabled in normal builds, folks should stop
using it with W=1 as well.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: How to disable the touchscreen so it does not draw power?
From: Paul Menzel @ 2023-12-03 17:41 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Dmitry Torokhov, linux-input
In-Reply-To: <99dcbb8d-06a8-4c32-ab03-94bc3bf5658f@redhat.com>

Dear Hans,


Thank you for taking the time to reply.


Am 13.11.23 um 13:08 schrieb Hans de Goede:

> On 11/13/23 09:39, Paul Menzel wrote:

>> On a Dell XPS 13 9360 with a touchscreen, when it’s connect to an
>> external monitor, that is set up as the only display, the
>> touchscreen – although the internal monitor is disabled (in GNOME)
>> – is still active and draws power according to PowerTOP: >>
>>      $ uname -a
>>      Linux abreu 6.5.0-4-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.5.10-1 (2023-11-03) x86_64 GNU/Linux
>>      $ lsusb  | grep Touchscreen
>>      Bus 001 Device 003: ID 04f3:2234 Elan Microelectronics Corp. Touchscreen
>>      $ sudo LANG= powertop
>>      Power est.              Usage       Events/s    Category Description
>>        9.39 W    100.0%                      Device         USB device: DELL DA300 (Bizlink)
>>        1.39 W    100.0%                      Device         USB device: Touchscreen (ELAN)
>>
>> Is there a way to disable the touchscreen, so it does not draw power?
>>
>> `sudo modprobe -r hid-multitouch` also affects the touchpad, which is not wanted.
> 
> Ideally userspace would close the /dev/input/event node belonging to
> the touchscreen when the internal panel is off. Please file an issue
> for that against libinput (to add the plumbing for this to libinput,
> ultimately the wayland-compositor, e.g. mutter, then needs to use
> that plumbing).
I created issue *Disabling the panel of a touchscreen does not disable 
the input/touch part wasting energy* [1].

[…]


Kind regards,

Paul


[1]: https://gitlab.freedesktop.org/libinput/libinput/-/issues/956

^ permalink raw reply

* [PATCH] ipaq-micro-keys: Add error handling for devm_kmemdup
From: Haoran Liu @ 2023-12-03 16:46 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Haoran Liu

Check the return value of i2c_add_adapter. Static analysis revealed that
the function did not properly handle potential failures of
i2c_add_adapter, which could lead to partial initialization of the I2C
adapter and unstable operation.

Signed-off-by: Haoran Liu <liuhaoran14@163.com>
---
Although the error addressed by this patch may not occur in the current
environment, I still suggest implementing these error handling routines
if the function is not highly time-sensitive. As the environment evolves
or the code gets reused in different contexts, there's a possibility that
these errors might occur. In case you find this addition unnecessary, I
completely understand and respect your perspective. My intention was to
enhance the robustness of the code, but I acknowledge that practical
considerations and current functionality might not warrant this change
at this point.
---
 drivers/input/keyboard/ipaq-micro-keys.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
index 7b509bce2b33..1d71dd79ffd2 100644
--- a/drivers/input/keyboard/ipaq-micro-keys.c
+++ b/drivers/input/keyboard/ipaq-micro-keys.c
@@ -105,6 +105,9 @@ static int micro_key_probe(struct platform_device *pdev)
 	keys->codes = devm_kmemdup(&pdev->dev, micro_keycodes,
 			   keys->input->keycodesize * keys->input->keycodemax,
 			   GFP_KERNEL);
+	if (!keys->codes)
+		return -ENOMEM;
+
 	keys->input->keycode = keys->codes;
 
 	__set_bit(EV_KEY, keys->input->evbit);
-- 
2.17.1


^ permalink raw reply related

* [PATCH] Input: expand keycode space
From: Wei Shuai @ 2023-12-03 12:50 UTC (permalink / raw)
  To: Dmitry Torokhov,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list

Dear Dmitry Torokhov,

Today I found out that in 2008, you have expanded the number of
potential key codes from 512 to 768.

I request to expand keycode space again, from 0x2ff to 0x4ff. I have
tested this patch on Ubuntu Kernel, it works well with a USB HID
joystick that has more than 80 keys.

Story back to I found Linux kernel has max USB HID joystick buttons up
to 80, no more.  But Windows OS and Mac OS have no such limitations.

I have a USB HID joystick device, which has 104 buttons, larger than 80.

I did a lot of google search, but nothing I got. then I have to look
at Kernel source, to find out where this number max 80 comes from

Eventually, I found the final limitation

#define BTN_JOYSTICK 0x120
#define BTN_DEAD 0x12f
#define BTN_TRIGGER_HAPPY 0x2c0
#define KEY_MAX 0x2ff

include/uapi/linux/input-event-codes.h

according to function hidinput_configure_usage() in file drivers/hid/hid-input.c

the USB joystick button mapping is not a continuous space, generally speak
the mapping space is from

BTN_JOYSTICK~BTN_DEAD 0x120~0x12f=0x10
BTN_TRIGGER_HAPPY~KEY_MAX 0x2c0~0x2ff=40

0x10+0x40=0x60=80

and finally, I got the max limitation of 80 for joystick keys.

This patch is about expanding keycode space from 0x2ff to 0x4ff. I
expect this patch won't go with ease, because it affects user space
application definition as well. I believe sooner or later Linux kernel
will face keycode is not enough


Signed-off-by: Wei Shuai <cpuwolf@gmail.com>
---
 include/linux/mod_devicetable.h        | 2 +-
 include/uapi/linux/input-event-codes.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 8d764aab29de..35eb59ae1f19 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -311,7 +311,7 @@ struct pcmcia_device_id {
 /* Input */
 #define INPUT_DEVICE_ID_EV_MAX         0x1f
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING    0x71
-#define INPUT_DEVICE_ID_KEY_MAX                0x2ff
+#define INPUT_DEVICE_ID_KEY_MAX                0x4ff
 #define INPUT_DEVICE_ID_REL_MAX                0x0f
 #define INPUT_DEVICE_ID_ABS_MAX                0x3f
 #define INPUT_DEVICE_ID_MSC_MAX                0x07
diff --git a/include/uapi/linux/input-event-codes.h
b/include/uapi/linux/input-event-codes.h
index b6a835d37826..ad1b9bed3828 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -774,7 +774,7 @@

 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING    KEY_MUTE
-#define KEY_MAX                        0x2ff
+#define KEY_MAX                        0x4ff
 #define KEY_CNT                        (KEY_MAX+1)

 /*
--
2.17.1

^ permalink raw reply related

* RE: [PATCH v3 00/11] Convert DA906{1,2} bindings to json-schema
From: Biju Das @ 2023-12-03 12:43 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, biju.das.au,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <20231203-unthread-suffering-411df4cb0f4c@spud>

Hi Conor,

Thanks for the feedback.

> Subject: Re: [PATCH v3 00/11] Convert DA906{1,2} bindings to json-schema
> 
> On Sun, Dec 03, 2023 at 11:31:48AM +0000, Biju Das wrote:
> > Convert the below bindings to json-schema
> > 1) DA906{1,2} mfd bindings
> > 2) DA906{1,2,3} onkey bindings
> > 3) DA906{1,2,3} thermal bindings
> >
> > Also add fallback for DA9061 watchdog device and document
> > DA9063 watchdog device.
> 
> Horrible timing, you sent this after I started looking at the previous
> revision of the patchset :( I left some comments and tags on the previous
> version which I would imagine that some of them also apply here.


I missed some checks which is pointed out by bot. So I thought of sending v3
Before anyone started reviewing it.

Sure, I will take care you review comments in v2 when sending new version.

Cheers,
Biju



> 
> >
> > v2->v3:
> >  * Updated Maintainer entries for watchdog,onkey and thermal bindings
> >  * Fixed bot errors related to MAINTAINERS entry, invalid doc
> >    references and thermal examples by merging patch#4.
> >
> > v1->v2:
> >  Ref:
> > https://lore.kernel.org/all/20231201110840.37408-5-biju.das.jz@bp.rene
> > sas.com/
> >  * DA9062 and DA9061 merged with DA9063
> >  * Sorted the child devices
> >  * mfd,onkey and thermal are pointing to child bindings
> >
> >
> >
> > Biju Das (11):
> >   MAINTAINERS: Update da9062-watchdog bindings
> >   dt-bindings: watchdog: dlg,da9062-watchdog: Add fallback for DA9061
> >     watchdog
> >   dt-bindings: watchdog: dlg,da9062-watchdog: Document DA9063 watchdog
> >   dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
> >   dt-bindings: mfd: dlg,da9063: Update watchdog property
> >   dt-bindings: mfd: dlg,da9063: Update onkey property
> >   dt-bindings: mfd: dlg,da9063: Sort child devices
> >   dt-bindings: mfd: da9062: Update watchdog description
> >   dt-bindings: mfd: da9062: Update onkey description
> >   dt-bindings: mfd: da9062: Update thermal description
> >   dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
> >
> >  .../bindings/input/da9062-onkey.txt           |  47 ----
> >  .../bindings/input/dlg,da9062-onkey.yaml      |  60 +++++
> >  .../devicetree/bindings/mfd/da9062.txt        | 124 ----------
> >  .../devicetree/bindings/mfd/dlg,da9063.yaml   | 221 +++++++++++++++---
> >  .../bindings/thermal/da9062-thermal.txt       |  36 ---
> >  .../bindings/thermal/dlg,da9062-thermal.yaml  |  78 +++++++
> >  .../watchdog/dlg,da9062-watchdog.yaml         |  12 +-
> >  MAINTAINERS                                   |   6 +-
> >  8 files changed, 336 insertions(+), 248 deletions(-)  delete mode
> > 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
> >  delete mode 100644
> > Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> >  create mode 100644
> > Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
> >
> > --
> > 2.39.2
> >

^ permalink raw reply

* [PATCH] HID: apple: Add "hfd.cn" and "WKB603" to the list of non-apple keyboards
From: yjun @ 2023-12-03 11:50 UTC (permalink / raw)
  To: jkosina; +Cc: jikos, benjamin.tissoires, linux-input, yjun

JingZao(京造) WKB603 keyboard is a rebranded product
of Jamesdonkey RS2 keyboard, identified as "hfd.cn WKB603"
in wired mode, "WKB603" in bluetooth mode. Adding them
to the list of non-apple keyboards fixes function key.

Signed-off-by: Yan Jun <jerrysteve1101@gmail.com>
---
 drivers/hid/hid-apple.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index d9e9829b2200..b9c7c0ed7bcc 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -347,6 +347,8 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
 	{ "Hailuck" },
 	{ "Jamesdonkey" },
 	{ "A3R" },
+	{ "hfd.cn" },
+	{ "WKB603" },
 };
 
 static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v3 00/11] Convert DA906{1,2} bindings to json-schema
From: Conor Dooley @ 2023-12-03 11:43 UTC (permalink / raw)
  To: Biju Das
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input, devicetree,
	linux-pm, Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231203113159.92316-1-biju.das.jz@bp.renesas.com>

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

On Sun, Dec 03, 2023 at 11:31:48AM +0000, Biju Das wrote:
> Convert the below bindings to json-schema
> 1) DA906{1,2} mfd bindings
> 2) DA906{1,2,3} onkey bindings
> 3) DA906{1,2,3} thermal bindings
> 
> Also add fallback for DA9061 watchdog device and document
> DA9063 watchdog device.

Horrible timing, you sent this after I started looking at the previous
revision of the patchset :( I left some comments and tags on the
previous version which I would imagine that some of them also apply
here.

> 
> v2->v3:
>  * Updated Maintainer entries for watchdog,onkey and thermal bindings
>  * Fixed bot errors related to MAINTAINERS entry, invalid doc
>    references and thermal examples by merging patch#4. 
> 
> v1->v2:
>  Ref: https://lore.kernel.org/all/20231201110840.37408-5-biju.das.jz@bp.renesas.com/
>  * DA9062 and DA9061 merged with DA9063
>  * Sorted the child devices
>  * mfd,onkey and thermal are pointing to child bindings
> 
> 
> 
> Biju Das (11):
>   MAINTAINERS: Update da9062-watchdog bindings
>   dt-bindings: watchdog: dlg,da9062-watchdog: Add fallback for DA9061
>     watchdog
>   dt-bindings: watchdog: dlg,da9062-watchdog: Document DA9063 watchdog
>   dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
>   dt-bindings: mfd: dlg,da9063: Update watchdog property
>   dt-bindings: mfd: dlg,da9063: Update onkey property
>   dt-bindings: mfd: dlg,da9063: Sort child devices
>   dt-bindings: mfd: da9062: Update watchdog description
>   dt-bindings: mfd: da9062: Update onkey description
>   dt-bindings: mfd: da9062: Update thermal description
>   dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
> 
>  .../bindings/input/da9062-onkey.txt           |  47 ----
>  .../bindings/input/dlg,da9062-onkey.yaml      |  60 +++++
>  .../devicetree/bindings/mfd/da9062.txt        | 124 ----------
>  .../devicetree/bindings/mfd/dlg,da9063.yaml   | 221 +++++++++++++++---
>  .../bindings/thermal/da9062-thermal.txt       |  36 ---
>  .../bindings/thermal/dlg,da9062-thermal.yaml  |  78 +++++++
>  .../watchdog/dlg,da9062-watchdog.yaml         |  12 +-
>  MAINTAINERS                                   |   6 +-
>  8 files changed, 336 insertions(+), 248 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
>  create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
>  delete mode 100644 Documentation/devicetree/bindings/thermal/da9062-thermal.txt
>  create mode 100644 Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
> 
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v2 03/11] dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
From: Conor Dooley @ 2023-12-03 11:36 UTC (permalink / raw)
  To: Biju Das
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Support Opensource, linux-input, devicetree, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231202192536.266885-4-biju.das.jz@bp.renesas.com>

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]

On Sat, Dec 02, 2023 at 07:25:27PM +0000, Biju Das wrote:
> diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> new file mode 100644
> index 000000000000..34f2e00cf045
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/dlg,da9062-onkey.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dialog DA9061/62/63 OnKey Module
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +description: |
> +  This module is part of the DA9061/DA9062/DA9063. For more details about entire
> +  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
> +  For DA906{1,2,3} see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> +
> +  This module provides the KEY_POWER event.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - dlg,da9062-onkey
> +              - dlg,da9063-onkey
> +      - items:
> +          - enum:
> +              - dlg,da9061-onkey
> +          - const: dlg,da9062-onkey # da9062-onkey fallback

Same comments here.

Cheers,
Conor.

> +  dlg,disable-key-power:
> +    type: boolean
> +    description:
> +      Disable power-down using a long key-press. If this entry exists
> +      the OnKey driver will remove support for the KEY_POWER key press
> +      when triggered using a long press of the OnKey.
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pmic@58 {
> +        compatible = "dlg,da9063";
> +        reg = <0x58>;
> +        interrupt-parent = <&gpio6>;
> +        interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> +        interrupt-controller;
> +
> +        onkey {
> +          compatible = "dlg,da9063-onkey";
> +          dlg,disable-key-power;
> +        };
> +      };
> +    };
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* [PATCH v3 11/11] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Biju Das @ 2023-12-03 11:31 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones
  Cc: Biju Das, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input, devicetree,
	linux-pm, Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231203113159.92316-1-biju.das.jz@bp.renesas.com>

Convert the da9062 PMIC and da906{1,2} thermal device tree binding
documentation to json-schema.

Update the example to match reality and the MAINTAINERS entry for
da906{1,2} thermal device tree binding. Update description for
da9062-onkey bindings.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Fixed bot errors related to MAINTAINERS entry, invalid doc
   references and thermal examples by merging patch#4.
v2:
 * New patch
---
 .../bindings/input/dlg,da9062-onkey.yaml      |   1 -
 .../devicetree/bindings/mfd/da9062.txt        | 124 ------------
 .../devicetree/bindings/mfd/dlg,da9063.yaml   | 185 +++++++++++++++++-
 .../bindings/thermal/da9062-thermal.txt       |  36 ----
 .../bindings/thermal/dlg,da9062-thermal.yaml  |  78 ++++++++
 MAINTAINERS                                   |   2 +-
 6 files changed, 258 insertions(+), 168 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
 delete mode 100644 Documentation/devicetree/bindings/thermal/da9062-thermal.txt
 create mode 100644 Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml

diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
index 34f2e00cf045..859238d5df80 100644
--- a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
+++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
@@ -11,7 +11,6 @@ maintainers:
 
 description: |
   This module is part of the DA9061/DA9062/DA9063. For more details about entire
-  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
   For DA906{1,2,3} see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
 
   This module provides the KEY_POWER event.
diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
deleted file mode 100644
index 7dd641deea1d..000000000000
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ /dev/null
@@ -1,124 +0,0 @@
-* Dialog DA9062 Power Management Integrated Circuit (PMIC)
-
-Product information for the DA9062 and DA9061 devices can be found here:
-- https://www.dialog-semiconductor.com/products/da9062
-- https://www.dialog-semiconductor.com/products/da9061
-
-The DA9062 PMIC consists of:
-
-Device                   Supply Names    Description
-------                   ------------    -----------
-da9062-regulator        :               : LDOs & BUCKs
-da9062-rtc              :               : Real-Time Clock
-da9062-onkey            :               : On Key
-da9062-watchdog         :               : Watchdog Timer
-da9062-thermal          :               : Thermal
-da9062-gpio             :               : GPIOs
-
-The DA9061 PMIC consists of:
-
-Device                   Supply Names    Description
-------                   ------------    -----------
-da9062-regulator        :               : LDOs & BUCKs
-da9062-onkey            :               : On Key
-da9062-watchdog         :               : Watchdog Timer
-da9062-thermal          :               : Thermal
-
-======
-
-Required properties:
-
-- compatible : Should be
-    "dlg,da9062" for DA9062
-    "dlg,da9061" for DA9061
-- reg : Specifies the I2C slave address (this defaults to 0x58 but it can be
-  modified to match the chip's OTP settings).
-
-Optional properties:
-
-- gpio-controller : Marks the device as a gpio controller.
-- #gpio-cells     : Should be two. The first cell is the pin number and the
-                    second cell is used to specify the gpio polarity.
-
-See Documentation/devicetree/bindings/gpio/gpio.txt for further information on
-GPIO bindings.
-
-- interrupts : IRQ line information.
-- interrupt-controller
-
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
-further information on IRQ bindings.
-
-Sub-nodes:
-
-- regulators : This node defines the settings for the LDOs and BUCKs.
-  The DA9062 regulators are bound using their names listed below:
-
-    buck1    : BUCK_1
-    buck2    : BUCK_2
-    buck3    : BUCK_3
-    buck4    : BUCK_4
-    ldo1     : LDO_1
-    ldo2     : LDO_2
-    ldo3     : LDO_3
-    ldo4     : LDO_4
-
-  The DA9061 regulators are bound using their names listed below:
-
-    buck1    : BUCK_1
-    buck2    : BUCK_2
-    buck3    : BUCK_3
-    ldo1     : LDO_1
-    ldo2     : LDO_2
-    ldo3     : LDO_3
-    ldo4     : LDO_4
-
-  The component follows the standard regulator framework and the bindings
-  details of individual regulator device can be found in:
-  Documentation/devicetree/bindings/regulator/regulator.txt
-
-  regulator-initial-mode may be specified for buck regulators using mode values
-  from include/dt-bindings/regulator/dlg,da9063-regulator.h.
-
-- rtc : This node defines settings required for the Real-Time Clock associated
-  with the DA9062. There are currently no entries in this binding, however
-  compatible = "dlg,da9062-rtc" should be added if a node is created.
-
-- onkey : See ../input/dlg,da9062-onkey.yaml
-
-- watchdog: See ../watchdog/da9062-watchdog.yaml
-
-- thermal : See ../thermal/dlg,da9062-thermal.yaml
-
-Example:
-
-	pmic0: da9062@58 {
-		compatible = "dlg,da9062";
-		reg = <0x58>;
-		interrupt-parent = <&gpio6>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-
-		rtc {
-			compatible = "dlg,da9062-rtc";
-		};
-
-		regulators {
-			DA9062_BUCK1: buck1 {
-				regulator-name = "BUCK1";
-				regulator-min-microvolt = <300000>;
-				regulator-max-microvolt = <1570000>;
-				regulator-min-microamp = <500000>;
-				regulator-max-microamp = <2000000>;
-				regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
-				regulator-boot-on;
-			};
-			DA9062_LDO1: ldo1 {
-				regulator-name = "LDO_1";
-				regulator-min-microvolt = <900000>;
-				regulator-max-microvolt = <3600000>;
-				regulator-boot-on;
-			};
-		};
-	};
-
diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
index ecdef322723d..54bb23dbc73f 100644
--- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/mfd/dlg,da9063.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Dialog DA9063/DA9063L Power Management Integrated Circuit (PMIC)
+title: Dialog DA906{3L,3,2,1} Power Management Integrated Circuit (PMIC)
 
 maintainers:
   - Steve Twiss <stwiss.opensource@diasemi.com>
@@ -17,13 +17,17 @@ description: |
   moment where all voltage monitors are disabled. Next, as da9063 only supports
   UV *and* OV monitoring, both must be set to the same severity and value
   (0: disable, 1: enable).
-  Product information for the DA906{3L,3} devices can be found here:
+  Product information for the DA906{3L,3,2,1} devices can be found here:
   - https://www.dialog-semiconductor.com/products/da9063l
   - https://www.dialog-semiconductor.com/products/da9063
+  - https://www.dialog-semiconductor.com/products/da9062
+  - https://www.dialog-semiconductor.com/products/da9061
 
 properties:
   compatible:
     enum:
+      - dlg,da9061
+      - dlg,da9062
       - dlg,da9063
       - dlg,da9063l
 
@@ -38,6 +42,19 @@ properties:
   "#interrupt-cells":
     const: 2
 
+  gpio:
+    type: object
+    $ref: /schemas/gpio/gpio.yaml#
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-gpio
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
   onkey:
     $ref: /schemas/input/dlg,da9062-onkey.yaml
 
@@ -45,7 +62,7 @@ properties:
     type: object
     additionalProperties: false
     patternProperties:
-      "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$":
+      "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck[1-4])$":
         $ref: /schemas/regulator/regulator.yaml
         unevaluatedProperties: false
 
@@ -55,7 +72,12 @@ properties:
     unevaluatedProperties: false
     properties:
       compatible:
-        const: dlg,da9063-rtc
+        enum:
+          - dlg,da9063-rtc
+          - dlg,da9062-rtc
+
+  thermal:
+    $ref: /schemas/thermal/dlg,da9062-thermal.yaml
 
   watchdog:
     $ref: /schemas/watchdog/dlg,da9062-watchdog.yaml
@@ -63,8 +85,65 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-controller
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - dlg,da9063
+              - dlg,da9063l
+    then:
+      properties:
+        thermal: false
+        gpio: false
+        gpio-controller: false
+        "#gpio-cells": false
+        regulators:
+          patternProperties:
+            "^buck[1-4]$": false
+      required:
+        - interrupts
+        - interrupt-controller
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - dlg,da9062
+    then:
+      properties:
+        regulators:
+          patternProperties:
+            "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$": false
+      required:
+        - gpio
+        - onkey
+        - rtc
+        - thermal
+        - watchdog
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - dlg,da9061
+    then:
+      properties:
+        gpio: false
+        gpio-controller: false
+        "#gpio-cells": false
+        regulators:
+          patternProperties:
+            "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck4)$": false
+        rtc: false
+      required:
+        - onkey
+        - thermal
+        - watchdog
 
 additionalProperties: false
 
@@ -121,4 +200,98 @@ examples:
         };
       };
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/regulator/dlg,da9063-regulator.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@58 {
+        compatible = "dlg,da9062";
+        reg = <0x58>;
+        #interrupt-cells = <2>;
+        interrupt-parent = <&gpio1>;
+        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+        interrupt-controller;
+
+        gpio {
+          compatible = "dlg,da9062-gpio";
+          status = "disabled";
+        };
+
+        onkey {
+          compatible = "dlg,da9062-onkey";
+        };
+
+        regulators {
+          buck1 {
+            regulator-name = "vdd_arm";
+            regulator-min-microvolt = <925000>;
+            regulator-max-microvolt = <1380000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck2 {
+            regulator-name = "vdd_soc";
+            regulator-min-microvolt = <1150000>;
+            regulator-max-microvolt = <1380000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck3 {
+            regulator-name = "vdd_ddr3";
+            regulator-min-microvolt = <1500000>;
+            regulator-max-microvolt = <1500000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck4 {
+            regulator-name = "vdd_eth";
+            regulator-min-microvolt = <1200000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          ldo1 {
+            regulator-name = "vdd_snvs";
+            regulator-min-microvolt = <3000000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-always-on;
+          };
+          ldo2 {
+            regulator-name = "vdd_high";
+            regulator-min-microvolt = <3000000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-always-on;
+          };
+          ldo3 {
+            regulator-name = "vdd_eth_io";
+            regulator-min-microvolt = <2500000>;
+            regulator-max-microvolt = <2500000>;
+          };
+          ldo4 {
+            regulator-name = "vdd_emmc";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+            regulator-always-on;
+          };
+        };
+
+        rtc {
+          compatible = "dlg,da9062-rtc";
+          status = "disabled";
+        };
+
+        thermal {
+          compatible = "dlg,da9062-thermal";
+          status = "disabled";
+        };
+
+        watchdog {
+          compatible = "dlg,da9062-watchdog";
+          dlg,use-sw-pm;
+        };
+      };
+    };
 ...
diff --git a/Documentation/devicetree/bindings/thermal/da9062-thermal.txt b/Documentation/devicetree/bindings/thermal/da9062-thermal.txt
deleted file mode 100644
index e241bb5a5584..000000000000
--- a/Documentation/devicetree/bindings/thermal/da9062-thermal.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-* Dialog DA9062/61 TJUNC Thermal Module
-
-This module is part of the DA9061/DA9062. For more details about entire
-DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
-
-Junction temperature thermal module uses an interrupt signal to identify
-high THERMAL_TRIP_HOT temperatures for the PMIC device.
-
-Required properties:
-
-- compatible: should be one of the following valid compatible string lines:
-        "dlg,da9061-thermal", "dlg,da9062-thermal"
-        "dlg,da9062-thermal"
-
-Optional properties:
-
-- polling-delay-passive : Specify the polling period, measured in
-    milliseconds, between thermal zone device update checks.
-
-Example: DA9062
-
-	pmic0: da9062@58 {
-		thermal {
-			compatible = "dlg,da9062-thermal";
-			polling-delay-passive = <3000>;
-		};
-	};
-
-Example: DA9061 using a fall-back compatible for the DA9062 onkey driver
-
-	pmic0: da9061@58 {
-		thermal {
-			compatible = "dlg,da9061-thermal", "dlg,da9062-thermal";
-			polling-delay-passive = <3000>;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
new file mode 100644
index 000000000000..1c15b10e797c
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/dlg,da9062-thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dialog DA9062/61 TJUNC Thermal Module
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  This module is part of the DA9061/DA9062. For more details about entire
+  DA906{1,2} see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+
+  Junction temperature thermal module uses an interrupt signal to identify
+  high THERMAL_TRIP_HOT temperatures for the PMIC device.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - dlg,da9062-thermal
+      - items:
+          - enum:
+              - dlg,da9061-thermal
+          - const: dlg,da9062-thermal # da9062-thermal fallback
+
+  polling-delay-passive:
+    description:
+      Specify the polling period, measured in milliseconds, between
+      thermal zone device update checks.
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@58 {
+        compatible = "dlg,da9062";
+        reg = <0x58>;
+        interrupt-parent = <&gpio6>;
+        interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+        interrupt-controller;
+
+        gpio {
+          compatible = "dlg,da9062-gpio";
+          status = "disabled";
+        };
+
+        onkey {
+          compatible = "dlg,da9062-onkey";
+          status = "disabled";
+        };
+
+         rtc {
+          compatible = "dlg,da9062-rtc";
+          status = "disabled";
+        };
+
+        thermal {
+          compatible = "dlg,da9062-thermal";
+          polling-delay-passive = <3000>;
+        };
+
+        watchdog {
+          compatible = "dlg,da9062-watchdog";
+          status = "disabled";
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index f2afaa22229d..d629eb14ec47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6136,7 +6136,7 @@ F:	Documentation/devicetree/bindings/regulator/da92*.txt
 F:	Documentation/devicetree/bindings/regulator/dlg,da9*.yaml
 F:	Documentation/devicetree/bindings/regulator/dlg,slg51000.yaml
 F:	Documentation/devicetree/bindings/sound/da[79]*.txt
-F:	Documentation/devicetree/bindings/thermal/da90??-thermal.txt
+F:	Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
 F:	Documentation/devicetree/bindings/watchdog/dlg,da9062-watchdog.yaml
 F:	Documentation/hwmon/da90??.rst
 F:	drivers/gpio/gpio-da90??.c
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 04/11] dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
From: Biju Das @ 2023-12-03 11:31 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Biju Das, Support Opensource, linux-input, devicetree,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231203113159.92316-1-biju.das.jz@bp.renesas.com>

Convert the da906{1,2,3} onkey device tree binding documentation to
json-schema.

Update MAINTAINERS entries.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Updated MAINTAINERS entries.
v2:
 * New patch
---
 .../bindings/input/da9062-onkey.txt           | 47 --------------
 .../bindings/input/dlg,da9062-onkey.yaml      | 61 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 62 insertions(+), 48 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
 create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml

diff --git a/Documentation/devicetree/bindings/input/da9062-onkey.txt b/Documentation/devicetree/bindings/input/da9062-onkey.txt
deleted file mode 100644
index e5eef59a93dc..000000000000
--- a/Documentation/devicetree/bindings/input/da9062-onkey.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-* Dialog DA9061/62/63 OnKey Module
-
-This module is part of the DA9061/DA9062/DA9063. For more details about entire
-DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
-For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
-
-This module provides the KEY_POWER event.
-
-Required properties:
-
-- compatible: should be one of the following valid compatible string lines:
-	"dlg,da9061-onkey", "dlg,da9062-onkey"
-	"dlg,da9062-onkey"
-	"dlg,da9063-onkey"
-
-Optional properties:
-
-- dlg,disable-key-power : Disable power-down using a long key-press. If this
-    entry exists the OnKey driver will remove support for the KEY_POWER key
-    press when triggered using a long press of the OnKey.
-
-Example: DA9063
-
-	pmic0: da9063@58 {
-		onkey {
-			compatible = "dlg,da9063-onkey";
-			dlg,disable-key-power;
-		};
-	};
-
-Example: DA9062
-
-	pmic0: da9062@58 {
-		onkey {
-			compatible = "dlg,da9062-onkey";
-			dlg,disable-key-power;
-		};
-	};
-
-Example: DA9061 using a fall-back compatible for the DA9062 onkey driver
-
-	pmic0: da9061@58 {
-		onkey {
-			compatible = "dlg,da9061-onkey", "dlg,da9062-onkey";
-			dlg,disable-key-power;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
new file mode 100644
index 000000000000..34f2e00cf045
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/dlg,da9062-onkey.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dialog DA9061/62/63 OnKey Module
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  This module is part of the DA9061/DA9062/DA9063. For more details about entire
+  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
+  For DA906{1,2,3} see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+
+  This module provides the KEY_POWER event.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - dlg,da9062-onkey
+              - dlg,da9063-onkey
+      - items:
+          - enum:
+              - dlg,da9061-onkey
+          - const: dlg,da9062-onkey # da9062-onkey fallback
+
+  dlg,disable-key-power:
+    type: boolean
+    description:
+      Disable power-down using a long key-press. If this entry exists
+      the OnKey driver will remove support for the KEY_POWER key press
+      when triggered using a long press of the OnKey.
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@58 {
+        compatible = "dlg,da9063";
+        reg = <0x58>;
+        interrupt-parent = <&gpio6>;
+        interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+        interrupt-controller;
+
+        onkey {
+          compatible = "dlg,da9063-onkey";
+          dlg,disable-key-power;
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index ab455fc2d64a..f2afaa22229d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6128,7 +6128,7 @@ DIALOG SEMICONDUCTOR DRIVERS
 M:	Support Opensource <support.opensource@diasemi.com>
 S:	Supported
 W:	http://www.dialog-semiconductor.com/products
-F:	Documentation/devicetree/bindings/input/da90??-onkey.txt
+F:	Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
 F:	Documentation/devicetree/bindings/input/dlg,da72??.txt
 F:	Documentation/devicetree/bindings/mfd/da90*.txt
 F:	Documentation/devicetree/bindings/mfd/dlg,da90*.yaml
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 00/11] Convert DA906{1,2} bindings to json-schema
From: Biju Das @ 2023-12-03 11:31 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones
  Cc: Biju Das, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input, devicetree,
	linux-pm, Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc

Convert the below bindings to json-schema
1) DA906{1,2} mfd bindings
2) DA906{1,2,3} onkey bindings
3) DA906{1,2,3} thermal bindings

Also add fallback for DA9061 watchdog device and document
DA9063 watchdog device.

v2->v3:
 * Updated Maintainer entries for watchdog,onkey and thermal bindings
 * Fixed bot errors related to MAINTAINERS entry, invalid doc
   references and thermal examples by merging patch#4. 

v1->v2:
 Ref: https://lore.kernel.org/all/20231201110840.37408-5-biju.das.jz@bp.renesas.com/
 * DA9062 and DA9061 merged with DA9063
 * Sorted the child devices
 * mfd,onkey and thermal are pointing to child bindings



Biju Das (11):
  MAINTAINERS: Update da9062-watchdog bindings
  dt-bindings: watchdog: dlg,da9062-watchdog: Add fallback for DA9061
    watchdog
  dt-bindings: watchdog: dlg,da9062-watchdog: Document DA9063 watchdog
  dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
  dt-bindings: mfd: dlg,da9063: Update watchdog property
  dt-bindings: mfd: dlg,da9063: Update onkey property
  dt-bindings: mfd: dlg,da9063: Sort child devices
  dt-bindings: mfd: da9062: Update watchdog description
  dt-bindings: mfd: da9062: Update onkey description
  dt-bindings: mfd: da9062: Update thermal description
  dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema

 .../bindings/input/da9062-onkey.txt           |  47 ----
 .../bindings/input/dlg,da9062-onkey.yaml      |  60 +++++
 .../devicetree/bindings/mfd/da9062.txt        | 124 ----------
 .../devicetree/bindings/mfd/dlg,da9063.yaml   | 221 +++++++++++++++---
 .../bindings/thermal/da9062-thermal.txt       |  36 ---
 .../bindings/thermal/dlg,da9062-thermal.yaml  |  78 +++++++
 .../watchdog/dlg,da9062-watchdog.yaml         |  12 +-
 MAINTAINERS                                   |   6 +-
 8 files changed, 336 insertions(+), 248 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
 create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
 delete mode 100644 Documentation/devicetree/bindings/thermal/da9062-thermal.txt
 create mode 100644 Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml

-- 
2.39.2


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox