headers
Phil Sutter | 1 Nov 2008 17:09

Re: [PATCH] fix pata-rb532-cf

Hi,

On Fri, Oct 31, 2008 at 01:38:27PM +0300, Sergei Shtylyov wrote:
> >The original driver (see my patch) accesses 0x80D to read error codes,
> >so I assumed this is correct. Indeed, the driver works also with the
> >standard setting. Is there a way I can clearly verify which is the right
> >offset?
> >  
> 
>   I assume you don't have the documentation? I wodner why they needed 
> to duplicate (or remap) this register...

Sadly not. All I have is the specification of the SoC by IDT, but the
CompactFlash slot was added by Mikrotik, so it's not mentioned in there.
The routerboard user's manual Mikrotik has published doesn't contain
much more information about it than "it's there".

>   No. The alternatives would be to use local variable as a loop counter 
> or, better yet, using readsb()/writesb() instead of the loops...
> Wait! The original driver used 32-bit I/O to this register, not 8-bit -- 
> so it looks like you have artificially slowed it down... :-/

Ok, so this should be clear now. I changed the code to use readl() and
writel() (the reads*() and writes*() versions sadly aren't useable as
they increment the target memory pointer) which worked fine.

When it comes to the register offsets I'd say in dubio pro reo and keep
the custom error address. Changing it afterwards to the standard should
also be much easier than fiddling out what the custom offset was again.
(Continue reading)

Permalink | Reply | 18 comments |
headers
Phil Sutter | 1 Nov 2008 17:12

[PATCH] fix pata-rb532-cf

After applying the following changes I could verify functionality by
mounting a filesystem on the cfdisk and reading/writing files in it.

The symbols rb532_gpio_set_ilevel and rb532_gpio_set_istat are not yet
available in a vanilla kernel, an appropriate patch has already been
sent to the linux-mips mailinglist.

Also change rb532_pata_data_xfer() so it reads and writes 4-byte blocks,
like the original driver did. Rename the offset definition of the
buffered data register for clearness.

Signed-off-by: Phil Sutter <n0-1 <at> freewrt.org>
---
 drivers/ata/pata_rb532_cf.c |   35 +++++++++++++++++++++++------------
 1 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index f8b3ffc..bdf413e 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c
 <at>  <at>  -31,6 +31,7  <at>  <at> 
 #include <scsi/scsi_host.h>

 #include <asm/gpio.h>
+#include <asm/mach-rc32434/gpio.h>

 #define DRV_NAME	"pata-rb532-cf"
 #define DRV_VERSION	"0.1.0"
 <at>  <at>  -39,9 +40,11  <at>  <at> 
 #define RB500_CF_MAXPORTS	1
(Continue reading)

Permalink | Reply | 16 comments |
headers
Sergei Shtylyov | 1 Nov 2008 17:16

Re: [PATCH] fix pata-rb532-cf

Hello.

Phil Sutter wrote:

>>   No. The alternatives would be to use local variable as a loop counter 
>> or, better yet, using readsb()/writesb() instead of the loops...
>> Wait! The original driver used 32-bit I/O to this register, not 8-bit -- 
>> so it looks like you have artificially slowed it down... :-/
>>     
>
> Ok, so this should be clear now. I changed the code to use readl() and
> writel() (the reads*() and writes*() versions sadly aren't useable as
> they increment the target memory pointer) which worked fine.
>   

   So what? This is perfectly valid. Doesn't the current code increment it?

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
Sergei Shtylyov | 1 Nov 2008 17:26

Re: [PATCH] fix pata-rb532-cf

Hello.

Phil Sutter wrote:

> After applying the following changes I could verify functionality by
> mounting a filesystem on the cfdisk and reading/writing files in it.
>
> The symbols rb532_gpio_set_ilevel and rb532_gpio_set_istat are not yet
> available in a vanilla kernel, an appropriate patch has already been
> sent to the linux-mips mailinglist.
>
> Also change rb532_pata_data_xfer() so it reads and writes 4-byte blocks,
> like the original driver did. Rename the offset definition of the
> buffered data register for clearness.
>   

   Looks ike I'll have to NAK this part...

> diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
> index f8b3ffc..bdf413e 100644
> --- a/drivers/ata/pata_rb532_cf.c
> +++ b/drivers/ata/pata_rb532_cf.c
>   
[...]
>  <at>  <at>  -39,9 +40,11  <at>  <at> 
>  #define RB500_CF_MAXPORTS	1
>  #define RB500_CF_IO_DELAY	400
>  
> -#define RB500_CF_REG_CMD	0x0800
> +#define RB500_CF_REG_BASE	0x0800
(Continue reading)

Permalink | Reply | 15 comments |
headers
Phil Sutter | 1 Nov 2008 17:45

Re: [PATCH] fix pata-rb532-cf

Hi,

On Sat, Nov 01, 2008 at 07:26:34PM +0300, Sergei Shtylyov wrote:
>   So, I didn't get what was wrong with using readsl() and writesl()?
>   Besides, using readl() and witel() this way would be wrong on BE mode 
> since the data is expected to be stored to memory in the LE order.

Sorry, my fault. When having a glance at the macro definition, I messed
up the parameter's meanings and assumed the target address would be
incremented, which is normally the case when writing regular memory, not
to IO addresses.

I changed (and successfully tested) the code, patch follows.

Thanks again, Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 12 comments |
headers
Sergei Shtylyov | 1 Nov 2008 17:45

Re: [PATCH] fix pata-rb532-cf

Hello, I wrote:

>> After applying the following changes I could verify functionality by
>> mounting a filesystem on the cfdisk and reading/writing files in it.
>>
>> The symbols rb532_gpio_set_ilevel and rb532_gpio_set_istat are not yet
>> available in a vanilla kernel, an appropriate patch has already been
>> sent to the linux-mips mailinglist.
>>
>> Also change rb532_pata_data_xfer() so it reads and writes 4-byte blocks,
>> like the original driver did. Rename the offset definition of the
>> buffered data register for clearness.
>>   
>
>   Looks ike I'll have to NAK this part...

   I'd advise to break up the patch since you're fixing 2 unrelated 
things now...

>> index f8b3ffc..bdf413e 100644
>> --- a/drivers/ata/pata_rb532_cf.c
>> +++ b/drivers/ata/pata_rb532_cf.c
>>   
> diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
> [...]
>>  <at>  <at>  -39,9 +40,11  <at>  <at> 
>>  #define RB500_CF_MAXPORTS    1
>>  #define RB500_CF_IO_DELAY    400
>>  
>> -#define RB500_CF_REG_CMD    0x0800
(Continue reading)

Permalink | Reply | 0 comments |
headers
Phil Sutter | 1 Nov 2008 19:09

[PATCH] fix pata-rb532-cf

After applying the following changes I could verify functionality by
mounting a filesystem on the cfdisk and reading/writing files in it.

The symbols rb532_gpio_set_ilevel and rb532_gpio_set_istat are not yet
available in a vanilla kernel, an appropriate patch has already been
sent to the linux-mips mailinglist.

Signed-off-by: Phil Sutter <n0-1 <at> freewrt.org>
---
 drivers/ata/pata_rb532_cf.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index f8b3ffc..7b11f40 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c
 <at>  <at>  -31,6 +31,7  <at>  <at> 
 #include <scsi/scsi_host.h>

 #include <asm/gpio.h>
+#include <asm/mach-rc32434/gpio.h>

 #define DRV_NAME	"pata-rb532-cf"
 #define DRV_VERSION	"0.1.0"
 <at>  <at>  -39,7 +40,8  <at>  <at> 
 #define RB500_CF_MAXPORTS	1
 #define RB500_CF_IO_DELAY	400

-#define RB500_CF_REG_CMD	0x0800
+#define RB500_CF_REG_BASE	0x0800
(Continue reading)

Permalink | Reply | 11 comments |
headers
Phil Sutter | 1 Nov 2008 19:11

[PATCH] read and write data in 4-byte blocks

* rename the offset definition to avoid abiguity with the standard ATA
  IO address
* read and write four bytes at once
* use writesl() and readsl() which implicitly iterate over the data

Signed-off-by: Phil Sutter <n0-1 <at> freewrt.org>
---
 drivers/ata/pata_rb532_cf.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index 7b11f40..0ae377f 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c
 <at>  <at>  -43,7 +43,8  <at>  <at> 
 #define RB500_CF_REG_BASE	0x0800
 #define RB500_CF_REG_ERR	0x080D
 #define RB500_CF_REG_CTRL	0x080E
-#define RB500_CF_REG_DATA	0x0C00
+/* 32bit buffered data register offset */
+#define RB500_CF_REG_DBUF32	0x0C00

 struct rb532_cf_info {
 	void __iomem	*iobase;
 <at>  <at>  -80,15 +81,13  <at>  <at>  static void rb532_pata_data_xfer(struct ata_device *adev, unsigned char *buf,
 	struct ata_port *ap = adev->link->ap;
 	void __iomem *ioaddr = ap->ioaddr.data_addr;

-	if (write_data) {
-		for (; buflen > 0; buflen--, buf++)
(Continue reading)

Permalink | Reply | 8 comments |
headers
Phil Sutter | 1 Nov 2008 19:21

[PATCH] read and write data in 4-byte blocks

I forgot to include the one line changing rb532_pata_data_xfer()'s signature,
sorry for that. So here's the correct version.

* rename the offset definition to avoid abiguity with the standard ATA
  IO address
* read and write four bytes at once
* use writesl() and readsl() which implicitly iterate over the data
* fix the signature of rb532_pata_data_xfer() to mach the function
  pointer definition and return the number of bytes consumed

Signed-off-by: Phil Sutter <n0-1 <at> freewrt.org>
---
 drivers/ata/pata_rb532_cf.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
index 7b11f40..e265577 100644
--- a/drivers/ata/pata_rb532_cf.c
+++ b/drivers/ata/pata_rb532_cf.c
 <at>  <at>  -43,7 +43,8  <at>  <at> 
 #define RB500_CF_REG_BASE	0x0800
 #define RB500_CF_REG_ERR	0x080D
 #define RB500_CF_REG_CTRL	0x080E
-#define RB500_CF_REG_DATA	0x0C00
+/* 32bit buffered data register offset */
+#define RB500_CF_REG_DBUF32	0x0C00

 struct rb532_cf_info {
 	void __iomem	*iobase;
 <at>  <at>  -74,21 +75,19  <at>  <at>  static void rb532_pata_exec_command(struct ata_port *ap,
(Continue reading)

Permalink | Reply | 7 comments |
headers
Borislav Petkov | 1 Nov 2008 21:14

Re: [RFC PATCH] ide-floppy partitions

On Wed, Oct 29, 2008 at 08:13:19AM +0100, Borislav Petkov wrote:
> Hi Tejun,
> 
> recent changes at 0762b8bde9729f10f8e6249809660ff2ec3ad735 and around
> break ide-floppy. Since it is a removable media drive and the partition
> scan during boot returns empty (no media in the drive), when you later
> put in a disk and try to mount it, mount returns saying
> 
> /dev/hdc4 is not a valid block device.
> 
> Which brings me to the other possible issue: Since having a hdc4
> partition as a single FAT16 partition on a ZIP drive is the "factory
> default" you could fabricate a case where you have a partition number> 1
> as the only partition on a hard drive too, i.e. no continuous
> partition numbering and the mount would theoretically fail there too
> since, for example, there's a check in disk_get_part() which does:
> 
> if (likely(partno < ptbl->len)) {
> 
> and in this case the check will fail if partno >= 1 while you have only
> one partition on the disk with a number higher than the partition table
> length and the above described failure will happen too. Anyways, this is
> just a hypothesis, but it happens with the ZIP drive here so other block
> devices should behave similarly.
> 
> Here's a patch that fixes the ide-floppy case a bit clumsily, I admit.
> 
> ---
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 88a776f..b798ea0 100644
(Continue reading)

Permalink | Reply | 10 comments |

Gmane