Re: [PATCH v1 1/2] usb: host: ehci-sunplus: Add driver for ehci in Sunplus SP7021

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Greg
    About this issue, my colleague Hammer Hsieh has explained it to
you recently in the mail of "[PATCH v7 2/2] serial: sunplus-uart: Add
Sunplus
SoC UART Driver". The ehci driver and uart one are in the same Sunplus Soc.
    Thanks for your review.

Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 於 2022年2月14日 週一 下午4:38寫道:
>
> On Mon, Feb 14, 2022 at 04:32:41PM +0800, Vincent Shih wrote:
> > Add driver for ehci in Sunplus SP7021
> >
> > Signed-off-by: Vincent Shih <vincent.sunplus@xxxxxxxxx>
> > ---
> >  MAINTAINERS                     |   6 +
> >  drivers/usb/host/Kconfig        |  11 +
> >  drivers/usb/host/Makefile       |   1 +
> >  drivers/usb/host/ehci-sunplus.c | 527 ++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 545 insertions(+)
> >  create mode 100644 drivers/usb/host/ehci-sunplus.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 80eebc1..6c2faf3 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -17947,6 +17947,12 @@ L:   netdev@xxxxxxxxxxxxxxx
> >  S:   Maintained
> >  F:   drivers/net/ethernet/dlink/sundance.c
> >
> > +SUNPLUS USB EHCI DRIVER
> > +M:   Vincent Shih <vincent.sunplus@xxxxxxxxx>
> > +L:   linux-usb@xxxxxxxxxxxxxxx
> > +S:   Maintained
> > +F:   drivers/usb/host/ehci-sunplus.c
> > +
> >  SUPERH
> >  M:   Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx>
> >  M:   Rich Felker <dalias@xxxxxxxx>
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > index c4736d1..9728d3b 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -339,6 +339,17 @@ config USB_OCTEON_EHCI
> >         USB 2.0 device support.  All CN6XXX based chips with USB are
> >         supported.
> >
> > +config USB_EHCI_SUNPLUS
> > +     tristate "EHCI support for Sunplus USB controller"
> > +     depends on SOC_SP7021
> > +     select NVMEM_SUNPLUS_OCOTP
> > +     help
> > +       Enables support for the on-chip EHCI controller in Sunplus
> > +       SoCs. It supports 32-bit AHB/AXI config bus and 64-bit AXI
> > +       data bus.
> > +       This driver can also be built as a module. If so, the module
> > +       will be called ehci-sunplus.
> > +
> >  endif # USB_EHCI_HCD
> >
> >  config USB_OXU210HP_HCD
> > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> > index 171de4d..7e9b5f1 100644
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
> > @@ -46,6 +46,7 @@ obj-$(CONFIG_USB_EHCI_HCD_STI)      += ehci-st.o
> >  obj-$(CONFIG_USB_EHCI_EXYNOS)        += ehci-exynos.o
> >  obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o
> >  obj-$(CONFIG_USB_EHCI_BRCMSTB)       += ehci-brcm.o
> > +obj-$(CONFIG_USB_EHCI_SUNPLUS)       += ehci-sunplus.o
> >
> >  obj-$(CONFIG_USB_OXU210HP_HCD)       += oxu210hp-hcd.o
> >  obj-$(CONFIG_USB_ISP116X_HCD)        += isp116x-hcd.o
> > diff --git a/drivers/usb/host/ehci-sunplus.c b/drivers/usb/host/ehci-sunplus.c
> > new file mode 100644
> > index 0000000..007923a
> > --- /dev/null
> > +++ b/drivers/usb/host/ehci-sunplus.c
> > @@ -0,0 +1,527 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * The EHCI driver for Sunplus SP7021
> > + *
> > + * Copyright (C) 2019 Sunplus Technology Inc., All rights reserved.
>
> You have not touched this file since 2019?
>
>
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/usb.h>
> > +#include <linux/usb/ehci_pdriver.h>
> > +#include <linux/usb/hcd.h>
> > +
> > +#include "ehci.h"
> > +
> > +#define RF_MASK_V(mask, val)                 (((mask) << 16) | (val))
> > +#define RF_MASK_V_CLR(mask)                  (((mask) << 16) | 0)
> > +
> > +#define USB_PORT0_ID                         0
> > +#define USB_PORT1_ID                         1
> > +#define USB_PORT_NUM                         2
> > +
> > +#define MASK_BITS                            0xffff
> > +
> > +#define OTP_DISC_LEVEL_DEFAULT                       0xd
> > +#define OTP_DISC_LEVEL_BITS                  0x5
> > +
> > +// GROUP 140/150 UPHY0/UPHY1
> > +#define CONFIG1                                      0x4
> > +#define J_HS_TX_PWRSAV                               BIT(5)
> > +#define CONFIG3                                      0xc
> > +#define J_FORCE_DISC_ON                              BIT(5)
> > +#define J_DEBUG_CTRL_ADDR_MACRO                      BIT(0)
> > +#define CONFIG7                                      0x1c
> > +#define J_DISC                                       0X1f
> > +#define CONFIG9                                      0x24
> > +#define J_ECO_PATH                           BIT(6)
> > +#define CONFIG16                             0x40
> > +#define J_TBCWAIT_MASK                               GENMASK(6, 5)
> > +#define J_TBCWAIT_1P1_MS                     FIELD_PREP(J_TBCWAIT_MASK, 0)
> > +#define J_TVDM_SRC_DIS_MASK                  GENMASK(4, 3)
> > +#define J_TVDM_SRC_DIS_8P2_MS                        FIELD_PREP(J_TVDM_SRC_DIS_MASK, 3)
> > +#define J_TVDM_SRC_EN_MASK                   GENMASK(2, 1)
> > +#define J_TVDM_SRC_EN_1P6_MS                 FIELD_PREP(J_TVDM_SRC_EN_MASK, 0)
> > +#define J_BC_EN                                      BIT(0)
> > +#define CONFIG17                             0x44
> > +#define IBG_TRIM0_MASK                               GENMASK(7, 5)
> > +#define IBG_TRIM0_SSLVHT                     FIELD_PREP(IBG_TRIM0_MASK, 4)
> > +#define J_VDATREE_TRIM_MASK                  GENMASK(4, 1)
> > +#define J_VDATREE_TRIM_DEFAULT                       FIELD_PREP(J_VDATREE_TRIM_MASK, 9)
> > +#define CONFIG23                             0x5c
> > +#define PROB_MASK                            GENMASK(5, 3)
> > +#define PROB                                 FIELD_PREP(PROB_MASK, 7)
> > +
> > +// GROUP 0 MOON 0
> > +#define HARDWARE_RESET_CONTROL2                      0x5c
> > +#define UPHY1_RESET                          BIT(14)
> > +#define UPHY0_RESET                          BIT(13)
> > +#define USBC1_RESET                          BIT(11)
> > +#define USBC0_RESET                          BIT(10)
> > +
> > +// GROUP 4 MOON 4
> > +#define USBC_CONTROL                         0x44
> > +#define MO1_USBC1_USB0_SEL                   BIT(13)
> > +#define MO1_USBC1_USB0_CTRL                  BIT(12)
> > +#define MO1_USBC0_USB0_SEL                   BIT(5)
> > +#define MO1_USBC0_USB0_CTRL                  BIT(4)
> > +#define UPHY0_CONTROL0                               0x48
> > +#define UPHY0_CONTROL1                               0x4c
> > +#define UPHY0_CONTROL2                               0x50
> > +#define MO1_UPHY0_RX_CLK_SEL                 BIT(6)
> > +#define UPHY0_CONTROL3                               0x54
> > +#define MO1_UPHY0_PLL_POWER_OFF_SEL          BIT(7)
> > +#define MO1_UPHY0_PLL_POWER_OFF                      BIT(3)
> > +#define UPHY1_CONTROL0                               0x58
> > +#define UPHY1_CONTROL1                               0x5c
> > +#define UPHY1_CONTROL2                               0x60
> > +#define MO1_UPHY1_RX_CLK_SEL                 BIT(6)
> > +#define UPHY1_CONTROL3                               0x64
> > +#define MO1_UPHY1_PLL_POWER_OFF_SEL          BIT(7)
> > +#define MO1_UPHY1_PLL_POWER_OFF                      BIT(3)
> > +
> > +#define hcd_to_sp_ehci_priv(h) \
> > +             ((struct sp_ehci_priv *)hcd_to_ehci(h)->priv)
> > +
> > +struct sp_ehci_priv {
> > +     struct resource *uphy_res_mem[USB_PORT_NUM];
> > +     struct resource *moon0_res_mem;
> > +     struct resource *moon4_res_mem;
> > +     struct clk *uphy_clk[USB_PORT_NUM];
> > +     struct clk *ehci_clk[USB_PORT_NUM];
> > +     void __iomem *uphy_regs[USB_PORT_NUM];
> > +     void __iomem *moon0_regs;
> > +     void __iomem *moon4_regs;
> > +};
> > +
> > +static int ehci_sunplus_reset(struct usb_hcd *hcd)
> > +{
> > +     struct platform_device *pdev = to_platform_device(hcd->self.controller);
> > +     struct usb_ehci_pdata *pdata = pdev->dev.platform_data;
> > +     struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > +     int retval;
> > +
> > +     hcd->has_tt = pdata->has_tt;
> > +     ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
> > +     ehci->big_endian_desc = pdata->big_endian_desc;
> > +     ehci->big_endian_mmio = pdata->big_endian_mmio;
> > +
> > +     ehci->caps = hcd->regs + pdata->caps_offset;
> > +     retval = ehci_setup(hcd);
> > +
> > +     return retval;
> > +}
> > +
> > +static struct hc_driver __read_mostly ehci_sunplus_driver;
> > +
> > +static const struct ehci_driver_overrides ehci_sunplus_overrides __initconst = {
> > +     .reset =                ehci_sunplus_reset,
> > +     .extra_priv_size =      sizeof(struct sp_ehci_priv),
> > +};
> > +
> > +static int uphy0_init(struct platform_device *pdev, struct sp_ehci_priv *sp_priv)
> > +{
> > +     struct nvmem_cell *cell;
> > +     char *disc_name = "disc_vol";
> > +     ssize_t otp_l = 0;
> > +     char *otp_v;
> > +     int port = pdev->id - 1;
> > +     u32 val, set, pll_pwr_on, pll_pwr_off;
> > +
> > +     /* Default value modification */
> > +     writel(RF_MASK_V(MASK_BITS, 0x4002), sp_priv->moon4_regs + UPHY0_CONTROL0);
> > +     writel(RF_MASK_V(MASK_BITS, 0x8747), sp_priv->moon4_regs + UPHY0_CONTROL1);
> > +
> > +     /* PLL power off/on twice */
> > +     pll_pwr_off = (readl(sp_priv->moon4_regs + UPHY0_CONTROL3) & ~MASK_BITS)
> > +                     | MO1_UPHY0_PLL_POWER_OFF_SEL | MO1_UPHY0_PLL_POWER_OFF;
> > +     pll_pwr_on = (readl(sp_priv->moon4_regs + UPHY0_CONTROL3) & ~MASK_BITS)
> > +                     | MO1_UPHY0_PLL_POWER_OFF_SEL;
> > +
> > +     writel(RF_MASK_V(MASK_BITS, pll_pwr_off), sp_priv->moon4_regs + UPHY0_CONTROL3);
> > +     mdelay(1);
> > +     writel(RF_MASK_V(MASK_BITS, pll_pwr_on), sp_priv->moon4_regs + UPHY0_CONTROL3);
> > +     mdelay(1);
> > +     writel(RF_MASK_V(MASK_BITS, pll_pwr_off), sp_priv->moon4_regs + UPHY0_CONTROL3);
> > +     mdelay(1);
> > +     writel(RF_MASK_V(MASK_BITS, pll_pwr_on), sp_priv->moon4_regs + UPHY0_CONTROL3);
> > +     mdelay(1);
> > +     writel(RF_MASK_V(MASK_BITS, 0x0), sp_priv->moon4_regs + UPHY0_CONTROL3);
> > +
>
> Why not just do a read after a write so that you know that is the
> correct delay?  Doing a sleep like this seems slow and possibly wrong.
>
> > +     /* reset UPHY0 */
> > +     writel(RF_MASK_V(UPHY0_RESET, UPHY0_RESET), sp_priv->moon0_regs + HARDWARE_RESET_CONTROL2);
> > +     writel(RF_MASK_V_CLR(UPHY0_RESET), sp_priv->moon0_regs + HARDWARE_RESET_CONTROL2);
> > +     mdelay(1);
>
> Again, read()?
>
> > +
> > +     /* board uphy 0 internal register modification for tid certification */
> > +     cell = nvmem_cell_get(&pdev->dev, disc_name);
> > +     if (IS_ERR_OR_NULL(cell)) {
> > +             if (PTR_ERR(cell) == -EPROBE_DEFER)
> > +                     return -EPROBE_DEFER;
> > +     }
> > +
> > +     otp_v = nvmem_cell_read(cell, &otp_l);
> > +     nvmem_cell_put(cell);
> > +
> > +     if (otp_v)
> > +             set = ((*otp_v >> OTP_DISC_LEVEL_BITS) |
> > +                     (*(otp_v + 1) << (sizeof(char) * 8 - OTP_DISC_LEVEL_BITS))) & J_DISC;
> > +
> > +     if (!otp_v || set == 0)
> > +             set = OTP_DISC_LEVEL_DEFAULT;
> > +
> > +     val = readl(sp_priv->uphy_regs[port] + CONFIG7);
> > +     val = (val & ~J_DISC) | set;
> > +     writel(val, sp_priv->uphy_regs[port] + CONFIG7);
> > +
> > +     val = readl(sp_priv->uphy_regs[port] + CONFIG9);
> > +     val &= ~(J_ECO_PATH);
> > +     writel(val, sp_priv->uphy_regs[port] + CONFIG9);
> > +
> > +     val = readl(sp_priv->uphy_regs[port] + CONFIG1);
> > +     val &= ~(J_HS_TX_PWRSAV);
> > +     writel(val, sp_priv->uphy_regs[port] + CONFIG1);
> > +
> > +     val = readl(sp_priv->uphy_regs[port] + CONFIG23);
> > +     val = (val & ~PROB) | PROB;
> > +     writel(val, sp_priv->uphy_regs[port] + CONFIG23);
> > +
> > +     /* USBC 0 reset */
> > +     writel(RF_MASK_V(USBC0_RESET, USBC0_RESET), sp_priv->moon0_regs + HARDWARE_RESET_CONTROL2);
> > +     writel(RF_MASK_V_CLR(USBC0_RESET), sp_priv->moon0_regs + HARDWARE_RESET_CONTROL2);
> > +
> > +     /* port 0 uphy clk fix */
> > +     writel(RF_MASK_V(MO1_UPHY0_RX_CLK_SEL, MO1_UPHY0_RX_CLK_SEL),
> > +            sp_priv->moon4_regs + UPHY0_CONTROL2);
> > +
> > +     /* switch to host */
> > +     writel(RF_MASK_V(MO1_USBC0_USB0_SEL | MO1_USBC0_USB0_CTRL,
> > +                      MO1_USBC0_USB0_SEL | MO1_USBC0_USB0_CTRL),
> > +            sp_priv->moon4_regs + USBC_CONTROL);
> > +
> > +     /* battery charger */
> > +     writel(J_TBCWAIT_1P1_MS | J_TVDM_SRC_DIS_8P2_MS | J_TVDM_SRC_EN_1P6_MS | J_BC_EN,
> > +            sp_priv->uphy_regs[port] + CONFIG16);
> > +     writel(IBG_TRIM0_SSLVHT | J_VDATREE_TRIM_DEFAULT, sp_priv->uphy_regs[port] + CONFIG17);
> > +     writel(J_FORCE_DISC_ON | J_DEBUG_CTRL_ADDR_MACRO, sp_priv->uphy_regs[port] + CONFIG3);
>
> What about all of these writes, are you sure they made it to the
> hardware ok?  No need to read?
>
> Same for elsewhere in the file.
>
> thanks,
>
> greg k-h




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux