| Message ID | 20230825114623.16884-3-macpaul.lin@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] arm64: dts: mediatek: mt8195-demo: fix the memory size to 8GB | expand |
On 8/28/23 18:51, Chen-Yu Tsai wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > On Mon, Aug 28, 2023 at 5:59 PM Macpaul Lin <macpaul.lin@mediatek.com> wrote: >> >> >> On 8/28/23 12:36, Chen-Yu Tsai wrote: >> > >> > >> > External email : Please do not click links or open attachments until you >> > have verified the sender or the content. >> > >> > On Fri, Aug 25, 2023 at 7:46 PM Macpaul Lin <macpaul.lin@mediatek.com> wrote: >> >> >> >> MT6360 is the secondary PMIC for MT8195. >> >> It supports USB Type-C and PD functions. >> >> Add MT6360 related common nodes which is used for MT8195 platform, includes >> >> - charger >> >> - ADC >> >> - LED >> >> - regulators >> >> >> >> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> >> >> --- >> >> arch/arm64/boot/dts/mediatek/mt6360.dtsi | 112 +++++++++++++++++++++++ >> >> [snip..] >> >> >> + regulator { >> >> + compatible = "mediatek,mt6360-regulator"; >> >> + LDO_VIN3-supply = <&mt6360_buck2>; >> >> + >> >> + mt6360_buck1: buck1 { >> >> + regulator-compatible = "BUCK1"; >> >> + regulator-name = "mt6360,buck1"; >> > >> > Normally there's no need to provide a default name. Any used regulator >> > should have been named to match the power rail name from the board's >> > schematics. >> > >> >> I have 2 schematics on hand. One is mt8195-demo board and the other is >> genio-1200-evk board. There are 2 PMIC used on these board >> with "the same" sub power rail name for "BUCK1~BUCK4". One is mt6315, >> and the other is mt6360. > > This is more of an board level integration thing. Regulator names are > expected to be named after the actual power rail names. For example, > take a look at Rock Pi 4 schematics [1], the power rail from BUCK1 of > the RK808 PMIC is named "VDD_CENTER". And in the dts file [1] we can > see the regulator is named that as well (albeit with some style changes). > > Now if a project really chooses meaningless names like BUCKx or LDOy > for their power rails, then so be it. However those are board level > decisions. The names are there to help with integration debugging, so > it makes sense to have names that match the power rail names in the > schematics. Default names rarely make sense. > > [1]https://dl.radxa.com/rockpi4/docs/hw/rockpi4/rockpi4_v13_sch_20181112.pdf <https://urldefense.com/v3/__https://dl.radxa.com/rockpi4/docs/hw/rockpi4/rockpi4_v13_sch_20181112.pdf__;!!CTRNKA9wMg0ARbw!g4T6kWfnETA38Kc_yc6dx6gYi7zzW2m6YU0ybNN5vbTWjK5SfapEQEQMrtxg8E9xRNdpJm678Rj3uLrWHeM$> > [2]https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi#L267 <https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi*L267__;Iw!!CTRNKA9wMg0ARbw!g4T6kWfnETA38Kc_yc6dx6gYi7zzW2m6YU0ybNN5vbTWjK5SfapEQEQMrtxg8E9xRNdpJm678Rj3hdwm0VA$> > >> I've also inspected other dtsi of the regulators, such as mt6357 and >> mt6359. They have regulator nodes with named for their purpose. For the >> schematics of mt8195-demo and genio-1200-evk boards, there are no >> explicit usage for "BUCK1~BUCK4" for both mt6135 and mt6360. In order to >> specify the sub power rail for mt6360, MediaTek chooses name like >> "mt6360,buck1" instead of simple name "buck1" for distinguish with >> "buck1" of mt6351. >> >> >> + regulator-min-microvolt = <300000>; >> >> + regulator-max-microvolt = <1300000>; >> > >> > These values correspond to the regulator's range. They make no sense as >> > regulator constraints. The min/max values are supposed to be the most >> > restrictive set of voltages of the regulator consumers. If what is fed >> > by this regulator can only take 0.7V ~ 1.1V, then it should save 0.7V >> > and 1.1V here. If the regulator is unused, then there are no constraints, >> > and these can be left out. >> > >> > Just leave them out of the file. >> >Alexandre Mergnat <amergnat@baylibre.com> >> > Both comments apply to all the regulators. >> > >> > ChenYu >> >> There are some common circuit design for these regulators like mt6359, >> mt6360 and mt6315 used on many products. MediaTek put the most common >> and expected default values in their dtsi. However, some changes still >> need to be applied to derivative boards according to product's >> requirements. The actual value be used will be applied in board's dts >> file to override the default settings in dtsi. > > The values here are definitely not some product's expected values. > They are the full range of output voltages supported, as seen in the > driver. > > The regulator binding says: > > regulator-min-microvolt: > description: smallest voltage consumers may set > > regulator-max-microvolt: > description: largest voltage consumers may set > > The constraints given in the regulator node are those of the consumers, > not the PMIC regulator itself. As you mentioned, a board may need to > adjust the range based on its design, i.e. what the board has connected > to the regulator. > > So either something is connected, and the consumer's constraints are set > by overriding the default in the board .dts file; or, nothing is connected > and the constraints don't matter, as nothing is going to set the voltage > or enable the regulator. So there's no reason to give a default. For > unused regulator outputs, their device nodes don't even have to exist. > > I'm trying to get people to _not_ write default values, as they don't > make any sense. The full voltage range is already implied by the > compatible string. > > ChenYu Thanks for the explanation in detail. I'll update the patch v2 for these modification. Best regards, Macpaul Lin
diff --git a/arch/arm64/boot/dts/mediatek/mt6360.dtsi b/arch/arm64/boot/dts/mediatek/mt6360.dtsi new file mode 100644 index 000000000000..e841f4e5a54b --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt6360.dtsi @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Copyright (C) 2023 MediaTek Inc. + */ + +#include <dt-bindings/regulator/mediatek,mt6360-regulator.h> + +&mt6360 { + interrupt-controller; + interrupt-parent = <&pio>; + interrupt-names = "IRQB"; + + charger { + compatible = "mediatek,mt6360-chg"; + richtek,vinovp-microvolt = <14500000>; + + otg_vbus_regulator: usb-otg-vbus-regulator { + regulator-compatible = "usb-otg-vbus"; + regulator-name = "usb-otg-vbus"; + regulator-min-microvolt = <4425000>; + regulator-max-microvolt = <5825000>; + }; + }; + + adc { + compatible = "mediatek,mt6360-adc"; + #io-channel-cells = <1>; + }; + + led { + compatible = "mediatek,mt6360-led"; + }; + + regulator { + compatible = "mediatek,mt6360-regulator"; + LDO_VIN3-supply = <&mt6360_buck2>; + + mt6360_buck1: buck1 { + regulator-compatible = "BUCK1"; + regulator-name = "mt6360,buck1"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <1300000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP + MT6360_OPMODE_ULP>; + }; + + mt6360_buck2: buck2 { + regulator-compatible = "BUCK2"; + regulator-name = "mt6360,buck2"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <1300000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP + MT6360_OPMODE_ULP>; + }; + + mt6360_ldo1: ldo1 { + regulator-compatible = "LDO1"; + regulator-name = "mt6360,ldo1"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo2: ldo2 { + regulator-compatible = "LDO2"; + regulator-name = "mt6360,ldo2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo3: ldo3 { + regulator-compatible = "LDO3"; + regulator-name = "mt6360,ldo3"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo5: ldo5 { + regulator-compatible = "LDO5"; + regulator-name = "mt6360,ldo5"; + regulator-min-microvolt = <2700000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo6: ldo6 { + regulator-compatible = "LDO6"; + regulator-name = "mt6360,ldo6"; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <2100000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo7: ldo7 { + regulator-compatible = "LDO7"; + regulator-name = "mt6360,ldo7"; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <2100000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + }; +};
MT6360 is the secondary PMIC for MT8195. It supports USB Type-C and PD functions. Add MT6360 related common nodes which is used for MT8195 platform, includes - charger - ADC - LED - regulators Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> --- arch/arm64/boot/dts/mediatek/mt6360.dtsi | 112 +++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 arch/arm64/boot/dts/mediatek/mt6360.dtsi