From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2F5753B29A; Tue, 2 Apr 2024 07:48:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712044132; cv=none; b=d3aPi6jfMUFzRSLky+xQOTPwMhl6/bMOS+kT827pzC3ZHhE7GvxRLAjgp05WYaVarXibm6CHvoZPEyA71ViqRM26sviEQkmzIoPQ8W17HWeBdr8orhDbuMY3pZMfC2NEPXVKxCtYKnq4pstMHLe1OxEPco4TKVQxKfqa83ElZbM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712044132; c=relaxed/simple; bh=xudrbFG1OklbyuU2y6nzlMSY6MlqTsTkKxZNUae6GEs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ob6bMVZHeZiHChgNKymtnc81jxf0YrV0iQOK/fkruCTSoQyMHAOF/ZfLCdK9y7CxbApmcaaISnmDptR9YbjiAvZnOrOcrOdmqo43Jq7Hr5rgnI9IOEOl2SWv6KPBKbKGpTbrAJiD1Kq1qe0DYz50EzZ8AUByzh/D07pLvI70ENs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D201B1042; Tue, 2 Apr 2024 00:49:20 -0700 (PDT) Received: from pluto (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0C72A3F64C; Tue, 2 Apr 2024 00:48:46 -0700 (PDT) Date: Tue, 2 Apr 2024 08:48:44 +0100 From: Cristian Marussi To: Peng Fan Cc: Andy Shevchenko , "Peng Fan (OSS)" , Sudeep Holla , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Linus Walleij , Dan Carpenter , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-gpio@vger.kernel.org" , Oleksii Moisieiev Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Message-ID: References: <20240323-pinctrl-scmi-v6-0-a895243257c0@nxp.com> <20240323-pinctrl-scmi-v6-3-a895243257c0@nxp.com> Precedence: bulk X-Mailing-List: linux-gpio@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote: > Hi Andy, > Hi Peng, > > Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol > > protocol basic support > > > > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti: > > > From: Peng Fan > > > > > > Add basic implementation of the SCMI v3.2 pincontrol protocol. > > > > ... > > > > > scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o > > > scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o > > > scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o > > > system.o voltage.o powercap.o > > > > Actually you want to have := here. > > > > > +scmi-protocols-y += pinctrl.o > > > > > > > > > scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) > > > $(scmi-transport-y) > > > > Side note: The -objs has to be -y > > > > ... > > > > > +#include > > > +#include > > > +#include > > > > This is semi-random list of headers. Please, follow IWYU principle (include > > what you use). There are a lot of inclusions I see missing (just in the context of > > this page I see bits.h, types.h, and asm/byteorder.h). > > Is there any documentation about this requirement? > Some headers are already included by others. > Andy made (mostly) the same remarks on this same patch ~1-year ago on this same patch while it was posted by Oleksii. And I told that time that most of the remarks around devm_ usage were wrong due to how the SCMI core handles protocol initialization (using a devres group transparently). This is what I answered that time. https://lore.kernel.org/linux-arm-kernel/ZJ78hBcjAhiU+ZBO@e120937-lin/#t I wont repeat myself, but, in a nutshell the memory allocation like it is now is fine: a bit happens via devm_ at protocol initialization, the other is doe via explicit kmalloc at runtime and freed via kfree at remove time (if needed...i.e. checking the present flag of some structs) I'll made further remarks on v7 that you just posted. Thanks, Cristian 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 48BA3CD1284 for ; Tue, 2 Apr 2024 07:49:07 +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=WLry0Y4ML/uQmiSnz5OzThggCvtwgJbJ9DWTB0a2Wjc=; b=f80XryM+h8JTXx y1kju51IcKI3emo/TJSrP9U0FBQ82pM1YFHlcunsu/IYqJs/nivlhiuD9/LwqMpHjxhed+cMUnLi/ DsJzbYj+FwP5NssKPeZ/RlXe04mRnSFoDpa94+MbsvExYFzURoKx3ljeNVqF3K9DeOyPzXRKaru6o 3i1vMq7os+zHOwgSDIlA9nLxy97Wd4xW4BR1BQb7E9SBps2I59w5T4Lx6vc/HKrX4gTiUHixUyml7 pldrvbGLz/iN0G1UTi+oTsPw63QT/YXZ1OzXeuxj68YfzsewlaJIaVZ0KtR+3qJ7uviL21IWE8wzh 4TB+oSNpPrKd1K6RnlsA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrYt7-0000000A7PB-1iEV; Tue, 02 Apr 2024 07:48:57 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrYt4-0000000A7OZ-0Xvc for linux-arm-kernel@lists.infradead.org; Tue, 02 Apr 2024 07:48:56 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D201B1042; Tue, 2 Apr 2024 00:49:20 -0700 (PDT) Received: from pluto (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0C72A3F64C; Tue, 2 Apr 2024 00:48:46 -0700 (PDT) Date: Tue, 2 Apr 2024 08:48:44 +0100 From: Cristian Marussi To: Peng Fan Cc: Andy Shevchenko , "Peng Fan (OSS)" , Sudeep Holla , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Linus Walleij , Dan Carpenter , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-gpio@vger.kernel.org" , Oleksii Moisieiev Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Message-ID: References: <20240323-pinctrl-scmi-v6-0-a895243257c0@nxp.com> <20240323-pinctrl-scmi-v6-3-a895243257c0@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240402_004854_235926_D7AF8E24 X-CRM114-Status: GOOD ( 18.75 ) 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 Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote: > Hi Andy, > Hi Peng, > > Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol > > protocol basic support > > > > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti: > > > From: Peng Fan > > > > > > Add basic implementation of the SCMI v3.2 pincontrol protocol. > > > > ... > > > > > scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o > > > scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o > > > scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o > > > system.o voltage.o powercap.o > > > > Actually you want to have := here. > > > > > +scmi-protocols-y += pinctrl.o > > > > > > > > > scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) > > > $(scmi-transport-y) > > > > Side note: The -objs has to be -y > > > > ... > > > > > +#include > > > +#include > > > +#include > > > > This is semi-random list of headers. Please, follow IWYU principle (include > > what you use). There are a lot of inclusions I see missing (just in the context of > > this page I see bits.h, types.h, and asm/byteorder.h). > > Is there any documentation about this requirement? > Some headers are already included by others. > Andy made (mostly) the same remarks on this same patch ~1-year ago on this same patch while it was posted by Oleksii. And I told that time that most of the remarks around devm_ usage were wrong due to how the SCMI core handles protocol initialization (using a devres group transparently). This is what I answered that time. https://lore.kernel.org/linux-arm-kernel/ZJ78hBcjAhiU+ZBO@e120937-lin/#t I wont repeat myself, but, in a nutshell the memory allocation like it is now is fine: a bit happens via devm_ at protocol initialization, the other is doe via explicit kmalloc at runtime and freed via kfree at remove time (if needed...i.e. checking the present flag of some structs) I'll made further remarks on v7 that you just posted. Thanks, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel