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;
Subject: Re: [PATCH] mt76: mt7915: add LED support
To:     MeiChia Chiu <MeiChia.Chiu@mediatek.com>
Cc:     Lorenzo Bianconi <lorenzo.bianconi@redhat.com>,
        Shayne Chen <shayne.chen@mediatek.com>,
        Evelyn Tsai <evelyn.tsai@mediatek.com>,
        linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org,
        Ryder Lee <ryder.lee@mediatek.com>,
        Money Wang <money.wang@mediatek.com>
References: <20210621141430.17577-1-MeiChia.Chiu@mediatek.com>
From:   Felix Fietkau <nbd@nbd.name>
Message-ID: <046a19be-b804-94a1-7c8c-5c4f3bfa8eac@nbd.name>
Date:   Fri, 27 Aug 2021 19:03:56 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <20210621141430.17577-1-MeiChia.Chiu@mediatek.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2021-06-21 16:14, MeiChia Chiu wrote:
> From: MeiChia Chiu <meichia.chiu@mediatek.com>
> 
> Initialize brightness_set and blink_set callbacks to enable LED support.
> 
> Signed-off-by: MeiChia Chiu <meichia.chiu@mediatek.com>
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> Signed-off-by: Money Wang <money.wang@mediatek.com>
> ---
>  .../net/wireless/mediatek/mt76/mt7915/init.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  .../net/wireless/mediatek/mt76/mt7915/mmio.c |  6 +++--
>  .../net/wireless/mediatek/mt76/mt7915/regs.h | 19 ++++++++++++++
>  3 files changed, 92 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> index af712a9..43f9245 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> @@ -92,10 +92,12 @@ static u32 __mt7915_reg_addr(struct mt7915_dev *dev, u32 addr)
>  	}
>  
>  	if ((addr >= 0x18000000 && addr < 0x18c00000) ||
> -	    (addr >= 0x70000000 && addr < 0x78000000) ||
> -	    (addr >= 0x7c000000 && addr < 0x7c400000))
> +	    (addr >= 0x70000000 && addr < 0x78000000))
>  		return mt7915_reg_map_l1(dev, addr);
>  
> +	if (addr >= 0x7c000000 && addr < 0x7c400000)
> +		return mt7915_reg_map_l1(dev, addr - 0x64000000);

This part doesn't make any sense to me. So you're defining registers in
the 0x7c000000 range, but subtracting 0x64000000 from the address, which
means we land at 0x18000000, which is already covered by the l1-mapped
ranges.
Why not just skip this weird magic trick and define the registers as
being in the 0x18000000 range?

- Felix