diff mbox series

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

Message ID 20210617194154.2397-2-linux.amoon@gmail.com (mailing list archive)
State New, archived
Headers show
Series Meson-8b and Meson-gxbb USB phy code re-structure | expand

Commit Message

Anand Moon June 17, 2021, 7:41 p.m. UTC 
Use clock bulk helpers to get/enable/disable clocks,
it will be easier to handle clocks. No functional change intended.

Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/phy/amlogic/phy-meson8b-usb2.c | 44 ++++++++++++++------------
 1 file changed, 23 insertions(+), 21 deletions(-)

Comments

Martin Blumenstingl June 17, 2021, 10:33 p.m. UTC | #1 
Hi Anand,

On Thu, Jun 17, 2021 at 9:42 PM Anand Moon <linux.amoon@gmail.com> 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
Anand Moon June 18, 2021, 3:32 p.m. UTC | #2 
Hi Martin,

On Fri, 18 Jun 2021 at 04:03, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Thu, Jun 17, 2021 at 9:42 PM Anand Moon <linux.amoon@gmail.com> 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
diff mbox series

Patch

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)