DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 76DA220825
Message-ID: <0244e9225446db503360d9c024e0eb7e25c29944.camel@perches.com>
Subject: Re: [PATCH 4/8] staging: wilc1000: avoid line over 80 chars in
 wilc_parse_network_info()
From:   Joe Perches <joe@perches.com>
To:     Ajay.Kathat@microchip.com, linux-wireless@vger.kernel.org
Cc:     devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
        Ganesh.Krishna@microchip.com, Aditya.Shankar@microchip.com,
        Adham.Abozaeid@microchip.com, johannes@sipsolutions.net
Date:   Mon, 05 Nov 2018 02:57:54 -0800
In-Reply-To: <1541090691-31928-5-git-send-email-ajay.kathat@microchip.com>
References: <1541090691-31928-1-git-send-email-ajay.kathat@microchip.com>
         <1541090691-31928-5-git-send-email-ajay.kathat@microchip.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.30.1-1build1 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Thu, 2018-11-01 at 16:45 +0000, Ajay.Kathat@microchip.com wrote:
> From: Ajay Singh <ajay.kathat@microchip.com>
> 
> Use shorter name for 'network_info' variable to avoid line over 80 chars
> issue.


I suppose this is OK, though perhaps unnecessary.

As well, perhaps there are defects in the original code
in a couple places.

> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
[]
> -		network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
> -		network_info->tsf_lo = (u32)network_info->tsf_hi;

Perhaps there is a defect for both tsf_hi assignments
as it appears as if both are missing ">> 32"

> +		info->cap_info = le16_to_cpu(mgt->u.beacon.capab_info);
> +		info->beacon_period = le16_to_cpu(mgt->u.beacon.beacon_int);
> +		info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp);
> +		info->tsf_lo = (u32)info->tsf_hi;

Perhaps this should be 

		network_info->tsf_hi = le64_to_cpu(mgt->u.beacon.timestamp) >> 32;

or

		network_info->tsf_hi = upper_32_bits(le64_to_cpu(...));
		network_info->tsf_lo = lower_32_bits(le64_to_cpu(...));