Received-SPF: pass (google.com: domain of linux-wireless+bounces-5851-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <335bba80-addd-4443-b67e-d3de114b91ba@quicinc.com>
Date: Wed, 3 Apr 2024 12:51:17 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 11/13] wifi: mt76: mt7915: add fallback in case of missing
 precal data
Content-Language: en-US
To: Felix Fietkau <nbd@nbd.name>, <linux-wireless@vger.kernel.org>
References: <20240403103032.54823-1-nbd@nbd.name>
 <20240403103032.54823-11-nbd@nbd.name>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20240403103032.54823-11-nbd@nbd.name>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 4/3/2024 3:30 AM, Felix Fietkau wrote:
> When pre-calibration data is missing, do not fail the driver probe.
> Instead, just print a warning and fall back to regular calibration.
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>  .../net/wireless/mediatek/mt76/mt7915/eeprom.c    | 15 ++++++++++-----
>  drivers/net/wireless/mediatek/mt76/mt7915/init.c  |  2 +-
>  drivers/net/wireless/mediatek/mt76/mt7915/main.c  |  2 +-
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c b/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c
> index 5fdcbb521ff7..bfdbc15abaa9 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/eeprom.c
> @@ -28,7 +28,15 @@ static int mt7915_eeprom_load_precal(struct mt7915_dev *dev)
>  	if (!ret)
>  		return ret;
>  
> -	return mt76_get_of_data_from_nvmem(mdev, dev->cal, "precal", size);
> +	ret = mt76_get_of_data_from_nvmem(mdev, dev->cal, "precal", size);
> +	if (!ret)
> +		return ret;
> +
> +	dev_warn(mdev->dev, "missing precal data, size=%d\n", size);
> +	devm_kfree(mdev->dev, dev->cal);

if you are going to devm_kfree the memory for this case, why not also for the
mt76_get_of_data_from_mtd() failure case?

> +	dev->cal = NULL;
> +
> +	return ret;
>  }
>  
>  static int mt7915_check_eeprom(struct mt7915_dev *dev)
> @@ -254,10 +262,7 @@ int mt7915_eeprom_init(struct mt7915_dev *dev)
>  			return ret;
>  	}
>  
> -	ret = mt7915_eeprom_load_precal(dev);
> -	if (ret)
> -		return ret;
> -
> +	mt7915_eeprom_load_precal(dev);

if you're not going to check the status, why not make it a void function?