Re: [PATCH v3 3/3] i2c: Add i2c-nt726xx.c I2C driver for Novatek NT726xx SoCs

[nina_kuo]

From: Ben Huang <Ben_Huang@xxxxxxxxxxxxxx>

On Tuesday, July 21, 2026 at 3:14 PM, Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
> On Wed, Jul 15, 2026 at 03:35:34PM +0800, Nina_Kuo@xxxxxxxxxxxxxx wrote:
>> +#define I2C_INFO_LOG(fmt, ...)\
>> +pr_info("[I/SOC_I2C] " fmt, ##__VA_ARGS__)
>> +
>> +#define I2C_ERR_LOG(fmt, ...)\
>> +pr_err("[E/SOC_I2C] " fmt, ##__VA_ARGS__)
>
> No, you do not get own printing stuff.
>
> This is not Linux coding style. Learn how Linux treats your own new
> abstraction layers.
>
> ...
>
>> +static int nvt_i2c_probe(struct platform_device *pdev)
>> +{
>> +struct nvt_i2c_bus *i2c;
>> +struct resource *res;
>> +int ret, irq;
>> +
>> +i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
>> +if (!i2c)
>> +return -ENOMEM;
>> +
>> +res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +i2c->base = devm_ioremap_resource(&pdev->dev, res);
>
> Look at other drivers
>
>> +if (IS_ERR(i2c->base)) {
>> +I2C_ERR_LOG("failed to map controller\n");
>> +return PTR_ERR(i2c->base);
>
> Look at other drivers how they do it
>
>> +}
>> +
>> +init_completion(&i2c->msg_complete);
>> +i2c->dev = &pdev->dev;
>> +
>> +ret = nvt_i2c_parse_dts(i2c);
>> +if (ret)
>> +return ret;
>> +
>> +nvt_i2c_init(i2c);
>> +
>> +irq = platform_get_irq(pdev, 0);
>> +if (irq < 0) {
>> +I2C_ERR_LOG("No IRQ resource\n");
>
> Look at other drivers how they do it
>
>> +return irq;
>> +}
>> +
>> +ret = devm_request_irq(&pdev->dev, irq, nvt_i2c_isr,
>> +       IRQF_SHARED | IRQF_TRIGGER_HIGH, I2C_NAME, i2c);
>> +if (ret) {
>> +I2C_ERR_LOG("devm_request_irq fail\n");
>
> Look at other drivers how they do it
>

Log functions will be changed to dev_err() for error and dev_info()
for info use.

>> +static struct platform_driver nvt_i2c_driver = {
>> +.probe = nvt_i2c_probe,
>> +.remove = nvt_i2c_remove,
>> +.driver = {
>> +.name = "nvt_nt726xx_i2c",
>> +.owner = THIS_MODULE,
>
> NAK, you just sent us 13 year old downstream junk code. This is not
> acceptable, because by doing that you ignore all the bugs or issues we
> fixed over last 13 years.

owner will be removed.

>> +.pm = &nvt_i2c_pm_ops,
>> +.of_match_table = of_match_ptr(nvt_i2c_of_match),
>> +},
>> +};
>> +
>> +static int __init nvt_i2c_platform_init(void)
>> +{
>> +return platform_driver_register(&nvt_i2c_driver);
>> +}
>> +postcore_initcall(nvt_i2c_platform_init);
>
> NAK, this is not postcore!
>
> You have standard module. Full stop.

This will be replaced by module_init().

Sincerely,
Ben Huang