From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1EF63C19F2C for ; Wed, 27 Jul 2022 07:37:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230354AbiG0Hhk (ORCPT ); Wed, 27 Jul 2022 03:37:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40460 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229680AbiG0Hhh (ORCPT ); Wed, 27 Jul 2022 03:37:37 -0400 Received: from mail-qt1-x831.google.com (mail-qt1-x831.google.com [IPv6:2607:f8b0:4864:20::831]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D27D63B950; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) Received: by mail-qt1-x831.google.com with SMTP id y9so12106192qtv.5; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7wBtopi5nrrnF3/tK6d3mUXKRzW2T+DIHWRi/Z/g55U=; b=JrXv07SnypKXIiO9c1hDaO1ve2XFoCQh3YJA9N0xzJ8VKdcy+NOWbVByEfE93yA1bT Tl79vU21bY1Riyh5vdmQjiXNRM6Y+lisVzjlFGA12N5TkfsJVvBTZVM+DXhLMCRLgtAT CWazI0B5gaYgTPfT/LBhBjVu7eM5hrdl/RUhg4FDmsUWR19oAbFy6HoE9edFHPyenD8T Qf2WPKI/e5SPnIeUMPrpopEK3VVMRKkcz81I6LE01vsC+va5BFGobzRlWIltgg+x4gCQ EHVJv70L2zauSaiZEF5yiD6RME26xNIBHXc93G+wfeZxZyIdgW4ZKl6QFkorqrFDfTKD 6LhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7wBtopi5nrrnF3/tK6d3mUXKRzW2T+DIHWRi/Z/g55U=; b=6Jat2MyN2X/bCcy4KChsFEgToktjw+TM2nIp1GsWV6ZzmD6Rvuyrj1WVCZLEay7Yy8 q74Yij3pzdIaz/b3Dx1814Y7F7SZ4G/HV33u1NUwo1HFCib9KKUGzmTEDTFzeUjfU0Bs QX31Xn8Ix8XeXydaf8AcdSF3TvwfwPs2BaLCmimOEG8poqkdfChiJgGU3sbSMzaEOAPp tjrZy4no79UXKPkp/eBai9E81Hw5Bzcf2HNvuOYOYl0cPXcPSChnTM9IDK5Jn7McZXoP xHO/bCA9YNSGqGzehOVJ5hVsFJnBC1xruUQWUOhNGDVuRflhxLBmPlRaFHVf97VUhQHe GsuA== X-Gm-Message-State: AJIora+ycXd0R3vO91GJKaKDVU9HJPaB0TnRR6u89peGwkCT5WgXMS1m XRVpcxeT3Rf2n10XEOj8napxn7O9AWFngkOo0UeP0b2UjH0= X-Google-Smtp-Source: AGRyM1sXpKQZrYG0IbOgoxNKanSErlzr5LcY9lrHn4FGhRFmisG96yuJDLZQZGtegCOB6F8ydUZqqM5XWqwFIZtxfFQ= X-Received: by 2002:a05:622a:6098:b0:2f0:f0d2:b5f0 with SMTP id hf24-20020a05622a609800b002f0f0d2b5f0mr17361865qtb.583.1658907456035; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) MIME-Version: 1.0 References: <20220722102407.2205-1-peterwu.pub@gmail.com> <20220722102407.2205-12-peterwu.pub@gmail.com> In-Reply-To: From: ChiaEn Wu Date: Wed, 27 Jul 2022 15:36:59 +0800 Message-ID: Subject: Re: [PATCH v6 11/13] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support To: Andy Shevchenko Cc: Lee Jones , Daniel Thompson , Jingoo Han , Pavel Machek , Rob Herring , Krzysztof Kozlowski , Matthias Brugger , Sebastian Reichel , Chunfeng Yun , Greg Kroah-Hartman , Jonathan Cameron , Lars-Peter Clausen , Liam Girdwood , Mark Brown , Guenter Roeck , "Krogerus, Heikki" , Helge Deller , ChiaEn Wu , Alice Chen , cy_huang , dri-devel , Linux LED Subsystem , devicetree , linux-arm Mailing List , "moderated list:ARM/Mediatek SoC support" , Linux Kernel Mailing List , Linux PM , USB , linux-iio , "open list:FRAMEBUFFER LAYER" , szuni chen Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-leds@vger.kernel.org On Tue, Jul 26, 2022 at 8:18 PM Andy Shevchenko wrote: ... > > Just for saving memory space. > > Because these led_classdevs do not be used at the same time. > > Or do you think it would be better to rewrite it as follows? > > ------------------------------------------------------------------------------------- > > struct mt6370_led { > > struct led_classdev isink; > > struct led_classdev_mc mc; > > struct mt6370_priv *priv; > > u32 default_state; > > u32 index; > > }; > > ------------------------------------------------------------------------------------- > > You obviously didn't get what I'm talking about... > Each union to work properly should have an associated variable that > holds the information of which field of the union is in use. Do you > have such a variable? If not, how does your code know which one to > use? If yes, add a proper comment there. > Ummm... from my understanding, if the colors of these four LEDs are set to 'LED_COLOR_ID_RGB' or 'LED_COLOR_ID_MULTI' in DT, their 'led->index' will be set to 'MT6370_VIRTUAL_MULTICOLOR' in 'mt6370_leds_probe()'. If so, these led devices will be set as 'struct led_classdev_mc' and use related ops functions in 'mt6370_init_led_properties()'. Instead, they whose 'led->index' is not 'MT6370_VIRTUAL_MULTICOLOR' will be set as 'struct led_classdev'. So, maybe the member 'index' of the 'struct mt6370_led' is what you describe the information of which field of the union is in use? I will add the proper comment here to describe this thing. I'm so sorry for misunderstanding your mean last time. Thanks again for your review. -- Best Regards, ChiaEn Wu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 53A63C04A68 for ; Wed, 27 Jul 2022 07:37:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D2B0C1E8D; Wed, 27 Jul 2022 07:37:38 +0000 (UTC) Received: from mail-qt1-x82e.google.com (mail-qt1-x82e.google.com [IPv6:2607:f8b0:4864:20::82e]) by gabe.freedesktop.org (Postfix) with ESMTPS id D7D80C1E8D for ; Wed, 27 Jul 2022 07:37:36 +0000 (UTC) Received: by mail-qt1-x82e.google.com with SMTP id b21so12073663qte.12 for ; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7wBtopi5nrrnF3/tK6d3mUXKRzW2T+DIHWRi/Z/g55U=; b=JrXv07SnypKXIiO9c1hDaO1ve2XFoCQh3YJA9N0xzJ8VKdcy+NOWbVByEfE93yA1bT Tl79vU21bY1Riyh5vdmQjiXNRM6Y+lisVzjlFGA12N5TkfsJVvBTZVM+DXhLMCRLgtAT CWazI0B5gaYgTPfT/LBhBjVu7eM5hrdl/RUhg4FDmsUWR19oAbFy6HoE9edFHPyenD8T Qf2WPKI/e5SPnIeUMPrpopEK3VVMRKkcz81I6LE01vsC+va5BFGobzRlWIltgg+x4gCQ EHVJv70L2zauSaiZEF5yiD6RME26xNIBHXc93G+wfeZxZyIdgW4ZKl6QFkorqrFDfTKD 6LhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7wBtopi5nrrnF3/tK6d3mUXKRzW2T+DIHWRi/Z/g55U=; b=JZXzbsyb5AVqhOg6L+EdqAv2gbveZidrz4DOECV76ip9bJtEBaX7emmAkNBj/avHul 9hh7DIHQf1luWifA2hNgp1Kx2Eju4cPvc68oSGktXQwzR/GiO19N01a+HnP+1hjEuvFG h1Ds9g8NytSv5BcEsWOuTPChrfHdDK+Yw/7lAmFoPq5uBMmEL7AtPKWQnrsxMaMfppfH w3/KYHJlKJAbe4CTVYrNxcuZEX0HfXawd5fQSoBm3gH54nSa8qqYRtrEy2VX5T2yoBbH ONOqLY3B3cS5MEFrHw5/JD8OEvdxS1jGh6yGDuN9JM5npYkY44FMP+zfIuzaYw8Tm6x+ MAYw== X-Gm-Message-State: AJIora8efyz452VeIjuf3bd17ug3eNhLlV+Y0wD7c9an2ypi8Xh5dMJ/ NPw9hUWSzZPCXy4mADkjCVkXe2HzmN0vJ0Z7yjI= X-Google-Smtp-Source: AGRyM1sXpKQZrYG0IbOgoxNKanSErlzr5LcY9lrHn4FGhRFmisG96yuJDLZQZGtegCOB6F8ydUZqqM5XWqwFIZtxfFQ= X-Received: by 2002:a05:622a:6098:b0:2f0:f0d2:b5f0 with SMTP id hf24-20020a05622a609800b002f0f0d2b5f0mr17361865qtb.583.1658907456035; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) MIME-Version: 1.0 References: <20220722102407.2205-1-peterwu.pub@gmail.com> <20220722102407.2205-12-peterwu.pub@gmail.com> In-Reply-To: From: ChiaEn Wu Date: Wed, 27 Jul 2022 15:36:59 +0800 Message-ID: Subject: Re: [PATCH v6 11/13] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support To: Andy Shevchenko Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "open list:FRAMEBUFFER LAYER" , "Krogerus, Heikki" , Krzysztof Kozlowski , Alice Chen , linux-iio , dri-devel , Liam Girdwood , cy_huang , Pavel Machek , Lee Jones , Linux LED Subsystem , Daniel Thompson , Helge Deller , Rob Herring , Chunfeng Yun , Guenter Roeck , devicetree , Linux PM , szuni chen , Mark Brown , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , linux-arm Mailing List , Jingoo Han , USB , Sebastian Reichel , Linux Kernel Mailing List , ChiaEn Wu , Greg Kroah-Hartman , Jonathan Cameron Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, Jul 26, 2022 at 8:18 PM Andy Shevchenko wrote: ... > > Just for saving memory space. > > Because these led_classdevs do not be used at the same time. > > Or do you think it would be better to rewrite it as follows? > > ------------------------------------------------------------------------------------- > > struct mt6370_led { > > struct led_classdev isink; > > struct led_classdev_mc mc; > > struct mt6370_priv *priv; > > u32 default_state; > > u32 index; > > }; > > ------------------------------------------------------------------------------------- > > You obviously didn't get what I'm talking about... > Each union to work properly should have an associated variable that > holds the information of which field of the union is in use. Do you > have such a variable? If not, how does your code know which one to > use? If yes, add a proper comment there. > Ummm... from my understanding, if the colors of these four LEDs are set to 'LED_COLOR_ID_RGB' or 'LED_COLOR_ID_MULTI' in DT, their 'led->index' will be set to 'MT6370_VIRTUAL_MULTICOLOR' in 'mt6370_leds_probe()'. If so, these led devices will be set as 'struct led_classdev_mc' and use related ops functions in 'mt6370_init_led_properties()'. Instead, they whose 'led->index' is not 'MT6370_VIRTUAL_MULTICOLOR' will be set as 'struct led_classdev'. So, maybe the member 'index' of the 'struct mt6370_led' is what you describe the information of which field of the union is in use? I will add the proper comment here to describe this thing. I'm so sorry for misunderstanding your mean last time. Thanks again for your review. -- Best Regards, ChiaEn Wu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A7826C04A68 for ; Wed, 27 Jul 2022 07:38:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=rX/E5DXnRLMfLpAijmy+C1U4p3xUonJDjB6LIGeijio=; b=g8J4aaolH9rjWI vl3IqncJQwulsbv1NbufAaVqqUKdQ1G6nct5rqGxuwNTLeghpHlviRM3nq8HCGGjcZlpnQ/TAw0E4 O1kcJYhM1CX5WS3KciqkzB2y42C+YX3W0zRZ3+fsbmMzZjizU8sCUvePN2LdDZ/L6dieZE0x9N+j+ EMa+ubSDKaH5RUnV639JUJQ5UXeYJ3s5cOk4RXOd5RQnX1A7exEvW7FXjjq+YinWC66TiwC9+RsEV LW/Z4tCIDLJqfBD5tCgbVRiGi7JmqZC8YGMru8/riBynyEoPO2JnIKkCLeMqnIRvTPcxgbc3AP35W FxSRZYSErBGJ5eSRO63g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oGbbw-00AcBg-ME; Wed, 27 Jul 2022 07:37:40 +0000 Received: from mail-qt1-x831.google.com ([2607:f8b0:4864:20::831]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oGbbt-00Ac8k-TZ; Wed, 27 Jul 2022 07:37:39 +0000 Received: by mail-qt1-x831.google.com with SMTP id u12so12128952qtk.0; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7wBtopi5nrrnF3/tK6d3mUXKRzW2T+DIHWRi/Z/g55U=; b=JrXv07SnypKXIiO9c1hDaO1ve2XFoCQh3YJA9N0xzJ8VKdcy+NOWbVByEfE93yA1bT Tl79vU21bY1Riyh5vdmQjiXNRM6Y+lisVzjlFGA12N5TkfsJVvBTZVM+DXhLMCRLgtAT CWazI0B5gaYgTPfT/LBhBjVu7eM5hrdl/RUhg4FDmsUWR19oAbFy6HoE9edFHPyenD8T Qf2WPKI/e5SPnIeUMPrpopEK3VVMRKkcz81I6LE01vsC+va5BFGobzRlWIltgg+x4gCQ EHVJv70L2zauSaiZEF5yiD6RME26xNIBHXc93G+wfeZxZyIdgW4ZKl6QFkorqrFDfTKD 6LhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7wBtopi5nrrnF3/tK6d3mUXKRzW2T+DIHWRi/Z/g55U=; b=y9G6fX+LvNpFqKWlpEOph+kbCes40R9x2gLDA6xjcDJYDEplZbjcQB40EvoC7t3Apc oAIqIAu5QX1HE2hvSa/pgh4GhJj5EfZGP1YubrhhpYYUv+rqVadPfvb/zQe6ZBvaPOw0 I8ph/cNCSWCCMXmSQO1bpu4mx7uTY9TWUyRPLc83xRIIjY0ojmKYSUIhuPCn/NRLyCXR ezyGqcQCmX68ygXBHfrfkv1JazwoHhMqSPp5XiVApRh/oFUEqULEsT7JSGnVBXgSHGr6 h789/16otsFjW31CSRBOWmTx7LrBmW9m2R4LFg3kqoq+TjW5rqBIRpkahjm9juRnHh2A kr4Q== X-Gm-Message-State: AJIora+6E67otB1vvEzuqGMKUqke+/DC6B7TmpQc/giVI2Hsza9yyQJz fyKJSg3dSPRxzToe1lQ3UhpBVsHgPmjO6jDJsLU= X-Google-Smtp-Source: AGRyM1sXpKQZrYG0IbOgoxNKanSErlzr5LcY9lrHn4FGhRFmisG96yuJDLZQZGtegCOB6F8ydUZqqM5XWqwFIZtxfFQ= X-Received: by 2002:a05:622a:6098:b0:2f0:f0d2:b5f0 with SMTP id hf24-20020a05622a609800b002f0f0d2b5f0mr17361865qtb.583.1658907456035; Wed, 27 Jul 2022 00:37:36 -0700 (PDT) MIME-Version: 1.0 References: <20220722102407.2205-1-peterwu.pub@gmail.com> <20220722102407.2205-12-peterwu.pub@gmail.com> In-Reply-To: From: ChiaEn Wu Date: Wed, 27 Jul 2022 15:36:59 +0800 Message-ID: Subject: Re: [PATCH v6 11/13] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support To: Andy Shevchenko Cc: Lee Jones , Daniel Thompson , Jingoo Han , Pavel Machek , Rob Herring , Krzysztof Kozlowski , Matthias Brugger , Sebastian Reichel , Chunfeng Yun , Greg Kroah-Hartman , Jonathan Cameron , Lars-Peter Clausen , Liam Girdwood , Mark Brown , Guenter Roeck , "Krogerus, Heikki" , Helge Deller , ChiaEn Wu , Alice Chen , cy_huang , dri-devel , Linux LED Subsystem , devicetree , linux-arm Mailing List , "moderated list:ARM/Mediatek SoC support" , Linux Kernel Mailing List , Linux PM , USB , linux-iio , "open list:FRAMEBUFFER LAYER" , szuni chen X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220727_003737_996645_1A3DF0F5 X-CRM114-Status: GOOD ( 22.63 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jul 26, 2022 at 8:18 PM Andy Shevchenko wrote: ... > > Just for saving memory space. > > Because these led_classdevs do not be used at the same time. > > Or do you think it would be better to rewrite it as follows? > > ------------------------------------------------------------------------------------- > > struct mt6370_led { > > struct led_classdev isink; > > struct led_classdev_mc mc; > > struct mt6370_priv *priv; > > u32 default_state; > > u32 index; > > }; > > ------------------------------------------------------------------------------------- > > You obviously didn't get what I'm talking about... > Each union to work properly should have an associated variable that > holds the information of which field of the union is in use. Do you > have such a variable? If not, how does your code know which one to > use? If yes, add a proper comment there. > Ummm... from my understanding, if the colors of these four LEDs are set to 'LED_COLOR_ID_RGB' or 'LED_COLOR_ID_MULTI' in DT, their 'led->index' will be set to 'MT6370_VIRTUAL_MULTICOLOR' in 'mt6370_leds_probe()'. If so, these led devices will be set as 'struct led_classdev_mc' and use related ops functions in 'mt6370_init_led_properties()'. Instead, they whose 'led->index' is not 'MT6370_VIRTUAL_MULTICOLOR' will be set as 'struct led_classdev'. So, maybe the member 'index' of the 'struct mt6370_led' is what you describe the information of which field of the union is in use? I will add the proper comment here to describe this thing. I'm so sorry for misunderstanding your mean last time. Thanks again for your review. -- Best Regards, ChiaEn Wu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel