public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* Fw: ACARD SCSI driver update for Linux kernel v2.6
@ 2004-12-30  9:58 jameshsu
  2004-12-30 15:20 ` James Bottomley
  2005-01-06 23:10 ` Alan Cox
  0 siblings, 2 replies; 10+ messages in thread
From: jameshsu @ 2004-12-30  9:58 UTC (permalink / raw)
  To: alan; +Cc: linux-scsi

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="big5", Size: 2617 bytes --]

Hi Alan,

Thanks for your fast response.  Still need to bother you few minutes for
following questions.
1) Do you think this is necessary to forward this email to James Bottomley??
(Don't know his email & how can I submit??)
2) A set of patch files attached with this email. Should this be good enough
for process??

Methodology to use patch file with current kernel:
1.tgz & source code
2.cp -af linux-2.6.10 linux-2.6.10.og
3.cp atp870u-new-patch linux-2.6.10.og
4.cd linux-2.6.10.og
5.patch -p1 < atp870u-new-patch

Files modified after patch:
drivers/pci/pci.ids
drivers/scsi/atp870u.c
drivers/scsi/atp870u.h
drivers/scsi/Kconfig
include/linux/pci_ids.h

3) I send this email attached with patch file to Linux-SCSI associations as
well as to you because no idea about the submit process. Please let me know
if anything is missing.

Happy holiday season and best regards,

James Hsu,
Manager, Engineering Testing Dept.
ACARD TECHNOLOGY CORP.
Taipei Hsien,Taiwan
6F No78,Sec 1,Kwang Fu Road,Sang Chung,
(02)8512-2290 x 2311(O),(02)8512-1524(FAX)

----- Original Message -----
From: "Alan Cox" <alan@lxorguk.ukuu.org.uk>
To: "jameshsu" <jameshsu@mail.acard.com>
Cc: <arjanv@redhat.com>; "JimYeh(?‰ä?毅\)" <jimyeh98@acard.com>
Sent: Tuesday, December 21, 2004 11:56 PM
Subject: Re: ACARD SCSI driver update for Linux kernel v2.6


> On Maw, 2004-12-21 at 06:27, jameshsu wrote:
> > Dear Arjan and Alan,
> >
> > My name is James Hsu from Acard Technology, http://www.acard.com.
> > Our company does design and manufacture for SCSI IC & HBA product.
> > Under Linux system for kernel v2.4 or v2.6, you could find our product
> > module ACARD 870U/W and driver ATP870U.o
>
> Yes I helped members of your company when they kindly submitted the
> original driver and I fixed several bugs in it as well as doing the
> initial 2.6 port of the driver.
>
> > The reason to write you this email is to figure out the way how to
> > update our driver for the latest Linux kernel released by association.
> > If you have the time, please let us know the procedure or show me the
> > direction. I appreciate for your kindly assistance.
>
> The preferred procedure is to generate a set of patch files that contain
> the changes you want to make along with explanations of what they do.
> These can then be reviewed and submitted to James Bottomley the SCSI
> layer maintainer for incorporation into the kernel proper.
>
> I'll be happy to assist you in reviewing changes although as it is
> Christmas here things will be slow until early January.
>
> Alan
>


