All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] soundwire: qcom: Update error prints to debug prints
@ 2022-07-13 12:22 Srinivasa Rao Mandadapu
  2022-07-13 13:56   ` Andrew Halaney
  2022-07-13 14:23 ` Pierre-Louis Bossart
  0 siblings, 2 replies; 9+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-07-13 12:22 UTC (permalink / raw)
  To: vkoul, agross, bjorn.andersson, lgirdwood, broonie, robh+dt,
	quic_plai, bgoswami, perex, tiwai, srinivas.kandagatla,
	quic_rohkumar, linux-arm-msm, alsa-devel, devicetree,
	linux-kernel, swboyd, judyhsiao
  Cc: Srinivasa Rao Mandadapu

Upadte error prints to debug prints to avoid redundant logging in kernel
boot time, as these prints are informative prints in irq handler.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
 drivers/soundwire/qcom.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 9df970e..a4293d0 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 				break;
 			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
 			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
-				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
-					__func__);
+				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);
 				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
 				if (swrm->slave_status == slave_status) {
-					dev_err(swrm->dev, "Slave status not changed %x\n",
+					dev_dbg(swrm->dev, "Slave status not changed %x\n",
 						slave_status);
 				} else {
 					qcom_swrm_get_device_status(swrm);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
  2022-07-13 12:22 [PATCH] soundwire: qcom: Update error prints to debug prints Srinivasa Rao Mandadapu
@ 2022-07-13 13:56   ` Andrew Halaney
  2022-07-13 14:23 ` Pierre-Louis Bossart
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Halaney @ 2022-07-13 13:56 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: vkoul, agross, bjorn.andersson, lgirdwood, broonie, robh+dt,
	quic_plai, bgoswami, perex, tiwai, srinivas.kandagatla,
	quic_rohkumar, linux-arm-msm, alsa-devel, devicetree,
	linux-kernel, swboyd, judyhsiao

A couple of drive by nits:

On Wed, Jul 13, 2022 at 05:52:01PM +0530, Srinivasa Rao Mandadapu wrote:
> Upadte error prints to debug prints to avoid redundant logging in kernel
> boot time, as these prints are informative prints in irq handler.

s/Upadte/Update/

> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
>  drivers/soundwire/qcom.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 9df970e..a4293d0 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  				break;
>  			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>  			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
> -					__func__);
> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);

There's no need for __func__ usage with dev_dbg() when giving +f flag
when enabling adds this for you!

With those changes feel free to add:

    Reviewed-by: Andrew Halaney <ahalaney@redhat.com>

Thanks,
Andrew

>  				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
>  				if (swrm->slave_status == slave_status) {
> -					dev_err(swrm->dev, "Slave status not changed %x\n",
> +					dev_dbg(swrm->dev, "Slave status not changed %x\n",
>  						slave_status);
>  				} else {
>  					qcom_swrm_get_device_status(swrm);
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
@ 2022-07-13 13:56   ` Andrew Halaney
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Halaney @ 2022-07-13 13:56 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: devicetree, alsa-devel, quic_rohkumar, linux-arm-msm, broonie,
	tiwai, lgirdwood, robh+dt, bjorn.andersson, vkoul, agross,
	srinivas.kandagatla, bgoswami, quic_plai, swboyd, judyhsiao,
	linux-kernel

A couple of drive by nits:

On Wed, Jul 13, 2022 at 05:52:01PM +0530, Srinivasa Rao Mandadapu wrote:
> Upadte error prints to debug prints to avoid redundant logging in kernel
> boot time, as these prints are informative prints in irq handler.

s/Upadte/Update/

> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
>  drivers/soundwire/qcom.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 9df970e..a4293d0 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  				break;
>  			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>  			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
> -					__func__);
> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);

There's no need for __func__ usage with dev_dbg() when giving +f flag
when enabling adds this for you!

With those changes feel free to add:

    Reviewed-by: Andrew Halaney <ahalaney@redhat.com>

Thanks,
Andrew

>  				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
>  				if (swrm->slave_status == slave_status) {
> -					dev_err(swrm->dev, "Slave status not changed %x\n",
> +					dev_dbg(swrm->dev, "Slave status not changed %x\n",
>  						slave_status);
>  				} else {
>  					qcom_swrm_get_device_status(swrm);
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
  2022-07-13 12:22 [PATCH] soundwire: qcom: Update error prints to debug prints Srinivasa Rao Mandadapu
  2022-07-13 13:56   ` Andrew Halaney
@ 2022-07-13 14:23 ` Pierre-Louis Bossart
  2022-07-13 14:52   ` Srinivasa Rao Mandadapu
  1 sibling, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2022-07-13 14:23 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, vkoul, agross, bjorn.andersson,
	lgirdwood, broonie, robh+dt, quic_plai, bgoswami, perex, tiwai,
	srinivas.kandagatla, quic_rohkumar, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel, swboyd, judyhsiao



On 7/13/22 07:22, Srinivasa Rao Mandadapu wrote:
> Upadte error prints to debug prints to avoid redundant logging in kernel

update

> boot time, as these prints are informative prints in irq handler.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
>  drivers/soundwire/qcom.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 9df970e..a4293d0 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>  				break;
>  			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>  			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
> -					__func__);
> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);

