On Tue, Jul 06, 2021 at 10:10:54AM +0900, Jaehoon Chung wrote: > Hi Stephan, > > On 7/6/21 1:28 AM, Stephan Gerhold wrote: > > The AB8500 PMIC contains an USB PHY that needs to be set up in > > device or host mode to make USB work properly. Add a simple driver > > for the generic PHY uclass that allows enabling it. > > > > The if (CONFIG_IS_ENABLED(USB_MUSB_HOST)) might be a bit strange. > > The USB PHY must be configured in either host or device mode and > > somehow the USB PHY driver must be made aware of the mode. > > > > Actually, the MUSB driver used together with this PHY does not > > support dynamic selection of host/device mode in U-Boot at the moment. > > Therefore, one very simple approach that works fine is to select > > the mode to configure at compile time. When the MUSB driver is > > configured in host mode the PHY is configured in host mode, and > > similarly when the MUSB driver is configured in device/gadget mode. > > > > Cc: Linus Walleij <linus.wall...@linaro.org> > > Signed-off-by: Stephan Gerhold <step...@gerhold.net> > > --- > > > > Better suggestions to make the USB PHY driver aware of the mode > > are very welcome. :) I'm not sure it's worth it though, the compile > > time selection is not ideal but it does the job just fine for now. > > --- > > drivers/phy/Kconfig | 6 ++++ > > drivers/phy/Makefile | 1 + > > drivers/phy/phy-ab8500-usb.c | 54 ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 61 insertions(+) > > create mode 100644 drivers/phy/phy-ab8500-usb.c > > > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > > index 008186a10d..92c74b9d0b 100644 > > --- a/drivers/phy/Kconfig > > +++ b/drivers/phy/Kconfig > > @@ -64,6 +64,12 @@ config MIPI_DPHY_HELPERS > > help > > Provides a number of helpers a core functions for MIPI D-PHY drivers. > > > > +config AB8500_USB_PHY > > + bool "AB8500 USB PHY Driver" > > + depends on PHY && PMIC_AB8500 > > + help > > + Support for the USB OTG PHY in ST-Ericsson AB8500. > > + > > config BCM6318_USBH_PHY > > bool "BCM6318 USBH PHY support" > > depends on PHY && ARCH_BMIPS > > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > > index 3c4a673a83..bf03d05d9b 100644 > > --- a/drivers/phy/Makefile > > +++ b/drivers/phy/Makefile > > @@ -6,6 +6,7 @@ > > obj-$(CONFIG_$(SPL_)PHY) += phy-uclass.o > > obj-$(CONFIG_$(SPL_)NOP_PHY) += nop-phy.o > > obj-$(CONFIG_MIPI_DPHY_HELPERS) += phy-core-mipi-dphy.o > > +obj-$(CONFIG_AB8500_USB_PHY) += phy-ab8500-usb.o > > obj-$(CONFIG_BCM6318_USBH_PHY) += bcm6318-usbh-phy.o > > obj-$(CONFIG_BCM6348_USBH_PHY) += bcm6348-usbh-phy.o > > obj-$(CONFIG_BCM6358_USBH_PHY) += bcm6358-usbh-phy.o > > diff --git a/drivers/phy/phy-ab8500-usb.c b/drivers/phy/phy-ab8500-usb.c > > new file mode 100644 > > index 0000000000..4ecb03a9cb > > --- /dev/null > > +++ b/drivers/phy/phy-ab8500-usb.c > > @@ -0,0 +1,54 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* Copyright (C) 2019 Stephan Gerhold */ > > + > > +#include <common.h> > > +#include <dm.h> > > +#include <generic-phy.h> > > +#include <linux/bitops.h> > > +#include <power/pmic.h> > > +#include <power/ab8500.h> > > + > > +#define AB8500_USB_PHY_CTRL_REG AB8500_USB(0x8A) > > +#define AB8500_BIT_PHY_CTRL_HOST_EN BIT(0) > > +#define AB8500_BIT_PHY_CTRL_DEVICE_EN BIT(1) > > +#define AB8500_USB_PHY_CTRL_MASK (AB8500_BIT_PHY_CTRL_HOST_EN |\ > > + AB8500_BIT_PHY_CTRL_DEVICE_EN) > > + > > +static int ab8500_usb_phy_power_on(struct phy *phy) > > +{ > > + struct udevice *dev = phy->dev; > > + uint set; > > + > > unit set = AB8500_BIT_PHY_CTRL_DEVICE_EN; > > if (CONFIG_IS_ENABLED(USB)MUSB_HOST)) > set = ...; > > how about that? >
Both looks fine to me, I can change it in v2. > > + if (CONFIG_IS_ENABLED(USB_MUSB_HOST)) > > + set = AB8500_BIT_PHY_CTRL_HOST_EN; > > + else > > + set = AB8500_BIT_PHY_CTRL_DEVICE_EN; > > + > > + return pmic_clrsetbits(dev->parent, AB8500_USB_PHY_CTRL_REG, > > + AB8500_USB_PHY_CTRL_MASK, set); > > +} Thanks for the review! Stephan