lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 01 Oct 2013 14:39:45 -0600 From: Stephen Warren <swarren@...dotorg.org> To: Laxman Dewangan <ldewangan@...dia.com> CC: lee.jones@...aro.org, sameo@...ux.intel.com, broonie@...nel.org, linus.walleij@...aro.org, akpm@...ux-foundation.org, devicetree@...r.kernel.org, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org, rtc-linux@...glegroups.com, rob.herring@...xeda.com, mark.rutland@....com, pawel.moll@....com, rob@...dley.net, ijc+devicetree@...lion.org.uk, grant.likely@...aro.org, Florian Lobmaier <florian.lobmaier@....com> Subject: Re: [PATCH V3 1/3] mfd: add support for AMS AS3722 PMIC On 09/24/2013 05:58 AM, Laxman Dewangan wrote: > The AMS AS3722 is a compact system PMU suitable for mobile phones, > tablets etc. It has 4 DC/DC step-down regulators, 3 DC/DC step-down > controller, 11 LDOs, RTC, automatic battery, temperature and > over-current monitoring, 8 GPIOs, ADC and watchdog. > > Add MFD core driver for the AS3722 to support core functionality. > diff --git a/Documentation/devicetree/bindings/mfd/as3722.txt b/Documentation/devicetree/bindings/mfd/as3722.txt > +Required properties: > +------------------- > +- compatible : Must be "ams,as3722". > +- reg: I2C device address. > +- interrupt-controller : AS3722 has its own internal IRQs You should at least specify that the property is a Boolean. That description a justification for why that property should be present, not a description of the property. If the device only has *internal* IRQs, you shouldn't need any of the IRQ properties in DT. Rather, I believe you need the IRQ properties because the device has *external* IRQ inputs via the "gpio-in-interrupt" GPIO mux function. > +- interrupt-parent : The parent interrupt controller. While that property is allowed, it's not required. It's also unusual to mention it in individual bindings, since it's a system-defined property. > +Optional properties: > +------------------- > +- pinctrl-names: It is define in the > + Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt You need to specify which values must be present in this property. Why is the property optional? If you're going to mention pinctrl-names, shouldn't you also mention pinctrl-$n? Instead, I would simply say: ========== Optional properties: A pinctrl state named "default" must be defined, using the bindings in ../pinctrl/pinctrl-binding.txt. ========== Oh, and don't use an absolute file-name within the Linux kernel source tree, since that path name will change after the binding docs are moved out of the kernel source tree. As I mentioned when reviewing the pinmux binding, that binding should be described here. In the example, I see a regulators binding. That should be here too. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists