From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (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 D0D5A371895 for ; Sat, 25 Jul 2026 11:33:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784979196; cv=none; b=NX5ZBXjopoSTDrK/wBz86W5eBcyyDlMEdyavDGM9rdZo/ppJzOBY2utxTbmaGY4vfPN1wxKd6YVIYgFoeBV/0Z8kKRkGt5cbLreqpUFKesf7wc8cC1sdTc0vatN1HYKmpR1o+6OYTOoQmSKJYROkgiRPJgZT+3ldhKVpwwOLhCs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784979196; c=relaxed/simple; bh=bjNsegpkQVYzEQDWLtmBk5ZxfdVigDusypTpo+aG2Ds=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=gAxOuMWvuprriETtKF4+LgTkf+EiS0CnZzo25/Fj/AtnneKI0vCnQ5BYlGXZ9nMSyVlDSxidf7l4MBugejD9huhvP7tzRcsOjYY2CsOGHRRUm5kzO4KJk4v5aEjLhLMSnxlsRbFFGp3J00W2OegrfGx0fANuh1jB78R9WabAU90= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=OAtOvrjx; arc=none smtp.client-ip=209.85.167.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="OAtOvrjx" Received: by mail-lf1-f52.google.com with SMTP id 2adb3069b0e04-5aeb906d6c6so158785e87.2 for ; Sat, 25 Jul 2026 04:33:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1784979193; x=1785583993; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from:references :cc:to:subject:user-agent:mime-version:date:message-id:from:to:cc :subject:date:message-id:reply-to:content-type; bh=kWWe3xhReWpC2nYwU2tMGGrj5mSNsd5np1dCXgjryBY=; b=OAtOvrjxpR88gga+FqNoPRzyKdACVjo1q4xZTkbS5kT6ui5fvO8bwTrKb68ugEOt52 r7lprTNHOVWVNWUeuD+e9YHDkbFYjb7lji4GsuS0h8Eb8+oBA2q7ptEAdwovorsClY4P 205uyRpL1RXPLuaidptd1wWQ8QJn/tMDWqpgJkPp3DfR7QChmNT01bgXrvGo6ITVeFS7 +z4Usxw4YRasuA2T3OeM4RySr90h9tt+zWLBKK982WTHDoz55vbw9ZYk4FTTrb2b29Wz lZzpKpxXkNEo2mfVRcVNTDvnALrHRSsL6mcJcBaY/C1AIoCHY477gtgqZLKi/BSLc/6K qNVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784979193; x=1785583993; h=content-transfer-encoding:content-type:in-reply-to:from: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 :content-type; bh=kWWe3xhReWpC2nYwU2tMGGrj5mSNsd5np1dCXgjryBY=; b=NoyrgBpeDz8wm8TCMtmcXJrUbuLyn7H/yRdxgq3KijofZN7B2YiAj8vpeSQvsGTLKk DxVb8K+peiX1IBTFQBJe7Ff6mHp+BS5CcD4ym1sUOHbL+smoQBhA7YqG5JYaM904Vm/S Fog0CUE5GKwN8GoRwjfifGUymZoHSLYOt9FnjYd50aR/UdkVlZy1IsEN5vpq/Cow8XE4 PDdwKHqzFtuM1vacVVkZV01/S+Cjkt0sWD/l4GVq8eJUqFVyAdzDKoBC+BXar4mS8PJI cTMwFU2rjyyYlBOtbeoINLO+Q6XpxQQigsAYdq95J03UVCmhCUN9hDtQ3K+UxQQ0g53n JNZw== X-Forwarded-Encrypted: i=1; AHgh+RofJtOCiiNdtQuu2z9KDlbS1TMM0bQ5SiV1aKtCn/DfUOrsCCMhCPCv65J3UHdn3A4sMNYYUq7Sbel8HiU=@vger.kernel.org X-Gm-Message-State: AOJu0YwTJqQ4fPiTscfDttCM7XtUXfYy0QqaVAdvJwW1UevkauW3dF32 JbkqCIW0ds5i03+G/5CxB7wcF/A0aBBWyC1PzKblXsCYcdYNak1bUJ9jBen/76K+ZI0= X-Gm-Gg: AR+sD12VbEKlIZ38UZ86GHfClsYsJpXnWJnAythz6se+lEOItgMxIWAqqUg7NKnwQzi NQ7nISwVQ/SWv1F9w7E/lJWc+1ZJ9cf7NZzH6L7RdaPUHkvi1dB85lI/J235VNEMx5lvBRsdni9 i1qpYRGFGOyiFIp6yzPg90pWduxZpT2fBG9Y8fWmyZYMgjy4iK0mjuAab8AQGaTwKxAs3uNizRn 4OZRnWiTlyTLxUpCoNay2JVLhI/5MoNzXRJH7cOeGe5ytI4KCHKpwvZQGGf3VYtzHBYMeLkc9wk /x6WUmmO7IBh5JdSjOrx+OQqM6E01IepDeqSvd/Q6BCcklCc9poqsrkp7amOvnh/olbgj0GH5Qz wGzTTuHVyqLclyIgioOh91udC/23BdncbFpYxn+LjreKngQeBs4UGzqLr/8XCaDQmDH0a4hPam/ BrL6X7grBeJonqXlwNoU1d6atymQAxT8EZG4ddfPQHs5nGBykMAB0MfCD7 X-Received: by 2002:ac2:51d0:0:b0:5b1:5b49:9463 with SMTP id 2adb3069b0e04-5b2c1b3d56fmr323900e87.4.1784979192811; Sat, 25 Jul 2026 04:33:12 -0700 (PDT) Received: from [192.168.1.100] (91-159-24-186.elisa-laajakaista.fi. [91.159.24.186]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5b2be0881d3sm410737e87.28.2026.07.25.04.33.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 25 Jul 2026 04:33:12 -0700 (PDT) Message-ID: <141c9f00-6fe9-4003-af7e-182cebcce22e@linaro.org> Date: Sat, 25 Jul 2026 14:33:11 +0300 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 v3 17/17] media: i2c: os05b10: remove unused control fields, simplify error handling To: Tarang Raval , sakari.ailus@linux.intel.com, mehdi.djait@linux.intel.com Cc: Himanshu Bhavani , Elgin Perumbilly , Mauro Carvalho Chehab , Hans Verkuil , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org References: <20260718200912.16001-1-tarang.raval@siliconsignals.io> <20260718200912.16001-18-tarang.raval@siliconsignals.io> From: Vladimir Zapolskiy In-Reply-To: <20260718200912.16001-18-tarang.raval@siliconsignals.io> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/18/26 23:09, Tarang Raval wrote: > link_freq and gain don't need to be stored in struct os05b10; make > them local. > > Parse the fwnode properties up front and register them before the > single ctrl_hdlr->error check, so all controls are covered by one > check and the per-control NULL guards before setting flags can be > dropped. > > Signed-off-by: Tarang Raval > --- > drivers/media/i2c/os05b10.c | 55 +++++++++++++++++-------------------- > 1 file changed, 25 insertions(+), 30 deletions(-) > > diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c > index b4dd3137ad5c..c355ac197eb2 100644 > --- a/drivers/media/i2c/os05b10.c > +++ b/drivers/media/i2c/os05b10.c > @@ -557,11 +557,9 @@ struct os05b10 { > > /* V4L2 Controls */ > struct v4l2_ctrl_handler handler; > - struct v4l2_ctrl *link_freq; > struct v4l2_ctrl *pixel_rate; > struct v4l2_ctrl *hblank; > struct v4l2_ctrl *vblank; > - struct v4l2_ctrl *gain; > struct v4l2_ctrl *exposure; > struct v4l2_ctrl *vflip; > struct v4l2_ctrl *hflip; > @@ -1276,9 +1274,14 @@ static int os05b10_init_controls(struct os05b10 *os05b10) > struct v4l2_fwnode_device_properties props; > struct v4l2_ctrl_handler *ctrl_hdlr; > u64 vblank_def, exp_max, pixel_rate; > + struct v4l2_ctrl *link_freq; > s64 hblank_def; > int ret; > > + ret = v4l2_fwnode_device_parse(os05b10->dev, &props); > + if (ret) > + return ret; > + > ctrl_hdlr = &os05b10->handler; > v4l2_ctrl_handler_init(ctrl_hdlr, 12); > > @@ -1287,22 +1290,18 @@ static int os05b10_init_controls(struct os05b10 *os05b10) > V4L2_CID_PIXEL_RATE, pixel_rate, > pixel_rate, 1, pixel_rate); > > - os05b10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, &os05b10_ctrl_ops, > - V4L2_CID_LINK_FREQ, > - ARRAY_SIZE(link_frequencies_4lane) - 1, > - os05b10->link_freq_index, > - (os05b10->data_lanes == 2) ? > - link_frequencies_2lane : > - link_frequencies_4lane); > - if (os05b10->link_freq) > - os05b10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, &os05b10_ctrl_ops, > + V4L2_CID_LINK_FREQ, > + ARRAY_SIZE(link_frequencies_4lane) - 1, > + os05b10->link_freq_index, > + (os05b10->data_lanes == 2) ? > + link_frequencies_2lane : > + link_frequencies_4lane); > > hblank_def = (s64)mode->hts -(s64)mode->width; > os05b10->hblank = v4l2_ctrl_new_std(ctrl_hdlr, NULL, V4L2_CID_HBLANK, > hblank_def, hblank_def, > 1, hblank_def); > - if (os05b10->hblank) > - os05b10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > vblank_def = mode->vts - mode->height; > os05b10->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, > @@ -1317,12 +1316,10 @@ static int os05b10_init_controls(struct os05b10 *os05b10) > exp_max, OS05B10_EXPOSURE_STEP, > mode->exp); > > - os05b10->gain = v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, > - V4L2_CID_ANALOGUE_GAIN, > - OS05B10_ANALOG_GAIN_MIN, > - OS05B10_ANALOG_GAIN_MAX, > - OS05B10_ANALOG_GAIN_STEP, > - OS05B10_ANALOG_GAIN_DEFAULT); > + v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, > + V4L2_CID_ANALOGUE_GAIN, OS05B10_ANALOG_GAIN_MIN, > + OS05B10_ANALOG_GAIN_MAX, OS05B10_ANALOG_GAIN_STEP, > + OS05B10_ANALOG_GAIN_DEFAULT); > > v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, V4L2_CID_DIGITAL_GAIN, > OS05B10_DIGITAL_GAIN_MIN, OS05B10_DIGITAL_GAIN_MAX, > @@ -1330,33 +1327,31 @@ static int os05b10_init_controls(struct os05b10 *os05b10) > > os05b10->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, > V4L2_CID_HFLIP, 0, 1, 1, 0); > - if (os05b10->hflip) > - os05b10->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > os05b10->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &os05b10_ctrl_ops, > V4L2_CID_VFLIP, 0, 1, 1, 0); > - if (os05b10->vflip) > - os05b10->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &os05b10_ctrl_ops, > V4L2_CID_TEST_PATTERN, > ARRAY_SIZE(os05b10_test_pattern_menu) - 1, > 0, 0, os05b10_test_pattern_menu); > > + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &os05b10_ctrl_ops, > + &props); > + if (ret) > + goto error; > + > if (ctrl_hdlr->error) { > ret = ctrl_hdlr->error; > dev_err(os05b10->dev, "control init failed (%d)\n", ret); > goto error; > } > > - ret = v4l2_fwnode_device_parse(os05b10->dev, &props); > - if (ret) > - goto error; > > - ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &os05b10_ctrl_ops, > - &props); > - if (ret) > - goto error; > + link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + os05b10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + os05b10->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > + os05b10->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > os05b10->sd.ctrl_handler = ctrl_hdlr; > Reviewed-by: Vladimir Zapolskiy I would suggest to reorganize and reorder changes in the changeset, moving simple changes like this one to the very top. Also it might be possible to merge a number of trivial changes earlier, this will save time and efforts. -- Best wishes, Vladimir