From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 36E177F7D3; Tue, 2 Apr 2024 13:14:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.7 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712063680; cv=none; b=eeb1zP6iV6oQobFm/w2/g2toUxGubF0MMn5Jn7HxGE0oumjw2Y5b8KrX/joOlDPFDR0a/aRfI4zLr/rWacu0j96trq2hnetjZgLf8osA6KtBZdoqTl2esYRLItTi3igyyqWRG6ZkMwnft45itqkJGtxqRm01NNEopkTzAKeXUyA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712063680; c=relaxed/simple; bh=dNhWs1tSERBEqpT1H1jI1gPA9tR9hFX3rMeei4wKg3I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=I134HRyrxsSzAy1HGBhTJfHT50fAtJzSYqdzO4v1QonQf1AONYVbrxQRtnf9rzI+vITmzwmlN2hVdWoldxGxCLVs6nNN44+CgclIf3MnWcAqMOK/voc+LfW307o9L4ZqMTc8Tym1dasHZQx5Vdr7ZiZXzUuFdGcHC/FEyDJM+YY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=hNvqjAaL; arc=none smtp.client-ip=192.198.163.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="hNvqjAaL" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1712063678; x=1743599678; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=dNhWs1tSERBEqpT1H1jI1gPA9tR9hFX3rMeei4wKg3I=; b=hNvqjAaLYDpyPnV29zeS0RCRihuhJ4+iuBXIlRjln1/7L1Izt20oRT1x zfS4hQKklR4w/eDqQvPZUG2y1az0nFdk18TpPbo1ChFVNkGfwUcZWj+KJ FjMuGFNZbOomFEd48FHGJW1kahivd7doYoLiNqs98mHBHYqKH3W1QVtTT 5UwWv4WkHQF+F3sqStjAA6v3WAH3auvVwptU5ew672nISWbe7wMsHyYzE WhdoNAMw2nMljMZnbKIYeT2TtqNg88BU9tMslcEgEMAC0i2dwsMUYiVkw 2RJxsizkVIdALxc9l7jVrSlzZLUWykfI3UOixapWmu1yEDi+V0muR+gcO g==; X-CSE-ConnectionGUID: 5FvuF+rFR/Wd59lPMsobUg== X-CSE-MsgGUID: S5p3n9rESEi3PJiQadbSug== X-IronPort-AV: E=McAfee;i="6600,9927,11031"; a="32630976" X-IronPort-AV: E=Sophos;i="6.07,175,1708416000"; d="scan'208";a="32630976" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2024 06:14:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,11031"; a="915140683" X-IronPort-AV: E=Sophos;i="6.07,175,1708416000"; d="scan'208";a="915140683" Received: from smile.fi.intel.com (HELO smile) ([10.237.72.54]) by fmsmga002.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2024 06:14:33 -0700 Received: from andy by smile with local (Exim 4.97) (envelope-from ) id 1rrdyB-00000000oc0-1Wc3; Tue, 02 Apr 2024 16:14:31 +0300 Date: Tue, 2 Apr 2024 16:14:31 +0300 From: Andy Shevchenko To: "Peng Fan (OSS)" Cc: Sudeep Holla , Cristian Marussi , 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, Peng Fan , Oleksii Moisieiev Subject: Re: [PATCH v7 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Message-ID: References: <20240402-pinctrl-scmi-v7-0-3ea519d12cf7@nxp.com> <20240402-pinctrl-scmi-v7-3-3ea519d12cf7@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: <20240402-pinctrl-scmi-v7-3-3ea519d12cf7@nxp.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo On Tue, Apr 02, 2024 at 10:22:23AM +0800, Peng Fan (OSS) wrote: ... > +#include > +#include > +#include Please, follow IWYU principle, a lot of headers are missed. > +#include "common.h" > +#include "protocols.h" ... > + ret = scmi_pinctrl_get_pin_info(ph, selector, > + &pi->pins[selector]); It's netter as a single line. > + if (ret) > + return ret; > + } ... > +static int scmi_pinctrl_protocol_init(const struct scmi_protocol_handle *ph) > +{ > + int ret; > + u32 version; > + struct scmi_pinctrl_info *pinfo; > + > + ret = ph->xops->version_get(ph, &version); > + if (ret) > + return ret; > + > + dev_dbg(ph->dev, "Pinctrl Version %d.%d\n", > + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version)); > + > + pinfo = devm_kzalloc(ph->dev, sizeof(*pinfo), GFP_KERNEL); > + if (!pinfo) > + return -ENOMEM; > + > + ret = scmi_pinctrl_attributes_get(ph, pinfo); > + if (ret) > + return ret; > + > + pinfo->pins = devm_kcalloc(ph->dev, pinfo->nr_pins, > + sizeof(*pinfo->pins), GFP_KERNEL); > + if (!pinfo->pins) > + return -ENOMEM; > + > + pinfo->groups = devm_kcalloc(ph->dev, pinfo->nr_groups, > + sizeof(*pinfo->groups), GFP_KERNEL); > + if (!pinfo->groups) > + return -ENOMEM; > + > + pinfo->functions = devm_kcalloc(ph->dev, pinfo->nr_functions, > + sizeof(*pinfo->functions), GFP_KERNEL); > + if (!pinfo->functions) > + return -ENOMEM; > + > + pinfo->version = version; > + > + return ph->set_priv(ph, pinfo, version); Same comments as per previous version. devm_ here is simply wrong. It breaks the order of freeing resources. I.o.w. I see *no guarantee* that these init-deinit functions will be properly called from the respective probe-remove. Moreover the latter one may also have its own devm allocations (which are rightfully placed) and you get completely out of control the resource management. > +} -- With Best Regards, Andy Shevchenko 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 239C3CD128D for ; Tue, 2 Apr 2024 13:14:54 +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=/ccodUlLiEeKKMy3n0KTIUUYgvy44QReIYddj+iQRSQ=; b=p+S9kstAdpMUR4 HcHjaPirQH2oZb/V3I9CZWcjioL8rNGpFQ1khVl0V6skuyCUpGurzitPtC03SZu8PTIAOLPvwRGuY GuQ2dKY1DisfpmF9oydo13rSKln+KKr9BlIChLfzXywOTXvHPj6CvuRW/DE1PDOKQfIs8WM4g7MnP cVuS7kBJMMZAGFDCEnXHLfoH3JdSyEtELwafemLNJPfrtfKrE3Vn3fpcIZrBawxPT5ZDdNGbbyrgo SHcIhx9eJVI3YmjvCSF76ljxvNd5R2f6y2smeOMyrsKAZrgmTL9cgrSmQuIpFPacMN9YaChB+1oaO 3Re/F+bAXI7LmvtoyFyA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrdyL-0000000BIqt-1GCP; Tue, 02 Apr 2024 13:14:41 +0000 Received: from mgamail.intel.com ([192.198.163.7]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrdyI-0000000BIpT-38Y2 for linux-arm-kernel@lists.infradead.org; Tue, 02 Apr 2024 13:14:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1712063679; x=1743599679; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=dNhWs1tSERBEqpT1H1jI1gPA9tR9hFX3rMeei4wKg3I=; b=KhiGlwgyNJys0c1z+O3Tvl45ORytE/aTFNAUQsRLAhmYcw8tDojfR++q vXFtG4y1iWUF9B9O1IW7rgBexS/MyJQOOyMIwEOlHowEFNiDMdKxpjeNM DWPix7wWkEhSRYg+ssLi4dFp6Madf+OwZlxcTvLgR2bBF+QOMNZfmNmPt ShSDAU5Pud8UNdIWlJnjZwb71NC4XoS2wlh7Wm4jo5785evXZWVYnPaKq 2yEyrvnBlxA7VJEpUYEEJQ54/jTXvwdLsTqVMmdwGPWs4cJbJV9u+EvAC xyfQts4StCdh3ygJAP3S3OSwMPtoL7y+GXQ7FqTZ71rnPDBgWc27HzyQb A==; X-CSE-ConnectionGUID: uelafxApRLCpCz4ytk4V6w== X-CSE-MsgGUID: GWrpH8QaTaqxUvlbvBFn1w== X-IronPort-AV: E=McAfee;i="6600,9927,11031"; a="32630979" X-IronPort-AV: E=Sophos;i="6.07,175,1708416000"; d="scan'208";a="32630979" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2024 06:14:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,11031"; a="915140683" X-IronPort-AV: E=Sophos;i="6.07,175,1708416000"; d="scan'208";a="915140683" Received: from smile.fi.intel.com (HELO smile) ([10.237.72.54]) by fmsmga002.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2024 06:14:33 -0700 Received: from andy by smile with local (Exim 4.97) (envelope-from ) id 1rrdyB-00000000oc0-1Wc3; Tue, 02 Apr 2024 16:14:31 +0300 Date: Tue, 2 Apr 2024 16:14:31 +0300 From: Andy Shevchenko To: "Peng Fan (OSS)" Cc: Sudeep Holla , Cristian Marussi , 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, Peng Fan , Oleksii Moisieiev Subject: Re: [PATCH v7 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Message-ID: References: <20240402-pinctrl-scmi-v7-0-3ea519d12cf7@nxp.com> <20240402-pinctrl-scmi-v7-3-3ea519d12cf7@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240402-pinctrl-scmi-v7-3-3ea519d12cf7@nxp.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240402_061438_855116_C32B7C60 X-CRM114-Status: GOOD ( 12.70 ) 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, Apr 02, 2024 at 10:22:23AM +0800, Peng Fan (OSS) wrote: ... > +#include > +#include > +#include Please, follow IWYU principle, a lot of headers are missed. > +#include "common.h" > +#include "protocols.h" ... > + ret = scmi_pinctrl_get_pin_info(ph, selector, > + &pi->pins[selector]); It's netter as a single line. > + if (ret) > + return ret; > + } ... > +static int scmi_pinctrl_protocol_init(const struct scmi_protocol_handle *ph) > +{ > + int ret; > + u32 version; > + struct scmi_pinctrl_info *pinfo; > + > + ret = ph->xops->version_get(ph, &version); > + if (ret) > + return ret; > + > + dev_dbg(ph->dev, "Pinctrl Version %d.%d\n", > + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version)); > + > + pinfo = devm_kzalloc(ph->dev, sizeof(*pinfo), GFP_KERNEL); > + if (!pinfo) > + return -ENOMEM; > + > + ret = scmi_pinctrl_attributes_get(ph, pinfo); > + if (ret) > + return ret; > + > + pinfo->pins = devm_kcalloc(ph->dev, pinfo->nr_pins, > + sizeof(*pinfo->pins), GFP_KERNEL); > + if (!pinfo->pins) > + return -ENOMEM; > + > + pinfo->groups = devm_kcalloc(ph->dev, pinfo->nr_groups, > + sizeof(*pinfo->groups), GFP_KERNEL); > + if (!pinfo->groups) > + return -ENOMEM; > + > + pinfo->functions = devm_kcalloc(ph->dev, pinfo->nr_functions, > + sizeof(*pinfo->functions), GFP_KERNEL); > + if (!pinfo->functions) > + return -ENOMEM; > + > + pinfo->version = version; > + > + return ph->set_priv(ph, pinfo, version); Same comments as per previous version. devm_ here is simply wrong. It breaks the order of freeing resources. I.o.w. I see *no guarantee* that these init-deinit functions will be properly called from the respective probe-remove. Moreover the latter one may also have its own devm allocations (which are rightfully placed) and you get completely out of control the resource management. > +} -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel