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

[Krzysztof Kozlowski]

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.

> +	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?

> +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.

Best regards,
Krzysztof