Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] fix Input: bcm-keypad - Correct dev_err_probe() error
From: Krzysztof Kozlowski @ 2023-07-11  7:24 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede, Andy Shevchenko, linux-input,
	linux-kernel
  Cc: kernel test robot, Dan Carpenter
In-Reply-To: <20230711072254.43273-1-krzysztof.kozlowski@linaro.org>

On 11/07/2023 09:22, Krzysztof Kozlowski wrote:
> Pass proper PTR_ERR as dev_err_probe() argument.

I need to fix subject. :(

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH] fix Input: bcm-keypad - Correct dev_err_probe() error
From: Krzysztof Kozlowski @ 2023-07-11  7:22 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede, Krzysztof Kozlowski,
	Andy Shevchenko, linux-input, linux-kernel
  Cc: kernel test robot, Dan Carpenter

Pass proper PTR_ERR as dev_err_probe() argument.

Fixes: a2c795b696b2 ("Input: bcm-keypad - simplify with dev_err_probe()")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202306261505.wTjCXRIO-lkp@intel.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/input/keyboard/bcm-keypad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c
index 05b09066df84..bcc480ff7c3b 100644
--- a/drivers/input/keyboard/bcm-keypad.c
+++ b/drivers/input/keyboard/bcm-keypad.c
@@ -367,7 +367,7 @@ static int bcm_kp_probe(struct platform_device *pdev)
 	/* Enable clock */
 	kp->clk = devm_clk_get_optional(&pdev->dev, "peri_clk");
 	if (IS_ERR(kp->clk)) {
-		return dev_err_probe(&pdev->dev, error, "Failed to get clock\n");
+		return dev_err_probe(&pdev->dev, PTR_ERR(kp->clk), "Failed to get clock\n");
 	} else if (!kp->clk) {
 		dev_dbg(&pdev->dev, "No clock specified. Assuming it's enabled\n");
 	} else {
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH 00/17] fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags
From: Thomas Zimmermann @ 2023-07-11  6:24 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: deller, javierm, linux-fbdev, kvm, linux-hyperv, linux-sh,
	linux-staging, linux-kernel, amd-gfx, linux-geode, dri-devel,
	linux-input, linux-nvidia, linux-omap, linuxppc-dev,
	linux-arm-kernel, linux-media
In-Reply-To: <20230710171903.GA14712@ravnborg.org>


