All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Richtek Jeff Chang <richtek.jeff.chang@gmail.com>
Cc: lgirdwood@gmail.com, perex@perex.cz, tiwai@suse.com,
	matthias.bgg@gmail.com, alsa-devel@alsa-project.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [MT6660] Mediatek Smart Amplifier Driver
Date: Wed, 25 Sep 2019 09:50:23 -0700	[thread overview]
Message-ID: <20190925165023.GJ2036@sirena.org.uk> (raw)
In-Reply-To: <3a9f66b3-bdb7-9bec-a9c4-ac58d3efa543@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]

On Wed, Sep 25, 2019 at 06:04:23PM +0800, Richtek Jeff Chang wrote:
> Mark Brown 於 2019/9/4 下午7:56 寫道:
> > On Wed, Sep 04, 2019 at 03:07:06PM +0800, Richtek Jeff Chang wrote:

> > > > 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!

If you use reg_read() and reg_write() operations you can hide the
register size within them - the rest of the code thinks the
registers are all the 32 bits but when doing I/O it can use the
appropriate size for a given register.

> > > > > +	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.

You should probably be using the regmap register patch feature,
it's for things like this where the chip should always be used
with a different set of defaults to the silicon.

> Should I send new patch file to you in this mail loop, or I should send new
> patch via new Email Loop?

A new one please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Richtek Jeff Chang <richtek.jeff.chang@gmail.com>
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	tiwai@suse.com, lgirdwood@gmail.com,
	linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [alsa-devel] [PATCH] [MT6660] Mediatek Smart Amplifier Driver
Date: Wed, 25 Sep 2019 09:50:23 -0700	[thread overview]
Message-ID: <20190925165023.GJ2036@sirena.org.uk> (raw)
In-Reply-To: <3a9f66b3-bdb7-9bec-a9c4-ac58d3efa543@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2008 bytes --]

On Wed, Sep 25, 2019 at 06:04:23PM +0800, Richtek Jeff Chang wrote:
> Mark Brown 於 2019/9/4 下午7:56 寫道:
> > On Wed, Sep 04, 2019 at 03:07:06PM +0800, Richtek Jeff Chang wrote:

> > > > 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!

If you use reg_read() and reg_write() operations you can hide the
register size within them - the rest of the code thinks the
registers are all the 32 bits but when doing I/O it can use the
appropriate size for a given register.

> > > > > +	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.

You should probably be using the regmap register patch feature,
it's for things like this where the chip should always be used
with a different set of defaults to the silicon.

> Should I send new patch file to you in this mail loop, or I should send new
> patch via new Email Loop?

A new one please.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Richtek Jeff Chang <richtek.jeff.chang@gmail.com>
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	tiwai@suse.com, lgirdwood@gmail.com,
	linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
	perex@perex.cz, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] [MT6660] Mediatek Smart Amplifier Driver
Date: Wed, 25 Sep 2019 09:50:23 -0700	[thread overview]
Message-ID: <20190925165023.GJ2036@sirena.org.uk> (raw)
In-Reply-To: <3a9f66b3-bdb7-9bec-a9c4-ac58d3efa543@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2008 bytes --]

On Wed, Sep 25, 2019 at 06:04:23PM +0800, Richtek Jeff Chang wrote:
> Mark Brown 於 2019/9/4 下午7:56 寫道:
> > On Wed, Sep 04, 2019 at 03:07:06PM +0800, Richtek Jeff Chang wrote:

> > > > 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!

If you use reg_read() and reg_write() operations you can hide the
register size within them - the rest of the code thinks the
registers are all the 32 bits but when doing I/O it can use the
appropriate size for a given register.

> > > > > +	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.

You should probably be using the regmap register patch feature,
it's for things like this where the chip should always be used
with a different set of defaults to the silicon.

> Should I send new patch file to you in this mail loop, or I should send new
> patch via new Email Loop?

A new one please.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-09-25 16:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03  7:08 [PATCH] [MT6660] Mediatek Smart Amplifier Driver richtek.jeff.chang
2019-09-03  7:08 ` richtek.jeff.chang
2019-09-03  7:08 ` [alsa-devel] " richtek.jeff.chang
2019-09-03 16:38 ` Mark Brown
2019-09-03 16:38   ` Mark Brown
2019-09-03 16:38   ` [alsa-devel] " Mark Brown
2019-09-04  7:07   ` Richtek Jeff Chang
2019-09-04  7:07     ` Richtek Jeff Chang
2019-09-04 11:56     ` Mark Brown
2019-09-04 11:56       ` Mark Brown
2019-09-04 11:56       ` [alsa-devel] " Mark Brown
2019-09-25 10:04       ` Richtek Jeff Chang
2019-09-25 10:04         ` Richtek Jeff Chang
2019-09-25 16:50         ` Mark Brown [this message]
2019-09-25 16:50           ` Mark Brown
2019-09-25 16:50           ` [alsa-devel] " Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190925165023.GJ2036@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=perex@perex.cz \
    --cc=richtek.jeff.chang@gmail.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.