Re: [PATCH] mt76x0: phy: fix restore phase in mt76x0_phy_recalibrate_after_assoc

[Lorenzo Bianconi]

> > I think is needed we do:
> > 
> > reg_val = mt76_rr(dev, 0x2124); 
> > mt76_wr(dev, 0x2124, 0xff7e);
> > 
> > CALIBRATE
> > 
> > mt76_wr(dev, 0x2124, reg_val);
> > 
> > so seems we have to restore orginal value after calibration.
> > 
> > Referencing as MT_BBP(IBI, 9) is obviously fine.
> Yes, that makes more sense.
> 
> - Felix

Ack, I will send v2 with proper register name.

Regards,
Lorenzo

Re: [PATCH] mt76x0: phy: fix restore phase in mt76x0_phy_recalibrate_after_assoc

[Felix Fietkau]

On 2018-10-09 09:35, Stanislaw Gruszka wrote:
> On Mon, Oct 08, 2018 at 08:11:45PM +0200, Felix Fietkau wrote:
>> On 2018-10-08 14:40, Lorenzo Bianconi wrote:
>> > Fix restore value configured in 0x2124 register in
>> > mt76x0_phy_recalibrate_after_assoc routine.
>> > 
>> > Fixes: 10de7a8b4ab9 ("mt76x0: phy files")
>> > Signed-off-by: Lorenzo Bianconi 
>> > ---
>> >  drivers/net/wireless/mediatek/mt76/mt76x0/phy.c | 3 +--
>> >  1 file changed, 1 insertion(+), 2 deletions(-)
>> > 
>> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c 
>> > b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
>> > index 99e0a91a2f99..d18942e54048 100644
>> > --- a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
>> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
>> > @@ -734,8 +734,7 @@ void mt76x0_phy_recalibrate_after_assoc(struct 
>> > mt76x02_dev *dev)
>> >usleep_range(500, 700);
>> >  
>> >reg_val = mt76_rr(dev, 0x2124);
>> > -  reg_val &= 0xff7e;
>> > -  mt76_wr(dev, 0x2124, reg_val);
>> > +  mt76_wr(dev, 0x2124, 0xff7e);
>> I'm pretty sure you can drop the mt76_rr as well. Also, you can refer to
>> 0x2124 as MT_BBP(IBI, 9).
> 
> I think is needed we do:
> 
>   reg_val = mt76_rr(dev, 0x2124); 
>   mt76_wr(dev, 0x2124, 0xff7e);
> 
>   CALIBRATE
> 
>   mt76_wr(dev, 0x2124, reg_val);
> 
> so seems we have to restore orginal value after calibration.
> 
> Referencing as MT_BBP(IBI, 9) is obviously fine.
Yes, that makes more sense.

- Felix

Re: [PATCH] mt76x0: phy: fix restore phase in mt76x0_phy_recalibrate_after_assoc

[Stanislaw Gruszka]

On Mon, Oct 08, 2018 at 08:11:45PM +0200, Felix Fietkau wrote:
> On 2018-10-08 14:40, Lorenzo Bianconi wrote:
> > Fix restore value configured in 0x2124 register in
> > mt76x0_phy_recalibrate_after_assoc routine.
> > 
> > Fixes: 10de7a8b4ab9 ("mt76x0: phy files")
> > Signed-off-by: Lorenzo Bianconi 
> > ---
> >  drivers/net/wireless/mediatek/mt76/mt76x0/phy.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c 
> > b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> > index 99e0a91a2f99..d18942e54048 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> > @@ -734,8 +734,7 @@ void mt76x0_phy_recalibrate_after_assoc(struct 
> > mt76x02_dev *dev)
> > usleep_range(500, 700);
> >  
> > reg_val = mt76_rr(dev, 0x2124);
> > -   reg_val &= 0xff7e;
> > -   mt76_wr(dev, 0x2124, reg_val);
> > +   mt76_wr(dev, 0x2124, 0xff7e);
> I'm pretty sure you can drop the mt76_rr as well. Also, you can refer to
> 0x2124 as MT_BBP(IBI, 9).

I think is needed we do:

reg_val = mt76_rr(dev, 0x2124); 
mt76_wr(dev, 0x2124, 0xff7e);

CALIBRATE

mt76_wr(dev, 0x2124, reg_val);

so seems we have to restore orginal value after calibration.

Referencing as MT_BBP(IBI, 9) is obviously fine.

Thanks
Stanislaw
 

Re: [PATCH] mt76x0: phy: fix restore phase in mt76x0_phy_recalibrate_after_assoc

[Lorenzo Bianconi]

> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c 
> > b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> > index 99e0a91a2f99..d18942e54048 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> > @@ -734,8 +734,7 @@ void mt76x0_phy_recalibrate_after_assoc(struct 
> > mt76x02_dev *dev)
> >   usleep_range(500, 700);
> >
> >   reg_val = mt76_rr(dev, 0x2124);
> > - reg_val &= 0xff7e;
> > - mt76_wr(dev, 0x2124, reg_val);
> > + mt76_wr(dev, 0x2124, 0xff7e);
> I'm pretty sure you can drop the mt76_rr as well. Also, you can refer to
> 0x2124 as MT_BBP(IBI, 9).
>

Do you mean we do not need to reset the original value in MT_BBP(IBI, 9)?

Regards,
Lorenzo

> - Felix

Re: [PATCH] mt76x0: phy: fix restore phase in mt76x0_phy_recalibrate_after_assoc

[Felix Fietkau]

On 2018-10-08 14:40, Lorenzo Bianconi wrote:
> Fix restore value configured in 0x2124 register in
> mt76x0_phy_recalibrate_after_assoc routine.
> 
> Fixes: 10de7a8b4ab9 ("mt76x0: phy files")
> Signed-off-by: Lorenzo Bianconi 
> ---
>  drivers/net/wireless/mediatek/mt76/mt76x0/phy.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c 
> b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> index 99e0a91a2f99..d18942e54048 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/phy.c
> @@ -734,8 +734,7 @@ void mt76x0_phy_recalibrate_after_assoc(struct 
> mt76x02_dev *dev)
>   usleep_range(500, 700);
>  
>   reg_val = mt76_rr(dev, 0x2124);
> - reg_val &= 0xff7e;
> - mt76_wr(dev, 0x2124, reg_val);
> + mt76_wr(dev, 0x2124, 0xff7e);
I'm pretty sure you can drop the mt76_rr as well. Also, you can refer to
0x2124 as MT_BBP(IBI, 9).

- Felix