Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 2 Apr 2020 18:18:25 +0200
From:   Oscar Carter <oscar.carter@gmx.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>,
        Malcolm Priestley <tvboxspy@gmail.com>,
        Quentin Deslandes <quentin.deslandes@itdev.co.uk>
Cc:     Oscar Carter <oscar.carter@gmx.com>,
        Forest Bond <forest@alittletooquiet.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Amir Mahdi Ghorbanian <indigoomega021@gmail.com>,
        Colin Ian King <colin.king@canonical.com>,
        Gabriela Bittencourt <gabrielabittencourt00@gmail.com>,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: vt6656: Use defines in vnt_mac_reg_bits_*
 functions
Message-ID: <20200402161825.GA3939@ubuntu>
References: <20200328095433.7879-1-oscar.carter@gmx.com>
 <20200331102906.GA2066@kadam>
 <20200401165537.GC3109@ubuntu>
 <20200402091917.GA17323@jiffies>
 <4e5fc495-9355-02b7-4148-ea4de5370617@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4e5fc495-9355-02b7-4148-ea4de5370617@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Apr 02, 2020 at 11:58:07AM +0100, Malcolm Priestley wrote:
>
>
> On 02/04/2020 10:19, Quentin Deslandes wrote:
> > On 04/01/20 18:55:38, Oscar Carter wrote:
> > > On Tue, Mar 31, 2020 at 01:29:06PM +0300, Dan Carpenter wrote:
> > > > On Sat, Mar 28, 2020 at 10:54:33AM +0100, Oscar Carter wrote:
> > > > > Define the necessary bits in the CHANNEL, PAPEDELAY and GPIOCTL0
> > > > > registers to can use them in the calls to vnt_mac_reg_bits_on an=
d
> > > > > vnt_mac_reg_bits_off functions. In this way, avoid the use of BI=
T()
> > > > > macros and clarify the code.
> > > > >
> > > > > Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in vnt_ma=
c_reg_bits_* functions")
> > > > > Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > > > ---
> > > > >   drivers/staging/vt6656/baseband.c |  6 ++++--
> > > > >   drivers/staging/vt6656/card.c     |  3 +--
> > > > >   drivers/staging/vt6656/mac.h      | 12 ++++++++++++
> > > > >   drivers/staging/vt6656/main_usb.c |  2 +-
> > > > >   4 files changed, 18 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging=
/vt6656/baseband.c
> > > > > index a19a563d8bcc..dd3c3bf5e8b5 100644
> > > > > --- a/drivers/staging/vt6656/baseband.c
> > > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > > @@ -442,7 +442,8 @@ int vnt_vt3184_init(struct vnt_private *priv=
)
> > > > >   		if (ret)
> > > > >   			goto end;
> > > > >
> > > > > -		ret =3D vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0));
> > > > > +		ret =3D vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY,
> > > > > +					  PAPEDELAY_B0);
> > > >
> > > > This doesn't clarify anything.  It makes it less clear because som=
eone
> > > > would assume B0 means something but it's just hiding a magic numbe=
r
> > > > behind a meaningless define.  B0 means BIT(0) which means nothing.=
  So
> > > > now we have to jump through two hoops to find out that we don't kn=
ow
> > > > anything.
> > > >
> > > I created this names due to the lack of information about the hardwa=
re. I
> > > searched but I did not find anything.
> > >
> > > > Just leave it as-is.  Same for the rest.
> > > Ok.
> > >
> > > >
> > > > There problem is a hardware spec which explains what this stuff is=
.
> > > >
> > > It's possible to find a datasheet of this hardware to make this modi=
fication
> > > accordingly to the correct bit names of registers ?
> >
> > I haven't found any so far, if your researches are luckier than mine, =
I
> > would be interested too. Even getting your hands on the actual device =
is
> > complicated.
>
> All I can tell you is it related to command above it MAC_REG_ITRTMSET
> without it the device will not associate with access point is probably T=
X
> timing/power rated.
>
> Other bits in MAC_REG_PAPEDELAY are related to LED function and defined =
in
> LEDSTS_* in mac.h.
>
> I think it is some kind of enable so something like ITRTMSET_ENABLE woul=
d
> do.
>
I think the best for now is leave it as-is due to the lack of information =
about
bit names of registers.

> Regards
>
> Malcolm

Thanks,

Oscar carter