On Sat, 8 Jan 2022, Mark Cave-Ayland wrote:
On 06/11/2021 16:52, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu
drivers/usbohci.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/usbohci.c b/drivers/usbohci.c index 774164b..c2c0f78 100644 --- a/drivers/usbohci.c +++ b/drivers/usbohci.c @@ -232,12 +232,12 @@ ohci_init (void *bar) usb_debug("HCCA addr %p\n", OHCI_INST(controller)->hcca); /* Initialize interrupt table. */
- u32 *const intr_table =
OHCI_INST(controller)->hcca->HccaInterruptTable;
- ohci_t *const ohci = OHCI_INST(controller); ed_t *const periodic_ed; ofmem_posix_memalign((void **)&periodic_ed, sizeof(ed_t),
sizeof(ed_t)); memset((void *)periodic_ed, 0, sizeof(*periodic_ed)); for (i = 0; i < 32; ++i)
intr_table[i] = __cpu_to_le32(virt_to_phys(periodic_ed));
ohci->hcca->HccaInterruptTable[i] =__cpu_to_le32(virt_to_phys(periodic_ed)); OHCI_INST (controller)->periodic_ed = periodic_ed;
The changes to intr_table appear to make things less readable to me.
Yes but I could not find a simpler way to fix this easily.
Presumably the error is that the existing code is attempting to write through a const pointer? In which case I think simply dropping the const from
No, it's about possible unalligned access, see my answer to Philippe.
intr_table both here and below should fix the issue, since it is clearly intended to be used for writing.
I don't think that would fix the problem.
OHCI_INST (controller)->opreg->HcHCCA = __cpu_to_le32(virt_to_phys(OHCI_INST(controller)->hcca)); @@ -607,7 +607,7 @@ struct _intrq_td { u8 *data; struct _intrq_td *next; struct _intr_queue *intrq; -}; +} __attribute__ ((packed));
This looks like a different fix that should be in a separate patch (wasn't there a similar patch for QEMU recently?).
It's also fixing a build issue with gcc 10 and the problem is similar so I've put it in one patch but feel free to split it up to as many patches as you like or change it or fix the problem in an alternative way. I'm OK with you taking this patch and changing it or doing it differently (you have my S-o-b, just add your maintainer comment and S-o-b after that if you change the patch) but I don't have time to make changes to this now.
Regards, BALATON Zoltan
struct _intr_queue { volatile ed_t ed; @@ -698,14 +698,13 @@ ohci_create_intr_queue(endpoint_t *const ep, const int reqsize, /* Insert ED into periodic table. */ int nothing_placed = 1; ohci_t *const ohci = OHCI_INST(ep->dev->controller);
- u32 *const intr_table = ohci->hcca->HccaInterruptTable; const u32 dummy_ptr =
__cpu_to_le32(virt_to_phys(ohci->periodic_ed)); for (i = 0; i < 32; i += reqtiming) { /* Advance to the next free position. */
while ((i < 32) && (intr_table[i] != dummy_ptr)) ++i;
while ((i < 32) && (ohci->hcca->HccaInterruptTable[i] !=dummy_ptr)) ++i; if (i < 32) { usb_debug("Placed endpoint %lx to %d\n", virt_to_phys(&intrq->ed), i);
intr_table[i] =__cpu_to_le32(virt_to_phys(&intrq->ed));
ohci->hcca->HccaInterruptTable[i] =__cpu_to_le32(virt_to_phys(&intrq->ed)); nothing_placed = 0; } } @@ -729,10 +728,9 @@ ohci_destroy_intr_queue(endpoint_t *const ep, void *const q_) /* Remove interrupt queue from periodic table. */ ohci_t *const ohci = OHCI_INST(ep->dev->controller);
- u32 *const intr_table = ohci->hcca->HccaInterruptTable; for (i=0; i < 32; ++i) {
if (intr_table[i] == __cpu_to_le32(virt_to_phys(intrq)))intr_table[i] =__cpu_to_le32(virt_to_phys(ohci->periodic_ed));
if (ohci->hcca->HccaInterruptTable[i] ==__cpu_to_le32(virt_to_phys(intrq)))
ohci->hcca->HccaInterruptTable[i] =__cpu_to_le32(virt_to_phys(ohci->periodic_ed)); } /* Wait for frame to finish. */ mdelay(1);
ATB,
Mark.