Re: [RFC 01/16] rtl8xxxu: Treat REG_9346CR as an 8-bit wide register

[Jakub Sitnicki <jsitnicki@gmail.com>]

On Wed, Oct 21, 2015 at 02:29 AM CEST, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> Jakub Sitnicki <jsitnicki@gmail.com> writes:
>> rtl8723au driver treats REG_9346CR register as either 8- or 16-bit wide,
>> while rtl8192cu driver treats it as 8-bit wide, and so does rtl8188eu
>> driver.  Use the lowest common denominator strategy.
>>
>> Signed-off-by: Jakub Sitnicki <jsitnicki@gmail.com>
>> ---
>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> NACK - this is a 16 bit register and the vendor drivers take the lazy
> approach of just reading/writing the lower 8 bits when they feel like
> it.
>
> My code is more consistent, so this one is not going in.

Makes sense.  It didn't occur to me to refer to register offset
definitions and check if REG_9346CR really is 16-bit wide.  Now that you
pointed it out I see it:

#define REG_9346CR                      0x000A
#define REG_EE_VPD                      0x000C

-Jakub

>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> index 11fcfda..5f90261 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> @@ -1928,10 +1928,10 @@ static int rtl8xxxu_read_efuse(struct rtl8xxxu_priv *priv)
>>  	u16 val16, efuse_addr, offset;
>>  	u32 val32;
>>  
>> -	val16 = rtl8xxxu_read16(priv, REG_9346CR);
>> -	if (val16 & EEPROM_ENABLE)
>> +	val8 = rtl8xxxu_read8(priv, REG_9346CR);
>> +	if (val8 & EEPROM_ENABLE)
>>  		priv->has_eeprom = 1;
>> -	if (val16 & EEPROM_BOOT)
>> +	if (val8 & EEPROM_BOOT)
>>  		priv->boot_eeprom = 1;
>>  
>>  	val32 = rtl8xxxu_read32(priv, REG_EFUSE_TEST);