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 E6A22EB64D7 for ; Wed, 21 Jun 2023 21:32:00 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BzIF/VgCOThozcfSW8S5Jup1434bUdEPMo4iJW1m++Y=; b=GzqRP1kuakwRjQ dCsmvjJy+FXZBSNoYsTYIJhJZ8CgEpC/h9JEFUJmlwJE9tE63+T5yz/+K3gmwtJ+pDtYr1PCoywof B+kf/BuET0yPWVXu5s+lL7VHnNfQig7FQs8DWYESrGBxiTRqZP9JjaxpjEhTrDudCp5C4vnzmR/Ke PAOPZNIO8CK+/aPa1s4F2ZVJCRTQ9c0ONRq8iTudlk//a39NsICX0MH/uq/7eBCpqrqVLcfp5zOm3 DFIgGwtWimuRpXDYy4nMcFG8ifjey9dUVKuR5mZst6RMq89YwLm/SlzZSeNpb6OBPrpG7To6ploRU ldDTA+zBBr06TMEFRcmQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qC5QJ-00Fn3x-0u; Wed, 21 Jun 2023 21:31:31 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qC5QG-00Fn39-1l; Wed, 21 Jun 2023 21:31:30 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CEA8A616C2; Wed, 21 Jun 2023 21:31:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5B7DDC433C0; Wed, 21 Jun 2023 21:31:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1687383087; bh=HY2/KPCQv+TNb8BEUoFS/75P2bkM0JsT+gXSpFBNCJA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=inJljWazMcccbQVlONx/s92wacqeR1e65s1uXDfzP8ddik+z5pUlpBQKE5kSFyfME VAkZAODssc039oYcAHlCKCTi7j5WiJy6YESntQDym3N5YRUSPeuy5YL94ZdxMxeQXY I7VWEg9/4cM2agT4V2XhXpmeGxdg3TP/ZxnQBvntnfC4J8xFGdJd0exvsJgIfFpIus EKy2GZ5bK/cvf4Ghg3/KblBZTk3EclqnYSzKcKxBTHU0px6Hh+9emJD9cCxfwT1E5Y UvEEE8BNXEncBVjoeaw0CDY9o7wB13lrmROFp0Mowsyxs2LbLrqR35UnHU4/pCCslh BOXqTkjIVAM1A== Date: Wed, 21 Jun 2023 21:31:24 +0000 From: Nathan Chancellor To: AngeloGioacchino Del Regno Cc: pavel@ucw.cz, lee@kernel.org, sean.wang@mediatek.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com, linux-leds@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@collabora.com, llvm@lists.linux.dev Subject: Re: [PATCH v3 8/8] leds: leds-mt6323: Add support for WLEDs and MT6332 Message-ID: <20230621213124.GA2689001@dev-arch.thelio-3990X> References: <20230601110813.2373764-1-angelogioacchino.delregno@collabora.com> <20230601110813.2373764-9-angelogioacchino.delregno@collabora.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230601110813.2373764-9-angelogioacchino.delregno@collabora.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230621_143128_672308_7EBEEEFF X-CRM114-Status: GOOD ( 20.61 ) 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 Hi Angelo, On Thu, Jun 01, 2023 at 01:08:13PM +0200, AngeloGioacchino Del Regno wrote: > Add basic code to turn on and off WLEDs and wire up MT6332 support > to take advantage of it. > This is a simple approach due to the aforementioned PMIC supporting > only on/off status so, at the time of writing, it is impossible for me > to validate more advanced functionality due to lack of hardware. > > Signed-off-by: AngeloGioacchino Del Regno After this patch as commit 9bb0a9e0626c ("leds: leds-mt6323: Add support for WLEDs and MT6332") in -next, I see the following warnings from clang, which are basically flagging potential kernel Control Flow Integrity [1] violations that will be visible at runtime (this warning is not enabled for the kernel yet but we would like it to be): drivers/leds/leds-mt6323.c:598:49: error: incompatible function pointer types assigning to 'int (*)(struct led_classdev *, enum led_brightness)' from 'int (struct led_classdev *, unsigned int)' [-Werror,-Wincompatible-function-pointer-types-strict] 598 | leds->led[reg]->cdev.brightness_set_blocking = | ^ 599 | mt6323_wled_set_brightness; | ~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/leds/leds-mt6323.c:600:40: error: incompatible function pointer types assigning to 'enum led_brightness (*)(struct led_classdev *)' from 'unsigned int (struct led_classdev *)' [-Werror,-Wincompatible-function-pointer-types-strict] 600 | leds->led[reg]->cdev.brightness_get = | ^ 601 | mt6323_get_wled_brightness; | ~~~~~~~~~~~~~~~~~~~~~~~~~~ 2 errors generated. >From what I can tell/understand, 'enum led_brightness' is obsolete and the value that is passed via ->brightness_set_blocking() is an 'unsigned int' as well but it seems 'enum led_brightness' is used as the parameter in a lot of different callback implementations, so the prototype cannot be easily updated without a lot of extra work. Is there any reason not to just do something like this to avoid this issue? [1]: https://lwn.net/Articles/898040/ Cheers, Nathan diff --git a/drivers/leds/leds-mt6323.c b/drivers/leds/leds-mt6323.c index e8fecfc2e90a..24f35bdb55fb 100644 --- a/drivers/leds/leds-mt6323.c +++ b/drivers/leds/leds-mt6323.c @@ -76,7 +76,7 @@ struct mt6323_led { int id; struct mt6323_leds *parent; struct led_classdev cdev; - unsigned int current_brightness; + enum led_brightness current_brightness; }; /** @@ -451,7 +451,7 @@ static int mtk_wled_hw_off(struct led_classdev *cdev) return 0; } -static unsigned int mt6323_get_wled_brightness(struct led_classdev *cdev) +static enum led_brightness mt6323_get_wled_brightness(struct led_classdev *cdev) { struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev); struct mt6323_leds *leds = led->parent; @@ -471,7 +471,7 @@ static unsigned int mt6323_get_wled_brightness(struct led_classdev *cdev) } static int mt6323_wled_set_brightness(struct led_classdev *cdev, - unsigned int brightness) + enum led_brightness brightness) { struct mt6323_led *led = container_of(cdev, struct mt6323_led, cdev); struct mt6323_leds *leds = led->parent; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel