On Mon, 21 Jul 2014 20:41:33 +0100 Ian Campbell <i...@hellion.org.uk> wrote:
> On Fri, 2014-07-18 at 19:23 +0300, Siarhei Siamashka wrote: > > It is going to be useful in more than one place. > > > > Signed-off-by: Siarhei Siamashka <siarhei.siamas...@gmail.com> > > --- > > arch/arm/cpu/armv7/sunxi/dram.c | 30 +++++++++++++++++++----------- > > 1 file changed, 19 insertions(+), 11 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/sunxi/dram.c > > b/arch/arm/cpu/armv7/sunxi/dram.c > > index 18a5c3b..49d1770 100644 > > --- a/arch/arm/cpu/armv7/sunxi/dram.c > > +++ b/arch/arm/cpu/armv7/sunxi/dram.c > > @@ -115,23 +115,31 @@ static void mctl_enable_dll0(u32 phase) > > udelay(22); > > } > > > > +/* Get the number of DDR byte lanes */ > > +static u32 mctl_get_number_of_lanes(void) > > +{ > > + struct sunxi_dram_reg *dram = (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE; > > + switch (readl(&dram->dcr) & DRAM_DCR_BUS_WIDTH_MASK) { > > + case DRAM_DCR_BUS_WIDTH(DRAM_DCR_BUS_WIDTH_32BIT): > > + return 4; > > + case DRAM_DCR_BUS_WIDTH(DRAM_DCR_BUS_WIDTH_16BIT): > > + return 2; > > + default: > > + return 1; > > + } > > +} > > + > > /* > > * Note: This differs from pm/standby in that it checks the bus width > > */ > > static void mctl_enable_dllx(u32 phase) > > { > > struct sunxi_dram_reg *dram = (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE; > > - u32 i, n, bus_width; > > - > > - bus_width = readl(&dram->dcr); > > + u32 i, number_of_lanes; > > > > - if ((bus_width & DRAM_DCR_BUS_WIDTH_MASK) == > > - DRAM_DCR_BUS_WIDTH(DRAM_DCR_BUS_WIDTH_32BIT)) > > - n = DRAM_DCR_NR_DLLCR_32BIT; > > - else > > - n = DRAM_DCR_NR_DLLCR_16BIT; > > Either DRAM_DCR_NR_DLLCR_??BIT are obsolete now and should be removed or > they should be be adjusted and used in the new function. > > ISTM they don't add much so removing would be fine by me. Agreed, I also think that they are better to be dropped. > > + number_of_lanes = mctl_get_number_of_lanes(); > > There is a subtle functional change here since number_of_lanes can be 1 > whereas n could never have been 2. Is that intended/ok? Please mention > in the commit message. I tried to experiment with setting the 8-bit bus width and it is semi-workable (single byte access is OK, but accessing more than one byte is broken). This part of the patch looks like a forgotten leftover of these experiments. But it clearly has no practical value and we only normally deal with the 16-bit or 32-bit bus width. The most correct way of handling this unexpected code branch would be to panic. But that's an unnecessarily increase of the code size. So I think that the best solution is just to keep the old code logic (expect only 16-bit or 32-bit bus width and 2 or 4 lanes). -- Best regards, Siarhei Siamashka -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.