From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ua1-f49.google.com (mail-ua1-f49.google.com [209.85.222.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4474A27FD74 for ; Wed, 19 Nov 2025 13:57:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763560646; cv=none; b=hiSmFai8FGl1lGA5jYNATDFAoN52TV7s/zyIjoYaBBGJW4VgTd+eQHyWQeTOfT60dLcM/AnS4krDBB5/g23rrKtrew9mlTcb3fx+Rz5a2UlDTx2DSCAThyyPRxdvNsEOAjvxg6bVFN7l7/mB7IhhKz0yOIFDTDLdcDC62bmRV7Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763560646; c=relaxed/simple; bh=t95h38urR4hl2sxtnta6IJGy18VlxrnggA0cIH6J4oM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=P744xwHPERGS7GkbN63EFbnMBLDdnO/hwpeMvl+aIWjS3EynbsB7Ldy1cyu8it4cgllgnt5wqcNxE8QiuslZ6Sflj9Gyvt4wthMW7X/vM6uxRXbOAX7446AcvwQd58/e/2iqLvEzn5B7+5pTnnyt0MKvmrq+6wjBwAOP0Zp3Be8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=B4tuqFEF; arc=none smtp.client-ip=209.85.222.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="B4tuqFEF" Received: by mail-ua1-f49.google.com with SMTP id a1e0cc1a2514c-93516cbe2bbso1834141241.2 for ; Wed, 19 Nov 2025 05:57:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763560644; x=1764165444; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=de5iNSvcqaaOwtf/lxZWP8NugFiYs5OOT1HDIPeVtSo=; b=B4tuqFEFHnE1mhQwgZKhstQvJ9jV7vzNPczEOQ0HLynwoVm53wZhNd+sKrTPMIYC5t 91ZEZbqhbjVRb0NNkkU1GknV2EHUdEwJ9fvqscHEg6YaiXA1BHEFn/srOvl8EYQkYiFn CF/A3hD9Wn/YCz5nxm6PHF/23FIDX30qds/DP0WQ6rRyFQByR7+Bt0qerwxRoMDu0WeR nKHliUkxljE0JnjM1Gqh4XyoaT1eN13IZ22YXeBrwHl1LXLpDWvITtHYoVzghbS2Oo1B PzWWav7RNYW8XWqzLBATfiNFGATCYFp7tq+Mu6E4r3/yGizvAbng2jwC88rLZWHVk2oY a1hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763560644; x=1764165444; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=de5iNSvcqaaOwtf/lxZWP8NugFiYs5OOT1HDIPeVtSo=; b=cTmB1Mx6xSuywu4lv5/VEsFMXM4ivZEdq2gC69RNgAGa+gQ3fmwrb7Lp+cfI+tFQWg suEo9y14ju146xD2ChGGBSsXHxin12fpTbonnezWXmaa4fWOyFRdkVVRLNBTdHfgZjYq xk6Q2ggayx0wayaYGNCKL0WK6BCQZrSVor140D9vfhBiJ9I7fFMtHVrYeToEAoMwx3WA mrq61IEE9eDNeQsXMMXD6LPBDd3p3htoHobXEGgpeu7TSZ74KzBfTj6DJfrZQLjMZyw+ rHuRU31cQJ0AFChmB249icckttcYowqeXv7DX/m/raNtw691TaincfWEQZdO68H39AYp 2K3g== X-Forwarded-Encrypted: i=1; AJvYcCWjb6C0BRQpYvKhS6jPuDmrXIu6sTCf/cFL/9RcAFMs9zCF8Xp/r9YviygSqSJhvLfOtZFDDjwaEdsaldk=@vger.kernel.org X-Gm-Message-State: AOJu0YzFOFI0LbPBNXHgxG06gcfEjDc62nIuGhX6jPmCKftuLlXUwrP+ dBukXeAoGRXf2p3JK6wCFApik6ppP7C2xFpW8kqaFZhEIlyFwEK+oZWu X-Gm-Gg: ASbGncvIy45JuThrmmbxTUwRh8wZP+D7pYF3YJeq+WbTeMYAQij2UIo1OnRrSykDIO7 c3fZEPeamS0K1Ak12QOWPUVzBo50lQMEHvMIhjTjBD5fdumcjxbzepYdtfXzNn7O9JaBNpMZL3h vuAktO+BaI+Gzfhcs5IAYQpRW8jcMpJF3wAsj+lvUS6484EDg9iKwXlXsRUGNj82HfuEdsoAip8 8WGMqTrankYjRMMMjxrAMm2oqKx/soqhGGU8CYVmXxl8o+YCEGLlJf7g+pv5RuoUU0KD0Xx9V7x b0KRMUx82GmBAYyPvAk96PIPCuJKdrqzKhfnftu9XAv5JJgg16QBT4ov/TjvL7PRoaADsKC3gU6 nWFvl6AY3plsjaApuPU68x4Jd4UJ0q/Sh8MXiE0U5A7O4qpe7khgF0W67T8T11wiF9h1Eqs213s nNeA74K5fJodGgzpF+A3suBIT9DP8i4W3Aifpmgw== X-Google-Smtp-Source: AGHT+IGa7rzG95o6W6cj6AePLxJr9+GNclEXHGzK7/ZUIMQGaEM6Y41RRqYBi1xyySb7CTzboZJPMg== X-Received: by 2002:a05:6102:2906:b0:5d7:de08:dcd6 with SMTP id ada2fe7eead31-5dfc54eb53dmr6337612137.2.1763560644030; Wed, 19 Nov 2025 05:57:24 -0800 (PST) Received: from [192.168.1.145] ([104.203.11.126]) by smtp.gmail.com with ESMTPSA id ada2fe7eead31-5dfb726ff96sm6675438137.14.2025.11.19.05.57.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Nov 2025 05:57:23 -0800 (PST) Message-ID: <4ec784a5-0f67-4fd3-9d51-d89a9fa9a385@gmail.com> Date: Wed, 19 Nov 2025 08:57:21 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] fbdev: q40fb: request memory region To: Sukrut Heroorkar , Helge Deller , "open list:FRAMEBUFFER LAYER" , "open list:FRAMEBUFFER LAYER" , open list Cc: shuah@kernel.org, david.hunter.linux@gmail.com References: <20251118095700.393474-1-hsukrut3@gmail.com> Content-Language: en-US From: David Hunter In-Reply-To: <20251118095700.393474-1-hsukrut3@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 11/18/25 04:56, Sukrut Heroorkar wrote: > The q40fb driver uses a fixed physical address but never reserves > the corresponding I/O region. Reserve the range as suggested in > Documentation/gpu/todo.rst ("Request memory regions in all fbdev drivers"). > > No functional change beyond claming the resource. This change is compile > tested only. Reserving memory is a significant "functional" change, so you should not put "No functional change...". I have noticed that in the mentorship program, mentees might say this often times when they have not done testing. Thank you for describing that you did a compile test, but I believe that more testing should be done before this patch is accepted. As a result, if you are unable to test this device, I believe that an RFT tag should be used. Also, the testing information goes below the "---". This puts it in the change log and would make it so that if a patch is accepted, everything below the change log is not put in the commit message. > > Signed-off-by: Sukrut Heroorkar > --- > drivers/video/fbdev/q40fb.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/video/fbdev/q40fb.c b/drivers/video/fbdev/q40fb.c > index 1ff8fa176124..935260326c6f 100644 > --- a/drivers/video/fbdev/q40fb.c > +++ b/drivers/video/fbdev/q40fb.c > @@ -101,6 +101,12 @@ static int q40fb_probe(struct platform_device *dev) > info->par = NULL; > info->screen_base = (char *) q40fb_fix.smem_start; > > + if (!request_mem_region(q40fb_fix.smem_start, q40fb_fix.smem_len, > + "q40fb")) { > + dev_err(&dev->dev, "cannot reserve video memory at 0x%lx\n", > + q40fb_fix.smem_start); > + } > + Is this correct? It seems to me that in the case of an error, all you are doing is simply logging the error and proceeding. Would this cause the device to continue to try to use space that it was not able to reserve? I do not have experience with this device or the driver, but that does not seem correct to me. > if (fb_alloc_cmap(&info->cmap, 256, 0) < 0) { > framebuffer_release(info); > return -ENOMEM; > @@ -144,6 +150,7 @@ static int __init q40fb_init(void) > if (ret) > platform_driver_unregister(&q40fb_driver); > } > + > return ret; > } >