2021-06-17 21:29:14

by Anand Moon

Subject: [RFCv1 1/8] phy: amlogic: meson8b-usb2: Use clock bulk to get clocks for phy

Use clock bulk helpers to get/enable/disable clocks,
it will be easier to handle clocks. No functional change intended.

Cc: Martin Blumenstingl <[email protected]>
Signed-off-by: Anand Moon <[email protected]>
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 44 ++++++++++++++------------
1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 03c061dd5f0d..771b73f3b44e 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -121,11 +121,16 @@ struct phy_meson8b_usb2_match_data {
bool host_enable_aca;
};

+static const char * const meson_phy_clks[] = {
+ "usb_general",
+ "usb",
+};
+
struct phy_meson8b_usb2_priv {
struct regmap *regmap;
enum usb_dr_mode dr_mode;
- struct clk *clk_usb_general;
- struct clk *clk_usb;
+ int num_clks;
+ struct clk_bulk_data *clks;
struct reset_control *reset;
const struct phy_meson8b_usb2_match_data *match;
};
@@ -151,16 +156,9 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
}
}

- ret = clk_prepare_enable(priv->clk_usb_general);
+ ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks);
if (ret) {
- dev_err(&phy->dev, "Failed to enable USB general clock\n");
- return ret;
- }
-
- ret = clk_prepare_enable(priv->clk_usb);
- if (ret) {
- dev_err(&phy->dev, "Failed to enable USB DDR clock\n");
- clk_disable_unprepare(priv->clk_usb_general);
+ dev_err(&phy->dev, "Failed to enable USB clock\n");
return ret;
}

@@ -197,8 +195,7 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
regmap_read(priv->regmap, REG_ADP_BC, &reg);
if (reg & REG_ADP_BC_ACA_PIN_FLOAT) {
dev_warn(&phy->dev, "USB ID detect failed!\n");
- clk_disable_unprepare(priv->clk_usb);
- clk_disable_unprepare(priv->clk_usb_general);
+ clk_bulk_disable_unprepare(priv->num_clks, priv->clks);
return -EINVAL;
}
}
@@ -216,8 +213,7 @@ static int phy_meson8b_usb2_power_off(struct phy *phy)
REG_DBG_UART_SET_IDDQ,
REG_DBG_UART_SET_IDDQ);

- clk_disable_unprepare(priv->clk_usb);
- clk_disable_unprepare(priv->clk_usb_general);
+ clk_bulk_disable_unprepare(priv->num_clks, priv->clks);

return 0;
}
@@ -234,6 +230,7 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
struct phy *phy;
struct phy_provider *phy_provider;
void __iomem *base;
+ int i, ret;

priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -252,13 +249,18 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);

- priv->clk_usb_general = devm_clk_get(&pdev->dev, "usb_general");
- if (IS_ERR(priv->clk_usb_general))
- return PTR_ERR(priv->clk_usb_general);
+ priv->num_clks = ARRAY_SIZE(meson_phy_clks);
+ priv->clks = devm_kcalloc(&pdev->dev, priv->num_clks,
+ sizeof(*priv->clks), GFP_KERNEL);
+ if (!priv->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < priv->num_clks; i++)
+ priv->clks[i].id = meson_phy_clks[i];

- priv->clk_usb = devm_clk_get(&pdev->dev, "usb");
- if (IS_ERR(priv->clk_usb))
- return PTR_ERR(priv->clk_usb);
+ ret = devm_clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks);
+ if (ret)
+ return ret;

priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
--
2.31.1

2021-06-18 02:46:50

by Martin Blumenstingl

Subject: Re: [RFCv1 1/8] phy: amlogic: meson8b-usb2: Use clock bulk to get clocks for phy

Hi Anand,

On Thu, Jun 17, 2021 at 9:42 PM Anand Moon <[email protected]> wrote:
[...]
> + int num_clks;
> + struct clk_bulk_data *clks;
personally I'd get rid of the num_clks as this value is static
instead I would add a #define with the number of clocks and then...

> struct reset_control *reset;
... use something like:
struct clk_bulk_data clks[MESON8B_USB2_PHY_NUM_CLKS];

[...]
> + dev_err(&phy->dev, "Failed to enable USB clock\n");
clock -> clocks

[...]
> + priv->num_clks = ARRAY_SIZE(meson_phy_clks);
> + priv->clks = devm_kcalloc(&pdev->dev, priv->num_clks,
> + sizeof(*priv->clks), GFP_KERNEL);
> + if (!priv->clks)
> + return -ENOMEM;
by using a fixed-size array as suggested above you don't need to
dynamically allocate memory anymore


Best regards,
Martin

2021-06-18 18:29:46

by Anand Moon

Subject: Re: [RFCv1 1/8] phy: amlogic: meson8b-usb2: Use clock bulk to get clocks for phy

Hi Martin,

On Fri, 18 Jun 2021 at 04:03, Martin Blumenstingl
<[email protected]> wrote:
>
> Hi Anand,
>
> On Thu, Jun 17, 2021 at 9:42 PM Anand Moon <[email protected]> wrote:
> [...]
> > + int num_clks;
> > + struct clk_bulk_data *clks;
> personally I'd get rid of the num_clks as this value is static
> instead I would add a #define with the number of clocks and then...
>
> > struct reset_control *reset;
> ... use something like:
> struct clk_bulk_data clks[MESON8B_USB2_PHY_NUM_CLKS];
>
> [...]
> > + dev_err(&phy->dev, "Failed to enable USB clock\n");
> clock -> clocks
>
> [...]
> > + priv->num_clks = ARRAY_SIZE(meson_phy_clks);
> > + priv->clks = devm_kcalloc(&pdev->dev, priv->num_clks,
> > + sizeof(*priv->clks), GFP_KERNEL);
> > + if (!priv->clks)
> > + return -ENOMEM;
> by using a fixed-size array as suggested above you don't need to
> dynamically allocate memory anymore
>
>

Thanks for this tip.It works for me.

> Best regards,
> Martin

Thanks

-Anand