Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers

[Daniel Scally <djrscally@gmail.com>]

Hi Hans

On 22/02/2022 14:44, Hans de Goede wrote:
> Hi,
>
> On 2/17/22 04:22, kernel test robot wrote:
>> Hi Daniel,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on rafael-pm/linux-next]
>> [also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
>> [cannot apply to platform-drivers-x86/for-next]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url:    https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
>> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp@intel.com/config)
>> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> smatch warnings:
>> drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
> Right this needs to be an int, not an unsigned int. Daniel, please fix this for v2.


Will do! And thanks for your comments / tags on the rest of the series;
I'll post a new version some time this week.


Dan

>
> Regards,
>
> Hans
>
>
>
>> vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
>>
>>    142	
>>    143	static int skl_int3472_tps68470_probe(struct i2c_client *client)
>>    144	{
>>    145		struct acpi_device *adev = ACPI_COMPANION(&client->dev);
>>    146		const struct int3472_tps68470_board_data *board_data;
>>    147		struct tps68470_clk_platform_data *clk_pdata;
>>    148		unsigned int n_consumers;
>>    149		struct mfd_cell *cells;
>>    150		struct regmap *regmap;
>>    151		int device_type;
>>    152		int ret;
>>    153	
>>    154		n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
>>  > 155		if (n_consumers < 0)
>>    156			return n_consumers;
>>    157	
>>    158		regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>>    159		if (IS_ERR(regmap)) {
>>    160			dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
>>    161			return PTR_ERR(regmap);
>>    162		}
>>    163	
>>    164		i2c_set_clientdata(client, regmap);
>>    165	
>>    166		ret = tps68470_chip_init(&client->dev, regmap);
>>    167		if (ret < 0) {
>>    168			dev_err(&client->dev, "TPS68470 init error %d\n", ret);
>>    169			return ret;
>>    170		}
>>    171	
>>    172		device_type = skl_int3472_tps68470_calc_type(adev);
>>    173		switch (device_type) {
>>    174		case DESIGNED_FOR_WINDOWS:
>>    175			board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
>>    176			if (!board_data)
>>    177				return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
>>    178	
>>    179			cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
>>    180			if (!cells)
>>    181				return -ENOMEM;
>>    182	
>>    183			/*
>>    184			 * The order of the cells matters here! The clk must be first
>>    185			 * because the regulator depends on it. The gpios must be last,
>>    186			 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
>>    187			 * the clk + regulators must be ready when this happens.
>>    188			 */
>>    189			cells[0].name = "tps68470-clk";
>>    190			cells[0].platform_data = clk_pdata;
>>    191			cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
>>    192			cells[1].name = "tps68470-regulator";
>>    193			cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
>>    194			cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
>>    195			cells[2].name = "tps68470-gpio";
>>    196	
>>    197			gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
>>    198	
>>    199			ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>>    200						   cells, TPS68470_WIN_MFD_CELL_COUNT,
>>    201						   NULL, 0, NULL);
>>    202			kfree(cells);
>>    203	
>>    204			if (ret)
>>    205				gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
>>    206	
>>    207			break;
>>    208		case DESIGNED_FOR_CHROMEOS:
>>    209			ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>>    210						   tps68470_cros, ARRAY_SIZE(tps68470_cros),
>>    211						   NULL, 0, NULL);
>>    212			break;
>>    213		default:
>>    214			dev_err(&client->dev, "Failed to add MFD devices\n");
>>    215			return device_type;
>>    216		}
>>    217	
>>    218		/*
>>    219		 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
>>    220		 * for the GPIO cell already does this.
>>    221		 */
>>    222	
>>    223		return ret;
>>    224	}
>>    225	
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>>