Re: [PATCH] phy: hisilicon: Adopt phy-hisi-inno-usb2 to Hi3798MV100

[Vinod Koul]

On 08-05-23, 19:39, Yangfl wrote:
> Vinod Koul <vkoul@xxxxxxxxxx> 于2023年5月8日周一 15:41写道：
> >
> > On 07-05-23, 23:46, David Yang wrote:
> > > Hisilicon also uses phy-hisi-inno-usb2 on Hi3798MV100, with a slightly
> > > different register convention.
> >
> > OK, so what should I expect from this patch, pls document that here...
> 
> Hi3798MV100 usb2 phy. Which kind of document should I put here?

Pls add the details on what is being changed in this patch. this is the
place where patch description is provided, what are all the things that
are getting changed

> 
> >
> > >
> > > Signed-off-by: David Yang <mmyangfl@xxxxxxxxx>
> > > ---
> > >  drivers/phy/hisilicon/Kconfig              |  2 +-
> > >  drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 67 ++++++++++++++++------
> > >  2 files changed, 51 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
> > > index d3b92c288554..6c89136fc8c2 100644
> > > --- a/drivers/phy/hisilicon/Kconfig
> > > +++ b/drivers/phy/hisilicon/Kconfig
> > > @@ -54,7 +54,7 @@ config PHY_HISTB_COMBPHY
> > >
> > >  config PHY_HISI_INNO_USB2
> > >       tristate "HiSilicon INNO USB2 PHY support"
> > > -     depends on (ARCH_HISI && ARM64) || COMPILE_TEST
> > > +     depends on ARCH_HISI || COMPILE_TEST
> >
> > why this change?
> 
> Hi3798MV100 is a A9 ARM32 only soc.

OK pls document that, also suspect that might a separate patch..?

> 
> >
> > >       select GENERIC_PHY
> > >       select MFD_SYSCON
> > >       help
> > > diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> > > index b133ae06757a..b5d006f38934 100644
> > > --- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> > > +++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> > > @@ -9,7 +9,7 @@
> > >  #include <linux/delay.h>
> > >  #include <linux/io.h>
> > >  #include <linux/module.h>
> > > -#include <linux/platform_device.h>
> > > +#include <linux/of_device.h>
> > >  #include <linux/phy/phy.h>
> > >  #include <linux/reset.h>
> > >
> > > @@ -20,12 +20,28 @@
> > >  #define PHY_CLK_STABLE_TIME  2       /* unit:ms */
> > >  #define UTMI_RST_COMPLETE_TIME       2       /* unit:ms */
> > >  #define POR_RST_COMPLETE_TIME        300     /* unit:us */
> > > -#define PHY_TEST_DATA                GENMASK(7, 0)
> > > -#define PHY_TEST_ADDR                GENMASK(15, 8)
> > > -#define PHY_TEST_PORT                GENMASK(18, 16)
> > > -#define PHY_TEST_WREN                BIT(21)
> > > -#define PHY_TEST_CLK         BIT(22) /* rising edge active */
> > > -#define PHY_TEST_RST         BIT(23) /* low active */
> > > +
> > > +#define PHY_TYPE_0   0
> > > +#define PHY_TYPE_1   1
> > > +
> > > +#define PHY0_TEST_DATA               GENMASK(7, 0)
> >
> > same as previous what changed??
> 
> Register convention for PHY1 below, as mentioned in commit message.

It is _exactly_ same as previous, so pls dont introduce unnecessary
changes..

-- 
~Vinod