| Message ID | 1474195951-7238-2-git-send-email-yadi.hu@windriver.com |
|---|---|
| State | Accepted |
| Headers | show |
On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote: > From: "Yadi.hu" <yadi.hu@windriver.com> > > the eg20t driver call request_irq() function before the pch_base_address, > base address of i2c controller's register, is assigned an effective value. > > there is one possible scenario that an interrupt which isn't inside eg20t > arrives immediately after request_irq() is executed when i2c controller > shares an interrupt number with others. since the interrupt handler > pch_i2c_handler() has already active as shared action, it will be called > and read its own register to determine if this interrupt is from itself. > > At that moment, since base address of i2c registers is not remapped > in kernel space yet,so the INT handler will access an illegal address > and then a error occurs. > > Signed-off-by: Yadi.hu <yadi.hu@windriver.com> Applied to for-next, thanks! Please make sure "V3" also appears in the patch subject since patch management tools pick this up. "-v <nr>" in recent git versions makes this super easy.
On Wed, Sep 21, 2016 at 06:16:09PM +0200, Wolfram Sang wrote: > On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote: > > From: "Yadi.hu" <yadi.hu@windriver.com> > > > > the eg20t driver call request_irq() function before the pch_base_address, > > base address of i2c controller's register, is assigned an effective value. > > > > there is one possible scenario that an interrupt which isn't inside eg20t > > arrives immediately after request_irq() is executed when i2c controller > > shares an interrupt number with others. since the interrupt handler > > pch_i2c_handler() has already active as shared action, it will be called > > and read its own register to determine if this interrupt is from itself. > > > > At that moment, since base address of i2c registers is not remapped > > in kernel space yet,so the INT handler will access an illegal address > > and then a error occurs. > > > > Signed-off-by: Yadi.hu <yadi.hu@windriver.com> > > Applied to for-next, thanks! I meant: applied to for-current!
On 2016年09月22日 00:16, Wolfram Sang wrote: > On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote: >> From: "Yadi.hu" <yadi.hu@windriver.com> >> >> the eg20t driver call request_irq() function before the pch_base_address, >> base address of i2c controller's register, is assigned an effective value. >> >> there is one possible scenario that an interrupt which isn't inside eg20t >> arrives immediately after request_irq() is executed when i2c controller >> shares an interrupt number with others. since the interrupt handler >> pch_i2c_handler() has already active as shared action, it will be called >> and read its own register to determine if this interrupt is from itself. >> >> At that moment, since base address of i2c registers is not remapped >> in kernel space yet,so the INT handler will access an illegal address >> and then a error occurs. >> >> Signed-off-by: Yadi.hu <yadi.hu@windriver.com> > Applied to for-next, thanks! > > Please make sure "V3" also appears in the patch subject since patch > management tools pick this up. "-v <nr>" in recent git versions makes > this super easy. Got it, I will resend the patch with V3 tag. Yadi -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016年09月22日 00:18, Wolfram Sang wrote: > On Wed, Sep 21, 2016 at 06:16:09PM +0200, Wolfram Sang wrote: >> On Sun, Sep 18, 2016 at 06:52:31PM +0800, Yadi Hu wrote: >>> From: "Yadi.hu" <yadi.hu@windriver.com> >>> >>> the eg20t driver call request_irq() function before the pch_base_address, >>> base address of i2c controller's register, is assigned an effective value. >>> >>> there is one possible scenario that an interrupt which isn't inside eg20t >>> arrives immediately after request_irq() is executed when i2c controller >>> shares an interrupt number with others. since the interrupt handler >>> pch_i2c_handler() has already active as shared action, it will be called >>> and read its own register to determine if this interrupt is from itself. >>> >>> At that moment, since base address of i2c registers is not remapped >>> in kernel space yet,so the INT handler will access an illegal address >>> and then a error occurs. >>> >>> Signed-off-by: Yadi.hu <yadi.hu@windriver.com> >> Applied to for-next, thanks! > I meant: applied to for-current! one quick and stupid question, what's different between for-current and for-next branch? Yadi > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >Applied to for-next, thanks! > > > >Please make sure "V3" also appears in the patch subject since patch > >management tools pick this up. "-v <nr>" in recent git versions makes > >this super easy. > > Got it, I will resend the patch with V3 tag. No need, it is already applied. for-current carries patches for 4.8, for-next for 4.9.
diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c index 137125b..5ce71ce 100644 --- a/drivers/i2c/busses/i2c-eg20t.c +++ b/drivers/i2c/busses/i2c-eg20t.c @@ -773,13 +773,6 @@ static int pch_i2c_probe(struct pci_dev *pdev, /* Set the number of I2C channel instance */ adap_info->ch_num = id->driver_data; - ret = request_irq(pdev->irq, pch_i2c_handler, IRQF_SHARED, - KBUILD_MODNAME, adap_info); - if (ret) { - pch_pci_err(pdev, "request_irq FAILED\n"); - goto err_request_irq; - } - for (i = 0; i < adap_info->ch_num; i++) { pch_adap = &adap_info->pch_data[i].pch_adapter; adap_info->pch_i2c_suspended = false; @@ -797,6 +790,17 @@ static int pch_i2c_probe(struct pci_dev *pdev, pch_adap->dev.of_node = pdev->dev.of_node; pch_adap->dev.parent = &pdev->dev; + } + + ret = request_irq(pdev->irq, pch_i2c_handler, IRQF_SHARED, + KBUILD_MODNAME, adap_info); + if (ret) { + pch_pci_err(pdev, "request_irq FAILED\n"); + goto err_request_irq; + } + + for (i = 0; i < adap_info->ch_num; i++) { + pch_adap = &adap_info->pch_data[i].pch_adapter; pch_i2c_init(&adap_info->pch_data[i]);