[PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[Markus Schneider-Pargmann]

The given operations are inverted for the wrong registers which makes
multiple of the mt8365 hardware units unusable. In my setup at least usb
did not work.

Fixed by swapping the operations with the inverted ones.

Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/clk/mediatek/clk-mt8365.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
index 6b4e193f648d..6d785ec5754d 100644
--- a/drivers/clk/mediatek/clk-mt8365.c
+++ b/drivers/clk/mediatek/clk-mt8365.c
@@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
 
 #define GATE_TOP0(_id, _name, _parent, _shift)			\
 	GATE_MTK(_id, _name, _parent, &top0_cg_regs,		\
-		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
+		 _shift, &mtk_clk_gate_ops_no_setclr)
 
 #define GATE_TOP1(_id, _name, _parent, _shift)			\
 	GATE_MTK(_id, _name, _parent, &top1_cg_regs,		\
-		 _shift, &mtk_clk_gate_ops_no_setclr)
+		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
 
 #define GATE_TOP2(_id, _name, _parent, _shift)			\
 	GATE_MTK(_id, _name, _parent, &top2_cg_regs,		\
-		 _shift, &mtk_clk_gate_ops_no_setclr)
+		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
 
 static const struct mtk_gate top_clk_gates[] = {
 	GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),
-- 
2.40.1

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[Alexandre Mergnat]

On 11/05/2023 15:32, Markus Schneider-Pargmann wrote:
> The given operations are inverted for the wrong registers which makes
> multiple of the mt8365 hardware units unusable. In my setup at least usb
> did not work.
> 
> Fixed by swapping the operations with the inverted ones.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Tested-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Regards,
Alexandre

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[Matthias Brugger]

On 11/05/2023 15:32, Markus Schneider-Pargmann wrote:
> The given operations are inverted for the wrong registers which makes
> multiple of the mt8365 hardware units unusable. In my setup at least usb
> did not work.
> 
> Fixed by swapping the operations with the inverted ones.
> 
> Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
>   drivers/clk/mediatek/clk-mt8365.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
> index 6b4e193f648d..6d785ec5754d 100644
> --- a/drivers/clk/mediatek/clk-mt8365.c
> +++ b/drivers/clk/mediatek/clk-mt8365.c
> @@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
>   
>   #define GATE_TOP0(_id, _name, _parent, _shift)			\
>   	GATE_MTK(_id, _name, _parent, &top0_cg_regs,		\
> -		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
> +		 _shift, &mtk_clk_gate_ops_no_setclr)
>   
>   #define GATE_TOP1(_id, _name, _parent, _shift)			\
>   	GATE_MTK(_id, _name, _parent, &top1_cg_regs,		\
> -		 _shift, &mtk_clk_gate_ops_no_setclr)
> +		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>   
>   #define GATE_TOP2(_id, _name, _parent, _shift)			\
>   	GATE_MTK(_id, _name, _parent, &top2_cg_regs,		\
> -		 _shift, &mtk_clk_gate_ops_no_setclr)
> +		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>   
>   static const struct mtk_gate top_clk_gates[] = {
>   	GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[AngeloGioacchino Del Regno]

Il 12/05/23 13:51, Matthias Brugger ha scritto:
> 
> 
> On 11/05/2023 15:32, Markus Schneider-Pargmann wrote:
>> The given operations are inverted for the wrong registers which makes
>> multiple of the mt8365 hardware units unusable. In my setup at least usb
>> did not work.
>>
>> Fixed by swapping the operations with the inverted ones.

...with the not inverted ones, you mean!

Anyway,

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

>>
>> Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
>> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate 
>> clocks")
>> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> 
> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> 
>> ---
>>   drivers/clk/mediatek/clk-mt8365.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
>> index 6b4e193f648d..6d785ec5754d 100644
>> --- a/drivers/clk/mediatek/clk-mt8365.c
>> +++ b/drivers/clk/mediatek/clk-mt8365.c
>> @@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
>>   #define GATE_TOP0(_id, _name, _parent, _shift)            \
>>       GATE_MTK(_id, _name, _parent, &top0_cg_regs,        \
>> -         _shift, &mtk_clk_gate_ops_no_setclr_inv)
>> +         _shift, &mtk_clk_gate_ops_no_setclr)
>>   #define GATE_TOP1(_id, _name, _parent, _shift)            \
>>       GATE_MTK(_id, _name, _parent, &top1_cg_regs,        \
>> -         _shift, &mtk_clk_gate_ops_no_setclr)
>> +         _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>   #define GATE_TOP2(_id, _name, _parent, _shift)            \
>>       GATE_MTK(_id, _name, _parent, &top2_cg_regs,        \
>> -         _shift, &mtk_clk_gate_ops_no_setclr)
>> +         _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>   static const struct mtk_gate top_clk_gates[] = {
>>       GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[Markus Schneider-Pargmann]

On Mon, May 15, 2023 at 01:49:54PM +0200, AngeloGioacchino Del Regno wrote:
> Il 12/05/23 13:51, Matthias Brugger ha scritto:
> > 
> > 
> > On 11/05/2023 15:32, Markus Schneider-Pargmann wrote:
> > > The given operations are inverted for the wrong registers which makes
> > > multiple of the mt8365 hardware units unusable. In my setup at least usb
> > > did not work.
> > > 
> > > Fixed by swapping the operations with the inverted ones.
> 
> ...with the not inverted ones, you mean!

Actually now I am not sure how to express it correctly, maybe I should
have just left that sentence out of the commit message.

I meant replacing the no_setclr_inv with no_setclr for TOP0 and no_setclr with
no_setclr_inv for TOP1/2.

Best,
Markus

> 
> Anyway,
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> > > 
> > > Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
> > > Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to
> > > mtk_gate clocks")
> > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > 
> > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> > 
> > > ---
> > >   drivers/clk/mediatek/clk-mt8365.c | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
> > > index 6b4e193f648d..6d785ec5754d 100644
> > > --- a/drivers/clk/mediatek/clk-mt8365.c
> > > +++ b/drivers/clk/mediatek/clk-mt8365.c
> > > @@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
> > >   #define GATE_TOP0(_id, _name, _parent, _shift)            \
> > >       GATE_MTK(_id, _name, _parent, &top0_cg_regs,        \
> > > -         _shift, &mtk_clk_gate_ops_no_setclr_inv)
> > > +         _shift, &mtk_clk_gate_ops_no_setclr)
> > >   #define GATE_TOP1(_id, _name, _parent, _shift)            \
> > >       GATE_MTK(_id, _name, _parent, &top1_cg_regs,        \
> > > -         _shift, &mtk_clk_gate_ops_no_setclr)
> > > +         _shift, &mtk_clk_gate_ops_no_setclr_inv)
> > >   #define GATE_TOP2(_id, _name, _parent, _shift)            \
> > >       GATE_MTK(_id, _name, _parent, &top2_cg_regs,        \
> > > -         _shift, &mtk_clk_gate_ops_no_setclr)
> > > +         _shift, &mtk_clk_gate_ops_no_setclr_inv)
> > >   static const struct mtk_gate top_clk_gates[] = {
> > >       GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),
> 
> 

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[AngeloGioacchino Del Regno]

Il 15/05/23 16:01, Markus Schneider-Pargmann ha scritto:
> On Mon, May 15, 2023 at 01:49:54PM +0200, AngeloGioacchino Del Regno wrote:
>> Il 12/05/23 13:51, Matthias Brugger ha scritto:
>>>
>>>
>>> On 11/05/2023 15:32, Markus Schneider-Pargmann wrote:
>>>> The given operations are inverted for the wrong registers which makes
>>>> multiple of the mt8365 hardware units unusable. In my setup at least usb
>>>> did not work.
>>>>
>>>> Fixed by swapping the operations with the inverted ones.
>>
>> ...with the not inverted ones, you mean!
> 
> Actually now I am not sure how to express it correctly, maybe I should
> have just left that sentence out of the commit message.
> 
> I meant replacing the no_setclr_inv with no_setclr for TOP0 and no_setclr with
> no_setclr_inv for TOP1/2.
> 

Yeah, the sentence may be a bit confusing, but you're actually inverting the
inverted ones and the non-inverted ones.

It's fine, anyway, this commit can get picked as-is imo :-D

> Best,
> Markus
> 
>>
>> Anyway,
>>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>
>>>>
>>>> Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
>>>> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to
>>>> mtk_gate clocks")
>>>> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
>>>
>>> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
>>>
>>>> ---
>>>>    drivers/clk/mediatek/clk-mt8365.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
>>>> index 6b4e193f648d..6d785ec5754d 100644
>>>> --- a/drivers/clk/mediatek/clk-mt8365.c
>>>> +++ b/drivers/clk/mediatek/clk-mt8365.c
>>>> @@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
>>>>    #define GATE_TOP0(_id, _name, _parent, _shift)            \
>>>>        GATE_MTK(_id, _name, _parent, &top0_cg_regs,        \
>>>> -         _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>>> +         _shift, &mtk_clk_gate_ops_no_setclr)
>>>>    #define GATE_TOP1(_id, _name, _parent, _shift)            \
>>>>        GATE_MTK(_id, _name, _parent, &top1_cg_regs,        \
>>>> -         _shift, &mtk_clk_gate_ops_no_setclr)
>>>> +         _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>>>    #define GATE_TOP2(_id, _name, _parent, _shift)            \
>>>>        GATE_MTK(_id, _name, _parent, &top2_cg_regs,        \
>>>> -         _shift, &mtk_clk_gate_ops_no_setclr)
>>>> +         _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>>>    static const struct mtk_gate top_clk_gates[] = {
>>>>        GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),
>>
>>

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[Markus Schneider-Pargmann]

