From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from layka.disroot.org (layka.disroot.org [178.21.23.139]) (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 1A63E33AD9A; Fri, 8 May 2026 17:27:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=178.21.23.139 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778261257; cv=none; b=XOA5K1cwqvAUrZz8aCSs3uU6OrYE0TygqfxrZgDo7uePokPzfBKRD1gI3V0fa+0FinSu2OC5sNmONwAecC+9YmU40LcPqm49Fp28QIwyQrHvyoxYJUXrLFGSaOUI0TRNU6LZaYiZoSLEgK2jXwH+gRx3knMp8MMy66KHBKvjb5k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778261257; c=relaxed/simple; bh=GVadrPaIpXvloCZ2qd0TqpTcfl5gklkUuZCeS18TOZ8=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=TuQlVjHFjLSV3OtU9nXZHwpcr1BKC7/afpf9k8hs6gbBBnXIf12+O0gL1BPXVptcKtHZJVZIERFywElaPCFaQJ/lqk53H1+QZcIk4/88nXKYcFSI224orhKbjNNkTBoclKnsJHUsO2cLTrcT58MF7si2Ls2cstynro1H18bdFU0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=disroot.org; spf=pass smtp.mailfrom=disroot.org; dkim=pass (2048-bit key) header.d=disroot.org header.i=@disroot.org header.b=FoabeDNM; arc=none smtp.client-ip=178.21.23.139 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=disroot.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=disroot.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=disroot.org header.i=@disroot.org header.b="FoabeDNM" Received: from mail01.disroot.lan (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id E957027342; Fri, 8 May 2026 19:27:31 +0200 (CEST) X-Virus-Scanned: SPAM Filter at disroot.org Received: from layka.disroot.org ([127.0.0.1]) by localhost (disroot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id Mz1FdCaqy_UM; Fri, 8 May 2026 19:27:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1778261251; bh=GVadrPaIpXvloCZ2qd0TqpTcfl5gklkUuZCeS18TOZ8=; h=Date:Cc:Subject:From:To:References:In-Reply-To; b=FoabeDNM7Jyz+x0UDJK6XcQU51z7Kdd5xB/++yvjt5c/PUyu4oQ9/i/J5rLglEHBS dUWSemd3ufA1LqftJjHvOJNQeLILdkgigpTYXJ2yrYSK8CzS8Zzi9ToGYHbDyHqm9B Gk9TQM/GcPz1LwiGOY6WlKmC6O1dIYOr1g5tuKQYVKrhB1o8MFyt+g2p+0+wKo/ycy biOp8HFVlb6UorCJfanOu/hhaTi4DerBImFFRf22ONmJaaaQMFuBHxnbpRYLC7RhS+ gCMGPYdj1nFOCgVNbRCX53LCTLIT52dQTXuQY8vrGE03YQSXwdKkpFADQXMVlAnouL 6p6aVHxrv7Mfg== Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 08 May 2026 22:57:12 +0530 Message-Id: Cc: "Pavel Machek" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "MyungJoo Ham" , "Chanwoo Choi" , "Sebastian Reichel" , "Krzysztof Kozlowski" , =?utf-8?q?Andr=C3=A9_Draszik?= , "Alexandre Belloni" , "Jonathan Corbet" , "Shuah Khan" , "Nam Tran" , =?utf-8?q?=C5=81ukasz_Lebiedzi=C5=84ski?= , , , , , , , Subject: Re: [PATCH v5 08/11] leds: rgb: add support for Samsung S2M series PMIC RGB LED device From: "Kaustabh Chakraborty" To: "Lee Jones" , "Kaustabh Chakraborty" References: <20260424-s2mu005-pmic-v5-0-fcbc9da5a004@disroot.org> <20260424-s2mu005-pmic-v5-8-fcbc9da5a004@disroot.org> <20260507190005.GT305027@google.com> In-Reply-To: <20260507190005.GT305027@google.com> On 2026-05-07 20:00 +01:00, Lee Jones wrote: > On Fri, 24 Apr 2026, Kaustabh Chakraborty wrote: [...] >> + >> + switch (rgb->device_type) { >> + case S2MU005: >> + lut_ramp_up =3D s2mu005_rgb_lut_ramp; >> + lut_ramp_up_len =3D ARRAY_SIZE(s2mu005_rgb_lut_ramp); >> + lut_ramp_dn =3D s2mu005_rgb_lut_ramp; >> + lut_ramp_dn_len =3D ARRAY_SIZE(s2mu005_rgb_lut_ramp); >> + lut_stay_hi =3D s2mu005_rgb_lut_stay_hi; >> + lut_stay_hi_len =3D ARRAY_SIZE(s2mu005_rgb_lut_stay_hi); >> + lut_stay_lo =3D s2mu005_rgb_lut_stay_lo; >> + lut_stay_lo_len =3D ARRAY_SIZE(s2mu005_rgb_lut_stay_lo); >> + break; >> + default: >> + /* execution shouldn't reach here */ > > Instead of a comment, perhaps a WARN_ON_ONCE(1); or similar would be > more robust here to catch unexpected device types? > [...] >> +static int s2m_rgb_pattern_clear(struct led_classdev *cdev) >> +{ >> + struct s2m_rgb *rgb =3D to_s2m_rgb(to_s2m_mc(cdev)); >> + int ret =3D 0; >> + >> + mutex_lock(&rgb->lock); >> + >> + switch (rgb->device_type) { >> + case S2MU005: >> + ret =3D s2mu005_rgb_reset_params(rgb); >> + break; >> + default: >> + /* execution shouldn't reach here */ >> + break; > > As above. > > And a single branch switch () makes little sense. Even with an `if`, since only one variant is supported we're sure that the control would never go to `else` anyway. I will flatten this block, and expect the switch to be added when another variant is added. >> +static struct mc_subled s2mu005_rgb_subled_info[] =3D { > > const? No, this is fed to (struct led_classdev_mc)::subled_info, which is not a const pointer. Relevant snip is marked below. "Assigning to 'struct mc_subled *' from const struct mc_subled[3] discards qualifiers." >> + { .channel =3D 0, .color_index =3D LED_COLOR_ID_BLUE }, >> + { .channel =3D 1, .color_index =3D LED_COLOR_ID_GREEN }, >> + { .channel =3D 2, .color_index =3D LED_COLOR_ID_RED }, >> +}; [...] >> + switch (rgb->device_type) { >> + case S2MU005: >> + rgb->mc.subled_info =3D s2mu005_rgb_subled_info; Here. >> + rgb->mc.num_colors =3D ARRAY_SIZE(s2mu005_rgb_subled_info); >> + break; >> + default: >> + return dev_err_probe(dev, -ENODEV, "device type %d is not supported b= y driver\n", >> + pmic_drvdata->device_type);