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 98921EB64DB for ; Thu, 22 Jun 2023 09:11:24 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=VHV50OC8rtfYUZ1hyBunWyBSPj3YhIJksphPy/Ky9gA=; b=CDRMXsD4uS7Rwe EpGHBfGyslDO6IYNGi2h1liQzFuGMK0r+xjKo7VhpSlQyK/30Gh6ZOyLWmEKqxZoa43Br5OMOSmCE eMK62qSGrsYn8n6C4skj8+Pc1c6CXEjnLGECl68hxb6d5WrWKR3uNuWnZuWx7K4jEflrGc+qvut4Y 5ibmzmQ5lAx2R1+6xTBGMmr17x/obima9h0lgHY9GZMVjL5nVhLLg2NKr3VWsv8udLVSth47burGr M4+eSZbvCTHPPx8N4Y8J/3d+rwbrNeHvz3tvHU4yTlLInfmeTcPsADaU9c5zPYSst/SvZuozHgZyl 3plMm6ZVXuNUTNUE2mEQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qCGL7-00HYIf-1y; Thu, 22 Jun 2023 09:10:53 +0000 Received: from madras.collabora.co.uk ([46.235.227.172]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qCGL4-00HYGp-32; Thu, 22 Jun 2023 09:10:52 +0000 Received: from [IPV6:2001:b07:2ed:14ed:c5f8:7372:f042:90a2] (unknown [IPv6:2001:b07:2ed:14ed:c5f8:7372:f042:90a2]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 7D0B8660706C; Thu, 22 Jun 2023 10:10:37 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1687425038; bh=5uxh5lKnVnM46Q8SEQodnpqC2oAD7Fjq5YEoHjE5Sx4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=KPUAgmSdKBe8F1sbK0PJohKxD/iIlloBxRbdx2mXToR6bT0FdS/9cnpdpHyDyWPiV 9LYisjtoi9lQjkY6At9xAj4YncbpeIshMKIzGUJIMWyOqj3QittZD7V+6FnIquTZnH kyANnfNm+j8bI8c/535TwPqRf62LvcR+18KpLAlQcC1hEy8GqAeUwjX2JbLeasux8I JMqpyRqAtZuGljolnyporGOo6z6ZzqXm//GGQROy/CNoq5PYvnLri1W+ItKNrAnqxa 9CVUOFrfDlfafgmh31726rulYjimvN7Q790qDQmFLPxCVksLehwVx3/BZVXkSSbYjN y7zmKhrAZTPiw== Message-ID: <99adec72-da7b-ce46-27be-b823a208a39e@collabora.com> Date: Thu, 22 Jun 2023 11:10:34 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v3 8/8] leds: leds-mt6323: Add support for WLEDs and MT6332 To: Nathan Chancellor 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 References: <20230601110813.2373764-1-angelogioacchino.delregno@collabora.com> <20230601110813.2373764-9-angelogioacchino.delregno@collabora.com> <20230621213124.GA2689001@dev-arch.thelio-3990X> Content-Language: en-US From: AngeloGioacchino Del Regno In-Reply-To: <20230621213124.GA2689001@dev-arch.thelio-3990X> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230622_021051_292032_BE9A0328 X-CRM114-Status: GOOD ( 28.25 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Il 21/06/23 23:31, Nathan Chancellor ha scritto: > 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? > I don't think that this would bring any issue to the table. The rework will possibly be done globally, for all drivers, when time comes... so feel free to send the proposed patch. Thanks, Angelo > [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