2003-08-29 17:14:17

by Martin Schwidefsky

[permalink] [raw]
Subject: [PATCH] s390 (5/8): common i/o layer.

- Remove initialization of device.name.
- Don't do put_device after failed get_device in get_ccwdev_by_busid.
- Fix read_dev_chars and read_conf_data.
- Call interrupt function of ccw device if path verification has been started.
- Replace atomic_return_add by atomic_add_return in qdio.

diffstat:
drivers/s390/cio/chsc.c | 6 +-----
drivers/s390/cio/css.c | 17 ++---------------
drivers/s390/cio/device.c | 9 +++++----
drivers/s390/cio/device_ops.c | 37 ++++++++++++++++++++++++-------------
drivers/s390/cio/qdio.c | 23 ++++++-----------------
5 files changed, 38 insertions(+), 54 deletions(-)

diff -urN linux-2.6/drivers/s390/cio/chsc.c linux-2.6-s390/drivers/s390/cio/chsc.c
--- linux-2.6/drivers/s390/cio/chsc.c Sat Aug 23 01:56:32 2003
+++ linux-2.6-s390/drivers/s390/cio/chsc.c Fri Aug 29 18:55:10 2003
@@ -1,7 +1,7 @@
/*
* drivers/s390/cio/chsc.c
* S/390 common I/O routines -- channel subsystem call
- * $Revision: 1.74 $
+ * $Revision: 1.76 $
*
* Copyright (C) 1999-2002 IBM Deutschland Entwicklung GmbH,
* IBM Corporation
@@ -865,10 +865,6 @@
chp->state = status;
chp->dev.parent = &css_bus_device;

- snprintf(chp->dev.name, DEVICE_NAME_SIZE,
- "channel path %x", chpid);
- snprintf(chp->dev.bus_id, DEVICE_ID_SIZE, "chp%x", chpid);
-
/* make it known to the system */
ret = device_register(&chp->dev);
if (ret) {
diff -urN linux-2.6/drivers/s390/cio/css.c linux-2.6-s390/drivers/s390/cio/css.c
--- linux-2.6/drivers/s390/cio/css.c Sat Aug 23 01:52:24 2003
+++ linux-2.6-s390/drivers/s390/cio/css.c Fri Aug 29 18:55:10 2003
@@ -1,7 +1,7 @@
/*
* drivers/s390/cio/css.c
* driver for channel subsystem
- * $Revision: 1.43 $
+ * $Revision: 1.48 $
*
* Copyright (C) 2002 IBM Deutschland Entwicklung GmbH,
* IBM Corporation
@@ -27,7 +27,6 @@
int css_init_done = 0;

struct device css_bus_device = {
- .name = "Channel Subsystem 0",
.bus_id = "css0",
};

@@ -79,17 +78,6 @@
static int
css_register_subchannel(struct subchannel *sch)
{
- static const char *subchannel_types[] = {
- "I/O Subchannel",
- "CHSC Subchannel",
- "Message Subchannel",
- "ADM Subchannel",
- "undefined subchannel type 4",
- "undefined subchannel type 5",
- "undefined subchannel type 6",
- "undefined subchannel type 7",
- "undefined subchannel type 8",
- };
int ret;

/* Initialize the subchannel structure */
@@ -97,8 +85,7 @@
sch->dev.bus = &css_bus_type;

/* Set a name for the subchannel */
- strlcpy (sch->dev.name, subchannel_types[sch->st], DEVICE_NAME_SIZE);
- snprintf (sch->dev.bus_id, DEVICE_ID_SIZE, "0:%04x", sch->irq);
+ snprintf (sch->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x", sch->irq);

/* make it known to the system */
ret = device_register(&sch->dev);
diff -urN linux-2.6/drivers/s390/cio/device.c linux-2.6-s390/drivers/s390/cio/device.c
--- linux-2.6/drivers/s390/cio/device.c Sat Aug 23 01:58:10 2003
+++ linux-2.6-s390/drivers/s390/cio/device.c Fri Aug 29 18:55:10 2003
@@ -1,7 +1,7 @@
/*
* drivers/s390/cio/device.c
* bus driver for ccw devices
- * $Revision: 1.60 $
+ * $Revision: 1.65 $
*
* Copyright (C) 2002 IBM Deutschland Entwicklung GmbH,
* IBM Corporation
@@ -537,8 +537,7 @@
init_timer(&cdev->private->timer);

/* Set an initial name for the device. */
- snprintf (cdev->dev.name, DEVICE_NAME_SIZE,"ccw device");
- snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0:%04x",
+ snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x",
sch->schib.pmcw.dev);

/* Increase counter of devices currently in recognition. */
@@ -704,8 +703,10 @@

if (dev && !strncmp(bus_id, dev->bus_id, DEVICE_ID_SIZE))
break;
- else
+ else if (dev) {
put_device(dev);
+ dev = NULL;
+ }
}
up_read(&drv->bus->subsys.rwsem);
put_driver(drv);
diff -urN linux-2.6/drivers/s390/cio/device_ops.c linux-2.6-s390/drivers/s390/cio/device_ops.c
--- linux-2.6/drivers/s390/cio/device_ops.c Sat Aug 23 01:58:10 2003
+++ linux-2.6-s390/drivers/s390/cio/device_ops.c Fri Aug 29 18:55:10 2003
@@ -160,13 +160,15 @@
* - we received an intermediate status
* - fast notification was requested (primary status)
* - unsolicited interrupts
+ * - device path verification has been started
*/
stctl = cdev->private->irb.scsw.stctl;
if (sch->schib.scsw.actl != 0 &&
!cdev->private->options.repall &&
!(stctl & SCSW_STCTL_INTER_STATUS) &&
!(cdev->private->options.fast &&
- (stctl & SCSW_STCTL_PRIM_STATUS)))
+ (stctl & SCSW_STCTL_PRIM_STATUS)) &&
+ (cdev->private->state != DEV_STATE_VERIFY))
return;

