linux-kernel - Re: [GPIO] Crashed when not using

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] [day] [month] [year] [list]
Message-ID: <CAMQu2gwbqdA1bjFWCFyCAUVsU5x=fr91xX17k4xiZQh4F44Jng@mail.gmail.com>
Date:	Mon, 6 Aug 2012 11:16:47 +0530
From:	"Shilimkar, Santosh" <santosh.shilimkar@...com>
To:	"Poddar, Sourav" <sourav.poddar@...com>
Cc:	balbi@...com, Kevin Hilman <khilman@...com>,
	"DebBarma, Tarun Kanti" <tarun.kanti@...com>,
	lo <linux-omap@...r.kernel.org>,
	Benoit Cousson <b-cousson@...com>, linux-kernel@...r.kernel.org
Subject: Re: [GPIO] Crashed when not using

On Tue, Jul 31, 2012 at 6:26 PM, Poddar, Sourav <sourav.poddar@...com> wrote:
> Hi Santosh,
>
> On Tue, Jul 31, 2012 at 6:02 PM, Shilimkar, Santosh
> <santosh.shilimkar@...com> wrote:
>> On Tue, Jul 31, 2012 at 8:52 AM, Felipe Balbi <balbi@...com> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Jul 31, 2012 at 10:23:16AM +0530, Poddar, Sourav wrote:
>>> > >>>> The device tree data for acquiring the above GPIO interrupt line
>>> > >>>> looks
>>> > >>>> like this.
>>> > >>>>
>>> > >>>> +++ linux-omap-storage/arch/arm/boot/dts/omap5-evm.dts  2012-07-30
>>> > >>>> 14:11:08.931694001 +0530
>>> > >>>> @@ -42,7 +42,8 @@
>>> > >>>>         tsl2771@39 {
>>> > >>>>                 compatible = "taos,tsl2771";
>>> > >>>>                 reg = <0x39>;
>>> > >>>> +                interrupt-parent = <&gpio5>;
>>> > >>>> +                interrupts = <21>; /* gpio line 149 */
>>> > >>>>         };
>>> > >>>>  };
>>> > >>>>
>>> > >>>> Note: using "gpio_request_one" in the driver solves the issue.
>>> > >>>> Is using this api in the driver required?
>>> > >>>> Any pointer on the above crash?
>>> > >>>
>>> > >> Hi Tarun,
>>> > >>> Any user/client driver of GPIO is supposed to go through
>>> > >>> gpio_request() API so that module clock
>>> > >>> is enabled correctly. Overriding of APIs would put the power
>>> > >>> management state machine in jeopardy.
>>> > >>> --
>>> > >> I tried putting "pm_runtime_get_sync" in gpio_irq_type api where the
>>> > >> kernel
>>> > >> is crashing and the crash is no longer observed. So indeed, its about
>>> > >> enabling clocks.
>>> > >>
>>> > >> One doubt: Can't we put runtime apis in "gpio_irq_type" and eliminate
>>> > >> the use of
>>> > >> "gpio_request_one"??
>>> > >
>>> > > No.
>>> > >
>>> > > You must use the GPIO requiest/free APIs to tell the GPIO core that
>>> > > the GPIO line is in use.
>>> > >
>>> > Thanks for this confirmation.
>>> > > Why do you want to avoid using gpio_request/gpio_free?
>>> > >
>>> > I was assuming that DT based gpio IRQ registration will automatically
>>> > take care of
>>> > the above APIs. But since that is not the case(as mentioned by
>>> > santosh),  we need to use the
>>> > gpio_request/free apis.
>>>
>>> Hang on for a while, let's try to get to the bottom of this debate first
>>> ;-)
>>>
>>> We have a canonical way of passing IRQ numbers to drivers through DT and
>>> that is the "interrupts" attribute. It shouldn't matter if that IRQ pin
>>> is connected to a real IRQ line or through a GPIO controller. In both
>>> cases we should use the "interrupts" attribute.
>>>
>>> If DT core doesn't allocate the GPIO for us then how does this work:
>>>
>>> (omap4-sdp.dts)
>>>
>>> 127 &mcspi1 {
>>> 128         eth@0 {
>>> 129                 compatible = "ks8851";
>>> 130                 spi-max-frequency = <24000000>;
>>> 131                 reg = <0>;
>>> 132                 interrupt-parent = <&gpio2>;
>>> 133                 interrupts = <2>; /* gpio line 34 */
>>> 134                 vdd-supply = <&vdd_eth>;
>>> 135         };
>>> 136 };
>>>
>>>
>>> There's no gpio request on the driver:
>>>
>>> $ git grep -e gpio_request drivers/net/ethernet/micrel/ks8851.c
>>> $
>>>
>>> Since Benoit was the one who added that to the dts file (commit
>>> e7c64db9), I assume he tested his patch before posting, so again I ask -
>>> How does that work and why doesn't this work for Sourav's tsl2771
>>> controller ?
>>>
>>> This is either a regression on drivers/of, or commit e7c64db9 is also
>>> broken...
>>>
>>> Benoit, do you know how should this work ?
>>>
>> I had a discussion with Benoit on this. In fact there is a way to actually
>> trigger the GPIO request.
>>
>>> 132                 interrupt-parent = <&gpio2>;
>>> 133                 interrupts = <2>; /* gpio line 34 */
>>
>> As above you can see, GPIO2 bank and 2nd line.
>> And then it will make use of gpio_irq chip properties
>> to probe the GPIO line.
>>
>> Saurabh can try this out for his use case.
>>
> I am using the above properties only and seeing the issue.
>
There seeems to be an issue with the GPIO DT probing. The Ethernet
works because there is another GPIO line from BANK2 is probed and since
BANK2 shares the clock, you won't see any issue.

We are looking at fixing the issue. Will keep the thread posted.

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