On 03/15/2016 11:49 PM, Marek Vasut wrote:

> On 03/15/2016 01:44 PM, Purna Chandra Mandal wrote:
>> This driver adds support of PIC32 MUSB OTG controller as dual role device.
>> It implements platform specific glue to reuse musb core.
>>
>> Signed-off-by: Cristian Birsan <cristian.bir...@microchip.com>
>> Signed-off-by: Purna Chandra Mandal <purna.man...@microchip.com>
> [...]
>
>> diff --git a/drivers/usb/musb-new/pic32.c b/drivers/usb/musb-new/pic32.c
>> new file mode 100644
>> index 0000000..980a971
>> --- /dev/null
>> +++ b/drivers/usb/musb-new/pic32.c
>> @@ -0,0 +1,294 @@
>> +/*
>> + * Microchip PIC32 MUSB "glue layer"
>> + *
>> + * Copyright (C) 2015, Microchip Technology Inc.
>> + *  Cristian Birsan <cristian.bir...@microchip.com>
>> + *  Purna Chandra Mandal <purna.man...@microchip.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + *
>> + * Based on the dsps "glue layer" code.
>> + */
>> +
>> +#include <common.h>
>> +#include <linux/usb/musb.h>
>> +#include "linux-compat.h"
>> +#include "musb_core.h"
>> +#include "musb_uboot.h"
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define PIC32_TX_EP_MASK    0x0f            /* EP0 + 7 Tx EPs */
>> +#define PIC32_RX_EP_MASK    0x0e            /* 7 Rx EPs */
>> +
>> +#define MUSB_SOFTRST                0x7f
>> +#define  MUSB_SOFTRST_NRST  BIT(0)
>> +#define  MUSB_SOFTRST_NRSTX BIT(1)
>> +
>> +#define USBCRCON            0
>> +#define  USBCRCON_USBWKUPEN BIT(0)  /* Enable Wakeup Interrupt */
>> +#define  USBCRCON_USBRIE    BIT(1)  /* Enable Remote resume Interrupt */
>> +#define  USBCRCON_USBIE             BIT(2)  /* Enable USB General interrupt 
>> */
>> +#define  USBCRCON_SENDMONEN BIT(3)  /* Enable Session End VBUS monitoring */
>> +#define  USBCRCON_BSVALMONEN        BIT(4)  /* Enable B-Device VBUS 
>> monitoring */
>> +#define  USBCRCON_ASVALMONEN        BIT(5)  /* Enable A-Device VBUS 
>> monitoring */
>> +#define  USBCRCON_VBUSMONEN BIT(6)  /* Enable VBUS monitoring */
>> +#define  USBCRCON_PHYIDEN   BIT(7)  /* PHY ID monitoring enable */
>> +#define  USBCRCON_USBIDVAL  BIT(8)  /* USB ID value */
>> +#define  USBCRCON_USBIDOVEN BIT(9)  /* USB ID override enable */
>> +#define  USBCRCON_USBWK             BIT(24) /* USB Wakeup Status */
>> +#define  USBCRCON_USBRF             BIT(25) /* USB Resume Status */
>> +#define  USBCRCON_USBIF             BIT(26) /* USB General Interrupt Status 
>> */
>> +
>> +static void __iomem *musb_glue;
> What would happen once you make a chip with two MUSB controllers ?

Currently PIC32 has only one MUSB controller and only one glue reg-space.
Don't know how the reg-map will be in future when PIC32 will have multiple
MUSB controllers. Assuming that glue address map will be separate for
each controller we can add logic to support multiple MUSB controller.

IMO, better if we don't assume something of the future and bloat logic.

