On 25/03/2022 15:10, ChiYuan Huang wrote:
> Krzysztof Kozlowski <krzk@xxxxxxxxxx> 於 2022年3月25日 週五 下午8:17寫道：
>>
>> On 25/03/2022 02:06, cy_huang wrote:
>>> From: ChiYuan Huang <cy_huang@xxxxxxxxxxx>
>>>
>>> Add support for Richtek RT5759 high-performance DCDC converter.
>>>
>>> Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx>
>>> ---
>>> drivers/regulator/Kconfig | 10 +
>>> drivers/regulator/Makefile | 1 +
>>> drivers/regulator/rt5759-regulator.c | 372 +++++++++++++++++++++++++++++++++++
>>> 3 files changed, 383 insertions(+)
>>> create mode 100644 drivers/regulator/rt5759-regulator.c
>>>
>>
>> (...)
>>
>>> +static int rt5759_init_device_property(struct rt5759_priv *priv)
>>> +{
>>> + unsigned int val = 0;
>>> + bool wdt_enable;
>>> +
>>> + /*
>>> + * Only RT5759A support external watchdog input
>>> + */
>>> + if (priv->chip_type != CHIP_TYPE_RT5759A)
>>> + return 0;
>>> +
>>> + wdt_enable = device_property_read_bool(priv->dev,
>>> + "richtek,watchdog-enable");
>>> + if (wdt_enable)
>>
>> No need for separate wdt_enable variable.
>>
> Ack in next.
>>> + val = RT5759A_WDTEN_MASK;
>>> +
>>> + return regmap_update_bits(priv->regmap, RT5759A_REG_WDTEN,
>>> + RT5759A_WDTEN_MASK, val);
>>> +}
>>> +
>>> +static int rt5759_manufacturer_check(struct rt5759_priv *priv)
>>> +{
>>> + unsigned int vendor;
>>> + int ret;
>>> +
>>> + ret = regmap_read(priv->regmap, RT5759_REG_VENDORINFO, &vendor);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (vendor != RT5759_MANUFACTURER_ID) {
>>> + dev_err(priv->dev, "vendor info not correct (%d)\n", vendor);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static bool rt5759_is_accessible_reg(struct device *dev, unsigned int reg)
>>> +{
>>> + struct rt5759_priv *priv = dev_get_drvdata(dev);
>>> +
>>> + if (reg <= RT5759_REG_DCDCSET)
>>> + return true;
>>> +
>>> + if (priv->chip_type == CHIP_TYPE_RT5759A && reg == RT5759A_REG_WDTEN)
>>> + return true;
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static const struct regmap_config rt5759_regmap_config = {
>>> + .reg_bits = 8,
>>> + .val_bits = 8,
>>> + .max_register = RT5759A_REG_WDTEN,
>>> + .readable_reg = rt5759_is_accessible_reg,
>>> + .writeable_reg = rt5759_is_accessible_reg,
>>> +};
>>> +
>>> +static int rt5759_probe(struct i2c_client *i2c)
>>> +{
>>> + struct rt5759_priv *priv;
>>> + int ret;
>>> +
>>> + priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> +
>>> + priv->dev = &i2c->dev;
>>> + priv->chip_type = (unsigned long)of_device_get_match_data(&i2c->dev);
>>> + i2c_set_clientdata(i2c, priv);
>>> +
>>> + priv->regmap = devm_regmap_init_i2c(i2c, &rt5759_regmap_config);
>>> + if (IS_ERR(priv->regmap)) {
>>> + ret = PTR_ERR(priv->regmap);
>>> + dev_err(&i2c->dev, "Failed to allocate regmap (%d)\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret = rt5759_manufacturer_check(priv);
>>> + if (ret) {
>>> + dev_err(&i2c->dev, "Failed to check device (%d)\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret = rt5759_init_device_property(priv);
>>> + if (ret) {
>>> + dev_err(&i2c->dev, "Failed to init device (%d)\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + return rt5759_regulator_register(priv);
>>> +}
>>> +
>>> +static const struct of_device_id __maybe_unused rt5759_device_table[] = {
>>
>> I don't think this can be __maybe_unused. It is always referenced via
>> of_match_table, isn't it?
>>
> I think it can declared as '__maybe_unused'.
> If 'of_device_id' is unused, then in probe stage,
> 'of_device_get_match_data' will return NULL.
But your of_device_id cannot be unused. It is always referenced.
> priv->chip_type will get zero as the return value. And it will be
> treated as rt5759, not rt5759a.
> The difference between these two are only watchdog function supported or not.
Best regards,
Krzysztof
