* Re: [PATCH v10 2/2] Input: Add Novatek NT36xxx touchscreen driver
From: Joel Selvaraj @ 2023-08-16 1:09 UTC (permalink / raw)
To: Konrad Dybcio, AngeloGioacchino Del Regno, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg
Cc: Marijn Suijten, linux-input, devicetree, linux-kernel, Dang Huynh,
Amit Pundir
In-Reply-To: <20230808-topic-nt36xxx-v10-2-dd135dfa0b5e@linaro.org>
Hi Konrad Dybcio,
On 8/8/23 18:38, Konrad Dybcio wrote:
> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> This is a driver for the Novatek in-cell touch controller and
> supports various chips from the NT36xxx family, currently
> including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
In kernel v6.4, a basic novatek touchscreen driver was introduced [1].
I was able to tweak IT a bit (add devicetree compatible, regulator
support, remove chip id hardcode) and get it properly working in my
Xiaomi Poco F1 which has Novatek NT36672A touchscreen. Probably the
other ICs will also work. So, do we really need a separate touchscreen
driver? Maybe the existing one can be improved to add more features if
needed?
Personally I have been looking forward to the v10 of this patchseries :)
Thanks for working on this! But, yeah, we need to decide if we need this
to be a separate driver.
Link:
https://lore.kernel.org/all/20230326212308.55730-1-hdegoede@redhat.com/
Regards
Joel Selvaraj
^ permalink raw reply
* Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic CS40L50
From: James Ogletree @ 2023-08-15 22:33 UTC (permalink / raw)
To: Charles Keepax
Cc: Dmitry Torokhov, Fred Treven, Ben Bright, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
Joel Stanley, Arnd Bergmann, Jacky Bai, Jean Delvare, Eddie James,
Markus Schneider-Pargmann, ChiYuan Huang, Randy Dunlap,
Wolfram Sang, patches@opensource.cirrus.com,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20230810103005.GZ103419@ediswmail.ad.cirrus.com>
> On Aug 10, 2023, at 5:30 AM, Charles Keepax <ckeepax@opensource.cirrus.com> wrote:
>
> On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote:
>> +
>> +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data)
>> +{
>> +
>> +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles)
>> +{
>
> These pseq and OWT bits, could they be shared with l26?
> Definitely worth syncing with those guys, my assumption is the
> wavetable/pseq won't have changed much and it might be nice to
> factor these bits out into some library code that both drivers
> can use.
The pseq code most certainly can, likely the OWT code, perhaps others. I assume it is
acceptable to move some of these functions to a library in this patch set, even though this is
the only driver utilizing them as far as mainline is concerned? In other words, we shouldn’t
wait for one of L26 or L50 drivers to be accepted before splitting off the common code as part
of the others’ patchset? I’m probably overcomplicating; just want to be sure on the process here.
Everything else in your review will be fixed in V4. Thank you.
Regards,
James Ogletree
^ permalink raw reply
* Re: [PATCH] Input: pwm-beeper - Support volume setting via sysfs
From: Dmitry Torokhov @ 2023-08-15 21:33 UTC (permalink / raw)
To: Traut Manuel LCPF-CH
Cc: Takashi Iwai, Jeff LaBundy, Marek Vasut,
linux-input@vger.kernel.org, Uwe Kleine-König,
Frieder Schrempf, Thierry Reding, linux-pwm@vger.kernel.org,
alsa-devel@alsa-project.org, Jaroslav Kysela, Takashi Iwai
In-Reply-To: <AS8PR03MB7621C4E9B338259499AD24DBFA10A@AS8PR03MB7621.eurprd03.prod.outlook.com>
On Fri, Aug 11, 2023 at 10:47:34AM +0000, Traut Manuel LCPF-CH wrote:
> Hi
>
> > On Fri, 11 Aug 2023 06:19:50 +0200,
> > Jeff LaBundy wrote:
> > >
> > > Hi Marek, Dmitry and Takashi,
> > >
> > > On Tue, Aug 01, 2023 at 01:51:50PM +0200, Marek Vasut wrote:
> > > > On 8/1/23 09:28, Dmitry Torokhov wrote:
> > > > > On Mon, Jul 31, 2023 at 09:56:09PM -0500, Jeff LaBundy wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > On Mon, Jul 31, 2023 at 07:49:50PM +0200, Marek Vasut wrote:
> > > > > > > On 7/31/23 18:24, Dmitry Torokhov wrote:
> > > > > > > > On Mon, Jul 31, 2023 at 04:36:01PM +0200, Marek Vasut wrote:
> > > > > > > > > On 7/31/23 16:20, Takashi Iwai wrote:
> > > > > > > > >
> > > > > > > > > [...]
> > > > > > > > >
> > > > > > > > > > > > > Uh, I don't need a full sound device to emit
> > > > > > > > > > > > > beeps, that's not even possible with this hardware.
> > > > > > > > > > > >
> > > > > > > > > > > > Heh, I also don't recommend that route, either :)
> > > > > > > > > > > > (Though, it must be possible to create a sound
> > > > > > > > > > > > device with that beep control in theory)
> > > > > > > > > > >
> > > > > > > > > > > I mean, I can imagine one could possibly use PCM DMA
> > > > > > > > > > > to cook samples to feed some of the PWM devices so
> > > > > > > > > > > they could possibly be used to generate low quality
> > > > > > > > > > > audio, as a weird limited DAC, but ... that's not really generic,
> > and not what I want.
> > > > > > > > > >
> > > > > > > > > > Oh I see how the misunderstanding came; I didn't mean
> > > > > > > > > > the PCM implementation like pcsp driver. The pcsp
> > > > > > > > > > driver is a real hack and it's there just for fun, not for any real
> > practical use.
> > > > > > > > >
> > > > > > > > > Ah :)
> > > > > > > > >
> > > > > > > > > > What I meant was rather that you can create a sound
> > > > > > > > > > device containing a mixer volume control that serves
> > > > > > > > > > exactly like the sysfs or whatever other interface, without any
> > PCM stream or other interface.
> > > > > > > > >
> > > > > > > > > Ahhh, hum, I still feel like this might be a bit overkill here.
> > > > > > > > >
> > > > > > > > > > > > > I only need to control loudness of the beeper that
> > > > > > > > > > > > > is controlled by PWM output. That's why I am
> > > > > > > > > > > > > trying to extend the pwm-beeper driver, which
> > > > > > > > > > > > > seems the best fit for such a device, it is only missing this
> > one feature (loudness control).
> > > > > > > > > > > >
> > > > > > > > > > > > So the question is what's expected from user-space
> > > > > > > > > > > > POV. If a more generic control of beep volume is
> > > > > > > > > > > > required, e.g. for desktop-like usages, an implementation
> > of sound driver wouldn't be too bad.
> > > > > > > > > > > > OTOH, for other specific use-cases, it doesn't
> > > > > > > > > > > > matter much in which interface it's implemented, and sysfs
> > could be an easy choice.
> > > > > > > > > > >
> > > > > > > > > > > The whole discussion above has been exactly about
> > > > > > > > > > > this. Basically the thing is, we can either have:
> > > > > > > > > > > - SND_TONE (via some /dev/input/eventX) + sysfs volume
> > control
> > > > > > > > > > > -> This is simple, but sounds racy between input
> > > > > > > > > > > and sysfs accesses
> > > > > > > > > >
> > > > > > > > > > Hmm, how can it be racy if you do proper locking?
> > > > > > > > >
> > > > > > > > > I can imagine two applications can each grab one of the
> > > > > > > > > controls and that makes the interface a bit not nice. That
> > > > > > > > > would require extra synchronization in userspace and so on.
> > > > > > > > >
> > > > > > > > > > > - SND_TONE + SND_TONE_SET_VOLUME
> > > > > > > > > > > -> User needs to do two ioctls, hum
> > > > > > > > > > > - some new SND_TONE_WITH_VOLUME
> > > > > > > > > > > -> Probably the best option, user sets both tone frequency
> > and volume
> > > > > > > > > > > in one go, and it also only extends the IOCTL interface, so
> > older
> > > > > > > > > > > userspace won't have issues
> > > > > > > > > >
> > > > > > > > > > Those are "extensions" I have mentioned, and I'm not a
> > > > > > > > > > big fan for that, honestly speaking.
> > > > > > > > > >
> > > > > > > > > > The fact that the beep *output* stuff is provided by the
> > > > > > > > > > *input* device is already confusing
> > > > > > > > >
> > > > > > > > > I agree, this confused me as well.
> > > > > > > >
> > > > > > > > This comes from the times when keyboards themselves were
> > > > > > > > capable of emitting bells (SUN, DEC, etc). In hindsight it
> > > > > > > > was not the best way of structuring things, same with the
> > > > > > > > keyboard LEDs (that are now plugged into the LED subsystem, but
> > still allow be driven through input).
> > > > > > > >
> > > > > > > > And in the same vein I wonder if we should bite the bullet
> > > > > > > > and pay with a bit of complexity but move sound-related things to
> > sound subsystem.
> > > > > > >
> > > > > > > I am not sure that's the right approach here, since the device
> > > > > > > cannot do PCM playback, just bleeps.
> > > > > > >
> > > > > > > > > > (it was so just because of historical reason), and yet
> > > > > > > > > > you start implementing more full-featured mixer control.
> > > > > > > > > > I'd rather keep fingers away.
> > > > > > > > > >
> > > > > > > > > > Again, if user-space requires the compatible behavior
> > > > > > > > > > like the existing desktop usages
> > > > > > > > >
> > > > > > > > > It does not. These pwm-beeper devices keep showing up in
> > > > > > > > > various embedded devices these days.
> > > > > > > > >
> > > > > > > > > > , it can be implemented in a similar way like the
> > > > > > > > > > existing ones; i.e. provide a mixer control with a
> > > > > > > > > > proper sound device. The sound device doesn't need to
> > > > > > > > > > provide a PCM interface but just with a mixer interface.
> > > > > > > > > >
> > > > > > > > > > Or, if the purpose of your target device is a special
> > > > > > > > > > usage, you don't need to consider too much about the
> > > > > > > > > > existing interface, and try to keep the change as
> > > > > > > > > > minimal as possible without too intrusive API changes.
> > > > > > > > >
> > > > > > > > > My use case is almost perfectly matched by the current
> > > > > > > > > input pwm-beeper driver, the only missing bit is the
> > > > > > > > > ability to control the loudness at runtime. I think adding
> > > > > > > > > the SND_TONE_WITH_VOLUME parameter would cover it, with
> > least intrusive API changes.
> > > > > > > > >
> > > > > > > > > The SND_TONE already supports configuring tone frequency
> > > > > > > > > in Hz as its parameter. Since anything above 64 kHz is
> > > > > > > > > certainly not hearable by humans, I would say the
> > > > > > > > > SND_TONE_WITH_VOLUME could use 16 LSbits for frequency (so
> > up to 65535 Hz , 0 is OFF), and 16 MSbits for volume .
> > > > > > > > >
> > > > > > > > > I'm hesitant to overcomplicate something which can
> > > > > > > > > currently be controlled via single ioctl by pulling in sound
> > subsystem into the picture.
> > > > > > > >
> > > > > > > > Can you tell a bit more about your use case? What needs to
> > > > > > > > control the volume of beeps? Is this the only source of sounds on
> > the system?
> > > > > > >
> > > > > > > Custom user space application. The entire userspace is custom
> > > > > > > built in this case.
> > > > > > >
> > > > > > > In this case, it is a single-use device (think e.g. the kind
> > > > > > > of thermometer you stick in your ear when you're ill, to find out
> > how warm you are).
> > > > > > >
> > > > > > > The beeper there is used to do just that, bleep (with
> > > > > > > different frequencies to indicate different stuff), and that
> > > > > > > works. What I need in addition to that is control the volume
> > > > > > > of the bleeps from the application, so it isn't too noisy. And
> > > > > > > that needs to be user-controllable at runtime, so not something that
> > goes in DT.
> > > > > > >
> > > > > > > Right now there is just the bleeper , yes.
> > > > > >
> > > > > > It sounds like we essentially need an option within pcsp to
> > > > > > drive PWM instead of PCM, but input already has pwm-beeper; it
> > > > > > seems harmless to gently extend the latter for this use-case as
> > > > > > opposed to reworking the former.
> > > > > >
> > > > > > I agree that we should not invest too heavily in a legacy ABI,
> > > > > > however something like SND_BELL_VOL seems like a low-cost
> > > > > > addition that doesn't work against extending pcsp in the future.
> > > > > > In fact, input already has precedent for this exact same thing
> > > > > > by way of FF rumble effects, which are often PWM-based themselves.
> > > > > >
> > > > > > If SND_BELL_VOL or similar is not acceptable, then the original
> > > > > > sysfs approach seems like the next-best compromise. My only
> > > > > > issue with it was that I felt the range was not abstracted enough.
> > > > >
> > > > > If we want to extend the API we will need to define exactly how it
> > > > > will all work. I.e. what happens if userspace mixes the old
> > > > > SND_TONE and SND_BELL with the new SND_BELL_VOL or whatever.
> > Does
> > > > > it play with previously set volume? The default one?
> > > >
> > > > Default one, to preserve current behavior, yes.
> > >
> > > This was my idea as well, but I appreciate that the devil is in the
> > > details and each driver may have to duplicate some overhead.
> > >
> > > >
> > > > > How to set the default one?
> > > >
> > > > We do not, we can call pwm_get_duty_cycle() to get the current duty
> > > > cycle of the PWM to figure out the default.
> > > >
> > > > > How
> > > > > to figure out what the current volume is if we decide to make
> > > > > volume "sticky"?
> > > >
> > > > The patch stores the current volume configured via sysfs into
> > > > beeper->duty_cycle .
> > > >
> > > > > As far as userspace I expect it is more common to have one program
> > > > > (or component of a program) to set volume and then something else
> > > > > requests sound, so having one-shot API is of dubious value to me.
> > > >
> > > > Currently the use case I have for this is a single user facing
> > > > application which configures both.
> > > >
> > > > > I hope we can go with Takashi's proposal downthread, but if not I
> > > > > wonder if the sysfs approach is not the simplest one. Do we expect
> > > > > more beepers that can control volume besides pwm-beeper?
> > > >
> > > > It seems to me pulling in dependency on the entire sound subsystem
> > > > only to set beeper volume is overkill. I currently don't even have
> > > > sound subsystem compiled in.
> > >
> > > I like Takashi's patch; it seems like a more scalable solution.
> > > However, I can appreciate the reluctance to bring in the entire sound
> > > subsytem for what is probably a tiny piezoelectric buzzer.
> > >
> > > It seems like the sysfs solution is the best compromise in the
> > > meantime. If more and more users need to shoe-horn these kind of
> > > features in the future, we can make more informed decisions as to how to
> > extend the API (if at all).
> >
> > That's my impression, too. The original sysfs usage would be the right fit at
> > this moment.
>
> I am fine with both using the Sound API and sysfs. I would additionally like to
> specify the pwm values in device-tree like done in pwm-backlight. It really depends
> on the hardware which values actually make a difference in volume.
OK, let's go with the sysfs API for now as I am not sure if we have more
drivers being able to control volume of beeps.
Marek, I think there were some minor comments on the patch, could you
please address them and respin?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] HID: apple: Add "Hailuck" to the list of non-apple keyboards
From: Nils Tonnaett @ 2023-08-15 20:19 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
Cc: Rahul Rameshbabu, Nils Tonnaett
In-Reply-To: <20230811202932.30413-1-ntonnatt@ccrma.stanford.edu>
Powzan keyboards KB750 and KB770 identify as
"Hailuck Co.,Ltd USB Keyboard". Adding "Hailuck" to the list
of non-apple keyboards fixes function keys for these models.
Signed-off-by: Nils Tonnaett <ntonnatt@ccrma.stanford.edu>
---
V1 -> V2:
- Start commit message subject with HID: apple: instead of hid:
- Comma terminate last member of array
drivers/hid/hid-apple.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index d7b932925730..3ca45975c686 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -343,7 +343,8 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
{ "SONiX USB DEVICE" },
{ "Keychron" },
{ "AONE" },
- { "GANSS" }
+ { "GANSS" },
+ { "Hailuck" },
};
static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic CS40L50
From: Krzysztof Kozlowski @ 2023-08-15 19:23 UTC (permalink / raw)
To: James Ogletree
Cc: Dmitry Torokhov, Fred Treven, Ben Bright, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
Joel Stanley, Arnd Bergmann, Jacky Bai, Jean Delvare, Eddie James,
Markus Schneider-Pargmann, ChiYuan Huang, Randy Dunlap,
Wolfram Sang, patches@cirrus.com, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <D4B53566-9F7D-46DA-8BAE-0572B9A8AA7A@cirrus.com>
On 15/08/2023 17:56, James Ogletree wrote:
>
>
>> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 09/08/2023 21:10, James Ogletree wrote:
>>
>>> +
>>> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50)
>>> +{
>>> + cs40l50->dsp.num = 1;
>>> + cs40l50->dsp.type = WMFW_HALO;
>>> + cs40l50->dsp.dev = cs40l50->dev;
>>> + cs40l50->dsp.regmap = cs40l50->regmap;
>>> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE;
>>> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID;
>>> + cs40l50->dsp.mem = cs40l50_dsp_regions;
>>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
>>> + cs40l50->dsp.lock_regions = 0xFFFFFFFF;
>>> + cs40l50->dsp.no_core_startstop = true;
>>> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
>>> +
>>> + return cs_dsp_halo_init(&cs40l50->dsp);
>>> +}
>>> +
>>> +int cs40l50_probe(struct cs40l50_private *cs40l50)
>>> +{
>>> + int error, i, irq;
>>> + u32 val;
>>> +
>>> + mutex_init(&cs40l50->lock);
>>> +
>>> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies),
>>> + cs40l50_supplies);
>>> + if (error)
>>> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n");
>>> +
>>> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
>>> + if (error)
>>> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n");
>>> +
>>> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH);
>>
>> None of the lines above or below seem to be wrapped according to Linux
>> coding style (80).
>
> This patch abides by the 100-column line length limit which checkpatch.pl enforces.
Checkpatch does not enforce 100, because it accepts also 80. It
complains if something is longer than 100, but coding style is still the
same. Format everything according to coding style and use 100 only if it
increases readability.
> However, I can see how some of the lines might be less jarring to the eyes if wrapped.
> That will be addressed in V4.
>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 1/4] dt-bindings: touchscreen: convert neonode,zforce to json-schema
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
devicetree, linux-kernel, linux-arm-kernel
Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>
Convert Neonode infrared touchscreen controller binding to DT schema.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
.../input/touchscreen/neonode,zforce.yaml | 67 +++++++++++++++++++
.../bindings/input/touchscreen/zforce_ts.txt | 34 ----------
2 files changed, 67 insertions(+), 34 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
diff --git a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
new file mode 100644
index 000000000000..1c45adb2407a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/neonode,zforce.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Neonode infrared touchscreen controller
+
+maintainers:
+ - Heiko Stuebner <heiko.stuebner@bqreaders.com>
+
+properties:
+ compatible:
+ const: neonode,zforce
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ irq-gpios:
+ maxItems: 1
+
+ x-size:
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ vdd-supply: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - reset-gpios
+ - x-size
+ - y-size
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@50 {
+ compatible = "neonode,zforce";
+ reg = <0x50>;
+ interrupts = <2 0>;
+ vdd-supply = <®_zforce_vdd>;
+
+ reset-gpios = <&gpio5 9 0>; /* RST */
+ irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
+
+ x-size = <800>;
+ y-size = <600>;
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
deleted file mode 100644
index e3c27c4fd9c8..000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-* Neonode infrared touchscreen controller
-
-Required properties:
-- compatible: must be "neonode,zforce"
-- reg: I2C address of the chip
-- interrupts: interrupt to which the chip is connected
-- reset-gpios: reset gpio the chip is connected to
-- x-size: horizontal resolution of touchscreen
-- y-size: vertical resolution of touchscreen
-
-Optional properties:
-- irq-gpios : interrupt gpio the chip is connected to
-- vdd-supply: Regulator controlling the controller supply
-
-Example:
-
- i2c@00000000 {
- /* ... */
-
- zforce_ts@50 {
- compatible = "neonode,zforce";
- reg = <0x50>;
- interrupts = <2 0>;
- vdd-supply = <®_zforce_vdd>;
-
- reset-gpios = <&gpio5 9 0>; /* RST */
- irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
-
- x-size = <800>;
- y-size = <600>;
- };
-
- /* ... */
- };
--
2.39.2
^ permalink raw reply related
* [PATCH 4/4] ARM: dts: imx6sl-tolino-shine2hd: fix touchscreen rotation
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
devicetree, linux-kernel, linux-arm-kernel
Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>
The display is in landscape orientation, but the touchscreen is in portrait
orientation. Specify that properly in the devicetree.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts b/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
index 815119c12bd4..5636fb3661e8 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
@@ -141,8 +141,10 @@ zforce: touchscreen@50 {
interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
vdd-supply = <&ldo1_reg>;
reset-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;
- x-size = <1072>;
- y-size = <1448>;
+ touchscreen-size-x = <1072>;
+ touchscreen-size-y = <1448>;
+ touchscreen-swapped-x-y;
+ touchscreen-inverted-x;
};
/* TODO: TPS65185 PMIC for E Ink at 0x68 */
--
2.39.2
^ permalink raw reply related
* [PATCH 0/4] Input: zforce_ts: standard properties
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
devicetree, linux-kernel, linux-arm-kernel
Cc: Andreas Kemnade
Accept standard touchscreen properties to also enable specification
of touchscreen orientation.
Andreas Kemnade (4):
dt-bindings: touchscreen: convert neonode,zforce to json-schema
dt-bindings: touchscreen: neonode,zforce: Use standard properties
Input: zforce_ts: Accept standard touchscreen properties
ARM: dts: imx6sl-tolino-shine2hd: fix touchscreen rotation
.../input/touchscreen/neonode,zforce.yaml | 72 +++++++++++++++++++
.../bindings/input/touchscreen/zforce_ts.txt | 34 ---------
.../dts/nxp/imx/imx6sl-tolino-shine2hd.dts | 6 +-
drivers/input/touchscreen/zforce_ts.c | 36 +++++-----
4 files changed, 94 insertions(+), 54 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
--
2.39.2
^ permalink raw reply
* [PATCH 2/4] dt-bindings: touchscreen: neonode,zforce: Use standard properties
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
devicetree, linux-kernel, linux-arm-kernel
Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>
Enable touchscreen orientation to be specified by using standard
properties.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
.../bindings/input/touchscreen/neonode,zforce.yaml | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
index 1c45adb2407a..03513f38c9cc 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
@@ -9,6 +9,9 @@ title: Neonode infrared touchscreen controller
maintainers:
- Heiko Stuebner <heiko.stuebner@bqreaders.com>
+allOf:
+ - $ref: touchscreen.yaml#
+
properties:
compatible:
const: neonode,zforce
@@ -26,9 +29,11 @@ properties:
maxItems: 1
x-size:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
y-size:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
vdd-supply: true
@@ -38,8 +43,6 @@ required:
- reg
- interrupts
- reset-gpios
- - x-size
- - y-size
unevaluatedProperties: false
@@ -60,8 +63,10 @@ examples:
reset-gpios = <&gpio5 9 0>; /* RST */
irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
- x-size = <800>;
- y-size = <600>;
+ touchscreen-min-x = <0>;
+ touchscreen-size-x = <800>;
+ touchscreen-min-y = <0>;
+ touchscreen-size-y = <600>;
};
};
...
--
2.39.2
^ permalink raw reply related
* [PATCH 3/4] Input: zforce_ts: Accept standard touchscreen properties
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
devicetree, linux-kernel, linux-arm-kernel
Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>
Only driver-specific properties were accepted, change it
to use the now-available standard properties.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
drivers/input/touchscreen/zforce_ts.c | 36 +++++++++++++--------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 5be5112845e1..f4397497bbe9 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -20,6 +20,7 @@
#include <linux/device.h>
#include <linux/sysfs.h>
#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
#include <linux/platform_data/zforce_ts.h>
#include <linux/regulator/consumer.h>
#include <linux/of.h>
@@ -106,6 +107,7 @@ struct zforce_point {
struct zforce_ts {
struct i2c_client *client;
struct input_dev *input;
+ struct touchscreen_properties prop;
const struct zforce_ts_platdata *pdata;
char phys[32];
@@ -266,7 +268,6 @@ static int zforce_setconfig(struct zforce_ts *ts, char b1)
static int zforce_start(struct zforce_ts *ts)
{
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = ts->pdata;
int ret;
dev_dbg(&client->dev, "starting device\n");
@@ -277,7 +278,7 @@ static int zforce_start(struct zforce_ts *ts)
return ret;
}
- ret = zforce_resolution(ts, pdata->x_max, pdata->y_max);
+ ret = zforce_resolution(ts, ts->prop.max_x, ts->prop.max_y);
if (ret) {
dev_err(&client->dev, "Unable to set resolution, %d\n", ret);
goto error;
@@ -337,7 +338,6 @@ static int zforce_stop(struct zforce_ts *ts)
static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
{
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = ts->pdata;
struct zforce_point point;
int count, i, num = 0;
@@ -355,8 +355,8 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
point.coord_y =
payload[9 * i + 4] << 8 | payload[9 * i + 3];
- if (point.coord_x > pdata->x_max ||
- point.coord_y > pdata->y_max) {
+ if (point.coord_x > ts->prop.max_x ||
+ point.coord_y > ts->prop.max_y) {
dev_warn(&client->dev, "coordinates (%d,%d) invalid\n",
point.coord_x, point.coord_y);
point.coord_x = point.coord_y = 0;
@@ -390,10 +390,11 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
point.state != STATE_UP);
if (point.state != STATE_UP) {
- input_report_abs(ts->input, ABS_MT_POSITION_X,
- point.coord_x);
- input_report_abs(ts->input, ABS_MT_POSITION_Y,
- point.coord_y);
+ touchscreen_report_pos(ts->input,
+ &ts->prop,
+ point.coord_x,
+ point.coord_y,
+ true);
input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
point.area_major);
input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
@@ -719,15 +720,8 @@ static struct zforce_ts_platdata *zforce_parse_dt(struct device *dev)
return ERR_PTR(-ENOMEM);
}
- if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
- dev_err(dev, "failed to get x-size property\n");
- return ERR_PTR(-EINVAL);
- }
-
- if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
- dev_err(dev, "failed to get y-size property\n");
- return ERR_PTR(-EINVAL);
- }
+ of_property_read_u32(np, "x-size", &pdata->x_max);
+ of_property_read_u32(np, "y-size", &pdata->y_max);
return pdata;
}
@@ -856,6 +850,12 @@ static int zforce_probe(struct i2c_client *client)
input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
pdata->y_max, 0, 0);
+ touchscreen_parse_properties(input_dev, true, &ts->prop);
+ if (ts->prop.max_x == 0 || ts->prop.max_y == 0) {
+ dev_err(&client->dev, "no size specified\n");
+ return -EINVAL;
+ }
+
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
ZFORCE_MAX_AREA, 0, 0);
input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0,
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] drivers/input/touchscreen/goodix.c: Add support for ACPI ID GDX9110
From: Dmitry Torokhov @ 2023-08-15 18:39 UTC (permalink / raw)
To: Felix Engelhardt; +Cc: Bastien Nocera, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <20230807124723.382899-1-felix.engelhardt@eidu.com>
On Mon, Aug 07, 2023 at 02:47:23PM +0200, Felix Engelhardt wrote:
> The Goodix touchscreen controller with ACPI ID GDX9110 was not recognized
> by the goodix driver. This patch adds this ID to the list of supported IDs,
> allowing the driver to be used with this device. The change will allow
> Linux to be used on ~1 million tablet devices used in Kenyan primary
> schools.
>
> Signed-off-by: Felix Engelhardt <felix.engelhardt@eidu.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic CS40L50
From: James Ogletree @ 2023-08-15 15:56 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dmitry Torokhov, Fred Treven, Ben Bright, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
Joel Stanley, Arnd Bergmann, Jacky Bai, Jean Delvare, Eddie James,
Markus Schneider-Pargmann, ChiYuan Huang, Randy Dunlap,
Wolfram Sang, patches@cirrus.com, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <d1b9e063-54df-2dae-1487-c7c932fda145@linaro.org>
> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>
> On 09/08/2023 21:10, James Ogletree wrote:
>
>> +
>> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50)
>> +{
>> + cs40l50->dsp.num = 1;
>> + cs40l50->dsp.type = WMFW_HALO;
>> + cs40l50->dsp.dev = cs40l50->dev;
>> + cs40l50->dsp.regmap = cs40l50->regmap;
>> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE;
>> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID;
>> + cs40l50->dsp.mem = cs40l50_dsp_regions;
>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
>> + cs40l50->dsp.lock_regions = 0xFFFFFFFF;
>> + cs40l50->dsp.no_core_startstop = true;
>> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
>> +
>> + return cs_dsp_halo_init(&cs40l50->dsp);
>> +}
>> +
>> +int cs40l50_probe(struct cs40l50_private *cs40l50)
>> +{
>> + int error, i, irq;
>> + u32 val;
>> +
>> + mutex_init(&cs40l50->lock);
>> +
>> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies),
>> + cs40l50_supplies);
>> + if (error)
>> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n");
>> +
>> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
>> + if (error)
>> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n");
>> +
>> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH);
>
> None of the lines above or below seem to be wrapped according to Linux
> coding style (80).
This patch abides by the 100-column line length limit which checkpatch.pl enforces.
However, I can see how some of the lines might be less jarring to the eyes if wrapped.
That will be addressed in V4.
>
>> + if (IS_ERR(cs40l50->reset_gpio)) {
>> + error = PTR_ERR(cs40l50->reset_gpio);
>> + goto err;
>
> Why do you disable IRQ now?
>
> I asked to test this. Your entire cleanup path is:
> 1. not tested.
> 2. buggy.
> 3. done differently than Linux style. Use proper cleanup calls and
> multiple goto labels.
Disabling IRQ there is a mistake. The probe error path will be tightened up and made
to utilize multiple goto labels instead in V4.
Regards,
James Ogletree
^ permalink raw reply
* [PATCH v1 3/3] iio: hid-sensor: Use aligned data type for timestamp
From: Andy Shevchenko @ 2023-08-15 15:40 UTC (permalink / raw)
To: Andy Shevchenko, Jonathan Cameron, Lorenzo Bianconi,
Philipp Jungkamp, Peter Zijlstra, linux-input, linux-iio,
linux-kernel
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Lars-Peter Clausen
In-Reply-To: <20230815154027.12468-1-andriy.shevchenko@linux.intel.com>
Use aligned_s64 for the timestamp field.
Note, the actual data is signed, hence with this we also amend that.
While at it, drop redundant __alignment directive.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 3 +--
drivers/iio/gyro/hid-sensor-gyro-3d.c | 2 +-
drivers/iio/humidity/hid-sensor-humidity.c | 2 +-
drivers/iio/light/hid-sensor-als.c | 2 +-
drivers/iio/orientation/hid-sensor-incl-3d.c | 2 +-
drivers/iio/orientation/hid-sensor-rotation.c | 4 ++--
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 2 +-
drivers/iio/pressure/hid-sensor-press.c | 2 +-
drivers/iio/temperature/hid-sensor-temperature.c | 2 +-
9 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 5eac7ea19993..f739589564c5 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -25,10 +25,9 @@ struct accel_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info accel[ACCEL_3D_CHANNEL_MAX];
- /* Ensure timestamp is naturally aligned */
struct {
u32 accel_val[3];
- s64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index 698c50da1f10..a7050a6328d6 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -27,7 +27,7 @@ struct gyro_3d_state {
struct hid_sensor_hub_attribute_info gyro[GYRO_3D_CHANNEL_MAX];
struct {
u32 gyro_val[GYRO_3D_CHANNEL_MAX];
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index fa0fe404a70a..0e484b78b735 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -18,7 +18,7 @@ struct hid_humidity_state {
struct hid_sensor_hub_attribute_info humidity_attr;
struct {
s32 humidity_data;
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index eb1aedad7edc..8a906d95edd4 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -27,7 +27,7 @@ struct als_state {
struct hid_sensor_hub_attribute_info als_illum;
struct {
u32 illum[CHANNEL_SCAN_INDEX_MAX];
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index ba5b581d5b25..3e5f2c58dfa9 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -29,7 +29,7 @@ struct incl_3d_state {
struct hid_sensor_hub_attribute_info incl[INCLI_3D_CHANNEL_MAX];
struct {
u32 incl_val[INCLI_3D_CHANNEL_MAX];
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index a033699910e8..864ecbcad26e 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -19,8 +19,8 @@ struct dev_rot_state {
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info quaternion;
struct {
- s32 sampled_vals[4] __aligned(16);
- u64 timestamp __aligned(8);
+ s32 sampled_vals[4];
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
index 07c30d217255..48005b568dd9 100644
--- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
+++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
@@ -39,7 +39,7 @@ struct hinge_state {
const char *labels[CHANNEL_SCAN_INDEX_MAX];
struct {
u32 hinge_val[3];
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index a9215eb32d70..a964c7b65402 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -24,7 +24,7 @@ struct press_state {
struct hid_sensor_hub_attribute_info press_attr;
struct {
u32 press_data;
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index d40f235af1d4..32f4b13fd554 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -18,7 +18,7 @@ struct temperature_state {
struct hid_sensor_hub_attribute_info temperature_attr;
struct {
s32 temperature_data;
- u64 timestamp __aligned(8);
+ aligned_s64 timestamp;
} scan;
int scale_pre_decml;
int scale_post_decml;
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 1/3] types: Complement the aligned types with signed 64-bit one
From: Andy Shevchenko @ 2023-08-15 15:40 UTC (permalink / raw)
To: Andy Shevchenko, Jonathan Cameron, Lorenzo Bianconi,
Philipp Jungkamp, Peter Zijlstra, linux-input, linux-iio,
linux-kernel
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Lars-Peter Clausen
Some user may want to use aligned signed 64-bit type.
Provide it for them.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/types.h | 3 ++-
include/uapi/linux/types.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/types.h b/include/linux/types.h
index 253168bb3fe1..78d87c751ff5 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -115,8 +115,9 @@ typedef u64 u_int64_t;
typedef s64 int64_t;
#endif
-/* this is a special 64bit data type that is 8-byte aligned */
+/* These are the special 64-bit data types that are 8-byte aligned */
#define aligned_u64 __aligned_u64
+#define aligned_s64 __aligned_s64
#define aligned_be64 __aligned_be64
#define aligned_le64 __aligned_le64
diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h
index 6375a0684052..48b933938877 100644
--- a/include/uapi/linux/types.h
+++ b/include/uapi/linux/types.h
@@ -53,6 +53,7 @@ typedef __u32 __bitwise __wsum;
* No conversions are necessary between 32-bit user-space and a 64-bit kernel.
*/
#define __aligned_u64 __u64 __attribute__((aligned(8)))
+#define __aligned_s64 __s64 __attribute__((aligned(8)))
#define __aligned_be64 __be64 __attribute__((aligned(8)))
#define __aligned_le64 __le64 __attribute__((aligned(8)))
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH v1 2/3] iio: imu: st_lsm6dsx: Use aligned data type for timestamp
From: Andy Shevchenko @ 2023-08-15 15:40 UTC (permalink / raw)
To: Andy Shevchenko, Jonathan Cameron, Lorenzo Bianconi,
Philipp Jungkamp, Peter Zijlstra, linux-input, linux-iio,
linux-kernel
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
Lars-Peter Clausen
In-Reply-To: <20230815154027.12468-1-andriy.shevchenko@linux.intel.com>
Use __aligned_s64 for the timestamp field.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index c19237717e81..d3d4b3281aa6 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -442,10 +442,9 @@ struct st_lsm6dsx_hw {
const struct st_lsm6dsx_settings *settings;
struct iio_mount_matrix orientation;
- /* Ensure natural alignment of buffer elements */
struct {
__le16 channels[3];
- s64 ts __aligned(8);
+ __aligned_s64 ts;
} scan[ST_LSM6DSX_ID_MAX];
};
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related
* [PATCH -next] Input: walkera0701 - Use module_parport_driver macro to simplify the code
From: Li Zetao @ 2023-08-15 8:01 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: lizetao1, linux-input
Use the module_parport_driver macro to simplify the code, which is the
same as declaring with module_init() and module_exit().
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/input/joystick/walkera0701.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index 56abc8c6c763..27d95d6cf56e 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -296,15 +296,4 @@ static struct parport_driver walkera0701_parport_driver = {
.devmodel = true,
};
-static int __init walkera0701_init(void)
-{
- return parport_register_driver(&walkera0701_parport_driver);
-}
-
-static void __exit walkera0701_exit(void)
-{
- parport_unregister_driver(&walkera0701_parport_driver);
-}
-
-module_init(walkera0701_init);
-module_exit(walkera0701_exit);
+module_parport_driver(walkera0701_parport_driver);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v6 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Krzysztof Kozlowski @ 2023-08-15 7:33 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt,
robh+dt, agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input, devicetree
Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <20230815064917.387235-3-quic_fenglinw@quicinc.com>
On 15/08/2023 08:49, Fenglin Wu wrote:
> Add compatible strings to support vibrator module inside PMI632,
> PMI7250B, PM7325B, PM7550BA.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Riwen Lu @ 2023-08-15 6:58 UTC (permalink / raw)
To: Rahul Rameshbabu
Cc: Jiri Kosina, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
rrangel, u.kleine-koenig, linux-input, linux-kernel, Riwen Lu
In-Reply-To: <87fs4kn77j.fsf@protonmail.com>
在 2023/8/15 14:35, Rahul Rameshbabu 写道:
>
> On Tue, 15 Aug, 2023 14:02:40 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
>> 在 2023/8/14 17:26, Jiri Kosina 写道:
>>> On Mon, 3 Jul 2023, Riwen Lu wrote:
>>>
>>>> From: Riwen Lu <luriwen@kylinos.cn>
>>>>
>>>> The format '%*ph' print up to 64 bytes long as a hex string with ' '
>>>> sepatator. Usually the size of report descriptor is larger than 64
>>>> bytes, so consider using print_hex_dump_debug to print out all of it for
>>>> better debugging.
>>>>
>>>> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
>>>> ---
>>>> drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>> index efbba0465eef..8e97fc01c852 100644
>>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
>>>> }
>>>> }
>>>>
>>>> - i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
>>>> + i2c_hid_dbg(ihid, "Report Descriptor\n");
>>>> + print_hex_dump_debug(" ", DUMP_PREFIX_OFFSET, 16, 1,
>>>> + rdesc, rsize, false);
>
> Maybe it makes sense to use a prefix for the hex dump that is easy to
> trace rather than padding whitespace? This looks good when you do not
> see any other kernel message log lines get interlaced when written.
> However, if you have a lot of concurrent kernel message output, I think
> it can be tough to piece together the lines of the dump with this
> prefix. Just my opinion.
>
That's a good opinion. I'll make a v2 version.
Thanks.
>>>
>>> But that would dump it unconditionally, while i2c_hid_dbg() is
>>> conditional.
>>>
>> Function print_hex_dump_debug() dump messages is as conditional as
>> i2c_hid_dbg().
>>
>> The function i2c_hid_dbg() defines as follows:
>> #define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__)
>>
>> dev_dbg() depends on the same macro as print_hex_dump_debug().
>
> I agree with this point. Both dev_dbg and print_hex_dump_debug are noops
> if neither CONFIG_DYNAMIC_DEBUG is set or DEBUG is defined.
>
> --
> Thanks,
>
> Rahul Rameshbabu
>
^ permalink raw reply
* [PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu @ 2023-08-15 6:49 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar,
Luca Weiss
In-Reply-To: <20230815064917.387235-1-quic_fenglinw@quicinc.com>
Add new SPMI vibrator module which is very similar to the SPMI vibrator
module inside PM8916 but just has a finer drive voltage step (1mV vs
100mV) hence its drive level control is expanded to across 2 registers.
The vibrator module can be found in Qualcomm PMIC PMI632, then following
PM7250B, PM7325B, PM7550BA PMICs.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
---
drivers/input/misc/pm8xxx-vibrator.c | 55 +++++++++++++++++++++++++---
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index d6b468324c77..990e8a9ac018 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -21,6 +21,13 @@
#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
#define SPMI_VIB_DRV_SHIFT 0
+#define SPMI_VIB_GEN2_DRV_REG 0x40
+#define SPMI_VIB_GEN2_DRV_MASK GENMASK(7, 0)
+#define SPMI_VIB_GEN2_DRV_SHIFT 0
+#define SPMI_VIB_GEN2_DRV2_REG 0x41
+#define SPMI_VIB_GEN2_DRV2_MASK GENMASK(3, 0)
+#define SPMI_VIB_GEN2_DRV2_SHIFT 8
+
#define SPMI_VIB_EN_REG 0x46
#define SPMI_VIB_EN_BIT BIT(7)
@@ -33,12 +40,14 @@
enum vib_hw_type {
SSBI_VIB,
SPMI_VIB,
+ SPMI_VIB_GEN2
};
struct pm8xxx_vib_data {
enum vib_hw_type hw_type;
unsigned int enable_addr;
unsigned int drv_addr;
+ unsigned int drv2_addr;
};
static const struct pm8xxx_vib_data ssbi_vib_data = {
@@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
.drv_addr = SPMI_VIB_DRV_REG,
};
+static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
+ .hw_type = SPMI_VIB_GEN2,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_GEN2_DRV_REG,
+ .drv2_addr = SPMI_VIB_GEN2_DRV2_REG,
+};
+
/**
* struct pm8xxx_vib - structure to hold vibrator data
* @vib_input_dev: input device supporting force feedback
@@ -85,12 +101,24 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
{
int rc;
unsigned int val = vib->reg_vib_drv;
- u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
- u32 shift = SPMI_VIB_DRV_SHIFT;
+ u32 mask, shift;
- if (vib->data->hw_type == SSBI_VIB) {
+
+ switch (vib->data->hw_type) {
+ case SSBI_VIB:
mask = SSBI_VIB_DRV_LEVEL_MASK;
shift = SSBI_VIB_DRV_SHIFT;
+ break;
+ case SPMI_VIB:
+ mask = SPMI_VIB_DRV_LEVEL_MASK;
+ shift = SPMI_VIB_DRV_SHIFT;
+ break;
+ case SPMI_VIB_GEN2:
+ mask = SPMI_VIB_GEN2_DRV_MASK;
+ shift = SPMI_VIB_GEN2_DRV_SHIFT;
+ break;
+ default:
+ return -EINVAL;
}
if (on)
@@ -104,6 +132,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
vib->reg_vib_drv = val;
+ if (vib->data->hw_type == SPMI_VIB_GEN2) {
+ mask = SPMI_VIB_GEN2_DRV2_MASK;
+ shift = SPMI_VIB_GEN2_DRV2_SHIFT;
+ if (on)
+ val = (vib->level >> shift) & mask;
+ else
+ val = 0;
+ rc = regmap_update_bits(vib->regmap,
+ vib->reg_base + vib->data->drv2_addr, mask, val);
+ if (rc < 0)
+ return rc;
+ }
+
if (vib->data->hw_type == SSBI_VIB)
return 0;
@@ -128,10 +169,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
vib->active = true;
vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
VIB_MIN_LEVEL_mV;
- vib->level /= 100;
+ if (vib->data->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
} else {
vib->active = false;
- vib->level = VIB_MIN_LEVEL_mV / 100;
+ vib->level = VIB_MIN_LEVEL_mV;
+ if (vib->data->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
}
pm8xxx_vib_set(vib, vib->active);
@@ -266,6 +310,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
{ .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
+ { .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* [PATCH v6 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-08-15 6:49 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input, devicetree
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815064917.387235-1-quic_fenglinw@quicinc.com>
Add compatible strings to support vibrator module inside PMI632,
PMI7250B, PM7325B, PM7550BA.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
.../bindings/input/qcom,pm8xxx-vib.yaml | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index c8832cd0d7da..2025d6a5423e 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -11,10 +11,18 @@ maintainers:
properties:
compatible:
- enum:
- - qcom,pm8058-vib
- - qcom,pm8916-vib
- - qcom,pm8921-vib
+ oneOf:
+ - enum:
+ - qcom,pm8058-vib
+ - qcom,pm8916-vib
+ - qcom,pm8921-vib
+ - qcom,pmi632-vib
+ - items:
+ - enum:
+ - qcom,pm7250b-vib
+ - qcom,pm7325b-vib
+ - qcom,pm7550ba-vib
+ - const: qcom,pmi632-vib
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-08-15 6:49 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815064917.387235-1-quic_fenglinw@quicinc.com>
Currently, all vibrator control register addresses are hard coded,
including the base address and the offset, it's not flexible to support
new SPMI vibrator module which is usually included in different PMICs
with different base address. Refactor this by defining register offset
with HW type combination, and register base address which is defined
in 'reg' property is added for SPMI vibrators.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 122 ++++++++++++++++-----------
1 file changed, 73 insertions(+), 49 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..d6b468324c77 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,44 @@
#include <linux/regmap.h>
#include <linux/slab.h>
+#define SSBL_VIB_DRV_REG 0x4A
+#define SSBI_VIB_DRV_EN_MANUAL_MASK GENMASK(7, 2)
+#define SSBI_VIB_DRV_LEVEL_MASK GENMASK(7, 3)
+#define SSBI_VIB_DRV_SHIFT 3
+
+#define SPMI_VIB_DRV_REG 0x41
+#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
+#define SPMI_VIB_DRV_SHIFT 0
+
+#define SPMI_VIB_EN_REG 0x46
+#define SPMI_VIB_EN_BIT BIT(7)
+
#define VIB_MAX_LEVEL_mV (3100)
#define VIB_MIN_LEVEL_mV (1200)
#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
#define MAX_FF_SPEED 0xff
-struct pm8xxx_regs {
- unsigned int enable_addr;
- unsigned int enable_mask;
+enum vib_hw_type {
+ SSBI_VIB,
+ SPMI_VIB,
+};
- unsigned int drv_addr;
- unsigned int drv_mask;
- unsigned int drv_shift;
- unsigned int drv_en_manual_mask;
+struct pm8xxx_vib_data {
+ enum vib_hw_type hw_type;
+ unsigned int enable_addr;
+ unsigned int drv_addr;
};
-static const struct pm8xxx_regs pm8058_regs = {
- .drv_addr = 0x4A,
- .drv_mask = 0xf8,
- .drv_shift = 3,
- .drv_en_manual_mask = 0xfc,
+static const struct pm8xxx_vib_data ssbi_vib_data = {
+ .hw_type = SSBI_VIB,
+ .drv_addr = SSBL_VIB_DRV_REG,
};
-static struct pm8xxx_regs pm8916_regs = {
- .enable_addr = 0xc046,
- .enable_mask = BIT(7),
- .drv_addr = 0xc041,
- .drv_mask = 0x1F,
- .drv_shift = 0,
- .drv_en_manual_mask = 0,
+static const struct pm8xxx_vib_data spmi_vib_data = {
+ .hw_type = SPMI_VIB,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_DRV_REG,
};
/**
@@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
* @vib_input_dev: input device supporting force feedback
* @work: work structure to set the vibration parameters
* @regmap: regmap for register read/write
- * @regs: registers' info
+ * @data: vibrator HW info
+ * @reg_base: the register base of the module
* @speed: speed of vibration set from userland
* @active: state of vibrator
* @level: level of vibration to set in the chip
@@ -59,7 +68,8 @@ struct pm8xxx_vib {
struct input_dev *vib_input_dev;
struct work_struct work;
struct regmap *regmap;
- const struct pm8xxx_regs *regs;
+ const struct pm8xxx_vib_data *data;
+ unsigned int reg_base;
int speed;
int level;
bool active;
@@ -75,24 +85,31 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
{
int rc;
unsigned int val = vib->reg_vib_drv;
- const struct pm8xxx_regs *regs = vib->regs;
+ u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
+ u32 shift = SPMI_VIB_DRV_SHIFT;
+
+ if (vib->data->hw_type == SSBI_VIB) {
+ mask = SSBI_VIB_DRV_LEVEL_MASK;
+ shift = SSBI_VIB_DRV_SHIFT;
+ }
if (on)
- val |= (vib->level << regs->drv_shift) & regs->drv_mask;
+ val |= (vib->level << shift) & mask;
else
- val &= ~regs->drv_mask;
+ val &= ~mask;
- rc = regmap_write(vib->regmap, regs->drv_addr, val);
+ rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, val);
if (rc < 0)
return rc;
vib->reg_vib_drv = val;
- if (regs->enable_mask)
- rc = regmap_update_bits(vib->regmap, regs->enable_addr,
- regs->enable_mask, on ? ~0 : 0);
+ if (vib->data->hw_type == SSBI_VIB)
+ return 0;
- return rc;
+ mask = SPMI_VIB_EN_BIT;
+ val = on ? SPMI_VIB_EN_BIT : 0;
+ return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
}
/**
@@ -102,13 +119,6 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
static void pm8xxx_work_handler(struct work_struct *work)
{
struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
- const struct pm8xxx_regs *regs = vib->regs;
- int rc;
- unsigned int val;
-
- rc = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (rc < 0)
- return;
/*
* pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
@@ -168,9 +178,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
{
struct pm8xxx_vib *vib;
struct input_dev *input_dev;
+ const struct pm8xxx_vib_data *data;
int error;
- unsigned int val;
- const struct pm8xxx_regs *regs;
+ unsigned int val, reg_base;
vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
if (!vib)
@@ -187,19 +197,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
INIT_WORK(&vib->work, pm8xxx_work_handler);
vib->vib_input_dev = input_dev;
- regs = of_device_get_match_data(&pdev->dev);
+ data = of_device_get_match_data(&pdev->dev);
+ if (!data)
+ return -EINVAL;
- /* operate in manual mode */
- error = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (error < 0)
- return error;
+ if (data->hw_type != SSBI_VIB) {
+ error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", ®_base);
+ if (error < 0) {
+ dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
+ return error;
+ }
+
+ vib->reg_base += reg_base;
+ }
- val &= regs->drv_en_manual_mask;
- error = regmap_write(vib->regmap, regs->drv_addr, val);
+ error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
if (error < 0)
return error;
- vib->regs = regs;
+ /* operate in manual mode */
+ if (data->hw_type == SSBI_VIB) {
+ val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+ error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
+ if (error < 0)
+ return error;
+ }
+
+ vib->data = data;
vib->reg_vib_drv = val;
input_dev->name = "pm8xxx_vib_ffmemless";
@@ -239,9 +263,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
static const struct of_device_id pm8xxx_vib_id_table[] = {
- { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs },
+ { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v5 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-08-15 6:42 UTC (permalink / raw)
To: Luca Weiss, linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt,
robh+dt, agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input, devicetree
Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <CUSWRRL6QOPU.1YM7S0F8F3V2D@otso>
On 8/15/2023 2:35 PM, Luca Weiss wrote:
> Hi Fenglin,
>
> On Tue Aug 15, 2023 at 8:03 AM CEST, Fenglin Wu wrote:
>> Add compatible strings to support vibrator module inside PMI632,
>> PMI7250B, PM7325B, PM7550BA.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>> .../bindings/input/qcom,pm8xxx-vib.yaml | 15 +++++++++++----
>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> index c8832cd0d7da..72b72c67a9b6 100644
>> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> @@ -11,10 +11,17 @@ maintainers:
>>
>> properties:
>> compatible:
>> - enum:
>> - - qcom,pm8058-vib
>> - - qcom,pm8916-vib
>> - - qcom,pm8921-vib
>> + oneOf:
>> + - enum:
>> + - qcom,pm8058-vib
>> + - qcom,pm8916-vib
>> + - qcom,pm8921-vib
>> + - items:
>> + - enum:
>> + - qcom,pm7250b-vib
>> + - qcom,pm7325b-vib
>> + - qcom,pm7550ba-vib
>> + - const: qcom,pmi632-vib
>
> With the new revision the standalone 'compatible = "qcom,pmi632-vib";'
> doesn't pass validation anymore.
>
> foo.dtb: vibrator@5700: compatible: 'oneOf' conditional failed, one must be fixed:
> ['qcom,pmi632-vib'] is too short
> 'qcom,pmi632-vib' is not one of ['qcom,pm8058-vib', 'qcom,pm8916-vib', 'qcom,pm8921-vib']
> 'qcom,pmi632-vib' is not one of ['qcom,pm7250b-vib', 'qcom,pm7325b-vib', 'qcom,pm7550ba-vib']
> from schema $id: http://devicetree.org/schemas/input/qcom,pm8xxx-vib.yaml#
>
> I believe you need to add the compatible also like this:
>
> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> index 72b72c67a9b6..2025d6a5423e 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> @@ -16,6 +16,7 @@ properties:
> - qcom,pm8058-vib
> - qcom,pm8916-vib
> - qcom,pm8921-vib
> + - qcom,pmi632-vib
> - items:
> - enum:
> - qcom,pm7250b-vib
>
Yeah, thanks for catching this. I will update it soon.
>
> Regards
> Luca
>
>>
>> reg:
>> maxItems: 1
>
^ permalink raw reply
* Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Rahul Rameshbabu @ 2023-08-15 6:35 UTC (permalink / raw)
To: Riwen Lu
Cc: Jiri Kosina, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
rrangel, u.kleine-koenig, linux-input, linux-kernel, Riwen Lu
In-Reply-To: <TYCP286MB260715E63D023C52591264C5B114A@TYCP286MB2607.JPNP286.PROD.OUTLOOK.COM>
On Tue, 15 Aug, 2023 14:02:40 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
> 在 2023/8/14 17:26, Jiri Kosina 写道:
>> On Mon, 3 Jul 2023, Riwen Lu wrote:
>>
>>> From: Riwen Lu <luriwen@kylinos.cn>
>>>
>>> The format '%*ph' print up to 64 bytes long as a hex string with ' '
>>> sepatator. Usually the size of report descriptor is larger than 64
>>> bytes, so consider using print_hex_dump_debug to print out all of it for
>>> better debugging.
>>>
>>> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
>>> ---
>>> drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>>> index efbba0465eef..8e97fc01c852 100644
>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>>> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
>>> }
>>> }
>>>
>>> - i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
>>> + i2c_hid_dbg(ihid, "Report Descriptor\n");
>>> + print_hex_dump_debug(" ", DUMP_PREFIX_OFFSET, 16, 1,
>>> + rdesc, rsize, false);
Maybe it makes sense to use a prefix for the hex dump that is easy to
trace rather than padding whitespace? This looks good when you do not
see any other kernel message log lines get interlaced when written.
However, if you have a lot of concurrent kernel message output, I think
it can be tough to piece together the lines of the dump with this
prefix. Just my opinion.
>>
>> But that would dump it unconditionally, while i2c_hid_dbg() is
>> conditional.
>>
> Function print_hex_dump_debug() dump messages is as conditional as
> i2c_hid_dbg().
>
> The function i2c_hid_dbg() defines as follows:
> #define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__)
>
> dev_dbg() depends on the same macro as print_hex_dump_debug().
I agree with this point. Both dev_dbg and print_hex_dump_debug are noops
if neither CONFIG_DYNAMIC_DEBUG is set or DEBUG is defined.
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH v5 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Luca Weiss @ 2023-08-15 6:35 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt,
robh+dt, agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input, devicetree
Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <20230815060314.352103-3-quic_fenglinw@quicinc.com>
Hi Fenglin,
On Tue Aug 15, 2023 at 8:03 AM CEST, Fenglin Wu wrote:
> Add compatible strings to support vibrator module inside PMI632,
> PMI7250B, PM7325B, PM7550BA.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> .../bindings/input/qcom,pm8xxx-vib.yaml | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> index c8832cd0d7da..72b72c67a9b6 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> @@ -11,10 +11,17 @@ maintainers:
>
> properties:
> compatible:
> - enum:
> - - qcom,pm8058-vib
> - - qcom,pm8916-vib
> - - qcom,pm8921-vib
> + oneOf:
> + - enum:
> + - qcom,pm8058-vib
> + - qcom,pm8916-vib
> + - qcom,pm8921-vib
> + - items:
> + - enum:
> + - qcom,pm7250b-vib
> + - qcom,pm7325b-vib
> + - qcom,pm7550ba-vib
> + - const: qcom,pmi632-vib
With the new revision the standalone 'compatible = "qcom,pmi632-vib";'
doesn't pass validation anymore.
foo.dtb: vibrator@5700: compatible: 'oneOf' conditional failed, one must be fixed:
['qcom,pmi632-vib'] is too short
'qcom,pmi632-vib' is not one of ['qcom,pm8058-vib', 'qcom,pm8916-vib', 'qcom,pm8921-vib']
'qcom,pmi632-vib' is not one of ['qcom,pm7250b-vib', 'qcom,pm7325b-vib', 'qcom,pm7550ba-vib']
from schema $id: http://devicetree.org/schemas/input/qcom,pm8xxx-vib.yaml#
I believe you need to add the compatible also like this:
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index 72b72c67a9b6..2025d6a5423e 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -16,6 +16,7 @@ properties:
- qcom,pm8058-vib
- qcom,pm8916-vib
- qcom,pm8921-vib
+ - qcom,pmi632-vib
- items:
- enum:
- qcom,pm7250b-vib
Regards
Luca
>
> reg:
> maxItems: 1
^ permalink raw reply related
* [PATCH v5 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-08-15 6:03 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815060314.352103-1-quic_fenglinw@quicinc.com>
Currently, all vibrator control register addresses are hard coded,
including the base address and the offset, it's not flexible to support
new SPMI vibrator module which is usually included in different PMICs
with different base address. Refactor this by defining register offset
with HW type combination, and register base address which is defined
in 'reg' property is added for SPMI vibrators.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 122 ++++++++++++++++-----------
1 file changed, 73 insertions(+), 49 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..d6b468324c77 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,44 @@
#include <linux/regmap.h>
#include <linux/slab.h>
+#define SSBL_VIB_DRV_REG 0x4A
+#define SSBI_VIB_DRV_EN_MANUAL_MASK GENMASK(7, 2)
+#define SSBI_VIB_DRV_LEVEL_MASK GENMASK(7, 3)
+#define SSBI_VIB_DRV_SHIFT 3
+
+#define SPMI_VIB_DRV_REG 0x41
+#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
+#define SPMI_VIB_DRV_SHIFT 0
+
+#define SPMI_VIB_EN_REG 0x46
+#define SPMI_VIB_EN_BIT BIT(7)
+
#define VIB_MAX_LEVEL_mV (3100)
#define VIB_MIN_LEVEL_mV (1200)
#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
#define MAX_FF_SPEED 0xff
-struct pm8xxx_regs {
- unsigned int enable_addr;
- unsigned int enable_mask;
+enum vib_hw_type {
+ SSBI_VIB,
+ SPMI_VIB,
+};
- unsigned int drv_addr;
- unsigned int drv_mask;
- unsigned int drv_shift;
- unsigned int drv_en_manual_mask;
+struct pm8xxx_vib_data {
+ enum vib_hw_type hw_type;
+ unsigned int enable_addr;
+ unsigned int drv_addr;
};
-static const struct pm8xxx_regs pm8058_regs = {
- .drv_addr = 0x4A,
- .drv_mask = 0xf8,
- .drv_shift = 3,
- .drv_en_manual_mask = 0xfc,
+static const struct pm8xxx_vib_data ssbi_vib_data = {
+ .hw_type = SSBI_VIB,
+ .drv_addr = SSBL_VIB_DRV_REG,
};
-static struct pm8xxx_regs pm8916_regs = {
- .enable_addr = 0xc046,
- .enable_mask = BIT(7),
- .drv_addr = 0xc041,
- .drv_mask = 0x1F,
- .drv_shift = 0,
- .drv_en_manual_mask = 0,
+static const struct pm8xxx_vib_data spmi_vib_data = {
+ .hw_type = SPMI_VIB,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_DRV_REG,
};
/**
@@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
* @vib_input_dev: input device supporting force feedback
* @work: work structure to set the vibration parameters
* @regmap: regmap for register read/write
- * @regs: registers' info
+ * @data: vibrator HW info
+ * @reg_base: the register base of the module
* @speed: speed of vibration set from userland
* @active: state of vibrator
* @level: level of vibration to set in the chip
@@ -59,7 +68,8 @@ struct pm8xxx_vib {
struct input_dev *vib_input_dev;
struct work_struct work;
struct regmap *regmap;
- const struct pm8xxx_regs *regs;
+ const struct pm8xxx_vib_data *data;
+ unsigned int reg_base;
int speed;
int level;
bool active;
@@ -75,24 +85,31 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
{
int rc;
unsigned int val = vib->reg_vib_drv;
- const struct pm8xxx_regs *regs = vib->regs;
+ u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
+ u32 shift = SPMI_VIB_DRV_SHIFT;
+
+ if (vib->data->hw_type == SSBI_VIB) {
+ mask = SSBI_VIB_DRV_LEVEL_MASK;
+ shift = SSBI_VIB_DRV_SHIFT;
+ }
if (on)
- val |= (vib->level << regs->drv_shift) & regs->drv_mask;
+ val |= (vib->level << shift) & mask;
else
- val &= ~regs->drv_mask;
+ val &= ~mask;
- rc = regmap_write(vib->regmap, regs->drv_addr, val);
+ rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, val);
if (rc < 0)
return rc;
vib->reg_vib_drv = val;
- if (regs->enable_mask)
- rc = regmap_update_bits(vib->regmap, regs->enable_addr,
- regs->enable_mask, on ? ~0 : 0);
+ if (vib->data->hw_type == SSBI_VIB)
+ return 0;
- return rc;
+ mask = SPMI_VIB_EN_BIT;
+ val = on ? SPMI_VIB_EN_BIT : 0;
+ return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
}
/**
@@ -102,13 +119,6 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
static void pm8xxx_work_handler(struct work_struct *work)
{
struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
- const struct pm8xxx_regs *regs = vib->regs;
- int rc;
- unsigned int val;
-
- rc = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (rc < 0)
- return;
/*
* pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
@@ -168,9 +178,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
{
struct pm8xxx_vib *vib;
struct input_dev *input_dev;
+ const struct pm8xxx_vib_data *data;
int error;
- unsigned int val;
- const struct pm8xxx_regs *regs;
+ unsigned int val, reg_base;
vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
if (!vib)
@@ -187,19 +197,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
INIT_WORK(&vib->work, pm8xxx_work_handler);
vib->vib_input_dev = input_dev;
- regs = of_device_get_match_data(&pdev->dev);
+ data = of_device_get_match_data(&pdev->dev);
+ if (!data)
+ return -EINVAL;
- /* operate in manual mode */
- error = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (error < 0)
- return error;
+ if (data->hw_type != SSBI_VIB) {
+ error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", ®_base);
+ if (error < 0) {
+ dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
+ return error;
+ }
+
+ vib->reg_base += reg_base;
+ }
- val &= regs->drv_en_manual_mask;
- error = regmap_write(vib->regmap, regs->drv_addr, val);
+ error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
if (error < 0)
return error;
- vib->regs = regs;
+ /* operate in manual mode */
+ if (data->hw_type == SSBI_VIB) {
+ val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+ error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
+ if (error < 0)
+ return error;
+ }
+
+ vib->data = data;
vib->reg_vib_drv = val;
input_dev->name = "pm8xxx_vib_ffmemless";
@@ -239,9 +263,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
static const struct of_device_id pm8xxx_vib_id_table[] = {
- { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs },
+ { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox