Re: [PATCH v2 05/10] phy: add A3700 UTMI PHY driver

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

 



Hi Gregory,

Gregory CLEMENT <gregory.clement@xxxxxxxxxxx> wrote on Fri, 18 Jan 2019
17:36:36 +0100:

> Hi Miquel,
> 
> I have only a few smallish remarks:
> 
>  On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> 
> > Marvell Armada 3700 SoC has two USB controllers, each of them being
> > wired to an internal UTMI PHY. Add a driver to control them.
> >
> > Igal Liberman worked on supporting the PHY, I took the while 'register
> > configuration' from his work and rewrote almost entirely the
> > driver/bindings around it.
> >
> > Co-developed-by: Igal Liberman <igall@xxxxxxxxxxx>
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > Signed-off-by: Igal Liberman <igall@xxxxxxxxxxx>
> > ---  
> [...]
> 
> > diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> > new file mode 100644
> > index 000000000000..97d8235d661d
> > --- /dev/null
> > +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> > @@ -0,0 +1,297 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Marvell
> > + *
> > + * Authors:
> > + *   Evan Wang <xswang@xxxxxxxxxxx>
> > + *   Miquèl Raynal <miquel.raynal@xxxxxxxxxxx>  
> 
> If it is co-developed with Igal Liberman, shouldn't be added here too?

In the original patch, the author in the commit log was Igal and the
MODULE_AUTHOR macro was saying Evan. I think the macro is wrong, I will
keep Igal.

> [...]
> 
> > +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> > +{
> > +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> > +	struct device *dev = &phy->dev;
> > +	int usb32 = utmi->caps->usb32;
> > +	int ret = 0;
> > +	u32 reg;
> > +
> > +	if (!utmi)
> > +		return -ENODEV;
> > +
> > +	/*
> > +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.  
>                                                           bein?
> > +	 * See "PLL Settings for Typical REFCLK" table.  
> 
> [...]
> 
> > +
> > +	/* Wait for squetch calibration */  
>                     squetch?

Good catch. It's written "squelch" in the datasheet, but I don't know
what it means exactly.


Thanks,
Miquèl



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux