linux-kernel - RE: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000

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: <50B33AC5ED75F74F991980326F1C438D0FBE89FB@PGSMSX108.gar.corp.intel.com>
Date:	Fri, 26 Sep 2014 09:36:06 +0000
From:	"Chang, Rebecca Swee Fun" <rebecca.swee.fun.chang@...el.com>
To:	'Mika Westerberg' <mika.westerberg@...ux.intel.com>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000



> -----Original Message-----
> From: 'Mika Westerberg' [mailto:mika.westerberg@...ux.intel.com]
> Sent: 26 September, 2014 5:18 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; linux-gpio@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
> 
> On Fri, Sep 26, 2014 at 09:14:48AM +0000, Chang, Rebecca Swee Fun wrote:
> > > > @@ -203,6 +376,17 @@ static int sch_gpio_probe(struct
> > > > platform_device
> > > *pdev)
> > > >  				 pdev->name))
> > > >  		return -EBUSY;
> > > >
> > > > +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > > > +	sch->irq_support = !!irq;
> > > > +	if (sch->irq_support) {
> > > > +		sch->irq_num = irq->start;
> > > > +		if (sch->irq_num < 0) {
> > > > +			dev_warn(&pdev->dev,
> > > > +				 "failed to obtain irq number for device\n");
> > > > +			sch->irq_support = 0;
> > > > +		}
> > > > +	}
> > > > +
> > > >  	spin_lock_init(&sch->lock);
> > > >  	sch->iobase = res->start;
> > > >  	sch->chip = sch_gpio_chip;
> > > > @@ -251,17 +435,72 @@ static int sch_gpio_probe(struct
> > > > platform_device
> > > *pdev)
> > > >  		return -ENODEV;
> > > >  	}
> > > >
> > > > +	err = gpiochip_add(&sch->chip);
> > > > +	if (err < 0)
> > > > +		goto err_sch_gpio;
> > > > +
> > > > +	if (sch->irq_support) {
> > > > +		sch->irq_base = irq_alloc_descs(-1, 0, sch->chip.ngpio,
> > > > +						NUMA_NO_NODE);
> > > > +		if (sch->irq_base < 0) {
> > > > +			dev_err(&pdev->dev,
> > > > +				"failed to add GPIO IRQ descs\n");
> > > > +			sch->irq_base = -1;
> > > > +			goto err_sch_intr_chip;
> > > > +		}
> > >
> > > Was there some reason why you can't use gpiochip_irqchip_* helpers here?
> >
> > I will look into the helpers function and see what I can change here.
> >
> > >
> > > > +
> > > > +		/* disable interrupts */
> > > > +		sch_gpio_irq_disable_all(sch, sch->chip.ngpio);
> > > > +
> > > > +		err = request_irq(sch->irq_num, sch_gpio_irq_handler,
> > > > +				  IRQF_SHARED, KBUILD_MODNAME, sch);
> > >
> > > This seems weird, typically irqchip drivers don't call request_irq()
> > > directly but instead irq_set_chained_handler() or similar. With
> > > gpiochip_irqchip_* stuff you don't need even that.
> > >
> > Regarding this, gpio-sch is actually using shared interrupts and the
> > IRQ resources are from ACPI SCI. As per my understanding, resources
> > from ACPI SCI might be shared for power management purposes. In this
> > case, irq_set_chained_handler() might not be able to use here. What do
> > you think?
> 
> I think you are right. And then you can't use gpiochip_irqchip_* helpers either :-
> (

Then I shall submit the changes for the first patch in V2 series for further review. Thanks.

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