mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Jan Luebbe <jlu@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH] ARM: mvebu: armada-xp: configure PLL and PHY register
Date: Wed, 16 Aug 2017 10:17:41 +0200	[thread overview]
Message-ID: <20170816081741.bcoctdw4dtai3tb4@pengutronix.de> (raw)
In-Reply-To: <20170815094737.27407-1-jlu@pengutronix.de>

Hallo Jan,

On Tue, Aug 15, 2017 at 11:47:37AM +0200, Jan Luebbe wrote:
> The PLL setup is needed to use the USB ports in Linux.
> 
> This code is ported from mainline U-Boot arch/arm/mach-mvebu/cpu.c.

Actually this is a work around because Linux doesn't do the necessary
setup before making use of the hardware, right?

If so, wouldn't it be better to teach Linux about the needed setup?

> +static void setup_usb_phys(void)
> +{
> +	int dev;
> +
> +	/*
> +	 * USB PLL init
> +	 */
> +
> +	/* Setup PLL frequency */
> +	/* USB REF frequency = 25 MHz */
> +	clrsetbits_le32(MV_USB_PHY_PLL_REG(1), 0x3ff, 0x605);
> +
> +	/* Power up PLL and PHY channel */
> +	setbits_le32(MV_USB_PHY_PLL_REG(2), BIT(9));
> + 
> +	/* Assert VCOCAL_START */
> +	setbits_le32(MV_USB_PHY_PLL_REG(1), BIT(21));
> + 
> +	mdelay(1);
> + 
> +	/*
> +	 * USB PHY init (change from defaults) specific for 40nm (78X30 78X60)

I assume this comment also comes from U-Boot. Still I wonder if it makes
sense to use the more common names instead of "78X30 78X60". That would
be "Armada XP" but not "Armada 370", wouldn't it? (Armada 370 = 88F6710,
88F6707, and 88F6W11). If so, this code shouldn't be reached on Armada
370?

Some other places in barebox suggest, that MV78xx0 is neither XP nor 370
(e.g.

	dts/Bindings/net/marvell-orion-mdio.txt
	dts/Bindings/arm/marvell/mvebu-system-controller.txt
	drivers/bus/Kconfig
	drivers/spi/mvebu_spi.c
)

> +	 */
> +
> +	for (dev = 0; dev < 3; dev++) {
> +		setbits_le32(MV_USB_X3_PHY_CHANNEL(dev, 3), BIT(15));
> +
> +		/* Assert REG_RCAL_START in channel REG 1 */
> +		setbits_le32(MV_USB_X3_PHY_CHANNEL(dev, 1), BIT(12));
> +		udelay(40);
> +		clrbits_le32(MV_USB_X3_PHY_CHANNEL(dev, 1), BIT(12));
> +	}

Can this be made quicker using:

	for (dev = 0; dev < 3; dev++) {
		setbits_le32(MV_USB_X3_PHY_CHANNEL(dev, 3), BIT(15));

		/* Assert REG_RCAL_START in channel REG 1 */
		setbits_le32(MV_USB_X3_PHY_CHANNEL(dev, 1), BIT(12));
	}

	udelay(40;

	for (dev = 0; dev < 3; dev++)
		clrbits_le32(MV_USB_X3_PHY_CHANNEL(dev, 1), BIT(12));

?

> +}
> +
>  static int armada_370_xp_init_soc(void)
>  {
>  	u32 reg;
> @@ -109,6 +151,9 @@ static int armada_370_xp_init_soc(void)
>  		reg = readl(ARMADA_XP_PUP_ENABLE);
>  		reg |= GE0_PUP_EN | GE1_PUP_EN | LCD_PUP_EN | NAND_PUP_EN | SPI_PUP_EN;
>  		writel(reg, ARMADA_XP_PUP_ENABLE);
> +
> +		/* Configure USB PLL and PHYs on AXP */
> +		setup_usb_phys();
>  	}
>  
>  	return 0;
> diff --git a/arch/arm/mach-mvebu/include/mach/armada-370-xp-regs.h b/arch/arm/mach-mvebu/include/mach/armada-370-xp-regs.h
> index 1dad05317211..b972df151a74 100644
> --- a/arch/arm/mach-mvebu/include/mach/armada-370-xp-regs.h
> +++ b/arch/arm/mach-mvebu/include/mach/armada-370-xp-regs.h
> @@ -72,4 +72,6 @@
>  	 (((port) % 4) * ARMADA_370_XP_PCIE_PORT_OFFSET))
>  #define  PCIE_DEVICE_VENDOR_ID		0x000
>  
> +#define ARMADA_370_XP_USB_BASE		(ARMADA_370_XP_INT_REGS_BASE + 0x50000)

Instead of hardcoding this address, this could be done depending on the
device at the same address specified in the device tree.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2017-08-16  8:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-15  9:47 Jan Luebbe
2017-08-16  8:17 ` Uwe Kleine-König [this message]
2017-08-16 10:51   ` Jan Lübbe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170816081741.bcoctdw4dtai3tb4@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=jlu@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox