Re: [rtc-linux] [PATCH 2/6] mfd: max77620: add core driver for MAX77620/MAX20024

From: Laxman Dewangan
Date: Fri Jan 08 2016 - 04:26:58 EST


Hi Krzysztof,
Thanks for review.
I will fix most of your comment on my next patch.

Answering to some of comment/query.

On Friday 08 January 2016 07:05 AM, Krzysztof Kozlowski wrote:
()2016-01-07 23:38 GMT+09:00 Laxman Dewangan <ldewangan@xxxxxxxxxx>:
+ dev_err(dev,
+ "FPS enable-input %u is not supported\n",
+ pval);
Indentation of arguments does not seem equal here or maybe this is
just my email client. Have you run this through checkpatch? And
sparse? And coccicheck (that one definitely not because kbuild is
complaining)?
I ran checkpatch before I sent.

+ chip->rmap[i] = devm_regmap_init_i2c(chip->clients[i],
+ (const struct regmap_config *)&max77620_regmap_config[i]);
Indentation looks weird here (or again this is my email client...).
The cast is even weirder?!? Why casting?
There is some parameter difference for MAX77620 and MAX20024. I have only one structure for it and changing tun time so I have not define this structure as constant.
Now API needs const type structure and hence casting it.

However, I have define different structure for MAX77620 and MAX20024 which are const type and hence no need to explicitly casting here. This will be in my next patch.

+static inline int max77620_reg_update(struct device *dev, int sid,
+ unsigned int reg, unsigned int mask, unsigned int val)
+{
+ struct max77620_chip *chip = dev_get_drvdata(dev);
+
+ return regmap_update_bits(chip->rmap[sid], reg, mask, val);
+}

I think all these shouldn't be static inlines in header. Although some
of them are one-liners but rest are not. Let the compiler decide what
to do with these wrappers.

If I dont make inline from header then this will complain as unused static function on related C compilation if it is not used on C. This header included from all sub module driver and they are not using all these APIs.

To avoid compilation warning, I need to use inline here.