Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 於 2022年6月8日 週三 下午3:02寫道： > > On 08/06/2022 04:52, ChiYuan Huang wrote: > > Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 於 2022年6月7日 週二 下午7:52寫道： > >> > >> On 07/06/2022 07:52, cy_huang wrote: > >>> From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > >>> > >>> Add Richtek RT5120 PMIC devicetree document. > >>> > >>> Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > >>> --- > >>> .../devicetree/bindings/mfd/richtek,rt5120.yaml | 180 +++++++++++++++++++++ > >>> 1 file changed, 180 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/mfd/richtek,rt5120.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/mfd/richtek,rt5120.yaml b/Documentation/devicetree/bindings/mfd/richtek,rt5120.yaml > >>> new file mode 100644 > >>> index 00000000..376bf73 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/mfd/richtek,rt5120.yaml > >>> @@ -0,0 +1,180 @@ > >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/mfd/richtek,rt5120.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Richtek RT5120 PMIC > >>> + > >>> +maintainers: > >>> + - ChiYuan Huang <cy_huang@xxxxxxxxxxx> > >>> + > >>> +description: | > >>> + The RT5120 provides four high-efficiency buck converters and one LDO voltage > >>> + regulator. The device is targeted at providingthe processor voltage, memory, > >>> + I/O, and peripheral rails in home entertainment devices. The I2C interface is > >>> + used for dynamic voltage scaling of the processor voltage, power rails on/off > >>> + sequence control, operation mode selection. > >>> + > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - richtek,rt5120 > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + interrupts: > >>> + maxItems: 1 > > Your powerkey driver takes two interrupts. You should describe them in > the powerkey. > > >>> + > >>> + interrupt-controller: true > >>> + > >>> + "#interrupt-cells": > >>> + const: 1 > >>> + > >>> + wakeup-source: true > >>> + > >>> + richtek,enable-undervolt-hiccup: > >>> + type: boolean > >>> + description: | > >>> + If used, under voltage protection trigger hiccup behavior, else latchup as > >>> + default > >>> + > >>> + richtek,enable-overvolt-hiccup: > >>> + type: boolean > >>> + description: > >>> + Like as 'enable-uv-hiccup', it configures over voltage protection to > >>> + hiccup, else latchup as default > >>> + > >>> + vin1-supply: > >>> + description: phandle for buck1 input power source > >>> + > >>> + vin2-supply: > >>> + description: phandle for buck2 input power source > >>> + > >>> + vin3-supply: > >>> + description: phandle for buck3 input power source > >>> + > >>> + vin4-supply: > >>> + description: phandle for buck4 input power source > >>> + > >>> + vinldo-supply: > >>> + description: phandle for ldo input power source > >>> + > >>> + regulators: > >>> + type: object > >>> + > >>> + patternProperties: > >>> + "^buck[1-4]$": > >>> + type: object > >>> + $ref: /schemas/regulator/regulator.yaml# > >>> + > >>> + properties: > >>> + regulator-allowed-modes: > >>> + description: | > >>> + Used to specify the allowed buck converter operating mode > >>> + mode mapping: > >>> + 0: auto mode > >>> + 1: force pwm mode > >>> + items: > >>> + enum: [0, 1] > >>> + > >>> + unevaluatedProperties: false > >> > >> Better to put it after '$ref' for readability. > > OK, Fix in next > >> > >>> + > >>> + "^(ldo|exten)$": > >>> + type: object > >>> + $ref: /schemas/regulator/regulator.yaml# > >> > >> You need here unevaluatedProperties:false as well (for the ldo/exten > >> properties) > > Fix in next. > >> > >>> + > >>> + additionalProperties: false > >>> + > >>> + powerkey: > >>> + type: object > >>> + description: > >>> + The power key driver may be optional. If not used, change node status to > >>> + 'disabled' > >> > >> This description is not helpful, does not describe the hardware. Please > >> describe hardware, not Devicetree usage. > > That's because it's a PMIC. Power key is also connected to it. > > For power key press, all power rails will start to power up. > > But in the application, there may be other PMIC that's also connected > > to power key. > > That's why this power key driver may need to be optional. > > One system only need one driver to report the power key status. > > > > Currently in some linux OS, it uses the auto module loading mechanism. > > All kernel module files may be all the same, but it uses the > > devicetree to decide how many devices > > need to be declared. Since RT5120 power key device may be optional, > > following by mfd_add_device, if of_node is > > found, and status is "disabled", the sub device would be skipped. > > > > Actually, I'm also confused about it. There may be three ways to implement it > > 1. not to build this kernel module -> seems to violate my above application > > 2. Use one boolean property to decide power key cell need to be used or not?? > > 3. like as now, use the node status to decide it. > > > > Is there the better way to do it? > > The status does not determine whether device in the bindings is optional > or not. Rather it's presence. In the term of bindings the "optional" > means that something might not be there physically. E.g. clock line > connected or not. System implementation - MFD, power off handling - is > here (almost) irrelevant. > > In your case, the power key feature seems to be there always, so the > "powerkey" node should be required and not disabled. Don't mention in > description of hardware anything about disabling it or not. > > In your application, I would say it is interesting design that someone > connects one power up line to two different PMICs in a conflicting way. > This sounds like total mistake from hardware point of view. > > Anyway it is not the job for this patch to solve such conflicts. > Thanks, I think your point is 'optional' keyword. If there's only redundant description line, I may decide to remove it. The property name already show its usage. > Best regards, > Krzysztof