[-- Attachment #1.1: Type: text/plain, Size: 2076 bytes --]

Hi Sam

Am 10.07.23 um 19:19 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote:
>> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from
>> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe
>> useful when fbdev had special handling for driver modules. With
>> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0
>> and have no further effect.
>>
>> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5
>> split this by the way the fb_info struct is being allocated. All flags
>> are cleared to zero during the allocation.
>>
>> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes
>> an actual bug in how arch/sh uses the tokne for struct fb_videomode,
>> which is unrelated.
>>
>> Patch 17 removes both flag constants from <linux/fb.h>
> 
> We have a few more flags that are unused - should they be nuked too?
> FBINFO_HWACCEL_FILLRECT
> FBINFO_HWACCEL_ROTATE
> FBINFO_HWACCEL_XPAN

It seems those are there for completeness. Nothing sets _ROTATE, the 
others are simply never checked. According to the comments, some are 
required, some are optional. I don't know what that means.

IIRC there were complains about performance when Daniel tried to remove 
fbcon acceleration, so not all _HWACCEL_ flags are unneeded.

Leaving them in for reference/completeness might be an option; or not. I 
have no strong feelings about those flags.

> 
> Unused as in no references from fbdev/core/*
> 
> I would rather see one series nuke all unused FBINFO flags in one go.
> Assuming my quick grep are right and the above can be dropped.

I would not want to extend this series. I'm removing _DEFAULT as it's 
absolutely pointless and confusing.

Best regards
Thomas

> 
> 	Sam

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply

* Re: [PATCH 09/17] auxdisplay: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers
From: Thomas Zimmermann @ 2023-07-11  6:10 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: deller, javierm, linux-sh, dri-devel, linux-kernel, amd-gfx,
	linux-input, linux-media, linux-fbdev, linux-staging,
	linux-arm-kernel, linux-geode, linux-nvidia, linux-hyperv,
	linux-omap, linuxppc-dev, kvm, Miguel Ojeda, Robin van der Gracht
In-Reply-To: <CANiq72kPh2KE=ADUxhPyyr7noWhC0fkzmDu8EBn_20focnZqtw@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 806 bytes --]

Hi

Am 10.07.23 um 18:25 schrieb Miguel Ojeda:
> On Mon, Jul 10, 2023 at 5:22 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> I'll append a patch to the series that documents this.
>>
>> Sure.
> 
> Thanks!
> 
> If you are planning to take it into some other tree:
> 
> Acked-by: Miguel Ojeda <ojeda@kernel.org>

Thanks.

> 
> Otherwise, I can take it into the `auxdisplay` tree.

I'd like to take the patchset into drm-misc. It's part of a larger 
cleanup of the fbdev modules and its interfaces.

Best regards
Thomas

> 
> Cheers,
> Miguel

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply

* Re: [PATCH] HID: usbhid: Fix use assignment in if condition
From: Greg KH @ 2023-07-11  4:09 UTC (permalink / raw)
  To: wuyonggang001
  Cc: jikos, benjamin.tissoires, linux-usb, linux-input, linux-kernel
In-Reply-To: <a4ca3852846e220cf378a664cf1c6213@208suo.com>

On Tue, Jul 11, 2023 at 09:47:06AM +0800, wuyonggang001@208suo.com wrote:
> Fix the following checkpatch error(s):
> 
> drivers/hid/usbhid/usbkbd.c:238:240:242:246: ERROR: do not use assignment in
> if condition
> 
> Signed-off-by: Yonggang Wu <wuyonggang001@208suo.com>
> ---
>  drivers/hid/usbhid/usbkbd.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
> index c439ed2f16db..cde7f82b7070 100644
> --- a/drivers/hid/usbhid/usbkbd.c
> +++ b/drivers/hid/usbhid/usbkbd.c
> @@ -235,15 +235,29 @@ static void usb_kbd_close(struct input_dev *dev)
> 
>  static int usb_kbd_alloc_mem(struct usb_device *dev, struct usb_kbd *kbd)
>  {
> -    if (!(kbd->irq = usb_alloc_urb(0, GFP_KERNEL)))
> +    kbd->irq = usb_alloc_urb(0, GFP_KERNEL)
> +
> +    if (!kbd->irq)
>          return -1;
> -    if (!(kbd->led = usb_alloc_urb(0, GFP_KERNEL)))
> +
> +    kbd->led = usb_alloc_urb(0, GFP_KERNEL)
> +
> +    if (!kbd->led)
>          return -1;
> -    if (!(kbd->new = usb_alloc_coherent(dev, 8, GFP_KERNEL,
> &kbd->new_dma)))
> +
> +    kbd->new = usb_alloc_coherent(dev, 8, GFP_KERNEL, &kbd->new_dma)
> +
> +    if (!kbd->new)
>          return -1;
> -    if (!(kbd->cr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL)))
> +
> +    kbd->cr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL)
> +
> +    if (!kbd->cr)
>          return -1;
> -    if (!(kbd->leds = usb_alloc_coherent(dev, 1, GFP_KERNEL,
> &kbd->leds_dma)))
> +
> +    kbd->leds = usb_alloc_coherent(dev, 1, GFP_KERNEL, &kbd->leds_dma)
> +
> +    if (!kbd->leds)
>          return -1;
> 
>      return 0;

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch is malformed (tabs converted to spaces, linewrapped, etc.)
  and can not be applied.  Please read the file,
  Documentation/process/email-clients.rst in order to fix this.


If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

^ permalink raw reply

* [PATCH] HID: usbhid: Fix use assignment in if condition
From: wuyonggang001 @ 2023-07-11  1:47 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-usb, linux-input, linux-kernel
In-Reply-To: <20230711014359.11991-1-zhanglibing@cdjrlc.com>

Fix the following checkpatch error(s):

drivers/hid/usbhid/usbkbd.c:238:240:242:246: ERROR: do not use 
assignment in if condition

Signed-off-by: Yonggang Wu <wuyonggang001@208suo.com>
---
  drivers/hid/usbhid/usbkbd.c | 24 +++++++++++++++++++-----
  1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index c439ed2f16db..cde7f82b7070 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -235,15 +235,29 @@ static void usb_kbd_close(struct input_dev *dev)

  static int usb_kbd_alloc_mem(struct usb_device *dev, struct usb_kbd 
*kbd)
  {
-    if (!(kbd->irq = usb_alloc_urb(0, GFP_KERNEL)))
+    kbd->irq = usb_alloc_urb(0, GFP_KERNEL)
+
+    if (!kbd->irq)
          return -1;
-    if (!(kbd->led = usb_alloc_urb(0, GFP_KERNEL)))
+
+    kbd->led = usb_alloc_urb(0, GFP_KERNEL)
+
+    if (!kbd->led)
          return -1;
-    if (!(kbd->new = usb_alloc_coherent(dev, 8, GFP_KERNEL, 
&kbd->new_dma)))
+
+    kbd->new = usb_alloc_coherent(dev, 8, GFP_KERNEL, &kbd->new_dma)
+
+    if (!kbd->new)
          return -1;
-    if (!(kbd->cr = kmalloc(sizeof(struct usb_ctrlrequest), 
GFP_KERNEL)))
+
+    kbd->cr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL)
+
+    if (!kbd->cr)
          return -1;
-    if (!(kbd->leds = usb_alloc_coherent(dev, 1, GFP_KERNEL, 
&kbd->leds_dma)))
+
+    kbd->leds = usb_alloc_coherent(dev, 1, GFP_KERNEL, &kbd->leds_dma)
+
+    if (!kbd->leds)
          return -1;

      return 0;

^ permalink raw reply related

* Re: [PATCH 01/10] Input: bcm-keypad - Convert to devm_platform_ioremap_resource()
From: Dmitry Torokhov @ 2023-07-11  0:43 UTC (permalink / raw)
  To: Yangtao Li; +Cc: linux-input, linux-kernel
In-Reply-To: <20230705052346.39337-1-frank.li@vivo.com>

On Wed, Jul 05, 2023 at 01:23:37PM +0800, Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>

I applied the lot with the exception of patch #9 which is not needed,
with minor edits in patches #8 and #10.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 08/10] Input: nomadik-ske-keypad - Convert to use devm_* api
From: Dmitry Torokhov @ 2023-07-11  0:42 UTC (permalink / raw)
  To: Yangtao Li; +Cc: linux-input, linux-kernel
In-Reply-To: <20230705052346.39337-8-frank.li@vivo.com>

Hi,

On Wed, Jul 05, 2023 at 01:23:44PM +0800, Yangtao Li wrote:
> @@ -305,7 +288,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>  	error = clk_prepare_enable(keypad->pclk);
>  	if (error) {
>  		dev_err(&pdev->dev, "Failed to prepare/enable pclk\n");
> -		goto err_clk;
> +		return error;
>  	}
>  
>  	error = clk_prepare_enable(keypad->clk);

We should not mix managed (devm) and normal resources, because doing so
wrecks the order of resource unwinding. Lucklily we have
devm_clk_get_enabled() now, so I switched the driver to use it and
applied the patch.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 09/10] Input: lpc32xx_ts - stop_tsc when driver remove
From: Dmitry Torokhov @ 2023-07-11  0:40 UTC (permalink / raw)
  To: Yangtao Li
  Cc: Vladimir Zapolskiy, Kevin Wells, Durgesh Pattamatta, linux-input,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20230705052346.39337-9-frank.li@vivo.com>

Hi,

On Wed, Jul 05, 2023 at 01:23:45PM +0800, Yangtao Li wrote:
> When the driver is removed, we need to close the device.
> 
> Fixes: 3045a5f5202a ("Input: add LPC32xx touchscreen controller driver")
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  drivers/input/touchscreen/lpc32xx_ts.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/lpc32xx_ts.c b/drivers/input/touchscreen/lpc32xx_ts.c
> index 15b5cb763526..ffdd748a9992 100644
> --- a/drivers/input/touchscreen/lpc32xx_ts.c
> +++ b/drivers/input/touchscreen/lpc32xx_ts.c
> @@ -305,6 +305,8 @@ static int lpc32xx_ts_remove(struct platform_device *pdev)
>  	struct lpc32xx_tsc *tsc = platform_get_drvdata(pdev);
>  	struct resource *res;
>  
> +	lpc32xx_stop_tsc(tsc);
> +

This change is not needed because lpc32xx_stop_tsc() is already being
called from lpc32xx_ts_close() which is called when we unregister input
device (provided that open() was called earlier).

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: tegra-kbc - use devm_platform_ioremap_resource
From: Dmitry Torokhov @ 2023-07-10 19:08 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Laxman Dewangan, Thierry Reding, Jonathan Hunter, linux-input,
	linux-tegra, linux-kernel
In-Reply-To: <20230709134109.182418-1-martin@kaiser.cx>

On Sun, Jul 09, 2023 at 03:41:08PM +0200, Martin Kaiser wrote:
> devm_platform_get_and_ioremap_resource maps a resource and returns its
> physical address. If we don't need the physical address, we should call
> devm_platform_ioremap_resource instead.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 0/4] Add support for Azoteq IQS7222D
From: Dmitry Torokhov @ 2023-07-10 19:08 UTC (permalink / raw)
  To: Jeff LaBundy
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, linux-input,
	devicetree
In-Reply-To: <ZKrotyhz7eueujys@nixie71>

On Sun, Jul 09, 2023 at 12:04:55PM -0500, Jeff LaBundy wrote:
> This series adds support for the latest member of the Azoteq IQS7222x
> family of devices; it also sneaks in a bug fix and a clarification to
> the binding.
> 
> Changes in v2:
>  - Added patches [1/4] and [2/4]
>  - Updated patch [3/4] as follows:
>    - Wrapped words instead of splitting them
>    - Specified units for the 'azoteq,top-speed', 'azoteq,bottom-speed'
>      and 'azoteq,gesture-dist' properties as in patch [2/4]
>    - Corrected the 'azoteq,gesture-dist' property's role in the case of
>      tap gestures
>    - Updated the if/then/else schema to consolidate restrictions common
>      to both IQS7222B and IQS7222D, and to indicate that the latter can
>      support the 'azoteq,ulp-allow' property
>    - Added more details to the commit message
>  - Removed unused variable 'val' from patch [4/4]
> 
> Jeff LaBundy (4):
>   Input: iqs7222 - configure power mode before triggering ATI
>   dt-bindings: input: iqs7222: Define units for slider properties
>   dt-bindings: input: iqs7222: Add properties for Azoteq IQS7222D
>   Input: iqs7222 - add support for Azoteq IQS7222D

Applied the lot, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: xpad - add support for HyperX Game Controllers
From: Dmitry Torokhov @ 2023-07-10 18:59 UTC (permalink / raw)
  To: Nguyen, Max; +Cc: linux-input@vger.kernel.org
In-Reply-To: <MW4PR84MB17807ECDCBA17E38BA87F9A5EB32A@MW4PR84MB1780.NAMPRD84.PROD.OUTLOOK.COM>

Hi Max,

On Sat, Jul 08, 2023 at 01:30:16AM +0000, Nguyen, Max wrote:
> Add HyperX controller support to xpad_device and xpad_table.

Unfortunately the patch is whitespace-damaged, could you please
resubmit?

> 
> Reported-by: Chris Toledanes <chris.toledanes@hp.com>
> Acked-by: Carl Ng <carl.ng@hp.com>
> Acked-by: Jessie Jin <yan.jin1@hp.com>

Acks should be given at least in public and they are typically reserved
for maintainers or people actively working on the code/driver in
question. See Documentation/process/5.Posting.rst

> Signed-off-by: Maxwell Nguyen <maxwell.nguyen@hp.com>

> ---
> 
> drivers/input/joystick/xpad.c | 3 ++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index cdb193317c3b..a5e507b8f619 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -130,6 +130,7 @@ static const struct xpad_device {
>                  { 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
>                  { 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
>                  { 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
> +                { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
>                  { 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>                  { 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>                  { 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> @@ -457,6 +458,8 @@ static const struct usb_device_id xpad_table[] = {
>                  { USB_INTERFACE_INFO('X', 'B', 0) },          /* Xbox USB-IF not-approved class */
>                  XPAD_XBOX360_VENDOR(0x0079),                             /* GPD Win 2 controller */
>                  XPAD_XBOX360_VENDOR(0x03eb),                             /* Wooting Keyboards (Legacy) */
> +                XPAD_XBOX360_VENDOR(0x03f0),                              /* HyperX Xbox 360 controllers */
> +                XPAD_XBOXONE_VENDOR(0x03f0),                            /* HyperX Xbox One controllers */
>                  XPAD_XBOX360_VENDOR(0x044f),                              /* Thrustmaster Xbox 360 controllers */
>                  XPAD_XBOX360_VENDOR(0x045e),                             /* Microsoft Xbox 360 controllers */
>                  XPAD_XBOXONE_VENDOR(0x045e),                            /* Microsoft Xbox One controllers */
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH resend 2/2] Input: novatek-nvt-ts - add touchscreen model number to description
From: Dmitry Torokhov @ 2023-07-10 18:51 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-input
In-Reply-To: <20230710123633.323937-2-hdegoede@redhat.com>

