On 04/14/11 04:10, Jiejing.Zhang wrote: > Hi Jonathan, > > 2011/4/13 Jonathan Cameron <jic23@xxxxxxxxx>: >> On 04/13/11 13:33, Jean Delvare wrote: >>> On Wed, 13 Apr 2011 19:56:53 +0800, Zhang Jiejing wrote: >>>> This patch add basic support for mma8450 mma8451 gravity >>>> sensor chips, and will support mma8452, mma8453 in same >>>> driver file. >>>> >>>> They are i2c controller and support 3-axis gravity accelerator >>>> sensor. >>>> >>>> mma8450 have some different from mma845[1,2,3] in register map, so >>>> there are some switch case between mma8450 and others. >>>> >>>> Product Information can be found here: >>>> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MMA8450Q >>>> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MMA8451Q >>>> >>>> Signed-off-by: Zhang Jiejing <jiejing.zhang@xxxxxxxxxxxxx> >>>> --- >>>> drivers/hwmon/Kconfig | 10 + >>>> drivers/hwmon/Makefile | 1 + >>>> drivers/hwmon/mma845x.c | 568 +++++++++++++++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 579 insertions(+), 0 deletions(-) >>>> create mode 100644 drivers/hwmon/mma845x.c >>> >>> Nack. Accelerometers don't belong to hwmon. Several such drivers were >>> kicked out of drivers/hwmon recently, and we won't accept any such >>> driver in the future. Push this to drivers/misc, drivers/input or >>> drivers/staging/iio, whichever your prefer, but not drivers/hwmon. >> Taking a quick look, you already have a polled input device in here >> and it makes no use of hwmon interfaces whatsoever so input is the >> obvious target. >> >> Couple of quick things I noticed whilst scan reading... >> >> id table contains only 8451 and 8450, get_mmax845x_name contains >> 8452 and 8453. >> >> Use the names in the id table to provide you with the names rather than >> an additional function. >> >> Take a look at how other drivers handle subtly different parts. There >> are much neater ways of doing it than things like get_ctrl_register. > Could you give a filename or example driver name ? I can use them as an example. > > > since mma845[1-3] have same operation flow, and data byte order, but > mma8450 is different on both register address and data byte order. > > Do you think split mma8450 and mma845x (8451, 8452, 8453) into two > file is better? Difficult to know without analysing full code. Right now you are only supporting a small corner of what the chips supply, so probably depends more on what features get added to the drivers in future.. > > >> >> Shift it over to input and I'll do a full review. >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-input" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html