Linux kernel staging patches
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Chhabilal Dangal <yogeshdangal66@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>,
	linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev,
	devel@driverdev.osuosl.org
Subject: Re: [PATCH] staging: sm750fb: Add suspend checks to copyarea and imageblit
Date: Thu, 14 May 2026 15:18:19 +0300	[thread overview]
Message-ID: <agW9i3X6HOL4Zt5n@stanley.mountain> (raw)
In-Reply-To: <20260514120102.71615-1-yogeshdangal66@gmail.com>

On Thu, May 14, 2026 at 05:46:02PM +0545, Chhabilal Dangal wrote:


> The suspend path calls fb_set_suspend(), which sets the framebuffer
> state to FBINFO_STATE_SUSPENDED. Add matching state checks in the
> remaining accelerated callbacks for consistency with fillrect().

This is stating obvious but unrelated things as if it means something.
It feels like information but it's a waste of time.  fillrect() does
not mirror fb_set_suspend().  This whole paragraph is garbage.

> lynxfb_ops_fillrect() already checks info->state before accessing
> the hardware 2D engine, but lynxfb_ops_copyarea() and
> lynxfb_ops_imageblit() do not.
> 

This paragraph is useful because it gives the motivation. But last
time you hadn't checked that the AI was correct.  You're just leaving
it up to us to check.  Why is your time more valuable than our time
that you can't be bothered to check this stuff?

We need to know:
1) Is the AI correct this time?
2) What is the implication of the bug.

Please do this research on your own, don't just enter my questions into
an AI.

3) Wouldn't it be better to do it at a lower level?

Smatch says that out of the callers only fbcon_bmove_rec(), ypan_up()
and ypan_down() don't check.  So check those to see if this is a
real bug.

drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |      fbcon_bmove_rec | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |    fbcon_redraw_blit | (struct fbcon_bitops)->bmove |        PARAM_VALUE |  1 |     info->state | 0
drivers/video/fbdev/core/fbcon.c |              ypan_up | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)
drivers/video/fbdev/core/fbcon.c |            ypan_down | (struct fbcon_bitops)->bmove |           INTERNAL | -1 |                 | void(*)(struct vc_data*, struct fb_info*, int, int, int, int, int, int)

> 
> Assisted-by: Claude Opus 4.6 (Anthropic LLM)
> Compile-tested only.

Put this under the --- cut off line.

> 
> Signed-off-by: Chhabilal Dangal <yogeshdangal66@gmail.com>
> ---
  ^^^
Here.

No Fixes tag.
Please wait a day between resends.

regards,
dan carpenter


  reply	other threads:[~2026-05-14 12:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-14 12:01 [PATCH] staging: sm750fb: Add suspend checks to copyarea and imageblit Chhabilal Dangal
2026-05-14 12:18 ` Dan Carpenter [this message]
2026-05-14 12:41 ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agW9i3X6HOL4Zt5n@stanley.mountain \
    --to=error27@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=sudip.mukherjee@codethink.co.uk \
    --cc=yogeshdangal66@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox