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 <[email protected]>
Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
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
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 <[email protected]>
Tested-by: Alexandre Mergnat <[email protected]>
--
Regards,
Alexandre
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 <[email protected]>
> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
> ---
> 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),
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 <[email protected]>
>>
>> Reported-by: Alexandre Mergnat <[email protected]>
>> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate
>> clocks")
>> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
>
> Reviewed-by: Matthias Brugger <[email protected]>
>
>> ---
>> 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),
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 <[email protected]>
>
> > >
> > > Reported-by: Alexandre Mergnat <[email protected]>
> > > Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to
> > > mtk_gate clocks")
> > > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> >
> > Reviewed-by: Matthias Brugger <[email protected]>
> >
> > > ---
> > > 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),
>
>
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 <[email protected]>
>>
>>>>
>>>> Reported-by: Alexandre Mergnat <[email protected]>
>>>> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to
>>>> mtk_gate clocks")
>>>> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
>>>
>>> Reviewed-by: Matthias Brugger <[email protected]>
>>>
>>>> ---
>>>> 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),
>>
>>
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 <[email protected]>
> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
> 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
>
Markus Schneider-Pargmann <[email protected]> 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 <[email protected]>
>> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
>> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
>> ---
>> 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
>>
Kevin Hilman <[email protected]> writes:
> Markus Schneider-Pargmann <[email protected]> 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?
Steven: gentle reminder ping.
Quoting Markus Schneider-Pargmann (2023-05-11 06:32:26)
> 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 <[email protected]>
Checkpatch complains now that there isn't a Closes: tag here.
> Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
Applied to clk-fixes
Quoting Kevin Hilman (2023-06-09 11:21:30)
> Kevin Hilman <[email protected]> writes:
>
> > Markus Schneider-Pargmann <[email protected]> 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?
>
> Steven: gentle reminder ping.
Who's Steven :)
Hi Stephen,
On Mon, Jun 12, 2023 at 06:13:10PM -0700, Stephen Boyd wrote:
> Quoting Markus Schneider-Pargmann (2023-05-11 06:32:26)
> > 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 <[email protected]>
>
> Checkpatch complains now that there isn't a Closes: tag here.
Alexandre reported this internally, so there is nothing I could point to
for a Closes tag.
>
> > Fixes: 905b7430d3cc ("clk: mediatek: mt8365: Convert simple_gate to mtk_gate clocks")
> > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > ---
>
> Applied to clk-fixes
Thank you.
Best,
Markus
Stephen Boyd <[email protected]> writes:
> Quoting Kevin Hilman (2023-06-09 11:21:30)
>> Kevin Hilman <[email protected]> writes:
>>
>> > Markus Schneider-Pargmann <[email protected]> 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?
>>
>> Steven: gentle reminder ping.
>
> Who's Steven :)
Sorry, I confused you with Steven Spielberg. I bet that happens to you
a lot on the mailing list. ;) (either that, or I can't spell.)
Thanks for queuing this up Stephen.
Kevin