/*
@@ -211,11 +213,19 @@
static void
ccw_device_wake_up(struct ccw_device *cdev, unsigned long ip, struct irb *irb)
{
- struct subchannel *sch;
+ if (!ip)
+ /* unsolicited interrupt */
+ return;

- sch = to_subchannel(cdev->dev.parent);
- if (!IS_ERR(irb))
- memcpy(&sch->schib.scsw, &irb->scsw, sizeof(struct scsw));
+ /* Abuse intparm for error reporting. */
+ if (IS_ERR(irb))
+ cdev->private->intparm = -EIO;
+ else if ((irb->scsw.dstat !=
+ (DEV_STAT_CHN_END|DEV_STAT_DEV_END)) ||
+ (irb->scsw.cstat != 0))
+ cdev->private->intparm = -EIO;
+ else
+ cdev->private->intparm = 0;
wake_up(&cdev->private->wait_q);
}

@@ -270,14 +280,14 @@
break;
if (ret == 0) {
/* Wait for end of request. */
+ cdev->private->intparm = 0x00524443;
spin_unlock_irqrestore(&sch->lock, flags);
wait_event(cdev->private->wait_q,
- sch->schib.scsw.actl == 0);
+ (cdev->private->intparm == -EIO) ||
+ (cdev->private->intparm == 0));
spin_lock_irqsave(&sch->lock, flags);
/* Check at least for channel end / device end */
- if ((sch->schib.scsw.dstat !=
- (DEV_STAT_CHN_END|DEV_STAT_DEV_END)) ||
- (sch->schib.scsw.cstat != 0)) {
+ if (cdev->private->intparm) {
ret = -EIO;
continue;
}
@@ -350,13 +360,14 @@
if (ret)
continue;
/* Wait for end of request. */
+ cdev->private->intparm = 0x00524344;
spin_unlock_irqrestore(&sch->lock, flags);
- wait_event(cdev->private->wait_q, sch->schib.scsw.actl == 0);
+ wait_event(cdev->private->wait_q,
+ (cdev->private->intparm == -EIO) ||
+ (cdev->private->intparm == 0));
spin_lock_irqsave(&sch->lock, flags);
/* Check at least for channel end / device end */
- if ((sch->schib.scsw.dstat !=
- (DEV_STAT_CHN_END|DEV_STAT_DEV_END)) ||
- (sch->schib.scsw.cstat != 0)) {
+ if (cdev->private->intparm) {
ret = -EIO;
continue;
}
diff -urN linux-2.6/drivers/s390/cio/qdio.c linux-2.6-s390/drivers/s390/cio/qdio.c
--- linux-2.6/drivers/s390/cio/qdio.c Sat Aug 23 01:57:09 2003
+++ linux-2.6-s390/drivers/s390/cio/qdio.c Fri Aug 29 18:55:10 2003
@@ -55,7 +55,7 @@
#include "ioasm.h"
#include "chsc.h"

-#define VERSION_QDIO_C "$Revision: 1.55 $"
+#define VERSION_QDIO_C "$Revision: 1.58 $"

/****************** MODULE PARAMETER VARIABLES ********************/
MODULE_AUTHOR("Utz Bacher <[email protected]>");
@@ -120,15 +120,7 @@
static inline unsigned long
qdio_get_millis(void)
{
- return (unsigned long)(qdio_get_micros()>>12);
-}
-
-static __inline__ int
-atomic_return_add (int i, atomic_t *v)
-{
- int old, new;
- __CS_LOOP(old, new, v, i, "ar");
- return old;
+ return (unsigned long)(qdio_get_micros()>>10);
}

static void
@@ -181,7 +173,7 @@
static inline int
qdio_reserve_q(struct qdio_q *q)
{
- return atomic_return_add(1,&q->use_count);
+ return atomic_add_return(1,&q->use_count) - 1;
}

static inline void
@@ -1221,21 +1213,18 @@
qdio_release_irq_memory(struct qdio_irq *irq_ptr)
{
int i;
- int available;

for (i=0;i<QDIO_MAX_QUEUES_PER_IRQ;i++) {
if (!irq_ptr->input_qs[i])
goto next;
- available=0;

if (irq_ptr->input_qs[i]->slib)
kfree(irq_ptr->input_qs[i]->slib);
- kfree(irq_ptr->input_qs[i]);
+ kfree(irq_ptr->input_qs[i]);

next:
if (!irq_ptr->output_qs[i])
continue;
- available=0;

if (irq_ptr->output_qs[i]->slib)
kfree(irq_ptr->output_qs[i]->slib);
@@ -2855,7 +2844,7 @@
int used_elements;

/* This is the inbound handling of queues */
- used_elements=atomic_return_add(count, &q->number_of_buffers_used);
+ used_elements=atomic_add_return(count, &q->number_of_buffers_used) - count;

qdio_do_qdio_fill_input(q,qidx,count,buffers);

@@ -2897,7 +2886,7 @@

qdio_do_qdio_fill_output(q,qidx,count,buffers);

- used_elements=atomic_return_add(count, &q->number_of_buffers_used);
+ used_elements=atomic_add_return(count, &q->number_of_buffers_used) - count;

if (callflags&QDIO_FLAG_DONT_SIGA) {
#ifdef QDIO_PERFORMANCE_STATS


2003-08-29 19:26:08

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

Martin Schwidefsky <[email protected]> writes:

> --- linux-2.6/drivers/s390/cio/css.c Sat Aug 23 01:52:24 2003
> +++ linux-2.6-s390/drivers/s390/cio/css.c Fri Aug 29 18:55:10 2003

[...]

> @@ -97,8 +85,7 @@
> sch->dev.bus = &css_bus_type;
>
> /* Set a name for the subchannel */
> - strlcpy (sch->dev.name, subchannel_types[sch->st], DEVICE_NAME_SIZE);
> - snprintf (sch->dev.bus_id, DEVICE_ID_SIZE, "0:%04x", sch->irq);
> + snprintf (sch->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x", sch->irq);
>
> /* make it known to the system */
> ret = device_register(&sch->dev);
> diff -urN linux-2.6/drivers/s390/cio/device.c linux-2.6-s390/drivers/s390/cio/device.c
> --- linux-2.6/drivers/s390/cio/device.c Sat Aug 23 01:58:10 2003
> +++ linux-2.6-s390/drivers/s390/cio/device.c Fri Aug 29 18:55:10 2003

[...]

> @@ -537,8 +537,7 @@
> init_timer(&cdev->private->timer);
>
> /* Set an initial name for the device. */
> - snprintf (cdev->dev.name, DEVICE_NAME_SIZE,"ccw device");
> - snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0:%04x",
> + snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x",
> sch->schib.pmcw.dev);
>
> /* Increase counter of devices currently in recognition. */

Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?
--
OGAWA Hirofumi <[email protected]>

2003-08-29 20:24:50

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

OGAWA Hirofumi <[email protected]> writes:

> > @@ -537,8 +537,7 @@
> > init_timer(&cdev->private->timer);
> >
> > /* Set an initial name for the device. */
> > - snprintf (cdev->dev.name, DEVICE_NAME_SIZE,"ccw device");
> > - snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0:%04x",
> > + snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x",
> > sch->schib.pmcw.dev);
> >
> > /* Increase counter of devices currently in recognition. */
>
> Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?

Ooops. Sorry, I miss read it. I thought it's DEVICE_NAME_SIZE.
--
OGAWA Hirofumi <[email protected]>

2003-08-29 20:25:51

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

On Sat, Aug 30, 2003 at 04:25:21AM +0900, OGAWA Hirofumi wrote:
> Martin Schwidefsky <[email protected]> writes:
>
> > --- linux-2.6/drivers/s390/cio/css.c Sat Aug 23 01:52:24 2003
> > +++ linux-2.6-s390/drivers/s390/cio/css.c Fri Aug 29 18:55:10 2003
>
> [...]
>
> > @@ -97,8 +85,7 @@
> > sch->dev.bus = &css_bus_type;
> >
> > /* Set a name for the subchannel */
> > - strlcpy (sch->dev.name, subchannel_types[sch->st], DEVICE_NAME_SIZE);
> > - snprintf (sch->dev.bus_id, DEVICE_ID_SIZE, "0:%04x", sch->irq);
> > + snprintf (sch->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x", sch->irq);
> >
> > /* make it known to the system */
> > ret = device_register(&sch->dev);
> > diff -urN linux-2.6/drivers/s390/cio/device.c linux-2.6-s390/drivers/s390/cio/device.c
> > --- linux-2.6/drivers/s390/cio/device.c Sat Aug 23 01:58:10 2003
> > +++ linux-2.6-s390/drivers/s390/cio/device.c Fri Aug 29 18:55:10 2003
>
> [...]
>
> > @@ -537,8 +537,7 @@
> > init_timer(&cdev->private->timer);
> >
> > /* Set an initial name for the device. */
> > - snprintf (cdev->dev.name, DEVICE_NAME_SIZE,"ccw device");
> > - snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0:%04x",
> > + snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x",
> > sch->schib.pmcw.dev);
> >
> > /* Increase counter of devices currently in recognition. */
>
> Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?

Yes, DEVICE_ID_SIZE should be removed from the kernel, as it's not used
for any structures, only some modules that use it improperly (including
the USB core, I'll go fix that up right now...)

thanks,

greg k-h

2003-08-29 20:35:38

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

OGAWA Hirofumi wrote:

> Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?

Right. Actually, all uses of DEVICE_ID_SIZE in drivers/s390 are wrong.
I'll take care of that.

The only other user of DEVICE_ID_SIZE right now is drivers/usb/core/file.c
and I'm not sure if it's used in the intended way there.
Greg, maybe you want to get rid of it as well, or move the definition
into file.c.

Arnd <><

2003-08-29 20:41:26

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

On Fri, Aug 29, 2003 at 10:31:47PM +0200, Arnd Bergmann wrote:
> OGAWA Hirofumi wrote:
>
> > Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?
>
> Right. Actually, all uses of DEVICE_ID_SIZE in drivers/s390 are wrong.
> I'll take care of that.
>
> The only other user of DEVICE_ID_SIZE right now is drivers/usb/core/file.c
> and I'm not sure if it's used in the intended way there.
> Greg, maybe you want to get rid of it as well, or move the definition
> into file.c.

I'm deleting it right now... :)

thanks,

greg k-h

2003-08-29 20:52:57

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

On Fri, Aug 29, 2003 at 01:40:17PM -0700, Greg KH wrote:
> On Fri, Aug 29, 2003 at 10:31:47PM +0200, Arnd Bergmann wrote:
> > OGAWA Hirofumi wrote:
> >
> > > Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?
> >
> > Right. Actually, all uses of DEVICE_ID_SIZE in drivers/s390 are wrong.
> > I'll take care of that.
> >
> > The only other user of DEVICE_ID_SIZE right now is drivers/usb/core/file.c
> > and I'm not sure if it's used in the intended way there.
> > Greg, maybe you want to get rid of it as well, or move the definition
> > into file.c.
>
> I'm deleting it right now... :)

And here's the patch that I added to my trees.

thanks,

greg k-h


# USB: remove usage of DEVICE_ID_SIZE from usb core as it should not be used.

diff -Nru a/drivers/usb/core/file.c b/drivers/usb/core/file.c
--- a/drivers/usb/core/file.c Fri Aug 29 13:49:19 2003
+++ b/drivers/usb/core/file.c Fri Aug 29 13:49:20 2003
@@ -129,7 +129,7 @@
int retval = -EINVAL;
int minor_base = class_driver->minor_base;
int minor = 0;
- char name[DEVICE_ID_SIZE];
+ char name[BUS_ID_SIZE];
struct class_device *class_dev;
char *temp;

@@ -166,7 +166,7 @@
intf->minor = minor;

/* handle the devfs registration */
- snprintf(name, DEVICE_ID_SIZE, class_driver->name, minor - minor_base);
+ snprintf(name, BUS_ID_SIZE, class_driver->name, minor - minor_base);
devfs_mk_cdev(MKDEV(USB_MAJOR, minor), class_driver->mode, name);

/* create a usb class device for this usb interface */
@@ -211,7 +211,7 @@
struct usb_class_driver *class_driver)
{
int minor_base = class_driver->minor_base;
- char name[DEVICE_ID_SIZE];
+ char name[BUS_ID_SIZE];

#ifdef CONFIG_USB_DYNAMIC_MINORS
minor_base = 0;
@@ -226,7 +226,7 @@
usb_minors[intf->minor] = NULL;
spin_unlock (&minor_lock);

- snprintf(name, DEVICE_ID_SIZE, class_driver->name, intf->minor - minor_base);
+ snprintf(name, BUS_ID_SIZE, class_driver->name, intf->minor - minor_base);
devfs_remove (name);

if (intf->class_dev) {

2003-08-29 21:09:49

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

On Fri, Aug 29, 2003 at 11:06:18PM +0200, Arnd Bergmann wrote:
> On Friday 29 August 2003 22:50, Greg KH wrote:
> > diff -Nru a/drivers/usb/core/file.c b/drivers/usb/core/file.c
> > --- a/drivers/usb/core/file.c Fri Aug 29 13:49:19 2003
> > +++ b/drivers/usb/core/file.c Fri Aug 29 13:49:20 2003
> > @@ -129,7 +129,7 @@
> > int retval = -EINVAL;
> > int minor_base = class_driver->minor_base;
> > int minor = 0;
> > - char name[DEVICE_ID_SIZE];
> > + char name[BUS_ID_SIZE];
> > struct class_device *class_dev;
> > char *temp;
> >
>
> In your case, BUS_ID_SIZE doesn't look appropriate here either, because
> the name is never used directly as a bus_id. You should probably use
> your own constant here.

Look a little lower and see where I copy that name into a struct
class_device.class_id, which is BUS_ID_SIZE big :)

thanks,

greg k-h

2003-08-29 21:06:02

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.

On Friday 29 August 2003 22:50, Greg KH wrote:
> diff -Nru a/drivers/usb/core/file.c b/drivers/usb/core/file.c
> --- a/drivers/usb/core/file.c Fri Aug 29 13:49:19 2003
> +++ b/drivers/usb/core/file.c Fri Aug 29 13:49:20 2003
> @@ -129,7 +129,7 @@
> int retval = -EINVAL;
> int minor_base = class_driver->minor_base;
> int minor = 0;
> - char name[DEVICE_ID_SIZE];
> + char name[BUS_ID_SIZE];
> struct class_device *class_dev;
> char *temp;
>

In your case, BUS_ID_SIZE doesn't look appropriate here either, because
the name is never used directly as a bus_id. You should probably use
your own constant here.

Arnd <><

2003-09-01 07:10:27

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [PATCH] s390 (5/8): common i/o layer.


> > @@ -537,8 +537,7 @@
< > init_timer(&cdev->private->timer);
> >
> > /* Set an initial name for the device. */
> > - snprintf (cdev->dev.name, DEVICE_NAME_SIZE,"ccw device");
> > - snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0:%04x",
> > + snprintf (cdev->dev.bus_id, DEVICE_ID_SIZE, "0.0.%04x",
> > sch->schib.pmcw.dev);
> >
> > /* Increase counter of devices currently in recognition. */
>
> Shouldn't the above use BUS_ID_SIZE instead of DEVICE_ID_SIZE?

Yes, indeed. DEVICE_NAME_SIZE is 32 and BUS_ID_SIZE is 20. Luckily the
printed string is always shorter than 20 byte but nevertheless its wrong
to use DEVICE_ID_SIZE. Thanks for the hint.

blue skies,
Martin