| Message ID | 1510313868-24810-3-git-send-email-pdeschrijver@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | MBIST WAR for Tegra210 | expand |
On 11/10, Peter De Schrijver wrote: > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com> Some commit text perhaps explaining why we add this? > --- > drivers/clk/tegra/clk.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h > index 872f118..3b8947e 100644 > --- a/drivers/clk/tegra/clk.h > +++ b/drivers/clk/tegra/clk.h > @@ -809,4 +809,11 @@ static inline struct clk *tegra_clk_register_emc(void __iomem *base, > u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate); > int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div); > > +/* Combined read fence with delay */ > +#define fence_udelay(delay, reg) \ > + do { \ > + readl_relaxed(reg); \ > + udelay(delay); \ Does the udelay() need to be after the readl()? As far as I recall, the delay can start spinning before the read returns because there isn't any sort of barrier between the two. One approach would be to have an mb() between the read and the delay so that the read is ordered with respect to the delay. If not, we need a better comment than "combined read fence with delay".
On Tue, Nov 14, 2017 at 04:41:32PM -0800, Stephen Boyd wrote: > On 11/10, Peter De Schrijver wrote: > > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com> > > Some commit text perhaps explaining why we add this? > > > --- > > drivers/clk/tegra/clk.h | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h > > index 872f118..3b8947e 100644 > > --- a/drivers/clk/tegra/clk.h > > +++ b/drivers/clk/tegra/clk.h > > @@ -809,4 +809,11 @@ static inline struct clk *tegra_clk_register_emc(void __iomem *base, > > u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate); > > int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div); > > > > +/* Combined read fence with delay */ > > +#define fence_udelay(delay, reg) \ > > + do { \ > > + readl_relaxed(reg); \ > > + udelay(delay); \ > > Does the udelay() need to be after the readl()? As far as I > recall, the delay can start spinning before the read returns > because there isn't any sort of barrier between the two. One > approach would be to have an mb() between the read and the delay > so that the read is ordered with respect to the delay. If not, we > need a better comment than "combined read fence with delay". > Yes. the udelay() needs to be after the readl, but it indeed needs to be readl() so we're sure the read cycle has been completed. Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h index 872f118..3b8947e 100644 --- a/drivers/clk/tegra/clk.h +++ b/drivers/clk/tegra/clk.h @@ -809,4 +809,11 @@ static inline struct clk *tegra_clk_register_emc(void __iomem *base, u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate); int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div); +/* Combined read fence with delay */ +#define fence_udelay(delay, reg) \ + do { \ + readl_relaxed(reg); \ + udelay(delay); \ + } while(0) + #endif /* TEGRA_CLK_H */
Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com> --- drivers/clk/tegra/clk.h | 7 +++++++ 1 file changed, 7 insertions(+)