Hi,
I have been in the process of trying to write a new PCI driver. The
hardware is Memory Mapped IO device similar to the Fusion 878, but not
that complicated, but simpler
Now that i have been trying to implement the driver using the new PCI
API, i feel a bit lost at the different changes gone into the PCI API.
So if someone could give me a brief idea how a minimal PCI probe routine
should consist of, that would be quite helpful.
Thanks,
Manu
Manu Abraham napsal(a):
> Now that i have been trying to implement the driver using the new PCI
> API, i feel a bit lost at the different changes gone into the PCI API.
> So if someone could give me a brief idea how a minimal PCI probe
> routine should consist of, that would be quite helpful.
Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12, pages 311+.
regards,
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Now that i have been trying to implement the driver using the new PCI
>> API, i feel a bit lost at the different changes gone into the PCI
>> API. So if someone could give me a brief idea how a minimal PCI probe
>> routine should consist of, that would be quite helpful.
>
>
> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12, pages
> 311+.
>
I have been updating myself from LDD2 to LDD3. What i was wondering was
in what order should i be calling the functions.
Thanks,
Manu
Manu Abraham napsal(a):
> Jiri Slaby wrote:
>
>> Manu Abraham napsal(a):
>>
>>> Now that i have been trying to implement the driver using the new
>>> PCI API, i feel a bit lost at the different changes gone into the
>>> PCI API. So if someone could give me a brief idea how a minimal PCI
>>> probe routine should consist of, that would be quite helpful.
>>
>>
>>
>> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12,
>> pages 311+.
>>
>
> I have been updating myself from LDD2 to LDD3. What i was wondering
> was in what order should i be calling the functions.
You won't call anything, kernel does. You only register driver.
struct pcitbl {venids, devids}
struct driver ... = {probe=a, remove=b, tbl=pcitbl};
a() { if device from pcitbl is in the system (or has been added before
some little time) this function is called}
b() {if the device was removed from system: modules and hotplug: never
called; modules: so if modules unload; if both: if the device was
removed on the fly, or module unload}
module_init() { register_driver(driver)}
module_exit() { unregister_driver(driver); }
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Jiri Slaby wrote:
>>
>>> Manu Abraham napsal(a):
>>>
>>>> Now that i have been trying to implement the driver using the new
>>>> PCI API, i feel a bit lost at the different changes gone into the
>>>> PCI API. So if someone could give me a brief idea how a minimal PCI
>>>> probe routine should consist of, that would be quite helpful.
>>>
>>>
>>>
>>>
>>> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12,
>>> pages 311+.
>>>
>>
>> I have been updating myself from LDD2 to LDD3. What i was wondering
>> was in what order should i be calling the functions.
>
>
> You won't call anything, kernel does. You only register driver.
> struct pcitbl {venids, devids}
>
> struct driver ... = {probe=a, remove=b, tbl=pcitbl};
> a() { if device from pcitbl is in the system (or has been added before
> some little time) this function is called}
> b() {if the device was removed from system: modules and hotplug: never
> called; modules: so if modules unload; if both: if the device was
> removed on the fly, or module unload}
>
> module_init() { register_driver(driver)}
> module_exit() { unregister_driver(driver); }
>
I was wondering whether pci_enable_device() should come first or
pci_dev_put() in the probe routine.
Thanks,
Manu
Manu Abraham napsal(a):
> Jiri Slaby wrote:
>
>> Manu Abraham napsal(a):
>>
>>> Jiri Slaby wrote:
>>>
>>>> Manu Abraham napsal(a):
>>>>
>>>>> Now that i have been trying to implement the driver using the new
>>>>> PCI API, i feel a bit lost at the different changes gone into the
>>>>> PCI API. So if someone could give me a brief idea how a minimal PCI
>>>>> probe routine should consist of, that would be quite helpful.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12,
>>>> pages 311+.
>>>>
>>>
>>> I have been updating myself from LDD2 to LDD3. What i was wondering
>>> was in what order should i be calling the functions.
>>
>>
>>
>> You won't call anything, kernel does. You only register driver.
>> struct pcitbl {venids, devids}
>>
>> struct driver ... = {probe=a, remove=b, tbl=pcitbl};
>> a() { if device from pcitbl is in the system (or has been added before
>> some little time) this function is called}
>> b() {if the device was removed from system: modules and hotplug: never
>> called; modules: so if modules unload; if both: if the device was
>> removed on the fly, or module unload}
>>
>> module_init() { register_driver(driver)}
>> module_exit() { unregister_driver(driver); }
>>
>
> I was wondering whether pci_enable_device() should come first or
> pci_dev_put() in the probe routine.
pci_dev_put? No, it counts down reference count, so you would loose the
structure. You do NOT do pci_dev_put anymore with pci probing (but some very
very specific cases).
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Jiri Slaby wrote:
>>
>>> Manu Abraham napsal(a):
>>>
>>>> Jiri Slaby wrote:
>>>>
>>>>> Manu Abraham napsal(a):
>>>>>
>>>>>> Now that i have been trying to implement the driver using the new
>>>>>> PCI API, i feel a bit lost at the different changes gone into the
>>>>>> PCI API. So if someone could give me a brief idea how a minimal
>>>>>> PCI probe routine should consist of, that would be quite helpful.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12,
>>>>> pages 311+.
>>>>>
>>>>
>>>> I have been updating myself from LDD2 to LDD3. What i was wondering
>>>> was in what order should i be calling the functions.
>>>
>>>
>>>
>>>
>>> You won't call anything, kernel does. You only register driver.
>>> struct pcitbl {venids, devids}
>>>
>>> struct driver ... = {probe=a, remove=b, tbl=pcitbl};
>>> a() { if device from pcitbl is in the system (or has been added
>>> before some little time) this function is called}
>>> b() {if the device was removed from system: modules and hotplug:
>>> never called; modules: so if modules unload; if both: if the device
>>> was removed on the fly, or module unload}
>>>
>>> module_init() { register_driver(driver)}
>>> module_exit() { unregister_driver(driver); }
>>>
>>
>> I was wondering whether pci_enable_device() should come first or
>> pci_dev_put() in the probe routine.
>
> pci_dev_put? No, it counts down reference count, so you would loose
> the structure. You do NOT do pci_dev_put anymore with pci probing (but
> some very very specific cases).
Oh, i thought after a pci_dev_get() one does a pci_dev_put()
I wrote something like this, i was now confused with what to do with
pci_get_drvdata() and pci_set_drvdata()
I am not very sure whether i am doing it right in the first place, since
my mapped memory seems to be wrong, but first i have to clear up about
pci_get/set_drvdata().
Manu
static int mantis_pci_probe(*struct* pci_dev *pdev, const *struct* pci_device_id *mantis_pci_table)
{
*struct* mantis_pci *mantis;
u8 revision, latency;
pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11, NULL);
*if* (pdev) {
dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
*if* ((mantis = (*struct* mantis_pci *) kmalloc(*sizeof* (*struct* mantis_pci), GFP_KERNEL)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
*return* -ENOMEM;
}
*if* (pci_enable_device(pdev) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Could not enable device");
kfree(mantis);
*return* -ENODEV;
}
mantis->pdev = pdev;
mantis->mantis_addr = pci_resource_start(pdev, 0);
*if* (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
kfree(mantis);
*return* -EBUSY;
}
pci_read_config_byte(mantis->pdev, PCI_CLASS_REVISION, &revision);
pci_read_config_byte(mantis->pdev, PCI_LATENCY_TIMER, &latency);
mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
mmwrite(0, MANTIS_INT_STAT); //* Clear interrupts *//
*if* (request_irq(mantis->pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
release_mem_region(pci_resource_start(mantis->pdev, 0),
pci_resource_len(mantis->pdev, 0));
pci_disable_device(pdev);
kfree(mantis);
}
pci_dev_put(pdev);
}
*return* 0;
}
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Jiri Slaby wrote:
>>
>>> Manu Abraham napsal(a):
>>>
>>>> Jiri Slaby wrote:
>>>>
>>>>> Manu Abraham napsal(a):
>>>>>
>>>>>> Now that i have been trying to implement the driver using the new
>>>>>> PCI API, i feel a bit lost at the different changes gone into the
>>>>>> PCI API. So if someone could give me a brief idea how a minimal
>>>>>> PCI probe routine should consist of, that would be quite helpful.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12,
>>>>> pages 311+.
>>>>>
>>>>
>>>> I have been updating myself from LDD2 to LDD3. What i was wondering
>>>> was in what order should i be calling the functions.
>>>
>>>
>>>
>>>
>>> You won't call anything, kernel does. You only register driver.
>>> struct pcitbl {venids, devids}
>>>
>>> struct driver ... = {probe=a, remove=b, tbl=pcitbl};
>>> a() { if device from pcitbl is in the system (or has been added
>>> before some little time) this function is called}
>>> b() {if the device was removed from system: modules and hotplug:
>>> never called; modules: so if modules unload; if both: if the device
>>> was removed on the fly, or module unload}
>>>
>>> module_init() { register_driver(driver)}
>>> module_exit() { unregister_driver(driver); }
>>>
>>
>> I was wondering whether pci_enable_device() should come first or
>> pci_dev_put() in the probe routine.
>
> pci_dev_put? No, it counts down reference count, so you would loose
> the structure. You do NOT do pci_dev_put anymore with pci probing (but
> some very very specific cases).
Oh, i thought after a pci_dev_get() one does a pci_dev_put()
I wrote something like this, i was now confused with what to do with
pci_get_drvdata() and pci_set_drvdata()
I am not very sure whether i am doing it right in the first place, since
my mapped memory seems to be wrong, but first i have to clear up about
pci_get/set_drvdata().
Manu
static int mantis_pci_probe(*struct* pci_dev *pdev, const *struct*
pci_device_id *mantis_pci_table)
{
*struct* mantis_pci *mantis;
u8 revision, latency;
pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11,
NULL);
*if* (pdev) {
dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
*if* ((mantis = (*struct* mantis_pci *) kmalloc(*sizeof* (*struct*
mantis_pci), GFP_KERNEL)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
*return* -ENOMEM;
}
*if* (pci_enable_device(pdev) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Could not enable device");
kfree(mantis);
*return* -ENODEV;
}
mantis->pdev = pdev;
mantis->mantis_addr = pci_resource_start(pdev, 0);
*if* (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
kfree(mantis);
*return* -EBUSY;
}
pci_read_config_byte(mantis->pdev, PCI_CLASS_REVISION, &revision);
pci_read_config_byte(mantis->pdev, PCI_LATENCY_TIMER, &latency);
mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
mmwrite(0, MANTIS_INT_STAT); //* Clear interrupts *//
*if* (request_irq(mantis->pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
release_mem_region(pci_resource_start(mantis->pdev, 0),
pci_resource_len(mantis->pdev, 0));
pci_disable_device(pdev);
kfree(mantis);
}
pci_dev_put(pdev);
}
*return* 0;
}
Manu Abraham napsal(a):
> Jiri Slaby wrote:
>
>> Manu Abraham napsal(a):
>>
>>> Jiri Slaby wrote:
>>>
>>>> Manu Abraham napsal(a):
>>>>
>>>>> Jiri Slaby wrote:
>>>>>
>>>>>> Manu Abraham napsal(a):
>>>>>>
>>>>>>> Now that i have been trying to implement the driver using the
>>>>>>> new PCI API, i feel a bit lost at the different changes gone
>>>>>>> into the PCI API. So if someone could give me a brief idea how a
>>>>>>> minimal PCI probe routine should consist of, that would be quite
>>>>>>> helpful.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Maybe, you want to read http://lwn.net/Kernel/LDD3/, chapter 12,
>>>>>> pages 311+.
>>>>>>
>>>>>
>>>>> I have been updating myself from LDD2 to LDD3. What i was
>>>>> wondering was in what order should i be calling the functions.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> You won't call anything, kernel does. You only register driver.
>>>> struct pcitbl {venids, devids}
>>>>
>>>> struct driver ... = {probe=a, remove=b, tbl=pcitbl};
>>>> a() { if device from pcitbl is in the system (or has been added
>>>> before some little time) this function is called}
>>>> b() {if the device was removed from system: modules and hotplug:
>>>> never called; modules: so if modules unload; if both: if the device
>>>> was removed on the fly, or module unload}
>>>>
>>>> module_init() { register_driver(driver)}
>>>> module_exit() { unregister_driver(driver); }
>>>>
>>>
>>> I was wondering whether pci_enable_device() should come first or
>>> pci_dev_put() in the probe routine.
>>
>>
>> pci_dev_put? No, it counts down reference count, so you would loose
>> the structure. You do NOT do pci_dev_put anymore with pci probing
>> (but some very very specific cases).
>
>
>
> Oh, i thought after a pci_dev_get() one does a pci_dev_put()
> I wrote something like this, i was now confused with what to do with
> pci_get_drvdata() and pci_set_drvdata()
> I am not very sure whether i am doing it right in the first place, since
> my mapped memory seems to be wrong, but first i have to clear up about
> pci_get/set_drvdata().
>
>
> Manu
>
> static int mantis_pci_probe(*struct* pci_dev *pdev, const *struct*
> pci_device_id *mantis_pci_table)
> {
> *struct* mantis_pci *mantis;
> u8 revision, latency;
>
> pdev = pci_get_device(PCI_VENDOR_ID_MANTIS,
> PCI_DEVICE_ID_MANTIS_R11, NULL);
you do NOT do this at all, because you have pdev already (the param of
the probe function)
> *if* (pdev) {
> dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
> *if* ((mantis = (*struct* mantis_pci *) kmalloc(*sizeof*
> (*struct* mantis_pci), GFP_KERNEL)) == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
> *return* -ENOMEM;
> }
> *if* (pci_enable_device(pdev) < 0) {
you do only this
> dprintk(verbose, MANTIS_ERROR, 1, "Could not enable device");
> kfree(mantis);
> *return* -ENODEV;
gotos and return on one place
> }
> mantis->pdev = pdev;
you don't need this, need you? you put mantis with pci_setdrvdata and
whenever you want it, you get it from it
> mantis->mantis_addr = pci_resource_start(pdev, 0);
> *if* (!request_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0), DRIVER_NAME)) {
> kfree(mantis);
> *return* -EBUSY;
> }
> pci_read_config_byte(mantis->pdev, PCI_CLASS_REVISION,
> &revision);
> pci_read_config_byte(mantis->pdev, PCI_LATENCY_TIMER, &latency);
> mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
> mmwrite(0, MANTIS_INT_STAT); //* Clear interrupts *//
> *if* (request_irq(mantis->pdev->irq, (void *) mantis_pci_irq,
> SA_SHIRQ |
> SA_INTERRUPT, DRIVER_NAME, (void *) mantis) <
> 0) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration
> failed");
> release_mem_region(pci_resource_start(mantis->pdev, 0),
> pci_resource_len(mantis->pdev, 0));
> pci_disable_device(pdev);
> kfree(mantis);
> }
> pci_dev_put(pdev);
don't do that
>
>
> }
>
>
> *return* 0;
> }
>
>
Jiri Slaby wrote:
> you do NOT do this at all, because you have pdev already (the param of
> the probe function)
>
I rewrote the entire thing like this including the pci_remove function
too, but now it so seems that in the remove function,
pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at module
removal.
Manu
struct mantis_pci {
/* PCI stuff */
__u16 id;
__u16 vendor_id;
__u16 device_id;
__u16 sub_vendor_id;
__u16 sub_device_id;
__u8 latency;
/* Linux PCI */
struct pci_dev *pdev;
__u32 mantis_addr;
volatile __u8 __iomem *mantis_mmio;
__u8 irq;
__u8 revision;
__u16 mantis_card_num;
/* RISC Core */
__u32 block_count;
__u32 block_bytes;
__u32 line_bytes;
__u32 line_count;
__u32 risc_pos;
__u32 buf_size;
__u8 *buf_cpu;
dma_addr_t buf_dma;
__u32 risc_size;
__u32 *risc_cpu;
dma_addr_t risc_dma;
};
static int mantis_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *mantis_pci_table)
{
struct mantis_pci *mantis;
struct mantis_eeprom eeprom;
u8 revision, latency;
u8 data[2];
if (pci_enable_device(pdev)) {
dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
if ((mantis = (struct mantis_pci *) kmalloc(sizeof (struct
mantis_pci), GFP_KERNEL)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
return -ENOMEM;
}
pci_set_master(pdev);
mantis->mantis_addr = pci_resource_start(pdev, 0);
if (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
kfree(mantis);
return -EBUSY;
}
pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
pci_set_drvdata(pdev, mantis);
if (request_irq(pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
pci_disable_device(pdev);
kfree(mantis);
return -ENODEV;
}
mantis_reg_dump(mantis);
}
return 0;
}
static void mantis_pci_remove(struct pci_dev *pdev)
{
struct mantis_pci *mantis;
dprintk(verbose, MANTIS_DEBUG, 1, "Removing Mantis device");
mantis = pci_get_drvdata(pdev);
if (mantis == NULL)
dprintk(verbose, MANTIS_DEBUG, 1, "Aeio, mantis");
dprintk(verbose, MANTIS_ERROR, 1, "Mantis irq: %d, latency: %d\nmemory:
0x%04x, mmio: 0x%p", pdev->irq, mantis->latency,
mantis->mantis_addr, mantis->mantis_mmio);
free_irq(pdev->irq, mantis);
dprintk(verbose, MANTIS_ERROR, 1, "Mantis @ 0x%p", mantis->mantis_mmio);
if (mantis->mantis_mmio)
iounmap((u8 *) mantis->mantis_mmio);
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
pci_disable_device(pdev);
pci_set_drvdata(pdev, NULL);
kfree(mantis);
}
Manu Abraham napsal(a):
> Jiri Slaby wrote:
>
>> you do NOT do this at all, because you have pdev already (the param of
>> the probe function)
>>
>
> I rewrote the entire thing like this including the pci_remove function
> too, but now it so seems that in the remove function,
> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at module
> removal.
Maybe because this is badly written driver.
> static int mantis_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *mantis_pci_table)
> {
> struct mantis_pci *mantis;
> struct mantis_eeprom eeprom;
> u8 revision, latency;
> u8 data[2];
>
> if (pci_enable_device(pdev)) {
> dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
> if ((mantis = (struct mantis_pci *) kmalloc(sizeof (struct
> mantis_pci), GFP_KERNEL)) == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
> return -ENOMEM;
> }
> pci_set_master(pdev);
> mantis->mantis_addr = pci_resource_start(pdev, 0);
> if (!request_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0), DRIVER_NAME)) {
> kfree(mantis);
> return -EBUSY;
> }
> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
> mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
> pci_set_drvdata(pdev, mantis);
if pci_enable_device fails, you set this?? Maybe you haven't read the doc enough.
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Jiri Slaby wrote:
>>
>>> you do NOT do this at all, because you have pdev already (the param
>>> of the probe function)
>>>
>>
>> I rewrote the entire thing like this including the pci_remove function
>> too, but now it so seems that in the remove function,
>> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at module
>> removal.
>
> Maybe because this is badly written driver.
I have not written the driver, but this is my first go at it ..
>
>> static int mantis_pci_probe(struct pci_dev *pdev, const struct
>> pci_device_id *mantis_pci_table)
>> {
>> struct mantis_pci *mantis;
>> struct mantis_eeprom eeprom;
>> u8 revision, latency;
>> u8 data[2];
>> if (pci_enable_device(pdev)) { dprintk(verbose,
>> MANTIS_DEBUG, 1, "Found a mantis chip");
>> if ((mantis = (struct mantis_pci *) kmalloc(sizeof (struct
>> mantis_pci), GFP_KERNEL)) == NULL) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
>> return -ENOMEM;
>> }
>> pci_set_master(pdev);
>> mantis->mantis_addr = pci_resource_start(pdev, 0);
>> if (!request_mem_region(pci_resource_start(pdev, 0),
>> pci_resource_len(pdev, 0), DRIVER_NAME)) {
>> kfree(mantis);
>> return -EBUSY;
>> }
>> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
>> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
>> mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
>> pci_set_drvdata(pdev, mantis);
>
> if pci_enable_device fails, you set this?? Maybe you haven't read the
> doc enough.
I just found that, pci_enable_device() fails. So what's the way to go
ahead ?
Manu
Manu Abraham napsal(a):
> Jiri Slaby wrote:
>
>> Manu Abraham napsal(a):
>>
>>> Jiri Slaby wrote:
>>>
>>>> you do NOT do this at all, because you have pdev already (the param
>>>> of the probe function)
>>>>
>>>
>>> I rewrote the entire thing like this including the pci_remove
>>> function too, but now it so seems that in the remove function,
>>> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at module
>>> removal.
>>
>>
>> Maybe because this is badly written driver.
>
>
> I have not written the driver, but this is my first go at it ..
>
>>
>>> static int mantis_pci_probe(struct pci_dev *pdev, const struct
>>> pci_device_id *mantis_pci_table)
>>> {
>>> struct mantis_pci *mantis;
>>> struct mantis_eeprom eeprom;
>>> u8 revision, latency;
>>> u8 data[2]; if (pci_enable_device(pdev)) {
>>> dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
>>> if ((mantis = (struct mantis_pci *) kmalloc(sizeof (struct
>>> mantis_pci), GFP_KERNEL)) == NULL) {
>>> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
>>> return -ENOMEM;
>>> }
>>> pci_set_master(pdev);
>>> mantis->mantis_addr = pci_resource_start(pdev, 0);
>>> if (!request_mem_region(pci_resource_start(pdev, 0),
>>> pci_resource_len(pdev, 0), DRIVER_NAME)) {
>>> kfree(mantis);
>>> return -EBUSY;
>>> }
>>> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
>>> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
>>> mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
>>> pci_set_drvdata(pdev, mantis);
>>
>>
>> if pci_enable_device fails, you set this?? Maybe you haven't read the
>> doc enough.
>
>
>
> I just found that, pci_enable_device() fails. So what's the way to go
> ahead ?
JESUS.
int retval = 0;
if ((retval = pci_enable_device()))
goto end;
...
pci_set_drvdata(pdev, mantis);
...
end:
return retval;
not
if (pci_enable_device())
do something
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Jiri Slaby wrote:
>>
>>> Manu Abraham napsal(a):
>>>
>>>> Jiri Slaby wrote:
>>>>
>>>>> you do NOT do this at all, because you have pdev already (the param
>>>>> of the probe function)
>>>>>
>>>>
>>>> I rewrote the entire thing like this including the pci_remove
>>>> function too, but now it so seems that in the remove function,
>>>> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at
>>>> module removal.
>>>
>>>
>>>
>>> Maybe because this is badly written driver.
>>
>>
>>
>> I have not written the driver, but this is my first go at it ..
>>
>>>
>>>> static int mantis_pci_probe(struct pci_dev *pdev, const struct
>>>> pci_device_id *mantis_pci_table)
>>>> {
>>>> struct mantis_pci *mantis;
>>>> struct mantis_eeprom eeprom;
>>>> u8 revision, latency;
>>>> u8 data[2]; if (pci_enable_device(pdev)) {
>>>> dprintk(verbose, MANTIS_DEBUG, 1, "Found a mantis chip");
>>>> if ((mantis = (struct mantis_pci *) kmalloc(sizeof (struct
>>>> mantis_pci), GFP_KERNEL)) == NULL) {
>>>> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
>>>> return -ENOMEM;
>>>> }
>>>> pci_set_master(pdev);
>>>> mantis->mantis_addr = pci_resource_start(pdev, 0);
>>>> if (!request_mem_region(pci_resource_start(pdev, 0),
>>>> pci_resource_len(pdev, 0), DRIVER_NAME)) {
>>>> kfree(mantis);
>>>> return -EBUSY;
>>>> }
>>>> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
>>>> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
>>>> mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000);
>>>> pci_set_drvdata(pdev, mantis);
>>>
>>>
>>>
>>> if pci_enable_device fails, you set this?? Maybe you haven't read the
>>> doc enough.
>>
>>
>>
>>
>> I just found that, pci_enable_device() fails. So what's the way to go
>> ahead ?
>
> JESUS.
What i meant is i do have to enable the device, not just exit.
I understood that i have to exit if pci_enable_device() fails, but what
i am looking for is why i can't enable it in the first place.
> int retval = 0;
>
> if ((retval = pci_enable_device()))
> goto end;
>
> ...
> pci_set_drvdata(pdev, mantis);
> ...
>
> end:
> return retval;
>
> not
> if (pci_enable_device())
> do something
Regards,
Manu
Jiri Slaby wrote:
> Manu Abraham napsal(a):
>
>> Jiri Slaby wrote:
>>
>>> Manu Abraham napsal(a):
>>>
>>>> Jiri Slaby wrote:
>>>>
>>>>> you do NOT do this at all, because you have pdev already (the param
>>>>> of the probe function)
>>>>>
>>>>
>>>> I rewrote the entire thing like this including the pci_remove
>>>> function too, but now it so seems that in the remove function,
>>>> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at
>>>> module removal.
>>>
>> I just found that, pci_enable_device() fails. So what's the way to go
>> ahead ?
>
> JESUS.
Hmm.. i finally got it to work. It seems pci_get_device() is necessary,
i can't seem to enable the device or request for an IRQ the way you
suggested. It looks some quirks are there though ..
If only i could explain why it works this way and not the other way ..
Thanks for the help,
Regards,
Manu
[ 81.269655] mantis_pci_probe: Got a device
[ 81.269825] mantis_pci_probe: We got an IRQ
[ 81.269987] mantis_pci_probe: We finally enabled the device
[ 81.270191] Mantis Rev 1, irq: 23, latency: 32
[ 81.270289] memory: 0xefeff000, mmio: f9218000
[ 81.270519] Trying to free free IRQ23
[ 90.485885] mantis_pci_remove: Removing -->Mantis irq: 23, latency: 32
[ 90.485887] memory: 0xefeff000, mmio: 0xf9218000
[ 90.486293] Trying to free free IRQ23
[ 90.486429] Trying to free nonexistent resource <efeff000-efefffff>
static int __devinit mantis_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *mantis_pci_table)
{
u8 revision, latency;
u8 data[2];
struct mantis_pci *mantis;
mantis = (struct mantis_pci *) kmalloc(sizeof (struct mantis_pci),
GFP_KERNEL);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
return -ENOMEM;
}
pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11,
NULL);
if (pdev) {
dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
mantis->mantis_addr = pci_resource_start(pdev, 0);
if (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
dprintk(verbose, MANTIS_ERROR, 1, "Request for memory region failed");
goto err0;
}
if ((mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000)) ==
NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
goto err1;
}
if (request_irq(pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
goto err2;
}
dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
if (pci_enable_device(pdev)) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI device enable failed");
goto err3;
}
dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
pci_set_master(pdev);
pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
mantis->latency = latency;
mantis->revision = revision;
if (!latency) {
pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
}
pci_set_drvdata(pdev, mantis);
dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\nmemory:
0x%04x, mmio: %p\n", pdev->irq, mantis->latency,
mantis->mantis_addr, mantis->mantis_mmio);
pci_dev_put(pdev);
} else {
dprintk(verbose, MANTIS_ERROR, 1, "No device found");
return -ENODEV;
}
err3:
free_irq(pdev->irq, pdev);
err2:
if (mantis->mantis_mmio)
iounmap(mantis->mantis_mmio);
err1:
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
err0:
kfree(mantis);
return 0;
}
static void __devexit mantis_pci_remove(struct pci_dev *pdev)
{
struct mantis_pci *mantis = pci_get_drvdata(pdev);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Aeio, MAntis NULL ptr");
}
dprintk(verbose, MANTIS_ERROR, 1, "Removing -->Mantis irq: %d, latency:
%d\nmemory: 0x%04x, mmio: 0x%p", pdev->irq, mantis->latency,
mantis->mantis_addr, mantis->mantis_mmio);
free_irq(pdev->irq, pdev);
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
pci_disable_device(pdev);
pci_set_drvdata(pdev, NULL);
kfree(mantis);
}
Manu Abraham wrote:
>Jiri Slaby wrote:
>> Manu Abraham napsal(a):
>>> Jiri Slaby wrote:
>>>> Manu Abraham napsal(a):
>>>>> Jiri Slaby wrote:
>>>>>> you do NOT do this at all, because you have pdev already (the param
>>>>>> of the probe function)
>>>>>
>>>>> I rewrote the entire thing like this including the pci_remove
>>>>> function too, but now it so seems that in the remove function,
>>>>> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at
>>>>> module removal.
>>>
>>> I just found that, pci_enable_device() fails. So what's the way to go
>>> ahead ?
>>
>> JESUS.
>
>Hmm.. i finally got it to work. It seems pci_get_device() is necessary,
>i can't seem to enable the device or request for an IRQ the way you
>suggested. It looks some quirks are there though ..
>
>If only i could explain why it works this way and not the other way ..
Because pci_enable_device() works like most other kernel (and also libc)
functions: it returns 0 if everything went fine.
>[ 81.269655] mantis_pci_probe: Got a device
>[ 81.269825] mantis_pci_probe: We got an IRQ
>[ 81.269987] mantis_pci_probe: We finally enabled the device
>[ 81.270191] Mantis Rev 1, irq: 23, latency: 32
>[ 81.270289] memory: 0xefeff000, mmio: f9218000
>[ 81.270519] Trying to free free IRQ23
>[ 90.485885] mantis_pci_remove: Removing -->Mantis irq: 23, latency: 32
>[ 90.485887] memory: 0xefeff000, mmio: 0xf9218000
>[ 90.486293] Trying to free free IRQ23
>[ 90.486429] Trying to free nonexistent resource <efeff000-efefffff>
You should introduce a table of PCI devices here that your driver feels
responsible for. Then put this into a struct pci_driver which you pass to
pci_module_init. Take a look on a random other PCI driver,
drivers/net/8139too.c, drivers/scsi/aic7xxx/aic7xxx_osm_pci.c, whatever.
>static int __devinit mantis_pci_probe(struct pci_dev *pdev, const struct
>pci_device_id *mantis_pci_table)
>{
> u8 revision, latency;
> u8 data[2];
> struct mantis_pci *mantis;
> mantis = (struct mantis_pci *) kmalloc(sizeof (struct mantis_pci),
>GFP_KERNEL);
> if (mantis == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
> return -ENOMEM;
> }
>
> pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11,
>NULL);
This is not needed anymore then. Your probe function will get called with for
any pci dev your driver can handle.
> if (pdev) {
> dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
> mantis->mantis_addr = pci_resource_start(pdev, 0);
> if (!request_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0), DRIVER_NAME)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Request for memory region failed");
Line length is maximum 80 characters. See Documentation/CodingStyle
> goto err0;
> }
> if ((mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
> goto err1;
> }
> if (request_irq(pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
> SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
You don't need to cast a pointer to void* or vice versa.
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
> goto err2;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
> if (pci_enable_device(pdev)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI device enable failed");
> goto err3;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
> pci_set_master(pdev);
> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
> mantis->latency = latency;
> mantis->revision = revision;
> if (!latency) {
> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
> }
The value in mantis->latency and the one in the card's address space now differ.
> pci_set_drvdata(pdev, mantis);
> dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\nmemory:
>0x%04x, mmio: %p\n", pdev->irq, mantis->latency,
> mantis->mantis_addr, mantis->mantis_mmio);
>
> pci_dev_put(pdev);
No, DON'T DO THAT! This will drop the a reference count from the struct
pci_dev, which means it can get freed while your driver still wants to work
with it.
> } else {
> dprintk(verbose, MANTIS_ERROR, 1, "No device found");
> return -ENODEV;
> }
>
>err3:
> free_irq(pdev->irq, pdev);
>err2:
> if (mantis->mantis_mmio)
> iounmap(mantis->mantis_mmio);
>err1:
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
>err0:
> kfree(mantis);
>
> return 0;
>}
>
>
>static void __devexit mantis_pci_remove(struct pci_dev *pdev)
>{
> struct mantis_pci *mantis = pci_get_drvdata(pdev);
> if (mantis == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Aeio, MAntis NULL ptr");
a) this should really never happen. If it happens, it's a kernel bug.
b) if you catch this error while debugging, you should return here so you do
not dereference this NULL pointer.
> }
> dprintk(verbose, MANTIS_ERROR, 1, "Removing -->Mantis irq: %d, latency:
>%d\nmemory: 0x%04x, mmio: 0x%p", pdev->irq, mantis->latency,
> mantis->mantis_addr, mantis->mantis_mmio);
>
> free_irq(pdev->irq, pdev);
>
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
> pci_disable_device(pdev);
> pci_set_drvdata(pdev, NULL);
> kfree(mantis);
>}
Eike
Rolf Eike Beer wrote:
>Manu Abraham wrote:
>
>
>>Jiri Slaby wrote:
>>
>>
>>>Manu Abraham napsal(a):
>>>
>>>
>>>>Jiri Slaby wrote:
>>>>
>>>>
>>>>>Manu Abraham napsal(a):
>>>>>
>>>>>
>>>>>>Jiri Slaby wrote:
>>>>>>
>>>>>>
>>>>>>>you do NOT do this at all, because you have pdev already (the param
>>>>>>>of the probe function)
>>>>>>>
>>>>>>>
>>>>>>I rewrote the entire thing like this including the pci_remove
>>>>>>function too, but now it so seems that in the remove function,
>>>>>>pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at
>>>>>>module removal.
>>>>>>
>>>>>>
>>>>I just found that, pci_enable_device() fails. So what's the way to go
>>>>ahead ?
>>>>
>>>>
>>>JESUS.
>>>
>>>
>>Hmm.. i finally got it to work. It seems pci_get_device() is necessary,
>>i can't seem to enable the device or request for an IRQ the way you
>>suggested. It looks some quirks are there though ..
>>
>>If only i could explain why it works this way and not the other way ..
>>
>>
>
>Because pci_enable_device() works like most other kernel (and also libc)
>functions: it returns 0 if everything went fine.
>
>
>
>
>>[ 81.269655] mantis_pci_probe: Got a device
>>[ 81.269825] mantis_pci_probe: We got an IRQ
>>[ 81.269987] mantis_pci_probe: We finally enabled the device
>>[ 81.270191] Mantis Rev 1, irq: 23, latency: 32
>>[ 81.270289] memory: 0xefeff000, mmio: f9218000
>>[ 81.270519] Trying to free free IRQ23
>>[ 90.485885] mantis_pci_remove: Removing -->Mantis irq: 23, latency: 32
>>[ 90.485887] memory: 0xefeff000, mmio: 0xf9218000
>>[ 90.486293] Trying to free free IRQ23
>>[ 90.486429] Trying to free nonexistent resource <efeff000-efefffff>
>>
>>
>
>You should introduce a table of PCI devices here that your driver feels
>responsible for. Then put this into a struct pci_driver which you pass to
>
>
I am in fact doing that ..
static *struct* pci_device_id mantis_pci_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11) },
{ 0 },
};
MODULE_DEVICE_TABLE(pci, mantis_pci_table);
static *struct* pci_driver mantis_pci_driver = {
.name = "Mantis PCI combo driver",
.id_table = mantis_pci_table,
.probe = mantis_pci_probe,
.remove = mantis_pci_remove,
};
static int __devinit mantis_pci_init(void)
{
*return* pci_register_driver(&mantis_pci_driver);
}
static void __devexit mantis_pci_exit(void)
{
pci_unregister_driver(&mantis_pci_driver);
}
module_init(mantis_pci_init);
module_exit(mantis_pci_exit);
MODULE_DESCRIPTION("Mantis PCI DTV bridge driver");
MODULE_AUTHOR("Manu Abraham");
MODULE_LICENSE("GPL");
>pci_module_init. Take a look on a random other PCI driver,
>drivers/net/8139too.c, drivers/scsi/aic7xxx/aic7xxx_osm_pci.c, whatever.
>
>
>
>>static int __devinit mantis_pci_probe(struct pci_dev *pdev, const struct
>>pci_device_id *mantis_pci_table)
>>{
>> u8 revision, latency;
>> u8 data[2];
>> struct mantis_pci *mantis;
>> mantis = (struct mantis_pci *) kmalloc(sizeof (struct mantis_pci),
>>GFP_KERNEL);
>> if (mantis == NULL) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
>> return -ENOMEM;
>> }
>>
>> pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11,
>>NULL);
>>
>>
>
>This is not needed anymore then. Your probe function will get called with for
>any pci dev your driver can handle.
>
>
>
I will just check it up again to see what went wrong ..
>> if (pdev) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
>> mantis->mantis_addr = pci_resource_start(pdev, 0);
>> if (!request_mem_region(pci_resource_start(pdev, 0),
>> pci_resource_len(pdev, 0), DRIVER_NAME)) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Request for memory region failed");
>>
>>
>
>Line length is maximum 80 characters. See Documentation/CodingStyle
>
>
That was not meant to go into the kernel straight away, as it needs
*lot* of work, the PCI part is only something extremely small.
>
>
>> goto err0;
>> }
>> if ((mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
>> dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
>> goto err1;
>> }
>> if (request_irq(pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
>> SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
>>
>>
>
>You don't need to cast a pointer to void* or vice versa.
>
>
>
Ah, thanks ...
>> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
>> goto err2;
>> }
>> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
>> if (pci_enable_device(pdev)) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI device enable failed");
>> goto err3;
>> }
>> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
>> pci_set_master(pdev);
>> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
>> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
>> mantis->latency = latency;
>> mantis->revision = revision;
>> if (!latency) {
>> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
>> }
>>
>>
>
>The value in mantis->latency and the one in the card's address space now differ.
>
>
Yes, i set it to the default latency as specified by vendor.. But
temporarily to test the card ..
>
>
>> pci_set_drvdata(pdev, mantis);
>> dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
>> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\nmemory:
>>0x%04x, mmio: %p\n", pdev->irq, mantis->latency,
>> mantis->mantis_addr, mantis->mantis_mmio);
>>
>> pci_dev_put(pdev);
>>
>>
>
>No, DON'T DO THAT! This will drop the a reference count from the struct
>pci_dev, which means it can get freed while your driver still wants to work
>with it.
>
>
>
Hmm.. I thought after i make a call to pci_get_device(), i have to do a
pci_dev_put() after the usage ..
I was a bit lost when to use pci_dev_put() in this case.
>> } else {
>> dprintk(verbose, MANTIS_ERROR, 1, "No device found");
>> return -ENODEV;
>> }
>>
>>err3:
>> free_irq(pdev->irq, pdev);
>>err2:
>> if (mantis->mantis_mmio)
>> iounmap(mantis->mantis_mmio);
>>err1:
>> release_mem_region(pci_resource_start(pdev, 0),
>> pci_resource_len(pdev, 0));
>>err0:
>> kfree(mantis);
>>
>> return 0;
>>}
>>
>>
>>static void __devexit mantis_pci_remove(struct pci_dev *pdev)
>>{
>> struct mantis_pci *mantis = pci_get_drvdata(pdev);
>> if (mantis == NULL) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Aeio, MAntis NULL ptr");
>>
>>
>
>a) this should really never happen. If it happens, it's a kernel bug.
>b) if you catch this error while debugging, you should return here so you do
> not dereference this NULL pointer.
>
>
>
Ack.
>> }
>> dprintk(verbose, MANTIS_ERROR, 1, "Removing -->Mantis irq: %d, latency:
>>%d\nmemory: 0x%04x, mmio: 0x%p", pdev->irq, mantis->latency,
>> mantis->mantis_addr, mantis->mantis_mmio);
>>
>> free_irq(pdev->irq, pdev);
>>
>> release_mem_region(pci_resource_start(pdev, 0),
>> pci_resource_len(pdev, 0));
>> pci_disable_device(pdev);
>> pci_set_drvdata(pdev, NULL);
>> kfree(mantis);
>>}
>>
>>
Thanks ..
Regards,
Manu
Manu Abraham wrote:
>Rolf Eike Beer wrote:
>>Manu Abraham wrote:
>>>static int __devinit mantis_pci_probe(struct pci_dev *pdev, const struct
>>>pci_device_id *mantis_pci_table)
>>>{
>>> u8 revision, latency;
>>> u8 data[2];
>>> struct mantis_pci *mantis;
>>> mantis = (struct mantis_pci *) kmalloc(sizeof (struct mantis_pci),
>>>GFP_KERNEL);
>>> if (mantis == NULL) {
>>> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
>>> return -ENOMEM;
>>> }
>>>
>>> pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11,
>>>NULL);
>>
>>This is not needed anymore then. Your probe function will get called with
>> for any pci dev your driver can handle.
>
>I will just check it up again to see what went wrong ..
>
>>> if (pdev) {
>>> dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
>>> mantis->mantis_addr = pci_resource_start(pdev, 0);
>>> if (!request_mem_region(pci_resource_start(pdev, 0),
>>> pci_resource_len(pdev, 0), DRIVER_NAME)) {
>>> dprintk(verbose, MANTIS_ERROR, 1, "Request for memory region failed");
[...]
>>> pci_set_drvdata(pdev, mantis);
>>> dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
>>> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\nmemory:
>>>0x%04x, mmio: %p\n", pdev->irq, mantis->latency,
>>> mantis->mantis_addr, mantis->mantis_mmio);
>>>
>>> pci_dev_put(pdev);
>>
>>No, DON'T DO THAT! This will drop the a reference count from the struct
>>pci_dev, which means it can get freed while your driver still wants to work
>>with it.
>
>Hmm.. I thought after i make a call to pci_get_device(), i have to do a
>pci_dev_put() after the usage ..
>I was a bit lost when to use pci_dev_put() in this case.
That is true, but you should not call pci_get_device() in this function at
all.
Eike
Rolf Eike Beer wrote:
>That is true, but you should not call pci_get_device() in this function at
>all.
>
>
>
I reworked the whole thing over .. but i am feeling that something's a
bit wrong somewhere ..
The log i get on a load - unload ..
[ 102.261264] mantis_pci_probe: Got a device
[ 102.262852] mantis_pci_probe: We got an IRQ
[ 102.264392] mantis_pci_probe: We finally enabled the device
[ 102.266020] Mantis Rev 1, irq: 23, latency: 32
[ 102.266118] memory: 0xefeff000, mmio: f9218000
[ 102.269162] Trying to free free IRQ23
[ 110.297341] mantis_pci_remove: Removing -->Mantis irq: 23,
latency: 32
[ 110.297344] memory: 0xefeff000, mmio: 0xf9218000
[ 110.301326] Trying to free free IRQ23
[ 110.303445] Trying to free nonexistent resource <efeff000-efefffff>
#include <asm/io.h>
#include <asm/pgtable.h>
#include <asm/page.h>
#include <linux/interrupt.h>
#include <linux/kmod.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/device.h>
#include "mantis_common.h"
#include "mantis_dma.h"
#include "mantis_i2c.h"
#include "mantis_eeprom.h"
#define DRIVER_NAME "Mantis"
static struct pci_device_id mantis_pci_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11) },
{ 0 },
};
MODULE_DEVICE_TABLE(pci, mantis_pci_table);
static irqreturn_t mantis_pci_irq(int irq, void *dev_id, struct pt_regs
*regs)
{
struct mantis_pci *mantis;
dprintk(verbose, MANTIS_DEBUG, 1, "Mantis PCI IRQ");
mantis = (struct mantis_pci *) dev_id;
if (mantis == NULL)
dprintk(verbose, MANTIS_DEBUG, 1, "Aeio, mantis ISR");
/* Events
* (1) PCMCIA insert
* (2) PCMCIA extract
* (3) I2C complete
*/
return IRQ_HANDLED;
}
static int mantis_i2c_setup(struct mantis_pci *mantis)
{
u32 config = 0;
// mmwrite(0x80, MANTIS_DMA_CTL); // MCU i2c read
config = mmread(MANTIS_DMA_CTL);
dprintk(verbose, MANTIS_DEBUG, 1, "Mantis Ctl reg=0x%04x", config);
return 0;
}
static int mantis_reg_dump(struct mantis_pci *mantis)
{
u32 ctlreg, intstat, intmask, i2cdata;
ctlreg = mmread(MANTIS_DMA_CTL);
intstat = mmread(MANTIS_INT_STAT);
intmask = mmread(MANTIS_INT_MASK);
i2cdata = mmread(MANTIS_I2C_DATA);
dprintk(verbose, MANTIS_DEBUG, 1, "CTL_REG=0x%04x, INT_STAT=0x%04x, \
INT_MASK=0x%04x, I2C_DATA=0x%04x", ctlreg, intstat, \
intmask, i2cdata);
return 0;
}
static int __devinit mantis_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *mantis_pci_table)
{
u8 revision, latency;
// u8 data[2];
struct mantis_pci *mantis;
mantis = (struct mantis_pci *)
kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
return -ENOMEM;
}
dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
mantis->mantis_addr = pci_resource_start(pdev, 0);
if (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
dprintk(verbose, MANTIS_ERROR, 1, "Request mem region failed");
goto err0;
}
if ((mantis->mantis_mmio =
ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
goto err1;
}
if (request_irq(pdev->irq, mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
goto err2;
}
dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
if (pci_enable_device(pdev)) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
goto err3;
}
dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
pci_set_master(pdev);
pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
mantis->latency = latency;
mantis->revision = revision;
if (!latency) {
pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
}
pci_set_drvdata(pdev, mantis);
dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
mantis->mantis_addr, mantis->mantis_mmio);
err3:
free_irq(pdev->irq, pdev);
err2:
if (mantis->mantis_mmio)
iounmap(mantis->mantis_mmio);
err1:
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
err0:
kfree(mantis);
return 0;
}
static void __devexit mantis_pci_remove(struct pci_dev *pdev)
{
struct mantis_pci *mantis = pci_get_drvdata(pdev);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Aeio, MAntis NULL ptr");
return;
}
dprintk(verbose, MANTIS_ERROR, 1, "Removing -->Mantis irq: %d, \
latency: %d\n memory: 0x%04x, mmio: 0x%p",
pdev->irq, mantis->latency, mantis->mantis_addr,
mantis->mantis_mmio);
free_irq(pdev->irq, pdev);
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
pci_set_drvdata(pdev, NULL);
pci_disable_device(pdev);
kfree(mantis);
}
static struct pci_driver mantis_pci_driver = {
.name = "Mantis PCI combo driver",
.id_table = mantis_pci_table,
.probe = mantis_pci_probe,
.remove = mantis_pci_remove,
};
static int __devinit mantis_pci_init(void)
{
return pci_register_driver(&mantis_pci_driver);
}
static void __devexit mantis_pci_exit(void)
{
pci_unregister_driver(&mantis_pci_driver);
}
module_init(mantis_pci_init);
module_exit(mantis_pci_exit);
MODULE_DESCRIPTION("Mantis PCI DTV bridge driver");
MODULE_AUTHOR("Manu Abraham");
MODULE_LICENSE("GPL");
Manu Abraham wrote:
>Rolf Eike Beer wrote:
>>That is true, but you should not call pci_get_device() in this function at
>>all.
>
>I reworked the whole thing over .. but i am feeling that something's a
>bit wrong somewhere ..
>
>The log i get on a load - unload ..
>
>[ 102.261264] mantis_pci_probe: Got a device
>[ 102.262852] mantis_pci_probe: We got an IRQ
>[ 102.264392] mantis_pci_probe: We finally enabled the device
>[ 102.266020] Mantis Rev 1, irq: 23, latency: 32
>[ 102.266118] memory: 0xefeff000, mmio: f9218000
>[ 102.269162] Trying to free free IRQ23
>[ 110.297341] mantis_pci_remove: Removing -->Mantis irq: 23,
>latency: 32
>[ 110.297344] memory: 0xefeff000, mmio: 0xf9218000
>[ 110.301326] Trying to free free IRQ23
>[ 110.303445] Trying to free nonexistent resource <efeff000-efefffff>
>
>
>#include <asm/io.h>
>#include <asm/pgtable.h>
>#include <asm/page.h>
>#include <linux/interrupt.h>
>#include <linux/kmod.h>
>#include <linux/vmalloc.h>
>#include <linux/init.h>
>#include <linux/sched.h>
>#include <linux/device.h>
>#include "mantis_common.h"
>#include "mantis_dma.h"
>#include "mantis_i2c.h"
>#include "mantis_eeprom.h"
>
>#define DRIVER_NAME "Mantis"
>
>static struct pci_device_id mantis_pci_table[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11) },
> { 0 },
>};
>
>MODULE_DEVICE_TABLE(pci, mantis_pci_table);
>
>static irqreturn_t mantis_pci_irq(int irq, void *dev_id, struct pt_regs
>*regs)
>{
> struct mantis_pci *mantis;
>
> dprintk(verbose, MANTIS_DEBUG, 1, "Mantis PCI IRQ");
> mantis = (struct mantis_pci *) dev_id;
> if (mantis == NULL)
> dprintk(verbose, MANTIS_DEBUG, 1, "Aeio, mantis ISR");
>
> /* Events
> * (1) PCMCIA insert
> * (2) PCMCIA extract
> * (3) I2C complete
> */
>
> return IRQ_HANDLED;
>}
You must check here if this interrupt was really from your device. If not, you
must return IRQ_NONE. You have requested your interrupt as a shared one so
this may happen at any time.
>static int mantis_i2c_setup(struct mantis_pci *mantis)
>{
> u32 config = 0;
You don't need to set this here, you will overwrite it anyway before looking
at it.
>// mmwrite(0x80, MANTIS_DMA_CTL); // MCU i2c read
> config = mmread(MANTIS_DMA_CTL);
> dprintk(verbose, MANTIS_DEBUG, 1, "Mantis Ctl reg=0x%04x", config);
>
> return 0;
>}
>static int __devinit mantis_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *mantis_pci_table)
>{
> u8 revision, latency;
>// u8 data[2];
> struct mantis_pci *mantis;
Please insert a blank line here, this improves readability.
> mantis = (struct mantis_pci *)
> kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
> if (mantis == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
> return -ENOMEM;
> }
> dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
> mantis->mantis_addr = pci_resource_start(pdev, 0);
> if (!request_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0), DRIVER_NAME)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Request mem region failed");
> goto err0;
I would prefer to use more descriptive names for the labels, like err_memreg,
err_iomem, err_irq or something like that.
Also you will get wrong assignements here. You must call pci_enable_device()
_first_, it will set up the BARs of that device. Also I think
pci_request_regions() might be a better way to get this assignements.
> }
> if ((mantis->mantis_mmio =
> ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
> goto err1;
> }
> if (request_irq(pdev->irq, mantis_pci_irq, SA_SHIRQ |
> SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
> goto err2;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
> if (pci_enable_device(pdev)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
> goto err3;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
> pci_set_master(pdev);
> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
> mantis->latency = latency;
> mantis->revision = revision;
> if (!latency) {
> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
> }
> pci_set_drvdata(pdev, mantis);
> dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
> memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
> mantis->mantis_addr, mantis->mantis_mmio);
>err3:
> free_irq(pdev->irq, pdev);
>err2:
> if (mantis->mantis_mmio)
> iounmap(mantis->mantis_mmio);
>err1:
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
>err0:
> kfree(mantis);
>
> return 0;
>}
>
>static void __devexit mantis_pci_remove(struct pci_dev *pdev)
>{
> struct mantis_pci *mantis = pci_get_drvdata(pdev);
> if (mantis == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Aeio, MAntis NULL ptr");
> return;
> }
> dprintk(verbose, MANTIS_ERROR, 1, "Removing -->Mantis irq: %d, \
> latency: %d\n memory: 0x%04x, mmio: 0x%p",
> pdev->irq, mantis->latency, mantis->mantis_addr,
> mantis->mantis_mmio);
>
> free_irq(pdev->irq, pdev);
>
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
pci_release_regions(pdev);
> pci_set_drvdata(pdev, NULL);
> pci_disable_device(pdev);
> kfree(mantis);
>}
HTH
Eike
Hi Manu,
Manu Abraham writes:
> [ 102.261264] mantis_pci_probe: Got a device
> [ 102.262852] mantis_pci_probe: We got an IRQ
> [ 102.264392] mantis_pci_probe: We finally enabled the device
> [ 102.266020] Mantis Rev 1, irq: 23, latency: 32
> [ 102.266118] memory: 0xefeff000, mmio: f9218000
> [ 102.269162] Trying to free free IRQ23
> [ 110.297341] mantis_pci_remove: Removing -->Mantis irq: 23,
> latency: 32
> [ 110.297344] memory: 0xefeff000, mmio: 0xf9218000
> [ 110.301326] Trying to free free IRQ23
> [ 110.303445] Trying to free nonexistent resource <efeff000-efefffff>
I think you should call pci_enable_device() before request_irq, etc.
AFAIK, the pci_enable_device() can change resources like IRQ.
That's probably what causes these errors. Just print out the irq
number before and after pci_enable_device() to check if that's the
problem.
Ralph
Ralph Metzler wrote:
>Hi Manu,
>
>
>
Hello Ralph,
It's been a long time since heard your voice. I thought you had been
damn busy.
Nice to hear from you.
>Manu Abraham writes:
> > [ 102.261264] mantis_pci_probe: Got a device
> > [ 102.262852] mantis_pci_probe: We got an IRQ
> > [ 102.264392] mantis_pci_probe: We finally enabled the device
> > [ 102.266020] Mantis Rev 1, irq: 23, latency: 32
> > [ 102.266118] memory: 0xefeff000, mmio: f9218000
> > [ 102.269162] Trying to free free IRQ23
> > [ 110.297341] mantis_pci_remove: Removing -->Mantis irq: 23,
> > latency: 32
> > [ 110.297344] memory: 0xefeff000, mmio: 0xf9218000
> > [ 110.301326] Trying to free free IRQ23
> > [ 110.303445] Trying to free nonexistent resource <efeff000-efefffff>
>
>
>I think you should call pci_enable_device() before request_irq, etc.
>
>
Sure i will try that out..
>AFAIK, the pci_enable_device() can change resources like IRQ.
>That's probably what causes these errors. Just print out the irq
>number before and after pci_enable_device() to check if that's the
>problem.
>
>
>
I will check this out. I will come back on this soon.
Regards,
Manu
Ralph Metzler wrote:
>Hi Manu,
>
>Manu Abraham writes:
> > [ 102.261264] mantis_pci_probe: Got a device
> > [ 102.262852] mantis_pci_probe: We got an IRQ
> > [ 102.264392] mantis_pci_probe: We finally enabled the device
> > [ 102.266020] Mantis Rev 1, irq: 23, latency: 32
> > [ 102.266118] memory: 0xefeff000, mmio: f9218000
> > [ 102.269162] Trying to free free IRQ23
> > [ 110.297341] mantis_pci_remove: Removing -->Mantis irq: 23,
> > latency: 32
> > [ 110.297344] memory: 0xefeff000, mmio: 0xf9218000
> > [ 110.301326] Trying to free free IRQ23
> > [ 110.303445] Trying to free nonexistent resource <efeff000-efefffff>
>
>
>I think you should call pci_enable_device() before request_irq, etc.
>AFAIK, the pci_enable_device() can change resources like IRQ.
>
>
>That's probably what causes these errors. Just print out the irq
>number before and after pci_enable_device() to check if that's the
>problem.
>
>
>
Hmm.. not much of a change i can say ..
Manu
[ 631.211320] mantis_pci_probe: <1:>IRQ=23
[ 631.211495] mantis_pci_probe: <2:>IRQ=23
[ 631.211664] mantis_pci_probe: Got a device
[ 631.211850] mantis_pci_probe: We got an IRQ
[ 631.212013] mantis_pci_probe: We finally enabled the device
[ 631.212236] Mantis Rev 1, irq: 23, latency: 32
[ 631.212322] memory: 0xefeff000, mmio: f9218000
[ 639.259136] mantis_pci_remove: Removing -->Mantis irq: 23,
latency: 32
[ 639.259138] memory: 0xefeff000, mmio: 0xf9218000
[ 639.259504] Trying to free free IRQ23
[ 639.259673] Trying to free nonexistent resource <efeff000-efefffff>
static int __devinit mantis_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *mantis_pci_table)
{
u8 revision, latency;
// u8 data[2];
struct mantis_pci *mantis;
dprintk(verbose, MANTIS_ERROR, 1, "<1:>IRQ=%d", pdev->irq);
if (pci_enable_device(pdev)) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
goto err;
}
dprintk(verbose, MANTIS_ERROR, 1, "<2:>IRQ=%d", pdev->irq);
mantis = (struct mantis_pci *)
kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
return -ENOMEM;
}
dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
mantis->mantis_addr = pci_resource_start(pdev, 0);
if (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
dprintk(verbose, MANTIS_ERROR, 1, "Request mem region failed");
goto err0;
}
if ((mantis->mantis_mmio =
ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
goto err1;
}
if (request_irq(pdev->irq, mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
goto err2;
}
dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
pci_set_master(pdev);
pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
mantis->latency = latency;
mantis->revision = revision;
if (!latency) {
pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
}
pci_set_drvdata(pdev, mantis);
dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
mantis->mantis_addr, mantis->mantis_mmio);
err2:
if (mantis->mantis_mmio)
iounmap(mantis->mantis_mmio);
err1:
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
err0:
kfree(mantis);
err:
return 0;
}
Am Donnerstag, 15. September 2005 13:42 schrieb Manu Abraham:
>Ralph Metzler wrote:
>>Hi Manu,
>>
>>Manu Abraham writes:
>> > [ 102.261264] mantis_pci_probe: Got a device
>> > [ 102.262852] mantis_pci_probe: We got an IRQ
>> > [ 102.264392] mantis_pci_probe: We finally enabled the device
>> > [ 102.266020] Mantis Rev 1, irq: 23, latency: 32
>> > [ 102.266118] memory: 0xefeff000, mmio: f9218000
>> > [ 102.269162] Trying to free free IRQ23
>> > [ 110.297341] mantis_pci_remove: Removing -->Mantis irq: 23,
>> > latency: 32
>> > [ 110.297344] memory: 0xefeff000, mmio: 0xf9218000
>> > [ 110.301326] Trying to free free IRQ23
>> > [ 110.303445] Trying to free nonexistent resource <efeff000-efefffff>
>>
>>I think you should call pci_enable_device() before request_irq, etc.
>>AFAIK, the pci_enable_device() can change resources like IRQ.
>>
>>
>>That's probably what causes these errors. Just print out the irq
>>number before and after pci_enable_device() to check if that's the
>>problem.
>
>Hmm.. not much of a change i can say ..
>[ 631.211320] mantis_pci_probe: <1:>IRQ=23
>[ 631.211495] mantis_pci_probe: <2:>IRQ=23
>[ 631.211664] mantis_pci_probe: Got a device
>[ 631.211850] mantis_pci_probe: We got an IRQ
>[ 631.212013] mantis_pci_probe: We finally enabled the device
>[ 631.212236] Mantis Rev 1, irq: 23, latency: 32
>[ 631.212322] memory: 0xefeff000, mmio: f9218000
>[ 639.259136] mantis_pci_remove: Removing -->Mantis irq: 23,
>latency: 32
>[ 639.259138] memory: 0xefeff000, mmio: 0xf9218000
>[ 639.259504] Trying to free free IRQ23
>[ 639.259673] Trying to free nonexistent resource <efeff000-efefffff>
Oh yes, of course. Now I see what's wrong. See changes below.
>static int __devinit mantis_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *mantis_pci_table)
>{
> u8 revision, latency;
>// u8 data[2];
> struct mantis_pci *mantis;
>
> dprintk(verbose, MANTIS_ERROR, 1, "<1:>IRQ=%d", pdev->irq);
> if (pci_enable_device(pdev)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
> goto err;
> }
> dprintk(verbose, MANTIS_ERROR, 1, "<2:>IRQ=%d", pdev->irq);
>
> mantis = (struct mantis_pci *)
> kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
> if (mantis == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
> return -ENOMEM;
> }
> dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
> mantis->mantis_addr = pci_resource_start(pdev, 0);
> if (!request_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0), DRIVER_NAME)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Request mem region failed");
> goto err0;
> }
> if ((mantis->mantis_mmio =
> ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
> goto err1;
> }
> if (request_irq(pdev->irq, mantis_pci_irq, SA_SHIRQ |
> SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
> goto err2;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
> pci_set_master(pdev);
> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
> mantis->latency = latency;
> mantis->revision = revision;
> if (!latency) {
> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
> }
> pci_set_drvdata(pdev, mantis);
> dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
> memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
> mantis->mantis_addr, mantis->mantis_mmio);
return 0;
>err2:
> if (mantis->mantis_mmio)
> iounmap(mantis->mantis_mmio);
>err1:
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
>err0:
> kfree(mantis);
>err:
return -ENODEV.
> return 0;
>}
You release the regions directly after you've claimed them. Freeing them again
in release function is impossible because you don't own them any longer.
Eike
Manu Abraham wrote:
> Ralph Metzler wrote:
>
> SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
> goto err2;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
> pci_set_master(pdev);
> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
> mantis->latency = latency;
> mantis->revision = revision;
> if (!latency) {
> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
> }
> pci_set_drvdata(pdev, mantis); dprintk(verbose, MANTIS_ERROR, 0,
> "Mantis Rev %d, ", mantis->revision);
> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
> memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
> mantis->mantis_addr, mantis->mantis_mmio);
Success! So don't enter the failure path, return 0 here.
> err2:
> if (mantis->mantis_mmio)
> iounmap(mantis->mantis_mmio);
> err1:
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
> err0:
> kfree(mantis);
> err:
> return 0;
This is your failure path, return nonzero here, preferable describing the
error condition.
Tony
Antonino A. Daplas wrote:
>Manu Abraham wrote:
>> Ralph Metzler wrote:
>>
>>
>> SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
>> goto err2;
>> }
>> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
>> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
>> pci_set_master(pdev);
>> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
>> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
>> mantis->latency = latency;
>> mantis->revision = revision;
>> if (!latency) {
>> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
>> }
>> pci_set_drvdata(pdev, mantis); dprintk(verbose, MANTIS_ERROR, 0,
>> "Mantis Rev %d, ", mantis->revision);
>> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
>> memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
>> mantis->mantis_addr, mantis->mantis_mmio);
>
>Success! So don't enter the failure path, return 0 here.
>
>> err2:
>> if (mantis->mantis_mmio)
>> iounmap(mantis->mantis_mmio);
>> err1:
>> release_mem_region(pci_resource_start(pdev, 0),
>> pci_resource_len(pdev, 0));
>> err0:
>> kfree(mantis);
>> err:
>> return 0;
>
>This is your failure path, return nonzero here, preferable describing the
>error condition.
Hey Tony, you get a penalty for not using PGP. If you had used it my mail
would not have been sent 7 seconds after yours :))
Eike
Antonino A. Daplas wrote:
>This is your failure path, return nonzero here, preferable describing the
>error condition.
>
>
Thanks a lot guys, I mean all of you. Otherwise i would have pulled out
my hair for some more time !
The IRQ issue still i don't feel a too comfortable though.
Thanks,
Manu
[ 3689.479339] mantis_pci_probe: <1:>IRQ=23
[ 3689.479518] mantis_pci_probe: <2:>IRQ=23
[ 3689.479685] mantis_pci_probe: Got a device
[ 3689.479869] mantis_pci_probe: We got an IRQ
[ 3689.480036] mantis_pci_probe: We finally enabled the device
[ 3689.480290] Mantis Rev 1, irq: 23, latency: 32
[ 3689.480403] memory: 0xefeff000, mmio: f92a4000
[ 3695.984470] mantis_pci_remove: Removing -->Mantis irq: 23,
latency: 32
[ 3695.984473] memory: 0xefeff000, mmio: 0xf92a4000
[ 3695.984934] Trying to free free IRQ23
Rolf Eike Beer wrote:
>
>You must check here if this interrupt was really from your device. If not, you
>must return IRQ_NONE. You have requested your interrupt as a shared one so
>this may happen at any time.
>
>
>
Fixed this one, temporarily ..
>>static int mantis_i2c_setup(struct mantis_pci *mantis)
>>{
>> u32 config = 0;
>>
>>
>
>You don't need to set this here, you will overwrite it anyway before looking
>at it.
>
>
>
Oh, that was the remnants of some i2c tests that i was trying to do ..
The other i2c parts i removed for testing ..
>
>Also you will get wrong assignements here. You must call pci_enable_device()
>_first_, it will set up the BARs of that device. Also I think
>pci_request_regions() might be a better way to get this assignements.
>
>
>
Fixed that one too ..
>>
>>
>
>pci_release_regions(pdev);
>
>
>
Fixed ..
So it now looks like this but i have another problem now, after
consecutive, load/unload, i get an oops ..
-----------------------------------------------------------------
[ 330.625715] mantis_pci_probe: <1:>IRQ=23
[ 330.631130] mantis_pci_probe: <2:>IRQ=23
[ 330.636138] mantis_pci_probe: Got a device
[ 330.641635] mantis_pci_probe: We got an IRQ
[ 330.647189] mantis_pci_probe: We finally enabled the device
[ 330.652468] Mantis Rev 1, irq: 23, latency: 32
[ 330.652577] memory: 0xefeff000, mmio: f9218000
[ 332.615009] mantis_pci_remove: Removing -->Mantis irq: 23, latency: 32
[ 332.615011] memory: 0xefeff000, mmio: 0xf9218000
[ 332.627714] Trying to free free IRQ23
[ 334.131573] mantis_pci_probe: <1:>IRQ=23
[ 334.138602] mantis_pci_probe: <2:>IRQ=23
[ 334.145178] mantis_pci_probe: Got a device
[ 334.152287] mantis_pci_probe: We got an IRQ
[ 334.159038] mantis_pci_probe: We finally enabled the device
[ 334.166326] Mantis Rev 1, irq: 23, latency: 32
[ 334.166437] memory: 0xefeff000, mmio: f92a4000
[ 335.716624] mantis_pci_remove: Removing -->Mantis irq: 23, latency: 32
[ 335.716626] memory: 0xefeff000, mmio: 0xf92a4000
[ 335.732424] Trying to free free IRQ23
[ 337.110714] mantis_pci_probe: <1:>IRQ=23
[ 337.118943] mantis_pci_probe: <2:>IRQ=23
[ 337.127573] mantis_pci_probe: Got a device
[ 337.135847] Unable to handle kernel paging request at virtual address f92bd772
[ 337.144907] printing eip:
[ 337.153542] c013e87f
[ 337.162663] *pde = 37ecf067
[ 337.171417] *pte = 00000000
[ 337.180682] Oops: 0000 [#1]
[ 337.189583] SMP
[ 337.198762] Modules linked in: mantis i2c_core mb86a15 dvb_core 3c59x piix sd_mod
[ 337.208187] CPU: 0
[ 337.208188] EIP: 0060:[<c013e87f>] Not tainted VLI
[ 337.208189] EFLAGS: 00010286 (2.6.13)
[ 337.235401] EIP is at name_unique+0x2f/0x60
[ 337.244234] eax: 00000b4d ebx: f6594ee0 ecx: 00000001 edx: f678e520
[ 337.252586] esi: f92a7773 edi: f92bd772 ebp: f6594ee0 esp: f4a3fdb0
[ 337.261383] ds: 007b es: 007b ss: 0068
[ 337.269415] Process modprobe (pid: 2554, threadinfo=f4a3f000 task=f63e7a20)
[ 337.269680] Stack: 00000000 00000017 00000017 c013e92f 00000017 f6594ee0 00000046 000030ec
[ 337.278569] 000030bb c0103a04 000030ec 00000000 000030ec 000030bb 00000296 00000008
[ 337.287187] 00000001 f92bd000 00000000 c0110cca c0429250 00000008 c011cbfd 000030bb
[ 337.296352] Call Trace:
[ 337.314072] [<c013e92f>] register_handler_proc+0x7f/0xe0
[ 337.323270] [<c0103a04>] apic_timer_interrupt+0x1c/0x24
[ 337.332356] [<c0110cca>] smp_call_function+0x11a/0x170
[ 337.341707] [<c011cbfd>] release_console_sem+0x7d/0xc0
[ 337.350561] [<c011ca54>] vprintk+0x194/0x240
[ 337.359944] [<c0110b53>] flush_tlb_all+0x33/0x40
[ 337.368978] [<c013dfc9>] setup_irq+0xd9/0x120
[ 337.378465] [<f92a6000>] mantis_pci_irq+0x0/0x60 [mantis]
[ 337.387679] [<c013e1a5>] request_irq+0x85/0xa0
[ 337.397350] [<f92a6238>] mantis_pci_probe+0x138/0x3e0 [mantis]
[ 337.406839] [<f92a6000>] mantis_pci_irq+0x0/0x60 [mantis]
[ 337.416374] [<c020f38f>] __pci_device_probe+0x5f/0x70
[ 337.426156] [<c020f3cf>] pci_device_probe+0x2f/0x50
[ 337.435457] [<c0247c38>] driver_probe_device+0x38/0xb0
[ 337.445227] [<c0247d30>] __driver_attach+0x0/0x60
[ 337.454521] [<c0247d80>] __driver_attach+0x50/0x60
[ 337.464090] [<c0247219>] bus_for_each_dev+0x69/0x80
[ 337.473215] [<c0247db5>] driver_attach+0x25/0x30
[ 337.482410] [<c0247d30>] __driver_attach+0x0/0x60
[ 337.492096] [<c024776d>] bus_add_driver+0x8d/0xe0
[ 337.501394] [<c020f69b>] pci_register_driver+0x7b/0xa0
[ 337.511118] [<f92a65cf>] mantis_pci_init+0xf/0x20 [mantis]
[ 337.520399] [<c0139032>] sys_init_module+0x162/0x200
[ 337.530161] [<c0102f5f>] sysenter_past_esp+0x54/0x75
[ 337.539433] Code: 44 24 10 8b 5c 24 14 c1 e0 07 8b 90 88 18 42 c0 85 d2 74 33 90 8d b4 26 00 00 00 00 39 da 74 20 8b 7a 0c 85 ff 74 19 8b 73 0c ac <ae> 75 08 84 c0 75 f8 31 c0 eb 04 19 c0 0c 01 31 c9 85 c0 74 0c
[ 337.560185]
-----------------------------------------------------------------
#include <asm/io.h>
#include <asm/pgtable.h>
#include <asm/page.h>
#include <linux/interrupt.h>
#include <linux/kmod.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/device.h>
#include "mantis_common.h"
#include "mantis_dma.h"
#include "mantis_i2c.h"
#include "mantis_eeprom.h"
unsigned int verbose = 1;
module_param(verbose, int, 0644);
MODULE_PARM_DESC(verbose, "verbose startup messages, default is 1 (yes)");
#define PCI_VENDOR_ID_MANTIS 0x1822
#define PCI_DEVICE_ID_MANTIS_R11 0x4e35
#define DRIVER_NAME "Mantis"
static struct pci_device_id mantis_pci_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11) },
{ 0 },
};
MODULE_DEVICE_TABLE(pci, mantis_pci_table);
static irqreturn_t mantis_pci_irq(int irq, void *dev_id, struct pt_regs
*regs)
{
struct mantis_pci *mantis;
dprintk(verbose, MANTIS_DEBUG, 1, "Mantis PCI IRQ");
mantis = (struct mantis_pci *) dev_id;
if (mantis == NULL)
dprintk(verbose, MANTIS_DEBUG, 1, "Aeio, mantis ISR");
/* Events
* (1) PCMCIA insert
* (2) PCMCIA extract
* (3) I2C complete
*/
/*
return IRQ_HANDLED;
*/
return IRQ_NONE; // temporary, for now
}
static int mantis_i2c_setup(struct mantis_pci *mantis)
{
u32 config = 0;
// mmwrite(0x80, MANTIS_DMA_CTL); // MCU i2c read
config = mmread(MANTIS_DMA_CTL);
dprintk(verbose, MANTIS_DEBUG, 1, "Mantis Ctl reg=0x%04x", config);
return 0;
}
static int mantis_reg_dump(struct mantis_pci *mantis)
{
u32 ctlreg, intstat, intmask, i2cdata;
ctlreg = mmread(MANTIS_DMA_CTL);
intstat = mmread(MANTIS_INT_STAT);
intmask = mmread(MANTIS_INT_MASK);
i2cdata = mmread(MANTIS_I2C_DATA);
dprintk(verbose, MANTIS_DEBUG, 1, "CTL_REG=0x%04x, INT_STAT=0x%04x, \
INT_MASK=0x%04x, I2C_DATA=0x%04x", ctlreg, intstat, \
intmask, i2cdata);
return 0;
}
static int __devinit mantis_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *mantis_pci_table)
{
u8 revision, latency;
// u8 data[2];
struct mantis_pci *mantis;
dprintk(verbose, MANTIS_ERROR, 1, "<1:>IRQ=%d", pdev->irq);
if (pci_enable_device(pdev)) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
goto err;
}
dprintk(verbose, MANTIS_ERROR, 1, "<2:>IRQ=%d", pdev->irq);
mantis = (struct mantis_pci *)
kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
return -ENOMEM;
}
dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
mantis->mantis_addr = pci_resource_start(pdev, 0);
if (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
dprintk(verbose, MANTIS_ERROR, 1, "Request mem region failed");
goto err0;
}
if ((mantis->mantis_mmio =
ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
goto err1;
}
mmwrite(0, MANTIS_INT_STAT);
if (request_irq(pdev->irq, mantis_pci_irq, SA_SHIRQ |
SA_INTERRUPT, DRIVER_NAME, mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
goto err2;
}
dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
pci_set_master(pdev);
pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
mantis->latency = latency;
mantis->revision = revision;
if (!latency) {
pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
}
pci_set_drvdata(pdev, mantis);
dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\n \
memory: 0x%04x, mmio: %p\n", pdev->irq, mantis->latency, \
mantis->mantis_addr, mantis->mantis_mmio);
return 0;
err2:
if (mantis->mantis_mmio)
iounmap(mantis->mantis_mmio);
err1:
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
err0:
kfree(mantis);
err:
return -ENODEV;
}
static void __devexit mantis_pci_remove(struct pci_dev *pdev)
{
struct mantis_pci *mantis = pci_get_drvdata(pdev);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Aeio, MAntis NULL ptr");
return;
}
dprintk(verbose, MANTIS_ERROR, 1, "Removing -->Mantis irq: %d, \
latency: %d\n memory: 0x%04x, mmio: 0x%p",
pdev->irq, mantis->latency, mantis->mantis_addr,
mantis->mantis_mmio);
free_irq(pdev->irq, pdev);
// release_mem_region(pci_resource_start(pdev, 0),
// pci_resource_len(pdev, 0));
pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
pci_disable_device(pdev);
kfree(mantis);
}
static struct pci_driver mantis_pci_driver = {
.name = "Mantis PCI combo driver",
.id_table = mantis_pci_table,
.probe = mantis_pci_probe,
.remove = mantis_pci_remove,
};
static int __devinit mantis_pci_init(void)
{
return pci_register_driver(&mantis_pci_driver);
}
static void __devexit mantis_pci_exit(void)
{
pci_unregister_driver(&mantis_pci_driver);
}
module_init(mantis_pci_init);
module_exit(mantis_pci_exit);
MODULE_DESCRIPTION("Mantis PCI DTV bridge driver");
MODULE_AUTHOR("Manu Abraham");
MODULE_LICENSE("GPL");
Manu
Manu Abraham wrote:
>So it now looks like this but i have another problem now, after
>consecutive, load/unload, i get an oops ..
No idea, sorry.
>static int __devinit mantis_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *mantis_pci_table)
>{
> u8 revision, latency;
>// u8 data[2];
> struct mantis_pci *mantis;
>
> dprintk(verbose, MANTIS_ERROR, 1, "<1:>IRQ=%d", pdev->irq);
> if (pci_enable_device(pdev)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
> goto err;
> }
> dprintk(verbose, MANTIS_ERROR, 1, "<2:>IRQ=%d", pdev->irq);
>
> mantis = (struct mantis_pci *)
> kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
mantis = kmalloc(sizeof(*mantis), GFP_KERNEL);
You don't have to cast a void* to any other pointer and this way you will
always get the correct size of memory allocated, even if mantis will become
another pointer type.
Eike
Rolf Eike Beer wrote:
>Manu Abraham wrote:
>
>
>
>>So it now looks like this but i have another problem now, after
>>consecutive, load/unload, i get an oops ..
>>
>>
>
>No idea, sorry.
>
>
>
>>static int __devinit mantis_pci_probe(struct pci_dev *pdev,
>> const struct pci_device_id *mantis_pci_table)
>>{
>> u8 revision, latency;
>>// u8 data[2];
>> struct mantis_pci *mantis;
>>
>> dprintk(verbose, MANTIS_ERROR, 1, "<1:>IRQ=%d", pdev->irq);
>> if (pci_enable_device(pdev)) {
>> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
>> goto err;
>> }
>> dprintk(verbose, MANTIS_ERROR, 1, "<2:>IRQ=%d", pdev->irq);
>>
>> mantis = (struct mantis_pci *)
>> kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
>>
>>
>
>mantis = kmalloc(sizeof(*mantis), GFP_KERNEL);
>
>You don't have to cast a void* to any other pointer and this way you will
>always get the correct size of memory allocated, even if mantis will become
>another pointer type.
>
>
>
Thanks i will keep it in mind, while i get on with the rest. Stuck on
with the Oops at the moment.
FIrst load unload seems to be okay, second one also, but sometimes it is
the 3rd load, or sometimes it is the unload ..
Sounds a bit weird also ..
Thanks,
Manu
Rolf Eike Beer wrote:
>Manu Abraham wrote:
>
>
>
>>So it now looks like this but i have another problem now, after
>>consecutive, load/unload, i get an oops ..
>>
>>
Ah, one more thing i noticed, in the probe function, whatever i do to
generate interrupts, the interrupt handler does not receive any
interrupts, which makes me to think whether request_irq() really yielded
me an IRQ.
One more thing after a module load unload cycle, i did a cat
/proc/interrupts, which led to another oops..
Which makes me to think that request_irq did not work out as expected or
something like that. Any clues .. ?
Regards,
Manu
[ 4001.876841] mantis_pci_probe: <1:>IRQ=23
[ 4001.877014] mantis_pci_probe: <2:>IRQ=23
[ 4001.877176] mantis_pci_probe: Got a device
[ 4001.877356] mantis_pci_probe: We got an IRQ
[ 4001.877514] mantis_pci_probe: We finally enabled the device
[ 4001.877730] Mantis Rev 1, irq: 23, latency: 32
[ 4001.877836] memory: 0xefeff000, mmio: f9218000
[ 4009.156947] mantis_pci_remove: Removing -->Mantis irq: 23,
latency: 32
[ 4009.156949] memory: 0xefeff000, mmio: 0xf9218000
[ 4009.157391] Trying to free free IRQ23
[ 4516.358401] Unable to handle kernel paging request at virtual address
f92bd7e2
[ 4516.360543] printing eip:
[ 4516.362599] c02086fe
[ 4516.364574] *pde = 37ecf067
[ 4516.366591] *pte = 00000000
[ 4516.368554] Oops: 0000 [#1]
[ 4516.370304] SMP
[ 4516.372372] Modules linked in: i2c_core mb86a15 dvb_core 3c59x piix
sd_mod
[ 4516.374903] CPU: 0
[ 4516.374904] EIP: 0060:[<c02086fe>] Not tainted VLI
[ 4516.374906] EFLAGS: 00010097 (2.6.13)
[ 4516.382874] EIP is at vsnprintf+0x35e/0x4f0
[ 4516.385638] eax: f92bd7e2 ebx: 0000000a ecx: f92bd7e2 edx: fffffffe
[ 4516.388683] esi: f4322122 edi: 00000000 ebp: f4322fff esp: f2b0fec0
[ 4516.391978] ds: 007b es: 007b ss: 0068
[ 4516.395387] Process cat (pid: 3375, threadinfo=f2b0f000 task=f3c61020)
[ 4516.395641] Stack: f2b0ff04 f4322fff 00000000 00000000 0000000a
0000000a 00000000 00000000
[ 4516.399463] ffffffff ffffffff f2e328c0 f5f5dfa0 00000017
f2e328c0 c01801d7 f4322120
[ 4516.403230] 00000ee0 c0369c81 f2b0ff20 00000008 c0105a4c
f2e328c0 c0369c7e f92bd7e2
[ 4516.407497] Call Trace:
[ 4516.415981] [<c01801d7>] seq_printf+0x37/0x60
[ 4516.420212] [<c0105a4c>] show_interrupts+0x2bc/0x3b0
[ 4516.424905] [<c017fcd6>] seq_read+0x1d6/0x2d0
[ 4516.429671] [<c015e186>] vfs_read+0xb6/0x180
[ 4516.434507] [<c015e531>] sys_read+0x51/0x80
[ 4516.439056] [<c0102f5f>] sysenter_past_esp+0x54/0x75
[ 4516.444082] Code: 00 83 cf 01 89 44 24 24 eb bb 8b 44 24 48 8b 54 24
20 83 44 24 48 04 8b 08 b8 3c 95 36 c0 81 f9 ff 0f 00 00 0f 46 c8 89 c8
eb 06 <80> 38 00 74 07 40 4a 83 fa ff 75 f4 29 c8 83 e7 10 89 c3 75 20
[ 4516.456263]
Am Donnerstag, 15. September 2005 00:27 schrieben Sie:
>Jiri Slaby wrote:
>> Manu Abraham napsal(a):
>>> Jiri Slaby wrote:
>>>> Manu Abraham napsal(a):
>>>>> Jiri Slaby wrote:
>>>>>> you do NOT do this at all, because you have pdev already (the param
>>>>>> of the probe function)
>>>>>
>>>>> I rewrote the entire thing like this including the pci_remove
>>>>> function too, but now it so seems that in the remove function,
>>>>> pci_get_drvdata(pdev) returns NULL, and hence i get an Oops at
>>>>> module removal.
>>>
>>> I just found that, pci_enable_device() fails. So what's the way to go
>>> ahead ?
>>
>> JESUS.
>
>Hmm.. i finally got it to work. It seems pci_get_device() is necessary,
>i can't seem to enable the device or request for an IRQ the way you
>suggested. It looks some quirks are there though ..
>
>If only i could explain why it works this way and not the other way ..
>
>Thanks for the help,
>Regards,
>Manu
>
>
>
>[ 81.269655] mantis_pci_probe: Got a device
>[ 81.269825] mantis_pci_probe: We got an IRQ
>[ 81.269987] mantis_pci_probe: We finally enabled the device
>[ 81.270191] Mantis Rev 1, irq: 23, latency: 32
>[ 81.270289] memory: 0xefeff000, mmio: f9218000
>[ 81.270519] Trying to free free IRQ23
>[ 90.485885] mantis_pci_remove: Removing -->Mantis irq: 23, latency: 32
>[ 90.485887] memory: 0xefeff000, mmio: 0xf9218000
>[ 90.486293] Trying to free free IRQ23
>[ 90.486429] Trying to free nonexistent resource <efeff000-efefffff>
>
>
>
>
>static int __devinit mantis_pci_probe(struct pci_dev *pdev, const struct
>pci_device_id *mantis_pci_table)
>{
> u8 revision, latency;
> u8 data[2];
> struct mantis_pci *mantis;
> mantis = (struct mantis_pci *) kmalloc(sizeof (struct mantis_pci),
>GFP_KERNEL);
Cast is unneeded.
> if (mantis == NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
> return -ENOMEM;
> }
>
> pdev = pci_get_device(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11,
>NULL);
Hm, this is wrong. pdev contains the struct pci_dev of the found device. If
you do it your way the first device is always used (and the found one is
ignored). When you have two mantis cards in your system it will happily trash
the reference counting. The driver needs to work if you comment out this
line.
> if (pdev) {
This if is not needed either, the probe function is only called if pdev is
set.
> dprintk(verbose, MANTIS_ERROR, 1, "Got a device");
> mantis->mantis_addr = pci_resource_start(pdev, 0);
> if (!request_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0), DRIVER_NAME)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Request for memory region failed");
> goto err0;
> }
> if ((mantis->mantis_mmio = ioremap(mantis->mantis_addr, 0x1000)) ==
>NULL) {
> dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
> goto err1;
> }
> if (request_irq(pdev->irq, (void *) mantis_pci_irq, SA_SHIRQ |
> SA_INTERRUPT, DRIVER_NAME, (void *) mantis) < 0) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ registration failed");
> goto err2;
> }
> dprintk(verbose, MANTIS_DEBUG, 1, "We got an IRQ");
> if (pci_enable_device(pdev)) {
> dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI device enable failed");
> goto err3;
> }
IIRC the call to pci_enable_device() must be the first thing you do. This will
do the things like assigning memory regions to the device and so on.
> dprintk(verbose, MANTIS_DEBUG, 1, "We finally enabled the device");
> pci_set_master(pdev);
> pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
> pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
> mantis->latency = latency;
> mantis->revision = revision;
> if (!latency) {
> pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
> }
> pci_set_drvdata(pdev, mantis);
> dprintk(verbose, MANTIS_ERROR, 0, "Mantis Rev %d, ", mantis->revision);
> dprintk(verbose, MANTIS_ERROR, 0, "irq: %d, latency: %d\nmemory:
>0x%04x, mmio: %p\n", pdev->irq, mantis->latency,
> mantis->mantis_addr, mantis->mantis_mmio);
>
> pci_dev_put(pdev);
>
> } else {
> dprintk(verbose, MANTIS_ERROR, 1, "No device found");
> return -ENODEV;
> }
return 0;
>err3:
> free_irq(pdev->irq, pdev);
>err2:
> if (mantis->mantis_mmio)
> iounmap(mantis->mantis_mmio);
>err1:
> release_mem_region(pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
>err0:
> kfree(mantis);
>
> return 0;
>}
Returning 0 in error cases is just wrong. And you free the assignments even in
case of success AFAICS. Try the return I introduced above and see what
happens.
Eike
/*
Mantis PCI bridge driver
Copyright (C) 2005 Manu Abraham ([email protected])
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
#ifndef _MANTIS_REG_H_
#define _MANTIS_REG_H_
/* Interrupts */
#define MANTIS_INT_STAT 0x00
#define MANTIS_INT_MASK 0x04
#define MANTIS_INT_RISCSTAT (0xf << 28)
#define MANTIS_INT_RISCEN (1 << 27)
#define MANTIS_INT_I2CRACK (1 << 26)
#define MANTIS_INT_IRQ0 (1 << 11)
#define MANTIS_INT_IRQ1 (1 << 10)
#define MANTIS_INT_OCERR (1 << 8)
#define MANTIS_INT_PABORT (1 << 7)
#define MANTIS_INT_RIPERR (1 << 6)
#define MANTIS_INT_PPERR (1 << 5)
#define MANTIS_INT_FTRGT (1 << 3)
#define MANTIS_INT_RISCI (1 << 1)
#define MANTIS_INT_I2CDONE (1 << 0)
/* DMA */
#define MANTIS_DMA_CTL 0x08
#define MANTIS_I2C_RD (1 << 7)
#define MANTIS_I2C_WR (1 << 6)
#define MANTIS_DCAP_MODE (1 << 5)
#define MANTIS_FIFO_TP_4 (0 << 3)
#define MANTIS_FIFO_TP_8 (1 << 3)
#define MANTIS_FIFO_TP_16 (2 << 3)
#define MANTIS_FIFO_EN (1 << 2)
#define MANTIS_DCAP_EN (1 << 1)
#define MANTIS_RISC_EN (1 << 0)
#define MANTIS_RISC_START 0x10
#define MANTIS_RISC_PC 0x14
/* I2C */
#define MANTIS_I2CDATA_CTL 0x18
#define MANTIS_I2C_RATE_1 (0 << 6)
#define MANTIS_I2C_RATE_2 (1 << 6)
#define MANTIS_I2C_RATE_3 (2 << 6)
#define MANTIS_I2C_RATE_4 (3 << 6)
#define MANTIS_I2C_STOP (1 << 5)
#define MANTIS_I2C_PGMODE (1 << 3)
#define MANTIS_GPIF_HIFADDR 0xb0
#define MANTIS_GPIF_HIFDOUT 0xb4
#endif
Jiri Slaby wrote:
>Oh no, Manu's nightmare is back :d.
>
If you don't help people, why do you make a big noise ?
You could atleast keep quiet, to reduce the spam as you termed it. Right ?
Manu
Am Montag, 10. Oktober 2005 14:48 schrieben Sie:
>Rolf Eike Beer wrote:
>>IIRC the call to pci_enable_device() must be the first thing you do. This
>> will do the things like assigning memory regions to the device and so on.
>
>I fixed this one
>
>>Returning 0 in error cases is just wrong. And you free the assignments even
>> in case of success AFAICS. Try the return I introduced above and see what
>> happens.
>
>I fixed this one too ..
>
>
>I have fixed most of the stuff, it is partly working, not ready yet as
>there are some more things to be added to ..
>I have attached what i was working on.
If the kmalloc() fails in mantis_pci_probe() you don't call
pci_disable_device(). And you should kzalloc() instead of kmalloc() and
memset().
It looks like you never use "__u16 vendor_id;" and "__u16 device_id;" in
struct mantis_pci.
Eike
Rolf Eike Beer wrote:
>Am Montag, 10. Oktober 2005 14:48 schrieben Sie:
>
>
>>Rolf Eike Beer wrote:
>>
>>
>>>IIRC the call to pci_enable_device() must be the first thing you do. This
>>>will do the things like assigning memory regions to the device and so on.
>>>
>>>
>>I fixed this one
>>
>>
>>
>>>Returning 0 in error cases is just wrong. And you free the assignments even
>>>in case of success AFAICS. Try the return I introduced above and see what
>>>happens.
>>>
>>>
>>I fixed this one too ..
>>
>>
>>I have fixed most of the stuff, it is partly working, not ready yet as
>>there are some more things to be added to ..
>>I have attached what i was working on.
>>
>>
>
>If the kmalloc() fails in mantis_pci_probe() you don't call
>pci_disable_device(). And you should kzalloc() instead of kmalloc() and
>memset().
>
>
>
Yep, thanks for pointing it out ..
>It looks like you never use "__u16 vendor_id;" and "__u16 device_id;" in
>struct mantis_pci.
>
>
>
I was working on that part, not yet finished on that ..
Thanks,
Manu
Jiri Slaby wrote:
>On 10/10/05, Manu Abraham <[email protected]> wrote:
>
>
>>I have fixed most of the stuff, it is partly working, not ready yet as
>>there are some more things to be added to ..
>>I have attached what i was working on.
>>
>>
>I'm not so bad, I help people (but not 10 times with the same point),
>fuck it and forget.
>
>
>
presumption is not always good, and it is very clear. Most of the
flamewars blaze up because of that. Anyway let's not hover on that ..
>Some points to the new driver:
>indentation isn't so good, the code is not readable well (80 chars on
>a line mainly).
>
>
>
I do agree that indentation is not so good, but how would you suggest
the code be wrapped ?
The problem with wrapping is that readability goes down horribly, but
while debugging a driver, this is too painful.
Considering that it has a long way to go still..
>>static irqreturn_t mantis_pci_irq(int irq, void *dev_id, struct pt_regs *regs)
>>{
>> int count = 0, interrupts = 0;
>> u32 stat = 0, mask = 0, temp;
>>
>> struct mantis_pci *mantis;
>>
>> mantis = (struct mantis_pci *) dev_id;
>>
>>
>you don't need to cast from void *
>struct mantis_pci *mantis = dev_id; is enough
>
>
>
Thanks, Ack'd.
>> mantis->pdev = pdev;
>>
>>
>If you work with this out from pci functions, you should call
>pci_get_dev and in exit function pci_dev_put, otherwise you don't need
>it at all.
>
>
>
Well i am using it in mantis_dma.c, pci_alloc/free
You mean rather than saving off the pointer, i do a pci_get_dev()
and later on in the exit routine, i do a pci_dev_put() .. ?
>mantis_dma_init and others could be __devinit too, or not? Try to use
>it as much as possible.
>
>
>
Any thoughts as to why ? I am not saying it is not needed, but i would
like to know what was the idea.
>>static struct pci_driver mantis_pci_driver = {
>> .name = DRIVER_NAME,
>> .id_table = mantis_pci_table,
>> .probe = mantis_pci_probe,
>> .remove = mantis_pci_remove,
>>
>>
>here should be = __devexit_p(...),
>
>
Ack'd, Thanks ..
>>};
>>
>>
>
>
>
>>err0:
>>
>>
>you should change naming of labels such as errfree, where do you do kfree etc.
>
>
>
Ah , okay.
Thanks,
Manu
Jiri Slaby wrote:
>On 10/10/05, Manu Abraham <[email protected]> wrote:
>
>
>>I have fixed most of the stuff, it is partly working, not ready yet as
>>there are some more things to be added to ..
>>I have attached what i was working on.
>>
>>
>One more thing.
>
>
>
>>static int __devinit mantis_pci_init(void)
>>{
>> return pci_register_driver(&mantis_pci_driver);
>>}
>>
>>static void __devexit mantis_pci_exit(void)
>>{
>> pci_unregister_driver(&mantis_pci_driver);
>>}
>>
>>
>These should be __init and __exit, which could be freed in more cases
>(only few bytes, but...).
>
>
Ack'd..
Thanks,
Manu
Jiri Slaby wrote:
>On 10/10/05, Manu Abraham <[email protected]> wrote:
>
>
>>Jiri Slaby wrote:
>>
>>
>>>Some points to the new driver:
>>>indentation isn't so good, the code is not readable well (80 chars on
>>>a line mainly).
>>>
>>>
>>I do agree that indentation is not so good, but how would you suggest
>>the code be wrapped ?
>>
>>
>Divide strings not "text blabalba \
> continue"
>but with better "text blablabalasjdl"
> "continue"
>
>
The dprintk() macro (in mantis_common.h ) was looking very badly with
wrap, which Andrew also commented on (about the col's) (the macro being
the same, eventhough i was using it elsewhere) it being more than 80
cols, but wrapping the macro made it look like hell.
>wrap line, if it is longer than 80 columns, near last comma, |, & and so on
>You seem to use some editor with tab to be less than 8 columns and
>some headers are indented bad because of it.
>
>
My editor uses 8 columns only as a tab, Thunderbird does really handle
things a bit different though.
>>The problem with wrapping is that readability goes down horribly, but
>>while debugging a driver, this is too painful.
>>Considering that it has a long way to go still..
>>
>>
>>
>>>> mantis->pdev = pdev;
>>>>
>>>>
>>>>
>>>>
>>>If you work with this out from pci functions, you should call
>>>pci_get_dev and in exit function pci_dev_put, otherwise you don't need
>>>it at all.
>>>
>>>
>>Well i am using it in mantis_dma.c, pci_alloc/free
>>
>>
>And it is called only from places, where pdev is known (i.e. in
>parameter of function, e.g. mantis_pci_probe). So you don't need it to
>store in mantis, but only call mantis_dma_init(mantis, pdev). Read
>below.
>
>
>>You mean rather than saving off the pointer, i do a pci_get_dev()
>>and later on in the exit routine, i do a pci_dev_put() .. ?
>>
>>
>But if you really want it, call pci_get_dev() and store it into mantis
>struct. In the _device_ exit routine call the latter. But I think,
>that not to store is better, or the best is to call
>mantis_dma_init(pdev) and do pci_get_drvdata inside.
>
>
i think will pass (pdev) it as a function argument. Looks a bit more cleaner
But what i fail to understand is , if you can pass it as an argument,
why can't you save the pointer in the struct ?
>>>mantis_dma_init and others could be __devinit too, or not? Try to use
>>>it as much as possible.
>>>
>>>
>>Any thoughts as to why ? I am not saying it is not needed, but i would
>>like to know what was the idea.
>>
>>
>Kernel frees up the sections that won't be needed anymore. You put the
>function in some section by this.
>
>
Ok,
Thanks.
Manu
/*
Mantis PCI bridge driver
Copyright (C) 2005 Manu Abraham ([email protected])
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
#include <asm/io.h>
#include <asm/pgtable.h>
#include <asm/page.h>
#include <linux/kmod.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
#include <linux/device.h>
#include "mantis_common.h"
#include "mantis_core.h"
#include <asm/irq.h>
#include <linux/signal.h>
#include <linux/sched.h>
#include <linux/interrupt.h>
unsigned int verbose = 1;
module_param(verbose, int, 0644);
MODULE_PARM_DESC(verbose, "verbose startup messages, default is 1 (yes)");
#define PCI_VENDOR_ID_MANTIS 0x1822
#define PCI_DEVICE_ID_MANTIS_R11 0x4e35
#define DRIVER_NAME "Mantis"
static struct pci_device_id mantis_pci_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_MANTIS, PCI_DEVICE_ID_MANTIS_R11) },
{ 0 },
};
MODULE_DEVICE_TABLE(pci, mantis_pci_table);
static irqreturn_t mantis_pci_irq(int irq, void *dev_id, struct pt_regs *regs)
{
int count = 0, interrupts = 0;
u32 stat = 0, mask = 0, temp;
struct mantis_pci *mantis = dev_id;
if (mantis == NULL)
dprintk(verbose, MANTIS_DEBUG, 1, "Aeio, mantis ISR");
/*
* Don't read each and everytime, but read in one shot
* We need to save the states for later too ..
*
*/
stat = mmread(MANTIS_INT_STAT);
mask = mmread(MANTIS_INT_MASK);
/*
* To speed up things, do a basic check whether it is our
* interrupt.
* Also, we don't have a mask for I2CRACK
*/
if (!(stat & (mask | MANTIS_INT_I2CRACK)) ) {
dprintk(verbose, MANTIS_ERROR, 1, "Not ours !");
return IRQ_NONE;
}
/*
* The int is for us, clear int condition
*/
mmwrite(stat, MANTIS_INT_STAT);
/*
* Check how many 1's are in the INT_STAT, Count will reflect
* no. of interrupts, we should loop count times.
*
*/
temp = stat;
while (temp) {
if (temp & 0x01) {
interrupts++;
dprintk(verbose, MANTIS_DEBUG, 1, "Interrupt @ %d", interrupts);
}
temp >>= 1;
count++;
/*
* We should never loop more than reg. width
* else, Bail out
*/
if (count > 32) {
dprintk(verbose, MANTIS_ERROR, 1, "Bailing out !!");
break;
}
}
count = 0;
while (count < interrupts) {
if ((stat & mask) & MANTIS_INT_RISCEN) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** DMA enabl ****");
mantis->mantis_int_stat |= MANTIS_INT_RISCEN;
stat &= ~MANTIS_INT_RISCEN;
/*
* It's a shame that there is no mask for I2CRACK !
*/
} else if (stat & MANTIS_INT_I2CRACK) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** I2C R-ACK ****");
mantis->mantis_int_stat |= MANTIS_INT_I2CRACK;
stat &= ~MANTIS_INT_I2CRACK;
} else if ((stat & mask) & MANTIS_INT_IRQ0) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT IRQ-0 ****");
mantis->mantis_int_stat |= MANTIS_INT_IRQ0;
stat &= ~MANTIS_INT_IRQ0;
} else if ((stat & mask) & MANTIS_INT_IRQ1) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT IRQ-1 ****");
mantis->mantis_int_stat |= MANTIS_INT_IRQ1;
stat &= ~MANTIS_INT_IRQ1;
} else if ((stat & mask) & MANTIS_INT_OCERR) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT OCERR ****");
mantis->mantis_int_stat |= MANTIS_INT_OCERR;
stat &= ~MANTIS_INT_OCERR;
} else if ((stat & mask) & MANTIS_INT_PABORT) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT PABRT ****");
mantis->mantis_int_stat |= MANTIS_INT_PABORT;
stat &= ~MANTIS_INT_PABORT;
} else if ((stat & mask) & MANTIS_INT_RIPERR) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT RIPRR ****");
mantis->mantis_int_stat |= MANTIS_INT_RIPERR;
stat &= ~MANTIS_INT_RIPERR;
} else if ((stat & mask) & MANTIS_INT_PPERR) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT PPERR ****");
mantis->mantis_int_stat |= MANTIS_INT_PPERR;
stat &= ~MANTIS_INT_PPERR;
} else if ((stat & mask) & MANTIS_INT_FTRGT) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** INT FTRGT ****");
mantis->mantis_int_stat |= MANTIS_INT_FTRGT;
stat &= ~MANTIS_INT_FTRGT;
} else if ((stat & mask) & MANTIS_INT_RISCI) {
mantis->finished_block = (mantis->mantis_int_stat & MANTIS_INT_RISCSTAT) >> 28;
tasklet_schedule(&mantis->tasklet);
stat &= ~MANTIS_INT_RISCI;
} else if ((stat & mask) & MANTIS_INT_I2CDONE) {
dprintk(verbose, MANTIS_DEBUG, 1, "**** I2C DONE ****");
mantis->mantis_int_stat |= MANTIS_INT_I2CDONE;
stat &= ~MANTIS_INT_I2CDONE;
} else {
dprintk(verbose, MANTIS_DEBUG, 1, "Unknown INT ???");
}
count++;
/*
* Being paranoid, check whether we are too loopy !
*/
if (count > 32) {
dprintk(verbose, MANTIS_ERROR, 1, "IRQ Lockup, clearing INT mask");
dprintk(verbose, MANTIS_ERROR, 1, "Count=%d, Interrupts=%d, stat=[0x%08x]", count, interrupts, stat);
mmwrite(0, MANTIS_INT_MASK);
stat = 0, interrupts = 0;
break;
}
}
return IRQ_HANDLED;
}
static int __devinit mantis_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *mantis_pci_table)
{
u8 revision, latency;
struct mantis_pci *mantis;
if (pci_enable_device(pdev)) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis PCI enable failed");
goto exit;
}
/*
* We need to change this to kzalloc on newer kernels
* kzalloc() = kmalloc() + memset()
* Code simplification for 2.6.14 and upwards
*/
mantis = kmalloc(sizeof (struct mantis_pci), GFP_KERNEL);
// mantis = kzalloc(sizeof (struct mantis_pci), GFP_KERNEL);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Out of memory");
goto disable;
}
memset(mantis, 0, sizeof (struct mantis_pci));
mantis->mantis_addr = pci_resource_start(pdev, 0);
if (!request_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0), DRIVER_NAME)) {
goto free;
}
if ((mantis->mantis_mmio =
ioremap(mantis->mantis_addr, 0x1000)) == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "IO remap failed");
goto release;
}
/*
* Clear and disable all interrupts at startup
* to avoid lockup situations
*/
mmwrite(0x00, MANTIS_INT_STAT);
mmwrite(0x00, MANTIS_INT_MASK);
if (request_irq(pdev->irq, mantis_pci_irq, SA_SHIRQ | SA_INTERRUPT,
DRIVER_NAME, mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis IRQ reg failed");
goto unmap;
}
pci_set_master(pdev);
pci_set_drvdata(pdev, mantis);
pci_read_config_byte(pdev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
mantis->latency = latency;
mantis->revision = revision;
/*
* Setup default latency 32 if none specified
*/
if (!latency)
pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 32);
printk("%s: Mantis Rev %d, ", __func__, mantis->revision);
printk("irq: %d, latency: %d\nmemory: 0x%lx, mmio: 0x%p\n",
pdev->irq, mantis->latency, mantis->mantis_addr,
mantis->mantis_mmio);
if ((mantis_dma_init(pdev)) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis DMA init failed");
goto disable;
}
/*
* No more PCI specific stuff !
*/
if (mantis_core_init(mantis) < 0) {
dprintk(verbose, MANTIS_ERROR, 1, "Mantis core init failed");
goto dma_exit;
}
return 0;
/*
* Error conditions ..
*/
dma_exit:
if (mantis_dma_exit(pdev) < 0)
dprintk(verbose, MANTIS_ERROR, 1, "Mantis DMA exit failed");
dprintk(verbose, MANTIS_DEBUG, 1, "Err: IO Unmap");
unmap:
if (mantis->mantis_mmio)
iounmap(mantis->mantis_mmio);
release:
dprintk(verbose, MANTIS_DEBUG, 1, "Err: Release regions");
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
free:
kfree(mantis);
disable:
pci_disable_device(pdev);
dprintk(verbose, MANTIS_DEBUG, 1, "Err: Disabling device");
exit:
dprintk(verbose, MANTIS_DEBUG, 1, "Err:");
return -ENODEV;
}
static void __devexit mantis_pci_remove(struct pci_dev *pdev)
{
struct mantis_pci *mantis = pci_get_drvdata(pdev);
if (mantis == NULL) {
dprintk(verbose, MANTIS_ERROR, 1, "Aeio, Mantis NULL ptr");
return;
}
mantis_core_exit(mantis);
printk("%s: Mantis irq: %d,latency: %d\n memory: 0x%lx, mmio: 0x%p",
__func__, pdev->irq, mantis->latency, mantis->mantis_addr,
mantis->mantis_mmio);
free_irq(pdev->irq, mantis);
pci_release_regions(pdev);
if (mantis_dma_exit(pdev) < 0)
dprintk(verbose, MANTIS_ERROR, 1, "DMA exit failed");
pci_set_drvdata(pdev, NULL);
pci_disable_device(pdev);
kfree(mantis);
}
static struct pci_driver mantis_pci_driver = {
.name = DRIVER_NAME,
.id_table = mantis_pci_table,
.probe = mantis_pci_probe,
.remove = __devexit_p(mantis_pci_remove),
};
static int __init mantis_pci_init(void)
{
return pci_register_driver(&mantis_pci_driver);
}
static void __exit mantis_pci_exit(void)
{
pci_unregister_driver(&mantis_pci_driver);
}
module_init(mantis_pci_init);
module_exit(mantis_pci_exit);
MODULE_DESCRIPTION("Mantis PCI DTV bridge driver");
MODULE_AUTHOR("Manu Abraham");
MODULE_LICENSE("GPL");