Hi Krzysztof,
Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 於 2023年12月4日 週一 下午4:06寫道：
>
> On 04/12/2023 06:56, baneric926@xxxxxxxxx wrote:
> > From: Ban Feng <kcfeng0@xxxxxxxxxxx>
> >
> > NCT736X is an I2C based hardware monitoring chip from Nuvoton.
> >
> > Signed-off-by: Ban Feng <kcfeng0@xxxxxxxxxxx>
> > ---
>
>
> > +
> > +static const struct i2c_device_id nct736x_id[] = {
> > + {"nct7362", nct7362},
> > + {"nct7363", nct7363},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, nct736x_id);
> > +
>
> All ID tables are next to each other. Move it down. Why does it not
> match of_device_id?
ok, I'll put all ID tables together,
and add .data to of_device_id so that matching i2c_device_id.
>
> ...
>
> > +
> > +static int nct736x_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct nct736x_data *data;
> > + struct device *hwmon_dev;
> > + u32 pwm_mask, fanin_mask, val, wdt_cfg;
> > + int ret;
> > +
> > + data = devm_kzalloc(dev, sizeof(struct nct736x_data), GFP_KERNEL);
>
> sizeof(*)
ok, I'll modify it in v2.
>
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + i2c_set_clientdata(client, data);
> > + mutex_init(&data->update_lock);
> > +
> > + data->client = client;
> > +
> > + if (of_property_read_u32(dev->of_node, "nuvoton,pwm-mask", &pwm_mask))
> > + pwm_mask = 0;
> > + if (of_property_read_u32(dev->of_node,
> > + "nuvoton,fanin-mask", &fanin_mask))
> > + fanin_mask = 0;
> > + if (of_property_read_u32(dev->of_node, "nuvoton,wdt-timeout", &val))
> > + wdt_cfg = 0xff;
> > + else
> > + wdt_cfg = WDT_CFG(val) | EN_WDT;
> > +
> > + /* Initialize the chip */
> > + ret = nct736x_init_chip(client, pwm_mask, fanin_mask, wdt_cfg);
> > + if (ret)
> > + return ret;
> > +
> > + data->fan_mask = (u16)fanin_mask;
> > + data->pwm_mask = (u16)pwm_mask;
> > +
> > + data = nct736x_update_device(dev);
> > +
> > + data->groups[0] = &nct736x_group_fan;
> > + data->groups[1] = &nct736x_group_pwm;
> > + data->groups[2] = NULL;
> > +
> > + hwmon_dev = devm_hwmon_device_register_with_groups(dev,
> > + client->name,
> > + data, data->groups);
> > + return PTR_ERR_OR_ZERO(hwmon_dev);
> > +}
> > +
> > +static const struct of_device_id nct736x_of_match[] = {
> > + { .compatible = "nuvoton,nct7362" },
> > + { .compatible = "nuvoton,nct7363" },
>
> This means your devices are compatible. Express compatibility in your
> bindings (specific compatible followed by fallback). But then your
> i2c_device_id is not matching this one here... confusing and clearly wrong.
>
Same as above.
> Best regards,
> Krzysztof
>
