From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4D3263A3803 for ; Tue, 24 Mar 2026 06:10:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774332608; cv=none; b=dfgunSblauLEpzykke52RFpDMBArXxEZRj0/x52/bkEV2HIdcLvRqr8LKzQ9vxoVxaZDoCXVenlZoI7ZkCtIdRtKa5GRA95YJOXIyeNDCDbvD9H0BaWXkIGMYL0D5wCZe9ApiliX5PnoSpWttw9vqF24MCyhjeO4ggxJlk2v5Q8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774332608; c=relaxed/simple; bh=/FgkbgcpoO7PcaB8vSKnNV13orQdV6npMMwLpWb8/gs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=O9+CG1DAAFgW6GJUs6bBqeFkiwmf+cmIjHJ6gYDDARZ5McOXGGjgbFomNjQHFeA8IF360A6QqFnnKzrWybRYzplzjJkdmCy/3qczvlDZh5jmExwFezyS1+Mn4Sv3d2aDWXl8yzFaPb3eXRfKL4KsUvBjPGg+ryS/oS687BA4U6c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=xGSc/mOL; arc=none smtp.client-ip=209.85.128.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="xGSc/mOL" Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-48541edecf9so7187065e9.1 for ; Mon, 23 Mar 2026 23:10:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1774332606; x=1774937406; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+5ScTfIumgjrbpgqD0j/yOTuHEj5IzOxqi+eo7Q3Siw=; b=xGSc/mOLl7VEhSiGPlcShuNx2blLuGQR3LMopy6bvCRuhBDp3kpU5cRYRvSHEt81kq d+gnb/PdvmeernQPi1wsKvXYvL66nVSheFVl9No9+8mtc2a+zuRdNxQPK2kllJHKSK9R 1AtILyYk9wawcz2rcPBVIcDQJaCGZFmLPoJzfvvqsHMPDoZM8hkyAB33AqeobFfrUBSZ hjVKcXhovUffpfy2xzmWppozoa1bRHab4Teh5ICBZ7y7HXiv+F8SI0z+ZFtRt+VOUKhm H9R5g71ubehxt+x4K4LqE6qHJ0qAUG9O/qvGd0ZseyTMBgY7BPG53RWUNtsyv8CzDoKQ yR8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774332606; x=1774937406; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+5ScTfIumgjrbpgqD0j/yOTuHEj5IzOxqi+eo7Q3Siw=; b=i11Asy5t/1f66bxH3MpKbASRriBBAwExnjNYzCeLDb3b8W2bEGNS/gMosmfDQpSOfC hhI72NfhnnzCN5Cdwm+RPzgU1uASPKkWvK6ojAIA6q7+K285UWVhJb9LOzBkbA3iRhuS I6WvZUW+COnhkN7SezE9VQYwYyI0LyCcAFnD3u12SHrjBmAUIeR/LWKaKgdHmL/nrosO 6CwWw70NTYtoVN7H/jEjjBy2Vcku0Zx2QSbjbHHNzdTItQ9927w+8p9FQy7HH76kNkhg KatFxC0H0MZc3bAZrd0whZN2rLneBUDE0Oy/MPfTE4rLMU3IRDBNdWnPPyuudiUjXpTA Pc1g== X-Forwarded-Encrypted: i=1; AJvYcCWsQtg3kTdLnDxeotQi2gKoxRdy0ox297Vr8o9iaq1IfQLJVX1NWpEqfSqNptF5tPP41jjGYGk/FjH/Vuk=@vger.kernel.org X-Gm-Message-State: AOJu0YwzVUzXnrYBNyh0EoBFXBf27bx6gwR4yNWpXclJYUTfVd05PpFn iJSA6XDUIx7pV62oYQhZhICT5ayRuGAyJ2/wl/HHOOcBPjGb8pyd4rOZ7GXqhdZkfbVVepP1C1J JFeDB X-Gm-Gg: ATEYQzydefp7CCIh+BSux6J62cFWXbW/XhLvSkwTweKEUji6vKe6C06b6Q+UihgQkyM 5Wqqk6lXdlSj6+B8Gs10PIQyVxuZAmz0hQZJRnWHxu8pz9WIoKFv6bOOwmhW7a+9Z1jJAL2XNhR n0BDC8FhnkKRZ5SdtfjllSDWO3HOqIzy6PB6bY8qgYHno1SpVDR5Q9fguyG5lJbe3lf7iLNrAjP n098qkwRef4zDEycjg5SZX0llnvRbn3Nd+vrJ6aQwbik7NZORmRhjbKYsIJrhWFJu/sOK0WPp5b mhw2hnuYdWlWShdr1Dr58SIz7wdubZgh7e/ZiMh9Cj93mT0wtN5Py9/v4xbEQLIGwf3xEzkNgMY SHysnEEq1Bm7FSAbBRIDFW/+xH3tmJBErxZeUQQxXbtd3pcmX4/ImXguy+miSnOKHEhpK0xn2Mz UeqCmgYVfn43A9MAxYkLh79YsgDDcI X-Received: by 2002:a05:600c:3e8d:b0:485:3a03:ceca with SMTP id 5b1f17b1804b1-486fee26501mr189987685e9.23.1774332605571; Mon, 23 Mar 2026 23:10:05 -0700 (PDT) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43b64703c27sm35131046f8f.18.2026.03.23.23.10.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Mar 2026 23:10:05 -0700 (PDT) Date: Tue, 24 Mar 2026 09:10:02 +0300 From: Dan Carpenter To: Omer El Idrissi Cc: gregkh@linuxfoundation.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees Message-ID: References: <20260323232526.25288-1-omer.e.idrissi@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260323232526.25288-1-omer.e.idrissi@gmail.com> The subject is a bit long. It's not really a "fix" it's a cleanup. Subject: [PATCH] staging: rtl8723bs: cleanup return in sdio_dvobj_init() On Tue, Mar 24, 2026 at 12:25:26AM +0100, Omer El Idrissi wrote: > Return proper errno values instead of vendor-defined non-descriptive > _SUCCESS/_FAIL macros > Callers only check for non-zero return values, so this does not change > behaviour while improving correctness. > This is how your email looks like to me: https://lore.kernel.org/all/20260323232526.25288-1-omer.e.idrissi@gmail.com/ I can't find the subject so I only read the commit message. The commit message should mention sdio_dvobj_init(). But really sdio_dvobj_init() is not a leaf function, it's a branch. sdio_init() is the leaf at the end of the call tree. The subject should really say: [PATCH] staging: rtl8723bs: cleanup return in sdio_init() > Signed-off-by: Omer El Idrissi > --- > drivers/staging/rtl8723bs/os_dep/os_intfs.c | 7 ++++--- > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 19 +++++++++++-------- > 2 files changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c > index 7ba689f2dfc8..80ff3154f6e7 100644 > --- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c > +++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c > @@ -1136,9 +1136,10 @@ static int rtw_resume_process_normal(struct adapter *padapter) > pmlmepriv = &padapter->mlmepriv; > /* interface init */ > /* if (sdio_init(adapter_to_dvobj(padapter)) != _SUCCESS) */ > - if ((padapter->intf_init) && (padapter->intf_init(adapter_to_dvobj(padapter)) != _SUCCESS)) { > - ret = -1; > - goto exit; > + if (padapter->intf_init) { > + ret = padapter->intf_init(adapter_to_dvobj(padapter)); > + if (ret) > + goto exit; > } > rtw_hal_disable_interrupt(padapter); > /* if (sdio_alloc_irq(adapter_to_dvobj(padapter)) != _SUCCESS) */ > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > index d664e254912c..0d6475bfbaba 100644 > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > @@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj) > release: > sdio_release_host(func); > > - if (err) > - return _FAIL; > - return _SUCCESS; > + return err; > } > > static void sdio_deinit(struct dvobj_priv *dvobj) > @@ -159,16 +157,19 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func) > struct dvobj_priv *dvobj = NULL; > struct sdio_data *psdio; > > - dvobj = devobj_init(); > - if (!dvobj) > + dvobj = devobj_init(); > + if (!dvobj) { > + dvobj = ERR_PTR(-ENOMEM); > goto exit; > + } > > sdio_set_drvdata(func, dvobj); > > psdio = &dvobj->intf_data; > psdio->func = func; > > - if (sdio_init(dvobj) != _SUCCESS) > + status = sdio_init(dvobj); > + if (status) > goto free_dvobj; > > rtw_reset_continual_io_error(dvobj); > @@ -180,7 +181,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func) > > devobj_deinit(dvobj); > > - dvobj = NULL; > + dvobj = ERR_PTR(status); > } > exit: > return dvobj; This function does some nonsense stuff. First convert it to direct returns and then your other patch will be easier. Actually, I would leave devobj_init() as returning NULL. Your conversion to error pointers isn't wrong but it's isn't necessary either. [PATCH 1] staging: rtl8723bs: use direct returns in sdio_dvobj_init() [PATCH 2] staging: rtl8723bs: cleanup return in sdio_init() regards, dan carpenter diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c index d664e254912c..5e093324bbd6 100644 --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c @@ -161,7 +161,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func) dvobj = devobj_init(); if (!dvobj) - goto exit; + return NULL; sdio_set_drvdata(func, dvobj); @@ -172,18 +172,13 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func) goto free_dvobj; rtw_reset_continual_io_error(dvobj); - status = _SUCCESS; + return dvobj; free_dvobj: - if (status != _SUCCESS && dvobj) { - sdio_set_drvdata(func, NULL); - - devobj_deinit(dvobj); + sdio_set_drvdata(func, NULL); + devobj_deinit(dvobj); - dvobj = NULL; - } -exit: - return dvobj; + return NULL; } static void sdio_dvobj_deinit(struct sdio_func *func)