> > I used the define 'USE_32BIT' to 0.
> > Isn't the ISP1362 16-bit only? You can do 32-bit accesses on
> > the 16-bit bus, but then you violate the (fussy) timing for
> > sure!
> >
> That depends on the capabilities of the processors memory
> interface. At least on PXA you can program the memory timing so
> that individual memory cycles don't require any software delays.
> The timing specs are fully met even for 32 bit accesses.
Okay, that makes sense. I'm pretty sure I need this software
timing for the LH7A40x; I looked into this more extensively with
the old driver.
> > I had a bigger problem with the isp1362_[read,write]_fifo
> > functions.. the writesl / writesw calls also completely
> > violated the timing requirements. They caused back-to-back
> > writes (and I'm assuming reads) with 20ns
...
> >
> If this is the case you probably also had to introduce software
> delays in the isp1362_read_data16() and isp1362_write_data16()
> functions.
No, I didn't need to do that. As long as they're separate
instructions (vs. 32-bit read/writes), the timing seems just fine.
> And you probably also should have an OR gate in your hardware
> that prevents the chips WE being asserted when CS is deasserted
> as recommended in the ISP1362 Errata Document. Otherwise you
> possibly cannot guarantee the 132ns idle time between successive
> assertions of WE (no matter whether CS is asserted also).
Yes, we did that a while ago. I did get the ohci-isp1362 driver
working (with caveats), and this was one problem we fixed.
> Did you enable the CHIP_BUFFER_TEST to assert that your accesses
> to the chip registers and buffers are working porperly?
Yes, I needed that while finding and fixing the timing problems. I
also added the scratch buffer test at the very beginning. With my
changes, I pass the chip buffer test every time.
>
> > if (len >= 2) {
> > RDBG("%s: Using readsw for %d words\n",
> > __FUNCTION__, len >> 1);
> > readsw(isp1362_hcd->data_reg, dp, len >> 1);
> > @@ -736,6 +771,22 @@
> > (u8)data, (u32)dp);
> > *dp = (u8)data;
> > }
> > +#else
> > + while ( len >= 2 )
> > + {
> > + data = isp1362_read_data16(isp1362_hcd);
> > + *dp++ = (u8)data;
> > + *dp++ = (u8)(data >> 8);
> > + len -= 2;
> > + }
> > +
> > + if ( len > 0 )
> > + {
> > + data = isp1362_read_data16(isp1362_hcd);
> > + *dp++ = (u8)data;
> > + len--;
> > + }
> > +#endif
> > }
> The '(len > 0)' case should be the same under all circumstances,
> so
> the '#endif' could be placed right before it.
>
As far as this goes, do you think it makes sense to add it to your
patch (and to the mainstream kernel eventually)? More people may
run into it, so it's good to have the option and a comment to go
with it. We could probably make a better (or another) define for
this behavior as well.
Thanks for your response.
Mike
-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
