Re: [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb

2024-02-20 Thread Javier Martinez Canillas
Thomas Zimmermann  writes:

> The driver sets struct fb_info.bl_dev to the correct backlight
> device. Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: "Bruno Prémont" 
> ---

[...]

> +#ifdef CONFIG_HID_PICOLCD_BACKLIGHT
> + info->bl_dev = data->backlight;
> +#endif
> +

The robot complains about this, I think that you also need to guard
against CONFIG_FB_BACKLIGHT being defined. Alternatively, you could
include a preparatory patch to fix the HID_PICOLCD_BACKLIGHT config
symbol dependencies.

Other than that,

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat



Re: [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb

2024-02-15 Thread Thomas Zimmermann

Hi

Am 15.02.24 um 13:06 schrieb Daniel Thompson:

On Mon, Feb 12, 2024 at 05:16:37PM +0100, Thomas Zimmermann wrote:

The driver sets struct fb_info.bl_dev to the correct backlight
device.

This looks like it was copied from a different patch since you
added code to do this as part of the patch!


Yeah, I did. I'll reword this sentence to make I more precise.

Best regards
Thomas




Thus rely on the backlight core code to match backlight
and framebuffer devices, and remove the extra check_fb function
from struct backlight_ops.

diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index d799d325e..4500f6e03d32f 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -493,6 +493,10 @@ int picolcd_init_framebuffer(struct picolcd_data *data)
info->fix = picolcdfb_fix;
info->fix.smem_len   = PICOLCDFB_SIZE*8;

+#ifdef CONFIG_HID_PICOLCD_BACKLIGHT
+   info->bl_dev = data->backlight;
+#endif
+
fbdata = info->par;
spin_lock_init(&fbdata->lock);
fbdata->picolcd = data;


Daniel.


--
--
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)



Re: [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb

2024-02-15 Thread Daniel Thompson
On Mon, Feb 12, 2024 at 05:16:37PM +0100, Thomas Zimmermann wrote:
> The driver sets struct fb_info.bl_dev to the correct backlight
> device.

This looks like it was copied from a different patch since you
added code to do this as part of the patch!

> Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
> 
> diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
> index d799d325e..4500f6e03d32f 100644
> --- a/drivers/hid/hid-picolcd_fb.c
> +++ b/drivers/hid/hid-picolcd_fb.c
> @@ -493,6 +493,10 @@ int picolcd_init_framebuffer(struct picolcd_data *data)
>   info->fix = picolcdfb_fix;
>   info->fix.smem_len   = PICOLCDFB_SIZE*8;
>
> +#ifdef CONFIG_HID_PICOLCD_BACKLIGHT
> + info->bl_dev = data->backlight;
> +#endif
> +
>   fbdata = info->par;
>   spin_lock_init(&fbdata->lock);
>   fbdata->picolcd = data;


Daniel.


Re: [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb

2024-02-13 Thread kernel test robot
Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on lee-backlight/for-backlight-next]
[also build test ERROR on lee-backlight/for-backlight-fixes hid/for-next 
lee-leds/for-leds-next linus/master v6.8-rc4 next-20240213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/backlight-Match-backlight-device-against-struct-fb_info-bl_dev/20240213-002853
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git 
for-backlight-next
patch link:
https://lore.kernel.org/r/20240212162645.5661-5-tzimmermann%40suse.de
patch subject: [PATCH 04/10] hid/hid-picolcd: Remove struct 
backlight_ops.check_fb
config: x86_64-rhel-8.3 
(https://download.01.org/0day-ci/archive/20240214/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240214/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

   drivers/hid/hid-picolcd_fb.c: In function 'picolcd_init_framebuffer':
>> drivers/hid/hid-picolcd_fb.c:497:13: error: 'struct fb_info' has no member 
>> named 'bl_dev'
 497 | info->bl_dev = data->backlight;
 | ^~


vim +497 drivers/hid/hid-picolcd_fb.c

   459  
   460  static DEVICE_ATTR(fb_update_rate, 0664, picolcd_fb_update_rate_show,
   461  picolcd_fb_update_rate_store);
   462  
   463  /* initialize Framebuffer device */
   464  int picolcd_init_framebuffer(struct picolcd_data *data)
   465  {
   466  struct device *dev = &data->hdev->dev;
   467  struct fb_info *info = NULL;
   468  struct picolcd_fb_data *fbdata = NULL;
   469  int i, error = -ENOMEM;
   470  u32 *palette;
   471  
   472  /* The extra memory is:
   473   * - 256*u32 for pseudo_palette
   474   * - struct fb_deferred_io
   475   */
   476  info = framebuffer_alloc(256 * sizeof(u32) +
   477  sizeof(struct fb_deferred_io) +
   478  sizeof(struct picolcd_fb_data) +
   479  PICOLCDFB_SIZE, dev);
   480  if (!info)
   481  goto err_nomem;
   482  
   483  info->fbdefio = info->par;
   484  *info->fbdefio = picolcd_fb_defio;
   485  info->par += sizeof(struct fb_deferred_io);
   486  palette = info->par;
   487  info->par += 256 * sizeof(u32);
   488  for (i = 0; i < 256; i++)
   489  palette[i] = i > 0 && i < 16 ? 0xff : 0;
   490  info->pseudo_palette = palette;
   491  info->fbops = &picolcdfb_ops;
   492  info->var = picolcdfb_var;
   493  info->fix = picolcdfb_fix;
   494  info->fix.smem_len   = PICOLCDFB_SIZE*8;
   495  
   496  #ifdef CONFIG_HID_PICOLCD_BACKLIGHT
 > 497  info->bl_dev = data->backlight;
   498  #endif
   499  
   500  fbdata = info->par;
   501  spin_lock_init(&fbdata->lock);
   502  fbdata->picolcd = data;
   503  fbdata->update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT;
   504  fbdata->bpp = picolcdfb_var.bits_per_pixel;
   505  fbdata->force   = 1;
   506  fbdata->vbitmap = info->par + sizeof(struct picolcd_fb_data);
   507  fbdata->bitmap  = vmalloc(PICOLCDFB_SIZE*8);
   508  if (fbdata->bitmap == NULL) {
   509  dev_err(dev, "can't get a free page for framebuffer\n");
   510  goto err_nomem;
   511  }
   512  info->flags |= FBINFO_VIRTFB;
   513  info->screen_buffer = fbdata->bitmap;
   514  info->fix.smem_start = (unsigned long)fbdata->bitmap;
   515  memset(fbdata->vbitmap, 0xff, PICOLCDFB_SIZE);
   516  data->fb_info = info;
   517  
   518  error = picolcd_fb_reset(data, 1);
   519  if (error) {
   520  dev_err(dev, "failed to configure display\n");
   521  goto err_cleanup;
   522  }
   523  
   524  error = device_create_file(dev, &dev_attr_fb_update_rate);
   525  if (error) {
   526  dev_err(dev, "failed to create sysfs attributes\n");
   527

Re: [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb

2024-02-13 Thread kernel test robot
Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on lee-backlight/for-backlight-next]
[also build test ERROR on lee-backlight/for-backlight-fixes hid/for-next 
lee-leds/for-leds-next linus/master v6.8-rc4 next-20240213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/backlight-Match-backlight-device-against-struct-fb_info-bl_dev/20240213-002853
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git 
for-backlight-next
patch link:
https://lore.kernel.org/r/20240212162645.5661-5-tzimmermann%40suse.de
patch subject: [PATCH 04/10] hid/hid-picolcd: Remove struct 
backlight_ops.check_fb
config: x86_64-rhel-8.3-rust 
(https://download.01.org/0day-ci/archive/20240213/[email protected]/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 
6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240213/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/hid/hid-picolcd_fb.c:497:8: error: no member named 'bl_dev' in 
>> 'struct fb_info'
 497 | info->bl_dev = data->backlight;
 |   ^
   1 error generated.


vim +497 drivers/hid/hid-picolcd_fb.c

   459  
   460  static DEVICE_ATTR(fb_update_rate, 0664, picolcd_fb_update_rate_show,
   461  picolcd_fb_update_rate_store);
   462  
   463  /* initialize Framebuffer device */
   464  int picolcd_init_framebuffer(struct picolcd_data *data)
   465  {
   466  struct device *dev = &data->hdev->dev;
   467  struct fb_info *info = NULL;
   468  struct picolcd_fb_data *fbdata = NULL;
   469  int i, error = -ENOMEM;
   470  u32 *palette;
   471  
   472  /* The extra memory is:
   473   * - 256*u32 for pseudo_palette
   474   * - struct fb_deferred_io
   475   */
   476  info = framebuffer_alloc(256 * sizeof(u32) +
   477  sizeof(struct fb_deferred_io) +
   478  sizeof(struct picolcd_fb_data) +
   479  PICOLCDFB_SIZE, dev);
   480  if (!info)
   481  goto err_nomem;
   482  
   483  info->fbdefio = info->par;
   484  *info->fbdefio = picolcd_fb_defio;
   485  info->par += sizeof(struct fb_deferred_io);
   486  palette = info->par;
   487  info->par += 256 * sizeof(u32);
   488  for (i = 0; i < 256; i++)
   489  palette[i] = i > 0 && i < 16 ? 0xff : 0;
   490  info->pseudo_palette = palette;
   491  info->fbops = &picolcdfb_ops;
   492  info->var = picolcdfb_var;
   493  info->fix = picolcdfb_fix;
   494  info->fix.smem_len   = PICOLCDFB_SIZE*8;
   495  
   496  #ifdef CONFIG_HID_PICOLCD_BACKLIGHT
 > 497  info->bl_dev = data->backlight;
   498  #endif
   499  
   500  fbdata = info->par;
   501  spin_lock_init(&fbdata->lock);
   502  fbdata->picolcd = data;
   503  fbdata->update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT;
   504  fbdata->bpp = picolcdfb_var.bits_per_pixel;
   505  fbdata->force   = 1;
   506  fbdata->vbitmap = info->par + sizeof(struct picolcd_fb_data);
   507  fbdata->bitmap  = vmalloc(PICOLCDFB_SIZE*8);
   508  if (fbdata->bitmap == NULL) {
   509  dev_err(dev, "can't get a free page for framebuffer\n");
   510  goto err_nomem;
   511  }
   512  info->flags |= FBINFO_VIRTFB;
   513  info->screen_buffer = fbdata->bitmap;
   514  info->fix.smem_start = (unsigned long)fbdata->bitmap;
   515  memset(fbdata->vbitmap, 0xff, PICOLCDFB_SIZE);
   516  data->fb_info = info;
   517  
   518  error = picolcd_fb_reset(data, 1);
   519  if (error) {
   520  dev_err(dev, "failed to configure display\n");
   521  goto err_cleanup;
   522  }
   523  
   524  error = device_create_file(dev, &dev_attr_fb_update_rate);
   525  if (error) {
   526  dev_err(dev, "failed to create sysfs attributes\n");
   527 

[PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb

2024-02-12 Thread Thomas Zimmermann
The driver sets struct fb_info.bl_dev to the correct backlight
device. Thus rely on the backlight core code to match backlight
and framebuffer devices, and remove the extra check_fb function
from struct backlight_ops.

Signed-off-by: Thomas Zimmermann 
Cc: "Bruno Prémont" 
---
 drivers/hid/hid-picolcd_backlight.c | 7 ---
 drivers/hid/hid-picolcd_fb.c| 4 
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-picolcd_backlight.c 
b/drivers/hid/hid-picolcd_backlight.c
index 5bd2a8c4bbd66..08d16917eb60b 100644
--- a/drivers/hid/hid-picolcd_backlight.c
+++ b/drivers/hid/hid-picolcd_backlight.c
@@ -9,7 +9,6 @@
 
 #include 
 
-#include 
 #include 
 
 #include "hid-picolcd.h"
@@ -39,15 +38,9 @@ static int picolcd_set_brightness(struct backlight_device 
*bdev)
return 0;
 }
 
-static int picolcd_check_bl_fb(struct backlight_device *bdev, struct fb_info 
*fb)
-{
-   return fb && fb == picolcd_fbinfo((struct picolcd_data 
*)bl_get_data(bdev));
-}
-
 static const struct backlight_ops picolcd_blops = {
.update_status  = picolcd_set_brightness,
.get_brightness = picolcd_get_brightness,
-   .check_fb   = picolcd_check_bl_fb,
 };
 
 int picolcd_init_backlight(struct picolcd_data *data, struct hid_report 
*report)
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index d799d325e..4500f6e03d32f 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -493,6 +493,10 @@ int picolcd_init_framebuffer(struct picolcd_data *data)
info->fix = picolcdfb_fix;
info->fix.smem_len   = PICOLCDFB_SIZE*8;
 
+#ifdef CONFIG_HID_PICOLCD_BACKLIGHT
+   info->bl_dev = data->backlight;
+#endif
+
fbdata = info->par;
spin_lock_init(&fbdata->lock);
fbdata->picolcd = data;
-- 
2.43.0