diff mbox series

[4/5] i2c: piix4: Adjust the SMBus debug message

Message ID 20240822142200.686842-5-Shyam-sundar.S-k@amd.com
State Changes Requested
Delegated to: Andi Shyti
Headers show
Series Add ASF Controller Support to the i2c-piix4 driver | expand

Commit Message

Shyam Sundar S K Aug. 22, 2024, 2:21 p.m. UTC 
With the addition of ASF, the current adapter information must now
correctly print the SMBus node details, whether it pertains to PIIX4 or
ASF. Update the driver to reflect this change accordingly.

Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/i2c/busses/i2c-piix4.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Andi Shyti Sept. 3, 2024, 9:51 p.m. UTC | #1 
Hi Shyam,

> @@ -1194,6 +1194,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>  {
>  	struct i2c_adapter *adap;
>  	struct i2c_piix4_adapdata *adapdata;
> +	char *node = "PIIX4";

please, make this const and initialize it...

>  	int retval;
>  
>  	adap = kzalloc(sizeof(*adap), GFP_KERNEL);
> @@ -1213,6 +1214,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>  		adap->algo = &piix4_smbus_algorithm_sb800;
>  		break;

... here.

>  	case SMBUS_ASF:
> +		node = "ASF";
>  		adap->algo = &sb800_asf_smbus_algorithm;
>  		break;

shall we have a default case here? I thought checkpatch complains
when no default is specified.

Thanks,
Andi
Shyam Sundar S K Sept. 4, 2024, 11:01 a.m. UTC | #2 
Hi Andi,

On 9/4/2024 03:21, Andi Shyti wrote:
> Hi Shyam,
> 
>> @@ -1194,6 +1194,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>>  {
>>  	struct i2c_adapter *adap;
>>  	struct i2c_piix4_adapdata *adapdata;
>> +	char *node = "PIIX4";
> 
> please, make this const and initialize it...
> 
>>  	int retval;
>>  
>>  	adap = kzalloc(sizeof(*adap), GFP_KERNEL);
>> @@ -1213,6 +1214,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
>>  		adap->algo = &piix4_smbus_algorithm_sb800;
>>  		break;
> 
> ... here.
> 
>>  	case SMBUS_ASF:
>> +		node = "ASF";
>>  		adap->algo = &sb800_asf_smbus_algorithm;
>>  		break;
> 
> shall we have a default case here? I thought checkpatch complains
> when no default is specified.

checkpatch does not complain about it. I have added a default case
based on your suggestion in v2. Kindly have a look.

Thanks,
Shyam

> 
> Thanks,
> Andi
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 00fc641e6277..446e9235f900 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -1194,6 +1194,7 @@  static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 {
 	struct i2c_adapter *adap;
 	struct i2c_piix4_adapdata *adapdata;
+	char *node = "PIIX4";
 	int retval;
 
 	adap = kzalloc(sizeof(*adap), GFP_KERNEL);
@@ -1213,6 +1214,7 @@  static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 		adap->algo = &piix4_smbus_algorithm_sb800;
 		break;
 	case SMBUS_ASF:
+		node = "ASF";
 		adap->algo = &sb800_asf_smbus_algorithm;
 		break;
 	}
@@ -1240,7 +1242,7 @@  static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 	}
 
 	snprintf(adap->name, sizeof(adap->name),
-		"SMBus PIIX4 adapter%s at %04x", name, smba);
+		"SMBus %s adapter%s at %04x", node, name, smba);
 
 	i2c_set_adapdata(adap, adapdata);