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
prev parent 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