[U-Boot] [PATCH v3] sunxi: Make DRAM_ODT_EN Kconfig setting a bool

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 05/19/2015 04:13 PM, Ian Campbell wrote:
> On Tue, 2015-05-19 at 14:56 +0200, Hans de Goede wrote:
>> diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun4i.c b/arch/arm/cpu/armv7/sunxi/dram_sun4i.c
>> index c736fa3..f7b4915 100644
>> --- a/arch/arm/cpu/armv7/sunxi/dram_sun4i.c
>> +++ b/arch/arm/cpu/armv7/sunxi/dram_sun4i.c
>> @@ -508,7 +508,7 @@ static void mctl_ddr3_initialize(void)
>>   /*
>>    * Perform impedance calibration on the DRAM controller side of the wire.
>>    */
>> -static void mctl_set_impedance(u32 zq, u32 odt_en)
>> +static void mctl_set_impedance(u32 zq, bool odt_en)
>>   {
>>   	struct sunxi_dram_reg *dram = (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE;
>>   	u32 reg_val;
>> @@ -556,7 +556,7 @@ static void mctl_set_impedance(u32 zq, u32 odt_en)
>>   	clrbits_le32(&dram->zqcr0, DRAM_ZQCR0_ZCAL);
>>
>>   	/* Set I/O configure register */
>> -	writel(DRAM_IOCR_ODT_EN(odt_en), &dram->iocr);
>> +	writel(DRAM_IOCR_ODT_EN, &dram->iocr);
>
> I think at this point previously odt_en would always be 0x1 here (0x0
> having been short circuited earlier).
>
> Whereas this...
>
>> -#define DRAM_IOCR_ODT_EN(n) ((((n) & 0x3) << 30) | ((n) & 0x3) << 0)
>> -#define DRAM_IOCR_ODT_EN_MASK DRAM_IOCR_ODT_EN(0x3)
>> +#define DRAM_IOCR_ODT_EN ((3 << 30) | (3 << 0))
>
> ... now behaves as if it were always 0x3.
>
> AFAICT up until now, at least with the in-tree defconfigs, odt_en was
> always 0 for sun4i anyway, but this is a surprising change I think.

This is deliberate, as Siarhei explained in his reply to v2 the 2 bits
we are setting here enable odt for the DQ resp. DQS lines, having them
enabled for one but not the other does not really make sense.

I'll amend the commit message to explicitly mention this.

Regards,

Hans


>
> DRAM_IOCR_ODT_EN_MASK (which did use 0x3) seems to have been unused. in
> tree.
>
>> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h b/arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h
>> index 06adee2..316a333 100644
>> --- a/arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h
>> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun8i_a23.h
>> @@ -19,6 +19,7 @@ struct dram_para {
>>   	u32 type;
>>   	u32 zq;
>>   	u32 odt_en;
>
> Make this (and maybe others) an actual bool?
>
> Keeping it a u32 might make hex editing easier though?
>
> Ian.
>