Re: [PATCH 1/4 v3] EHCI: split ehci_qh - header file changes

[Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>]

On Thu, 25 Jun 2009, Alek Du wrote:

> Here is the example, the here.hw_next must be a __hc32 in EHCI HW periodic list.
> 
> static void periodic_unlink (struct ehci_hcd *ehci, unsigned frame, void *ptr)
> {
>         union ehci_shadow       *prev_p = &ehci->pshadow[frame];
>         __hc32                  *hw_p = &ehci->periodic[frame];
>         union ehci_shadow       here = *prev_p;
> 
>         /* find predecessor of "ptr"; hw and shadow lists are in sync */
>         while (here.ptr && here.ptr != ptr) {
>                 prev_p = periodic_next_shadow(ehci, prev_p,
>                                 Q_NEXT_TYPE(ehci, *hw_p));
>                 hw_p = here.hw_next;
>                 here = *prev_p;
>         }
> 
> static union ehci_shadow *
> periodic_next_shadow(struct ehci_hcd *ehci, union ehci_shadow *periodic,
>                 __hc32 tag)
> {
>         switch (hc32_to_cpu(ehci, tag)) {
>         case Q_TYPE_QH:
>                 return &periodic->qh->qh_next;
>         case Q_TYPE_FSTN:
>                 return &periodic->fstn->fstn_next;
>         case Q_TYPE_ITD:
>                 return &periodic->itd->itd_next;
>         // case Q_TYPE_SITD:
>         default:
>                 return &periodic->sitd->sitd_next;
>         }
> }

Fair enough.  Here's how this could be rewritten, assuming we make the 
qh member of ehci_shadow point to the software structure (this was 
composed in email, so it's not compile-tested or anything):

static void periodic_unlink(struct ehci_hcd *ehci, unsigned frame, void *ptr)
{
	union ehci_shadow	*prev_p = &ehci->pshadow[frame];
	union ehci_shadow	here = *prev_p;
	__hc32			*hw_p = &ehci->periodic[frame];
	__hc32			tag = Q_NEXT_TYPE(ehci, *hw_p);

	/* find predecessor of "ptr"; hw and shadow lists are in sync */
	while (here.ptr && here.ptr != ptr) {
		if (hc32_to_cpu(tag) == Q_TYPE_QH)
			hw_p = &HW(here->qh)->hw_next;
		else
			hw_p = here.hw_next;
		prev_p = periodic_next_shadow(ehci, prev_p, tag);
		here = *prev_p;
		tag = Q_NEXT_TYPE(ehci, *hw_p);
	}

However, I would prefer to redo all this in terms of a special-purpose
iterator (something I've been considering adding).  The iterator code
could be reused in several places within ehci-sched.c:

struct periodic_iterator {
	union ehci_shadow	*prev_p;
	__hc32			*hw_p;
	unsigned		tag;
}

static union ehci_shadow periodic_iterator_init(struct ehci_hcd *ehci,
		unsigned frame, struct periodic_iterator *iter)
{
	iter->prev_p = &ehci->pshadow[frame];
	iter->hw_p = &ehci->periodic[frame];
	iter->tag = hc32_to_cpu(ehci, *iter->hw_p);
	if (iter->tag & 1)		/* End of the list */
		return (union ehci_shadow) NULL;
	iter->tag &= (3 << 1);
	return *iter->prev_p;
}

static union ehci_shadow periodic_iterator_next(struct ehci_hcd *ehci,
		struct periodic_iterator *iter)
{
	union ehci_shadow	here = *iter->prev_p;

	iter->hw_p = here->hw_next;
	switch (iter->tag) {
	case Q_TYPE_QH:
		iter->prev_p = &here->qh->qh_next;
		iter->hw_p = &HW(here->qh)->hw_next;
		break;
	case Q_TYPE_FSTN:
		iter->prev_p = &here->fstn->fstn_next;
		break;
	case Q_TYPE_ITD:
		iter->prev_p = &here->itd->itd_next;
		break;
	// case Q_TYPE_SITD:
	default:
		iter->prev_p = &here->sitd->sitd_next;
		break;
	}

	iter->tag = hc32_to_cpu(ehci, *iter->hw_p);
	if (iter->tag & 1)		/* End of the list */
		return (union ehci_shadow) NULL;
	iter->tag &= (3 << 1);
	return *iter->prev_p;
}

static void periodic_unlink(struct ehci_hcd *ehci, unsigned frame, void *ptr)
{
	struct periodic_iterator	iter;
	union ehci_shadow		here;

	/* find predecessor of "ptr"; hw and shadow lists are in sync */
	here = periodic_iterator_init(ehci, frame, &iter);
	while (here.ptr && here.ptr != ptr)
		here = periodic_iterator_next(ehci, &iter);

	/* Now use iter.hw_p and iter.prev_p... */

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html