[PATCH] staging: wilc1000: Remove unnecessary pointer check
Clang warns that the address of a pointer will always evaluated as true
in a boolean context:
drivers/staging/wilc1000/linux_wlan.c:267:20: warning: address of
'vif->ndev->dev' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (!(&vif->ndev->dev))
~ ~~~^~~
1 warning generated.
Since this statement always evaluates to false due to the logical not,
remove it.
Link: https://github.com/ClangBuiltLinux/linux/issues/121
Signed-off-by: Nathan Chancellor
---
drivers/staging/wilc1000/linux_wlan.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/wilc1000/linux_wlan.c
b/drivers/staging/wilc1000/linux_wlan.c
index 49afda669393..323593440e40 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -264,9 +264,6 @@ static int wilc_wlan_get_firmware(struct net_device *dev)
netdev_info(dev, "loading firmware %s\n", firmware);
- if (!(&vif->ndev->dev))
- goto fail;
-
if (request_firmware(&wilc_firmware, firmware, wilc->dev) != 0) {
netdev_err(dev, "%s - firmware not available\n", firmware);
ret = -1;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: Simplify memcmp logical checks
Clang generates a warning when it sees a logical not followed by a
conditional operator like ==, >, or < because it thinks that the logical
not should be applied to the whole statement:
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:293:8: warning: logical
not is only applied to the left hand side of this comparison
[-Wlogical-not-parentheses]
It assumes the author might have made a mistake in their logic:
if (!a == b) -> if (!(a == b))
Sometimes that is the case; other times, it's just a super convoluted
way of saying 'if (a)' when b = 0:
if (!1 == 0) -> if (0 == 0) -> if (true)
Alternatively:
if (!1 == 0) -> if (!!1) -> if (1)
Simplify these comparisons so that Clang doesn't complain.
Link: https://github.com/ClangBuiltLinux/linux/issues/161
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +-
drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
drivers/staging/rtl8188eu/core/rtw_recv.c | 4 ++--
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
index 9e5c7e62d26f..20f34d25c369 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
@@ -284,7 +284,7 @@ unsigned char *rtw_get_wpa_ie(unsigned char *pie, uint
*wpa_ie_len, int limit)
if (pbuf) {
/* check if oui matches... */
- if (!memcmp((pbuf + 2), wpa_oui_type,
sizeof(wpa_oui_type)) == false)
+ if (memcmp((pbuf + 2), wpa_oui_type,
sizeof(wpa_oui_type)))
goto check_next_ie;
/* check version... */
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index ef8a7dc4bd34..43d6513484c5 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1437,7 +1437,7 @@ static int rtw_check_join_candidate(struct mlme_priv
*pmlmepriv
/* check ssid, if needed */
if (pmlmepriv->assoc_ssid.SsidLength) {
if (competitor->network.Ssid.SsidLength !=
pmlmepriv->assoc_ssid.SsidLength ||
- !memcmp(competitor->network.Ssid.Ssid,
pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength) == false)
+ memcmp(competitor->network.Ssid.Ssid,
pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
goto exit;
}
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index ab9638d618a9..f3eb63f8cf0b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -1283,8 +1283,8 @@ static int wlanhdr_to_ethhdr(struct recv_frame
*precvframe)
psnap_type = ptr+pattrib->hdrlen + pattrib->iv_len+SNAP_SIZE;
/* convert hdr + possible LLC headers into Ethernet header */
if ((!memcmp(psnap, rtw_rfc1042_header, SNAP_SIZE) &&
-(!memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) == false) &&
-(!memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2) == false)) ||
+memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) &&
+memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2)) ||
!memcmp(psnap, rtw_bridge_tunnel_header, SNAP_SIZE)) {
/* remove RFC1042 or Bridge-Tunnel encapsulation and replace
EtherType */
bsnaphdr = true;
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index fb496ab5a862..51cf78150168 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -856,7 +856,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
*pframe, u32 packet_len)
return _FAIL;
}
- if (!memcmp(cur_network->network.MacAddress, pbssid, 6) == false) {
+ if (memcmp(cur_network->network.MacAddress, pbssid, 6) {
DBG_88E("Oops: rtw_check_network_encrypt linked but recv other
bssid bcn\n%pM %pM\n",
(pbssid), (cur_network->network.MacAddress));
return true;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Remove unnecessary pointer check
On Fri, Sep 21, 2018 at 01:25:32AM -0400, valdis.kletni...@vt.edu wrote:
> On Thu, 20 Sep 2018 14:26:49 -0700, Nathan Chancellor said:
> > Clang warns that the address of a pointer will always evaluated as true
> > in a boolean context:
> >
> > drivers/staging/wilc1000/linux_wlan.c:267:20: warning: address of
> > 'vif->ndev->dev' will always evaluate to 'true'
> > [-Wpointer-bool-conversion]
> > if (!(&vif->ndev->dev))
> > ~ ~~~^~~
> > 1 warning generated.
> >
> > Since this statement always evaluates to false due to the logical not,
> > remove it.
>
> Often, "just nuke it because it's now dead code" isn't the best answer...
>
> At one time, that was likely intended to be checking whether ->dev was a null
> pointer, to make sure we don't pass request_firmware() a null pointer and oops
> the kernel, or other things that go pear-shaped
>
> So the question becomes: Is it safe to just remove it, or was it intended to
> test for something that could legitimately be null if we've hit an error
> along
> the way (which means we should fix the condition to be proper and acceptable
> to both gcc and clang)?
>
>
I certainly considered whether or not removing the check versus fixing
it was the correct answer. Given that this check can be traced back to
the initial check in of the driver in 2015, I figured it was safe to
remove it (since a null pointer dereference would most likely have been
noticed by now).
Most patches addressing this warning just remove the check given that it's
not actually changing the code, such as commit a7dc662c6a7b ("ASoC: codecs:
PCM1789: unconditionally flush work"). However, if the driver authors and/or
maintainers think that this check should be something else (maybe checking
that the contents of dev is not null versus the address, I'm perfectly
happy to submit a v2 with this change.
Thank you for the response and review!
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: Remove unnecessary parentheses
Clang warns when multiple pairs of parentheses are used for a single
conditional statement.
drivers/staging/rtl8188eu/core/rtw_pwrctrl.c:295:21: warning: equality
comparison with extraneous parentheses [-Wparentheses-equality]
if ((pwrpriv->rpwm == pslv)) {
~~^~~
drivers/staging/rtl8188eu/core/rtw_pwrctrl.c:295:21: note: remove
extraneous parentheses around the comparison to silence this warning
if ((pwrpriv->rpwm == pslv)) {
~ ^ ~
drivers/staging/rtl8188eu/core/rtw_pwrctrl.c:295:21: note: use '=' to
turn this equality comparison into an assignment
if ((pwrpriv->rpwm == pslv)) {
^~
=
drivers/staging/rtl8188eu/hal/odm.c:1062:27: warning: equality
comparison with extraneous parentheses [-Wparentheses-equality]
if ((pregpriv->wifi_spec == 1))/* (pmlmeinfo->HT_enable == 0))
*/
^~~~
drivers/staging/rtl8188eu/hal/odm.c:1062:27: note: remove extraneous
parentheses around the comparison to silence this warning
if ((pregpriv->wifi_spec == 1))/* (pmlmeinfo->HT_enable == 0))
*/
~^ ~
drivers/staging/rtl8188eu/hal/odm.c:1062:27: note: use '=' to turn this
equality comparison into an assignment
if ((pregpriv->wifi_spec == 1))/* (pmlmeinfo->HT_enable == 0))
*/
^~
=
Link: https://github.com/ClangBuiltLinux/linux/issues/163
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8188eu/core/rtw_pwrctrl.c | 2 +-
drivers/staging/rtl8188eu/hal/odm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_pwrctrl.c
b/drivers/staging/rtl8188eu/core/rtw_pwrctrl.c
index 5ab6fc22a156..a3d3e9eb133c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/rtl8188eu/core/rtw_pwrctrl.c
@@ -292,7 +292,7 @@ void rtw_set_rpwm(struct adapter *padapter, u8 pslv)
pslv = PS_STATE_S3;
}
- if ((pwrpriv->rpwm == pslv)) {
+ if (pwrpriv->rpwm == pslv) {
RT_TRACE(_module_rtl871x_pwrctrl_c_, _drv_err_,
("%s: Already set rpwm[0x%02X], new=0x%02X!\n",
__func__, pwrpriv->rpwm, pslv));
return;
diff --git a/drivers/staging/rtl8188eu/hal/odm.c
b/drivers/staging/rtl8188eu/hal/odm.c
index 48738f94cf88..5a0c38ecb15e 100644
--- a/drivers/staging/rtl8188eu/hal/odm.c
+++ b/drivers/staging/rtl8188eu/hal/odm.c
@@ -1059,7 +1059,7 @@ void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm)
struct mlme_ext_priv*pmlmeext = &(Adapter->mlmeextpriv);
struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
- if ((pregpriv->wifi_spec == 1))/* (pmlmeinfo->HT_enable == 0)) */
+ if (pregpriv->wifi_spec == 1) /* (pmlmeinfo->HT_enable == 0)) */
goto dm_CheckEdcaTurbo_EXIT;
if (pmlmeinfo->assoc_AP_vendor >= HT_IOT_PEER_MAX)
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtlwifi: Use proper enumerated types for Wi-Fi only interface
Clang warns when one enumerated type is implicitly converted to another.
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1264:34: warning:
implicit conversion from enumeration type 'enum btc_chip_interface' to
different enumeration type 'enum wifionly_chip_interface'
[-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_PCI;
~ ^~~~
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1267:34: warning:
implicit conversion from enumeration type 'enum btc_chip_interface' to
different enumeration type 'enum wifionly_chip_interface'
[-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_USB;
~ ^~~~
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1270:34: warning:
implicit conversion from enumeration type 'enum btc_chip_interface' to
different enumeration type 'enum wifionly_chip_interface'
[-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
~ ^~~~
3 warnings generated.
Use the values from the correct enumerated type, wifionly_chip_interface.
BTC_INTF_UNKNOWN = WIFIONLY_INTF_UNKNOWN = 0
BTC_INTF_PCI = WIFIONLY_INTF_PCI = 0
BTC_INTF_USB = WIFIONLY_INTF_USB = 0
Link: https://github.com/ClangBuiltLinux/linux/issues/171
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
index 4d1f9bf53c53..85a7490e6bbd 100644
--- a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -1261,13 +1261,13 @@ bool exhalbtc_initlize_variables_wifi_only(struct
rtl_priv *rtlpriv)
switch (rtlpriv->rtlhal.interface) {
case INTF_PCI:
- wifionly_cfg->chip_interface = BTC_INTF_PCI;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_PCI;
break;
case INTF_USB:
- wifionly_cfg->chip_interface = BTC_INTF_USB;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_USB;
break;
default:
- wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_UNKNOWN;
break;
}
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtlwifi: Use proper enumerated types for Wi-Fi only interface
Clang warns when one enumerated type is implicitly converted to another.
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1264:34: warning:
implicit conversion from enumeration type 'enum btc_chip_interface' to
different enumeration type 'enum wifionly_chip_interface'
[-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_PCI;
~ ^~~~
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1267:34: warning:
implicit conversion from enumeration type 'enum btc_chip_interface' to
different enumeration type 'enum wifionly_chip_interface'
[-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_USB;
~ ^~~~
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1270:34: warning:
implicit conversion from enumeration type 'enum btc_chip_interface' to
different enumeration type 'enum wifionly_chip_interface'
[-Wenum-conversion]
wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
~ ^~~~
3 warnings generated.
Use the values from the correct enumerated type, wifionly_chip_interface.
BTC_INTF_UNKNOWN = WIFIONLY_INTF_UNKNOWN = 0
BTC_INTF_PCI = WIFIONLY_INTF_PCI = 1
BTC_INTF_USB = WIFIONLY_INTF_USB = 2
Link: https://github.com/ClangBuiltLinux/linux/issues/171
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Fix enum values in commit message (previously all 0)
drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
index 4d1f9bf53c53..85a7490e6bbd 100644
--- a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -1261,13 +1261,13 @@ bool exhalbtc_initlize_variables_wifi_only(struct
rtl_priv *rtlpriv)
switch (rtlpriv->rtlhal.interface) {
case INTF_PCI:
- wifionly_cfg->chip_interface = BTC_INTF_PCI;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_PCI;
break;
case INTF_USB:
- wifionly_cfg->chip_interface = BTC_INTF_USB;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_USB;
break;
default:
- wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
+ wifionly_cfg->chip_interface = WIFIONLY_INTF_UNKNOWN;
break;
}
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: Use proper enumerated types for Wi-Fi only interface
On Fri, Sep 21, 2018 at 03:12:02PM -0700, Nathan Chancellor wrote:
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1264:34: warning:
> implicit conversion from enumeration type 'enum btc_chip_interface' to
> different enumeration type 'enum wifionly_chip_interface'
> [-Wenum-conversion]
> wifionly_cfg->chip_interface = BTC_INTF_PCI;
> ~ ^~~~
> drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1267:34: warning:
> implicit conversion from enumeration type 'enum btc_chip_interface' to
> different enumeration type 'enum wifionly_chip_interface'
> [-Wenum-conversion]
> wifionly_cfg->chip_interface = BTC_INTF_USB;
> ~ ^~~~
> drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c:1270:34: warning:
> implicit conversion from enumeration type 'enum btc_chip_interface' to
> different enumeration type 'enum wifionly_chip_interface'
> [-Wenum-conversion]
> wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
> ~ ^~~~
> 3 warnings generated.
>
> Use the values from the correct enumerated type, wifionly_chip_interface.
>
> BTC_INTF_UNKNOWN = WIFIONLY_INTF_UNKNOWN = 0
> BTC_INTF_PCI = WIFIONLY_INTF_PCI = 0
> BTC_INTF_USB = WIFIONLY_INTF_USB = 0
>
I have sent a v2 making these values correct.
I will make sure the '--in-reply-to' option in the future.
Nathan
> Link: https://github.com/ClangBuiltLinux/linux/issues/171
> Signed-off-by: Nathan Chancellor
> ---
> drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> index 4d1f9bf53c53..85a7490e6bbd 100644
> --- a/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/staging/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -1261,13 +1261,13 @@ bool exhalbtc_initlize_variables_wifi_only(struct
> rtl_priv *rtlpriv)
>
> switch (rtlpriv->rtlhal.interface) {
> case INTF_PCI:
> - wifionly_cfg->chip_interface = BTC_INTF_PCI;
> + wifionly_cfg->chip_interface = WIFIONLY_INTF_PCI;
> break;
> case INTF_USB:
> - wifionly_cfg->chip_interface = BTC_INTF_USB;
> + wifionly_cfg->chip_interface = WIFIONLY_INTF_USB;
> break;
> default:
> - wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
> + wifionly_cfg->chip_interface = WIFIONLY_INTF_UNKNOWN;
> break;
> }
>
> --
> 2.19.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8188eu: Simplify memcmp logical checks
Clang generates a warning when it sees a logical not followed by a
conditional operator like ==, >, or < because it thinks that the logical
not should be applied to the whole statement:
drivers/staging/rtl8188eu/core/rtw_ieee80211.c:293:8: warning: logical
not is only applied to the left hand side of this comparison
[-Wlogical-not-parentheses]
It assumes the author might have made a mistake in their logic:
if (!a == b) -> if (!(a == b))
Sometimes that is the case; other times, it's just a super convoluted
way of saying 'if (a)' when b = 0:
if (!1 == 0) -> if (0 == 0) -> if (true)
Alternatively:
if (!1 == 0) -> if (!!1) -> if (1)
Simplify these comparisons so that Clang doesn't complain.
Link: https://github.com/ClangBuiltLinux/linux/issues/161
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Fix missing parenthesis in last hunk
drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +-
drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
drivers/staging/rtl8188eu/core/rtw_recv.c | 4 ++--
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
index 9e5c7e62d26f..20f34d25c369 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
@@ -284,7 +284,7 @@ unsigned char *rtw_get_wpa_ie(unsigned char *pie, uint
*wpa_ie_len, int limit)
if (pbuf) {
/* check if oui matches... */
- if (!memcmp((pbuf + 2), wpa_oui_type,
sizeof(wpa_oui_type)) == false)
+ if (memcmp((pbuf + 2), wpa_oui_type,
sizeof(wpa_oui_type)))
goto check_next_ie;
/* check version... */
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index ef8a7dc4bd34..43d6513484c5 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -1437,7 +1437,7 @@ static int rtw_check_join_candidate(struct mlme_priv
*pmlmepriv
/* check ssid, if needed */
if (pmlmepriv->assoc_ssid.SsidLength) {
if (competitor->network.Ssid.SsidLength !=
pmlmepriv->assoc_ssid.SsidLength ||
- !memcmp(competitor->network.Ssid.Ssid,
pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength) == false)
+ memcmp(competitor->network.Ssid.Ssid,
pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
goto exit;
}
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index ab9638d618a9..f3eb63f8cf0b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -1283,8 +1283,8 @@ static int wlanhdr_to_ethhdr(struct recv_frame
*precvframe)
psnap_type = ptr+pattrib->hdrlen + pattrib->iv_len+SNAP_SIZE;
/* convert hdr + possible LLC headers into Ethernet header */
if ((!memcmp(psnap, rtw_rfc1042_header, SNAP_SIZE) &&
-(!memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) == false) &&
-(!memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2) == false)) ||
+memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) &&
+memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2)) ||
!memcmp(psnap, rtw_bridge_tunnel_header, SNAP_SIZE)) {
/* remove RFC1042 or Bridge-Tunnel encapsulation and replace
EtherType */
bsnaphdr = true;
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index fb496ab5a862..53fac22ff621 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -856,7 +856,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
*pframe, u32 packet_len)
return _FAIL;
}
- if (!memcmp(cur_network->network.MacAddress, pbssid, 6) == false) {
+ if (memcmp(cur_network->network.MacAddress, pbssid, 6)) {
DBG_88E("Oops: rtw_check_network_encrypt linked but recv other
bssid bcn\n%pM %pM\n",
(pbssid), (cur_network->network.MacAddress));
return true;
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: Simplify memcmp logical checks
On Tue, Sep 25, 2018 at 09:07:11PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 20, 2018 at 05:22:21PM -0700, Nathan Chancellor wrote:
> > Clang generates a warning when it sees a logical not followed by a
> > conditional operator like ==, >, or < because it thinks that the logical
> > not should be applied to the whole statement:
> >
> > drivers/staging/rtl8188eu/core/rtw_ieee80211.c:293:8: warning: logical
> > not is only applied to the left hand side of this comparison
> > [-Wlogical-not-parentheses]
> >
> > It assumes the author might have made a mistake in their logic:
> >
> > if (!a == b) -> if (!(a == b))
> >
> > Sometimes that is the case; other times, it's just a super convoluted
> > way of saying 'if (a)' when b = 0:
> >
> > if (!1 == 0) -> if (0 == 0) -> if (true)
> >
> > Alternatively:
> >
> > if (!1 == 0) -> if (!!1) -> if (1)
> >
> > Simplify these comparisons so that Clang doesn't complain.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/161
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +-
> > drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
> > drivers/staging/rtl8188eu/core/rtw_recv.c | 4 ++--
> > drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 +-
> > 4 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > index 9e5c7e62d26f..20f34d25c369 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c
> > @@ -284,7 +284,7 @@ unsigned char *rtw_get_wpa_ie(unsigned char *pie, uint
> > *wpa_ie_len, int limit)
> >
> > if (pbuf) {
> > /* check if oui matches... */
> > - if (!memcmp((pbuf + 2), wpa_oui_type,
> > sizeof(wpa_oui_type)) == false)
> > + if (memcmp((pbuf + 2), wpa_oui_type,
> > sizeof(wpa_oui_type)))
> > goto check_next_ie;
> >
> > /* check version... */
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > index ef8a7dc4bd34..43d6513484c5 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > @@ -1437,7 +1437,7 @@ static int rtw_check_join_candidate(struct mlme_priv
> > *pmlmepriv
> > /* check ssid, if needed */
> > if (pmlmepriv->assoc_ssid.SsidLength) {
> > if (competitor->network.Ssid.SsidLength !=
> > pmlmepriv->assoc_ssid.SsidLength ||
> > - !memcmp(competitor->network.Ssid.Ssid,
> > pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength) == false)
> > + memcmp(competitor->network.Ssid.Ssid,
> > pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
> > goto exit;
> > }
> >
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
> > b/drivers/staging/rtl8188eu/core/rtw_recv.c
> > index ab9638d618a9..f3eb63f8cf0b 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> > @@ -1283,8 +1283,8 @@ static int wlanhdr_to_ethhdr(struct recv_frame
> > *precvframe)
> > psnap_type = ptr+pattrib->hdrlen + pattrib->iv_len+SNAP_SIZE;
> > /* convert hdr + possible LLC headers into Ethernet header */
> > if ((!memcmp(psnap, rtw_rfc1042_header, SNAP_SIZE) &&
> > -(!memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) == false) &&
> > -(!memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2) == false)) ||
> > +memcmp(psnap_type, SNAP_ETH_TYPE_IPX, 2) &&
> > +memcmp(psnap_type, SNAP_ETH_TYPE_APPLETALK_AARP, 2)) ||
> > !memcmp(psnap, rtw_bridge_tunnel_header, SNAP_SIZE)) {
> > /* remove RFC1042 or Bridge-Tunnel encapsulation and replace
> > EtherType */
> > bsnaphdr = true;
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > index fb496ab5a862..51cf78150168 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
> > @@ -856,7 +856,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
> > *pframe, u32 pa
[PATCH] staging: rtl8723bs: Remove ACPI table declaration
Clang warns that the acpi_id declaration is not going to be emitted
in the final assembly:
drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
'acpi_ids' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static const struct acpi_device_id acpi_ids[] = {
^
1 warning generated.
This is because it's marked as static const and it is not used anywhere
in this file. Doing a git grep on this driver for 'acpi' shows that this
declaration has been unused since the driver's initial induction. Remove
it since it's not doing anything.
Link: https://github.com/ClangBuiltLinux/linux/issues/169
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 6d02904de63f..d473f9bd08c3 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -22,13 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
{ SDIO_DEVICE(0x024c, 0xb723), },
{ /* end: all zeroes */ },
};
-static const struct acpi_device_id acpi_ids[] = {
- {"OBDA8723", 0x},
- {}
-};
-
MODULE_DEVICE_TABLE(sdio, sdio_ids);
-MODULE_DEVICE_TABLE(acpi, acpi_ids);
static int rtw_drv_init(struct sdio_func *func, const struct sdio_device_id
*id);
static void rtw_dev_remove(struct sdio_func *func);
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Remove ACPI table declaration
On Wed, Sep 26, 2018 at 08:22:45AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 25, 2018 at 10:39:10PM -0700, Nathan Chancellor wrote:
> > Clang warns that the acpi_id declaration is not going to be emitted
> > in the final assembly:
> >
> > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
> > 'acpi_ids' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static const struct acpi_device_id acpi_ids[] = {
> >^
> > 1 warning generated.
> >
> > This is because it's marked as static const and it is not used anywhere
> > in this file. Doing a git grep on this driver for 'acpi' shows that this
> > declaration has been unused since the driver's initial induction. Remove
> > it since it's not doing anything.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/169
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 6 --
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > index 6d02904de63f..d473f9bd08c3 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > @@ -22,13 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
> > { SDIO_DEVICE(0x024c, 0xb723), },
> > { /* end: all zeroes */ },
> > };
> > -static const struct acpi_device_id acpi_ids[] = {
> > - {"OBDA8723", 0x},
> > - {}
> > -};
> > -
> > MODULE_DEVICE_TABLE(sdio, sdio_ids);
> > -MODULE_DEVICE_TABLE(acpi, acpi_ids);
>
> You just removed the ability for the driver to be automatically loaded
> if that acpi id is present.
>
I am not sure I understand. Every instance of acpi_device_id that I
looked at in the kernel before sending this patch uses MODULE_DEVICE_TABLE
but then that acpi_device_id declaration is always used in the driver
definition either under the acpi_match_table member or ids member
depending on what type of driver it is. Should this one do that as well?
Is that even possible with an sdio driver? I apologize if I am not
making sense, I'm not super familiar with these interfaces.
I also read 'Documentation/acpi/enumeration.txt' which makes it seem
like the declaration needs to be added to the device definition as well.
> Not good :(
>
> I think you need to fix up your scripts, this is valid code...
>
No scripts here, just a human interpreting warnings manually.
> greg k-h
Thanks for the review,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Remove ACPI table declaration
On Wed, Sep 26, 2018 at 08:49:15AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 25, 2018 at 11:44:37PM -0700, Nathan Chancellor wrote:
> > On Wed, Sep 26, 2018 at 08:22:45AM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Sep 25, 2018 at 10:39:10PM -0700, Nathan Chancellor wrote:
> > > > Clang warns that the acpi_id declaration is not going to be emitted
> > > > in the final assembly:
> > > >
> > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
> > > > 'acpi_ids' is not needed and will not be emitted
> > > > [-Wunneeded-internal-declaration]
> > > > static const struct acpi_device_id acpi_ids[] = {
> > > >^
> > > > 1 warning generated.
> > > >
> > > > This is because it's marked as static const and it is not used anywhere
> > > > in this file. Doing a git grep on this driver for 'acpi' shows that this
> > > > declaration has been unused since the driver's initial induction. Remove
> > > > it since it's not doing anything.
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169
> > > > Signed-off-by: Nathan Chancellor
> > > > ---
> > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 6 --
> > > > 1 file changed, 6 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > index 6d02904de63f..d473f9bd08c3 100644
> > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > @@ -22,13 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
> > > > { SDIO_DEVICE(0x024c, 0xb723), },
> > > > { /* end: all zeroes */ },
> > > > };
> > > > -static const struct acpi_device_id acpi_ids[] = {
> > > > - {"OBDA8723", 0x},
> > > > - {}
> > > > -};
> > > > -
> > > > MODULE_DEVICE_TABLE(sdio, sdio_ids);
> > > > -MODULE_DEVICE_TABLE(acpi, acpi_ids);
> > >
> > > You just removed the ability for the driver to be automatically loaded
> > > if that acpi id is present.
> > >
> >
> > I am not sure I understand. Every instance of acpi_device_id that I
> > looked at in the kernel before sending this patch uses MODULE_DEVICE_TABLE
> > but then that acpi_device_id declaration is always used in the driver
> > definition either under the acpi_match_table member or ids member
> > depending on what type of driver it is. Should this one do that as well?
>
> I don't know, but it's not always necessary.
>
> > Is that even possible with an sdio driver? I apologize if I am not
> > making sense, I'm not super familiar with these interfaces.
> >
> > I also read 'Documentation/acpi/enumeration.txt' which makes it seem
> > like the declaration needs to be added to the device definition as well.
>
> If it is a real acpi driver, yes, you need to actually register it with
> the acpi core.
>
> But, if it is just using the auto-load functionality of "if this acpi
> string is found, load the module!" then the code is correct as-is.
>
> Yeah, it's a hack, but does work and helps out for auto-loading drivers
> for "odd" hardware that is not always self-describing.
>
> thanks,
>
> greg k-h
Thank you for the explanation, I appreciate that and I'll keep it in
mind should I run across this kind of warning in the future. I'll send a
v2 now that silences this warning through the '__maybe_unused' attribute
like a few other commits that deal with this warning.
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused
Clang emits the following warning:
drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
'acpi_ids' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static const struct acpi_device_id acpi_ids[] = {
^
1 warning generated.
Mark the declaration as maybe unused like a few other instances of this
construct in the kernel.
Link: https://github.com/ClangBuiltLinux/linux/issues/169
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 6d02904de63f..3285bf36291b 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
{ SDIO_DEVICE(0x024c, 0xb723), },
{ /* end: all zeroes */ },
};
-static const struct acpi_device_id acpi_ids[] = {
+static const struct acpi_device_id acpi_ids[] __maybe_unused = {
{"OBDA8723", 0x},
{}
};
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused
On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote:
> > Clang emits the following warning:
> >
> > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
> > 'acpi_ids' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static const struct acpi_device_id acpi_ids[] = {
> >^
> > 1 warning generated.
> >
> > Mark the declaration as maybe unused like a few other instances of this
> > construct in the kernel.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/169
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > index 6d02904de63f..3285bf36291b 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
> > { SDIO_DEVICE(0x024c, 0xb723), },
> > { /* end: all zeroes */ },
> > };
> > -static const struct acpi_device_id acpi_ids[] = {
> > +static const struct acpi_device_id acpi_ids[] __maybe_unused = {
>
> But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE()
> macro does a functional thing. Why is gcc not reporting an issue with
> this and clang is?
>
> thanks,
>
> greg k-h
I am not entirely sure, I've added Nick to this thread to see what he
thinks. I'm by no means a Clang expert at the moment.
Thanks,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: Mark ACPI table declaration as used
Clang emits the following warning:
drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
'acpi_ids' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static const struct acpi_device_id acpi_ids[] = {
^
1 warning generated.
Mark acpi_ids with the attribute __used, which makes it clear to Clang
that we don't want this warning while not inhibiting Clang's dead code
elimination from removing the unreferenced internal symbol when moving
the data to the globally available symbol with MODULE_DEVICE_TABLE.
$ nm -S drivers/staging/rtl8723bs/os_dep/sdio_intf.o | grep acpi
0040 R __mod_acpi__acpi_ids_device_table
Link: https://github.com/ClangBuiltLinux/linux/issues/169
Suggested-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index 6d02904de63f..7c03b69b8ed3 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
{ SDIO_DEVICE(0x024c, 0xb723), },
{ /* end: all zeroes */ },
};
-static const struct acpi_device_id acpi_ids[] = {
+static const struct acpi_device_id acpi_ids[] __used = {
{"OBDA8723", 0x},
{}
};
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: Use proper enum in rtl8188eu_config_rf_reg
Clang warns when an enumerated type is implicitly converted to another. drivers/staging/rtl8188eu/hal/rf_cfg.c:180:25: warning: implicit conversion from enumeration type 'enum rf90_radio_path' to different enumeration type 'enum rf_radio_path' [-Wenum-conversion] rtl_rfreg_delay(adapt, RF90_PATH_A, addr | maskforphyset, ~~~^~~ 1 warning generated. Avoid this by using the equivalent value from the expected type, rf_radio_path: RF90_PATH_A = RF_PATH_A = 0 Link: https://github.com/ClangBuiltLinux/linux/issues/164 Signed-off-by: Nathan Chancellor --- drivers/staging/rtl8188eu/hal/rf_cfg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/hal/rf_cfg.c b/drivers/staging/rtl8188eu/hal/rf_cfg.c index 0700d8bd448d..02aeb12c9870 100644 --- a/drivers/staging/rtl8188eu/hal/rf_cfg.c +++ b/drivers/staging/rtl8188eu/hal/rf_cfg.c @@ -177,7 +177,7 @@ static void rtl8188e_config_rf_reg(struct adapter *adapt, u32 content = 0x1000; /*RF Content: radio_a_txt*/ u32 maskforphyset = content & 0xE000; - rtl_rfreg_delay(adapt, RF90_PATH_A, addr | maskforphyset, + rtl_rfreg_delay(adapt, RF_PATH_A, addr | maskforphyset, RFREG_OFFSET_MASK, data); } -- 2.19.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning
Clang warns:
drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
variable 'mains_freq_qmenu' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static const s64 mains_freq_qmenu[] = {
^
1 warning generated.
This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
macro, which is a compile time evaluation in this case. Avoid this by
adding mains_freq_qmenu as the imenu member of this structure, which
matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.
Link: https://github.com/ClangBuiltLinux/linux/issues/122
Signed-off-by: Nathan Chancellor
---
drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c
b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index cff7b1e07153..a2c55cb2192a 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl
v4l2_ctrls[V4L2_CTRL_COUNT] = {
{
V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
0, ARRAY_SIZE(mains_freq_qmenu) - 1,
- 1, 1, NULL,
+ 1, 1, mains_freq_qmenu,
MMAL_PARAMETER_FLICKER_AVOID,
&ctrl_set_flicker_avoidance,
false
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning
On Fri, Sep 28, 2018 at 10:04:29AM +0100, Dave Stevenson wrote:
> Hi Nate
>
> Thanks for the patch.
>
> On Fri, 28 Sep 2018 at 01:53, Nathan Chancellor
> wrote:
> >
> > Clang warns:
> >
> > drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
> > variable 'mains_freq_qmenu' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static const s64 mains_freq_qmenu[] = {
> > ^
> > 1 warning generated.
> >
> > This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
> > macro, which is a compile time evaluation in this case. Avoid this by
> > adding mains_freq_qmenu as the imenu member of this structure, which
> > matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
> > This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
> > defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
> > definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.
>
> There's a slight confusion betwen V4L2_CID_MPEG_VIDEO_BITRATE_MODE and
> V4L2_CID_POWER_LINE_FREQUENCY in your description.
>
> However you're right that MMAL_CONTROL_TYPE_STD_MENU calls
> v4l2_ctrl_new_std_menu which doesn't need a menu array (it's
> v4l2_ctrl_new_int_menu that does). That means the correct fix is to
> define the max value using the relevant enum (in this case
> V4L2_CID_POWER_LINE_FREQUENCY_AUTO) and remove the array.
>
> The same is true for V4L2_CID_MPEG_VIDEO_BITRATE_MODE - max should be
> V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, and remove the bitrate_mode_qmenu
> array.
>
> Thanks,
> Dave
>
Hi Dave,
Thank you for the review, I appreciate it. I had certainly considered
that solution first butnfigured this was the more conservative change.
I will send a v2 soon with the suggested change.
Nathan
> > Link: https://github.com/ClangBuiltLinux/linux/issues/122
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > index cff7b1e07153..a2c55cb2192a 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > @@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl
> > v4l2_ctrls[V4L2_CTRL_COUNT] = {
> > {
> > V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
> > 0, ARRAY_SIZE(mains_freq_qmenu) - 1,
> > - 1, 1, NULL,
> > + 1, 1, mains_freq_qmenu,
> > MMAL_PARAMETER_FLICKER_AVOID,
> > &ctrl_set_flicker_avoidance,
> > false
> > --
> > 2.19.0
> >
> >
> > ___
> > linux-rpi-kernel mailing list
> > linux-rpi-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtlwifi: Remove unnecessary parentheses
Clang warns when multiple pairs of parentheses are used for a single
conditional statement.
drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c:558:33: warning:
equality comparison with extraneous parentheses [-Wparentheses-equality]
} else if ((is_enable_la_mode == 1)) {
~~^~~~
drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c:558:33: note: remove
extraneous parentheses around the comparison to silence this warning
} else if ((is_enable_la_mode == 1)) {
~ ^ ~
drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c:558:33: note: use '='
to turn this equality comparison into an assignment
} else if ((is_enable_la_mode == 1)) {
^~
=
1 warning generated.
Link: https://github.com/ClangBuiltLinux/linux/issues/172
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
b/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
index 42020101380a..d6cea73fa185 100644
--- a/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
+++ b/drivers/staging/rtlwifi/phydm/phydm_adc_sampling.c
@@ -555,7 +555,7 @@ void phydm_lamode_trigger_setting(void *dm_void, char
input[][16], u32 *_used,
output + used, out_len - used,
"{En} {0:BB,1:BB_MAC,2:RF0,3:RF1,4:MAC}\n
{BB:dbg_port[bit],BB_MAC:0-ok/1-fail/2-cca,MAC:ref} {DMA type} {TrigTime}\n
{polling_time/ref_mask} {dbg_port} {0:P_Edge, 1:N_Edge}
{SpRate:0-80M,1-40M,2-20M} {Capture num}\n");
/**/
- } else if ((is_enable_la_mode == 1)) {
+ } else if (is_enable_la_mode == 1) {
PHYDM_SSCANF(input[2], DCMD_DECIMAL, &var1[1]);
trig_mode = (u8)var1[1];
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: Remove unnecessary parentheses and dead commented code
Clang warns when multiple pairs of parentheses are used for a single
conditional statement.
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c:3351:20: warning:
equality comparison with extraneous parentheses [-Wparentheses-equality]
The commented code is pointless since _HW_STATE_AP_ is handled right
below this block.
Link: https://github.com/ClangBuiltLinux/linux/issues/168
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index 592917fc00aa..c7e55618b9a8 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -3348,7 +3348,7 @@ static void hw_var_set_opmode(struct adapter *padapter,
u8 variable, u8 *val)
/* disable atim wnd */
rtw_write8(padapter, REG_BCN_CTRL,
DIS_TSF_UDT|EN_BCN_FUNCTION|DIS_ATIM);
/* rtw_write8(padapter, REG_BCN_CTRL, 0x18); */
- } else if ((mode == _HW_STATE_ADHOC_) /*|| (mode ==
_HW_STATE_AP_)*/) {
+ } else if (mode == _HW_STATE_ADHOC_) {
ResumeTxBeacon(padapter);
rtw_write8(padapter, REG_BCN_CTRL,
DIS_TSF_UDT|EN_BCN_FUNCTION|DIS_BCNQ_SUB);
} else if (mode == _HW_STATE_AP_) {
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: emxx_udc: Remove unused device_desc declaration
Clang warns:
drivers/staging/emxx_udc/emxx_udc.c:1373:37: warning: variable
'device_desc' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static struct usb_device_descriptor device_desc = {
^
1 warning generated.
This definition hasn't been attached to anything since the driver was
introduced in commit 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile
USB Gadget driver") and neither GCC nor Clang emit any reference to the
variable in the final assembly. The only reason GCC doesn't warn about
this variable being unused is the sizeof function.
Reported-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
This seems kind of wrong given this is a USB driver but there isn't an
instance of a platform_driver in the kernel tree having a usb device
descriptor declaration so I'm unsure of how to handle this warning aside
from just removing the definition but I'm certainly open to suggestions.
drivers/staging/emxx_udc/emxx_udc.c | 19 ---
1 file changed, 19 deletions(-)
diff --git a/drivers/staging/emxx_udc/emxx_udc.c
b/drivers/staging/emxx_udc/emxx_udc.c
index 0e8d3f232fe9..65cc3d9af972 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -1368,25 +1368,6 @@ static void _nbu2ss_set_endpoint_stall(
}
}
-/*-*/
-/* Device Descriptor */
-static struct usb_device_descriptor device_desc = {
- .bLength = sizeof(device_desc),
- .bDescriptorType = USB_DT_DEVICE,
- .bcdUSB = cpu_to_le16(0x0200),
- .bDeviceClass = USB_CLASS_VENDOR_SPEC,
- .bDeviceSubClass = 0x00,
- .bDeviceProtocol = 0x00,
- .bMaxPacketSize0 = 64,
- .idVendor = cpu_to_le16(0x0409),
- .idProduct= cpu_to_le16(0xfff0),
- .bcdDevice= 0x,
- .iManufacturer= 0x00,
- .iProduct = 0x00,
- .iSerialNumber= 0x00,
- .bNumConfigurations = 0x01,
-};
-
/*-*/
static void _nbu2ss_set_test_mode(struct nbu2ss_udc *udc, u32 mode)
{
--
2.19.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: qlge/qlge_main: Use min_t instead of min
On Fri, Feb 05, 2021 at 03:24:51AM +0530, ameynarkhed...@gmail.com wrote: > From: Amey Narkhede > > Use min_t instead of min function in qlge/qlge_main.c > Fixes following checkpatch.pl warning: > WARNING: min() should probably be min_t(int, MAX_CPUS, num_online_cpus()) > > Signed-off-by: Amey Narkhede > --- > drivers/staging/qlge/qlge_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/qlge/qlge_main.c > b/drivers/staging/qlge/qlge_main.c > index 402edaeff..29606d1eb 100644 > --- a/drivers/staging/qlge/qlge_main.c > +++ b/drivers/staging/qlge/qlge_main.c > @@ -3938,7 +3938,7 @@ static int ql_configure_rings(struct ql_adapter *qdev) > int i; > struct rx_ring *rx_ring; > struct tx_ring *tx_ring; > - int cpu_cnt = min(MAX_CPUS, (int)num_online_cpus()); > + int cpu_cnt = min_t(int, MAX_CPUS, (int)num_online_cpus()); You should remove the cast on num_online_cpus() like checkpatch suggests. min_t adds the cast to int on both of the inputs for you. > > /* In a perfect world we have one RSS ring for each CPU >* and each has it's own vector. To do that we ask for > -- > 2.30.0 Cheers, Nathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: andriod: ashmem: Declared file operation with const keyword
On Sat, Mar 13, 2021 at 10:59:43PM +0530, B K Karthik wrote: > On Sat, Mar 13, 2021 at 10:57 PM namratajanawade > wrote: > > > > Warning found by checkpatch.pl script. > > That doesn't tell what you did or why you did it. Please write an > appropriate commit description and resend the patch. > > karthik This patch will not even build, it has been sent several times before... https://lore.kernel.org/r/2020101605.1947-1-kirank.su...@gmail.com/ https://lore.kernel.org/r/20200328151523.17516-1-sandeshkenjanaas...@gmail.com/ https://lore.kernel.org/r/20201128121627.GA27317@worker-node1/ https://lore.kernel.org/r/20201227112645.256943-1-senguptaangshuma...@gmail.com/ https://lore.kernel.org/r/20201228051301.14983-1-jovin...@gmail.com/ https://lore.kernel.org/r/20210214023136.8916-1-thaiscamac...@gmail.com/ https://lore.kernel.org/r/20210219101338.2670-1-amritkher...@gmail.com/ https://lore.kernel.org/r/20210306063817.674041-1-nabil.ibn.mah...@gmail.com/ I once considered adding a comment above it saying that it should not be marked const but it is a good benchmark for seeing if people compile their patches before sending them out. Cheers, Nathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [staging:staging-next 201/268] drivers/staging/rtl8723bs/core/rtw_security.c:89:6: warning: stack frame size of 1120 bytes in function 'rtw_wep_encrypt'
Hi Fabio, On Fri, May 21, 2021 at 04:26:57PM +0200, Fabio Aiuto wrote: > Hi robot, > > On Thu, May 20, 2021 at 05:03:14PM +0800, kernel test robot wrote: > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git > > staging-next > > head: b9f38e913a926b455e5048a95f53a993b515509f > > commit: 1b11e893eda0907fc9b28696271e2d9c4337e42d [201/268] staging: > > rtl8723bs: replace private arc4 encryption with in-kernel one > > config: powerpc64-randconfig-r011-20210520 (attached as .config) > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project > > bf9ef3efaa99c02e7bfc4c57207301b8de39a278) > > reproduce (this is a W=1 build): > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # install powerpc64 cross compiling tool for clang build > > # apt-get install binutils-powerpc64-linux-gnu > > # > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=1b11e893eda0907fc9b28696271e2d9c4337e42d > > git remote add staging > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git > > git fetch --no-tags staging staging-next > > git checkout 1b11e893eda0907fc9b28696271e2d9c4337e42d > > # save the attached .config to linux build tree > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > > ARCH=powerpc64 > > can't compile this, I get the following error: > > make CONFIG_OF_ALL_DTBS=y CONFIG_DTC=y > HOSTCC=/home/fabio/0day/clang/bin/clang CC=/home/fabio/0day/clang/bin/clang > LD=/home/fabio/0day/clang/bin/ld.lld HOSTLD=/home/fabio/0day/clang/bin/ld.lld > AR=/home/fabio/0day/clang/bin/llvm-ar NM=/home/fabio/0day/clang/bin/llvm-nm > STRIP=/home/fabio/0day/clang/bin/llvm-strip > OBJDUMP=/home/fabio/0day/clang/bin/llvm-objdump > OBJSIZE=/home/fabio/0day/clang/bin/llvm-size > READELF=/home/fabio/0day/clang/bin/llvm-readelf > HOSTCXX=/home/fabio/0day/clang/bin/clang++ > HOSTAR=/home/fabio/0day/clang/bin/llvm-ar CROSS_COMPILE=powerpc-linux-gnu- > --jobs=8 LLVM_IAS=1 ARCH=powerpc drivers/staging/rtl8723bs/ > CC scripts/mod/empty.o > clang: error: unsupported argument '-me500' to option 'Wa,' > make[1]: *** [scripts/Makefile.build:272: scripts/mod/empty.o] Error 1 > make[1]: *** Attesa per i processi non terminati > make: *** [Makefile:1226: prepare0] Error 2 > > moreover I had to add LLVM_IAS=1, and ARCH=powerpc64 is not a valid > architecture (used powerpc instead). > > Could you help me? > This is not a clang specific issue, I would not bother with trying to use the bot's reproducer steps. I can reproduce it with GCC 11.1.0 using the following commands: $ make -skj"$(nproc)" ARCH=i386 defconfig $ scripts/config -e MMC -e STAGING -m RTL8723BS $ make -skj"$(nproc)" ARCH=i386 olddefconfig drivers/staging/rtl8723bs/ drivers/staging/rtl8723bs/core/rtw_security.c: In function ‘rtw_wep_encrypt’: drivers/staging/rtl8723bs/core/rtw_security.c:91:1: warning: the frame size of 1084 bytes is larger than 1024 bytes [-Wframe-larger-than=] 91 | } | ^ drivers/staging/rtl8723bs/core/rtw_security.c: In function ‘rtw_wep_decrypt’: drivers/staging/rtl8723bs/core/rtw_security.c:128:1: warning: the frame size of 1060 bytes is larger than 1024 bytes [-Wframe-larger-than=] 128 | } | ^ drivers/staging/rtl8723bs/core/rtw_security.c: In function ‘rtw_tkip_encrypt’: drivers/staging/rtl8723bs/core/rtw_security.c:531:1: warning: the frame size of 1112 bytes is larger than 1024 bytes [-Wframe-larger-than=] 531 | } | ^ drivers/staging/rtl8723bs/core/rtw_security.c: In function ‘rtw_tkip_decrypt’: drivers/staging/rtl8723bs/core/rtw_security.c:633:1: warning: the frame size of 1084 bytes is larger than 1024 bytes [-Wframe-larger-than=] 633 | } | ^ Your commit introduced this because the size of the arc4_ctx structure is 1032 bytes so allocating it on the stack will cause it to go over the 32-bit limit of 1024 bytes. The previous arc4context was only 264 bytes. For that large of structure, I would recommend allocating it on the heap with kzalloc() and freeing with kfree_sensitive(). Cheers, Nathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > The new field 'dma_range_map' in struct device is used to facilitate the > use of single or multiple offsets between mapping regions of cpu addrs and > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > capable of holding a single uniform offset and had no region bounds > checking. > > The function of_dma_get_range() has been modified so that it takes a single > argument -- the device node -- and returns a map, NULL, or an error code. > The map is an array that holds the information regarding the DMA regions. > Each range entry contains the address offset, the cpu_start address, the > dma_start address, and the size of the region. > > of_dma_configure() is the typical manner to set range offsets but there are > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > driver code. These cases now invoke the function > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > Signed-off-by: Jim Quinlan > --- > arch/arm/include/asm/dma-mapping.h| 10 +-- > arch/arm/mach-keystone/keystone.c | 17 +++-- > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > arch/x86/pci/sta2x11-fixup.c | 7 +- > drivers/acpi/arm64/iort.c | 5 +- > drivers/base/core.c | 2 + > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > drivers/iommu/io-pgtable-arm.c| 2 +- > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > drivers/of/address.c | 72 +-- > drivers/of/device.c | 43 ++- > drivers/of/of_private.h | 10 +-- > drivers/of/unittest.c | 34 ++--- > drivers/remoteproc/remoteproc_core.c | 8 ++- > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > drivers/usb/core/message.c| 9 ++- > drivers/usb/core/usb.c| 7 +- > include/linux/device.h| 4 +- > include/linux/dma-direct.h| 8 +-- > include/linux/dma-mapping.h | 36 ++ > kernel/dma/coherent.c | 10 +-- > kernel/dma/mapping.c | 66 + > 23 files changed, 265 insertions(+), 115 deletions(-) Apologies if this has already been reported or is known but this commit is now in next-20200902 and it causes my Raspberry Pi 4 to no longer make it to userspace, instead spewing mmc errors: That commit causes my Raspberry Pi 4 to no longer make it to userspace, instead spewing mmc errors: [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 13:48:49 MST 2020 [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 ... [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from 2020-08-24T18:50:56 [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy regulator [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy regulator [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] using PIO [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.705173] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.713788] mmc0: queuing unknown CIS tuple 0x80 (7 bytes) [1.721228] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.732062] mmc1: SDHCI controller on fe34.emmc2 [fe34.emmc2] using ADMA [1.741828] ALSA device list: [1.744885] No soundcards found. [1.748540] Waiting for root device PARTUUID=45a8dd8a-02... [1.788865] random: fast init done [1.793489] mmc1: unrecognised SCR structure version 4 [1.798814] mmc1: error -22 whilst initialising SD card [1.813969] mmc0: new high speed SDIO card at address 0001 [1.883178] mmc1: unrecognised SCR structure version 2 [1.888423] mmc1: error -22 whilst initialising SD card [1.964069] mmc1: unrecognised SCR structure version 4 [1.969314] mmc1: error -22 whilst initialising SD card [2.061225] mmc1: unrecognised SCR structure version 4 [2.066470] mmc1: error -22 whilst initialising SD card [3.160476] mmc1: unrecognised SCR structure version 4 [3.165718] mmc1: error -22 whilst initialising SD card This is what it looks like before that co
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 02, 2020 at 06:11:08PM -0400, Jim Quinlan wrote: > On Wed, Sep 2, 2020 at 5:53 PM Nathan Chancellor > wrote: > > > > On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > > > The new field 'dma_range_map' in struct device is used to facilitate the > > > use of single or multiple offsets between mapping regions of cpu addrs and > > > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > > > capable of holding a single uniform offset and had no region bounds > > > checking. > > > > > > The function of_dma_get_range() has been modified so that it takes a > > > single > > > argument -- the device node -- and returns a map, NULL, or an error code. > > > The map is an array that holds the information regarding the DMA regions. > > > Each range entry contains the address offset, the cpu_start address, the > > > dma_start address, and the size of the region. > > > > > > of_dma_configure() is the typical manner to set range offsets but there > > > are > > > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > > > driver code. These cases now invoke the function > > > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > > > > > Signed-off-by: Jim Quinlan > > > --- > > > arch/arm/include/asm/dma-mapping.h| 10 +-- > > > arch/arm/mach-keystone/keystone.c | 17 +++-- > > > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > > > arch/x86/pci/sta2x11-fixup.c | 7 +- > > > drivers/acpi/arm64/iort.c | 5 +- > > > drivers/base/core.c | 2 + > > > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > > > drivers/iommu/io-pgtable-arm.c| 2 +- > > > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > > > drivers/of/address.c | 72 +-- > > > drivers/of/device.c | 43 ++- > > > drivers/of/of_private.h | 10 +-- > > > drivers/of/unittest.c | 34 ++--- > > > drivers/remoteproc/remoteproc_core.c | 8 ++- > > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > > > drivers/usb/core/message.c| 9 ++- > > > drivers/usb/core/usb.c| 7 +- > > > include/linux/device.h| 4 +- > > > include/linux/dma-direct.h| 8 +-- > > > include/linux/dma-mapping.h | 36 ++ > > > kernel/dma/coherent.c | 10 +-- > > > kernel/dma/mapping.c | 66 + > > > 23 files changed, 265 insertions(+), 115 deletions(-) > > > > Apologies if this has already been reported or is known but this commit > > is now in next-20200902 and it causes my Raspberry Pi 4 to no longer > > make it to userspace, instead spewing mmc errors: > > > > That commit causes my Raspberry Pi 4 to no longer make it to userspace, > > instead spewing mmc errors: > > > > [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] > > [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty > > (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 > > (https://github.com/llvm/llvm-project.git > > b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 > > (https://github.com/llvm/llvm-project.git > > b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 > > 13:48:49 MST 2020 > > [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 > > ... > > [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from > > 2020-08-24T18:50:56 > > [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy > > regulator > > [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy > > regulator > > [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in > > SPRAM > > [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq > > [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] > > using PIO > > [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) > > [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > > [1.705173] mmc0: queuing unknown C
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote: > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote: > [snip] > > > Hello Nathan, > > > > > > Can you tell me how much memory your RPI has and if all of it is > > > > This is the 4GB version. > > > > > accessible by the PCIe device? Could you also please include the DTS > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the > > > PCIe DT before Linux boots -- could you describe what is going on > > > there? > > > > Unfortunately, I am not familiar with how to get this information. If > > you could provide some instructions for how to do so, I am more than > > happy to. I am not very knowleagable about the inner working of the Pi, > > I mainly use it as a test platform for making sure that LLVM does not > > cause problems on real devices. > > Can you bring the dtc application to your Pi root filesystem, and if so, can > you run the following: > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb Sure, the result is attached. > or cat /sys/firmware/fdt > device.dtb > > and attach the resulting file? > > > > > > Finally, can you attach the text of the full boot log? > > > > I have attached a working and broken boot log. Thank you for the quick > > response! > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any > chance? Of course. A new log is attached with the debug output from that config. > I have a suspicion that this part of the DTS for the bcm2711.dtsi platform > is at fault: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 > > and the resulting dma-ranges parsing is just not working for reasons to be > determined. > -- > Florian Let me know if you need anything else out of me. Cheers, Nathan device.dtb Description: Binary data [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.9.0-rc3-next-20200902-dirty (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 17:41:42 MST 2020 [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 [0.00] efi: UEFI not found. [0.00] Reserved memory: created CMA memory pool at 0x3740, size 64 MiB [0.00] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool [0.00] NUMA: No NUMA configuration found [0.00] NUMA: Faking a node at [mem 0x-0xfbff] [0.00] NUMA: NODE_DATA [mem 0xfb81f100-0xfb820fff] [0.00] Zone ranges: [0.00] DMA [mem 0x-0x3fff] [0.00] DMA32[mem 0x4000-0xfbff] [0.00] Normal empty [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x3b3f] [0.00] node 0: [mem 0x4000-0xfbff] [0.00] Initmem setup node 0 [mem 0x-0xfbff] [0.00] percpu: Embedded 23 pages/cpu s54168 r8192 d31848 u94208 [0.00] Detected PIPT I-cache on CPU0 [0.00] CPU features: detected: EL2 vector hardening [0.00] CPU features: kernel page table isolation forced ON by KASLR [0.00] CPU features: detected: Kernel page table isolation (KPTI) [0.00] ARM_SMCCC_ARCH_WORKAROUND_1 missing from firmware [0.00] CPU features: detected: ARM errata 1165522, 1319367, or 1530923 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 996912 [0.00] Policy zone: DMA32 [0.00] Kernel command line: dma.dmachans=0x71f5 bcm2709.boardrev=0xc03112 bcm2709.serial=0xb78d398 bcm2709.uart_clock=4800 bcm2709.disk_led_gpio=42 bcm2709.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:60:6C:87 vc_mem.mem_base=0x3ec0 vc_mem.mem_size=0x4000 console=ttyS1,115200 console=tty1 root=PARTUUID=45a8dd8a-02 rootfstype=ext4 elevator=deadline fsck.repair=yes rootwait plymouth.ignore-serial-consoles [0.00] Kernel parameter elevator= does not have any effect anymore. [0.00] Please use sysfs to set IO scheduler for individual devices. [0.00] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes, linear) [0.00] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] software IO TLB: mapped [mem 0x3340-0x3740] (64
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 08, 2020 at 11:43:45AM +0200, Christoph Hellwig wrote: > And because I like replying to myself so much, here is a link to the > version with the arm cleanup patch applied. Unlike the previous two > attempts this has at least survived very basic sanity testing: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 Tested-by: Nathan Chancellor Thank you for fixing this up quickly! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 8/8] staging: wfx: improve robustness of wfx_get_hw_rate()
On Fri, Oct 09, 2020 at 07:13:07PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller
>
> Smatch complains:
>
> data_tx.c:37 wfx_get_hw_rate() warn: constraint '(struct
> ieee80211_supported_band)->bitrates' overflow 'band->bitrates' 0 <= abs_rl
> '0-127' user_rl '' required = '(struct ieee80211_supported_band)->n_bitrates'
> 23 struct ieee80211_supported_band *band;
> 24
> 25 if (rate->idx < 0)
> 26 return -1;
> 27 if (rate->flags & IEEE80211_TX_RC_MCS) {
> 28 if (rate->idx > 7) {
> 29 WARN(1, "wrong rate->idx value: %d",
> rate->idx);
> 30 return -1;
> 31 }
> 32 return rate->idx + 14;
> 33 }
> 34 // WFx only support 2GHz, else band information should be
> retrieved
> 35 // from ieee80211_tx_info
> 36 band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
> 37 return band->bitrates[rate->idx].hw_value;
>
> Add a simple check to make Smatch happy.
>
> Reported-by: Dan Carpenter
> Signed-off-by: Jérôme Pouiller
> ---
> drivers/staging/wfx/data_tx.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
> index 8db0be08daf8..41f6a604a697 100644
> --- a/drivers/staging/wfx/data_tx.c
> +++ b/drivers/staging/wfx/data_tx.c
> @@ -31,6 +31,10 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
> }
> return rate->idx + 14;
> }
> + if (rate->idx >= band->n_bitrates) {
> + WARN(1, "wrong rate->idx value: %d", rate->idx);
> + return -1;
> + }
> // WFx only support 2GHz, else band information should be retrieved
> // from ieee80211_tx_info
> band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
> --
> 2.28.0
>
This now causes a clang warning:
drivers/staging/wfx/data_tx.c:34:19: warning: variable 'band' is uninitialized
when used here [-Wuninitialized]
if (rate->idx >= band->n_bitrates) {
^~~~
drivers/staging/wfx/data_tx.c:23:39: note: initialize the variable 'band' to
silence this warning
struct ieee80211_supported_band *band;
^
= NULL
1 warning generated.
Cheers,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: wfx: fix use of uninitialized pointer
On Mon, Oct 19, 2020 at 06:06:03PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller
>
> With -Wuninitialized, the compiler complains:
>
> drivers/staging/wfx/data_tx.c:34:19: warning: variable 'band' is
> uninitialized when used here [-Wuninitialized]
> if (rate->idx >= band->n_bitrates) {
> ^~~~
>
> Reported-by: kernel test robot
> Reported-by: Nathan Chancellor
> Fixes: 868fd970e187 ("staging: wfx: improve robustness of wfx_get_hw_rate()")
> Signed-off-by: Jérôme Pouiller
Reviewed-by: Nathan Chancellor
> ---
> drivers/staging/wfx/data_tx.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
> index 41f6a604a697..36b36ef39d05 100644
> --- a/drivers/staging/wfx/data_tx.c
> +++ b/drivers/staging/wfx/data_tx.c
> @@ -31,13 +31,13 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
> }
> return rate->idx + 14;
> }
> - if (rate->idx >= band->n_bitrates) {
> - WARN(1, "wrong rate->idx value: %d", rate->idx);
> - return -1;
> - }
> // WFx only support 2GHz, else band information should be retrieved
> // from ieee80211_tx_info
> band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
> + if (rate->idx >= band->n_bitrates) {
> + WARN(1, "wrong rate->idx value: %d", rate->idx);
> + return -1;
> + }
> return band->bitrates[rate->idx].hw_value;
> }
>
> --
> 2.28.0
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: Fix build error with Clang when inlining is disabled
When CONFIG_NO_AUTO_INLINE was present in linux-next (which added
'-fno-inline-functions' to KBUILD_CFLAGS), an allyesconfig build with
Clang failed at the modpost stage:
ERROR: "is_broadcast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
ERROR: "is_zero_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
ERROR: "is_multicast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
These functions were marked as extern inline, meaning that if inlining
doesn't happen, the function will be undefined, as it is above.
This happens to work with GCC because the '-fno-inline-functions' option
respects the __inline attribute so all instances of these functions are
inlined as expected and the definition doesn't actually matter. However,
with Clang and '-fno-inline-functions', a function has to be marked with
the __always_inline attribute to be considered for inlining, which none
of these functions are. Clang tries to find the symbol definition
elsewhere as it was told and fails, which trickles down to modpost.
To make sure that this code compiles regardless of compiler and make the
intention of the code clearer, use 'static __always_inline' to ensure
that these functions are always inlined. Some alternative solutions
included 'extern __always_inline' or converting these functions to
macros (so the preprocessor deals with them) but I would argue this is
the more "standard" solution.
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8723bs/include/ieee80211.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h
b/drivers/staging/rtl8723bs/include/ieee80211.h
index bcc8dfa8e672..959e822315b5 100644
--- a/drivers/staging/rtl8723bs/include/ieee80211.h
+++ b/drivers/staging/rtl8723bs/include/ieee80211.h
@@ -850,18 +850,18 @@ enum ieee80211_state {
#define IP_FMT "%pI4"
#define IP_ARG(x) (x)
-extern __inline int is_multicast_mac_addr(const u8 *addr)
+static __always_inline int is_multicast_mac_addr(const u8 *addr)
{
return ((addr[0] != 0xff) && (0x01 & addr[0]));
}
-extern __inline int is_broadcast_mac_addr(const u8 *addr)
+static __always_inline int is_broadcast_mac_addr(const u8 *addr)
{
return ((addr[0] == 0xff) && (addr[1] == 0xff) && (addr[2] == 0xff) &&
\
(addr[3] == 0xff) && (addr[4] == 0xff) && (addr[5] == 0xff));
}
-extern __inline int is_zero_mac_addr(const u8 *addr)
+static __always_inline int is_zero_mac_addr(const u8 *addr)
{
return ((addr[0] == 0x00) && (addr[1] == 0x00) && (addr[2] == 0x00) &&
\
(addr[3] == 0x00) && (addr[4] == 0x00) && (addr[5] == 0x00));
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Fix build error with Clang when inlining is disabled
On Wed, Jan 16, 2019 at 07:42:02AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 15, 2019 at 11:03:02PM -0700, Nathan Chancellor wrote:
> > When CONFIG_NO_AUTO_INLINE was present in linux-next (which added
> > '-fno-inline-functions' to KBUILD_CFLAGS), an allyesconfig build with
> > Clang failed at the modpost stage:
> >
> > ERROR: "is_broadcast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko]
> > undefined!
> > ERROR: "is_zero_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
> > ERROR: "is_multicast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko]
> > undefined!
> >
> > These functions were marked as extern inline, meaning that if inlining
> > doesn't happen, the function will be undefined, as it is above.
> >
> > This happens to work with GCC because the '-fno-inline-functions' option
> > respects the __inline attribute so all instances of these functions are
> > inlined as expected and the definition doesn't actually matter. However,
> > with Clang and '-fno-inline-functions', a function has to be marked with
> > the __always_inline attribute to be considered for inlining, which none
> > of these functions are. Clang tries to find the symbol definition
> > elsewhere as it was told and fails, which trickles down to modpost.
> >
> > To make sure that this code compiles regardless of compiler and make the
> > intention of the code clearer, use 'static __always_inline' to ensure
> > that these functions are always inlined. Some alternative solutions
> > included 'extern __always_inline' or converting these functions to
> > macros (so the preprocessor deals with them) but I would argue this is
> > the more "standard" solution.
> >
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/staging/rtl8723bs/include/ieee80211.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h
> > b/drivers/staging/rtl8723bs/include/ieee80211.h
> > index bcc8dfa8e672..959e822315b5 100644
> > --- a/drivers/staging/rtl8723bs/include/ieee80211.h
> > +++ b/drivers/staging/rtl8723bs/include/ieee80211.h
> > @@ -850,18 +850,18 @@ enum ieee80211_state {
> > #define IP_FMT "%pI4"
> > #define IP_ARG(x) (x)
> >
> > -extern __inline int is_multicast_mac_addr(const u8 *addr)
> > +static __always_inline int is_multicast_mac_addr(const u8 *addr)
>
> Ick, really? This is in a .h file, the .c file sees this, so why isn't
> clang picking it up? Worst case it just makes it a "normal" function
> and doesn't inline it, right?
>
As I understand it, the meaning of 'extern inline' is basically use this
defintion when inlining, otherwise use one from a different file or
translation unit. See commit d0a8d9378d16 ("x86/paravirt: Make
native_save_fl() extern inline"). These functions don't have any other
declaration/definition so when they aren't inlined, they don't exist.
> How about just replacing "extern" with "static", that should solve this,
> adding "__always_inline" everywhere is not going to be fun and doesn't
> make any sense.
>
Yes, that would work (and what I originally tested). My assumption with
the code is that it was intended for this function to always be inlined
but if you'd rather it just be 'static', I am happy to send a v2!
> thanks,
>
> greg k-h
Thanks for the quick reply and review,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Fix build error with Clang when inlining is disabled
On Wed, Jan 16, 2019 at 09:46:58AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 15, 2019 at 11:53:40PM -0700, Nathan Chancellor wrote:
> > On Wed, Jan 16, 2019 at 07:42:02AM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Jan 15, 2019 at 11:03:02PM -0700, Nathan Chancellor wrote:
> > > > When CONFIG_NO_AUTO_INLINE was present in linux-next (which added
> > > > '-fno-inline-functions' to KBUILD_CFLAGS), an allyesconfig build with
> > > > Clang failed at the modpost stage:
> > > >
> > > > ERROR: "is_broadcast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko]
> > > > undefined!
> > > > ERROR: "is_zero_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko]
> > > > undefined!
> > > > ERROR: "is_multicast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko]
> > > > undefined!
> > > >
> > > > These functions were marked as extern inline, meaning that if inlining
> > > > doesn't happen, the function will be undefined, as it is above.
> > > >
> > > > This happens to work with GCC because the '-fno-inline-functions' option
> > > > respects the __inline attribute so all instances of these functions are
> > > > inlined as expected and the definition doesn't actually matter. However,
> > > > with Clang and '-fno-inline-functions', a function has to be marked with
> > > > the __always_inline attribute to be considered for inlining, which none
> > > > of these functions are. Clang tries to find the symbol definition
> > > > elsewhere as it was told and fails, which trickles down to modpost.
> > > >
> > > > To make sure that this code compiles regardless of compiler and make the
> > > > intention of the code clearer, use 'static __always_inline' to ensure
> > > > that these functions are always inlined. Some alternative solutions
> > > > included 'extern __always_inline' or converting these functions to
> > > > macros (so the preprocessor deals with them) but I would argue this is
> > > > the more "standard" solution.
> > > >
> > > > Signed-off-by: Nathan Chancellor
> > > > ---
> > > > drivers/staging/rtl8723bs/include/ieee80211.h | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h
> > > > b/drivers/staging/rtl8723bs/include/ieee80211.h
> > > > index bcc8dfa8e672..959e822315b5 100644
> > > > --- a/drivers/staging/rtl8723bs/include/ieee80211.h
> > > > +++ b/drivers/staging/rtl8723bs/include/ieee80211.h
> > > > @@ -850,18 +850,18 @@ enum ieee80211_state {
> > > > #define IP_FMT "%pI4"
> > > > #define IP_ARG(x) (x)
> > > >
> > > > -extern __inline int is_multicast_mac_addr(const u8 *addr)
> > > > +static __always_inline int is_multicast_mac_addr(const u8 *addr)
> > >
> > > Ick, really? This is in a .h file, the .c file sees this, so why isn't
> > > clang picking it up? Worst case it just makes it a "normal" function
> > > and doesn't inline it, right?
> > >
> >
> > As I understand it, the meaning of 'extern inline' is basically use this
> > defintion when inlining, otherwise use one from a different file or
> > translation unit. See commit d0a8d9378d16 ("x86/paravirt: Make
> > native_save_fl() extern inline"). These functions don't have any other
> > declaration/definition so when they aren't inlined, they don't exist.
> >
> > > How about just replacing "extern" with "static", that should solve this,
> > > adding "__always_inline" everywhere is not going to be fun and doesn't
> > > make any sense.
> > >
> >
> > Yes, that would work (and what I originally tested). My assumption with
> > the code is that it was intended for this function to always be inlined
> > but if you'd rather it just be 'static', I am happy to send a v2!
>
> This code is not trying to be that smart, someone just put the extern in
> there without realizing it. So making it static is the correct fix, so
> a v2 would be great.
>
Done!
> Odds are, almost all other places in the kernel that you hit with this
> also needs the same fixup, no need to be fancy here :)
>
Thankfully, with arm, arm64, and x86_64, there was only one other place
that where that config caused an error:
https://lore.kernel.org/lkml/20181210234538.5405-1-natechancel...@gmail.com/
> thanks,
>
> greg k-h
Thanks,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8723bs: Fix build error with Clang when inlining is disabled
When CONFIG_NO_AUTO_INLINE was present in linux-next (which added
'-fno-inline-functions' to KBUILD_CFLAGS), an allyesconfig build with
Clang failed at the modpost stage:
ERROR: "is_broadcast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
ERROR: "is_zero_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
ERROR: "is_multicast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
These functions were marked as extern inline, meaning that if inlining
doesn't happen, the function will be undefined, as it is above.
This happens to work with GCC because the '-fno-inline-functions' option
respects the __inline attribute so all instances of these functions are
inlined as expected and the definition doesn't actually matter. However,
with Clang and '-fno-inline-functions', a function has to be marked with
the __always_inline attribute to be considered for inlining, which none
of these functions are. Clang tries to find the symbol definition
elsewhere as it was told and fails, which trickles down to modpost.
To make sure that this code compiles regardless of compiler and make the
intention of the code clearer, use 'static' to ensure these functions
are always defined, regardless of inlining. Additionally, silence a
checkpatch warning by switching from '__inline' to 'inline'.
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Don't add __always_inline.
drivers/staging/rtl8723bs/include/ieee80211.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h
b/drivers/staging/rtl8723bs/include/ieee80211.h
index bcc8dfa8e672..9efb4dcb9d3a 100644
--- a/drivers/staging/rtl8723bs/include/ieee80211.h
+++ b/drivers/staging/rtl8723bs/include/ieee80211.h
@@ -850,18 +850,18 @@ enum ieee80211_state {
#define IP_FMT "%pI4"
#define IP_ARG(x) (x)
-extern __inline int is_multicast_mac_addr(const u8 *addr)
+static inline int is_multicast_mac_addr(const u8 *addr)
{
return ((addr[0] != 0xff) && (0x01 & addr[0]));
}
-extern __inline int is_broadcast_mac_addr(const u8 *addr)
+static inline int is_broadcast_mac_addr(const u8 *addr)
{
return ((addr[0] == 0xff) && (addr[1] == 0xff) && (addr[2] == 0xff) &&
\
(addr[3] == 0xff) && (addr[4] == 0xff) && (addr[5] == 0xff));
}
-extern __inline int is_zero_mac_addr(const u8 *addr)
+static inline int is_zero_mac_addr(const u8 *addr)
{
return ((addr[0] == 0x00) && (addr[1] == 0x00) && (addr[2] == 0x00) &&
\
(addr[3] == 0x00) && (addr[4] == 0x00) && (addr[5] == 0x00));
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtlwifi: Use proper enum for return in halmac_parse_psd_data_88xx
Clang warns:
drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c:2472:11:
warning: implicit conversion from enumeration type 'enum
halmac_cmd_process_status' to different enumeration type 'enum
halmac_ret_status' [-Wenum-conversion]
return HALMAC_CMD_PROCESS_ERROR;
~~ ^~~~
1 warning generated.
Fix this by using the proper enum for allocation failures,
HALMAC_RET_MALLOC_FAIL, which is used in the rest of this file.
Fixes: e4b08e16b7d9 ("staging: r8822be: check kzalloc return or bail")
Link: https://github.com/ClangBuiltLinux/linux/issues/375
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
b/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
index ec742da030db..ddbeff8224ab 100644
--- a/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
+++ b/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
@@ -2469,7 +2469,7 @@ halmac_parse_psd_data_88xx(struct halmac_adapter
*halmac_adapter, u8 *c2h_buf,
if (!psd_set->data) {
psd_set->data = kzalloc(psd_set->data_size, GFP_KERNEL);
if (!psd_set->data)
- return HALMAC_CMD_PROCESS_ERROR;
+ return HALMAC_RET_MALLOC_FAIL;
}
if (segment_id == 0)
--
2.21.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: next/master build: 221 builds: 11 failed, 210 passed, 13 errors, 1174 warnings (next-20190731)
On Wed, Jul 31, 2019 at 06:00:43PM +0200, Greg KH wrote: > On Wed, Jul 31, 2019 at 08:48:24AM -0700, David Miller wrote: > > From: Greg Kroah-Hartman > > Date: Wed, 31 Jul 2019 13:35:22 +0200 > > > > > On Wed, Jul 31, 2019 at 12:24:41PM +0100, Mark Brown wrote: > > >> On Wed, Jul 31, 2019 at 04:07:41AM -0700, kernelci.org bot wrote: > > >> > > >> Today's -next fails to build an ARM allmodconfig due to: > > >> > > >> > allmodconfig (arm, gcc-8) ― FAIL, 1 error, 40 warnings, 0 section > > >> > mismatches > > >> > > > >> > Errors: > > >> > drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration > > >> > of function 'writeq'; did you mean 'writel'? > > >> > [-Werror=implicit-function-declaration] > > >> > > >> as a result of the changes that introduced: > > >> > > >> WARNING: unmet direct dependencies detected for MDIO_OCTEON > > >> Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=m] && MDIO_BUS [=m] > > >> && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=m] > > >> Selected by [m]: > > >> - OCTEON_ETHERNET [=m] && STAGING [=y] && (CAVIUM_OCTEON_SOC && > > >> NETDEVICES [=y] || COMPILE_TEST [=y]) > > >> > > >> which is triggered by the staging OCTEON_ETHERNET driver which misses a > > >> 64BIT dependency but added COMPILE_TEST in 171a9bae68c72f2 > > >> (staging/octeon: Allow test build on !MIPS). > > > > > > A patch was posted for this, but it needs to go through the netdev tree > > > as that's where the offending patches are coming from. > > > > I didn't catch that, was netdev CC:'d? > > Nope, just you :( > > I'll resend it now and cc: netdev. > > thanks, > > greg k-h If it is this patch: https://lore.kernel.org/netdev/20190731160219.ga2...@kroah.com/ It doesn't resolve that issue. I applied it and tested on next-20190731. $ make -j$(nproc) -s ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- O=out distclean allyesconfig drivers/net/phy/ WARNING: unmet direct dependencies detected for MDIO_OCTEON Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y] Selected by [y]: - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC || COMPILE_TEST [=y]) && NETDEVICES [=y] ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe': ../drivers/net/phy/mdio-octeon.c:48:3: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 48 | (u64)devm_ioremap(&pdev->dev, mdio_phys, regsize); | ^ In file included from ../drivers/net/phy/mdio-octeon.c:14: ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration] 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) |^~ ../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro 'oct_mdio_writeq' 56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); | ^~~ ../drivers/net/phy/mdio-cavium.h:111:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) |^ ../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro 'oct_mdio_writeq' 56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); | ^~~ ../drivers/net/phy/mdio-cavium.h:111:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) |^ ../drivers/net/phy/mdio-octeon.c:77:2: note: in expansion of macro 'oct_mdio_writeq' 77 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); | ^~~ ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_remove': ../drivers/net/phy/mdio-cavium.h:111:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr) |^ ../drivers/net/phy/mdio-octeon.c:91:2: note: in expansion of macro 'oct_mdio_writeq' 91 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); | ^~~ cc1: some warnings being treated as errors make[3]: *** [../scripts/Makefile.build:274: drivers/net/phy/mdio-octeon.o] Error 1 make[3]: *** Waiting for unfinished jobs make[2]: *** [../Makefile:1780: drivers/net/phy/] Error 2 make[1]: *** [/home/nathan/cbl/linux-next/Makefile:330: __build_one_by_one] Error 2 make: *** [Makefile:179: sub-make] Error 2 This is the diff that I came up with to solve the errors plus the casting warnings but it doesn't feel proper to me. If you all feel otherwise, I can draft up a formal commit message. diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 20f14c5fbb7e..ed2edf4b5b0e 100644 --- a/drivers/net/phy/Kconfig +++
[PATCH] net: mdio-octeon: Fix build error and Kconfig warning
arm allyesconfig warns:
WARNING: unmet direct dependencies detected for MDIO_OCTEON
Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y]
&& 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y]
Selected by [y]:
- OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC &&
NETDEVICES [=y] || COMPILE_TEST [=y])
and errors:
In file included from ../drivers/net/phy/mdio-octeon.c:14:
../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe':
../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of
function 'writeq'; did you mean 'writeb'?
[-Werror=implicit-function-declaration]
111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
|^~
../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro
'oct_mdio_writeq'
56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
| ^~~
cc1: some warnings being treated as errors
This allows MDIO_OCTEON to be built with COMPILE_TEST as well and
includes the proper header for readq/writeq. This does not address
the several -Wint-to-pointer-cast and -Wpointer-to-int-cast warnings
that appeared as a result of commit 171a9bae68c7 ("staging/octeon:
Allow test build on !MIPS") in these files.
Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
Reported-by: kbuild test robot
Reported-by: Mark Brown
Reported-by: Randy Dunlap
Signed-off-by: Nathan Chancellor
---
drivers/net/phy/Kconfig | 2 +-
drivers/net/phy/mdio-cavium.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 20f14c5fbb7e..ed2edf4b5b0e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,7 +159,7 @@ config MDIO_MSCC_MIIM
config MDIO_OCTEON
tristate "Octeon and some ThunderX SOCs MDIO buses"
- depends on 64BIT
+ depends on 64BIT || COMPILE_TEST
depends on HAS_IOMEM && OF_MDIO
select MDIO_CAVIUM
help
diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
index ed5f9bb5448d..b7f89ad27465 100644
--- a/drivers/net/phy/mdio-cavium.h
+++ b/drivers/net/phy/mdio-cavium.h
@@ -108,6 +108,8 @@ static inline u64 oct_mdio_readq(u64 addr)
return cvmx_read_csr(addr);
}
#else
+#include
+
#define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
#define oct_mdio_readq(addr) readq((void *)addr)
#endif
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] net: mdio-octeon: Fix build error and Kconfig warning
On Fri, Aug 02, 2019 at 06:11:32PM -0700, David Miller wrote: > From: Nathan Chancellor > Date: Wed, 31 Jul 2019 11:50:24 -0700 > > > arm allyesconfig warns: > > > > WARNING: unmet direct dependencies detected for MDIO_OCTEON > > Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] > > && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y] > > Selected by [y]: > > - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && > > NETDEVICES [=y] || COMPILE_TEST [=y]) > > > > and errors: > > > > In file included from ../drivers/net/phy/mdio-octeon.c:14: > > ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe': > > ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of > > function 'writeq'; did you mean 'writeb'? > > The proper way to fix this is to include either > > linux/io-64-nonatomic-hi-lo.h > > or > > linux/io-64-nonatomic-lo-hi.h > > whichever is appropriate. H, is that not what I did? Although I did not know about io-64-nonatomic-hi-lo.h. What is the difference and which one is needed here? There is apparently another failure when OF_MDIO is not set, I guess I can try to look into that as well and respin into a series if necessary. Cheers, Nathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] net: mdio-octeon: Fix Kconfig warnings and build errors
After commit 171a9bae68c7 ("staging/octeon: Allow test build on
!MIPS"), the following combination of configs cause a few Kconfig
warnings and build errors (distilled from arm allyesconfig and Randy's
randconfig builds):
CONFIG_NETDEVICES=y
CONFIG_STAGING=y
CONFIG_COMPILE_TEST=y
and CONFIG_OCTEON_ETHERNET as either a module or built-in.
WARNING: unmet direct dependencies detected for MDIO_OCTEON
Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y]
&& 64BIT [=n] && HAS_IOMEM [=y] && OF_MDIO [=n]
Selected by [y]:
- OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC ||
COMPILE_TEST [=y]) && NETDEVICES [=y]
In file included from ../drivers/net/phy/mdio-octeon.c:14:
../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of
function ‘writeq’; did you mean ‘writel’?
[-Werror=implicit-function-declaration]
111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
|^~
CONFIG_64BIT is not strictly necessary if the proper readq/writeq
definitions are included from io-64-nonatomic-lo-hi.h.
CONFIG_OF_MDIO is not needed when CONFIG_COMPILE_TEST is enabled because
of commit f9dc9ac51610 ("of/mdio: Add dummy functions in of_mdio.h.").
Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
Reported-by: kbuild test robot
Reported-by: Mark Brown
Reported-by: Randy Dunlap
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Address Randy's reported failure here:
https://lore.kernel.org/netdev/b3444283-7a77-ece8-7ac6-41756aa7d...@infradead.org/
by not requiring CONFIG_OF_MDIO when CONFIG_COMPILE_TEST is set.
* Improve commit message
drivers/net/phy/Kconfig | 4 ++--
drivers/net/phy/mdio-cavium.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 20f14c5fbb7e..0e3d9e3d3533 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,8 +159,8 @@ config MDIO_MSCC_MIIM
config MDIO_OCTEON
tristate "Octeon and some ThunderX SOCs MDIO buses"
- depends on 64BIT
- depends on HAS_IOMEM && OF_MDIO
+ depends on (64BIT && OF_MDIO) || COMPILE_TEST
+ depends on HAS_IOMEM
select MDIO_CAVIUM
help
This module provides a driver for the Octeon and ThunderX MDIO
diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
index ed5f9bb5448d..b7f89ad27465 100644
--- a/drivers/net/phy/mdio-cavium.h
+++ b/drivers/net/phy/mdio-cavium.h
@@ -108,6 +108,8 @@ static inline u64 oct_mdio_readq(u64 addr)
return cvmx_read_csr(addr);
}
#else
+#include
+
#define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
#define oct_mdio_readq(addr) readq((void *)addr)
#endif
--
2.23.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] net: mdio-octeon: Fix build error and Kconfig warning
On Fri, Aug 02, 2019 at 06:39:52PM -0700, Matthew Wilcox wrote: > On Fri, Aug 02, 2019 at 06:30:31PM -0700, Nathan Chancellor wrote: > > On Fri, Aug 02, 2019 at 06:11:32PM -0700, David Miller wrote: > > > The proper way to fix this is to include either > > > > > > linux/io-64-nonatomic-hi-lo.h > > > > > > or > > > > > > linux/io-64-nonatomic-lo-hi.h > > > > > > whichever is appropriate. > > > > H, is that not what I did? > > > > Although I did not know about io-64-nonatomic-hi-lo.h. What is the > > difference and which one is needed here? > > Whether you write the high or low 32 bits first. For this, it doesn't > matter, since the compiled driver will never be run on real hardware. That's what I figured. I have only seen lo-hi used personally, which is what I went with here. Thanks for the confirmation! > > > There is apparently another failure when OF_MDIO is not set, I guess I > > can try to look into that as well and respin into a series if > > necessary. > > Thanks for taking care of that! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] net: mdio-octeon: Fix Kconfig warnings and build errors
On Tue, Aug 06, 2019 at 02:11:33PM -0700, David Miller wrote:
> From: Nathan Chancellor
> Date: Fri, 2 Aug 2019 23:01:56 -0700
>
> > After commit 171a9bae68c7 ("staging/octeon: Allow test build on
> > !MIPS"), the following combination of configs cause a few Kconfig
> > warnings and build errors (distilled from arm allyesconfig and Randy's
> > randconfig builds):
> >
> > CONFIG_NETDEVICES=y
> > CONFIG_STAGING=y
> > CONFIG_COMPILE_TEST=y
> >
> > and CONFIG_OCTEON_ETHERNET as either a module or built-in.
> >
> > WARNING: unmet direct dependencies detected for MDIO_OCTEON
> > Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y]
> > && 64BIT [=n] && HAS_IOMEM [=y] && OF_MDIO [=n]
> > Selected by [y]:
> > - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC ||
> > COMPILE_TEST [=y]) && NETDEVICES [=y]
> >
> > In file included from ../drivers/net/phy/mdio-octeon.c:14:
> > ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of
> > function ‘writeq’; did you mean ‘writel’?
> > [-Werror=implicit-function-declaration]
> > 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
> > |^~
> >
> > CONFIG_64BIT is not strictly necessary if the proper readq/writeq
> > definitions are included from io-64-nonatomic-lo-hi.h.
> >
> > CONFIG_OF_MDIO is not needed when CONFIG_COMPILE_TEST is enabled because
> > of commit f9dc9ac51610 ("of/mdio: Add dummy functions in of_mdio.h.").
> >
> > Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
> > Reported-by: kbuild test robot
> > Reported-by: Mark Brown
> > Reported-by: Randy Dunlap
> > Signed-off-by: Nathan Chancellor
>
> Applied to net-next.
>
> Please make it clear what tree your changes are targetting in the future,
> thank you.
Sorry for the confusion, I'll do my best to add a patch suffix in the
future.
Thank you for picking this up!
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: exfat: fix uninitialized variable ret
On Fri, Aug 30, 2019 at 07:46:44PM +0100, Colin King wrote:
> From: Colin Ian King
>
> Currently there are error return paths in ffsReadFile that
> exit via lable err_out that return and uninitialized error
> return in variable ret. Fix this by initializing ret to zero.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: c48c9f7ff32b ("staging: exfat: add exfat filesystem code to staging")
> Signed-off-by: Colin Ian King
Clang also warns about this:
drivers/staging/exfat/exfat_super.c:885:6: warning: variable 'ret' is used
uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (p_fs->dev_ejected)
^
drivers/staging/exfat/exfat_super.c:892:9: note: uninitialized use occurs here
return ret;
^~~
drivers/staging/exfat/exfat_super.c:885:2: note: remove the 'if' if its
condition is always true
if (p_fs->dev_ejected)
^~
drivers/staging/exfat/exfat_super.c:776:9: note: initialize the variable 'ret'
to silence this warning
int ret;
^
= 0
1 warning generated.
Reviewed-by: Nathan Chancellor
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: kpc2000: Use memset to initialize resources
Clang warns:
drivers/staging/kpc2000/kpc2000/cell_probe.c:96:38: warning: suggest
braces around initialization of subobject [-Wmissing-braces]
struct resource resources[2] = {0};
^
{}
drivers/staging/kpc2000/kpc2000/cell_probe.c:314:38: warning: suggest
braces around initialization of subobject [-Wmissing-braces]
struct resource resources[2] = {0};
^
{}
2 warnings generated.
One way to fix these warnings is to add additional braces like Clang
suggests; however, there has been a bit of push back from some
maintainers, who just prefer memset as it is unambiguous, doesn't
depend on a particular compiler version, and properly initializes all
subobjects [1][2]. Do that here so there are no more warnings.
[1]: https://lore.kernel.org/lkml/022e41c0-8465-dc7a-a45c-64187ecd9...@amd.com/
[2]:
https://lore.kernel.org/lkml/20181128.215241.702406654469517539.da...@davemloft.net/
Link: https://github.com/ClangBuiltLinux/linux/issues/455
Signed-off-by: Nathan Chancellor
---
drivers/staging/kpc2000/kpc2000/cell_probe.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c
b/drivers/staging/kpc2000/kpc2000/cell_probe.c
index ad2cc0a3bfa1..13f544f3c0b9 100644
--- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
+++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
@@ -93,8 +93,8 @@ void parse_core_table_entry(struct core_table_entry *cte,
const u64 read_val, co
int probe_core_basic(unsigned int core_num, struct kp2000_device *pcard, char
*name, const struct core_table_entry cte)
{
struct mfd_cell cell = {0};
-struct resource resources[2] = {0};
-
+struct resource resources[2];
+
struct kpc_core_device_platdata core_pdata = {
.card_id = pcard->card_id,
.build_version = pcard->build_version,
@@ -112,6 +112,8 @@ int probe_core_basic(unsigned int core_num, struct
kp2000_device *pcard, char *
cell.id = core_num;
cell.num_resources = 2;
+memset(&resources, 0, sizeof(resources));
+
resources[0].start = cte.offset;
resources[0].end = cte.offset + (cte.length - 1);
resources[0].flags = IORESOURCE_MEM;
@@ -311,8 +313,8 @@ int probe_core_uio(unsigned int core_num, struct
kp2000_device *pcard, char *na
static int create_dma_engine_core(struct kp2000_device *pcard, size_t
engine_regs_offset, int engine_num, int irq_num)
{
struct mfd_cell cell = {0};
-struct resource resources[2] = {0};
-
+struct resource resources[2];
+
dev_dbg(&pcard->pdev->dev, "create_dma_core(pcard = [%p],
engine_regs_offset = %zx, engine_num = %d)\n", pcard, engine_regs_offset,
engine_num);
cell.platform_data = NULL;
@@ -321,6 +323,8 @@ static int create_dma_engine_core(struct kp2000_device
*pcard, size_t engine_re
cell.name = KP_DRIVER_NAME_DMA_CONTROLLER;
cell.num_resources = 2;
+memset(&resources, 0, sizeof(resources));
+
resources[0].start = engine_regs_offset;
resources[0].end = engine_regs_offset + (KPC_DMA_ENGINE_SIZE - 1);
resources[0].flags = IORESOURCE_MEM;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: kpc2000: kpc_spi: Fix build error for {read,write}q
drivers/staging/kpc2000/kpc_spi/spi_driver.c:158:11: error: implicit
declaration of function 'readq' [-Werror,-Wimplicit-function-declaration]
drivers/staging/kpc2000/kpc_spi/spi_driver.c:167:5: error: implicit
declaration of function 'writeq' [-Werror,-Wimplicit-function-declaration]
Same as commit 91b6cb7216cd ("staging: kpc2000: fix up build problems
with readq()").
Signed-off-by: Nathan Chancellor
---
drivers/staging/kpc2000/kpc_spi/spi_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/kpc2000/kpc_spi/spi_driver.c
b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
index 074a578153d0..3ace4e5c1284 100644
--- a/drivers/staging/kpc2000/kpc_spi/spi_driver.c
+++ b/drivers/staging/kpc2000/kpc_spi/spi_driver.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8192u: Remove an unnecessary NULL check
Clang warns:
drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:2663:47: warning:
address of array 'param->u.wpa_ie.data' will always evaluate to 'true'
[-Wpointer-bool-conversion]
(param->u.wpa_ie.len && !param->u.wpa_ie.data))
~^~~~
This was exposed by commit deabe03523a7 ("Staging: rtl8192u: ieee80211:
Use !x in place of NULL comparisons") because we disable the warning
that would have pointed out the comparison against NULL is also false:
drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:2663:46: warning:
comparison of array 'param->u.wpa_ie.data' equal to a null pointer is
always false [-Wtautological-pointer-compare]
(param->u.wpa_ie.len && param->u.wpa_ie.data == NULL))
^~~~
Remove it so clang no longer warns.
Link: https://github.com/ClangBuiltLinux/linux/issues/487
Signed-off-by: Nathan Chancellor
---
drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index f38f9d8b78bb..e0da0900a4f7 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -2659,8 +2659,7 @@ static int ieee80211_wpa_set_wpa_ie(struct
ieee80211_device *ieee,
{
u8 *buf;
- if (param->u.wpa_ie.len > MAX_WPA_IE_LEN ||
- (param->u.wpa_ie.len && !param->u.wpa_ie.data))
+ if (param->u.wpa_ie.len > MAX_WPA_IE_LEN)
return -EINVAL;
if (param->u.wpa_ie.len) {
--
2.22.0.rc1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: Remove an unnecessary NULL check
On Wed, May 22, 2019 at 10:04:18AM +0300, Dan Carpenter wrote:
> On Tue, May 21, 2019 at 10:42:21AM -0700, Nathan Chancellor wrote:
> > Clang warns:
> >
> > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:2663:47: warning:
> > address of array 'param->u.wpa_ie.data' will always evaluate to 'true'
> > [-Wpointer-bool-conversion]
> > (param->u.wpa_ie.len && !param->u.wpa_ie.data))
> > ~^~~~
> >
> > This was exposed by commit deabe03523a7 ("Staging: rtl8192u: ieee80211:
> > Use !x in place of NULL comparisons") because we disable the warning
> > that would have pointed out the comparison against NULL is also false:
> >
>
> Heh. Weird. Why would people disable one and not the other?
>
> regards,
> dan carpenter
>
-Wtautological-compare has a lot of different sub-warnings under it,
one of which is the one shown. -Wno-tautological-compare turns off all
of those other sub-warnings. The reason that was done is there are quite
a few of them:
https://gist.github.com/nathanchance/3336adc6e796b57eadd53b106b96c569
https://clang.llvm.org/docs/DiagnosticsReference.html#wtautological-compare
It is probably worth looking into turning that on, I'm going to try to
do that as I have time.
Cheers,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Trivial numbering change in comments.
On Tue, Aug 28, 2018 at 04:08:18PM -0400, Ray Clinton wrote:
> I'm trying to get my feet wet in kernel dev and while working on a patch
> I noticed in a lengthy comment block that the number "2" was skipped
> in a description of the steps of a protocol. This patch is simply correcting
> this. This is for 4.18.0.
>
> It is a very trivial patch, but this is my first actual try at one and
> thought I
> might start out small (and I don't think you can get smaller than a single
> character change). Any and all advice (about this patch, email, or
> anything else) is very welcome.
>
> Thanks so much!
> Ray
>
> Signed-off-by: Ray Clinton
> ---
> drivers/staging/android/uapi/vsoc_shm.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/android/uapi/vsoc_shm.h
> b/drivers/staging/android/uapi/vsoc_shm.h
> index 6291fb2..0301172 100644
> --- a/drivers/staging/android/uapi/vsoc_shm.h
> +++ b/drivers/staging/android/uapi/vsoc_shm.h
> @@ -92,7 +92,7 @@ struct fd_scoped_permission_arg {
> *The transmitter can choose any appropriate hashing algorithm, including
> *hash = futex_offset & ((1 << num_nodes_lg2) - 1)
> *
> - * 3. The transmitter should atomically compare and swap futex_offset with 0
> + * 2. The transmitter should atomically compare and swap futex_offset with 0
> *at hash. There are 3 possible outcomes
> * a. The swap fails because the futex_offset is already in the table.
> * The transmitter should stop.
> --
> 2.7.4
Hi Ray,
Welcome to Linux kernel development and good first patch. Just a few
words of advice for any future patches that you submit!
Make sure to include what subsystem you are modifying in the commit
title. You can see examples by using 'git log --online '; for
this subsystem, it's usually "staging: android: vsoc:", making this
patch's title "staging: android: vsoc: Trivial numbering change in comments".
I'd recommend keeping author details (such as your experience and other
info) in-between the '---' and the file names modified in the patch so
that they can be read by the maintainers/reviewers but not added to the
commit message by 'git am'. This is also helpful for adding comments
like what changed between versions of the patch or maybe something like
"I'm not sure this change is correct, it could also be done via ,
I'd like some review".
Small nits in the grand scheme of things but they'll come in handy as
you develop more and more complex patches and series.
Reviewed-by: Nathan Chancellor
Cheers!
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: board: Replace license boilerplate with SPDX identifiers
This satisfies a checkpatch.pl warning and is the preferred method for notating the license due to its lack of ambiguity. Signed-off-by: Nathan Chancellor --- drivers/staging/board/armadillo800eva.c | 10 +- drivers/staging/board/board.c | 5 + 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/staging/board/armadillo800eva.c b/drivers/staging/board/armadillo800eva.c index 4de4fd06eebc..962cc0c79988 100644 --- a/drivers/staging/board/armadillo800eva.c +++ b/drivers/staging/board/armadillo800eva.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Staging board support for Armadillo 800 eva. * Enable not-yet-DT-capable devices here. @@ -6,15 +7,6 @@ * * Copyright (C) 2012 Renesas Solutions Corp. * Copyright (C) 2012 Kuninori Morimoto - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #include diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c index 86dc41101610..cb6feb34dd40 100644 --- a/drivers/staging/board/board.c +++ b/drivers/staging/board/board.c @@ -1,10 +1,7 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Copyright (C) 2014 Magnus Damm * Copyright (C) 2015 Glider bvba - * - * This file is subject to the terms and conditions of the GNU General Public - * License. See the file "COPYING" in the main directory of this archive - * for more details. */ #define pr_fmt(fmt)"board_staging: " fmt -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: Remove unused local variable
Fixes the following W=1 warning: variable ‘intf_id’ set but
not used [-Wunused-but-set-variable]
Signed-off-by: Nathan Chancellor
---
drivers/staging/greybus/svc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c
index a874fed761a1..a2bb7e1a3db3 100644
--- a/drivers/staging/greybus/svc.c
+++ b/drivers/staging/greybus/svc.c
@@ -1137,7 +1137,6 @@ static int gb_svc_intf_reset_recv(struct gb_operation *op)
struct gb_svc *svc = gb_connection_get_data(op->connection);
struct gb_message *request = op->request;
struct gb_svc_intf_reset_request *reset;
- u8 intf_id;
if (request->payload_size < sizeof(*reset)) {
dev_warn(&svc->dev, "short reset request received (%zu <
%zu)\n",
@@ -1146,8 +1145,6 @@ static int gb_svc_intf_reset_recv(struct gb_operation *op)
}
reset = request->payload;
- intf_id = reset->intf_id;
-
/* FIXME Reset the interface here */
return 0;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: wilc1000: Remove useless function
GCC warns that 'wid' is unused in wilc_remove_key and it's correct; the
variable is only local. Get rid of the function (since it just returns
zero) and shuffle the remaining code into one if statement.
Signed-off-by: Nathan Chancellor
---
drivers/staging/wilc1000/host_interface.c | 12
drivers/staging/wilc1000/host_interface.h | 1 -
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 14 +-
3 files changed, 5 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 3fd4c8e62da6..b5f3829e9903 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2606,18 +2606,6 @@ static void timer_connect_cb(struct timer_list *t)
wilc_enqueue_cmd(&msg);
}
-s32 wilc_remove_key(struct host_if_drv *hif_drv, const u8 *sta_addr)
-{
- struct wid wid;
-
- wid.id = (u16)WID_REMOVE_KEY;
- wid.type = WID_STR;
- wid.val = (s8 *)sta_addr;
- wid.size = 6;
-
- return 0;
-}
-
int wilc_remove_wep_key(struct wilc_vif *vif, u8 index)
{
int result = 0;
diff --git a/drivers/staging/wilc1000/host_interface.h
b/drivers/staging/wilc1000/host_interface.h
index 7a26f341e0ba..08b3ba7df8b4 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -303,7 +303,6 @@ struct add_sta_param {
};
struct wilc_vif;
-s32 wilc_remove_key(struct host_if_drv *hif_drv, const u8 *sta_addr);
int wilc_remove_wep_key(struct wilc_vif *vif, u8 index);
int wilc_set_wep_default_keyid(struct wilc_vif *vif, u8 index);
int wilc_add_wep_key_bss_sta(struct wilc_vif *vif, const u8 *key, u8 len,
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 76b4afaef423..b499fb987395 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1054,15 +1054,11 @@ static int del_key(struct wiphy *wiphy, struct
net_device *netdev,
}
}
- if (key_index >= 0 && key_index <= 3) {
- if (priv->wep_key_len[key_index]) {
- memset(priv->wep_key[key_index], 0,
- priv->wep_key_len[key_index]);
- priv->wep_key_len[key_index] = 0;
- wilc_remove_wep_key(vif, key_index);
- }
- } else {
- wilc_remove_key(priv->hif_drv, mac_addr);
+ if (key_index >= 0 && key_index <= 3 && priv->wep_key_len[key_index]) {
+ memset(priv->wep_key[key_index], 0,
+ priv->wep_key_len[key_index]);
+ priv->wep_key_len[key_index] = 0;
+ wilc_remove_wep_key(vif, key_index);
}
return 0;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: wilc1000: Remove unnecessary array index check
This statment triggers GCC's -Wtype-limit since key_index is an
unsigned integer so it cannot be less than zero.
Signed-off-by: Nathan Chancellor
---
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index b499fb987395..e0015ca6c21a 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1054,7 +1054,7 @@ static int del_key(struct wiphy *wiphy, struct net_device
*netdev,
}
}
- if (key_index >= 0 && key_index <= 3 && priv->wep_key_len[key_index]) {
+ if (key_index <= 3 && priv->wep_key_len[key_index]) {
memset(priv->wep_key[key_index], 0,
priv->wep_key_len[key_index]);
priv->wep_key_len[key_index] = 0;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: wilc1000: Remove unused variables
GCC warns these variables are all set but never used so remove them.
Signed-off-by: Nathan Chancellor
---
drivers/staging/wilc1000/host_interface.c | 12
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 6 --
2 files changed, 18 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index 28edd904b33a..3fd4c8e62da6 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1432,13 +1432,7 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif
*vif,
{
s32 result = 0;
u8 msg_type = 0;
- u8 msg_id = 0;
- u16 msg_len = 0;
- u16 wid_id = (u16)WID_NIL;
- u8 wid_len = 0;
u8 mac_status;
- u8 mac_status_reason_code;
- u8 mac_status_additional_info;
struct host_if_drv *hif_drv = vif->hif_drv;
if (!rcvd_info->buffer) {
@@ -1472,13 +1466,7 @@ static s32 handle_rcvd_gnrl_async_info(struct wilc_vif
*vif,
return -EFAULT;
}
- msg_id = rcvd_info->buffer[1];
- msg_len = MAKE_WORD16(rcvd_info->buffer[2],
rcvd_info->buffer[3]);
- wid_id = MAKE_WORD16(rcvd_info->buffer[4],
rcvd_info->buffer[5]);
- wid_len = rcvd_info->buffer[6];
mac_status = rcvd_info->buffer[7];
- mac_status_reason_code = rcvd_info->buffer[8];
- mac_status_additional_info = rcvd_info->buffer[9];
if (hif_drv->hif_state == HOST_IF_WAITING_CONN_RESP) {
host_int_parse_assoc_resp_info(vif, mac_status);
} else if ((mac_status == MAC_STATUS_DISCONNECTED) &&
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 8be3c4c57579..76b4afaef423 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -917,12 +917,10 @@ static int add_key(struct wiphy *wiphy, struct net_device
*netdev, u8 key_index,
const u8 *tx_mic = NULL;
u8 mode = NO_ENCRYPT;
u8 op_mode;
- struct wilc *wl;
struct wilc_vif *vif;
priv = wiphy_priv(wiphy);
vif = netdev_priv(netdev);
- wl = vif->wilc;
switch (params->cipher) {
case WLAN_CIPHER_SUITE_WEP40:
@@ -1885,12 +1883,10 @@ static int start_ap(struct wiphy *wiphy, struct
net_device *dev,
struct cfg80211_ap_settings *settings)
{
struct cfg80211_beacon_data *beacon = &settings->beacon;
- struct wilc_priv *priv;
s32 ret = 0;
struct wilc *wl;
struct wilc_vif *vif;
- priv = wiphy_priv(wiphy);
vif = netdev_priv(dev);
wl = vif->wilc;
@@ -2016,14 +2012,12 @@ static int change_station(struct wiphy *wiphy, struct
net_device *dev,
const u8 *mac, struct station_parameters *params)
{
s32 ret = 0;
- struct wilc_priv *priv;
struct add_sta_param sta_params = { {0} };
struct wilc_vif *vif;
if (!wiphy)
return -EFAULT;
- priv = wiphy_priv(wiphy);
vif = netdev_priv(dev);
if (vif->iftype == AP_MODE || vif->iftype == GO_MODE) {
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] staging: ks7010: Remove unnecessary parentheses
Fixes checkpatch.pl warnings.
Signed-off-by: Nathan Chancellor
---
drivers/staging/ks7010/ks_hostif.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index a2833707e0bf..094f8c11b4ab 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -35,7 +35,7 @@ static inline u8 get_byte(struct ks_wlan_private *priv)
{
u8 data;
- data = *(priv->rxp)++;
+ data = *priv->rxp++;
/* length check in advance ! */
--(priv->rx_size);
return data;
@@ -171,7 +171,7 @@ int get_current_ap(struct ks_wlan_private *priv, struct
link_ap_info *ap_info)
"- rate_set_size=%d\n",
ap->bssid[0], ap->bssid[1], ap->bssid[2],
ap->bssid[3], ap->bssid[4], ap->bssid[5],
- &(ap->ssid.body[0]),
+ &ap->ssid.body[0],
ap->rate_set.body[0], ap->rate_set.body[1],
ap->rate_set.body[2], ap->rate_set.body[3],
ap->rate_set.body[4], ap->rate_set.body[5],
@@ -734,7 +734,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv)
netdev_dbg(priv->net_dev, " scan_ind_count=%d ::
aplist.size=%d\n",
priv->scan_ind_count, priv->aplist.size);
get_ap_information(priv, (struct ap_info *)(priv->rxp),
- &(priv->aplist.ap[priv->scan_ind_count -
1]));
+ &priv->aplist.ap[priv->scan_ind_count - 1]);
priv->aplist.size = priv->scan_ind_count;
} else {
netdev_dbg(priv->net_dev, " count over :: scan_ind_count=%d\n",
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/7] staging: ks7010: Remove unused variables
GCC warns these variables are all set but never used so remove them.
Signed-off-by: Nathan Chancellor
---
drivers/staging/ks7010/ks_hostif.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index c0a9a67dc0b4..4c2f8f710c6e 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -354,7 +354,6 @@ void hostif_data_indication(struct ks_wlan_private *priv)
unsigned short auth_type;
unsigned char temp[256];
struct ether_hdr *eth_hdr;
- unsigned short eth_proto;
struct ieee802_1x_hdr *aa1x_hdr;
size_t size;
int ret;
@@ -369,7 +368,6 @@ void hostif_data_indication(struct ks_wlan_private *priv)
get_word(priv); /* Reserve Area */
eth_hdr = (struct ether_hdr *)(priv->rxp);
- eth_proto = ntohs(eth_hdr->h_proto);
/* source address check */
if (ether_addr_equal(&priv->eth_addr[0], eth_hdr->h_source)) {
@@ -464,13 +462,9 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
struct net_device *dev = priv->net_dev;
u32 mib_status;
u32 mib_attribute;
- u16 mib_val_size;
- u16 mib_val_type;
mib_status = get_dword(priv); /* MIB status */
mib_attribute = get_dword(priv);/* MIB atttibute */
- mib_val_size = get_word(priv); /* MIB value size */
- mib_val_type = get_word(priv); /* MIB value type */
if (mib_status) {
netdev_err(priv->net_dev, "attribute=%08X, status=%08X\n",
@@ -792,9 +786,6 @@ void hostif_ps_adhoc_set_confirm(struct ks_wlan_private
*priv)
static
void hostif_infrastructure_set_confirm(struct ks_wlan_private *priv)
{
- u16 result_code;
-
- result_code = get_word(priv);
priv->infra_status = 1; /* infrastructure mode set */
hostif_sme_enqueue(priv, SME_MODE_SET_CONFIRM);
}
@@ -872,14 +863,13 @@ static
void hostif_phy_information_confirm(struct ks_wlan_private *priv)
{
struct iw_statistics *wstats = &priv->wstats;
- unsigned char rssi, signal, noise;
+ unsigned char rssi, signal;
unsigned char link_speed;
unsigned int transmitted_frame_count, received_fragment_count;
unsigned int failed_count, fcs_error_count;
rssi = get_byte(priv);
signal = get_byte(priv);
- noise = get_byte(priv);
link_speed = get_byte(priv);
transmitted_frame_count = get_dword(priv);
received_fragment_count = get_dword(priv);
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] staging: ks7010: Replace license boilerplate with SPDX identifiers
This satisfies a checkpatch.pl warning and is the preferred method for notating the license due to its lack of ambiguity. Signed-off-by: Nathan Chancellor --- drivers/staging/ks7010/ks7010_sdio.c | 5 + drivers/staging/ks7010/ks7010_sdio.h | 5 + drivers/staging/ks7010/ks_hostif.c | 5 + drivers/staging/ks7010/ks_hostif.h | 5 + drivers/staging/ks7010/ks_wlan.h | 5 + drivers/staging/ks7010/ks_wlan_ioctl.h | 5 + drivers/staging/ks7010/ks_wlan_net.c | 5 + drivers/staging/ks7010/michael_mic.c | 5 + drivers/staging/ks7010/michael_mic.h | 5 + 9 files changed, 9 insertions(+), 36 deletions(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index b29f48c421cc..6a5565d479ac 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -1,13 +1,10 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Driver for KeyStream, KS7010 based SDIO cards. * * Copyright (C) 2006-2008 KeyStream Corp. * Copyright (C) 2009 Renesas Technology Corp. * Copyright (C) 2016 Sang Engineering, Wolfram Sang - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h index 95ac86b94a7c..831b2f105f58 100644 --- a/drivers/staging/ks7010/ks7010_sdio.h +++ b/drivers/staging/ks7010/ks7010_sdio.h @@ -1,12 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Driver for KeyStream, KS7010 based SDIO cards. * * Copyright (C) 2006-2008 KeyStream Corp. * Copyright (C) 2009 Renesas Technology Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #ifndef _KS7010_SDIO_H #define _KS7010_SDIO_H diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 9a05374d950e..c0a9a67dc0b4 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -1,12 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Driver for KeyStream wireless LAN cards. * * Copyright (C) 2005-2008 KeyStream Corp. * Copyright (C) 2009 Renesas Technology Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include diff --git a/drivers/staging/ks7010/ks_hostif.h b/drivers/staging/ks7010/ks_hostif.h index 05ff5ca5da19..172d38f17595 100644 --- a/drivers/staging/ks7010/ks_hostif.h +++ b/drivers/staging/ks7010/ks_hostif.h @@ -1,12 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Driver for KeyStream wireless LAN * * Copyright (c) 2005-2008 KeyStream Corp. * Copyright (C) 2009 Renesas Technology Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #ifndef _KS_HOSTIF_H_ diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 2894b0c3816c..5070af86115f 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -1,12 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Driver for KeyStream IEEE802.11 b/g wireless LAN cards. * * Copyright (C) 2006-2008 KeyStream Corp. * Copyright (C) 2009 Renesas Technology Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #ifndef _KS_WLAN_H diff --git a/drivers/staging/ks7010/ks_wlan_ioctl.h b/drivers/staging/ks7010/ks_wlan_ioctl.h index e45a33291b46..97c7d95de411 100644 --- a/drivers/staging/ks7010/ks_wlan_ioctl.h +++ b/drivers/staging/ks7010/ks_wlan_ioctl.h @@ -1,12 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Driver for KeyStream 11b/g wireless LAN * * Copyright (c) 2005-2008 KeyStream Corp. * Copyright (C) 2009 Renesas Technology Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #ifndef _KS_WLAN_IOCTL_H diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index 694decc05c88..e96477937f65 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -1,12 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Driver for KeyStream 11b/g wireless LAN * * Copyright (C) 2005-2008 KeyStream
[PATCH 3/7] staging: ks7010: Remove unnecessary limit checks
uwrq is an unsigned 32-bit integer, it cannot be less than zero. Signed-off-by: Nathan Chancellor --- drivers/staging/ks7010/ks_wlan_net.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index e96477937f65..0c83d6fe270f 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -1928,7 +1928,7 @@ static int ks_wlan_set_beacon_lost(struct net_device *dev, if (priv->sleep_mode == SLP_SLEEP) return -EPERM; /* for SLEEP MODE */ - if (*uwrq < BEACON_LOST_COUNT_MIN || *uwrq > BEACON_LOST_COUNT_MAX) + if (*uwrq > BEACON_LOST_COUNT_MAX) return -EINVAL; priv->reg.beacon_lost_count = *uwrq; @@ -2133,7 +2133,7 @@ static int ks_wlan_set_tx_gain(struct net_device *dev, if (priv->sleep_mode == SLP_SLEEP) return -EPERM; /* for SLEEP MODE */ - if (*uwrq < 0 || *uwrq > 0xFF) + if (*uwrq > 0xFF) return -EINVAL; priv->gain.tx_gain = (uint8_t)*uwrq; @@ -2165,7 +2165,7 @@ static int ks_wlan_set_rx_gain(struct net_device *dev, if (priv->sleep_mode == SLP_SLEEP) return -EPERM; /* for SLEEP MODE */ - if (*uwrq < 0 || *uwrq > 0xFF) + if (*uwrq > 0xFF) return -EINVAL; priv->gain.rx_gain = (uint8_t)*uwrq; -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] staging: ks7010: Move from bool to int in structs
Fixes checkpatch.pl warnings.
Signed-off-by: Nathan Chancellor
---
drivers/staging/ks7010/ks_hostif.c | 4 ++--
drivers/staging/ks7010/ks_wlan.h | 4 ++--
drivers/staging/ks7010/ks_wlan_net.c | 8
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 628171091786..43090922daff 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -475,7 +475,7 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
case DOT11_MAC_ADDRESS:
hostif_sme_enqueue(priv, SME_GET_MAC_ADDRESS);
ether_addr_copy(priv->eth_addr, priv->rxp);
- priv->mac_address_valid = true;
+ priv->mac_address_valid = 1;
ether_addr_copy(dev->dev_addr, priv->eth_addr);
netdev_info(dev, "MAC ADDRESS = %pM\n", priv->eth_addr);
break;
@@ -582,7 +582,7 @@ void hostif_mib_set_confirm(struct ks_wlan_private *priv)
hostif_sme_enqueue(priv, SME_MULTICAST_CONFIRM);
break;
case LOCAL_CURRENTADDRESS:
- priv->mac_address_valid = true;
+ priv->mac_address_valid = 1;
break;
case DOT11_RSN_CONFIG_MULTICAST_CIPHER:
hostif_sme_enqueue(priv, SME_RSN_MCAST_CONFIRM);
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index 5070af86115f..ffe0e80277c9 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -411,7 +411,7 @@ struct ks_wlan_private {
unsigned int need_commit; /* for ioctl */
/* DeviceIoControl */
- bool is_device_open;
+ int is_device_open;
atomic_t event_count;
atomic_t rec_count;
int dev_count;
@@ -423,7 +423,7 @@ struct ks_wlan_private {
unsigned char firmware_version[128 + 1];
int version_size;
- bool mac_address_valid; /* Mac Address Status */
+ int mac_address_valid; /* Mac Address Status */
int dev_state;
diff --git a/drivers/staging/ks7010/ks_wlan_net.c
b/drivers/staging/ks7010/ks_wlan_net.c
index 0c83d6fe270f..30c03f5d0ec1 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2504,7 +2504,7 @@ int ks_wlan_set_mac_address(struct net_device *dev, void
*addr)
memcpy(dev->dev_addr, mac_addr->sa_data, dev->addr_len);
memcpy(priv->eth_addr, mac_addr->sa_data, ETH_ALEN);
- priv->mac_address_valid = false;
+ priv->mac_address_valid = 0;
hostif_sme_enqueue(priv, SME_MACADDRESS_SET_REQUEST);
netdev_info(dev, "ks_wlan: MAC ADDRESS = %pM\n", priv->eth_addr);
return 0;
@@ -2627,8 +2627,8 @@ int ks_wlan_net_start(struct net_device *dev)
/* int rc; */
priv = netdev_priv(dev);
- priv->mac_address_valid = false;
- priv->is_device_open = true;
+ priv->mac_address_valid = 0;
+ priv->is_device_open = 1;
priv->need_commit = 0;
/* phy information update timer */
atomic_set(&update_phyinfo, 0);
@@ -2652,7 +2652,7 @@ int ks_wlan_net_stop(struct net_device *dev)
{
struct ks_wlan_private *priv = netdev_priv(dev);
- priv->is_device_open = false;
+ priv->is_device_open = 0;
del_timer_sync(&update_phyinfo_timer);
if (netif_running(dev))
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging: ks7010: Adjust spacing around functions and declarations
checkpatch.pl warns about too many or not enough spaces in these
locations. Adjust them accordingly.
Signed-off-by: Nathan Chancellor
---
drivers/staging/ks7010/ks7010_sdio.c | 1 -
drivers/staging/ks7010/ks_hostif.c | 2 +-
drivers/staging/ks7010/michael_mic.c | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c
b/drivers/staging/ks7010/ks7010_sdio.c
index 6a5565d479ac..e23173a81f24 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -776,7 +776,6 @@ static int ks7010_upload_firmware(struct ks_sdio_card *card)
int ret;
const struct firmware *fw_entry = NULL;
-
sdio_claim_host(card->func);
/* Firmware running ? */
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 4c2f8f710c6e..a2833707e0bf 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -191,7 +191,6 @@ static u8 read_ie(unsigned char *bp, u8 max, u8 *body)
return size;
}
-
static
int get_ap_information(struct ks_wlan_private *priv, struct ap_info *ap_info,
struct local_ap *ap)
@@ -2272,6 +2271,7 @@ void hostif_sme_enqueue(struct ks_wlan_private *priv,
unsigned short event)
static inline void hostif_aplist_init(struct ks_wlan_private *priv)
{
size_t size = LOCAL_APLIST_MAX * sizeof(struct local_ap);
+
priv->aplist.size = 0;
memset(&priv->aplist.ap[0], 0, size);
}
diff --git a/drivers/staging/ks7010/michael_mic.c
b/drivers/staging/ks7010/michael_mic.c
index e6bd70846e98..3acd79615f98 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -11,7 +11,6 @@
#include
#include "michael_mic.h"
-
// Reset the state to the empty message.
static inline void michael_clear(struct michael_mic *mic)
{
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] staging: ks7010: Adjust alignment to open parenthesis
Fixes a checkpatch.pl warning.
Signed-off-by: Nathan Chancellor
---
drivers/staging/ks7010/ks_hostif.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/ks7010/ks_hostif.c
b/drivers/staging/ks7010/ks_hostif.c
index 094f8c11b4ab..628171091786 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -732,7 +732,7 @@ void hostif_scan_indication(struct ks_wlan_private *priv)
priv->scan_ind_count++;
if (priv->scan_ind_count < LOCAL_APLIST_MAX + 1) {
netdev_dbg(priv->net_dev, " scan_ind_count=%d ::
aplist.size=%d\n",
- priv->scan_ind_count, priv->aplist.size);
+ priv->scan_ind_count, priv->aplist.size);
get_ap_information(priv, (struct ap_info *)(priv->rxp),
&priv->aplist.ap[priv->scan_ind_count - 1]);
priv->aplist.size = priv->scan_ind_count;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: android: vsoc: Fix ending '(' warnings in function defintions
Fixes checkpatch.pl warnings about lines ending with parentheses.
Signed-off-by: Nathan Chancellor
---
drivers/staging/android/vsoc.c | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c
index 6ef7a011d789..b5307fa584d3 100644
--- a/drivers/staging/android/vsoc.c
+++ b/drivers/staging/android/vsoc.c
@@ -148,13 +148,13 @@ static int vsoc_release(struct inode *, struct file *);
static ssize_t vsoc_read(struct file *, char __user *, size_t, loff_t *);
static ssize_t vsoc_write(struct file *, const char __user *, size_t, loff_t
*);
static loff_t vsoc_lseek(struct file *filp, loff_t offset, int origin);
-static int do_create_fd_scoped_permission(
- struct vsoc_device_region *region_p,
- struct fd_scoped_permission_node *np,
- struct fd_scoped_permission_arg __user *arg);
-static void do_destroy_fd_scoped_permission(
- struct vsoc_device_region *owner_region_p,
- struct fd_scoped_permission *perm);
+static int
+do_create_fd_scoped_permission(struct vsoc_device_region *region_p,
+ struct fd_scoped_permission_node *np,
+ struct fd_scoped_permission_arg __user *arg);
+static void
+do_destroy_fd_scoped_permission(struct vsoc_device_region *owner_region_p,
+ struct fd_scoped_permission *perm);
static long do_vsoc_describe_region(struct file *,
struct vsoc_device_region __user *);
static ssize_t vsoc_get_area(struct file *filp, __u32 *perm_off);
@@ -203,14 +203,14 @@ static inline phys_addr_t shm_off_to_phys_addr(__u32
offset)
* Convenience functions to obtain the region from the inode or file.
* Dangerous to call before validating the inode/file.
*/
-static inline struct vsoc_device_region *vsoc_region_from_inode(
- struct inode *inode)
+static
+inline struct vsoc_device_region *vsoc_region_from_inode(struct inode *inode)
{
return &vsoc_dev.regions[iminor(inode)];
}
-static inline struct vsoc_device_region *vsoc_region_from_filep(
- struct file *inode)
+static
+inline struct vsoc_device_region *vsoc_region_from_filep(struct file *inode)
{
return vsoc_region_from_inode(file_inode(inode));
}
@@ -250,10 +250,10 @@ static struct pci_driver vsoc_pci_driver = {
.remove = vsoc_remove_device,
};
-static int do_create_fd_scoped_permission(
- struct vsoc_device_region *region_p,
- struct fd_scoped_permission_node *np,
- struct fd_scoped_permission_arg __user *arg)
+static int
+do_create_fd_scoped_permission(struct vsoc_device_region *region_p,
+ struct fd_scoped_permission_node *np,
+ struct fd_scoped_permission_arg __user *arg)
{
struct file *managed_filp;
s32 managed_fd;
@@ -344,9 +344,9 @@ static int do_create_fd_scoped_permission(
return 0;
}
-static void do_destroy_fd_scoped_permission_node(
- struct vsoc_device_region *owner_region_p,
- struct fd_scoped_permission_node *node)
+static void
+do_destroy_fd_scoped_permission_node(struct vsoc_device_region *owner_region_p,
+struct fd_scoped_permission_node *node)
{
if (node) {
do_destroy_fd_scoped_permission(owner_region_p,
@@ -358,9 +358,9 @@ static void do_destroy_fd_scoped_permission_node(
}
}
-static void do_destroy_fd_scoped_permission(
- struct vsoc_device_region *owner_region_p,
- struct fd_scoped_permission *perm)
+static void
+do_destroy_fd_scoped_permission(struct vsoc_device_region *owner_region_p,
+ struct fd_scoped_permission *perm)
{
atomic_t *owner_ptr = NULL;
int prev = 0;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: android: Kconfig; Remove excessive hyphens
Fixes the following checkpatch.pl warning: "prefer 'help' over '---help---' for new help texts" Signed-off-by: Nathan Chancellor --- drivers/staging/android/Kconfig | 4 ++-- drivers/staging/android/ion/Kconfig | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index 29d891355f7a..17c5587805f5 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -6,7 +6,7 @@ config ASHMEM bool "Enable the Anonymous Shared Memory Subsystem" default n depends on SHMEM - ---help--- + help The ashmem subsystem is a new shared memory allocator, similar to POSIX SHM but with different behavior and sporting a simpler file-based API. @@ -18,7 +18,7 @@ config ANDROID_VSOC tristate "Android Virtual SoC support" default n depends on PCI_MSI - ---help--- + help This option adds support for the Virtual SoC driver needed to boot a 'cuttlefish' Android image inside QEmu. The driver interacts with a QEmu ivshmem device. If built as a module, it will be called vsoc. diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig index 898e9a834ccc..c16dd16afe6a 100644 --- a/drivers/staging/android/ion/Kconfig +++ b/drivers/staging/android/ion/Kconfig @@ -3,7 +3,7 @@ menuconfig ION depends on HAVE_MEMBLOCK && HAS_DMA && MMU select GENERIC_ALLOCATOR select DMA_SHARED_BUFFER - ---help--- + help Choose this option to enable the ION Memory Manager, used by Android to efficiently allocate buffers from userspace that can be shared between drivers. -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: android: Clean up license identifiers
Add the identifiers when missing and fix the ones already present according to checkpatch.pl. Signed-off-by: Nathan Chancellor --- drivers/staging/android/ashmem.h| 6 +- drivers/staging/android/uapi/ashmem.h | 6 +- drivers/staging/android/uapi/vsoc_shm.h | 10 +- drivers/staging/android/vsoc.c | 11 +-- 4 files changed, 4 insertions(+), 29 deletions(-) diff --git a/drivers/staging/android/ashmem.h b/drivers/staging/android/ashmem.h index 60d7208f110a..1a478173cd21 100644 --- a/drivers/staging/android/ashmem.h +++ b/drivers/staging/android/ashmem.h @@ -1,13 +1,9 @@ -// SPDX-License-Identifier: (GPL-2.0 OR Apache-2.0) +/* SPDX-License-Identifier: GPL-2.0 OR Apache-2.0 */ /* * include/linux/ashmem.h * * Copyright 2008 Google Inc. * Author: Robert Love - * - * This file is dual licensed. It may be redistributed and/or modified - * under the terms of the Apache 2.0 License OR version 2 of the GNU - * General Public License. */ #ifndef _LINUX_ASHMEM_H diff --git a/drivers/staging/android/uapi/ashmem.h b/drivers/staging/android/uapi/ashmem.h index 5b531af6820e..5442e0019dcd 100644 --- a/drivers/staging/android/uapi/ashmem.h +++ b/drivers/staging/android/uapi/ashmem.h @@ -1,13 +1,9 @@ -// SPDX-License-Identifier: (GPL-2.0 OR Apache-2.0) +/* SPDX-License-Identifier: GPL-2.0 OR Apache-2.0 */ /* * drivers/staging/android/uapi/ashmem.h * * Copyright 2008 Google Inc. * Author: Robert Love - * - * This file is dual licensed. It may be redistributed and/or modified - * under the terms of the Apache 2.0 License OR version 2 of the GNU - * General Public License. */ #ifndef _UAPI_LINUX_ASHMEM_H diff --git a/drivers/staging/android/uapi/vsoc_shm.h b/drivers/staging/android/uapi/vsoc_shm.h index 741b1387c25b..6291fb24efb2 100644 --- a/drivers/staging/android/uapi/vsoc_shm.h +++ b/drivers/staging/android/uapi/vsoc_shm.h @@ -1,15 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Copyright (C) 2017 Google, Inc. * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * */ #ifndef _UAPI_LINUX_VSOC_SHM_H diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c index 954ed2c5d807..6ef7a011d789 100644 --- a/drivers/staging/android/vsoc.c +++ b/drivers/staging/android/vsoc.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/android/staging/vsoc.c * @@ -7,16 +8,6 @@ * * Author: ghart...@google.com * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * * Based on drivers/char/kvm_ivshmem.c - driver for KVM Inter-VM shared memory * Copyright 2009 Cam Macdonell * -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: android: vsoc: Fix ending '(' warnings in vsoc_ioctl
Fixes checkpatch.pl warnings about lines ending with parentheses.
Signed-off-by: Nathan Chancellor
---
drivers/staging/android/vsoc.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c
index 7e9cf3e4fa04..c460740f9561 100644
--- a/drivers/staging/android/vsoc.c
+++ b/drivers/staging/android/vsoc.c
@@ -552,10 +552,10 @@ static long vsoc_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
if (!node)
return -ENOMEM;
INIT_LIST_HEAD(&node->list);
- rv = do_create_fd_scoped_permission(
- region_p,
- node,
- (struct fd_scoped_permission_arg __user *)arg);
+ rv = do_create_fd_scoped_permission
+ (region_p,
+node,
+(struct fd_scoped_permission_arg __user *)arg);
if (!rv) {
mutex_lock(&vsoc_dev.mtx);
list_add(&node->list, &vsoc_dev.permissions);
@@ -582,9 +582,7 @@ static long vsoc_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
break;
case VSOC_MAYBE_SEND_INTERRUPT_TO_HOST:
- if (!atomic_xchg(
- reg_data->outgoing_signalled,
- 1)) {
+ if (!atomic_xchg(reg_data->outgoing_signalled, 1)) {
writel(reg_num, vsoc_dev.regs + DOORBELL);
return 0;
} else {
@@ -595,17 +593,16 @@ static long vsoc_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
case VSOC_SEND_INTERRUPT_TO_HOST:
writel(reg_num, vsoc_dev.regs + DOORBELL);
return 0;
-
case VSOC_WAIT_FOR_INCOMING_INTERRUPT:
- wait_event_interruptible(
- reg_data->interrupt_wait_queue,
- (atomic_read(reg_data->incoming_signalled) != 0));
+ wait_event_interruptible
+ (reg_data->interrupt_wait_queue,
+(atomic_read(reg_data->incoming_signalled) != 0));
break;
case VSOC_DESCRIBE_REGION:
- return do_vsoc_describe_region(
- filp,
- (struct vsoc_device_region __user *)arg);
+ return do_vsoc_describe_region
+ (filp,
+(struct vsoc_device_region __user *)arg);
case VSOC_SELF_INTERRUPT:
atomic_set(reg_data->incoming_signalled, 1);
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: android: vsoc: Fix ending '(' warnings in vsoc_probe_device
Fixes checkpatch.pl warnings about lines ending with parentheses.
Signed-off-by: Nathan Chancellor
---
drivers/staging/android/vsoc.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c
index c460740f9561..806beda1040b 100644
--- a/drivers/staging/android/vsoc.c
+++ b/drivers/staging/android/vsoc.c
@@ -841,8 +841,8 @@ static int vsoc_probe_device(struct pci_dev *pdev,
vsoc_dev.regions = (struct vsoc_device_region __force *)
((void *)vsoc_dev.layout +
vsoc_dev.layout->vsoc_region_desc_offset);
- vsoc_dev.msix_entries = kcalloc(
- vsoc_dev.layout->region_count,
+ vsoc_dev.msix_entries =
+ kcalloc(vsoc_dev.layout->region_count,
sizeof(vsoc_dev.msix_entries[0]), GFP_KERNEL);
if (!vsoc_dev.msix_entries) {
dev_err(&vsoc_dev.dev->dev,
@@ -850,8 +850,8 @@ static int vsoc_probe_device(struct pci_dev *pdev,
vsoc_remove_device(pdev);
return -ENOSPC;
}
- vsoc_dev.regions_data = kcalloc(
- vsoc_dev.layout->region_count,
+ vsoc_dev.regions_data =
+ kcalloc(vsoc_dev.layout->region_count,
sizeof(vsoc_dev.regions_data[0]), GFP_KERNEL);
if (!vsoc_dev.regions_data) {
dev_err(&vsoc_dev.dev->dev,
@@ -913,8 +913,8 @@ static int vsoc_probe_device(struct pci_dev *pdev,
name_sz);
dev_info(&pdev->dev, "region %d name=%s\n",
i, vsoc_dev.regions_data[i].name);
- init_waitqueue_head(
- &vsoc_dev.regions_data[i].interrupt_wait_queue);
+ init_waitqueue_head
+ (&vsoc_dev.regions_data[i].interrupt_wait_queue);
init_waitqueue_head(&vsoc_dev.regions_data[i].futex_wait_queue);
vsoc_dev.regions_data[i].incoming_signalled =
shm_off_to_virtual_addr(region->region_begin_offset) +
@@ -922,11 +922,10 @@ static int vsoc_probe_device(struct pci_dev *pdev,
vsoc_dev.regions_data[i].outgoing_signalled =
shm_off_to_virtual_addr(region->region_begin_offset) +
g_to_h_signal_table->interrupt_signalled_offset;
- result = request_irq(
- vsoc_dev.msix_entries[i].vector,
- vsoc_interrupt, 0,
- vsoc_dev.regions_data[i].name,
- vsoc_dev.regions_data + i);
+ result = request_irq(vsoc_dev.msix_entries[i].vector,
+vsoc_interrupt, 0,
+vsoc_dev.regions_data[i].name,
+vsoc_dev.regions_data + i);
if (result) {
dev_info(&pdev->dev,
"request_irq failed irq=%d vector=%d\n",
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: android: vsoc: Fix ending '(' warnings in do_destroy_fd_scoped_permission
Fixes checkpatch.pl warnings about lines ending with parentheses. Signed-off-by: Nathan Chancellor --- drivers/staging/android/vsoc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/vsoc.c b/drivers/staging/android/vsoc.c index b5307fa584d3..7e9cf3e4fa04 100644 --- a/drivers/staging/android/vsoc.c +++ b/drivers/staging/android/vsoc.c @@ -367,8 +367,8 @@ do_destroy_fd_scoped_permission(struct vsoc_device_region *owner_region_p, if (!perm) return; - owner_ptr = (atomic_t *)shm_off_to_virtual_addr( - owner_region_p->region_begin_offset + perm->owner_offset); + owner_ptr = (atomic_t *)shm_off_to_virtual_addr + (owner_region_p->region_begin_offset + perm->owner_offset); prev = atomic_xchg(owner_ptr, VSOC_REGION_FREE); if (prev != perm->owned_value) dev_err(&vsoc_dev.dev->dev, -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: android: ion: Fix license identifier comment format
checkpatch.pl complains these are invalid because the rules in Documentation/process/license-rules.rst state that C headers should have "/* */" style comments. Signed-off-by: Nathan Chancellor --- drivers/staging/android/ion/ion.h | 2 +- drivers/staging/android/uapi/ion.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index ea0897812780..16cbd38a7160 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +/* SPDX-License-Identifier: GPL-2.0 */ /* * drivers/staging/android/ion/ion.h * diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h index 825d3e95ccd3..5d7009884c13 100644 --- a/drivers/staging/android/uapi/ion.h +++ b/drivers/staging/android/uapi/ion.h @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +/* SPDX-License-Identifier: GPL-2.0 */ /* * drivers/staging/android/uapi/ion.h * -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: android: ion: Remove unnecessary blank line
Fixes a checkpatch.pl warning. Signed-off-by: Nathan Chancellor --- drivers/staging/android/ion/ion.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 269a431646be..d10b60fe4a29 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -95,7 +95,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap, goto err1; } - INIT_LIST_HEAD(&buffer->attachments); mutex_init(&buffer->lock); mutex_lock(&dev->buffer_lock); -- 2.17.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
A few questions about warnings in the ion driver
Hi everyone,
I compiled the ion driver with W=1 where I encountered the two following
warnings and I had a couple of questions about solving them.
1.
drivers/staging/android/ion/ion.c: In function ‘ion_dma_buf_begin_cpu_access’:
drivers/staging/android/ion/ion.c:316:8: warning: variable ‘vaddr’ set but not
used [-Wunused-but-set-variable]
which concerns the following function:
static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
enum dma_data_direction direction)
{
struct ion_buffer *buffer = dmabuf->priv;
void *vaddr;
struct ion_dma_buf_attachment *a;
/*
* TODO: Move this elsewhere because we don't always need a vaddr
*/
if (buffer->heap->ops->map_kernel) {
mutex_lock(&buffer->lock);
vaddr = ion_buffer_kmap_get(buffer);
mutex_unlock(&buffer->lock);
}
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
direction);
}
mutex_unlock(&buffer->lock);
return 0;
}
Can vaddr be removed and just have ion_buffer_kmap_get remain, since it is
never used again in the function?
2.
drivers/staging/android/ion/ion_carveout_heap.c:106:18: warning: no previous
prototype for ‘ion_carveout_heap_create’ [-Wmissing-prototypes]
drivers/staging/android/ion/ion_chunk_heap.c:111:18: warning: no previous
prototype for ‘ion_chunk_heap_create’ [-Wmissing-prototypes]
It appears neither of these functions are used since commit 2f87f50b2340
("staging: android: ion: Rework heap registration/enumeration"). Ultimately,
removing the whole file fixes this warning; is there still a need to rework
them or can they be removed?
If any part of this email was formatted incorrectly or could be done better,
please let me know!
Thank you,
Nathan Chancellor
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: android: ion: Fix license identifier comment format
On Mon, May 07, 2018 at 06:37:52AM -0700, Laura Abbott wrote: > On 05/06/2018 06:18 PM, Nathan Chancellor wrote: > > checkpatch.pl complains these are invalid because the rules in > > Documentation/process/license-rules.rst state that C headers should > > have "/* */" style comments. > > > > The SPDX markings are special, but I don't see anything from a > quick read of the SPDX specification that says they have to use //. > I think this is going to generate a lot of possible noise so it > might be worth adjusting checkpatch. > > Thanks, > Laura Under section 2 of "License identifier syntax" in the license rules file, it shows the preferred style for each type of file. Apparently there was some build breakage with // in header files so I assume they want /* */ for uniformity sake. Thanks! Nathan > > > Signed-off-by: Nathan Chancellor > > --- > > drivers/staging/android/ion/ion.h | 2 +- > > drivers/staging/android/uapi/ion.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/android/ion/ion.h > > b/drivers/staging/android/ion/ion.h > > index ea0897812780..16cbd38a7160 100644 > > --- a/drivers/staging/android/ion/ion.h > > +++ b/drivers/staging/android/ion/ion.h > > @@ -1,4 +1,4 @@ > > -// SPDX-License-Identifier: GPL-2.0 > > +/* SPDX-License-Identifier: GPL-2.0 */ > > /* > >* drivers/staging/android/ion/ion.h > >* > > diff --git a/drivers/staging/android/uapi/ion.h > > b/drivers/staging/android/uapi/ion.h > > index 825d3e95ccd3..5d7009884c13 100644 > > --- a/drivers/staging/android/uapi/ion.h > > +++ b/drivers/staging/android/uapi/ion.h > > @@ -1,4 +1,4 @@ > > -// SPDX-License-Identifier: GPL-2.0 > > +/* SPDX-License-Identifier: GPL-2.0 */ > > /* > >* drivers/staging/android/uapi/ion.h > >* > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: A few questions about warnings in the ion driver
On Mon, May 07, 2018 at 06:46:23AM -0700, Laura Abbott wrote:
> On 05/06/2018 06:43 PM, Nathan Chancellor wrote:
> > Hi everyone,
> >
> > I compiled the ion driver with W=1 where I encountered the two following
> > warnings and I had a couple of questions about solving them.
> >
> >
> > 1.
> >
> > drivers/staging/android/ion/ion.c: In function
> > ‘ion_dma_buf_begin_cpu_access’:
> > drivers/staging/android/ion/ion.c:316:8: warning: variable ‘vaddr’ set but
> > not used [-Wunused-but-set-variable]
> >
> > which concerns the following function:
> >
> > static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > enum dma_data_direction direction)
> > {
> > struct ion_buffer *buffer = dmabuf->priv;
> > void *vaddr;
> > struct ion_dma_buf_attachment *a;
> >
> > /*
> > * TODO: Move this elsewhere because we don't always need a vaddr
> > */
> > if (buffer->heap->ops->map_kernel) {
> > mutex_lock(&buffer->lock);
> > vaddr = ion_buffer_kmap_get(buffer);
> > mutex_unlock(&buffer->lock);
> > }
> >
> > mutex_lock(&buffer->lock);
> > list_for_each_entry(a, &buffer->attachments, list) {
> > dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
> > direction);
> > }
> > mutex_unlock(&buffer->lock);
> >
> > return 0;
> > }
> >
> > Can vaddr be removed and just have ion_buffer_kmap_get remain, since it is
> > never used again in the function?
> >
>
> I think a better solution is to check the return value of vaddr
> and error out if it fails.
>
Something like this? I was unsure if -ENOMEM or -EINVAL was a better
error return code in this context.
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index d10b60fe4a29..d1c149bb15c3 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -315,6 +315,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
struct ion_buffer *buffer = dmabuf->priv;
void *vaddr;
struct ion_dma_buf_attachment *a;
+ int ret = 0;
/*
* TODO: Move this elsewhere because we don't always need a vaddr
@@ -322,6 +323,10 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
if (buffer->heap->ops->map_kernel) {
mutex_lock(&buffer->lock);
vaddr = ion_buffer_kmap_get(buffer);
+ if (IS_ERR(vaddr)) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
mutex_unlock(&buffer->lock);
}
@@ -330,9 +335,10 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
direction);
}
- mutex_unlock(&buffer->lock);
- return 0;
+unlock:
+ mutex_unlock(&buffer->lock);
+ return ret;
}
static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
> >
> > 2.
> >
> > drivers/staging/android/ion/ion_carveout_heap.c:106:18: warning: no
> > previous prototype for ‘ion_carveout_heap_create’ [-Wmissing-prototypes]
> > drivers/staging/android/ion/ion_chunk_heap.c:111:18: warning: no previous
> > prototype for ‘ion_chunk_heap_create’ [-Wmissing-prototypes]
> >
> > It appears neither of these functions are used since commit 2f87f50b2340
> > ("staging: android: ion: Rework heap registration/enumeration"). Ultimately,
> > removing the whole file fixes this warning; is there still a need to rework
> > them or can they be removed?
> >
>
> I'd still like to delete it. I haven't seen anyone come out to re-work it.
> That said, I think my preference is still to keep it for now until
> we do another round of updates.
>
Understood, I figured it probably isn't wise to remove stuff in the
middle of a release cycle but hey, never know. I will leave it alone
for now.
Thanks!
Nathan
> Thanks for looking at these.
>
> Laura
>
> >
> > If any part of this email was formatted incorrectly or could be done better,
> > please let me know!
> >
> > Thank you,
> > Nathan Chancellor
> >
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/7] staging: ks7010: Remove unused variables
On Sun, May 06, 2018 at 03:02:59PM -0700, Nathan Chancellor wrote:
> GCC warns these variables are all set but never used so remove them.
>
> Signed-off-by: Nathan Chancellor
> ---
> drivers/staging/ks7010/ks_hostif.c | 12 +---
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks_hostif.c
> b/drivers/staging/ks7010/ks_hostif.c
> index c0a9a67dc0b4..4c2f8f710c6e 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -354,7 +354,6 @@ void hostif_data_indication(struct ks_wlan_private *priv)
> unsigned short auth_type;
> unsigned char temp[256];
> struct ether_hdr *eth_hdr;
> - unsigned short eth_proto;
> struct ieee802_1x_hdr *aa1x_hdr;
> size_t size;
> int ret;
> @@ -369,7 +368,6 @@ void hostif_data_indication(struct ks_wlan_private *priv)
> get_word(priv); /* Reserve Area */
>
> eth_hdr = (struct ether_hdr *)(priv->rxp);
> - eth_proto = ntohs(eth_hdr->h_proto);
>
> /* source address check */
> if (ether_addr_equal(&priv->eth_addr[0], eth_hdr->h_source)) {
> @@ -464,13 +462,9 @@ void hostif_mib_get_confirm(struct ks_wlan_private *priv)
> struct net_device *dev = priv->net_dev;
> u32 mib_status;
> u32 mib_attribute;
> - u16 mib_val_size;
> - u16 mib_val_type;
>
> mib_status = get_dword(priv); /* MIB status */
> mib_attribute = get_dword(priv);/* MIB atttibute */
> - mib_val_size = get_word(priv); /* MIB value size */
> - mib_val_type = get_word(priv); /* MIB value type */
>
> if (mib_status) {
> netdev_err(priv->net_dev, "attribute=%08X, status=%08X\n",
> @@ -792,9 +786,6 @@ void hostif_ps_adhoc_set_confirm(struct ks_wlan_private
> *priv)
> static
> void hostif_infrastructure_set_confirm(struct ks_wlan_private *priv)
> {
> - u16 result_code;
> -
> - result_code = get_word(priv);
> priv->infra_status = 1; /* infrastructure mode set */
> hostif_sme_enqueue(priv, SME_MODE_SET_CONFIRM);
> }
> @@ -872,14 +863,13 @@ static
> void hostif_phy_information_confirm(struct ks_wlan_private *priv)
> {
> struct iw_statistics *wstats = &priv->wstats;
> - unsigned char rssi, signal, noise;
> + unsigned char rssi, signal;
> unsigned char link_speed;
> unsigned int transmitted_frame_count, received_fragment_count;
> unsigned int failed_count, fcs_error_count;
>
> rssi = get_byte(priv);
> signal = get_byte(priv);
> - noise = get_byte(priv);
> link_speed = get_byte(priv);
> transmitted_frame_count = get_dword(priv);
> received_fragment_count = get_dword(priv);
> --
> 2.17.0
>
On second glance, I did not realize most of these functions involve
pointer semantics. Please do not apply this. I can send a v2 series if
necessary although every other patch is fine.
Thanks,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/7] staging: ks7010: Move from bool to int in structs
On Tue, May 08, 2018 at 01:34:31PM +0200, Greg Kroah-Hartman wrote:
> On Sun, May 06, 2018 at 03:03:04PM -0700, Nathan Chancellor wrote:
> > Fixes checkpatch.pl warnings.
> >
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/staging/ks7010/ks_hostif.c | 4 ++--
> > drivers/staging/ks7010/ks_wlan.h | 4 ++--
> > drivers/staging/ks7010/ks_wlan_net.c | 8
> > 3 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/ks7010/ks_hostif.c
> > b/drivers/staging/ks7010/ks_hostif.c
> > index 628171091786..43090922daff 100644
> > --- a/drivers/staging/ks7010/ks_hostif.c
> > +++ b/drivers/staging/ks7010/ks_hostif.c
> > @@ -475,7 +475,7 @@ void hostif_mib_get_confirm(struct ks_wlan_private
> > *priv)
> > case DOT11_MAC_ADDRESS:
> > hostif_sme_enqueue(priv, SME_GET_MAC_ADDRESS);
> > ether_addr_copy(priv->eth_addr, priv->rxp);
> > - priv->mac_address_valid = true;
> > + priv->mac_address_valid = 1;
>
> Wait, why? This should be bool, not an int. Why would checkpatch say
> this is incorrect?
>
> confused,
>
> greg k-h
CHECK: Avoid using bool structure members because of possible alignment issues
- see: https://lkml.org/lkml/2017/11/21/384
#417: FILE: drivers/staging/ks7010/ks_wlan.h:417:
+ bool is_device_open;
Introduced by commit a4c4c0492dad ("checkpatch: add a --strict test for
structs with bool member definitions"). If this is wrong, please feel
free to ignore it!
Thanks,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] staging: ks7010: Replace license boilerplate with SPDX identifiers
On Tue, May 08, 2018 at 01:43:13PM +0200, Greg Kroah-Hartman wrote: > On Sun, May 06, 2018 at 03:02:58PM -0700, Nathan Chancellor wrote: > > This satisfies a checkpatch.pl warning and is the preferred method for > > notating the license due to its lack of ambiguity. > > > > Signed-off-by: Nathan Chancellor > > --- > > drivers/staging/ks7010/ks7010_sdio.c | 5 + > > drivers/staging/ks7010/ks7010_sdio.h | 5 + > > drivers/staging/ks7010/ks_hostif.c | 5 + > > drivers/staging/ks7010/ks_hostif.h | 5 + > > drivers/staging/ks7010/ks_wlan.h | 5 + > > drivers/staging/ks7010/ks_wlan_ioctl.h | 5 + > > drivers/staging/ks7010/ks_wlan_net.c | 5 + > > drivers/staging/ks7010/michael_mic.c | 5 + > > drivers/staging/ks7010/michael_mic.h | 5 + > > 9 files changed, 9 insertions(+), 36 deletions(-) > > This patch does not apply to my staging.git tree :( Looks like someone beat me to most if not all of my patches (they weren't pushed out when I did these on Sunday). I will take a look later today to see if any of them are still relevant. Thanks! Nathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ks7010: Remove unused define
After commit 6d6612deaf55 ("staging: ks7010: Remove unnecessary limit
checks"), this define is not used anywhere. Remove it as well.
Suggested-by: Dan Carpenter
Signed-off-by: Nathan Chancellor
---
drivers/staging/ks7010/ks_wlan.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index 655f1e3a2157..cd2ae8871aaa 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -33,7 +33,6 @@ struct ks_wlan_parameter {
u8 preamble;
u8 power_mgmt;
u32 scan_type;
-#define BEACON_LOST_COUNT_MIN 0
#define BEACON_LOST_COUNT_MAX 65535
u32 beacon_lost_count;
u32 rts;
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/7] staging: ks7010: Remove unnecessary limit checks
On Mon, May 14, 2018 at 04:17:36PM +0300, Dan Carpenter wrote: > On Sun, May 06, 2018 at 03:03:00PM -0700, Nathan Chancellor wrote: > > uwrq is an unsigned 32-bit integer, it cannot be less than zero. > > > > Signed-off-by: Nathan Chancellor > > --- > > drivers/staging/ks7010/ks_wlan_net.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/staging/ks7010/ks_wlan_net.c > > b/drivers/staging/ks7010/ks_wlan_net.c > > index e96477937f65..0c83d6fe270f 100644 > > --- a/drivers/staging/ks7010/ks_wlan_net.c > > +++ b/drivers/staging/ks7010/ks_wlan_net.c > > @@ -1928,7 +1928,7 @@ static int ks_wlan_set_beacon_lost(struct net_device > > *dev, > > if (priv->sleep_mode == SLP_SLEEP) > > return -EPERM; > > /* for SLEEP MODE */ > > - if (*uwrq < BEACON_LOST_COUNT_MIN || *uwrq > BEACON_LOST_COUNT_MAX) > > + if (*uwrq > BEACON_LOST_COUNT_MAX) > > I believe Smatch is supposed to ignore this sort of code because > comparing "if (foo < 0 || foo > max) " is pretty readable and idiomatic. > > Presumably this was so we could redefine BEACON_LOST_COUNT_MIN, but it's > fine to unused code. The define isn't needed at all, so you can > delete that as well. > > regards, > dan carpenter > Hi Dan, Thanks for the suggestion, I just sent a patch. This warning came from GCC as a -Wtype-limit warning. I should have put that in the commit message to be more clear. I will keep this in mind for the future if I come across any more checks like this with defines. Thanks! Nathan Chancellor ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Check return value of ion_buffer_kmap_get
GCC warns that vaddr is set but unused. Check the return value of
ion_buffer_kmap_get to make vaddr useful and make sure everything
is properly configured before beginning a DMA.
Suggested-by: Laura Abbott
Signed-off-by: Nathan Chancellor
---
drivers/staging/android/ion/ion.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index d10b60fe4a29..af682cbde767 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -315,6 +315,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
struct ion_buffer *buffer = dmabuf->priv;
void *vaddr;
struct ion_dma_buf_attachment *a;
+ int ret = 0;
/*
* TODO: Move this elsewhere because we don't always need a vaddr
@@ -322,6 +323,10 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
if (buffer->heap->ops->map_kernel) {
mutex_lock(&buffer->lock);
vaddr = ion_buffer_kmap_get(buffer);
+ if (IS_ERR(vaddr)) {
+ ret = PTR_ERR(vaddr);
+ goto unlock;
+ }
mutex_unlock(&buffer->lock);
}
@@ -330,9 +335,10 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
direction);
}
- mutex_unlock(&buffer->lock);
- return 0;
+unlock:
+ mutex_unlock(&buffer->lock);
+ return ret;
}
static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
--
2.17.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ashmem: Remove deadlock
On Wed, Mar 07, 2018 at 01:40:30PM -0800, Paul Lawrence wrote:
> Regression introduced in commit ce8a3a9e76d0193e2e8d74a06d275b3c324ca652
> ("staging: android: ashmem: Fix a race condition in pin ioctls")
> causing deadlock.
>
> No need to hold ashmem_mutex while copying from user
>
> Stacks are:
>
> ashmem_mmap+0x53/0x400 drivers/staging/android/ashmem.c:379
> mmap_region+0x7dd/0xfd0 mm/mmap.c:1694
> do_mmap+0x57b/0xbe0 mm/mmap.c:1473
>
> and
>
> lock_acquire+0x12e/0x410 kernel/locking/lockdep.c:3756
> __might_fault+0x14a/0x1d0 mm/memory.c:4014
> copy_from_user arch/x86/include/asm/uaccess.h:705 [inline]
> ashmem_pin_unpin drivers/staging/android/ashmem.c:719 [inline]
>
> Signed-off-by: Paul Lawrence
> Cc: # 4.9.x
> Cc: # 4.4.x
> Cc: # 3.18.x: ce8a3a9e76d01
> Cc: # 3.18.x
> Cc: Ben Hutchings
> ---
> drivers/staging/android/ashmem.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/android/ashmem.c
> b/drivers/staging/android/ashmem.c
> index 6dbba5aff191..8c55706c2cfa 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -702,16 +702,14 @@ static int ashmem_pin_unpin(struct ashmem_area *asma,
> unsigned long cmd,
> size_t pgstart, pgend;
> int ret = -EINVAL;
>
> + if (unlikely(copy_from_user(&pin, p, sizeof(pin
> + return -EFAULT;
> +
> mutex_lock(&ashmem_mutex);
>
> if (unlikely(!asma->file))
> goto out_unlock;
>
> - if (unlikely(copy_from_user(&pin, p, sizeof(pin {
> - ret = -EFAULT;
> - goto out_unlock;
> - }
> -
> /* per custom, you can pass zero for len to mean "everything onward" */
> if (!pin.len)
> pin.len = PAGE_ALIGN(asma->size) - pin.offset;
> --
> 2.16.2.395.g2e18187dfd-goog
>
Hey Paul,
Looks like this same patch is already in Greg's tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=740a5759bf222332fbb5eda42f89aa25ba38f9b2
Cheers!
Nathan Chancellor
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] media: atomisp: Address several clang warnings
Hi all,
This series aims to clean up the code while addressing the majority of
clang warnings in this driver, some found by the 0day bot and others
found by me.
There are several enum conversion warnings that happen, which I do not
really know how to solve without understanding how exactly this driver
works. I would appreciate some guidance or a solution. Below are the
warnings, sorry for not wrapping them but they would be hard to read
otherwise.
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0,
CSS_FRAME_FORMAT_NV21 },
~
^
../drivers/staging/media/atomisp//pci/atomisp_compat.h:101:32: note: expanded
from macro 'CSS_FRAME_FORMAT_NV21'
#define CSS_FRAME_FORMAT_NV21 CSS_ID(CSS_FRAME_FORMAT_NV21)
^
../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:117:22: note:
expanded from macro 'CSS_ID'
#define CSS_ID(val) (IA_ ## val)
^~
:69:1: note: expanded from here
IA_CSS_FRAME_FORMAT_NV21
^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0,
CSS_FRAME_FORMAT_NV21 },
~^
../drivers/staging/media/atomisp//pci/atomisp_compat.h:101:32: note: expanded
from macro 'CSS_FRAME_FORMAT_NV21'
#define CSS_FRAME_FORMAT_NV21 CSS_ID(CSS_FRAME_FORMAT_NV21)
^
../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:117:22: note:
expanded from macro 'CSS_ID'
#define CSS_ID(val) (IA_ ## val)
^~
:68:1: note: expanded from here
IA_CSS_FRAME_FORMAT_NV21
^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:48:65: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0,
CSS_FRAME_FORMAT_NV12 },
~
^
../drivers/staging/media/atomisp//pci/atomisp_compat.h:99:32: note: expanded
from macro 'CSS_FRAME_FORMAT_NV12'
#define CSS_FRAME_FORMAT_NV12 CSS_ID(CSS_FRAME_FORMAT_NV12)
^
../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:117:22: note:
expanded from macro 'CSS_ID'
#define CSS_ID(val) (IA_ ## val)
^~
:67:1: note: expanded from here
IA_CSS_FRAME_FORMAT_NV12
^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:48:39: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0,
CSS_FRAME_FORMAT_NV12 },
~^
../drivers/staging/media/atomisp//pci/atomisp_compat.h:99:32: note: expanded
from macro 'CSS_FRAME_FORMAT_NV12'
#define CSS_FRAME_FORMAT_NV12 CSS_ID(CSS_FRAME_FORMAT_NV12)
^
../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:117:22: note:
expanded from macro 'CSS_ID'
#define CSS_ID(val) (IA_ ## val)
^~
:66:1: note: expanded from here
IA_CSS_FRAME_FORMAT_NV12
^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:47:34: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0,
ATOMISP_INPUT_FORMAT_BINARY_8 },
~ ^
../drivers/staging/media/atomisp//pci/atomisp_compat.h:118:35: note: expanded
from macro 'CSS_FRAME_FORMAT_BINARY_8'
#define CSS_FRAME_FORMAT_BINARY_8 CSS_ID(CSS_FRAME_FORMAT_BINARY_8)
^
../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:117:22: note:
expanded from macro 'CSS_ID'
#define CSS_ID(val) (IA_ ## val)
^~
:65:1: note: expanded from here
IA_CSS_FRAME_FORMAT_BINARY_8
^~
[PATCH 2/7] media: atomisp: Remove second increment of count in atomisp_subdev_probe
Clang warns: ../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:1097:3: warning: variable 'count' is incremented both in the loop header and in the loop body [-Wfor-loop-analysis] count++; ^ This was probably unintentional, remove it. Link: https://github.com/ClangBuiltLinux/linux/issues/1036 Reported-by: kbuild test robot Signed-off-by: Nathan Chancellor --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 694268d133c0..c42999a55303 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1094,7 +1094,6 @@ static int atomisp_subdev_probe(struct atomisp_device *isp) if (camera_count) break; msleep(SUBDEV_WAIT_TIMEOUT); - count++; } /* Wait more time to give more time for subdev init code to finish */ msleep(5 * SUBDEV_WAIT_TIMEOUT); -- 2.27.0.rc0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] media: atomisp: Remove unnecessary NULL checks in ia_css_pipe_load_extension
Clang warns:
../drivers/staging/media/atomisp/pci/sh_css.c:8537:14: warning: address
of 'pipe->output_stage' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (&pipe->output_stage)
~~ ~~^~~~
../drivers/staging/media/atomisp/pci/sh_css.c:8545:14: warning: address
of 'pipe->vf_stage' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (&pipe->vf_stage)
~~ ~~^~~~
output_stage and vf_stage are pointers in the middle of a struct, their
addresses cannot be NULL if pipe is not NULL and pipe is already checked
for NULL in this function. Simplify this if block.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor
---
drivers/staging/media/atomisp/pci/sh_css.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c
b/drivers/staging/media/atomisp/pci/sh_css.c
index d77432254a2c..b8626cdb2436 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -8533,22 +8533,9 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
}
if (firmware->info.isp.type == IA_CSS_ACC_OUTPUT)
- {
- if (&pipe->output_stage)
- append_firmware(&pipe->output_stage, firmware);
- else {
- IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR);
- return IA_CSS_ERR_INTERNAL_ERROR;
- }
- } else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER)
- {
- if (&pipe->vf_stage)
- append_firmware(&pipe->vf_stage, firmware);
- else {
- IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR);
- return IA_CSS_ERR_INTERNAL_ERROR;
- }
- }
+ append_firmware(&pipe->output_stage, firmware);
+ else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER)
+ append_firmware(&pipe->vf_stage, firmware);
err = acc_load_extension(firmware);
IA_CSS_LEAVE_ERR_PRIVATE(err);
--
2.27.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/7] media: atomisp: Add stub for atomisp_mrfld_power
Clang warns:
../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:764:12: warning:
unused function 'atomisp_mrfld_power' [-Wunused-function]
static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
^
Use an '#if 0' preprocessor define to hide the broken code, leaving the
FIXME comment intact, and creating an atomisp_mrfld_power stub function
that just returns 0.
Fixes: 95d1f398c4dc ("media: atomisp: keep the ISP powered on when setting it")
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor
---
drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index c42999a55303..41aa6018d254 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -736,6 +736,8 @@ static int atomisp_mrfld_pre_power_down(struct
atomisp_device *isp)
* WA for DDR DVFS enable/disable
* By default, ISP will force DDR DVFS 1600MHz before disable DVFS
*/
+/* FIXME: at least with ISP2401, the code below causes the driver to break */
+#if 0
static void punit_ddr_dvfs_enable(bool enable)
{
int door_bell = 1 << 8;
@@ -820,20 +822,23 @@ static int atomisp_mrfld_power(struct atomisp_device
*isp, bool enable)
dev_err(isp->dev, "IUNIT power-%s timeout.\n", enable ? "on" : "off");
return -EBUSY;
}
+#else
+static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
+{
+ return 0;
+}
+#endif
/* Workaround for pmu_nc_set_power_state not ready in MRFLD */
int atomisp_mrfld_power_down(struct atomisp_device *isp)
{
- return 0;
-// FIXME: at least with ISP2401, the code below causes the driver to break
-// return atomisp_mrfld_power(isp, false);
+ return atomisp_mrfld_power(isp, false);
}
/* Workaround for pmu_nc_set_power_state not ready in MRFLD */
int atomisp_mrfld_power_up(struct atomisp_device *isp)
{
return 0;
-// FIXME: at least with ISP2401, the code below causes the driver to break
// return atomisp_mrfld_power(isp, true);
}
--
2.27.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] media: atomisp: Clean up if block in sh_css_sp_init_stage
Clang warns:
../drivers/staging/media/atomisp/pci/sh_css_sp.c:1039:23: warning:
address of 'binary->in_frame_info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
} else if (&binary->in_frame_info) {
~~ ^
in_frame_info is not a pointer so if binary is not NULL, in_frame_info's
address cannot be NULL. Change this to an else since it will always be
evaluated as one.
While we are here, clean up this if block. The contents of both if
blocks are the same but a check against "stage == 0" is added when
ISP2401 is defined. USE_INPUT_SYSTEM_VERSION_2401 is only defined when
isp2401_system_global.h is included, which only happens when ISP2401. In
other words, USE_INPUT_SYSTEM_VERSION_2401 always requires ISP2401 to be
defined so the '#ifndef ISP2401' makes no sense. Remove that part of the
block to simplify everything.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Reported-by: kbuild test robot
Signed-off-by: Nathan Chancellor
---
drivers/staging/media/atomisp/pci/sh_css_sp.c | 27 +++
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c
b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index e574396ad0f4..e242a539d3d8 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -1015,34 +1015,15 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
return err;
#ifdef USE_INPUT_SYSTEM_VERSION_2401
-#ifndef ISP2401
- if (args->in_frame)
- {
- pipe =
find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
- if (!pipe)
- return IA_CSS_ERR_INTERNAL_ERROR;
- ia_css_get_crop_offsets(pipe, &args->in_frame->info);
- } else if (&binary->in_frame_info)
- {
+ if (stage == 0) {
pipe =
find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
if (!pipe)
return IA_CSS_ERR_INTERNAL_ERROR;
- ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
-#else
- if (stage == 0)
- {
- if (args->in_frame) {
- pipe =
find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
- if (!pipe)
- return IA_CSS_ERR_INTERNAL_ERROR;
+
+ if (args->in_frame)
ia_css_get_crop_offsets(pipe, &args->in_frame->info);
- } else if (&binary->in_frame_info) {
- pipe =
find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
- if (!pipe)
- return IA_CSS_ERR_INTERNAL_ERROR;
+ else
ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
- }
-#endif
}
#else
(void)pipe; /*avoid build warning*/
base-commit: 938b29db3aa9c293c7c1366b16e55e308f1a1ddd
--
2.27.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] media: atomisp: Remove binary_supports_input_format
Clang warns:
drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64:
warning: implicit conversion from enumeration type 'const enum
ia_css_frame_format' to different enumeration type 'enum
atomisp_input_format' [-Wenum-conversion]
binary_supports_input_format(xcandidate, req_in_info->format));
~^~
As it turns out, binary_supports_input_format only asserts that
xcandidate is not NULL and just returns true so this call is never
actually made.
There are other functions that are called that assert info is not NULL
so this function actually serves no purpose. Remove it. It can be
brought back if needed later.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor
---
.../atomisp/pci/runtime/binary/src/binary.c | 21 ---
1 file changed, 21 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index 2a23b7c6aeeb..0be2331c66cd 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -857,18 +857,6 @@ binary_supports_output_format(const struct
ia_css_binary_xinfo *info,
return false;
}
-#ifdef ISP2401
-static bool
-binary_supports_input_format(const struct ia_css_binary_xinfo *info,
-enum atomisp_input_format format)
-{
- assert(info);
- (void)format;
-
- return true;
-}
-#endif
-
static bool
binary_supports_vf_format(const struct ia_css_binary_xinfo *info,
enum ia_css_frame_format format)
@@ -1699,15 +1687,6 @@ ia_css_binary_find(struct ia_css_binary_descr *descr,
binary_supports_output_format(xcandidate, req_bin_out_info->format));
continue;
}
-#ifdef ISP2401
- if (!binary_supports_input_format(xcandidate,
descr->stream_format)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d]
continue: !%d\n",
- __LINE__,
-
binary_supports_input_format(xcandidate, req_in_info->format));
- continue;
- }
-#endif
if (xcandidate->num_output_pins > 1 &&
/* in case we have a second output pin, */
req_vf_info && /* and we need vf output.
*/
--
2.27.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] media: atomisp: Avoid overflow in compute_blending
Clang warns: drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35: warning: implicit conversion from 'unsigned long' to 'int32_t' (aka 'int') changes value from 18446744073709543424 to -8192 [-Wconstant-conversion] return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR); ~~ ~~^~~ XNR_BLENDING_SCALE_FACTOR is BIT(13), or 8192, which will easily fit into a signed 32-bit integer. However, it is an unsigned long, which means that negating it is the same as subtracting that value from ULONG_MAX + 1, which causes it to be larger than a signed 32-bit integer so it gets implicitly converted. We can avoid this by using the variable isp_scale, which holds the value of XNR_BLENDING_SCALE_FACTOR already, where the implicit conversion from unsigned long to s32 already happened. If that were to ever overflow, clang would warn: https://godbolt.org/z/EeSxLG Link: https://github.com/ClangBuiltLinux/linux/issues/1036 Signed-off-by: Nathan Chancellor --- .../atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c index a9db6366d20b..629f07faf20a 100644 --- a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c +++ b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c @@ -126,7 +126,7 @@ compute_blending(int strength) * exactly as s0.11 fixed point, but -1.0 can. */ isp_strength = -(((strength * isp_scale) + offset) / host_scale); - return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR); + return MAX(MIN(isp_strength, 0), -isp_scale); } void -- 2.27.0.rc0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] media: atomisp: Remove unnecessary NULL check in atomisp_param
Clang warns:
drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: warning:
address of 'config->info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (!&config->info) {
~ ^~~~
config cannot be NULL because it comes from an ioctl, which ensures that
the user is not giving us an invalid pointer through copy_from_user. If
config is not NULL, info cannot be NULL. Remove this check.
Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor
---
drivers/staging/media/atomisp/pci/atomisp_cmd.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 5be690f876c1..105c5aeb83ac 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -4264,7 +4264,6 @@ int atomisp_set_parameters(struct video_device *vdev,
int atomisp_param(struct atomisp_sub_device *asd, int flag,
struct atomisp_parm *config)
{
- struct atomisp_device *isp = asd->isp;
struct ia_css_pipe_config *vp_cfg =
&asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].
pipe_configs[IA_CSS_PIPE_ID_VIDEO];
@@ -4275,10 +4274,6 @@ int atomisp_param(struct atomisp_sub_device *asd, int
flag,
atomisp_css_get_dvs_grid_info(
&asd->params.curr_grid_info);
- if (!&config->info) {
- dev_err(isp->dev, "ERROR: NULL pointer in grid_info\n");
- return -EINVAL;
- }
atomisp_curr_user_grid_info(asd, &config->info);
/* We always return the resolution and stride even if there is
--
2.27.0.rc0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/7] media: atomisp: Address several clang warnings
On Wed, May 27, 2020 at 10:45:25AM +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 27 May 2020 00:11:43 -0700
> Nathan Chancellor escreveu:
>
> > Hi all,
> >
> > This series aims to clean up the code while addressing the majority of
> > clang warnings in this driver, some found by the 0day bot and others
> > found by me.
> >
> > There are several enum conversion warnings that happen, which I do not
> > really know how to solve without understanding how exactly this driver
> > works. I would appreciate some guidance or a solution. Below are the
> > warnings, sorry for not wrapping them but they would be hard to read
> > otherwise.
>
> ...
> > ../drivers/staging/media/atomisp//pci/atomisp_compat_css20.h:117:22: note:
> > expanded from macro 'CSS_ID'
> > #define CSS_ID(val) (IA_ ## val)
> ...
>
> I actually wrote a patch getting rid of this ugly thing:
>
>
> https://git.linuxtv.org/mchehab/experimental.git/commit/?h=atomisp_v3&id=cf6a15543ace1e99364911c0b7a2f6b8f2f43021
>
> This one was already submitted upstream (not merged yet), but there
> are also lots of other patches on my working tree.
Ah excellent, that makes the warnings a lot more readable. I am still
not sure how to reconcile the differences, it might be easier to just
change the types in the struct to int.
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:68: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, IA_CSS_FRAME_FORMAT_NV21, 0,
IA_CSS_FRAME_FORMAT_NV21 },
~
^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, IA_CSS_FRAME_FORMAT_NV21, 0,
IA_CSS_FRAME_FORMAT_NV21 },
~^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:48:68: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, IA_CSS_FRAME_FORMAT_NV12, 0,
IA_CSS_FRAME_FORMAT_NV12 },
~
^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:48:39: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, IA_CSS_FRAME_FORMAT_NV12, 0,
IA_CSS_FRAME_FORMAT_NV12 },
~^~~~
../drivers/staging/media/atomisp/pci/atomisp_subdev.c:47:34: warning: implicit
conversion from enumeration type 'enum ia_css_frame_format' to different
enumeration type 'enum atomisp_input_format' [-Wenum-conversion]
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, IA_CSS_FRAME_FORMAT_BINARY_8, 0,
ATOMISP_INPUT_FORMAT_BINARY_8 },
~ ^~~~
5 warnings generated.
> I'll try to apply your patch series on it, once I'll be able to
> fix a bug with mmap support.
It looks like all of them apply to your experimental branch aside from
patch 3, which you handled in a different way.
Cheers,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
On Fri, May 29, 2020 at 10:31:44PM +0200, Arnd Bergmann wrote:
> On Fri, May 29, 2020 at 10:23 PM Arnd Bergmann wrote:
> >
> > On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built
> > Linux wrote:
> > >
> > > See also Nathan's 7 patch series.
> > > https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancel...@gmail.com/
> > >
> > > Might be some overlap between series?
> > >
> >
> > Probably. I really should have checked when I saw the number of warnings.
> >
> > At least this gives Mauro a chance to double-check the changes and see if
> > Nathan and I came to different conclusions on any of them.
>
> I checked now and found that the overlap is smaller than I expected.
> In each case, Nathans' solution seems more complete than mine,
> so this patch ("staging: media: atomisp: fix incorrect NULL pointer check")
> and also "staging: media: atomisp: fix a type conversion warning" can be
> dropped, but I think the others are still needed.
>
> Arnd
Thanks for double checking! I will read through the rest of the series
and review as I can.
Cheers,
Nathan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it
On Fri, May 29, 2020 at 10:00:24PM +0200, Arnd Bergmann wrote:
> In some configurations, including this header leads to a warning:
>
> drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error:
> declaration of 'struct device' will not be visible outside of this function
> [-Werror,-Wvisibility]
>
> Make sure the struct tag is known before declaring a function
> that uses it as an argument.
>
> Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
> Signed-off-by: Arnd Bergmann
Reviewed-by: Nathan Chancellor
> ---
> drivers/staging/media/atomisp/pci/sh_css_firmware.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> index f6253392a6c9..317559c7689f 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> +++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> @@ -37,6 +37,7 @@ extern unsigned int sh_css_num_binaries;
> char
> *sh_css_get_fw_version(void);
>
> +struct device;
> bool
> sh_css_check_firmware_version(struct device *dev, const char *fw_data);
>
> --
> 2.26.2
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
On Fri, May 29, 2020 at 10:00:27PM +0200, Arnd Bergmann wrote:
> When building with clang, multiple copies of the structures to be
> initialized are passed around on the stack and copied locally, using an
> insane amount of stack space:
>
> drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of
> 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]
>
> Use constantly-allocated variables plus an explicit memcpy()
> to avoid that.
>
> Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use
> compound-literals with designated initializers")
> Signed-off-by: Arnd Bergmann
Reviewed-by: Nathan Chancellor
> ---
> drivers/staging/media/atomisp/pci/sh_css.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c
> b/drivers/staging/media/atomisp/pci/sh_css.c
> index e91c6029c651..1e8b9d637116 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -2264,6 +2264,12 @@ static enum ia_css_err
> init_pipe_defaults(enum ia_css_pipe_mode mode,
> struct ia_css_pipe *pipe,
> bool copy_pipe) {
> + static const struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
> + static const struct ia_css_preview_settings preview =
> IA_CSS_DEFAULT_PREVIEW_SETTINGS;
> + static const struct ia_css_capture_settings capture =
> IA_CSS_DEFAULT_CAPTURE_SETTINGS;
> + static const struct ia_css_video_settings video =
> IA_CSS_DEFAULT_VIDEO_SETTINGS;
> + static const struct ia_css_yuvpp_settings yuvpp =
> IA_CSS_DEFAULT_YUVPP_SETTINGS;
> +
> if (!pipe)
> {
> IA_CSS_ERROR("NULL pipe parameter");
> @@ -2271,14 +2277,14 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
> }
>
> /* Initialize pipe to pre-defined defaults */
> - *pipe = IA_CSS_DEFAULT_PIPE;
> + memcpy(pipe, &default_pipe, sizeof(default_pipe));
>
> /* TODO: JB should not be needed, but temporary backward reference */
> switch (mode)
> {
> case IA_CSS_PIPE_MODE_PREVIEW:
> pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
> - pipe->pipe_settings.preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
> + memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview));
> break;
> case IA_CSS_PIPE_MODE_CAPTURE:
> if (copy_pipe) {
> @@ -2286,11 +2292,11 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
> } else {
> pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
> }
> - pipe->pipe_settings.capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
> + memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture));
> break;
> case IA_CSS_PIPE_MODE_VIDEO:
> pipe->mode = IA_CSS_PIPE_ID_VIDEO;
> - pipe->pipe_settings.video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
> + memcpy(&pipe->pipe_settings.video, &video, sizeof(video));
> break;
> case IA_CSS_PIPE_MODE_ACC:
> pipe->mode = IA_CSS_PIPE_ID_ACC;
> @@ -2300,7 +2306,7 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
> break;
> case IA_CSS_PIPE_MODE_YUVPP:
> pipe->mode = IA_CSS_PIPE_ID_YUVPP;
> - pipe->pipe_settings.yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
> + memcpy(&pipe->pipe_settings.yuvpp, &yuvpp, sizeof(yuvpp));
> break;
> default:
> return IA_CSS_ERR_INVALID_ARGUMENTS;
> --
> 2.26.2
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/9] staging: media: atomisp: annotate an unused function
On Fri, May 29, 2020 at 10:00:25PM +0200, Arnd Bergmann wrote:
> atomisp_mrfld_power() has no more callers and produces
> a warning:
>
> drivers/staging/media/atomisp/pci/atomisp_v4l2.c:764:12: error: unused
> function 'atomisp_mrfld_power' [-Werror,-Wunused-function]
>
> Mark the function as unused while the PM code is being
> debugged, expecting that it will be used again in the
> future and should not just be removed.
>
> Fixes: 95d1f398c4dc ("media: atomisp: keep the ISP powered on when setting
> it")
> Signed-off-by: Arnd Bergmann
Mauro fixed this in his experimental branch:
https://git.linuxtv.org/mchehab/experimental.git/commit/?id=dbcee8118fc9283401df9dbe8ba03ab9d17433ff
> ---
> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 694268d133c0..10abb35ba0e0 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -761,7 +761,8 @@ static void punit_ddr_dvfs_enable(bool enable)
> pr_info("DDR DVFS, door bell is not cleared within 3ms\n");
> }
>
> -static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> +static __attribute__((unused)) int
> +atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> {
> unsigned long timeout;
> u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON :
> --
> 2.26.2
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/9] staging: media: atomisp: fix enum type mixups
On Fri, May 29, 2020 at 10:00:29PM +0200, Arnd Bergmann wrote:
> Some function calls pass an incorrect enum type:
>
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16:
> error: implicit conversion from enumeration type 'input_system_ID_t' to
> different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> gp_device_rst(INPUT_SYSTEM0_ID);
> ~ ^~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19:
> error: implicit conversion from enumeration type 'input_system_ID_t' to
> different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> input_switch_rst(INPUT_SYSTEM0_ID);
> ^~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27:
> error: implicit conversion from enumeration type 'input_system_cfg_flag_t'
> to different enumeration type 'input_system_connection_t'
> [-Werror,-Wenum-conversion]
> config.multicast[i] =
> INPUT_SYSTEM_CFG_FLAG_RESET;
> ~ ^~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32:
> error: implicit conversion from enumeration type 'input_system_ID_t' to
> different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
> ~ ^~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19:
> error: implicit conversion from enumeration type 'input_system_ID_t' to
> different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
> ^~~~
>
> INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value
> of the expected types instead.
>
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Signed-off-by: Arnd Bergmann
Huh weird that I did not see this warning but you do randconfigs so
that's expected.
Reviewed-by: Nathan Chancellor
> ---
> .../pci/hive_isp_css_common/host/input_system.c| 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git
> a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> index 2114cf4f3fda..aa0f0fca9346 100644
> ---
> a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> +++
> b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> @@ -855,9 +855,9 @@ input_system_error_t
> input_system_configuration_reset(void)
>
> input_system_network_rst(INPUT_SYSTEM0_ID);
>
> - gp_device_rst(INPUT_SYSTEM0_ID);
> + gp_device_rst(GP_DEVICE0_ID);
>
> - input_switch_rst(INPUT_SYSTEM0_ID);
> + input_switch_rst(GP_DEVICE0_ID);
>
> //target_rst();
>
> @@ -873,7 +873,7 @@ input_system_error_t
> input_system_configuration_reset(void)
>
> for (i = 0; i < N_CSI_PORTS; i++) {
> config.csi_buffer_flags[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
> - config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
> + config.multicast[i] = INPUT_SYSTEM_DISCARD_ALL;
> }
>
> config.source_type_flags =
> INPUT_SYSTEM_CFG_FLAG_RESET;
> @@ -1323,10 +1323,10 @@ static input_system_error_t
> configuration_to_registers(void)
> } // end of switch (source_type)
>
> // Set input selector.
> - input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
> + input_selector_cfg_for_sensor(GP_DEVICE0_ID);
>
> // Set input switch.
> - input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
> + input_switch_cfg(GP_DEVICE0_ID, &config.input_switch_cfg);
>
> // Set input formatters.
> // AM: IF are set dynamically.
> --
> 2.26.2
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 8/9] staging: media: atomisp: disable all custom formats
On Fri, May 29, 2020 at 10:00:30PM +0200, Arnd Bergmann wrote:
> clang points out the usage of an incorrect enum type in the
> list of supported image formats:
>
> drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit
> conversion from enumeration type 'enum ia_css_frame_format' to different
> enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
> { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0,
> CSS_FRAME_FORMAT_NV21 },
> drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit
> conversion from enumeration type 'enum ia_css_frame_format' to different
> enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
> { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0,
> CSS_FRAME_FORMAT_NV21 },
> { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0,
> CSS_FRAME_FORMAT_NV12 },
> { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0,
> ATOMISP_INPUT_FORMAT_BINARY_8 },
>
> Checking the git history, I found a commit that disabled one such case
> because it did not work. It seems likely that the incorrect enum was
> part of the original problem and that the others do not work either,
> or have never been tested.
>
> Disable all the ones that cause a warning.
>
> Fixes: cb02ae3d71ea ("media: staging: atomisp: Disable custom format for now")
> Signed-off-by: Arnd Bergmann
I have this patch in my local tree and debated sending it myself. I
think that this is the right fix for now, as the driver is being cleaned
up. Maybe add a FIXME like the rest of this driver?
Regardless of that last point:
Reviewed-by: Nathan Chancellor
> ---
> drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> index 46590129cbe3..8bce466cc128 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> @@ -44,9 +44,11 @@ const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = {
> { MEDIA_BUS_FMT_SRGGB12_1X12, 12, 12, ATOMISP_INPUT_FORMAT_RAW_12,
> CSS_BAYER_ORDER_RGGB, CSS_FORMAT_RAW_12 },
> { MEDIA_BUS_FMT_UYVY8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0,
> ATOMISP_INPUT_FORMAT_YUV422_8 },
> { MEDIA_BUS_FMT_YUYV8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0,
> ATOMISP_INPUT_FORMAT_YUV422_8 },
> +#if 0
> { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0,
> ATOMISP_INPUT_FORMAT_BINARY_8 },
> { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0,
> CSS_FRAME_FORMAT_NV12 },
> { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0,
> CSS_FRAME_FORMAT_NV21 },
> +#endif
> { V4L2_MBUS_FMT_CUSTOM_YUV420, 12, 12,
> ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY, 0, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY
> },
> #if 0
> { V4L2_MBUS_FMT_CUSTOM_M10MO_RAW, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0,
> ATOMISP_INPUT_FORMAT_BINARY_8 },
> --
> 2.26.2
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency
On Fri, May 29, 2020 at 10:00:31PM +0200, Arnd Bergmann wrote: > Without that driver, there is a link failure in > > ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element" > [drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined! > > Add an explicit Kconfig dependency. > > Signed-off-by: Arnd Bergmann It'd be interesting to know if this is strictly required for the driver to work properly. The call to intel_soc_pmic_exec_mipi_pmic_seq_element has some error handling after it, maybe that should just be surrounded by an #ifdef or IS_ENABLED for PMIC_OPREGION, like some other drivers do. Regardless of that: Reviewed-by: Nathan Chancellor > --- > drivers/staging/media/atomisp/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/media/atomisp/Kconfig > b/drivers/staging/media/atomisp/Kconfig > index c4f3049b0706..e86311c14329 100644 > --- a/drivers/staging/media/atomisp/Kconfig > +++ b/drivers/staging/media/atomisp/Kconfig > @@ -11,6 +11,7 @@ menuconfig INTEL_ATOMISP > config VIDEO_ATOMISP > tristate "Intel Atom Image Signal Processor Driver" > depends on VIDEO_V4L2 && INTEL_ATOMISP > + depends on PMIC_OPREGION > select IOSF_MBI > select VIDEOBUF_VMALLOC > ---help--- > -- > 2.26.2 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