On Mon, Jul 10, 2023 at 02:36:33PM +0200, Hans de Goede wrote:
> A phoronix forum member actual found documentation on what the model
> number for the touchscreen controller on the Acer Iconia One 7 B1-750 is.
> 
> Update the driver's description to include this.
> 
> Link: https://www.phoronix.com/forums/forum/hardware/general-hardware/1382535-10-years-later-linux-getting-a-touchscreen-driver-for-a-once-popular-tablet?p=1384707#post1384707
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH resend 1/2] Input: novatek-nvt-ts - fix input_register_device() failure error message
From: Dmitry Torokhov @ 2023-07-10 18:50 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-input, Peter Hutterer
In-Reply-To: <20230710123633.323937-1-hdegoede@redhat.com>

On Mon, Jul 10, 2023 at 02:36:32PM +0200, Hans de Goede wrote:
> Fix input_register_device() failure logging "failed to request irq"
> as error message.
> 
> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 00/17] fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags
From: Sam Ravnborg @ 2023-07-10 17:19 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: deller, javierm, linux-fbdev, kvm, linux-hyperv, linux-sh,
	linux-staging, linux-kernel, amd-gfx, linux-geode, dri-devel,
	linux-input, linux-nvidia, linux-omap, linuxppc-dev,
	linux-arm-kernel, linux-media
