Received: by 2002:a05:6358:16cc:b0:ea:6187:17c9 with SMTP id r12csp7070712rwl; Mon, 9 Jan 2023 17:35:50 -0800 (PST) X-Google-Smtp-Source: AMrXdXuCqKzTEpJBrK+c+ZPyIV0fZXLT9fb3dPDO+/r1xDzYGQe64qLhpWiPNi9OFXXIOH2YpuwO X-Received: by 2002:a17:906:11ce:b0:84d:3289:3287 with SMTP id o14-20020a17090611ce00b0084d32893287mr10827390eja.43.1673314550112; Mon, 09 Jan 2023 17:35:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673314550; cv=none; d=google.com; s=arc-20160816; b=Epf5QQ9yzWiDY/ZxwzoDG1HZvRfrLBIGFXV1CWxiZchUEtLlwdxBHIHrJfroiE7hAU CVQzAvkClD6UijdPBHAUZjOngCQxoZBcGLaRuuz1yy5vpBuNvEM/IwSZ+caYcVcAMXSF LpPdPv6i191T1076AHh1w6x855JAYlxCyNtJ+7esBR+GA4uh0wffN39aPUeIOR4fpnA6 rW9XqnFjPD+17/CL3j2XkSM7HhNnu8f8xUV1gze/zLNn/pXZnkBOE9nflyLQf1GUAo7m mFJFW+ge1xOrTvAIM/kP+dxeZ3Pp6XDQ4evat6LrwiUpFYWpk3Rykjgmg3jK9V01fGSl PpMg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:references:in-reply-to:message-id :cc:to:subject:from:date; bh=6PNo/JvKI4wvEeDr6jXRImPD0L9RO7iJWnpRy6d0mGY=; b=1K6bbDgVmGBBc3RQ6OOaTiQa9ln4+XPVM93Z5cqPfJiqrdJYqWe9Q0kx+A7BWhNV09 pnQWzmnt9Mc8v8lsTfL/RUzUZOth5FkJVYKFuPm9mQDXLhYGA/Poz6RXkFtw6JUxM1FA wnLIUjwH2VSxBBihdFS1OZ0T5Gwe293QP03fxjH4MkGm9FuqkxPfkbTI8UrriffUdPe2 b2nG4YTijVY2iVwADzGU1AMOycsWMiOCCzzVmWD/WCILdL3xJuAPPnZSRlGRra/ztK55 qWEbs4/UgtLhLLJLIMXJY1ocXzYXj1RMa3r3ohVrthpW2VBda/eJE4hgINbtkUmPwhqP /oRw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id wy10-20020a170906fe0a00b0084d34979425si7781554ejb.324.2023.01.09.17.35.37; Mon, 09 Jan 2023 17:35:49 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230233AbjAJAdQ (ORCPT + 53 others); Mon, 9 Jan 2023 19:33:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34186 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235498AbjAJAdL (ORCPT ); Mon, 9 Jan 2023 19:33:11 -0500 Received: from relay03.th.seeweb.it (relay03.th.seeweb.it [5.144.164.164]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5F3B7624A for ; Mon, 9 Jan 2023 16:33:08 -0800 (PST) Received: from [192.168.2.144] (adsl-d248.84-47-10.t-com.sk [84.47.10.248]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by m-r1.th.seeweb.it (Postfix) with ESMTPSA id 7A5631F6C6; Tue, 10 Jan 2023 01:33:04 +0100 (CET) Date: Tue, 10 Jan 2023 01:32:58 +0100 From: Martin Botka Subject: Re: [PATCH v5 2/3] mfd: ax20x: Add suppport for AXP1530 PMIC To: Andre Przywara Cc: martin.botka1@gmail.com, Konrad Dybcio , AngeloGioacchino Del Regno , Marijn Suijten , Jami Kettunen , Paul Bouchara , Jan Trmal , Lee Jones , Rob Herring , Krzysztof Kozlowski , Chen-Yu Tsai , Liam Girdwood , Mark Brown , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Message-Id: In-Reply-To: <20230110000025.221430b6@slackpad.lan> References: <20221214190305.3354669-1-martin.botka@somainline.org> <20221214190305.3354669-3-martin.botka@somainline.org> <20221216181752.1d839233@donnerap.cambridge.arm.com> <1C9140E8-1476-4EA0-B685-A733990C5E0F@somainline.org> <20230110000025.221430b6@slackpad.lan> X-Mailer: geary/43.0 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed X-Spam-Status: No, score=0.0 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_HTTP,RCVD_IN_SORBS_SOCKS,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 10 2023 at 12:00:25 AM +00:00:00, Andre Przywara wrote: > On Sat, 17 Dec 2022 01:13:01 +0100 > Martin Botka wrote: > > Hi Martin, Hello Andre, > > hope you had a good break! Did you have any chance to come back to > this > again? Now would be a good time to send a new version, otherwise it's > getting pretty tight for v6.3 already. > I did have a good break :) I unfortunately did not. Was bit busy with issues to one of my projects. Will see if I can sort this out tomorrow but you said you already have AXP313A driver ready. So it may be better if you send your driver. > On Friday, "junari" in the #linux-sunxi IRC channel, made some > interesting discovery: he is playing around with an AXP313a on some > H616 device and figured that DCDC3 is not behaving like the datasheet: > https://oftc.irclog.whitequark.org/linux-sunxi/2023-01-06#31784528; > He later confirmed the voltage: > https://oftc.irclog.whitequark.org/linux-sunxi/2023-01-08#31788373; > Ah interesting. What i also found out right after signing off is from a friend at BIQU that the AXP1530 is the AXP313A. It is the same chip. The only difference being that AXP1530 is the internal naming AXP uses. I mean we kinda figured that one out but its nice to have confirmation on this :) > Basically it looks like the DCDC3 parameters you harvested from the > BSP > code seem to be correct after all. Do you have any chance to measure > the voltage? I wish i had. Dont think my multimeter would be able to be so precise. Will try to source some oscilloscope but that wont be in time unfortunately. > If not, can we try to deduce what the right settings are? The voltage > difference seems to be significant (860mV vs 1200mV), I wonder if any > device connected there (DRAM?) would work with the wrong setting? I will try to be around in IRC sunxi channel "today" from around 13:00UTC+1. Would be prob best to discuss this there and then share our findings here on LKML :) Cheers, Martin > > Cheers, > Andre > >> On December 16, 2022 7:17:52 PM GMT+01:00, Andre Przywara >> wrote: >> >On Wed, 14 Dec 2022 20:03:04 +0100 >> >Martin Botka wrote: >> > >> >Hi Martin, >> > >> >> AXP1530 is a PMIC chip produced by X-Powers and an be connected >> via >> >> I2C bus. >> >> Where AXP313A seems to be closely related so the same driver can >> be used and >> >> seen it only paired with H616 SoC. >> > >> >So as mentioned, I am pretending this is for the AXP313A now, >> looking at >> >its datasheet. >> >Of course the elephant in the room is s/AXP1530/AXP313A/, but >> other than >> >that: >> > >> >> >> >> Signed-off-by: Martin Botka >> >> --- >> >> drivers/mfd/axp20x-i2c.c | 2 ++ >> >> drivers/mfd/axp20x.c | 62 >> ++++++++++++++++++++++++++++++++++++++ >> >> include/linux/mfd/axp20x.h | 32 ++++++++++++++++++++ >> >> 3 files changed, 96 insertions(+) >> >> >> >> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c >> >> index 8fd6727dc30a..6bfb931a580e 100644 >> >> --- a/drivers/mfd/axp20x-i2c.c >> >> +++ b/drivers/mfd/axp20x-i2c.c >> >> @@ -60,6 +60,7 @@ static void axp20x_i2c_remove(struct >> i2c_client *i2c) >> >> #ifdef CONFIG_OF >> >> static const struct of_device_id axp20x_i2c_of_match[] = { >> >> { .compatible = "x-powers,axp152", .data = (void *)AXP152_ID }, >> >> + { .compatible = "x-powers,axp1530", .data = (void >> *)AXP1530_ID}, >> >> { .compatible = "x-powers,axp202", .data = (void *)AXP202_ID }, >> >> { .compatible = "x-powers,axp209", .data = (void *)AXP209_ID }, >> >> { .compatible = "x-powers,axp221", .data = (void *)AXP221_ID }, >> >> @@ -73,6 +74,7 @@ MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match); >> >> >> >> static const struct i2c_device_id axp20x_i2c_id[] = { >> >> { "axp152", 0 }, >> >> + { "axp1530", 0 }, >> >> { "axp202", 0 }, >> >> { "axp209", 0 }, >> >> { "axp221", 0 }, >> >> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c >> >> index 880c41fa7021..6caa7e87ad80 100644 >> >> --- a/drivers/mfd/axp20x.c >> >> +++ b/drivers/mfd/axp20x.c >> >> @@ -34,6 +34,7 @@ >> >> >> >> static const char * const axp20x_model_names[] = { >> >> "AXP152", >> >> + "AXP1530", >> >> "AXP202", >> >> "AXP209", >> >> "AXP221", >> >> @@ -66,6 +67,24 @@ static const struct regmap_access_table >> axp152_volatile_table = { >> >> .n_yes_ranges = ARRAY_SIZE(axp152_volatile_ranges), >> >> }; >> >> >> >> +static const struct regmap_range axp1530_writeable_ranges[] = { >> >> + regmap_reg_range(AXP1530_ON_INDICATE, AXP1530_FREQUENCY), >> > >> >Where does this FREQUENCY register come from? BSP source? Is that >> the >> >lost register to set the PWM frequency? >> >The 313 datasheet doesn't mention it, and since we deny >> programming the >> >frequency, I would just leave it out. >> >If people find it existing (and useful!) later on, we should be >> able to >> >add it without breaking anything. >> >> BSP. Ack. >> > >> >> +}; >> >> + >> >> +static const struct regmap_range axp1530_volatile_ranges[] = { >> >> + regmap_reg_range(AXP1530_ON_INDICATE, AXP1530_FREQUENCY), >> >> +}; >> >> + >> >> +static const struct regmap_access_table axp1530_writeable_table >> = { >> >> + .yes_ranges = axp1530_writeable_ranges, >> >> + .n_yes_ranges = ARRAY_SIZE(axp1530_writeable_ranges), >> >> +}; >> >> + >> >> +static const struct regmap_access_table axp1530_volatile_table >> = { >> >> + .yes_ranges = axp1530_volatile_ranges, >> >> + .n_yes_ranges = ARRAY_SIZE(axp1530_volatile_ranges), >> >> +}; >> >> + >> >> static const struct regmap_range axp20x_writeable_ranges[] = { >> >> regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE), >> >> regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2), >> >> @@ -245,6 +264,15 @@ static const struct regmap_config >> axp152_regmap_config = { >> >> .cache_type = REGCACHE_RBTREE, >> >> }; >> >> >> >> +static const struct regmap_config axp1530_regmap_config = { >> >> + .reg_bits = 8, >> >> + .val_bits = 8, >> >> + .wr_table = &axp1530_writeable_table, >> >> + .volatile_table = &axp1530_volatile_table, >> >> + .max_register = AXP1530_FREQUENCY, >> >> + .cache_type = REGCACHE_RBTREE, >> >> +}; >> >> + >> >> static const struct regmap_config axp20x_regmap_config = { >> >> .reg_bits = 8, >> >> .val_bits = 8, >> >> @@ -304,6 +332,16 @@ static const struct regmap_irq >> axp152_regmap_irqs[] = { >> >> INIT_REGMAP_IRQ(AXP152, GPIO0_INPUT, 2, 0), >> >> }; >> >> >> >> +static const struct regmap_irq axp1530_regmap_irqs[] = { >> >> + INIT_REGMAP_IRQ(AXP1530, KEY_L2H_EN, 0, 7), >> >> + INIT_REGMAP_IRQ(AXP1530, KEY_H2L_EN, 0, 6), >> >> + INIT_REGMAP_IRQ(AXP1530, POKSIRQ_EN, 0, 5), >> >> + INIT_REGMAP_IRQ(AXP1530, POKLIRQ_EN, 0, 4), >> > >> >Are those identifiers from the BSP source? The (translated) manual >> gives >> >some explanation, namely: >> > PWRON key rising edge >> > PWRON key falling edge >> > Short press the PWRON button >> > Long press the PWRON button >> > >> >So I'd suggest we follow the existing naming: >> > PEK_RIS_EDGE, PEK_FAL_EDGE, PEK_SHORT, PEK_LONG (respectively) >> > >> >Or come up with names that people could actually decipher ;-) >> > >> > >> Ack. >> >> + INIT_REGMAP_IRQ(AXP1530, DCDC3_UNDER, 0, 3), >> >> + INIT_REGMAP_IRQ(AXP1530, DCDC2_UNDER, 0, 2), >> >> + INIT_REGMAP_IRQ(AXP1530, TEMP_OVER, 0, 0), >> >> +}; >> >> + >> >> static const struct regmap_irq axp20x_regmap_irqs[] = { >> >> INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V, 0, 7), >> >> INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN, 0, 6), >> >> @@ -514,6 +552,18 @@ static const struct regmap_irq_chip >> axp152_regmap_irq_chip = { >> >> .num_regs = 3, >> >> }; >> >> >> >> +static const struct regmap_irq_chip axp1530_regmap_irq_chip = { >> >> + .name = "axp1530_irq_chip", >> >> + .status_base = AXP1530_IRQ_STATUS1, >> >> + .ack_base = AXP1530_IRQ_STATUS1, >> >> + .mask_base = AXP1530_IRQ_ENABLE1, >> >> + .mask_invert = true, >> >> + .init_ack_masked = true, >> >> + .irqs = axp1530_regmap_irqs, >> >> + .num_irqs = ARRAY_SIZE(axp1530_regmap_irqs), >> >> + .num_regs = 1, >> >> +}; >> >> + >> >> static const struct regmap_irq_chip axp20x_regmap_irq_chip = { >> >> .name = "axp20x_irq_chip", >> >> .status_base = AXP20X_IRQ1_STATE, >> >> @@ -683,6 +733,12 @@ static const struct mfd_cell axp152_cells[] >> = { >> >> }, >> >> }; >> >> >> >> +static struct mfd_cell axp1530_cells[] = { >> >> + { >> >> + .name = "axp20x-regulator", >> >> + }, >> >> +}; >> >> + >> >> static const struct resource axp288_adc_resources[] = { >> >> DEFINE_RES_IRQ_NAMED(AXP288_IRQ_GPADC, "GPADC"), >> >> }; >> >> @@ -874,6 +930,12 @@ int axp20x_match_device(struct axp20x_dev >> *axp20x) >> >> axp20x->regmap_cfg = &axp152_regmap_config; >> >> axp20x->regmap_irq_chip = &axp152_regmap_irq_chip; >> >> break; >> >> + case AXP1530_ID: >> >> + axp20x->nr_cells = ARRAY_SIZE(axp1530_cells); >> >> + axp20x->cells = axp1530_cells; >> >> + axp20x->regmap_cfg = &axp1530_regmap_config; >> >> + axp20x->regmap_irq_chip = &axp1530_regmap_irq_chip; >> >> + break; >> >> case AXP202_ID: >> >> case AXP209_ID: >> >> axp20x->nr_cells = ARRAY_SIZE(axp20x_cells); >> >> diff --git a/include/linux/mfd/axp20x.h >> b/include/linux/mfd/axp20x.h >> >> index 9ab0e2fca7ea..cad25754500f 100644 >> >> --- a/include/linux/mfd/axp20x.h >> >> +++ b/include/linux/mfd/axp20x.h >> >> @@ -12,6 +12,7 @@ >> >> >> >> enum axp20x_variants { >> >> AXP152_ID = 0, >> >> + AXP1530_ID, >> >> AXP202_ID, >> >> AXP209_ID, >> >> AXP221_ID, >> >> @@ -45,6 +46,18 @@ enum axp20x_variants { >> >> #define AXP152_DCDC_FREQ 0x37 >> >> #define AXP152_DCDC_MODE 0x80 >> >> >> >> +#define AXP1530_ON_INDICATE 0x00 >> >> +#define AXP1530_OUTPUT_CONTROL 0x10 >> >> +#define AXP1530_DCDC1_CONRTOL 0x13 >> >> +#define AXP1530_DCDC2_CONRTOL 0x14 >> >> +#define AXP1530_DCDC3_CONRTOL 0x15 >> >> +#define AXP1530_ALDO1_CONRTOL 0x16 >> >> +#define AXP1530_DLDO1_CONRTOL 0x17 >> >> +#define AXP1530_OUTOUT_MONITOR 0x1D >> > >> >Shall this read AXP1530_OUTPUT_MONITOR? >> > >> >> +#define AXP1530_IRQ_ENABLE1 0x20 >> >> +#define AXP1530_IRQ_STATUS1 0x21 >> > >> >There is only one interrupt register, so can we drop the trailing >> number? >> > >> Yep. >> >> +#define AXP1530_FREQUENCY 0x87 >> > >> >As mentioned, the manual does not mention it, and we don't use it >> anyway. >> > >> Ack. >> >> + >> >> #define AXP20X_PWR_INPUT_STATUS 0x00 >> >> #define AXP20X_PWR_OP_MODE 0x01 >> >> #define AXP20X_USB_OTG_STATUS 0x02 >> >> @@ -287,6 +300,15 @@ enum axp20x_variants { >> >> #define AXP288_FG_TUNE5 0xed >> >> >> >> /* Regulators IDs */ >> >> +enum { >> >> + AXP1530_DCDC1 = 0, >> >> + AXP1530_DCDC2, >> >> + AXP1530_DCDC3, >> >> + AXP1530_LDO1, >> >> + AXP1530_LDO2, >> > >> >I guess we should add the RTC LDO as LDO3 here. >> > >> >The rest of the numbers match with the datasheet. >> > >> Ack. >> >> I will take some time off due to Uni so v6 will be delayed peobably >> last holidays. >> >> Best regards and happy holidays, >> Martin >> >Cheers, >> >Andre >> > >> >> + AXP1530_REG_ID_MAX, >> >> +}; >> >> + >> >> enum { >> >> AXP20X_LDO1 = 0, >> >> AXP20X_LDO2, >> >> @@ -440,6 +462,16 @@ enum { >> >> AXP152_IRQ_GPIO0_INPUT, >> >> }; >> >> >> >> +enum axp1530_irqs { >> >> + AXP1530_IRQ_TEMP_OVER, >> >> + AXP1530_IRQ_DCDC2_UNDER = 2, >> >> + AXP1530_IRQ_DCDC3_UNDER, >> >> + AXP1530_IRQ_POKLIRQ_EN, >> >> + AXP1530_IRQ_POKSIRQ_EN, >> >> + AXP1530_IRQ_KEY_L2H_EN, >> >> + AXP1530_IRQ_KEY_H2L_EN, >> >> +}; >> >> + >> >> enum { >> >> AXP20X_IRQ_ACIN_OVER_V = 1, >> >> AXP20X_IRQ_ACIN_PLUGIN, >> > >