* Lee Jones <lee.jones@xxxxxxxxxx> [161124 00:56]: > On Wed, 23 Nov 2016, Tony Lindgren wrote: > > > * Lee Jones <lee.jones@xxxxxxxxxx> [161121 03:43]: > > > On Fri, 18 Nov 2016, Tony Lindgren wrote: > > > > --- a/drivers/mfd/Makefile > > > > +++ b/drivers/mfd/Makefile > > > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o > > > > obj-$(CONFIG_MFD_CORE) += mfd-core.o > > > > > > > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o > > > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o > > > > > > Who is the manufacturer? > > > > Hmm that I don't know. There seems to be both ST and TI versions > > of this chip manufactured for Motorola. So my guess is that it > > should be Motorola unless there's some similar catalog part > > available from ST used by others. If anybody has more info > > on this please let me know :) > > If this IP is shared amongst vendors, it usually means it was designed > by someone else? Synopsis perhaps? After searching around, many specs say "ST Ericsson CPCAP". So let's assume the manufacturer should be ste. > > > > + cpcap->vendor = (val >> 6) & 0x0007; > > > > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038); > > > > > > Lots of magic numbers here. I suggest you define them. > > > > I'll check if some earlier code has these defined. Otherwise I'll > > just add a comment on the lack of available documentation. > > *sad face* > > Does that mean you don't even know what they're for? Luckily the Motorola driver folks documented all the registers. Unfortunately the register bits just have names. I need to check if we have names for these bits. > > > > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16); > > > > > > 'ret' is more traditional. > > > > FYI error seems to be preferred over ret as it's meaning is > > clear, git grep "error =" drivers/input for example. > > I can of course change it if you prefer ret over error. > > I'd prefer to stick to the conventions of *this* subsystem. > > ... and the most common convention used kernel wide: > > $ git grep "ret =" | wc -l > 117976 > $ git grep "err =" | wc -l > 56708 > $ git grep "error =" | wc -l > 14427 OK sure will rename. > > > > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */ > > > > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */ > > > > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */ > > > > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */ > > > > + > > > > +#define CPCAP_REG_TEST 0x7c00 /* Test */ > > > > + > > > > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */ > > > > + > > > > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */ > > > > > > It would be nice to line up the entire file. #OCD > > > > Hmm care to clarify what you mean here? I think it's lined up with > > I'm missing context now you've <snip>ed. > > These look straight, however is the whole file lined up (as much as > *practically* possible)? Yeah it should be, I'll check. > > tabs to line up. I left empty lines where the registers are not > > contiguous. What does #OCD mean, Obsessive Compulsive Disorder over > > header files maybe? :) > > Yes, that's what it means. > > /me likes straight lines. :) Sure nothing wrong with that ;) Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
