Re: [GPIO] Crashed when not using

[Felipe Balbi]

Hi,

On Tue, Jul 31, 2012 at 02:32:43PM +0200, Shilimkar, Santosh wrote:
> On Tue, Jul 31, 2012 at 8:52 AM, Felipe Balbi <balbi@xxxxxx> 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.

Isn't this exactly what Sourav has ?

> > > >>>> +++ 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 */
> > > >>>>         };
> > > >>>>  };

???

-- 
balbi

Attachment: signature.asc
Description: Digital signature