From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Andr=c3=a9_Przywara?= Date: Tue, 22 Jan 2019 23:36:58 +0000 Subject: [U-Boot] [PATCH v3 7/9] dm: mmc: sunxi: Add CLK and RESET support In-Reply-To: <20190121103115.17794-8-jagan@amarulasolutions.com> References: <20190121103115.17794-1-jagan@amarulasolutions.com> <20190121103115.17794-8-jagan@amarulasolutions.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 21/01/2019 10:31, Jagan Teki wrote: > Now CLK and RESET driver for Allwinner SoC are available, > so add the relevant operations on mmc sunxi driver. > > Cc: Jaehoon Chung > Signed-off-by: Jagan Teki > --- > Changes for v3: > - Grab changes for ML > > drivers/mmc/sunxi_mmc.c | 52 +++++++++++++++++++++++++++++++++++++---- > 1 file changed, 47 insertions(+), 5 deletions(-) > > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c > index 0c443a732d..3c17958c95 100644 > --- a/drivers/mmc/sunxi_mmc.c > +++ b/drivers/mmc/sunxi_mmc.c > @@ -8,10 +8,12 @@ > */ > > #include > +#include > #include > #include > #include > #include > +#include > #include > #include > #include > @@ -21,7 +23,6 @@ > > #ifdef CONFIG_DM_MMC > struct sunxi_mmc_variant { > - u16 gate_offset; > u16 mclk_offset; > }; > #endif > @@ -41,6 +42,8 @@ struct sunxi_mmc_priv { > struct mmc_config cfg; > #ifdef CONFIG_DM_MMC > const struct sunxi_mmc_variant *variant; > + struct clk clk_ahb; > + struct reset_ctl reset; Where is that used again outside of the probe() function? > #endif > }; > > @@ -602,6 +605,32 @@ static const struct dm_mmc_ops sunxi_mmc_ops = { > .get_cd = sunxi_mmc_getcd, > }; > > +static int sunxi_mmc_enable(struct udevice *dev) > +{ > + struct sunxi_mmc_priv *priv = dev_get_priv(dev); > + int ret; > + > + ret = clk_enable(&priv->clk_ahb); > + if (ret) { > + dev_err(dev, "failed to enable AHB clock\n"); > + return ret; > + } > + > + if (reset_valid(&priv->reset)) { > + ret = reset_deassert(&priv->reset); > + if (ret) { > + dev_err(dev, "failed to deassert reset\n"); > + goto err_clk_ahb; > + } > + } > + > + return 0; > + > +err_clk_ahb: > + clk_disable(&priv->clk_ahb); > + return ret; > +} > + Frankly, that's a lots of lines for very little effect. Why not just use the much smaller version I had and add an "else dev_err(...)" to it, if you are keen about error reports? Saves you the variables, saves you the reset_valid() call, saves you the extra function. Less code to read, smaller image, less bugs. Cheers, Andre. > static int sunxi_mmc_probe(struct udevice *dev) > { > struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); > @@ -609,7 +638,7 @@ static int sunxi_mmc_probe(struct udevice *dev) > struct sunxi_mmc_priv *priv = dev_get_priv(dev); > struct mmc_config *cfg = &plat->cfg; > struct ofnode_phandle_args args; > - u32 *gate_reg, *ccu_reg; > + u32 *ccu_reg; > int bus_width, ret; > > cfg->name = dev->name; > @@ -641,8 +670,22 @@ static int sunxi_mmc_probe(struct udevice *dev) > priv->mmc_no = ((uintptr_t)priv->reg - SUNXI_MMC0_BASE) / 0x1000; > priv->mclkreg = (void *)ccu_reg + > (priv->variant->mclk_offset + (priv->mmc_no * 4)); > - gate_reg = (void *)ccu_reg + priv->variant->gate_offset; > - setbits_le32(gate_reg, BIT(AHB_GATE_OFFSET_MMC(priv->mmc_no))); > + > + ret = clk_get_by_name(dev, "ahb", &priv->clk_ahb); > + if (ret) { > + dev_err(dev, "failed to get AHB clock\n"); > + return ret; > + } > + > + ret = reset_get_by_name(dev, "ahb", &priv->reset); > + if (ret && ret != -ENOENT) { > + dev_err(dev, "failed to get reset\n"); > + return ret; > + } > + > + ret = sunxi_mmc_enable(dev); > + if (ret) > + return ret; > > ret = mmc_set_mod_clk(priv, 24000000); > if (ret) > @@ -676,7 +719,6 @@ static int sunxi_mmc_bind(struct udevice *dev) > } > > static const struct sunxi_mmc_variant sun4i_a10_variant = { > - .gate_offset = 0x60, > .mclk_offset = 0x88, > }; > >