Hi,
On Tue, Apr 10, 2012 at 05:25:12PM +0300, Alexander Shishkin wrote:
> Currently, the register prefixes in the driver seem to be mixed: the
> capability registers are the ones that contain capability information,
> such as number of hardware endpoints, while the registers that are
> used to program the controller are called operational registers.
>
> Normally, capability registers start at 0x100 offset of the register
> window and are followed by operational registers. In some versions,
> however, capability registers start at 0x0 offset.
>
> This patch renames the register and adjusts their offsets appropriately,
> leaving the possibility of having a non-standard capability offset.
>
> I couldn't find any mentions of the TESTMODE register anywhere, so I
> suspect it might only be enabled in chipidea internal versions of the
> controller and I'm really inclined to remove it from the driver or at
> least hiding it behind a config option.
>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/ci13xxx_msm.c | 3 +-
> drivers/usb/gadget/ci13xxx_pci.c | 7 +-
> drivers/usb/gadget/ci13xxx_udc.c | 229 +++++++++++++++++++-------------------
> drivers/usb/gadget/ci13xxx_udc.h | 3 +
> 4 files changed, 126 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/usb/gadget/ci13xxx_msm.c b/drivers/usb/gadget/ci13xxx_msm.c
> index d07e44c..6e77446 100644
> --- a/drivers/usb/gadget/ci13xxx_msm.c
> +++ b/drivers/usb/gadget/ci13xxx_msm.c
> @@ -79,7 +79,8 @@ static int ci13xxx_msm_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - ret = udc_probe(&ci13xxx_msm_udc_driver, &pdev->dev, regs);
> + ret = udc_probe(&ci13xxx_msm_udc_driver, &pdev->dev, regs,
> + DEF_CAPOFFSET);
> if (ret < 0) {
> dev_err(&pdev->dev, "udc_probe failed\n");
> goto iounmap;
> diff --git a/drivers/usb/gadget/ci13xxx_pci.c b/drivers/usb/gadget/ci13xxx_pci.c
> index 883ab5e..59373d5 100644
> --- a/drivers/usb/gadget/ci13xxx_pci.c
> +++ b/drivers/usb/gadget/ci13xxx_pci.c
> @@ -55,6 +55,7 @@ static int __devinit ci13xxx_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> {
> void __iomem *regs = NULL;
> + uintptr_t capoffset = DEF_CAPOFFSET;
> int retval = 0;
>
> if (id == NULL)
> @@ -86,7 +87,11 @@ static int __devinit ci13xxx_pci_probe(struct pci_dev *pdev,
> pci_set_master(pdev);
> pci_try_set_mwi(pdev);
>
> - retval = udc_probe(&ci13xxx_pci_udc_driver, &pdev->dev, regs);
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL)
> + capoffset = 0;
> +
> + retval = udc_probe(&ci13xxx_pci_udc_driver, &pdev->dev, regs,
> + capoffset);
> if (retval)
> goto iounmap;
>
> diff --git a/drivers/usb/gadget/ci13xxx_udc.c b/drivers/usb/gadget/ci13xxx_udc.c
> index d4b9c1e..9da5044 100644
> --- a/drivers/usb/gadget/ci13xxx_udc.c
> +++ b/drivers/usb/gadget/ci13xxx_udc.c
> @@ -138,7 +138,8 @@ static int ffs_nr(u32 x)
> static struct {
> unsigned lpm; /* is LPM? */
> void __iomem *abs; /* bus map offset */
> - void __iomem *cap; /* bus map offset + CAP offset + CAP data */
> + void __iomem *cap; /* bus map offset + CAP offset */
> + void __iomem *op; /* bus map offset + OP offset */
> size_t size; /* bank size */
> } hw_bank;
>
> @@ -146,26 +147,26 @@ static struct {
> #define ABS_AHBBURST (0x0090UL)
> #define ABS_AHBMODE (0x0098UL)
> /* UDC register map */
> -#define ABS_CAPLENGTH (0x100UL)
> -#define ABS_HCCPARAMS (0x108UL)
> -#define ABS_DCCPARAMS (0x124UL)
> +#define CAP_CAPLENGTH (0x000UL)
> +#define CAP_HCCPARAMS (0x008UL)
> +#define CAP_DCCPARAMS (0x024UL)
> #define ABS_TESTMODE (hw_bank.lpm ? 0x0FCUL : 0x138UL)
> /* offset to CAPLENTGH (addr + data) */
> -#define CAP_USBCMD (0x000UL)
> -#define CAP_USBSTS (0x004UL)
> -#define CAP_USBINTR (0x008UL)
> -#define CAP_DEVICEADDR (0x014UL)
> -#define CAP_ENDPTLISTADDR (0x018UL)
> -#define CAP_PORTSC (0x044UL)
> -#define CAP_DEVLC (0x084UL)
> -#define CAP_USBMODE (hw_bank.lpm ? 0x0C8UL : 0x068UL)
> -#define CAP_ENDPTSETUPSTAT (hw_bank.lpm ? 0x0D8UL : 0x06CUL)
> -#define CAP_ENDPTPRIME (hw_bank.lpm ? 0x0DCUL : 0x070UL)
> -#define CAP_ENDPTFLUSH (hw_bank.lpm ? 0x0E0UL : 0x074UL)
> -#define CAP_ENDPTSTAT (hw_bank.lpm ? 0x0E4UL : 0x078UL)
> -#define CAP_ENDPTCOMPLETE (hw_bank.lpm ? 0x0E8UL : 0x07CUL)
> -#define CAP_ENDPTCTRL (hw_bank.lpm ? 0x0ECUL : 0x080UL)
> -#define CAP_LAST (hw_bank.lpm ? 0x12CUL : 0x0C0UL)
> +#define OP_USBCMD (0x000UL)
> +#define OP_USBSTS (0x004UL)
> +#define OP_USBINTR (0x008UL)
> +#define OP_DEVICEADDR (0x014UL)
> +#define OP_ENDPTLISTADDR (0x018UL)
> +#define OP_PORTSC (0x044UL)
> +#define OP_DEVLC (0x084UL)
> +#define OP_USBMODE (hw_bank.lpm ? 0x0C8UL : 0x068UL)
> +#define OP_ENDPTSETUPSTAT (hw_bank.lpm ? 0x0D8UL : 0x06CUL)
> +#define OP_ENDPTPRIME (hw_bank.lpm ? 0x0DCUL : 0x070UL)
> +#define OP_ENDPTFLUSH (hw_bank.lpm ? 0x0E0UL : 0x074UL)
> +#define OP_ENDPTSTAT (hw_bank.lpm ? 0x0E4UL : 0x078UL)
> +#define OP_ENDPTCOMPLETE (hw_bank.lpm ? 0x0E8UL : 0x07CUL)
> +#define OP_ENDPTCTRL (hw_bank.lpm ? 0x0ECUL : 0x080UL)
> +#define OP_LAST (hw_bank.lpm ? 0x12CUL : 0x0C0UL)
>
> /* maximum number of enpoints: valid only after hw_device_reset() */
> static unsigned hw_ep_max;
> @@ -193,85 +194,85 @@ static int ep_to_bit(int n)
> }
>
> /**
> - * hw_aread: reads from register bitfield
> - * @addr: address relative to bus map
> + * hw_cread: reads from capability register bitfield
> + * @addr: address relative to capability register base
> * @mask: bitfield mask
> *
> * This function returns register bitfield data
> */
> -static u32 hw_aread(u32 addr, u32 mask)
> +static u32 hw_cread(u32 addr, u32 mask)
> {
> - return ioread32(addr + hw_bank.abs) & mask;
> + return ioread32(addr + hw_bank.cap) & mask;
> }
>
> /**
> - * hw_awrite: writes to register bitfield
> - * @addr: address relative to bus map
> + * hw_cwrite: writes to capability register bitfield
> + * @addr: address relative to capability register base
> * @mask: bitfield mask
> * @data: new data
> */
> -static void hw_awrite(u32 addr, u32 mask, u32 data)
> +static void hw_cwrite(u32 addr, u32 mask, u32 data)
> {
> - iowrite32(hw_aread(addr, ~mask) | (data & mask),
> - addr + hw_bank.abs);
> + iowrite32(hw_cread(addr, ~mask) | (data & mask),
> + addr + hw_bank.cap);
> }
>
> /**
> - * hw_cread: reads from register bitfield
> - * @addr: address relative to CAP offset plus content
> + * hw_oread: reads from operational register bitfield
> + * @addr: address relative to operational register base
> * @mask: bitfield mask
> *
> * This function returns register bitfield data
> */
> -static u32 hw_cread(u32 addr, u32 mask)
> +static u32 hw_oread(u32 addr, u32 mask)
> {
> - return ioread32(addr + hw_bank.cap) & mask;
> + return ioread32(addr + hw_bank.op) & mask;
> }
>
> /**
> - * hw_cwrite: writes to register bitfield
> - * @addr: address relative to CAP offset plus content
> + * hw_owrite: writes to operational register bitfield
> + * @addr: address relative to operational register base
> * @mask: bitfield mask
> * @data: new data
> */
> -static void hw_cwrite(u32 addr, u32 mask, u32 data)
> +static void hw_owrite(u32 addr, u32 mask, u32 data)
> {
> - iowrite32(hw_cread(addr, ~mask) | (data & mask),
> - addr + hw_bank.cap);
> + iowrite32(hw_oread(addr, ~mask) | (data & mask),
> + addr + hw_bank.op);
> }
why do you need separate functions to read capability or operational
registers ? They look the same. You could just define them a little
differently:
static u32 hw_read(void *base, u32 offset, u32 mask)
{
return ioread32(base + offset) & mask;
}
then, when calling it you can use:
hw_read(hw_bank.op, ADDRESS, mask);
hw_read(hw_bank.cap, ADDRESS, mask);
--
balbi
Attachment:
signature.asc
Description: Digital signature
