linux-kernel - Re: [PATCH v2 1/3] arm/dts: Add twl6030-usb data

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <504F0CD7.1090302@ti.com>
Date:	Tue, 11 Sep 2012 12:05:11 +0200
From:	Benoit Cousson <b-cousson@...com>
To:	"ABRAHAM, KISHON VIJAY" <kishon@...com>
CC:	<balbi@...com>, <linux@....linux.org.uk>,
	<rob.herring@...xeda.com>, <grant.likely@...retlab.ca>,
	<tony@...mide.com>, <rnayak@...com>, <hvaibhav@...com>,
	<peter.ujfalusi@...com>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] arm/dts: Add twl6030-usb data

On 09/11/2012 11:39 AM, ABRAHAM, KISHON VIJAY wrote:
> Hi,
> 
> On Tue, Sep 11, 2012 at 2:56 PM, Benoit Cousson <b-cousson@...com> wrote:
>> On 09/11/2012 08:36 AM, Kishon Vijay Abraham I wrote:
>>> Add twl6030-usb data node in twl6030 device tree file
>>>
>>> Acked-by: Felipe Balbi <balbi@...com>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@...com>
>>> ---
>>>  arch/arm/boot/dts/omap4-panda.dts |    4 ++++
>>>  arch/arm/boot/dts/omap4-sdp.dts   |    4 ++++
>>>  arch/arm/boot/dts/twl6030.dtsi    |    5 +++++
>>>  3 files changed, 13 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
>>> index 9880c12..2999eba 100644
>>> --- a/arch/arm/boot/dts/omap4-panda.dts
>>> +++ b/arch/arm/boot/dts/omap4-panda.dts
>>> @@ -126,3 +126,7 @@
>>>       ti,non-removable;
>>>       bus-width = <4>;
>>>  };
>>> +
>>> +&twlusb {
>>> +     usb-supply = <&vusb>;
>>> +};
>>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
>>> index 72216e9..d8290c0 100644
>>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>>> @@ -226,3 +226,7 @@
>>>       bus-width = <4>;
>>>       ti,non-removable;
>>>  };
>>> +
>>> +&twlusb {
>>> +     usb-supply = <&vusb>;
>>> +};
>>> diff --git a/arch/arm/boot/dts/twl6030.dtsi b/arch/arm/boot/dts/twl6030.dtsi
>>> index 3b2f351..8e3aac9 100644
>>> --- a/arch/arm/boot/dts/twl6030.dtsi
>>> +++ b/arch/arm/boot/dts/twl6030.dtsi
>>> @@ -83,4 +83,9 @@
>>>       clk32kg: regulator@12 {
>>>               compatible = "ti,twl6030-clk32kg";
>>>       };
>>> +
>>> +     twlusb: twl6030-usb {
>>
>> That name should be a generic device class name is possible.
>> What is twl6030-usb exactly? an USB PHY?
> 
> Of late we are calling it the comparator as it's used only to detect
> VBUS/ID events.
> Should it be like usb-comparator? What should be the label?

usb-comparator looks good and generic enough.

The label is useless until you reference it somewhere else. But in the
case of the label, it can be more specific.
But you can just use "twl-usb-comparator" for example.

>>
>>> +             compatible = "ti,twl6030-usb";
>>> +             interrupts = < 4 10 >;
>>
>> If this is for two interrupts, you'd better split them to avoid
>> confusion with irq specifiers that requires several attributes like for
>> the GIC.
> 
> yeah. Thats for 2 interrupts.
>>
>> +               interrupts = <4>, <10>; /* IRQ1 blabla, IRQ2 blabla*/
>>
>> The comments are not mandatory assuming the binding is documented.
> 
> It's documented *usb: twl6030: Add dt support for twl6030 usb*

Yeah, sorry for that, but it is indeed very confusing to review the DTS
without having the binding documentation already merged :-(

Regards,
Benoit


--
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/