linux-kernel - Re: [PATCH v2 2/2] arm: dts: add support for AM437x StarterKit

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: <53A24A39.4080100@ti.com>
Date:	Wed, 18 Jun 2014 21:26:01 -0500
From:	Nishanth Menon <nm@...com>
To:	balbi@...com
CC:	devicetree@...r.kernel.org, linux@....linux.org.uk,
	Benoit Cousson <bcousson@...libre.com>,
	Tony Lindgren <tony@...mide.com>,
	Rajendra Nayak <rnayak@...com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Darren Etheridge <detheridge@...com>, r.sricharan@...com,
	robh+dt@...nel.org, Josh Elliot <jelliott@...com>,
	galak@...eaurora.org,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	Linux ARM Kernel Mailing List 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/2] arm: dts: add support for AM437x StarterKit

On 06/18/2014 06:19 PM, Felipe Balbi wrote:
[...]
>>>>> Add support for TI's AM437x StarterKit Evaluation
>>>>> Module.
>>>>
>>>> is there a link for this platform?
>>>
>>> internal only
>>
>> but will eventually be sold externally? I assume this is not an TI
>
> probably, but there's nothing public yet.
>
>> internal only board.
>
> correct assumption for all I know.

Yikes.. ok.. I'd let Tony et.al make the call on this, I guess.

[...]
>>>>> +	edt-ft5306@38 {
>>>>> +		status = "okay";
>>>>> +		compatible = "edt,edt-ft5306", "edt,edt-ft5x06";
>>>>> +		pinctrl-names = "default";
>>>>> +		pinctrl-0 = <&edt_ft5306_ts_pins>;
>>>>> +		reg = <0x38>;
>>>>> +		interrupt-parent = <&gpio0>;
>>>>> +		interrupts = <31 0>;
>>>>> +
>>>>> +		wake-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>>>>
>>>> why wake-gpios? we should be using pinctrl with interrupt-extended to
>>>> do wakeup sequence, no?
>>>
>>> sure, can you patch the edt driver ? I'll fix the DTS after that gets
>>> merged
>>
>> If you really want to go down that road, so you could probably help
>> review the pinctrl patches I posted to enable pinctrl wakeup[1]?
>>
>> Come on, as of today, there is no ability to suspend AM437x without
>> doing [1], let alone talk about wakeup gpio vs interrupt-extended. and
>> do we really want to wakeup from suspend when touch screen is touched?
>>
>> Do you expect wake-gpio to work even after doing interrupt based
>> solution? I am no edt driver expert... maybe you can help me here.
>
> you missed the point entirely. This pin is not used for the touchscreen
> to wake SoC up, it's the other way around, see how the pin is an
> *output*. Pull it low and the touchscreen won't generate IRQs, won't
> respond to i2c accesses, etc. Pull it high, and the thing wakes up.

Aaah.. My apologies.. I was confused. Thanks for clarifying.

[...]
>>>>> +	cd-gpios = <&gpio0 6 GPIO_ACTIVE_HIGH>;
>>>>> +};
>>>>> +
>>>>> +&usb2_phy1 {
>>>>> +	status = "okay";
>>>>> +};
>>>>> +
>>>>> +&usb1 {
>>>>> +	dr_mode = "peripheral";
>>>>> +	status = "okay";
>>>>> +};
>>>>> +
>>>>> +&usb2_phy2 {
>>>>> +	status = "okay";
>>>>> +};
>>>>> +
>>>>> +&usb2 {
>>>>> +	dr_mode = "host";
>>>>> +	status = "okay";
>>>>> +};
>>>> none of the above need pinctrl? no regulator supplies?
>>>
>>> pins in default states, drivers don't use regulators.
>>
>> USB works without a supply? even a fixed voltage supply? that is
>> weird.
>
> take a look at the minicom output I posted if you don't believe. Well,
> to be exact, tps63010 [1] is the one which generates the regulated V5_0D
> which is used as VBUS_USB. The enable pin in that device is tied to the
> 3v3 rail (dcdc4 regulator in the PMIC as most everything else) but
> there's no way (otherwise) to control that thing. There's no control
> bus, no way to write a driver.
>
> Since the board will anyways turn off if you disable the 3v3 rail, it's
> pretty much pointless to figure out a hack just to add this to DTS.
>
> [1] http://www.ti.com/product/TPS63010

I am sure to trust you on the test log :) ->  but then from dts 
description perspective, it is good if we describe the supplies, even as 
a always on fixed-regulator. We had instances like 2430SDP ethernet 
where... umm... we originally missed describing ethernet supply and 
boom, one fine morning, no more nfs filesystem - I mean, it is a one off 
scenario there, but describing regulators helps us atleast understand 
the power tree of the board a little better.

Again, no strong opinions on my side, it is a good thing to do is all I 
feel about it.

--
Regards,
Nishanth Menon
--
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/