On Wed, Jan 14, 2015 at 3:19 PM, Vince Hsu <[email protected]> wrote:
> Signed-off-by: Vince Hsu <[email protected]>
Please have a short commit message even if the subject seems to be
self-explanatory.
> ---
> drivers/memory/tegra/tegra124.c | 108
> +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 107 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/memory/tegra/tegra124.c b/drivers/memory/tegra/tegra124.c
> index 278d40b854c1..cce255d3df5c 100644
> --- a/drivers/memory/tegra/tegra124.c
> +++ b/drivers/memory/tegra/tegra124.c
> @@ -6,6 +6,8 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> #include <linux/of.h>
> #include <linux/mm.h>
>
> @@ -15,7 +17,7 @@
>
> #include "mc.h"
>
> -static const struct tegra_mc_client tegra124_mc_clients[] = {
> +static struct tegra_mc_client tegra124_mc_clients[] = {
This const drop also seems out-of-place. If they are needed at all,
please have them all done in the same patch, and only when they become
needed.
> {
> .id = 0x00,
> .name = "ptcr",
> @@ -959,7 +961,108 @@ static const struct tegra_smmu_swgroup
> tegra124_swgroups[] = {
> { .swgroup = TEGRA_SWGROUP_VI, .reg = 0x280 },
> };
>
> +static struct tegra_mc_hotreset tegra124_mc_hotreset[] = {
> + {TEGRA_SWGROUP_AFI, 0x200, 0x204, 0},
> + {TEGRA_SWGROUP_AVPC, 0x200, 0x204, 1},
> + {TEGRA_SWGROUP_DC, 0x200, 0x204, 2},
> + {TEGRA_SWGROUP_DCB, 0x200, 0x204, 3},
> + {TEGRA_SWGROUP_HC, 0x200, 0x204, 6},
> + {TEGRA_SWGROUP_HDA, 0x200, 0x204, 7},
> + {TEGRA_SWGROUP_ISP2, 0x200, 0x204, 8},
> + {TEGRA_SWGROUP_MPCORE, 0x200, 0x204, 9},
> + {TEGRA_SWGROUP_MPCORELP, 0x200, 0x204, 10},
> + {TEGRA_SWGROUP_MSENC, 0x200, 0x204, 11},
> + {TEGRA_SWGROUP_PPCS, 0x200, 0x204, 14},
> + {TEGRA_SWGROUP_SATA, 0x200, 0x204, 15},
> + {TEGRA_SWGROUP_VDE, 0x200, 0x204, 16},
> + {TEGRA_SWGROUP_VI, 0x200, 0x204, 17},
> + {TEGRA_SWGROUP_VIC, 0x200, 0x204, 18},
> + {TEGRA_SWGROUP_XUSB_HOST, 0x200, 0x204, 19},
> + {TEGRA_SWGROUP_XUSB_DEV, 0x200, 0x204, 20},
> + {TEGRA_SWGROUP_TSEC, 0x200, 0x204, 22},
> + {TEGRA_SWGROUP_SDMMC1A, 0x200, 0x204, 29},
> + {TEGRA_SWGROUP_SDMMC2A, 0x200, 0x204, 30},
> + {TEGRA_SWGROUP_SDMMC3A, 0x200, 0x204, 31},
> + {TEGRA_SWGROUP_SDMMC4A, 0x970, 0x974, 0},
> + {TEGRA_SWGROUP_ISP2B, 0x970, 0x974, 1},
> + {TEGRA_SWGROUP_GPU, 0x970, 0x974, 2},
> +};
> +
> #ifdef CONFIG_ARCH_TEGRA_124_SOC
> +
> +static bool tegra124_stable_hotreset_check(struct tegra_mc *mc,
> + u32 reg, u32 *stat)
> +{
> + int i;
> + u32 cur_stat;
> + u32 prv_stat;
> +
> + /*
> + * There might be a glitch seen with the status register if we program
> + * the control register and then read the status register in a short
> + * window due to a HW bug. So here we poll for a stable status read.
> + */
> + prv_stat = mc_readl(mc, reg);
> + for (i = 0; i < 5; i++) {
Why 5 times?
> + cur_stat = mc_readl(mc, reg);
> + if (cur_stat != prv_stat)
> + return false;
> + }
> + *stat = cur_stat;
> + return true;
> +}
> +
> +static int tegra124_mc_flush(struct tegra_mc *mc,
> + const struct tegra_mc_hotreset *hotreset)
> +{
> + u32 val;
> +
> + if (!mc || !hotreset)
> + return -EINVAL;
> +
> + /* XXX add mutex */
I guess this is a TODO for when this series exists the RFC state. :)
> +
> + val = mc_readl(mc, hotreset->ctrl);
> + val |= BIT(hotreset->bit);
> + mc_writel(mc, val, hotreset->ctrl);
> + mc_readl(mc, hotreset->ctrl);
> +
> + /* poll till the flush is done */
> + do {
> + udelay(10);
> + val = 0;
> + if (!tegra124_stable_hotreset_check(mc, hotreset->status,
> &val))
> + continue;
> + } while (!(val & BIT(hotreset->bit)));
So here you are waiting for the right bit in hotreset->status to turn
to 1. Why is the whole thing with tegra124_stable_hotreset_check()
needed? Could it switch from 1 to 0 to 1 again, with the first window
at 1 being invalid? In that case isn't there a risk that
tegra124_stable_hotreset_check() might do 5 consecutive reads on that
invalid state and let us continue before the flush is completed?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
