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 B6E16C28D13 for ; Thu, 25 Aug 2022 09:18:42 +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=7JdF1p8JymPgHpF3kCGeXNbjubHL7PwvL8l78SLqnkg=; b=WorKnqgOAIfRUa 2pugIP46ADryVnrVsoEGFf8tWSza1/Goe1tNvtkkpJsjdWcVj0r0AZsg3GiAUejd+tkz01Fg9D7xx oU6HdTrEEukBkClmk+B+GAplRlvX2lF5FFs9wA7G0wMxnBjBcnLbWNJD1z6dbs+W0Sn+N05aVV9v4 G+Vm/FPNgMRTENc0DwXQFzDmTSSMyzTo9euK5s2SIbQfDm7vQY9A/JZD5/MVNZQbbn2Ff75c8Oemh gueQICptIv/A1kFJ3yN4JqszEsnVD3zK574d87PNIyP99BT6VbvJwdjXcqogTbqIgiBQDPng+8Qr8 9WM3mAEARAfQkQVxl8Ww==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oR8zc-00ArU5-9T; Thu, 25 Aug 2022 09:17:40 +0000 Received: from mail-wr1-x42b.google.com ([2a00:1450:4864:20::42b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oR8zX-00ArLD-9r for linux-arm-kernel@lists.infradead.org; Thu, 25 Aug 2022 09:17:38 +0000 Received: by mail-wr1-x42b.google.com with SMTP id u14so23759426wrq.9 for ; Thu, 25 Aug 2022 02:17:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc; bh=hfCNevAxgJvLuTWr8SaR94vFPaHmg4R55Me0Hy0yr2E=; b=sufzBfPfVuYtZY3PY6wJVunOCdpmjJtQQ3FShdHH3SEHEyKMq5q/+m/yebsuQQx1qS llyt6o4dRKXF3slpTAZiYDGMSflHDfzU4hYAuErYehIDmubif2jMd/K0skC2jf/jbcFy Jw3Dfj9frjUsQW7jTk0q/e8lf4AwohHw0nzfuSQkrgKW12Cig8SaWrl+4/1EICnw8Cge 4mg+TsmDVqY9AVATxJbHX6ESt19x22Ryvn3LmTVUk/Rtcs75HBZiZgapVQXeih8iZqwn 4rfKr85mbi78EPBASw1zKovtnQIOgt9fZtwqY7ECo8F1eoNvryGVjqB8WxQCC1dvfm2/ DfKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=hfCNevAxgJvLuTWr8SaR94vFPaHmg4R55Me0Hy0yr2E=; b=lyg93DZuV/ctxZW/UnhNm511axQiQ+PDpTPTwnhYKL8LdsOTGEGkJCnaeh4Cx2MOuq GH/FwB5OTm13pStf1sn+hxbXyA/kYBf77QelgGPP2iyGPs156gEw5wubgjE+uPQU9e7R TQeagwQcsMsKBmZRsq/mwwhCrMlAQnwMx63umxXDniGzZXw1SDEF56y3UXI/b15Vv/Az WWMJ6OFpV5DjFGt8zTQ5dDyrjfreSFya1Vh1msPH4RMXDZClS/wjAdbQbmiOPGgVtejs 1S7ZbDeyZxcMBxCTZQFrLU7gTQof/Ae1m6n4w/hHsHsCpHYCULxON/1IQGWSX65flmy2 wKUA== X-Gm-Message-State: ACgBeo17WgbN47f1OmbM14GevZXprJDANhKkhrfDUiQzslKRdGQAWMld ENQc7zuecneCF7+LuJiHNzbnUQ== X-Google-Smtp-Source: AA6agR7yRTc1Q1vuAsghCBmXdphB36jE4XFkP7yyuUUAWwoYNpY0fEPzoiHLn303PEfylE6f5KDSxA== X-Received: by 2002:a05:6000:12cd:b0:225:35b2:c020 with SMTP id l13-20020a05600012cd00b0022535b2c020mr1638661wrx.61.1661419052365; Thu, 25 Aug 2022 02:17:32 -0700 (PDT) Received: from blmsp ([2001:4090:a245:8020:2658:1f7c:362:3e99]) by smtp.gmail.com with ESMTPSA id o8-20020a05600c4fc800b003a603fbad5bsm4956489wmq.45.2022.08.25.02.17.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Aug 2022 02:17:31 -0700 (PDT) Date: Thu, 25 Aug 2022 11:17:30 +0200 From: Markus Schneider-Pargmann To: Lee Jones Cc: Jerome Neanne , lgirdwood@gmail.com, broonie@kernel.org, robh+dt@kernel.org, nm@ti.com, kristo@kernel.org, khilman@baylibre.com, narmstrong@baylibre.com, j-keerthy@ti.com, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Lee Jones Subject: Re: [PATCH v2 06/11] mfd: drivers: Add TI TPS65219 PMIC support Message-ID: <20220825091730.i7qi7lnfq44cffjh@blmsp> References: <20220726103355.17684-1-jneanne@baylibre.com> <20220726103355.17684-7-jneanne@baylibre.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-20220825_021735_587556_C77BE568 X-CRM114-Status: GOOD ( 38.02 ) 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 Lee, thanks for your comments. I am pushing the next version v4 for Jerome and fixed most of your comments. Some inline comments. On Mon, Aug 08, 2022 at 04:49:57PM +0100, Lee Jones wrote: > On Tue, 26 Jul 2022, Jerome Neanne wrote: > > > The TPS65219 is a power management IC PMIC designed > > to supply a wide range of SoCs > > in both portable and stationary applications. > > Any SoC can control TPS65219 over a standard I2C interface. > > Really odd line break choices here. > > > It contains the following components: > > - Regulators. > > - Over Temperature warning and Shut down. > > - GPIOs > > - Multi Function Pins (MFP) > > > > This patch adds support for tps65219 mfd device. At this time only > > No such thing as an MFD device, what is it? PMIC, no? > > > the functionalities listed below are made available: > > > > - Regulators probe and functionalities > > - warm and cold reset support > > - SW shutdown support > > > > Signed-off-by: Jerome Neanne > > --- > > MAINTAINERS | 1 + > > drivers/mfd/Kconfig | 15 +++ > > drivers/mfd/Makefile | 1 + > > drivers/mfd/tps65219.c | 214 +++++++++++++++++++++++++++++ > > include/linux/mfd/tps65219.h | 251 +++++++++++++++++++++++++++++++++++ > > 5 files changed, 482 insertions(+) > > create mode 100644 drivers/mfd/tps65219.c > > create mode 100644 include/linux/mfd/tps65219.h > > ... > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index 858cacf659d6..a8ff3d6ea3ab 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile ... > > > +// > > +// Copyright (C) 2022 BayLibre Incorporated - https://www.baylibre.com/ > > +// > > +// This implementation derived from tps65218 authored by > > +// "J Keerthy " > > Which he probably copied from elsewhere. > > Please drop this line. > > > +// > > Drop this empty comment > > Only C++ for the SPDX line please. Mark Brown wanted this the other way around: "Please make the entire comment a C++ one so things look more intentional." https://lore.kernel.org/all/YtayikFdidxXXubS@sirena.org.uk/ Let me know what I should do here. ... > > > + if (pm_power_off) > > + dev_warn(tps->dev, "Setup as system-power-controller but pm_power_off function already registered, overwriting\n"); > > Is that really what you want? > > What about the other driver that thinks it's the > system-power-controller? Seems wrong. I am now not overwriting pm_power_off but print an error and probe successfully. ... > > diff --git a/include/linux/mfd/tps65219.h b/include/linux/mfd/tps65219.h > > new file mode 100644 > > index 000000000000..e9197ab8bc75 > > --- /dev/null > > +++ b/include/linux/mfd/tps65219.h > > @@ -0,0 +1,251 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * linux/mfd/tps65219.h > > No filenames please. They tend to become unsynced. > > > + * Functions to access TPS65219 power management chip. > > As above. > > > + * Copyright (C) 2022 BayLibre Incorporated - https://www.baylibre.com/ > > + */ > > + > > +#ifndef __LINUX_MFD_TPS65219_H > > +#define __LINUX_MFD_TPS65219_H > > LINUX? This seems to be quite common in include/linux/mfd/*.h to have __LINUX_MFD_ as prefix (probably for the directory they are in). I am assuming that's not the right style. What should be used instead? __MFD_TPS65219_H or MFD_TPS65219_H or maybe something else? ... > > + > > +/** > > + * struct tps65219 - tps65219 sub-driver chip access routines > > + * > > + * Device data may be used to access the TPS65219 chip > > + */ > > This is not a complete kerneldoc header. > > Please compile test with W=1. I didn't see any errors reported for this struct with W=1. I am assuming you meant missing field descriptions and added them. Thanks, Markus _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel