Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211027080747.24388-1-shayne.chen@mediatek.com>
In-Reply-To: <20211027080747.24388-1-shayne.chen@mediatek.com>
From:   Julian Calaby <julian.calaby@gmail.com>
Date:   Fri, 29 Oct 2021 11:55:52 +1100
Message-ID: <CAGRGNgUvTU=yYfuKcB2VvHjqLBKVPpVJaLvMpxK926y=yniKsg@mail.gmail.com>
Subject: Re: [PATCH] mt76: mt7915: add default calibrated data support
To:     Shayne Chen <shayne.chen@mediatek.com>
Cc:     Felix Fietkau <nbd@nbd.name>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Lorenzo Bianconi <lorenzo.bianconi@redhat.com>,
        Ryder Lee <ryder.lee@mediatek.com>,
        Evelyn Tsai <evelyn.tsai@mediatek.com>,
        linux-mediatek <linux-mediatek@lists.infradead.org>,
        Bo Jiao <Bo.Jiao@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Shayne,

On Thu, Oct 28, 2021 at 6:25 AM Shayne Chen <shayne.chen@mediatek.com> wrote:
>
> Load the default eeprom data when the content of flash/efuse is invalid.
> This could help to eliminate some issues due to incorrect or
> insufficient rf values.
>
> Co-developed-by: Bo Jiao <Bo.Jiao@mediatek.com>
> Signed-off-by: Bo Jiao <Bo.Jiao@mediatek.com>
> Signed-off-by: Shayne Chen <shayne.chen@mediatek.com>
> Reviewed-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  .../wireless/mediatek/mt76/mt7915/eeprom.c    | 83 +++++++++++++++----
>  .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 24 ++++++
>  .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
>  .../wireless/mediatek/mt76/mt7915/mt7915.h    |  4 +
>  4 files changed, 98 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c b/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c
> index ee3d644..626ea4a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c
> @@ -42,20 +89,28 @@ static int mt7915_eeprom_load(struct mt7915_dev *dev)
>                                               i * MT7915_EEPROM_BLOCK_SIZE);
>         }
>
> -       return ret;
> -}
> -
> -static int mt7915_check_eeprom(struct mt7915_dev *dev)
> -{
> -       u8 *eeprom = dev->mt76.eeprom.data;
> -       u16 val = get_unaligned_le16(eeprom);
> +       if (!dev->flash_mode) {
> +               u8 free_block_num;
> +
> +               mt7915_mcu_get_eeprom_free_block(dev, &free_block_num);
> +               if (free_block_num >= 29) {
> +                       dev_warn(dev->mt76.dev,
> +                                "efuse info not enough, use default bin\n");
> +                       ret = mt7915_eeprom_load_default(dev);
> +                       if (ret)
> +                               return ret;

You've got two instances of the code where it tries to load the
default if the EEPROM data isn't valid.

You could potentially simplify this by structuring the code that calls
this function so it's something like:

ret = mt7915_eeprom_load();

if (!ret) {
    ret = mt7915_eeprom_load_default();
}

return ret;

with mt7915_eeprom_load() just returning -EINVAL if the EEPROM
contents aren't valid instead of trying to fix it itself.

This would also make the code simpler if there ends up being another
way to get EEPROM data in the future, e.g. an NVRAM partition
referenced through the device tree, etc.

> +               }
> +       }
>
> -       switch (val) {
> -       case 0x7915:
> -               return 0;
> -       default:
> -               return -EINVAL;
> +       ret = mt7915_check_eeprom(dev);
> +       if (ret) {
> +               dev_warn(dev->mt76.dev, "eeprom check fail, use default bin\n");
> +               ret = mt7915_eeprom_load_default(dev);
> +               if (ret)
> +                       return ret;
>         }
> +
> +       return ret;
>  }
>
>  void mt7915_eeprom_parse_band_config(struct mt7915_phy *phy)

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/