Re: [PATCH 3/8] mfd: twl-core: Rename TWL4030_CORE Kconfig option to TWL_CORE

Grygorii Strashko
2016-04-05T10:21:58Z

On 04/02/2016 11:12 PM, Paul Kocialkowski wrote:
> Le mercredi 30 mars 2016 à 18:34 +0300, Grygorii Strashko a écrit :
>> On 03/29/2016 10:22 PM, Paul Kocialkowski wrote:
>>>
>>> Since the twl-core mfd driver is not limited to twl4030 support, but also
>>> supports other chips of the TWL family, it makes sense to rename its
>>> matching
>>> Kconfig opton to a non-model-specific name.
>>
>> Personally I don't see too much reason for this, but it's up to maintainers to
>> decide.
>
> I'm a strong advocate of consistency in naming and using twl4030 prefixes for
> drivers that are common to more twl chips looks very inconsistent to me.
> Besides, some common twl drivers are already using the twl prefix, so this can
> also be seen as harmonisation.

Yah. May be.

But I see the problem with your series - small amount of functional changes
is hidden in the fog of huge amount of renaming/beautification ;)

But, again, this is my personal opinion - final decision is up to maintainers

+ cc: Lee Jones

>
>> Regarding this patch - pls, do not mix Kconfig and code changes.
>
> Do you mean that for code changes that are directly affected by Kconfig changes
> (e.g. #ifdef CONFIG_)? Is the policy that "each commit must produce a working
> result" or is it okay to break drivers in between commits of the same series?

I mean this:
*/
-#define TWL4030_CORE_NR_IRQS 8
+#define TWL_CORE_NR_IRQS 8
#define TWL4030_PWR_NR_IRQS 8

/* PIH register offsets */
@@ -693,7 +693,7 @@ int twl4030_init_irq(struct device *dev, int irq_num)
* the hwirqs numbers are defined contiguously from 1 to 15.
* Create only one domain for both.
*/
- nr_irqs = TWL4030_PWR_NR_IRQS + TWL4030_CORE_NR_IRQS;
+ nr_irqs = TWL4030_PWR_NR_IRQS + TWL_CORE_NR_IRQS;

irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0);
if (IS_ERR_VALUE(irq_base)) {
@@ -704,7 +704,7 @@ int twl4030_init_irq(struct device *dev, int irq_num)
irq_domain_add_legacy(node, nr_irqs, irq_base, 0,
&irq_domain_simple_ops, NULL);

- irq_end = irq_base + TWL4030_CORE_NR_IRQS;
+ irq_end = irq_base + TWL_CORE_NR_IRQS;

>
>> And it will be good to have cover letter.
>
> Noted, I'll come up with something for v2.
>
>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>> ---
>>> arch/arm/mach-omap2/Kconfig | 2 +-
>>> arch/arm/mach-omap2/Makefile | 2 +-
>>> arch/arm/mach-omap2/pm.h | 2 +-
>>> arch/arm/plat-omap/Kconfig | 2 +-

[..]

--
regards,
-grygorii