diff mbox series

[1/2] ASoC: codecs: wcd9335: Add define for number of DAIs

Message ID 20241205084021.35610-1-krzysztof.kozlowski@linaro.org (mailing list archive)
State Superseded
Headers show
Series [1/2] ASoC: codecs: wcd9335: Add define for number of DAIs | expand

Commit Message

Krzysztof Kozlowski Dec. 5, 2024, 8:40 a.m. UTC
Number of DAIs in the codec is not really a binding, because it could
grow, e.g. when we implement missing features.  Add the define to the
driver, which will replace the one in the binding header.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Cc: Dzmitry Sankouski <dsankouski@gmail.com>
---
 sound/soc/codecs/wcd9335.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Mark Brown Dec. 5, 2024, 11:27 a.m. UTC | #1
On Thu, Dec 05, 2024 at 09:40:20AM +0100, Krzysztof Kozlowski wrote:

>  sound/soc/codecs/wcd9335.c | 2 ++
>  1 file changed, 2 insertions(+)

> +#define NUM_CODEC_DAIS          (AIF4_PB + 1)

Several other Qualcomm CODECs appear to use this define?
Krzysztof Kozlowski Dec. 5, 2024, 11:33 a.m. UTC | #2
On 05/12/2024 12:27, Mark Brown wrote:
> On Thu, Dec 05, 2024 at 09:40:20AM +0100, Krzysztof Kozlowski wrote:
> 
>>  sound/soc/codecs/wcd9335.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
>> +#define NUM_CODEC_DAIS          (AIF4_PB + 1)
> 
> Several other Qualcomm CODECs appear to use this define?

Each wcd93xx driver has it in the driver, not in the binding. All have
different values. wcd9335 was the first case when this was added to the
binding and I think it was a mistake (including my mistake of not
noticing it during review).

I am not sure if I got your comment or question correctly. I hope above
answers, but in it does not, please provide some context so I will
understand the question.

Best regards,
Krzysztof
Mark Brown Dec. 5, 2024, 12:33 p.m. UTC | #3
On Thu, Dec 05, 2024 at 09:40:20AM +0100, Krzysztof Kozlowski wrote:
> Number of DAIs in the codec is not really a binding, because it could
> grow, e.g. when we implement missing features.  Add the define to the
> driver, which will replace the one in the binding header.

This breaks an allmodconfig build:

/build/stage/linux/sound/soc/codecs/wcd9335.c:162: error: "NUM_CODEC_DAIS" redef
ined [-Werror]
  162 | #define NUM_CODEC_DAIS          (AIF4_PB + 1)
      | 
In file included from /build/stage/linux/sound/soc/codecs/wcd9335.c:28:
/build/stage/linux/include/dt-bindings/sound/qcom,wcd9335.h:13: note: this is th
e location of the previous definition
   13 | #define NUM_CODEC_DAIS          7
      |
Krzysztof Kozlowski Dec. 5, 2024, 12:40 p.m. UTC | #4
On 05/12/2024 13:33, Mark Brown wrote:
> On Thu, Dec 05, 2024 at 09:40:20AM +0100, Krzysztof Kozlowski wrote:
>> Number of DAIs in the codec is not really a binding, because it could
>> grow, e.g. when we implement missing features.  Add the define to the
>> driver, which will replace the one in the binding header.
> 
> This breaks an allmodconfig build:
> 
> /build/stage/linux/sound/soc/codecs/wcd9335.c:162: error: "NUM_CODEC_DAIS" redef
> ined [-Werror]
>   162 | #define NUM_CODEC_DAIS          (AIF4_PB + 1)
>       | 
> In file included from /build/stage/linux/sound/soc/codecs/wcd9335.c:28:
> /build/stage/linux/include/dt-bindings/sound/qcom,wcd9335.h:13: note: this is th
> e location of the previous definition
>    13 | #define NUM_CODEC_DAIS          7
>       | 

Apologies, last minute change hoping 6+1 equals 7, but obviously it does
not.

