On Wed, Sep 15, 2021 at 11:11:00PM +0200, Fabio M. De Francesco wrote:
> Clean up rtw_read{8,16,32}() and rtw_write{8,16,32,N}() in usb_ops_linux.c.
>
It would be good to know what you did more specifically.
1) Rename variables:
pio_priv => io_priv
pintfhdl => intfhdl
wvalue => address.
2) Remove unnecessary casts.
3) Fix types. Use __le16 instead of __le32.
The last one is a small KASan bug fix. So good job on that.
> Co-developed-by: Pavel Skripkin <paskripkin@xxxxxxxxx>
> Signed-off-by: Pavel Skripkin <paskripkin@xxxxxxxxx>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx>
> ---
> drivers/staging/r8188eu/hal/usb_ops_linux.c | 68 ++++++++++-----------
> 1 file changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 2098ce935dc0..d87da84eca07 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -91,91 +91,91 @@ static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u1
>
> u8 rtw_read8(struct adapter *adapter, u32 addr)
> {
> - struct io_priv *pio_priv = &adapter->iopriv;
> - struct intf_hdl *pintfhdl = &pio_priv->intf;
> - u16 wvalue = (u16)(addr & 0x0000ffff);
> + struct io_priv *io_priv = &adapter->iopriv;
> + struct intf_hdl *intfhdl = &io_priv->intf;
> + u16 address = addr & 0xffff;
> u8 data;
> -
Deleting this line introduces a checkpatch warning.
> - usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
> + usbctrl_vendorreq(intfhdl, address, &data, 1, REALTEK_USB_VENQT_READ);
>
> return data;
> }
>
> u16 rtw_read16(struct adapter *adapter, u32 addr)
> {
> - struct io_priv *pio_priv = &adapter->iopriv;
> - struct intf_hdl *pintfhdl = &pio_priv->intf;
> - u16 wvalue = (u16)(addr & 0x0000ffff);
> - __le32 data;
> + struct io_priv *io_priv = &adapter->iopriv;
> + struct intf_hdl *intfhdl = &io_priv->intf;
> + u16 address = addr & 0xffff;
> + __le16 data;
>
> - usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
> + usbctrl_vendorreq(intfhdl, address, &data, 2, REALTEK_USB_VENQT_READ);
>
> - return (u16)(le32_to_cpu(data) & 0xffff);
> + return le16_to_cpu(data);
The last two bytes of "data" are not initialized. I do not think that
will cause a bug on either endian type of system during runtime but I
this that KASan will catch it and complain.
> }
>
> u32 rtw_read32(struct adapter *adapter, u32 addr)
> {
> - struct io_priv *pio_priv = &adapter->iopriv;
> - struct intf_hdl *pintfhdl = &pio_priv->intf;
> - u16 wvalue = (u16)(addr & 0x0000ffff);
> + struct io_priv *io_priv = &adapter->iopriv;
> + struct intf_hdl *intfhdl = &io_priv->intf;
> + u16 address = addr & 0xffff;
> __le32 data;
>
> - usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
> + usbctrl_vendorreq(intfhdl, address, &data, 4, REALTEK_USB_VENQT_READ);
>
> return le32_to_cpu(data);
> }
>
> int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
> {
> - struct io_priv *pio_priv = &adapter->iopriv;
> - struct intf_hdl *pintfhdl = &pio_priv->intf;
> - u16 wvalue = (u16)(addr & 0x0000ffff);
> + struct io_priv *io_priv = &adapter->iopriv;
> + struct intf_hdl *intfhdl = &io_priv->intf;
> + u16 address = addr & 0xffff;
> int ret;
>
> - ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
> + ret = usbctrl_vendorreq(intfhdl, address, &val, 1, REALTEK_USB_VENQT_WRITE);
>
> return RTW_STATUS_CODE(ret);
> }
>
> int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
> {
> - struct io_priv *pio_priv = &adapter->iopriv;
> - struct intf_hdl *pintfhdl = &pio_priv->intf;
> - u16 wvalue = (u16)(addr & 0x0000ffff);
> - __le32 data = cpu_to_le32(val & 0x0000ffff);
> + struct io_priv *io_priv = &adapter->iopriv;
> + struct intf_hdl *intfhdl = &io_priv->intf;
> + __le16 data = cpu_to_le16(val);
This is the other interesting change. I think the original code works
though.
> + u16 address = addr & 0xffff;
> int ret;
>
> - ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
> + ret = usbctrl_vendorreq(intfhdl, address, &data, 2, REALTEK_USB_VENQT_WRITE);
>
> return RTW_STATUS_CODE(ret);
> }
regards,
dan carpenter