any reason why the rate limitation was dropped?

>  				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
>  				if (swrm->slave_status == slave_status) {
> -					dev_err(swrm->dev, "Slave status not changed %x\n",
> +					dev_dbg(swrm->dev, "Slave status not changed %x\n",
>  						slave_status);
>  				} else {
>  					qcom_swrm_get_device_status(swrm);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
  2022-07-13 13:56   ` Andrew Halaney
@ 2022-07-13 14:51     ` Srinivasa Rao Mandadapu
  -1 siblings, 0 replies; 9+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-07-13 14:51 UTC (permalink / raw)
  To: Andrew Halaney
  Cc: vkoul, agross, bjorn.andersson, lgirdwood, broonie, robh+dt,
	quic_plai, bgoswami, perex, tiwai, srinivas.kandagatla,
	quic_rohkumar, linux-arm-msm, alsa-devel, devicetree,
	linux-kernel, swboyd, judyhsiao


On 7/13/2022 7:26 PM, Andrew Halaney wrote:
Thanks for your time Andrew.
> A couple of drive by nits:
>
> On Wed, Jul 13, 2022 at 05:52:01PM +0530, Srinivasa Rao Mandadapu wrote:
>> Upadte error prints to debug prints to avoid redundant logging in kernel
>> boot time, as these prints are informative prints in irq handler.
> s/Upadte/Update/
Okay. Will fix it.
>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>>   drivers/soundwire/qcom.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>> index 9df970e..a4293d0 100644
>> --- a/drivers/soundwire/qcom.c
>> +++ b/drivers/soundwire/qcom.c
>> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>>   				break;
>>   			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>>   			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
>> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
>> -					__func__);
>> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);
> There's no need for __func__ usage with dev_dbg() when giving +f flag
> when enabling adds this for you!
Okay. Will remove __func__ and change dev_dbg() to dev_dbg_ratelimited().
>
> With those changes feel free to add:
>
>      Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
>
> Thanks,
> Andrew
>
>>   				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
>>   				if (swrm->slave_status == slave_status) {
>> -					dev_err(swrm->dev, "Slave status not changed %x\n",
>> +					dev_dbg(swrm->dev, "Slave status not changed %x\n",
>>   						slave_status);
>>   				} else {
>>   					qcom_swrm_get_device_status(swrm);
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
@ 2022-07-13 14:51     ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 9+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-07-13 14:51 UTC (permalink / raw)
  To: Andrew Halaney
  Cc: devicetree, alsa-devel, quic_rohkumar, linux-arm-msm, broonie,
	tiwai, lgirdwood, robh+dt, bjorn.andersson, vkoul, agross,
	srinivas.kandagatla, bgoswami, quic_plai, swboyd, judyhsiao,
	linux-kernel


On 7/13/2022 7:26 PM, Andrew Halaney wrote:
Thanks for your time Andrew.
> A couple of drive by nits:
>
> On Wed, Jul 13, 2022 at 05:52:01PM +0530, Srinivasa Rao Mandadapu wrote:
>> Upadte error prints to debug prints to avoid redundant logging in kernel
>> boot time, as these prints are informative prints in irq handler.
> s/Upadte/Update/
Okay. Will fix it.
>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>>   drivers/soundwire/qcom.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>> index 9df970e..a4293d0 100644
>> --- a/drivers/soundwire/qcom.c
>> +++ b/drivers/soundwire/qcom.c
>> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>>   				break;
>>   			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>>   			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
>> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
>> -					__func__);
>> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);
> There's no need for __func__ usage with dev_dbg() when giving +f flag
> when enabling adds this for you!
Okay. Will remove __func__ and change dev_dbg() to dev_dbg_ratelimited().
>
> With those changes feel free to add:
>
>      Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
>
> Thanks,
> Andrew
>
>>   				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
>>   				if (swrm->slave_status == slave_status) {
>> -					dev_err(swrm->dev, "Slave status not changed %x\n",
>> +					dev_dbg(swrm->dev, "Slave status not changed %x\n",
>>   						slave_status);
>>   				} else {
>>   					qcom_swrm_get_device_status(swrm);
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
  2022-07-13 14:23 ` Pierre-Louis Bossart
@ 2022-07-13 14:52   ` Srinivasa Rao Mandadapu
  2022-07-13 16:01     ` Joe Perches
  0 siblings, 1 reply; 9+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-07-13 14:52 UTC (permalink / raw)
  To: Pierre-Louis Bossart, vkoul, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, quic_plai, bgoswami, perex, tiwai,
	srinivas.kandagatla, quic_rohkumar, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel, swboyd, judyhsiao


On 7/13/2022 7:53 PM, Pierre-Louis Bossart wrote:
Thanks for your time Pierre-Louis!!!
>
> On 7/13/22 07:22, Srinivasa Rao Mandadapu wrote:
>> Upadte error prints to debug prints to avoid redundant logging in kernel
> update
Okay. Will change it.
>
>> boot time, as these prints are informative prints in irq handler.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>>   drivers/soundwire/qcom.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>> index 9df970e..a4293d0 100644
>> --- a/drivers/soundwire/qcom.c
>> +++ b/drivers/soundwire/qcom.c
>> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>>   				break;
>>   			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>>   			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
>> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
>> -					__func__);
>> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);
> any reason why the rate limitation was dropped?
No Specific Reason as such. Will add ratelimited and re-spin it.
>
>>   				swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status);
>>   				if (swrm->slave_status == slave_status) {
>> -					dev_err(swrm->dev, "Slave status not changed %x\n",
>> +					dev_dbg(swrm->dev, "Slave status not changed %x\n",
>>   						slave_status);
>>   				} else {
>>   					qcom_swrm_get_device_status(swrm);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
  2022-07-13 14:52   ` Srinivasa Rao Mandadapu
@ 2022-07-13 16:01     ` Joe Perches
  2022-07-13 16:45       ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2022-07-13 16:01 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, Pierre-Louis Bossart, vkoul, agross,
	bjorn.andersson, lgirdwood, broonie, robh+dt, quic_plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, quic_rohkumar,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao

On Wed, 2022-07-13 at 20:22 +0530, Srinivasa Rao Mandadapu wrote:
> On 7/13/2022 7:53 PM, Pierre-Louis Bossart wrote:
> Thanks for your time Pierre-Louis!!!
> > 
> > On 7/13/22 07:22, Srinivasa Rao Mandadapu wrote:
> > > Upadte error prints to debug prints to avoid redundant logging in kernel
> > update
[]
> > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
[]
> > > @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
> > >   				break;
> > >   			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
> > >   			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
> > > -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
> > > -					__func__);
> > > +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);

Could also drop the "%s: ", __func__ as it's already a unique message
and dynamic debug could add it back if really desired.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] soundwire: qcom: Update error prints to debug prints
  2022-07-13 16:01     ` Joe Perches
@ 2022-07-13 16:45       ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 9+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-07-13 16:45 UTC (permalink / raw)
  To: Joe Perches, Pierre-Louis Bossart, vkoul, agross,
	bjorn.andersson, lgirdwood, broonie, robh+dt, quic_plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, quic_rohkumar,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao


On 7/13/2022 9:31 PM, Joe Perches wrote:
Thanks for your time Joe!!!
> On Wed, 2022-07-13 at 20:22 +0530, Srinivasa Rao Mandadapu wrote:
>> On 7/13/2022 7:53 PM, Pierre-Louis Bossart wrote:
>> Thanks for your time Pierre-Louis!!!
>>> On 7/13/22 07:22, Srinivasa Rao Mandadapu wrote:
>>>> Upadte error prints to debug prints to avoid redundant logging in kernel
>>> update
> []
>>>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> []
>>>> @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
>>>>    				break;
>>>>    			case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED:
>>>>    			case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS:
>>>> -				dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n",
>>>> -					__func__);
>>>> +				dev_dbg(swrm->dev, "%s: SWR new slave attached\n", __func__);
> Could also drop the "%s: ", __func__ as it's already a unique message
> and dynamic debug could add it back if really desired.
Okay. It's removed and posted v2 patch.
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-07-14 16:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13 12:22 [PATCH] soundwire: qcom: Update error prints to debug prints Srinivasa Rao Mandadapu
2022-07-13 13:56 ` Andrew Halaney
2022-07-13 13:56   ` Andrew Halaney
2022-07-13 14:51   ` Srinivasa Rao Mandadapu
2022-07-13 14:51     ` Srinivasa Rao Mandadapu
2022-07-13 14:23 ` Pierre-Louis Bossart
2022-07-13 14:52   ` Srinivasa Rao Mandadapu
2022-07-13 16:01     ` Joe Perches
2022-07-13 16:45       ` Srinivasa Rao Mandadapu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.