From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Vaussard Subject: Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver Date: Mon, 27 Jun 2016 07:46:49 +0200 Message-ID: <51f6c4d4-e5af-97b6-e7d9-61f0093de134@heig-vd.ch> References: <1466494154-3786-1-git-send-email-florian.vaussard@heig-vd.ch> <1466494154-3786-3-git-send-email-florian.vaussard@heig-vd.ch> <57695D45.60107@samsung.com> <20160626214902.GB21026@amd> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160626214902.GB21026@amd> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Pavel Machek , Jacek Anaszewski Cc: Florian Vaussard , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Richard Purdie , Rob Herring , Mark Rutland , linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Vaussard Florian List-Id: linux-leds@vger.kernel.org Hi Pavel, Le 26. 06. 16 =E0 23:49, Pavel Machek a =E9crit : > Hi! >=20 >>> +struct ncp5623_led { >>> + bool active; >>> + unsigned int led_no; >>> + struct led_classdev ldev; >>> + struct work_struct work; >>> + struct ncp5623_priv *priv; >>> +}; >>> + >>> +struct ncp5623_priv { >>> + struct ncp5623_led leds[NCP5623_MAX_LEDS]; >> >> Please allocate memory dynamically, depending on the number >> of LEDs defined in a Device Tree. >=20 > MAX_LEDs is three. Are you sure overhead of dynamic allocation is > worth it? >=20 > And if this is for RGB leds... very probably device will want to use > all 3 channels. >=20 I was about to raise the same question during the v2 of this patch. In = addition to your arguments, this also changes the way this array is indexed. Currently the LED number is used as index, but with dynamic allocation = I have to use an abstract index. This makes some logic a bit harder, especially t= o check if the same LED is declared twice in the device tree (duplicated 'reg' = property). Best, =46lorian -- To unsubscribe from this list: send the line "unsubscribe devicetree" i= n the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751875AbcF0FwI (ORCPT ); Mon, 27 Jun 2016 01:52:08 -0400 Received: from mailcl2.heig-vd.ch ([193.134.216.183]:44548 "EHLO heig-vd.ch" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751033AbcF0FwF (ORCPT ); Mon, 27 Jun 2016 01:52:05 -0400 Subject: Re: [PATCH 2/2] leds: Add driver for NCP5623 3-channel I2C LED driver To: Pavel Machek , Jacek Anaszewski References: <1466494154-3786-1-git-send-email-florian.vaussard@heig-vd.ch> <1466494154-3786-3-git-send-email-florian.vaussard@heig-vd.ch> <57695D45.60107@samsung.com> <20160626214902.GB21026@amd> CC: Florian Vaussard , , Richard Purdie , Rob Herring , Mark Rutland , , , Vaussard Florian From: Florian Vaussard Message-ID: <51f6c4d4-e5af-97b6-e7d9-61f0093de134@heig-vd.ch> Date: Mon, 27 Jun 2016 07:46:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160626214902.GB21026@amd> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.192.93.96] X-ClientProxiedBy: EINTFEA.einet.ad.eivd.ch (10.192.41.59) To EINTMBXC.einet.ad.eivd.ch (10.192.41.65) X-MailCleaner-RDNS: invalid reverse DNS for 10.192.41.59 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Pavel, Le 26. 06. 16 à 23:49, Pavel Machek a écrit : > Hi! > >>> +struct ncp5623_led { >>> + bool active; >>> + unsigned int led_no; >>> + struct led_classdev ldev; >>> + struct work_struct work; >>> + struct ncp5623_priv *priv; >>> +}; >>> + >>> +struct ncp5623_priv { >>> + struct ncp5623_led leds[NCP5623_MAX_LEDS]; >> >> Please allocate memory dynamically, depending on the number >> of LEDs defined in a Device Tree. > > MAX_LEDs is three. Are you sure overhead of dynamic allocation is > worth it? > > And if this is for RGB leds... very probably device will want to use > all 3 channels. > I was about to raise the same question during the v2 of this patch. In addition to your arguments, this also changes the way this array is indexed. Currently the LED number is used as index, but with dynamic allocation I have to use an abstract index. This makes some logic a bit harder, especially to check if the same LED is declared twice in the device tree (duplicated 'reg' property). Best, Florian