Linux wireless drivers development
 help / color / mirror / Atom feed
From: Walter Harms <wharms@bfs.de>
To: Dan Carpenter <error27@gmail.com>,
	Lachlan Hodges <lachlan.hodges@morsemicro.com>
Cc: Dan Callaghan <dan.callaghan@morsemicro.com>,
	Arien Judge <arien.judge@morsemicro.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	Chetan Mistry <chetan.mistry@morsemicro.com>,
	Sahand Maleki <sahand.maleki@morsemicro.com>,
	Simon Wadsworth <simon@morsemicro.com>,
	"James Herbert" <james.herbert@morsemicro.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>
Subject: AW: [PATCH next] wifi: mm81x: fix type bugs handling sdio_readl/writel()
Date: Fri, 21 Aug 2026 11:15:26 +0000	[thread overview]
Message-ID: <9dd07bf672c245a1a8656037d3b6ca72@bfs.de> (raw)
In-Reply-To: <aoa7Hj1W7ETOrOzO@stanley.mountain>

hi everyone,
Dan has a point here as the function should use int as requiered by the interface.
I would like to point to an other problem here;  too many casts.

  sdio_writel(func1,
          (__force u32)cpu_to_le32(val),

the code for sdio_writel() has this already ...    *(__le32 *)func->tmpbuf = cpu_to_le32(b); //=val
the conversion for address is missing, is that intentional ?

        (__force u32)cpu_to_le32(address), &ret);

By adding the cpu_to_le32() it should be possible to drop all the casts. That would improve the readability here.

BTW: all other familymembers (e.g. sdio_f0_readb) check from  func != NULL maybe this should be done here also ?

jm2c,
  WH
________________________________________
Von: Dan Carpenter <error27@gmail.com>
Gesendet: Donnerstag, 20. August 2026 10:30:22
An: Lachlan Hodges
Cc: Dan Callaghan; Arien Judge; Johannes Berg; Chetan Mistry; Sahand Maleki; Simon Wadsworth; James Herbert; linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
Betreff: [PATCH next] wifi: mm81x: fix type bugs handling sdio_readl/writel()

The sdio_readl() and sdio_writel() functions, instead of returning kernel
error codes, instead stores the error codes in a parameter.  These caller
functions pass should pass an int pointer to store the error code but
instead pass a signed long.

This will not work on big endian systems.  On little endian systems
passing a ssize_t means negative error codes are converted to positive
values near UINT_MAX.  This doesn't cause a problem at runtime because
in mm81x_sdio_reg32_write() the error codes are discarded and we always
return -EIO.  In mm81x_sdio_reg32_read() the high bits are truncated
away so the positive value is re-converted back to negative and the
code works as intended.

Either way, passing an int is the correct thing and is a cleanup.

Fixes: b1906cea00b0 ("wifi: mm81x: add mm81x Wi-Fi HaLow driver")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
 drivers/net/wireless/morsemicro/mm81x/sdio.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/morsemicro/mm81x/sdio.c b/drivers/net/wireless/morsemicro/mm81x/sdio.c
index 96fce187dd35..65277399bf18 100644
--- a/drivers/net/wireless/morsemicro/mm81x/sdio.c
+++ b/drivers/net/wireless/morsemicro/mm81x/sdio.c
@@ -381,7 +381,7 @@ static int mm81x_sdio_dm_read(struct mm81x *mors, u32 address, u8 *data,

 static int mm81x_sdio_reg32_write(struct mm81x *mors, u32 address, u32 val)
 {
-       ssize_t ret = 0;
+       int ret = 0;
        u32 original_address = address;
        struct mm81x_sdio *sdio = (struct mm81x_sdio *)mors->drv_priv;
        struct sdio_func *func1 = sdio->func->card->sdio_func[0];
@@ -391,7 +391,7 @@ static int mm81x_sdio_reg32_write(struct mm81x *mors, u32 address, u32 val)

        address &= 0x0000FFFF;
        sdio_writel(func1, (__force u32)cpu_to_le32(val),
-                   (__force u32)cpu_to_le32(address), (int *)&ret);
+                   (__force u32)cpu_to_le32(address), &ret);
        if (ret)
                goto error;

@@ -411,7 +411,7 @@ static int mm81x_sdio_reg32_write(struct mm81x *mors, u32 address, u32 val)
 static int mm81x_sdio_reg32_read(struct mm81x *mors, u32 address, u32 *val)
 {
        u32 value;
-       ssize_t ret = 0;
+       int ret = 0;
        struct mm81x_sdio *sdio = (struct mm81x_sdio *)mors->drv_priv;
        struct sdio_func *func1 = sdio->func->card->sdio_func[0];

@@ -419,8 +419,7 @@ static int mm81x_sdio_reg32_read(struct mm81x *mors, u32 address, u32 *val)
                                         MM81X_CONFIG_ACCESS_4BYTE);

        address &= 0x0000FFFF;
-       value = sdio_readl(func1, (__force u32)cpu_to_le32(address),
-                          (int *)&ret);
+       value = sdio_readl(func1, (__force u32)cpu_to_le32(address), &ret);
        if (ret)
                return ret;

--
2.53.0



      reply	other threads:[~2026-08-21 11:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20  8:30 [PATCH next] wifi: mm81x: fix type bugs handling sdio_readl/writel() Dan Carpenter
2026-08-21 11:15 ` Walter Harms [this message]

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=9dd07bf672c245a1a8656037d3b6ca72@bfs.de \
    --to=wharms@bfs.de \
    --cc=arien.judge@morsemicro.com \
    --cc=chetan.mistry@morsemicro.com \
    --cc=dan.callaghan@morsemicro.com \
    --cc=error27@gmail.com \
    --cc=james.herbert@morsemicro.com \
    --cc=johannes@sipsolutions.net \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=lachlan.hodges@morsemicro.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=sahand.maleki@morsemicro.com \
    --cc=simon@morsemicro.com \
    /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