Hi,

just wanted to ask if I need to do something specific for it to go into
a rc? Sorry if I missed doing something for that, I haven't had to fix
something in an rc that often before.

Best,
Markus

On Thu, May 11, 2023 at 03:32:26PM +0200, Markus Schneider-Pargmann wrote:
> The given operations are inverted for the wrong registers which makes
> multiple of the mt8365 hardware units unusable. In my setup at least usb
> did not work.
> 
> Fixed by swapping the operations with the inverted ones.
> 
> Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/clk/mediatek/clk-mt8365.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
> index 6b4e193f648d..6d785ec5754d 100644
> --- a/drivers/clk/mediatek/clk-mt8365.c
> +++ b/drivers/clk/mediatek/clk-mt8365.c
> @@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
>  
>  #define GATE_TOP0(_id, _name, _parent, _shift)			\
>  	GATE_MTK(_id, _name, _parent, &top0_cg_regs,		\
> -		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
> +		 _shift, &mtk_clk_gate_ops_no_setclr)
>  
>  #define GATE_TOP1(_id, _name, _parent, _shift)			\
>  	GATE_MTK(_id, _name, _parent, &top1_cg_regs,		\
> -		 _shift, &mtk_clk_gate_ops_no_setclr)
> +		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>  
>  #define GATE_TOP2(_id, _name, _parent, _shift)			\
>  	GATE_MTK(_id, _name, _parent, &top2_cg_regs,		\
> -		 _shift, &mtk_clk_gate_ops_no_setclr)
> +		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>  
>  static const struct mtk_gate top_clk_gates[] = {
>  	GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),
> -- 
> 2.40.1
> 

Re: [PATCH] clk: mediatek: mt8365: Fix inverted topclk operations

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> just wanted to ask if I need to do something specific for it to go into
> a rc? Sorry if I missed doing something for that, I haven't had to fix
> something in an rc that often before.

I think the  Fixes tag is enough to indicate that.

Steven, is there still time for this fix be queued up for v6.4-rc?

Thanks,

Kevin

> On Thu, May 11, 2023 at 03:32:26PM +0200, Markus Schneider-Pargmann wrote:
>> The given operations are inverted for the wrong registers which makes
>> multiple of the mt8365 hardware units unusable. In my setup at least usb
>> did not work.
>> 
>> Fixed by swapping the operations with the inverted ones.
>> 
>> Reported-by: Alexandre Mergnat <amergnat@baylibre.com>
>> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
>> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
>> ---
>>  drivers/clk/mediatek/clk-mt8365.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/clk/mediatek/clk-mt8365.c b/drivers/clk/mediatek/clk-mt8365.c
>> index 6b4e193f648d..6d785ec5754d 100644
>> --- a/drivers/clk/mediatek/clk-mt8365.c
>> +++ b/drivers/clk/mediatek/clk-mt8365.c
>> @@ -583,15 +583,15 @@ static const struct mtk_gate_regs top2_cg_regs = {
>>  
>>  #define GATE_TOP0(_id, _name, _parent, _shift)			\
>>  	GATE_MTK(_id, _name, _parent, &top0_cg_regs,		\
>> -		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>> +		 _shift, &mtk_clk_gate_ops_no_setclr)
>>  
>>  #define GATE_TOP1(_id, _name, _parent, _shift)			\
>>  	GATE_MTK(_id, _name, _parent, &top1_cg_regs,		\
>> -		 _shift, &mtk_clk_gate_ops_no_setclr)
>> +		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>  
>>  #define GATE_TOP2(_id, _name, _parent, _shift)			\
>>  	GATE_MTK(_id, _name, _parent, &top2_cg_regs,		\
>> -		 _shift, &mtk_clk_gate_ops_no_setclr)
>> +		 _shift, &mtk_clk_gate_ops_no_setclr_inv)
>>  
>>  static const struct mtk_gate top_clk_gates[] = {
>>  	GATE_TOP0(CLK_TOP_CONN_32K, "conn_32k", "clk32k", 10),
>> -- 
>> 2.40.1
>>