Hi Shimoda-san, On Tue, May 20, 2014 at 11:35 AM, Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > (2014/05/19 20:58), Geert Uytterhoeven wrote: >> On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda >> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > < snip > >>> +config USB_XHCI_RCAR >>> + tristate "xHCI support for Renesas R-Car SoCs" >>> + select USB_XHCI_PLATFORM >>> + depends on ARCH_SHMOBILE || COMPILE_TEST >>> + ---help--- >>> + Say 'Y' to enable the support for the xHCI host controller >>> + found in Renesas R-Car ARM SoCs. >> >> Does R-Car Gen1 also have xHCI, and is it compatible? >> If not, you may want to call this driver USB_XHCI_RCAR2. > > R-Car Gen1 doesn't have xHCI. > However, next generation of R-Car may have xHCI. (But, I don't know it is compatible.) > If we call this driver "USB_XHCI_RCAR2", should we also change filename to "xhci-rcar2.[ch]"? Iff you change the config symbol, please also change the filename. But given the uncertainty about future version, you can leave it like it is. >>> + xhci_rcar_start(hcd); >> >> If CONFIG_USB_XHCI_RCAR is not defined, xhci_rcar_start() is a dummy >> function, but the of_device_is_compatible() checks will still be compiled in. >> >> Hence perhaps an #ifdef CONFIG_USB_XHCI_RCAR is warranted here, >> possibly combined with inclusion of a C-source file, like is done in >> drivers/usb/host/ohci-hcd.c? It's up to the USB maintainer to decide this, >> though. > > This implementation is similar with the following patch. And the patch already got > "Acked-by" from Mathias Nyman of USB XHCI DRIVER's maintainer. > > http://marc.info/?l=linux-usb&m=140014933101775&w=2 Fine. It can be fixed later by the maintainer, when the driver has gained too many compatible checks ;-) >>> + for (index = 0; index < fw->size; index += 4) { >>> + for (data = 0, j = 3; j >= 0; j--) { >>> + if ((j + index) >= fw->size) >>> + continue; >>> + data |= fw->data[index + j] << (8 * j); >>> + } >> >> This is your custom get_unaligned_le32(), to avoid reading beyond the end >> of the buffer if its size is not a multiple of 4 bytes? > > Yes, I would like to avoid it. > >> Is there some way to just use get_unaligned_le32()? > > Yes, I will remove the custom get_unaligned_le32() and add the following code. > Do you think that this code is good? > > int i; > u32 data; > u8 buf[4]; > < snip > > for (i = 0; i < fw->size; i += 4) { > memset(buf, 0, sizeof(buf)); > memcpy(buf, &fw->data[i], min(sizeof(buf), fw->size - i)); > data = get_unaligned_le32(buf); I'm sorry, but IMHO this looks worse. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
