Re: [PATCH 1/2] soundwire: intel: uniquify debug message

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 6/10/22 00:25, Greg KH wrote:
> On Fri, Jun 10, 2022 at 10:35:36AM +0800, Bard Liao wrote:
>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>
>> The same debug message is replicated multiple time, add __func__ to
>> figure out what link is ignored.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
>> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>> ---
>>  drivers/soundwire/intel.c | 28 ++++++++++++++--------------
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
>> index 505c5ef061e3..808e2f320052 100644
>> --- a/drivers/soundwire/intel.c
>> +++ b/drivers/soundwire/intel.c
>> @@ -1328,8 +1328,8 @@ int intel_link_startup(struct auxiliary_device *auxdev)
>>  
>>  	if (bus->prop.hw_disabled) {
>>  		dev_info(dev,
>> -			 "SoundWire master %d is disabled, ignoring\n",
>> -			 sdw->instance);
>> +			 "%s: SoundWire master %d is disabled, ignoring\n",
>> +			 __func__, sdw->instance);
> 
> This is not a debug message, please make it such if you want to have
> __func__  And even then, it's not needed as you can get that from the
> kernel automatically.

Sorry, I don't understand the feedback at all.

This message was added precisely to figure out why the expected
programming sequence was not followed, only to discover that we have
devices with spurious PCI wakes handled below. Without this added
difference with __func__, we wouldn't know if the issue happened during
the expected/regular programming sequence or not.

> 
>>  		return 0;
>>  	}
>>  
>> @@ -1489,8 +1489,8 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
>>  	bus = &sdw->cdns.bus;
>>  
>>  	if (bus->prop.hw_disabled || !sdw->startup_done) {
>> -		dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n",
>> -			bus->link_id);
>> +		dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n",
>> +			__func__, bus->link_id);
>>  		return 0;
>>  	}
>>  
>> @@ -1549,8 +1549,8 @@ static int __maybe_unused intel_pm_prepare(struct device *dev)
>>  	int ret;
>>  
>>  	if (bus->prop.hw_disabled || !sdw->startup_done) {
>> -		dev_dbg(dev, "SoundWire master %d is disabled or not-started, ignoring\n",
>> -			bus->link_id);
>> +		dev_dbg(dev, "%s: SoundWire master %d is disabled or not-started, ignoring\n",
>> +			__func__, bus->link_id);
> 
> Not needed, it is provided automatically if you ask the kernel for this.
> Same for all other instances in this patch.

provided how? Your comment is a bit cryptic here.