diff mbox series

[1/2] cros_ec_commands: Add EC_CODEC_I2S_RX_RESET

Message ID 20210107085942.2891525-1-yuhsuan@chromium.org (mailing list archive)
State Superseded
Headers show
Series [1/2] cros_ec_commands: Add EC_CODEC_I2S_RX_RESET | expand

Commit Message

Yu-Hsuan Hsu Jan. 7, 2021, 8:59 a.m. UTC
Add the new command EC_CODEC_I2S_RX_RESET in ec_codec_i2s_rx_subcmd,
which is used for resetting the EC codec.

Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
---
 include/linux/platform_data/cros_ec_commands.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Mark Brown Jan. 7, 2021, 1:54 p.m. UTC | #1
On Thu, Jan 07, 2021 at 04:59:41PM +0800, Yu-Hsuan Hsu wrote:
> Add the new command EC_CODEC_I2S_RX_RESET in ec_codec_i2s_rx_subcmd,
> which is used for resetting the EC codec.

I think the request was to sync over all the commands that are supported
in the EC rather than just split this one addition into a separate
patch.
Yu-Hsuan Hsu Jan. 8, 2021, 4:57 a.m. UTC | #2
Mark Brown <broonie@kernel.org> 於 2021年1月7日 週四 下午9:55寫道:
>
> On Thu, Jan 07, 2021 at 04:59:41PM +0800, Yu-Hsuan Hsu wrote:
> > Add the new command EC_CODEC_I2S_RX_RESET in ec_codec_i2s_rx_subcmd,
> > which is used for resetting the EC codec.
>
> I think the request was to sync over all the commands that are supported
> in the EC rather than just split this one addition into a separate
> patch.
Got it. However, after running make_linux_ec_commands_h.sh to create
the new cros_ec_commands.h, I found there are lots of difference (1092
insertions(+), 66 deletions(-)). In addition, there are also some
redefined variables(most are in ./include/linux/usb/pd.h) causing the
compile error.

It seems not easy to sync cros_ec_commands.h. I'm afraid of breaking
something. Does anyone have any suggestion? Thanks.
Mark Brown Jan. 12, 2021, 12:42 a.m. UTC | #3
On Fri, Jan 08, 2021 at 12:57:51PM +0800, Yu-Hsuan Hsu wrote:
> Mark Brown <broonie@kernel.org> 於 2021年1月7日 週四 下午9:55寫道:

> > I think the request was to sync over all the commands that are supported
> > in the EC rather than just split this one addition into a separate
> > patch.

> Got it. However, after running make_linux_ec_commands_h.sh to create
> the new cros_ec_commands.h, I found there are lots of difference (1092
> insertions(+), 66 deletions(-)). In addition, there are also some
> redefined variables(most are in ./include/linux/usb/pd.h) causing the
> compile error.

> It seems not easy to sync cros_ec_commands.h. I'm afraid of breaking
> something. Does anyone have any suggestion? Thanks.

TBH that seems like a big enough change to split out from this and done
as a separate series, I'd be perfectly happy to apply your original
change.  I guess part of doing that sync up should ideally be to
refactor things so that it can be done mechanically in future.
Guenter Roeck Jan. 12, 2021, 1:52 a.m. UTC | #4
On Mon, Jan 11, 2021 at 4:42 PM Mark Brown <broonie@kernel.org> wrote:

> On Fri, Jan 08, 2021 at 12:57:51PM +0800, Yu-Hsuan Hsu wrote:
> > Mark Brown <broonie@kernel.org> 於 2021年1月7日 週四 下午9:55寫道:
>
> > > I think the request was to sync over all the commands that are
> supported
> > > in the EC rather than just split this one addition into a separate
> > > patch.
>
> > Got it. However, after running make_linux_ec_commands_h.sh to create
> > the new cros_ec_commands.h, I found there are lots of difference (1092
> > insertions(+), 66 deletions(-)). In addition, there are also some
> > redefined variables(most are in ./include/linux/usb/pd.h) causing the
> > compile error.
>
> > It seems not easy to sync cros_ec_commands.h. I'm afraid of breaking
> > something. Does anyone have any suggestion? Thanks.
>
> TBH that seems like a big enough change to split out from this and done
> as a separate series, I'd be perfectly happy to apply your original
> change.  I guess part of doing that sync up should ideally be to
> refactor things so that it can be done mechanically in future.
>

Being able to do it mechanically was the idea for introducing the script.
Unfortunately it doesn't help to have such a script if people don't use it.

Guenter
Mark Brown Jan. 12, 2021, 2:10 p.m. UTC | #5
On Mon, Jan 11, 2021 at 05:52:31PM -0800, Guenter Roeck wrote:
> On Mon, Jan 11, 2021 at 4:42 PM Mark Brown <broonie@kernel.org> wrote:

> > TBH that seems like a big enough change to split out from this and done
> > as a separate series, I'd be perfectly happy to apply your original
> > change.  I guess part of doing that sync up should ideally be to
> > refactor things so that it can be done mechanically in future.

> Being able to do it mechanically was the idea for introducing the script.
> Unfortunately it doesn't help to have such a script if people don't use it.

Looking at the issues Yu-Hsuan mentions and briefly at the code I guess
there's some updates needed with the script for namespacing around at
least pd_ to avoid the need for hand massaging things, that'll put
people off using the script.
Enric Balletbo i Serra Jan. 12, 2021, 4:40 p.m. UTC | #6
Hi,

On 12/1/21 15:10, Mark Brown wrote:
> On Mon, Jan 11, 2021 at 05:52:31PM -0800, Guenter Roeck wrote:
>> On Mon, Jan 11, 2021 at 4:42 PM Mark Brown <broonie@kernel.org> wrote:
> 
>>> TBH that seems like a big enough change to split out from this and done
>>> as a separate series, I'd be perfectly happy to apply your original
>>> change.  I guess part of doing that sync up should ideally be to
>>> refactor things so that it can be done mechanically in future.
> 
>> Being able to do it mechanically was the idea for introducing the script.
>> Unfortunately it doesn't help to have such a script if people don't use it.
> 
> Looking at the issues Yu-Hsuan mentions and briefly at the code I guess
> there's some updates needed with the script for namespacing around at
> least pd_ to avoid the need for hand massaging things, that'll put
> people off using the script.
> 

I even didn't know about that script :-(, or forget about it, assuming the files
are async again, I think I'm fine on only introduce that line of code instead of
a full sync (and lets think how we can do better work on this and solve in the
chrome-platform tree). I have some comments on patch 2, though.

Cheers,
  Enric
diff mbox series

Patch

diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 86376779ab31..95889ada83a3 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -4600,6 +4600,7 @@  enum ec_codec_i2s_rx_subcmd {
 	EC_CODEC_I2S_RX_SET_SAMPLE_DEPTH = 0x2,
 	EC_CODEC_I2S_RX_SET_DAIFMT = 0x3,
 	EC_CODEC_I2S_RX_SET_BCLK = 0x4,
+	EC_CODEC_I2S_RX_RESET = 0x5,
 	EC_CODEC_I2S_RX_SUBCMD_COUNT,
 };