Best regards,
Krzysztof
Dzmitry Sankouski Dec. 5, 2024, 1:29 p.m. UTC | #5
чт, 5 дек. 2024 г. в 15:33, Mark Brown <broonie@kernel.org>:
>
> On Thu, Dec 05, 2024 at 09:40:20AM +0100, Krzysztof Kozlowski wrote:
> > Number of DAIs in the codec is not really a binding, because it could
> > grow, e.g. when we implement missing features.  Add the define to the
> > driver, which will replace the one in the binding header.
>
> This breaks an allmodconfig build:
>
> /build/stage/linux/sound/soc/codecs/wcd9335.c:162: error: "NUM_CODEC_DAIS" redef
> ined [-Werror]
>   162 | #define NUM_CODEC_DAIS          (AIF4_PB + 1)
>       |
> In file included from /build/stage/linux/sound/soc/codecs/wcd9335.c:28:
> /build/stage/linux/include/dt-bindings/sound/qcom,wcd9335.h:13: note: this is th
> e location of the previous definition
>    13 | #define NUM_CODEC_DAIS          7
>       |

This is the 1st patch in series, and NUM_CODEC_DAIS redefine from bindings
is deleted in the 2nd one.
Mark Brown Dec. 5, 2024, 1:31 p.m. UTC | #6
On Thu, Dec 05, 2024 at 04:29:45PM +0300, Dzmitry Sankouski wrote:

> This is the 1st patch in series, and NUM_CODEC_DAIS redefine from bindings
> is deleted in the 2nd one.

I know, that still means this change is broken.
Dzmitry Sankouski Dec. 6, 2024, 8:42 p.m. UTC | #7
чт, 5 дек. 2024 г. в 16:31, Mark Brown <broonie@kernel.org>:
>
> On Thu, Dec 05, 2024 at 04:29:45PM +0300, Dzmitry Sankouski wrote:
>
> > This is the 1st patch in series, and NUM_CODEC_DAIS redefine from bindings
> > is deleted in the 2nd one.
>
> I know, that still means this change is broken.

How to avoid broken change, when moving constant from dt-binding to*.c
file, given we have constraint of separate patch for bindings?
Krzysztof Kozlowski Dec. 9, 2024, 9:43 a.m. UTC | #8
On Fri, Dec 06, 2024 at 11:42:24PM +0300, Dzmitry Sankouski wrote:
> чт, 5 дек. 2024 г. в 16:31, Mark Brown <broonie@kernel.org>:
> >
> > On Thu, Dec 05, 2024 at 04:29:45PM +0300, Dzmitry Sankouski wrote:
> >
> > > This is the 1st patch in series, and NUM_CODEC_DAIS redefine from bindings
> > > is deleted in the 2nd one.
> >
> > I know, that still means this change is broken.
> 
> How to avoid broken change, when moving constant from dt-binding to*.c
> file, given we have constraint of separate patch for bindings?

There is no problem here, I just screwed, because 6+1 != 7.

Best regards,
Krzysztof
Mark Brown Dec. 10, 2024, 1 p.m. UTC | #9
On Thu, 05 Dec 2024 09:40:20 +0100, Krzysztof Kozlowski wrote:
> Number of DAIs in the codec is not really a binding, because it could
> grow, e.g. when we implement missing features.  Add the define to the
> driver, which will replace the one in the binding header.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: codecs: wcd9335: Add define for number of DAIs
      commit: 76c29db042e4f0fdea75c76cd4ad5f36080cd1c9
[2/2] ASoC: codecs: wcd9335: Drop number of DAIs from the header
      (no commit info)

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index a2521e16c099..d2604da619f6 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -159,6 +159,8 @@ 
 	{"AMIC MUX" #id, "ADC5", "ADC5"},		\
 	{"AMIC MUX" #id, "ADC6", "ADC6"}
 
+#define NUM_CODEC_DAIS          (AIF4_PB + 1)
+
 enum {
 	WCD9335_RX0 = 0,
 	WCD9335_RX1,