>> +/* pic32_musb_disable - disable HDRC */
>> +static void pic32_musb_disable(struct musb *musb)
>> +{
> Is there no way to shut down the MUSB on the PIC32 ?

There is no way to disable MUSB.

>> +}
>> +
>> +/* pic32_musb_enable - enable HDRC */
>> +static int pic32_musb_enable(struct musb *musb)
>> +{
>> +    /* soft reset by NRSTx */
>> +    musb_writeb(musb->mregs, MUSB_SOFTRST, MUSB_SOFTRST_NRSTX);
>> +    /* set mode */
>> +    musb_platform_set_mode(musb, musb->board_mode);
>> +
>> +    return 0;
>> +}
>> +
>> +static irqreturn_t pic32_interrupt(int irq, void *hci)
>> +{
>> +    struct musb  *musb = hci;
>> +    irqreturn_t ret = IRQ_NONE;
>> +    u32 epintr, usbintr;
>> +
>> +    /* Get usb core interrupts */
> You mean "get" or "ack" here ?

I meant read-and-ack. Will update comment.

>> +    musb->int_usb = musb_readb(musb->mregs, MUSB_INTRUSB);
>> +    if (musb->int_usb)
>> +            musb_writeb(musb->mregs, MUSB_INTRUSB, musb->int_usb);
>> +
>> +    /* Get endpoint interrupts */
> DTTO

I meant read-and-ack.

>> +    musb->int_rx = musb_readw(musb->mregs, MUSB_INTRRX) & PIC32_RX_EP_MASK;
>> +    if (musb->int_rx)
>> +            musb_writew(musb->mregs, MUSB_INTRRX, musb->int_rx);
> Same here

ack. Will update comment.

>> +    musb->int_tx = musb_readw(musb->mregs, MUSB_INTRTX) & PIC32_TX_EP_MASK;
>> +    if (musb->int_tx)
>> +            musb_writew(musb->mregs, MUSB_INTRTX, musb->int_tx);
>> +
>> +    /* Drop spurious RX and TX if device is disconnected */
>> +    if (musb->int_usb & MUSB_INTR_DISCONNECT) {
>> +            musb->int_tx = 0;
>> +            musb->int_rx = 0;
>> +    }
>> +
>> +    if (musb->int_tx || musb->int_rx || musb->int_usb)
>> +            ret |= musb_interrupt(musb);
>> +
>> +    return ret;
>> +}
>> +
>> +static int pic32_musb_set_mode(struct musb *musb, u8 mode)
>> +{
>> +    struct device *dev = musb->controller;
>> +
>> +    switch (mode) {
>> +    case MUSB_HOST:
>> +            clrsetbits_le32(musb_glue + USBCRCON,
>> +                            USBCRCON_USBIDVAL, USBCRCON_USBIDOVEN);
>> +            break;
>> +    case MUSB_PERIPHERAL:
>> +            setbits_le32(musb_glue + USBCRCON,
>> +                         USBCRCON_USBIDVAL | USBCRCON_USBIDOVEN);
>> +            break;
>> +    case MUSB_OTG:
>> +            dev_err(dev, "MUSB OTG mode enabled\n");
> So having the core in OTG mode is an error ? Why ?

In PIC32 we haven't tested OTG. We use the controller as dual-role not as OTG.
In future we might enable that. 

>> +            break;
>> +    default:
>> +            dev_err(dev, "unsupported mode %d\n", mode);
>> +            return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int pic32_musb_init(struct musb *musb)
>> +{
>> +    u32 ctrl, hwvers;
>> +    u8 power;
>> +
>> +    /* Returns zero if not clocked */
>> +    hwvers = musb_read_hwvers(musb->mregs);
>> +    if (!hwvers)
>> +            return -ENODEV;
>> +
>> +    /* Reset the musb */
>> +    power = musb_readb(musb->mregs, MUSB_POWER);
>> +    power = power | MUSB_POWER_RESET;
>> +    musb_writeb(musb->mregs, MUSB_POWER, power);
>> +    mdelay(100);
>> +
>> +    /* Start the on-chip PHY and its PLL. */
>> +    power = power & ~MUSB_POWER_RESET;
>> +    musb_writeb(musb->mregs, MUSB_POWER, power);
>> +
>> +    musb->isr = pic32_interrupt;
>> +
>> +    ctrl =  USBCRCON_USBIF | USBCRCON_USBRF |
>> +            USBCRCON_USBWK | USBCRCON_USBIDOVEN |
>> +            USBCRCON_PHYIDEN | USBCRCON_USBIE |
>> +            USBCRCON_USBRIE | USBCRCON_USBWKUPEN |
>> +            USBCRCON_VBUSMONEN;
>> +    writel(ctrl, musb_glue + USBCRCON);
>> +
>> +    return 0;
>> +}
>> +
>> +/* PIC32 supports only 32bit read operation */
>> +void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst)
>> +{
>> +    void __iomem *fifo = hw_ep->fifo;
>> +    u32 val;
>> +    int i;
> This could become:
>  bulk_len = len / 4;
>  bulk_rem = len % 4;
>  readsl(fifo, dst, bulk_len);
>  if (rem) {
>   dst += len & ~0x3;
>   tmp = readl(fifo);
>   copy the remaining bytes according to endianness
>  }
>
> This is 10 LoC , not 50 ;-)

May be comments in my code are not very clear :)

All the extra code is for handling (4-byte-word) alignment of destination 
buffer.
Writing word to unaligned address will generate exception in MIPS which is not 
handled
in U-Boot software.
Note MIPS can't handle unaligned access in h/w unless specific unaligned
instructions are used.

>> +    /* Read for 32bit-aligned destination address */
>> +    if (likely((0x03 & (unsigned long)dst) == 0) && len >= 4) {
>> +            readsl(fifo, dst, len / 4);
>> +            dst += len & ~0x03;
>> +            len &= 0x03;
>> +    }
>> +
>> +    /*
>> +     * Now read the remaining 1 to 3 byte or complete length if
>> +     * unaligned address.
>> +     */
>> +    if (len > 4) {
>> +            for (i = 0; i < (len / 4); i++) {
>> +                    *(u32 *)dst = musb_readl(fifo, 0);
>> +                    dst += 4;
>> +            }
>> +            len &= 0x03;
>> +    }
>> +
>> +    if (len > 0) {
>> +            val = musb_readl(fifo, 0);
>> +            memcpy(dst, &val, len);
>> +    }
>> +}
> [...]
>

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to