10.03.2020 18:19, Thierry Reding пишет: > From: Joseph Lo <josephl@xxxxxxxxxx> > > The EMC clock needs to carefully coordinate with the EMC controller > programming to make sure external memory can be properly clocked. Do so > by hooking up the EMC clock with an EMC provider that will specify which > rates are supported by the EMC and provide a callback to use for setting > the clock rate at the EMC. > > Based on work by Peter De Schrijver <pdeschrijver@xxxxxxxxxx>. > > Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx> > --- > Changes in v5: > - major rework and cleanup ... > +static u8 tegra210_clk_emc_get_parent(struct clk_hw *hw) > +{ > + struct tegra210_clk_emc *emc = to_tegra210_clk_emc(hw); > + u32 value; > + u8 src; > + > + value = readl_relaxed(emc->regs + CLK_SOURCE_EMC); > + src = (value >> CLK_SOURCE_EMC_2X_CLK_SRC_SHIFT) & > + CLK_SOURCE_EMC_2X_CLK_SRC_MASK; What about to use a generic FIELD_GET/PREP()? > +static int tegra210_clk_emc_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct tegra210_clk_emc *emc = to_tegra210_clk_emc(hw); > + struct tegra210_clk_emc_provider *provider = emc->provider; > + struct tegra210_clk_emc_config *config; > + struct device *dev = provider->dev; > + struct clk_hw *old, *new, *parent; > + u8 old_idx, new_idx, index; > + struct clk *clk; > + unsigned int i; > + int err; > + > + if (!provider || !provider->configs || provider->num_configs == 0) > + return -EINVAL; Why all these checks are needed? I don't think it ever could fail, couldn't it? > +static int emc_table_lookup(struct tegra_emc *emc, unsigned long rate) > +{ > + int i; unsigned int Same for all other occurrences in the code.