Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

regulator: mt6358: Merge VCN33_* regulators

The VCN33_BT and VCN33_WIFI regulators are actually the same regulator,
having the same voltage setting and output pin. There are simply two
enable bits that are ORed together to enable the regulator.

Having two regulators representing the same output pin is misleading
from a design matching standpoint, and also error-prone in driver
implementations. If consumers try to set different voltages on either
regulator, the one set later would override the one set before. There
are ways around this, such as chaining them together and having the
downstream one act as a switch. But given there's only one output pin,
such a workaround doesn't match reality.

Remove the VCN33_WIFI regulator. During the probe phase, have the driver
sync the enable status of VCN33_WIFI to VCN33_BT. Also drop the suffix
so that the regulator name matches the pin name in the datasheet.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Link: https://lore.kernel.org/r/20230609083009.2822259-4-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>

authored by

Chen-Yu Tsai and committed by
Mark Brown
65bae54e 82f305b1

+52 -19
+50 -15
drivers/regulator/mt6358-regulator.c
··· 277 277 2800000, 2900000, 3000000, 278 278 }; 279 279 280 - static const unsigned int vcn33_bt_wifi_voltages[] = { 280 + static const unsigned int vcn33_voltages[] = { 281 281 3300000, 3400000, 3500000, 282 282 }; 283 283 ··· 321 321 0, 7, 9, 10, 11, 12, 322 322 }; 323 323 324 - static const u32 vcn33_bt_wifi_idx[] = { 324 + static const u32 vcn33_idx[] = { 325 325 1, 2, 3, 326 326 }; 327 327 ··· 566 566 MT6358_LDO_VCAMA1_CON0, 0, MT6358_VCAMA1_ANA_CON0, 0xf00), 567 567 MT6358_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx, 568 568 MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700), 569 - MT6358_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages, 570 - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0, 571 - 0, MT6358_VCN33_ANA_CON0, 0x300), 572 - MT6358_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages, 573 - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1, 574 - 0, MT6358_VCN33_ANA_CON0, 0x300), 569 + MT6358_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx, 570 + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300), 575 571 MT6358_LDO("ldo_vcama2", VCAMA2, vcama_voltages, vcama_idx, 576 572 MT6358_LDO_VCAMA2_CON0, 0, MT6358_VCAMA2_ANA_CON0, 0xf00), 577 573 MT6358_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx, ··· 658 662 MT6358_LDO_VMCH_CON0, 0, MT6358_VMCH_ANA_CON0, 0x700), 659 663 MT6366_LDO("ldo_vemc", VEMC, vmch_vemc_voltages, vmch_vemc_idx, 660 664 MT6358_LDO_VEMC_CON0, 0, MT6358_VEMC_ANA_CON0, 0x700), 661 - MT6366_LDO("ldo_vcn33_bt", VCN33_BT, vcn33_bt_wifi_voltages, 662 - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_0, 663 - 0, MT6358_VCN33_ANA_CON0, 0x300), 664 - MT6366_LDO("ldo_vcn33_wifi", VCN33_WIFI, vcn33_bt_wifi_voltages, 665 - vcn33_bt_wifi_idx, MT6358_LDO_VCN33_CON0_1, 666 - 0, MT6358_VCN33_ANA_CON0, 0x300), 665 + MT6366_LDO("ldo_vcn33", VCN33, vcn33_voltages, vcn33_idx, 666 + MT6358_LDO_VCN33_CON0_0, 0, MT6358_VCN33_ANA_CON0, 0x300), 667 667 MT6366_LDO("ldo_vmc", VMC, vmc_voltages, vmc_idx, 668 668 MT6358_LDO_VMC_CON0, 0, MT6358_VMC_ANA_CON0, 0xf00), 669 669 MT6366_LDO("ldo_vsim2", VSIM2, vsim_voltages, vsim_idx, ··· 682 690 MT6358_LDO_VSRAM_CON1, 0x7f), 683 691 }; 684 692 693 + static int mt6358_sync_vcn33_setting(struct device *dev) 694 + { 695 + struct mt6397_chip *mt6397 = dev_get_drvdata(dev->parent); 696 + unsigned int val; 697 + int ret; 698 + 699 + /* 700 + * VCN33_WIFI and VCN33_BT are two separate enable bits for the same 701 + * regulator. They share the same voltage setting and output pin. 702 + * Instead of having two potentially conflicting regulators, just have 703 + * one VCN33 regulator. Sync the two enable bits and only use one in 704 + * the regulator device. 705 + */ 706 + ret = regmap_read(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, &val); 707 + if (ret) { 708 + dev_err(dev, "Failed to read VCN33_WIFI setting\n"); 709 + return ret; 710 + } 711 + 712 + if (!(val & BIT(0))) 713 + return 0; 714 + 715 + /* Sync VCN33_WIFI enable status to VCN33_BT */ 716 + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_0, BIT(0), BIT(0)); 717 + if (ret) { 718 + dev_err(dev, "Failed to sync VCN33_WIFI setting to VCN33_BT\n"); 719 + return ret; 720 + } 721 + 722 + /* Disable VCN33_WIFI */ 723 + ret = regmap_update_bits(mt6397->regmap, MT6358_LDO_VCN33_CON0_1, BIT(0), 0); 724 + if (ret) { 725 + dev_err(dev, "Failed to disable VCN33_BT\n"); 726 + return ret; 727 + } 728 + 729 + return 0; 730 + } 731 + 685 732 static int mt6358_regulator_probe(struct platform_device *pdev) 686 733 { 687 734 struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent); 688 735 struct regulator_config config = {}; 689 736 struct regulator_dev *rdev; 690 737 struct mt6358_regulator_info *mt6358_info; 691 - int i, max_regulator; 738 + int i, max_regulator, ret; 739 + 740 + ret = mt6358_sync_vcn33_setting(&pdev->dev); 741 + if (ret) 742 + return ret; 692 743 693 744 if (mt6397->chip_id == MT6366_CHIP_ID) { 694 745 max_regulator = MT6366_MAX_REGULATOR;
+2 -4
include/linux/regulator/mt6358-regulator.h
··· 41 41 MT6358_ID_VIO28, 42 42 MT6358_ID_VA12, 43 43 MT6358_ID_VRF18, 44 - MT6358_ID_VCN33_BT, 45 - MT6358_ID_VCN33_WIFI, 44 + MT6358_ID_VCN33, 46 45 MT6358_ID_VCAMA2, 47 46 MT6358_ID_VMC, 48 47 MT6358_ID_VLDO28, ··· 84 85 MT6366_ID_VIO28, 85 86 MT6366_ID_VA12, 86 87 MT6366_ID_VRF18, 87 - MT6366_ID_VCN33_BT, 88 - MT6366_ID_VCN33_WIFI, 88 + MT6366_ID_VCN33, 89 89 MT6366_ID_VMC, 90 90 MT6366_ID_VAUD28, 91 91 MT6366_ID_VSIM2,