Re: [PATCH v1 2/3] i2c: qcom-slave: Add driver for Qualcomm I2C slave controller

[Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>]

On 6/29/2026 12:04 PM, Krzysztof Kozlowski wrote:
> On 28/06/2026 16:39, Viken Dadhaniya wrote:
>> +	slave->adap.owner = THIS_MODULE;
>> +	slave->adap.algo = &qcom_i2c_slave_algo;
>> +	slave->adap.dev.parent = dev;
>> +	slave->adap.dev.of_node = dev->of_node;
>> +	strscpy(slave->adap.name, "qcom-i2c-slave", sizeof(slave->adap.name));
>> +
>> +	i2c_set_adapdata(&slave->adap, slave);
>> +	platform_set_drvdata(pdev, slave);
>> +
>> +	ret = i2c_add_adapter(&slave->adap);
>> +	if (ret) {
>> +		dev_err(dev, "i2c_add_adapter failed: %d\n", ret);
>> +		icc_disable(slave->icc_path);
>> +		return ret;
>> +	}
>> +
>> +	dev_info(dev, "Qualcomm I2C slave probed at address 0x%x\n", addr);
> 
> NAK, as reviewed many times. Drivers must be silent and you don't even
> print any useful information, becausr address is fixed based on DT.

The dev_info() call has been removed in v2.

> 
>> +	return 0;
>> +}
>> +
>> +/**
>> + * qcom_i2c_slave_remove - remove the Qualcomm I2C slave controller
>> + * @pdev:	platform device
>> + *
>> + * Unregisters the I2C adapter and disables the interconnect path.
>> + * Controller clocks are disabled automatically by the devm framework.
>> + */
> 
> Really, what sort of coding style is that? Since when Linux kernel
> writes kerneldoc for standard driver hooks?

Acknowledged. All kerneldoc blocks on static functions have been
removed in v2.

> 
>> +static void qcom_i2c_slave_remove(struct platform_device *pdev)
>> +{
>> +	struct qcom_i2c_slave *slave = platform_get_drvdata(pdev);
>> +
>> +	i2c_del_adapter(&slave->adap);
>> +	icc_disable(slave->icc_path);
>> +	/* clocks are disabled automatically by devm */
>> +}
>> +
>> +/**
>> + * qcom_i2c_slave_suspend - suspend the controller
>> + * @dev:	device associated with the controller
>> + *
>> + * Disables the interrupt, releases the interconnect bandwidth vote, and
>> + * disables the controller clocks to allow the system to enter a low-power
>> + * state.
>> + *
>> + * Return: 0 always.
>> + */
> 
> 
> Please don't send us downstream code or LLM generated slop. There is no
> single driver written that way.

Acknowledged. All kerneldoc on static functions, probe/remove/suspend/
resume hooks, and internal helpers has been removed in v2.

> 
> Best regards,
> Krzysztof