Received: by 2002:a5b:505:0:0:0:0:0 with SMTP id o5csp7312135ybp; Wed, 16 Oct 2019 07:01:54 -0700 (PDT) X-Google-Smtp-Source: APXvYqwdkCicmYGCi9WmPa9jNdCtd76OWtfS9oacdrYYhBKoCbaoZrLbJS8nQwzNHRU+irRKwhnv X-Received: by 2002:a17:906:5381:: with SMTP id g1mr39769895ejo.159.1571234513983; Wed, 16 Oct 2019 07:01:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1571234513; cv=none; d=google.com; s=arc-20160816; b=GQCkUvpUhxb4J3mm+zAgP2ckeCxEyWgr1ultNkMUgAmk+BvwEjxq/ackN/XDQy+Akh brOEGxLM6c0fOzBlx1cHM6GtIPbgMFUJOgdpMuR9fqv9J+wpExyjPrpIpkZYT9K7PdLz cRZeh06XBVBVUQhm0b0F9dpBMx7Grv5rgeJb5f90V0+BFVwI11OQ1GPbV24X/5t1qDu0 x+BOzFG2JJNlGAjGCs+jLoeVoCT3oSz1QxQjdlWtmPRrajMYR0eKOlVDTuXCZV7Hq56g 4PMAUAwnFGebGmLgKp4j+b4eLGftR5RjzVY7MheMREd+j/84HrP9C2zWbiLzMNFi18Th O1Tw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=unjDeDVFZAxp2+92fhsHf2hbKpVLtBjf1aANazCtKf0=; b=kXsdM8DwdYNAfBPunC4a9LV6YReHKQGcbtFr+nfZmQVtQF+GwIM6N1DFehx38hI4at /ktlYhZT7AySzuPqTJJjU0AY9dbVb+Gmaq4SmOiuQtJBjJbDMrGKhOdne1OgTZVVWY+o FfNJKBa57yuHOWPLuuRJnuHddn+vfZzFcVRVeNHaEraCBxzpuH3HlDcZYThIKFSrhYQh oDnR4ytYRKapyeCxDseN98FA06Og131k6hw1jC/3vDnQ4fwbvFIiQtM29Zywn8ui+X5v dxdjdfe+Umyaf1eMq+LgL8c2L0mJS8SrVPWxIMEAwGY5fWOINqaO8NXLpssQ6mxbGc0R v3dg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=nZmxqNjR; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id v6si14885998edw.51.2019.10.16.07.01.29; Wed, 16 Oct 2019 07:01:53 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=nZmxqNjR; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388107AbfJPKEt (ORCPT + 99 others); Wed, 16 Oct 2019 06:04:49 -0400 Received: from mail-wm1-f65.google.com ([209.85.128.65]:35360 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728081AbfJPKEt (ORCPT ); Wed, 16 Oct 2019 06:04:49 -0400 Received: by mail-wm1-f65.google.com with SMTP id y21so2091653wmi.0 for ; Wed, 16 Oct 2019 03:04:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=unjDeDVFZAxp2+92fhsHf2hbKpVLtBjf1aANazCtKf0=; b=nZmxqNjRCjQGVFGncOVuTBc6+OjwGuyFGh3AGeAFJXEYsby+WpEIaigWlMDz+o7bnK oMKJT+bpCqqeV7sOn0rqAEIxmmWIJyvmYpfen0q1acy4ppnHAKtbHkT9iFotr+xPUmH9 BKVSLerC2qniMyDzuyETPiIS/5GenWH1PB5t4Cv3D4mYYl3cpIvAoPCsZ6fphHyAe9SS jB/1xq8oElbyT1e/Wr+J/NYV+jXD6iLlBRVoasvgZfgtLZyF+i8wo41pKR7gVpsTjCE4 bDh5JrxQi1Mut30aY273uCLYkf5z4N2MehKOuC5lHtm81ObWJ78baovc6Nxn78qqk1kX JYAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=unjDeDVFZAxp2+92fhsHf2hbKpVLtBjf1aANazCtKf0=; b=hlN2US64nO5smmJURkokYYkJlwF/bSlnBWEz5bMrgp5qSUc4cb9BL5djvxrU6/NNY4 /ctmSBpiHBLdNGlIzICOGXclY7AcxmXZ48rhCmBj6HGGw5nzVrqTYbcBTtBrWJrwciZY F5AHn422IjRxqnCUKgH92Be6d8xx1t/fkQGS2fYK8LvKMacOiESb1QyewnN+784qvEy7 REfPqLa+sNGTKSPoAhPArWqz/G03uL0bB0Daz/qspYydM4mqCsyyT4c4tzMe6ji6TBiI /gJFPpvP45rlcwnUORxJEn/xHDbx5Rp4X6SqHYylnMcYyzfuQtTJOWYzmaqi+w9JVscX FuHA== X-Gm-Message-State: APjAAAXf2V9HPH0QfIVjrqrkK+knW8R8zPVCQUY6KiDEs8rrI/cIrKcZ WiO2Wn5sMIG/NMu37Qn56eb2FQ== X-Received: by 2002:a1c:bc07:: with SMTP id m7mr2697525wmf.103.1571220280517; Wed, 16 Oct 2019 03:04:40 -0700 (PDT) Received: from dell ([95.149.164.86]) by smtp.gmail.com with ESMTPSA id z1sm6671584wrn.57.2019.10.16.03.04.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 16 Oct 2019 03:04:39 -0700 (PDT) Date: Wed, 16 Oct 2019 11:04:38 +0100 From: Lee Jones To: Gene Chen Cc: matthias.bgg@gmail.com, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, gene_chen@richtek.com, Wilma.Wu@mediatek.com, shufan_lee@richtek.com, cy_huang@richtek.com Subject: Re: [PATCH v3] mfd: mt6360: add pmic mt6360 driver Message-ID: <20191016100438.GF4365@dell> References: <1569338741-2784-1-git-send-email-gene.chen.richtek@gmail.com> <20191004133324.GE18429@dell> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 15 Oct 2019, Gene Chen wrote: > Hi Lee, > > we find OF_MFD_CELL is not defined in mfd/core.h, which is ready to > merge to next kernel version > https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next-history/+/master/Next/merge.log It's here: Merging mfd/for-mfd-next (38a6fc63a3ea mfd: db8500-prcmu: Example using new OF_MFD_CELL/MFD_CELL_BASIC MACROs) $ git merge mfd/for-mfd-next Merge made by the 'recursive' strategy. drivers/mfd/ab8500-core.c | 138 +++++++++++++------------------------------ drivers/mfd/db8500-prcmu.c | 21 +++---- drivers/mfd/intel-lpss-pci.c | 28 ++++++--- drivers/mfd/ipaq-micro.c | 6 +- drivers/mfd/rk808.c | 22 ++----- include/linux/mfd/core.h | 29 +++++++++ <===== [THIS ONE] include/linux/mfd/rk808.h | 2 +- 7 files changed, 105 insertions(+), 141 deletions(-) https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next-history/+/master/Next/merge.log#4470 > may i ask how can i upstream without this definition? > e.g. we pull this patch and build pass, new patch without add macro define > > 2019-10-08 2:24 GMT+08:00, Gene Chen : > > Hi Jones, > > > > Thanks for review, we will fix some comment which your suggestion in next > > patch > > > > Lee Jones 於 2019年10月4日 週五 下午9:33寫道： > >> > >> Wolfram, > >> > >> Would you be kind enough to grep for your name below? > >> > >> On Tue, 24 Sep 2019, Gene Chen wrote: > >> > >> > From: Gene Chen > >> > > >> > Add mfd driver for mt6360 pmic chip include > >> > Battery Charger/USB_PD/Flash LED/RGB LED/LDO/Buck > >> > > >> > Signed-off-by: Gene Chen >> > --- > >> > drivers/mfd/Kconfig | 12 + > >> > drivers/mfd/Makefile | 1 + > >> > drivers/mfd/mt6360-core.c | 463 > >> > +++++++++++++++++++++++++++++++++++++ > >> > include/linux/mfd/mt6360-private.h | 279 ++++++++++++++++++++++ > >> > include/linux/mfd/mt6360.h | 33 +++ > >> > 5 files changed, 788 insertions(+) > >> > create mode 100644 drivers/mfd/mt6360-core.c > >> > create mode 100644 include/linux/mfd/mt6360-private.h > >> > create mode 100644 include/linux/mfd/mt6360.h > >> > > >> > changelogs between v1 & v2 > >> > - include missing header file > >> > > >> > changelogs between v2 & v3 > >> > - add changelogs > >> > > >> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > >> > index f129f96..a422c76 100644 > >> > --- a/drivers/mfd/Kconfig > >> > +++ b/drivers/mfd/Kconfig > >> > @@ -862,6 +862,18 @@ config MFD_MAX8998 > >> > additional drivers must be enabled in order to use the > >> > functionality > >> > of the device. > >> > > >> > +config MFD_MT6360 > >> > + tristate "Mediatek MT6360 SubPMIC" > >> > + select MFD_CORE > >> > + select REGMAP_I2C > >> > + select REGMAP_IRQ > >> > + depends on I2C > >> > + help > >> > + Say Y here to enable MT6360 PMU/PMIC/LDO functional support. > >> > + PMU part include charger, flashlight, rgb led > >> > + PMIC part include 2-channel BUCKs and 2-channel LDOs > >> > + LDO part include 4-channel LDOs > >> > >> PMU part includes Charger, Flashlight, RGB and LED > >> PMIC part includes 2-channel BUCKs and 2-channel LDOs > >> LDO part includes 4-channel LDOs > >> > > > > ACK. RGB LED is one of indicator light, only single feature > > > >> > config MFD_MT6397 > >> > tristate "MediaTek MT6397 PMIC Support" > >> > select MFD_CORE > >> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > >> > index f026ada..77a8f0b 100644 > >> > --- a/drivers/mfd/Makefile > >> > +++ b/drivers/mfd/Makefile > >> > @@ -241,6 +241,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC) += > >> > intel-soc-pmic.o > >> > obj-$(CONFIG_INTEL_SOC_PMIC_BXTWC) += intel_soc_pmic_bxtwc.o > >> > obj-$(CONFIG_INTEL_SOC_PMIC_CHTWC) += intel_soc_pmic_chtwc.o > >> > obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += > >> > intel_soc_pmic_chtdc_ti.o > >> > +obj-$(CONFIG_MFD_MT6360) += mt6360-core.o > >> > obj-$(CONFIG_MFD_MT6397) += mt6397-core.o > >> > > >> > obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o > >> > diff --git a/drivers/mfd/mt6360-core.c b/drivers/mfd/mt6360-core.c > >> > new file mode 100644 > >> > index 0000000..d3580618 > >> > --- /dev/null > >> > +++ b/drivers/mfd/mt6360-core.c > >> > @@ -0,0 +1,463 @@ > >> > +// SPDX-License-Identifier: GPL-2.0 > >> > +/* > >> > + * Copyright (c) 2019 MediaTek Inc. > >> > + */ > >> > + > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > + > >> > +#include > >> > +#include > >> > + > >> > +/* reg 0 -> 0 ~ 7 */ > >> > +#define MT6360_CHG_TREG_EVT (4) > >> > +#define MT6360_CHG_AICR_EVT (5) > >> > +#define MT6360_CHG_MIVR_EVT (6) > >> > +#define MT6360_PWR_RDY_EVT (7) > >> > +/* REG 1 -> 8 ~ 15 */ > >> > +#define MT6360_CHG_BATSYSUV_EVT (9) > >> > +#define MT6360_FLED_CHG_VINOVP_EVT (11) > >> > +#define MT6360_CHG_VSYSUV_EVT (12) > >> > +#define MT6360_CHG_VSYSOV_EVT (13) > >> > +#define MT6360_CHG_VBATOV_EVT (14) > >> > +#define MT6360_CHG_VBUSOV_EVT (15) > >> > +/* REG 2 -> 16 ~ 23 */ > >> > +/* REG 3 -> 24 ~ 31 */ > >> > +#define MT6360_WD_PMU_DET (25) > >> > +#define MT6360_WD_PMU_DONE (26) > >> > +#define MT6360_CHG_TMRI (27) > >> > +#define MT6360_CHG_ADPBADI (29) > >> > +#define MT6360_CHG_RVPI (30) > >> > +#define MT6360_OTPI (31) > >> > +/* REG 4 -> 32 ~ 39 */ > >> > +#define MT6360_CHG_AICCMEASL (32) > >> > +#define MT6360_CHGDET_DONEI (34) > >> > +#define MT6360_WDTMRI (35) > >> > +#define MT6360_SSFINISHI (36) > >> > +#define MT6360_CHG_RECHGI (37) > >> > +#define MT6360_CHG_TERMI (38) > >> > +#define MT6360_CHG_IEOCI (39) > >> > +/* REG 5 -> 40 ~ 47 */ > >> > +#define MT6360_PUMPX_DONEI (40) > >> > +#define MT6360_BAT_OVP_ADC_EVT (41) > >> > +#define MT6360_TYPEC_OTP_EVT (42) > >> > +#define MT6360_ADC_WAKEUP_EVT (43) > >> > +#define MT6360_ADC_DONEI (44) > >> > +#define MT6360_BST_BATUVI (45) > >> > +#define MT6360_BST_VBUSOVI (46) > >> > +#define MT6360_BST_OLPI (47) > >> > +/* REG 6 -> 48 ~ 55 */ > >> > +#define MT6360_ATTACH_I (48) > >> > +#define MT6360_DETACH_I (49) > >> > +#define MT6360_QC30_STPDONE (51) > >> > +#define MT6360_QC_VBUSDET_DONE (52) > >> > +#define MT6360_HVDCP_DET (53) > >> > +#define MT6360_CHGDETI (54) > >> > +#define MT6360_DCDTI (55) > >> > +/* REG 7 -> 56 ~ 63 */ > >> > +#define MT6360_FOD_DONE_EVT (56) > >> > +#define MT6360_FOD_OV_EVT (57) > >> > +#define MT6360_CHRDET_UVP_EVT (58) > >> > +#define MT6360_CHRDET_OVP_EVT (59) > >> > +#define MT6360_CHRDET_EXT_EVT (60) > >> > +#define MT6360_FOD_LR_EVT (61) > >> > +#define MT6360_FOD_HR_EVT (62) > >> > +#define MT6360_FOD_DISCHG_FAIL_EVT (63) > >> > +/* REG 8 -> 64 ~ 71 */ > >> > +#define MT6360_USBID_EVT (64) > >> > +#define MT6360_APWDTRST_EVT (65) > >> > +#define MT6360_EN_EVT (66) > >> > +#define MT6360_QONB_RST_EVT (67) > >> > +#define MT6360_MRSTB_EVT (68) > >> > +#define MT6360_OTP_EVT (69) > >> > +#define MT6360_VDDAOV_EVT (70) > >> > +#define MT6360_SYSUV_EVT (71) > >> > +/* REG 9 -> 72 ~ 79 */ > >> > +#define MT6360_FLED_STRBPIN_EVT (72) > >> > +#define MT6360_FLED_TORPIN_EVT (73) > >> > +#define MT6360_FLED_TX_EVT (74) > >> > +#define MT6360_FLED_LVF_EVT (75) > >> > +#define MT6360_FLED2_SHORT_EVT (78) > >> > +#define MT6360_FLED1_SHORT_EVT (79) > >> > +/* REG 10 -> 80 ~ 87 */ > >> > +#define MT6360_FLED2_STRB_EVT (80) > >> > +#define MT6360_FLED1_STRB_EVT (81) > >> > +#define MT6360_FLED2_STRB_TO_EVT (82) > >> > +#define MT6360_FLED1_STRB_TO_EVT (83) > >> > +#define MT6360_FLED2_TOR_EVT (84) > >> > +#define MT6360_FLED1_TOR_EVT (85) > >> > +/* REG 11 -> 88 ~ 95 */ > >> > +/* REG 12 -> 96 ~ 103 */ > >> > +#define MT6360_BUCK1_PGB_EVT (96) > >> > +#define MT6360_BUCK1_OC_EVT (100) > >> > +#define MT6360_BUCK1_OV_EVT (101) > >> > +#define MT6360_BUCK1_UV_EVT (102) > >> > +/* REG 13 -> 104 ~ 111 */ > >> > +#define MT6360_BUCK2_PGB_EVT (104) > >> > +#define MT6360_BUCK2_OC_EVT (108) > >> > +#define MT6360_BUCK2_OV_EVT (109) > >> > +#define MT6360_BUCK2_UV_EVT (110) > >> > +/* REG 14 -> 112 ~ 119 */ > >> > +#define MT6360_LDO1_OC_EVT (113) > >> > +#define MT6360_LDO2_OC_EVT (114) > >> > +#define MT6360_LDO3_OC_EVT (115) > >> > +#define MT6360_LDO5_OC_EVT (117) > >> > +#define MT6360_LDO6_OC_EVT (118) > >> > +#define MT6360_LDO7_OC_EVT (119) > >> > +/* REG 15 -> 120 ~ 127 */ > >> > +#define MT6360_LDO1_PGB_EVT (121) > >> > +#define MT6360_LDO2_PGB_EVT (122) > >> > +#define MT6360_LDO3_PGB_EVT (123) > >> > +#define MT6360_LDO5_PGB_EVT (125) > >> > +#define MT6360_LDO6_PGB_EVT (126) > >> > +#define MT6360_LDO7_PGB_EVT (127) > >> > + > >> > +#define MT6360_REGMAP_IRQ_REG(_irq_evt) \ > >> > + REGMAP_IRQ_REG(_irq_evt, (_irq_evt) / 8, BIT((_irq_evt) % 8)) > >> > + > >> > +#define MT6360_MFD_CELL(_name) \ > >> > + { \ > >> > + .name = #_name, \ > >> > + .of_compatible = "mediatek," #_name, \ > >> > + .num_resources = ARRAY_SIZE(_name##_resources), \ > >> > + .resources = _name##_resources, \ > >> > + } > >> > >> Please do not roll your own MACROS like this. If they are helpful for > >> you, they are likely to be helpful for others. However, this is your > >> lucky day, as we've been here before. Please rebase onto the MFD tree > >> where you will find some pre-authored macros which aren't too > >> dissimilar to this one. Please use one of those instead. > >> > > > > ACK > > > >> > +static const struct regmap_irq mt6360_pmu_irqs[] = { > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_TREG_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_AICR_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_MIVR_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_PWR_RDY_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_BATSYSUV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED_CHG_VINOVP_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_VSYSUV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_VSYSOV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_VBATOV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_VBUSOV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_WD_PMU_DET), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_WD_PMU_DONE), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_TMRI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_ADPBADI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_RVPI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_OTPI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_AICCMEASL), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHGDET_DONEI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_WDTMRI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_SSFINISHI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_RECHGI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_TERMI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_IEOCI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_PUMPX_DONEI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHG_TREG_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BAT_OVP_ADC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_TYPEC_OTP_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_ADC_WAKEUP_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_ADC_DONEI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BST_BATUVI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BST_VBUSOVI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BST_OLPI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_ATTACH_I), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_DETACH_I), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_QC30_STPDONE), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_QC_VBUSDET_DONE), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_HVDCP_DET), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHGDETI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_DCDTI), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FOD_DONE_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FOD_OV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHRDET_UVP_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHRDET_OVP_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_CHRDET_EXT_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FOD_LR_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FOD_HR_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FOD_DISCHG_FAIL_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_USBID_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_APWDTRST_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_EN_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_QONB_RST_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_MRSTB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_OTP_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_VDDAOV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_SYSUV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED_STRBPIN_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED_TORPIN_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED_TX_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED_LVF_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED2_SHORT_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED1_SHORT_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED2_STRB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED1_STRB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED2_STRB_TO_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED1_STRB_TO_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED2_TOR_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_FLED1_TOR_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_OV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK1_UV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_OV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_BUCK2_UV_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO1_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO2_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO3_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO5_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO6_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO7_OC_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO1_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO2_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO3_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO5_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO6_PGB_EVT), > >> > + MT6360_REGMAP_IRQ_REG(MT6360_LDO7_PGB_EVT), > >> > +}; > >> > + > >> > +static int mt6360_pmu_handle_post_irq(void *irq_drv_data) > >> > +{ > >> > + struct mt6360_pmu_info *mpi = irq_drv_data; > >> > + > >> > + return regmap_update_bits(mpi->regmap, > >> > + MT6360_PMU_IRQ_SET, MT6360_IRQ_RETRIG, > >> > MT6360_IRQ_RETRIG); > >> > +} > >> > + > >> > +static const struct regmap_irq_chip mt6360_pmu_irq_chip = { > >> > + .irqs = mt6360_pmu_irqs, > >> > + .num_irqs = ARRAY_SIZE(mt6360_pmu_irqs), > >> > + .num_regs = MT6360_PMU_IRQ_REGNUM, > >> > + .mask_base = MT6360_PMU_CHG_MASK1, > >> > + .status_base = MT6360_PMU_CHG_IRQ1, > >> > + .ack_base = MT6360_PMU_CHG_IRQ1, > >> > + .init_ack_masked = true, > >> > + .use_ack = true, > >> > + .handle_post_irq = mt6360_pmu_handle_post_irq, > >> > +}; > >> > + > >> > +static const struct regmap_config mt6360_pmu_regmap_config = { > >> > + .reg_bits = 8, > >> > + .val_bits = 8, > >> > + .max_register = MT6360_PMU_MAXREG, > >> > +}; > >> > + > >> > +static const struct resource mt6360_adc_resources[] = { > >> > + DEFINE_RES_IRQ_NAMED(MT6360_ADC_DONEI, "adc_donei"), > >> > +}; > >> > + > >> > +static const struct resource mt6360_chg_resources[] = { > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_TREG_EVT, "chg_treg_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_PWR_RDY_EVT, "pwr_rdy_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_BATSYSUV_EVT, > >> > "chg_batsysuv_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_VSYSUV_EVT, "chg_vsysuv_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_VSYSOV_EVT, "chg_vsysov_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_VBATOV_EVT, "chg_vbatov_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_VBUSOV_EVT, "chg_vbusov_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_AICCMEASL, "chg_aiccmeasl"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_WDTMRI, "wdtmri"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_RECHGI, "chg_rechgi"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_TERMI, "chg_termi"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHG_IEOCI, "chg_ieoci"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_PUMPX_DONEI, "pumpx_donei"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_ATTACH_I, "attach_i"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_CHRDET_EXT_EVT, "chrdet_ext_evt"), > >> > +}; > >> > + > >> > +static const struct resource mt6360_led_resources[] = { > >> > + DEFINE_RES_IRQ_NAMED(MT6360_FLED_CHG_VINOVP_EVT, > >> > "fled_chg_vinovp_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_FLED_LVF_EVT, "fled_lvf_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_FLED2_SHORT_EVT, "fled2_short_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_FLED1_SHORT_EVT, "fled1_short_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_FLED2_STRB_TO_EVT, > >> > "fled2_strb_to_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_FLED1_STRB_TO_EVT, > >> > "fled1_strb_to_evt"), > >> > +}; > >> > + > >> > +static const struct resource mt6360_pmic_resources[] = { > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_PGB_EVT, "buck1_pgb_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_OC_EVT, "buck1_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_OV_EVT, "buck1_ov_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK1_UV_EVT, "buck1_uv_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_PGB_EVT, "buck2_pgb_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_OC_EVT, "buck2_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_OV_EVT, "buck2_ov_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_BUCK2_UV_EVT, "buck2_uv_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO6_OC_EVT, "ldo6_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO7_OC_EVT, "ldo7_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO6_PGB_EVT, "ldo6_pgb_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO7_PGB_EVT, "ldo7_pgb_evt"), > >> > +}; > >> > + > >> > +static const struct resource mt6360_ldo_resources[] = { > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO1_OC_EVT, "ldo1_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO2_OC_EVT, "ldo2_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO3_OC_EVT, "ldo3_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO5_OC_EVT, "ldo5_oc_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO1_PGB_EVT, "ldo1_pgb_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO2_PGB_EVT, "ldo2_pgb_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO3_PGB_EVT, "ldo3_pgb_evt"), > >> > + DEFINE_RES_IRQ_NAMED(MT6360_LDO5_PGB_EVT, "ldo5_pgb_evt"), > >> > +}; > >> > + > >> > +static const struct mfd_cell mt6360_devs[] = { > >> > + MT6360_MFD_CELL(mt6360_adc), > >> > + MT6360_MFD_CELL(mt6360_chg), > >> > + MT6360_MFD_CELL(mt6360_led), > >> > + MT6360_MFD_CELL(mt6360_pmic), > >> > + MT6360_MFD_CELL(mt6360_ldo), > >> > + /* tcpc dev */ > >> > + { > >> > + .name = "mt6360_tcpc", > >> > + .of_compatible = "mediatek,mt6360_tcpc", > >> > >> There is a macro for this too (OF_MFD_CELL()) > >> > > > > ACK > > > >> > + }, > >> > +}; > >> > + > >> > +static const unsigned short mt6360_slave_addr[MT6360_SLAVE_MAX] = { > >> > + MT6360_PMU_SLAVEID, > >> > + MT6360_PMIC_SLAVEID, > >> > + MT6360_LDO_SLAVEID, > >> > + MT6360_TCPC_SLAVEID, > >> > +}; > >> > + > >> > +static int mt6360_pmu_probe(struct i2c_client *client, > >> > + const struct i2c_device_id *id) > >> > >> If you use .probe_new (see below) you can omit the 'id' param. > >> > > > > ACK > > > >> > +{ > >> > + struct mt6360_pmu_info *mpi; > >> > >> We normally call this ddata. > >> > > > > ACK > > > >> > + unsigned int reg_data = 0; > >> > + int i, ret; > >> > + > >> > + mpi = devm_kzalloc(&client->dev, sizeof(*mpi), GFP_KERNEL); > >> > + if (!mpi) > >> > + return -ENOMEM; > >> > >> '\n' here. > >> > > > > ACK > > > >> > + mpi->dev = &client->dev; > >> > + i2c_set_clientdata(client, mpi); > >> > + > >> > + /* regmap regiser */ > >> > >> This comment is spelt incorrectly and doesn't really add anything. > >> > > > > ACK > > > >> > + mpi->regmap = devm_regmap_init_i2c(client, > >> > &mt6360_pmu_regmap_config); > >> > + if (IS_ERR(mpi->regmap)) { > >> > + dev_err(&client->dev, "regmap register fail\n"); > >> > >> "Failed to register regmap" > >> > > > > ACK > > > >> > + return PTR_ERR(mpi->regmap); > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + /* chip id check */ > >> > >> Again, the code is pretty obvious. > >> > > > > ACK > > > >> > + ret = regmap_read(mpi->regmap, MT6360_PMU_DEV_INFO, ®_data); > >> > + if (ret < 0) { > >> > + dev_err(&client->dev, "device not found\n"); > >> > >> "Device not found" > >> > > > > ACK > > > >> > + return ret; > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + if ((reg_data & CHIP_VEN_MASK) != CHIP_VEN_MT6360) { > >> > + dev_err(&client->dev, "not mt6360 chip\n"); > >> > >> "Device not supported" > >> > > > > ACK > > > >> > + return -ENODEV; > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + mpi->chip_rev = reg_data & CHIP_REV_MASK; > >> > >> Do this above the check, then do > >> > >> (mpi->chip_rev != CHIP_VEN_MT6360) > >> > >> ... above. > >> > > > > ACK > > > >> > + /* irq register */ > >> > >> Please remove all of these comments. > >> > > > > ACK > > > >> > + memcpy(&mpi->irq_chip, &mt6360_pmu_irq_chip, > >> > sizeof(mpi->irq_chip)); > >> > >> Why do we need to make a copy of it? > >> > > > > consider of using mutiple mt6360 chips, we can seperate diff i2c > > irq_chip.name by device_name originally > > but we can't find silimar case by overview other mfd driver > > we will delete this > > > >> > + mpi->irq_chip.name = dev_name(&client->dev); > >> > >> We already know the name. Why do we need to do this dynamically? > >> > > > > same as above > > > >> > + mpi->irq_chip.irq_drv_data = mpi; > >> > >> We already saved ddata. Why do we need to save it here as well? > >> > > > > we implement ops ".handle_post_irq" for irq retrigger when irq stuck keep > > low > > > >> > + ret = devm_regmap_add_irq_chip(&client->dev, mpi->regmap, > >> > client->irq, > >> > + IRQF_TRIGGER_FALLING, 0, > >> > &mpi->irq_chip, > >> > + &mpi->irq_data); > >> > + if (ret < 0) { > >> > >> Is (ret > 0) valid? > >> > > > > we consider mt6360 driver need add irq_chip for full functionality > > > >> > + dev_err(&client->dev, "regmap irq chip add fail\n"); > >> > >> "Failed to add Regmap IRQ Chip" > >> > > > > ACK > > > >> > + return ret; > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + /* new i2c slave device */ > >> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) { > >> > + if (mt6360_slave_addr[i] == client->addr) { > >> > + mpi->i2c[i] = client; > >> > + continue; > >> > + } > >> > + mpi->i2c[i] = i2c_new_dummy(client->adapter, > >> > + mt6360_slave_addr[i]); > >> > + if (!mpi->i2c[i]) { > >> > + dev_err(&client->dev, "new i2c dev [%d] fail\n", > >> > i); > >> > + ret = -ENODEV; > >> > + goto out; > >> > + } > >> > + i2c_set_clientdata(mpi->i2c[i], mpi); > >> > + } > >> > >> This doesn't look right to me. > >> > >> Wolfram, would you be kind enough to take a look? > >> > >> '\n' here. > >> > > > > ACK > > > >> > + /* mfd cell register */ > >> > + ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, > >> > + mt6360_devs, ARRAY_SIZE(mt6360_devs), > >> > NULL, > >> > + 0, > >> > regmap_irq_get_domain(mpi->irq_data)); > >> > + if (ret < 0) { > >> > + dev_err(&client->dev, "mfd add cells fail\n"); > >> > + goto out; > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + dev_info(&client->dev, "Successfully probed\n"); > >> > >> Please remove this line. It doesn't provide anything. > >> > > > > ACK > > > >> > + return 0; > >> > +out: > >> > + while (--i >= 0) { > >> > + if (mpi->i2c[i]->addr == client->addr) > >> > + continue; > >> > + i2c_unregister_device(mpi->i2c[i]); > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + return ret; > >> > +} > >> > + > >> > +static int mt6360_pmu_remove(struct i2c_client *client) > >> > +{ > >> > + struct mt6360_pmu_info *mpi = i2c_get_clientdata(client); > >> > + int i; > >> > + > >> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) { > >> > + if (mpi->i2c[i]->addr == client->addr) > >> > + continue; > >> > + i2c_unregister_device(mpi->i2c[i]); > >> > + } > >> > >> '\n' here. > >> > > > > ACK > > > >> > + return 0; > >> > +} > >> > + > >> > +static int __maybe_unused mt6360_pmu_suspend(struct device *dev) > >> > +{ > >> > + struct i2c_client *i2c = to_i2c_client(dev); > >> > + > >> > + if (device_may_wakeup(dev)) > >> > + enable_irq_wake(i2c->irq); > >> > >> '\n' here. > >> > > > > ACK > > > >> > + return 0; > >> > +} > >> > + > >> > +static int __maybe_unused mt6360_pmu_resume(struct device *dev) > >> > +{ > >> > + > >> > + struct i2c_client *i2c = to_i2c_client(dev); > >> > + > >> > + if (device_may_wakeup(dev)) > >> > + disable_irq_wake(i2c->irq); > >> > >> '\n' here. > >> > > > > ACK > > > >> > + return 0; > >> > +} > >> > + > >> > +static SIMPLE_DEV_PM_OPS(mt6360_pmu_pm_ops, > >> > + mt6360_pmu_suspend, mt6360_pmu_resume); > >> > + > >> > +static const struct of_device_id __maybe_unused mt6360_pmu_of_id[] = { > >> > + { .compatible = "mediatek,mt6360_pmu", }, > >> > + {}, > >> > +}; > >> > +MODULE_DEVICE_TABLE(of, mt6360_pmu_of_id); > >> > + > >> > +static const struct i2c_device_id mt6360_pmu_id[] = { > >> > + { "mt6360_pmu", 0 }, > >> > + {}, > >> > +}; > >> > +MODULE_DEVICE_TABLE(i2c, mt6360_pmu_id); > >> > >> If you use .probe_new (see below, you can remove this table. > >> > > > > ACK > > > >> > +static struct i2c_driver mt6360_pmu_driver = { > >> > + .driver = { > >> > + .name = "mt6360_pmu", > >> > + .owner = THIS_MODULE, > >> > >> This is no longer required. > >> > > > > ACK > > > >> > + .pm = &mt6360_pmu_pm_ops, > >> > + .of_match_table = of_match_ptr(mt6360_pmu_of_id), > >> > + }, > >> > + .probe = mt6360_pmu_probe, > >> > >> Use .probe_new here. > >> > > > > ACK > > > >> > + .remove = mt6360_pmu_remove, > >> > + .id_table = mt6360_pmu_id, > >> > +}; > >> > +module_i2c_driver(mt6360_pmu_driver); > >> > + > >> > +MODULE_AUTHOR("CY_Huang "); > >> > +MODULE_DESCRIPTION("MT6360 PMU I2C Driver"); > >> > +MODULE_LICENSE("GPL"); > >> > +MODULE_VERSION("1.0.0"); > >> > diff --git a/include/linux/mfd/mt6360-private.h > >> > b/include/linux/mfd/mt6360-private.h > >> > new file mode 100644 > >> > index 0000000..b07b3d9 > >> > --- /dev/null > >> > +++ b/include/linux/mfd/mt6360-private.h > >> > >> Why do you need 2 header files? > >> > > > > According to our architecture as attachment, > > mt6360 have 4 i2c slave address for different parts > > so we set whole register table in mt6360-private.h, it will be > > included by other modules > > we will delete it next patch > > and we will add until we use it > > > >> > @@ -0,0 +1,279 @@ > >> > +/* SPDX-License-Identifier: GPL-2.0 */ > >> > +/* > >> > + * Copyright (c) 2019 MediaTek Inc. > >> > + */ > >> > + > >> > +#ifndef __MT6360_PRIVATE_H__ > >> > +#define __MT6360_PRIVATE_H__ > >> > >> __MFD_MT6360_H__ > >> > >> > +#include > >> > + > >> > +/* PMU register defininition */ > >> > +#define MT6360_PMU_DEV_INFO (0x00) > >> > +#define MT6360_PMU_CORE_CTRL1 (0x01) > >> > +#define MT6360_PMU_RST1 (0x02) > >> > +#define MT6360_PMU_CRCEN (0x03) > >> > +#define MT6360_PMU_RST_PAS_CODE1 (0x04) > >> > +#define MT6360_PMU_RST_PAS_CODE2 (0x05) > >> > +#define MT6360_PMU_CORE_CTRL2 (0x06) > >> > +#define MT6360_PMU_TM_PAS_CODE1 (0x07) > >> > +#define MT6360_PMU_TM_PAS_CODE2 (0x08) > >> > +#define MT6360_PMU_TM_PAS_CODE3 (0x09) > >> > +#define MT6360_PMU_TM_PAS_CODE4 (0x0A) > >> > +#define MT6360_PMU_IRQ_IND (0x0B) > >> > +#define MT6360_PMU_IRQ_MASK (0x0C) > >> > +#define MT6360_PMU_IRQ_SET (0x0D) > >> > +#define MT6360_PMU_SHDN_CTRL (0x0E) > >> > +#define MT6360_PMU_TM_INF (0x0F) > >> > +#define MT6360_PMU_I2C_CTRL (0x10) > >> > +#define MT6360_PMU_CHG_CTRL1 (0x11) > >> > +#define MT6360_PMU_CHG_CTRL2 (0x12) > >> > +#define MT6360_PMU_CHG_CTRL3 (0x13) > >> > +#define MT6360_PMU_CHG_CTRL4 (0x14) > >> > +#define MT6360_PMU_CHG_CTRL5 (0x15) > >> > +#define MT6360_PMU_CHG_CTRL6 (0x16) > >> > +#define MT6360_PMU_CHG_CTRL7 (0x17) > >> > +#define MT6360_PMU_CHG_CTRL8 (0x18) > >> > +#define MT6360_PMU_CHG_CTRL9 (0x19) > >> > +#define MT6360_PMU_CHG_CTRL10 (0x1A) > >> > +#define MT6360_PMU_CHG_CTRL11 (0x1B) > >> > +#define MT6360_PMU_CHG_CTRL12 (0x1C) > >> > +#define MT6360_PMU_CHG_CTRL13 (0x1D) > >> > +#define MT6360_PMU_CHG_CTRL14 (0x1E) > >> > +#define MT6360_PMU_CHG_CTRL15 (0x1F) > >> > +#define MT6360_PMU_CHG_CTRL16 (0x20) > >> > +#define MT6360_PMU_CHG_AICC_RESULT (0x21) > >> > +#define MT6360_PMU_DEVICE_TYPE (0x22) > >> > +#define MT6360_PMU_QC_CONTROL1 (0x23) > >> > +#define MT6360_PMU_QC_CONTROL2 (0x24) > >> > +#define MT6360_PMU_QC30_CONTROL1 (0x25) > >> > +#define MT6360_PMU_QC30_CONTROL2 (0x26) > >> > +#define MT6360_PMU_USB_STATUS1 (0x27) > >> > +#define MT6360_PMU_QC_STATUS1 (0x28) > >> > +#define MT6360_PMU_QC_STATUS2 (0x29) > >> > +#define MT6360_PMU_CHG_PUMP (0x2A) > >> > +#define MT6360_PMU_CHG_CTRL17 (0x2B) > >> > +#define MT6360_PMU_CHG_CTRL18 (0x2C) > >> > +#define MT6360_PMU_CHRDET_CTRL1 (0x2D) > >> > +#define MT6360_PMU_CHRDET_CTRL2 (0x2E) > >> > +#define MT6360_PMU_DPDN_CTRL (0x2F) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL1 (0x30) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL2 (0x31) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL3 (0x32) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL4 (0x33) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL5 (0x34) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL6 (0x35) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL7 (0x36) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL8 (0x37) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL9 (0x38) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL10 (0x39) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL11 (0x3A) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL12 (0x3B) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL13 (0x3C) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL14 (0x3D) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL15 (0x3E) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL16 (0x3F) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL17 (0x40) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL18 (0x41) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL19 (0x42) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL20 (0x43) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL21 (0x44) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL22 (0x45) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL23 (0x46) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL24 (0x47) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL25 (0x48) > >> > +#define MT6360_PMU_BC12_CTRL (0x49) > >> > +#define MT6360_PMU_CHG_STAT (0x4A) > >> > +#define MT6360_PMU_RESV1 (0x4B) > >> > +#define MT6360_PMU_TYPEC_OTP_TH_SEL_CODEH (0x4E) > >> > +#define MT6360_PMU_TYPEC_OTP_TH_SEL_CODEL (0x4F) > >> > +#define MT6360_PMU_TYPEC_OTP_HYST_TH (0x50) > >> > +#define MT6360_PMU_TYPEC_OTP_CTRL (0x51) > >> > +#define MT6360_PMU_ADC_BAT_DATA_H (0x52) > >> > +#define MT6360_PMU_ADC_BAT_DATA_L (0x53) > >> > +#define MT6360_PMU_IMID_BACKBST_ON (0x54) > >> > +#define MT6360_PMU_IMID_BACKBST_OFF (0x55) > >> > +#define MT6360_PMU_ADC_CONFIG (0x56) > >> > +#define MT6360_PMU_ADC_EN2 (0x57) > >> > +#define MT6360_PMU_ADC_IDLE_T (0x58) > >> > +#define MT6360_PMU_ADC_RPT_1 (0x5A) > >> > +#define MT6360_PMU_ADC_RPT_2 (0x5B) > >> > +#define MT6360_PMU_ADC_RPT_3 (0x5C) > >> > +#define MT6360_PMU_ADC_RPT_ORG1 (0x5D) > >> > +#define MT6360_PMU_ADC_RPT_ORG2 (0x5E) > >> > +#define MT6360_PMU_BAT_OVP_TH_SEL_CODEH (0x5F) > >> > +#define MT6360_PMU_BAT_OVP_TH_SEL_CODEL (0x60) > >> > +#define MT6360_PMU_CHG_CTRL19 (0x61) > >> > +#define MT6360_PMU_VDDASUPPLY (0x62) > >> > +#define MT6360_PMU_BC12_MANUAL (0x63) > >> > +#define MT6360_PMU_CHGDET_FUNC (0x64) > >> > +#define MT6360_PMU_FOD_CTRL (0x65) > >> > +#define MT6360_PMU_CHG_CTRL20 (0x66) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL26 (0x67) > >> > +#define MT6360_PMU_CHG_HIDDEN_CTRL27 (0x68) > >> > +#define MT6360_PMU_RESV2 (0x69) > >> > +#define MT6360_PMU_USBID_CTRL1 (0x6D) > >> > +#define MT6360_PMU_USBID_CTRL2 (0x6E) > >> > +#define MT6360_PMU_USBID_CTRL3 (0x6F) > >> > +#define MT6360_PMU_FLED_CFG (0x70) > >> > +#define MT6360_PMU_RESV3 (0x71) > >> > +#define MT6360_PMU_FLED1_CTRL (0x72) > >> > +#define MT6360_PMU_FLED_STRB_CTRL (0x73) > >> > +#define MT6360_PMU_FLED1_STRB_CTRL2 (0x74) > >> > +#define MT6360_PMU_FLED1_TOR_CTRL (0x75) > >> > +#define MT6360_PMU_FLED2_CTRL (0x76) > >> > +#define MT6360_PMU_RESV4 (0x77) > >> > +#define MT6360_PMU_FLED2_STRB_CTRL2 (0x78) > >> > +#define MT6360_PMU_FLED2_TOR_CTRL (0x79) > >> > +#define MT6360_PMU_FLED_VMIDTRK_CTRL1 (0x7A) > >> > +#define MT6360_PMU_FLED_VMID_RTM (0x7B) > >> > +#define MT6360_PMU_FLED_VMIDTRK_CTRL2 (0x7C) > >> > +#define MT6360_PMU_FLED_PWSEL (0x7D) > >> > +#define MT6360_PMU_FLED_EN (0x7E) > >> > +#define MT6360_PMU_FLED_Hidden1 (0x7F) > >> > +#define MT6360_PMU_RGB_EN (0x80) > >> > +#define MT6360_PMU_RGB1_ISNK (0x81) > >> > +#define MT6360_PMU_RGB2_ISNK (0x82) > >> > +#define MT6360_PMU_RGB3_ISNK (0x83) > >> > +#define MT6360_PMU_RGB_ML_ISNK (0x84) > >> > +#define MT6360_PMU_RGB1_DIM (0x85) > >> > +#define MT6360_PMU_RGB2_DIM (0x86) > >> > +#define MT6360_PMU_RGB3_DIM (0x87) > >> > +#define MT6360_PMU_RESV5 (0x88) > >> > +#define MT6360_PMU_RGB12_Freq (0x89) > >> > +#define MT6360_PMU_RGB34_Freq (0x8A) > >> > +#define MT6360_PMU_RGB1_Tr (0x8B) > >> > +#define MT6360_PMU_RGB1_Tf (0x8C) > >> > +#define MT6360_PMU_RGB1_TON_TOFF (0x8D) > >> > +#define MT6360_PMU_RGB2_Tr (0x8E) > >> > +#define MT6360_PMU_RGB2_Tf (0x8F) > >> > +#define MT6360_PMU_RGB2_TON_TOFF (0x90) > >> > +#define MT6360_PMU_RGB3_Tr (0x91) > >> > +#define MT6360_PMU_RGB3_Tf (0x92) > >> > +#define MT6360_PMU_RGB3_TON_TOFF (0x93) > >> > +#define MT6360_PMU_RGB_Hidden_CTRL1 (0x94) > >> > +#define MT6360_PMU_RGB_Hidden_CTRL2 (0x95) > >> > +#define MT6360_PMU_RESV6 (0x97) > >> > +#define MT6360_PMU_SPARE1 (0x9A) > >> > +#define MT6360_PMU_SPARE2 (0xA0) > >> > +#define MT6360_PMU_SPARE3 (0xB0) > >> > +#define MT6360_PMU_SPARE4 (0xC0) > >> > +#define MT6360_PMU_CHG_IRQ1 (0xD0) > >> > +#define MT6360_PMU_CHG_IRQ2 (0xD1) > >> > +#define MT6360_PMU_CHG_IRQ3 (0xD2) > >> > +#define MT6360_PMU_CHG_IRQ4 (0xD3) > >> > +#define MT6360_PMU_CHG_IRQ5 (0xD4) > >> > +#define MT6360_PMU_CHG_IRQ6 (0xD5) > >> > +#define MT6360_PMU_QC_IRQ (0xD6) > >> > +#define MT6360_PMU_FOD_IRQ (0xD7) > >> > +#define MT6360_PMU_BASE_IRQ (0xD8) > >> > +#define MT6360_PMU_FLED_IRQ1 (0xD9) > >> > +#define MT6360_PMU_FLED_IRQ2 (0xDA) > >> > +#define MT6360_PMU_RGB_IRQ (0xDB) > >> > +#define MT6360_PMU_BUCK1_IRQ (0xDC) > >> > +#define MT6360_PMU_BUCK2_IRQ (0xDD) > >> > +#define MT6360_PMU_LDO_IRQ1 (0xDE) > >> > +#define MT6360_PMU_LDO_IRQ2 (0xDF) > >> > +#define MT6360_PMU_CHG_STAT1 (0xE0) > >> > +#define MT6360_PMU_CHG_STAT2 (0xE1) > >> > +#define MT6360_PMU_CHG_STAT3 (0xE2) > >> > +#define MT6360_PMU_CHG_STAT4 (0xE3) > >> > +#define MT6360_PMU_CHG_STAT5 (0xE4) > >> > +#define MT6360_PMU_CHG_STAT6 (0xE5) > >> > +#define MT6360_PMU_QC_STAT (0xE6) > >> > +#define MT6360_PMU_FOD_STAT (0xE7) > >> > +#define MT6360_PMU_BASE_STAT (0xE8) > >> > +#define MT6360_PMU_FLED_STAT1 (0xE9) > >> > +#define MT6360_PMU_FLED_STAT2 (0xEA) > >> > +#define MT6360_PMU_RGB_STAT (0xEB) > >> > +#define MT6360_PMU_BUCK1_STAT (0xEC) > >> > +#define MT6360_PMU_BUCK2_STAT (0xED) > >> > +#define MT6360_PMU_LDO_STAT1 (0xEE) > >> > +#define MT6360_PMU_LDO_STAT2 (0xEF) > >> > +#define MT6360_PMU_CHG_MASK1 (0xF0) > >> > +#define MT6360_PMU_CHG_MASK2 (0xF1) > >> > +#define MT6360_PMU_CHG_MASK3 (0xF2) > >> > +#define MT6360_PMU_CHG_MASK4 (0xF3) > >> > +#define MT6360_PMU_CHG_MASK5 (0xF4) > >> > +#define MT6360_PMU_CHG_MASK6 (0xF5) > >> > +#define MT6360_PMU_QC_MASK (0xF6) > >> > +#define MT6360_PMU_FOD_MASK (0xF7) > >> > +#define MT6360_PMU_BASE_MASK (0xF8) > >> > +#define MT6360_PMU_FLED_MASK1 (0xF9) > >> > +#define MT6360_PMU_FLED_MASK2 (0xFA) > >> > +#define MT6360_PMU_FAULTB_MASK (0xFB) > >> > +#define MT6360_PMU_BUCK1_MASK (0xFC) > >> > +#define MT6360_PMU_BUCK2_MASK (0xFD) > >> > +#define MT6360_PMU_LDO_MASK1 (0xFE) > >> > +#define MT6360_PMU_LDO_MASK2 (0xFF) > >> > +#define MT6360_PMU_MAXREG (MT6360_PMU_LDO_MASK2) > >> > + > >> > + > >> > +/* MT6360_PMU_IRQ_SET */ > >> > +#define MT6360_PMU_IRQ_REGNUM (MT6360_PMU_LDO_IRQ2 - > >> > MT6360_PMU_CHG_IRQ1 + 1) > >> > +#define MT6360_IRQ_RETRIG BIT(2) > >> > + > >> > +#define CHIP_VEN_MASK (0xF0) > >> > +#define CHIP_VEN_MT6360 (0x50) > >> > +#define CHIP_REV_MASK (0x0F) > >> > + > >> > +/* IRQ definitions */ > >> > >> Remove this please. > >> > >> > +struct mt6360_pmu_irq_desc { > >> > + const char *name; > >> > + irq_handler_t irq_handler; > >> > +}; > >> > >> Where is this used? > >> > >> > +#define MT6360_DT_VALPROP(name, type) \ > >> > + {#name, offsetof(type, name)} > >> > >> Where is this used? > >> > >> > +struct mt6360_val_prop { > >> > + const char *name; > >> > + size_t offset; > >> > +}; > >> > + > >> > +static inline void mt6360_dt_parser_helper(struct device_node *np, void > >> > *data, > >> > + const struct mt6360_val_prop > >> > *props, > >> > + int prop_cnt) > >> > +{ > >> > + int i; > >> > + > >> > + for (i = 0; i < prop_cnt; i++) { > >> > + if (unlikely(!props[i].name)) > >> > + continue; > >> > + of_property_read_u32(np, props[i].name, data + > >> > props[i].offset); > >> > + } > >> > +} > >> > >> What are you using this for? Why is the standard API not sufficient? > >> > >> > +#define MT6360_PDATA_VALPROP(name, type, reg, shift, mask, func, base) > >> > \ > >> > + {offsetof(type, name), reg, shift, mask, func, > >> > base} > >> > >> Where is this used? > >> > >> > +struct mt6360_pdata_prop { > >> > + size_t offset; > >> > + u8 reg; > >> > + u8 shift; > >> > + u8 mask; > >> > + u32 (*transform)(u32 val); > >> > + u8 base; > >> > +}; > >> > + > >> > +static inline int mt6360_pdata_apply_helper(void *context, void > >> > *pdata, > >> > + const struct mt6360_pdata_prop > >> > *prop, > >> > + int prop_cnt) > >> > +{ > >> > + int i, ret; > >> > + u32 val; > >> > + > >> > + for (i = 0; i < prop_cnt; i++) { > >> > + val = *(u32 *)(pdata + prop[i].offset); > >> > + if (prop[i].transform) > >> > + val = prop[i].transform(val); > >> > + val += prop[i].base; > >> > + ret = regmap_update_bits(context, > >> > + prop[i].reg, prop[i].mask, val << > >> > prop[i].shift); > >> > + if (ret < 0) > >> > + return ret; > >> > + } > >> > + return 0; > >> > +} > >> > >> Where is this used? What does it do? > >> > >> > +#endif /* __MT6360_PRIVATE_H__ */ > >> > diff --git a/include/linux/mfd/mt6360.h b/include/linux/mfd/mt6360.h > >> > new file mode 100644 > >> > index 0000000..ba2e80a > >> > --- /dev/null > >> > +++ b/include/linux/mfd/mt6360.h > >> > @@ -0,0 +1,33 @@ > >> > +/* SPDX-License-Identifier: GPL-2.0 */ > >> > +/* > >> > + * Copyright (c) 2019 MediaTek Inc. > >> > + */ > >> > + > >> > +#ifndef __MT6360_H__ > >> > +#define __MT6360_H__ > >> > + > >> > +#include > >> > + > >> > +enum { > >> > + MT6360_SLAVE_PMU = 0, > >> > + MT6360_SLAVE_PMIC, > >> > + MT6360_SLAVE_LDO, > >> > + MT6360_SLAVE_TCPC, > >> > + MT6360_SLAVE_MAX, > >> > +}; > >> > + > >> > +#define MT6360_PMU_SLAVEID (0x34) > >> > +#define MT6360_PMIC_SLAVEID (0x1A) > >> > +#define MT6360_LDO_SLAVEID (0x64) > >> > +#define MT6360_TCPC_SLAVEID (0x4E) > >> > >> What kind of slave ID? I2C address? > >> > >> > +struct mt6360_pmu_info { > >> > + struct i2c_client *i2c[MT6360_SLAVE_MAX]; > >> > + struct device *dev; > >> > >> > + struct regmap *regmap; > >> > + struct regmap_irq_chip_data *irq_data; > >> > + struct regmap_irq_chip irq_chip; > >> > + unsigned int chip_rev; > >> > >> Why are you saving these? > >> > >> Where do you reuse them? > >> > >> > +}; > >> > + > >> > +#endif /* __MT6360_H__ */ > >> > > -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog