Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <378747af00fdd3e64e9f5d7724f1b84759f95e8f.1576074028.git.lorenzo@kernel.org>
 <20c2faaa-7709-8486-a799-02e6ce5b7ef9@nbd.name>
In-Reply-To: <20c2faaa-7709-8486-a799-02e6ce5b7ef9@nbd.name>
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date:   Sat, 14 Dec 2019 22:59:59 +0100
Message-ID: <CAJ0CqmXqqu1UC610vG9sMBdw4ZZn5uvJ09=m7SXpegZkcMP=MA@mail.gmail.com>
Subject: Re: [PATCH] mt76: mt7615: introduce LED support
To:     Felix Fietkau <nbd@nbd.name>
Cc:     Lorenzo Bianconi <lorenzo@kernel.org>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Ryder Lee <ryder.lee@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

>
> On 2019-12-11 15:21, Lorenzo Bianconi wrote:
> > Initialize brightness_set and blink_set callbacks to
> > mt7615_led_set_brightness and mt7615_led_set_blink in order to enable
> > LED support in mt7615 driver
> >

[...]

> >  #define MT_TX_AGG_CNT(n)             MT_WF_MIB(0xa8 + ((n) << 2))
> >
> > +#define MT_LED_BASE_PHYS             0x80024000
> > +#define MT_LED_PHYS(_n)                      (MT_LED_BASE_PHYS + (_n))
> > +
> > +#define MT_LED_CTRL                  MT_LED_PHYS(0x00)
> > +
> > +#define MT_LED_CTRL_REPLAY(_n)               BIT(0 + (8 * (_n)))
> > +#define MT_LED_CTRL_POLARITY(_n)     BIT(1 + (8 * (_n)))
> > +#define MT_LED_CTRL_TX_BLINK_MODE(_n)        BIT(2 + (8 * (_n)))
> > +#define MT_LED_CTRL_TX_MANUAL_BLINK(_n)      BIT(3 + (8 * (_n)))
> > +#define MT_LED_CTRL_TX_OVER_BLINK(_n)        BIT(5 + (8 * (_n)))
> > +#define MT_LED_CTRL_KICK(_n)         BIT(7 + (8 * (_n)))
> > +
> > +#define MT_LED_STATUS_0(_n)          MT_LED_PHYS(0x10 + ((_n) * 8))
> > +#define MT_LED_STATUS_1(_n)          MT_LED_PHYS(0x14 + ((_n) * 8))
> > +#define MT_LED_STATUS_OFF_MASK               GENMASK(31, 24)
> > +#define MT_LED_STATUS_OFF(_v)                (((_v) << \
> > +                                       __ffs(MT_LED_STATUS_OFF_MASK)) & \
> > +                                      MT_LED_STATUS_OFF_MASK)
> > +#define MT_LED_STATUS_ON_MASK                GENMASK(23, 16)
> > +#define MT_LED_STATUS_ON(_v)         (((_v) << \
> > +                                       __ffs(MT_LED_STATUS_ON_MASK)) & \
> > +                                      MT_LED_STATUS_ON_MASK)
> > +#define MT_LED_STATUS_DURATION_MASK  GENMASK(15, 0)
> > +#define MT_LED_STATUS_DURATION(_v)   (((_v) << \
> > +                                       __ffs(MT_LED_STATUS_DURATION_MASK)) &\
> > +                                      MT_LED_STATUS_DURATION_MASK
>
> Why are you using separate _MASK fields and macros using __ffs in
> bitshift instead of FIELD_PREP like the rest of the code?
>

ack, I will fix it in v2.

Regards,
Lorenzo

> - Felix
>