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 and > > > > > vnt_mac_reg_bits_off functions. In this way, avoid the use of BIT() > > > > > macros and clarify the code. > > > > > > > > > > Fixes: 3017e587e368 ("staging: vt6656: Use BIT() macro in vnt_mac_reg_bits_* functions") > > > > > Suggested-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > > > > Signed-off-by: Oscar Carter <oscar.carter@xxxxxxx> > > > > > --- > > > > > 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 = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, BIT(0)); > > > > > + ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, > > > > > + PAPEDELAY_B0); > > > > > > > > This doesn't clarify anything. It makes it less clear because someone > > > > would assume B0 means something but it's just hiding a magic number > > > > 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 know > > > > anything. > > > > > > > I created this names due to the lack of information about the hardware. 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 modification > > > 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 TX > 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 would > 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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel