Provide dummy devm_ioport_* if !HAS_IOPORT
Subsystems (4)
PATCH 1/1 Provide dummy devm_ioport_* if !HAS_IOPORT
Provide an dummy implementation of devm_ioport_map() and devm_ioport_unmap() to allow drivers (eg, pata_platform) to build for platforms where CONFIG_NO_IOPORT is selected.
--- include/linux/io.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
include/linux/io.h +13 -0
@@ -33,9 +33,22 @@ int ioremap_page_range(unsigned long addr, unsigned long end,/* * Managed iomap interface */ +#ifdef CONFIG_HAS_IOPORT void __iomem *devm_ioport_map (struct device *dev,unsigned long port,unsigned int nr);void devm_ioport_unmap (struct device *dev,void __iomem *addr); +#else +static inline void __iomem *devm_ioport_map (struct device *dev, +unsigned long port, +unsigned int nr) +{ +return NULL ; +} + +static inline void devm_ioport_unmap (struct device *dev,void __iomem *addr) +{ +} +#endif void __iomem *devm_ioremap (struct device *dev,unsigned long offset,unsigned long size);
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
RFC 1/1 pata_platform for ARM RiscPC
Add pata_platform device for RiscPC, thereby converting the primary IDE channel on the machine to PATA.
---
Since this is dependent on the previous patch (to avoid build errors) this needs to wait until the devm_ioport patch is merged. I'll therefore push this through the ARM tree at the appropriate time.
arch/arm/mach-rpc/riscpc.c | 35 +++++++++++++++++++++++++++++++++++ drivers/ata/Kconfig | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-)
arch/arm/mach-rpc/riscpc.c +35 -0
@@ -17,6 +17,7 @@#include <linux/sched.h> #include <linux/device.h> #include <linux/serial_8250.h> +#include <linux/pata_platform.h> #include <asm/elf.h> #include <asm/io.h>
@@ -159,11 +160,45 @@ static struct plat_serial8250_port serial_platform_data[] = { }, }; +static struct pata_platform_info pata_platform_data = { + .ioport_shift =2 , +}; + +static struct resource pata_resources[] = { + [0 ] = { + .start =0x030107c0 , + .end =0x030107df , + .flags =IORESOURCE_MEM , + }, + [1 ] = { + .start =0x03010fd8 , + .end =0x03010fdb , + .flags =IORESOURCE_MEM , + }, + [2 ] = { + .start =IRQ_HARDDISK , + .end =IRQ_HARDDISK , + .flags =IORESOURCE_IRQ , + }, +}; + +static struct platform_device pata_device = { + .name ="pata_platform" , + .id =-1 , + .num_resources =ARRAY_SIZE (pata_resources), + .resource = pata_resources, + .dev = { + .platform_data = &pata_platform_data, + .coherent_dma_mask = ~0 ,/* grumble */ + }, +}; +static struct platform_device *devs[]__initdata = { &iomd_device, &kbd_device, &serial_device, &acornfb_device, + &pata_device, };static int __init rpc_init (void )
drivers/ata/Kconfig +1 -1
@@ -545,7 +545,7 @@ config PATA_WINBOND_VLB config PATA_PLATFORM tristate "Generic platform device PATA support" - depends on EMBEDDED + depends on EMBEDDED || ARCH_RPC help This option enables support for generic directly connected ATA devices commonly found on embedded systems.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_icside driver
Below is an initial attempt at converting the ICS IDE driver to fit into the PATA infrastructure.
There's a number of FIXMEs in there: due to the hardware missing resistors on the interrupt signals from the drives, a port without any drives attached results in spurious interrupts being generated.
To prevent this, we need to disable the interrupts from the port on the card if no drives are found, but unfortunately ATA doesn't call the "port_disable" method in this circumstance.
The second FIXME area is ata_irq_ack - it is unconditionally coded for SFF-type interfaces. I believe that using this function in non-BMDMA interfaces is wrong - it attempts to read from the BMDMA registers irrespective of whether ap->ioaddr.bmdma_addr is set or not. The question this poses is: what should non-BMDMA implementations use for this method? Note that pata_platform also uses this function despite not supporting BMDMA which seems even more suspicious.
drivers/ata/Kconfig +8 -0
@@ -552,6 +552,14 @@ config PATA_PLATFORM If unsure, say N. +config PATA_ICSIDE + tristate "Acorn ICS PATA support" + depends on ARM && ARCH_ACORN + help + On Acorn systems, say Y here if you wish to use the ICS PATA + interface card. This is not required for ICS partition support. + If you are unsure, say N to this. + config PATA_IXP4XX_CF tristate "IXP4XX Compact Flash support" depends on ARCH_IXP4XX
drivers/ata/Makefile +1 -0
@@ -61,6 +61,7 @@ obj-$(CONFIG_PATA_TRIFLEX) += pata_triflex.o obj-$(CONFIG_PATA_IXP4XX_CF) += pata_ixp4xx_cf.o obj-$(CONFIG_PATA_SCC) += pata_scc.o obj-$(CONFIG_PATA_PLATFORM) += pata_platform.o +obj-$(CONFIG_PATA_ICSIDE) += pata_icside.o # Should be last but one libata driver obj-$(CONFIG_ATA_GENERIC) += ata_generic.o # Should be last libata driver
drivers/ata/pata_icside.c new file +679 -0
679 lines not on this page · raw message
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_icside driver
Quoted 7 lines
The second FIXME area is
ata_irq_ack- it is unconditionally coded for SFF-type interfaces. I believe that using this function in non-BMDMA interfaces is wrong - it attempts to read from the BMDMA registers irrespective of whetherap->ioaddr.bmdma_addris set or not. The question this poses is: what should non-BMDMA implementations use for this method? Note thatpata_platformalso uses this function despite not supporting BMDMA which seems even more suspicious.
Thats a bug that has arrived again. The older code was corrected to handle this properly but the fix appears to have become lost. The ioread/iowrite code actually made quite a mess (all the address reporting is also broken) and we do some iffy things like compare the iomap result with zero and assume thats the same as checking for true bus zero addresses.
ata_irq_ack is part of the SFF layer so its fine that it assumes SFF but its wrong that it is used unconditionally and it shouldn't be used this way. It just needs a (!ap->ioaddr.bmdma_addr) test adding (assuming thats valid for iomap)
Alan
Re: [RFC] pata_icside driver
Quoted 15 lines · Alan Cox
Quoted 7 lines
The second FIXME area is
ata_irq_ack- it is unconditionally coded for SFF-type interfaces. I believe that using this function in non-BMDMA interfaces is wrong - it attempts to read from the BMDMA registers irrespective of whetherap->ioaddr.bmdma_addris set or not. The question this poses is: what should non-BMDMA implementations use for this method? Note thatpata_platformalso uses this function despite not supporting BMDMA which seems even more suspicious.Thats a bug that has arrived again. The older code was corrected to handle this properly but the fix appears to have become lost. The ioread/iowrite code actually made quite a mess (all the address reporting is also broken) and we do some iffy things like compare the iomap result with zero and assume thats the same as checking for true bus zero addresses.
Quoted 4 lines
ata_irq_ackis part of the SFF layer so its fine that it assumes SFF but its wrong that it is used unconditionally and it shouldn't be used this way. It just needs a (!ap->ioaddr.bmdma_addr) test adding (assuming thats valid for iomap)
No. It does not need such a test, as it requires BMDMA, not just an SFF-style Status register. It is up to the driver to decide whether or not ata_irq_ack() is appropriate for your hardware.
pata_icside needs its own ata_irq_ack -- which may just be as simple as reading the Status register to clear the interrupt condition.
If others need this as well, ata_sff_irq_ack() would be a good generic function to create.
Jeff
Re: [RFC] pata_icside driver
Quoted 8 lines · On Sun, 08 Apr 2007 21:03:10 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 4 lines
ata_irq_ackis part of the SFF layer so its fine that it assumes SFF but its wrong that it is used unconditionally and it shouldn't be used this way. It just needs a (!ap->ioaddr.bmdma_addr) test adding (assuming thats valid for iomap)No. It does not need such a test, as it requires BMDMA, not just an SFF-style Status register. It is up to the driver to decide whether or not
ata_irq_ack()is appropriate for your hardware.
Then no SFF hardware can use ata_irq_ack. Not one card: Because in every case it is permissible that BAR4 is not allocated and the device is running non-DMA, or that the SFF hardware does not support DMA.
Quoted 5 lines
pata_icsideneeds its ownata_irq_ack-- which may just be as simple as reading the Status register to clear the interrupt condition.If others need this as well,
ata_sff_irq_ack()would be a good generic function to create.
We should just rename ata_irq_ack(). It is in libata-sff, so it's either wrong (missing a test), or in the wrong file completely.
Alan
Re: [RFC] pata_icside driver
Quoted 1 line · Alan Cox
Then no SFF hardware can use
ata_irq_ack. Not one card: Because in every
OK
Quoted 2 lines
We should just rename
ata_irq_ack(). It is in libata-sff, so it's either wrong (missing a test), or in the wrong file completely.
There is plenty of BMDMA-specific stuff in libata-sff, so neither assertion is correct.
It's fine to rename it, though.
Jeff
Re: [RFC] pata_icside driver
It should be noted, mainly for russell's benefit, that the ->irq_ack() hook is only called if a debug define is manually enabled in libata.h, in a single callsite.
The number of people that actually use the hook -- kernel hackers debugging screaming interrupts -- can probably be counted on one hand.
Jeff
Re: [RFC] pata_icside driver
Quoted 6 lines · Jeff Garzik
It should be noted, mainly for russell's benefit, that the ->irq_ack() hook is only called if a debug define is manually enabled in
libata.h, in a single callsite.The number of people that actually use the hook -- kernel hackers debugging screaming interrupts -- can probably be counted on one hand.
Ok, in that case it makes sense for me to set this to ata_dummy_irq_ack since I don't have any additional status registers specific to the interface.
Presumably a similar change to pata_platform.c would be appropriate?
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_icside driver
Quoted 10 lines · Russell King
Quoted 3 lines · Jeff Garzik
It should be noted, mainly for russell's benefit, that the ->irq_ack() hook is only called if a debug define is manually enabled in
libata.h, in a single callsite.The number of people that actually use the hook -- kernel hackers debugging screaming interrupts -- can probably be counted on one hand.
Ok, in that case it makes sense for me to set this to
ata_dummy_irq_acksince I don't have any additional status registers specific to the interface.Presumably a similar change to
pata_platform.cwould be appropriate?
I was just pointing out the practical reality.
The proper implementation of the hook is likely /not/ a dummy on your platform.
I presume you clear the interrupt condition by reading the ATA shadow register 'Status', like most other SFF-like controllers, so your implementation of ->irq_ack() should reflect that.
The proper implementation is basically what Alan described: ata_irq_ack() minus the BMDMA bitbanging.
Jeff
Alan Cox
NAKRe: [RFC] pata_icside driver
In reply to Russell Kingdrivers/ata/pata_icside.c line 186
Quoted 5 lines
drivers/ata/pata_icside.c new file +5
-0+/* +* DMA is based on a 16MHz clock +*/ +if (ata_timing_compute (adev, adev->dma_mode, &t,1000 ,1 )) +return ;
This seems strange for a 16MHz clock.
drivers/ata/pata_icside.c
Quoted 6 lines
drivers/ata/pata_icside.c new file +6
-0+ +/* +* Now, properly adjust the timings. If we have a 62.5ns clock +* period and we ask for MWDMA2, it calculates the following +* timings: active 125ns, recovery 62.5ns, cycle 125ns. +* Quite obviously bogus.
NAK.
At this point you need to work out why you are getting bogus results and fix it or demonstrate a bug in the core code and fix that.
Re: [RFC] pata_icside driver
Quoted 7 lines · Alan Cox
drivers/ata/pata_icside.c line 186
Quoted 5 lines
drivers/ata/pata_icside.c new file +5
-0+/* +* DMA is based on a 16MHz clock +*/ +if (ata_timing_compute (adev, adev->dma_mode, &t,1000 ,1 )) +return ;This seems strange for a 16MHz clock.
We do this because, although the underlying clock is 16MHz, the DIOR and DIOW signals go through a bit of logic and are not synchronised to this clock.
As explained in the comments above:
+ * Type Active Recovery Cycle + * A 250 (250) 312 (550) 562 (800) + * B 187 (200) 250 (550) 437 (750) + * C 125 (125) 125 (375) 250 (500) + * D 62 (50) 125 (375) 187 (425)
+ *
+ * (figures in brackets are actual measured timings on DIOR/DIOW)
The figures outside the brackets are the documented timings on the host bus, but these are not what the drive sees. The timings which the drive sees are those in brackets.
The timings the drive sees clearly are not based upon a 16MHz clock period. Therefore, I'd prefer to get the nanoseconds from the calculation and work from that.
Quoted 11 lines
drivers/ata/pata_icside.c
Quoted 6 lines
drivers/ata/pata_icside.c new file +6
-0+ +/* +* Now, properly adjust the timings. If we have a 62.5ns clock +* period and we ask for MWDMA2, it calculates the following +* timings: active 125ns, recovery 62.5ns, cycle 125ns. +* Quite obviously bogus. NAK.
At this point you need to work out why you are getting bogus results and fix it or demonstrate a bug in the core code and fix that.
Obviously I can demonstrate a bug. 8)
Lets say that we want to do MW DMA mode 2. This has the minimum timing of 70ns active, 25ns recovery, 120ns cycle time.
When you quantise those figures using a clock period of 62.5ns (16MHz) you end up with: 2 clocks active (2*62.5 > 70), 1 clock recovery (1*62.5 > 25) and 2 clocks cycle (2*62.5 > 120).
Last time I checked, active + recovery must always be equal to the cycle time, and unless my math is failing me, 2 + 1 does not equal 2.
It's quite obvious that returning the active time _equal_ to the cycle time is even more utterly bogus - that means you're asking for the signal to remain active for the entire cycle without any recovery.
So, you probably need to incorporate that logic from pata_icside into the libata core.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_icside driver
Quoted 9 lines
Lets say that we want to do MW DMA mode 2. This has the minimum timing of 70ns active, 25ns recovery, 120ns cycle time.
When you quantise those figures using a clock period of 62.5ns (16MHz) you end up with: 2 clocks active (2*62.5 > 70), 1 clock recovery (1*62.5 > 25) and 2 clocks cycle (2*62.5 > 120).
Last time I checked, active + recovery must always be equal to the cycle time, and unless my math is failing me, 2 + 1 does not equal 2.
Do you mean active + recovery must be less than or equal to the cycle time? Because 70ns + 25ns does not equal 120ns either...
- R.
Re: [RFC] pata_icside driver
Quoted 7 lines · Roland Dreier
Quoted 7 lines
Lets say that we want to do MW DMA mode 2. This has the minimum timing of 70ns active, 25ns recovery, 120ns cycle time.
When you quantise those figures using a clock period of 62.5ns (16MHz) you end up with: 2 clocks active (2*62.5 > 70), 1 clock recovery (1*62.5 > 25) and 2 clocks cycle (2*62.5 > 120).
Quoted 5 lines
Quoted 2 lines
Last time I checked, active + recovery must always be equal to the cycle time, and unless my math is failing me, 2 + 1 does not equal 2.
Do you mean active + recovery must be less than or equal to the cycle time? Because 70ns + 25ns does not equal 120ns either...
Now that I've had some breakfast, yes.
In any case, active + recovery must never be greater than the cycle time.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_icside driver
In reply to Russell KingQuoted 3 lines
The timings the drive sees clearly are not based upon a 16MHz clock period. Therefore, I'd prefer to get the nanoseconds from the calculation and work from that.
Makes sense.
Quoted 4 lines
Obviously I can demonstrate a bug. 8)
Lets say that we want to do MW DMA mode 2. This has the minimum timing of 70ns active, 25ns recovery, 120ns cycle time.
Well it depends on the drive id data but assuming the defaults yes
Quoted 6 lines
When you quantise those figures using a clock period of 62.5ns (16MHz) you end up with: 2 clocks active (2*62.5 > 70), 1 clock recovery (1*62.5 > 25) and 2 clocks cycle (2*62.5 > 120).
Last time I checked, active + recovery must always be equal to the cycle time, and unless my math is failing me, 2 + 1 does not equal 2.
The libata code does the following:
if (t->active + t->recover < t->cycle) {
t->active += (t->cycle - (t->active + t->recover)) / 2;
t->recover = t->cycle - t->active;
}which probably means for 16MHz you don't have enough resolution to be sure you'll error in this direction in all cases. If so you just need to try adding
if (t->active + t->recover > t->cycle) t->cycle = t->active + t->recover
to stretch the cycle time to fit the resolution as well.
And we should get this tested/fixed by someone seeing the problem, before and instead of putting hacks/notes in drivers that may then get lost
Alan
Re: [RFC] pata_icside driver
Quoted 18 lines · Alan Cox
Quoted 6 lines
When you quantise those figures using a clock period of 62.5ns (16MHz) you end up with: 2 clocks active (2*62.5 > 70), 1 clock recovery (1*62.5 > 25) and 2 clocks cycle (2*62.5 > 120).
Last time I checked, active + recovery must always be equal to the cycle time, and unless my math is failing me, 2 + 1 does not equal 2.
The libata code does the following:
if (t->active + t->recover < t->cycle) { t->active += (t->cycle - (t->active + t->recover)) / 2; t->recover = t->cycle - t->active; }which probably means for 16MHz you don't have enough resolution to be sure you'll error in this direction in all cases. If so you just need to try adding
Quoted 7 lines
if (t->active + t->recover > t->cycle) t->cycle = t->active + t->recoverto stretch the cycle time to fit the resolution as well.
And we should get this tested/fixed by someone seeing the problem, before and instead of putting hacks/notes in drivers that may then get lost
I agree; this is why I sent the patch out as an RFC before wanting it merged. My feeling at the time when I wrote this is it's something that the libata timing calculation code should already be catering for.
I should've explicitly mentioned that in the comments, but forgot.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_icside driver
In reply to Russell KingQuoted 11 lines · Russell King
Below is an initial attempt at converting the ICS IDE driver to fit into the PATA infrastructure.
There's a number of FIXMEs in there: due to the hardware missing resistors on the interrupt signals from the drives, a port without any drives attached results in spurious interrupts being generated.
To prevent this, we need to disable the interrupts from the port on the card if no drives are found, but unfortunately ATA doesn't call the "
port_disable" method in this circumstance.
Here's an updated version. I've removed the correction of the cycle time - since we're checking whether all of active, recovery and cycle periods fit the hardware, the correction becomes unnecessary.
I still suggest that the PATA core folk consider fixing their timing calculation function in that respect though.
This driver continues to have the so far ignored issue concerning port_disable. It would be good to have some feedback on this instead of this driver continuing to be crippled by the libata core code. This really needs resolving before this driver can be merged, though I'm not sure how.
drivers/ata/Kconfig +8 -0
@@ -552,6 +552,14 @@ config PATA_PLATFORM If unsure, say N. +config PATA_ICSIDE + tristate "Acorn ICS PATA support" + depends on ARM && ARCH_ACORN + help + On Acorn systems, say Y here if you wish to use the ICS PATA + interface card. This is not required for ICS partition support. + If you are unsure, say N to this. + config PATA_IXP4XX_CF tristate "IXP4XX Compact Flash support" depends on ARCH_IXP4XX
drivers/ata/Makefile +1 -0
@@ -61,6 +61,7 @@ obj-$(CONFIG_PATA_TRIFLEX) += pata_triflex.o obj-$(CONFIG_PATA_IXP4XX_CF) += pata_ixp4xx_cf.o obj-$(CONFIG_PATA_SCC) += pata_scc.o obj-$(CONFIG_PATA_PLATFORM) += pata_platform.o +obj-$(CONFIG_PATA_ICSIDE) += pata_icside.o # Should be last but one libata driver obj-$(CONFIG_ATA_GENERIC) += ata_generic.o # Should be last libata driver
drivers/ata/pata_icside.c new file +686 -0
686 lines not on this page · raw message
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of:
Re: [RFC] pata_platform for ARM RiscPC
In reply to Russell KingQuoted 4 lines · Russell King
Add
pata_platformdevice for RiscPC, thereby converting the primary IDE channel on the machine to PATA.Signed-off-by Russell King <[email protected]>
ACK
BTW, please CC linux-ide on anything related to old-IDE or libata. Thanks.
Quoted 3 lines
Since this is dependent on the previous patch (to avoid build errors) this needs to wait until the
devm_ioportpatch is merged. I'll therefore push this through the ARM tree at the appropriate time.
Sounds good.
Jeff
Quoted 2 lines · Russell King
Provide an dummy implementation of
devm_ioport_map()anddevm_ioport_unmap()to allow drivers (eg,pata_platform) to build for
Btw, I there a chance we can collect all the devm_* crap into a single implementation and header file instead of spreading it all over the tree?