| Message ID | 1421881782-23716-1-git-send-email-rickard_strandqvist@spectrumdigital.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
The subject line is not a good one. Please elaborate. > Move struct ab8500_ext_regulator_cfg to the only file that is now used in. Attention to detail --^ > Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> > --- > drivers/regulator/ab8500-ext.c | 6 ++++++ > include/linux/regulator/ab8500.h | 5 ----- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/regulator/ab8500-ext.c b/drivers/regulator/ab8500-ext.c > index 84c1ee3..a40c701 100644 > --- a/drivers/regulator/ab8500-ext.c > +++ b/drivers/regulator/ab8500-ext.c > @@ -25,6 +25,12 @@ > #include <linux/mfd/abx500/ab8500.h> > #include <linux/regulator/ab8500.h> > > + Superfluous '\n'. > +/* AB8500 external regulators */ No need for this commit now. > +struct ab8500_ext_regulator_cfg { > + bool hwreq; /* requires hw mode or high power mode */ > +}; > + Are you sure a struct is required at all now? It only contains a single bool after all. > /** > * struct ab8500_ext_regulator_info - ab8500 regulator information > * @dev: device pointer > diff --git a/include/linux/regulator/ab8500.h b/include/linux/regulator/ab8500.h > index d8ecefa..802529b 100644 > --- a/include/linux/regulator/ab8500.h > +++ b/include/linux/regulator/ab8500.h > @@ -300,11 +300,6 @@ enum ab8540_regulator_reg { > AB8540_NUM_REGULATOR_REGISTERS, > }; > > -/* AB8500 external regulators */ > -struct ab8500_ext_regulator_cfg { > - bool hwreq; /* requires hw mode or high power mode */ > -}; > - > enum ab8500_ext_regulator_id { > AB8500_EXT_SUPPLY1, > AB8500_EXT_SUPPLY2,
2015-01-22 8:44 GMT+01:00 Lee Jones <lee.jones@linaro.org>: > > Are you sure a struct is required at all now? It only contains a > single bool after all. Hi Lee Okay, I will gladly remove ab8500_ext_regulator_cfg completely. And I will try to update the comments, but since I'm not so familiar with what this code was meant to do it is a little more difficult. Kind regards Rickard Strandqvist
On Saturday 24 January 2015 18:33:00 Rickard Strandqvist wrote: > 2015-01-22 8:44 GMT+01:00 Lee Jones <lee.jones@linaro.org>: > > > > Are you sure a struct is required at all now? It only contains a > > single bool after all. > > > Hi Lee > > Okay, I will gladly remove ab8500_ext_regulator_cfg completely. > And I will try to update the comments, but since I'm not so familiar > with what this code was meant to do it is a little more difficult. > Try to take small steps then: See if there is something in this code that is either never called, or data structures that are only written but never read, or pointers that are verifiably always NULL, then eliminate one piece of the code at a time, with a patch that explains what you found. Try to make sure everything still builds after each patch, and note that this was not the case with your first patch, though it would have worked in the reverse order. I suspect the best way forward is to actually move all of ab8500_regulator_plat_data and whatever it references into drivers/regulator/ab8500-ext.c because there now is only one remaining instance of that. After this move, you can delete a lot of code in arch/arm/mach-ux500 and possibly move all of include/linux/regulator/ab8500.h into drivers/regulator/ab8500-ext.c as well. Arnd
diff --git a/drivers/regulator/ab8500-ext.c b/drivers/regulator/ab8500-ext.c index 84c1ee3..a40c701 100644 --- a/drivers/regulator/ab8500-ext.c +++ b/drivers/regulator/ab8500-ext.c @@ -25,6 +25,12 @@ #include <linux/mfd/abx500/ab8500.h> #include <linux/regulator/ab8500.h> + +/* AB8500 external regulators */ +struct ab8500_ext_regulator_cfg { + bool hwreq; /* requires hw mode or high power mode */ +}; + /** * struct ab8500_ext_regulator_info - ab8500 regulator information * @dev: device pointer diff --git a/include/linux/regulator/ab8500.h b/include/linux/regulator/ab8500.h index d8ecefa..802529b 100644 --- a/include/linux/regulator/ab8500.h +++ b/include/linux/regulator/ab8500.h @@ -300,11 +300,6 @@ enum ab8540_regulator_reg { AB8540_NUM_REGULATOR_REGISTERS, }; -/* AB8500 external regulators */ -struct ab8500_ext_regulator_cfg { - bool hwreq; /* requires hw mode or high power mode */ -}; - enum ab8500_ext_regulator_id { AB8500_EXT_SUPPLY1, AB8500_EXT_SUPPLY2,
Move struct ab8500_ext_regulator_cfg to the only file that is now used in. Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> --- drivers/regulator/ab8500-ext.c | 6 ++++++ include/linux/regulator/ab8500.h | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-)