* Re: [PATCH -next v2 08/15] hwmon: (aquacomputer_d5next) Use devm_hid_hw_start_and_open in aqc_probe()
From: Guenter Roeck @ 2024-09-09 16:57 UTC (permalink / raw)
To: Li Zetao
Cc: jikos, bentiss, michael.zaidman, gupt21, djogorchock,
roderick.colenbrander, savicaleksa83, me, jdelvare, mail,
wilken.gottwalt, jonas, mezin.alexander, linux-input, linux-i2c,
linux-hwmon
In-Reply-To: <20240909012313.500341-9-lizetao1@huawei.com>
On Mon, Sep 09, 2024 at 09:23:06AM +0800, Li Zetao wrote:
> Currently, the aquacomputer_d5next module needs to maintain hid resources
> by itself. Use devm_hid_hw_start_and_open helper to ensure that hid
> resources are consistent with the device life cycle, and release
> hid resources before device is released. At the same time, it can avoid
> the goto-release encoding, drop the fail_and_close and fail_and_stop
> lables, and directly return the error code when an error occurs.
>
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v1 -> v2: Adjust commit information
> v1:
> https://lore.kernel.org/all/20240904123607.3407364-13-lizetao1@huawei.com/
>
> drivers/hwmon/aquacomputer_d5next.c | 39 +++++++----------------------
> 1 file changed, 9 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 8e55cd2f46f5..9b66ff0fe6e1 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
> @@ -1556,14 +1556,10 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (ret)
> return ret;
>
> - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
> if (ret)
> return ret;
>
> - ret = hid_hw_open(hdev);
> - if (ret)
> - goto fail_and_stop;
> -
> switch (hdev->product) {
> case USB_PRODUCT_ID_AQUAERO:
> /*
> @@ -1577,10 +1573,8 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> * they present. The two other devices have the type of the second element in
> * their respective collections set to 1, while the real device has it set to 0.
> */
> - if (hdev->collection[1].type != 0) {
> - ret = -ENODEV;
> - goto fail_and_close;
> - }
> + if (hdev->collection[1].type != 0)
> + return -ENODEV;
>
> priv->kind = aquaero;
>
> @@ -1740,10 +1734,8 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> * Choose the right Leakshield device, because
> * the other one acts as a keyboard
> */
> - if (hdev->type != 2) {
> - ret = -ENODEV;
> - goto fail_and_close;
> - }
> + if (hdev->type != 2)
> + return -ENODEV;
>
> priv->kind = leakshield;
>
> @@ -1865,30 +1857,20 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> priv->name = aqc_device_names[priv->kind];
>
> priv->buffer = devm_kzalloc(&hdev->dev, priv->buffer_size, GFP_KERNEL);
> - if (!priv->buffer) {
> - ret = -ENOMEM;
> - goto fail_and_close;
> - }
> + if (!priv->buffer)
> + return -ENOMEM;
>
> mutex_init(&priv->mutex);
>
> priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, priv->name, priv,
> &aqc_chip_info, NULL);
>
> - if (IS_ERR(priv->hwmon_dev)) {
> - ret = PTR_ERR(priv->hwmon_dev);
> - goto fail_and_close;
> - }
> + if (IS_ERR(priv->hwmon_dev))
> + return PTR_ERR(priv->hwmon_dev);
>
> aqc_debugfs_init(priv);
>
> return 0;
> -
> -fail_and_close:
> - hid_hw_close(hdev);
> -fail_and_stop:
> - hid_hw_stop(hdev);
> - return ret;
> }
>
> static void aqc_remove(struct hid_device *hdev)
> @@ -1897,9 +1879,6 @@ static void aqc_remove(struct hid_device *hdev)
>
> debugfs_remove_recursive(priv->debugfs);
> hwmon_device_unregister(priv->hwmon_dev);
> -
> - hid_hw_close(hdev);
> - hid_hw_stop(hdev);
> }
>
> static const struct hid_device_id aqc_table[] = {
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: update reference to m8921-keypad.yaml
From: Rob Herring (Arm) @ 2024-09-09 16:03 UTC (permalink / raw)
To: Simon Horman
Cc: Conor Dooley, devicetree, Krzysztof Kozlowski, linux-input,
Dmitry Torokhov, Dmitry Baryshkov, linux-arm-msm
In-Reply-To: <20240908-keypad-wakeup-ref-v1-1-762e4641468a@kernel.org>
On Sun, 08 Sep 2024 21:25:16 +0100, Simon Horman wrote:
> commit 53ed3233e6b5 ("dt-bindings: input: qcom,pm8921-keypad: convert to
> YAML format") resulted in a renaming of the output .txt file from
> qcom,pm8xxx-keypad.txt to qcom,pm8921-keypad.yaml.
>
> This patch makes a corresponding update to the link to that .txt file
> in wakeup-source.txt.
>
> Flagged by make htmldocs:
> Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
>
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
> Documentation/devicetree/bindings/power/wakeup-source.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] dt-binding: touchscreen: fix x-plat-ohm missing type definition
From: Sayyad Abid @ 2024-09-09 12:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Marek Vasut, Michael Welling, linux-input,
linux-kernel
In-Reply-To: <CACVUEB=eQUFbDV80D7sPkK84FmnR7j66gRGvB8eusV_cz-QmAQ@mail.gmail.com>
On Mon, Sep 9, 2024 at 1:36 PM Sayyad Abid <sayyad.abid16@gmail.com> wrote:
>
> On Mon, Sep 9, 2024 at 1:21 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 09/09/2024 09:48, Sayyad Abid wrote:
> > > On Mon, Sep 9, 2024 at 12:02 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >>
> > >> On Sun, Sep 08, 2024 at 08:47:43PM +0530, Sayyad Abid wrote:
> > >>> This patch fixes the issue with x-plat-ohm missing a type definition.
> > >>> The patch adds the fix for this issue by adding value of this property
> > >>
> > >> You repeated twice the same while it is unclear why this is missing.
> > > I must have overlooked it, my bad.
> >
> > Keep discussion public.
> I am away form my desktop and trying to reply with Gmail's client,
> I use mutt otherwise hence the trouble. I hit 'reply' instead of 'reply all'.
> >
> > >>
> > >> Also:
> > >> Please do not use "This commit/patch/change", but imperative mood. See
> > >> longer explanation here:
> > >> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> > > Okay, I'll refer to this article for any further patches. Thank you!
> > >>
> > >>
> > >>> should be a 32-bit unsigned integer.
> > >>>
> > >>> Signed-off-by: Sayyad Abid <sayyad.abid16@gmail.com>
> > >>>
> > >>> ---
> > >>> .../devicetree/bindings/input/touchscreen/ti,tsc2005.yaml | 2 ++
> > >>> 1 file changed, 2 insertions(+)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> > >>> index 7187c390b2f5..98ff65cf9f9f 100644
> > >>> --- a/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> > >>> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> > >>> @@ -38,6 +38,8 @@ properties:
> > >>>
> > >>> ti,x-plate-ohms:
> > >>> description: resistance of the touchscreen's X plates in ohm (defaults to 280)
> > >>
> > >>> + $ref: /schemas/types.yaml#/definitions/uint32
> > >>
> > >> $ref should not be needed, so what is exactly missing? Provide some
> > >> sort of proof that you are fixing real issue.
> > > I ran dt_bindings_check on the file which resulted in a warning
> > > "x-plate-ohm : missing type definition", to resolve this warning I
> > > looked at the other yaml files in the ti,txc2005.yaml directory
> > > (/Documentation/devicetree/bindings/input/touchscreen/), Where I found
> > > out that one of the TI's touchscreen (ti,am3359) binding used $ref
> > > property for the similar "x-plate-resistance" property.
> > >
> > > Adding the $ref resolved the warnings.
> >
> > You run some older dtschema.
> I'll update this and check again.
So I did a full clean clone of the kernel repo and followed the
dtschema installation
as mentioned in the Linux Kernel Documentation.
Running the dt_binding_check resulted in the same warning.
> >
> > Anyway, each commit must explain why you are doing this. Whatever
> > warning you fix, you should mention it in the commit msg, because that's
> > the answer to "why?" part.
> >
> Yes, will keep this in mind for any further patches.
> > Best regards,
> > Krzysztof
> >
> Thank you for your time and input.
>
> --
> Abid
What else should I try?
Thank You!
--
Abid
^ permalink raw reply
* Re: [PATCH] Input: keypad-nomadik-ske - remove the driver
From: Nuno Sá @ 2024-09-09 10:34 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Arnd Bergmann, Michael Hennerich, Linus Walleij, linux-input,
linux-kernel, Lee Jones, linux-arm-kernel, Utsav Agarwal
In-Reply-To: <ZtszE9A-576SmvsX@google.com>
On Fri, 2024-09-06 at 09:51 -0700, Dmitry Torokhov wrote:
> Hi Nuno,
>
> On Fri, Sep 06, 2024 at 10:38:35AM +0200, Nuno Sá wrote:
> >
> > Hi Dmitry,
> >
> > This is not forgotten and I plan to start working on this early next week.
> >
> > One thing I noticed and I might as well just ask before starting the work,
> > is that
> > the platform data allows, in theory, for you to have holes in your keymap
> > [1]. Like
> > enabling row 1 and 3 skipping 2. AFAICT, the matrix stuff does not allow
> > this out of
> > the box as we just define the number of rows/cols and then without any other
> > property
> > we assume (I think) that the map is contiguous.
> >
> > This is just early thinking but one way to support the current behavior
> > would be 2
> > custom DT properties that would be 2 u32 arrays specifying the enabled
> > columns and
> > rows. Out of it, we could build row and col masks and get the total number
> > of cols
> > and rows that we could pass to matrix_keypad_build_keymap().
>
> I'd ask DT maintainers but in my opinion we could add 2 u32 scalar
> properties to specify row and col masks (either enabled or disabled,
> whatever is more convenient) and then indeed we could figure out the
> resulting size of key matrix and use matrix_keypad_build_keymap() to
> load it.
>
Alright, I'll see how it looks like and DT maintainers can then comment on it.
I'm afraid they can complain about the masks (for not being super user friendly)
but I think key arrays should be acceptable. I'll try the masks anyways...
- Nuno Sá
>
^ permalink raw reply
* Re: [PATCH v3 1/3] input: touchscreem: ad7877: add match table
From: Nuno Sá @ 2024-09-09 10:30 UTC (permalink / raw)
To: Antoniu Miclaus, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Hennerich, Mark Brown,
linux-input, devicetree, linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-1-antoniu.miclaus@analog.com>
On Mon, 2024-09-09 at 12:30 +0300, Antoniu Miclaus wrote:
> Add match table for the ad7877 driver and define the compatible string.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> no changes in v3.
> drivers/input/touchscreen/ad7877.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ad7877.c
> b/drivers/input/touchscreen/ad7877.c
> index a0598e9c7aff..7886454a19c6 100644
> --- a/drivers/input/touchscreen/ad7877.c
> +++ b/drivers/input/touchscreen/ad7877.c
> @@ -805,10 +805,17 @@ static int ad7877_resume(struct device *dev)
>
> static DEFINE_SIMPLE_DEV_PM_OPS(ad7877_pm, ad7877_suspend, ad7877_resume);
>
> +static const struct of_device_id ad7877_of_match[] = {
> + { .compatible = "adi,ad7877", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad7877_of_match);
> +
Just curious, is there any reason for this patch to be split from patch 2? Also,
this patch should directly include mod_devicetable.h for 'struct of_device_id'
(instead of relying in other headers).
- Nuno Sá
^ permalink raw reply
* Re: [PATCH v3 3/3] dt-bindings: touchscreen: ad7877: add bindings
From: Krzysztof Kozlowski @ 2024-09-09 10:10 UTC (permalink / raw)
To: Antoniu Miclaus
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, linux-input, devicetree,
linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-3-antoniu.miclaus@analog.com>
On Mon, Sep 09, 2024 at 12:30:13PM +0300, Antoniu Miclaus wrote:
> Add device tree bindings for the ad7877 driver.
Bindings are for hardware, not driver.
Subject: "add bindings" is quite redundant. Rather say what device you
are adding.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 3/3] dt-bindings: touchscreen: ad7877: add bindings
From: Krzysztof Kozlowski @ 2024-09-09 10:09 UTC (permalink / raw)
To: Antoniu Miclaus
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, linux-input, devicetree,
linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-3-antoniu.miclaus@analog.com>
On Mon, Sep 09, 2024 at 12:30:13PM +0300, Antoniu Miclaus wrote:
> Add device tree bindings for the ad7877 driver.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v3:
> - use strings for polarity.
> - use unit siffix where applies.
> - add defaults where applies.
> - add complete example.
> .../input/touchscreen/adi,ad7877.yaml | 109 ++++++++++++++++++
> 1 file changed, 109 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
> new file mode 100644
> index 000000000000..7603ce63af7e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/adi,ad7877.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7877 Touch Screen Controller
> +
> +maintainers:
> + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +description: |
> + Analog Devices Touch Screen Controller
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7877.pdf
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad7877
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + spi-max-frequency:
> + description: AD7877 SPI bus clock frequency.
> + minimum: 10000
> + maximum: 20000000
> +
> + adi,stopacq-polarity:
> + description: The polarity of the signal applied to the STOPACQ pin.
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [low, high]
> +
> + adi,first-conv-delay-ns:
> + description: Delay in ns before the first conversion.
> + enum: [500, 128000, 1000000, 8000000]
> +
> + adi,pen-down-acc-interval:
> + description: Enable the ADC to repeatedly perform conversions.
> + 0 = convert once
> + 1 = every 0.5 ms
> + 2 = every 1 ms
> + 3 = every 8 ms
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 0
You got the comment about using units, right? It applied *everywhere*.
This is clearly a property in ms.
> +
> + adi,acquisition-time-us:
> + description: Select acquisition times in us for the ADC.
> + enum: [2, 4, 8, 16]
What is the default? The property is not reuired, so how does it work
without it?
> +
> + adi,vref-delay-us:
> + description: Delay required for the SPI transfers depending on the VREF used.
Same question.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 2/3] input: touchscreen: ad7877: add dt support
From: Krzysztof Kozlowski @ 2024-09-09 10:07 UTC (permalink / raw)
To: Antoniu Miclaus
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, linux-input, devicetree,
linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-2-antoniu.miclaus@analog.com>
On Mon, Sep 09, 2024 at 12:30:12PM +0300, Antoniu Miclaus wrote:
> Add devicetree support within the driver.
>
> Remove old platform data approach since it is no longer used.
What is the point of the previous patch? How can the device work if you
apply only patch 1?
This is somehow odd split...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/3] input: touchscreem: ad7877: add match table
From: Krzysztof Kozlowski @ 2024-09-09 10:05 UTC (permalink / raw)
To: Antoniu Miclaus
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, linux-input, devicetree,
linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-1-antoniu.miclaus@analog.com>
On Mon, Sep 09, 2024 at 12:30:11PM +0300, Antoniu Miclaus wrote:
> Add match table for the ad7877 driver and define the compatible string.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> no changes in v3.
> drivers/input/touchscreen/ad7877.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
> index a0598e9c7aff..7886454a19c6 100644
> --- a/drivers/input/touchscreen/ad7877.c
> +++ b/drivers/input/touchscreen/ad7877.c
> @@ -805,10 +805,17 @@ static int ad7877_resume(struct device *dev)
>
> static DEFINE_SIMPLE_DEV_PM_OPS(ad7877_pm, ad7877_suspend, ad7877_resume);
>
> +static const struct of_device_id ad7877_of_match[] = {
> + { .compatible = "adi,ad7877", },
Bindings are before their users.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v12 22/38] dt-bindings: input: Add Cirrus EP93xx keypad
From: Rob Herring (Arm) @ 2024-09-09 9:45 UTC (permalink / raw)
To: Nikita Shubin
Cc: linux-kernel, Conor Dooley, Krzysztof Kozlowski, Dmitry Torokhov,
Alexander Sverdlin, linux-input, Krzysztof Kozlowski, devicetree
In-Reply-To: <20240909-ep93xx-v12-22-e86ab2423d4b@maquefel.me>
On Mon, 09 Sep 2024 11:10:47 +0300, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC keypad.
>
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> .../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pwm/cirrus,ep9301-pwm.example.dts:18:18: fatal error: dt-bindings/clock/cirrus,ep9301-syscon.h: No such file or directory
18 | #include <dt-bindings/clock/cirrus,ep9301-syscon.h>
make[2]: *** [scripts/Makefile.lib:442: Documentation/devicetree/bindings/pwm/cirrus,ep9301-pwm.example.dtb] Error 1
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240909-ep93xx-v12-22-e86ab2423d4b@maquefel.me
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* [PATCH v3 3/3] dt-bindings: touchscreen: ad7877: add bindings
From: Antoniu Miclaus @ 2024-09-09 9:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, Antoniu Miclaus, linux-input,
devicetree, linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-1-antoniu.miclaus@analog.com>
Add device tree bindings for the ad7877 driver.
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
- use strings for polarity.
- use unit siffix where applies.
- add defaults where applies.
- add complete example.
.../input/touchscreen/adi,ad7877.yaml | 109 ++++++++++++++++++
1 file changed, 109 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
new file mode 100644
index 000000000000..7603ce63af7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/adi,ad7877.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/adi,ad7877.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7877 Touch Screen Controller
+
+maintainers:
+ - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+ Analog Devices Touch Screen Controller
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7877.pdf
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7877
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ spi-max-frequency:
+ description: AD7877 SPI bus clock frequency.
+ minimum: 10000
+ maximum: 20000000
+
+ adi,stopacq-polarity:
+ description: The polarity of the signal applied to the STOPACQ pin.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [low, high]
+
+ adi,first-conv-delay-ns:
+ description: Delay in ns before the first conversion.
+ enum: [500, 128000, 1000000, 8000000]
+
+ adi,pen-down-acc-interval:
+ description: Enable the ADC to repeatedly perform conversions.
+ 0 = convert once
+ 1 = every 0.5 ms
+ 2 = every 1 ms
+ 3 = every 8 ms
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+ default: 0
+
+ adi,acquisition-time-us:
+ description: Select acquisition times in us for the ADC.
+ enum: [2, 4, 8, 16]
+
+ adi,vref-delay-us:
+ description: Delay required for the SPI transfers depending on the VREF used.
+
+ touchscreen-average-samples:
+ enum: [1, 4, 8, 16]
+
+ touchscreen-x-plate-ohms:
+ default: 400
+
+ touchscreen-min-x: true
+ touchscreen-min-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-max-pressure: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - touchscreen-average-samples
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@0 {
+ compatible = "adi,ad7877";
+ reg = <0>;
+ spi-max-frequency = <20000000>;
+ interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+ adi,vref-delay-us = <100>;
+ adi,stopacq-polarity = "low";
+ adi,first-conv-delay-ns = <500>;
+ adi,pen-down-acc-interval = <0>;
+ adi,acquisition-time-us = <2>;
+ touchscreen-average-samples = <16>;
+ touchscreen-x-plate-ohms = <400>;
+ touchscreen-min-x = <0>;
+ touchscreen-min-y = <0>;
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ touchscreen-max-pressure = <4095>;
+ };
+ };
+...
--
2.46.0
^ permalink raw reply related
* [PATCH v3 2/3] input: touchscreen: ad7877: add dt support
From: Antoniu Miclaus @ 2024-09-09 9:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, Antoniu Miclaus, linux-input,
devicetree, linux-kernel, linux-spi
In-Reply-To: <20240909093101.14113-1-antoniu.miclaus@analog.com>
Add devicetree support within the driver.
Remove old platform data approach since it is no longer used.
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
- remove platform data instead of making it optional, as suggested.
- use touchscreen_parse_properties and touchscreen_report_pos
drivers/input/touchscreen/ad7877.c | 150 +++++++++++++++++++++++------
include/linux/spi/ad7877.h | 25 -----
2 files changed, 119 insertions(+), 56 deletions(-)
delete mode 100644 include/linux/spi/ad7877.h
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 7886454a19c6..bb1799bcace3 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -25,11 +25,12 @@
#include <linux/device.h>
#include <linux/delay.h>
#include <linux/input.h>
+#include <linux/input/touchscreen.h>
#include <linux/interrupt.h>
#include <linux/pm.h>
+#include <linux/property.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
-#include <linux/spi/ad7877.h>
#include <linux/module.h>
#include <asm/irq.h>
@@ -174,6 +175,8 @@ struct ad7877 {
u8 averaging;
u8 pen_down_acc_interval;
+ struct touchscreen_properties prop;
+
struct spi_transfer xfer[AD7877_NR_SENSE + 2];
struct spi_message msg;
@@ -353,8 +356,7 @@ static int ad7877_process_data(struct ad7877 *ts)
if (!timer_pending(&ts->timer))
input_report_key(input_dev, BTN_TOUCH, 1);
- input_report_abs(input_dev, ABS_X, x);
- input_report_abs(input_dev, ABS_Y, y);
+ touchscreen_report_pos(input_dev, &ts->prop, x, y, false);
input_report_abs(input_dev, ABS_PRESSURE, Rt);
input_sync(input_dev);
@@ -667,11 +669,118 @@ static void ad7877_setup_ts_def_msg(struct spi_device *spi, struct ad7877 *ts)
}
}
+static int ad7877_parse_props(struct ad7877 *ts)
+{
+ struct device *dev = &ts->spi->dev;
+ u32 value, average;
+ int ret;
+
+ ts->model = (uintptr_t)device_get_match_data(dev);
+ if (!ts->model)
+ ts->model = 7877;
+
+ ret = device_property_match_string(dev, "adi,stopacq-polarity", "low");
+ if (ret < 0) {
+ ret = device_property_match_string(dev, "adi,stopacq-polarity", "high");
+ if (ret < 0)
+ ts->stopacq_polarity = 0;
+ ts->stopacq_polarity = 1;
+ } else {
+ ts->stopacq_polarity = 0;
+ }
+
+ ret = device_property_read_u32(dev, "adi,first-conv-delay-ns", &value);
+ if (!ret) {
+ switch (value) {
+ case 500:
+ ts->first_conversion_delay = 0;
+ break;
+ case 128000:
+ ts->first_conversion_delay = 1;
+ break;
+ case 1000000:
+ ts->first_conversion_delay = 2;
+ break;
+ case 8000000:
+ ts->first_conversion_delay = 3;
+ break;
+ default:
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid adi,first-conv-delay-ns value\n");
+ }
+ }
+
+ device_property_read_u32(dev, "adi,pen-down-acc-interval",
+ &value);
+ ts->pen_down_acc_interval = (u8)value;
+
+ ret = device_property_read_u32(dev, "adi,acquisition-time-us", &value);
+ if (!ret) {
+ switch (value) {
+ case 2:
+ ts->acquisition_time = 0;
+ break;
+ case 4:
+ ts->acquisition_time = 1;
+ break;
+ case 8:
+ ts->acquisition_time = 2;
+ break;
+ case 16:
+ ts->acquisition_time = 3;
+ break;
+ default:
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid adi,first-conv-delay-ns value\n");
+ }
+ }
+
+ device_property_read_u32(dev, "adi,vref-delay-us",
+ &value);
+ if (!value)
+ ts->vref_delay_usecs = 100;
+ else
+ ts->vref_delay_usecs = (u16)value;
+
+ device_property_read_u32(dev, "touchscreen-x-plate-ohms", &value);
+ if (value)
+ ts->x_plate_ohms = (u16)value;
+ else
+ ts->x_plate_ohms = 400;
+
+ /*
+ * The property is parsed also in the touchscreen_parse_properties()
+ * but is required for the ad7877_process_data() so we need to store it.
+ */
+ device_property_read_u32(dev, "touchscreen-max-pressure", &value);
+ ts->pressure_max = (u16)value;
+
+ device_property_read_u32(dev, "touchscreen-average-samples", &average);
+ switch (average) {
+ case 1:
+ ts->averaging = 0;
+ break;
+ case 4:
+ ts->averaging = 1;
+ break;
+ case 8:
+ ts->averaging = 2;
+ break;
+ case 16:
+ ts->averaging = 3;
+ break;
+ default:
+ return dev_err_probe(dev, -EINVAL,
+ "touchscreen-average-samples must be 1, 4, 8, or 16\n");
+ }
+
+ return 0;
+}
+
static int ad7877_probe(struct spi_device *spi)
{
struct ad7877 *ts;
struct input_dev *input_dev;
- struct ad7877_platform_data *pdata = dev_get_platdata(&spi->dev);
int err;
u16 verify;
@@ -680,11 +789,6 @@ static int ad7877_probe(struct spi_device *spi)
return -ENODEV;
}
- if (!pdata) {
- dev_dbg(&spi->dev, "no platform data?\n");
- return -ENODEV;
- }
-
/* don't exceed max specified SPI CLK frequency */
if (spi->max_speed_hz > MAX_SPI_FREQ_HZ) {
dev_dbg(&spi->dev, "SPI CLK %d Hz?\n",spi->max_speed_hz);
@@ -714,27 +818,22 @@ static int ad7877_probe(struct spi_device *spi)
ts->spi = spi;
ts->input = input_dev;
+ err = ad7877_parse_props(ts);
+ if (err)
+ return err;
+
timer_setup(&ts->timer, ad7877_timer, 0);
mutex_init(&ts->mutex);
spin_lock_init(&ts->lock);
- ts->model = pdata->model ? : 7877;
- ts->vref_delay_usecs = pdata->vref_delay_usecs ? : 100;
- ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
- ts->pressure_max = pdata->pressure_max ? : ~0;
-
- ts->stopacq_polarity = pdata->stopacq_polarity;
- ts->first_conversion_delay = pdata->first_conversion_delay;
- ts->acquisition_time = pdata->acquisition_time;
- ts->averaging = pdata->averaging;
- ts->pen_down_acc_interval = pdata->pen_down_acc_interval;
-
snprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(&spi->dev));
input_dev->name = "AD7877 Touchscreen";
input_dev->phys = ts->phys;
input_dev->dev.parent = &spi->dev;
+ touchscreen_parse_properties(ts->input, false, &ts->prop);
+
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(BTN_TOUCH, input_dev->keybit);
__set_bit(EV_ABS, input_dev->evbit);
@@ -742,17 +841,6 @@ static int ad7877_probe(struct spi_device *spi)
__set_bit(ABS_Y, input_dev->absbit);
__set_bit(ABS_PRESSURE, input_dev->absbit);
- input_set_abs_params(input_dev, ABS_X,
- pdata->x_min ? : 0,
- pdata->x_max ? : MAX_12BIT,
- 0, 0);
- input_set_abs_params(input_dev, ABS_Y,
- pdata->y_min ? : 0,
- pdata->y_max ? : MAX_12BIT,
- 0, 0);
- input_set_abs_params(input_dev, ABS_PRESSURE,
- pdata->pressure_min, pdata->pressure_max, 0, 0);
-
ad7877_write(spi, AD7877_REG_SEQ1, AD7877_MM_SEQUENCE);
verify = ad7877_read(spi, AD7877_REG_SEQ1);
diff --git a/include/linux/spi/ad7877.h b/include/linux/spi/ad7877.h
deleted file mode 100644
index b7be843c88e2..000000000000
--- a/include/linux/spi/ad7877.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* linux/spi/ad7877.h */
-
-/* Touchscreen characteristics vary between boards and models. The
- * platform_data for the device's "struct device" holds this information.
- *
- * It's OK if the min/max values are zero.
- */
-struct ad7877_platform_data {
- u16 model; /* 7877 */
- u16 vref_delay_usecs; /* 0 for external vref; etc */
- u16 x_plate_ohms;
- u16 y_plate_ohms;
-
- u16 x_min, x_max;
- u16 y_min, y_max;
- u16 pressure_min, pressure_max;
-
- u8 stopacq_polarity; /* 1 = Active HIGH, 0 = Active LOW */
- u8 first_conversion_delay; /* 0 = 0.5us, 1 = 128us, 2 = 1ms, 3 = 8ms */
- u8 acquisition_time; /* 0 = 2us, 1 = 4us, 2 = 8us, 3 = 16us */
- u8 averaging; /* 0 = 1, 1 = 4, 2 = 8, 3 = 16 */
- u8 pen_down_acc_interval; /* 0 = covert once, 1 = every 0.5 ms,
- 2 = ever 1 ms, 3 = every 8 ms,*/
-};
--
2.46.0
^ permalink raw reply related
* [PATCH v3 1/3] input: touchscreem: ad7877: add match table
From: Antoniu Miclaus @ 2024-09-09 9:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Michael Hennerich, Mark Brown, Antoniu Miclaus, linux-input,
devicetree, linux-kernel, linux-spi
Add match table for the ad7877 driver and define the compatible string.
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v3.
drivers/input/touchscreen/ad7877.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index a0598e9c7aff..7886454a19c6 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -805,10 +805,17 @@ static int ad7877_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(ad7877_pm, ad7877_suspend, ad7877_resume);
+static const struct of_device_id ad7877_of_match[] = {
+ { .compatible = "adi,ad7877", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad7877_of_match);
+
static struct spi_driver ad7877_driver = {
.driver = {
.name = "ad7877",
.dev_groups = ad7877_groups,
+ .of_match_table = ad7877_of_match,
.pm = pm_sleep_ptr(&ad7877_pm),
},
.probe = ad7877_probe,
--
2.46.0
^ permalink raw reply related
* Re: [PATCH v2 0/3] iio: Introduce and use aligned_s64 type
From: Andy Shevchenko @ 2024-09-09 9:20 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Uwe Kleine-König, Jonathan Cameron, Srinivas Pandruvada,
Basavaraj Natikar, linux-input, linux-iio, linux-kernel,
Jiri Kosina, Lars-Peter Clausen, Lorenzo Bianconi
In-Reply-To: <20240907163752.2eb3be6a@jic23-huawei>
On Sat, Sep 07, 2024 at 04:37:52PM +0100, Jonathan Cameron wrote:
> On Tue, 3 Sep 2024 20:59:03 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > Instead of having open coded idea of aligned member, use
> > a newly defined type like it's done in, e.g., u64 case.
> > Update a few IIO drivers to show how to use it.
> >
> > v2 (took only one year from v1, not bad!):
> :)
>
> Applied with that tweak for patch 2 that you called out.
Please, also do
s/__aligned_s64/aligned_s64/
in the commit message there.
> Will probably be next cycle though before these go upstream
> (so think of this as queuing them up very early for 6.13 :)
Sure, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 00/22] iio: use dev_get_platdata() to access platform_data
From: Andy Shevchenko @ 2024-09-09 9:17 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, David Lechner, Michael Hennerich,
Antoniu Miclaus, Jinjie Ruan, Lorenzo Bianconi,
Srinivas Pandruvada, Basavaraj Natikar, linux-input, linux-iio,
linux-kernel, Jiri Kosina, Lars-Peter Clausen
In-Reply-To: <20240907164258.70772206@jic23-huawei>
On Sat, Sep 07, 2024 at 04:42:58PM +0100, Jonathan Cameron wrote:
> On Tue, 3 Sep 2024 20:57:27 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Sep 03, 2024 at 01:16:45AM +0300, Andy Shevchenko wrote:
> > > Unify how IIO drivers access platform_data field of struct device.
> > > In simple and straightforward cases constify the local variables.
> > >
> > > (Not tested)
> >
> > Jonathan, in case you are fine with the series, feel free to squash, e.g.,
> > changes against hid-sensor drivers.
> I don't follow, but maybe that will become clear once I've looked
> at rest of the stuff I haven't read yet.
I mean all the patches that starts with "iio: *: hid-sensor-*:" can be squashed
into one with "iio: hid-sensor:"
> Anyhow, applied to the togreg branch of iio.git and pushed out as testing
> for all the normal reasons. Another series that will probably be 6.13 material.
Thank you!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v12 00/38] ep93xx device tree conversion
From: Nikita Shubin @ 2024-09-09 9:02 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Arnd Bergmann, Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Wim Van Sebroeck, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Mark Brown, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Ralf Baechle, Wu, Aaron, Lee Jones, Olof Johansson, Niklas Cassel,
linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, linux-pm,
devicetree, dmaengine, linux-watchdog, linux-pwm, linux-spi,
netdev, linux-mtd, linux-ide, linux-input, linux-sound,
Bartosz Golaszewski, Krzysztof Kozlowski, Andy Shevchenko,
Andrew Lunn
In-Reply-To: <CAHp75Veusv=f6Xf9-gL3ctoO5Njn7wiWMw-aMN45KbZ=YB=mQw@mail.gmail.com>
Hi Andy!
On Mon, 2024-09-09 at 11:49 +0300, Andy Shevchenko wrote:
> On Mon, Sep 9, 2024 at 11:12 AM Nikita Shubin via B4 Relay
> <devnull+nikita.shubin.maquefel.me@kernel.org> wrote:
> >
> > The goal is to recieve ACKs for all patches in series to merge it
> > via Arnd branch.
> >
> > It was decided from the very beginning of these series, mostly
> > because
> > it's a full conversion of platform code to DT and it seemed not
> > convenient to maintain compatibility with both platform and DT.
> >
> > Following patches require attention from Stephen Boyd or clk
> > subsystem:
>
> Does it mean you still have a few patches without tags?
> What are their respective numbers?
The clk is the last one as i think, all others can be ACKed by
Alexander or by Arnd himself.
>
> > - clk: ep93xx: add DT support for Cirrus EP93xx:
> > - tristate
> > - drop MFD_SYSCON/REGMAP
> > - add AUXILIARY_BUS/REGMAP_MMIO
> > - prefixed all static with ep9xx_
> > - s/clk_hw_register_ddiv()/ep93xx_clk_register_ddiv()/
> > - s/clk_register_div()/ep93xx_clk_register_div()/
> > - dropped devm_ep93xx_clk_hw_register_fixed_rate_parent_data
> > macro
> > -
> > s/devm_ep93xx_clk_hw_register_fixed_rate_parent_data()/devm_clk_hw_
> > register_fixed_rate_parent_data()/
>
^ permalink raw reply
* Re: [PATCH v12 00/38] ep93xx device tree conversion
From: Andy Shevchenko @ 2024-09-09 8:49 UTC (permalink / raw)
To: nikita.shubin
Cc: Arnd Bergmann, Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Wim Van Sebroeck, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Mark Brown, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Damien Le Moal, Sergey Shtylyov,
Dmitry Torokhov, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Ralf Baechle, Wu, Aaron, Lee Jones, Olof Johansson, Niklas Cassel,
linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, linux-pm,
devicetree, dmaengine, linux-watchdog, linux-pwm, linux-spi,
netdev, linux-mtd, linux-ide, linux-input, linux-sound,
20240904-devm_clk_hw_register_fixed_rate_parent_data-v1-1-7f14d6b456e5,
20240829-cs4271-yaml-v3-1-f1624cc838f6, Bartosz Golaszewski,
Krzysztof Kozlowski, Andy Shevchenko, Andrew Lunn
In-Reply-To: <20240909-ep93xx-v12-0-e86ab2423d4b@maquefel.me>
On Mon, Sep 9, 2024 at 11:12 AM Nikita Shubin via B4 Relay
<devnull+nikita.shubin.maquefel.me@kernel.org> wrote:
>
> The goal is to recieve ACKs for all patches in series to merge it via Arnd branch.
>
> It was decided from the very beginning of these series, mostly because
> it's a full conversion of platform code to DT and it seemed not
> convenient to maintain compatibility with both platform and DT.
>
> Following patches require attention from Stephen Boyd or clk subsystem:
Does it mean you still have a few patches without tags?
What are their respective numbers?
> - clk: ep93xx: add DT support for Cirrus EP93xx:
> - tristate
> - drop MFD_SYSCON/REGMAP
> - add AUXILIARY_BUS/REGMAP_MMIO
> - prefixed all static with ep9xx_
> - s/clk_hw_register_ddiv()/ep93xx_clk_register_ddiv()/
> - s/clk_register_div()/ep93xx_clk_register_div()/
> - dropped devm_ep93xx_clk_hw_register_fixed_rate_parent_data macro
> - s/devm_ep93xx_clk_hw_register_fixed_rate_parent_data()/devm_clk_hw_register_fixed_rate_parent_data()/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v12 23/38] input: keypad: ep93xx: add DT support for Cirrus EP93xx
From: Nikita Shubin via B4 Relay @ 2024-09-09 8:10 UTC (permalink / raw)
To: Hartley Sweeten, Alexander Sverdlin, Russell King,
Dmitry Torokhov, Uwe Kleine-König, Nikita Shubin,
Linus Walleij, Damien Le Moal, Stephen Boyd
Cc: Thierry Reding, Sergey Shtylyov, linux-arm-kernel, linux-kernel,
linux-input
In-Reply-To: <20240909-ep93xx-v12-0-e86ab2423d4b@maquefel.me>
From: Nikita Shubin <nikita.shubin@maquefel.me>
- drop flags, they were not used anyway
- add OF ID match table
- process "autorepeat", "debounce-delay-ms", prescale from device tree
- drop platform data usage and it's header
- keymap goes from device tree now on
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-ep93xx/core.c | 46 ---------------------
drivers/input/keyboard/ep93xx_keypad.c | 74 ++++++++++------------------------
include/linux/soc/cirrus/ep93xx.h | 4 --
3 files changed, 22 insertions(+), 102 deletions(-)
diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c
index 03bce5e9d1f1..b99c46d22c4d 100644
--- a/arch/arm/mach-ep93xx/core.c
+++ b/arch/arm/mach-ep93xx/core.c
@@ -697,52 +697,6 @@ void __init ep93xx_register_keypad(struct ep93xx_keypad_platform_data *data)
platform_device_register(&ep93xx_keypad_device);
}
-int ep93xx_keypad_acquire_gpio(struct platform_device *pdev)
-{
- int err;
- int i;
-
- for (i = 0; i < 8; i++) {
- err = gpio_request(EP93XX_GPIO_LINE_C(i), dev_name(&pdev->dev));
- if (err)
- goto fail_gpio_c;
- err = gpio_request(EP93XX_GPIO_LINE_D(i), dev_name(&pdev->dev));
- if (err)
- goto fail_gpio_d;
- }
-
- /* Enable the keypad controller; GPIO ports C and D used for keypad */
- ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_KEYS |
- EP93XX_SYSCON_DEVCFG_GONK);
-
- return 0;
-
-fail_gpio_d:
- gpio_free(EP93XX_GPIO_LINE_C(i));
-fail_gpio_c:
- for (--i; i >= 0; --i) {
- gpio_free(EP93XX_GPIO_LINE_C(i));
- gpio_free(EP93XX_GPIO_LINE_D(i));
- }
- return err;
-}
-EXPORT_SYMBOL(ep93xx_keypad_acquire_gpio);
-
-void ep93xx_keypad_release_gpio(struct platform_device *pdev)
-{
- int i;
-
- for (i = 0; i < 8; i++) {
- gpio_free(EP93XX_GPIO_LINE_C(i));
- gpio_free(EP93XX_GPIO_LINE_D(i));
- }
-
- /* Disable the keypad controller; GPIO ports C and D used for GPIO */
- ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
- EP93XX_SYSCON_DEVCFG_GONK);
-}
-EXPORT_SYMBOL(ep93xx_keypad_release_gpio);
-
/*************************************************************************
* EP93xx I2S audio peripheral handling
*************************************************************************/
diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
index 6b811d6bf625..dcbc50304a5a 100644
--- a/drivers/input/keyboard/ep93xx_keypad.c
+++ b/drivers/input/keyboard/ep93xx_keypad.c
@@ -6,20 +6,13 @@
*
* Based on the pxa27x matrix keypad controller by Rodolfo Giometti.
*
- * NOTE:
- *
- * The 3-key reset is triggered by pressing the 3 keys in
- * Row 0, Columns 2, 4, and 7 at the same time. This action can
- * be disabled by setting the EP93XX_KEYPAD_DISABLE_3_KEY flag.
- *
- * Normal operation for the matrix does not autorepeat the key press.
- * This action can be enabled by setting the EP93XX_KEYPAD_AUTOREPEAT
- * flag.
*/
#include <linux/bits.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/interrupt.h>
#include <linux/clk.h>
#include <linux/io.h>
@@ -27,7 +20,6 @@
#include <linux/input/matrix_keypad.h>
#include <linux/slab.h>
#include <linux/soc/cirrus/ep93xx.h>
-#include <linux/platform_data/keypad-ep93xx.h>
#include <linux/pm_wakeirq.h>
/*
@@ -61,12 +53,16 @@
#define KEY_REG_KEY1_MASK GENMASK(5, 0)
#define KEY_REG_KEY1_SHIFT 0
+#define EP93XX_MATRIX_ROWS (8)
+#define EP93XX_MATRIX_COLS (8)
+
#define EP93XX_MATRIX_SIZE (EP93XX_MATRIX_ROWS * EP93XX_MATRIX_COLS)
struct ep93xx_keypad {
- struct ep93xx_keypad_platform_data *pdata;
struct input_dev *input_dev;
struct clk *clk;
+ unsigned int debounce;
+ u16 prescale;
void __iomem *mmio_base;
@@ -133,23 +129,11 @@ static irqreturn_t ep93xx_keypad_irq_handler(int irq, void *dev_id)
static void ep93xx_keypad_config(struct ep93xx_keypad *keypad)
{
- struct ep93xx_keypad_platform_data *pdata = keypad->pdata;
unsigned int val = 0;
- clk_set_rate(keypad->clk, pdata->clk_rate);
+ val |= (keypad->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK;
- if (pdata->flags & EP93XX_KEYPAD_DISABLE_3_KEY)
- val |= KEY_INIT_DIS3KY;
- if (pdata->flags & EP93XX_KEYPAD_DIAG_MODE)
- val |= KEY_INIT_DIAG;
- if (pdata->flags & EP93XX_KEYPAD_BACK_DRIVE)
- val |= KEY_INIT_BACK;
- if (pdata->flags & EP93XX_KEYPAD_TEST_MODE)
- val |= KEY_INIT_T2;
-
- val |= ((pdata->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK);
-
- val |= ((pdata->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK);
+ val |= (keypad->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK;
__raw_writel(val, keypad->mmio_base + KEY_INIT);
}
@@ -220,17 +204,10 @@ static int ep93xx_keypad_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(ep93xx_keypad_pm_ops,
ep93xx_keypad_suspend, ep93xx_keypad_resume);
-static void ep93xx_keypad_release_gpio_action(void *_pdev)
-{
- struct platform_device *pdev = _pdev;
-
- ep93xx_keypad_release_gpio(pdev);
-}
-
static int ep93xx_keypad_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct ep93xx_keypad *keypad;
- const struct matrix_keymap_data *keymap_data;
struct input_dev *input_dev;
int err;
@@ -238,14 +215,6 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
if (!keypad)
return -ENOMEM;
- keypad->pdata = dev_get_platdata(&pdev->dev);
- if (!keypad->pdata)
- return -EINVAL;
-
- keymap_data = keypad->pdata->keymap_data;
- if (!keymap_data)
- return -EINVAL;
-
keypad->irq = platform_get_irq(pdev, 0);
if (keypad->irq < 0)
return keypad->irq;
@@ -254,19 +223,13 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
if (IS_ERR(keypad->mmio_base))
return PTR_ERR(keypad->mmio_base);
- err = ep93xx_keypad_acquire_gpio(pdev);
- if (err)
- return err;
-
- err = devm_add_action_or_reset(&pdev->dev,
- ep93xx_keypad_release_gpio_action, pdev);
- if (err)
- return err;
-
keypad->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(keypad->clk))
return PTR_ERR(keypad->clk);
+ device_property_read_u32(dev, "debounce-delay-ms", &keypad->debounce);
+ device_property_read_u16(dev, "cirrus,prescale", &keypad->prescale);
+
input_dev = devm_input_allocate_device(&pdev->dev);
if (!input_dev)
return -ENOMEM;
@@ -278,13 +241,13 @@ static int ep93xx_keypad_probe(struct platform_device *pdev)
input_dev->open = ep93xx_keypad_open;
input_dev->close = ep93xx_keypad_close;
- err = matrix_keypad_build_keymap(keymap_data, NULL,
+ err = matrix_keypad_build_keymap(NULL, NULL,
EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS,
keypad->keycodes, input_dev);
if (err)
return err;
- if (keypad->pdata->flags & EP93XX_KEYPAD_AUTOREPEAT)
+ if (device_property_read_bool(&pdev->dev, "autorepeat"))
__set_bit(EV_REP, input_dev->evbit);
input_set_drvdata(input_dev, keypad);
@@ -313,10 +276,17 @@ static void ep93xx_keypad_remove(struct platform_device *pdev)
dev_pm_clear_wake_irq(&pdev->dev);
}
+static const struct of_device_id ep93xx_keypad_of_ids[] = {
+ { .compatible = "cirrus,ep9307-keypad" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ep93xx_keypad_of_ids);
+
static struct platform_driver ep93xx_keypad_driver = {
.driver = {
.name = "ep93xx-keypad",
.pm = pm_sleep_ptr(&ep93xx_keypad_pm_ops),
+ .of_match_table = ep93xx_keypad_of_ids,
},
.probe = ep93xx_keypad_probe,
.remove_new = ep93xx_keypad_remove,
diff --git a/include/linux/soc/cirrus/ep93xx.h b/include/linux/soc/cirrus/ep93xx.h
index a27447971302..8942bfaf1545 100644
--- a/include/linux/soc/cirrus/ep93xx.h
+++ b/include/linux/soc/cirrus/ep93xx.h
@@ -41,8 +41,6 @@ int ep93xx_pwm_acquire_gpio(struct platform_device *pdev);
void ep93xx_pwm_release_gpio(struct platform_device *pdev);
int ep93xx_ide_acquire_gpio(struct platform_device *pdev);
void ep93xx_ide_release_gpio(struct platform_device *pdev);
-int ep93xx_keypad_acquire_gpio(struct platform_device *pdev);
-void ep93xx_keypad_release_gpio(struct platform_device *pdev);
int ep93xx_i2s_acquire(void);
void ep93xx_i2s_release(void);
unsigned int ep93xx_chip_revision(void);
@@ -52,8 +50,6 @@ static inline int ep93xx_pwm_acquire_gpio(struct platform_device *pdev) { return
static inline void ep93xx_pwm_release_gpio(struct platform_device *pdev) {}
static inline int ep93xx_ide_acquire_gpio(struct platform_device *pdev) { return 0; }
static inline void ep93xx_ide_release_gpio(struct platform_device *pdev) {}
-static inline int ep93xx_keypad_acquire_gpio(struct platform_device *pdev) { return 0; }
-static inline void ep93xx_keypad_release_gpio(struct platform_device *pdev) {}
static inline int ep93xx_i2s_acquire(void) { return 0; }
static inline void ep93xx_i2s_release(void) {}
static inline unsigned int ep93xx_chip_revision(void) { return 0; }
--
2.43.2
^ permalink raw reply related
* [PATCH v12 22/38] dt-bindings: input: Add Cirrus EP93xx keypad
From: Nikita Shubin via B4 Relay @ 2024-09-09 8:10 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexander Sverdlin
Cc: linux-input, devicetree, linux-kernel, Krzysztof Kozlowski
In-Reply-To: <20240909-ep93xx-v12-0-e86ab2423d4b@maquefel.me>
From: Nikita Shubin <nikita.shubin@maquefel.me>
Add YAML bindings for ep93xx SoC keypad.
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
new file mode 100644
index 000000000000..a0d2460c55ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,ep9307-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus ep93xx keypad
+
+maintainers:
+ - Alexander Sverdlin <alexander.sverdlin@gmail.com>
+
+allOf:
+ - $ref: /schemas/input/matrix-keymap.yaml#
+
+description:
+ The KPP is designed to interface with a keypad matrix with 2-point contact
+ or 3-point contact keys. The KPP is designed to simplify the software task
+ of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
+ and decoding one or multiple keys pressed simultaneously on a keypad.
+
+properties:
+ compatible:
+ oneOf:
+ - const: cirrus,ep9307-keypad
+ - items:
+ - enum:
+ - cirrus,ep9312-keypad
+ - cirrus,ep9315-keypad
+ - const: cirrus,ep9307-keypad
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ debounce-delay-ms:
+ description: |
+ Time in microseconds that key must be pressed or
+ released for state change interrupt to trigger.
+
+ cirrus,prescale:
+ description: row/column counter pre-scaler load value
+ $ref: /schemas/types.yaml#/definitions/uint16
+ maximum: 1023
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/input/input.h>
+ #include <dt-bindings/clock/cirrus,ep9301-syscon.h>
+ keypad@800f0000 {
+ compatible = "cirrus,ep9307-keypad";
+ reg = <0x800f0000 0x0c>;
+ interrupt-parent = <&vic0>;
+ interrupts = <29>;
+ clocks = <&eclk EP93XX_CLK_KEYPAD>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&keypad_default_pins>;
+ linux,keymap = <KEY_UP>,
+ <KEY_DOWN>,
+ <KEY_VOLUMEDOWN>,
+ <KEY_HOME>,
+ <KEY_RIGHT>,
+ <KEY_LEFT>,
+ <KEY_ENTER>,
+ <KEY_VOLUMEUP>,
+ <KEY_F6>,
+ <KEY_F8>,
+ <KEY_F9>,
+ <KEY_F10>,
+ <KEY_F1>,
+ <KEY_F2>,
+ <KEY_F3>,
+ <KEY_POWER>;
+ };
--
2.43.2
^ permalink raw reply related
* [PATCH v12 00/38] ep93xx device tree conversion
From: Nikita Shubin via B4 Relay @ 2024-09-09 8:10 UTC (permalink / raw)
To: Arnd Bergmann, Hartley Sweeten, Alexander Sverdlin, Russell King,
Lukasz Majewski, Linus Walleij, Bartosz Golaszewski,
Andy Shevchenko, Michael Turquette, Stephen Boyd,
Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nikita Shubin, Vinod Koul, Wim Van Sebroeck, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Mark Brown,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Damien Le Moal, Sergey Shtylyov, Dmitry Torokhov, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Ralf Baechle, Wu, Aaron, Lee Jones,
Olof Johansson, Niklas Cassel
Cc: linux-arm-kernel, linux-kernel, linux-gpio, linux-clk, linux-pm,
devicetree, dmaengine, linux-watchdog, linux-pwm, linux-spi,
netdev, linux-mtd, linux-ide, linux-input, linux-sound,
20240904-devm_clk_hw_register_fixed_rate_parent_data-v1-1-7f14d6b456e5,
20240829-cs4271-yaml-v3-1-f1624cc838f6, Bartosz Golaszewski,
Krzysztof Kozlowski, Andy Shevchenko, Andy Shevchenko,
Andrew Lunn
The goal is to recieve ACKs for all patches in series to merge it via Arnd branch.
It was decided from the very beginning of these series, mostly because
it's a full conversion of platform code to DT and it seemed not
convenient to maintain compatibility with both platform and DT.
Following patches require attention from Stephen Boyd or clk subsystem:
- clk: ep93xx: add DT support for Cirrus EP93xx:
- tristate
- drop MFD_SYSCON/REGMAP
- add AUXILIARY_BUS/REGMAP_MMIO
- prefixed all static with ep9xx_
- s/clk_hw_register_ddiv()/ep93xx_clk_register_ddiv()/
- s/clk_register_div()/ep93xx_clk_register_div()/
- dropped devm_ep93xx_clk_hw_register_fixed_rate_parent_data macro
- s/devm_ep93xx_clk_hw_register_fixed_rate_parent_data()/devm_clk_hw_register_fixed_rate_parent_data()/
Fixed dtschema compain for cs4270, see deps.
Patches should be formated with '--histogram'
This series depends on:
- clk: fixed-rate: add devm_clk_hw_register_fixed_rate_parent_data()
(applied to clk-next)
- ASoC: dt-bindings: cirrus,cs4271: Convert to dtschema (applied to
bronie/sound for-next)
prerequisite-message-id: <20240904-devm_clk_hw_register_fixed_rate_parent_data-v1-1-7f14d6b456e5@maquefel.me>
prerequisite-patch-id: 7da0b1e1bcc0a4927607f77ffec6ec3c7c60165b
prerequisite-message-id: <20240829-cs4271-yaml-v3-1-f1624cc838f6@maquefel.me>
prerequisite-patch-id: bdaad57fbb09ee5b8acc08d84bba3ed5345f6dec
---
Changes in v12:
- ARM: dts: ep93xx: Add EDB9302 DT:
- s/reset-gpio/reset-gpios/
- clk: ep93xx: add DT support for Cirrus EP93x:
- tristate
- drop MFD_SYSCON/REGMAP
- add AUXILIARY_BUS/REGMAP_MMIO
- prefixed all static with ep9xx_
- s/clk_hw_register_ddiv()/ep93xx_clk_register_ddiv()/
- s/clk_register_div()/ep93xx_clk_register_div()/
- dropped devm_ep93xx_clk_hw_register_fixed_rate_parent_data macro
- s/devm_ep93xx_clk_hw_register_fixed_rate_parent_data()/devm_clk_hw_register_fixed_rate_parent_data()/
- Link to v11: https://lore.kernel.org/r/20240715-ep93xx-v11-0-4e924efda795@maquefel.me
Changes in v11:
- clk: ep93xx: add DT support for Cirrus EP93xx
- added devm_ep93xx_clk_hw_register_fixed_rate_parent_data() for
devm_ version of clk_hw_register_fixed_rate_parent_data()
- s/devm_clk_hw_register_fixed_rate()/devm_ep93xx_clk_hw_register_fixed_rate_parent_data()/
- replaced all devm_clk_hw_register_fixed_factor() to
devm_clk_hw_register_fixed_factor_parent_hw() or
devm_clk_hw_register_fixed_factor_index()
- s/devm_clk_hw_register_gate()/devm_clk_hw_register_gate_parent_data()
- Link to v10: https://lore.kernel.org/r/20240617-ep93xx-v10-0-662e640ed811@maquefel.me
Changes in v10:
Reordered SoB tags to make sure they appear before Rb and Acked tags.
dmaengine: cirrus: Convert to DT for Cirrus EP93xx
- s/dma/dmaengine/ title
dmaengine: cirrus: remove platform code
- s/dma/dmaengine/ title
soc: Add SoC driver for Cirrus ep93xx:
- added __init for ep93xx_adev_alloc(), ep93xx_controller_register()
- added static, __initconst for pinctrl_names[]
- clk revision for SPI is now resolved here through differently named
clk device
- more verbose Kconfig description
clk: ep93xx: add DT support for Cirrus EP93xx:
- dropped includes
- dropped ep93xx_soc_table[]
- add different named clk and dropped involved includes
- moved pll's and fclk, hclk, pclk init to separate function
- fixed ep93xx_clk_ids[] explicit lines
- Link to v9: https://lore.kernel.org/r/20240326-ep93xx-v9-0-156e2ae5dfc8@maquefel.me
- Link to v2 clk: https://lore.kernel.org/r/20240408-ep93xx-clk-v2-1-adcd68c13753@maquefel.me
Changes in v9:
ARM: dts: add Cirrus EP93XX SoC .dtsi
- added #interrupt-cells to gpio nodes with interrupts-controller
- fixed EOF
ARM: dts: ep93xx: Add EDB9302 DT
- Alexander Sverdlin: fixed bug in Device Tree resulting in CS4271 not working
input: keypad: ep93xx: add DT support for Cirrus EP93xx
- fixed identation and type
- Link to v8: https://lore.kernel.org/r/20240226-ep93xx-v8-0-3136dca7238f@maquefel.me/
Changes in v8:
soc: Add SoC driver for Cirrus ep93xx
- fixed freeing adev instead of rdev
- use __free() and no_free_ptr() for rdev allocation
- s/of_device_get_match_data()/device_get_match_data()/
ata: pata_ep93xx: add device tree support
- more appropriate usage of dev_err_probe()
pinctrl: add a Cirrus ep93xx SoC pin controller
- 8 per row in ide_9312_pins
mtd: rawnand: add support for ts72xx
- fwnode_handle_put() for fwnode in ts72xx_nand_remove()
- Link to v7: https://lore.kernel.org/r/20240118-ep93xx-v7-0-d953846ae771@maquefel.me
Changes in v7:
mtd: rawnand: add support for ts72xx
- fixed KConfig description
ARM: ep93xx: Add terminator to gpiod_lookup_table
- + Reported-by, Fixes
ARM: ep93xx: add regmap aux_dev
- + trailing comma
- - #include <linux/spinlock.h>
clk: ep93xx: add DT support for Cirrus EP93xx
- dropped unused defines
- return from default in ep93xx_mux_get_parent()
- use guard() in ep93xx_mux_set_parent_lock()
- <math.h> header for abs_diff()
- fixed comments
pinctrl: add a Cirrus ep93xx SoC pin controller
- dropped comments for DEVCFG defines
- <linux/array_size.h> for ARRAY_SIZE()
- + default in ep93xx_get_group_name()
- correct cast for id->driver_data
- s/device_set_of_node_from_dev()/device_set_node()/
power: reset: Add a driver for the ep93xx reset
- Add <linux/container_of.h>, <linux/errno.h>, <linux/slab.h>
- Add <linux/module.h>, <linux/mod_devicetable.h>
- Remove <platform_device.h>
spi: ep93xx: add DT support for Cirrus EP93xx
- Replace with ret = dev_err_probe(...);
ata: pata_ep93xx: add device tree support
- fixed wrong rebase with some partes leaked in "ata: pata_ep93xx: remove legacy pinctrl use"
- fix dma_request_chan() error processing
dma: cirrus: Convert to DT for Cirrus EP93xx
- fixed commit message (dropped explicit "only")
- fixed clk_get() processing to defer probe and log spamming
- refactor ep93xx_m2p_dma_filter()
- dropped blank line in ep93xx_m2p_dma_of_xlate()
- refactor ep93xx_m2m_dma_of_xlate()
dma: cirrus: remove platform code
- s/dma/DMA/ in commit message
soc: Add SoC driver for Cirrus ep93xx
- add period
- use cleanup and guard() for spinlocking
- correct cast for device_get_match_data()
- dropped dev_info() with SoC revision - i can't find it anywhere since 2.6 :/,
don't know why i was so sured that ep93xx always printed that
ata: pata_ep93xx: remove legacy pinctrl use
- made error handling in DMA as Uwe suggested
- Link to v6: https://lore.kernel.org/r/20231212-ep93xx-v6-0-c307b8ac9aa8@maquefel.me
Changes in v6:
- clk: ep93xx: add DT support for Cirrus EP93xx
- s/spin_lock_irqsave()/guard()/
- refactor index check in ep93xx_mux_set_parent_lock() to something more readable
- use in_range in ep93xx_mux_set_parent_lock()/ep93xx_ddiv_set_rate()
- use GENMASK() in ep93xx_ddiv_recalc_rate()
- comment reserved bit in ep93xx_ddiv_set_rate()
- move out from loop ClkDiv value assigment
- some style fixes
Andy, i was i asked to set index of XTALI explicitly, i am not setting ddiv_pdata
there becouse only XTALI is jnown in advance, and i think setting them in one place is more convenient.
- pinctrl: add a Cirrus ep93xx SoC pin controller
- drop OF from Kconfig
- droped linux/of.h include
- add space to */ where it is applicable
- add coma in multiline assigment
- "return NULL" as default case in ep93xx_get_group_name()
- fixed casting id->driver_data
- use device_set_of_node_from_dev()
- use dev_err_probe()
- power: reset: Add a driver for the ep93xx reset
- drop linux/of.h include
- soc: Add SoC driver for Cirrus ep93xx
- s/GPL-2.0/GPL-2.0-only/
- drop linux/kernel.h include
- + blank line before linux/soc/cirrus/ep93xx.h
- + blank line after ep93xx_get_soc_rev()
- + coma for pinctrl_names
- valid casting to int for of_device_get_match_data() return value
- mtd: rawnand: add support for ts72xx
- return as part of switch case
- s/iowrite8/iowrite8_rep/
- net: cirrus: add DT support for Cirrus EP93xx
- fix header sorting
- dma: cirrus: Convert to DT for Cirrus EP93xx
- use devm_clk_get
- use is_slave_direction
Changes in v5:
- gpio: ep93xx: split device in multiple
- ordered headers
- use irqd_to_hwirq()
- s/platform_get_irq()/platform_get_irq_optional()/
- [PATCH v4 02/42] ARM: ep93xx: add swlocked prototypes
- replaced with ARM: ep93xx: add regmap aux_dev
- [PATCH v4 03/42] dt-bindings: clock: Add Cirrus EP93xx
- fixed identation
- removed EP93XX_CLK_END
- and dropped it
- clock bindings moved to syscon with renaming to cirrus,ep9301-syscon.h
- clk: ep93xx: add DT support for Cirrus EP93xx
- convert to auxiliary and use parent device tree node
- moved all clocks except XTALI here
- used devm version everywhere and *_parent_hw() instead of passing name where it's possible
- unfortunately devm_clk_hw_register_fixed_rate doesn't have a parent index version
- [PATCH v4 05/42] dt-bindings: pinctrl: Add Cirrus EP93xx
- "unevaluatedProperties: false" for pins
- returned "additionalProperties: false" where it was
- and dropped it
- pinctrl: add a Cirrus ep93xx SoC pin controller
- sorted includes
- convert to auxiliary and use parent device tree node
- power: reset: Add a driver for the ep93xx reset
- convert to auxiliary device
- dt-bindings: soc: Add Cirrus EP93xx
- dropped all ref to reboot, clk, pinctrl subnodes
- added pins, as it's now used for pinctrl
- added #clock-cells, as it's now used for clk
- dt-bindings: pwm: Add Cirrus EP93xx
- $ref to pwm.yaml
- fixed 'pwm-cells'
- s/additionalProperties/unevaluatedProperties/
- soc: Add SoC driver for Cirrus ep93xx
- removed clocks, they are moved to clk auxiliary driver, as we dropped the clk dt node
- removed all swlocked exported functions
- dropped static spinlock
- added instantiating auxiliary reboot, clk, pinctrl
- dt-bindings: spi: Add Cirrus EP93xx
- Document DMA support
- spi: ep93xx: add DT support for Cirrus EP93xx
- dropped CONFIG_OF and SPI/DMA platform data entirely
- s/master/host/
- reworked DMA setup so we can use probe defer
- dt-bindings: dma: Add Cirrus EP93xx
- dropped bindings header (moved ports description to YAML)
- changed '#dma-cells' to 2, we use port, direction in cells so we can drop platform code completely
- dma: cirrus: add DT support for Cirrus EP93xx
- dropped platform probing completely
- dropped struct ep93xx_dma_data replaced with internal struct ep93xx_dma_chan_cfg with port/direction
- added xlate functions for m2m/m2p
- we require filters to set dma_cfg before hw_setup
- dt-bindings: ata: Add Cirrus EP93xx
- Document DMA support
- ata: pata_ep93xx: add device tree support
- drop DMA platform header with data
- use DMA OF so we can defer probing until DMA is up
- ARM: dts: add Cirrus EP93XX SoC .dtsi
- ARM: dts: ep93xx: add ts7250 board
- ARM: dts: ep93xx: Add EDB9302 DT
- replaced "eclk: clock-controller" to syscon reference
- replaced "pinctrl: pinctrl" to syscon reference
- gpios are now "enabled" by default
- reworked i2s node
- change all dma nodes and refs
- new additions to I2S
- Document DMA
- Document Audio Port usage
- drop legacy DMA support
- Link to v4: https://lore.kernel.org/r/20230915-ep93xx-v4-0-a1d779dcec10@maquefel.me
Changes in v4:
- gpio: ep93xx: split device in multiple
- s/generic_handle_irq/generic_handle_domain_irq/
- s/int offset/irq_hw_number_t offset/ though now it looks a bit odd to me
- drop i = 0
- drop 'error'
- use dev_err_probe withour printing devname once again
dt-bindings: clock: Add Cirrus EP93xx
- renamed cirrus,ep93xx-clock.h -> cirrus,ep9301-clk.h
clk: ep93xx: add DT support for Cirrus EP93xx
- drop unused includes
- use .name only for xtali, pll1, pll2 parents
- convert // to /*
- pass clk_parent_data instead of char* clock name
dt-bindings: pinctrl: Add Cirrus EP93xx
- s/additionalProperties/unevaluatedProperties/
dt-bindings: soc: Add Cirrus EP93xx
- move syscon to soc directory
- add vendor prefix
- make reboot same style as pinctrl, clk
- use absolute path for ref
- expand example
soc: Add SoC driver for Cirrus ep93xx
- s/0xf0000000/GENMASK(31, 28)/
- s/ret/ep93xx_chip_revision(map)/
- drop symbol exports
- convert to platform driver
dt-bindings: rtc: Add Cirrus EP93xx
- allOf: with $ref to rtc.yaml
- s/additionalProperties/unevaluatedProperties/
dt-bindings: watchdog: Add Cirrus EP93x
- drop description
- reword
power: reset: Add a driver for the ep93xx reset
- lets use 'GPL-2.0+' instead of '(GPL-2.0)'
- s/of_device/of/
- drop mdelay with warning
- return 0 at the end
net: cirrus: add DT support for Cirrus EP93xx
- fix leaking np
mtd: nand: add support for ts72xx
- +bits.h
- drop comment
- ok to fwnode_get_next_child_node
- use goto to put handle and nand and report error
ARM: dts: add Cirrus EP93XX SoC .dtsi
- add simple-bus for ebi, as we don't require to setup anything
- add arm,pl011 compatible to uart nodes
- drop i2c-gpio, as it's isn't used anywhere
ARM: dts: ep93xx: add ts7250 board
- generic node name for temperature-sensor
- drop i2c
- move nand, rtc, watchdog to ebi node
- Link to v3: https://lore.kernel.org/r/20230605-ep93xx-v3-0-3d63a5f1103e@maquefel.me
---
Alexander Sverdlin (3):
ASoC: ep93xx: Drop legacy DMA support
ARM: dts: ep93xx: Add EDB9302 DT
ASoC: cirrus: edb93xx: Delete driver
Nikita Shubin (35):
gpio: ep93xx: split device in multiple
ARM: ep93xx: add regmap aux_dev
clk: ep93xx: add DT support for Cirrus EP93xx
pinctrl: add a Cirrus ep93xx SoC pin controller
power: reset: Add a driver for the ep93xx reset
dt-bindings: soc: Add Cirrus EP93xx
soc: Add SoC driver for Cirrus ep93xx
dt-bindings: dma: Add Cirrus EP93xx
dmaengine: cirrus: Convert to DT for Cirrus EP93xx
dt-bindings: watchdog: Add Cirrus EP93x
watchdog: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: pwm: Add Cirrus EP93xx
pwm: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: spi: Add Cirrus EP93xx
spi: ep93xx: add DT support for Cirrus EP93xx
dt-bindings: net: Add Cirrus EP93xx
net: cirrus: add DT support for Cirrus EP93xx
dt-bindings: mtd: Add ts7200 nand-controller
mtd: rawnand: add support for ts72xx
dt-bindings: ata: Add Cirrus EP93xx
ata: pata_ep93xx: add device tree support
dt-bindings: input: Add Cirrus EP93xx keypad
input: keypad: ep93xx: add DT support for Cirrus EP93xx
wdt: ts72xx: add DT support for ts72xx
gpio: ep93xx: add DT support for gpio-ep93xx
ASoC: dt-bindings: ep93xx: Document DMA support
ASoC: dt-bindings: ep93xx: Document Audio Port support
ARM: dts: add Cirrus EP93XX SoC .dtsi
ARM: dts: ep93xx: add ts7250 board
ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
pwm: ep93xx: drop legacy pinctrl
ata: pata_ep93xx: remove legacy pinctrl use
ARM: ep93xx: delete all boardfiles
ARM: ep93xx: soc: drop defines
dmaengine: cirrus: remove platform code
.../bindings/arm/cirrus/cirrus,ep9301.yaml | 38 +
.../bindings/ata/cirrus,ep9312-pata.yaml | 42 +
.../bindings/dma/cirrus,ep9301-dma-m2m.yaml | 84 ++
.../bindings/dma/cirrus,ep9301-dma-m2p.yaml | 144 ++
.../bindings/input/cirrus,ep9307-keypad.yaml | 87 ++
.../devicetree/bindings/mtd/technologic,nand.yaml | 45 +
.../devicetree/bindings/net/cirrus,ep9301-eth.yaml | 59 +
.../devicetree/bindings/pwm/cirrus,ep9301-pwm.yaml | 53 +
.../bindings/soc/cirrus/cirrus,ep9301-syscon.yaml | 94 ++
.../bindings/sound/cirrus,ep9301-i2s.yaml | 16 +
.../devicetree/bindings/spi/cirrus,ep9301-spi.yaml | 70 +
.../bindings/watchdog/cirrus,ep9301-wdt.yaml | 42 +
arch/arm/Makefile | 1 -
arch/arm/boot/dts/cirrus/Makefile | 4 +
arch/arm/boot/dts/cirrus/ep93xx-bk3.dts | 125 ++
arch/arm/boot/dts/cirrus/ep93xx-edb9302.dts | 181 +++
arch/arm/boot/dts/cirrus/ep93xx-ts7250.dts | 145 ++
arch/arm/boot/dts/cirrus/ep93xx.dtsi | 444 ++++++
arch/arm/mach-ep93xx/Kconfig | 20 +-
arch/arm/mach-ep93xx/Makefile | 11 -
arch/arm/mach-ep93xx/clock.c | 733 ----------
arch/arm/mach-ep93xx/core.c | 1018 --------------
arch/arm/mach-ep93xx/dma.c | 114 --
arch/arm/mach-ep93xx/edb93xx.c | 368 -----
arch/arm/mach-ep93xx/ep93xx-regs.h | 38 -
arch/arm/mach-ep93xx/gpio-ep93xx.h | 111 --
arch/arm/mach-ep93xx/hardware.h | 25 -
arch/arm/mach-ep93xx/irqs.h | 76 --
arch/arm/mach-ep93xx/platform.h | 42 -
arch/arm/mach-ep93xx/soc.h | 212 ---
arch/arm/mach-ep93xx/timer-ep93xx.c | 143 --
arch/arm/mach-ep93xx/ts72xx.c | 422 ------
arch/arm/mach-ep93xx/ts72xx.h | 94 --
arch/arm/mach-ep93xx/vision_ep9307.c | 321 -----
drivers/ata/pata_ep93xx.c | 107 +-
drivers/clk/Kconfig | 8 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ep93xx.c | 846 ++++++++++++
drivers/dma/ep93xx_dma.c | 287 +++-
drivers/gpio/gpio-ep93xx.c | 345 ++---
drivers/input/keyboard/ep93xx_keypad.c | 74 +-
drivers/mtd/nand/raw/Kconfig | 6 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/technologic-nand-controller.c | 222 +++
drivers/net/ethernet/cirrus/ep93xx_eth.c | 63 +-
drivers/pinctrl/Kconfig | 7 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-ep93xx.c | 1434 ++++++++++++++++++++
drivers/power/reset/Kconfig | 10 +
drivers/power/reset/Makefile | 1 +
drivers/power/reset/ep93xx-restart.c | 84 ++
drivers/pwm/pwm-ep93xx.c | 26 +-
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/cirrus/Kconfig | 17 +
drivers/soc/cirrus/Makefile | 2 +
drivers/soc/cirrus/soc-ep93xx.c | 252 ++++
drivers/spi/spi-ep93xx.c | 66 +-
drivers/watchdog/ep93xx_wdt.c | 8 +
drivers/watchdog/ts72xx_wdt.c | 8 +
include/dt-bindings/clock/cirrus,ep9301-syscon.h | 46 +
include/linux/platform_data/dma-ep93xx.h | 94 --
include/linux/platform_data/eth-ep93xx.h | 10 -
include/linux/platform_data/keypad-ep93xx.h | 32 -
include/linux/platform_data/spi-ep93xx.h | 15 -
include/linux/soc/cirrus/ep93xx.h | 47 +-
sound/soc/cirrus/Kconfig | 9 -
sound/soc/cirrus/Makefile | 4 -
sound/soc/cirrus/edb93xx.c | 116 --
sound/soc/cirrus/ep93xx-i2s.c | 19 -
sound/soc/cirrus/ep93xx-pcm.c | 19 +-
71 files changed, 5161 insertions(+), 4550 deletions(-)
---
base-commit: fa050d2415bbf9d483fc948ecf224cc2be4a173a
change-id: 20230605-ep93xx-01c76317e2d2
Best regards,
--
Nikita Shubin <nikita.shubin@maquefel.me>
^ permalink raw reply
* Re: [PATCH] dt-binding: touchscreen: fix x-plat-ohm missing type definition
From: Sayyad Abid @ 2024-09-09 8:06 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Marek Vasut, Michael Welling, linux-input,
linux-kernel, Sayyad Abid
In-Reply-To: <edf29295-d360-4038-a490-3a5cbb8c28cd@kernel.org>
On Mon, Sep 9, 2024 at 1:21 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 09/09/2024 09:48, Sayyad Abid wrote:
> > On Mon, Sep 9, 2024 at 12:02 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Sun, Sep 08, 2024 at 08:47:43PM +0530, Sayyad Abid wrote:
> >>> This patch fixes the issue with x-plat-ohm missing a type definition.
> >>> The patch adds the fix for this issue by adding value of this property
> >>
> >> You repeated twice the same while it is unclear why this is missing.
> > I must have overlooked it, my bad.
>
> Keep discussion public.
I am away form my desktop and trying to reply with Gmail's client,
I use mutt otherwise hence the trouble. I hit 'reply' instead of 'reply all'.
>
> >>
> >> Also:
> >> Please do not use "This commit/patch/change", but imperative mood. See
> >> longer explanation here:
> >> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> > Okay, I'll refer to this article for any further patches. Thank you!
> >>
> >>
> >>> should be a 32-bit unsigned integer.
> >>>
> >>> Signed-off-by: Sayyad Abid <sayyad.abid16@gmail.com>
> >>>
> >>> ---
> >>> .../devicetree/bindings/input/touchscreen/ti,tsc2005.yaml | 2 ++
> >>> 1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> >>> index 7187c390b2f5..98ff65cf9f9f 100644
> >>> --- a/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> >>> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> >>> @@ -38,6 +38,8 @@ properties:
> >>>
> >>> ti,x-plate-ohms:
> >>> description: resistance of the touchscreen's X plates in ohm (defaults to 280)
> >>
> >>> + $ref: /schemas/types.yaml#/definitions/uint32
> >>
> >> $ref should not be needed, so what is exactly missing? Provide some
> >> sort of proof that you are fixing real issue.
> > I ran dt_bindings_check on the file which resulted in a warning
> > "x-plate-ohm : missing type definition", to resolve this warning I
> > looked at the other yaml files in the ti,txc2005.yaml directory
> > (/Documentation/devicetree/bindings/input/touchscreen/), Where I found
> > out that one of the TI's touchscreen (ti,am3359) binding used $ref
> > property for the similar "x-plate-resistance" property.
> >
> > Adding the $ref resolved the warnings.
>
> You run some older dtschema.
I'll update this and check again.
>
> Anyway, each commit must explain why you are doing this. Whatever
> warning you fix, you should mention it in the commit msg, because that's
> the answer to "why?" part.
>
Yes, will keep this in mind for any further patches.
> Best regards,
> Krzysztof
>
Thank you for your time and input.
--
Abid
^ permalink raw reply
* Re: [PATCH] dt-binding: touchscreen: fix x-plat-ohm missing type definition
From: Krzysztof Kozlowski @ 2024-09-09 6:32 UTC (permalink / raw)
To: Sayyad Abid
Cc: devicetree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Marek Vasut, Michael Welling, linux-input,
linux-kernel
In-Reply-To: <20240908151742.2453402-1-sayyad.abid16@gmail.com>
On Sun, Sep 08, 2024 at 08:47:43PM +0530, Sayyad Abid wrote:
> This patch fixes the issue with x-plat-ohm missing a type definition.
> The patch adds the fix for this issue by adding value of this property
You repeated twice the same while it is unclear why this is missing.
Also:
Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> should be a 32-bit unsigned integer.
>
> Signed-off-by: Sayyad Abid <sayyad.abid16@gmail.com>
>
> ---
> .../devicetree/bindings/input/touchscreen/ti,tsc2005.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> index 7187c390b2f5..98ff65cf9f9f 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti,tsc2005.yaml
> @@ -38,6 +38,8 @@ properties:
>
> ti,x-plate-ohms:
> description: resistance of the touchscreen's X plates in ohm (defaults to 280)
> + $ref: /schemas/types.yaml#/definitions/uint32
$ref should not be needed, so what is exactly missing? Provide some
sort of proof that you are fixing real issue.
> +
Just one blank line.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 1/2] dt-bindings: input: touchscreen: add Hynitron CST816X
From: Krzysztof Kozlowski @ 2024-09-09 6:25 UTC (permalink / raw)
To: Oleh Kuzhylnyi
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree, linux-kernel, Conor Dooley, igor.opaniuk,
Neil Armstrong, Jeff LaBundy
In-Reply-To: <20240908113027.69471-1-kuzhylol@gmail.com>
On Sun, Sep 08, 2024 at 01:30:26PM +0200, Oleh Kuzhylnyi wrote:
> Add documentation for the Hynitron CST816X touchscreen bindings.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Oleh Kuzhylnyi <kuzhylol@gmail.com>
> ---
> .../input/touchscreen/hynitron,cst816s.yaml | 57 +++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816s.yaml
>
I asked to do some minor tweak if new version was going to be send...
eh...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 15/22] Input: iqs7222 - use cleanup facility for fwnodes
From: Dmitry Torokhov @ 2024-09-09 1:34 UTC (permalink / raw)
To: Jeff LaBundy
Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
Eddie James, Andrey Moiseev, Hans de Goede, Javier Carrasco,
linux-kernel
In-Reply-To: <Zt49WnZBar2D822M@nixie71>
Hi Jeff,
On Sun, Sep 08, 2024 at 07:12:10PM -0500, Jeff LaBundy wrote:
> Hi Dmitry,
>
> On Tue, Sep 03, 2024 at 09:48:25PM -0700, Dmitry Torokhov wrote:
> > Use __free(fwnode_handle) cleanup facility to ensure that references to
> > acquired fwnodes are dropped at appropriate times automatically.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/misc/iqs7222.c | 30 ++++++++++++++----------------
> > 1 file changed, 14 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
> > index 9ca5a743f19f..d9b87606ff7a 100644
> > --- a/drivers/input/misc/iqs7222.c
> > +++ b/drivers/input/misc/iqs7222.c
> > @@ -2385,9 +2385,9 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222,
> > for (i = 0; i < ARRAY_SIZE(iqs7222_kp_events); i++) {
> > const char *event_name = iqs7222_kp_events[i].name;
> > u16 event_enable = iqs7222_kp_events[i].enable;
> > - struct fwnode_handle *event_node;
> >
> > - event_node = fwnode_get_named_child_node(chan_node, event_name);
> > + struct fwnode_handle *event_node __free(fwnode_handle) =
> > + fwnode_get_named_child_node(chan_node, event_name);
>
> This leaves a newline amongst the declarations, but not in between the
> declarations and code. I don't feel strongly against this, but it's
> opposite of what I understand to be more common; please let me know in
> case I have misunderstood. This comment applies to the other chunks as
> well.
Right, so this is actually combining declaration and initialization case
that I mentioned in the other email, and it is closer in spirit to the
code and the allocation check below. That is why it is separated from
the declaration block.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 14/22] Input: iqs626a - use cleanup facility for fwnodes
From: Dmitry Torokhov @ 2024-09-09 1:31 UTC (permalink / raw)
To: Jeff LaBundy
Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
Eddie James, Andrey Moiseev, Hans de Goede, Javier Carrasco,
linux-kernel
In-Reply-To: <Zt47Ic059YbOSbGy@nixie71>
Hi Jeff,
On Sun, Sep 08, 2024 at 07:02:41PM -0500, Jeff LaBundy wrote:
> Hi Dmitry,
>
> On Tue, Sep 03, 2024 at 09:48:13PM -0700, Dmitry Torokhov wrote:
> > Use __free(fwnode_handle) cleanup facility to ensure that references to
> > acquired fwnodes are dropped at appropriate times automatically.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/misc/iqs626a.c | 22 ++++++----------------
> > 1 file changed, 6 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> > index 0dab54d3a060..7a6e6927f331 100644
> > --- a/drivers/input/misc/iqs626a.c
> > +++ b/drivers/input/misc/iqs626a.c
> > @@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > {
> > struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> > struct i2c_client *client = iqs626->client;
> > - struct fwnode_handle *ev_node;
> > const char *ev_name;
> > u8 *thresh, *hyst;
> > unsigned int val;
> > @@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > if (!iqs626_channels[ch_id].events[i])
> > continue;
> >
> > + struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
>
> This seems to deviate from what I understand to be a more conventional
> style of declaring variables at the top of the scope, and separate from
> actual code, like below:
This is follows Linus' guidance on combining declaration and
initialization for variables using __free() cleanup annotations (where
possible). These annotations are the reason for dropping
-Wdeclaration-after-statement from our makefiles. See b5ec6fd286df
("kbuild: Drop -Wdeclaration-after-statement") and discussion in
https://lore.kernel.org/all/CAHk-=wi-RyoUhbChiVaJZoZXheAwnJ7OO=Gxe85BkPAd93TwDA@mail.gmail.com
>
>
> for (i = 0; i < ARRAY_SIZE(iqs626_events); i++) {
> struct fwnode_handle *ev_node __free(fwnode_handle);
>
> if (!iqs626_channels[ch_id].events[i])
> continue;
This will result in attempt to "put" a garbage pointer if we follow
"continue" code path. In general __free() pointers have to be
initialized, either to NULL or to a valid object (assuming that cleanup
function can deal with NULLs).
>
> I also did not see any reason to explicitly declare the variable as NULL;
> let me know in case I have misunderstood.
See the above. Yes, in this particular case it will get a value in both
branches, but I feel it is too fragile and may get messed up if someone
refactors code.
>
> > if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
> > /*
> > * Trackpad touch events are simply described under the
> > @@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > dev_err(&client->dev,
> > "Invalid input type: %u\n",
> > val);
> > - fwnode_handle_put(ev_node);
> > return -EINVAL;
> > }
> >
> > @@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > dev_err(&client->dev,
> > "Invalid %s channel hysteresis: %u\n",
> > fwnode_get_name(ch_node), val);
> > - fwnode_handle_put(ev_node);
> > return -EINVAL;
> > }
> >
> > @@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > dev_err(&client->dev,
> > "Invalid %s channel threshold: %u\n",
> > fwnode_get_name(ch_node), val);
> > - fwnode_handle_put(ev_node);
> > return -EINVAL;
> > }
> >
> > @@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > else
> > *(thresh + iqs626_events[i].th_offs) = val;
> > }
> > -
> > - fwnode_handle_put(ev_node);
> > }
> >
> > return 0;
> > @@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> > for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
> > u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
> > u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
> > - struct fwnode_handle *tc_node;
> > char tc_name[10];
> >
> > snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
> >
> > - tc_node = fwnode_get_named_child_node(ch_node, tc_name);
> > + struct fwnode_handle *tc_node __free(fwnode_handle) =
> > + fwnode_get_named_child_node(ch_node, tc_name);
>
> Same here.
Yes, combining declaration and initialization is preferred for such
pointers.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox