Re: [PATCH v2 5/7] mfd: qcom-pm8008: Add PM8010 support

[Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>]

Hi Konrad,

On 10/15/2026 5:34 PM, Konrad Dybcio wrote:
> On 10/14/26 12:32 PM, Jishnu Prakash wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> PM8010 is effectively the next generation of PM8008. The device is
>> generally designed to serve the exact same function - power camera
>> sensors and the associated supporting hardware.
> 
> [...]
> 
>> +	pmic_type = (unsigned long)of_device_get_match_data(dev);
> 
> I don't understand why you changed my match_data_struct approach
> here just to reconstruct the same data
> 

I was asked to do this by Lee here: https://lore.kernel.org/all/20260922093327.GA2308615@google.com/
and also by sashiko: https://lore.kernel.org/all/20260918165137.302631F000FF@smtp.kernel.org/.

Using an enum ID with switch() statement, instead of more complex
match_data structs, seems to be a convention to follow in MFD drivers.
For example, I could see the same being done in probes of
drivers/mfd/rohm-bd71828.c and drivers/mfd/max77620.c.

Thanks,
Jishnu

> Konrad
> 
>> +
>> +	switch (pmic_type) {
>> +	case TYPE_PM8008:
>> +		irq_chip_desc = &pm8008_irq_chip;
>> +		mfd_cells = pm8008_cells;
>> +		num_mfd_cells = ARRAY_SIZE(pm8008_cells);
>> +		break;
>> +	case TYPE_PM8010:
>> +		irq_chip_desc = &pm8010_irq_chip;
>> +		mfd_cells = pm8010_cells;
>> +		num_mfd_cells = ARRAY_SIZE(pm8010_cells);
>> +		break;
>> +	default:
>> +		dev_err(dev, "Unknown device type: %lu\n", pmic_type);
>> +		return -EINVAL;
>> +	}
>> +
>>  	dummy = devm_i2c_new_dummy_device(dev, client->adapter, client->addr + 1);
>>  	if (IS_ERR(dummy)) {
>>  		ret = PTR_ERR(dummy);
>> @@ -233,19 +293,20 @@ static int pm8008_probe(struct i2c_client *client)
>>  		return ret;
>>  
>>  	ret = devm_regmap_add_irq_chip_fwnode(dev, fwnode, regmap, client->irq,
>> -				IRQF_SHARED, 0, &pm8008_irq_chip, &irq_data);
>> +				IRQF_SHARED, 0, irq_chip_desc, &irq_data);
>>  	if (ret) {
>>  		dev_err(dev, "failed to add IRQ chip: %d\n", ret);
>>  		return ret;
>>  	}
>>  
>> -	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, pm8008_cells,
>> -				ARRAY_SIZE(pm8008_cells), NULL, 0,
>> +	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, mfd_cells,
>> +				num_mfd_cells, NULL, 0,
>>  				regmap_irq_get_domain(irq_data));
>>  }
>>  
>>  static const struct of_device_id pm8008_match[] = {
>> -	{ .compatible = "qcom,pm8008", },
>> +	{ .compatible = "qcom,pm8008", .data = (void *)TYPE_PM8008, },
>> +	{ .compatible = "qcom,pm8010-i2c", .data = (void *)TYPE_PM8010, },
>>  	{ },
>>  };
>>  MODULE_DEVICE_TABLE(of, pm8008_match);
>>