2005-09-02 00:19:00

by Daniel Drake

[permalink] [raw]
Subject: [PATCH] via82cxxx IDE: Support multiple controllers

Bartlomiej Zolnierkiewicz wrote:
> Same thing as with VT6420 support:
>
> I'm still concerned about VIA IDE chipset + VT6410 combo
> (AFAIR I've also seen VT6410 on PCI add-on card but I can be wrong).
>
> via82cxxx.c needs to be fixed to support multiple controllers first.

Hows this? I don't have any hardware with two VIA controllers, however I have
tested this on a pc which has a single vt8233a controller.

---

Support multiple controllers in the via82cxxx IDE driver

Signed-off-by: Daniel Drake <[email protected]>


Attachments:
via82cxxx-multi.patch (13.20 kB)

2005-09-09 22:20:01

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

Bart, any word on this? I know you're busy but it would be much appreciated if
you could comment, even if it is hurling abuse ;)

Thanks.

Daniel Drake wrote:
> Hows this? I don't have any hardware with two VIA controllers, however I
> have tested this on a pc which has a single vt8233a controller.
>
> ---
>
> Support multiple controllers in the via82cxxx IDE driver
>
> Signed-off-by: Daniel Drake <[email protected]>


Attachments:
via82cxxx-multi.patch (13.20 kB)
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

On 9/2/05, Daniel Drake <[email protected]> wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > Same thing as with VT6420 support:
> >
> > I'm still concerned about VIA IDE chipset + VT6410 combo
> > (AFAIR I've also seen VT6410 on PCI add-on card but I can be wrong).
> >
> > via82cxxx.c needs to be fixed to support multiple controllers first.
>
> Hows this? I don't have any hardware with two VIA controllers, however I have
> tested this on a pc which has a single vt8233a controller.
>
> ---
>
> Support multiple controllers in the via82cxxx IDE driver
>
> Signed-off-by: Daniel Drake <[email protected]>

--- linux/drivers/ide/pci/via82cxxx.c.orig 2005-08-31 01:32:05.000000000 +0100
+++ linux/drivers/ide/pci/via82cxxx.c 2005-09-02 01:16:59.000000000 +0100
@@ -101,11 +101,19 @@ static struct via_isa_bridge {
{ NULL }
};

-static struct via_isa_bridge *via_config;
-static unsigned int via_80w;
-static unsigned int via_clock;
static char *via_dma[] = { "MWDMA16", "UDMA33", "UDMA66", "UDMA100",
"UDMA133" };

I would really prefer not to add per host struct via82xxx_dev,
(making it per hwif and doing extra match in ->init_hwif() is acceptable).

+struct via82cxxx_dev
+{
+ struct pci_dev *pci_dev, *isa_dev;

pci_dev is needed only for /proc/via and I would prefer /proc/via
to vanish because it complicates driver needlessly (could you do
this in separate patch?).

isa_dev has no relevance for vt6410 and won't be needed if
/proc/via goes away

+ struct via_isa_bridge *via_config;

Please instead add via_config_find() which would
find proper via_config given PCI ID.

+ unsigned int via_clock;

Global via_clock is OK as IDE core doesn't
support per bus PCI clocks anyway.

+ unsigned int via_80w;

Cable detection code should be moved to separate function
and be called from ->init_hwif() (required for future hotplug support).

Otherwise patch looks fine.

Thanks and sorry for the delay,
Bartlomiej

2005-09-28 22:29:35

by Daniel Drake

[permalink] [raw]
Subject: [PATCH] via82cxxx IDE: Remove /proc/via entry

This entry adds needless complication to the driver as it requires the use of
global variables to be passed into via_get_info(), making things quite ugly
when we try and make this driver support multiple controllers simultaneously.

This patch removes /proc/via for simplicity.

Signed-off-by: Daniel Drake <[email protected]>


Attachments:
via82cxxx-no-proc.patch (6.52 kB)

2005-09-28 22:33:21

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers (v2)

Support multiple controllers in the via82cxxx IDE driver, revised patch. Cable
detection and ISA bridge finding have been moved into their own functions.

Unfortunately I won't have access to a via82cxxx machine until December now,
this patch is only compile-tested.

Signed-off-by: Daniel Drake <[email protected]>


Attachments:
via82cxxx-multi-2.patch (9.20 kB)

2005-09-28 22:37:24

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/via entry

On Wed, Sep 28, 2005 at 11:18:37PM +0100, Daniel Drake wrote:
> This entry adds needless complication to the driver as it requires the use
> of
> global variables to be passed into via_get_info(), making things quite ugly
> when we try and make this driver support multiple controllers
> simultaneously.
>
> This patch removes /proc/via for simplicity.
>
> Signed-off-by: Daniel Drake <[email protected]>

Care to explain
* where to get equivalent information?
* what hardware setup has more than one of those controllers?

2005-09-28 22:47:45

by Grzegorz Kulewski

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/via entry

On Wed, 28 Sep 2005, Daniel Drake wrote:

> This entry adds needless complication to the driver as it requires the use of
> global variables to be passed into via_get_info(), making things quite ugly
> when we try and make this driver support multiple controllers simultaneously.
>
> This patch removes /proc/via for simplicity.

Is similar data available from sysfs?

As a user of this controller, I think that if it is not then this patch
should be changed to export it or should be dropped. The data from that
file is really helpfull in debugging problems (for example related to bad
cables or breaking disks/cdroms).


Thanks,

Grzegorz Kulewski

2005-09-28 23:11:21

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/ide/via entry

Hi Al,

(btw, original subject was wrong, I actually meant /proc/ide/via)

Al Viro wrote:
> Care to explain
> * where to get equivalent information?

I don't think there is anywhere else that provides the whole range, but I do
question the usefulness of most of it :)

Here's my previous attempt at this patch:

http://marc.theaimsgroup.com/?l=linux-ide&m=112630444000358&w=2

If you can point out a way to keep /proc/ide/via around without causing the
kind of ugliness found above, then maybe Bart can be persuaded to keep it
around :)

