On 06/11/14 14:32, Srinivas Pandruvada wrote: > On Wed, 2014-11-05 at 13:19 -0800, Gwendal Grignou wrote: >> On Wed, Nov 5, 2014 at 6:31 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: >>> On 04/11/14 19:41, Gwendal Grignou wrote: >>>> File and config id remains the same, but driver is now named akxxxx. >>> >>> Don't do that. Please name the driver after one supported part rather >>> than a general wild card. Leaving it as ak8975 would be sensible. >>> Wild cards seem like a good idea until along comes a part that fits >>> in the naming scheme but is otherwise totally different. >>> >>> Much cleaner to just pick a part to use in naming the driver and >>> make it clear what parts are supported in kconfig (see iio/adc/max1363 for >>> example). >> Will do. It is cleaner indeed. >>> >>> I would have prefered this as a two patch series. The first combining >>> the existing drivers, and the second (small one) adding the new part >>> support. >> I will submit 3 patches soon: >> - minor fixes in the driver first, >> - one that refactor the current driver to introduce a definition data structure >> - the next one that remove ak9911.c and add support for ak09911 and >> ak9912 into ak8975.c >>> >>> Would have broken it up into logical steps thus making it a little >>> easier to check for feature parity etc. >>> >>> Once you've dropped the name change, this patch will contain less noise. >>> Also, there are a couple of sensible refactorings in there. I would >>> have prefered these to be in precursor patches that made much smaller >>> changes. >> The re-factoring became obvious after I introduce the definition data structure. >>> >>> This sort of merging of drivers is one of the hardest types of code to >>> review, so making it as easy as possible for reviewers by packaging >>> it up as baby steps is the way to go. >>> >>> There is still some work to be done in convincing people that merging >>> the drivers is sensible (I'm pretty much convinced having waded through >>> this) but that is an easier job with a well split up series of steps! >>> >>> Jonathan >>> >>>> Move AKM09911 support in the same file. >>>> >>>> Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx> >>>> --- >>>> >>>> Addressed all comments from Hartmut. >>>> >>>> drivers/iio/magnetometer/Kconfig | 19 +- >>>> drivers/iio/magnetometer/Makefile | 1 - >>>> drivers/iio/magnetometer/ak09911.c | 326 ------------------- >>>> drivers/iio/magnetometer/ak8975.c | 629 ++++++++++++++++++++++++++----------- >>>> 4 files changed, 444 insertions(+), 531 deletions(-) >>>> delete mode 100644 drivers/iio/magnetometer/ak09911.c >>>> >>>> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig >>>> index b2dba9e..67f005d 100644 >>>> --- a/drivers/iio/magnetometer/Kconfig >>>> +++ b/drivers/iio/magnetometer/Kconfig >>>> @@ -6,26 +6,15 @@ >>>> menu "Magnetometer sensors" >>>> >>>> config AK8975 >>>> - tristate "Asahi Kasei AK8975 3-Axis Magnetometer" >>>> + tristate "Asahi Kasei AK 3-Axis Magnetometer" >>>> depends on I2C >>>> depends on GPIOLIB >>>> help >>>> - Say yes here to build support for Asahi Kasei AK8975 3-Axis >>>> - Magnetometer. This driver can also support AK8963, if i2c >>>> - device name is identified as ak8963. >>>> + Say yes here to build support for Asahi Kasei AK8975, AK8963, >>>> + AK09911 or AK09912 3-Axis Magnetometer. >>>> >>>> To compile this driver as a module, choose M here: the module >>>> - will be called ak8975. >>>> - >>>> -config AK09911 >>>> - tristate "Asahi Kasei AK09911 3-axis Compass" >>>> - depends on I2C >>>> - help >>>> - Say yes here to build support for Asahi Kasei AK09911 3-Axis >>>> - Magnetometer. >>>> - >>>> - To compile this driver as a module, choose M here: the module >>>> - will be called ak09911. >>>> + will be called akxxxx. >>>> > We already have released products with CONFIG_AK09911, so will next > kernel upgrade will break them. Customers don't allow config changes. > So we can't remove this config. > Merging of drivers is fine. At the least we should keep the config and > select the AK8975 when defined in the config if we remove AK09911. > > Jonathan, > What do you think? This driver is part of a commercial released product. > As you know customers are very sensitive about .config changes. Absolutely. When merging drivers putting in some config magic to select the new driver is a common solution. > > Thanks, > Srinivas > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
