Here is the better version of the patch I sent you earlier. The first patch will make OpenBIOS compile without warnings or errors, but doesn't make USB work. This patch is an attempt to make USB work. It looks like changes were made to the original code to make it compile in OpenBIOS. These changes may have disabled the OHCI code.
https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ecec80e0... This page has very similar code to usbohci.c has. Using this page I filled in what I thought were the missing function calls. My changes no longer allow OpenBIOS to boot, so consider this a work in progress for now.
--- drivers/usbohci.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/usbohci.c b/drivers/usbohci.c index 774164b..222ab37 100644 --- a/drivers/usbohci.c +++ b/drivers/usbohci.c @@ -227,14 +227,14 @@ ohci_init (void *bar) OHCI_INST (controller)->opreg->HcCommandStatus = __cpu_to_le32(HostControllerReset); udelay (10); /* at most 10us for reset to complete. State must be set to Operational within 2ms (5.1.1.4) */ OHCI_INST (controller)->opreg->HcFmInterval = interval; - ofmem_posix_memalign((void **)&(OHCI_INST (controller)->hcca), 256, 256); + ofmem_posix_memalign((void *)(OHCI_INST (controller)->hcca), 256, 256); memset((void*)OHCI_INST (controller)->hcca, 0, 256);
usb_debug("HCCA addr %p\n", OHCI_INST(controller)->hcca); /* Initialize interrupt table. */ u32 *const intr_table = OHCI_INST(controller)->hcca->HccaInterruptTable; - ed_t *const periodic_ed; - ofmem_posix_memalign((void **)&periodic_ed, sizeof(ed_t), sizeof(ed_t)); + ed_t *const periodic_ed = (ed_t *)malloc(sizeof(ed_t)); + 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)); @@ -378,8 +378,8 @@ ohci_control (usbdev_t *dev, direction_t dir, int drlen, void *devreq, int dalen int pages = (dalen==0)?0:(last_page - first_page + 1);
/* First TD. */ - td_t *const first_td; - ofmem_posix_memalign((void **)&first_td, sizeof(td_t), sizeof(td_t)); + td_t *const first_td = (td_t *)malloc(sizeof(td_t)); + ofmem_posix_memalign((void *)first_td, sizeof(td_t), sizeof(td_t)); memset((void *)first_td, 0, sizeof(*first_td)); cur = first_td;
@@ -393,8 +393,8 @@ ohci_control (usbdev_t *dev, direction_t dir, int drlen, void *devreq, int dalen
while (pages > 0) { /* One more TD. */ - td_t *const next; - ofmem_posix_memalign((void **)&next, sizeof(td_t), sizeof(td_t)); + td_t *const next = (td_t *)malloc(sizeof(td_t)); + ofmem_posix_memalign((void *)next, sizeof(td_t), sizeof(td_t)); memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = __cpu_to_le32(virt_to_phys(next)); @@ -428,8 +428,8 @@ ohci_control (usbdev_t *dev, direction_t dir, int drlen, void *devreq, int dalen }
/* One more TD. */ - td_t *const next_td; - ofmem_posix_memalign((void **)&next_td, sizeof(td_t), sizeof(td_t)); + td_t *const next_td = (td_t *)malloc(sizeof(td_t)); + ofmem_posix_memalign((void *)next_td, sizeof(td_t), sizeof(td_t)); memset((void *)next_td, 0, sizeof(*next_td)); /* Linked to the previous. */ cur->next_td = __cpu_to_le32(virt_to_phys(next_td)); @@ -444,15 +444,15 @@ ohci_control (usbdev_t *dev, direction_t dir, int drlen, void *devreq, int dalen cur->buffer_end = 0;
/* Final dummy TD. */ - td_t *const final_td; - ofmem_posix_memalign((void **)&final_td, sizeof(td_t), sizeof(td_t)); + td_t *const final_td = (td_t *)malloc(sizeof(td_t)); + ofmem_posix_memalign((void *)final_td, sizeof(td_t), sizeof(td_t)); memset((void *)final_td, 0, sizeof(*final_td)); /* Linked to the previous. */ cur->next_td = __cpu_to_le32(virt_to_phys(final_td));
/* Data structures */ - ed_t *head; - ofmem_posix_memalign((void **)&head, sizeof(ed_t), sizeof(ed_t)); + ed_t *head = (ed_t *)malloc(sizeof(ed_t)); + ofmem_posix_memalign((void *)head, sizeof(ed_t), sizeof(ed_t)); memset((void*)head, 0, sizeof(*head)); head->config = __cpu_to_le32((dev->address << ED_FUNC_SHIFT) | (0 << ED_EP_SHIFT) | @@ -507,8 +507,8 @@ ohci_bulk (endpoint_t *ep, int dalen, u8 *data, int finalize) }
/* First TD. */ - td_t *const first_td; - ofmem_posix_memalign((void **)&first_td, sizeof(td_t), sizeof(td_t)); + td_t *const first_td = (td_t *)malloc(sizeof(td_t)); + ofmem_posix_memalign((void *)first_td, sizeof(td_t), sizeof(td_t)); memset((void *)first_td, 0, sizeof(*first_td)); cur = next = first_td;
@@ -546,7 +546,7 @@ ohci_bulk (endpoint_t *ep, int dalen, u8 *data, int finalize) data += second_page_size; } /* One more TD. */ - ofmem_posix_memalign((void **)&next, sizeof(td_t), sizeof(td_t)); + ofmem_posix_memalign((void *)next, sizeof(td_t), sizeof(td_t)); memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = __cpu_to_le32(virt_to_phys(next)); @@ -558,8 +558,8 @@ ohci_bulk (endpoint_t *ep, int dalen, u8 *data, int finalize) cur = next;
/* Data structures */ - ed_t *head; - ofmem_posix_memalign((void **)&head, sizeof(ed_t), sizeof(ed_t)); + ed_t *head = (ed_t *)malloc(sizeof(ed_t)); + ofmem_posix_memalign((void *)head, sizeof(ed_t), sizeof(ed_t)); memset((void*)head, 0, sizeof(*head)); head->config = __cpu_to_le32((ep->dev->address << ED_FUNC_SHIFT) | ((ep->endpoint & 0xf) << ED_EP_SHIFT) | @@ -652,8 +652,8 @@ ohci_create_intr_queue(endpoint_t *const ep, const int reqsize, if (reqsize > 4096) return NULL;
- intr_queue_t *const intrq; - ofmem_posix_memalign((void **)&intrq, sizeof(intrq->ed), sizeof(*intrq)); + intr_queue_t *const intrq = (intr_queue_t *)malloc(sizeof(*intrq)); + ofmem_posix_memalign((void *)intrq, sizeof(intrq->ed), sizeof(*intrq)); memset(intrq, 0, sizeof(*intrq)); intrq->data = (u8 *)malloc(reqcount * reqsize); intrq->reqsize = reqsize; @@ -662,8 +662,8 @@ ohci_create_intr_queue(endpoint_t *const ep, const int reqsize, /* Create #reqcount TDs. */ u8 *cur_data = intrq->data; for (i = 0; i < reqcount; ++i) { - intrq_td_t *const td; - ofmem_posix_memalign((void **)&td, sizeof(td->td), sizeof(*td)); + intrq_td_t *const td = (intrq_td_t *)malloc(sizeof(*td)); + ofmem_posix_memalign((void *)td, sizeof(td->td), sizeof(*td)); ++intrq->remaining_tds; ohci_fill_intrq_td(td, intrq, cur_data); cur_data += reqsize; @@ -675,8 +675,8 @@ ohci_create_intr_queue(endpoint_t *const ep, const int reqsize, }
/* Create last, dummy TD. */ - intrq_td_t *dummy_td; - ofmem_posix_memalign((void **)&dummy_td, sizeof(dummy_td->td), sizeof(*dummy_td)); + intrq_td_t *dummy_td = malloc(sizeof(*dummy_td)); + ofmem_posix_memalign((void *)dummy_td, sizeof(dummy_td->td), sizeof(*dummy_td)); memset(dummy_td, 0, sizeof(*dummy_td)); dummy_td->intrq = intrq; if (last_td)
On Sun, 16 Oct 2016, Programmingkid wrote:
make USB work. This patch is an attempt to make USB work. It looks like changes were made to the original code to make it compile in OpenBIOS.
Apart from integrating in OpenBIOS changes were needed to make it work on a big endian CPU as well.
These changes may have disabled the OHCI code.
Not likely but I don't know what are you trying to do.
https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ecec80e0... This page has very similar code to usbohci.c has.
The usb driver in OpenBIOS was ported from the libpayload library of the coreboot project. Your link says third_party so this is not the official repo. Maybe a better source would be the upstream repository at coreboot.org rather than Google's copy of it although that seems to be a bit hard to browse without cloning because it's behind a patch management system. I've found this link now (not sure if it has any relevant differences to the above but this should be the upstream source):
https://review.coreboot.org/cgit/coreboot.git
Using this page I filled in what I thought were the missing function calls. My changes no longer allow OpenBIOS to boot, so consider this a work in progress for now.
If this breaks it then it's likely not the correct fix. What was the problem in the first place this is trying to correct?
Regards, BALATON Zoltan
On Oct 17, 2016, at 6:28 AM, BALATON Zoltan wrote:
On Sun, 16 Oct 2016, Programmingkid wrote:
make USB work. This patch is an attempt to make USB work. It looks like changes were made to the original code to make it compile in OpenBIOS.
Apart from integrating in OpenBIOS changes were needed to make it work on a big endian CPU as well.
These changes may have disabled the OHCI code.
Not likely but I don't know what are you trying to do.
I'm trying to make the USB keyboard work. Using a simple command-line like this "qemu-system-ppc -usb -usb-kbd" does not appear to work. Nothing typed appears on the screen.
I noticed many of these kind of changes: Original coreboot version: ed_t *const periodic_ed = dma_memalign(sizeof(ed_t), sizeof(ed_t));
OpenBIOS version: ed_t *const periodic_ed;
All of the dma_memalign() calls were removed. How did this code work with so many uninitialized pointers?
https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ecec80e0... This page has very similar code to usbohci.c has.
The usb driver in OpenBIOS was ported from the libpayload library of the coreboot project. Your link says third_party so this is not the official repo. Maybe a better source would be the upstream repository at coreboot.org rather than Google's copy of it although that seems to be a bit hard to browse without cloning because it's behind a patch management system. I've found this link now (not sure if it has any relevant differences to the above but this should be the upstream source):
Thank you for this link.
Using this page I filled in what I thought were the missing function calls. My changes no longer allow OpenBIOS to boot, so consider this a work in progress for now.
If this breaks it then it's likely not the correct fix. What was the problem in the first place this is trying to correct?
When compiling OpenBIOS with gcc 4.2.3, a lot of warnings were issued by the compiler:
CC target/drivers/usbohci.o cc1: warnings being treated as errors /Users/john/desktop/openbios/drivers/usbohci.c: In function ‘ohci_init’: /Users/john/desktop/openbios/drivers/usbohci.c:230: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:237: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c: In function ‘ohci_control’: /Users/john/desktop/openbios/drivers/usbohci.c:382: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:397: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:432: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:448: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:455: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c: In function ‘ohci_bulk’: /Users/john/desktop/openbios/drivers/usbohci.c:511: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:549: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:562: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c: In function ‘ohci_create_intr_queue’: /Users/john/desktop/openbios/drivers/usbohci.c:656: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:666: warning: dereferencing type-punned pointer will break strict-aliasing rules /Users/john/desktop/openbios/drivers/usbohci.c:679: warning: dereferencing type-punned pointer will break strict-aliasing rules make[1]: *** [target/drivers/usbohci.o] Error 1 make: *** [subdir-ppc] Error 2
On Mon, 17 Oct 2016, Programmingkid wrote:
I noticed many of these kind of changes: Original coreboot version: ed_t *const periodic_ed = dma_memalign(sizeof(ed_t), sizeof(ed_t));
OpenBIOS version: ed_t *const periodic_ed;
Plus another line following this calling ofmem_posix_memalign() isn't it? This should be eqiuvalent to dma_memalign using OpenBIOS's facilities.
All of the dma_memalign() calls were removed. How did this code work with so many uninitialized pointers?
They are not supposed to be unintialised as ofmem_posix_memalign() function initialises them.
When compiling OpenBIOS with gcc 4.2.3, a lot of warnings were issued by the compiler:
CC target/drivers/usbohci.o cc1: warnings being treated as errors /Users/john/desktop/openbios/drivers/usbohci.c: In function ‘ohci_init’: /Users/john/desktop/openbios/drivers/usbohci.c:230: warning: dereferencing type-punned pointer will break strict-aliasing rules
These may be harmless caused but passing a pointer to a struct as void * which gcc does not like but apart from the warning it may actually work. I don't know how to supress this warning other than disabling it for this file. Maybe using a temporary void * pointer to allocate memory with ofmem_posix_memalign() and then assigning the result to the struct pointer could supress the warning but I'm not sure. In any case, I think these warnings and the problem seen with non-working keyboard are likely unrelated.
Regards, BALATON Zoltan