In-Reply-To: <20230710130113.14563-1-tzimmermann@suse.de>

Hi Thomas,

On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote:
> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from
> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe
> useful when fbdev had special handling for driver modules. With
> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0
> and have no further effect.
> 
> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5
> split this by the way the fb_info struct is being allocated. All flags
> are cleared to zero during the allocation.
> 
> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes
> an actual bug in how arch/sh uses the tokne for struct fb_videomode,
> which is unrelated.
> 
> Patch 17 removes both flag constants from <linux/fb.h>

We have a few more flags that are unused - should they be nuked too?
FBINFO_HWACCEL_FILLRECT
FBINFO_HWACCEL_ROTATE
FBINFO_HWACCEL_XPAN

Unused as in no references from fbdev/core/*

I would rather see one series nuke all unused FBINFO flags in one go.
Assuming my quick grep are right and the above can be dropped.

	Sam

^ permalink raw reply

* Re: [PATCH 09/17] auxdisplay: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers
From: Miguel Ojeda @ 2023-07-10 16:25 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: deller, javierm, linux-sh, dri-devel, linux-kernel, amd-gfx,
	linux-input, linux-media, linux-fbdev, linux-staging,
	linux-arm-kernel, linux-geode, linux-nvidia, linux-hyperv,
	linux-omap, linuxppc-dev, kvm, Miguel Ojeda, Robin van der Gracht
In-Reply-To: <733273ad-89e1-d952-37ee-bb75c3ab8188@suse.de>

On Mon, Jul 10, 2023 at 5:22 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> I'll append a patch to the series that documents this.
>
> Sure.

Thanks!

If you are planning to take it into some other tree:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Otherwise, I can take it into the `auxdisplay` tree.

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 09/17] auxdisplay: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers
From: Thomas Zimmermann @ 2023-07-10 15:22 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: deller, javierm, linux-sh, dri-devel, linux-kernel, amd-gfx,
	linux-input, linux-media, linux-fbdev, linux-staging,
	linux-arm-kernel, linux-geode, linux-nvidia, linux-hyperv,
	linux-omap, linuxppc-dev, kvm, Miguel Ojeda, Robin van der Gracht
In-Reply-To: <CANiq72=9PoV3FOcXx9FdiSLePKXDG4BSY_5-jddBkqDL=ua3FA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1085 bytes --]

Hi

Am 10.07.23 um 16:24 schrieb Miguel Ojeda:
> On Mon, Jul 10, 2023 at 3:01 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> The flag FBINFO_FLAG_DEFAULT is 0 and has no effect, as struct
>> fbinfo.flags has been allocated to zero by framebuffer_alloc(). So do
>> not set it.
> 
> `framebuffer_alloc()` does indeed use `kzalloc()`, but the docs do not
> mention the zeroing. Should that guarantee be documented?

I'll append a patch to the series that documents this.

> 
>> Flags should signal differences from the default values. After cleaning
>> up all occurences of FBINFO_FLAG_DEFAULT, the token can be removed.
> 
> occurences -> occurrences
> 
> can -> will maybe? Since the intention of the patch series is to
> remove it (them) altogether).

Sure.

Best regards
Thomas

> 
> Thanks!
> 
> Cheers,
> Miguel

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply

* Re: [PATCH 09/17] auxdisplay: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers
From: Miguel Ojeda @ 2023-07-10 14:24 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: deller, javierm, linux-sh, dri-devel, linux-kernel, amd-gfx,
	linux-input, linux-media, linux-fbdev, linux-staging,
	linux-arm-kernel, linux-geode, linux-nvidia, linux-hyperv,
	linux-omap, linuxppc-dev, kvm, Miguel Ojeda, Robin van der Gracht
In-Reply-To: <20230710130113.14563-10-tzimmermann@suse.de>

On Mon, Jul 10, 2023 at 3:01 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The flag FBINFO_FLAG_DEFAULT is 0 and has no effect, as struct
> fbinfo.flags has been allocated to zero by framebuffer_alloc(). So do
> not set it.

`framebuffer_alloc()` does indeed use `kzalloc()`, but the docs do not
mention the zeroing. Should that guarantee be documented?

> Flags should signal differences from the default values. After cleaning
> up all occurences of FBINFO_FLAG_DEFAULT, the token can be removed.

occurences -> occurrences

can -> will maybe? Since the intention of the patch series is to
remove it (them) altogether).

Thanks!

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 08/17] arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag
From: John Paul Adrian Glaubitz @ 2023-07-10 14:08 UTC (permalink / raw)
  To: Thomas Zimmermann, deller, javierm
  Cc: linux-sh, dri-devel, linux-kernel, amd-gfx, linux-input,
	linux-media, linux-fbdev, linux-staging, linux-arm-kernel,
	linux-geode, linux-nvidia, linux-hyperv, linux-omap, linuxppc-dev,
	kvm, Yoshinori Sato, Rich Felker
In-Reply-To: <b9fe2f32-f4f6-6158-267a-da83b731b794@suse.de>

Hi!

On Mon, 2023-07-10 at 16:04 +0200, Thomas Zimmermann wrote:
> > > I won't argue with that, but the flag itself is wrong.
> > > FBINFO_FLAG_DEFAULT is/was for struct fb_info.flags. You have struct
> > > fb_videomode.flag. The valid flags for this field are at [1]. If
> > > anything, the field could be initialized to FB_MODE_IS_UNKNOWN, which
> > > has the same value.
> > > 
> > > [1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L681
> > 
> > FB_MODE_IS_UNKNOWN sounds very reasonable to me. Would you agree using that instead?
> 
> Sure, I'll update the patch accordingly.

Thanks! I'll ack the updated patch.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply

* Re: [PATCH 08/17] arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag
From: Thomas Zimmermann @ 2023-07-10 14:04 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, deller, javierm
  Cc: linux-sh, dri-devel, linux-kernel, amd-gfx, linux-input,
	linux-media, linux-fbdev, linux-staging, linux-arm-kernel,
	linux-geode, linux-nvidia, linux-hyperv, linux-omap, linuxppc-dev,
	kvm, Yoshinori Sato, Rich Felker
In-Reply-To: <536cc8c679cc68829c8662176ff02a9052249a46.camel@physik.fu-berlin.de>


[-- Attachment #1.1: Type: text/plain, Size: 1242 bytes --]

Hi

Am 10.07.23 um 15:59 schrieb John Paul Adrian Glaubitz:
> Hi Thomas!
> 
> On Mon, 2023-07-10 at 15:52 +0200, Thomas Zimmermann wrote:
>>> I would argue that the current code is more readable that your proposed change.
>>>
>>> I agree that it's a no-op, but code is not just about functionality but also
>>> readability, isn't it?
>>
>> I won't argue with that, but the flag itself is wrong.
>> FBINFO_FLAG_DEFAULT is/was for struct fb_info.flags. You have struct
>> fb_videomode.flag. The valid flags for this field are at [1]. If
>> anything, the field could be initialized to FB_MODE_IS_UNKNOWN, which
>> has the same value.
>>
>> [1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L681
> 
> FB_MODE_IS_UNKNOWN sounds very reasonable to me. Would you agree using that instead?

Sure, I'll update the patch accordingly.

Best regards
Thomas

> 
>>>
>>> Also, I prefer "sh:" as the architecture prefix, not "arch/sh:".
>>
>> Ok.
> 
> Thanks.
> 
> Adrian
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply

* Re: [PATCH 08/17] arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag
From: John Paul Adrian Glaubitz @ 2023-07-10 13:59 UTC (permalink / raw)
  To: Thomas Zimmermann, deller, javierm
  Cc: linux-sh, dri-devel, linux-kernel, amd-gfx, linux-input,
	linux-media, linux-fbdev, linux-staging, linux-arm-kernel,
	linux-geode, linux-nvidia, linux-hyperv, linux-omap, linuxppc-dev,
	kvm, Yoshinori Sato, Rich Felker
In-Reply-To: <54e3e070-52fb-9ccb-bc47-0f41690f6bfa@suse.de>

Hi Thomas!

On Mon, 2023-07-10 at 15:52 +0200, Thomas Zimmermann wrote:
> > I would argue that the current code is more readable that your proposed change.
> > 
> > I agree that it's a no-op, but code is not just about functionality but also
> > readability, isn't it?
> 
> I won't argue with that, but the flag itself is wrong. 
> FBINFO_FLAG_DEFAULT is/was for struct fb_info.flags. You have struct 
> fb_videomode.flag. The valid flags for this field are at [1]. If 
> anything, the field could be initialized to FB_MODE_IS_UNKNOWN, which 
> has the same value.
> 
> [1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L681

FB_MODE_IS_UNKNOWN sounds very reasonable to me. Would you agree using that instead?

> > 
> > Also, I prefer "sh:" as the architecture prefix, not "arch/sh:".
> 
> Ok.

Thanks.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply

* Re: [PATCH 08/17] arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag
From: Thomas Zimmermann @ 2023-07-10 13:52 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, deller, javierm
  Cc: linux-sh, dri-devel, linux-kernel, amd-gfx, linux-input,
	linux-media, linux-fbdev, linux-staging, linux-arm-kernel,
	linux-geode, linux-nvidia, linux-hyperv, linux-omap, linuxppc-dev,
	kvm, Yoshinori Sato, Rich Felker
In-Reply-To: <0a47ed93fe90a77180533f8c2e42e402827e8f1c.camel@physik.fu-berlin.de>


[-- Attachment #1.1: Type: text/plain, Size: 2151 bytes --]

Hi

Am 10.07.23 um 15:42 schrieb John Paul Adrian Glaubitz:
> Hi Thomas!
> 
> On Mon, 2023-07-10 at 14:50 +0200, Thomas Zimmermann wrote:
>> FBINFO_FLAG_DEFAULT is a flag for a framebuffer in struct fb_info.
>> Flags for videomodes are prefixed with FB_MODE_. FBINFO_FLAG_DEFAULT
>> is 0 and the static declaration already clears the memory area of
>> sh7763fb_videomode. So remove the assignment.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
>> Cc: Rich Felker <dalias@libc.org>
>> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
>> ---
>>   arch/sh/boards/mach-sh7763rdp/setup.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/sh/boards/mach-sh7763rdp/setup.c b/arch/sh/boards/mach-sh7763rdp/setup.c
>> index 97e715e4e9b3..345f2b76c85a 100644
>> --- a/arch/sh/boards/mach-sh7763rdp/setup.c
>> +++ b/arch/sh/boards/mach-sh7763rdp/setup.c
>> @@ -119,7 +119,6 @@ static struct fb_videomode sh7763fb_videomode = {
>>   	.vsync_len = 1,
>>   	.sync = 0,
>>   	.vmode = FB_VMODE_NONINTERLACED,
>> -	.flag = FBINFO_FLAG_DEFAULT,
>>   };
>>   
>>   static struct sh7760fb_platdata sh7763fb_def_pdata = {
> 
> I would argue that the current code is more readable that your proposed change.
> 
> I agree that it's a no-op, but code is not just about functionality but also
> readability, isn't it?

I won't argue with that, but the flag itself is wrong. 
FBINFO_FLAG_DEFAULT is/was for struct fb_info.flags. You have struct 
fb_videomode.flag. The valid flags for this field are at [1]. If 
anything, the field could be initialized to FB_MODE_IS_UNKNOWN, which 
has the same value.

[1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L681

> 
> Also, I prefer "sh:" as the architecture prefix, not "arch/sh:".

Ok.

Best regards
Thomas
> 
> Thanks,
> Adrian
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply

* Re: [PATCH 08/17] arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag
From: John Paul Adrian Glaubitz @ 2023-07-10 13:42 UTC (permalink / raw)
  To: Thomas Zimmermann, deller, javierm
  Cc: linux-sh, dri-devel, linux-kernel, amd-gfx, linux-input,
	linux-media, linux-fbdev, linux-staging, linux-arm-kernel,
	linux-geode, linux-nvidia, linux-hyperv, linux-omap, linuxppc-dev,
	kvm, Yoshinori Sato, Rich Felker
In-Reply-To: <20230710130113.14563-9-tzimmermann@suse.de>

Hi Thomas!

On Mon, 2023-07-10 at 14:50 +0200, Thomas Zimmermann wrote:
> FBINFO_FLAG_DEFAULT is a flag for a framebuffer in struct fb_info.
> Flags for videomodes are prefixed with FB_MODE_. FBINFO_FLAG_DEFAULT
> is 0 and the static declaration already clears the memory area of
> sh7763fb_videomode. So remove the assignment.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> ---
>  arch/sh/boards/mach-sh7763rdp/setup.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/sh/boards/mach-sh7763rdp/setup.c b/arch/sh/boards/mach-sh7763rdp/setup.c
> index 97e715e4e9b3..345f2b76c85a 100644
> --- a/arch/sh/boards/mach-sh7763rdp/setup.c
> +++ b/arch/sh/boards/mach-sh7763rdp/setup.c
> @@ -119,7 +119,6 @@ static struct fb_videomode sh7763fb_videomode = {
>  	.vsync_len = 1,
>  	.sync = 0,
>  	.vmode = FB_VMODE_NONINTERLACED,
> -	.flag = FBINFO_FLAG_DEFAULT,
>  };
>  
>  static struct sh7760fb_platdata sh7763fb_def_pdata = {

I would argue that the current code is more readable that your proposed change.

I agree that it's a no-op, but code is not just about functionality but also
readability, isn't it?

Also, I prefer "sh:" as the architecture prefix, not "arch/sh:".

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply

* Re: [PATCH] Input: tegra-kbc - use devm_platform_ioremap_resource
From: Thierry Reding @ 2023-07-10 13:11 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Laxman Dewangan, Dmitry Torokhov, Jonathan Hunter, linux-input,
	linux-tegra, linux-kernel
In-Reply-To: <20230709134109.182418-1-martin@kaiser.cx>

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

On Sun, Jul 09, 2023 at 03:41:08PM +0200, Martin Kaiser wrote:
> devm_platform_get_and_ioremap_resource maps a resource and returns its
> physical address. If we don't need the physical address, we should call
> devm_platform_ioremap_resource instead.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/input/keyboard/tegra-kbc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Different variations of these have been going around for a while now. I
don't really see much use in these tiny conversions.

But the patch also isn't wrong, so while I'm at it:

Acked-by: Thierry Reding <treding@nvidia.com>


> 
> diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
> index d5a6c7d8eb25..c9a823ea45d0 100644
> --- a/drivers/input/keyboard/tegra-kbc.c
> +++ b/drivers/input/keyboard/tegra-kbc.c
> @@ -640,7 +640,7 @@ static int tegra_kbc_probe(struct platform_device *pdev)
>  
>  	timer_setup(&kbc->timer, tegra_kbc_keypress_timer, 0);
>  
> -	kbc->mmio = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +	kbc->mmio = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(kbc->mmio))
>  		return PTR_ERR(kbc->mmio);
>  
> -- 
> 2.30.2
> 

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

^ permalink raw reply

* Re: [PATCH 10/17] hid/picolcd: Remove flag FBINFO_FLAG_DEFAULT from fbdev driver
From: Benjamin Tissoires @ 2023-07-10 13:05 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: deller, javierm, linux-sh, dri-devel, linux-kernel, amd-gfx,
	linux-input, linux-media, linux-fbdev, linux-staging,
	linux-arm-kernel, linux-geode, linux-nvidia, linux-hyperv,
	linux-omap, linuxppc-dev, kvm, Bruno Prémont, Jiri Kosina
In-Reply-To: <20230710130113.14563-11-tzimmermann@suse.de>

On Mon, Jul 10, 2023 at 3:01 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The flag FBINFO_FLAG_DEFAULT is 0 and has no effect, as struct
> fbinfo.flags has been allocated to zero by framebuffer_alloc(). So do
> not set it.
>
> Flags should signal differences from the default values. After cleaning
> up all occurences of FBINFO_FLAG_DEFAULT, the token can be removed.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Feel free to take this through the DRI tree (or any other that handles
FB) with the rest of the series if you want.

Cheers,
Benjamin

> ---
>  drivers/hid/hid-picolcd_fb.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
> index dabcd054dad9..d726aaafb146 100644
> --- a/drivers/hid/hid-picolcd_fb.c
> +++ b/drivers/hid/hid-picolcd_fb.c
> @@ -527,7 +527,6 @@ int picolcd_init_framebuffer(struct picolcd_data *data)
>         info->var = picolcdfb_var;
>         info->fix = picolcdfb_fix;
>         info->fix.smem_len   = PICOLCDFB_SIZE*8;
> -       info->flags = FBINFO_FLAG_DEFAULT;
>
>         fbdata = info->par;
>         spin_lock_init(&fbdata->lock);
> --
> 2.41.0
>


^ permalink raw reply


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