Re: [PATCH 2/2] reset: bcm4908-usb: add driver for BCM4908 USB reset controller

[Rafał Miłecki]

On 04.12.2020 17:38, Florian Fainelli wrote:
> On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@xxxxxxxxxx>
> >
> > This controller is responsible for OHCI, EHCI, XHCI and PHYs setup that
> > has to be handled in the proper order.
> >
> > One unusual thing about this controller is that is provides access to
> > the MDIO bus. There are two registers (in the middle of block space)
> > responsible for that. For that reason this driver initializes regmap so
> > a proper MDIO driver can use them.
> >
> > Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
> > ---
>
> [snip]
>
> > +
> > +#define BCM4908_USB_RESET_SETUP						0x0000
> > +#define  BCM4908_USBH_IPP						(1<<5)
> > +#define  BCM4908_USBH_IOC						(1<<4)
> > +#define  BCM4908_USB2_OC_DISABLE_PORT0					(1<<28)
> > +#define  BCM4908_USB2_OC_DISABLE_PORT1					(1<<29)
> > +#define  BCM4908_USB3_OC_DISABLE_PORT0					(1<<30)
> > +#define  BCM4908_USB3_OC_DISABLE_PORT1					(1<<31)
> > +#define BCM4908_USB_RESET_PLL_CTL					0x0004
> > +#define BCM4908_USB_RESET_FLADJ_VALUE					0x0008
> > +#define BCM4908_USB_RESET_BRIDGE_CTL					0x000c
> > +#define BCM4908_USB_RESET_SPARE1					0x0010
> > +#define BCM4908_USB_RESET_MDIO						0x0014
> > +#define BCM4908_USB_RESET_MDIO2						0x0018
> > +#define BCM4908_USB_RESET_TEST_PORT_CONTROL				0x001c
> > +#define BCM4908_USB_RESET_USB_SIMCTL					0x0020
> > +#define  BCM4908_USBH_OHCI_MEM_REQ_DIS					(1<<1)
> > +#define BCM4908_USB_RESET_USB_TESTCTL					0x0024
> > +#define BCM4908_USB_RESET_USB_TESTMON					0x0028
> > +#define BCM4908_USB_RESET_UTMI_CTL_1					0x002c
> > +#define BCM4908_USB_RESET_SPARE2					0x0030
> > +#define BCM4908_USB_RESET_USB_PM					0x0034
> > +#define  BCM4908_XHC_SOFT_RESETB					(1<<22)
> > +#define  BCM4908_USB_PWRDWN						(1<<31)
> > +#define BCM4908_USB_RESET_USB_PM_STATUS					0x0038
> > +#define BCM4908_USB_RESET_SPARE3					0x003c
> > +#define BCM4908_USB_RESET_PLL_LDO_CTL					0x0040
> > +#define BCM4908_USB_RESET_PLL_LDO_PLLBIAS				0x0044
> > +#define BCM4908_USB_RESET_PLL_AFE_BG_CNTL				0x0048
> > +#define BCM4908_USB_RESET_AFE_USBIO_TST					0x004c
> > +#define BCM4908_USB_RESET_PLL_NDIV_FRAC					0x0050
> > +#define BCM4908_USB_RESET_TP_DIAG					0x0054
> > +#define BCM4908_USB_RESET_AHB_CAPTURE_FIFO				0x0058
> > +#define BCM4908_USB_RESET_SPARE4					0x005c
> > +#define BCM4908_USB_RESET_USB30_CTL1					0x0060
> > +#define  BCM4908_PHY3_PLL_SEQ_START					(1<<4)
> > +#define BCM4908_USB_RESET_USB30_CTL2					0x0064
> > +#define BCM4908_USB_RESET_USB30_CTL3					0x0068
> > +#define BCM4908_USB_RESET_USB30_CTL4					0x006c
> > +#define BCM4908_USB_RESET_USB30_PCTL					0x0070
> > +#define BCM4908_USB_RESET_USB30_CTL5					0x0074
> > +#define BCM4908_USB_RESET_SPARE5					0x0078
> > +#define BCM4908_USB_RESET_SPARE6					0x007c
> > +#define BCM4908_USB_RESET_SPARE7					0x0080
> > +#define BCM4908_USB_RESET_USB_DEVICE_CTL1				0x0090
> > +#define BCM4908_USB_RESET_USB_DEVICE_CTL2				0x0094
> > +#define BCM4908_USB_RESET_USB20_ID					0x0150
> > +#define BCM4908_USB_RESET_USB30_ID					0x0154
> > +#define BCM4908_USB_RESET_BDC_COREID					0x0158
> > +#define BCM4908_USB_RESET_USB_REVID					0x015c
>
> This register layout is nearly identical to the one described under
> drivers/phy/broadcom/phy-brcm-usb-init.c and this is because within
> Broadcom the same design group has been supplying the USB PHY and host
> controllers to the DSL and STB product lines.
>
> I would model this the same way we have done it for the Broadcom STB HCI
> drivers and add a separate compatible string along with an optional
> reset line.
>
> As far as MDIO goes as you can see the USB PHY driver uses a mix of
> memory mapped and MDIO accesses (eye fix, etc.) so it was deemed cleaner
> to not use the MDIO subsystem for the very few accesses that are required.
>
> This is different from the Northstar platform you have been working on
> where the USB PHYs are not memory maapped at all and only accessible
> over MDIO.
>
> Let me know if you think the existing driver would not be extensible to
> support 4908.

I see some / many similarities in that PHY driver, I'll try to reuse it, thanks!