Kishon & Heiko,
On Tue, Dec 15, 2015 at 2:53 AM, Kishon Vijay Abraham I <kishon at ti.com> wrote:
> Hi Mike,
>
> On Friday 20 November 2015 02:52 AM, Heiko Stuebner wrote:
>> The USB phys on Rockchip SoCs contain their own internal PLLs to create
>> the 480MHz needed. Additionally this PLL output is also fed back into the
>> core clock-controller as possible source for clocks like the GPU or others.
>>
>> Until now this was modelled incorrectly with a "virtual" factor clock in
>> the clock controller. The one big caveat is that if we turn off the usb phy
>> via the siddq signal, all analog components get turned off, including the
>> PLLs. It is therefore possible that a source clock gets disabled without
>> the clock driver ever knowing, possibly making the system hang.
>>
>> Therefore register the phy-plls as real clocks that the clock driver can
>> then reference again normally, making the clock hirarchy finally reflect
>> the actual hardware.
>>
>> The phy-ops get converted to simply turning that new clock on and off
>> which in turn controls the siddq signal of the phy.
>>
>> Through this the driver gains handling for platform-specific data, to
>> handle the phy->clock name association.
>
> Are you okay with this patch/ Can you give your Acked-by?
Acked-by: Michael Turquette <mturquette at baylibre.com>
>
> Thanks
> Kishon
>
>>
>> Signed-off-by: Heiko Stuebner <heiko at sntech.de>
>> Reviewed-by: Douglas Anderson <dianders at chromium.org>
>> ---
>> .../devicetree/bindings/phy/rockchip-usb-phy.txt | 1 +
>> drivers/phy/phy-rockchip-usb.c | 188 ++++++++++++++++++---
>> 2 files changed, 162 insertions(+), 27 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
>> index 9b37242..68498d5 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
>> @@ -24,6 +24,7 @@ required properties:
>> Optional Properties:
>> - clocks : phandle + clock specifier for the phy clocks
>> - clock-names: string, clock name, must be "phyclk"
>> +- #clock-cells: for users of the phy-pll, should be 0
>>
>> Example:
>>
>> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
>> index 16cd533..33a80eb 100644
>> --- a/drivers/phy/phy-rockchip-usb.c
>> +++ b/drivers/phy/phy-rockchip-usb.c
>> @@ -15,12 +15,14 @@
>> */
>>
>> #include <linux/clk.h>
>> +#include <linux/clk-provider.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/of_platform.h>
>> #include <linux/phy/phy.h>
>> #include <linux/platform_device.h>
>> #include <linux/regulator/consumer.h>
>> @@ -36,15 +38,28 @@
>> #define SIDDQ_ON BIT(13)
>> #define SIDDQ_OFF (0 << 13)
>>
>> +struct rockchip_usb_phys {
>> + int reg;
>> + const char *pll_name;
>> +};
>> +
>> +struct rockchip_usb_phy_pdata {
>> + struct rockchip_usb_phys *phys;
>> +};
>> +
>> struct rockchip_usb_phy_base {
>> struct device *dev;
>> struct regmap *reg_base;
>> + const struct rockchip_usb_phy_pdata *pdata;
>> };
>>
>> struct rockchip_usb_phy {
>> struct rockchip_usb_phy_base *base;
>> + struct device_node *np;
>> unsigned int reg_offset;
>> struct clk *clk;
>> + struct clk *clk480m;
>> + struct clk_hw clk480m_hw;
>> struct phy *phy;
>> };
>>
>> @@ -55,17 +70,59 @@ static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
>> SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
>> }
>>
>> -static int rockchip_usb_phy_power_off(struct phy *_phy)
>> +static unsigned long rockchip_usb_phy480m_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> {
>> - struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
>> - int ret = 0;
>> + return 480000000;
>> +}
>> +
>> +static void rockchip_usb_phy480m_disable(struct clk_hw *hw)
>> +{
>> + struct rockchip_usb_phy *phy = container_of(hw,
>> + struct rockchip_usb_phy,
>> + clk480m_hw);
>>
>> /* Power down usb phy analog blocks by set siddq 1 */
>> - ret = rockchip_usb_phy_power(phy, 1);
>> - if (ret)
>> + rockchip_usb_phy_power(phy, 1);
>> +}
>> +
>> +static int rockchip_usb_phy480m_enable(struct clk_hw *hw)
>> +{
>> + struct rockchip_usb_phy *phy = container_of(hw,
>> + struct rockchip_usb_phy,
>> + clk480m_hw);
>> +
>> + /* Power up usb phy analog blocks by set siddq 0 */
>> + return rockchip_usb_phy_power(phy, 0);
>> +}
>> +
>> +static int rockchip_usb_phy480m_is_enabled(struct clk_hw *hw)
>> +{
>> + struct rockchip_usb_phy *phy = container_of(hw,
>> + struct rockchip_usb_phy,
>> + clk480m_hw);
>> + int ret;
>> + u32 val;
>> +
>> + ret = regmap_read(phy->base->reg_base, phy->reg_offset, &val);
>> + if (ret < 0)
>> return ret;
>>
>> - clk_disable_unprepare(phy->clk);
>> + return (val & SIDDQ_ON) ? 0 : 1;
>> +}
>> +
>> +static const struct clk_ops rockchip_usb_phy480m_ops = {
>> + .enable = rockchip_usb_phy480m_enable,
>> + .disable = rockchip_usb_phy480m_disable,
>> + .is_enabled = rockchip_usb_phy480m_is_enabled,
>> + .recalc_rate = rockchip_usb_phy480m_recalc_rate,
>> +};
>> +
>> +static int rockchip_usb_phy_power_off(struct phy *_phy)
>> +{
>> + struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
>> +
>> + clk_disable_unprepare(phy->clk480m);
>>
>> return 0;
>> }
>> @@ -73,20 +130,8 @@ static int rockchip_usb_phy_power_off(struct phy *_phy)
>> static int rockchip_usb_phy_power_on(struct phy *_phy)
>> {
>> struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
>> - int ret = 0;
>>
>> - ret = clk_prepare_enable(phy->clk);
>> - if (ret)
>> - return ret;
>> -
>> - /* Power up usb phy analog blocks by set siddq 0 */
>> - ret = rockchip_usb_phy_power(phy, 0);
>> - if (ret) {
>> - clk_disable_unprepare(phy->clk);
>> - return ret;
>> - }
>> -
>> - return 0;
>> + return clk_prepare_enable(phy->clk480m);
>> }
>>
>> static const struct phy_ops ops = {
>> @@ -99,6 +144,9 @@ static void rockchip_usb_phy_action(void *data)
>> {
>> struct rockchip_usb_phy *rk_phy = data;
>>
>> + of_clk_del_provider(rk_phy->np);
>> + clk_unregister(rk_phy->clk480m);
>> +
>> if (rk_phy->clk)
>> clk_put(rk_phy->clk);
>> }
>> @@ -108,13 +156,16 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
>> {
>> struct rockchip_usb_phy *rk_phy;
>> unsigned int reg_offset;
>> - int err;
>> + const char *clk_name;
>> + struct clk_init_data init;
>> + int err, i;
>>
>> rk_phy = devm_kzalloc(base->dev, sizeof(*rk_phy), GFP_KERNEL);
>> if (!rk_phy)
>> return -ENOMEM;
>>
>> rk_phy->base = base;
>> + rk_phy->np = child;
>>
>> if (of_property_read_u32(child, "reg", ®_offset)) {
>> dev_err(base->dev, "missing reg property in node %s\n",
>> @@ -124,14 +175,54 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
>>
>> rk_phy->reg_offset = reg_offset;
>>
>> - err = devm_add_action(base->dev, rockchip_usb_phy_action, rk_phy);
>> - if (err)
>> - return err;
>> -
>> rk_phy->clk = of_clk_get_by_name(child, "phyclk");
>> if (IS_ERR(rk_phy->clk))
>> rk_phy->clk = NULL;
>>
>> + i = 0;
>> + init.name = NULL;
>> + while (base->pdata->phys[i].reg) {
>> + if (base->pdata->phys[i].reg == reg_offset) {
>> + init.name = base->pdata->phys[i].pll_name;
>> + break;
>> + }
>> + i++;
>> + }
>> +
>> + if (!init.name) {
>> + dev_err(base->dev, "phy data not found\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (rk_phy->clk) {
>> + clk_name = __clk_get_name(rk_phy->clk);
>> + init.flags = 0;
>> + init.parent_names = &clk_name;
>> + init.num_parents = 1;
>> + } else {
>> + init.flags = CLK_IS_ROOT;
>> + init.parent_names = NULL;
>> + init.num_parents = 0;
>> + }
>> +
>> + init.ops = &rockchip_usb_phy480m_ops;
>> + rk_phy->clk480m_hw.init = &init;
>> +
>> + rk_phy->clk480m = clk_register(base->dev, &rk_phy->clk480m_hw);
>> + if (IS_ERR(rk_phy->clk480m)) {
>> + err = PTR_ERR(rk_phy->clk480m);
>> + goto err_clk;
>> + }
>> +
>> + err = of_clk_add_provider(child, of_clk_src_simple_get,
>> + rk_phy->clk480m);
>> + if (err < 0)
>> + goto err_clk_prov;
>> +
>> + err = devm_add_action(base->dev, rockchip_usb_phy_action, rk_phy);
>> + if (err)
>> + goto err_devm_action;
>> +
>> rk_phy->phy = devm_phy_create(base->dev, child, &ops);
>> if (IS_ERR(rk_phy->phy)) {
>> dev_err(base->dev, "failed to create PHY\n");
>> @@ -141,13 +232,48 @@ static int rockchip_usb_phy_init(struct rockchip_usb_phy_base *base,
>>
>> /* only power up usb phy when it use, so disable it when init*/
>> return rockchip_usb_phy_power(rk_phy, 1);
>> +
>> +err_devm_action:
>> + of_clk_del_provider(child);
>> +err_clk_prov:
>> + clk_unregister(rk_phy->clk480m);
>> +err_clk:
>> + if (rk_phy->clk)
>> + clk_put(rk_phy->clk);
>> + return err;
>> }
>>
>> +static const struct rockchip_usb_phy_pdata rk3066a_pdata = {
>> + .phys = (struct rockchip_usb_phys[]){
>> + { .reg = 0x17c, .pll_name = "sclk_otgphy0_480m" },
>> + { .reg = 0x188, .pll_name = "sclk_otgphy1_480m" },
>> + { /* sentinel */ }
>> + },
>> +};
>> +
>> +static const struct rockchip_usb_phy_pdata rk3188_pdata = {
>> + .phys = (struct rockchip_usb_phys[]){
>> + { .reg = 0x10c, .pll_name = "sclk_otgphy0_480m" },
>> + { .reg = 0x11c, .pll_name = "sclk_otgphy1_480m" },
>> + { /* sentinel */ }
>> + },
>> +};
>> +
>> +static const struct rockchip_usb_phy_pdata rk3288_pdata = {
>> + .phys = (struct rockchip_usb_phys[]){
>> + { .reg = 0x320, .pll_name = "sclk_otgphy0_480m" },
>> + { .reg = 0x334, .pll_name = "sclk_otgphy1_480m" },
>> + { .reg = 0x348, .pll_name = "sclk_otgphy2_480m" },
>> + { /* sentinel */ }
>> + },
>> +};
>> +
>> static int rockchip_usb_phy_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> struct rockchip_usb_phy_base *phy_base;
>> struct phy_provider *phy_provider;
>> + const struct of_device_id *match;
>> struct device_node *child;
>> int err;
>>
>> @@ -155,6 +281,14 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
>> if (!phy_base)
>> return -ENOMEM;
>>
>> + match = of_match_device(dev->driver->of_match_table, dev);
>> + if (!match || !match->data) {
>> + dev_err(dev, "missing phy data\n");
>> + return -EINVAL;
>> + }
>> +
>> + phy_base->pdata = match->data;
>> +
>> phy_base->dev = dev;
>> phy_base->reg_base = syscon_regmap_lookup_by_phandle(dev->of_node,
>> "rockchip,grf");
>> @@ -176,9 +310,9 @@ static int rockchip_usb_phy_probe(struct platform_device *pdev)
>> }
>>
>> static const struct of_device_id rockchip_usb_phy_dt_ids[] = {
>> - { .compatible = "rockchip,rk3066a-usb-phy" },
>> - { .compatible = "rockchip,rk3188-usb-phy" },
>> - { .compatible = "rockchip,rk3288-usb-phy" },
>> + { .compatible = "rockchip,rk3066a-usb-phy", .data = &rk3066a_pdata },
>> + { .compatible = "rockchip,rk3188-usb-phy", .data = &rk3188_pdata },
>> + { .compatible = "rockchip,rk3288-usb-phy", .data = &rk3288_pdata },
>> {}
>> };
>>
>>
--
Michael Turquette
CEO
BayLibre - At the Heart of Embedded Linux
http://baylibre.com/