[-- Attachment #2: atp870u-new-patch.tgz --]
[-- Type: application/octet-stream, Size: 24917 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2004-12-30  9:58 Fw: ACARD SCSI driver update for Linux kernel v2.6 jameshsu
@ 2004-12-30 15:20 ` James Bottomley
  2005-01-06 23:10 ` Alan Cox
  1 sibling, 0 replies; 10+ messages in thread
From: James Bottomley @ 2004-12-30 15:20 UTC (permalink / raw)
  To: jameshsu; +Cc: Alan Cox, SCSI Mailing List

On Thu, 2004-12-30 at 17:58 +0800, jameshsu wrote:
> 1) Do you think this is necessary to forward this email to James Bottomley??
> (Don't know his email & how can I submit??)

The SCSI list is fine.  To find the emails of most maintainers, look in
the MAINTAINERS file in the kernel source.

> 2) A set of patch files attached with this email. Should this be good enough
> for process??

Not really.  Your patch is huge.  I'd like to see a set of incremental
patches each separately described adding individual features.
(Documentation/SubmittingPatches is a good place to start).

Some of the things in the current patch are obviously wrong:

+++ linux-2.6.10/drivers/scsi/atp870u.c	2004-12-29 15:13:55.000000000 -0500
@@ -1,21 +1,36 @@
-/*
- *  Copyright (C) 1997	Wu Ching Chen
- *  2.1.x update (C) 1998  Krzysztof G. Baranowski
- *  2.5.x update (C) 2002  Red Hat <alan@redhat.com>
- *  2.6.x update (C) 2004  Red Hat <alan@redhat.com>
+/* $Id: atp870u.c,v 2.0 2004/12/24 15:42:00 root Exp root $
  *
- * Marcelo Tosatti <marcelo@conectiva.com.br> : SMP fixes
- *

You can't remove people's copyrights from the file.  Even if you
originally owned it, several people have put effort into modifying the
file to make sure it still compiles and runs, the copyright marks their
ownership of the modifications they made.

-#include <scsi/scsi.h>
-#include <scsi/scsi_cmnd.h>
-#include <scsi/scsi_device.h>
-#include <scsi/scsi_host.h>
[...]
+#include "scsi.h"
+#include "hosts.h"

This is reversing the update to the new SCSI headers.  The old headers
are now deprecated, so this type of update isn't acceptable.

+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,5,0)
+static irqreturn_t atp885_intr_handle(int irq, void *dev_id, struct
pt_regs *regs)
+#else
+static void atp885_intr_handle(int irq, void *dev_id, struct pt_regs
*regs)
+#endif

Please don't do this type of thing.  the atp870u driver is nice at the
moment because it has no kernel version dependencies ... I'd really
rather it didn't get cluttered up with them.

+static struct Scsi_Host *atp_host[MAX_ADAPTER];
+static int host_index = 0;

This also isn't acceptable ... Zwane Mwaikambo did a lot of work early
this year breaking this driver's dependence on a static list and fixing
a whole host of other problems it had.  Static lists in drivers are bad,
so I don't want to see this reversed.

In short, it rather looks like you've been building a driver out of tree
for a while and have simply constructed diffs between that driver and
what's currently in the tree (the demolish and rebuild approach).
However, people have spent time correcting faults in the in-tree driver
which you haven't done for the out of tree one.  So please go back and
see what's been done in the tree that you can take advantage of.  Or,
start from the in-tree driver and work out what extra features need to
be added and what bugs need to be fixed and supply separate patches for
doing that.

James





^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2004-12-30  9:58 Fw: ACARD SCSI driver update for Linux kernel v2.6 jameshsu
  2004-12-30 15:20 ` James Bottomley
@ 2005-01-06 23:10 ` Alan Cox
  2005-01-07 10:05   ` jameshsu
  1 sibling, 1 reply; 10+ messages in thread
From: Alan Cox @ 2005-01-06 23:10 UTC (permalink / raw)
  To: jameshsu; +Cc: linux-scsi

The trouble with the patch as is, is that it mixes up

- Reversals of improvements like hotplug pci made outside Acard
- Addition of support for the AEC67160
- Other unknown improvements Acard may have made

In order for this to be useful we need to be able to see just the
changes that are important.

I can easily see for example the new PCI identifiers but not the other
needed changes. In order to do that I really need either a description
of the relevant changes or those changes versus the existing 2.6 driver
tree.

Any help welcomed.

Alan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2005-01-06 23:10 ` Alan Cox
@ 2005-01-07 10:05   ` jameshsu
  2005-01-07 10:11     ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: jameshsu @ 2005-01-07 10:05 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-scsi, James.Bottomley, EdwardLian, Jason Wu

Hi Alan,

We are in the process to modify the driver in order to meet your suggests.
Following change has been made.
1) Put it back for originator's copyrights to the file.
2) Go back to build up/modify driver from kernel 2.6.10. according to James
Bottomley's suggestions
3) use new header
     <scsi/scsi.h>
    <scsi/scsi_host.h>
    <scsi/scsi_device.h>
    <scsi/scsi_cmnd.h>)
4) enable .max_sectors = 128
5) Either  modify or remove all following declare:
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2,5,0)
+static irqreturn_t atp885_intr_handle(int irq, void *dev_id, struct
pt_regs *regs)
+#else
+static void atp885_intr_handle(int irq, void *dev_id, struct pt_regs
*regs)
+#endif

+static struct Scsi_Host *atp_host[MAX_ADAPTER];
+static int host_index = 0;

6) Reversals of some of improvements like hotplug pci made outside -
Addition of support for the AEC67160Acard
7) more.....
New driver will be submit around 1-2 week later.
During these period, if you have more suggestions, please let us know ASAP
while testing.
Again, Thanks for your help!

James Hsu,
Manager,Engineering Testing Dept.
ACARD TECHNOLOGY CORP.
Taipei Hsien,Taiwan
6F No78,Sec 1,Kwang Fu Road,Sang Chung,
(02)8512-2290 x 2311(O),(02)8512-1524(FAX)


----- Original Message -----
From: "Alan Cox" <alan@lxorguk.ukuu.org.uk>
To: "jameshsu" <jameshsu@mail.acard.com>
Cc: <linux-scsi@vger.kernel.org>
Sent: Friday, January 07, 2005 7:10 AM
Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6


> The trouble with the patch as is, is that it mixes up
>
> - Reversals of improvements like hotplug pci made outside Acard
> - Addition of support for the AEC67160
> - Other unknown improvements Acard may have made
>
> In order for this to be useful we need to be able to see just the
> changes that are important.
>
> I can easily see for example the new PCI identifiers but not the other
> needed changes. In order to do that I really need either a description
> of the relevant changes or those changes versus the existing 2.6 driver
> tree.
>
> Any help welcomed.
>
> Alan
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2005-01-07 10:05   ` jameshsu
@ 2005-01-07 10:11     ` Christoph Hellwig
  0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2005-01-07 10:11 UTC (permalink / raw)
  To: jameshsu; +Cc: Alan Cox, linux-scsi, James.Bottomley, EdwardLian, Jason Wu

> New driver will be submit around 1-2 week later.
> During these period, if you have more suggestions, please let us know ASAP
> while testing.

In general life for us and you is easier if you submit changes as you
make them in small chunks.  E.g. submit the new PCI ID asap, submit
small fixes you think are needed in the driver ASAP, etc..

Small patches are much easier review, and and you get obviously correct
pieces out ASAP.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
@ 2005-01-14 10:03 jameshsu
  2005-01-14 17:44 ` Matthew Wilcox
  0 siblings, 1 reply; 10+ messages in thread
From: jameshsu @ 2005-01-14 10:03 UTC (permalink / raw)
  To: Alan Cox, James.Bottomley, Christoph Hellwig
  Cc: Jason Wu, EdwardLian, linux-scsi

[-- Attachment #1: Type: text/plain, Size: 3495 bytes --]

Hi all,

We, Acard, total understand what you suggest. (including advise from
Christoph, James B. and Alan.)
Therefore, we modify the internal code to create this patch file from kernel
v2.6.10.
However, as you see this is the smallest patch files which could be
generated. (Sorry, unable to meet your expetation)
The reason is : this driver could supports ATP870 as well as mutiples of
Acard's chipsets. (this includes ATP880(AEC67160) & ATP885(AEC67162)). This
is why the patch files is huge than what you expected.
We did not make any change for ATP870(AEC6712) on this rev.. Besides, we
reverse following codes according to all of your suggestions. Please review
and let me know if this is feasible.  Welcome to any of question you may
have.
Thanks for your help!

Best regards,

Manager,Engineering Testing Dept.
ACARD TECHNOLOGY CORP.
Taipei Hsien,Taiwan
6F No78,Sec 1,Kwang Fu Road,Sang Chung,
(02)8512-2290 x 2311(O),(02)8512-1524(FAX)

----- Original Message -----
From: "jameshsu" <jameshsu@mail.acard.com>
To: "Alan Cox" <alan@lxorguk.ukuu.org.uk>
Cc: <linux-scsi@vger.kernel.org>; <James.Bottomley@SteelEye.com>;
"EdwardLian" <edwardlian@mail.acard.com>; "Jason Wu" <jason_wu@acard.com>
Sent: Friday, January 07, 2005 6:05 PM
Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6


> Hi Alan,
>
> We are in the process to modify the driver in order to meet your suggests.
> Following change has been made.
> 1) Put it back for originator's copyrights to the file.
> 2) Go back to build up/modify driver from kernel 2.6.10. according to
James
> Bottomley's suggestions
> 3) use new header
>      <scsi/scsi.h>
>     <scsi/scsi_host.h>
>     <scsi/scsi_device.h>
>     <scsi/scsi_cmnd.h>)
> 4) enable .max_sectors = 128
> 5) Either  modify or remove all following declare:
> +#if LINUX_VERSION_CODE > KERNEL_VERSION(2,5,0)
> +static irqreturn_t atp885_intr_handle(int irq, void *dev_id, struct
> pt_regs *regs)
> +#else
> +static void atp885_intr_handle(int irq, void *dev_id, struct pt_regs
> *regs)
> +#endif
>
> +static struct Scsi_Host *atp_host[MAX_ADAPTER];
> +static int host_index = 0;
>
> 6) Reversals of some of improvements like hotplug pci made outside -
> Addition of support for the AEC67160Acard
> 7) more.....
> New driver will be submit around 1-2 week later.
> During these period, if you have more suggestions, please let us know ASAP
> while testing.
> Again, Thanks for your help!
>
> James Hsu,
> Manager,Engineering Testing Dept.
> ACARD TECHNOLOGY CORP.
> Taipei Hsien,Taiwan
> 6F No78,Sec 1,Kwang Fu Road,Sang Chung,
> (02)8512-2290 x 2311(O),(02)8512-1524(FAX)
>
>
> ----- Original Message -----
> From: "Alan Cox" <alan@lxorguk.ukuu.org.uk>
> To: "jameshsu" <jameshsu@mail.acard.com>
> Cc: <linux-scsi@vger.kernel.org>
> Sent: Friday, January 07, 2005 7:10 AM
> Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
>
>
> > The trouble with the patch as is, is that it mixes up
> >
> > - Reversals of improvements like hotplug pci made outside Acard
> > - Addition of support for the AEC67160
> > - Other unknown improvements Acard may have made
> >
> > In order for this to be useful we need to be able to see just the
> > changes that are important.
> >
> > I can easily see for example the new PCI identifiers but not the other
> > needed changes. In order to do that I really need either a description
> > of the relevant changes or those changes versus the existing 2.6 driver
> > tree.
> >
> > Any help welcomed.
> >
> > Alan
> >
>

[-- Attachment #2: Re_ Fw_ ACARD SCSI driver update for Linux kernel v2.6.eml --]
[-- Type: message/rfc822, Size: 2028 bytes --]

From: Christoph Hellwig <hch@infradead.org>
To: jameshsu <jameshsu@mail.acard.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, linux-scsi@vger.kernel.org, James.Bottomley@SteelEye.com, EdwardLian <edwardlian@mail.acard.com>, Jason Wu <jason_wu@acard.com>
Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
Date: Fri, 7 Jan 2005 10:11:44 +0000
Message-ID: <20050107101144.GA6094@infradead.org>

> New driver will be submit around 1-2 week later.
> During these period, if you have more suggestions, please let us know ASAP
> while testing.

In general life for us and you is easier if you submit changes as you
make them in small chunks.  E.g. submit the new PCI ID asap, submit
small fixes you think are needed in the driver ASAP, etc..

Small patches are much easier review, and and you get obviously correct
pieces out ASAP.

[-- Attachment #3: atp870u-new-patch.tgz --]
[-- Type: application/octet-stream, Size: 18493 bytes --]

[-- Attachment #4: Change_histry.txt --]
[-- Type: text/plain, Size: 375 bytes --]

1.Add pci device id AEC67162   0x808a
		    AEC67160   0x8080
		    AEC67160S  0x8081

2.Add some structure in atp_unit,because AEC67162 was dual channel LVD160 SCSI adapter.

3.Add some function about AEC67162, is885() tscam_885().

4.In probe function,interrupt function,queuecommand function,send_s870 function
  ,add some access register action about AEC67162.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2005-01-14 10:03 jameshsu
@ 2005-01-14 17:44 ` Matthew Wilcox
  2005-01-19  2:05   ` jameshsu
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2005-01-14 17:44 UTC (permalink / raw)
  To: jameshsu
  Cc: Alan Cox, James.Bottomley, Christoph Hellwig, Jason Wu,
	EdwardLian, linux-scsi

On Fri, Jan 14, 2005 at 06:03:02PM +0800, jameshsu wrote:
> We, Acard, total understand what you suggest. (including advise from
> Christoph, James B. and Alan.)
> Therefore, we modify the internal code to create this patch file from kernel
> v2.6.10.
> However, as you see this is the smallest patch files which could be
> generated. (Sorry, unable to meet your expetation)
> The reason is : this driver could supports ATP870 as well as mutiples of
> Acard's chipsets. (this includes ATP880(AEC67160) & ATP885(AEC67162)). This
> is why the patch files is huge than what you expected.
> We did not make any change for ATP870(AEC6712) on this rev.. Besides, we
> reverse following codes according to all of your suggestions. Please review
> and let me know if this is feasible.  Welcome to any of question you may
> have.
> Thanks for your help!

The change to pci.ids needs to be handled through pciids.sourceforge.net

----

I really don't like the:
+               atp_dev.ioport[0] = base_io + 0x80;
+               atp_dev.ioport[1] = base_io + 0xc0;

Just record base_io in your atp_dev.  I don't like the way this patch
turns everything into a 2-d array ... surely there has to be a better
way to do it than this?

----

+               if (dev->dev_id == ATP885_DEVID) {
+                       tmpcip += 2;
+                       outb(0x06, tmpcip);
+                       tmpcip -= 2;

would be much better written as:

		if (dev->dev_id == ATP885_DEVID)
			outb(0x06, tmpcip+2);

(hmm, this seems to be common coding style throughout the driver ...)

----

-       if (pci_set_dma_mask(dev, 0xFFFFFFFFUL)) {
-               printk(KERN_ERR "atp870u: 32bit DMA mask required but not availa
ble.\n");
-               return -EIO;
-       }
+        if (!pci_set_dma_mask(pdev, 0xFFFFFFUL)) {
+                printk(KERN_INFO "atp870u: use 32bit DMA mask.\n");
+        } else {
+                printk(KERN_ERR "atp870u: DMA mask required but not available.\
n");
+                return -EIO;
+        }

You've actually set a 24-bit DMA mask there.  Should use the symbolic
constant DMA_32BIT_MASK anyway.

----

+       { PCI_DEVICE(PCI_VENDOR_ID_ARTOP, ATP885_DEVID)                   },

Please use the PCI_DEVICE_ID_ name instead

----



-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2005-01-14 17:44 ` Matthew Wilcox
@ 2005-01-19  2:05   ` jameshsu
  0 siblings, 0 replies; 10+ messages in thread
From: jameshsu @ 2005-01-19  2:05 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Alan Cox, James.Bottomley, Christoph Hellwig, Jason Wu,
	EdwardLian, linux-scsi

Hi all,

We are in the process to make some change to meet your expectation ASAP.
(e.g. (1) 2-d array => 1-d array (2) meet common coding style (3) 32-bit DMA
mask instead (4) PCI new device entry applied (5) use the PCI_DEVICE_ID_
name instead)
However, this is the revision we are sure it's working.
Therefore, if this is possible, can you patch this file for v2.6.10 this
time.
Within 1-2 week from now, we will submit another patch file to fix above
syntax issues if working smoothly.
In the mean time, we also in the process to submit the new device entry
(ATP880[AEC67160] LVD160 1 channel  /ATP885[AEC67162] LVD160 2 channel)
under Artop Electronics Corp.
Thanks for your help!

Best regards,

James
----- Original Message -----
From: "Matthew Wilcox" <matthew@wil.cx>
To: "jameshsu" <jameshsu@mail.acard.com>
Cc: "Alan Cox" <alan@lxorguk.ukuu.org.uk>; <James.Bottomley@SteelEye.com>;
"Christoph Hellwig" <hch@infradead.org>; "Jason Wu" <jason_wu@acard.com>;
"EdwardLian" <edwardlian@mail.acard.com>; <linux-scsi@vger.kernel.org>
Sent: Saturday, January 15, 2005 1:44 AM
Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6


> On Fri, Jan 14, 2005 at 06:03:02PM +0800, jameshsu wrote:
> > We, Acard, total understand what you suggest. (including advise from
> > Christoph, James B. and Alan.)
> > Therefore, we modify the internal code to create this patch file from
kernel
> > v2.6.10.
> > However, as you see this is the smallest patch files which could be
> > generated. (Sorry, unable to meet your expetation)
> > The reason is : this driver could supports ATP870 as well as mutiples of
> > Acard's chipsets. (this includes ATP880(AEC67160) & ATP885(AEC67162)).
This
> > is why the patch files is huge than what you expected.
> > We did not make any change for ATP870(AEC6712) on this rev.. Besides, we
> > reverse following codes according to all of your suggestions. Please
review
> > and let me know if this is feasible.  Welcome to any of question you may
> > have.
> > Thanks for your help!
>
> The change to pci.ids needs to be handled through pciids.sourceforge.net
>
> ----
>
> I really don't like the:
> +               atp_dev.ioport[0] = base_io + 0x80;
> +               atp_dev.ioport[1] = base_io + 0xc0;
>
> Just record base_io in your atp_dev.  I don't like the way this patch
> turns everything into a 2-d array ... surely there has to be a better
> way to do it than this?
>
> ----
>
> +               if (dev->dev_id == ATP885_DEVID) {
> +                       tmpcip += 2;
> +                       outb(0x06, tmpcip);
> +                       tmpcip -= 2;
>
> would be much better written as:
>
> if (dev->dev_id == ATP885_DEVID)
> outb(0x06, tmpcip+2);
>
> (hmm, this seems to be common coding style throughout the driver ...)
>
> ----
>
> -       if (pci_set_dma_mask(dev, 0xFFFFFFFFUL)) {
> -               printk(KERN_ERR "atp870u: 32bit DMA mask required but not
availa
> ble.\n");
> -               return -EIO;
> -       }
> +        if (!pci_set_dma_mask(pdev, 0xFFFFFFUL)) {
> +                printk(KERN_INFO "atp870u: use 32bit DMA mask.\n");
> +        } else {
> +                printk(KERN_ERR "atp870u: DMA mask required but not
available.\
> n");
> +                return -EIO;
> +        }
>
> You've actually set a 24-bit DMA mask there.  Should use the symbolic
> constant DMA_32BIT_MASK anyway.
>
> ----
>
> +       { PCI_DEVICE(PCI_VENDOR_ID_ARTOP,
              },
>
> Please use the PCI_DEVICE_ID_ name instead
>
> ----
>
>
>
> --
> "Next the statesmen will invent cheap lies, putting the blame upon
> the nation that is attacked, and every man will be glad of those
> conscience-soothing falsities, and will diligently study them, and refuse
> to examine any refutations of them; and thus he will by and by convince
> himself that the war is just, and will thank God for the better sleep
> he enjoys after this process of grotesque self-deception." -- Mark Twain
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
@ 2005-03-30 11:48 jameshsu
  2005-03-30 12:45 ` Alan Cox
  0 siblings, 1 reply; 10+ messages in thread
From: jameshsu @ 2005-03-30 11:48 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Alan Cox, James.Bottomley, Christoph Hellwig, linux-scsi,
	EdwardLian, Jason Wu, Hill Wu

Hello Matthew/Allen,

Few weeks  pass since we sent update driver for you to build-in in Jan 2005.
Latest,  we test Linux kernel rev 2.6.11 and found the SCSI driver not
update yet.
Is it possible to let us know anything Acard are still missing or any rules
not follow??!!
When 2.6.12 will be released?? Should we wait and submit the patch files for
v2.6.12??
Back to Jan, we promise you we will have new driver to release.
We are ready now for v2.6.10. However we would like to know if anything else
needed to be updated or modified. Therefore we will know what is next step
Acard should go.
Thanks for you help!

Best regard,

James Hsu,
Manager,Engineering Testing Dept.
ACARD TECHNOLOGY CORP. http://www.acard.com.tw
Taipei Hsien,Taiwan
6F No78,Sec 1,Kwang Fu Road,Sang Chung,
(02)8512-2290 x 2311(O),(02)8512-1524(FAX)
E-mail:jameshsu@acard.com

----- Original Message -----
From: "jameshsu" <jameshsu@mail.acard.com>
To: "Matthew Wilcox" <matthew@wil.cx>
Cc: "Alan Cox" <alan@lxorguk.ukuu.org.uk>; <James.Bottomley@SteelEye.com>;
"Christoph Hellwig" <hch@infradead.org>; "Jason Wu" <jason_wu@acard.com>;
"EdwardLian" <edwardlian@mail.acard.com>; <linux-scsi@vger.kernel.org>
Sent: Wednesday, January 19, 2005 10:05 AM
Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6


> Hi all,
>
> We are in the process to make some change to meet your expectation ASAP.
> (e.g. (1) 2-d array => 1-d array (2) meet common coding style (3) 32-bit
DMA
> mask instead (4) PCI new device entry applied (5) use the PCI_DEVICE_ID_
> name instead)
> However, this is the revision we are sure it's working.
> Therefore, if this is possible, can you patch this file for v2.6.10 this
> time.
> Within 1-2 week from now, we will submit another patch file to fix above
> syntax issues if working smoothly.
> In the mean time, we also in the process to submit the new device entry
> (ATP880[AEC67160] LVD160 1 channel  /ATP885[AEC67162] LVD160 2 channel)
> under Artop Electronics Corp.
> Thanks for your help!
>
> Best regards,
>
> James
> ----- Original Message -----
> From: "Matthew Wilcox" <matthew@wil.cx>
> To: "jameshsu" <jameshsu@mail.acard.com>
> Cc: "Alan Cox" <alan@lxorguk.ukuu.org.uk>; <James.Bottomley@SteelEye.com>;
> "Christoph Hellwig" <hch@infradead.org>; "Jason Wu" <jason_wu@acard.com>;
> "EdwardLian" <edwardlian@mail.acard.com>; <linux-scsi@vger.kernel.org>
> Sent: Saturday, January 15, 2005 1:44 AM
> Subject: Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
>
>
> > On Fri, Jan 14, 2005 at 06:03:02PM +0800, jameshsu wrote:
> > > We, Acard, total understand what you suggest. (including advise from
> > > Christoph, James B. and Alan.)
> > > Therefore, we modify the internal code to create this patch file from
> kernel
> > > v2.6.10.
> > > However, as you see this is the smallest patch files which could be
> > > generated. (Sorry, unable to meet your expetation)
> > > The reason is : this driver could supports ATP870 as well as mutiples
of
> > > Acard's chipsets. (this includes ATP880(AEC67160) & ATP885(AEC67162)).
> This
> > > is why the patch files is huge than what you expected.
> > > We did not make any change for ATP870(AEC6712) on this rev.. Besides,
we
> > > reverse following codes according to all of your suggestions. Please
> review
> > > and let me know if this is feasible.  Welcome to any of question you
may
> > > have.
> > > Thanks for your help!
> >
> > The change to pci.ids needs to be handled through pciids.sourceforge.net
> >
> > ----
> >
> > I really don't like the:
> > +               atp_dev.ioport[0] = base_io + 0x80;
> > +               atp_dev.ioport[1] = base_io + 0xc0;
> >
> > Just record base_io in your atp_dev.  I don't like the way this patch
> > turns everything into a 2-d array ... surely there has to be a better
> > way to do it than this?
> >
> > ----
> >
> > +               if (dev->dev_id == ATP885_DEVID) {
> > +                       tmpcip += 2;
> > +                       outb(0x06, tmpcip);
> > +                       tmpcip -= 2;
> >
> > would be much better written as:
> >
> > if (dev->dev_id == ATP885_DEVID)
> > outb(0x06, tmpcip+2);
> >
> > (hmm, this seems to be common coding style throughout the driver ...)
> >
> > ----
> >
> > -       if (pci_set_dma_mask(dev, 0xFFFFFFFFUL)) {
> > -               printk(KERN_ERR "atp870u: 32bit DMA mask required but
not
> availa
> > ble.\n");
> > -               return -EIO;
> > -       }
> > +        if (!pci_set_dma_mask(pdev, 0xFFFFFFUL)) {
> > +                printk(KERN_INFO "atp870u: use 32bit DMA mask.\n");
> > +        } else {
> > +                printk(KERN_ERR "atp870u: DMA mask required but not
> available.\
> > n");
> > +                return -EIO;
> > +        }
> >
> > You've actually set a 24-bit DMA mask there.  Should use the symbolic
> > constant DMA_32BIT_MASK anyway.
> >
> > ----
> >
> > +       { PCI_DEVICE(PCI_VENDOR_ID_ARTOP,
>               },
> >
> > Please use the PCI_DEVICE_ID_ name instead
> >
> > ----
> >
> >
> >
> > --
> > "Next the statesmen will invent cheap lies, putting the blame upon
> > the nation that is attacked, and every man will be glad of those
> > conscience-soothing falsities, and will diligently study them, and
refuse
> > to examine any refutations of them; and thus he will by and by convince
> > himself that the war is just, and will thank God for the better sleep
> > he enjoys after this process of grotesque self-deception." -- Mark Twain
> >
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Fw: ACARD SCSI driver update for Linux kernel v2.6
  2005-03-30 11:48 jameshsu
@ 2005-03-30 12:45 ` Alan Cox
  0 siblings, 0 replies; 10+ messages in thread
From: Alan Cox @ 2005-03-30 12:45 UTC (permalink / raw)
  To: jameshsu
  Cc: Matthew Wilcox, James Bottomley, Christoph Hellwig, linux-scsi,
	EdwardLian, Jason Wu, Hill Wu

On Mer, 2005-03-30 at 12:48, jameshsu wrote:
> Hello Matthew/Allen,
> 
> Few weeks  pass since we sent update driver for you to build-in in Jan 2005.
> Latest,  we test Linux kernel rev 2.6.11 and found the SCSI driver not
> update yet.

It is in 2.6.12 snapshots.

> Is it possible to let us know anything Acard are still missing or any rules
> not follow??!!

It took a long time because your changes removed other fixes made over
time and it took a lot of work to merge them back together

Alan


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2005-03-30 12:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-12-30  9:58 Fw: ACARD SCSI driver update for Linux kernel v2.6 jameshsu
2004-12-30 15:20 ` James Bottomley
2005-01-06 23:10 ` Alan Cox
2005-01-07 10:05   ` jameshsu
2005-01-07 10:11     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2005-01-14 10:03 jameshsu
2005-01-14 17:44 ` Matthew Wilcox
2005-01-19  2:05   ` jameshsu
2005-03-30 11:48 jameshsu
2005-03-30 12:45 ` Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox