2014-04-26 17:21:11

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure

On Wed, 2014-03-19 at 23:35 +1100, Finn Thain wrote:
> This patch series addresses several issues with NCR5380 drivers:
>
> 1. The complex network of #include directives.
>
> 2. Three inconsistent implementations of the core driver all attempting
> to share the same macro definitions in NCR5380.h.
>
> 3. Broken debugging code.
>
> In the past these issues have led to compiler warnings and ugly hacks to
> fix build failures.
>
> This patch series fixes the debugging code by reducing the divergence
> between the various core driver implementations.
>
> The final two patches in the series further reduce divergence by refactoring
> sun3_scsi.c and sun3_scsi_vme.c so that they follow the same structure as
> the other NCR5380 drivers.
>
> By the end of this patch series over 800 net lines of code have been
> removed. This is mostly duplicated code that's easily eliminated once the
> debugging code is made consistent (and some dead code is removed).
>
> Better uniformity and less duplication should assist future work such as
> modernization and trivial clean-up.
>
> To make code review easier I've tried to keep these patches succinct and
> free of extraneous changes. Though I did run checkpatch.pl, I've ignored
> whitespace issues in existing code. I will send separate patches for
> whitespace clean-up of NCR5380 drivers.
>
> All NCR5380 drivers have been compile-tested with this patch series:
> arm/cumana_1.c
> arm/oak.c
> atari_scsi.c
> dmx3191d.c
> dtc.c
> g_NCR5380.c
> g_NCR5380_mmio.c
> mac_scsi.c
> pas16.c
> sun3_scsi.c
> sun3_scsi_vme.c
> t128.c
>
> I've successfully regression tested this patch series using mac_scsi on a
> PowerBook 180. The debugging macros are now usable again.

OK, so this is a pretty big change to an unmaintained driver. I'll take
it if you're willing to maintain the driver afterwards ... in which case
I need another patch to add you to the MAINTAINERS file.

James


2014-04-29 02:22:40

by Finn Thain

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure


On Sat, 26 Apr 2014, James Bottomley wrote:

> OK, so this is a pretty big change to an unmaintained driver. I'll take
> it if you're willing to maintain the driver afterwards ... in which case
> I need another patch to add you to the MAINTAINERS file.

Sure, I'm happy to support these patches and future work I plan to do on
the driver.

What additional responsibilities would come with adding my name the
MAINTAINERS file?

Perhaps Michael and Sam would be interested in sharing the role, for atari
and sun3 NCR5380 drivers (?)

I only have hardware to test mac_scsi but I can obtain a Domex DMX3191D
PCI card on ebay.

--

2014-04-29 03:15:10

by Michael Schmitz

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure

Finn,

On Tue, Apr 29, 2014 at 2:22 PM, Finn Thain <[email protected]> wrote:
>
> On Sat, 26 Apr 2014, James Bottomley wrote:
>
>> OK, so this is a pretty big change to an unmaintained driver. I'll take
>> it if you're willing to maintain the driver afterwards ... in which case
>> I need another patch to add you to the MAINTAINERS file.
>
> Sure, I'm happy to support these patches and future work I plan to do on
> the driver.
>
> What additional responsibilities would come with adding my name the
> MAINTAINERS file?
>
> Perhaps Michael and Sam would be interested in sharing the role, for atari
> and sun3 NCR5380 drivers (?)

If you insist ...

(kidding - I"m OK with it if James thinks it's worth it)

Cheers,

Michael

>
> I only have hardware to test mac_scsi but I can obtain a Domex DMX3191D
> PCI card on ebay.
>
> --

2014-04-29 14:41:20

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure

On Tue, 2014-04-29 at 15:15 +1200, Michael Schmitz wrote:
> Finn,
>
> On Tue, Apr 29, 2014 at 2:22 PM, Finn Thain <[email protected]> wrote:
> >
> > On Sat, 26 Apr 2014, James Bottomley wrote:
> >
> >> OK, so this is a pretty big change to an unmaintained driver. I'll take
> >> it if you're willing to maintain the driver afterwards ... in which case
> >> I need another patch to add you to the MAINTAINERS file.
> >
> > Sure, I'm happy to support these patches and future work I plan to do on
> > the driver.
> >
> > What additional responsibilities would come with adding my name the
> > MAINTAINERS file?
> >
> > Perhaps Michael and Sam would be interested in sharing the role, for atari
> > and sun3 NCR5380 drivers (?)
>
> If you insist ...
>
> (kidding - I"m OK with it if James thinks it's worth it)

As long as you understand how it works and how to fix it, the more the
merrier. It gives me more people to yell at if something goes wrong
with the driver.

James

2014-04-30 07:45:34

by Michael Schmitz

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure

Hi James,
> Perhaps Michael and Sam would be interested in sharing the role, for atari
> and sun3 NCR5380 drivers (?)
>> If you insist ...
>>
>> (kidding - I"m OK with it if James thinks it's worth it)
> As long as you understand how it works and how to fix it, the more the
> merrier. It gives me more people to yell at if something goes wrong
> with the driver.

I used to have a fair understanding of the Atari driver years back -
just need to get my head around all the midlevel and error handling
changes since.

You'll be yelling at me regarding any breakage in the Atari NCR5380
driver anyway in the end - count me in.

Cheers,

Michael