Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: hal: Add spaces around operators
On Tue, Apr 06, 2021 at 03:01:38PM +0200, Fabio M. De Francesco wrote:
> Added spaces around operators in file HalBtc8723b1Ant.c. Issue detected
> by checkpatch.pl. Spaces are preferred to improve readibility.
>
> Signed-off-by: Fabio M. De Francesco
> ---
> .../staging/rtl8723bs/hal/HalBtc8723b1Ant.c | 98 +--
> 1 file changed, 49 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/HalBtc8723b1Ant.c
> b/drivers/staging/rtl8723bs/hal/HalBtc8723b1Ant.c
> index 3e794093092b..b2b872016e45 100644
> --- a/drivers/staging/rtl8723bs/hal/HalBtc8723b1Ant.c
> +++ b/drivers/staging/rtl8723bs/hal/HalBtc8723b1Ant.c
> @@ -38,7 +38,7 @@ static u8 halbtc8723b1ant_BtRssiState(
> (pCoexSta->preBtRssiState == BTC_RSSI_STATE_LOW) ||
> (pCoexSta->preBtRssiState == BTC_RSSI_STATE_STAY_LOW)
> ) {
> - if (btRssi >=
> (rssiThresh+BTC_RSSI_COEX_THRESH_TOL_8723B_1ANT)) {
> + if (btRssi >= (rssiThresh +
> BTC_RSSI_COEX_THRESH_TOL_8723B_1ANT)) {
>
> btRssiState = BTC_RSSI_STATE_HIGH;
> BTC_PRINT(
> @@ -85,7 +85,7 @@ static u8 halbtc8723b1ant_BtRssiState(
> (pCoexSta->preBtRssiState == BTC_RSSI_STATE_LOW) ||
> (pCoexSta->preBtRssiState == BTC_RSSI_STATE_STAY_LOW)
> ) {
> - if (btRssi >=
> (rssiThresh+BTC_RSSI_COEX_THRESH_TOL_8723B_1ANT)) {
> + if (btRssi >= (rssiThresh +
> BTC_RSSI_COEX_THRESH_TOL_8723B_1ANT)) {
> btRssiState = BTC_RSSI_STATE_MEDIUM;
> BTC_PRINT(
> BTC_MSG_ALGORITHM,
> @@ -104,7 +104,7 @@ static u8 halbtc8723b1ant_BtRssiState(
> (pCoexSta->preBtRssiState == BTC_RSSI_STATE_MEDIUM) ||
> (pCoexSta->preBtRssiState == BTC_RSSI_STATE_STAY_MEDIUM)
> ) {
> - if (btRssi >=
> (rssiThresh1+BTC_RSSI_COEX_THRESH_TOL_8723B_1ANT)) {
> + if (btRssi >= (rssiThresh1 +
> BTC_RSSI_COEX_THRESH_TOL_8723B_1ANT)) {
> btRssiState = BTC_RSSI_STATE_HIGH;
> BTC_PRINT(
> BTC_MSG_ALGORITHM,
> @@ -353,11 +353,11 @@ static void halbtc8723b1ant_MonitorBtCtr(struct
> btc_coexist *pBtCoexist)
>
> u4Tmp = pBtCoexist->fBtcRead4Byte(pBtCoexist, regHPTxRx);
> regHPTx = u4Tmp & bMaskLWord;
> - regHPRx = (u4Tmp & bMaskHWord)>>16;
> + regHPRx = (u4Tmp & bMaskHWord) >> 16;
>
> u4Tmp = pBtCoexist->fBtcRead4Byte(pBtCoexist, regLPTxRx);
> regLPTx = u4Tmp & bMaskLWord;
> - regLPRx = (u4Tmp & bMaskHWord)>>16;
> + regLPRx = (u4Tmp & bMaskHWord) >> 16;
>
> pCoexSta->highPriorityTx = regHPTx;
> pCoexSta->highPriorityRx = regHPRx;
> @@ -1317,7 +1317,7 @@ static void halbtc8723b1ant_SetFwPstdma(
> pBtCoexist->fBtcGet(pBtCoexist, BTC_GET_BL_WIFI_AP_MODE_ENABLE,
> );
>
> if (bApEnable) {
> - if (byte1 && !(byte1)) {
> + if (byte1 & BIT4 && !(byte1 & BIT5)) {
> BTC_PRINT(
> BTC_MSG_INTERFACE,
> INTF_NOTIFY,
> @@ -1349,9 +1349,9 @@ static void halbtc8723b1ant_SetFwPstdma(
> (
> "[BTCoex], PS-TDMA H2C cmd = 0x%x%08x\n",
> H2C_Parameter[0],
> - H2C_Parameter[1]<<24|
> - H2C_Parameter[2]<<16|
> - H2C_Parameter[3]<<8|
> + H2C_Parameter[1] << 24 |
> + H2C_Parameter[2] << 16 |
> + H2C_Parameter[3]<< 8 |
Why did you miss the space that is needed here too?
Did you run this patch through checkpatch.pl?
Please do so.
thanks,
greg k-h
Re: [Outreachy kernel] [PATCH v2] staging: rtl8723bs: hal: Remove camelcase in Hal8723BReg.h
On Tue, Apr 06, 2021 at 03:05:56PM +0200, Fabio M. De Francesco wrote: > Remove camelcase in some symbols defined in Hal8723BReg.h. These symbols > are not used anywhere else, therefore this patch does not break the driver. > > Signed-off-by: Fabio M. De Francesco > --- > drivers/staging/rtl8723bs/hal/Hal8723BReg.h | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) If this is "v2", you need to put below the --- line what changed from v1. Please fix up and send a v3. thanks, greg k-h
Re: linux-next: build warning after merge of the char-misc tree
On Tue, Apr 06, 2021 at 09:44:41PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the char-misc tree, today's linux-next build (htmldocs)
> produced this warning:
>
> Documentation/misc-devices/dw-xdata-pcie.rst:20: WARNING: Unexpected
> indentation.
> Documentation/misc-devices/dw-xdata-pcie.rst:24: WARNING: Unexpected
> indentation.
> Documentation/misc-devices/dw-xdata-pcie.rst:25: WARNING: Block quote ends
> without a blank line; unexpected unindent.
> Documentation/misc-devices/dw-xdata-pcie.rst:30: WARNING: Unexpected
> indentation.
> Documentation/misc-devices/dw-xdata-pcie.rst:34: WARNING: Unexpected
> indentation.
> Documentation/misc-devices/dw-xdata-pcie.rst:35: WARNING: Block quote ends
> without a blank line; unexpected unindent.
> Documentation/misc-devices/dw-xdata-pcie.rst:40: WARNING: Unexpected
> indentation.
>
> Introduced by commit
>
> e1181b5bbc3c ("Documentation: misc-devices: Add Documentation for
> dw-xdata-pcie driver")
Gustavo, can you send a follow-on patch to fix this up?
thanks,
greg k-h
Re: [PATCH] net: hso: fix null-ptr-deref during tty device unregistration
On Tue, Apr 06, 2021 at 06:13:59PM +0530, Anirudh Rayabharam wrote:
> Multiple ttys try to claim the same the minor number causing a double
> unregistration of the same device. The first unregistration succeeds
> but the next one results in a null-ptr-deref.
>
> The get_free_serial_index() function returns an available minor number
> but doesn't assign it immediately. The assignment is done by the caller
> later. But before this assignment, calls to get_free_serial_index()
> would return the same minor number.
>
> Fix this by modifying get_free_serial_index to assign the minor number
> immediately after one is found to be and rename it to obtain_minor()
> to better reflect what it does. Similary, rename set_serial_by_index()
> to release_minor() and modify it to free up the minor number of the
> given hso_serial. Every obtain_minor() should have corresponding
> release_minor() call.
>
> Reported-by: syzbot+c49fe6089f295a05e...@syzkaller.appspotmail.com
> Tested-by: syzbot+c49fe6089f295a05e...@syzkaller.appspotmail.com
>
> Signed-off-by: Anirudh Rayabharam
> ---
> drivers/net/usb/hso.c | 32
> 1 file changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index 31d51346786a..295ca330e70c 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -611,7 +611,7 @@ static struct hso_serial *get_serial_by_index(unsigned
> index)
> return serial;
> }
>
> -static int get_free_serial_index(void)
> +static int obtain_minor(struct hso_serial *serial)
> {
> int index;
> unsigned long flags;
> @@ -619,8 +619,10 @@ static int get_free_serial_index(void)
> spin_lock_irqsave(_table_lock, flags);
> for (index = 0; index < HSO_SERIAL_TTY_MINORS; index++) {
> if (serial_table[index] == NULL) {
> + serial_table[index] = serial->parent;
> + serial->minor = index;
> spin_unlock_irqrestore(_table_lock, flags);
> - return index;
> + return 0;
Minor note, you might want to convert this to use an idr structure in
the future, this "loop and find a free minor" isn't really needed now
that we have a data structure that does this all for us :)
But that's not going to fix this issue, that's for future changes.
thanks,
greg k-h
Re: [PATCH] net: hso: fix null-ptr-deref during tty device unregistration
On Tue, Apr 06, 2021 at 06:13:59PM +0530, Anirudh Rayabharam wrote: > Multiple ttys try to claim the same the minor number causing a double > unregistration of the same device. The first unregistration succeeds > but the next one results in a null-ptr-deref. > > The get_free_serial_index() function returns an available minor number > but doesn't assign it immediately. The assignment is done by the caller > later. But before this assignment, calls to get_free_serial_index() > would return the same minor number. > > Fix this by modifying get_free_serial_index to assign the minor number > immediately after one is found to be and rename it to obtain_minor() > to better reflect what it does. Similary, rename set_serial_by_index() > to release_minor() and modify it to free up the minor number of the > given hso_serial. Every obtain_minor() should have corresponding > release_minor() call. > > Reported-by: syzbot+c49fe6089f295a05e...@syzkaller.appspotmail.com > Tested-by: syzbot+c49fe6089f295a05e...@syzkaller.appspotmail.com > > Signed-off-by: Anirudh Rayabharam Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 1/4] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device
On Tue, Apr 06, 2021 at 08:45:51PM +0800, Yicong Yang wrote:
> +static int hisi_ptt_create_trace_entries(struct hisi_ptt *hisi_ptt)
> +{
> + struct hisi_ptt_debugfs_file_desc *trace_files;
> + struct dentry *dir;
> + int i, ret = 0;
> +
> + dir = debugfs_create_dir("trace", hisi_ptt->debugfs_dir);
> + if (IS_ERR(dir))
> + return PTR_ERR(dir);
No need to care about this, please do not check, code should not do
different things based on if debugfs is working or not.
> +
> + trace_files = devm_kmemdup(_ptt->pdev->dev, trace_entries,
> +sizeof(trace_entries), GFP_KERNEL);
> + if (IS_ERR(trace_files)) {
> + ret = PTR_ERR(trace_files);
> + goto err;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(trace_entries); ++i) {
> + struct dentry *file;
> +
> + trace_files[i].hisi_ptt = hisi_ptt;
> + file = debugfs_create_file(trace_files[i].name, 0600,
> +dir, _files[i],
> +trace_files[i].fops);
> + if (IS_ERR(file)) {
> + ret = PTR_ERR(file);
Same here, why check?
> +static int hisi_ptt_register_debugfs(void)
> +{
> + if (!debugfs_initialized()) {
> + pr_err("failed to create debugfs directory: debugfs
> uninitialized\n");
Why do you care? How can this happen?
> + return -ENOENT;
> + }
> +
> + hisi_ptt_debugfs_root = debugfs_create_dir("hisi_ptt", NULL);
> + if (IS_ERR(hisi_ptt_debugfs_root)) {
Again, no need to check.
If you are building the whole functionality of your code on if debugfs
is working or not, that feels really wrong. Debugfs is for random
kernel debug type things, not a whole driver infrastructure that somehow
relies on it working or not.
thanks,
greg k-h
Re: [PATCH 0/4] Add support for HiSilicon PCIe Tune and Trace device
On Tue, Apr 06, 2021 at 08:45:50PM +0800, Yicong Yang wrote: > HiSilicon PCIe tune and trace device(PTT) is a PCIe Root Complex > integrated Endpoint(RCiEP) device, providing the capability > to dynamically monitor and tune the PCIe traffic(tune), > and trace the TLP headers(trace). The driver exposes the user > interface through debugfs, so no need for extra user space tools. > The usage is described in the document. Why use debugfs and not the existing perf tools for debugging? thanks, greg k-h
Re: [PATCH] staging: greybus: Match parentheses alignment
On Tue, Apr 06, 2021 at 06:42:59PM +0600, Zhansaya Bagdauletkyzy wrote:
> Match next line with open parentheses by adding tabs/spaces
> to conform with Linux kernel coding style.
> Reported by checkpatch.
>
> Signed-off-by: Zhansaya Bagdauletkyzy
> ---
> drivers/staging/greybus/camera.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/greybus/camera.c
> b/drivers/staging/greybus/camera.c
> index cdbb42cd413b..5dca585694c0 100644
> --- a/drivers/staging/greybus/camera.c
> +++ b/drivers/staging/greybus/camera.c
> @@ -220,7 +220,7 @@ static int gb_camera_operation_sync_flags(struct
> gb_connection *connection,
> }
>
> static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
> - struct gb_camera_configure_streams_response *resp)
> + struct
> gb_camera_configure_streams_response *resp)
> {
> unsigned int max_pkt_size = 0;
> unsigned int i;
> @@ -378,8 +378,8 @@ struct ap_csi_config_request {
> #define GB_CAMERA_CSI_CLK_FREQ_MARGIN15000U
>
> static int gb_camera_setup_data_connection(struct gb_camera *gcam,
> - struct gb_camera_configure_streams_response *resp,
> - struct gb_camera_csi_params *csi_params)
> +struct
> gb_camera_configure_streams_response *resp,
> +struct gb_camera_csi_params
> *csi_params)
And now you violate another coding style requirement, which means
someone will send another patch to fix that up and around and around it
goes...
Sorry, not going to take this.
greg k-h
Re: [PATCH 3/4] staging: rtl8723bs: core: reorganize characters so the lines are under 100 ch
On Mon, Apr 05, 2021 at 06:29:19PM +0100, Beatriz Martins de Carvalho wrote:
> Cleans up warnings of "line over 100 characters" but avoinding
> more than 90 characters in file rtw_ap.c
>
> Signed-off-by: Beatriz Martins de Carvalho
>
> ---
> drivers/staging/rtl8723bs/core/rtw_ap.c | 22 ++
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c
> b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 4dab4d741675..ca6fec52d213 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -278,11 +278,13 @@ void expire_timeout_chk(struct adapter *padapter)
>
> if (psta->state & WIFI_SLEEP_STATE) {
> if (!(psta->state & WIFI_STA_ALIVE_CHK_STATE)) {
> - /* to check if alive by another methods
> if station is at ps mode. */
> + /* to check if alive by another methods
> */
> + /* if station is at ps mode. */
> psta->expire_to = pstapriv->expire_to;
> psta->state |= WIFI_STA_ALIVE_CHK_STATE;
>
> - /* DBG_871X("alive chk, sta:%pM is at
> ps mode!\n", MAC_ARG(psta->hwaddr)); */
> + /* DBG_871X("alive chk, sta:%pM is at
> ps */
> + /* mode!\n", MAC_ARG(psta->hwaddr)); */
You just wrapped a code line :(
Just remove it, it's not needed.
>
> /* to update bcn with tim_bitmap for
> this station */
> pstapriv->tim_bitmap |= BIT(psta->aid);
> @@ -309,7 +311,8 @@ void expire_timeout_chk(struct adapter *padapter)
> );
> updated = ap_free_sta(padapter, psta, false,
> WLAN_REASON_DEAUTH_LEAVING);
> } else {
> - /* TODO: Aging mechanism to digest frames in sleep_q to
> avoid running out of xmitframe */
> + /* TODO: Aging mechanism to digest frames in sleep_q to
> */
> + /* avoid running out of xmitframe */
> if (psta->sleepq_len > (NR_XMITFRAME /
> pstapriv->asoc_list_cnt)
> && padapter->xmitpriv.free_xmitframe_cnt < ((
> NR_XMITFRAME / pstapriv->asoc_list_cnt
> @@ -375,7 +378,8 @@ void expire_timeout_chk(struct adapter *padapter)
> if (list_empty(>asoc_list) == false) {
> list_del_init(>asoc_list);
> pstapriv->asoc_list_cnt--;
> - updated = ap_free_sta(padapter, psta, false,
> WLAN_REASON_DEAUTH_LEAVING);
> + updated = ap_free_sta(padapter, psta, false,
> +
> WLAN_REASON_DEAUTH_LEAVING);
> }
> spin_unlock_bh(>asoc_list_lock);
> }
> @@ -469,7 +473,8 @@ void update_bmc_sta(struct adapter *padapter)
>
> memset((void *)>sta_stats, 0, sizeof(struct
> stainfo_stats));
>
> - /* psta->dot118021XPrivacy = _NO_PRIVACY_;//!!! remove it,
> because it has been set before this. */
> + /* psta->dot118021XPrivacy = _NO_PRIVACY_;//!!! remove it, */
> + /* because it has been set before this. */
Again, look at what you are changing to see if it makes sense.
>
> /* prepare for add_RATid */
> supportRateNum = rtw_get_rateset_len((u8
> *)_network->SupportedRates);
> @@ -748,8 +753,8 @@ void start_bss_network(struct adapter *padapter, u8 *pbuf)
> cur_ch_offset = HAL_PRIME_CHNL_OFFSET_DONT_CARE;
>
> /* check if there is wps ie, */
> - /* if there is wpsie in beacon, the hostapd will update beacon twice
> when stating hostapd, */
> - /* and at first time the security ie (RSN/WPA IE) will not include in
> beacon. */
> + /* if there is wpsie in beacon, the hostapd will update beacon twice
> when stating */
> + /* hostapd, and at first time the security ie (RSN/WPA IE) will not
> include in beacon. */
These changes do not look correct, you made them longer?
thanks,
greg k-h
Re: [PATCH] staging: sm750fb: Convert camel case to snake case
On Tue, Apr 06, 2021 at 02:18:41AM -0700, Pavle Rohalj wrote: > - struct dvi_ctrl_device *pCurrentDviCtrl; > + struct dvi_ctrl_device *p_current_dvi_ctrl; Does this change make sense? Why keep the "p_" here? We do not need or use, this type of variable naming in the kernel. Also, please break this up into a patch series where you do one structure change at a time. thanks, greg k-h
Re: [PATCH v7] USB: serial: cp210x: Add support for GPIOs on CP2108
On Tue, Apr 06, 2021 at 09:25:18AM +, Pho Tran wrote: > Hi Greg! > Should I send the Patch with new version (PATCH v8) or keep version of the > Patch is v7? Again, please do not top-post. v8 is needed. thanks, greg k-h
Re: linux-next: manual merge of the tty tree with the net-next tree
On Tue, Apr 06, 2021 at 06:48:14PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the tty tree got a conflict in:
>
> net/nfc/nci/uart.c
>
> between commit:
>
> d3295869c40c ("net: nfc: Fix spelling errors in net/nfc module")
>
> from the net-next tree and commit:
>
> c2a5a45c0276 ("net: nfc: nci: drop nci_uart_ops::recv_buf")
>
> from the tty tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
Change looks correct to me, thanks!
greg k-h
Re: linux-next: manual merge of the driver-core tree with the devicetree tree
On Tue, Apr 06, 2021 at 06:19:45PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the driver-core tree got a conflict in:
>
> drivers/of/property.c
>
> between commit:
>
> 3915fed92365 ("of: property: Provide missing member description and remove
> excess param")
>
> from the devicetree tree and commit:
>
> f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint")
>
> from the driver-core tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
Change looks good to me, thanks!
greg k-h
Re: [PATCH v7] USB: serial: cp210x: Add support for GPIOs on CP2108
A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? On Tue, Apr 06, 2021 at 08:17:42AM +, Pho Tran wrote: > Hi Greg! > I am grateful for your promptly reply! > Yesterday, I got the response from the kernel test robot with this message: > " If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot ". > It's a reason why I added "Reported-by: kernel test robot " > to my new Patch. > Could you tell me what I need to do in the next step to submit this patch to > the kernel mainline? > Once again, Thank you a lot! If you fixed this as an add-on patch, yes it would belong in the reported-by: But as you are just fixing this up to a patch that has not been accepted yet, all you need to do is write it in the "what changed from the previous version" section of the changelog, below the --- line. thanks, greg k-h
Re: [RESEND PATCH v5 1/2] bio: limit bio max size
On Tue, Apr 06, 2021 at 10:31:28AM +0900, Changheun Lee wrote:
> > bio size can grow up to 4GB when muli-page bvec is enabled.
> > but sometimes it would lead to inefficient behaviors.
> > in case of large chunk direct I/O, - 32MB chunk read in user space -
> > all pages for 32MB would be merged to a bio structure if the pages
> > physical addresses are contiguous. it makes some delay to submit
> > until merge complete. bio max size should be limited to a proper size.
> >
> > When 32MB chunk read with direct I/O option is coming from userspace,
> > kernel behavior is below now in do_direct_IO() loop. it's timeline.
> >
> > | bio merge for 32MB. total 8,192 pages are merged.
> > | total elapsed time is over 2ms.
> > |-- ... --->|
> > | 8,192 pages merged a bio.
> > | at this time, first bio
> > submit is done.
> > | 1 bio is split to 32
> > read request and issue.
> > |--->
> > |--->
> >|--->
> > ..
> >
> > |--->
> >
> > |--->|
> > total 19ms elapsed to complete 32MB read done
> > from device. |
> >
> > If bio max size is limited with 1MB, behavior is changed below.
> >
> > | bio merge for 1MB. 256 pages are merged for each bio.
> > | total 32 bio will be made.
> > | total elapsed time is over 2ms. it's same.
> > | but, first bio submit timing is fast. about 100us.
> > |--->|--->|--->|---> ... -->|--->|--->|--->|--->|
> > | 256 pages merged a bio.
> > | at this time, first bio submit is done.
> > | and 1 read request is issued for 1 bio.
> > |--->
> >|--->
> > |--->
> > ..
> > |--->
> > |--->|
> > total 17ms elapsed to complete 32MB read done from device. |
> >
> > As a result, read request issue timing is faster if bio max size is limited.
> > Current kernel behavior with multipage bvec, super large bio can be created.
> > And it lead to delay first I/O request issue.
> >
> > Signed-off-by: Changheun Lee
> > ---
> > block/bio.c| 13 -
> > include/linux/bio.h| 2 +-
> > include/linux/blkdev.h | 3 +++
> > 3 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/bio.c b/block/bio.c
> > index 1f2cc1fbe283..c528e1f944c7 100644
> > --- a/block/bio.c
> > +++ b/block/bio.c
> > @@ -287,6 +287,17 @@ void bio_init(struct bio *bio, struct bio_vec *table,
> > }
> > EXPORT_SYMBOL(bio_init);
> >
> > +unsigned int bio_max_size(struct bio *bio)
> > +{
> > + struct request_queue *q = bio->bi_disk->queue;
> > +
> > + if (blk_queue_limit_bio_size(q))
> > + return blk_queue_get_max_sectors(q, bio_op(bio))
> > + << SECTOR_SHIFT;
> > +
> > + return UINT_MAX;
> > +}
> > +
> > /**
> > * bio_reset - reinitialize a bio
> > * @bio: bio to reset
> > @@ -877,7 +888,7 @@ bool __bio_try_merge_page(struct bio *bio, struct page
> > *page,
> > struct bio_vec *bv = >bi_io_vec[bio->bi_vcnt - 1];
> >
> > if (page_is_mergeable(bv, page, len, off, same_page)) {
> > - if (bio->bi_iter.bi_size > UINT_MAX - len) {
> > + if (bio->bi_iter.bi_size > bio_max_size(bio) - len) {
> > *same_page = false;
> > return false;
> > }
> > diff --git a/include/linux/bio.h b/include/linux/bio.h
> > index 1edda614f7ce..13b6f6562a5b 100644
> > --- a/include/linux/bio.h
> > +++ b/include/linux/bio.h
> > @@ -113,7 +113,7 @@ static inline bool bio_full(struct bio *bio, unsigned
> > len)
> > if (bio->bi_vcnt >= bio->bi_max_vecs)
> > return true;
> >
> > - if (bio->bi_iter.bi_size > UINT_MAX - len)
> > + if (bio->bi_iter.bi_size > bio_max_size(bio) - len)
> > return true;
> >
> > return false;
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index f94ee3089e01..3aeab9e7e97b 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -621,6 +621,7 @@ struct request_queue {
> > #define QUEUE_FLAG_RQ_ALLOC_TIME 27/* record rq->alloc_time_ns */
> > #define QUEUE_FLAG_HCTX_ACTIVE 28 /* at least one blk-mq hctx is
> > active */
> > #define QUEUE_FLAG_NOWAIT
Re: [PATCH] usb: gadget: aspeed: Remove unnecessary version.h includes
On Tue, Apr 06, 2021 at 11:59:58AM +0800, Jiapeng Chong wrote: > "make versioncheck" shows: > > ./drivers/usb/gadget/udc/aspeed-vhub/hub.c: 33 linux/version.h not > needed. Then you need to fix the tool, and always test-build patches before you send them out, as this is obviously wrong :(
Re: [PATCH v7] USB: serial: cp210x: Add support for GPIOs on CP2108
On Tue, Apr 06, 2021 at 11:02:38AM +0700, Pho Tran wrote: > From: Pho Tran > > Similar to other CP210x devices, GPIO interfaces (gpiochip) should be > supported for CP2108. > > CP2108 has 4 serial interfaces but only 1 set of GPIO pins are shared > to all of those interfaces. So, just need to initialize GPIOs of CP2108 > with only one interface (I use interface 0). It means just only 1 gpiochip > device file will be created for CP2108. > > CP2108 has 16 GPIOs, So data types of several variables need to be is u16 > instead of u8(in struct cp210x_serial_private). This doesn't affect other > CP210x devices. > > Because CP2108 has 16 GPIO pins, the parameter passed by cp210x functions > will be different from other CP210x devices. So need to check part number > of the device to use correct data format before sending commands to > devices. > > Like CP2104, CP2108 have GPIO pins with configurable options. Therefore, > should be mask all pins which are not in GPIO mode in cp2108_gpio_init() > function. > > Fix build test WARNING reported by kernel test robot. > > Reported-by: Kernel test robot The kernel test robot did not report this needed support :(
Re: [PATCH v4 9/9] w1: ds2438: support for writing to offset register
On Mon, Apr 05, 2021 at 09:44:01AM -0300, Luiz Sampaio wrote: > On Mon, Apr 05, 2021 at 01:04:59PM +0200, Greg KH wrote: > > On Mon, Apr 05, 2021 at 07:50:09AM -0300, Luiz Sampaio wrote: > > > Added a sysfs entry to support writing to the offset register on page1. > > > This register is used to calibrate the chip canceling offset errors in the > > > current ADC. This means that, over time, reading the IAD register will not > > > return the correct current measurement, it will have an offset. Writing to > > > the offset register if the two's complement of the current register while > > > passing zero current to the load will calibrate the measurements. This > > > change was tested on real hardware and it was able to calibrate the chip > > > correctly. > > > > > > Signed-off-by: Luiz Sampaio > > > --- > > > Documentation/w1/slaves/w1_ds2438.rst | 11 +- > > > drivers/w1/slaves/w1_ds2438.c | 49 +++ > > > 2 files changed, 59 insertions(+), 1 deletion(-) > > > > In this, and the previous patch, you added new sysfs files, but no > > update to Documentation/ABI/ for them. Please fix that up. > > > > thanks, > > > > greg k-h > > Hello! I'm sorry about some errors, this is my first patch and I'm not sure > about some things in the documentation. I really appreciate the responses and > guidance. No problem, again you should try wrapping your email lines :) > The file I need to add to Documentation/ABI/ is going to be in testing or > stable? And the file I need to create can be called, for instance, > sysfs-driver-w1_ds2438? stable I guess as you know this is what you want to do. And the file name seems right, thanks. greg k-h
Re: [PATCH v4 9/9] w1: ds2438: support for writing to offset register
On Mon, Apr 05, 2021 at 07:50:09AM -0300, Luiz Sampaio wrote: > Added a sysfs entry to support writing to the offset register on page1. > This register is used to calibrate the chip canceling offset errors in the > current ADC. This means that, over time, reading the IAD register will not > return the correct current measurement, it will have an offset. Writing to > the offset register if the two's complement of the current register while > passing zero current to the load will calibrate the measurements. This > change was tested on real hardware and it was able to calibrate the chip > correctly. > > Signed-off-by: Luiz Sampaio > --- > Documentation/w1/slaves/w1_ds2438.rst | 11 +- > drivers/w1/slaves/w1_ds2438.c | 49 +++ > 2 files changed, 59 insertions(+), 1 deletion(-) In this, and the previous patch, you added new sysfs files, but no update to Documentation/ABI/ for them. Please fix that up. thanks, greg k-h
Re: [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements
On Mon, Apr 05, 2021 at 07:50:00AM -0300, Luiz Sampaio wrote: > The following patches aim to make a user able to calibrate the current > measurement of the DS2438. This chip uses a offset register in page1, which > is added to the current register to give the user the current measurement. If > this value is wrong, the user will get an offset current value, even if the > current is zero, for instance. This patch gives support for reading the page1 > registers (including the offset register) and for writing to the offset > register. The DS2438 datasheet shows a calibration routine, and with this > patch, the user can do this quickly by writing the correct value to the > offset register. This patch was tested on real hardware using a power supply > and an electronic load. > Please help to review this series of patches. Please linewrap your text here :(
Re: [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style
On Mon, Apr 05, 2021 at 07:50:06AM -0300, Luiz Sampaio wrote: > Changed the permissions to preferred octal style. > > Signed-off-by: Luiz Sampaio > --- > drivers/w1/slaves/w1_ds2438.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c > index 56e53a748059..ccb06b8c2d78 100644 > --- a/drivers/w1/slaves/w1_ds2438.c > +++ b/drivers/w1/slaves/w1_ds2438.c > @@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject > *kobj, > return ret; > } > > -static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0); > +static BIN_ATTR(iad, 0664, iad_read, iad_write, 0); Why not BIN_ATTR_RW() instead? thanks, greg k-h
Re: [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read
On Mon, Apr 05, 2021 at 07:50:02AM -0300, Luiz Sampaio wrote: > Since there is only one statement inside the if clause, no brackets > are required. > > Signed-off-by: Luiz Sampaio > --- > drivers/w1/slaves/w1_ds2438.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) As this does the same type of fix as the next 3 patches, to the same file, they should be all merged together into 1 patch. thanks, greg k-h
Re: [PATCH v3 0/9] w1: ds2438: adding support for calibration of current measurements
On Sat, Apr 03, 2021 at 01:45:47AM -0300, Luiz Sampaio wrote: > The following patches aim to make a user able to calibrate the current > measurement of the DS2438. This chip uses a offset register in page1, which > is added to the current register to give the user the current measurement. If > this value is wrong, the user will get an offset current value, even if the > current is zero, for instance. This patch gives support for reading the page1 > registers (including the offset register) and for writing to the offset > register. The DS2438 datasheet shows a calibration routine, and with this > patch, the user can do this quickly by writing the correct value to the > offset register. This patch was tested on real hardware using a power supply > and an electronic load. > Please help to review this series of patches. > > Best regards! > Sampaio > --- > Changes in v3: > - I accidentally added a wrong line that would not compile. I'm sorry. Fixed > it. > > Changes in v2: > - Using git send-email to send the patches > - Adding documentation as requested > - Separating the coding style changes in different patches as requested > > Luiz Sampaio (9): > w1: ds2438: fixed a coding style issue > w1: ds2438: fixed a coding style issue > w1: ds2438: fixed a coding style issue > w1: ds2438: fixed a coding style issue > w1: ds2438: fixed a coding style issue > w1: ds2438: fixed a coding style issue You can not have different patches that do different things, yet have identical subject lines. Please make them unique. thanks, greg k-h
Re: [PATCHv2 RESEND] firmware: stratix10-svc: build only on 64-bit ARM
On Sun, Apr 04, 2021 at 10:20:26AM -0500, Dinh Nguyen wrote:
>
>
> On 4/4/21 9:08 AM, Greg KH wrote:
> > On Sun, Apr 04, 2021 at 07:46:09AM -0500, Dinh Nguyen wrote:
> > > From: Krzysztof Kozlowski
> > >
> > > The Stratix10 service layer and RCU drivers are useful only on
> > > Stratix10, so on ARMv8. Compile testing the RCU driver on 32-bit ARM
> > > fails:
> > >
> > >drivers/firmware/stratix10-rsu.c: In function 'rsu_status_callback':
> > >include/linux/compiler_types.h:320:38: error: call to
> > > '__compiletime_assert_179'
> > > declared with attribute error: FIELD_GET: type of reg too small for
> > > mask
> > > _compiletime_assert(condition, msg, __compiletime_assert_,
> > > __COUNTER__)
> > >...
> > >drivers/firmware/stratix10-rsu.c:96:26: note: in expansion of macro
> > > 'FIELD_GET'
> > > priv->status.version = FIELD_GET(RSU_VERSION_MASK,
> > >
> > > Fixes: 4483397b0353 ("ARM: socfpga: drop ARCH_SOCFPGA")
> >
> > Where is this commit id? I don't see it in Linus's tree, is it
> > somewhere else?
> >
>
> It's in the for-next branch in the soc tree. This patch fixes a patch that
> was just recently submitted by Krzysztof.
Then it needs to go through that tree, while you sent it "To:" me, and I
can't do anything with it at the moment :(
thanks,
greg k-h
Re: KASAN: use-after-free Read in cdev_del
On Sun, Apr 04, 2021 at 08:39:34PM +0800, Hillf Danton wrote:
> On Sun, 4 Apr 2021 17:05:17 Hao Sun wrote:
> > Besides, the 'refcount bug in cdev_del' bug still exists too.
>
> Thanks for your report, Hao.
> >
> > Here is the detailed information:
> > commit: 5e46d1b78a03d52306f21f77a4e4a144b6d31486
> > version: Linux 5.12-rc5
> > git tree:upstream
> > kernel config (KASAN not enabled) and reproducing program can be found
> > in the attachment.
> > Report:
> > [ cut here ]
> > refcount_t: underflow; use-after-free.
> > WARNING: CPU: 1 PID: 8923 at lib/refcount.c:28
> > refcount_warn_saturate+0x1cf/0x210 -origin/lib/refcount.c:28
> > Modules linked in:
> > CPU: 1 PID: 8923 Comm: executor Not tainted 5.12.0-rc5+ #8
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > 1.13.0-1ubuntu1.1 04/01/2014
> > RIP: 0010:refcount_warn_saturate+0x1cf/0x210 -origin/lib/refcount.c:28
> > Code: 4f ff ff ff e8 32 fa b5 fe 48 c7 c7 3d f8 f6 86 e8 d6 ab c6 fe
> > c6 05 7c 34 67 04 01 48 c7 c7 68 f8 6d 86 31 c0 e8 81 2e 9d fe <0f> 0b
> > e9 22 ff ff ff e8 05 fa b5 fe 48 c7 c7 3e f8 f6 86 e8 a9 ab
> > RSP: 0018:c90001633c60 EFLAGS: 00010246
> > RAX: 15d08b2e34b77800 RBX: 0003 RCX: 88804c056c80
> > RDX: RSI: RDI:
> > RBP: 0003 R08: 813767aa R09: 0001
> > R10: 0001 R11: 88804c056c80 R12: 888040b7d000
> > R13: 88804c206938 R14: 88804c206900 R15: 888041b18488
> > FS: 022c9940() GS:88807ec0() knlGS:000=
> > 0
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: 7f9f9b122008 CR3: 44b4b000 CR4: 00750ee0
> > PKRU: 5554
> > Call Trace:
> > __refcount_sub_and_test -origin/./include/linux/refcount.h:283 [inline]
> > __refcount_dec_and_test -origin/./include/linux/refcount.h:315 [inline]
> > refcount_dec_and_test -origin/./include/linux/refcount.h:333 [inline]
> > kref_put -origin/./include/linux/kref.h:64 [inline]
> > kobject_put+0x17b/0x180 -origin/lib/kobject.c:753
> > cdev_del+0x4b/0x50 -origin/fs/char_dev.c:597
> > tty_unregister_device+0x99/0xd0 -origin/drivers/tty/tty_io.c:3343
> > gsmld_detach_gsm -origin/drivers/tty/n_gsm.c:2409 [inline]
> > gsmld_close+0x6c/0x140 -origin/drivers/tty/n_gsm.c:2478
> > tty_ldisc_close -origin/drivers/tty/tty_ldisc.c:488 [inline]
> > tty_ldisc_kill -origin/drivers/tty/tty_ldisc.c:636 [inline]
> > tty_ldisc_release+0x1b6/0x400 -origin/drivers/tty/tty_ldisc.c:809
> > tty_release_struct+0x19/0xb0 -origin/drivers/tty/tty_io.c:1714
> > tty_release+0x9ad/0xa00 -origin/drivers/tty/tty_io.c:1885
> > __fput+0x260/0x4e0 -origin/fs/file_table.c:280
> > fput+0x11/0x20 -origin/fs/file_table.c:313
> > task_work_run+0x8e/0x110 -origin/kernel/task_work.c:140
> > tracehook_notify_resume -origin/./include/linux/tracehook.h:189 [inline]
> > exit_to_user_mode_loop -origin/kernel/entry/common.c:174 [inline]
> > exit_to_user_mode_prepare+0x16b/0x1a0 -origin/kernel/entry/common.c:208
> > __syscall_exit_to_user_mode_work -origin/kernel/entry/common.c:290 [inline]
> > syscall_exit_to_user_mode+0x20/0x40 -origin/kernel/entry/common.c:301
> > do_syscall_64+0x45/0x80 -origin/arch/x86/entry/common.c:56
> > entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> No error check on registering tty device - add it.
> And roll back in case of error.
>
> --- x/drivers/tty/n_gsm.c
> +++ y/drivers/tty/n_gsm.c
> @@ -2384,8 +2384,19 @@ static int gsmld_attach_gsm(struct tty_s
> /* Don't register device 0 - this is the control channel and not
> a usable tty interface */
> base = mux_num_to_base(gsm); /* Base for this MUX */
> - for (i = 1; i < NUM_DLCI; i++)
> - tty_register_device(gsm_tty_driver, base + i, NULL);
> + for (i = 1; i < NUM_DLCI; i++) {
> + struct device *dev;
> +
> + dev = tty_register_device(gsm_tty_driver,
> + base + i, NULL);
> + if (IS_ERR(dev)) {
> + for (i--; i >= 1; i--)
> + tty_unregister_device(gsm_tty_driver,
> + base + i);
> + ret = PTR_ERR(dev);
> + return ret;
> + }
> + }
> }
> return ret;
> }
Are you testing these patches somehow? Can you do that and then submit
them correctly?
thanks,
greg k-h
Re: [PATCH] Staging: rtl8192u: ieee80211: fixed a trailing statements of condition.
On Sun, Apr 04, 2021 at 04:26:14PM +0300, dev.dra...@bk.ru wrote:
> From: Dmitrii Wolf
>
> Fixed a coding style issue.
Really?
>
> Signed-off-by: Dmitrii Wolf
> ---
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> index 690b664df8fa..29a6ce20e2bd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> @@ -2048,12 +2048,12 @@ void ieee80211_softmac_xmit(struct ieee80211_txb
> *txb, struct ieee80211_device *
> /* if xmit available, just xmit it immediately, else just insert it to
> the wait queue */
> for (i = 0; i < txb->nr_frags; i++) {
> #ifdef USB_TX_DRIVER_AGGREGATION_ENABLE
> - if ((skb_queue_len(>skb_drv_aggQ[queue_index]) != 0) ||
> + if ((skb_queue_len(>skb_drv_aggQ[queue_index]) != 0)
> #else
> - if ((skb_queue_len(>skb_waitQ[queue_index]) != 0) ||
> + if ((skb_queue_len(>skb_waitQ[queue_index]) != 0)
> #endif
> - (!ieee->check_nic_enough_desc(ieee->dev, queue_index)) || \
> - (ieee->queue_stop)) {
> + || (!ieee->check_nic_enough_desc(ieee->dev, queue_index)) \
> + || (ieee->queue_stop)) {
This feels really wrong.
The || should be on the end of the line, why is checkpatch complaining
about this?
And that '\' is not needed at all :(
thanks,
greg k-h
Re: [PATCH] kpc2000: kpc2000: Removed extra blank line
On Sun, Apr 04, 2021 at 05:49:46PM -0500, David Villasana Jiménez wrote: > Fix checkpatch warning: > CHECK: Please don't use multiple blank lines > > Signed-off-by: David Villasana Jiménez > --- > drivers/staging/kpc2000/kpc2000/pcie.h | 1 - > 1 file changed, 1 deletion(-) Shouldn't the subject line something like: [PATCH] staging: kpc2000: pcie.h: Remove extra blank line ? Please fix up and try again. thanks, greg k-h
Re: Patch "kbuild: Add resolve_btfids clean to root clean target" has been added to the 5.11-stable tree
On Sat, Apr 03, 2021 at 02:56:50PM -0400, Sasha Levin wrote: > This is a note to let you know that I've just added the patch titled > > kbuild: Add resolve_btfids clean to root clean target > > to the 5.11-stable tree which can be found at: > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > The filename of the patch is: > kbuild-add-resolve_btfids-clean-to-root-clean-target.patch > and it can be found in the queue-5.11 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let know about it. This breaks 'make clean' on 5.10.y and 5.11.y, so I'm going to have to drop this. thanks, greg k-h
Re: [PATCH v6 1/4] usb: dwc3: of-simple: bail probe if no dwc3 child node
On Thu, Apr 01, 2021 at 11:36:49PM +0200, Johan Jonker wrote: > For some of the dwc3-of-simple compatible SoCs we > don't want to bind this driver to a dwc3 node, > but bind that node to the 'snps,dwc3' driver instead. > The kernel has no logic to decide which driver to bind > to if there are 2 matching drivers, so bail probe if no > dwc3 child node. > > Signed-off-by: Johan Jonker > --- > drivers/usb/dwc3/dwc3-of-simple.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c > b/drivers/usb/dwc3/dwc3-of-simple.c > index 71fd620c5..8d3baa5df 100644 > --- a/drivers/usb/dwc3/dwc3-of-simple.c > +++ b/drivers/usb/dwc3/dwc3-of-simple.c > @@ -38,6 +38,10 @@ static int dwc3_of_simple_probe(struct platform_device > *pdev) > > int ret; > > + /* Bail probe if no dwc3 child node. */ > + if (!of_get_compatible_child(dev->of_node, "snps,dwc3")) > + return -ENODEV; Why is this part of the "convert to yaml" patch series? Shouldn't this be a separate, independant patch? thanks, greg k-h
Re: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
On Mon, Apr 05, 2021 at 01:52:39AM +, Min Li wrote: > > > > Any specific reason you are not using the misc_device api? That would clean > > up this driver a lot, there's no need to create a whole class just for a > > single > > driver. > > > > Hi Greg > > No specific reason. I just didn't know the existence of misc_register API. Your file is in drivers/misc/ :) > Do you recommend using this API to create the device? Yes. > If yes, can you tell me how to obtain a appropriate MINOR number from > miscdevice.h? No need to reserve one, we don't do that anymore, just ask for a dynamic value and the next availble one will be given to your driver automatically. thanks, greg k-h
Re: [PATCH] staging: qlge: remove else after break
On Mon, Apr 05, 2021 at 06:11:43AM +0530, Mitali Borkar wrote:
> linux-staging@lists,linux-kernel@vger.kernel.org
> Bcc:
> Subject: [PATCH] staging: qlge:remove else after break
> Reply-To:
Very odd, why is this in the body of the email?
>
> Fixed Warning:- else is not needed after break
> break terminates the loop if encountered. else is unnecessary and
> increases indenatation
>
> Signed-off-by: Mitali Borkar
> ---
> drivers/staging/qlge/qlge_mpi.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge_mpi.c b/drivers/staging/qlge/qlge_mpi.c
> index 2630ebf50341..3a49f187203b 100644
> --- a/drivers/staging/qlge/qlge_mpi.c
> +++ b/drivers/staging/qlge/qlge_mpi.c
> @@ -935,13 +935,11 @@ static int qlge_idc_wait(struct qlge_adapter *qdev)
> netif_err(qdev, drv, qdev->ndev, "IDC Success.\n");
> status = 0;
> break;
> - } else {
> - netif_err(qdev, drv, qdev->ndev,
> + } netif_err(qdev, drv, qdev->ndev,
> "IDC: Invalid State 0x%.04x.\n",
> mbcp->mbox_out[0]);
> status = -EIO;
> break;
> - }
Indentation is now no longer correct :(
thanks,
greg k-h
Re: [PATCHv2 RESEND] firmware: stratix10-svc: build only on 64-bit ARM
On Sun, Apr 04, 2021 at 07:46:09AM -0500, Dinh Nguyen wrote:
> From: Krzysztof Kozlowski
>
> The Stratix10 service layer and RCU drivers are useful only on
> Stratix10, so on ARMv8. Compile testing the RCU driver on 32-bit ARM
> fails:
>
> drivers/firmware/stratix10-rsu.c: In function 'rsu_status_callback':
> include/linux/compiler_types.h:320:38: error: call to
> '__compiletime_assert_179'
> declared with attribute error: FIELD_GET: type of reg too small for mask
> _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> ...
> drivers/firmware/stratix10-rsu.c:96:26: note: in expansion of macro
> 'FIELD_GET'
> priv->status.version = FIELD_GET(RSU_VERSION_MASK,
>
> Fixes: 4483397b0353 ("ARM: socfpga: drop ARCH_SOCFPGA")
Where is this commit id? I don't see it in Linus's tree, is it
somewhere else?
thanks,
greg k-h
Re: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
On Sat, Apr 03, 2021 at 06:08:34PM -0400, min.li...@renesas.com wrote: > From: Min Li > > This driver is developed for the IDT ClockMatrix(TM) and 82P33xxx families > of timing and synchronization devices.It will be used by Renesas PTP Clock > Manager for Linux (pcm4l) software to provide support to GNSS assisted > partial timing support (APTS) and other networking timing functions. > > Current version provides kernel API's to support the following functions > -set combomode to enable SYNCE clock support > -read dpll's state to determine if the dpll is locked to the GNSS channel > -read dpll's ffo (fractional frequency offset) in ppqt > > Signed-off-by: Min Li Any specific reason you are not using the misc_device api? That would clean up this driver a lot, there's no need to create a whole class just for a single driver. thanks, greg k-h
Re: [BUG]: usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On Sun, Apr 04, 2021 at 11:29:00AM +0200, H. Nikolaus Schaller wrote:
> it seems as if the patch
>
> 9de47c37 ("usb: dwc3: gadget: Prevent EP queuing while stopping
> transfers") in v5.11.y
> f09ddcfcb8c5 ("usb: dwc3: gadget: Prevent EP queuing while stopping
> transfers") in v5.12-rc5
>
> reproducible breaks dwc3 RNDIS gadget, at least on the Pyra Handheld (OMAP5).
>
> The symptom of having this patch in tree (v5.11.10 or v5.12) is that
> rndis/gadget initially works after boot.
Should be fixed now by 5aef629704ad ("usb: dwc3: gadget: Clear DEP flags
after stop transfers in ep disable"). Can you test and verify this?
thanks,
greg k-h
Re: [PATCH] Staging: rtl8192u: ieee80211: fixed a trailing statements coding style issue.
On Sun, Apr 04, 2021 at 08:15:00AM +0300, dev.dra...@bk.ru wrote: > From: Dmitrii Wolf > > Signed-off-by: Dmitrii Wolf > --- > drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - You did not specify a description of why the patch is needed, or possibly, any description at all, in the email body. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what is needed in order to properly describe the change. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot
Re: [PATCH 0/7] staging: media: zoran: Eliminate camelcase
On Sun, Apr 04, 2021 at 12:08:57AM +0600, Zhansaya Bagdauletkyzy wrote: > This patchset fixes 'avoid camelcase' warning by converting local variables > to lowercase and separating words using '_'. > Renaming of each variable is implemented in separate patches. > > Zhansaya Bagdauletkyzy (7): > Rename 'HEnd' to 'h_end' > Rename 'VEnd' to 'v_end' > Rename 'DispMode' to 'disp_mode' > Rename 'VidWinWid' to 'vid_win_wid' > Rename 'VidWinHt' to 'vid_win_ht' > Rename 'We' to 'we' > Rename 'He' to 'he' > > drivers/staging/media/zoran/zoran_device.c | 48 +++--- > 1 file changed, 24 insertions(+), 24 deletions(-) You did not read the instructions for the outreachy work, sorry, but I can not take these. good luck! greg k-h
Re: [PATCH] staging: rtl8188eu: replace goto with direct return
On Sat, Apr 03, 2021 at 10:40:08PM -0700, Deborah Brouwer wrote: > To conform with Linux kernel coding style, replace goto statement that > does no cleanup with a direct return. To preserve meaning, copy comments > from the original goto statement to the return statement. Identified by > the checkpatch warning: WARNING: void function return statements are not > generally useful. > > Signed-off-by: Deborah Brouwer > --- > drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c > b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c > index 391c59490718..d21f21857c20 100644 > --- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c > +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c > @@ -139,7 +139,9 @@ void rtw_hal_dm_watchdog(struct adapter *Adapter) > hw_init_completed = Adapter->hw_init_completed; > > if (!hw_init_completed) > - goto skip_dm; > + /* Check GPIO to determine current RF on/off and Pbc status. */ > + /* Check Hardware Radio ON/OFF or not */ > + return; It does not make sense to have the comments in two places here. The original code is just fine, there's nothing wrong with the goto statement here. thanks, greg k-h
Re: [PATCH] docs: driver-model: Update the documentation for device class
On Sat, Apr 03, 2021 at 05:30:50PM +0530, Manivannan Sadhasivam wrote: > The current documentation about the device class is out of date such > that it refers to non-existent APIs and structures. This commit updates > them to the current device class APIs and structures, removes wordings > that no longer valid while trying to keep the original content intact. Thanks for working on this! One thing that instantly jumped out at me: > -Class drivers can export attributes using the DEVCLASS_ATTR macro that works > -similarly to the DEVICE_ATTR macro for devices. For example, a definition > +Class drivers can export attributes using the CLASS_ATTR_* macros that works > +similarly to the DEVICE_ATTR_* macros for devices. For example, a definition > like this:: > > - static DEVCLASS_ATTR(debug,0644,show_debug,store_debug); > + static CLASS_ATTR_RW(debug, 0644, show_debug, store_debug); CLASS_ATTR_RW(debug); is the correct way to write the above, what you added there will not build. But a meta-comment, should stuff like this go directly into the .c files itself so that the documentation is created automatically? the fact that this lives so "far away" from the source ensures that it will always be out of date. I know other subsystems (graphics, v4l2) have tied the documentation into their code files much better so I think the build and markup infrastructure is there today to do this. thanks, greg k-h
[GIT PULL] Driver core fix for 5.12-rc6
The following changes since commit 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b: Linux 5.12-rc4 (2021-03-21 14:56:43 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/driver-core-5.12-rc6 for you to fetch changes up to f0acf637d60ffcef3ccb6e279f743e587b3c7359: driver core: clear deferred probe reason on probe retry (2021-03-23 15:13:43 +0100) Driver core fix for 5.12-rc6 Here is a single driver core fix for a reported problem with differed probing. It has been in linux-next for a while with no reported problems. Signed-off-by: Greg Kroah-Hartman Ahmad Fatoum (1): driver core: clear deferred probe reason on probe retry drivers/base/dd.c | 3 +++ 1 file changed, 3 insertions(+)
[GIT PULL] Char/Misc driver fixes for 5.12-rc6
The following changes since commit 1e28eed17697bcf343c6743f0028cc3b5dd88bf0: Linux 5.12-rc3 (2021-03-14 14:41:02 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git tags/char-misc-5.12-rc6 for you to fetch changes up to 3756b6578709c55819742f6ba0c18f93e8901397: Merge tag 'icc-5.12-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/djakov/icc into char-misc-linus (2021-03-27 12:39:18 +0100) Char/Misc driver fixes for 5.12-rc6 Here are a few small driver char/misc changes for 5.12-rc6. Nothing major here, a few fixes for reported issues: - interconnect fixes for problems found - fbcon syzbot-found fix - extcon fixes - firmware stratix10 bugfix - MAINTAINERS file update. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Benjamin Li (1): interconnect: qcom: msm8939: remove rpm-ids from non-RPM nodes Dinghao Liu (1): extcon: Fix error handling in extcon_dev_register Du Cheng (1): drivers: video: fbcon: fix NULL dereference in fbcon_cursor() Georgi Djakov (1): interconnect: Fix kerneldoc warning Greg Kroah-Hartman (3): Merge tag 'extcon-fixes-for-5.12-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon into char-misc-next Merge tag 'fpga-fixes-for-5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/mdf/linux-fpga into char-misc-linus Merge tag 'icc-5.12-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/djakov/icc into char-misc-linus Jia-Ju Bai (1): interconnect: core: fix error return code of icc_link_destroy() Krzysztof Kozlowski (1): extcon: Add stubs for extcon_register_notifier_all() functions Richard Gong (1): firmware: stratix10-svc: reset COMMAND_RECONFIG_FLAG_PARTIAL to 0 Tomas Winkler (1): mei: allow map and unmap of client dma buffer only for disconnected client Vinod Koul (1): MAINTAINERS: Add linux-phy list and patchwork MAINTAINERS| 3 ++- drivers/extcon/extcon.c| 1 + drivers/interconnect/bulk.c| 2 +- drivers/interconnect/core.c| 2 ++ drivers/interconnect/qcom/msm8939.c| 16 +++ drivers/misc/mei/client.c | 17 +++- drivers/video/fbdev/core/fbcon.c | 3 +++ include/linux/extcon.h | 23 ++ .../linux/firmware/intel/stratix10-svc-client.h| 2 +- 9 files changed, 48 insertions(+), 21 deletions(-)
[GIT PULL] Staging driver fix for 5.12-rc6
The following changes since commit 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b: Linux 5.12-rc4 (2021-03-21 14:56:43 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git tags/staging-5.12-rc6 for you to fetch changes up to e78836ae76d20f38eed8c8c67f21db97529949da: staging: rtl8192e: Change state information from u16 to u8 (2021-03-23 13:32:40 +0100) Staging driver fixes for 5.12-rc6 Here are 2 rtl8192e staging driver fixes for reported problems. Both of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Atul Gopinathan (2): staging: rtl8192e: Fix incorrect source in memcpy() staging: rtl8192e: Change state information from u16 to u8 drivers/staging/rtl8192e/rtllib.h| 2 +- drivers/staging/rtl8192e/rtllib_rx.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
[GIT PULL] USB driver fixes for 5.12-rc6
The following changes since commit 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b: Linux 5.12-rc4 (2021-03-21 14:56:43 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git tags/usb-5.12-rc6 for you to fetch changes up to 93f672804bf2d7a49ef3fd96827ea6290ca1841e: usb: dwc2: Prevent core suspend when port connection flag is 0 (2021-03-26 14:51:34 +0100) USB fixes for 5.12-rc6 Here are a few small USB driver fixes for 5.12-rc6 to resolve reported problems. They include: - a number of cdc-acm fixes for reported problems. It seems more people are using this driver lately... - dwc3 driver fixes for reported problems, and fixes for the fixes :) - dwc2 driver fixes for reported issues. - musb driver fix. - new USB quirk additions. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Andy Shevchenko (1): usb: dwc3: pci: Enable dis_uX_susphy_quirk for Intel Merrifield Artur Petrosyan (2): usb: dwc2: Fix HPRT0.PrtSusp bit setting for HiKey 960 board. usb: dwc2: Prevent core suspend when port connection flag is 0 Chunfeng Yun (1): usb: xhci-mtk: fix broken streams issue on 0.96 xHCI Johan Hovold (8): USB: cdc-acm: fix double free on probe failure USB: cdc-acm: fix use-after-free after probe failure USB: cdc-acm: drop redundant driver-data assignment USB: cdc-acm: drop redundant driver-data reset USB: cdc-acm: clean up probe error labels USB: cdc-acm: use negation for NULL checks USB: cdc-acm: always claim data interface USB: cdc-acm: do not log successful probe on later errors Oliver Neukum (3): cdc-acm: fix BREAK rx code path adding necessary calls USB: cdc-acm: untangle a circular dependency between callback and softint USB: cdc-acm: downgrade message to debug Shawn Guo (1): usb: dwc3: qcom: skip interconnect init for ACPI probe Shuah Khan (1): usbip: vhci_hcd fix shift out-of-bounds in vhci_hub_control() Thinh Nguyen (2): usb: dwc3: gadget: Set gadget_max_speed when set ssp_rate usb: dwc3: gadget: Use max speed if unspecified Tong Zhang (1): usb: gadget: udc: amd5536udc_pci fix null-ptr-dereference Tony Lindgren (1): usb: musb: Fix suspend with devices connected for a64 Vincent Palatin (1): USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem Wesley Cheng (1): usb: dwc3: gadget: Clear DEP flags after stop transfers in ep disable drivers/usb/class/cdc-acm.c | 120 +++- drivers/usb/core/quirks.c | 4 ++ drivers/usb/dwc2/hcd.c | 5 +- drivers/usb/dwc3/dwc3-pci.c | 2 + drivers/usb/dwc3/dwc3-qcom.c| 3 + drivers/usb/dwc3/gadget.c | 11 +-- drivers/usb/gadget/udc/amd5536udc_pci.c | 10 +-- drivers/usb/host/xhci-mtk.c | 10 ++- drivers/usb/musb/musb_core.c| 12 ++-- drivers/usb/usbip/vhci_hcd.c| 2 + 10 files changed, 112 insertions(+), 67 deletions(-)
[GIT PULL] Serial driver fix for 5.12-rc6
The following changes since commit 1e28eed17697bcf343c6743f0028cc3b5dd88bf0: Linux 5.12-rc3 (2021-03-14 14:41:02 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tags/tty-5.12-rc6 for you to fetch changes up to 29d96eb261345c8d888e248ae79484e681be2faa: soc: qcom-geni-se: Cleanup the code to remove proxy votes (2021-03-26 15:16:05 +0100) Serial driver fix for 5.12-rc6 Here is a single serial driver fix for 5.12-rc6. Is is a revert of a change that showed up in 5.9 that has been reported to cause problems. It has been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Roja Rani Yarubandi (1): soc: qcom-geni-se: Cleanup the code to remove proxy votes drivers/soc/qcom/qcom-geni-se.c | 74 --- drivers/tty/serial/qcom_geni_serial.c | 7 include/linux/qcom-geni-se.h | 2 - 3 files changed, 83 deletions(-)
Re: [PATCH] platform/chrome: Update cros_ec sysfs attributes on sensors discovery
On Fri, Apr 02, 2021 at 11:20:31PM -0700, Gwendal Grignou wrote:
> When cros_ec_sysfs probe is called before cros_ec_sensorhub probe
> routine, the |kb_wake_angle| attribute will not be displayed, even if
> there are two accelerometers in the chromebook.
>
> Call sysfs_update_group() when accelerometers are enumerated if the
> cros_ec sysfs attributes group has already been created.
>
> Fixes: d60ac88a62df ("mfd / platform / iio: cros_ec: Register sensor through
> sensorhub")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Gwendal Grignou
> ---
> drivers/platform/chrome/cros_ec_sensorhub.c | 5 -
> drivers/platform/chrome/cros_ec_sysfs.c | 2 ++
> include/linux/platform_data/cros_ec_proto.h | 2 ++
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c
> b/drivers/platform/chrome/cros_ec_sensorhub.c
> index 9c4af76a9956e..78085ad362ca8 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> @@ -106,8 +106,11 @@ static int cros_ec_sensorhub_register(struct device *dev,
> sensor_type[sensorhub->resp->info.type]++;
> }
>
> - if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2)
> + if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
> ec->has_kb_wake_angle = true;
> + if (ec->group)
> + sysfs_update_group(>class_dev.kobj, ec->group);
> + }
>
> if (cros_ec_check_features(ec,
> EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS)) {
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c
> b/drivers/platform/chrome/cros_ec_sysfs.c
> index f07eabcf9494c..3838d5f51aadc 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -344,6 +344,8 @@ static int cros_ec_sysfs_probe(struct platform_device *pd)
> ret = sysfs_create_group(_dev->class_dev.kobj, _ec_attr_group);
This is odd, the platform/driver core should be creating these files,
you should never have to do this "by hand". If you do it this way, you
are racing with userspace and loosing.
Please set the dev_groups field in the driver structure to be this group
and then all will be fine automatically.
> if (ret < 0)
> dev_err(dev, "failed to create attributes. err=%d\n", ret);
> + else
> + ec_dev->group = _ec_attr_group;
>
> return ret;
> }
> diff --git a/include/linux/platform_data/cros_ec_proto.h
> b/include/linux/platform_data/cros_ec_proto.h
> index 02599687770c5..4cd06f68bc536 100644
> --- a/include/linux/platform_data/cros_ec_proto.h
> +++ b/include/linux/platform_data/cros_ec_proto.h
> @@ -191,6 +191,7 @@ struct cros_ec_platform {
> /**
> * struct cros_ec_dev - ChromeOS EC device entry point.
> * @class_dev: Device structure used in sysfs.
> + * @group: sysfs attributes group for this EC.
> * @ec_dev: cros_ec_device structure to talk to the physical device.
> * @dev: Pointer to the platform device.
> * @debug_info: cros_ec_debugfs structure for debugging information.
> @@ -200,6 +201,7 @@ struct cros_ec_platform {
> */
> struct cros_ec_dev {
> struct device class_dev;
> + struct attribute_group *group;
This should not be needed.
thanks,
greg k-h
Re: [PATCH v2 01/30] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_xmit.c
On Sat, Apr 03, 2021 at 09:48:06AM +0200, Fabio Aiuto wrote: > On Sat, Apr 03, 2021 at 09:40:08AM +0200, Greg KH wrote: > > On Fri, Apr 02, 2021 at 07:29:43PM +0200, Fabio Aiuto wrote: > > > remove all RT_TRACE logs > > > > > > > I don't mean to be a pain, but this changelog text needs some work. > > > > This says _what_ it does, but not _why_ you are doing this. The kernel > > documentation has a section on how to write a good changelog text, you > > might want to look at that. > > you are right, I spent time writing the cover, but not the changelog which > will remain in kernel history Take portions of what you write in the cover letter, into the changelog for the patches. Or the other way around is also good, depending on which you write first :) thanks, greg k-h
Re: [PATCH v2 01/30] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_xmit.c
On Fri, Apr 02, 2021 at 07:29:43PM +0200, Fabio Aiuto wrote: > remove all RT_TRACE logs > I don't mean to be a pain, but this changelog text needs some work. This says _what_ it does, but not _why_ you are doing this. The kernel documentation has a section on how to write a good changelog text, you might want to look at that. For this type of series, this could be as simple as: Remove all of the RT_TRACE_LOGs in the rtx_xmit.c file as they currently do nothing as they require the code to be modified by hand in order to be turned on. This obviously has not happened since the code was merged, so just remove them as they are unused. Or something like that. Most of the time, writing the changelog can take more work than the actual code change itself, but it's important as we need to know what is happening both for the reviewers, as well as people in the future who might have to look back and try to understand the reason for specific changes. Can you fix up this series based on this and resend? thanks, greg k-h
Re: [PATCH] firewire: nosy: Fix a use-after-free bug in nosy_ioctl()
On Sat, Apr 03, 2021 at 06:58:36AM +, Zheyu Ma wrote: > For each device, the nosy driver allocates a pcilynx structure. > A use-after-free might happen in the following scenario: > > 1. Open nosy device for the first time and call ioctl with command > NOSY_IOC_START, then a new client A will be malloced and added > to doubly linked list. > 2. Open nosy device for the second time and call ioctl with command > NOSY_IOC_START, then a new client B will be malloced and added > to doubly linked list. > 3. Call ioctl with command NOSY_IOC_START for client A, then client A > will be readded to the doubly linked list. Now the doubly linked > list is messed up. > 4. Close the first nosy device and nosy_release will be called. > In nosy_release, client A will be unlinked and freed. > 5. Close the second nosy device, and client A will be referenced, > resulting in UAF. > > The root cause of this bug is that the element in the doubly linked list > is reentered into the list. > Fix this bug by adding a check before inserting a client. If a client > is already in the linked list, don't insert it. > > The following KASAN report reveals it: > > [ 14.672676 ] BUG: KASAN: use-after-free in nosy_release+0x1ea/0x210 > [ 14.673113 ] Write of size 8 at addr 888102ad7360 by task poc > [ 14.673609 ] CPU: 3 PID: 337 Comm: poc Not tainted 5.12.0-rc5+ #6 > [ 14.673988 ] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), > BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > [ 14.674686 ] Call Trace: > [ 14.674843 ] dump_stack+0x8a/0xb5 > [ 14.675061 ] print_address_description.constprop.0+0x18/0x130 > [ 14.675428 ] ? nosy_release+0x1ea/0x210 > [ 14.675676 ] ? nosy_release+0x1ea/0x210 > [ 14.675916 ] kasan_report.cold+0x7f/0x111 > [ 14.676169 ] ? nosy_release+0x1ea/0x210 > [ 14.676409 ] nosy_release+0x1ea/0x210 > [ 14.676642 ] __fput+0x1e2/0x840 > [ 14.676844 ] task_work_run+0xe8/0x180 > [ 14.677083 ] exit_to_user_mode_prepare+0x114/0x120 > [ 14.677388 ] syscall_exit_to_user_mode+0x1d/0x40 > [ 14.677678 ] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 14.677995 ] RIP: 0033:0x7fc5a8666f30 > [ 14.678229 ] Code: 00 64 c7 00 0d 00 00 00 b8 ff ff ff ff eb 90 > b8 ff ff ff ff eb 89 0f 1f 40 00 83 3d d9 27 2c 00 00 75 10 b8 03 > 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e84 > [ 14.679385 ] RSP: 002b:7ffe9e94cd68 EFLAGS: 0246 > ORIG_RAX: 0003 > [ 14.679862 ] RAX: RBX: > RCX: 7fc5a8666f30 > [ 14.680301 ] RDX: 7ffe9e94ce78 RSI: 2601 > RDI: 0004 > [ 14.680743 ] RBP: 7ffe9e94cd80 R08: 564727400850 > R09: 7fc5a8939ba0 > [ 14.681180 ] R10: 0692 R11: 0246 > R12: 564727400610 > [ 14.681624 ] R13: 7ffe9e94ce60 R14: > R15: > [ 14.682072 ] > [ 14.682168 ] Allocated by task 337: > [ 14.682387 ] kasan_save_stack+0x1b/0x40 > [ 14.682633 ] __kasan_kmalloc+0x7a/0x90 > [ 14.682868 ] nosy_open+0x154/0x4d0 > [ 14.683089 ] misc_open+0x2ec/0x410 > [ 14.683313 ] chrdev_open+0x20d/0x5a0 > [ 14.683541 ] do_dentry_open+0x40f/0xe80 > [ 14.683787 ] path_openat+0x1cf9/0x37b0 > [ 14.684025 ] do_filp_open+0x16d/0x390 > [ 14.684253 ] do_sys_openat2+0x11d/0x360 > [ 14.684497 ] __x64_sys_open+0xfd/0x1a0 > [ 14.684736 ] do_syscall_64+0x33/0x40 > [ 14.684964 ] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 14.685283 ] > [ 14.685384 ] Freed by task 337: > [ 14.685580 ] kasan_save_stack+0x1b/0x40 > [ 14.685822 ] kasan_set_track+0x1c/0x30 > [ 14.686062 ] kasan_set_free_info+0x20/0x30 > [ 14.686324 ] __kasan_slab_free+0xe5/0x110 > [ 14.686581 ] kfree+0x8f/0x210 > [ 14.686775 ] nosy_release+0x158/0x210 > [ 14.687011 ] __fput+0x1e2/0x840 > [ 14.687213 ] task_work_run+0xe8/0x180 > [ 14.687449 ] exit_to_user_mode_prepare+0x114/0x120 > [ 14.687750 ] syscall_exit_to_user_mode+0x1d/0x40 > [ 14.688040 ] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 14.688356 ] > [ 14.688454 ] The buggy address belongs to the object at > 888102ad7300 > [ 14.688454 ] which belongs to the cache kmalloc-128 of size 128 > [ 14.689232 ] The buggy address is located 96 bytes inside of > [ 14.689232 ] 128-byte region [888102ad7300, 888102ad7380) > [ 14.689955 ] The buggy address belongs to the page: > [ 14.690258 ] page:46ca3dc1 refcount:1 mapcount:0 > mapping: index:0x888102ad7100 pfn:0x102ad6 > [ 14.690917 ] head:46ca3dc1 order:1 compound_mapcount:0 > [ 14.691278 ] flags: 0x2010200(slab|head)
Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate
On Fri, Apr 02, 2021 at 06:30:16PM +, Luis Chamberlain wrote: > On Fri, Apr 02, 2021 at 09:54:12AM +0200, Greg KH wrote: > > On Thu, Apr 01, 2021 at 11:59:25PM +, Luis Chamberlain wrote: > > > As for the syfs deadlock possible with drivers, this fixes it in a > > > generic way: > > > > > > commit fac43d8025727a74f80a183cc5eb74ed902a5d14 > > > Author: Luis Chamberlain > > > Date: Sat Mar 27 14:58:15 2021 + > > > > > > sysfs: add optional module_owner to attribute > > > > > > This is needed as otherwise the owner of the attribute > > > or group read/store might have a shared lock used on driver removal, > > > and deadlock if we race with driver removal. > > > > > > Signed-off-by: Luis Chamberlain > > > > No, please no. Module removal is a "best effort", > > Not for live patching. I am not sure if I am missing any other valid > use case? live patching removes modules? We have so many code paths that are "best effort" when it comes to module unloading, trying to resolve this one is a valiant try, but not realistic. > > if the system dies when it happens, that's on you. > > I think the better approach for now is simply to call testers / etc to > deal with this open coded. I cannot be sure that other than live > patching there may be other valid use cases for module removal, and for > races we really may care for where userspace *will* typically be mucking > with sysfs attributes. Monitoring my systems's sysfs attributes I am > actually quite surprised at the random pokes at them. > > > I am not willing to expend extra energy > > and maintance of core things like sysfs for stuff like this that does > > not matter in any system other than a developer's box. > > Should we document this as well? Without this it is unclear that tons of > random tests are sanely nullified. At least this dead lock I spotted can > be pretty common form on many drivers. What other drivers have this problem? > > Lock data, not code please. Trying to tie data structure's lifespans > > to the lifespan of code is a tangled mess, and one that I do not want to > > add to in any form. > > Driver developers will simply have to open code these protections. In > light of what I see on LTP / fuzzing, I suspect the use case will grow > and we'll have to revisit this in the future. But for now, sure, we can > just open code the required protections everywhere to not crash on module > removal. LTP and fuzzing too do not remove modules. So I do not understand the root problem here, that's just something that does not happen on a real system. thanks, greg k-h
Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver
On Fri, Apr 02, 2021 at 05:41:01PM +0200, Loic Poulain wrote: > On Fri, 2 Apr 2021 at 16:05, Greg KH wrote: > > > > On Fri, Apr 02, 2021 at 04:06:37PM +0200, Loic Poulain wrote: > > > The MHI WWWAN control driver allows MHI QCOM-based modems to expose > > > different modem control protocols/ports via the WWAN framework, so that > > > userspace modem tools or daemon (e.g. ModemManager) can control WWAN > > > config and state (APN config, SMS, provider selection...). A QCOM-based > > > modem can expose one or several of the following protocols: > > > - AT: Well known AT commands interactive protocol (microcom, minicom...) > > > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli) > > > - QMI: QCOM MSM/Modem Interface (libqmi, qmicli) > > > - QCDM: QCOM Modem diagnostic interface (libqcdm) > > > - FIREHOSE: XML-based protocol for Modem firmware management > > > (qmi-firmware-update) > > > > > > Note that this patch is mostly a rework of the earlier MHI UCI > > > tentative that was a generic interface for accessing MHI bus from > > > userspace. As suggested, this new version is WWAN specific and is > > > dedicated to only expose channels used for controlling a modem, and > > > for which related opensource userpace support exist. > > > > > > Signed-off-by: Loic Poulain > > > --- > > > v2: update copyright (2021) > > > v3: Move driver to dedicated drivers/net/wwan directory > > > v4: Rework to use wwan framework instead of self cdev management > > > v5: Fix errors/typos in Kconfig > > > v6: - Move to new wwan interface, No need dedicated call to > > > wwan_dev_create > > > - Cleanup code (remove legacy from mhi_uci, unused defines/vars...) > > > - Remove useless write_lock mutex > > > - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq > > > helpers > > > - Rework locking > > > - Add MHI_WWAN_TX_FULL flag > > > - Add support for NONBLOCK read/write > > > v7: Fix change log (mixed up 1/2 and 2/2) > > > v8: - Implement wwan_port_ops (instead of fops) > > > - Remove all mhi wwan data obsolete members (kref, lock, waitqueues) > > > - Add tracking of RX buffer budget > > > - Use WWAN TX flow control function to stop TX when MHI queue is full > > > > > > drivers/net/wwan/Kconfig | 14 +++ > > > drivers/net/wwan/Makefile| 2 + > > > drivers/net/wwan/mhi_wwan_ctrl.c | 253 > > > +++ > > > 3 files changed, 269 insertions(+) > > > create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c > > > > > > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig > > > index 545fe54..ce0bbfb 100644 > > > --- a/drivers/net/wwan/Kconfig > > > +++ b/drivers/net/wwan/Kconfig > > > @@ -19,4 +19,18 @@ config WWAN_CORE > > > To compile this driver as a module, choose M here: the module > > > will be > > > called wwan. > > > > > > +config MHI_WWAN_CTRL > > > + tristate "MHI WWAN control driver for QCOM-based PCIe modems" > > > + select WWAN_CORE > > > + depends on MHI_BUS > > > + help > > > + MHI WWAN CTRL allows QCOM-based PCIe modems to expose different > > > modem > > > + control protocols/ports to userspace, including AT, MBIM, QMI, > > > DIAG > > > + and FIREHOSE. These protocols can be accessed directly from > > > userspace > > > + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi, > > > + libqcdm...). > > > + > > > + To compile this driver as a module, choose M here: the module > > > will be > > > + called mhi_wwan_ctrl > > > + > > > endif # WWAN > > > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile > > > index 934590b..556cd90 100644 > > > --- a/drivers/net/wwan/Makefile > > > +++ b/drivers/net/wwan/Makefile > > > @@ -5,3 +5,5 @@ > > > > > > obj-$(CONFIG_WWAN_CORE) += wwan.o > > > wwan-objs += wwan_core.o > > > + > > > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o > > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c > > > b/drivers/net/wwan/mhi_wwan_ctrl.c > > > new file mode 100644 > > > index 000..f2fab23 > > > --- /dev/null > > > +++ b/d
Re: [PATCH] zero-fill colormap in drivers/video/fbdev/core/fbcmap.c
On Wed, Mar 31, 2021 at 11:07:19PM +0100, Phillip Potter wrote: > Use kzalloc() rather than kmalloc() for the dynamically allocated parts > of the colormap in fb_alloc_cmap_gfp, to prevent a leak of random kernel > data to userspace under certain circumstances. > > Fixes a KMSAN-found infoleak bug reported by syzbot at: > https://syzkaller.appspot.com/bug?id=741578659feabd108ad9e06696f0c1f2e69c4b6e > > Reported-by: syzbot+47fa9c9c648b76530...@syzkaller.appspotmail.com > Signed-off-by: Phillip Potter > --- > drivers/video/fbdev/core/fbcmap.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Daniel, want me to take this? thanks, greg k-h
Re: [PATCH] drivers core: don't do anything in device_del() when device_add() fail
On Thu, Apr 01, 2021 at 09:01:38AM -0400, Yufen Yu wrote: > Recently, our syzbot test reported NULL pointer dereference in > device_del() by injecting memory allocation fail in device_add(). Don't do that :) > For now, callers of device_add(), such as add_disk(), may ignore > device_add()'s fail and go on working. Please fix up those users, if device_add() fails there is no need to call device_del(). > In unregister path, it will > call device_del() from del_gendisk(). That can cause various NULL > pointer dereference, including dev->p is NULL in kill_device(), > 'name' is NULL in sysfs_remove_link(), kobj->sd is 'NULL' when call > dpm_sysfs_remove(), and so on. > > To avoid these kernel panic, we call device_del() only when device_add() > is success. As Rafael said, that's not what you are doing here, so even if I wanted to take this patch, I can't. > > Signed-off-by: Yufen Yu > --- > drivers/base/core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index f29839382f81..a10ec5dbc577 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -3380,6 +3380,9 @@ void device_del(struct device *dev) > struct class_interface *class_intf; > unsigned int noio_flag; > > + if (!dev->p) > + return; Also, this isn't a good test as it's not obvious what you are trying to check for here. thanks, greg k-h
Re: [PATCH] firmware: stratix10-svc: build only on 64-bit ARM
On Thu, Apr 01, 2021 at 10:13:29AM -0500, Dinh Nguyen wrote: > From: Krzysztof Kozlowski > > The Stratix10 service layer and RCU drivers are useful only on > Stratix10, so on ARMv8. Compile testing the RCU driver on 32-bit ARM > fails: > > drivers/firmware/stratix10-rsu.c: In function 'rsu_status_callback': > include/linux/compiler_types.h:320:38: error: call to > '__compiletime_assert_179' > declared with attribute error: FIELD_GET: type of reg too small for mask > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > ... > drivers/firmware/stratix10-rsu.c:96:26: note: in expansion of macro > 'FIELD_GET' > priv->status.version = FIELD_GET(RSU_VERSION_MASK, > > Signed-off-by: Krzysztof Kozlowski > Reported-by: kernel test robot > Acked-by: Richard Gong > Signed-off-by: Dinh Nguyen > --- > drivers/firmware/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) What commit caused this error to happen? Can you resend this with a "Fixes:" tag with that information in it? thanks, greg k-h
Re: [PATCH] firmware: qcom-scm: Fix QCOM_SCM configuration
On Wed, Mar 31, 2021 at 02:49:41AM -0400, He Ying wrote: > When CONFIG_QCOM_SCM is y while CONFIG_HAVE_ARM_SMCCC > is not set, compiling errors are encountered as follows: > > drivers/firmware/qcom_scm-smc.o: In function `__scm_smc_do_quirk': > qcom_scm-smc.c:(.text+0x36): undefined reference to `__arm_smccc_smc' > drivers/firmware/qcom_scm-legacy.o: In function `scm_legacy_call': > qcom_scm-legacy.c:(.text+0xe2): undefined reference to `__arm_smccc_smc' > drivers/firmware/qcom_scm-legacy.o: In function `scm_legacy_call_atomic': > qcom_scm-legacy.c:(.text+0x1f0): undefined reference to `__arm_smccc_smc' > > So add dependency on HAVE_ARM_SMCCC in QCOM_SCM configuration. > > Reported-by: Hulk Robot > Signed-off-by: He Ying > --- > drivers/firmware/Kconfig | 1 + > 1 file changed, 1 insertion(+) What commit caused this problem to show up? Please add a "Fixes:" tag in here and resend. thanks, greg k-h
Re: [PATCH next 2/2] misc: Add Renesas Synchronization Management Unit (SMU) support
On Mon, Mar 29, 2021 at 05:03:53PM +, Min Li wrote: > > > > Where is patch 1/2 of this series? > > > > Also, please fix up the errors that the testing bot found, and properly > > version > > your patch submission so I know which one is the "latest" one to look at. > > > > Hi Greg > > The first patch is mfd so I was not sure if I should send that to you guys in > the first place. Then the patches are independant and they should be sent as such, otherwise it causes confusion and our tools get messed up when trying to grab the whole "series" of patches. Can you please fix this up and just send two independant patches? thanks, greg k-h
Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver
On Fri, Apr 02, 2021 at 04:06:37PM +0200, Loic Poulain wrote:
> The MHI WWWAN control driver allows MHI QCOM-based modems to expose
> different modem control protocols/ports via the WWAN framework, so that
> userspace modem tools or daemon (e.g. ModemManager) can control WWAN
> config and state (APN config, SMS, provider selection...). A QCOM-based
> modem can expose one or several of the following protocols:
> - AT: Well known AT commands interactive protocol (microcom, minicom...)
> - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> - QMI: QCOM MSM/Modem Interface (libqmi, qmicli)
> - QCDM: QCOM Modem diagnostic interface (libqcdm)
> - FIREHOSE: XML-based protocol for Modem firmware management
> (qmi-firmware-update)
>
> Note that this patch is mostly a rework of the earlier MHI UCI
> tentative that was a generic interface for accessing MHI bus from
> userspace. As suggested, this new version is WWAN specific and is
> dedicated to only expose channels used for controlling a modem, and
> for which related opensource userpace support exist.
>
> Signed-off-by: Loic Poulain
> ---
> v2: update copyright (2021)
> v3: Move driver to dedicated drivers/net/wwan directory
> v4: Rework to use wwan framework instead of self cdev management
> v5: Fix errors/typos in Kconfig
> v6: - Move to new wwan interface, No need dedicated call to wwan_dev_create
> - Cleanup code (remove legacy from mhi_uci, unused defines/vars...)
> - Remove useless write_lock mutex
> - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq helpers
> - Rework locking
> - Add MHI_WWAN_TX_FULL flag
> - Add support for NONBLOCK read/write
> v7: Fix change log (mixed up 1/2 and 2/2)
> v8: - Implement wwan_port_ops (instead of fops)
> - Remove all mhi wwan data obsolete members (kref, lock, waitqueues)
> - Add tracking of RX buffer budget
> - Use WWAN TX flow control function to stop TX when MHI queue is full
>
> drivers/net/wwan/Kconfig | 14 +++
> drivers/net/wwan/Makefile| 2 +
> drivers/net/wwan/mhi_wwan_ctrl.c | 253
> +++
> 3 files changed, 269 insertions(+)
> create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
>
> diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> index 545fe54..ce0bbfb 100644
> --- a/drivers/net/wwan/Kconfig
> +++ b/drivers/net/wwan/Kconfig
> @@ -19,4 +19,18 @@ config WWAN_CORE
> To compile this driver as a module, choose M here: the module will be
> called wwan.
>
> +config MHI_WWAN_CTRL
> + tristate "MHI WWAN control driver for QCOM-based PCIe modems"
> + select WWAN_CORE
> + depends on MHI_BUS
> + help
> + MHI WWAN CTRL allows QCOM-based PCIe modems to expose different modem
> + control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> + and FIREHOSE. These protocols can be accessed directly from userspace
> + (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> + libqcdm...).
> +
> + To compile this driver as a module, choose M here: the module will be
> + called mhi_wwan_ctrl
> +
> endif # WWAN
> diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> index 934590b..556cd90 100644
> --- a/drivers/net/wwan/Makefile
> +++ b/drivers/net/wwan/Makefile
> @@ -5,3 +5,5 @@
>
> obj-$(CONFIG_WWAN_CORE) += wwan.o
> wwan-objs += wwan_core.o
> +
> +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c
> b/drivers/net/wwan/mhi_wwan_ctrl.c
> new file mode 100644
> index 000..f2fab23
> --- /dev/null
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2021, Linaro Ltd */
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* MHI wwan flags */
> +#define MHI_WWAN_DL_CAP BIT(0)
> +#define MHI_WWAN_UL_CAP BIT(1)
> +#define MHI_WWAN_STARTED BIT(2)
> +
> +#define MHI_WWAN_MAX_MTU 0x8000
> +
> +struct mhi_wwan_dev {
> + /* Lower level is a mhi dev, upper level is a wwan port */
> + struct mhi_device *mhi_dev;
> + struct wwan_port *wwan_port;
> +
> + /* State and capabilities */
> + unsigned long flags;
> + size_t mtu;
> +
> + /* Protect against concurrent TX and TX-completion (bh) */
> + spinlock_t tx_lock;
> +
> + struct work_struct rx_refill;
> + atomic_t rx_budget;
Why is this atomic if you have a real lock already?
> +};
> +
> +static bool mhi_wwan_ctrl_refill_needed(struct mhi_wwan_dev *mhiwwan)
> +{
> + if (!test_bit(MHI_WWAN_STARTED, >flags))
> + return false;
> +
> + if (!test_bit(MHI_WWAN_DL_CAP, >flags))
> + return false;
What prevents these bits from being changed right after reading them?
> +
> + if (!atomic_read(>rx_budget))
> + return false;
Why is this atomic? What happens if it changes right
Re: [PATCH net-next v8 1/2] net: Add a WWAN subsystem
On Fri, Apr 02, 2021 at 04:06:36PM +0200, Loic Poulain wrote:
> This change introduces initial support for a WWAN framework. Given the
> complexity and heterogeneity of existing WWAN hardwares and interfaces,
> there is no strict definition of what a WWAN device is and how it should
> be represented. It's often a collection of multiple devices that perform
> the global WWAN feature (netdev, tty, chardev, etc).
>
> One usual way to expose modem controls and configuration is via high
> level protocols such as the well known AT command protocol, MBIM or
> QMI. The USB modems started to expose that as character devices, and
> user daemons such as ModemManager learnt how to deal with them. This
> initial version adds the concept of WWAN port, which can be created
> by any driver to expose one of these protocols. The WWAN core takes
> care of the generic part, including character device management, and
> rely on port operations to received and submit protocol data.
>
> Since the different components/devices do no necesserarly know about
> each others, and can be created/removed in different orders, the
> WWAN core ensures that all WAN ports that contribute to the 'whole'
> WWAN feature are grouped under the same virtual WWAN device, relying
> on the provided parent device (e.g. mhi controller, USB device). It's
> a 'trick' I copied from Johannes's earlier WWAN subsystem proposal.
>
> This initial version is purposely minimalist, it's essentially moving
> the generic part of the previously proposed mhi_wwan_ctrl driver inside
> a common WWAN framework, but the implementation is open and flexible
> enough to allow extension for further drivers.
>
> Signed-off-by: Loic Poulain
Always run checkpatch before sending stuff off :(
Anyway, one thing did stand out:
> --- /dev/null
> +++ b/include/linux/wwan.h
> @@ -0,0 +1,127 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (c) 2021, Linaro Ltd */
> +
> +#ifndef __WWAN_H
> +#define __WWAN_H
> +
> +#include
> +#include
> +#include
> +
> +/**
> + * enum wwan_port_type - WWAN port types
> + * @WWAN_PORT_AT: AT commands
> + * @WWAN_PORT_MBIM: Mobile Broadband Interface Model control
> + * @WWAN_PORT_QMI: Qcom modem/MSM interface for modem control
> + * @WWAN_PORT_QCDM: Qcom Modem diagnostic interface
> + * @WWAN_PORT_FIREHOSE: XML based command protocol
> + * @WWAN_PORT_MAX
> + */
> +enum wwan_port_type {
> + WWAN_PORT_AT,
> + WWAN_PORT_MBIM,
> + WWAN_PORT_QMI,
> + WWAN_PORT_QCDM,
> + WWAN_PORT_FIREHOSE,
> + WWAN_PORT_MAX,
> +};
> +
> +/**
> + * struct wwan_port - The structure that defines a WWAN port
> + * @type: Port type
> + * @start_count: Port start counter
> + * @flags: Store port state and capabilities
> + * @ops: Pointer to WWAN port operations
> + * @ops_lock: Protect port ops
> + * @dev: Underlying device
> + * @rxq: Buffer inbound queue
> + * @waitqueue: The waitqueue for port fops (read/write/poll)
> + */
> +struct wwan_port {
> + enum wwan_port_type type;
> + unsigned int start_count;
> + unsigned long flags;
> + const struct wwan_port_ops *ops;
> + struct mutex ops_lock;
> + struct device dev;
> + struct sk_buff_head rxq;
> + wait_queue_head_t waitqueue;
> +};
No need to put the actual definition of struct wwan_port in this .h
file, keep it private in your .c file to keep wwan drivers from poking
around in it where they shouldn't be :)
thanks,
greg k-h
Re: [PATCH] USB:ohci:fix ohci interruption problem
On Fri, Apr 02, 2021 at 05:27:59PM +0800, Longfang Liu wrote: > The operating method of the system entering S4 sleep mode: > echo disk > /sys/power/state > > When OHCI enters the S4 sleep state, the USB sleep process will call > check_root_hub_suspend() and ohci_bus_suspend() instead of > ohci_suspend() and ohci_bus_suspend(), this causes the OHCI interrupt > to not be closed. > > At this time, if just one device interrupt is reported. Since rh_state > has been changed to OHCI_RH_SUSPENDED after ohci_bus_suspend(), the > driver will not process and close this device interrupt. It will cause > the entire system to be stuck during sleep, causing the device to > fail to respond. > > When the abnormal interruption reaches 100,000 times, the system will > forcibly close the interruption and make the device unusable. > > Because the root cause of the problem is that ohci_suspend is not > called to perform normal interrupt shutdown operations when the system > enters S4 sleep mode. > > Therefore, our solution is to specify freeze interface in this mode to > perform normal suspend_common() operations, and call ohci_suspend() > after check_root_hub_suspend() is executed through the suspend_common() > operation. > After using this solution, it is verified by the stress test of sleep > wake up in S4 mode for a long time that this problem no longer occurs. > > Signed-off-by: Longfang Liu > --- > drivers/usb/core/hcd-pci.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) What changed from the previous version sent for this patch? Always properly describe the changes below the --- line, and also version your subject line as documented. Please fix up and resend. thanks, greg k-h
Re: [PATCH -next] staging/speakup: Switch to kmemdup_nul()
On Fri, Apr 02, 2021 at 05:21:11PM +0800, Yang Yingliang wrote: > Use kmemdup_nul() helper instead of open-coding to > simplify the code. > > Reported-by: Hulk Robot > Signed-off-by: Yang Yingliang > --- > drivers/accessibility/speakup/i18n.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Your subject line is very odd, why does it have "staging"? Your robot is not doing well these days... greg k-h
Re: [PATCH 01/16] staging: rtl8723bs: remove RT_TRACE logs in core/rtw_xmit.c
On Fri, Apr 02, 2021 at 02:40:46PM +0200, Fabio Aiuto wrote:
> On Fri, Apr 02, 2021 at 02:56:26PM +0300, Dan Carpenter wrote:
> > On Fri, Apr 02, 2021 at 12:01:21PM +0200, Fabio Aiuto wrote:
> > > @@ -568,20 +561,11 @@ static s32 update_attrib_sec_info(struct adapter
> > > *padapter, struct pkt_attrib *p
> > > if (pattrib->encrypt > 0)
> > > memcpy(pattrib->dot118021x_UncstKey.skey,
> > > psta->dot118021x_UncstKey.skey, 16);
> > >
> > > - RT_TRACE(_module_rtl871x_xmit_c_, _drv_info_,
> > > - ("update_attrib: encrypt =%d securitypriv.sw_encrypt =%d\n",
> > > - pattrib->encrypt, padapter->securitypriv.sw_encrypt));
> > > -
> > > if (pattrib->encrypt &&
> > > - ((padapter->securitypriv.sw_encrypt == true) ||
> > > (psecuritypriv->hw_decrypted == false))) {
> > > + ((padapter->securitypriv.sw_encrypt) ||
> > > (!psecuritypriv->hw_decrypted)))
> >
> > You've done too much clean up here. Just remove the { but leave the
> > == true/false comparisons.
> >
> > If the patch is only changing five lines or code then fixing checkpatch
> > warnings on the line of code you are changing is fine, but in this case
> > you're doing a bunch of changes and these sort of cleanups make it hard
> > to review.
> >
> > Ease to spot that the curly brace changed:
> > - ((padapter->securitypriv.sw_encrypt == true) ||
> > (psecuritypriv->hw_decrypted == false))) {
> > + ((padapter->securitypriv.sw_encrypt == true) ||
> > (psecuritypriv->hw_decrypted == false)))
> >
> > Hard to spot:
> > - ((padapter->securitypriv.sw_encrypt == true) ||
> > (psecuritypriv->hw_decrypted == false))) {
> > + ((padapter->securitypriv.sw_encrypt) ||
> > (!psecuritypriv->hw_decrypted)))
> >
> > regards,
> > dan carpenter
> >
>
> thank you Dan, it's a good tuning process for me. Shall I resend the
> whole patchset? Maybe some of them are ok...
Yes please, you do not want me to pick and choose individual patches out
of this series. Our tools grab the whole series at once to apply them.
thanks,
greg k-h
Re: [PATCH] fs/debugfs: Convert to DEFINE_SHOW_ATTRIBUTE
On Fri, Apr 02, 2021 at 08:11:41PM +0800, zuoqil...@163.com wrote: > From: zuoqilin Please use your full/real name. thanks, greg k-h
Re: [PATCH v6 2/7] powercap/drivers/dtpm: Create a registering system
On Fri, Apr 02, 2021 at 01:10:51PM +0200, Daniel Lezcano wrote: > >> To answer your questions, there is a SoC vendor thermal daemon using > >> DTPM and there is a tool created to watch the thermal framework and read > >> the DTPM values, it is available at [4]. It is currently under > >> development with the goal of doing power rebalancing / capping across > >> the different nodes when there is a violation of the parent's power limit. > > > > Crazy ideas aside, your implementation of this is my main objection > > here. You are creating a user/kernel api that you will have to support > > for 20+ years, without a real userspace user just yet (from what I can > > tell). That's rough, and is going to mean that this gets messy over > > time. > > I'm not sure to understand, the API already exists since v3.3, it is the > powercap and DTPM is its backend. AFAICT, there are already users of it > except they create their own way to build the hierarchy today. The configfs api is what I am referring to here, the ones in this patch series... thanks, greg k-h
Re: [PATCH] staging: fbtft: change snprintf() to scnprintf()
On Fri, Apr 02, 2021 at 09:05:01AM +, Carlis wrote: > From: Xuezhi Zhang > > show() must not use snprintf() when formatting the value to > be returned to user space. Why not? The code is just fine as-is. > > Signed-off-by: Xuezhi Zhang > --- > drivers/staging/fbtft/fbtft-sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/fbtft/fbtft-sysfs.c > b/drivers/staging/fbtft/fbtft-sysfs.c > index 26e52cc2de64..7df92db648d6 100644 > --- a/drivers/staging/fbtft/fbtft-sysfs.c > +++ b/drivers/staging/fbtft/fbtft-sysfs.c > @@ -199,7 +199,7 @@ static ssize_t show_debug(struct device *device, > struct fb_info *fb_info = dev_get_drvdata(device); > struct fbtft_par *par = fb_info->par; > > - return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug); > + return scnprintf(buf, PAGE_SIZE, "%lu\n", par->debug); If you really want to "fix" this, please just use sysfs_emit(). This change as-is, does nothing. thanks, greg k-h
Re: [PATCH v6 2/7] powercap/drivers/dtpm: Create a registering system
On Fri, Apr 02, 2021 at 12:08:49AM +0200, Daniel Lezcano wrote:
>
> Hi Greg,
>
> On 01/04/2021 21:28, Greg KH wrote:
> > On Thu, Apr 01, 2021 at 08:36:49PM +0200, Daniel Lezcano wrote:
> >> A SoC can be differently structured depending on the platform and the
> >> kernel can not be aware of all the combinations, as well as the
> >> specific tweaks for a particular board.
> >>
> >> The creation of the hierarchy must be delegated to userspace.
> >
> > Why? Isn't this what DT is for?
>
> I've always been told the DT describes the hardware. Here we are more
> describing a configuration, that is the reason why I've let the
> userspace to handle that through configfs.
DT does describe how the hardware configuration is to be used. You are
saying that the hardware description does not work and somehow you need
a magic userspace tool to reconfigure things instead?
> > What "userspace tool" is going to be created to manage all of this?
> > Pointers to that codebase?
>
> You are certainly aware of most of it but let me give a bit more of context.
No, I am not aware of it at all, thanks :)
> The thermal framework has cooling devices which export their 'state', a
> representation of their performance level, in sysfs. Unfortunately that
> gives access from the user space to act on the performance as a power
> limiter in total conflict with the in-kernel thermal governor decisions.
Why not remove that conflict?
> That is done from thermal daemon the different SoC vendors tweak for
> their platform. Depending on the application running and identified as a
> scenario, the daemon acts proactively on the different cooling devices
> to ensure a skin temperature which is far below the thermal limit of the
> components.
So userspace is going to try to manage power settings in order to keep
thermal values under control? Has no one learned from our past mistakes
when we used to have to do this 10 years ago and it turned out so bad
that it was just baked into the hardware instead?
{sigh}
> This usage of the cooling devices hijacked the real purpose of the
> thermal framework which is to protect the silicon. Nobody to blame,
> there is no alternative for userspace.
Userspace should not care.
> The use case falls under the power limitation framework prerogative and
> that is the reason why we provided a new framework to limit the power
> based on the powercap framework. The thermal daemon can then use it and
> stop abusing the thermal framework.
>
> This DTPM framework allows to read the power consumption and set a power
> limit to a device.
>
> While the powercap simple backend gives a single device entry, DTPM
> aggregates the different devices power by summing their power and their
> limits. The tree representation of the different DTPM nodes describe how
> their limits are set and how the power is computed along the different
> devices.
That's all great, doing this in-kernel is fine, it's now the "userspace
must set this up properly that I'm objecting to as no one will know how
to do this.
> For more info, we did a presentation at ELC [1] and Linux PM
> microconference [2] and there is an article talking about it [3].
>
>
> To answer your questions, there is a SoC vendor thermal daemon using
> DTPM and there is a tool created to watch the thermal framework and read
> the DTPM values, it is available at [4]. It is currently under
> development with the goal of doing power rebalancing / capping across
> the different nodes when there is a violation of the parent's power limit.
Crazy ideas aside, your implementation of this is my main objection
here. You are creating a user/kernel api that you will have to support
for 20+ years, without a real userspace user just yet (from what I can
tell). That's rough, and is going to mean that this gets messy over
time.
Also there's the whole "tying sysfs to configfs" mess and reference
counting that I object to as well...
> >> The next changes will provide an userspace interface to create
> >> hierarchically the different nodes. Those will be created by name and
> >> found via the list filled by the different subsystem.
> >>
> >> If a specified name is not found in the list, it is assumed to be a
> >> virtual node which will have children and the default is to allocate
> >> such node.
> >
> > So userspace sets the name?
> >
> > Why not use the name in the device itself? I thought I asked that last
> > time...
>
> I probably missed it, sorry for that.
>
> When the userspace creates the directory in the configfs, there is a
> lookup with the name in the device list name. If it is found, then the
> device is used, othe
Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate
On Thu, Apr 01, 2021 at 11:59:25PM +, Luis Chamberlain wrote: > As for the syfs deadlock possible with drivers, this fixes it in a generic > way: > > commit fac43d8025727a74f80a183cc5eb74ed902a5d14 > Author: Luis Chamberlain > Date: Sat Mar 27 14:58:15 2021 + > > sysfs: add optional module_owner to attribute > > This is needed as otherwise the owner of the attribute > or group read/store might have a shared lock used on driver removal, > and deadlock if we race with driver removal. > > Signed-off-by: Luis Chamberlain No, please no. Module removal is a "best effort", if the system dies when it happens, that's on you. I am not willing to expend extra energy and maintance of core things like sysfs for stuff like this that does not matter in any system other than a developer's box. Lock data, not code please. Trying to tie data structure's lifespans to the lifespan of code is a tangled mess, and one that I do not want to add to in any form. sorry, greg k-h
Re: [PATCH] media: pvrusb2: fix warning in pvr2_i2c_core_done
On Thu, Apr 01, 2021 at 06:03:38PM +0530, Anirudh Rayabharam wrote:
> syzbot has reported the following warning in pvr2_i2c_done:
>
> sysfs group 'power' not found for kobject '1-0043'
>
> When the device is disconnected (pvr_hdw_disconnect), the i2c adapter is
> not unregistered along with the USB and vl42 teardown. As part of the
> USB device disconnect, the sysfs files of the subdevices are also
> deleted. So, by the time pvr_i2c_core_done is called by
> pvr_context_destroy, the sysfs files have been deleted.
>
> To fix this, unregister the i2c adapter too in pvr_hdw_disconnect. Make
> the device deregistration code shared by calling pvr_hdw_disconnect from
> pvr2_hdw_destory.
>
> Reported-and-tested-by: syzbot+e74a998ca8f1df9cc...@syzkaller.appspotmail.com
> Signed-off-by: Anirudh Rayabharam
> ---
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index f4a727918e35..791227787ff5 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -2676,9 +2676,7 @@ void pvr2_hdw_destroy(struct pvr2_hdw *hdw)
> pvr2_stream_destroy(hdw->vid_stream);
> hdw->vid_stream = NULL;
> }
> - pvr2_i2c_core_done(hdw);
> - v4l2_device_unregister(>v4l2_dev);
> - pvr2_hdw_remove_usb_stuff(hdw);
> + pvr2_hdw_disconnect(hdw);
> mutex_lock(_unit_mtx);
> do {
> if ((hdw->unit_number >= 0) &&
> @@ -2705,6 +2703,7 @@ void pvr2_hdw_disconnect(struct pvr2_hdw *hdw)
> {
> pvr2_trace(PVR2_TRACE_INIT,"pvr2_hdw_disconnect(hdw=%p)",hdw);
> LOCK_TAKE(hdw->big_lock);
> + pvr2_i2c_core_done(hdw);
> LOCK_TAKE(hdw->ctl_lock);
> pvr2_hdw_remove_usb_stuff(hdw);
> LOCK_GIVE(hdw->ctl_lock);
> --
> 2.26.2
Looks sane to me, nice work tracking this down.
Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 1/2] soundwire: add macro to selectively change error levels
On Thu, Apr 01, 2021 at 01:43:53PM -0500, Pierre-Louis Bossart wrote: > > > > > My bigger issue with this is that this macro is crazy. Why do you need > > > > debugging here at all for this type of thing? That's what ftrace is > > > > for, do not sprinkle code with "we got this return value from here!" all > > > > over the place like what this does. > > > > > > We are not sprinkling the code all over the place with any new logs, they > > > exist already in the SoundWire code and this patch helps filter them out. > > > See e.g. patch 2/2 > > > > > > - dev_err(>dev, > > > - "Clk Stop type =%d failed: %d\n", type, ret); > > > + sdw_dev_dbg_or_err(>dev, ret != -ENODATA, > > > +"Clk Stop mode %d type =%d failed: > > > %d\n", > > > +mode, type, ret); > > > > You just added a debug log for no reason. > > The number of logs is lower when dynamic debug is not enabled, and equal > when it is. there's no addition. > > The previous behavior was unconditional dev_err that everyone sees. > > Now it's dev_err ONLY when the code is NOT -ENODATA, and dev_dgb otherwise, > meaning it will seen ONLY be seen IF dynamic debug is enabled for > drivers/soundwire/bus.c > > Allow me to use another example from patch2: > > - if (ret == -ENODATA) > - dev_dbg(bus->dev, > - "ClockStopNow Broadcast msg ignored %d", ret); > - else > - dev_err(bus->dev, > - "ClockStopNow Broadcast msg failed %d", ret); > + sdw_dev_dbg_or_err(bus->dev, ret != -ENODATA, > +"ClockStopNow Broadcast msg failed %d\n", > ret); > > There's no new log, is there? No, but that is not what you showed above which was just an error message being replaced with both a debug and an error message. Just drop the debug messages, they are pointless, right? thanks, greg k-h
Re: [PATCH 1/5] mm: reuse only-pte-mapped KSM page in do_wp_page()
On Thu, Apr 01, 2021 at 11:17:37AM -0700, Suren Baghdasaryan wrote: > From: Kirill Tkhai > > Add an optimization for KSM pages almost in the same way that we have > for ordinary anonymous pages. If there is a write fault in a page, > which is mapped to an only pte, and it is not related to swap cache; the > page may be reused without copying its content. > > [ Note that we do not consider PageSwapCache() pages at least for now, > since we don't want to complicate __get_ksm_page(), which has nice > optimization based on this (for the migration case). Currenly it is > spinning on PageSwapCache() pages, waiting for when they have > unfreezed counters (i.e., for the migration finish). But we don't want > to make it also spinning on swap cache pages, which we try to reuse, > since there is not a very high probability to reuse them. So, for now > we do not consider PageSwapCache() pages at all. ] > > So in reuse_ksm_page() we check for 1) PageSwapCache() and 2) > page_stable_node(), to skip a page, which KSM is currently trying to > link to stable tree. Then we do page_ref_freeze() to prohibit KSM to > merge one more page into the page, we are reusing. After that, nobody > can refer to the reusing page: KSM skips !PageSwapCache() pages with > zero refcount; and the protection against of all other participants is > the same as for reused ordinary anon pages pte lock, page lock and > mmap_sem. > > [a...@linux-foundation.org: replace BUG_ON()s with WARN_ON()s] > Link: > http://lkml.kernel.org/r/154471491016.31352.1168978849911555609.stgit@localhost.localdomain > Signed-off-by: Kirill Tkhai > Reviewed-by: Yang Shi > Cc: "Kirill A. Shutemov" > Cc: Hugh Dickins > Cc: Andrea Arcangeli > Cc: Christian Koenig > Cc: Claudio Imbrenda > Cc: Rik van Riel > Cc: Huang Ying > Cc: Minchan Kim > Cc: Kirill Tkhai > Signed-off-by: Andrew Morton > Signed-off-by: Linus Torvalds > --- > include/linux/ksm.h | 7 +++ > mm/ksm.c| 30 -- > mm/memory.c | 16 ++-- > 3 files changed, 49 insertions(+), 4 deletions(-) You forgot to put the git commit id of the upstream commit in here somewhere so we can properly reference it and track it. When/if you resend this, please add it to all of the commits. thanks, greg k-h
Re: [PATCH v6 7/7] powercap/drivers/dtpm: Allow dtpm node device creation through configfs
On Thu, Apr 01, 2021 at 08:36:54PM +0200, Daniel Lezcano wrote:
> The DTPM framework is built on top of the powercap framework as a new
> controller to act on the power of the devices. The approach is to
> provide an unified API to do power limitation on devices which are
> capable of that with different techniques.
>
> In addition, the DTPM framework allows to create a hierarchical
> representation of the devices in order to reflect the dependencies of
> the power constraints we want to apply to a group of devices.
>
> The hierarchy can be different for the same platform as it will depend
> on the form factor (tablet, notebook, phone, ...), on other components
> and/or a policy, and application scenario.
>
> The kernel can not have such knowledge and only the SoC vendor can
> setup its platform to fulfill the objectives of its hardware.
>
> This patch adds the ability to create a DTPM hierarchy via
> configfs. All DTPM capable devices are registered in a list, and the
> creation of a configfs directory with the name of one of this device
> will create a DTPM node in the DTPM powercap sysfs. If the name is not
> in the list, a virtual node will be created instead. This virtual node
> in the DTPM semantic is to aggregate the power characteristics of the
> children.
>
> In order to do the connection between the configfs and sysfs easily, a
> 'device' file contains the path to the corresponding DTPM powercap
> node in sysfs (cross filesystems symlink is not supported by
> configfs).
>
> In order to not block any new features in the future, the hierarchical
> constraints are stored under a top folder 'constraints', so sibling
> can be created for other purposes if needed.
>
> When the configfs was populated, the module can not be unloaded until
> all the elements in the tree have been removed.
>
> 1) Resulting creation via mkdir:
>
> root@rock960:/sys/kernel/config# tree dtpm/
> dtpm/
> └── constraints
> └── platform
> ├── device
> └── soc
> ├── device
> └── pkg
>├── device
>├── cpu0-cpufreq
>│ └── device
>├── cpu4-cpufreq
>│ └── device
>└── panfrost-devfreq
> └── device
>
> 2) The content of the 'device' file above
>
> root@rock960:/sys/kernel/config# find dtpm/constraints -name "device" -exec
> cat {} \;
> /devices/virtual/powercap/dtpm/dtpm:0/dtpm:0:0/dtpm:0:0:1
> /devices/virtual/powercap/dtpm/dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:2
> /devices/virtual/powercap/dtpm/dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:1
> /devices/virtual/powercap/dtpm/dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:0
> /devices/virtual/powercap/dtpm/dtpm:0/dtpm:0:0/dtpm:0:0:0
> /devices/virtual/powercap/dtpm/dtpm:0/dtpm:0:0
> /devices/virtual/powercap/dtpm/dtpm:0
>
> 3) The dtpm device creation node is reflected in sysfs:
>
> root@rock960:/sys/devices/virtual/powercap/dtpm# find . -type d | grep dtpm
> ./dtpm:0
> ./dtpm:0/power
> ./dtpm:0/dtpm:0:0
> ./dtpm:0/dtpm:0:0/power
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:1
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:1/power
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/power
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:2
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:2/power
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:0
> ./dtpm:0/dtpm:0:0/dtpm:0:0:0/dtpm:0:0:0:0/power
> ./dtpm:0/dtpm:0:0/dtpm:0:0:1
> ./dtpm:0/dtpm:0:0/dtpm:0:0:1/power
Why have you not documented all of this in Documentation/ABI so that we
can find it later?
>
> Signed-off-by: Daniel Lezcano
> ---
> drivers/powercap/Kconfig | 8 ++
> drivers/powercap/Makefile| 1 +
> drivers/powercap/dtpm_configfs.c | 202 +++
> include/linux/dtpm.h | 2 +
> 4 files changed, 213 insertions(+)
> create mode 100644 drivers/powercap/dtpm_configfs.c
>
> diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
> index 8242e8c5ed77..599b41e4e0a7 100644
> --- a/drivers/powercap/Kconfig
> +++ b/drivers/powercap/Kconfig
> @@ -50,6 +50,14 @@ config DTPM
> This enables support for the power capping for the dynamic
> thermal power management userspace engine.
>
> +config DTPM_CONFIGFS
> + tristate "Dynamic Thermal Power Management configuration via configfs"
> + depends on DTPM && CONFIGFS_FS
> + help
> + This enables support for creating the DTPM device hierarchy
> + via configfs giving the userspace full control of the
> + thermal constraint representation.
Why does this have to be a module, shouldn't it just be part of the DTPM
code itself?
> +
> config DTPM_CPU
> bool "Add CPU power capping based on the energy model"
> depends on DTPM && ENERGY_MODEL
> diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile
> index fabcf388a8d3..519cabc624c3 100644
> --- a/drivers/powercap/Makefile
> +++
Re: [PATCH v6 2/7] powercap/drivers/dtpm: Create a registering system
On Thu, Apr 01, 2021 at 08:36:49PM +0200, Daniel Lezcano wrote: > A SoC can be differently structured depending on the platform and the > kernel can not be aware of all the combinations, as well as the > specific tweaks for a particular board. > > The creation of the hierarchy must be delegated to userspace. Why? Isn't this what DT is for? What "userspace tool" is going to be created to manage all of this? Pointers to that codebase? > These changes provide a registering mechanism where the different > subsystems will initialize their dtpm backends and register with a > name the dtpm node in a list. > > The next changes will provide an userspace interface to create > hierarchically the different nodes. Those will be created by name and > found via the list filled by the different subsystem. > > If a specified name is not found in the list, it is assumed to be a > virtual node which will have children and the default is to allocate > such node. So userspace sets the name? Why not use the name in the device itself? I thought I asked that last time... thanks, greg k-h
Re: [PATCH 1/2] soundwire: add macro to selectively change error levels
On Thu, Apr 01, 2021 at 01:07:49PM -0500, Pierre-Louis Bossart wrote:
>
> > > > > +#define sdw_dev_dbg_or_err(dev, is_err, fmt, ...)
> > > > > \
> > > > > + do {
> > > > > \
> > > > > + if (is_err)
> > > > > \
> > > > > + dev_err(dev, fmt, __VA_ARGS__);
> > > > > \
> > > > > + else
> > > > > \
> > > > > + dev_dbg(dev, fmt, __VA_ARGS__);
> > > > > \
> > > > > + } while (0)
> > > >
> > > > I see a variant in sof code and now here, why not add in a
> > > > dev_dbg_or_err() and use everywhere?
> > >
> > > Good point, I hesitated back and forth on specific v. generic macro.
> > >
> > > The main reason why I added this macro for SoundWire is that quite a few
> > > subsystems have their own debug functions (DRM, ACPI, etc), and I wasn't
> > > sure if there was any appetite to add more options in
> > > include/linux/dev_printk.h. SOF also uses a different format due to
> > > history.
> >
> > It is better if those other subsystems move to using the common kernel
> > debug functions. Historically they were all separate, there is no good
> > reason for them to be that way today.
> >
> > So please do not create custom subsystem debug macros like this just for
> > this tiny set of drivers.
> >
> > My bigger issue with this is that this macro is crazy. Why do you need
> > debugging here at all for this type of thing? That's what ftrace is
> > for, do not sprinkle code with "we got this return value from here!" all
> > over the place like what this does.
>
> We are not sprinkling the code all over the place with any new logs, they
> exist already in the SoundWire code and this patch helps filter them out.
> See e.g. patch 2/2
>
> - dev_err(>dev,
> - "Clk Stop type =%d failed: %d\n", type, ret);
> + sdw_dev_dbg_or_err(>dev, ret != -ENODATA,
> +"Clk Stop mode %d type =%d failed:
> %d\n",
> +mode, type, ret);
You just added a debug log for no reason.
That's what I was referring to :)
> If you see all my recent patches they were precisely trying to avoid
> polluting the console logs with too much information that is irrelevant from
> most users, and making sure that when a log is provided it's uniquely
> identifiable.
>
> There are similar macros where -EPROBE_DEFER is ignored.
deffered probe is a totally different beast and one that I constantly am
ashamed I accepted into the kernel as the added complexity it has caused
is crazy.
> This addresses a very SoundWire-specific case where if we see a -ENODATA
> error code (Command Ignored), we ignore it and don't report it by default.
> We still have a rare set of cases where this -ENODATA code shows up
> unexpectedly, possibly due to problematic reset sequences, and we want
> developers to help track them down what causes this sequence using dynamic
> debug.
>
> I am not arguing about ftrace v. dynamic debug, and that's also partly why I
> didn't feel comfortable expanding the generic set of debug functions.
Great, then don't add unneeded dev_dbg() lines :)
thanks,
greg k-h
Re: [PATCH 1/2] soundwire: add macro to selectively change error levels
On Thu, Apr 01, 2021 at 09:30:27AM -0500, Pierre-Louis Bossart wrote:
>
>
> On 4/1/21 2:24 AM, Vinod Koul wrote:
> > On 31-03-21, 09:13, Bard Liao wrote:
> > > From: Pierre-Louis Bossart
> > >
> > > We sometimes discard -ENODATA when reporting errors and lose all
> > > traces of issues in the console log, add a macro to add use dev_dbg()
> > > in such cases.
> > >
> > > Signed-off-by: Pierre-Louis Bossart
> > > Reviewed-by: Rander Wang
> > > Reviewed-by: Guennadi Liakhovetski
> > > Signed-off-by: Bard Liao
> > > ---
> > > drivers/soundwire/bus.h | 8
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
> > > index 40354469860a..8370216f95d4 100644
> > > --- a/drivers/soundwire/bus.h
> > > +++ b/drivers/soundwire/bus.h
> > > @@ -227,4 +227,12 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus,
> > > u16 dev_num, u32 addr, u8 val
> > > void sdw_clear_slave_status(struct sdw_bus *bus, u32 request);
> > > int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t
> > > size);
> > > +#define sdw_dev_dbg_or_err(dev, is_err, fmt, ...)
> > > \
> > > + do {\
> > > + if (is_err) \
> > > + dev_err(dev, fmt, __VA_ARGS__); \
> > > + else\
> > > + dev_dbg(dev, fmt, __VA_ARGS__); \
> > > + } while (0)
> >
> > I see a variant in sof code and now here, why not add in a
> > dev_dbg_or_err() and use everywhere?
>
> Good point, I hesitated back and forth on specific v. generic macro.
>
> The main reason why I added this macro for SoundWire is that quite a few
> subsystems have their own debug functions (DRM, ACPI, etc), and I wasn't
> sure if there was any appetite to add more options in
> include/linux/dev_printk.h. SOF also uses a different format due to history.
It is better if those other subsystems move to using the common kernel
debug functions. Historically they were all separate, there is no good
reason for them to be that way today.
So please do not create custom subsystem debug macros like this just for
this tiny set of drivers.
My bigger issue with this is that this macro is crazy. Why do you need
debugging here at all for this type of thing? That's what ftrace is
for, do not sprinkle code with "we got this return value from here!" all
over the place like what this does.
thanks,
greg k-h
Re: [PATCH] Revert "usb: dwc3: gadget: Prevent EP queuing while stopping transfers"
On Thu, Apr 01, 2021 at 01:55:58PM +0200, Martin Kepplinger wrote: > This reverts commit 9de47c3737e0c0207beb03615b320cabe495. > You do not put a reason here, so I can not take this :( Please fix up and resend. thanks, greg k-h
Re: [PATCH 0/2] staging: media: omap4iss: Patchsets cleans up in iss.c
On Thu, Apr 01, 2021 at 04:07:37PM +0100, Beatriz Martins de Carvalho wrote:
> Clean up checks of "Alignment should match open parenthesis" and
> "CHECK: Lines should not end with a '('" in iss.c
>
> Beatriz Martins de Carvalho (2):
> staging: media: omap4iss: Ending line with argument
> staging: media: omap4iss: align arguments with open parenthesis
>
> drivers/staging/media/omap4iss/iss.c | 48 +---
Please read the outreachy introduction email again :(
thanks,
greg k-h
Re: [PATCH v2] Revert "usb: dwc3: gadget: Prevent EP queuing while stopping transfers"
On Thu, Apr 01, 2021 at 03:11:08PM +0200, Martin Kepplinger wrote:
> commit 9de47c ("usb: dwc3: gadget: Prevent EP queuing while stopping
> transfers") results in the below error every time I connect the type-c
> connector to the dwc3, configured with serial and ethernet gadgets.
> I also apply the following to dwc3 on this port:
>
> dr_mode = "otg";
> snps,dis_u3_susphy_quirk;
> hnp-disable;
> srp-disable;
> adp-disable;
Why all the trailing whitespace?
> usb-role-switch;
>
> [ 51.148220] [ cut here ]
> [ 51.148241] dwc3 3810.usb: No resource for ep2in
> [ 51.148376] WARNING: CPU: 0 PID: 299 at drivers/usb/dwc3/gadget.c:360
> dwc3_send_gadget_ep_cmd+0x570/0x740 [dwc3]
> [ 51.148837] CPU: 0 PID: 299 Comm: irq/64-dwc3 Not tainted
> 5.11.11-librem5-00334-ge4c4ff3624e9 #218
> [ 51.148848] Hardware name: Purism Librem 5r4 (DT)
> [ 51.148854] pstate: 6085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
> [ 51.148863] pc : dwc3_send_gadget_ep_cmd+0x570/0x740 [dwc3]
> [ 51.148894] lr : dwc3_send_gadget_ep_cmd+0x570/0x740 [dwc3]
> [ 51.148924] sp : 800011cb3ac0
> [ 51.148929] x29: 800011cb3ac0 x28: 032a7b00
> [ 51.148942] x27: 0327da00 x26:
> [ 51.148954] x25: ffea x24: 0006
> [ 51.148967] x23: bee1c080 x22: 800011cb3b7c
> [ 51.148979] x21: 0406 x20: bf17
> [ 51.148992] x19: 0001 x18:
> [ 51.149004] x17: x16:
> [ 51.149016] x15: x14: 8000114512c0
> [ 51.149028] x13: 1698 x12: 0040
> [ 51.149040] x11: 80001151a6f8 x10: e000
> [ 51.149052] x9 : 8000100b2b7c x8 : 80001146a6f8
> [ 51.149065] x7 : 80001151a6f8 x6 :
> [ 51.149077] x5 : bf939948 x4 :
> [ 51.149089] x3 : 0027 x2 :
> [ 51.149101] x1 : x0 : 049ae040
> [ 51.149114] Call trace:
> [ 51.149120] dwc3_send_gadget_ep_cmd+0x570/0x740 [dwc3]
> [ 51.149150] __dwc3_gadget_ep_enable+0x288/0x4fc [dwc3]
> [ 51.149179] dwc3_gadget_ep_enable+0x6c/0x15c [dwc3]
> [ 51.149209] usb_ep_enable+0x48/0x110 [udc_core]
> [ 51.149251] rndis_set_alt+0x138/0x1c0 [usb_f_rndis]
> [ 51.149276] composite_setup+0x674/0x194c [libcomposite]
> [ 51.149314] dwc3_ep0_interrupt+0x9c4/0xb9c [dwc3]
> [ 51.149344] dwc3_thread_interrupt+0x8bc/0xe74 [dwc3]
> [ 51.149374] irq_thread_fn+0x38/0xb0
> [ 51.149388] irq_thread+0x170/0x294
> [ 51.149397] kthread+0x164/0x16c
> [ 51.149407] ret_from_fork+0x10/0x34
> [ 51.149419] ---[ end trace 62c6cc2ebfb18047 ]---
>
> Linus' tree isn't affected. Revert the change.
What does this mean?
>
> Signed-off-by: Martin Kepplinger
>
> ---
What changed from v1? Always put that below the --- line.
And as this is a revert, is a "Fixes:" line also relevant?
thanks,
greg k-h
Re: [PATCH] xhci: remove unused variable
On Thu, Apr 01, 2021 at 06:17:15PM +0800, Jiapeng Chong wrote:
> Fix the following clang warning:
>
> drivers/usb/host/xhci.c:1346:15: warning: variable ‘len’ set but not
> used [-Wunused-but-set-variable].
>
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
> ---
> drivers/usb/host/xhci.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 1975016..0ead09c 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1343,7 +1343,6 @@ static bool xhci_urb_temp_buffer_required(struct
> usb_hcd *hcd,
>
> static void xhci_unmap_temp_buf(struct usb_hcd *hcd, struct urb *urb)
> {
> - unsigned int len;
> unsigned int buf_len;
> enum dma_data_direction dir;
>
> @@ -1359,10 +1358,9 @@ static void xhci_unmap_temp_buf(struct usb_hcd *hcd,
> struct urb *urb)
>dir);
>
> if (usb_urb_dir_in(urb))
> - len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
> -urb->transfer_buffer,
> -buf_len,
> -0);
> + sg_pcopy_from_buffer(urb->sg, urb->num_sgs,
> + urb->transfer_buffer,
> + buf_len, 0);
>
> urb->transfer_flags &= ~URB_DMA_MAP_SINGLE;
> kfree(urb->transfer_buffer);
> --
> 1.8.3.1
>
Why resubmit this same change when I rejected it in the past?
{sigh}
greg k-h
Re: [PATCH] fix NULL pointer deference crash
On Thu, Apr 01, 2021 at 09:21:54AM +0300, Hassan Shahbazi wrote: > On Wed, Mar 31, 2021 at 07:32:06PM +0200, Greg KH wrote: > > On Wed, Mar 31, 2021 at 07:34:29PM +0300, Hassan Shahbazi wrote: > > > The patch has fixed a NULL pointer deference crash in hiding the cursor. > > > It > > > is verified by syzbot patch tester. > > > > > > Reported by: syzbot > > > https://syzkaller.appspot.com/bug?id=defb47bf56e1c14d5687280c7bb91ce7b608b94b > > > > > > Signed-off-by: Hassan Shahbazi > > > --- > > > drivers/video/fbdev/core/fbcon.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/video/fbdev/core/fbcon.c > > > b/drivers/video/fbdev/core/fbcon.c > > > index 44a5cd2f54cc..ee252d1c43c6 100644 > > > --- a/drivers/video/fbdev/core/fbcon.c > > > +++ b/drivers/video/fbdev/core/fbcon.c > > > @@ -1333,8 +1333,9 @@ static void fbcon_cursor(struct vc_data *vc, int > > > mode) > > > > > > ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1; > > > > > > - ops->cursor(vc, info, mode, get_color(vc, info, c, 1), > > > - get_color(vc, info, c, 0)); > > > + if (ops && ops->cursor) > > > > As ops obviously is not NULL here (you just used it on the line above), > > why are you checking it again? > > Yes, that's right. I will remove that check and will submit a new patch. > > > > And what makes curser be NULL here? How can that happen? > > Honestly, I don't know. I reproduced the crash on my local, followed the > stack trace, and then changed the line to avoid the crash. If you think this > patch is not the best solution, I can drop it and investigate more to find > the root cause. Finding the root cause would be good to do here, so that we can potentially fix that if it is needed. thanks, greg k-h
Re: [PATCH v5 2/5] powercap/drivers/dtpm: Create a registering system
On Wed, Mar 31, 2021 at 10:46:48PM +0200, Daniel Lezcano wrote: > > Hi Greg, > > On 31/03/2021 20:06, Greg KH wrote: > > On Wed, Mar 31, 2021 at 01:00:45PM +0200, Daniel Lezcano wrote: > >> +struct dtpm *dtpm_lookup(const char *name); > >> + > >> +int dtpm_add(const char *name, struct dtpm *dtpm); > >> + > >> +void dtpm_del(const char *name); > > > > You can not add new kernel apis that have no user. How do you know if > > they actually work or not? We have no idea as we do not see anyone > > using them :( > > > > So no need to add things with no user, feel free to just drop this patch > > until you have one. > > I've sent a couple of patches [1] on top of the previous series. I'm > finishing to respin it against this new one. > > -- Daniel > > [1] https://lkml.org/lkml/2021/3/12/1514 Please use lore.kernel.org, we do not control lkml and it has been down in the past and it's impossible to reply from. Please always provide a user of a function in the patch series, otherwise you will end up with comments like mine above. thanks, greg k-h
Re: [PATCH -next v2] staging: greybus: camera: Switch to memdup_user_nul()
On Thu, Apr 01, 2021 at 11:17:52AM +0800, Yang Yingliang wrote: > Use memdup_user_nul() helper instead of open-coding to > simplify the code. > > Reported-by: Hulk Robot > Signed-off-by: Yang Yingliang > --- > drivers/staging/greybus/camera.c | 13 +++-- > 1 file changed, 3 insertions(+), 10 deletions(-) What changed from v1? Always put that below the --- line like the documentation asks you to. Please fix up and send a v3. thanks, greg k-h
Re: [PATCH 0/3] Fix block comment warnings
On Wed, Mar 31, 2021 at 01:05:34PM -0700, Deborah Brouwer wrote: > This patchset fixes checkpatch warnings arising > from the block comments Note, your 0/X email subject should also have the subsystem/driver prefix in there so that we know what this series is for. Much like your individual patches do. thanks, greg k-h
Re: [PATCH v5 2/5] powercap/drivers/dtpm: Create a registering system
On Wed, Mar 31, 2021 at 01:00:45PM +0200, Daniel Lezcano wrote: > +struct dtpm *dtpm_lookup(const char *name); > + > +int dtpm_add(const char *name, struct dtpm *dtpm); > + > +void dtpm_del(const char *name); You can not add new kernel apis that have no user. How do you know if they actually work or not? We have no idea as we do not see anyone using them :( So no need to add things with no user, feel free to just drop this patch until you have one. thanks, greg k-h
Re: [PATCH v5 2/5] powercap/drivers/dtpm: Create a registering system
On Wed, Mar 31, 2021 at 01:00:45PM +0200, Daniel Lezcano wrote:
> A SoC can be differently structured depending on the platform and the
> kernel can not be aware of all the combinations, as well as the
> specific tweaks for a particular board.
>
> The creation of the hierarchy must be delegated to userspace.
Isn't that what DT is for?
> These changes provide a registering mechanism where the different
> subsystems will initialize their dtpm backends and register with a
> name the dtpm node in a list.
>
> The next changes will provide an userspace interface to create
> hierarchically the different nodes. Those will be created by name and
> found via the list filled by the different subsystem.
>
> If a specified name is not found in the list, it is assumed to be a
> virtual node which will have children and the default is to allocate
> such node.
There's no userspace portion here, so why talk about it?
>
> Cc: Greg KH
> Signed-off-by: Daniel Lezcano
> Reviewed-by: Lukasz Luba
> ---
>
> V5:
> - Decrease log level from 'info' to 'debug'
> - Remove the refcount, it is pointless, lifetime cycle is already
> handled by the device refcounting. The dtpm node allocator is in
> charge of freeing it.
> - Rename the functions to 'dtpm_add, dtpm_del, dtpm_lookup'
> - Fix missing kfrees when deleting the node from the list
> V4:
> - Fixed typo in the commit log
> V2:
> - Fixed error code path by dropping lock
> ---
> drivers/powercap/dtpm.c | 121 ++--
> drivers/powercap/dtpm_cpu.c | 8 +--
> include/linux/dtpm.h| 6 ++
> 3 files changed, 127 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index 58433b8ef9a1..8df7adeed0cf 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -34,6 +34,14 @@ static DEFINE_MUTEX(dtpm_lock);
> static struct powercap_control_type *pct;
> static struct dtpm *root;
>
> +struct dtpm_node {
> + const char *name;
> + struct dtpm *dtpm;
> + struct list_head node;
> +};
> +
> +static LIST_HEAD(dtpm_list);
> +
> static int get_time_window_us(struct powercap_zone *pcz, int cid, u64
> *window)
> {
> return -ENOSYS;
> @@ -152,6 +160,113 @@ static int __dtpm_update_power(struct dtpm *dtpm)
> return ret;
> }
>
> +static struct dtpm *__dtpm_lookup(const char *name)
> +{
> + struct dtpm_node *node;
> +
> + list_for_each_entry(node, _list, node) {
> + if (!strcmp(name, node->name))
> + return node->dtpm;
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + * dtpm_lookup - Lookup for a registered dtpm node given its name
> + * @name: the name of the dtpm device
> + *
> + * The function looks up in the list of the registered dtpm
> + * devices. This function must be called to create a dtpm node in the
> + * powercap hierarchy.
> + *
> + * Return: a pointer to a dtpm structure, NULL if not found.
> + */
> +struct dtpm *dtpm_lookup(const char *name)
> +{
> + struct dtpm *dtpm;
> +
> + mutex_lock(_lock);
> + dtpm = __dtpm_lookup(name);
> + mutex_unlock(_lock);
> +
> + return dtpm;
> +}
> +
> +/**
> + * dtpm_add - Add the dtpm in the dtpm list
> + * @name: a name used as an identifier
> + * @dtpm: the dtpm node to be registered
> + *
> + * Stores the dtpm device in a list. The list contains all the devices
> + * which are power capable in terms of limitation and power
> + * consumption measurements. Even if conceptually, a power capable
> + * device won't register itself twice, the function will check if it
> + * was already registered in order to prevent a misuse of the API.
> + *
> + * Return: 0 on success, -EEXIST if the device name is already present
> + * in the list, -ENOMEM in case of memory allocation failure.
> + */
> +int dtpm_add(const char *name, struct dtpm *dtpm)
Why not just use the name of the dtpm?
Where does this "new" name come from? And why would it differ?
> +{
> + struct dtpm_node *node;
> + int ret;
> +
> + mutex_lock(_lock);
> +
> + ret = -EEXIST;
> + if (__dtpm_lookup(name))
> + goto out_unlock;
Why do you have yet-another-list of these devices? They are already all
on a list, why do you need another?
And you seem to be ignoring reference count issues here, you add a
reference counted pointer to a random list in the kernel and never
increment the reference count. That's bad :(
So just don't use a new list please...
thanks,
greg k-h
Re: [PATCH] fix NULL pointer deference crash
On Wed, Mar 31, 2021 at 07:34:29PM +0300, Hassan Shahbazi wrote: > The patch has fixed a NULL pointer deference crash in hiding the cursor. It > is verified by syzbot patch tester. > > Reported by: syzbot > https://syzkaller.appspot.com/bug?id=defb47bf56e1c14d5687280c7bb91ce7b608b94b > > Signed-off-by: Hassan Shahbazi > --- > drivers/video/fbdev/core/fbcon.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index 44a5cd2f54cc..ee252d1c43c6 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1333,8 +1333,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode) > > ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1; > > - ops->cursor(vc, info, mode, get_color(vc, info, c, 1), > - get_color(vc, info, c, 0)); > + if (ops && ops->cursor) As ops obviously is not NULL here (you just used it on the line above), why are you checking it again? And what makes curser be NULL here? How can that happen? Also your subject line can use some work, please make it reflect the driver subsystem you are looking at. thanks, greg k-h
Re: [PATCH 03/40] staging: rtl8723bs: replace RT_TRACE with public printk wrappers in core/rtw_eeprom.c
On Wed, Mar 31, 2021 at 12:55:51PM +0200, Fabio Aiuto wrote:
> On Wed, Mar 31, 2021 at 12:27:20PM +0200, Greg KH wrote:
> > On Wed, Mar 31, 2021 at 11:39:31AM +0200, Fabio Aiuto wrote:
> > > replace private macro RT_TRACE for tracing with in-kernel
> > > pr_* printk wrappers
> > >
> > > Signed-off-by: Fabio Aiuto
> > > ---
> > > drivers/staging/rtl8723bs/core/rtw_eeprom.c | 26 ++---
> > > 1 file changed, 13 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > b/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > index 3cbd65dee741..6176d741d60e 100644
> > > --- a/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > +++ b/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > @@ -36,7 +36,7 @@ void shift_out_bits(_adapter *padapter, u16 data, u16
> > > count)
> > > _func_enter_;
> > >
> > > if (padapter->bSurpriseRemoved == true) {
> > > - RT_TRACE(_module_rtl871x_eeprom_c_, _drv_err_,
> > > ("padapter->bSurpriseRemoved==true"));
> > > + pr_err("%s padapter->bSurpriseRemoved==true", DRIVER_PREFIX);
> >
> > As Dan said, this is not the same thing. You are now always printing
> > out this mess, when before you were not unless you explicitly enabled
> > "tracing".
>
> RT_TRACE is enabled if is defined the symbol DEBUG_RTL871X. It doesn't seem
> to be related
> to tracing. DEBUG_RTL871X is never declared, is commented out in
> rtl8723bs/include/autoconf.h
That's a good sign this is never used and should just all be removed.
> that's why RT_TRACE is never printed. If we try to uncomment the symbol
> definition we have some
> comiling errors..
What errors do you get?
> > And you are sending it to the error log?
> >
> > And finally, drivers should never be using pr_*() for messages, they
> > should be using dev_*() instead as they are a driver and have access to
> > a device pointer.
> >
> > thanks,
> >
> > greg k-h
>
> I still wonder what's best...
Just delete them all please, they are obviously not used as no one
rebuilds the source just for this type of thing.
thanks,
greg k-h
Re: [PATCH 03/40] staging: rtl8723bs: replace RT_TRACE with public printk wrappers in core/rtw_eeprom.c
On Wed, Mar 31, 2021 at 12:36:31PM +0200, Fabio Aiuto wrote:
> On Wed, Mar 31, 2021 at 12:27:20PM +0200, Greg KH wrote:
> > On Wed, Mar 31, 2021 at 11:39:31AM +0200, Fabio Aiuto wrote:
> > > replace private macro RT_TRACE for tracing with in-kernel
> > > pr_* printk wrappers
> > >
> > > Signed-off-by: Fabio Aiuto
> > > ---
> > > drivers/staging/rtl8723bs/core/rtw_eeprom.c | 26 ++---
> > > 1 file changed, 13 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > b/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > index 3cbd65dee741..6176d741d60e 100644
> > > --- a/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > +++ b/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> > > @@ -36,7 +36,7 @@ void shift_out_bits(_adapter *padapter, u16 data, u16
> > > count)
> > > _func_enter_;
> > >
> > > if (padapter->bSurpriseRemoved == true) {
> > > - RT_TRACE(_module_rtl871x_eeprom_c_, _drv_err_,
> > > ("padapter->bSurpriseRemoved==true"));
>
> I looked at this ---^
> and so I thought that pr_err was good...
But you missed the fact that RT_TRACE() does not always spit this stuff
out.
And I don't know what _drv_err_ is, but tracing messages should never go
to an error log :)
> my aim was remove private macros replicating component tracing and log
> levels...
That's a great goal!
> so what's best? Keep a simplyfied RT_TRACE encapsulating a dev_* call?
replace them with dev_dbg() is one way, if they really are even needed
at all. At this point in time, I would strongly just recommend removing
them all as no one is using them for anything.
thanks,
greg k-h
Re: [PATCH -next] staging: rtl8723bs: os_dep: remove unused variable 'ret'
On Wed, Mar 31, 2021 at 05:42:47PM +0800, Yang Yingliang wrote:
> GCC reports the following warning with W=1:
>
> drivers/staging/rtl8723bs/os_dep/recv_linux.c:101:6: warning:
> variable ‘ret’ set but not used [-Wunused-but-set-variable]
> 101 | int ret;
> | ^~~
>
> This variable is not used in function , this commit
> remove it to fix the warning.
>
> Fixes: de69e2b3f105 ("staging: rtl8723bs: remove DBG_COUNTER calls from
> os_dep/recv_linux.c")
> Reported-by: Hulk Robot
> Signed-off-by: Yang Yingliang
> ---
> drivers/staging/rtl8723bs/os_dep/recv_linux.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> index fbdbcd04d44a..f6a9482be8e3 100644
> --- a/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/recv_linux.c
> @@ -98,7 +98,6 @@ struct sk_buff *rtw_os_alloc_msdu_pkt(union recv_frame
> *prframe, u16 nSubframe_L
> void rtw_os_recv_indicate_pkt(struct adapter *padapter, struct sk_buff *pkt,
> struct rx_pkt_attrib *pattrib)
> {
> struct mlme_priv *pmlmepriv = >mlmepriv;
> - int ret;
>
> /* Indicate the packets to upper layer */
> if (pkt) {
> @@ -140,7 +139,7 @@ void rtw_os_recv_indicate_pkt(struct adapter *padapter,
> struct sk_buff *pkt, str
>
> pkt->ip_summed = CHECKSUM_NONE;
>
> - ret = rtw_netif_rx(padapter->pnetdev, pkt);
> + rtw_netif_rx(padapter->pnetdev, pkt);
Why not handle the result of this call properly?
Re: [PATCH 03/40] staging: rtl8723bs: replace RT_TRACE with public printk wrappers in core/rtw_eeprom.c
On Wed, Mar 31, 2021 at 11:39:31AM +0200, Fabio Aiuto wrote:
> replace private macro RT_TRACE for tracing with in-kernel
> pr_* printk wrappers
>
> Signed-off-by: Fabio Aiuto
> ---
> drivers/staging/rtl8723bs/core/rtw_eeprom.c | 26 ++---
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> b/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> index 3cbd65dee741..6176d741d60e 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_eeprom.c
> @@ -36,7 +36,7 @@ void shift_out_bits(_adapter *padapter, u16 data, u16 count)
> _func_enter_;
>
> if (padapter->bSurpriseRemoved == true) {
> - RT_TRACE(_module_rtl871x_eeprom_c_, _drv_err_,
> ("padapter->bSurpriseRemoved==true"));
> + pr_err("%s padapter->bSurpriseRemoved==true", DRIVER_PREFIX);
As Dan said, this is not the same thing. You are now always printing
out this mess, when before you were not unless you explicitly enabled
"tracing".
And you are sending it to the error log?
And finally, drivers should never be using pr_*() for messages, they
should be using dev_*() instead as they are a driver and have access to
a device pointer.
thanks,
greg k-h
Re: [PATCH] binder: tell userspace to dump current backtrace when detecting oneway spamming
On Wed, Mar 31, 2021 at 03:34:16PM +0800, Hang Lu wrote: > When async binder buffer got exhausted, some normal oneway transaction > will also be discarded and finally caused system/app stop. By that time, > the binder debug information we dump may not relevant to the root cause. > And this issue is difficult to debug if without the backtrace of thread > sending spam. > > This change will send BR_ONEWAY_SPAM_SUSPECT to userspace when detecting > oneway spamming, request to dump current backtrace. The detection will > happened only once when exceeding the threshold (target process dips > below 80% of its oneway space, and current process is responsible for > either more than 50 transactions, or more than 50% of the oneway space). > And the detection will restart when the async buffer has returned to a > healthy state. > > Signed-off-by: Hang Lu > --- > drivers/android/binder.c| 25 ++--- > drivers/android/binder_alloc.c | 15 --- > drivers/android/binder_alloc.h | 8 +++- > drivers/android/binder_internal.h | 1 + > include/uapi/linux/android/binder.h | 8 > 5 files changed, 50 insertions(+), 7 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index c119736..28ceaf9 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -87,6 +87,7 @@ static DEFINE_SPINLOCK(binder_dead_nodes_lock); > static struct dentry *binder_debugfs_dir_entry_root; > static struct dentry *binder_debugfs_dir_entry_proc; > static atomic_t binder_last_id; > +static bool oneway_spam_detection_enabled; Why is this a "whole system" value and not a "per binder instance" value? You just allowed one binder instance to affect another one, which does not seem like a good idea to me :( thanks, greg k-h
Re: Memory leak in ath9k_hif_usb_dealloc_tx_urbs()
On Tue, Mar 30, 2021 at 10:36:52PM +0300, Pavel Skripkin wrote:
> Hi!
>
> I did some debugging on this
> https://syzkaller.appspot.com/bug?id=3ea507fb3c47426497b52bd82b8ef0dd5b6cc7ee
> and, I believe, I recognized the problem. The problem appears in case of
> ath9k_htc_hw_init() fail. In case of this fail all tx_buf->urb krefs will be
> initialized to 1, but in free function:
>
> static void ath9k_hif_usb_dealloc_tx_urbs(struct hif_device_usb *hif_dev)
>
>
>
> static void ath9k_hif_usb_dealloc_tx_urbs(struct hif_device_usb *hif_dev)
> {
> ...
> list_for_each_entry_safe(tx_buf, tx_buf_tmp,
>_dev->tx.tx_buf, list) {
> usb_get_urb(tx_buf->urb);
> ...
> usb_free_urb(tx_buf->urb);
> ...
> }
>
> Krefs are incremented and then decremented, that means urbs won't be freed.
> I found your patch and I can't properly understand why You added
> usb_get_urb(tx_buf->urb).
> Can You explain please, I believe this will help me or somebody to fix this
> ussue :)
I think almost everyone who has looked into this has given up due to the
mess of twisty-passages here with almost no real-world benefits for
unwinding them :)
Good luck!
greg k-h
Re: [PATCH] staging: rtl8723bs: fix block comments
On Tue, Mar 30, 2021 at 10:38:43PM -0700, Deborah Brouwer wrote: > Remove empty comment and fix checkpatch warnings: > WARNING: Block comments use * on subsequent lines > WARNING: Possible repeated word: 'very' > > Signed-off-by: Deborah Brouwer > --- > drivers/staging/rtl8723bs/core/rtw_xmit.c | 61 +++ > 1 file changed, 28 insertions(+), 33 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot
Re: [RFC PATCH v2 0/2] usb: typec: Add driver for Microchip sama7g5 tcpc
On Tue, Mar 30, 2021 at 11:54:40PM +0300, cristian.bir...@microchip.com wrote: > From: Cristian Birsan > > This patch set adds initial driver support for Microchip USB Type-C Port > Controller (TCPC) embedded in sama7g5 SoC. > > The controller does not implement power delivery and the driver uses dummy > functions to register the port with TCPM. The current silicon version is > not able to trigger interrupts so the driver will poll for changes on > CC1/CC2 lines. > > Support for sink is implemented and tested with an USB device. The plan is > to extend the driver and add source support. Why are these marked "RFC"? Do you really not think they should be accepted? Why not, what is left to do with them? I do not normally review "RFC" patches as the authors do not think they should be merged, and we have plenty of patches that are being asked to be merged already :) thanks, greg k-h
Re: [PATCH v2] wireless/nl80211.c: fix uninitialized variable
On Tue, Mar 30, 2021 at 06:37:05PM +0200, Alaa Emad wrote: > This change fix KMSAN uninit-value in net/wireless/nl80211.c:225 , That > because of `fixedlen` variable uninitialized,So I initialized it by zero. > > Reported-by: syzbot+72b99dcf4607e8c77...@syzkaller.appspotmail.com > Signed-off-by: Alaa Emad > --- > Changes in v2: > - Make the commit message more clearer. > --- > net/wireless/nl80211.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > index 775d0c4d86c3..b87ab67ad33d 100644 > --- a/net/wireless/nl80211.c > +++ b/net/wireless/nl80211.c > @@ -210,7 +210,7 @@ static int validate_beacon_head(const struct nlattr *attr, > const struct element *elem; > const struct ieee80211_mgmt *mgmt = (void *)data; > bool s1g_bcn = ieee80211_is_s1g_beacon(mgmt->frame_control); > - unsigned int fixedlen, hdrlen; > + unsigned int fixedlen = 0 , hdrlen; Please always use scripts/checkpatch.pl before sending out patches. It would have pointed out that this line is incorrect and needs to be fixed up :( thanks, greg k-h
Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host
On Tue, Mar 30, 2021 at 10:33:51AM -0600, Mathieu Poirier wrote: > On Tue, 30 Mar 2021 at 09:35, Greg KH wrote: > > > > On Tue, Mar 30, 2021 at 09:23:14AM -0600, Mathieu Poirier wrote: > > > On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote: > > > > On 26/03/2021 16:55, Mathieu Poirier wrote: > > > > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote: > > > > > > For a nvhe host, the EL2 must allow the EL1&0 translation > > > > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must > > > > > > be saved/restored over a trip to the guest. Also, before > > > > > > entering the guest, we must flush any trace data if the > > > > > > TRBE was enabled. And we must prohibit the generation > > > > > > of trace while we are in EL1 by clearing the TRFCR_EL1. > > > > > > > > > > > > For vhe, the EL2 must prevent the EL1 access to the Trace > > > > > > Buffer. > > > > > > > > > > > > Cc: Will Deacon > > > > > > Cc: Catalin Marinas > > > > > > Cc: Marc Zyngier > > > > > > Cc: Mark Rutland > > > > > > Cc: Anshuman Khandual > > > > > > Acked-by: Mathieu Poirier > > > > > > Signed-off-by: Suzuki K Poulose > > > > > > --- > > > > > > arch/arm64/include/asm/el2_setup.h | 13 + > > > > > > arch/arm64/include/asm/kvm_arm.h | 2 ++ > > > > > > arch/arm64/include/asm/kvm_host.h | 2 ++ > > > > > > arch/arm64/kernel/hyp-stub.S | 3 ++- > > > > > > arch/arm64/kvm/debug.c | 6 ++--- > > > > > > arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 > > > > > > ++ > > > > > > arch/arm64/kvm/hyp/nvhe/switch.c | 1 + > > > > > > 7 files changed, 65 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > Marc - do you want me to pick up this one? > > > > > > > > I think the kvmarm tree is the best route for this patch, given the > > > > amount > > > > of changes the tree is going through, in the areas this patch > > > > touches. Or else there would be conflicts with merging. And this patch > > > > depends on the patches from this series that were queued. > > > > > > > > Here is the depency tree : > > > > > > > > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued > > > > in > > > > v5.12-rc3) > > > > > > > > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6) > > > > > > > > c) kvm-arm TRBE host support (Patch 7) > > > > > > > > d) TRBE driver support (and the ETE changes) > > > > > > > > > > > > (c) code merge depends on -> (a) + (b) > > > > (d) build (no conflicts) depends on -> (b) > > > > > > > > > > > > Now (d) has an indirect dependency on (c) for operational correctness at > > > > runtime. > > > > So, if : > > > > > > > > kvmarm tree picks up : b + c > > > > coresight tree picksup : b + d > > > > > > > > and if we could ensure the merge order of the trees are in > > > > kvmarm > > > > greg-kh (device-misc tree) (coresight goes via this tree) > > > > > > > > > > Greg's char-misc tree is based on the rc releases rather than next. As > > > such it > > > is a while before other branches like kvmarm get merged, causing all sort > > > of > > > compilation breakage. > > > > My tree can not be based on -next, and neither can any other > > maintainer's tree, as next is composed of maintainer trees :) > > > > Exactly > > > > > we should be fine. > > > > > > > > Additionally, we could rip out the Kconfig changes from the TRBE patch > > > > and add it only at the rc1, once we verify both the trees are in to make > > > > sure the runtime operation dependency is not triggered. > > > > > > > > > > We could also do that but Greg might frown at the tactic, and rightly so. > > > The > > > usual way to work with complex merge dependencies is to proceed in steps, > > > which > > > would mean that all KVM related patches go in the v5.13 merge window. > > > When that > > > is done we add the ETE/TRBE for the v5.14 merge window. I agree that we > > > waste > > > an entire cycle but it guarantees to avoid breaking builds and follows the > > > conventional way to do things. > > > > Or someone creates a single branch with a signed tag and it gets pulled > > into multiple maintainer's trees and never rebased. We've done that > > lots of time, nothing new there. Or everything goes through one tree, > > or you wait a release cycle. > > > > You have 3 choices, pick one :) > > I'm perfectly happy with getting this entire set merged via Marc's > kvmarm tree, as long as you are fine with it. No objection from me at all for this to go that way. thanks, greg k-h
Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host
On Tue, Mar 30, 2021 at 09:23:14AM -0600, Mathieu Poirier wrote: > On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote: > > On 26/03/2021 16:55, Mathieu Poirier wrote: > > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote: > > > > For a nvhe host, the EL2 must allow the EL1&0 translation > > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must > > > > be saved/restored over a trip to the guest. Also, before > > > > entering the guest, we must flush any trace data if the > > > > TRBE was enabled. And we must prohibit the generation > > > > of trace while we are in EL1 by clearing the TRFCR_EL1. > > > > > > > > For vhe, the EL2 must prevent the EL1 access to the Trace > > > > Buffer. > > > > > > > > Cc: Will Deacon > > > > Cc: Catalin Marinas > > > > Cc: Marc Zyngier > > > > Cc: Mark Rutland > > > > Cc: Anshuman Khandual > > > > Acked-by: Mathieu Poirier > > > > Signed-off-by: Suzuki K Poulose > > > > --- > > > > arch/arm64/include/asm/el2_setup.h | 13 + > > > > arch/arm64/include/asm/kvm_arm.h | 2 ++ > > > > arch/arm64/include/asm/kvm_host.h | 2 ++ > > > > arch/arm64/kernel/hyp-stub.S | 3 ++- > > > > arch/arm64/kvm/debug.c | 6 ++--- > > > > arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++ > > > > arch/arm64/kvm/hyp/nvhe/switch.c | 1 + > > > > 7 files changed, 65 insertions(+), 4 deletions(-) > > > > > > > > > > Marc - do you want me to pick up this one? > > > > I think the kvmarm tree is the best route for this patch, given the amount > > of changes the tree is going through, in the areas this patch > > touches. Or else there would be conflicts with merging. And this patch > > depends on the patches from this series that were queued. > > > > Here is the depency tree : > > > > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued in > > v5.12-rc3) > > > > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6) > > > > c) kvm-arm TRBE host support (Patch 7) > > > > d) TRBE driver support (and the ETE changes) > > > > > > (c) code merge depends on -> (a) + (b) > > (d) build (no conflicts) depends on -> (b) > > > > > > Now (d) has an indirect dependency on (c) for operational correctness at > > runtime. > > So, if : > > > > kvmarm tree picks up : b + c > > coresight tree picksup : b + d > > > > and if we could ensure the merge order of the trees are in > > kvmarm > > greg-kh (device-misc tree) (coresight goes via this tree) > > > > Greg's char-misc tree is based on the rc releases rather than next. As such > it > is a while before other branches like kvmarm get merged, causing all sort of > compilation breakage. My tree can not be based on -next, and neither can any other maintainer's tree, as next is composed of maintainer trees :) > > we should be fine. > > > > Additionally, we could rip out the Kconfig changes from the TRBE patch > > and add it only at the rc1, once we verify both the trees are in to make > > sure the runtime operation dependency is not triggered. > > > > We could also do that but Greg might frown at the tactic, and rightly so. The > usual way to work with complex merge dependencies is to proceed in steps, > which > would mean that all KVM related patches go in the v5.13 merge window. When > that > is done we add the ETE/TRBE for the v5.14 merge window. I agree that we waste > an entire cycle but it guarantees to avoid breaking builds and follows the > conventional way to do things. Or someone creates a single branch with a signed tag and it gets pulled into multiple maintainer's trees and never rebased. We've done that lots of time, nothing new there. Or everything goes through one tree, or you wait a release cycle. You have 3 choices, pick one :) thanks, greg k-h
Re: [PATCH v1 1/4] docs: make reporting-issues.rst official and delete reporting-bugs.rst
On Tue, Mar 30, 2021 at 04:13:04PM +0200, Thorsten Leemhuis wrote: > Remove the WIP and two FIXME notes in the text to make it official, as > it's now considered fully ready for consumption. To make sure this > step is okay for people the intent of this change and the latest version > of the text were posted to ksummit-discuss; nobody complained, thus > lets move ahead. > > Add a footer to point out people can contact Thorsten directly in case > they find something to improve in the text. > > Dear reporting-bugs.rst, I'm sorry to tell you, but that makes you fully > obsolete and we thus have to let you go now. Thank you very much for > your service, you in one form or another have been around for a long > time. I'm sure over the years you got read a lot and helped quite a few > people. But it's time to retire now. Rest in peace. > > Signed-off-by: Thorsten Leemhuis > CC: Harry Wei > CC: Alex Shi > CC: Federico Vaga > CC: Greg KH Reviewed-by: Greg Kroah-Hartman
Re: [PATCH v1 1/4] docs: make reporting-issues.rst official and delete reporting-bugs.rst
On Tue, Mar 30, 2021 at 04:13:04PM +0200, Thorsten Leemhuis wrote: > Removing Documentation/admin-guide/reporting-bugs.rst will break links > in some of the translations. I was unsure if simply changing them to > Documentation/admin-guide/reporting-issue.rst was wise, so I didn't > touch anything for now and CCed the maintainers for the Chinese and > Italian translation. I couldn't find one for the Japanse translation. > > Please advice. For completeness, this are the places where things will > break afaics: > > $ grep -ri 'reporting-bugs.rst' Documentation/ > Documentation/translations/zh_CN/SecurityBugs:是有帮助的信息，那就请重温一下admin-guide/reporting-bugs.rst文件中的概述过程。任 > Documentation/translations/zh_CN/process/howto.rst:内核源码主目录中的:ref:`admin-guide/reporting-bugs.rst > ` > Documentation/translations/zh_CN/admin-guide/reporting-issues.rst: > 本文档将取代“Documentation/admin-guide/reporting-bugs.rst”。主要的工作 > Documentation/translations/zh_CN/admin-guide/reporting-issues.rst: > “Documentation/admin-guide/reporting-bugs.rst”中的旧文字非常相似。它和它 > Documentation/translations/it_IT/process/howto.rst:Il file > admin-guide/reporting-bugs.rst nella cartella principale del kernel > Documentation/translations/ja_JP/howto.rst:admin-guide/reporting-bugs.rstはカーネルバグらしいものについてどうレポー Traditionally translations catch up much later on, you shouldn't have to worry about them the authors will clean them up and submit patches for them when they get the chance. thanks, greg k-h
Re: [PATCH 0/4] USB: serial: add support for multi-interface functions
On Tue, Mar 30, 2021 at 04:38:16PM +0200, Johan Hovold wrote: > A single USB function can be implemented using a group of interfaces and > this is for example commonly used for Communication Class devices. > > This series adds support for multi-interface functions to USB serial > core and exports an interface that allows drivers to claim a second > sibling interface. The interface could easily be extended to allow > claiming further interfaces if ever needed. > > The final patch uses the new interface to properly claim both the > control and data interface of Maxlinear/Exar devices. Looks good, thanks for adding this: Reviewed-by: Greg Kroah-Hartman
Re: Compiling kernel-3.4.xxx with gcc-9.x. Need some help.
On Tue, Mar 30, 2021 at 03:23:10PM +0200, Fawad Lateef wrote: > So can I still use kernel-3.4 compiled with gcc-5.5, and boot full > user-space with gcc-9.1? Yes, of course. > I was expecting it to be possible but might not work due to > incompatibility? As I know that when I tried to compile buildroot-2019 > (with latest version of openssl and others) it needs kernel headers > and then I likely can't use 3.4 kernel. buildroot might be different, as that is how you are building your whole system, but there is no dependency on the kernel and userspace to use the same version of the compiler. Otherwise everyone would have to rebuild the world for every time they updated their kernel, this isn't the BSDs :) thanks, greg k-h
Re: [PATCH] crypto: hisilicon - check if debugfs opened
On Tue, Mar 30, 2021 at 08:40:07PM +0800, tanghui20 wrote: > > > On 2021/3/30 20:23, Greg KH wrote: > > On Tue, Mar 30, 2021 at 08:09:46PM +0800, tanghui20 wrote: > > > > > > > > > On 2021/3/28 23:09, Greg KH wrote: > > > > On Sat, Mar 27, 2021 at 04:33:00PM +0800, Hui Tang wrote: > > > > > 'xx_debugfs_init' check if debugfs opened. > > > > > > > > > > Signed-off-by: Hui Tang > > > > > --- > > > > > drivers/crypto/hisilicon/hpre/hpre_main.c | 5 - > > > > > drivers/crypto/hisilicon/qm.c | 3 +++ > > > > > drivers/crypto/hisilicon/sec2/sec_main.c | 5 - > > > > > drivers/crypto/hisilicon/zip/zip_main.c | 3 +++ > > > > > 4 files changed, 14 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c > > > > > b/drivers/crypto/hisilicon/hpre/hpre_main.c > > > > > index c7ab06d..f2605c4 100644 > > > > > --- a/drivers/crypto/hisilicon/hpre/hpre_main.c > > > > > +++ b/drivers/crypto/hisilicon/hpre/hpre_main.c > > > > > @@ -779,6 +779,9 @@ static int hpre_debugfs_init(struct hisi_qm *qm) > > > > > struct device *dev = >pdev->dev; > > > > > int ret; > > > > > > > > > > + if (!debugfs_initialized()) > > > > > + return -ENOENT; > > > > > > > > Why? What does this help with? Why does the code care if debugfs is > > > > running or not? > > > > > > > When !CONFIG_DEBUG_FS, there is no problem if debugfs is not checked, > > > but if checking debugfs, a series of stub functions of debugfs can be > > > skipped and 'xx_debugfs_init' will be return immediately. > > > > And have you measured an actual speed difference for that? I would be > > amazed if you could... > > > > I think what you said makes sense. > I am confused when to use 'debugfs_initialized'. Never, you should not care about that at all. thanks, greg k-h
Re: [PATCH] crypto: hisilicon - check if debugfs opened
On Tue, Mar 30, 2021 at 08:09:46PM +0800, tanghui20 wrote: > > > On 2021/3/28 23:09, Greg KH wrote: > > On Sat, Mar 27, 2021 at 04:33:00PM +0800, Hui Tang wrote: > > > 'xx_debugfs_init' check if debugfs opened. > > > > > > Signed-off-by: Hui Tang > > > --- > > > drivers/crypto/hisilicon/hpre/hpre_main.c | 5 - > > > drivers/crypto/hisilicon/qm.c | 3 +++ > > > drivers/crypto/hisilicon/sec2/sec_main.c | 5 - > > > drivers/crypto/hisilicon/zip/zip_main.c | 3 +++ > > > 4 files changed, 14 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/crypto/hisilicon/hpre/hpre_main.c > > > b/drivers/crypto/hisilicon/hpre/hpre_main.c > > > index c7ab06d..f2605c4 100644 > > > --- a/drivers/crypto/hisilicon/hpre/hpre_main.c > > > +++ b/drivers/crypto/hisilicon/hpre/hpre_main.c > > > @@ -779,6 +779,9 @@ static int hpre_debugfs_init(struct hisi_qm *qm) > > > struct device *dev = >pdev->dev; > > > int ret; > > > > > > + if (!debugfs_initialized()) > > > + return -ENOENT; > > > > Why? What does this help with? Why does the code care if debugfs is > > running or not? > > > When !CONFIG_DEBUG_FS, there is no problem if debugfs is not checked, > but if checking debugfs, a series of stub functions of debugfs can be > skipped and 'xx_debugfs_init' will be return immediately. And have you measured an actual speed difference for that? I would be amazed if you could... thanks, greg k-h
Re: [PATCH v26 07/13] mm/damon: Implement a debugfs-based user space interface
On Tue, Mar 30, 2021 at 09:05:31AM +, sj38.p...@gmail.com wrote:
> +static int __init __damon_dbgfs_init(void)
> +{
> + struct dentry *dbgfs_root;
> + const char * const file_names[] = {"monitor_on"};
> + const struct file_operations *fops[] = {_on_fops};
> + int i;
> +
> + dbgfs_root = debugfs_create_dir("damon", NULL);
> +
> + for (i = 0; i < ARRAY_SIZE(file_names); i++)
> + debugfs_create_file(file_names[i], 0600, dbgfs_root, NULL,
> + fops[i]);
> + dbgfs_fill_ctx_dir(dbgfs_root, dbgfs_ctxs[0]);
> +
> + dbgfs_dirs = kmalloc_array(1, sizeof(dbgfs_root), GFP_KERNEL);
No error checking for memory allocation failures?
> + dbgfs_dirs[0] = dbgfs_root;
> +
> + return 0;
> +}
> +
> +/*
> + * Functions for the initialization
> + */
> +
> +static int __init damon_dbgfs_init(void)
> +{
> + int rc;
> +
> + dbgfs_ctxs = kmalloc(sizeof(*dbgfs_ctxs), GFP_KERNEL);
No error checking?
> + dbgfs_ctxs[0] = dbgfs_new_ctx();
> + if (!dbgfs_ctxs[0])
> + return -ENOMEM;
> + dbgfs_nr_ctxs = 1;
> +
> + rc = __damon_dbgfs_init();
> + if (rc)
> + pr_err("%s: dbgfs init failed\n", __func__);
Shouldn't the error be printed out in the function that failed, not in
this one?
thanks,
greg k-h
