Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

[Krzysztof Kozlowski]

On 07/11/2023 04:14, Rui Zhou wrote:
> Add compatible string "mediatek,mt8188-es8326" to support new board
> with es8326 codec.

Please use standard email subjects, so with the PATCH keyword in the
title. `git format-patch` helps here to create proper versioned patches.
Another useful tool is b4. Skipping the PATCH keyword makes filtering of
emails more difficult thus making the review process less convenient.

> 
> Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>
> ---
>  .../devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml        | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> index 4c8c95057ef7..9df5e44176d0 100644
> --- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> @@ -18,6 +18,7 @@ properties:
>        - mediatek,mt8188-mt6359-evb
>        - mediatek,mt8188-nau8825
>        - mediatek,mt8188-rt5682s
> +      - mediatek,mt8188-es8326

Entries should be sorted alphabetically.

Best regards,
Krzysztof

Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

[Rui Zhou]

Thank you very much for your reminder, I will pay attention next time.


On Tue, Nov 7, 2023 at 3:11 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 07/11/2023 04:14, Rui Zhou wrote:
> > Add compatible string "mediatek,mt8188-es8326" to support new board
> > with es8326 codec.
>
> Please use standard email subjects, so with the PATCH keyword in the
> title. `git format-patch` helps here to create proper versioned patches.
> Another useful tool is b4. Skipping the PATCH keyword makes filtering of
> emails more difficult thus making the review process less convenient.
>
> >
> > Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>
> > ---
> >  .../devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml        | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> > index 4c8c95057ef7..9df5e44176d0 100644
> > --- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> > @@ -18,6 +18,7 @@ properties:
> >        - mediatek,mt8188-mt6359-evb
> >        - mediatek,mt8188-nau8825
> >        - mediatek,mt8188-rt5682s
> > +      - mediatek,mt8188-es8326
>
> Entries should be sorted alphabetically.
>
> Best regards,
> Krzysztof
>

Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

[Krzysztof Kozlowski]

On 07/11/2023 08:59, Rui Zhou wrote:
> Thank you very much for your reminder, I will pay attention next time.

Next time? Does it mean you are not going to fix the patch as I asked
about the order?

Best regards,
Krzysztof

Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

[Rui Zhou]

Sorry, I didn't express myself clearly. What I mean is that I will
modify everything according to your suggestions in the V2 version.

On Tue, Nov 7, 2023 at 4:06 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 07/11/2023 08:59, Rui Zhou wrote:
> > Thank you very much for your reminder, I will pay attention next time.
>
> Next time? Does it mean you are not going to fix the patch as I asked
> about the order?
>
> Best regards,
> Krzysztof
>

Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

[AngeloGioacchino Del Regno]

Il 07/11/23 04:14, Rui Zhou ha scritto:
> Add compatible string "mediatek,mt8188-es8326" to support new board
> with es8326 codec.
> 
> Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [v1 2/3] ASoC: mediatek: mt8188-mt6359: add es8326 support

[AngeloGioacchino Del Regno]

Il 07/11/23 04:14, Rui Zhou ha scritto:
> To use ES8326 as the codec, add a new sound card
> named mt8186_es8326.
> 
> Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [v1 3/3] ASoC: mediatek: mt8188-mt6359: add two amps support

[AngeloGioacchino Del Regno]

Il 07/11/23 04:14, Rui Zhou ha scritto:
> Support codec rt5682 using two amps.
> 
> Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>

Can you please change the commit description so that it's clear that you're adding
the two amps support to the RT5682s variant? As it is right now, it reads like you
are adding that globally.

ASoC: mediatek: mt8188-mt6359: Enable dual amp for mt8188_rt5682s

After which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> index e994faa3556b..917640939621 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> @@ -1549,7 +1549,7 @@ static struct mt8188_card_data mt8188_nau8825_card = {
>   
>   static struct mt8188_card_data mt8188_rt5682s_card = {
>   	.name = "mt8188_rt5682s",
> -	.quirk = RT5682S_HS_PRESENT,
> +	.quirk = RT5682S_HS_PRESENT | MAX98390_TWO_AMP,
>   };
>   
>   static struct mt8188_card_data mt8188_es8326_card = {

Re: [v1 1/3] ASoC: dt-bindings: mt8188-mt6359: add es8326 support

[AngeloGioacchino Del Regno]

Il 07/11/23 04:14, Rui Zhou ha scritto:
> Add compatible string "mediatek,mt8188-es8326" to support new board
> with es8326 codec.
> 
> Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>
> ---
>   .../devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml        | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> index 4c8c95057ef7..9df5e44176d0 100644
> --- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> @@ -18,6 +18,7 @@ properties:
>         - mediatek,mt8188-mt6359-evb
>         - mediatek,mt8188-nau8825
>         - mediatek,mt8188-rt5682s
> +      - mediatek,mt8188-es8326

Whoops, sorry, I just noticed that this is not alphabetically ordered.
Please move that before mediatek,mt8188-mt6359-evb.

Thanks,
Angelo

>   
>     audio-routing:
>       description:

Re: [v1 3/3] ASoC: mediatek: mt8188-mt6359: add two amps support

[Rui Zhou]

OK, I'll revise it as soon as possible.

On Tue, Nov 7, 2023 at 4:31 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 07/11/23 04:14, Rui Zhou ha scritto:
> > Support codec rt5682 using two amps.
> >
> > Signed-off-by: Rui Zhou <zhourui@huaqin.corp-partner.google.com>
>
> Can you please change the commit description so that it's clear that you're adding
> the two amps support to the RT5682s variant? As it is right now, it reads like you
> are adding that globally.
>
> ASoC: mediatek: mt8188-mt6359: Enable dual amp for mt8188_rt5682s
>
> After which:
>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> > ---
> >   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> > index e994faa3556b..917640939621 100644
> > --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> > +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> > @@ -1549,7 +1549,7 @@ static struct mt8188_card_data mt8188_nau8825_card = {
> >
> >   static struct mt8188_card_data mt8188_rt5682s_card = {
> >       .name = "mt8188_rt5682s",
> > -     .quirk = RT5682S_HS_PRESENT,
> > +     .quirk = RT5682S_HS_PRESENT | MAX98390_TWO_AMP,
> >   };
> >
> >   static struct mt8188_card_data mt8188_es8326_card = {
>
>