Re: [PATCH v2 2/2] usb: gadget: Add xilinx axi usb2 device support

[Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

>> +struct xusb_udc {
>> +	struct usb_gadget gadget;
>> +	struct xusb_ep ep[8];
>> +	struct usb_gadget_driver *driver;
>> +	struct cmdbuf ch9cmd;
>> +	u32 usb_state;
>> +	u32 remote_wkp;
>> +	unsigned int (*read_fn)(void __iomem *);
>> +	void (*write_fn)(void __iomem *, u32, u32);
> 
> why do you need these to be function pointers ? Because of endianness ?
> generic readl()/writel() already take care of that.

readl from asm-generic/io.h is converting value from little endian IO
to cpu endianness.
This IP exists also in big endian version.
It means we have to support all possible combinations.
IP little and reading it on little or big endian CPU
IP big and reading it on little or big endian CPU.

Look below.

>> +	spin_lock_init(&udc->lock);
>> +
>> +	/* Check for IP endianness */
>> +	udc->write_fn = xudc_write32_be;
>> +	udc->read_fn = xudc_read32_be;
>> +	udc->write_fn(udc->base_address, XUSB_TESTMODE_OFFSET, TEST_J);
>> +	if ((udc->read_fn(udc->base_address + XUSB_TESTMODE_OFFSET))
>> +			!= TEST_J) {
>> +		udc->write_fn = xudc_write32;
>> +		udc->read_fn = xudc_read32;
>> +	}
> 
> hmm... isn't there a configuration register to check this out ?

Sundeep can tell us if there is any configuration register but
I don't think so in connection to my experience with Xilinx soft IPs.

Endian detection directly on IP itself came from my discussion
on drivers/spi/spi-xilinx.c that this is only one way how to do it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]