On Wed, Jul 29, 2020 at 5:04 AM Sean Anderson <[email protected]> wrote: > > On 6/24/20 1:09 AM, Jassi Brar wrote: > > On Tue, Jun 16, 2020 at 2:29 AM Lukasz Majewski <[email protected]> wrote: > >> > >> Hi Jassi, > >> > >>> ... a polite ping, Lukasz. > >> > >> The only excuse for so long lack of my response are my personal issues > >> caused by the covid-19. > >> > > Sorry if I came across as pestering you. I hope all is well now. > > > > ..... > >>>> + > >>>> +#define MAX3420_REG_IOPINS 20 > >>>> +#define MAX3420_REG_IOPINS2 21 > >>>> +#define MAX3420_REG_GPINIRQ 22 > >>>> +#define MAX3420_REG_GPINIEN 23 > >>>> +#define MAX3420_REG_GPINPOL 24 > >>>> +#define MAX3420_REG_HIRQ 25 > >>>> +#define MAX3420_REG_HIEN 26 > >>>> +#define MAX3420_REG_MODE 27 > >>>> +#define MAX3420_REG_PERADDR 28 > >>>> +#define MAX3420_REG_HCTL 29 > >>>> +#define MAX3420_REG_HXFR 30 > >>>> +#define MAX3420_REG_HRSL 31 > >>>> + > >> > >> When I do look into drivers/usb/gadget/f_dfu.* the defines are placed > >> in the f_dfu.h file. > >> > > One school of thought is to contain all code in one file, especially > > when no other file should access it -- these defines are very max3420 > > specific and none else should need these. > > But I am fine if you want them in a separate file. > > > >>>> +#define field(val, bit) ((val) << (bit)) > >>>> + > >>>> +#define msleep(a) udelay((a) * 1000) > >>>> + > >> > >> Aren't those two above already defined in some *.h files? > >> > > I replaced msleep with mdelay as Marek suggested. > > I couldn't find the simple shift op define as field. > > Perhaps check out <linux/bitfield.h>? I believe FIELD_PREP is similar to > what you have there. > One has to go two levels deep to see what FIELD_PREP, as compared to the simple (value << shift) needed in the code. But I am ok doing that, whatever floats the boat.
Cheers!
