Re: [PATCH] phy: add Rockchip RK3288 USB2 PHY driver.

["Heiko Stübner" <heiko@sntech.de>]

Hi Roy,

Am Mittwoch, 3. Dezember 2014, 21:46:50 schrieb LiYunzhi:
> From: lyz <lyz@rock-chips.com>
> 
> Add a driver for the Rockchip SoC internal USB2.0 PHY.
> This driver currently support RK3288.
> 
> Signed-off-by: lyz <lyz@rock-chips.com>
> ---

[...]

> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> new file mode 100644
> index 0000000..2586b76
> --- /dev/null
> +++ b/drivers/phy/phy-rockchip-usb.c
> @@ -0,0 +1,179 @@
> +/*
> + * Rockchip usb PHY driver
> + *
> + * Copyright (C) 2014 Roy Li <lyz@rock-chips.com>
> + * Copyright (C) 2014 ROCKCHIP, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +#define ROCKCHIP_RK3288_UOC(n)	(0x320 + n * 0x14)
> +
> +#define SIDDQ_MSK		(1 << (13 + 16))
> +#define SIDDQ_ON		(1 << 13)
> +#define SIDDQ_OFF		(0 << 13)

In the rockchip clock driver [in drivers/clk/rockchip/clk.h] exist a macro 
HIWORD_UPDATE that removes the need to declare the write-enable bits 
separately.


> +
> +enum rk3288_phy_id {
> +	RK3288_OTG,
> +	RK3288_HOST0,
> +	RK3288_HOST1,
> +	RK3288_NUM_PHYS,
> +};
> +
> +struct rockchip_usb_phy {
> +	struct regmap *reg_base;
> +	unsigned int reg_offset;
> +	struct clk *clk;
> +	struct phy *phy;
> +};
> +
> +static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
> +					   bool siddq)
> +{
> +	return regmap_write(phy->reg_base, phy->reg_offset,
> +			    SIDDQ_MSK | (siddq ? SIDDQ_ON : SIDDQ_OFF));

just for my understanding:

You're using the SIDDQ bit, which supposedly "powers down all analog blocks" 
for IDDQ testing to control the phy power.

What is the difference to usbotg_disable (bit 4 of uoc_con0) that is supposed 
to "power down the USB OTG/HOST block"?

Similarly, where is the difference to usbotg_sleepm [uoc_con2 bit 10] combined 
with usbotg_common_on_n [uoc_con0 bit 0]?


Thanks
Heiko