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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