> * what hardware setup has more than one of those controllers?

I'm pushing to get a simple patch merged, which adds ID's for a VIA VT6410
controller. Apparently these are available in PCI-card form as well as
onboard-PCI-chip form. Bart raised the concern that this driver wouldn't cope
well with 2 different controllers in use, so I'm trying to address this.

Thanks,
Daniel

Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/ide/via entry

Hi,

On 9/29/05, Daniel Drake <[email protected]> wrote:
> Hi Al,
>
> (btw, original subject was wrong, I actually meant /proc/ide/via)
>
> Al Viro wrote:
> > Care to explain
> > * where to get equivalent information?
>
> I don't think there is anywhere else that provides the whole range, but I do
> question the usefulness of most of it :)

Exactly, all the important information is available through other sources
(dmesg, lspci and of course /proc/ide/hd?/*) and configuration of timing
registers etc. shouldn't be of user concern (and it is available from PCI
configuration space so code to parse it can be easily moved to user-space).

Bartlomiej

Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/via entry

On 9/29/05, Grzegorz Kulewski <[email protected]> wrote:
> On Wed, 28 Sep 2005, Daniel Drake wrote:
>
> > This entry adds needless complication to the driver as it requires the use of
> > global variables to be passed into via_get_info(), making things quite ugly
> > when we try and make this driver support multiple controllers simultaneously.
> >
> > This patch removes /proc/via for simplicity.
>
> Is similar data available from sysfs?
>
> As a user of this controller, I think that if it is not then this patch
> should be changed to export it or should be dropped. The data from that
> file is really helpfull in debugging problems (for example related to bad
> cables or breaking disks/cdroms).

Could you please give some details (which data is useful)?

Bartlomiej

2005-10-09 15:16:21

by Grzegorz Kulewski

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/via entry

On Thu, 29 Sep 2005, Bartlomiej Zolnierkiewicz wrote:

> On 9/29/05, Grzegorz Kulewski <[email protected]> wrote:
>> On Wed, 28 Sep 2005, Daniel Drake wrote:
>>
>>> This entry adds needless complication to the driver as it requires the use of
>>> global variables to be passed into via_get_info(), making things quite ugly
>>> when we try and make this driver support multiple controllers simultaneously.
>>>
>>> This patch removes /proc/via for simplicity.
>>
>> Is similar data available from sysfs?
>>
>> As a user of this controller, I think that if it is not then this patch
>> should be changed to export it or should be dropped. The data from that
>> file is really helpfull in debugging problems (for example related to bad
>> cables or breaking disks/cdroms).
>
> Could you please give some details (which data is useful)?

Sorry for delay - I was very busy during past week.

Well, I guess that thanks to this file I am able to know that the drive is
connected to via chipset and not to some pseudo raid controller (it is
important if I am working remotely), if it has 80w cables attached, what
is attached where, what are controller parameters, dma rate and timings
(if they are strange I can suspect that something bad is happening with
the drive / controler).

I am not saying that these data are not available elsewhere (but some are
not for sure). But they are presented nicely in one standard place and
this is in my opinion good.

But I understand the desire to remove all files that can be removed from
/proc. So I am just suggesting to move some of the data (like type of
cable detected for example) to sysfs if they are not already there.


Thanks,

Grzegorz Kulewski

2005-10-12 15:38:28

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

Bartlomiej Zolnierkiewicz wrote:
> I would prefer /proc/via
> to vanish because it complicates driver needlessly (could you do
> this in separate patch?).

I'm working on a user-space app to provide the same info. It's nearly there
but lacking some timing info.

Do you have any suggestions for how I can compute the value of via_clock in
userspace? (i.e. some equivalent of system_bus_clock())

Thanks,
Daniel

2005-10-12 15:49:19

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

Daniel Drake wrote:
> Bartlomiej Zolnierkiewicz wrote:
>
>> I would prefer /proc/via
>> to vanish because it complicates driver needlessly (could you do
>> this in separate patch?).
>
>
> I'm working on a user-space app to provide the same info. It's nearly
> there but lacking some timing info.
>
> Do you have any suggestions for how I can compute the value of via_clock
> in userspace? (i.e. some equivalent of system_bus_clock())

A sysfs read-only attribute, associated with the PCI device?

Of course, procfs sure seems a whole lot easier some days...

Jeff


2005-10-12 15:57:29

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

Daniel Drake wrote:
> Do you have any suggestions for how I can compute the value of via_clock
> in userspace? (i.e. some equivalent of system_bus_clock())

Uh, looks like the kernel just assumes 33mhz unless overriden by the user. Is
this assumption generally accurate?
If it is not, then there's probably no point displaying timing info...

Daniel

2005-10-12 17:24:15

by Alan

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

On Mer, 2005-10-12 at 16:57 +0100, Daniel Drake wrote:
> Uh, looks like the kernel just assumes 33mhz unless overriden by the user. Is
> this assumption generally accurate?
> If it is not, then there's probably no point displaying timing info...

A small number of 486 systems use 25Mhz, some boards allow overclock at
37.5Mhz on the PCI. I've been looking at this the past couple of days
for the libata via driver which I've been porting over and unfortunately
having been through the Northbridge manuals I can find no way to ask the
chipset what the PCI clock is set too.

Alan

2005-10-13 11:16:32

by Alan

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

On Mer, 2005-10-12 at 18:52 +0100, Alan Cox wrote:
> On Mer, 2005-10-12 at 16:57 +0100, Daniel Drake wrote:
> > Uh, looks like the kernel just assumes 33mhz unless overriden by the user. Is
> > this assumption generally accurate?
> > If it is not, then there's probably no point displaying timing info...
>
> A small number of 486 systems use 25Mhz, some boards allow overclock at
> 37.5Mhz on the PCI. I've been looking at this the past couple of days
> for the libata via driver which I've been porting over and unfortunately
> having been through the Northbridge manuals I can find no way to ask the
> chipset what the PCI clock is set too.

Ok I found what seems to be a pattern for the early chipsets with 25MHz
support.

If the bus speed of your 486 is 25Mhz the chipset is at 25MHz as is your
IDE (ie 486/25, DX2/50, 3/75 - not sure about 4/100 etc). Now does
anyone know how you find out if the CPU is 25MHz bus clocked on a 486 8)

2005-10-13 14:41:57

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

Alan Cox wrote:
>
> If the bus speed of your 486 is 25Mhz the chipset is at 25MHz as is your
> IDE (ie 486/25, DX2/50, 3/75 - not sure about 4/100 etc). Now does
> anyone know how you find out if the CPU is 25MHz bus clocked on a 486 8)

Same method as /proc/cpuinfo, for an approximation? :)

2005-10-13 15:01:08

by Alan

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers

On Iau, 2005-10-13 at 10:41 -0400, Mark Lord wrote:
> Alan Cox wrote:
> >
> > If the bus speed of your 486 is 25Mhz the chipset is at 25MHz as is your
> > IDE (ie 486/25, DX2/50, 3/75 - not sure about 4/100 etc). Now does
> > anyone know how you find out if the CPU is 25MHz bus clocked on a 486 8)
>
> Same method as /proc/cpuinfo, for an approximation? :)

Unfortunately cpuinfo doesn't know the difference between a 100Mhz
(4x25) and 100Mhz (3x33). Late 486s have cpuid which helps a bit but
many do not have that (it comes in with writeback cache) and they don't
have rdmsr to access the processor boot bus speed bits as the preventium
and later do.

Alan

2005-10-13 22:52:34

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/via entry

Hi,

Grzegorz Kulewski wrote:
>>> As a user of this controller, I think that if it is not then this patch
>>> should be changed to export it or should be dropped. The data from that
>>> file is really helpfull in debugging problems (for example related to
>>> bad
>>> cables or breaking disks/cdroms).

Per Bart's suggestion, I've created a user-space app which shows identical
data (and doesn't even rely on the via82cxxx IDE driver).

http://www.reactivated.net/software/viaideinfo/

So, I think we should be clear to drop /proc/ide/via now.

Daniel

2005-11-04 10:52:41

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers (v2)

Daniel Drake wrote:
> Support multiple controllers in the via82cxxx IDE driver, revised patch.
> Cable detection and ISA bridge finding have been moved into their own
> functions.
>
> Unfortunately I won't have access to a via82cxxx machine until December
> now, this patch is only compile-tested.

I went home a little earlier than expected and tested this patch on my
single-controller via machine. It works fine. Is this ok to be merged?

> Signed-off-by: Daniel Drake <[email protected]>
>
>
> ------------------------------------------------------------------------
>
> --- linux-2.6.14-rc1/drivers/ide/pci/via82cxxx.c.orig 2005-09-28 22:31:22.000000000 +0100
> +++ linux-2.6.14-rc1/drivers/ide/pci/via82cxxx.c 2005-09-28 23:06:50.000000000 +0100

Subject: Re: [PATCH] via82cxxx IDE: Remove /proc/via entry

On 10/13/05, Daniel Drake <[email protected]> wrote:
> Hi,
>
> Grzegorz Kulewski wrote:
> >>> As a user of this controller, I think that if it is not then this patch
> >>> should be changed to export it or should be dropped. The data from that
> >>> file is really helpfull in debugging problems (for example related to
> >>> bad
> >>> cables or breaking disks/cdroms).
>
> Per Bart's suggestion, I've created a user-space app which shows identical
> data (and doesn't even rely on the via82cxxx IDE driver).
>
> http://www.reactivated.net/software/viaideinfo/
>
> So, I think we should be clear to drop /proc/ide/via now.

patch applied, thanks for working on this

Subject: Re: [PATCH] via82cxxx IDE: Support multiple controllers (v2)

On 11/4/05, Daniel Drake <[email protected]> wrote:
> Daniel Drake wrote:
> > Support multiple controllers in the via82cxxx IDE driver, revised patch.
> > Cable detection and ISA bridge finding have been moved into their own
> > functions.
> >
> > Unfortunately I won't have access to a via82cxxx machine until December
> > now, this patch is only compile-tested.
>
> I went home a little earlier than expected and tested this patch on my
> single-controller via machine. It works fine. Is this ok to be merged?

applied