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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Jul 2012 09:52:26 +0300
From:	Felipe Balbi <balbi@...com>
To:	"Poddar, Sourav" <sourav.poddar@...com>
Cc:	Kevin Hilman <khilman@...com>,
	"DebBarma, Tarun Kanti" <tarun.kanti@...com>,
	lo <linux-omap@...r.kernel.org>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Benoit Cousson <b-cousson@...com>,
	linux-kernel@...r.kernel.org, Felipe Balbi <balbi@...com>
Subject: Re: [GPIO] Crashed when not using

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 ?

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)