mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: <Ajay.Kathat@microchip.com>
To: <jmilan.dev@gmail.com>, <Larry.Finger@lwfinger.net>
Cc: <linux-kernel@vger.kernel.org>, <devel@driverdev.osuosl.org>,
	<linux-wireless@vger.kernel.org>, <gregkh@linuxfoundation.org>,
	<Adham.Abozaeid@microchip.com>
Subject: Re: [PATCH v2] staging: wilc1000: fix cast to restricted __le32
Date: Mon, 7 Jan 2019 06:06:55 +0000	[thread overview]
Message-ID: <1c5c8efb-055f-31b2-b25f-18d3f6e64226@microchip.com> (raw)
In-Reply-To: <CAP=P++DRiM8Rb8vg9hBJ-N2uhfL0Rrj+njYwgZW80yQZ4jQH4g@mail.gmail.com>

Hi Julius,

On 1/6/2019 12:48 PM, Július Milan wrote:
>> Before you send V3, are you sure this is the correct fix? As "frame_type" is
>> input as u16, it seems to me that the frame_type member of struct wilc_reg_frame
>> should be __le16, not __le32.
> 
> Yes, I am confident about it.
> The frame_type member of struct wilc_reg_frame contains in some cases
> 32 bit value
> as you can see in function wilc_wlan_cfg_set_wid.
> Cast to 32 bits is also safe, due to resultant endianness.

Thanks for submitting your patch.

But as Larry pointed we need to change the 'frame_type' type to '__le16'
in 'wilc_reg_frame' struct.
The correct fix for this issue is to change the datatype from ‘__le32’
to ‘__le16’, as the firmware expects it to be a 16bit value.

As wilc_wlan_cfg_set_wid(), is a generic function to pack different
types of WID's e.g char u8 (0x0xxx), short (u16) (0x1xxx), int (u32)
(0x2xxx), byte-string (0x3xxx) and binary data(0x4xxx). And based on the
WID type the specific pack format is used.
To frame register, WID_REGISTER_FRAME(0x3085) WID value is used and it's
of byte-string type. So the packed struct value is transmitted as an
array of u8 data. IMO there is no endianness issue provided firmware
extracts members information in the correct structure order.

Please resubmit the patch by changing 'frame_type' type to '__le16' in
'wilc_reg_frame' struct.

Regards,
Ajay

  reply	other threads:[~2019-01-07  6:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-05  9:10 Július Milan
2019-01-05  9:16 ` Greg KH
2019-01-05 16:02 ` Larry Finger
2019-01-06  7:18   ` Július Milan
2019-01-07  6:06     ` Ajay.Kathat [this message]
2019-01-07 14:48       ` Július Milan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1c5c8efb-055f-31b2-b25f-18d3f6e64226@microchip.com \
    --to=ajay.kathat@microchip.com \
    --cc=Adham.Abozaeid@microchip.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jmilan.dev@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome