| Message ID | 20230511133226.913600-1-msp@baylibre.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: mediatek: mt8365: Fix inverted topclk operations | expand |
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>
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),
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),
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), > >
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), >> >>
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 >
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 >>
Kevin Hilman <khilman@kernel.org> writes: > 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? Steven: gentle reminder ping.
Quoting Kevin Hilman (2023-06-09 11:21:30) > Kevin Hilman <khilman@kernel.org> writes: > > > 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? > > Steven: gentle reminder ping. Who's Steven :)
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 <amergnat@baylibre.com> 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 <msp@baylibre.com> > --- Applied to clk-fixes
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 <amergnat@baylibre.com> > > 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 <msp@baylibre.com> > > --- > > Applied to clk-fixes Thank you. Best, Markus
Stephen Boyd <sboyd@kernel.org> writes: > Quoting Kevin Hilman (2023-06-09 11:21:30) >> Kevin Hilman <khilman@kernel.org> writes: >> >> > 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? >> >> 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
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),
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(-)