Re: [PATCH]: drivers: staging: most: Fixed styling issue.

2021-02-08 Thread Greg KH
On Tue, Feb 09, 2021 at 11:53:11AM +0530, Mukul Mehar wrote:

> >From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
> From: Mukul Mehar 
> Date: Mon, 8 Feb 2021 01:03:06 +0530
> Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.

Why is this still an attached file?  These lines should not show up in
the body of the email.  Look at the thousands of examples on the mailing
list as what needs to be done here.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH]: drivers: staging: most: Fixed styling issue.

2021-02-08 Thread Mukul Mehar
>From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
From: Mukul Mehar 
Date: Mon, 8 Feb 2021 01:03:06 +0530
Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.

This patch fixes a warning, of the line ending with a '(',
generated by checkpatch.pl.

Signed-off-by: Mukul Mehar 
---
 drivers/staging/most/sound/sound.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 3a1a59058042..4dd1bf95d1ce 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -228,12 +228,12 @@ static int playback_thread(void *data)
 		struct mbo *mbo = NULL;
 		bool period_elapsed = false;
 
-		wait_event_interruptible(
-			channel->playback_waitq,
-			kthread_should_stop() ||
-			(channel->is_stream_running &&
-			 (mbo = most_get_mbo(channel->iface, channel->id,
-	 ;
+		wait_event_interruptible(channel->playback_waitq,
+	 kthread_should_stop() ||
+	 (channel->is_stream_running &&
+	 (mbo = most_get_mbo(channel->iface,
+	 channel->id,
+	 ;
 		if (!mbo)
 			continue;
 
-- 
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: hikey9xx: fix checkpatch error and warning

2021-02-08 Thread Greg KH
On Tue, Feb 09, 2021 at 11:27:04AM +0530, Atul Gopinathan wrote:
> Fix the following types of checkpatch error and warning:
> 
> ERROR: code indent should use tabs where possible
> WARNING: struct phy_ops should normally be const

That is 2 different things, which means this should be 2 different
patches.  Please make this a patch series and resend.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: hikey9xx: fix checkpatch error and warning

2021-02-08 Thread Atul Gopinathan
Fix the following types of checkpatch error and warning:

ERROR: code indent should use tabs where possible
WARNING: struct phy_ops should normally be const

Signed-off-by: Atul Gopinathan 
---
 drivers/staging/hikey9xx/hi6421-spmi-pmic.c | 2 +-
 drivers/staging/hikey9xx/hi6421v600-regulator.c | 2 +-
 drivers/staging/hikey9xx/phy-hi3670-usb3.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c 
b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
index 2301f4fcd48d..9c5e113e1a81 100644
--- a/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
+++ b/drivers/staging/hikey9xx/hi6421-spmi-pmic.c
@@ -177,7 +177,7 @@ static void hi6421_spmi_pmic_irq_init(struct 
hi6421_spmi_pmic *ddata)
 
for (i = 0; i < HISI_IRQ_ARRAY; i++)
regmap_write(ddata->regmap, SOC_PMIC_IRQ_MASK_0_ADDR + i,
-   HISI_MASK);
+   HISI_MASK);
 
for (i = 0; i < HISI_IRQ_ARRAY; i++) {
regmap_read(ddata->regmap, SOC_PMIC_IRQ0_ADDR + i, );
diff --git a/drivers/staging/hikey9xx/hi6421v600-regulator.c 
b/drivers/staging/hikey9xx/hi6421v600-regulator.c
index c801bb840962..f6a14e9c3cbf 100644
--- a/drivers/staging/hikey9xx/hi6421v600-regulator.c
+++ b/drivers/staging/hikey9xx/hi6421v600-regulator.c
@@ -106,7 +106,7 @@ static int hi6421_spmi_regulator_enable(struct 
regulator_dev *rdev)
 
ret = regmap_update_bits(pmic->regmap, rdev->desc->enable_reg,
 rdev->desc->enable_mask,
-rdev->desc->enable_mask);
+rdev->desc->enable_mask);
 
/* Avoid powering up multiple devices at the same time */
usleep_range(rdev->desc->off_on_delay, rdev->desc->off_on_delay + 60);
diff --git a/drivers/staging/hikey9xx/phy-hi3670-usb3.c 
b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
index 8918f3665f8e..e7e579ce0302 100644
--- a/drivers/staging/hikey9xx/phy-hi3670-usb3.c
+++ b/drivers/staging/hikey9xx/phy-hi3670-usb3.c
@@ -585,7 +585,7 @@ static int hi3670_phy_exit(struct phy *phy)
return ret;
 }
 
-static struct phy_ops hi3670_phy_ops = {
+static const struct phy_ops hi3670_phy_ops = {
.init   = hi3670_phy_init,
.exit   = hi3670_phy_exit,
.owner  = THIS_MODULE,
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fix ignoring return value warning

2021-02-08 Thread Youling Tang

Hi, Dan


On 02/09/2021 03:02 AM, Dan Carpenter wrote:

On Mon, Feb 08, 2021 at 04:06:18PM +0100, Sascha Hauer wrote:

Hi Dan,

On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:

On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:

Fix the below ignoring return value warning for device_reset.

drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
of function declared with 'warn_unused_result' attribute [-Wunused-result]
 device_reset(>dev);
 ^~~~ ~~
drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
of function declared with 'warn_unused_result' attribute [-Wunused-result]
 device_reset(>dev);
 ^~~~ ~~


We can't really do this sort of fix without the hardware to test it.
This could be the correct fix or perhaps switching to device_reset_optional()
is the correct fix.  We can't know unless we have the hardware to test.

When device_reset() is the wrong function then adding a return value
check will turn this into a runtime error for those who have the
hardware which will hopefully trigger them to tell us why reset_device
is wrong for them.
At least for a staging driver I find this procedure opportune.


That seems like sort of a jerk move...  What's the rush?  Someone will
eventually be able to test this if we just wait around for a bit.
Otherwise if no one has the hardware then eventually the driver will be
deleted.

regards,
dan carpenter

We do not have the relevant hardware to test, this is just to solve a
compile-time warning.

Thanks,
Youling.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: rtl8723bs: fix braces for os_dep/mlme_linux.c

2021-02-08 Thread Phillip Potter
Add braces to both branches of an if block for consistency, and also
remove braces from a single line for loop. Fixes a checkpatch check
and warning, thus clearing this file of any brace check/warning
notices.

Signed-off-by: Phillip Potter 
---
 drivers/staging/rtl8723bs/os_dep/mlme_linux.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/mlme_linux.c 
b/drivers/staging/rtl8723bs/os_dep/mlme_linux.c
index fb2df871c0cb..d46c65ab384b 100644
--- a/drivers/staging/rtl8723bs/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/mlme_linux.c
@@ -48,8 +48,9 @@ void rtw_os_indicate_connect(struct adapter *adapter)
if ((check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE) == true) ||
(check_fwstate(pmlmepriv, WIFI_ADHOC_STATE) == true)) {
rtw_cfg80211_ibss_indicate_connect(adapter);
-   } else
+   } else {
rtw_cfg80211_indicate_connect(adapter);
+   }
 
rtw_indicate_wx_assoc_event(adapter);
netif_carrier_on(adapter->pnetdev);
@@ -163,9 +164,8 @@ void rtw_report_sec_ie(struct adapter *adapter, u8 
authmode, u8 *sec_ie)
len = sec_ie[1] + 2;
len = (len < IW_CUSTOM_MAX) ? len : IW_CUSTOM_MAX;
 
-   for (i = 0; i < len; i++) {
+   for (i = 0; i < len; i++)
p += sprintf(p, "%02x", sec_ie[i]);
-   }
 
p += sprintf(p, ")");
 
-- 
2.29.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: rtl8723bs: remove braces from two single line if blocks

2021-02-08 Thread Phillip Potter
Remove braces from both occurences of single line if blocks in
include/rtw_mlme.h, fixes two checkpatch warnings, thus clearing
this type of warning from this file.

Also swaps two if statement comparisons around, so the variable is on
the left in each one. This fixes two warnings also.

Signed-off-by: Phillip Potter 
---
 drivers/staging/rtl8723bs/include/rtw_mlme.h | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/include/rtw_mlme.h 
b/drivers/staging/rtl8723bs/include/rtw_mlme.h
index ea0dd156051e..d8655cb619a1 100644
--- a/drivers/staging/rtl8723bs/include/rtw_mlme.h
+++ b/drivers/staging/rtl8723bs/include/rtw_mlme.h
@@ -524,18 +524,16 @@ static inline void set_fwstate(struct mlme_priv 
*pmlmepriv, sint state)
 {
pmlmepriv->fw_state |= state;
/* FOR HW integration */
-   if (_FW_UNDER_SURVEY == state) {
+   if (state == _FW_UNDER_SURVEY)
pmlmepriv->bScanInProcess = true;
-   }
 }
 
 static inline void _clr_fwstate_(struct mlme_priv *pmlmepriv, sint state)
 {
pmlmepriv->fw_state &= ~state;
/* FOR HW integration */
-   if (_FW_UNDER_SURVEY == state) {
+   if (state == _FW_UNDER_SURVEY)
pmlmepriv->bScanInProcess = false;
-   }
 }
 
 /*
-- 
2.29.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v8 11/14] spmi: hisi-spmi-controller: move driver from staging

2021-02-08 Thread Stephen Boyd
Quoting Mauro Carvalho Chehab (2021-01-29 11:51:57)
> The Hisilicon 6421v600 SPMI driver is ready for mainstream.
> 
> So, move it from staging.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---

Acked-by: Stephen Boyd 

Rob had some comments on the binding that don't look to be addressed
though so I'd prefer we get the binding into shape before graduating
this driver from staging.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Randy Dunlap
On 2/8/21 11:51 AM, Mukul Mehar wrote:

More comments (probably duplicates):

a. The "checkpatch:" in the subject says that you are making a patch
to the "checkpatch" script. You are not doing that. The subject should
be more like: "staging: most: fix a style issue in sound.c" e.g.

b. This comment:
This is my first patch.

should not be in the comment message. If included at all, it should be
after the "---" line, like so:

---
This is my first patch.

.


thanks.
-- 
~Randy

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

2021-02-08 Thread Guenter Roeck
On 2/5/21 12:08 PM, Bjorn Helgaas wrote:
> On Fri, Feb 05, 2021 at 10:28:32AM -0800, Guenter Roeck wrote:
>> On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote:
>>> From: Junhao He 
>>>
>>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
>>> settings from Kconfig when traversing subdirectories.
>>>
>>> Suggested-by: Bjorn Helgaas 
>>> Signed-off-by: Junhao He 
>>> Signed-off-by: Yicong Yang 
>>
>> What problem does this fix ? Maybe I am missing it, but I don't see
>> DEBUG being used in a subdirectory of drivers/hwmon.
> 
> It's my fault for raising this question [1].  Yicong fixed a real
> problem in drivers/pci, where we are currently using
> 
>   ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
> 
> so CONFIG_PCI_DEBUG=y turns on debug in drivers/pci, but not in the
> subdirectories.  That's surprising to users.
> 
> So my question was whether we should default to using subdir-ccflags
> for -DDEBUG in general, and only use ccflags when we have
> subdirectories that have their own debug options, e.g.,
> 
>   drivers/i2c/Makefile:ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG
>   drivers/i2c/algos/Makefile:ccflags-$(CONFIG_I2C_DEBUG_ALGO) := -DDEBUG
>   drivers/i2c/busses/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
>   drivers/i2c/muxes/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> 
> I mentioned drivers/hwmon along with a few others that have
> subdirectories, do not have per-subdirectory debug options, and use
> ccflags.  I didn't try to determine whether those subdirectories
> currently use -DDEBUG.
> 
> In the case of drivers/hwmon, several drivers do use pr_debug(),
> and CONFIG_HWMON_DEBUG_CHIP=y turns those on.  But if somebody
> were to add pr_debug() to drivers/hwmon/occ/common.c, for example,
> CONFIG_HWMON_DEBUG_CHIP=y would *not* turn it on.  That sounds
> surprising to me, but if that's what you intend, that's totally fine.
> 

That does make sense, but that explanation is missing from the
description.

Guenter

> [1] https://lore.kernel.org/r/20210204161048.GA68790@bjorn-Precision-5520
> 
>>> ---
>>>  drivers/hwmon/Makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index 09a86c5..1c0c089 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
>>>  obj-$(CONFIG_SENSORS_OCC)  += occ/
>>>  obj-$(CONFIG_PMBUS)+= pmbus/
>>>  
>>> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>>> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>>>  
>>> -- 
>>> 2.8.1
>>>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus

2021-02-08 Thread Nicolas Saenz Julienne
On Mon, 2021-01-18 at 13:32 +0100, Nicolas Saenz Julienne wrote:
> Adds support to control the PWM bus available in official Raspberry Pi
> PoE HAT. Only RPi's co-processor has access to it, so commands have to
> be sent through RPi's firmware mailbox interface.
> 
> Signed-off-by: Nicolas Saenz Julienne 
> 
> ---

ping :)

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH AUTOSEL 5.10 14/36] media: rkisp1: uapi: change hist_bins array type from __u16 to __u32

2021-02-08 Thread Hans Verkuil
On 08/02/2021 18:57, Sasha Levin wrote:
> From: Dafna Hirschfeld 
> 
> [ Upstream commit 31f190e0ccac8b75d33fdc95a797c526cf9b149e ]
> 
> Each entry in the array is a 20 bits value composed of 16 bits unsigned
> integer and 4 bits fractional part. So the type should change to __u32.
> In addition add a documentation of how the measurements are done.

Dafna, Helen, does it make sense at all to backport these three patches to
when rkisp1 was a staging driver?

I would be inclined not to backport this.

Regards,

Hans

> 
> Signed-off-by: Dafna Hirschfeld 
> Acked-by: Helen Koike 
> Signed-off-by: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 
> Signed-off-by: Sasha Levin 
> ---
>  drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h 
> b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> index 432cb6be55b47..c19fe059c2442 100644
> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> @@ -848,13 +848,18 @@ struct rkisp1_cif_isp_af_stat {
>  /**
>   * struct rkisp1_cif_isp_hist_stat - statistics histogram data
>   *
> - * @hist_bins: measured bin counters
> + * @hist_bins: measured bin counters. Each bin is a 20 bits unsigned fixed 
> point
> + *  type. Bits 0-4 are the fractional part and bits 5-19 are the
> + *  integer part.
>   *
> - * Measurement window divided into 25 sub-windows, set
> - * with ISP_HIST_XXX
> + * The window of the measurements area is divided to 5x5 sub-windows. The
> + * histogram is then computed for each sub-window independently and the final
> + * result is a weighted average of the histogram measurements on all
> + * sub-windows. The window of the measurements area and the weight of each
> + * sub-window are configurable using struct @rkisp1_cif_isp_hst_config.
>   */
>  struct rkisp1_cif_isp_hist_stat {
> - __u16 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
> + __u32 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
>  };
>  
>  /**
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Greg KH
On Tue, Feb 09, 2021 at 01:21:16AM +0530, Mukul Mehar wrote:

> >From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
> From: Mukul Mehar 
> Date: Mon, 8 Feb 2021 01:03:06 +0530
> Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.
> 
> This patch fixes a warning, of the line ending with a '(',
> generated by checkpatch.pl.
> This is my first patch.
> 
> Signed-off-by: Mukul Mehar 
> ---
>  drivers/staging/most/sound/sound.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/most/sound/sound.c 
> b/drivers/staging/most/sound/sound.c
> index 3a1a59058042..4dd1bf95d1ce 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -228,12 +228,12 @@ static int playback_thread(void *data)
>   struct mbo *mbo = NULL;
>   bool period_elapsed = false;
>  
> - wait_event_interruptible(
> - channel->playback_waitq,
> - kthread_should_stop() ||
> - (channel->is_stream_running &&
> -  (mbo = most_get_mbo(channel->iface, channel->id,
> -  ;
> + wait_event_interruptible(channel->playback_waitq,
> +  kthread_should_stop() ||
> +  (channel->is_stream_running &&
> +  (mbo = most_get_mbo(channel->iface,
> +  channel->id,
> +  ;
>   if (!mbo)
>   continue;
>  
> -- 
> 2.25.1


Again, all of the things that my bot found last time are still relevant
here.  Please fix them all.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Mukul Mehar
>From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
From: Mukul Mehar 
Date: Mon, 8 Feb 2021 01:03:06 +0530
Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.

This patch fixes a warning, of the line ending with a '(',
generated by checkpatch.pl.
This is my first patch.

Signed-off-by: Mukul Mehar 
---
 drivers/staging/most/sound/sound.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 3a1a59058042..4dd1bf95d1ce 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -228,12 +228,12 @@ static int playback_thread(void *data)
 		struct mbo *mbo = NULL;
 		bool period_elapsed = false;
 
-		wait_event_interruptible(
-			channel->playback_waitq,
-			kthread_should_stop() ||
-			(channel->is_stream_running &&
-			 (mbo = most_get_mbo(channel->iface, channel->id,
-	 ;
+		wait_event_interruptible(channel->playback_waitq,
+	 kthread_should_stop() ||
+	 (channel->is_stream_running &&
+	 (mbo = most_get_mbo(channel->iface,
+	 channel->id,
+	 ;
 		if (!mbo)
 			continue;
 
-- 
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fix ignoring return value warning

2021-02-08 Thread Dan Carpenter
On Mon, Feb 08, 2021 at 04:06:18PM +0100, Sascha Hauer wrote:
> Hi Dan,
> 
> On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:
> > On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> > > Fix the below ignoring return value warning for device_reset.
> > > 
> > > drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return 
> > > value
> > > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > > device_reset(>dev);
> > > ^~~~ ~~
> > > drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return 
> > > value
> > > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > > device_reset(>dev);
> > > ^~~~ ~~
> > > 
> > 
> > We can't really do this sort of fix without the hardware to test it.
> > This could be the correct fix or perhaps switching to 
> > device_reset_optional()
> > is the correct fix.  We can't know unless we have the hardware to test.
> 
> When device_reset() is the wrong function then adding a return value
> check will turn this into a runtime error for those who have the
> hardware which will hopefully trigger them to tell us why reset_device
> is wrong for them.
> At least for a staging driver I find this procedure opportune.
> 

That seems like sort of a jerk move...  What's the rush?  Someone will
eventually be able to test this if we just wait around for a bit.
Otherwise if no one has the hardware then eventually the driver will be
deleted.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Greg KH
On Mon, Feb 08, 2021 at 11:58:02PM +0530, Mukul Mehar wrote:

> >From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
> From: Mukul Mehar 
> Date: Mon, 8 Feb 2021 01:03:06 +0530
> Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.
> 
> This patch fixes a warning, of the line ending with a '(',
> generated by checkpatch.pl.
> This is my first patch.
> 
> Signed-off-by: Mukul Mehar 
> ---
>  drivers/staging/most/sound/sound.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)



You ignored everything my bot sent last time, why, do you somehow think
it was not correct?  {hint, it was...}

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Mukul Mehar
>From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
From: Mukul Mehar 
Date: Mon, 8 Feb 2021 01:03:06 +0530
Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.

This patch fixes a warning, of the line ending with a '(',
generated by checkpatch.pl.
This is my first patch.

Signed-off-by: Mukul Mehar 
---
 drivers/staging/most/sound/sound.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 3a1a59058042..4dd1bf95d1ce 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -228,12 +228,12 @@ static int playback_thread(void *data)
 		struct mbo *mbo = NULL;
 		bool period_elapsed = false;
 
-		wait_event_interruptible(
-			channel->playback_waitq,
-			kthread_should_stop() ||
-			(channel->is_stream_running &&
-			 (mbo = most_get_mbo(channel->iface, channel->id,
-	 ;
+		wait_event_interruptible(channel->playback_waitq,
+	 kthread_should_stop() ||
+	 (channel->is_stream_running &&
+	 (mbo = most_get_mbo(channel->iface,
+	 channel->id,
+	 ;
 		if (!mbo)
 			continue;
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH AUTOSEL 5.10 16/36] media: rkisp1: stats: mask the hist_bins values

2021-02-08 Thread Sasha Levin
From: Dafna Hirschfeld 

[ Upstream commit a802a0430b863f03bc01aaea2d2bf6ff464f03e7 ]

hist_bins is an array of type __u32. Each entry represents
a 20 bit value. So mask out the unused bits.

Signed-off-by: Dafna Hirschfeld 
Acked-by: Helen Koike 
Signed-off-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Sasha Levin 
---
 drivers/staging/media/rkisp1/rkisp1-regs.h  | 1 +
 drivers/staging/media/rkisp1/rkisp1-stats.c | 8 +---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-regs.h 
b/drivers/staging/media/rkisp1/rkisp1-regs.h
index 049f6c3a11df5..5819e58b2e557 100644
--- a/drivers/staging/media/rkisp1/rkisp1-regs.h
+++ b/drivers/staging/media/rkisp1/rkisp1-regs.h
@@ -365,6 +365,7 @@
 #define RKISP1_CIF_ISP_MAX_HIST_PREDIVIDER 0x007F
 #define RKISP1_CIF_ISP_HIST_ROW_NUM5
 #define RKISP1_CIF_ISP_HIST_COLUMN_NUM 5
+#define RKISP1_CIF_ISP_HIST_GET_BIN(x) ((x) & 0x000F)
 
 /* AUTO FOCUS MEASUREMENT:  ISP_AFM_CTRL */
 #define RKISP1_ISP_AFM_CTRL_ENABLE BIT(0)
diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c 
b/drivers/staging/media/rkisp1/rkisp1-stats.c
index 8fdf646c4b75b..e52ba9b154e90 100644
--- a/drivers/staging/media/rkisp1/rkisp1-stats.c
+++ b/drivers/staging/media/rkisp1/rkisp1-stats.c
@@ -251,9 +251,11 @@ static void rkisp1_stats_get_hst_meas(struct rkisp1_stats 
*stats,
unsigned int i;
 
pbuf->meas_type |= RKISP1_CIF_ISP_STAT_HIST;
-   for (i = 0; i < RKISP1_CIF_ISP_HIST_BIN_N_MAX; i++)
-   pbuf->params.hist.hist_bins[i] =
-   rkisp1_read(rkisp1, RKISP1_CIF_ISP_HIST_BIN_0 + i * 4);
+   for (i = 0; i < RKISP1_CIF_ISP_HIST_BIN_N_MAX; i++) {
+   u32 reg_val = rkisp1_read(rkisp1, RKISP1_CIF_ISP_HIST_BIN_0 + i 
* 4);
+
+   pbuf->params.hist.hist_bins[i] = 
RKISP1_CIF_ISP_HIST_GET_BIN(reg_val);
+   }
 }
 
 static void rkisp1_stats_get_bls_meas(struct rkisp1_stats *stats,
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH AUTOSEL 5.10 15/36] media: rkisp1: stats: remove a wrong cast to u8

2021-02-08 Thread Sasha Levin
From: Dafna Hirschfeld 

[ Upstream commit a76f8dc8be471028540df24749e99a3ec0ac7c94 ]

hist_bins is an array of type __u32. Each entry represent
a 20 bit fixed point value as documented inline.
The cast to u8 when setting the values is wrong. Remove it.

Signed-off-by: Dafna Hirschfeld 
Reviewed-by: Heiko Stuebner 
Acked-by: Helen Koike 
Signed-off-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Sasha Levin 
---
 drivers/staging/media/rkisp1/rkisp1-stats.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c 
b/drivers/staging/media/rkisp1/rkisp1-stats.c
index 51c64f75fe29a..8fdf646c4b75b 100644
--- a/drivers/staging/media/rkisp1/rkisp1-stats.c
+++ b/drivers/staging/media/rkisp1/rkisp1-stats.c
@@ -253,8 +253,7 @@ static void rkisp1_stats_get_hst_meas(struct rkisp1_stats 
*stats,
pbuf->meas_type |= RKISP1_CIF_ISP_STAT_HIST;
for (i = 0; i < RKISP1_CIF_ISP_HIST_BIN_N_MAX; i++)
pbuf->params.hist.hist_bins[i] =
-   (u8)rkisp1_read(rkisp1,
-   RKISP1_CIF_ISP_HIST_BIN_0 + i * 4);
+   rkisp1_read(rkisp1, RKISP1_CIF_ISP_HIST_BIN_0 + i * 4);
 }
 
 static void rkisp1_stats_get_bls_meas(struct rkisp1_stats *stats,
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH AUTOSEL 5.10 14/36] media: rkisp1: uapi: change hist_bins array type from __u16 to __u32

2021-02-08 Thread Sasha Levin
From: Dafna Hirschfeld 

[ Upstream commit 31f190e0ccac8b75d33fdc95a797c526cf9b149e ]

Each entry in the array is a 20 bits value composed of 16 bits unsigned
integer and 4 bits fractional part. So the type should change to __u32.
In addition add a documentation of how the measurements are done.

Signed-off-by: Dafna Hirschfeld 
Acked-by: Helen Koike 
Signed-off-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Sasha Levin 
---
 drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h 
b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
index 432cb6be55b47..c19fe059c2442 100644
--- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
+++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
@@ -848,13 +848,18 @@ struct rkisp1_cif_isp_af_stat {
 /**
  * struct rkisp1_cif_isp_hist_stat - statistics histogram data
  *
- * @hist_bins: measured bin counters
+ * @hist_bins: measured bin counters. Each bin is a 20 bits unsigned fixed 
point
+ *type. Bits 0-4 are the fractional part and bits 5-19 are the
+ *integer part.
  *
- * Measurement window divided into 25 sub-windows, set
- * with ISP_HIST_XXX
+ * The window of the measurements area is divided to 5x5 sub-windows. The
+ * histogram is then computed for each sub-window independently and the final
+ * result is a weighted average of the histogram measurements on all
+ * sub-windows. The window of the measurements area and the weight of each
+ * sub-window are configurable using struct @rkisp1_cif_isp_hst_config.
  */
 struct rkisp1_cif_isp_hist_stat {
-   __u16 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
+   __u32 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
 };
 
 /**
-- 
2.27.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fix ignoring return value warning

2021-02-08 Thread Sascha Hauer
Hi Dan,

On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:
> On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> > Fix the below ignoring return value warning for device_reset.
> > 
> > drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > device_reset(>dev);
> > ^~~~ ~~
> > drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return 
> > value
> > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > device_reset(>dev);
> > ^~~~ ~~
> > 
> 
> We can't really do this sort of fix without the hardware to test it.
> This could be the correct fix or perhaps switching to device_reset_optional()
> is the correct fix.  We can't know unless we have the hardware to test.

When device_reset() is the wrong function then adding a return value
check will turn this into a runtime error for those who have the
hardware which will hopefully trigger them to tell us why reset_device
is wrong for them.
At least for a staging driver I find this procedure opportune.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Sound issues with the 5.10.x kernel (alsa)

2021-02-08 Thread Diederik de Haas
On maandag 8 februari 2021 15:00:26 CET Ryutaroh Matsumoto wrote:
> > With kernel version 5.10.9 (linux-image-5.10.0-2-arm64) it all seemed
> > fixed, but returned with 5.10.12 (linux-image-5.10.0-3-arm64) and is also
> > present with 5.10.13.
> 
> The difference in Debian kernel 5.10.9 was that vc4.ko was disabled by
> mistake. I guess that module_blacklist=vc4 in kernel command line
> suppresses the symptom.
> All other Debian kernel 5.10.x have vc4.ko enabled in arm64.

Indeed, you're correct! Thanks.
I haven't tried blacklisting it, but IIUC that means it doesn't work in 5.10 
with vc4 at all (on arm64 at least).

I've now setup another RPi3B+ ~ the same as my rpi-mpd, but / moved to a 
PiDrive and created 52Gi swap partition to build the upstream kernel (+config) 
and subsequently one with the requested patches.

diederik@rpi-mpd:~$ git diff /boot/config-5.10.0-2-arm64 /boot/config-5.10.0-3-
arm64
diff --git a/boot/config-5.10.0-2-arm64 b/boot/config-5.10.0-3-arm64
index 35d6f98..e8eafc9 100644
--- a/boot/config-5.10.0-2-arm64
+++ b/boot/config-5.10.0-3-arm64
@@ -1,6 +1,6 @@
 #
 # Automatically generated file; DO NOT EDIT.
-# Linux/arm64 5.10.9 Kernel Configuration
+# Linux/arm64 5.10.13 Kernel Configuration
 #
 CONFIG_CC_VERSION_TEXT="gcc-10 (Debian 10.2.1-6) 10.2.1 20210110"
 CONFIG_CC_IS_GCC=y
@@ -23,7 +23,7 @@ CONFIG_INIT_ENV_ARG_LIMIT=32
 # CONFIG_COMPILE_TEST is not set
 CONFIG_LOCALVERSION=""
 # CONFIG_LOCALVERSION_AUTO is not set
-CONFIG_BUILD_SALT="5.10.0-2-arm64"
+CONFIG_BUILD_SALT="5.10.0-3-arm64"
 CONFIG_DEFAULT_INIT=""
 CONFIG_DEFAULT_HOSTNAME="(none)"
 CONFIG_SWAP=y
@@ -737,7 +737,7 @@ CONFIG_CRYPTO_AES_ARM64_CE_BLK=m
 CONFIG_CRYPTO_AES_ARM64_NEON_BLK=m
 CONFIG_CRYPTO_CHACHA20_NEON=m
 CONFIG_CRYPTO_POLY1305_NEON=m
-# CONFIG_CRYPTO_NHPOLY1305_NEON is not set
+CONFIG_CRYPTO_NHPOLY1305_NEON=m
 CONFIG_CRYPTO_AES_ARM64_BS=m
 
 #
@@ -3339,7 +3339,7 @@ CONFIG_USB_IPHETH=m
 CONFIG_USB_SIERRA_NET=m
 CONFIG_USB_VL600=m
 CONFIG_USB_NET_CH9200=m
-# CONFIG_USB_NET_AQC111 is not set
+CONFIG_USB_NET_AQC111=m
 CONFIG_WLAN=y
 # CONFIG_WIRELESS_WDS is not set
 CONFIG_WLAN_VENDOR_ADMTEK=y
@@ -5937,7 +5937,6 @@ CONFIG_DRM_AMDGPU_USERPTR=y
 # Display Engine Configuration
 #
 CONFIG_DRM_AMD_DC=y
-CONFIG_DRM_AMD_DC_DCN=y
 CONFIG_DRM_AMD_DC_HDCP=y
 CONFIG_DRM_AMD_DC_SI=y
 # end of Display Engine Configuration
@@ -6097,7 +6096,8 @@ CONFIG_DRM_DW_HDMI_I2S_AUDIO=m
 CONFIG_DRM_DW_MIPI_DSI=m
 # end of Display Interface Bridges
 
-# CONFIG_DRM_VC4 is not set
+CONFIG_DRM_VC4=m
+CONFIG_DRM_VC4_HDMI_CEC=y
 # CONFIG_DRM_ETNAVIV is not set
 # CONFIG_DRM_ARCPGU is not set
 CONFIG_DRM_HISI_HIBMC=m
@@ -9531,8 +9531,6 @@ CONFIG_CRYPTO_WP512=m
 #
 CONFIG_CRYPTO_AES=m
 CONFIG_CRYPTO_AES_TI=m
-CONFIG_CRYPTO_ANUBIS=m
-CONFIG_CRYPTO_ARC4=m
 CONFIG_CRYPTO_BLOWFISH=m
 CONFIG_CRYPTO_BLOWFISH_COMMON=m
 CONFIG_CRYPTO_CAMELLIA=m
@@ -9541,13 +9539,10 @@ CONFIG_CRYPTO_CAST5=m
 CONFIG_CRYPTO_CAST6=m
 CONFIG_CRYPTO_DES=m
 CONFIG_CRYPTO_FCRYPT=m
-CONFIG_CRYPTO_KHAZAD=m
 CONFIG_CRYPTO_SALSA20=m
 CONFIG_CRYPTO_CHACHA20=m
-CONFIG_CRYPTO_SEED=m
 CONFIG_CRYPTO_SERPENT=m
 # CONFIG_CRYPTO_SM4 is not set
-CONFIG_CRYPTO_TEA=m
 CONFIG_CRYPTO_TWOFISH=m
 CONFIG_CRYPTO_TWOFISH_COMMON=m
 
@@ -9577,7 +9572,7 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=m
 CONFIG_CRYPTO_USER_API_RNG=m
 # CONFIG_CRYPTO_USER_API_RNG_CAVP is not set
 CONFIG_CRYPTO_USER_API_AEAD=m
-CONFIG_CRYPTO_USER_API_ENABLE_OBSOLETE=y
+# CONFIG_CRYPTO_USER_API_ENABLE_OBSOLETE is not set
 # CONFIG_CRYPTO_STATS is not set
 CONFIG_CRYPTO_HASH_INFO=y



signature.asc
Description: This is a digitally signed message part.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: wfx: fix possible panic with re-queued frames

2021-02-08 Thread Jerome Pouiller
From: Jérôme Pouiller 

When the firmware rejects a frame (because station become asleep or
disconnected), the frame is re-queued in mac80211. However, the
re-queued frame was 8 bytes longer than the original one (the size of
the ICV for the encryption). So, when mac80211 try to send this frame
again, it is a little bigger than expected.
If the frame is re-queued secveral time it end with a skb_over_panic
because the skb buffer is not large enough.

Note it only happens when device acts as an AP and encryption is
enabled.

This patch more or less reverts the commit 049fde130419 ("staging: wfx:
drop useless field from struct wfx_tx_priv").

Fixes: 049fde130419 ("staging: wfx: drop useless field from struct wfx_tx_priv")
Signed-off-by: Jérôme Pouiller 
---
 drivers/staging/wfx/data_tx.c | 10 +-
 drivers/staging/wfx/data_tx.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 36b36ef39d05..77fb104efdec 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -331,6 +331,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct 
ieee80211_sta *sta,
 {
struct hif_msg *hif_msg;
struct hif_req_tx *req;
+   struct wfx_tx_priv *tx_priv;
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
struct ieee80211_key_conf *hw_key = tx_info->control.hw_key;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
@@ -344,11 +345,14 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct 
ieee80211_sta *sta,
 
// From now tx_info->control is unusable
memset(tx_info->rate_driver_data, 0, sizeof(struct wfx_tx_priv));
+   // Fill tx_priv
+   tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
+   tx_priv->icv_size = wfx_tx_get_icv_len(hw_key);
 
// Fill hif_msg
WARN(skb_headroom(skb) < wmsg_len, "not enough space in skb");
WARN(offset & 1, "attempt to transmit an unaligned frame");
-   skb_put(skb, wfx_tx_get_icv_len(hw_key));
+   skb_put(skb, tx_priv->icv_size);
skb_push(skb, wmsg_len);
memset(skb->data, 0, wmsg_len);
hif_msg = (struct hif_msg *)skb->data;
@@ -484,6 +488,7 @@ static void wfx_tx_fill_rates(struct wfx_dev *wdev,
 
 void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)
 {
+   const struct wfx_tx_priv *tx_priv;
struct ieee80211_tx_info *tx_info;
struct wfx_vif *wvif;
struct sk_buff *skb;
@@ -495,6 +500,7 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct 
hif_cnf_tx *arg)
return;
}
tx_info = IEEE80211_SKB_CB(skb);
+   tx_priv = wfx_skb_tx_priv(skb);
wvif = wdev_to_wvif(wdev, ((struct hif_msg *)skb->data)->interface);
WARN_ON(!wvif);
if (!wvif)
@@ -503,6 +509,8 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct 
hif_cnf_tx *arg)
// Note that wfx_pending_get_pkt_us_delay() get data from tx_info
_trace_tx_stats(arg, skb, wfx_pending_get_pkt_us_delay(wdev, skb));
wfx_tx_fill_rates(wdev, tx_info, arg);
+   skb_trim(skb, skb->len - tx_priv->icv_size);
+
// From now, you can touch to tx_info->status, but do not touch to
// tx_priv anymore
// FIXME: use ieee80211_tx_info_clear_status()
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index 46c9fff7a870..401363d6b563 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -35,6 +35,7 @@ struct tx_policy_cache {
 
 struct wfx_tx_priv {
ktime_t xmit_timestamp;
+   unsigned char icv_size;
 };
 
 void wfx_tx_policy_init(struct wfx_vif *wvif);
-- 
2.30.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fix ignoring return value warning

2021-02-08 Thread Sascha Hauer
On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> Fix the below ignoring return value warning for device_reset.
> 
> drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(>dev);
> ^~~~ ~~
> drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return 
> value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(>dev);
> ^~~~ ~~
> 
> Signed-off-by: Youling Tang 
> ---
>  drivers/staging/mt7621-dma/mtk-hsdma.c| 6 +-
>  drivers/staging/ralink-gdma/ralink-gdma.c | 6 +-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c 
> b/drivers/staging/mt7621-dma/mtk-hsdma.c
> index bc4bb43..d4ffa52 100644
> --- a/drivers/staging/mt7621-dma/mtk-hsdma.c
> +++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
> @@ -682,7 +682,11 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
>   return ret;
>   }
>  
> - device_reset(>dev);
> + ret = device_reset(>dev);
> + if (ret) {
> + dev_err(>dev, "failed to reset device\n");
> + return ret;
> + }

Normally you don't want to see this error message when -EPROBE_DEFER is
returned because that would mean the reset controller is not yet
available and the driver should probe later again. There is
dev_err_probe() now for exactly this usecase.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[staging:staging-next 67/73] drivers/staging/hikey9xx/hi6421-spmi-pmic.c:238 hi6421_spmi_pmic_probe() warn: 'ddata->irq' not released on lines: 238.

2021-02-08 Thread Dan Carpenter
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
staging-next
head:   06b0c0dce88e2aa2f01343db0f26d214d7f264a0
commit: a2e904fc59e15d9e4128415579a2664ab3a1ed14 [67/73] staging: hikey9xx: 
hi6421-spmi-pmic: cleanup probe code
config: x86_64-randconfig-m001-20210207 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 
Reported-by: Dan Carpenter 

New smatch warnings:
drivers/staging/hikey9xx/hi6421-spmi-pmic.c:238 hi6421_spmi_pmic_probe() warn: 
'ddata->irq' not released on lines: 238.

Old smatch warnings:
drivers/staging/hikey9xx/hi6421-spmi-pmic.c:52 hi6421_spmi_irq_handler() error: 
uninitialized symbol 'offset'.

vim +238 drivers/staging/hikey9xx/hi6421-spmi-pmic.c

1eb2784a90925d Mauro Carvalho Chehab 2020-08-17  164  static int 
hi6421_spmi_pmic_probe(struct spmi_device *pdev)
4524ac56cdcabf Mayulong  2020-08-17  165  {
4524ac56cdcabf Mayulong  2020-08-17  166struct device *dev = 
>dev;
4524ac56cdcabf Mayulong  2020-08-17  167struct device_node *np 
= dev->of_node;
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  168struct hi6421_spmi_pmic 
*ddata;
4524ac56cdcabf Mayulong  2020-08-17  169unsigned int virq;
6b946699252c68 Mauro Carvalho Chehab 2020-08-17  170int ret, i;
4524ac56cdcabf Mayulong  2020-08-17  171  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  172ddata = 
devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  173if (!ddata)
4524ac56cdcabf Mayulong  2020-08-17  174return -ENOMEM;
4524ac56cdcabf Mayulong  2020-08-17  175  
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  176ddata->regmap = 
devm_regmap_init_spmi_ext(pdev, _config);
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  177if 
(IS_ERR(ddata->regmap))
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  178return 
PTR_ERR(ddata->regmap);
fb02e3ebfb2da2 Mauro Carvalho Chehab 2021-01-29  179  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  180
spin_lock_init(>lock);
4524ac56cdcabf Mayulong  2020-08-17  181  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  182ddata->dev = dev;
4524ac56cdcabf Mayulong  2020-08-17  183  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  184ddata->gpio = 
of_get_gpio(np, 0);
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  185if (ddata->gpio < 0)
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  186return 
ddata->gpio;
4524ac56cdcabf Mayulong  2020-08-17  187  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  188if 
(!gpio_is_valid(ddata->gpio))
4524ac56cdcabf Mayulong  2020-08-17  189return -EINVAL;
4524ac56cdcabf Mayulong  2020-08-17  190  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  191ret = 
devm_gpio_request_one(dev, ddata->gpio, GPIOF_IN, "pmic");
4524ac56cdcabf Mayulong  2020-08-17  192if (ret < 0) {
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  193dev_err(dev, 
"Failed to request gpio%d\n", ddata->gpio);
4524ac56cdcabf Mayulong  2020-08-17  194return ret;
4524ac56cdcabf Mayulong  2020-08-17  195}
4524ac56cdcabf Mayulong  2020-08-17  196  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  197ddata->irq = 
gpio_to_irq(ddata->gpio);
4524ac56cdcabf Mayulong  2020-08-17  198  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  199
hi6421_spmi_pmic_irq_prc(ddata);
4524ac56cdcabf Mayulong  2020-08-17  200  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  201ddata->irqs = 
devm_kzalloc(dev, HISI_IRQ_NUM * sizeof(int), GFP_KERNEL);
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  202if (!ddata->irqs)
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  203return -ENOMEM;
4524ac56cdcabf Mayulong  2020-08-17  204  
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  205ddata->domain = 
irq_domain_add_simple(np, HISI_IRQ_NUM, 0,
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  206
 _spmi_domain_ops, ddata);
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  207if (!ddata->domain) {
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  208dev_err(dev, 
"Failed to create IRQ domain\n");
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  209return -ENODEV;
4524ac56cdcabf Mayulong  2020-08-17  210}
4524ac56cdcabf Mayulong  2020-08-17  211  
b240d0143bfbc9 Mauro Carvalho Chehab 2020-08-18  212for (i = 0; i < 
HISI_IRQ_NUM; i++) {
fcd732406c5d65 Mauro Carvalho Chehab 2021-01-29  213virq = 
irq_create_mapping(ddata->domain, i);
6b946699252c68 Mauro Carvalho Chehab 2020-08-17  214if (!virq) {
a2e904fc59e15d Mauro Carvalho Chehab 2021-01-29  

Re: [PATCH] staging: fix ignoring return value warning

2021-02-08 Thread Dan Carpenter
On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> Fix the below ignoring return value warning for device_reset.
> 
> drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(>dev);
> ^~~~ ~~
> drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return 
> value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(>dev);
> ^~~~ ~~
> 

We can't really do this sort of fix without the hardware to test it.
This could be the correct fix or perhaps switching to device_reset_optional()
is the correct fix.  We can't know unless we have the hardware to test.

People think silencing warnings is good, but it's actually bad.  The
warning is there to show us a potential bug.  If we silence the warning
without fixing the bug, then we it's like when your mom tells you to
clean up the room and you instead just switch off the light.  It doesn't
fix the problem, it only makes it harder to find.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Sound issues with the 5.10.x kernel (alsa)

2021-02-08 Thread Diederik de Haas
On maandag 8 februari 2021 13:22:56 CET Stefan Wahren wrote:
> > TL;DR: I have a RPi 3B+ running pure Debian Bullseye arm64 (~from
> > raspi.debian.net), named rpi-mpd, connected via HDMI cable to my AV
> > Receiver.
> can you please confirm that the bcm2835-audio driver causing the issues?

How can I confirm that?

> > Playing music worked fine with kernel 5.9, but with 5.10.2-1 the music
> > (quality) became not good to quite horribly, because of (static) noise and
> > distortion.
> > With kernel version 5.10.9 (linux-image-5.10.0-2-arm64) it all seemed
> > fixed, but returned with 5.10.12 (linux-image-5.10.0-3-arm64) and is also
> > present with 5.10.13.
> 
> I cannot explain why 5.10.9 works for you, but we had multiple
> regressions in 5.10. Currently i cannot see any of the fixes by Phil
> Elwell in linux-stable. Maybe they won't apply and needs to be backport
> manually.

FTR: All the kernel versions I mentioned are the Debian kernel versions (and 
thus also Debian's config).

> Just for reference here are the revelant patches:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/d
> rivers/staging/vc04_services?h=next-20210205=96ae327678eceabf455b11a88ba1
> 4ad540d4b046
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/d
> rivers/staging/vc04_services?h=next-20210205=88753cc19f087abe0d39644b844e
> 67a59cfb5a3d
> 
> Could you please try?

I'll try. But it may take a while as I don't know how to (properly) do that.
And I'm guessing that building on a RPi3B+ isn't exactly fast. But I assume 
I'm able to 'overcome' all that.

signature.asc
Description: This is a digitally signed message part.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fieldbus: arcx-anybus: constify static structs

2021-02-08 Thread Sven Van Asbroeck
Hi Rikard, thank you for the contribution.

On Sun, Feb 7, 2021 at 3:25 PM Rikard Falkeborn
 wrote:
>
> Constify two static structs which are never modified, to allow the
> compiler to put them in read-only memory.
>
> The only usage of controller_attribute_group is to put its address in an
> array of pointers to const struct attribute_group, and the only usage of
> can_power_ops is to assign its address to the 'ops' field in the
> regulator_desc struct, which is a pointer to const struct regulator_ops.
>
> Signed-off-by: Rikard Falkeborn 

Reviewed-by: Sven Van Asbroeck 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Greg KH
On Mon, Feb 08, 2021 at 06:36:14PM +0530, Mukul Mehar wrote:

> >From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
> From: Mukul Mehar 
> Date: Mon, 8 Feb 2021 01:03:06 +0530
> Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.
> Signed-off-by: Mukul Mehar 
> 
> 
> This patch fixes a warning, of the line ending with a '(',
> generated by checkpatch.pl.
> This is my first patch.
> ---
>  drivers/staging/most/sound/sound.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/most/sound/sound.c 
> b/drivers/staging/most/sound/sound.c
> index 3a1a59058042..4dd1bf95d1ce 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -228,12 +228,12 @@ static int playback_thread(void *data)
>   struct mbo *mbo = NULL;
>   bool period_elapsed = false;
>  
> - wait_event_interruptible(
> - channel->playback_waitq,
> - kthread_should_stop() ||
> - (channel->is_stream_running &&
> -  (mbo = most_get_mbo(channel->iface, channel->id,
> -  ;
> + wait_event_interruptible(channel->playback_waitq,
> +  kthread_should_stop() ||
> +  (channel->is_stream_running &&
> +  (mbo = most_get_mbo(channel->iface,
> +  channel->id,
> +  ;
>   if (!mbo)
>   continue;
>  
> -- 
> 2.25.1
> 

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 was attached, please place it inline so that it can be
  applied directly from the email message itself.

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/SubmittingPatches and resend it after
  adding that line.  Note, the line needs to be in the body of the
  email, before the patch, not at the bottom of the patch or in the
  email signature.

- 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.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag

2021-02-08 Thread Yicong Yang
On 2021/2/8 18:47, Greg KH wrote:
> On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote:
>> Hi Greg,
>>
>> On 2021/2/5 17:53, Greg KH wrote:
>>> On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
 From: Junhao He 

 Use subdir-ccflags-* instead of ccflags-* to inherit the debug
 settings from Kconfig when traversing subdirectories.
>>>
>>> That says what you do, but not _why_ you are doing it.
>>
>> i'll illustrate the reason and reword the commit.
>>
>>>
>>> What does this offer in benefit of the existing way?  What is it fixing?
>>> Why do this "churn"?
>>
>> currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the 
>> Makefile
>> of driver/base and driver/base/power, but not in the subdirectory
>> driver/base/firmware_loader. we cannot turn the debug on for subdirectory
>> firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
>> for the it.
> 
> Is that necessary?  Does that directory need it?

there are several debug prints in firmware_loader/main.c:

./main.c:207:   pr_debug("%s: fw-%s fw_priv=%p\n", __func__, fw_name, fw_priv);
./main.c:245:   pr_debug("batched request - sharing the same 
struct fw_priv and lookup for multiple requests\n");
./main.c:269:   pr_debug("%s: fw-%s fw_priv=%p data=%p size=%u\n",
./main.c:594:   pr_debug("%s: fw-%s fw_priv=%p data=%p size=%u\n",
./main.c:605:   pr_debug("%s: fw_name-%s devm-%p released\n",
./main.c:1175:  pr_debug("%s: %s\n", __func__, fw_name);
./main.c:1181:  pr_debug("%s: %s ret=%d\n", __func__, fw_name, ret);
./main.c:1214:  pr_debug("%s: %s\n", __func__, fw_name);
./main.c:1272:  pr_debug("%s: fw: %s\n", __func__, name);
./main.c:1389:  pr_debug("%s\n", __func__);
./main.c:1415:  pr_debug("%s\n", __func__);


> 
> thanks,
> 
> greg k-h
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH]: checkpatch: Fixed styling issue

2021-02-08 Thread Mukul Mehar
>From 29bcaf0066003983da29b1e026b985c0727b091a Mon Sep 17 00:00:00 2001
From: Mukul Mehar 
Date: Mon, 8 Feb 2021 01:03:06 +0530
Subject: [PATCH] Drivers: staging: most: sound: Fixed style issue.
Signed-off-by: Mukul Mehar 


This patch fixes a warning, of the line ending with a '(',
generated by checkpatch.pl.
This is my first patch.
---
 drivers/staging/most/sound/sound.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 3a1a59058042..4dd1bf95d1ce 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -228,12 +228,12 @@ static int playback_thread(void *data)
 		struct mbo *mbo = NULL;
 		bool period_elapsed = false;
 
-		wait_event_interruptible(
-			channel->playback_waitq,
-			kthread_should_stop() ||
-			(channel->is_stream_running &&
-			 (mbo = most_get_mbo(channel->iface, channel->id,
-	 ;
+		wait_event_interruptible(channel->playback_waitq,
+	 kthread_should_stop() ||
+	 (channel->is_stream_running &&
+	 (mbo = most_get_mbo(channel->iface,
+	 channel->id,
+	 ;
 		if (!mbo)
 			continue;
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Sound issues with the 5.10.x kernel (alsa)

2021-02-08 Thread Stefan Wahren
Hi Diederik,

Am 08.02.21 um 12:47 schrieb Diederik de Haas:
> Hi,
>
> This is basically me forwarding the bug I reported on Debian's BTS:
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=978025
>
> TL;DR: I have a RPi 3B+ running pure Debian Bullseye arm64 (~from 
> raspi.debian.net), named rpi-mpd, connected via HDMI cable to my AV Receiver.
can you please confirm that the bcm2835-audio driver causing the issues?
> Playing music worked fine with kernel 5.9, but with 5.10.2-1 the music 
> (quality) became not good to quite horribly, because of (static) noise and 
> distortion.
> With kernel version 5.10.9 (linux-image-5.10.0-2-arm64) it all seemed fixed, 
> but returned with 5.10.12 (linux-image-5.10.0-3-arm64) and is also present 
> with 5.10.13.

I cannot explain why 5.10.9 works for you, but we had multiple
regressions in 5.10. Currently i cannot see any of the fixes by Phil
Elwell in linux-stable. Maybe they won't apply and needs to be backport
manually.

Just for reference here are the revelant patches:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/staging/vc04_services?h=next-20210205=96ae327678eceabf455b11a88ba14ad540d4b046

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/staging/vc04_services?h=next-20210205=88753cc19f087abe0d39644b844e67a59cfb5a3d

Could you please try?

Best regards
Stefan

>
> I can consistently reproduce this issue, rebooting into 5.10.9 and playing 
> music is fine again, so I *think* this is not a local issue.
> This ML is the closest I can think of as 'upstream', so hopefully more 
> insight  
> can be accomplished here ... and a fix.
>
> Cheers,
>   Diederik
>
> ___
> linux-rpi-kernel mailing list
> linux-rpi-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: octeon: remove braces from single-line block

2021-02-08 Thread Phillip Potter
On Mon, Feb 08, 2021 at 08:14:02AM +0100, Alexander Sverdlin wrote:
> Hi!
> 
> On 06/02/2021 21:17, Phillip Potter wrote:
> > This removes the braces from the if statement that checks the
> > physical node return value in cvm_oct_phy_setup_device, as this
> > block contains only one statement. Fixes a style warning.
> > 
> > Signed-off-by: Phillip Potter 
> 
> Reviewed-by: Alexander Sverdlin 
> 

Thank you Alexander.

> > ---
> >  drivers/staging/octeon/ethernet-mdio.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/octeon/ethernet-mdio.c 
> > b/drivers/staging/octeon/ethernet-mdio.c
> > index 0bf545849b11..b0fd083a5bf2 100644
> > --- a/drivers/staging/octeon/ethernet-mdio.c
> > +++ b/drivers/staging/octeon/ethernet-mdio.c
> > @@ -146,9 +146,8 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
> > goto no_phy;
> >  
> > phy_node = of_parse_phandle(priv->of_node, "phy-handle", 0);
> > -   if (!phy_node && of_phy_is_fixed_link(priv->of_node)) {
> > +   if (!phy_node && of_phy_is_fixed_link(priv->of_node))
> > phy_node = of_node_get(priv->of_node);
> > -   }
> > if (!phy_node)
> > goto no_phy;
> >  
> 
> -- 
> Best regards,
> Alexander Sverdlin.

Regards,
Phil Potter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

2021-02-08 Thread Yicong Yang
On 2021/2/6 4:08, Bjorn Helgaas wrote:
> On Fri, Feb 05, 2021 at 10:28:32AM -0800, Guenter Roeck wrote:
>> On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote:
>>> From: Junhao He 
>>>
>>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
>>> settings from Kconfig when traversing subdirectories.
>>>
>>> Suggested-by: Bjorn Helgaas 
>>> Signed-off-by: Junhao He 
>>> Signed-off-by: Yicong Yang 
>>
>> What problem does this fix ? Maybe I am missing it, but I don't see
>> DEBUG being used in a subdirectory of drivers/hwmon.
> 
> It's my fault for raising this question [1].  Yicong fixed a real
> problem in drivers/pci, where we are currently using
> 
>   ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
> 
> so CONFIG_PCI_DEBUG=y turns on debug in drivers/pci, but not in the
> subdirectories.  That's surprising to users.
> 
> So my question was whether we should default to using subdir-ccflags
> for -DDEBUG in general, and only use ccflags when we have
> subdirectories that have their own debug options, e.g.,
> 
>   drivers/i2c/Makefile:ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG
>   drivers/i2c/algos/Makefile:ccflags-$(CONFIG_I2C_DEBUG_ALGO) := -DDEBUG
>   drivers/i2c/busses/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
>   drivers/i2c/muxes/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> 
> I mentioned drivers/hwmon along with a few others that have
> subdirectories, do not have per-subdirectory debug options, and use
> ccflags.  I didn't try to determine whether those subdirectories
> currently use -DDEBUG.
> 
> In the case of drivers/hwmon, several drivers do use pr_debug(),
> and CONFIG_HWMON_DEBUG_CHIP=y turns those on.  But if somebody
> were to add pr_debug() to drivers/hwmon/occ/common.c, for example,
> CONFIG_HWMON_DEBUG_CHIP=y would *not* turn it on.  That sounds
> surprising to me, but if that's what you intend, that's totally fine.

i thought CONFIG_HWMON_DEBUG_CHIP=y means to enable debug including the
subdirectories, so use subdir-ccflags-* will make sure the debug
message on in the subdirectories, if there will be.

please let me know if i understand wrong.

Thanks,
Yicong

> 
> [1] https://lore.kernel.org/r/20210204161048.GA68790@bjorn-Precision-5520
> 
>>> ---
>>>  drivers/hwmon/Makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index 09a86c5..1c0c089 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
>>>  obj-$(CONFIG_SENSORS_OCC)  += occ/
>>>  obj-$(CONFIG_PMBUS)+= pmbus/
>>>  
>>> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>>> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>>>  
>>> -- 
>>> 2.8.1
>>>
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag

2021-02-08 Thread Greg KH
On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote:
> Hi Greg,
> 
> On 2021/2/5 17:53, Greg KH wrote:
> > On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
> >> From: Junhao He 
> >>
> >> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
> >> settings from Kconfig when traversing subdirectories.
> > 
> > That says what you do, but not _why_ you are doing it.
> 
> i'll illustrate the reason and reword the commit.
> 
> > 
> > What does this offer in benefit of the existing way?  What is it fixing?
> > Why do this "churn"?
> 
> currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the 
> Makefile
> of driver/base and driver/base/power, but not in the subdirectory
> driver/base/firmware_loader. we cannot turn the debug on for subdirectory
> firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
> for the it.

Is that necessary?  Does that directory need it?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag

2021-02-08 Thread Yicong Yang
Hi Greg,

On 2021/2/5 17:53, Greg KH wrote:
> On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
>> From: Junhao He 
>>
>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
>> settings from Kconfig when traversing subdirectories.
> 
> That says what you do, but not _why_ you are doing it.

i'll illustrate the reason and reword the commit.

> 
> What does this offer in benefit of the existing way?  What is it fixing?
> Why do this "churn"?

currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the 
Makefile
of driver/base and driver/base/power, but not in the subdirectory
driver/base/firmware_loader. we cannot turn the debug on for subdirectory
firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
for the it.

as there is no other debug config in the subdirectory of driver/base,
CONFIG_DEBUG_DRIVER may also mean turn on the debug in the subdirectories, so 
use
subdir-ccflags-* instead for the -DDEBUG will allow us to enable debug for all
the subdirectories.

Thanks,
Yicong

> 
> thanks,
> 
> greg k-h
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: emxx_udc: Fix incorrectly defined global

2021-02-08 Thread Geert Uytterhoeven
Hi Kumar,

CC Nishad, Magnus, linux-renesas-soc,

On Sun, Feb 7, 2021 at 8:40 AM Kumar Kartikeya Dwivedi  wrote:
> On Sun, Feb 07, 2021 at 12:04:41PM IST, Stephen Rothwell wrote:
> > Given that drivers/staging/emxx_udc/emxx_udc.h is only included by
> > drivers/staging/emxx_udc/emxx_udc.c, shouldn't these variables just be
> > declared static in emxx_udc.c and removed from emxx_udc.h?
> >
>
> Either would be correct. I went this way because it was originally trying to
> (incorrectly) define a global variable instead. I guess they can be static now
> and when more users are added, the linkage can be adjusted as needed.
>
> Here's another version of the patch:
>
> --
> From 5835ad916ceeba620c85bc32f52ecd69f21f8dab Mon Sep 17 00:00:00 2001
> From: Kumar Kartikeya Dwivedi 
> Date: Sun, 7 Feb 2021 12:53:39 +0530
> Subject: [PATCH] staging: emxx_udc: Make incorrectly defined global static
>
> The global gpio_desc pointer and int vbus_irq were defined in the header,
> instead put the definitions in the translation unit and make them static as
> there's only a single consumer in emxx_udc.c.
>
> This fixes the following sparse warnings for this driver:
> drivers/staging/emxx_udc/emxx_udc.c: note: in included file:
> drivers/staging/emxx_udc/emxx_udc.h:23:18: warning: symbol 'vbus_gpio' was not
> declared. Should it be static?
> drivers/staging/emxx_udc/emxx_udc.h:24:5: warning: symbol 'vbus_irq' was not
> declared. Should it be static?
>
> Signed-off-by: Kumar Kartikeya Dwivedi 

Thanks for your patch!

> --- a/drivers/staging/emxx_udc/emxx_udc.c
> +++ b/drivers/staging/emxx_udc/emxx_udc.c
> @@ -34,6 +34,9 @@
>  #defineDRIVER_DESC "EMXX UDC driver"
>  #defineDMA_ADDR_INVALID(~(dma_addr_t)0)
>
> +static struct gpio_desc *vbus_gpio;
> +static int vbus_irq;

While I agree these must be static, I expect the driver to be still
broken, as vbus_gpio is never set?

Commit 48101806c52203f6 ("Staging: emxx_udc: Switch to the gpio
descriptor interface") introduced both variables, which was presumably
never tested.

Magnus: perhaps we should just remove this driver?

> +
>  static const char  driver_name[] = "emxx_udc";
>  static const char  driver_desc[] = DRIVER_DESC;
>
> diff --git a/drivers/staging/emxx_udc/emxx_udc.h 
> b/drivers/staging/emxx_udc/emxx_udc.h
> index bca614d69..c9e37a1b8 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.h
> +++ b/drivers/staging/emxx_udc/emxx_udc.h
> @@ -20,8 +20,6 @@
>  /* below hacked up for staging integration */
>  #define GPIO_VBUS 0 /* GPIO_P153 on KZM9D */
>  #define INT_VBUS 0 /* IRQ for GPIO_P153 */
> -struct gpio_desc *vbus_gpio;
> -int vbus_irq;

Gr{oetje,eeting}s,

Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re Thanks.

2021-02-08 Thread Mr. Richard Thomas
Dear Friend,
I will be pleased if you can allow me to invest $104M Dollars in
Estate Management,in your company or any area you best that will be
of good profit to both of us

Please do well to respond including your information for more details.

Thanks.
Mr.Richard Thomas
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel