On 6/8/21 3:36 PM, Alexandre Belloni wrote:
> On 08/06/2021 10:07:47-0700, Randy Dunlap wrote:
>> On 6/8/21 5:24 AM, Alexandre Belloni wrote:
>>> On 06/06/2021 09:24:23-0700, Randy Dunlap wrote:
>>>> REG_OFFSET is defined both here and in arch/arm/mach-ixp4xx/, which
>>>> causes a build warning, so rename this macro in the RTC driver.
>>>>
>>>> ../drivers/rtc/rtc-pcf8523.c:44: warning: "REG_OFFSET" redefined
>>>> 44 | #define REG_OFFSET 0x0e
>>>>
>>>> ../arch/arm/mach-ixp4xx/include/mach/platform.h:25: note: this is the location of the previous definition
>>>> 25 | #define REG_OFFSET 3
>>>>
>>>> Fixes: bc3bee025272 ("rtc: pcf8523: add support for trimming the RTC oscillator")
>>>> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
>>>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>>>> Cc: Russell King <rmk+kernel@xxxxxxxxxxxxxxx>
>>>> Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>
>>>> Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
>>>> Cc: linux-rtc@xxxxxxxxxxxxxxx
>>>> ---
>>>> drivers/rtc/rtc-pcf8523.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> --- linux-next-20210604.orig/drivers/rtc/rtc-pcf8523.c
>>>> +++ linux-next-20210604/drivers/rtc/rtc-pcf8523.c
>>>> @@ -41,7 +41,7 @@
>>>> #define REG_WEEKDAY_ALARM 0x0d
>>>> #define ALARM_DIS BIT(7)
>>>>
>>>> -#define REG_OFFSET 0x0e
>>>> +#define REG_OFFSET_TUNE 0x0e /* offset register is used for tuning */
>>>
>>> All the other defines are using the names from the datasheet, probably
>>> ixp4xx should be fixed instead?
>>
>> That would be something like 25 changes in 14 files instead
>> of 3 changes in one file.
>>
>
> But REG_OFFSET from mach-ixp4xx/include/mach/platform.h is the one
> leaking out. I think you can agree that rtc-pcf8523.c is self contained.
>
>> But for both locations, names like REG_OFFSET are just too generic.
>> They should be more specific so that name clashes won't happen.
>>
>
> This name clash wouldn't happen if ixp4xx was converted to multiplatform
> as this would prevent it from including random headers globally but
> there hasn't been any significant move in that direction since February
> 2019. I know nslu2 is a popular platform but maybe at some point we should
> consider simply dropping ixp4 support if it doesn't improve.
>
> I remember that at the time I took the patch I though the REG_ prefix
> was a bit generic but again, this is pretty self-contained in the
> driver.
OK, our criteria for what or where to patch differ. That's OK.
pfc8523 is self-contained (so simpler to patch or muck up)
and newer than ixp4xx. Those are my top criteria.
G'day.
--
~Randy
