2022-07-13 12:28:58

by Srinivasa Rao Mandadapu

Subject: [PATCH] soundwire: qcom: Update error prints to debug prints

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 <[email protected]>
---
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

2022-07-13 14:53:45

by Andrew Halaney

Subject: Re: [PATCH] soundwire: qcom: Update error prints to debug prints

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 <[email protected]>
> ---
> 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 <[email protected]>

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
>

2022-07-13 15:19:05

by Srinivasa Rao Mandadapu

Subject: Re: [PATCH] soundwire: qcom: Update error prints to debug prints

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
> 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
>>

2022-07-13 16:17:03

by Srinivasa Rao Mandadapu

Subject: Re: [PATCH] soundwire: qcom: Update error prints to debug prints

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 <[email protected]>
>> ---
>> 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);

2022-07-13 16:17:26

by Pierre-Louis Bossart

Subject: Re: [PATCH] soundwire: qcom: Update error prints to debug prints

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 <[email protected]>
> ---
> 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);

2022-07-13 16:24:02

by Joe Perches

Subject: Re: [PATCH] soundwire: qcom: Update error prints to debug prints

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.

2022-07-13 17:00:17

by Srinivasa Rao Mandadapu

Subject: Re: [PATCH] soundwire: qcom: Update error prints to debug prints

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