Dear Mark:
Thanks for your reply.
But I still have some questions. please check the red comment.
Mark Brown 於 2019/9/4 下午7:56 寫道:
On Wed, Sep 04, 2019 at 03:07:06PM +0800, Richtek Jeff Chang wrote:
+static int32_t mt6660_i2c_update_bits(struct mt6660_chip *chip,
- uint32_t addr, uint32_t mask, uint32_t data)
+{
It would be good to implement a regmap rather than open coding *everything* - it'd give you things like this without needing to open code them. Providing you don't use the cache code it should cope fine with variable register sizes.
Due to our hardware design, it is hard to implement regmap for MT6660.
You definitely can't use all the functionality due to the variable register sizes but using reg_write() and reg_read() should get you most of it.
How can I fill the val_bits for variable register size?
I try to use all 32 bits val_bits, but our chip some registers are overlap...
Do you have any suggestion for this issue? Thank you very much!
+static int mt6660_i2c_init_setting(struct mt6660_chip *chip) +{
- int i, len, ret;
- const struct codec_reg_val *init_table;
- init_table = e4_reg_inits;
- len = ARRAY_SIZE(e4_reg_inits);
- for (i = 0; i < len; i++) {
ret = mt6660_i2c_update_bits(chip, init_table[i].addr,init_table[i].mask, init_table[i].data);if (ret < 0)return ret;Why are we not using the chip defaults here?
Because MT6660 needs this initial setting for working well.
What are these settings? Are you sure they are generic settings and not board specific?
Yes, they are generic setting. It comes from our hardware designers.
- if (on_off) {
if (chip->pwr_cnt == 0) {ret = mt6660_i2c_update_bits(chip,MT6660_REG_SYSTEM_CTRL, 0x01, 0x00);val = mt6660_i2c_read(chip, MT6660_REG_IRQ_STATUS1);dev_info(chip->dev,"%s reg0x05 = 0x%x\n", __func__, val);}chip->pwr_cnt++;This looks like you're open coding runtime PM stuff? AFAICT the issue is that you need to write to this register to do any I/O. Just implement this via the standard runtime PM framework, you'll need to do something about the register I/O in the controls (ideally in the framework, it'd be a lot easier if you did have a cache) but you could cut out this bit.
In our experience, some Customer platform doesn't support runtime PM.
Tell your customers to turn it on, it's a standard kernel framework and there's really no excuse for open coding it. If there's some reason why runtime PM can't work for them then we should get that fixed but it really is *very* widely deployed.
We already implement it in runtime PM.
For all your comment, regmap is a hard thing to modify. I also survey others driver about variable register size. But it seems no one like our MT6660 chip...
Should I send new patch file to you in this mail loop, or I should send new patch via new Email Loop?
Thanks a lot.
Richtek Jeff Chang